fix(security): headers for the API host, and the measurement that stopped the CSP hardening - #11213
Conversation
… before hardening it api.anyplot.ai is a separate origin with no nginx in front of it, so it inherited nothing from app/security-headers.conf — only /proxy/html set nosniff and a Referrer-Policy, on that one response. An outermost middleware now setdefaults both on every response. NOT X-Frame-Options: the SPA embeds /proxy/html cross-origin in an iframe, and SAMEORIGIN would break every interactive preview. Both /_health locations set an add_header without re-including the snippet, and nginx drops every inherited header in such a location — the rule the file states at the top, and the one place that had missed it. tests/unit/api/test_csp_policy.py found that one, and pins the rest: object-src and base-uri stay closed, report-to never sits beside report-uri (Chromium then reports nothing), the API headers are present and X-Frame-Options is not, and the sha256 hashes the policy holds in reserve still describe index.html's inline scripts. In reserve, not in force, for a measured reason. Mounted over the live production bundle through a local proxy, a hash-only script-src blocks exactly one script: the inline one Cloudflare JavaScript Detections injects at the edge, whose body carries a per-response ray id and therefore has no fixed hash. With 'unsafe-inline' its hidden iframe appears; with hashes it does not, and the console reads "The action has been blocked". Hardening would have silently cost bot detection on a site whose origin gate leans on the edge. The way out is a nonce — Cloudflare stamps its injected script with the nonce it parses from this header — which needs an nginx sub_filter no test here can prove. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UEScQMZFvxxNNyNJYryfa3
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UEScQMZFvxxNNyNJYryfa3
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
🟡 Changes recommended
API 500 responses remain uncovered, and the CSP reporting guard rejects a valid configuration.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds API and nginx security headers while documenting why CSP hash enforcement remains deferred.
Changes:
- Adds API-wide baseline headers.
- Restores headers on nginx health endpoints.
- Adds security-policy regression tests and documentation.
File summaries
| File | Description |
|---|---|
tests/unit/api/test_csp_policy.py |
Adds policy tests, including an incorrect report-to/report-uri exclusion. |
CHANGELOG.md |
Records changes but repeats the incorrect CSP reporting claim. |
app/security-headers.conf |
Documents CSP findings and reserved hashes. |
app/nginx.conf |
Restores security headers on health responses. |
api/main.py |
Adds header middleware, but misses unhandled 500 responses. |
Review details
Suppressed comments (1)
CHANGELOG.md:153
- This repeats the incorrect claim enforced by the new test: Chromium prefers
report-towhen both directives exist, whilereport-urican validly remain as a legacy fallback. The observed silence points to an unusable or missing reporting endpoint configuration, not to the two CSP directives coexisting; update this entry after correcting the guard.
`tests/unit/api/test_csp_policy.py`, which also pins that the CSP keeps `object-src
'none'` and `base-uri 'self'`, that it never carries `report-to` beside `report-uri`
(measured in the sibling repo: Chromium then reports nothing at all), and that the
- Files reviewed: 5/5 changed files
- Comments generated: 2
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
CodeQL py/bad-tag-filter: `</script>` misses `</script >`, which HTML permits — and a missed close swallows the rest of the document into one script body and hashes that. This parses a file people edit, so the strictness is earned. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UEScQMZFvxxNNyNJYryfa3
There was a problem hiding this comment.
🟡 Changes recommended
Fix unhandled-500 header coverage and correct the CSP reporting and script parsing tests.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (4)
Previously missed (1) — in code that hasn't changed since the last review.
CHANGELOG.md:145
- This changelog claim overstates the middleware's coverage: an unhandled exception is converted to a 500 by Starlette's outer
ServerErrorMiddleware, after this user middleware unwinds, so that response does not receive these headers. Keep this wording only after the header logic wraps the final ASGI app outside FastAPI.
CHANGELOG.md:153
- This repeats the incorrect conclusion that merely placing
report-tobesidereport-uridisables reporting. Standards-compatible deployments commonly send both: supporting browsers usereport-to, and older browsers fall back toreport-uri. Remove this claim or describe the actual missing/invalid endpoint configuration that caused the measured failure.
`tests/unit/api/test_csp_policy.py`, which also pins that the CSP keeps `object-src
'none'` and `base-uri 'self'`, that it never carries `report-to` beside `report-uri`
(measured in the sibling repo: Chromium then reports nothing at all), and that the
api/main.py:315
- Unhandled exceptions do not return from
call_next: Starlette's always-outerServerErrorMiddlewareinvokes the registeredExceptionhandler and sends that 500 outside all user middleware. Therefore these headers are still absent from the unexpected-500 responses this change explicitly claims to cover. Apply the header mutation in an ASGI wrapper around the finished app (or directly in the generic handler), and add a route that raises withraise_server_exceptions=Falseto pin this path.
response: Response = await call_next(request)
response.headers.setdefault("X-Content-Type-Options", "nosniff")
response.headers.setdefault("Referrer-Policy", "strict-origin-when-cross-origin")
tests/unit/api/test_csp_policy.py:177
- Coexistence does not suppress CSP reports. Supporting browsers use
report-toand ignorereport-uri, while browsers withoutreport-tosupport fall back toreport-uri; carrying both is the standard cross-browser migration pattern. A Chromium run that emitted nothing indicates an unconfigured or invalid reporting group, not a conflict between these directives. This assertion would reject the compatible setup and force a reporting gap; instead validate that anyreport-togroup has a matching reporting-endpoint configuration.
directives = csp_directives()
assert not ("report-to" in directives and "report-uri" in directives), (
"CSP carries both report-to and report-uri — Chromium then reports nothing. Keep one."
- Files reviewed: 5/5 changed files
- Comments generated: 1
- Review effort level: Balanced
CodeQL again, and again right: after `</script` a browser skips to the first `>`, so `</script\t\n bar>` closes the element too. The lookahead keeps `</scriptfoo>` from counting as one. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UEScQMZFvxxNNyNJYryfa3
Three Copilot findings, all real. 1. ServerErrorMiddleware wraps every user middleware, so a route that raises makes `await call_next` raise with it and the Exception handler's 500 is built OUTSIDE the stack — the middleware's claim to cover it was wrong. The two headers now live in api/security_headers.py and both exits call the same `stamp`; a test registers a raising route on the real app and asserts the 500 carries them. 2. The report-to/report-uri mutual exclusion rejected a legitimate migration: Chromium prefers report-to and keeps report-uri as the fallback for clients without it. Replaced by the check that actually catches silence — a `report-to <group>` must be defined by a Reporting-Endpoints header, because reports to an undeclared group go nowhere and nowhere reads exactly like "no violations". 3. `"src=" in attrs` misclassified `<script SRC = "…">` as inline and would have demanded a hash for a script with no body. Now `\bsrc\s*=`, case-insensitive. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UEScQMZFvxxNNyNJYryfa3
The brief was "swap
script-src 'unsafe-inline'for sha256 hashes, then walk the public pages with the console open". The walk is what changed the answer, so the measurement comes first.The measurement: hash-only
script-srcbreaks Cloudflare, not the SPAapp/index.htmlhas three executable inline scripts (theme resolver, Eruda loader, Plausible stub) and three JSON-LD data blocks that need no hash.yarn buildwas verified to copy the three through byte-for-byte, so hashing the source file describes what nginx serves.The candidate policy was then mounted over the live production bundle — a local proxy that mirrors
https://anyplot.aiand re-stamps the response with each header set — and loaded twice in Chrome:script-src'self' 'unsafe-inline' cdn.jsdelivr.net(today)'self' <3 × sha256> cdn.jsdelivr.netExecuting inline script violates the following Content Security Policy directive … The action has been blocked.Exactly one script is blocked, and it is not ours. Cloudflare JavaScript Detections injects an inline script into every HTML response at the edge, after nginx:
The ray id and timestamp change per response, so its hash does too — it can never be listed. Confirmed the same script and iframe on
https://anyplot.aiitself, so this is production behaviour, not a proxy artefact.Shipping the hardened policy would have silently degraded bot detection on a site whose brand-new origin gate (#11208) explicitly leans on the edge. So it is not shipped, and — equally deliberately — the hashes are not added next to
'unsafe-inline'either: a browser ignores'unsafe-inline'the moment a hash appears, so that combination is the identical breakage wearing a stricter-looking label. A test now forbids it.The way out is a nonce, not a hash. Cloudflare documents that it parses this response header and stamps its injected script with the nonce it finds, and recommends that over
'unsafe-inline'. That needs nginx to mint one per request and rewrite index.html's<script>tags (sub_filterplusgzip_static offfor the shell) — a delivery change no test in this repo can prove and no local nginx here can run. Owner call, and the alternative (turning JavaScript Detections off in the zone) is a security trade rather than a fix. The reasoning, the numbers and the three hashes sit insecurity-headers.confat the directive they explain, so the switch is a one-line edit whenever one of the two happens.What does ship
The API host stamps its own baseline headers.
api.anyplot.aiis a separate origin with no nginx in front of it, so it inherited none ofapp/security-headers.conf— only/proxy/htmlsetnosniffand aReferrer-Policy, by hand, on that one response. An outermost middleware nowsetdefaults both on every response, including CORS preflights, the origin gate's 403 and the exception handlers' 500s. Deliberately notX-Frame-Options: the SPA embeds/proxy/htmlcross-origin in an iframe (frame-src https://api.anyplot.ai), andSAMEORIGINwould break every interactive plot preview — the test asserts its absence so nobody adds it as an obvious-looking improvement.Both
/_healthlocations stop dropping the site's headers. They set anadd_headerof their own, and nginx drops every inherited header in such a location — the rule stated at the top ofsecurity-headers.conf, and the one place innginx.confthat had missed it. Found by the new test, verified againstorigin/main:tests/unit/api/test_csp_policy.py— six checks over files that nothing else compiles or imports: the reserve hashes still matchindex.html;script-srcnever mixes'unsafe-inline'with a hash;object-src 'none'andbase-uri 'self'stay closed; the policy never carriesreport-tobesidereport-uri(measured in the sibling repo: Chromium then reports nothing at all — neither is set here today, so this exists for whoever adds reporting); every nginx location with its own header re-includes the snippet; and the API host's two headers are present whileX-Frame-Optionsis not.One anyplot-specific trap it had to handle:
index.htmldocuments its own Eruda loader with the wordsPlain <script> (not type="module")inside an HTML comment, and a script regex that reads that as a tag hashes the comment prose instead of the script — silently, with a hash that looks perfectly plausible. Comments are stripped first.Items from the brief that turned out to be no-ops here
report-tobesidereport-uri— anyplot's CSP has neither. Encoded as a test instead of a fix.bluetooth=()in the Permissions-Policy — anyplot sends noPermissions-Policyat all. Nothing to remove. Adding one is a separate, easy hardening pass; not folded in here.Verification
pytest tests/unit tests/integration— 1913 passed, 1 skipped.ruff check,ruff format --checkandmypy api coreclean. The CSP walk itself is the table above, run against the live bundle in Chrome.🤖 Generated with Claude Code
https://claude.ai/code/session_01UEScQMZFvxxNNyNJYryfa3