From 3fc0ea14ae64545c1851709a01f07ecf47d64501 Mon Sep 17 00:00:00 2001 From: amossamuel851-tech Date: Wed, 26 Aug 2026 07:40:56 +0000 Subject: [PATCH] fix(csrf): fail closed when CSRF_SECRET is unset MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove the hardcoded fallback CSRF secret so tokens can no longer be minted or verified with a publicly-known key. Minting now throws via the existing requireEnvStrict helper, and verification returns false, while validate-env.js and .env.example require the variable. Closes #817 🤖 Generated with Codebuff Co-Authored-By: Codebuff --- .env.example | 9 ++ scripts/validate-env.js | 4 + src/lib/__tests__/csrf.test.ts | 194 +++++++++++++++++++++++++++++++++ src/lib/csrf.ts | 23 +++- 4 files changed, 224 insertions(+), 6 deletions(-) create mode 100644 src/lib/__tests__/csrf.test.ts diff --git a/.env.example b/.env.example index ab60862c..c2c26ca5 100644 --- a/.env.example +++ b/.env.example @@ -25,6 +25,15 @@ ANALYZE=false # Enable CSP enforcement (set to "true" after report-only rollout) CSP_ENFORCE=false +# ----------------------------------------------------------------------------- +# Security / CSRF Protection +# ----------------------------------------------------------------------------- + +# HMAC signing secret for CSRF tokens. REQUIRED: CSRF token minting and +# verification fail closed when this is unset. Generate a strong random value, +# e.g.: openssl rand -hex 32 +CSRF_SECRET= + # ----------------------------------------------------------------------------- # API Configurations # ----------------------------------------------------------------------------- diff --git a/scripts/validate-env.js b/scripts/validate-env.js index c6d18b5b..a42edd6c 100644 --- a/scripts/validate-env.js +++ b/scripts/validate-env.js @@ -38,6 +38,10 @@ const envSchema = { validate: (v) => ["development", "staging", "production"].includes(v), default: "development", }, + CSRF_SECRET: { + validate: (v) => typeof v === "string" && v.length > 0, + default: undefined, + }, ANALYZE: { validate: (v) => !v || v === "true" || v === "false", default: "false", diff --git a/src/lib/__tests__/csrf.test.ts b/src/lib/__tests__/csrf.test.ts new file mode 100644 index 00000000..327f4610 --- /dev/null +++ b/src/lib/__tests__/csrf.test.ts @@ -0,0 +1,194 @@ +import crypto from 'crypto'; +import { readFileSync } from 'fs'; +import { join } from 'path'; +import { NextResponse } from 'next/server'; +import { + generateTokenForSession, + validateCsrf, + withCsrf, +} from '@/lib/csrf'; +import type { NextRequest } from 'next/server'; + +// The jsdom test environment does not provide the Fetch API globals that +// `NextRequest` needs, so mock the server response primitive instead. +jest.mock('next/server', () => ({ + NextResponse: { + json: jest.fn( + (body: unknown, init?: { status?: number }) => ({ + status: init?.status ?? 200, + body, + /** Resolve the mocked body like NextResponse.json().json(). */ + json: async () => body, + }) + ), + }, +})); + +const OLD_ENV = process.env; +const TEST_SECRET = 'test-csrf-secret-0123456789abcdef0123456789abcdef'; +// The literal fallback that used to be hardcoded in src/lib/csrf.ts (issue #817). +const LEGACY_FALLBACK_SECRET = 'default-fallback-csrf-secret-key-32-chars-long!'; + +const csrfSourcePath = join(__dirname, '..', 'csrf.ts'); + +interface MockRequest { + headers: { get(name: string): string | null }; + cookies: { get(name: string): { value: string } | undefined }; +} + +beforeEach(() => { + jest.resetModules(); + process.env = { ...OLD_ENV, CSRF_SECRET: TEST_SECRET }; +}); + +afterAll(() => { + process.env = OLD_ENV; +}); + +/** Build a minimal request-shaped object for the functions under test. */ +const createRequest = (opts: { + csrfToken?: string; + sessionId?: string; + authToken?: string; +} = {}): MockRequest => { + const headers = new Map(); + const cookies = new Map(); + if (opts.csrfToken) headers.set('x-csrf-token', opts.csrfToken); + if (opts.sessionId) cookies.set('csrf-session', opts.sessionId); + if (opts.authToken) cookies.set('auth-token', opts.authToken); + return { + headers: { + /** Return the header value or null, mirroring the Headers API. */ + get: (name) => headers.get(name) ?? null, + }, + cookies: { + /** Return the cookie or undefined, mirroring NextRequest cookies. */ + get: (name) => { + const value = cookies.get(name); + return value === undefined ? undefined : { value }; + }, + }, + }; +}; + +/** Cast the mock request to the type expected by the CSRF helpers. */ +const asNextRequest = (req: MockRequest) => req as unknown as NextRequest; + +/** + * Sign the same session/auth payload the module signs, with an arbitrary key. + * Used to prove tokens minted with a different secret are rejected. + */ +const signWithSecret = (secret: string, sessionId: string, authState: string) => + crypto + .createHmac('sha256', secret) + .update(`${sessionId}:${authState}`) + .digest('hex'); + +describe('generateTokenForSession', () => { + it('fails closed (throws) when CSRF_SECRET is unset', () => { + delete process.env.CSRF_SECRET; + expect(() => generateTokenForSession('session-1', '')).toThrow( + 'Missing required environment variable: CSRF_SECRET' + ); + }); + + it('fails closed (throws) when CSRF_SECRET is an empty string', () => { + process.env.CSRF_SECRET = ''; + expect(() => generateTokenForSession('session-1', '')).toThrow( + 'Missing required environment variable: CSRF_SECRET' + ); + }); + + it('mints an HMAC-SHA256 token when CSRF_SECRET is set', () => { + const token = generateTokenForSession('session-1', 'auth-1'); + expect(token).toMatch(/^[0-9a-f]{64}$/); + expect(token).toBe(signWithSecret(TEST_SECRET, 'session-1', 'auth-1')); + }); + + it('produces different tokens for different sessions/auth states', () => { + const tokenA = generateTokenForSession('session-1', 'auth-1'); + const tokenB = generateTokenForSession('session-2', 'auth-1'); + expect(tokenA).not.toBe(tokenB); + }); +}); + +describe('validateCsrf', () => { + it('accepts a valid token minted with the configured secret', () => { + const sessionId = 'session-1'; + const token = generateTokenForSession(sessionId, ''); + expect(validateCsrf(asNextRequest(createRequest({ csrfToken: token, sessionId })))).toBe(true); + }); + + it('rejects a token signed with the old hardcoded fallback secret', () => { + const sessionId = 'session-1'; + const forgedToken = signWithSecret(LEGACY_FALLBACK_SECRET, sessionId, ''); + const req = createRequest({ csrfToken: forgedToken, sessionId }); + expect(validateCsrf(asNextRequest(req))).toBe(false); + }); + + it('fails closed when CSRF_SECRET is unset, even with a previously valid token', () => { + const sessionId = 'session-1'; + const token = generateTokenForSession(sessionId, ''); + delete process.env.CSRF_SECRET; + const req = createRequest({ csrfToken: token, sessionId }); + expect(validateCsrf(asNextRequest(req))).toBe(false); + }); + + it('rejects a request with no CSRF token header', () => { + const req = createRequest({ sessionId: 'session-1' }); + expect(validateCsrf(asNextRequest(req))).toBe(false); + }); + + it('rejects a request with no session cookie', () => { + const token = generateTokenForSession('session-1', ''); + const req = createRequest({ csrfToken: token }); + expect(validateCsrf(asNextRequest(req))).toBe(false); + }); + + it('rejects a tampered token', () => { + const sessionId = 'session-1'; + const token = generateTokenForSession(sessionId, ''); + const req = createRequest({ csrfToken: `${token}ff`, sessionId }); + expect(validateCsrf(asNextRequest(req))).toBe(false); + }); +}); + +describe('withCsrf', () => { + /** Handler that resolves successfully when CSRF validation passes. */ + const okHandler = async () => NextResponse.json({ ok: true }); + + it('returns 403 when CSRF validation fails', async () => { + const wrapped = withCsrf(okHandler); + const res = await wrapped(asNextRequest(createRequest({ sessionId: 'session-1' }))); + expect(res.status).toBe(403); + }); + + it('invokes the handler when CSRF validation passes', async () => { + const wrapped = withCsrf(okHandler); + const sessionId = 'session-1'; + const token = generateTokenForSession(sessionId, ''); + const res = await wrapped( + asNextRequest(createRequest({ csrfToken: token, sessionId })) + ); + expect(res.status).toBe(200); + expect(await res.json()).toEqual({ ok: true }); + }); + + it('returns 403 when CSRF_SECRET is unset', async () => { + delete process.env.CSRF_SECRET; + const sessionId = 'session-1'; + const token = signWithSecret(TEST_SECRET, sessionId, ''); + const wrapped = withCsrf(okHandler); + const res = await wrapped( + asNextRequest(createRequest({ csrfToken: token, sessionId })) + ); + expect(res.status).toBe(403); + }); +}); + +describe('src/lib/csrf.ts (regression guard)', () => { + it('no longer contains the hardcoded fallback secret', () => { + const source = readFileSync(csrfSourcePath, 'utf-8'); + expect(source).not.toContain(LEGACY_FALLBACK_SECRET); + }); +}); diff --git a/src/lib/csrf.ts b/src/lib/csrf.ts index 33df2ccf..13799183 100644 --- a/src/lib/csrf.ts +++ b/src/lib/csrf.ts @@ -1,7 +1,7 @@ import crypto from 'crypto'; import { NextRequest, NextResponse } from 'next/server'; +import { requireEnvStrict } from '@/lib/requireEnv'; -const CSRF_SECRET = process.env.CSRF_SECRET || 'default-fallback-csrf-secret-key-32-chars-long!'; const CSRF_SESSION_COOKIE = 'csrf-session'; /** @@ -26,10 +26,14 @@ export function getAuthStatePart(request: NextRequest): string { /** * Generates an HMAC-SHA256 token bound to a session and authentication state. + * + * Fails closed: throws when `CSRF_SECRET` is not configured, so no token is + * ever minted with a guessable or hardcoded key. */ export function generateTokenForSession(sessionId: string, authState: string): string { + const secret = requireEnvStrict('CSRF_SECRET'); return crypto - .createHmac('sha256', CSRF_SECRET) + .createHmac('sha256', secret) .update(`${sessionId}:${authState}`) .digest('hex'); } @@ -49,7 +53,14 @@ export function validateCsrf(request: NextRequest): boolean { } const authState = getAuthStatePart(request); - const expectedToken = generateTokenForSession(sessionId, authState); + + let expectedToken: string; + try { + expectedToken = generateTokenForSession(sessionId, authState); + } catch { + // Fail closed: without a configured secret no token can ever be valid. + return false; + } try { const tokenBuffer = Buffer.from(tokenFromHeader); @@ -68,10 +79,10 @@ export function validateCsrf(request: NextRequest): boolean { /** * A middleware wrapper to enforce CSRF token validation on write handlers. */ -export function withCsrf( - handler: (request: NextRequest, ...args: any[]) => Promise | NextResponse> +export function withCsrf( + handler: (request: NextRequest, ...args: unknown[]) => Promise | NextResponse> ) { - return async function (request: NextRequest, ...args: any[]): Promise { + return async function (request: NextRequest, ...args: unknown[]): Promise { if (!validateCsrf(request)) { return NextResponse.json( { error: 'Invalid or missing CSRF token' },