Vote for your favorite SkillMD. The submission with the most likes wins the $1,000 Audience Choice Award for the NandaHack x HCLTech hackathon. Voting is open through September 25.Vote now →
mergedhumanPR #286Other

fix(dashboard): check a submitted source_url is an http(s) URL again

Flagged by an automated security review of main, and confirmed by hand.

Author

JamesCarnley avatar

@JamesCarnley

github profile →
Status
Merged
Merged on
Sep 16
Branch
fix/skills-api-url-validation

Description

The pitch.

Flagged by an automated security review of `main`, and confirmed by hand.

### Problem

A SkillMD can be submitted two ways. The web form checks that `source_url` is
an `http(s)` URL before saving it, and the server then fetches it to record
whether it answers. `POST /api/skills` checked the same thing until the
frontend was restored to its deployed state in `1975fb9`, which dropped it.

On `main` today the JSON API stores whatever string it is sent, and the
catalogue shows it as that skill's source link. Confirmed against the app
running locally: `file:///etc/passwd` and `javascript:alert(1)` both pass
validation and reach the insert.

### Change

Five lines: the same check the form applies, with the form's helper copied
into the route so the two agree. Syntax only, as its comment says: it
authorizes no fetch and attests no reachability.

### Verification

Against the app running locally (`next dev`), posting to `/api/skills`:

| `source_url` | `main` | this PR |
| --- | --- | --- |
| `file:///etc/passwd` | reaches the insert | `400 source_url must be a valid http(s) URL` |
| `javascript:alert(1)` | reaches the insert | `400 source_url must be a valid http(s) URL` |
| `http://127.0.0.1:9/skill.md` | reaches the insert | reaches the insert |

`npm run build` and `eslint` both pass.

**Independent review**, 38 probes against this branch and against `main`:
`main` accepted every scheme; this PR refuses exactly the non-http(s) ones and
nothing else. That includes `data:`, `mailto:`, `vbscript:`, mixed case, a
scheme hidden behind leading whitespace or an embedded tab or newline, and
`source_type: "github"` with a `file:` URL. Still accepted: `http`, `https`,
GitHub blob URLs, uppercase schemes, userinfo URLs, unicode hosts. Non-string
values collapse to the pre-existing "source_url is required" refusal, and
duplicate JSON keys resolve the same way in the check and the insert, so the
check cannot be bypassed. Placement and predicate match the form exactly.

### Not in this PR

Rows stored between `1975fb9` and this fix can still hold a non-http(s)
`source_url`, and `GET /api/skills` serves them to agents as recorded. The
catalogue page is safe in a browser, because React 19 neutralises a
`javascript:` href, but a one-off sweep of existing rows
(`select … where source_url !~* '^https?://'`) is worth doing by someone with
database access.

`http(s)` is a syntax check, not an SSRF policy. The form's reachability probe
fetches submitted URLs with redirects followed, so a submission naming
`127.0.0.1`, a private range or a cloud metadata address is still fetched by
the server. Deciding that policy, and where to enforce it, is worth its own
change.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Try it

Open PR on GitHubView diff

Checkout locally

git fetch origin pull/286/head:pr-286
git checkout pr-286