From 1223b381856fd7266e01d0d5b2bfa5c5a4a16157 Mon Sep 17 00:00:00 2001 From: Adam Spiers Date: Mon, 20 Apr 2026 21:13:41 +0000 Subject: [PATCH 1/8] feat(auth-service): nonce-based CSP and deny-by-default /metrics Replace the auth service's Content-Security-Policy script-src 'unsafe-inline' with a per-response nonce. The security-headers middleware now generates a fresh base64url nonce on every request, stamps it into script-src, and exposes it via res.locals.cspNonce so templates can emit ` +` + +/** + * Inline ` +} + +/** + * Back-compat: the no-nonce variant of the inline script tag. Callers on + * services that set a CSP with `script-src 'nonce-...'` must use + * {@link previewClientIdScriptHtml} instead, passing the request nonce. + */ +export const PREVIEW_CLIENT_ID_SCRIPT_HTML = previewClientIdScriptHtml() From 5b31f3b0cab7840824c5c0013e98161fe411cfcb Mon Sep 17 00:00:00 2001 From: Adam Spiers Date: Tue, 21 Apr 2026 22:27:18 +0000 Subject: [PATCH 2/8] fix(auth-service): review follow-ups on CSP nonce + /metrics auth - Thread cspNonce through 3 missed renderChooseHandlePage call sites (the /_internal/check-handle catch arm in choose-handle.ts, plus both /preview/choose-handle routes). Without the nonce argument the template emits a bare ` } From 9a0c74a249b876eea5325e5e46c6e9652ef5ae5e Mon Sep 17 00:00:00 2001 From: Adam Spiers Date: Tue, 21 Apr 2026 22:38:19 +0000 Subject: [PATCH 4/8] test(auth-service): unit-test /metrics Basic auth check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extract the /metrics auth-check branching into a pure checkMetricsAuth() helper in lib/metrics-auth.ts. The route handler in index.ts becomes a thin wrapper that maps the helper's result onto the Express response. Covers every branch: - adminPassword unset or empty → 401 even with a valid-looking header - adminPassword set, header missing/empty → 401 - wrong password, wrong username, wrong scheme → 401 - length-matched-but-different payload → 401 (regression guard for timingSafeEqual short-circuit behaviour) - matching Basic admin: → ok Every 401 path carries WWW-Authenticate: Basic realm="metrics". Co-Authored-By: Claude Opus 4.7 --- .../src/__tests__/metrics-auth.test.ts | 76 +++++++++++++++++++ packages/auth-service/src/index.ts | 36 +++------ packages/auth-service/src/lib/metrics-auth.ts | 58 ++++++++++++++ 3 files changed, 144 insertions(+), 26 deletions(-) create mode 100644 packages/auth-service/src/__tests__/metrics-auth.test.ts create mode 100644 packages/auth-service/src/lib/metrics-auth.ts diff --git a/packages/auth-service/src/__tests__/metrics-auth.test.ts b/packages/auth-service/src/__tests__/metrics-auth.test.ts new file mode 100644 index 00000000..e0d517ae --- /dev/null +++ b/packages/auth-service/src/__tests__/metrics-auth.test.ts @@ -0,0 +1,76 @@ +import { describe, it, expect } from 'vitest' +import { checkMetricsAuth } from '../lib/metrics-auth.js' + +function basic(user: string, password: string): string { + return 'Basic ' + Buffer.from(`${user}:${password}`).toString('base64') +} + +describe('checkMetricsAuth', () => { + describe('deny-by-default (no admin password configured)', () => { + it('returns 401 when adminPassword is undefined, even with a header', () => { + const r = checkMetricsAuth(basic('admin', 'whatever'), undefined) + expect(r.ok).toBe(false) + if (!r.ok) { + expect(r.status).toBe(401) + expect(r.headers['WWW-Authenticate']).toBe('Basic realm="metrics"') + expect(r.body).toEqual({ error: 'Unauthorized' }) + } + }) + + it('returns 401 when adminPassword is an empty string', () => { + const r = checkMetricsAuth(basic('admin', 'whatever'), '') + expect(r.ok).toBe(false) + }) + + it('returns 401 when both adminPassword and header are missing', () => { + const r = checkMetricsAuth(undefined, undefined) + expect(r.ok).toBe(false) + }) + }) + + describe('password configured', () => { + const adminPassword = 'supersecret' + + it('returns 401 when Authorization header is missing', () => { + const r = checkMetricsAuth(undefined, adminPassword) + expect(r.ok).toBe(false) + if (!r.ok) { + expect(r.headers['WWW-Authenticate']).toBe('Basic realm="metrics"') + } + }) + + it('returns 401 when Authorization header is the empty string', () => { + const r = checkMetricsAuth('', adminPassword) + expect(r.ok).toBe(false) + }) + + it('returns 401 when the password is wrong', () => { + const r = checkMetricsAuth(basic('admin', 'wrong'), adminPassword) + expect(r.ok).toBe(false) + }) + + it('returns 401 when the username is not "admin"', () => { + const r = checkMetricsAuth(basic('root', adminPassword), adminPassword) + expect(r.ok).toBe(false) + }) + + it('returns 401 when the header uses a different scheme (Bearer)', () => { + const r = checkMetricsAuth(`Bearer ${adminPassword}`, adminPassword) + expect(r.ok).toBe(false) + }) + + it('accepts a correctly formed Basic header with matching password', () => { + const r = checkMetricsAuth(basic('admin', adminPassword), adminPassword) + expect(r.ok).toBe(true) + }) + + it('rejects a header whose length happens to match the expected value', () => { + // Guard against a regression that would short-circuit timingSafeEqual + // when lengths differ. Use a same-length but different payload. + const expected = basic('admin', adminPassword) + const samelengthWrong = 'X'.repeat(expected.length) + const r = checkMetricsAuth(samelengthWrong, adminPassword) + expect(r.ok).toBe(false) + }) + }) +}) diff --git a/packages/auth-service/src/index.ts b/packages/auth-service/src/index.ts index b416bec3..cc5f95f5 100644 --- a/packages/auth-service/src/index.ts +++ b/packages/auth-service/src/index.ts @@ -1,8 +1,4 @@ -import { - createLogger, - getEpdsVersion, - timingSafeEqual, -} from '@certified-app/shared' +import { createLogger, getEpdsVersion } from '@certified-app/shared' import express from 'express' import cookieParser from 'cookie-parser' import * as path from 'node:path' @@ -25,6 +21,7 @@ import { createRootRouter } from './routes/root.js' import { createTestHooksRouter } from './routes/test-hooks.js' import { resolveAuthPort } from './lib/resolve-port.js' import { createSecurityHeadersMiddleware } from './lib/security-headers.js' +import { checkMetricsAuth } from './lib/metrics-auth.js' import { validateOtpCharset, validateOtpLength, @@ -101,28 +98,15 @@ export function createAuthService(config: AuthServiceConfig): { app.use(createPreviewRouter(ctx)) app.use(createPreviewEmailsRouter(ctx)) - // Metrics endpoint (protect with admin auth in production) + // Metrics endpoint — HTTP Basic auth, deny-by-default. See + // packages/auth-service/src/lib/metrics-auth.ts for the rules. app.get('/metrics', (req, res) => { - // Metrics expose process-level signal (uptime, RSS, DB counts) that - // we don't want leaking unauthenticated. Deny-by-default: if no - // admin password is configured, the endpoint is unavailable rather - // than open. - const adminPassword = process.env.PDS_ADMIN_PASSWORD - if (!adminPassword) { - res - .set('WWW-Authenticate', 'Basic realm="metrics"') - .status(401) - .json({ error: 'Unauthorized' }) - return - } - const authHeader = req.headers.authorization - const expected = - 'Basic ' + Buffer.from('admin:' + adminPassword).toString('base64') - if (!authHeader || !timingSafeEqual(authHeader, expected)) { - res - .set('WWW-Authenticate', 'Basic realm="metrics"') - .status(401) - .json({ error: 'Unauthorized' }) + const auth = checkMetricsAuth( + req.headers.authorization, + process.env.PDS_ADMIN_PASSWORD, + ) + if (!auth.ok) { + res.set(auth.headers).status(auth.status).json(auth.body) return } const metrics = ctx.db.getMetrics() diff --git a/packages/auth-service/src/lib/metrics-auth.ts b/packages/auth-service/src/lib/metrics-auth.ts new file mode 100644 index 00000000..449d23b5 --- /dev/null +++ b/packages/auth-service/src/lib/metrics-auth.ts @@ -0,0 +1,58 @@ +/** + * HTTP Basic authentication check for the /metrics endpoint. + * + * Deny-by-default: if `PDS_ADMIN_PASSWORD` is unset the endpoint is + * unavailable rather than open — historically an unset password meant + * "no auth required", which leaked process uptime, RSS, and DB + * counters. Operators who want to scrape /metrics must set + * `PDS_ADMIN_PASSWORD` and send HTTP Basic auth as `admin:`. + * + * Kept as a pure helper so the wiring in the /metrics handler is a + * thin wrapper and the branching logic is unit-testable without + * spinning up Express. + */ +import { timingSafeEqual } from '@certified-app/shared' + +/** + * Result of the /metrics Basic-auth check. When `ok` is false the + * caller should return a 401 with both `headers` set (for RFC 7235 + * `WWW-Authenticate`) and `body` as the JSON payload. + */ +export type MetricsAuthResult = + | { ok: true } + | { + ok: false + status: 401 + headers: { 'WWW-Authenticate': string } + body: { error: string } + } + +const UNAUTHORIZED: Extract = { + ok: false, + status: 401, + headers: { 'WWW-Authenticate': 'Basic realm="metrics"' }, + body: { error: 'Unauthorized' }, +} + +/** + * Validate the `Authorization` header for a /metrics request against + * the configured admin password. + * + * - `adminPassword` unset/empty → always 401 (deny-by-default) + * - Header missing → 401 + * - Header present but does not match `Basic )>` → + * 401 (comparison uses `timingSafeEqual` to avoid leaking the + * password byte-by-byte) + * - Header matches → `{ ok: true }` + */ +export function checkMetricsAuth( + authHeader: string | undefined, + adminPassword: string | undefined, +): MetricsAuthResult { + if (!adminPassword) return UNAUTHORIZED + if (!authHeader) return UNAUTHORIZED + const expected = + 'Basic ' + Buffer.from('admin:' + adminPassword).toString('base64') + if (!timingSafeEqual(authHeader, expected)) return UNAUTHORIZED + return { ok: true } +} From 43ff66562053497cd32084e91b71ae6c82376d0f Mon Sep 17 00:00:00 2001 From: Adam Spiers Date: Tue, 21 Apr 2026 22:52:18 +0000 Subject: [PATCH 5/8] docs(shared): clarify previewClientIdScriptHtml no-nonce branch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The existing docstring claimed pds-core "doesn't set a CSP on preview pages", which was misleading — pds-core's /preview/consent route does set a CSP, it just uses 'unsafe-inline' rather than a nonce. Reword so the no-nonce branch is explicitly described as for CSPs that allow inline scripts without a nonce. Co-Authored-By: Claude Opus 4.7 --- packages/shared/src/preview-ui.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/shared/src/preview-ui.ts b/packages/shared/src/preview-ui.ts index c61ebb8b..883591ae 100644 --- a/packages/shared/src/preview-ui.ts +++ b/packages/shared/src/preview-ui.ts @@ -696,8 +696,9 @@ const PREVIEW_CLIENT_ID_SCRIPT_BODY = `(function () { /** * Inline