fix(csrf): fail closed when CSRF_SECRET is unset - #938
Merged
nanaf6203-bit merged 2 commits intoAug 27, 2026
Merged
Conversation
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 MettaChain#817 🤖 Generated with Codebuff Co-Authored-By: Codebuff <[email protected]>
|
@amossamuel851-tech Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
nanaf6203-bit
approved these changes
Aug 26, 2026
nanaf6203-bit
left a comment
Contributor
There was a problem hiding this comment.
Nice work, thanks for getting this over the line!
🔒 Preview Environment DestroyedThe preview environment for this PR has been torn down. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #817
Removes the hardcoded fallback CSRF secret from
src/lib/csrf.tsso tokens can no longer be minted or verified with a publicly-known key, and makes the module fail closed whenCSRF_SECRETis unset: minting throws a clear error (no token issued) and verification returnsfalse(all protected write handlers keep returning 403). The same change makesCSRF_SECRETrequired inscripts/validate-env.jsand documents it in.env.example, so deployments that never set the variable are caught before the enforcement lands.The key design decision: enforcement lives at the mint/verify boundary (reusing the existing
requireEnvStricthelper fromsrc/lib/requireEnv.ts) rather than at module load. That keeps the security guarantee unconditional — no code path can ever produce or accept a token without a configured secret — without breakingnext buildfor environments that are mid-migration, whilevalidate-env.jsgives operators the early, explicit failure.Why
src/lib/csrf.tsline 4 previously derived the secret asprocess.env.CSRF_SECRET || 'default-fallback-csrf-secret-key-32-chars-long!'. In any deployment where the env var is unset, every CSRF token was signed with a key committed to the repository, so an attacker could forge valid tokens for every state-changing request protected by this module — making the CSRF defense equivalent to not having it. The failure was silent: minting and verification both "worked" with plausible-looking values. This is the same defect class the issue notes was already fixed forJWT_SECRETin the backend, butcsrf.tswas a distinct, still-live instance.What was built
src/lib/csrf.tsgenerateTokenForSessionnow reads the secret lazily via the existingrequireEnvStrict('CSRF_SECRET'), throwingMissing required environment variable: CSRF_SECRETwhen unset — so no token is ever minted with a guessable key.validateCsrfcatches that and returnsfalse, so thewithCsrfwrapper fails closed with a 403 instead of crashing. Token format is unchanged (HMAC-SHA256 oversessionId:authState), so behavior with a configured secret is identical. Also tightened the pre-existinganyin thewithCsrfgeneric tounknown, which removes the only remainingno-explicit-anywarnings in the file.src/lib/__tests__/csrf.test.tsCSRF_SECRETis unset or empty; correct minting with a configured secret; valid-token acceptance; rejection of tokens signed with the old hardcoded fallback; fail-closed verification when the secret is removed at runtime; missing header/session; tampered tokens;withCsrf403/200 behavior; and a regression guard asserting the fallback literal is absent from the source.scripts/validate-env.jsCSRF_SECRETto the env schema with a required (non-empty) validator, sonpm run validate:envexits 1 withCSRF_SECRET: Invalid valuewhen unset and exits 0 when set (verified)..env.exampleCSRF_SECRETas required for CSRF protection, with a generation hint (openssl rand -hex 32).The tests are written against the exact security property from the issue — a forged token using the old committed fallback string must be rejected — and the implementation/tests are in lockstep: the fail-closed tests fail against the pre-fix code (verified) and pass against this change.
Integration changes outside
src/lib/scripts/validate-env.js—CSRF_SECRETis now a required environment variable in the schema..env.example— newSecurity / CSRF Protectionsection documentingCSRF_SECRET.src/lib/__tests__/csrf.test.ts— new test file; no other tests were modified.No existing source files outside
src/lib/csrf.tswere modified.Acceptance criteria coverage
CSRF_SECRETunset, token minting/verification fails closed (no tokens issued, or a clear error), and the literal fallback string is gone fromsrc/lib/csrf.ts(src/lib/__tests__/csrf.test.ts— "fails closed (throws) when CSRF_SECRET is unset", "fails closed when CSRF_SECRET is unset, even with a previously valid token", "returns 403 when CSRF_SECRET is unset", "no longer contains the hardcoded fallback secret")CSRF_SECRETset, current behavior is unchanged (tokens mint and verify) (csrf.test.ts— "mints an HMAC-SHA256 token when CSRF_SECRET is set", "accepts a valid token minted with the configured secret", "invokes the handler when CSRF validation passes")csrf.test.ts— "rejects a token signed with the old hardcoded fallback secret")scripts/validate-env.js(or the equivalent env documentation) requiresCSRF_SECRET(scripts/validate-env.jsschema entry; verified exit 1 without / exit 0 with; documented in.env.example)npm run typecheck,npm test, andnpm run lintpass — not fully satisfiable in this repository as committed; see Test plan. This change introduces zero new failures across all three gates (verified by baseline comparison).Test plan
npx jest src/lib/__tests__/csrf.test.ts— 14/14 passing (14 new tests)npm test— 834/973 passing, 139 failing (65 suites) — all failures pre-existing: the identical baseline without my changes is 828/973 passing, 145 failing (66 suites), so this change introduces 0 new failures and adds 6 previously-failing assertions to the passing set. Pre-existing failures are unrelated (e.g. theviemmock lacksdefineChain; several component/error-boundary suites).npm run typecheck— 36 errors, all pre-existing in 8 untouched files (src/stories/ResponsiveContainerExample.stories.ts,src/lib/toast.ts,src/components/PropertyCard.tsx,src/components/TransactionConfirmation.tsx,src/app/compare/page.tsx, and 3 others). Verified by stashing my changes: identical 36 errors without them.0errors in or caused by my files.npm run lint— cannot run on main as committed:eslint.config.mjsimportseslint-plugin-jsdoc, which is not declared inpackage.json, the lockfile, or installed —ERR_MODULE_NOT_FOUNDon a clean checkout. After installing it locally with--no-save(no repo changes), my changed files lint clean:src/lib/csrf.ts,src/lib/__tests__/csrf.test.ts,scripts/validate-env.jsall exit 0 (the repo-wide run reports ~3555 pre-existing violations).npm run build— blocked by the pre-existing typecheck errors above (buildrunstypecheckfirst).npx next buildadditionally fails on a pre-existingioredisbundling issue (Can't resolve 'dns'/'fs'/'net') in untouched code paths.Env vars / Notes
CSRF_SECRET(HMAC signing secret for CSRF tokens). Generate withopenssl rand -hex 32or an equivalent CSPRNG.CSRF_SECRETbefore deploying this change; without it, the CSRF token endpoint returns an error, allwithCsrf-protected write routes return 403, andnpm run validate:envexits 1. No migration is required — this is a configuration requirement, not a data-shape change.src/config/env/schema.ts(the Zod env schema is loaded at module init across many modules; makingCSRF_SECRETrequired there would breaknext buildfor environments still mid-migration, which is exactly whatvalidate-env.jsis for), and the pre-existingany-related lint state elsewhere in the repo.