Skip to content

fix(auth): read JWT_SECRET per call instead of memoizing it - #405

Merged
cevheri merged 2 commits into
mainfrom
fix/jwt-secret-cache-test-seam
Aug 16, 2026
Merged

cevheri merged 2 commits into
mainfrom
fix/jwt-secret-cache-test-seam

Conversation

@yusuf-gundogdu

@yusuf-gundogdu yusuf-gundogdu commented Aug 16, 2026 •

Copy link
Copy Markdown
Member

What

src/lib/auth.ts no longer memoizes the JWT secret: signJWT and verifyJWT call
getJwtSecret() directly and the module-level _jwtSecret cache is gone, so there is no state left
to reset and no resetJwtSecretCache seam.

tests/unit/lib/auth-jwt-config.test.ts runs the successful dev-fallback case first and gains a
secret-rotation case. The file is now a regression test for the memo instead of a victim of it:
reintroduce a cache in auth.ts and these cases fail immediately, in any process layout.

Why

The original diagnosis on this branch was right and the defect is real. getJwtSecret() reads the
environment on every call, but auth.ts wrapped it in a module-level memo that only ever holds a
successful read. Once anything in the process has signed a token, the cached key answers and the
guard is never reached — so "JWT_SECRET missing in production" and "JWT_SECRET shorter than 32
characters" cannot be observed.

Reproduced on an otherwise untouched tree, with only the file's own success case moved to the top:

(fail) auth JWT_SECRET config guard > throws AuthConfigError when JWT_SECRET is missing in production
(fail) auth JWT_SECRET config guard > throws AuthConfigError when JWT_SECRET is shorter than 32 characters
 1 pass, 2 fail

Those are exactly the two failures reported here. Whether they surface under bun run test depends
on file load order and on bun re-evaluating auth.ts when this file mocks next/headers, which is
why the same command is green on one machine and red on another. A guard whose observability rests
on that is broken either way — and a cache that can hide a misconfiguration is not only a test
problem.

Why the memo goes rather than gaining a seam

The first revision exported resetJwtSecretCache(). That follows a real convention here
(resetSecurityConfigWarnings, resetAgentCapabilityCache, and resetCookieSecurityWarning in
this very file), but @/lib/auth is not comparable to those modules in one respect: 36 test files
replace it wholesale
with an object literal (tests/api/auth/login.test.ts:10), against 1 for the
cited precedents. None of them carries the new name, so a load order that puts one first turns this
file's await import("@/lib/auth") into a link-time Export named 'resetJwtSecretCache' not found
— the failure mode the file's own header comment already documents for next/headers. That trades
an order-dependent assertion failure for an order-dependent link failure with a wider trigger.

Removing the state avoids both:

  • auth-env.ts already documents the reader as stateless — "reads the environment on every call".
  • oidc.ts, drive-token.ts and storage/encryption.ts all call it fresh; the memo was the odd one
    out (proxy.ts keeps its own, in a separate runtime).
  • The cost is one TextEncoder pass per sign, against the HMAC that immediately follows.
  • Laziness is unchanged: a misconfigured secret still throws AuthConfigError at sign/verify time,
    never at module load, so the login route keeps turning it into a clear 503.
  • A rotated JWT_SECRET now takes effect without a process restart — pinned by the new fourth case.

src/lib/auth-compare.ts referenced the removed memo by line number; its comment now points at
proxy.ts and records why the key there is load-bearing (it is generated, not read, so re-deriving
it per call would make two digests of the same input differ).

Verification

All six gates locally: format, lint (0 errors), typecheck, knip, test, build.
bun run test — the command in the original report — is green end to end: 7070 core, then
All 30 groups passed!. Coverage holds the gate: check-coverage: OK — 36004/36004 lines (100.00%).

Out of scope

The four scripts/lib/pack-standalone-zip.sh failures in the original report are a missing local
7z, as noted there — unrelated to this change and untouched.

yusuf-gundogdu and others added 2 commits August 17, 2026 02:15
getJwtSecret() memoizes its first successful read in a module-level variable.
A suite that shares one process therefore cannot observe the guard's throwing
cases once any earlier file has signed a token: the cached key answers before
the guard is reached.

`bun run test` runs tests/unit in a single process, so both throw cases in
auth-jwt-config.test.ts failed there while passing in isolation. The file's own
header note ("this file runs in its own process") described the workaround
rather than the fix.

resetJwtSecretCache() follows the test-seam pattern already used for
process-wide state in this repository (resetSecurityConfigWarnings,
resetAgentCapabilityCache). The test clears the memo around every case instead
of assuming an empty one.

`bun run test` now passes end to end.
The module-level `_jwtSecret` memo in src/lib/auth.ts only ever held a SUCCESSFUL
read, so once anything in the process had signed a token the cached key answered
and the config guard was never reached. That is what made the two throwing cases
in tests/unit/lib/auth-jwt-config.test.ts unobservable: move the file's own
dev-fallback success case to the top on an otherwise untouched tree and both fail.

Drop the memo rather than add a way to reset it. auth-env.ts already documents the
reader as stateless ("reads the environment on every call"), and oidc.ts,
drive-token.ts and storage/encryption.ts all call it fresh - the cache was the
exception, worth one TextEncoder pass against the HMAC that follows it. Laziness is
unchanged: a misconfigured secret still throws AuthConfigError at sign/verify time,
never at module load, so the login route keeps turning it into a 503.

The earlier revision of this branch exported a resetJwtSecretCache() test seam. It
works, but 25 files under tests/api/ replace @/lib/auth wholesale with an object
literal that does not carry the new name, so an unlucky load order turns the import
into a link-time "Export named 'resetJwtSecretCache' not found" - a wider trigger
than the assertion failure it fixes.

The test file now runs the success case first and pins a secret rotation, so it
fails if a memo is ever reintroduced instead of being silenced by one.
@cevheri
cevheri force-pushed the fix/jwt-secret-cache-test-seam branch from 734bb2b to 382adf4 Compare August 16, 2026 23:25
@cevheri cevheri changed the title test(auth): reset the JWT secret memo between cases fix(auth): read JWT_SECRET per call instead of memoizing it Aug 16, 2026
@sonarqubecloud

Copy link
Copy Markdown

@cevheri
cevheri merged commit 53c351c into main Aug 16, 2026
23 checks passed
@cevheri
cevheri deleted the fix/jwt-secret-cache-test-seam branch August 16, 2026 23:33
cevheri added a commit that referenced this pull request Aug 17, 2026
#405 landed on main and supersedes this branch's first commit. Both started from
the same observation - the module-level JWT secret memo made the config guard's
throwing cases unobservable in a shared-process run - but reached it differently:
this branch added a resetJwtSecretCache() test seam, main dropped the memo
outright so there is no cache to reset.

Main's answer wins here, and not only because it landed first. 25 files under
tests/api/ replace @/lib/auth wholesale with an object literal that does not
carry the new export, so the seam turns an assertion failure into a link-time
"Export named 'resetJwtSecretCache' not found" under an unlucky load order - a
wider trigger than the problem it fixed.

Resolution:

  src/lib/auth.ts                        git auto-merged this file and produced
                                         code that does not compile: main deleted
                                         _jwtSecret, this branch added a function
                                         assigning to it, and the two edits did
                                         not overlap textually. Took main's file.
                                         The orphaned JSDoc ("the memo above")
                                         went with it.

  tests/unit/lib/auth-jwt-config.test.ts conflicted. Took main's file: it orders
                                         the success case first and pins a secret
                                         rotation, so a reintroduced memo fails
                                         the suite instead of being reset around.

What remains of this branch against main is the agent tool-layer change alone.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants