Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions .changeset/csp-nonce-and-metrics-auth.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
'ePDS': minor
---

Auth service tightens its Content-Security-Policy and locks down the metrics endpoint.

**Affects:** Operators

**Operators:** the auth service's `Content-Security-Policy` response header now uses a per-response nonce on the `script-src` directive instead of `'unsafe-inline'`. The resulting policy looks like `default-src 'self'; script-src 'self' 'nonce-<base64url>'; style-src 'self' 'unsafe-inline'; img-src 'self' data: [client-origin]; connect-src 'self'`. All inline `<script>` tags that ePDS ships are already stamped with the matching nonce, so there is nothing to do on upgrade — but any operator-supplied HTML overlay or injected script that the auth service happens to serve inline will now be blocked by the browser unless it is updated to read `res.locals.cspNonce` and stamp `<script nonce="...">`. External scripts loaded via `src=` are unaffected.

The `/metrics` endpoint on the auth service is now deny-by-default: if `PDS_ADMIN_PASSWORD` is unset, the endpoint returns `401 Unauthorized` instead of serving metrics unauthenticated. Previously, unset meant "no auth required", which leaked process uptime, RSS memory, and database counters to anyone who could reach the auth service's `/metrics` path. Operators who relied on the open endpoint must set `PDS_ADMIN_PASSWORD` and send HTTP Basic auth as `admin:<password>` to continue scraping.
210 changes: 210 additions & 0 deletions e2e/step-definitions/security.steps.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,210 @@
/**
* Step definitions for security.feature. These scenarios run direct HTTP
* requests (no browser) because they assert on response headers, status
* codes, and raw HTML — not user interaction.
*/

import { When, Then } from '@cucumber/cucumber'
import type { DataTable } from '@cucumber/cucumber'
import type { EpdsWorld } from '../support/world.js'
import { testEnv } from '../support/env.js'

/** Response captured by the most recent direct-fetch step. */
interface CapturedResponse {
status: number
headers: Headers
body: string
}

const capturedBySymbol = new WeakMap<EpdsWorld, CapturedResponse>()

function setCapturedResponse(
world: EpdsWorld,
response: CapturedResponse,
): void {
capturedBySymbol.set(world, response)
world.lastHttpStatus = response.status
}

function getCapturedResponse(world: EpdsWorld): CapturedResponse {
const captured = capturedBySymbol.get(world)
if (!captured) {
throw new Error('No response has been captured by a prior step')
}
return captured
}

async function captureGet(
world: EpdsWorld,
url: string,
init?: RequestInit,
): Promise<void> {
const res = await fetch(url, { redirect: 'manual', ...init })
const body = await res.text()
setCapturedResponse(world, { status: res.status, headers: res.headers, body })
}

// ---------------------------------------------------------------------------
// CSRF scenarios
// ---------------------------------------------------------------------------

When('the recovery page is loaded', async function (this: EpdsWorld) {
const recoveryUrl = `${testEnv.authUrl}/auth/recover?request_uri=urn:ietf:params:oauth:request_uri:req-security-probe`
await captureGet(this, recoveryUrl)
})

Then('the response sets a CSRF cookie', function (this: EpdsWorld) {
const { headers } = getCapturedResponse(this)
const setCookie = headers.get('set-cookie') ?? ''
if (!/epds_csrf=/.test(setCookie)) {
throw new Error(
`Expected Set-Cookie to include epds_csrf=..., got: ${setCookie || '(none)'}`,
)
}
})

Then(
'the HTML form contains a hidden CSRF token field',
function (this: EpdsWorld) {
const { body } = getCapturedResponse(this)
if (!/<input[^>]*type="hidden"[^>]*name="csrf"[^>]*>/.test(body)) {
throw new Error('HTML response has no hidden CSRF input field')
}
},
)

When(
'a POST request is sent to the recovery endpoint without a CSRF token',
async function (this: EpdsWorld) {
const res = await fetch(`${testEnv.authUrl}/auth/recover`, {
method: 'POST',
headers: { 'Content-Type': 'application/x-www-form-urlencoded' },
body: new URLSearchParams({
request_uri: 'urn:ietf:params:oauth:request_uri:req-security-probe',
email: 'noone@example.com',
}).toString(),
redirect: 'manual',
})
const body = await res.text()
setCapturedResponse(this, {
status: res.status,
headers: res.headers,
body,
})
},
)

