Merge pull request 'feat(security): soak the tightened CSP on report-only, with a same-origin sink' (#100) from feature/csp-report-only into main

Reviewed-on: #100
Reviewed-by: Colby Whitlock <whitlocktech@gmail.com>

server/routes.guards.json

@@ -7,6 +7,14 @@
       "handlers": 1,
       "gates": []
     },
+    {
+      "method": "POST",
+      "path": "/api/csp-report",
+      "handlers": 6,
+      "gates": [
+        "jsonParser"
+      ]
+    },
     {
       "method": "GET",
       "path": "/api/docs.json",

server/routes.manifest.json

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

server/src/app.js

@@ -11,7 +11,10 @@ const swaggerUi = require('swagger-ui-express')
 
 const apiRouter = require('./router/api.router')
 const wellKnown = require('./router/wellKnown.controller')
+const cspReport = require('./router/cspReport.controller')
 const brand = require('./config/brand')
+const csp = require('./config/csp')
+const { cspReportLimiter } = require('./middleware/rateLimit')
 const createLogger = require('./utils/logger')
 const { applyTrustProxy, trustProxyDebug } = require('./utils/trustProxy')
 const botScore = require('./middleware/botScore')
@@ -37,41 +40,31 @@ app.use(trustProxyDebug)
 app.use(botScore.guard)
 
 // Security headers, including a Content-Security-Policy tuned for the built React
-// SPA. Notes on each non-'self' allowance:
+// SPA. The policies themselves (and the reasoning behind every non-'self' allowance)
-//   • style-src 'unsafe-inline' — React renders pervasive inline `style={{…}}`
+// live in config/csp.js. The interactive API docs at /api/docs get their own looser
-//     attributes, and CSP style *attributes* cannot be nonce'd; this is required.
+// policy below.
-//     Also whitelists the Google Fonts stylesheet host.
-//   • font-src — Google Fonts (Cinzel) serves the font files from gstatic.
-//   • img-src https:/data: — uploaded images are same-origin, but wiki/news bodies
-//     (sanitizeHtml allows <img> over http/https) and BRAND_* logo/hero/favicon may
-//     point at external https images. http images are blocked by mixed-content on
-//     the https site anyway.
-//   • connect-src 'self' — the REST API and SSE streams are same-origin.
-//   • upgrade-insecure-requests is intentionally dropped: TLS is terminated at the
-//     proxy, there are no mixed-content subresources to upgrade, and leaving it on
-//     breaks a local `npm start` served over plain http.
-// The interactive API docs at /api/docs get their own looser policy below.
 app.use(
   helmet({
-    contentSecurityPolicy: {
+    contentSecurityPolicy: { useDefaults: true, directives: csp.enforced },
-      useDefaults: true,
-      directives: {
-        'default-src': ["'self'"],
-        'script-src': ["'self'"],
-        'style-src': ["'self'", "'unsafe-inline'", 'https://fonts.googleapis.com'],
-        'font-src': ["'self'", 'https://fonts.gstatic.com'],
-        'img-src': ["'self'", 'data:', 'https:'],
-        'connect-src': ["'self'"],
-        'frame-ancestors': ["'self'"],
-        'object-src': ["'none'"],
-        'base-uri': ["'self'"],
-        'upgrade-insecure-requests': null,
-      },
-    },
     crossOriginResourcePolicy: { policy: 'cross-origin' },
   }),
 )
 
+// The tightened policy rides alongside on Content-Security-Policy-Report-Only for one
+// release, then replaces the enforced one (docs/website/API_V2_PLAN.md § Phase 1).
+// Both headers are served at once on purpose: the live policy keeps protecting users
+// while anything the tightened version would have broken shows up as a report at
+// /api/csp-report instead of as a broken page. Reports are same-origin — they
+// describe attacks on this site and are not handed to a third party.
+app.use(csp.reportingEndpoints)
+app.use(
+  helmet.contentSecurityPolicy({
+    useDefaults: true,
+    reportOnly: true,
+    directives: csp.reportOnly,
+  }),
+)
+
 // CORS only when a separate client origin is configured (local Vite dev). In
 // production the SPA is same-origin, so no CORS is needed.
 if (process.env.CLIENT_ORIGIN) {
@@ -179,6 +172,11 @@ app.get(
   /* #swagger.responses[200] = { description: 'Service is up', content: { "application/json": { schema: { type: "object", properties: { status: { type: "string", example: "ok" } } } } } } */
   (req, res) => res.json({ status: 'ok' }),
 )
+// CSP violation sink. Mounted here, ahead of the /api 404, and outside /api/v1: it is
+// not part of the versioned client contract — it exists for the browser, which learns
+// the path from the policy header, never from a client build.
+app.post(csp.REPORT_PATH, cspReportLimiter, ...cspReport.parsers, cspReport.receive)
+
 app.use('/api', apiRouter)
 app.use('/api', (req, res) => res.status(404).json({ message: 'Not found' }))
 

server/src/config/csp.js

@@ -0,0 +1,101 @@
+// ── Content-Security-Policy ────────────────────────────────────────────────
+//
+// Two policies ship at once, on two different headers:
+//
+//   Content-Security-Policy              → `enforced`   (today's policy, unchanged)
+//   Content-Security-Policy-Report-Only  → `reportOnly` (the target, + a report sink)
+//
+// Report-only first, one release of observation, then the two collapse into one
+// enforced policy (docs/website/API_V2_PLAN.md § Phase 1). Shipping the tightened
+// policy straight to `Content-Security-Policy` would mean discovering any legitimate
+// use we forgot as a broken page in production; shipping it *alongside* the current
+// one means a violation report instead, with the live policy still protecting users
+// the whole time.
+//
+// Notes on each non-'self' allowance in the base policy:
+//   • style-src 'unsafe-inline' — React renders pervasive inline `style={{…}}`
+//     attributes, and CSP style *attributes* cannot be nonce'd; this is required.
+//     It permits inline styling, not script execution. Also whitelists the Google
+//     Fonts stylesheet host.
+//   • font-src — Google Fonts (Cinzel) serves the font files from gstatic.
+//   • img-src https:/data: — uploaded images are same-origin, but wiki/news bodies
+//     (sanitizeHtml allows <img> over http/https) and BRAND_* logo/hero/favicon may
+//     point at external https images. http images are blocked by mixed-content on
+//     the https site anyway.
+//   • connect-src 'self' — the REST API and SSE streams are same-origin. This is the
+//     exfiltration channel; do not widen it unless the API genuinely becomes
+//     cross-origin (which would also reopen the auth-merge question — see the plan).
+//   • script-src 'self' with no 'unsafe-inline'/'unsafe-eval' is the primary defense.
+//     Vite is configured with `modulePreload: { polyfill: false }` (client/vite.config.js)
+//     precisely so the build emits no inline bootstrap script for this to trip on.
+//   • upgrade-insecure-requests is intentionally dropped: TLS is terminated at the
+//     proxy, there are no mixed-content subresources to upgrade, and leaving it on
+//     breaks a local `npm start` served over plain http.
+//
+// The interactive API docs at /api/docs get their own looser policy (swagger-ui
+// injects an inline bootstrap script); that carve-out lives in app.js and stays
+// scoped to the one route.
+
+// Where violation reports are POSTed, and the Reporting-API group name that points
+// at it. Same-origin on purpose — reports describe attacks against this site and
+// must not be shipped to a third party.
+const REPORT_PATH = '/api/csp-report'
+const REPORT_GROUP = 'csp-endpoint'
+
+// The policy in force today. Behaviourally unchanged by this phase — it is the safety
+// net while the tightened twin is only being observed.
+const enforced = {
+  'default-src': ["'self'"],
+  'script-src': ["'self'"],
+  'style-src': ["'self'", "'unsafe-inline'", 'https://fonts.googleapis.com'],
+  'font-src': ["'self'", 'https://fonts.gstatic.com'],
+  'img-src': ["'self'", 'data:', 'https:'],
+  'connect-src': ["'self'"],
+  'frame-ancestors': ["'self'"],
+  'object-src': ["'none'"],
+  'base-uri': ["'self'"],
+  // Blocks an injected `<form action="https://evil">` from POSTing credentials
+  // off-origin — an exfil path connect-src does not cover. Already emitted today via
+  // helmet's `useDefaults`, and pinned here on purpose: a security directive should
+  // not depend on a third-party library's default surviving its next major version.
+  // Adding it changes the header's *contents* not at all.
+  'form-action': ["'self'"],
+  'upgrade-insecure-requests': null,
+}
+
+// The one directive this phase actually changes, and so the only thing a report can
+// legitimately be about:
+//
+//   • frame-ancestors 'self' → 'none'. Nothing legitimately frames the site, and
+//     'self' only means anything if some same-origin page frames another; none does.
+//     Worth a soak rather than a straight flip precisely because a violation report
+//     is how we would find out that something *does* — the report comes from the
+//     browser of whoever framed us, which is information we cannot get any other way.
+//
+// Derived from `enforced` rather than written out again, so the two policies cannot
+// silently drift apart and this object stays a readable diff of the change.
+const tightened = {
+  ...enforced,
+  'frame-ancestors': ["'none'"],
+}
+
+const reportOnly = {
+  ...tightened,
+  // Both mechanisms, deliberately: `report-to` is the current Reporting API (Chrome,
+  // needs the Reporting-Endpoints header below), `report-uri` is deprecated but is
+  // still the only one Firefox and Safari implement. Browsers that support both send
+  // one report, not two.
+  'report-to': [REPORT_GROUP],
+  'report-uri': [REPORT_PATH],
+}
+
+/**
+ * Names the Reporting-API group that `report-to` refers to. Without this header the
+ * `report-to` directive is inert, and only the `report-uri` fallback would fire.
+ */
+function reportingEndpoints(req, res, next) {
+  res.setHeader('Reporting-Endpoints', `${REPORT_GROUP}="${REPORT_PATH}"`)
+  next()
+}
+
+module.exports = { enforced, tightened, reportOnly, reportingEndpoints, REPORT_PATH, REPORT_GROUP }

server/src/middleware/rateLimit.js

@@ -118,6 +118,18 @@ const passwordResetConfirmLimiter = makeLimiter({
   message: 'Too many attempts. Please try again later.',
 })
 
+// CSP violation reports. Unauthenticated by necessity (browsers send them with no
+// session), and every accepted report writes a log line — so an attacker who can get
+// a victim to load a page could otherwise use it as a log-flood amplifier. Generous
+// enough for the real case: a genuinely broken directive fires a handful of times per
+// page load, and browsers already de-duplicate identical violations per document.
+const cspReportLimiter = makeLimiter({
+  windowMs: 5 * 60 * 1000,
+  max: 60,
+  label: 'csp-report',
+  message: 'Too many reports.',
+})
+
 module.exports = {
   loginLimiter,
   registerLimiter,
@@ -129,4 +141,5 @@ module.exports = {
   mobileSsoExchangeLimiter,
   passwordResetRequestLimiter,
   passwordResetConfirmLimiter,
+  cspReportLimiter,
 }

server/src/router/cspReport.controller.js

@@ -0,0 +1,119 @@
+// ── POST /api/csp-report — Content-Security-Policy violation sink ─────────────
+//
+// The target policy ships on Content-Security-Policy-Report-Only for one release
+// before it is enforced (docs/website/API_V2_PLAN.md § Phase 1). That soak is only
+// worth anything if the reports land somewhere a human reads, so `report-to` /
+// `report-uri` point here (see config/csp.js) and this writes them to the `csp` log
+// tag. It is deliberately same-origin: reports describe attacks against this site
+// and must not be handed to a third-party collector.
+//
+// This is an unauthenticated public POST — browsers send reports with no session and
+// no CSRF token, and gating it would silence exactly the anonymous visitors whose
+// pages we most want to hear about. So treat every field as hostile:
+//   • the body is parsed under a small cap (browsers send a few KB),
+//   • the rate limiter blunts a flood, since each accepted report writes a log line,
+//   • every logged field is truncated, and only a fixed allowlist of fields is read.
+// Nothing is echoed back and nothing is persisted to the database.
+//
+// Retiring this: when the tightened policy flips to enforced and the report-only
+// twin is removed, this endpoint goes with it — unless a `report-to` group is kept
+// on the enforced policy, which is a reasonable thing to want.
+
+const express = require('express')
+
+const log = require('../utils/logger')('csp')
+
+// Browsers send a few KB at most. A cap this low means a junk POST is rejected by
+// the parser before any of this code runs.
+const BODY_LIMIT = '16kb'
+
+// Keep log lines bounded: `script-sample` in particular is attacker-influenced and
+// can carry a whole inline script.
+const MAX_FIELD = 200
+const clip = (value) => {
+  if (value == null) return undefined
+  const s = String(value)
+  return s.length > MAX_FIELD ? `${s.slice(0, MAX_FIELD)}…` : s
+}
+
+/**
+ * Normalize the two wire formats into one shape.
+ *
+ * `report-uri` (Firefox, Safari) POSTs `application/csp-report` with a single
+ * `{ "csp-report": { … } }` object and hyphenated keys. `report-to` (Chrome) POSTs
+ * `application/reports+json` with an *array* of envelopes whose `body` uses camelCase
+ * keys. Reading only one of them would silently drop half the browsers.
+ */
+function normalize(body) {
+  if (Array.isArray(body)) {
+    return body
+      .filter((entry) => entry && entry.type === 'csp-violation' && entry.body)
+      .map((entry) => ({
+        documentUrl: entry.body.documentURL || entry.url,
+        directive: entry.body.effectiveDirective,
+        blockedUrl: entry.body.blockedURL,
+        disposition: entry.body.disposition,
+        sample: entry.body.sample,
+      }))
+  }
+  if (body && typeof body === 'object' && body['csp-report']) {
+    const r = body['csp-report']
+    return [
+      {
+        documentUrl: r['document-uri'],
+        directive: r['effective-directive'] || r['violated-directive'],
+        blockedUrl: r['blocked-uri'],
+        disposition: r.disposition,
+        sample: r['script-sample'],
+      },
+    ]
+  }
+  return []
+}
+
+// POST /api/csp-report
+function receive(req, res) {
+  /* #swagger.tags = ['Health']
+     #swagger.summary = 'Content-Security-Policy violation report sink'
+     #swagger.description = 'Receives CSP violation reports from browsers (both the `report-uri` `application/csp-report` format and the Reporting API `application/reports+json` format). Unauthenticated by necessity — browsers send reports with no session. Reports are logged, never stored or echoed. Always answers 204.'
+     #swagger.security = []
+     #swagger.responses[204] = { description: 'Report accepted (or ignored). No content.' }
+     #swagger.responses[429] = { description: 'Too many reports from this address.' }
+  */
+
+  // 204 regardless of what arrived. A browser cannot act on an error here, and a
+  // non-2xx would only make it retry or log noise in the user's console.
+  for (const v of normalize(req.body)) {
+    if (!v.directive) continue
+    log.warn('csp violation', {
+      // 'report' = the report-only policy fired (expected during the soak);
+      // 'enforce' = the live policy actually blocked something.
+      disposition: clip(v.disposition) || 'report',
+      directive: clip(v.directive),
+      blocked: clip(v.blockedUrl),
+      document: clip(v.documentUrl),
+      sample: clip(v.sample),
+      ip: req.ip,
+    })
+  }
+  res.status(204).end()
+}
+
+/**
+ * The full middleware chain for the endpoint. Both content types get their own
+ * parser because express.json() only matches `application/json` by default, and a
+ * report that arrives unparsed is a report silently discarded.
+ */
+const parsers = [
+  express.json({ limit: BODY_LIMIT, type: 'application/csp-report' }),
+  express.json({ limit: BODY_LIMIT, type: 'application/reports+json' }),
+  express.json({ limit: BODY_LIMIT }),
+  // Swallow malformed / oversized bodies here rather than letting them reach the
+  // global error handler, which would answer 400 and write an ERROR line quoting the
+  // junk — turning "POST garbage at the open endpoint" into a log-flood primitive.
+  // A browser has nothing useful to do with a 4xx from a report sink anyway.
+  // eslint-disable-next-line no-unused-vars
+  (err, req, res, next) => res.status(204).end(),
+]
+
+module.exports = { receive, parsers, normalize }

server/swagger/swagger-output.json

@@ -125,6 +125,24 @@
         }
       }
     },
