fix(security): stop caching tokens.json/users.json in memory#79
Merged
Conversation
Both FileTokenStore and FileUserStore cached their JSON file's contents in memory on first read and never invalidated it. Their own doc comments stated the assumption plainly: "the daemon is single-process". That's false in practice — `llmux token create`/`revoke`, `user reset-passphrase`, etc. all run as separate short-lived CLI processes against the same tokens.json/users.json files while a daemon may already be running. Consequence, confirmed live while verifying fix/loopback-auth-bypass: a token minted via the CLI while a daemon was already running was NOT recognized by that daemon until restart, and revoking a token via the CLI did NOT take effect against an already-running daemon until restart either — the exact opposite of what revocation is supposed to guarantee (immediate access cutoff). Mutations made through the daemon's own in-process handlers (e.g. the web UI's revoke button) worked fine, since they share the same cache instance — only cross-process mutations were invisible. Fix: stop caching. load() now always re-reads and re-parses the file. Both files are a handful of tokens/users — re-parsing on every call is cheap next to the rest of a request, and it's a much smaller, more correct fix than adding cross-process invalidation (file watching, a reload signal, etc.) for what a single readFileSync already solves. Verified end-to-end against a running daemon: minted a token via a separate CLI process — immediately usable (200) against the already- running daemon. Revoked that same token via another separate CLI process — the running daemon immediately rejected it (401), no restart. Normal auth (GET /, GET /account with a valid bearer token) confirmed unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Au8T9RPCb6wbgiEecQrBfZ
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
Found while verifying
fix/loopback-auth-bypass(#77):FileTokenStoreandFileUserStorecached their JSON file's contents in memory on first read and never invalidated it. Their own doc comments stated the assumption plainly — "the daemon is single-process" — which is false in practice:llmux token create/revoke,user reset-passphrase, etc. all run as separate short-lived CLI processes against the same files while a daemon may already be running.Confirmed live: a token minted via the CLI while a daemon was already running was not recognized by that daemon until restart, and revoking a token via the CLI did not take effect against an already-running daemon until restart — the exact opposite of what revocation is supposed to guarantee. Mutations through the daemon's own in-process handlers (e.g. the web UI's revoke button) worked fine, since they share the same cache instance; only cross-process mutations were invisible.
Fix
Stop caching.
load()now always re-reads and re-parses the file. Both files are a handful of tokens/users — re-parsing on every call is cheap next to the rest of a request, and it's a much smaller, more correct fix than adding cross-process invalidation (file watching, a reload signal) for what a singlereadFileSyncalready solves.Test plan
npm run typecheck/npm run buildclean200) against the already-running daemon401), no restartGET /,GET /accountwith a valid bearer token) confirmed unaffected🤖 Generated with Claude Code
https://claude.ai/code/session_01Au8T9RPCb6wbgiEecQrBfZ