fix(security): suppress the CodeQL password-hash false positive on sha256Hex - #5982
Conversation
CodeQL fails #5963 (the v0.7.46 release PR) with 2 high-severity js/insufficient-password-hash alerts on packages/security/src/hash.ts. Both are false positives that surfaced now because d362e68 added sha256Base64Url to this file — touching it made every pre-existing taint path ending here count as 'new code changed by this pull request'. The flagged lines and all their callers are unchanged. Neither taint source is a human password: - hashApiKey passes an API key built from generateSecureToken(24) — 192 bits of randomBytes. A fast digest is the right construction for indexed lookup; a slow KDF would run on every authenticated request and buy nothing against an unsearchable keyspace. - passwordSlot passes an already-encrypted value, hashed to an 8-char discriminator that invalidates deployment auth tokens when the password changes — not credential storage. User credentials never reach this helper; Better Auth owns them and applies its own KDF. Suppress with the inline lgtm comments this repo already uses for the same class of judgement (see uuidV5 in ee/workspace-forking/lib/remap/block-identity.ts), and document the invariant on sha256Hex so a future caller knows a genuine password belongs in Better Auth, not here. Only the two flagged lines are suppressed — sha256Base64Url is left unannotated so a real finding there would still surface.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryLow Risk Overview Adds inline Reviewed by Cursor Bugbot for commit 491c318. Bugbot is set up for automated code reviews on this repo. Configure here. |
Greptile SummaryThis PR documents the intended non-password uses of
Confidence Score: 5/5The PR appears safe to merge because it changes only comments and accurately preserves the helper’s existing runtime behavior and usage contract. The documented invariant matches current callers, and the inline annotations do not alter hashing inputs, outputs, control flow, or security boundaries.
|
| Filename | Overview |
|---|---|
| packages/security/src/hash.ts | Adds accurate usage guidance and targeted static-analysis suppressions without changing hashing behavior. |
Reviews (1): Last reviewed commit: "fix(security): suppress the CodeQL passw..." | Re-trigger Greptile
Summary
CodeQL fails #5963 (the v0.7.46 release PR) with 2 high-severity
js/insufficient-password-hashalerts onpackages/security/src/hash.ts:12and:14. Both are false positives, but the check is required, so the release is blocked.Why they appeared now.
d362e6816e(setup wizard) addedsha256Base64Urlto this file. Touching it made every pre-existing taint path that ends here count as "new code changed by this pull request" — CodeQL's own summary hints at this ("Alerts not introduced by this pull request might have been detected because the code changes were too large"). The flagged lines are insha256Hex, which was not modified, and neither were any of its callers.Why they're false positives. Neither taint source is a human-chosen password:
hashApiKey(plainKey)generateSecureToken(24)=randomBytes(24)— 192 bits. A fast digest is the correct construction for an indexedWHERE key_hash = $1lookup; a slow KDF would run on every authenticated request and buy nothing against a keyspace that cannot be searched.passwordSlot(encryptedPassword)User credentials never reach this helper — Better Auth owns them and applies its own KDF. I verified there is no other password-shaped caller.
Approach
Inline
// lgtm[js/insufficient-password-hash]on the two flagged lines, matching the convention this repo already uses for the same class of judgement (uuidV5inapps/sim/ee/workspace-forking/lib/remap/block-identity.tssuppressesjs/weak-cryptographic-algorithmthe same way; there are currently 0 open alerts for that rule, so the mechanism works).The
sha256Hexdoc comment now states the invariant — high-entropy secrets and content digests only, genuine passwords go to Better Auth — so a future caller has the rule in front of them rather than inheriting a silent suppression.sha256Base64Urlis deliberately left unannotated even though it is the same primitive: it was not flagged, and a speculative suppression there would hide a real finding later.Type of Change
Testing
packages/securitysuite passes (35 tests),bun run lintclean. No behavior change — comments only.Checklist