+    "/api/csp-report": {
+      "post": {
+        "tags": [
+          "Health"
+        ],
+        "summary": "Content-Security-Policy violation report sink",
+        "description": "Receives CSP violation reports from browsers (both the `report-uri` `application/csp-report` format and the Reporting API `application/reports+json` format). Unauthenticated by necessity — browsers send reports with no session. Reports are logged, never stored or echoed. Always answers 204.",
+        "responses": {
+          "204": {
+            "description": "Report accepted (or ignored). No content."
+          },
+          "429": {
+            "description": "Too many reports from this address."
+          }
+        },
+        "security": []
+      }
+    },
     "/api/v1/auth/login": {
       "post": {
         "tags": [

server/test/csp.test.js

@@ -0,0 +1,171 @@
+// CSP phase 1: the tightened policy rides on Content-Security-Policy-Report-Only for
+// one release before it is enforced (docs/website/API_V2_PLAN.md § Phase 1). The whole
+// point of that soak is that the live policy is untouched, so the first test here
+// pins the enforced header verbatim — if a future edit to config/csp.js changes what
+// users are actually protected by, it fails rather than shipping quietly.
+//
+// Dead-port DB before requiring the app: it pulls in every model, which builds a
+// mariadb pool at require time. No route touched here queries anything.
+process.env.DB_HOST = '127.0.0.1'
+process.env.DB_PORT = '59999'
+
+const { test, after } = require('node:test')
+const assert = require('node:assert/strict')
+
+const app = require('../src/app')
+const csp = require('../src/config/csp')
+const cspReport = require('../src/router/cspReport.controller')
+const db = require('../src/utils/db')
+
+after(() => db.close())
+
+// The exact header served before this phase. Not a restatement of config/csp.js —
+// captured from the running app on main, so it also catches helmet changing its
+// `useDefaults` set underneath us (which is how `form-action` got here in the first
+// place: it was already being emitted, and is now pinned explicitly rather than
+// inherited from a dependency's defaults).
+const ENFORCED_BASELINE =
+  "default-src 'self';script-src 'self';" +
+  "style-src 'self' 'unsafe-inline' https://fonts.googleapis.com;" +
+  "font-src 'self' https://fonts.gstatic.com;img-src 'self' data: https:;" +
+  "connect-src 'self';frame-ancestors 'self';object-src 'none';base-uri 'self';" +
+  "form-action 'self';script-src-attr 'none'"
+
+async function withServer(fn) {
+  const server = await new Promise((resolve) => {
+    const s = app.listen(0, '127.0.0.1', () => resolve(s))
+  })
+  try {
+    return await fn(`http://127.0.0.1:${server.address().port}`)
+  } finally {
+    await new Promise((resolve) => server.close(resolve))
+  }
+}
+
+test('the enforced policy is byte-for-byte what it was before the soak', async () => {
+  const res = await withServer((base) => fetch(`${base}/api/health`))
+  assert.equal(
+    res.headers.get('content-security-policy'),
+    ENFORCED_BASELINE,
+    'Report-only phase must not change what is actually enforced.',
+  )
+})
+
+test('the report-only policy differs from the enforced one only by frame-ancestors', async () => {
+  const res = await withServer((base) => fetch(`${base}/api/health`))
+  const reportOnly = res.headers.get('content-security-policy-report-only')
+  assert.ok(reportOnly, 'Content-Security-Policy-Report-Only must be served')
+
+  assert.match(reportOnly, /frame-ancestors 'none'/)
+  assert.doesNotMatch(reportOnly, /frame-ancestors 'self'/)
+
+  // Strip the report plumbing and the one tightened directive; what is left must be
+  // the live policy. Anything else appearing here would be silently soaking an
+  // untracked change.
+  const stripped = reportOnly
+    .split(';')
+    .filter((d) => !d.startsWith('report-to') && !d.startsWith('report-uri'))
+    .map((d) => (d.startsWith('frame-ancestors') ? "frame-ancestors 'self'" : d))
+    .join(';')
+  assert.equal(stripped, ENFORCED_BASELINE)
+})
+
+test('report-to is backed by a Reporting-Endpoints header, and report-uri by the same path', async () => {
+  // Without the header the report-to directive is inert and only Firefox/Safari's
+  // deprecated report-uri would ever fire — i.e. a soak that silently half-works.
+  const res = await withServer((base) => fetch(`${base}/api/health`))
+  assert.equal(res.headers.get('reporting-endpoints'), `${csp.REPORT_GROUP}="${csp.REPORT_PATH}"`)
+
+  const reportOnly = res.headers.get('content-security-policy-report-only')
+  assert.ok(reportOnly.includes(`report-to ${csp.REPORT_GROUP}`))
+  assert.ok(reportOnly.includes(`report-uri ${csp.REPORT_PATH}`))
+})
+
+test('the sink is same-origin — reports never leave this host', () => {
+  assert.ok(csp.REPORT_PATH.startsWith('/'), 'a third-party collector would receive attack data')
+})
+
+test('the sink accepts both wire formats and always answers 204', async () => {
+  const statuses = await withServer(async (base) => {
+    const post = (type, body) =>
+      fetch(`${base}${csp.REPORT_PATH}`, {
+        method: 'POST',
+        headers: { 'Content-Type': type },
+        body,
+      }).then((r) => r.status)
+
+    return {
+      // Firefox / Safari
+      reportUri: await post(
+        'application/csp-report',
+        JSON.stringify({ 'csp-report': { 'effective-directive': 'frame-ancestors' } }),
+      ),
+      // Chrome's Reporting API
+      reportsJson: await post(
+        'application/reports+json',
+        JSON.stringify([{ type: 'csp-violation', body: { effectiveDirective: 'frame-ancestors' } }]),
+      ),
+      // Junk must not produce a 4xx + an ERROR log line, or the open endpoint becomes
+      // a log-flood primitive.
+      malformed: await post('application/csp-report', 'not json'),
+      oversized: await post(
+        'application/csp-report',
+        JSON.stringify({ 'csp-report': { 'script-sample': 'x'.repeat(40000) } }),
+      ),
+      empty: await post('application/csp-report', ''),
+    }
+  })
+
+  assert.deepEqual(statuses, {
+    reportUri: 204,
+    reportsJson: 204,
+    malformed: 204,
+    oversized: 204,
+    empty: 204,
+  })
+})
+
+test('the sink is POST-only', async () => {
+  const res = await withServer((base) => fetch(`${base}${csp.REPORT_PATH}`))
+  assert.equal(res.status, 404)
+})
+
+test('normalize() reads the hyphenated report-uri body', () => {
+  const [v] = cspReport.normalize({
+    'csp-report': {
+      'document-uri': 'https://site/page',
+      'effective-directive': 'frame-ancestors',
+      'blocked-uri': 'https://evil/',
+      'script-sample': 'alert(1)',
+      disposition: 'report',
+    },
+  })
+  assert.deepEqual(v, {
+    documentUrl: 'https://site/page',
+    directive: 'frame-ancestors',
+    blockedUrl: 'https://evil/',
+    disposition: 'report',
+    sample: 'alert(1)',
+  })
+})
+
+test('normalize() reads the camelCase Reporting-API envelope and drops other report types', () => {
+  const out = cspReport.normalize([
+    {
+      type: 'csp-violation',
+      url: 'https://site/page',
+      body: { effectiveDirective: 'form-action', blockedURL: 'https://evil/' },
+    },
+    // Browsers deliver deprecation/intervention reports to the same group.
+    { type: 'deprecation', url: 'https://site/page', body: { id: 'x' } },
+  ])
+  assert.equal(out.length, 1)
+  assert.equal(out[0].directive, 'form-action')
+  assert.equal(out[0].documentUrl, 'https://site/page')
+})
+
+test('normalize() shrugs at anything else', () => {
+  assert.deepEqual(cspReport.normalize(undefined), [])
+  assert.deepEqual(cspReport.normalize({}), [])
+  assert.deepEqual(cspReport.normalize('nonsense'), [])
+})