Skip to content

Use the configured secret when signing and verifying CSRF tokens #584

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
Copilot wants to merge 6 commits into
base: main
Choose a base branch
from
Draft

Use the configured secret when signing and verifying CSRF tokens #584

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
afc620e
Initial plan
Copilot Mar 24, 2026
d35b1a5
fix: use csrf secret when signing tokens
Copilot Mar 24, 2026
7973fee
chore: remove npm lockfile artifact
Copilot Mar 24, 2026
e8b052c
test: clarify csrf secret regression case
Copilot Mar 24, 2026
9e9c227
Update src/server/csrf.test.ts
sergiodxa Mar 24, 2026
c96353b
fix: use timing-safe csrf signature comparison
Copilot Mar 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions src/server/csrf.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,21 @@ describe("CSRF", () => {
);
});

test("rejects CSRF tokens signed with a different secret", async () => {
let token = new CSRF({ cookie, secret: "my-secret" }).generate();

let formData = new FormData();
formData.set("csrf", token);

let headers = new Headers({
cookie: await cookie.serialize(token),
});

expect(
new CSRF({ cookie, secret: "different-secret" }).validate(formData, headers),
).rejects.toThrow(new CSRFError("tampered_token_in_cookie", "Tampered CSRF token in cookie."));
Comment thread
sergiodxa marked this conversation as resolved.
});

test("commits the token to a cookie", async () => {
let [token, cookieHeader] = await csrf.commitToken();
let parsedCookie = await cookie.parse(cookieHeader);
Expand Down
6 changes: 4 additions & 2 deletions src/server/csrf.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,8 @@
* @author [Sergio Xalambrí](https://sergiodxa.com)
* @module Server/CSRF
*/
import { sha256 } from "@oslojs/crypto/sha2";
import { hmac } from "@oslojs/crypto/hmac";
import { SHA256 } from "@oslojs/crypto/sha2";
import { encodeBase64url } from "@oslojs/encoding";
import type { Cookie } from "react-router";
import { randomString } from "../common/crypto.js";
Expand Down Expand Up @@ -343,7 +344,8 @@ export class CSRF {

private sign(token: string) {
if (!this.secret) return token;
return encodeBase64url(sha256(new TextEncoder().encode(token)));
let encoder = new TextEncoder();
return encodeBase64url(hmac(SHA256, encoder.encode(this.secret), encoder.encode(token)));
Comment on lines +358 to +359
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Owner

@sergiodxa sergiodxa Mar 24, 2026

Choose a reason for hiding this comment

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

@copilot apply changes based on this feedback

Copy link
Copy Markdown
Author

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

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

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.

}

private verifySignature(token: string) {
Expand Down
Loading