fix(web): replace the CSP unsafe-inline script policy with a per-request nonce (#936)

[frankbria]

Closes #936.

The problem, in the file's own words

Production shipped script-src 'self' 'unsafe-inline', and security-headers.js conceded what that meant:

with the JWT in localStorage and inline scripts allowed, an injected inline script can read the token. connect-src/img-src/object-src below close the fetch/XHR/img/plugin exfil channels, but NOT top-level navigation (window.location = attacker + token) — CSP has no deployable navigate-to directive, so that channel stays open until the real fix.

Closing exfiltration channels one at a time is a losing game once arbitrary script is already running. A nonce fixes the cause: an injected inline script has no nonce, so it never executes.

What changed

• src/proxy.ts mints a nonce per request (crypto.getRandomValues, Edge-runtime safe) and sets the CSP on both the request headers — which is how Next.js knows to stamp the nonce onto its own bootstrap scripts — and the response headers, which is what the browser enforces.
• buildScriptSrc() emits 'self' 'nonce-<n>' 'strict-dynamic', with no 'unsafe-inline'. unsafe-eval remains dev-only.
• securityHeaders() no longer carries a CSP. Two CSP headers on one response are intersected by the browser, so leaving a static nonce-less policy alongside the proxy's would have blocked every script.
• layout.tsx sets export const dynamic = 'force-dynamic' — see below.

style-src deliberately keeps 'unsafe-inline': Tailwind and React set style attributes directly, and a stolen style is not a stolen session. The threat here is script execution. A test pins that decision so changing it has to be deliberate.

The part the build does not catch

Next.js can only stamp a per-request nonce onto a document it renders at request time. A statically prerendered page keeps whatever was baked in at build time — nothing.

Measured against a real next start, before adding force-dynamic:

Route	Rendering	Scripts carrying the nonce
/login	○ static	0 / 21 → blank page
/sessions/[id]	ƒ dynamic	22 / 22 ✅

The build succeeded in both cases. Shipping the proxy without force-dynamic would have produced a white screen on 14 of 17 routes with a green CI.

After force-dynamic, every route is ƒ and every script and preload is nonced:

/login     scripts=20  nonced=22
/tasks     scripts=21  nonced=23
/settings  scripts=20  nonced=22

(Nonced exceeds the <script count because Next stamps preload <link> tags too.)

Distinct nonce per request, confirmed on two consecutive requests:

script-src 'self' 'nonce-3OYI/yCrBNhOtpIzsF/NIg==' 'strict-dynamic'
script-src 'self' 'nonce-PgTBsISXjlWvMYjNLQKOQg==' 'strict-dynamic'

Cost of force-dynamic

These pages are 'use client' shells — the static prerender was an empty loading skeleton, and the data is fetched client-side either way. JS/CSS chunks are still served statically from /_next/static, which the proxy matcher skips. So the cost is rendering an empty shell per document request, not per-request data work.

If that trade is unwanted, the AC's other option (moving the token to an httpOnly cookie) removes the need for the nonce entirely — but it is a much larger change: login response, axios interceptor, SSE stream tickets, WebSocket auth, the CLI API client, and CSRF protection.

Acceptance criteria

• Production CSP no longer contains 'unsafe-inline' in script-src (per-request nonce middleware)
• A CI check asserts the production CSP string has no 'unsafe-inline' in script-src — security-headers.test.js, which the frontend-tests job runs
• The residual-risk comment reflects the shipped mitigation

Tests

15 CSP tests. Two guard the coupling that the build cannot: the root layout must keep force-dynamic, and proxy.ts must set the CSP on both header sets. One pre-existing test asserted the static header list contained a CSP; it is updated with a comment explaining why the CSP moved.

Full web-ui suite: 1083 passed across 96 suites. npm run build clean.

Known limitations

• style-src 'unsafe-inline' stays, as above.
• force-dynamic is applied at the root layout, so it covers every route. A per-route opt-in would preserve static generation for any page that genuinely does not need the nonce — but every page in this app serves the authenticated shell, so there is nothing to exempt today.
• The nonce is not threaded to next/script or any inline <script> a future component might add; those would need nonce={await headers().get('x-nonce')}. The proxy already exposes it as x-nonce for that purpose.

[github-actions]

GLM Precision Review — CSP Nonce (#936)

Bug-hunting review in progress. Posting findings as inline comments + a summary comment when done.

• Gather PR diff + existing comments
• Read changed files + callers/callees
• Analyze for concrete defects (logic, security, race, contract)
• Post inline findings
• Post summary comment

View job run

[frankbria]

Third-party review — codex review --base main

Two P2 findings, both real, both fixed in 414b50a. Root cause: I ran jest and npm run build but not npm run lint, which is a blocking gate here (eslint . --max-warnings 0).

1. require() in proxy.ts

@typescript-eslint/no-require-imports forbids it. Switched to an ESM import. security-headers.js stays CommonJS on purpose — next.config.js require()s it and the file is documented as dependency-free — so I added security-headers.d.ts to give the ESM import proper types rather than casting it away.

2. Unused env parameter on securityHeaders()

Correct — once the CSP moved out to the proxy, the function stopped reading env. Dropped the parameter.

And a third, found by actually running the gate

Fixing those two and running npm run lint properly surfaced the same rule violated in my own test file (require('fs'), require('path')). Converted to ESM imports.

npm run lint   → exit 0
jest           → 15 passed
npm run build  → clean, 14 dynamic routes

Re-verified against a live next start after the ESM switch, since an import change could plausibly have broken the nonce plumbing:

nonce: JtUtZUEQfxmyTQsZFmyMrw==
nonced elements on /login: 22
unsafe-inline in script-src: 0

[github-actions]

Claude finished @frankbria's task in 4m 8s —— View job

---

Claude Precision Review — CSP Nonce (#936)

Bug-hunting review complete. Scope: logic errors, security, races, contract breaks. (Style/coverage are out of scope — a separate general review covers those.)

• Gather PR diff + existing comments
• Read changed files + callers/callees
• Analyze for concrete defects (logic, security, race, contract)
• Post inline findings
• Post summary

✅ No defects found.

What I verified (and one thing that looks like a bug but isn't)

• src/proxy.ts IS the correct Next.js 16 convention. Next.js 16 renamed middleware → proxy (v16.0.0 changelog). The file must be proxy.{ts,js} in root or src/ exporting a function named proxy (or default) — exactly what this PR ships. (src/proxy.ts is the sole convention file; there is no middleware.ts, and none is needed.) Flagging "wrong filename" here would be a false positive.
• Nonce entropy: 16 random bytes (128 bits) from crypto.getRandomValues, base64'd via btoa(String.fromCharCode(...bytes)). fromCharCode of bytes 0–255 yields a valid Latin-1 string, so btoa cannot throw. Per-request, no shared mutable state — no race.
• Both header sets set: Content-Security-Policy + x-nonce on the request (Next.js reads x-nonce to auto-stamp its bootstrap scripts) and Content-Security-Policy on the response (browser enforcement). The two-CSP-header-intersection failure mode is correctly avoided by dropping CSP from securityHeaders().
• force-dynamic coupling is real: without it, a static prerender carries no nonce → with 'strict-dynamic' and no 'unsafe-inline', blank page. The build doesn't catch this; the fs-based test does. Sound.
• Hardening headers still cover everything: next.config.js headers() still applies securityHeaders() (now non-CSP: X-Content-Type-Options, X-Frame-Options, Referrer-Policy) to /:path*, including the proxy-excluded paths (api, _next/static, …).
• Matcher: the negative-lookahead + dual missing (prefetch skip) is the documented Next.js pattern; prefetches are RSC/flight, not HTML documents, so skipping them loses no nonce coverage.

Notes (not defects)

• The nonce-less branch of buildScriptSrc (keeps 'unsafe-inline') is unreachable in production — buildCsp is only called from proxy.ts with a nonce; securityHeaders() no longer calls buildCsp. It survives as the intentional non-document fallback and is only exercised by tests.
• /auth/* requests are matched by the proxy (not excluded) and rewritten to FastAPI, so they receive x-nonce/Content-Security-Policy as request headers. Harmless — FastAPI ignores them and the responses aren't HTML.

No inline comments posted. No files modified, no commits pushed (review-only).