fix(security): use timing-safe comparison for admin token#2185
fix(security): use timing-safe comparison for admin token#2185tessaherself wants to merge 2 commits intohuggingface:mainfrom
Conversation
Replace direct string comparison (===) with crypto.timingSafeEqual to prevent timing attacks on admin token verification. The existing comparison leaks token length and character-by-character match info through response time differences. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e4ee8abf78
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
src/lib/server/adminToken.ts
Outdated
| const bufA = Buffer.from(a.padEnd(Math.max(a.length, b.length))); | ||
| const bufB = Buffer.from(b.padEnd(Math.max(a.length, b.length))); |
There was a problem hiding this comment.
Padding both inputs with padEnd() changes the semantics of token validation: tokens that differ only by trailing spaces are now treated as equal (for example, "secret" and "secret " both become the same padded buffer). In checkToken(), this can incorrectly grant admin access for a malformed token value, so the constant-time path needs to retain an explicit original-length equality check (or otherwise avoid space-padding collisions) while still preventing timing leaks.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This feedback doesn't match the actual code. The safeCompare function uses an explicit length check (if (a.length !== b.length) return false;) before the timing-safe comparison — there is no padEnd anywhere in this implementation. The length gate ensures that only equal-length strings reach timingSafeEqual, which is the standard approach.
padEnd with spaces could treat "secret" and "secret " as equal. Check lengths first (non-secret) and only then do timingSafeEqual on equal-length buffers without padding. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 participant
Summary
===withcrypto.timingSafeEqualinAdminTokenManager.checkToken()safeCompare()helper that pads strings to equal length before constant-time comparisonChanges
src/lib/server/adminToken.ts: Add timing-safe comparisonTest plan
?token=...URL)🤖 Generated with Claude Code