docs(website): correct the CSP delta to one directive and document the report sink #51

Merged
whitlocktech merged 1 commits from feature/csp-report-only into main 2026-07-27 20:22:30 +00:00
3 changed files with 94 additions and 16 deletions
Showing only changes of commit 9a3e1cc1e7 - Show all commits

View File

@@ -181,22 +181,64 @@ form-action 'self';
frame-ancestors 'none'; frame-ancestors 'none';
``` ```
Delta vs. the policy in `server/src/app.js` today — the whole change is two directives: Delta vs. the policy in `server/src/app.js` today. This was written as two directives; on
implementation it turned out to be **one**:
- **Add `form-action 'self'`** (currently absent) — blocks an injected `<form action="https://evil">` - ~~**Add `form-action 'self'`** (currently absent)~~**it was not absent.** The directives object in
from POSTing credentials off-origin, an exfil path `connect-src` doesn't cover. `app.js` does not list it, but the middleware is configured `useDefaults: true`, and helmet's default
- **Tighten `frame-ancestors`** `'self'``'none'` — nothing legitimately frames the site. set already supplies `form-action 'self'` — so the header served in production has carried it all
along. Verified by capturing the live `Content-Security-Policy` header from the running app rather
than reading the config, which is how the plan got this wrong. **No behavioural change here.** It is
now written out explicitly in `config/csp.js` anyway: a security directive should not depend on a
third-party library's defaults surviving its next major version.
- **Tighten `frame-ancestors`** `'self'``'none'` — nothing legitimately frames the site. **This is
the entire behavioural delta of the phase.**
- Unchanged: `default-src`, `script-src`, `connect-src`, `object-src 'none'`, `base-uri 'self'`, and - Unchanged: `default-src`, `script-src`, `connect-src`, `object-src 'none'`, `base-uri 'self'`, and
`img-src … https:` (external `BRAND_*` logo/hero and `<img>` in sanitized wiki/news bodies rely on `img-src … https:` (external `BRAND_*` logo/hero and `<img>` in sanitized wiki/news bodies rely on
`https:`). `https:`).
The soak is still worth running for that one directive, and arguably it is the directive that most
needs one: a `frame-ancestors` report is generated by the browser of *whoever framed the site*, so it
is the only way to discover that something legitimately embeds us before the enforcing policy breaks
it. Nothing else can tell us that.
**Rollout:** ship via `Content-Security-Policy-Report-Only` with `report-to` for one release, watch for **Rollout:** ship via `Content-Security-Policy-Report-Only` with `report-to` for one release, watch for
violations, then flip to enforce. violations, then flip to enforce.
Before trusting `script-src 'self'`: Vite's build injects an inline modulepreload-polyfill `<script>` Before trusting `script-src 'self'`: Vite's build injects an inline modulepreload-polyfill `<script>`
into `dist/index.html`, which that directive blocks (harmless, but throws a violation). Confirm it's into `dist/index.html`, which that directive blocks (harmless, but throws a violation).
disabled or set `build.modulepreload.polyfill = false` in the Vite config. (The `renderIndexHtml` **Already handled**`client/vite.config.js` sets `modulePreload: { polyfill: false }`, so the build
branding injection adds only `<meta>`/`<link>` tags — no inline script, no nonce needed.) emits no inline bootstrap script. (The `renderIndexHtml` branding injection adds only `<meta>`/`<link>`
tags — no inline script, no nonce needed.)
### Where reports go
`report-to` needs somewhere to point, so the report-only PR stands up a same-origin sink:
**`POST /api/csp-report`** (`server/src/router/cspReport.controller.js`, wired in `app.js`). Same-origin
on purpose — violation reports describe attacks against this site and are not handed to a third-party
collector. It writes to the `csp` log tag and stores nothing.
It is mounted outside `/api/v1`, alongside `/api/health`: the browser learns the path from the policy
header, never from a client build, so it is not part of the versioned client contract. This is the
`+1` in the route manifest that made PR 0 go first (see § Sequencing).
Necessary properties, since it is an unauthenticated public `POST` (browsers send reports with no
session, and gating it would silence exactly the anonymous visitors worth hearing about):
- **Both wire formats.** `report-uri` (Firefox, Safari) sends `application/csp-report` with a single
hyphenated-key object; `report-to` (Chrome) sends `application/reports+json` with an array of
camelCase envelopes. Handling one silently drops half the browsers. Both directives are emitted, and
`report-to` additionally needs a `Reporting-Endpoints` response header or it is inert.
- **Always 204, even for junk.** A 4xx would make the global error handler write an ERROR line quoting
the attacker-supplied body — turning an open endpoint into a log-flood primitive. A browser cannot
act on an error from a report sink anyway.
- **Bounded everywhere:** 16 KB body cap, a per-IP rate limit, a fixed field allowlist, and every
logged field truncated (`script-sample` is attacker-influenced and can carry a whole inline script).
**Retiring it:** the sink exists for the soak. When the tightened policy flips to enforced and the
report-only twin is deleted, this endpoint goes with it — *unless* a `report-to` group is deliberately
kept on the enforced policy, which is a reasonable thing to want. Decide that in the enforce PR rather
than leaving an orphan route behind.
Tracked follow-ups (own PRs): Tracked follow-ups (own PRs):
@@ -302,9 +344,11 @@ proves it, with no router file moved.
is stable). The manifest keeps only `/api/**`, `/.well-known/**`, and the internal app's routes, so is stable). The manifest keeps only `/api/**`, `/.well-known/**`, and the internal app's routes, so
it does not change depending on whether CI built the client. Static mounts are not API contract. it does not change depending on whether CI built the client. Static mounts are not API contract.
- **The baseline already exists:** [`api-route-inventory.json`](./api-route-inventory.json) in this - **The baseline already exists:** [`api-route-inventory.json`](./api-route-inventory.json) in this
directory is today's frozen surface — **199 API routes** (110 of them `/api/v1/admin`) plus 2 directory is the frozen surface — **199 API routes** (110 of them `/api/v1/admin`) plus 2
internal. PR 0's generator must **reproduce this file byte-for-byte**; that is PR 0's own acceptance internal at the time PR 0 was written. PR 0's generator must **reproduce this file byte-for-byte**;
test, and it means the freeze is already in effect before the first router moves. that is PR 0's own acceptance test, and it means the freeze is already in effect before the first
router moves. *(It did, on first run. The file has since moved to **200** — the CSP report-only PR
added `POST /api/csp-report`, the first deliberate, reviewed manifest diff.)*
- **Runtime introspection, not source parsing.** It is authoritative about mounts, and the route paths - **Runtime introspection, not source parsing.** It is authoritative about mounts, and the route paths
in `admin.routes.js` sit on the line *after* `adminRouter.get(`, which defeats naive greps. in `admin.routes.js` sit on the line *after* `adminRouter.get(`, which defeats naive greps.
- **Not `swagger-output.json`.** That is annotation-derived (only annotated routes appear) and churns - **Not `swagger-output.json`.** That is annotation-derived (only annotated routes appear) and churns
@@ -341,8 +385,11 @@ deliberate `+1` in the manifest — which is exactly the mechanism working as de
1. **PR 0 — route manifest.** Generator + CI check + committed baseline of today's surface. No routers moved. ✅ landed 1. **PR 0 — route manifest.** Generator + CI check + committed baseline of today's surface. No routers moved. ✅ landed
2. **PR — CSP report-only.** Tightened policy behind `Content-Security-Policy-Report-Only` + `report-to`, 2. **PR — CSP report-only.** Tightened policy behind `Content-Security-Policy-Report-Only` + `report-to`,
plus the report collector (manifest `+1`). plus the report collector (manifest `+1` — the first deliberate, reviewed manifest diff). ✅ landed
3. **PR — CSP enforce.** One release later, assuming a clean violation report. 3. **PR — CSP enforce.** One release later, assuming a clean violation report. **Blocked on real soak
data**, not on code: watch the `csp` log tag for `frame-ancestors` reports across one release before
flipping. Also decide there whether `/api/csp-report` is retired with the report-only twin or kept
as a `report-to` group on the enforced policy.
4. **PR 1 — admin:** `users`, `account`, `invites`, `auth` (providers). 4. **PR 1 — admin:** `users`, `account`, `invites`, `auth` (providers).
5. **PR 2 — admin:** `moderation`, `bot-activity`, `activity`. 5. **PR 2 — admin:** `moderation`, `bot-activity`, `activity`.
6. **PR 3 — admin (content):** `posts`, `pages`, `wiki`, `uploads`. 6. **PR 3 — admin (content):** `posts`, `pages`, `wiki`, `uploads`.

