Skip to content

fix: generate the search token with a CSPRNG - #2217

Merged
felladrin merged 1 commit into
felladrin:mainfrom
snowyukitty:fix/search-token-csprng
Jul 27, 2026
Merged

fix: generate the search token with a CSPRNG#2217
felladrin merged 1 commit into
felladrin:mainfrom
snowyukitty:fix/search-token-csprng

Conversation

@snowyukitty

Copy link
Copy Markdown
Contributor

Description

Fixes #2181.

server/searchToken.ts mints the shared secret behind every /search/* and /inference request — the client hashes it with argon2id as VITE_SEARCH_TOKEN, and verifyTokenAndRateLimit authorizes by verifying that hash against getSearchToken(). It was generated with Math.random().toString(36).substring(2), so the secret came from a non-cryptographic PRNG and carried at most the 52 bits a double's mantissa can hold. The encoded string is also variable length — 7 to 15 characters over 2M samples on Node 22, not the ~11 it looks like.

Now it's 32 bytes from node:crypto, hex-encoded. The client already reaches for the CSPRNG (crypto.getRandomValues in accessKey.ts and searchTokenHash.ts), so this brings the server side in line.

docs/security.md called out the weak source under "Security Best Practices", so that line is updated in the same commit.

Scope note: this changes the quality of the secret, not its lifetime — regenerateSearchToken() still only runs on build, and getSearchToken() still returns the cached temp-file value.

The other Math.random() call sites (history.ts, logEntries.ts, querySuggestions.ts, wllama.ts, shared/openaiModels.ts, and the backoff jitter in server/utils/streamUtils.ts) are all non-security and left alone.

The new test pins the invariant rather than the implementation: it spies on Math.random, asserts it is never reached, and checks two successive tokens differ. On the previous implementation it fails with expected "random" to not be called at all, but actually been called 2 times.

Type of Change

  • Bug fix
  • New feature
  • Documentation
  • Other (refactor, build, chore)

Checklist

  • npm run lint passes
  • Tests pass (npm run test), with tests added where it made sense
Security, performance, or breaking changes? Expand if relevant.

Security-positive, no migration needed. The token moves from ≤52 bits of Math.random() output to 256 bits of CSPRNG output, and from a 7–15 character base-36 string to a fixed 64-character hex string. Existing deployments pick up the new token on their next build, exactly as they already do today — the temp file is rewritten by regenerateSearchToken() and re-inlined as VITE_SEARCH_TOKEN. argon2id takes the longer password unchanged (verified with a real hash-wasm round trip using the client's exact parameters).

The search token is the shared secret behind every /search/* and /inference
request: the client hashes it with argon2id and the server verifies that hash.
Math.random().toString(36).substring(2) drew it from a non-cryptographic PRNG
and produced a 7-15 character base-36 string, so the secret carried at most the
52 bits a double's mantissa can hold.

- Generate the token from 32 bytes of node:crypto randomness, hex-encoded.
- Cover the invariant with a test that fails on the previous implementation
  because Math.random is reached.
- Update docs/security.md, which flagged the weak source.

@felladrin felladrin left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent! Thanks for another contribution!

@felladrin
felladrin merged commit 742b810 into felladrin:main Jul 27, 2026
3 checks passed
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.

Search/CSRF token uses Math.random() instead of a CSPRNG

2 participants