fix(share): response-header discipline — default no-store + HSTS/XFO#370
Merged
Conversation
Two header-layer hardening fixes from the post-launch audit pass.
Backend middleware default Cache-Control:
Previously only mutation responses (POST/PUT/PATCH/DELETE) got
Cache-Control: no-store when the handler set nothing. Authenticated
GET endpoints (/api/me, /api/me/shares, /api/admin/audit) each set
their own header — but if ANY future GET forgot the line, the
response would be served without cache protection and a shared
proxy could fan out one user's private body to other readers.
Default ALL /api/* responses (regardless of method) to no-store
when the handler doesn't set its own value. The three public-read
endpoints (/api/snapshots/:id, /api/og/:id.png, /api/profiles/:handle)
already set their own "public, max-age=30, must-revalidate" on
success and the tombstone branch sets no-store — both override
the new default.
share-web global headers:
public/_headers had per-route CSP with frame-ancestors 'none' on
/s/*, /@*, /me, /sign-in — but the root /, 404, and any future
route had NO clickjacking protection. HSTS was entirely absent so
a first-visit downgrade attack was viable on a fresh visitor.
Add globally:
Strict-Transport-Security: max-age=63072000; includeSubDomains; preload
X-Frame-Options: DENY
X-Frame-Options is belt-and-braces with the per-route
frame-ancestors 'none' (some older clients honour XFO but not CSP);
HSTS is the real upgrade.
Tests:
- middleware: assert no-store default on /api/* GET when handler
sets nothing (the regression test for the prior gap)
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Two header-layer hardening fixes from the post-launch audit pass.
Backend middleware default
Cache-ControlPreviously only mutation responses (POST/PUT/PATCH/DELETE) got
Cache-Control: no-storewhen the handler set nothing. Authenticated GET endpoints (/api/me,/api/me/shares,/api/admin/audit) each set their own header — but if ANY future GET forgot the line, the response would be served without cache protection and a shared proxy could fan out one user's private body to other readers.Default ALL
/api/*responses (regardless of method) tono-storewhen the handler doesn't set its own value.flowchart TD REQ["Request to /api/*"] --> H{"Handler set Cache-Control?"} H -->|yes| KEEP["Keep handler value<br/>e.g. public + max-age=30"] H -->|no| METHOD{"Method?"} METHOD -->|"POST/PUT/PATCH/DELETE (before + after)"| MUT["Set no-store"] METHOD -->|"GET — before"| RAW["Pass through<br/>no Cache-Control<br/>relies on every handler"] METHOD -->|"GET — after"| ALL["Set no-store as default"] style RAW fill:#fee,stroke:#c00 style ALL fill:#efe,stroke:#0a0 style MUT fill:#efe,stroke:#0a0The three public-read endpoints (
/api/snapshots/:id,/api/og/:id.png,/api/profiles/:handle) already set their ownpublic, max-age=30, must-revalidateon success; tombstone branches setno-storeexplicitly. All override the new default cleanly.share-web global headers
public/_headershad per-route CSP withframe-ancestors 'none'on/s/*,/@*,/me,/sign-in— but the root/,/404, and any future route had NO clickjacking protection. HSTS was entirely absent so a first-visit downgrade attack was viable on a fresh visitor.Add globally:
X-Frame-Optionsis belt-and-braces with the per-routeframe-ancestors 'none'(some older clients honour XFO but not CSP); HSTS is the real upgrade — 2-year window, includes subdomains, eligible for the browser preload list.Verification
no-storedefault on/api/*GET when handler sets nothingpnpm --filter @spool/share-backend test→ 175/175 green.Risk
no-storedefault is additive — existing handlers that set their own value override it. No behavior change for/api/snapshots/*,/api/og/*,/api/profiles/*,/api/me*,/api/admin/*,/api/health.frame-ancestors 'none'already in force.Submitted by @graydawnc.