Then('the response status is {int}', function (this: EpdsWorld, code: number) {
const { status } = getCapturedResponse(this)
if (status !== code) {
throw new Error(`Expected status ${code}, got ${status}`)
}
})

// ---------------------------------------------------------------------------
// Security headers scenario
// ---------------------------------------------------------------------------

When(
'any page is loaded from the auth service',
async function (this: EpdsWorld) {
await captureGet(this, `${testEnv.authUrl}/health`)
},
)

Then(
'the response includes the following security headers:',
function (this: EpdsWorld, table: DataTable) {
const { headers } = getCapturedResponse(this)
const missing: string[] = []
for (const row of table.hashes()) {
const expected = row.value
const actual = headers.get(row.header)
if (actual !== expected) {
missing.push(
`${row.header}: expected "${expected}", got "${actual ?? '(missing)'}"`,
)
}
}
if (missing.length) {
throw new Error(`Security header mismatch:\n ${missing.join('\n ')}`)
}
},
)

// ---------------------------------------------------------------------------
// CSP scenario
// ---------------------------------------------------------------------------

When('the login page is loaded', async function (this: EpdsWorld) {
// /oauth/authorize on the PDS renders the auth-service login page via
// the epds-callback redirect chain, but hitting it without a valid
// request_uri triggers an error response before headers are set the
// way we want. The auth service exposes a preview route that renders
// the same login template, guarded by AUTH_PREVIEW_ROUTES. If preview
// is off, fall back to a probe of any auth-service page — the CSP
// header is applied globally by the security-headers middleware, so
// an auth-service /health response carries the same header.
const previewUrl = `${testEnv.authUrl}/preview/login`
let res = await fetch(previewUrl, { redirect: 'manual' })
if (res.status === 404) {
res = await fetch(`${testEnv.authUrl}/health`, { redirect: 'manual' })
}
const body = await res.text()
setCapturedResponse(this, { status: res.status, headers: res.headers, body })
})

Then(
'the Content-Security-Policy header is present',
function (this: EpdsWorld) {
const { headers } = getCapturedResponse(this)
if (!headers.get('content-security-policy')) {
throw new Error('Content-Security-Policy header is not set')
}
},
)

function getScriptSrcDirective(csp: string): string {
const match = /(?:^|;\s*)script-src\s+([^;]+)/.exec(csp)
if (!match) {
throw new Error(`CSP is missing a script-src directive: "${csp}"`)
}
return match[1].trim()
}

Then(
'the script-src directive does not allow unsafe-inline',
function (this: EpdsWorld) {
const { headers } = getCapturedResponse(this)
const csp = headers.get('content-security-policy') ?? ''
const scriptSrc = getScriptSrcDirective(csp)
if (/'unsafe-inline'/.test(scriptSrc)) {
throw new Error(
`script-src directive allows 'unsafe-inline': "${scriptSrc}"`,
)
}
},
)

Then(
'the script-src directive carries a per-response nonce',
function (this: EpdsWorld) {
const { headers } = getCapturedResponse(this)
const csp = headers.get('content-security-policy') ?? ''
const scriptSrc = getScriptSrcDirective(csp)
if (!/'nonce-[A-Za-z0-9_-]+'/.test(scriptSrc)) {
throw new Error(`script-src directive has no 'nonce-...': "${scriptSrc}"`)
}
},
)

// ---------------------------------------------------------------------------
// Metrics scenario
// ---------------------------------------------------------------------------

