Skip to content

Commit 0fd54dc

Browse files
nanaf6203-bitnanaf6203-bit
authored
Merge pull request #938 from amossamuel851-tech/fix/issue-817-csrf-fail-closed
fix(csrf): fail closed when CSRF_SECRET is unset
2 parents 2988180 + c597884 commit 0fd54dc

4 files changed

Lines changed: 206 additions & 110 deletions

File tree

‎.env.example‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,15 @@ ANALYZE=false
2525
# Enable CSP enforcement (set to "true" after report-only rollout)
2626
CSP_ENFORCE=false
2727

28+
# -----------------------------------------------------------------------------
29+
# Security / CSRF Protection
30+
# -----------------------------------------------------------------------------
31+
32+
# HMAC signing secret for CSRF tokens. REQUIRED: CSRF token minting and
33+
# verification fail closed when this is unset. Generate a strong random value,
34+
# e.g.: openssl rand -hex 32
35+
CSRF_SECRET=
36+
2837
# -----------------------------------------------------------------------------
2938
# API Configurations
3039
# -----------------------------------------------------------------------------

‎scripts/validate-env.js‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,10 @@ const envSchema = {
3838
validate: (v) => ["development", "staging", "production"].includes(v),
3939
default: "development",
4040
},
41+
CSRF_SECRET: {
42+
validate: (v) => typeof v === "string" && v.length > 0,
43+
default: undefined,
44+
},
4145
ANALYZE: {
4246
validate: (v) => !v || v === "true" || v === "false",
4347
default: "false",

‎src/lib/__tests__/csrf.test.ts‎

Lines changed: 176 additions & 104 deletions
Original file line numberDiff line numberDiff line change
@@ -1,122 +1,194 @@
11
import crypto from 'crypto';
2-
3-
jest.mock('next/headers', () => ({
4-
cookies: jest.fn(),
2+
import { readFileSync } from 'fs';
3+
import { join } from 'path';
4+
import { NextResponse } from 'next/server';
5+
import {
6+
generateTokenForSession,
7+
validateCsrf,
8+
withCsrf,
9+
} from '@/lib/csrf';
10+
import type { NextRequest } from 'next/server';
11+
12+
// The jsdom test environment does not provide the Fetch API globals that
13+
// `NextRequest` needs, so mock the server response primitive instead.
14+
jest.mock('next/server', () => ({
15+
NextResponse: {
16+
json: jest.fn(
17+
(body: unknown, init?: { status?: number }) => ({
18+
status: init?.status ?? 200,
19+
body,
20+
/** Resolve the mocked body like NextResponse.json().json(). */
21+
json: async () => body,
22+
})
23+
),
24+
},
525
}));
626

7-
import { cookies } from 'next/headers';
8-
import { getCsrfSessionId, getAuthStatePart, generateTokenForSession, validateCsrf, withCsrf } from '../../csrf';
27+
const OLD_ENV = process.env;
28+
const TEST_SECRET = 'test-csrf-secret-0123456789abcdef0123456789abcdef';
29+
// The literal fallback that used to be hardcoded in src/lib/csrf.ts (issue #817).
30+
const LEGACY_FALLBACK_SECRET = 'default-fallback-csrf-secret-key-32-chars-long!';
31+
32+
const csrfSourcePath = join(__dirname, '..', 'csrf.ts');
33+
34+
interface MockRequest {
35+
headers: { get(name: string): string | null };
36+
cookies: { get(name: string): { value: string } | undefined };
37+
}
38+
39+
beforeEach(() => {
40+
jest.resetModules();
41+
process.env = { ...OLD_ENV, CSRF_SECRET: TEST_SECRET };
42+
});
43+
44+
afterAll(() => {
45+
process.env = OLD_ENV;
46+
});
947

10-
function makeRequest(headers: Record<string, string> = {}, cookieValues: Record<string, string> = {}) {
11-
const cookieStore = new Map(Object.entries(cookieValues));
48+
/** Build a minimal request-shaped object for the functions under test. */
49+
const createRequest = (opts: {
50+
csrfToken?: string;
51+
sessionId?: string;
52+
authToken?: string;
53+
} = {}): MockRequest => {
54+
const headers = new Map<string, string>();
55+
const cookies = new Map<string, string>();
56+
if (opts.csrfToken) headers.set('x-csrf-token', opts.csrfToken);
57+
if (opts.sessionId) cookies.set('csrf-session', opts.sessionId);
58+
if (opts.authToken) cookies.set('auth-token', opts.authToken);
1259
return {
1360
headers: {
14-
get: (name: string) => headers[name] ?? null,
61+
/** Return the header value or null, mirroring the Headers API. */
62+
get: (name) => headers.get(name) ?? null,
1563
},
1664
cookies: {
17-
get: (name: string) => (cookieStore.has(name) ? { value: cookieStore.get(name) } : undefined),
65+
/** Return the cookie or undefined, mirroring NextRequest cookies. */
66+
get: (name) => {
67+
const value = cookies.get(name);
68+
return value === undefined ? undefined : { value };
69+
},
1870
},
19-
} as any;
20-
}
71+
};
72+
};
73+
74+
/** Cast the mock request to the type expected by the CSRF helpers. */
75+
const asNextRequest = (req: MockRequest) => req as unknown as NextRequest;
76+
77+
/**
78+
* Sign the same session/auth payload the module signs, with an arbitrary key.
79+
* Used to prove tokens minted with a different secret are rejected.
80+
*/
81+
const signWithSecret = (secret: string, sessionId: string, authState: string) =>
82+
crypto
83+
.createHmac('sha256', secret)
84+
.update(`${sessionId}:${authState}`)
85+
.digest('hex');
86+
87+
describe('generateTokenForSession', () => {
88+
it('fails closed (throws) when CSRF_SECRET is unset', () => {
89+
delete process.env.CSRF_SECRET;
90+
expect(() => generateTokenForSession('session-1', '')).toThrow(
91+
'Missing required environment variable: CSRF_SECRET'
92+
);
93+
});
94+
95+
it('fails closed (throws) when CSRF_SECRET is an empty string', () => {
96+
process.env.CSRF_SECRET = '';
97+
expect(() => generateTokenForSession('session-1', '')).toThrow(
98+
'Missing required environment variable: CSRF_SECRET'
99+
);
100+
});
101+
102+
it('mints an HMAC-SHA256 token when CSRF_SECRET is set', () => {
103+
const token = generateTokenForSession('session-1', 'auth-1');
104+
expect(token).toMatch(/^[0-9a-f]{64}$/);
105+
expect(token).toBe(signWithSecret(TEST_SECRET, 'session-1', 'auth-1'));
106+
});
107+
108+
it('produces different tokens for different sessions/auth states', () => {
109+
const tokenA = generateTokenForSession('session-1', 'auth-1');
110+
const tokenB = generateTokenForSession('session-2', 'auth-1');
111+
expect(tokenA).not.toBe(tokenB);
112+
});
113+
});
114+
115+
describe('validateCsrf', () => {
116+
it('accepts a valid token minted with the configured secret', () => {
117+
const sessionId = 'session-1';
118+
const token = generateTokenForSession(sessionId, '');
119+
expect(validateCsrf(asNextRequest(createRequest({ csrfToken: token, sessionId })))).toBe(true);
120+
});
121+
122+
it('rejects a token signed with the old hardcoded fallback secret', () => {
123+
const sessionId = 'session-1';
124+
const forgedToken = signWithSecret(LEGACY_FALLBACK_SECRET, sessionId, '');
125+
const req = createRequest({ csrfToken: forgedToken, sessionId });
126+
expect(validateCsrf(asNextRequest(req))).toBe(false);
127+
});
128+
129+
it('fails closed when CSRF_SECRET is unset, even with a previously valid token', () => {
130+
const sessionId = 'session-1';
131+
const token = generateTokenForSession(sessionId, '');
132+
delete process.env.CSRF_SECRET;
133+
const req = createRequest({ csrfToken: token, sessionId });
134+
expect(validateCsrf(asNextRequest(req))).toBe(false);
135+
});
136+
137+
it('rejects a request with no CSRF token header', () => {
138+
const req = createRequest({ sessionId: 'session-1' });
139+
expect(validateCsrf(asNextRequest(req))).toBe(false);
140+
});
141+
142+
it('rejects a request with no session cookie', () => {
143+
const token = generateTokenForSession('session-1', '');
144+
const req = createRequest({ csrfToken: token });
145+
expect(validateCsrf(asNextRequest(req))).toBe(false);
146+
});
147+
148+
it('rejects a tampered token', () => {
149+
const sessionId = 'session-1';
150+
const token = generateTokenForSession(sessionId, '');
151+
const req = createRequest({ csrfToken: `${token}ff`, sessionId });
152+
expect(validateCsrf(asNextRequest(req))).toBe(false);
153+
});
154+
});
21155

22-
describe('CSRF server-side lib', () => {
23-
describe('getCsrfSessionId', () => {
24-
it('returns existing cookie value', () => {
25-
const request = makeRequest({}, { 'csrf-session': 'existing-session-id' });
26-
const { sessionId, isNew } = getCsrfSessionId(request);
27-
expect(sessionId).toBe('existing-session-id');
28-
expect(isNew).toBe(false);
29-
});
30-
31-
it('generates new UUID when no cookie', () => {
32-
const request = makeRequest();
33-
const { sessionId, isNew } = getCsrfSessionId(request);
34-
expect(isNew).toBe(true);
35-
expect(sessionId).toMatch(
36-
/^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i,
37-
);
38-
});
156+
describe('withCsrf', () => {
157+
/** Handler that resolves successfully when CSRF validation passes. */
158+
const okHandler = async () => NextResponse.json({ ok: true });
159+
160+
it('returns 403 when CSRF validation fails', async () => {
161+
const wrapped = withCsrf(okHandler);
162+
const res = await wrapped(asNextRequest(createRequest({ sessionId: 'session-1' })));
163+
expect(res.status).toBe(403);
39164
});
40165

41-
describe('generateTokenForSession', () => {
42-
it('returns a string token', () => {
43-
const token = generateTokenForSession('session-1', 'auth-1');
44-
expect(typeof token).toBe('string');
45-
expect(token.length).toBeGreaterThan(0);
46-
});
47-
48-
it('produces consistent output for same inputs', () => {
49-
const t1 = generateTokenForSession('s', 'a');
50-
const t2 = generateTokenForSession('s', 'a');
51-
expect(t1).toBe(t2);
52-
});
53-
54-
it('produces different output for different inputs', () => {
55-
const t1 = generateTokenForSession('s1', 'a');
56-
const t2 = generateTokenForSession('s2', 'a');
57-
expect(t1).not.toBe(t2);
58-
});
166+
it('invokes the handler when CSRF validation passes', async () => {
167+
const wrapped = withCsrf(okHandler);
168+
const sessionId = 'session-1';
169+
const token = generateTokenForSession(sessionId, '');
170+
const res = await wrapped(
171+
asNextRequest(createRequest({ csrfToken: token, sessionId }))
172+
);
173+
expect(res.status).toBe(200);
174+
expect(await res.json()).toEqual({ ok: true });
59175
});
60176

61-
describe('validateCsrf', () => {
62-
it('returns true for matching token', () => {
63-
const sessionId = 'test-session';
64-
const authState = 'test-auth';
65-
const token = generateTokenForSession(sessionId, authState);
66-
67-
const request = makeRequest(
68-
{ 'x-csrf-token': token },
69-
{ 'csrf-session': sessionId, 'auth-token': authState },
70-
);
71-
72-
expect(validateCsrf(request)).toBe(true);
73-
});
74-
75-
it('returns false for mismatched token', () => {
76-
const request = makeRequest(
77-
{ 'x-csrf-token': 'wrong-token' },
78-
{ 'csrf-session': 'test-session', 'auth-token': 'test-auth' },
79-
);
80-
81-
expect(validateCsrf(request)).toBe(false);
82-
});
83-
84-
it('returns false when no token provided', () => {
85-
const request = makeRequest({}, { 'csrf-session': 'test-session' });
86-
expect(validateCsrf(request)).toBe(false);
87-
});
88-
89-
it('returns false when no session cookie', () => {
90-
const request = makeRequest({ 'x-csrf-token': 'some-token' }, {});
91-
expect(validateCsrf(request)).toBe(false);
92-
});
177+
it('returns 403 when CSRF_SECRET is unset', async () => {
178+
delete process.env.CSRF_SECRET;
179+
const sessionId = 'session-1';
180+
const token = signWithSecret(TEST_SECRET, sessionId, '');
181+
const wrapped = withCsrf(okHandler);
182+
const res = await wrapped(
183+
asNextRequest(createRequest({ csrfToken: token, sessionId }))
184+
);
185+
expect(res.status).toBe(403);
93186
});
187+
});
94188

95-
describe('withCsrf', () => {
96-
it('calls handler when CSRF is valid', async () => {
97-
const sessionId = 's1';
98-
const authState = 'a1';
99-
const token = generateTokenForSession(sessionId, authState);
100-
const request = makeRequest(
101-
{ 'x-csrf-token': token },
102-
{ 'csrf-session': sessionId, 'auth-token': authState },
103-
);
104-
105-
const handler = jest.fn().mockResolvedValue({ status: 200 });
106-
const wrapped = withCsrf(handler);
107-
108-
await wrapped(request);
109-
expect(handler).toHaveBeenCalledWith(request);
110-
});
111-
112-
it('returns 403 when CSRF is invalid', async () => {
113-
const request = makeRequest({}, {});
114-
const handler = jest.fn();
115-
const wrapped = withCsrf(handler);
116-
117-
const response = await wrapped(request);
118-
expect(handler).not.toHaveBeenCalled();
119-
expect(response.status).toBe(403);
120-
});
189+
describe('src/lib/csrf.ts (regression guard)', () => {
190+
it('no longer contains the hardcoded fallback secret', () => {
191+
const source = readFileSync(csrfSourcePath, 'utf-8');
192+
expect(source).not.toContain(LEGACY_FALLBACK_SECRET);
121193
});
122194
});

‎src/lib/csrf.ts‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import crypto from 'crypto';
22
import { NextRequest, NextResponse } from 'next/server';
3+
import { requireEnvStrict } from '@/lib/requireEnv';
34

4-
const CSRF_SECRET = process.env.CSRF_SECRET || 'default-fallback-csrf-secret-key-32-chars-long!';
55
const CSRF_SESSION_COOKIE = 'csrf-session';
66

77
/**
@@ -26,10 +26,14 @@ export function getAuthStatePart(request: NextRequest): string {
2626

2727
/**
2828
* Generates an HMAC-SHA256 token bound to a session and authentication state.
29+
*
30+
* Fails closed: throws when `CSRF_SECRET` is not configured, so no token is
31+
* ever minted with a guessable or hardcoded key.
2932
*/
3033
export function generateTokenForSession(sessionId: string, authState: string): string {
34+
const secret = requireEnvStrict('CSRF_SECRET');
3135
return crypto
32-
.createHmac('sha256', CSRF_SECRET)
36+
.createHmac('sha256', secret)
3337
.update(`${sessionId}:${authState}`)
3438
.digest('hex');
3539
}
@@ -49,7 +53,14 @@ export function validateCsrf(request: NextRequest): boolean {
4953
}
5054

5155
const authState = getAuthStatePart(request);
52-
const expectedToken = generateTokenForSession(sessionId, authState);
56+
57+
let expectedToken: string;
58+
try {
59+
expectedToken = generateTokenForSession(sessionId, authState);
60+
} catch {
61+
// Fail closed: without a configured secret no token can ever be valid.
62+
return false;
63+
}
5364

5465
try {
5566
const tokenBuffer = Buffer.from(tokenFromHeader);
@@ -68,10 +79,10 @@ export function validateCsrf(request: NextRequest): boolean {
6879
/**
6980
* A middleware wrapper to enforce CSRF token validation on write handlers.
7081
*/
71-
export function withCsrf<T = any>(
72-
handler: (request: NextRequest, ...args: any[]) => Promise<NextResponse<T> | NextResponse>
82+
export function withCsrf<T = unknown>(
83+
handler: (request: NextRequest, ...args: unknown[]) => Promise<NextResponse<T> | NextResponse>
7384
) {
74-
return async function (request: NextRequest, ...args: any[]): Promise<NextResponse> {
85+
return async function (request: NextRequest, ...args: unknown[]): Promise<NextResponse> {
7586
if (!validateCsrf(request)) {
7687
return NextResponse.json(
7788
{ error: 'Invalid or missing CSRF token' },

0 commit comments

Comments
 (0)