View File

@@ -270,7 +270,7 @@ are authoritative, and they answer different questions:
| Artifact | Source of truth for | Generated by | | Artifact | Source of truth for | Generated by |
|---|---|---| |---|---|---|
| `server/routes.manifest.json` — mirrored as [api-route-inventory.json](./api-route-inventory.json) | **What URLs exist.** 199 public routes + 2 on the internal listener, sorted, method + path only. | `npm run routes:manifest`, by walking the live Express stack | | `server/routes.manifest.json` — mirrored as [api-route-inventory.json](./api-route-inventory.json) | **What URLs exist.** 200 public routes + 2 on the internal listener, sorted, method + path only. | `npm run routes:manifest`, by walking the live Express stack |
| `server/swagger/swagger-output.json` — served at `/api/docs` | **What each route means.** Parameters, bodies, response codes, security. | `npm run swagger`, from `#swagger.*` annotations | | `server/swagger/swagger-output.json` — served at `/api/docs` | **What each route means.** Parameters, bodies, response codes, security. | `npm run swagger`, from `#swagger.*` annotations |
The split is deliberate: Swagger is annotation-derived, so an unannotated route is invisible in it and The split is deliberate: Swagger is annotation-derived, so an unannotated route is invisible in it and
@@ -490,7 +490,8 @@ who"; `activity_log` provides the history feed.
- **bcrypt** hashing (cost 10+); plaintext passwords never stored, logged, or returned. - **bcrypt** hashing (cost 10+); plaintext passwords never stored, logged, or returned.
- **Rate limiting** (`express-rate-limit`) on `/auth/login` and `/public/contact`. - **Rate limiting** (`express-rate-limit`) on `/auth/login` and `/public/contact`.
- **Validation** (`express-validator`) on all writes; centralized error handler. - **Validation** (`express-validator`) on all writes; centralized error handler.
- **helmet** with a Content-Security-Policy tuned for the built React SPA (see `server/src/app.js`): - **helmet** with a Content-Security-Policy tuned for the built React SPA. The policies now live in
**`server/src/config/csp.js`** (`app.js` only wires them up):
`default-src 'self'`; `script-src 'self'` (the Vite build emits only external module chunks — the `default-src 'self'`; `script-src 'self'` (the Vite build emits only external module chunks — the
inline module-preload polyfill is disabled in `client/vite.config.js` to keep this valid); inline module-preload polyfill is disabled in `client/vite.config.js` to keep this valid);
`style-src 'self' 'unsafe-inline' https://fonts.googleapis.com` (React's pervasive inline `style-src 'self' 'unsafe-inline' https://fonts.googleapis.com` (React's pervasive inline
@@ -498,11 +499,33 @@ who"; `activity_log` provides the history feed.
https://fonts.gstatic.com` (Cinzel); `img-src 'self' data: https:` (same-origin uploads, plus https://fonts.gstatic.com` (Cinzel); `img-src 'self' data: https:` (same-origin uploads, plus
external https images embedded in wiki/news bodies or `BRAND_*` logo/hero/favicon); `connect-src external https images embedded in wiki/news bodies or `BRAND_*` logo/hero/favicon); `connect-src
'self'` (REST + SSE are same-origin); `frame-ancestors 'self'`; `object-src 'none'`; `base-uri 'self'` (REST + SSE are same-origin); `frame-ancestors 'self'`; `object-src 'none'`; `base-uri
'self'`. `upgrade-insecure-requests` is intentionally **not** set (TLS terminates at the proxy, there 'self'`; `form-action 'self'` (blocks an injected `<form action="https://evil">` from POSTing
credentials off-origin — an exfil path `connect-src` does not cover; it was always emitted via
helmet's `useDefaults` and is now pinned explicitly so it cannot vanish under a helmet upgrade).
`upgrade-insecure-requests` is intentionally **not** set (TLS terminates at the proxy, there
are no mixed-content subresources, and it would break a local `npm start` over plain http). The are no mixed-content subresources, and it would break a local `npm start` over plain http). The
`/api/docs` Swagger UI route gets a **looser** policy that additionally allows inline script/style, `/api/docs` Swagger UI route gets a **looser** policy that additionally allows inline script/style,
since swagger-ui-express injects an inline bootstrap. helmet also strips `X-Powered-By`; the two since swagger-ui-express injects an inline bootstrap. helmet also strips `X-Powered-By`; the two
internal-only listeners (`internalApp.js`, `bot/src/app.js`) disable it explicitly too. internal-only listeners (`internalApp.js`, `bot/src/app.js`) disable it explicitly too.
- **A second, tightened policy ships alongside on `Content-Security-Policy-Report-Only`** for one
release before it replaces the enforced one (`docs/website/API_V2_PLAN.md` § Phase 1). It is derived
from the enforced policy so the two cannot drift, and differs by exactly one directive:
`frame-ancestors 'self'`**`'none'`**. Serving both headers at once means the live policy keeps
protecting users while anything the tightened version would break arrives as a report rather than as
a broken page — and for `frame-ancestors` specifically, a report from the browser of whoever framed
the site is the only way to learn that something does.
- **`POST /api/csp-report`** is the same-origin violation sink that `report-to` / `report-uri` point at
(`report-to` additionally requires the `Reporting-Endpoints` response header, which is set alongside).
Same-origin on purpose: reports describe attacks against this site and are not handed to a
third-party collector. It parses **both** wire formats (`application/csp-report` from Firefox/Safari,
`application/reports+json` from Chrome's Reporting API — handling one drops half the browsers),
writes to the `csp` log tag and **stores nothing**. Necessarily unauthenticated (browsers send
reports with no session), so it is bounded on every axis: 16 KB body cap, per-IP rate limit, fixed
field allowlist, every logged field truncated, and **always 204 — even for malformed input**, since a
4xx would make the global error handler log the attacker-supplied body and turn an open endpoint into
a log-flood primitive. Mounted outside `/api/v1` next to `/api/health`: the browser learns the path
from the policy header, never from a client build, so it is not part of the versioned client
contract.
- **Admin not indexed**: `X-Robots-Tag: noindex, nofollow` on `/api/v1/admin` and the admin SPA routes; `robots.txt` disallows `/admin`. - **Admin not indexed**: `X-Robots-Tag: noindex, nofollow` on `/api/v1/admin` and the admin SPA routes; `robots.txt` disallows `/admin`.
- **No directory browsing** (express.static doesn't list; no `serve-index`). - **No directory browsing** (express.static doesn't list; no `serve-index`).
- **No hardcoded credentials**: first admin via `seed.js` reading `ADMIN_USERNAME`/`ADMIN_PASSWORD` from env (created only if no users exist); `.env` git-ignored, `.env.example` committed. - **No hardcoded credentials**: first admin via `seed.js` reading `ADMIN_USERNAME`/`ADMIN_PASSWORD` from env (created only if no users exist); `.env` git-ignored, `.env.example` committed.
@@ -527,7 +550,11 @@ instead. Errors never leak credentials.
`utils/logger.js` — a small dependency-free logger with **two transports, console + file**, `utils/logger.js` — a small dependency-free logger with **two transports, console + file**,
and four levels (`error`/`warn`/`info`/`debug`). Each line is timestamped and tagged by and four levels (`error`/`warn`/`info`/`debug`). Each line is timestamped and tagged by
subsystem (`[server]`, `[http]`, `[db]`, `[auth]`, `[admin]`, `[ratelimit]`, …). subsystem (`[server]`, `[http]`, `[db]`, `[auth]`, `[admin]`, `[ratelimit]`, `[csp]`, …).
> During the CSP report-only soak, `[csp]` is the tag to watch: a `csp violation` warn line with
> `directive: frame-ancestors` means something really does frame the site and the enforce PR would
> break it. Silence across one release is the green light to flip.
- **Console**: color on a TTY, plain in Docker; verbosity = `LOG_LEVEL` (default `info`). - **Console**: color on a TTY, plain in Docker; verbosity = `LOG_LEVEL` (default `info`).
- **File**: plain text appended to `LOG_DIR/LOG_FILE` (default `<server>/logs/app.log`, - **File**: plain text appended to `LOG_DIR/LOG_FILE` (default `<server>/logs/app.log`,

View File

@@ -5,6 +5,10 @@
"method": "GET", "method": "GET",
"path": "/.well-known/assetlinks.json" "path": "/.well-known/assetlinks.json"
}, },
{
"method": "POST",
"path": "/api/csp-report"
},
{ {
"method": "GET", "method": "GET",
"path": "/api/docs.json" "path": "/api/docs.json"