fix(shared): timingSafeEqual byte-length check + CodeRabbit follow-ups

- shared.timingSafeEqual: compare UTF-8 byte lengths, not JS code units.
  Previously a non-ASCII input with the same .length as the expected
  value would reach crypto.timingSafeEqual with different byte lengths
  and throw RangeError — turning a 401 into a 500. Guards every
  caller (metrics-auth, recovery, callback signatures).
- metrics-auth: regression test for non-ASCII Authorization header
  with matching code-unit length.
- security.steps.ts: Headers.getSetCookie() for reliable multi-cookie
  access; nonce step now fetches twice and asserts nonces differ, so
  a hardcoded constant would no longer pass.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Author: aspiers

e2e/step-definitions/security.steps.ts

@@ -55,10 +55,15 @@ When('the recovery page is loaded', async function (this: EpdsWorld) {
 
 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)) {
+  // `headers.get('set-cookie')` collapses multiple Set-Cookie values
+  // into a single comma-separated string in the Fetch API, which is
+  // ambiguous because cookie values may themselves contain commas
+  // (e.g. Expires=Thu, 01 Jan …). Use Headers.getSetCookie() (Node
+  // >=18) to get each cookie as its own entry.
+  const setCookies = headers.getSetCookie()
+  if (!setCookies.some((cookie) => /^epds_csrf=/.test(cookie))) {
     throw new Error(
-      `Expected Set-Cookie to include epds_csrf=..., got: ${setCookie || '(none)'}`,
+      `Expected Set-Cookie to include epds_csrf=..., got: ${setCookies.join(', ') || '(none)'}`,
     )
   }
 })
@@ -186,14 +191,38 @@ Then(
   },
 )
 
+function extractScriptSrcNonce(csp: string): string {
+  const scriptSrc = getScriptSrcDirective(csp)
+  const match = /'nonce-([A-Za-z0-9_-]+)'/.exec(scriptSrc)
+  if (!match) {
+    throw new Error(`script-src directive has no 'nonce-...': "${scriptSrc}"`)
+  }
+  return match[1]
+}
+
 Then(
   'the script-src directive carries a per-response nonce',
-  function (this: EpdsWorld) {
+  async function (this: EpdsWorld) {
+    // Assert two things: (a) the current response has a nonce-shaped
+    // token in script-src, and (b) the nonce is freshly generated per
+    // response — otherwise a hardcoded constant would pass (a) but
+    // defeat the whole point of a CSP nonce. Hit the same endpoint a
+    // second time and compare.
     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}"`)
+    const firstCsp = headers.get('content-security-policy') ?? ''
+    const firstNonce = extractScriptSrcNonce(firstCsp)
+
+    const previewUrl = `${testEnv.authUrl}/preview/login`
+    let second = await fetch(previewUrl, { redirect: 'manual' })
+    if (second.status === 404) {
+      second = await fetch(`${testEnv.authUrl}/health`, { redirect: 'manual' })
+    }
+    const secondCsp = second.headers.get('content-security-policy') ?? ''
+    const secondNonce = extractScriptSrcNonce(secondCsp)
+    if (firstNonce === secondNonce) {
+      throw new Error(
+        `CSP nonce reused across responses: "${firstNonce}" — expected a fresh nonce per request`,
+      )
     }
   },
 )

packages/auth-service/src/__tests__/metrics-auth.test.ts

@@ -72,5 +72,19 @@ describe('checkMetricsAuth', () => {
       const r = checkMetricsAuth(samelengthWrong, adminPassword)
       expect(r.ok).toBe(false)
     })
+
+    it('rejects a non-ASCII header with the same code-unit length as expected', () => {
+      // Guard against the timingSafeEqual footgun where an attacker
+      // can trigger RangeError (500) instead of 401 by sending a
+      // header whose JS .length matches but whose UTF-8 byte length
+      // does not (e.g. multibyte chars). Must return 401, not throw.
+      const expected = basic('admin', adminPassword)
+      const sameCodeUnitsWrong = 'é'.repeat(expected.length)
+      expect(() =>
+        checkMetricsAuth(sameCodeUnitsWrong, adminPassword),
+      ).not.toThrow()
+      const r = checkMetricsAuth(sameCodeUnitsWrong, adminPassword)
+      expect(r.ok).toBe(false)
+    })
   })
 })

packages/shared/src/__tests__/crypto.test.ts

@@ -57,6 +57,18 @@ describe('timingSafeEqual', () => {
   it('returns false for different lengths', () => {
     expect(timingSafeEqual('short', 'longer-string')).toBe(false)
   })
+
+  it('returns false (no throw) for inputs of equal code-unit length but different byte length', () => {
+    // Guards against a regression where the length check was done in
+    // JavaScript code units (a.length) but the underlying
+    // crypto.timingSafeEqual compares UTF-8 byte length, which throws
+    // RangeError when the two differ. A non-ASCII attacker-supplied
+    // value trivially hits this path.
+    const ascii = 'x'.repeat(30)
+    const nonAscii = 'é'.repeat(30)
+    expect(() => timingSafeEqual(nonAscii, ascii)).not.toThrow()
+    expect(timingSafeEqual(nonAscii, ascii)).toBe(false)
+  })
 })
 
 describe('verifyInternalSecret', () => {

packages/shared/src/crypto.ts

@@ -24,14 +24,26 @@ export function hashToken(token: string): string {
   return crypto.createHash('sha256').update(token).digest('hex')
 }
 
-/** Timing-safe comparison of two strings. */
+/**
+ * Timing-safe comparison of two strings.
+ *
+ * Encodes both inputs as UTF-8 Buffers and compares by byte length.
+ * Comparing `a.length !== b.length` (JavaScript code units) is not
+ * sufficient: a non-ASCII input with the same code-unit count as the
+ * expected value has a different UTF-8 byte length, and
+ * `crypto.timingSafeEqual` throws `RangeError` when byte lengths
+ * differ. This matters for any caller that feeds attacker-controlled
+ * text (HTTP headers, cookies) in as `a`.
+ */
 export function timingSafeEqual(a: string, b: string): boolean {
-  if (a.length !== b.length) {
-    const dummy = Buffer.alloc(a.length)
+  const ab = Buffer.from(a)
+  const bb = Buffer.from(b)
+  if (ab.length !== bb.length) {
+    const dummy = Buffer.alloc(bb.length)
     crypto.timingSafeEqual(dummy, dummy)
     return false
   }
-  return crypto.timingSafeEqual(Buffer.from(a), Buffer.from(b))
+  return crypto.timingSafeEqual(ab, bb)
 }
 
 /**