When(
'GET \\/metrics is called on the auth service without credentials',

Check warning on line 206 in e2e/step-definitions/security.steps.ts

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

`String.raw` should be used to avoid escaping `\`.

See more on https://sonarcloud.io/project/issues?id=hypercerts-org_ePDS&issues=AZ2svryjMEXeMQGE_I6P&open=AZ2svryjMEXeMQGE_I6P&pullRequest=100
async function (this: EpdsWorld) {
await captureGet(this, `${testEnv.authUrl}/metrics`)
},
)
40 changes: 19 additions & 21 deletions features/security.feature
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,17 @@ Feature: Security measures

# --- CSRF protection ---

@pending
Scenario: Forms include CSRF protection
When the login page is loaded
Scenario: Server-rendered forms include a CSRF token
# The login page submits via JS fetch to better-auth, which enforces its
# own CSRF protections at the handler level. Server-rendered HTML forms
# (recovery, choose-handle, account-settings) use ePDS's CSRF middleware
# and must carry a matching hidden token field.
When the recovery page is loaded
Then the response sets a CSRF cookie
And the HTML form contains a hidden CSRF token field

@pending
Scenario: POST without CSRF token is rejected
When a POST request is sent to the OTP verification endpoint without a CSRF token
Scenario: POST to a CSRF-protected route without a token is rejected
When a POST request is sent to the recovery endpoint without a CSRF token
Then the response status is 403

# --- Rate limiting ---
Expand All @@ -34,21 +36,20 @@ Feature: Security measures

# --- Security headers ---

@pending
Scenario: Auth service responses include security headers
When any page is loaded from the auth service via Caddy
Then the response includes:
| header | value |
| Strict-Transport-Security | max-age=31536000 |
| X-Frame-Options | DENY |
| X-Content-Type-Options | nosniff |
| Referrer-Policy | no-referrer |

@pending
Scenario: Content-Security-Policy restricts inline content
When any page is loaded from the auth service
Then the response includes the following security headers:
| header | value |
| Strict-Transport-Security | max-age=63072000; includeSubDomains; preload |
| X-Frame-Options | DENY |
| X-Content-Type-Options | nosniff |
| Referrer-Policy | no-referrer |

Scenario: Content-Security-Policy restricts inline scripts
When the login page is loaded
Then the Content-Security-Policy header is present
And it does not allow unsafe-inline scripts
And the script-src directive does not allow unsafe-inline
And the script-src directive carries a per-response nonce

# --- Monitoring ---

Expand All @@ -59,12 +60,9 @@ Feature: Security measures
When GET /health is called on the PDS core
Then it returns status 200 with { "status": "ok" }

@pending
Scenario: Metrics endpoint requires authentication
When GET /metrics is called on the auth service without credentials
Then the response status is 401
When GET /metrics is called with valid Basic auth credentials
Then the response includes uptime and memory usage metrics

# --- Same-site deployment topology (sec-fetch-site) ---
#
Expand Down
44 changes: 44 additions & 0 deletions packages/auth-service/src/__tests__/security-headers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,20 @@ describe('buildAuthServiceCsp', () => {
expect(csp).toContain("style-src 'self' 'unsafe-inline'")
expect(csp).toContain("connect-src 'self'")
})

it('uses the nonce in script-src and drops unsafe-inline from script-src when nonce is supplied', () => {
const csp = buildAuthServiceCsp(null, 'abc123')
expect(csp).toContain("script-src 'self' 'nonce-abc123'")
expect(csp).not.toContain("script-src 'self' 'unsafe-inline'")
// style-src keeps 'unsafe-inline' — scoped fallback for our own stylesheets.
expect(csp).toContain("style-src 'self' 'unsafe-inline'")
})

it('still widens img-src when both client_id and nonce are supplied', () => {
const csp = buildAuthServiceCsp('https://app.example.com/cm.json', 'abc123')
expect(csp).toContain("img-src 'self' data: https://app.example.com")
expect(csp).toContain("script-src 'self' 'nonce-abc123'")
})
})

describe('extractClientIdFromRequest', () => {
Expand Down Expand Up @@ -107,6 +121,7 @@ describe('createSecurityHeadersMiddleware', () => {
setHeader: vi.fn((name: string, value: string) => {
calls.push([name, value])
}),
locals: {} as Record<string, unknown>,
}
return { res, calls }
}
Expand Down Expand Up @@ -163,6 +178,35 @@ describe('createSecurityHeadersMiddleware', () => {
expect(csp).toContain("img-src 'self' data: https://app.example.com")
})

it('writes a base64url nonce to res.locals.cspNonce', () => {
const mw = createSecurityHeadersMiddleware()
const { res } = makeRes()
mw({ query: {} }, res, () => {})
const nonce = res.locals.cspNonce
expect(typeof nonce).toBe('string')
expect(nonce as string).toMatch(/^[A-Za-z0-9_-]+$/)
// 16 random bytes base64url-encoded = 22 chars (no padding).
expect((nonce as string).length).toBeGreaterThanOrEqual(20)
})

it('uses the same nonce value in script-src that it wrote to res.locals', () => {
const mw = createSecurityHeadersMiddleware()
const { res, calls } = makeRes()
mw({ query: {} }, res, () => {})
const nonce = res.locals.cspNonce as string
const csp = calls.find(([name]) => name === 'Content-Security-Policy')?.[1]
expect(csp).toContain(`script-src 'self' 'nonce-${nonce}'`)
})

it('generates a fresh nonce per request', () => {
const mw = createSecurityHeadersMiddleware()
const first = makeRes()
const second = makeRes()
mw({ query: {} }, first.res, () => {})
mw({ query: {} }, second.res, () => {})
expect(first.res.locals.cspNonce).not.toBe(second.res.locals.cspNonce)
})

it('prefers direct client_id over authFlowLookup', () => {
const lookup = vi.fn(() => 'https://from-lookup.example/cm.json')
const mw = createSecurityHeadersMiddleware({ authFlowLookup: lookup })
Expand Down
24 changes: 14 additions & 10 deletions packages/auth-service/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,17 +80,21 @@ export function createAuthService(config: AuthServiceConfig): {

// Metrics endpoint (protect with admin auth in production)
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) {
const authHeader = req.headers.authorization
if (
!authHeader ||
authHeader !==
'Basic ' + Buffer.from('admin:' + adminPassword).toString('base64')
) {
res.status(401).json({ error: 'Unauthorized' })
return
}
if (!adminPassword) {
res.status(401).json({ error: 'Unauthorized' })
return
}
const authHeader = req.headers.authorization
const expected =
'Basic ' + Buffer.from('admin:' + adminPassword).toString('base64')
if (!authHeader || authHeader !== expected) {
Comment on lines +92 to +95
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Use timingSafeEqual() for the Basic auth secret comparison.

authHeader !== expected compares a secret-derived value with a regular string comparison. Use the shared constant-time helper for this check. As per coding guidelines, "Use timingSafeEqual() for all secret and token comparisons".

🛡️ Proposed fix
-import { createLogger, getEpdsVersion } from '@certified-app/shared'
+import {
+  createLogger,
+  getEpdsVersion,
+  timingSafeEqual,
+} from '@certified-app/shared'
@@
-    if (!authHeader || authHeader !== expected) {
+    if (!authHeader || !timingSafeEqual(authHeader, expected)) {
       res.status(401).json({ error: 'Unauthorized' })
       return
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const authHeader = req.headers.authorization
const expected =
'Basic ' + Buffer.from('admin:' + adminPassword).toString('base64')
if (!authHeader || authHeader !== expected) {
import {
createLogger,
getEpdsVersion,
timingSafeEqual,
} from '@certified-app/shared'
// ... other code ...
const authHeader = req.headers.authorization
const expected =
'Basic ' + Buffer.from('admin:' + adminPassword).toString('base64')
if (!authHeader || !timingSafeEqual(authHeader, expected)) {
res.status(401).json({ error: 'Unauthorized' })
return
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/auth-service/src/index.ts` around lines 73 - 76, The direct string
comparison authHeader !== expected must be replaced with the constant-time
helper timingSafeEqual to avoid timing attacks: convert both values to Buffers
(ensure authHeader is a string or use an empty string if missing), check lengths
equal, then call timingSafeEqual(authBuf, expectedBuf) and branch on its boolean
result instead of !==; update the conditional around authHeader/expected (the
symbols authHeader, expected) to use this helper so secret comparisons are done
in constant time.

res.status(401).json({ error: 'Unauthorized' })
return
}
const metrics = ctx.db.getMetrics()
res.json({
Expand Down
Loading
Loading