diff --git a/CHANGELOG.md b/CHANGELOG.md index 0475c20e96..a4691c9de5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -136,6 +136,37 @@ aggregate instead: an italic *Catalog* line at the end of the version section an ### Fixed +- **The API host stamps its own security headers, `/_health` stops dropping the site's, + and the CSP is now guarded by a test that also explains why `script-src` still says + `'unsafe-inline'`** — api.anyplot.ai is a separate origin with no nginx in front of it, + so it inherited none of `app/security-headers.conf`: only `/proxy/html` set + `nosniff` and a `Referrer-Policy`, on that one response. An outermost middleware now + `setdefault`s both on every response that leaves through the stack — CORS preflights, the + origin gate's 403, an `HTTPException`'s 4xx — and the unhandled-500 handler stamps the + same pair through the same helper, because `ServerErrorMiddleware` wraps every user + middleware and builds that response outside the stack, which is the one exit a middleware + cannot reach. Deliberately **not** `X-Frame-Options`, because + the SPA embeds `/proxy/html` cross-origin in an iframe and `SAMEORIGIN` would break + every interactive preview. On the website, both `/_health` locations set an + `add_header` of their own 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. Both were found by the new + `tests/unit/api/test_csp_policy.py`, which also pins that the CSP keeps `object-src + 'none'` and `base-uri 'self'`, that a `report-to` group it names is actually defined by a + `Reporting-Endpoints` header (reports to an undeclared group go nowhere, and nowhere reads + exactly like "no violations"), and that the + three sha256 hashes the policy holds in reserve still describe `app/index.html`'s + inline scripts. Those hashes are in reserve rather than 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 so 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. All of it is + written down at the directive it explains. (#11213) + - **The IndexNow workflow no longer waits eight minutes behind an edge 403** — its key-file readiness loop treated every non-200 as "not deployed yet"; a GitHub runner that Cloudflare's bot management answers with 403 would have slept the full budget on diff --git a/api/exceptions.py b/api/exceptions.py index c0817cdc27..2938a21034 100644 --- a/api/exceptions.py +++ b/api/exceptions.py @@ -10,6 +10,8 @@ from fastapi.responses import JSONResponse from pydantic import BaseModel +from api.security_headers import stamp as stamp_security_headers + logger = logging.getLogger(__name__) @@ -140,10 +142,17 @@ async def generic_exception_handler(request: Request, exc: Exception) -> JSONRes Never reflects the raw exception text back to clients — `str(exc)` can leak DSN fragments, table names, file-path traceback fragments, and other internal state. The full traceback goes to the server log instead. + + Stamps the security headers itself. `ServerErrorMiddleware` wraps every user + middleware, so this response is built OUTSIDE the http middleware stack and + is the one exit `api/main.py`'s header middleware cannot reach (Copilot + review). Same helper on both paths, so the two cannot drift. """ logger.exception("Unhandled exception on %s", request.url.path) - return JSONResponse( - status_code=500, content={"status": 500, "message": "Internal server error", "path": _public_path(request)} + return stamp_security_headers( + JSONResponse( + status_code=500, content={"status": 500, "message": "Internal server error", "path": _public_path(request)} + ) ) diff --git a/api/main.py b/api/main.py index d69e48b8f8..451eb0da4c 100644 --- a/api/main.py +++ b/api/main.py @@ -46,6 +46,7 @@ from api.routers.plots import _refresh_filter_all # noqa: E402 from api.routers.specs import _refresh_specs_list, _refresh_specs_map # noqa: E402 from api.routers.stats import _refresh_stats # noqa: E402 +from api.security_headers import stamp as stamp_security_headers # noqa: E402 from api.version import APP_VERSION # noqa: E402 from core.config import settings # noqa: E402 from core.constants import LANGUAGES_METADATA, LIBRARIES_METADATA # noqa: E402 @@ -166,7 +167,7 @@ async def lifespan(app: FastAPI): # `@app.middleware` both wrap what is already there — so reading this file from # here down gives the order a request actually travels, in reverse: # -# cache headers → CORS → origin gate → bot counter → gzip → router +# security headers → cache headers → CORS → origin gate → bot counter → gzip → router # # (`HeadAsGetMiddleware` and `MCPTrailingSlashMiddleware` wrap the whole app # further out still; both only rewrite the scope.) @@ -286,6 +287,24 @@ async def add_cache_headers(request: Request, call_next): return response +# Added LAST, so it is the OUTERMOST http middleware and every response that +# leaves through the stack passes back through it — CORS preflights, the origin +# gate's 403, an HTTPException's 4xx. +# +# It cannot be the only place, though. `ServerErrorMiddleware` wraps every user +# middleware, so a route that RAISES makes `await call_next(request)` raise too +# and the registered `Exception` handler's 500 is built outside this stack. That +# path stamps the same headers itself, through the same helper +# (`api/security_headers.py`, `api/exceptions.py::generic_exception_handler`). +@app.middleware("http") +async def add_security_headers(request: Request, call_next): + """Stamp the baseline security headers the API host was missing. + + Which headers, and why not `X-Frame-Options`: `api/security_headers.py`. + """ + return stamp_security_headers(await call_next(request)) + + # Mount MCP server for AI assistant integration app.mount("/mcp", mcp_http_app) diff --git a/api/security_headers.py b/api/security_headers.py new file mode 100644 index 0000000000..e35fce3ba2 --- /dev/null +++ b/api/security_headers.py @@ -0,0 +1,50 @@ +"""The two security headers every API response carries, in one place. + +`app/security-headers.conf` gives the website its headers through nginx. +api.anyplot.ai is a separate origin with no nginx in front of it, so it inherits +none of them — and served none until this module existed, apart from the pair +`/proxy/html` set by hand on that one response. + +One place, because there are TWO exits from the app and only one of them is a +middleware. Starlette's `ServerErrorMiddleware` wraps every user middleware, so +when a route raises, `await call_next(request)` raises with it and the response +the registered `Exception` handler builds is produced OUTSIDE the stack — an +unhandled 500 would leave without headers while the middleware's docstring +claimed otherwise (Copilot review). Both paths call `stamp` instead. + +Deliberately NOT `X-Frame-Options`: the SPA embeds `/proxy/html` in an iframe +from a different origin (`frame-src https://api.anyplot.ai` in the site's CSP), +and `SAMEORIGIN` would break every interactive plot preview. +""" + +from __future__ import annotations + +from typing import TypeVar + +from starlette.responses import Response + + +# So a caller that hands in a JSONResponse gets a JSONResponse back — the +# exception handler's signature promises one, and a bare `Response` return would +# make the helper the reason mypy fails there. +ResponseT = TypeVar("ResponseT", bound=Response) + +SECURITY_HEADERS = { + # The API returns JSON, PNG and (on /proxy/html) HTML from the same host, so + # content-type sniffing is exactly the confusion to forbid. + "X-Content-Type-Options": "nosniff", + # The same value the website sends, so a link followed out of an API-served + # page leaks no path. + "Referrer-Policy": "strict-origin-when-cross-origin", +} + + +def stamp(response: ResponseT) -> ResponseT: + """Add the baseline headers, keeping any a route set on purpose. + + `setdefault`, so a response with a reason to say something else — as + `/proxy/html` does — keeps its own value. + """ + for name, value in SECURITY_HEADERS.items(): + response.headers.setdefault(name, value) + return response diff --git a/app/nginx.conf b/app/nginx.conf index 05aba16f26..b4c3ad6405 100644 --- a/app/nginx.conf +++ b/app/nginx.conf @@ -307,6 +307,10 @@ server { access_log off; return 200 "OK"; add_header Content-Type text/plain; + # This location's own add_header drops every inherited one, which is + # the rule the top of security-headers.conf states and the one place in + # this file that had missed it (found by tests/unit/api/test_csp_policy.py). + include /etc/nginx/security-headers.conf; } # Proxy sitemap.xml to backend API (dynamic generation) @@ -459,6 +463,8 @@ server { access_log off; return 200 "OK"; add_header Content-Type text/plain; + # Same as the main block: an own add_header drops the inherited ones. + include /etc/nginx/security-headers.conf; } location = /sitemap.xml { diff --git a/app/security-headers.conf b/app/security-headers.conf index ef73e7ab79..2e3c38e2b5 100644 --- a/app/security-headers.conf +++ b/app/security-headers.conf @@ -7,9 +7,54 @@ # location, re-include this file there. # # CSP notes (must not break the SPA — see app/index.html and app/src): -# - script-src 'unsafe-inline': index.html ships inline scripts (theme -# resolver, Eruda loader, Plausible stub); no nonce infra for a static file. +# - script-src 'unsafe-inline': index.html ships three executable inline +# scripts (theme resolver, Eruda loader, Plausible stub), and a FOURTH one +# arrives that this repository does not write — see the block below. # - script-src cdn.jsdelivr.net: on-device debug console (Eruda) behind ?debug=1. +# +# Why script-src still says 'unsafe-inline' (measured 2026-09-03) +# --------------------------------------------------------------- +# Replacing 'unsafe-inline' with the sha256 of each inline script is the +# obvious hardening — index.html is static, so its scripts are fixed at build +# time, and `yarn build` was verified to copy them through byte-for-byte. The +# three hashes are recorded below and pinned by tests/unit/api/test_csp_policy.py +# so they never go stale. +# +# They cannot be ENFORCED yet. Cloudflare JavaScript Detections injects an +# inline script into every HTML response at the edge, after nginx, and its body +# carries a per-response ray id and timestamp — so its hash differs on every +# request and cannot be listed here. The whole policy was mounted over the LIVE +# production bundle through a local proxy and loaded twice, once with each +# script-src: +# +# 'unsafe-inline' → Cloudflare's script runs (its hidden iframe appears) +# hashes only → "Executing inline script violates … The action has +# been blocked", no iframe, no JS-detection signal +# +# Exactly one script is blocked, and it is the edge's. Shipping the hash policy +# would silently degrade bot detection on a site whose origin gate leans on the +# edge — so it is not shipped, and 'unsafe-inline' is NOT joined by hashes +# either: a browser ignores 'unsafe-inline' as soon as a hash is present, so the +# two together are the same breakage wearing a stricter-looking policy. +# +# The way out is a NONCE, not a hash. Cloudflare parses this response header +# and stamps its own injected script with the nonce it finds there (their +# JavaScript Detections docs say so explicitly, and recommend it over +# 'unsafe-inline'). That needs nginx to mint one per request and rewrite +# index.html's ``, so a single +# re-indent invalidates one. The test recomputes them from index.html on every +# run, which is what keeps this block honest while it waits. # - style-src 'unsafe-inline': MUI/emotion inject inline styles. # - img/font/connect storage.googleapis.com: plot previews + MonoLisa fonts on GCS. # - img/connect/frame api.anyplot.ai: API calls, og images, interactive-preview diff --git a/tests/unit/api/test_csp_policy.py b/tests/unit/api/test_csp_policy.py new file mode 100644 index 0000000000..e41a63327c --- /dev/null +++ b/tests/unit/api/test_csp_policy.py @@ -0,0 +1,248 @@ +"""The delivery-side security policy, held against the files it describes. + +`app/security-headers.conf` is a string in an nginx include. Nothing compiles +it, nothing imports it, and three things in it can drift silently — each of +which has cost someone a day somewhere: + +1. **The inline-script hashes.** `script-src` cannot enforce them yet — + Cloudflare JavaScript Detections injects a fourth inline script at the edge + whose body changes per response (measured 2026-09-03, the reasoning is in + `security-headers.conf`) — so the file records them in a comment instead, + ready for the day a nonce or a zone setting makes the switch possible. A + recorded hash that no longer matches its script is worse than none: it looks + like readiness. This recomputes them on every run. + +2. **nginx's `add_header` inheritance.** A location with any `add_header` of + its own drops every inherited one. `app/nginx.conf` therefore re-includes + the snippet in each such location, and forgetting that in a new location is + invisible in review and invisible in the browser until someone checks that + one URL. + +3. **The API host's own headers.** api.anyplot.ai is a separate origin with no + nginx in front of it, so nothing there inherits anything from the website — + and it has two exits, only one of which is a middleware. + +The nginx parse is deliberately crude — a brace counter over one file we write +ourselves, not a config parser. It only has to be right about this file. +""" + +from __future__ import annotations + +import base64 +import hashlib +import re +from pathlib import Path + +from fastapi.testclient import TestClient + +from api.main import app, fastapi_app + + +ROOT = Path(__file__).resolve().parents[3] +INDEX_HTML = ROOT / "app" / "index.html" +HEADERS_CONF = ROOT / "app" / "security-headers.conf" +NGINX_CONF = ROOT / "app" / "nginx.conf" + +INCLUDE_LINE = "include /etc/nginx/security-headers.conf;" + +# Comments are stripped BEFORE the script scan. index.html documents its own +# Eruda loader with the words `Plain `, ``). A +# regex that missed one of those would swallow the rest of the document into a +# single "script body" and hash that, silently. The lookahead is what keeps +# `` from counting as a close. +_SCRIPT = re.compile(r"[^>]*)>(?P.*?)])[^>]*>", re.DOTALL | re.IGNORECASE) +_TYPE = re.compile(r"""type\s*=\s*["']?([^"'\s>]+)""", re.IGNORECASE) +# An EXTERNAL script, which CSP judges by its URL and never by a hash. HTML +# attribute names are case-insensitive and whitespace around `=` is legal, so +# `