Use the configured secret when signing and verifying CSRF tokens#584
Use the configured secret when signing and verifying CSRF tokens#584
Conversation
Co-authored-by: sergiodxa <1312018+sergiodxa@users.noreply.github.com> Agent-Logs-Url: https://github.com/sergiodxa/remix-utils/sessions/fc7d0948-723d-4c89-94cf-e54238606193
Co-authored-by: sergiodxa <1312018+sergiodxa@users.noreply.github.com> Agent-Logs-Url: https://github.com/sergiodxa/remix-utils/sessions/fc7d0948-723d-4c89-94cf-e54238606193
Co-authored-by: sergiodxa <1312018+sergiodxa@users.noreply.github.com> Agent-Logs-Url: https://github.com/sergiodxa/remix-utils/sessions/fc7d0948-723d-4c89-94cf-e54238606193
There was a problem hiding this comment.
Pull request overview
Updates the server-side CSRF token signing to incorporate the configured secret value into the signature, and adds a regression test ensuring tokens signed with one secret are rejected when validated with another.
Changes:
- Replace plain hashing with HMAC-SHA256 keyed by the configured
secretfor CSRF token signatures. - Add a test that validates secret-specific signing/verification behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/server/csrf.ts |
Switches CSRF signing from hash-only to secret-keyed HMAC signing. |
src/server/csrf.test.ts |
Adds coverage to ensure tokens are rejected when validated with a different secret. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let encoder = new TextEncoder(); | ||
| return encodeBase64url(hmac(SHA256, encoder.encode(this.secret), encoder.encode(token))); |
There was a problem hiding this comment.
verifySignature compares the provided signature to the expected signature using plain string equality. For authentication/verification primitives like HMAC, this can leak information via timing differences. Consider switching to a timing-safe comparison (e.g., compare decoded bytes in constant time, or reuse the repo’s timing-safe helper pattern) when checking the signature.
There was a problem hiding this comment.
Applied in c96353b. CSRF signature verification now uses a timing-safe comparison, and I also fixed the CSRF test formatting so the Code Quality check passes.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: sergiodxa <1312018+sergiodxa@users.noreply.github.com> Agent-Logs-Url: https://github.com/sergiodxa/remix-utils/sessions/c5e1ab17-b229-448b-867e-6801c944186b
The
CSRFclass accepted asecretoption but did not incorporate its value into the token signature. As a result, signed tokens were effectively derived from the token payload alone instead of the configured secret.Signing behavior
secret.secretis configured.Verification behavior
secret.Regression coverage
Original prompt
⌨️ Start Copilot coding agent tasks without leaving your editor — available in VS Code, Visual Studio, JetBrains IDEs and Eclipse.