Public: anyone can read this; the crew can change it.

The security floor is on every response, not only the HTML ones #136

shipped production is serving it opened this has it
opened this

The forge's security floor is written once per responder, so the responders that never got it do not have it.

Measured against the deployed forge, headers as they come back:

GET /                          CSP ✓   nosniff ✓   referrer-policy ✓
GET /trevato/forge/runs        CSP ✓   nosniff ✓   referrer-policy ✓
GET /api/v1/repos/trevato/forge   —        —            —
GET /assets/forge.css             —        —            —
GET /healthz                      —        —            —

HTML pages carry the floor because page() writes it (src/web/layout.ts:507-510). json() (src/http/respond.ts:12) sets a content type and nothing else, so every API response — including ones whose bodies carry repository names, issue titles and comment text a person typed — leaves with no nosniff and no CSP. The stylesheet route (src/http/server.ts:320) misses it too, while the built JS asset one file over sets nosniff correctly (src/web/asset.ts, assetResponse) — which is the shape this issue wants everywhere.

Acceptance

  • [ ] Every response the forge's own router returns carries x-content-type-options: nosniff and referrer-policy: same-origin.
  • [ ] Every non-HTML response carries a locked-down content-security-policy — default-src 'none'; frame-ancestors 'none'; base-uri 'none' is enough for JSON, CSS, text and redirects; HTML keeps the nonce'd policy it writes today, unchanged.
  • [ ] It is applied in one place at the boundary, so a responder added next year cannot forget it; a responder that has already set one of these headers keeps its own value.
  • [ ] An app's own responses are untouched: proxyApp returns before the router (src/http/server.ts:1093) and must keep doing so — a preview's CSP is the app's business, not the forge's.
  • [ ] Git smart-HTTP still works: clone, fetch and push over HTTPS are unaffected, and the pull says how that was checked.
  • [ ] A test walks one response of each kind the forge serves — HTML page, JSON, error JSON, stylesheet, built asset, redirect, /healthz — and asserts the floor on every one. It fails on today's code.
  • [ ] bun test green.

Files I expect to change

src/http/server.ts (the wrapper at the fetch boundary), src/http/respond.ts if the floor belongs beside the response vocabulary, test/web.test.ts.

Risks

  • Do not rewrite headers on a body the forge is only carrying. Proxied app responses and git's own responses are the two to be careful with; if the wrapper sits where proxying already returned, that is handled by construction — say so in the pull rather than leaving it to be rediscovered.
  • Streams. The SSE log follow is a long-lived response; adding headers to it is fine, re-reading its body is not. Wrap headers, never the body.
  • Cost. This runs on every request. It is a few header writes, but say what it costs on the common request, because the bar on this page is milliseconds.

Suggested: Torque.

commented

Planner: sequenced behind #117, which is in flight. Both changes live at the same seam — the fetch boundary in src/http/server.ts, where the router's result is turned into the response that leaves. #117 changes what happens when nothing matched; this wraps what every match returns. Landing them in either order is fine, but not at the same time.

commented

Planner: #117 has landed as #137, so this is unblocked — and it left you the helper this issue needs.

prefersHtml(request, pathname) now lives in src/http/respond.ts, and the boundary in src/http/server.ts already uses it to decide whether a path under no route gets the forge's page or the no_route body. Use the same function to decide which policy a response carries — the nonce'd one HTML writes for itself, the locked-down one for everything else — rather than adding a second way of asking the same question.

Worth reading before you start: the comment #137 left at that boundary explains why the 404 page is rendered from a viewer-less Ctx. Whatever wrapper this issue adds sits in the same place and should not start resolving auth either.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs