fix(security): close the CSRF gap on the 15 unguarded mutating routes

[lightcloud00]

The gap

Studio already ships a requireJsonContentType() helper — but it is applied to only 33 of the 48 route files that declare a POST/PUT/PATCH/DELETE handler. The remaining 15 had no CSRF defense at all:

Route	What a forged request does
start-agent.ts, start-hermes.ts, systemd-control.ts	spawns a process
skills/install.ts	downloads from GitHub and writes executable skill files to disk
skills/uninstall.ts	deletes skill directories
mcp/reload.ts	triggers an MCP reload on the agent
hermes-proxy/$.ts	forwards POST/PATCH/DELETE to the gateway
hermes-jobs*.ts, hermes-runs.ts, hermes-config.ts, crews/templates/$id.ts, skills/settings.ts, oauth.*.ts	job/run/config mutations

Any page a signed-in user visited could drive these.

Why not just require JSON everywhere

That was the first thing I tried, and it breaks real callers. Several of these mutations legitimately send no Content-Type:

• bodiless DELETEs,
• action POSTs — /api/hermes-jobs/{id}?action=pause, /api/crews/templates/{id}, /api/mcp/reload,
• /api/hermes-proxy, which forwards whatever it is given, multipart included.

I audited the actual call sites before choosing; forcing JSON would have 415'd the app's own UI.

The approach

New rejectCrossSiteMutation() in src/server/rate-limit.ts checks the initiator instead. Browsers set Sec-Fetch-Site on every request and page JavaScript cannot forge it, so cross-site/same-site reliably mean a foreign origin started it.

Non-browser clients (curl, the CLI, scripts) send no Sec-Fetch-* header at all, and an absent value is allowed — so command-line use is unaffected. This needs zero client changes.

Rate limits are added to the six routes where a request is genuinely expensive rather than merely noisy — each starts a process, writes to disk, or pulls remote code.

The test

The suite was green through all of this, because nothing asserted which routes call a guard. Added src/test/csrf-coverage.test.ts, which walks src/routes/api and fails the build when a mutating route ships without one.

It deliberately avoids being the kind of test that only looks like it works:

• it matches the call form with comments stripped, so a commented-out guard fails rather than satisfying a substring search;
• it asserts the guard's actual behaviour — 403 for cross-site and same-site, pass-through for same-origin, absent header, and safe methods — because coverage alone proves only that a route mentions a guard, not that it works.

Both directions were verified by hand: removing a guard, and commenting one out, each fail the test and name the offending file.

Verification

• vitest run → 199 passed (193 before, +6 behavioural).
• tsc --noEmit → unchanged at the pre-existing 4-error baseline; nothing new introduced.
• Diff is +318 / −0 across 17 files — guard hunks only, no reformatting.

Note on conflicts

This overlaps #25 on mcp/reload.ts and skills/install.ts, on the same lines. The two are independent otherwise — happy to rebase this on whichever you merge first, just say the word.

🤖 Generated with Claude Code

[vercel]

@lightcloud00 is attempting to deploy a commit to the Joerg Peetz's projects Team on Vercel.

A member of the Team first needs to authorize it.

[lightcloud00]

Closing this — we're continuing this work on our own repo (lightcloud00/Hermes-Studio) rather than upstream. Nothing here is abandoned; the branch and all its commits are preserved there. Thanks for the project, and apologies for the churn in your PR queue.