fix(2fa): fail closed in production without TOTP_ENCRYPTION_KEY; accept AUTH_SECRET in dev - #45
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Mutation TestingScore: 70.82% (break threshold 60% — ✅ pass). 318 detected / 449 valid mutants (killed 191, timeout 127, survived 30, no-coverage 101). Full report. |
…pt AUTH_SECRET in dev The encryption-key fallback read NEXTAUTH_SECRET — the NextAuth v4 name. This app is on v5 and configures AUTH_SECRET, so the "dev convenience" fallback was dead: on Vercel neither TOTP_ENCRYPTION_KEY nor NEXTAUTH_SECRET is set, and every 2FA setup/verify threw at runtime. Rework getEncryptionKey to mirror getHmacSecret in auditHashChain.ts: - TOTP_ENCRYPTION_KEY first, always. - NEXTAUTH_SECRET stays honored in EVERY environment — the k8s templates still set it, and TOTP secrets encrypted under that fallback must remain decryptable. Dropping it would strand them. - AUTH_SECRET now works as the dev/test fallback, but deliberately does NOT count in production: 2FA fails closed with an actionable message until a dedicated key is set, instead of silently deriving 2FA crypto from the session-signing secret. No production ciphertexts are affected: with both lookup names unset, encryptSecret always threw, so nothing was ever encrypted under a key this change replaces. Tests pin all four behaviors (v4 fallback, v5 dev fallback, production fail-closed with AUTH_SECRET set, production NEXTAUTH_SECRET back-compat), and .env.example now documents the variable — it was documented nowhere. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…06 -> 2909, branches 87.38) Generated with `npm run readme:test-table` — its first run on this Windows machine, via the portability fix from #41. Both CI gates verified locally: check-coverage-claims and check-test-count pass against the fresh run. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review finding on the previous commit: the first lookup used ?? — but .env.example now ships TOTP_ENCRYPTION_KEY="" and an empty string is not nullish, so a copied-but-unfilled template value would block the NEXTAUTH_SECRET fallback entirely. In production on k8s (where NEXTAUTH_SECRET is the configured key) that combination would throw even though a valid legacy key was present. Use || instead: an empty string is never a valid key, and truthiness is what the mirrored getHmacSecret pattern already uses (`if (secret) return secret`). Pinned with a test that fails under ??. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…test Verified against a fresh full run: 2835 Jest green, both README gates pass, coverage percentages unchanged (the || swap adds no branch counts). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
MikkoNumminen
force-pushed
the
fix/totp-key-fallback-and-docs
branch
from
July 24, 2026 02:36
bf695b7 to
f46bbb1
Compare
Mutation TestingScore: 71.33% (break threshold 60% — ✅ pass). 321 detected / 450 valid mutants (killed 194, timeout 127, survived 28, no-coverage 101). Full report. |
This was referenced Jul 24, 2026
MikkoNumminen
added a commit
that referenced
this pull request
Jul 24, 2026
Merging #46 — pure k8s manifests and Helm chart — triggered a full Vercel production rebuild of identical app code, because k8s/ was in scope for the ignore command. Kubernetes is a different deployment target entirely; nothing under k8s/ can affect what Vercel builds. Verified against real history: the #46 merge diff now skips, the #45 (app code) diff still builds. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
1 participant
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.
Fixes the second production-config finding from the #39 follow-ups: 2FA was broken in production — every TOTP setup/verify threw at runtime.
Root cause
getEncryptionKey()readTOTP_ENCRYPTION_KEY ?? NEXTAUTH_SECRET. The second name is NextAuth v4; this app is on v5 and configuresAUTH_SECRET. The Vercel project sets neither v4 name nor a dedicated key, so the "dev convenience" fallback was dead code in the exact environment it was meant to help — confirmed byvercel env ls:AUTH_SECRETpresent in all three environments,TOTP_ENCRYPTION_KEY/NEXTAUTH_SECRETabsent everywhere. The variable was also documented nowhere — not in.env.example, not in k8s.Fix (mirrors
getHmacSecretinauditHashChain.ts)Precedence, chosen for decryptability of existing secrets:
TOTP_ENCRYPTION_KEY— always.NEXTAUTH_SECRET— honored in every environment. The k8s templates still set it, and TOTP secrets encrypted under that fallback must stay decryptable; dropping it would strand them.AUTH_SECRET— dev/test only. In production it deliberately does not count: 2FA fails closed with an actionable error until a dedicated key is set, rather than silently deriving 2FA crypto from the session-signing secret.No production ciphertexts are at risk: with both lookup names unset,
encryptSecretalways threw, so nothing was ever encrypted under a key this change replaces.Environment side (already done via CLI)
TOTP_ENCRYPTION_KEYadded to Vercel Production and Preview with the same value — Hobby shares one database, so a secret encrypted by a preview must decrypt in production and vice versa. Takes effect on the next deploy, i.e. when this merges.DIRECT_URLentries (Production, Development) removed — the loose end from fix(build): drop the inert directUrl from prisma.config.ts (unblocks preview deploys) #42.Tests
Four behaviors pinned in
totpCrypto.test.ts(22/22, plus the 37 2FA action tests green):NEXTAUTH_SECRET, devAUTH_SECRET, devAUTH_SECRET, productionTOTP_ENCRYPTION_KEY is not setNEXTAUTH_SECRET, production.env.examplenow documents the variable next toAUDIT_HMAC_SECRET, including the do-not-rotate warning.Second commit: README test table regenerated (2906 → 2909, branches 87.36 → 87.38) with
npm run readme:test-table— its first production use on Windows via the #41 fix. Both CI gates verified locally.Still open (not fixable from here)
MONGODB_URLis unset on Vercel, so the production audit trail silently no-ops (deferAuditreturns early;drainAuditOutboxreturns without draining, so if theaudit-use-outboxflag is ON, outbox rows accumulate in Postgres unboundedly). Needs a real MongoDB — e.g. an Atlas M0 free cluster — thennpx vercel env add MONGODB_URL productionwith the connection string./api/readyreportsmongodb: ok, latencyMs: 0when the URL is absent by design, so don't take that probe as evidence it's wired.🤖 Generated with Claude Code