From dcb2e6d29cf4c986ff8980dab50bcfcb8110a749 Mon Sep 17 00:00:00 2001 From: Taesu <166604494+bytaesu@users.noreply.github.com> Date: Wed, 20 May 2026 01:35:44 +0900 Subject: [PATCH] fix(cookies): percent-encode values on Cookie header serialize (#9631) --- .changeset/cookie-serialize-validate.md | 6 +++ .../better-auth/src/cookies/cookie-utils.ts | 45 +++++++++++++------ .../better-auth/src/cookies/cookies.test.ts | 24 ++++++++++ .../better-auth/src/plugins/bearer/index.ts | 11 ++--- packages/electron/src/cookies.ts | 16 +++---- packages/electron/test/electron.test.ts | 41 +++++++++++++++++ 6 files changed, 115 insertions(+), 28 deletions(-) create mode 100644 .changeset/cookie-serialize-validate.md diff --git a/.changeset/cookie-serialize-validate.md b/.changeset/cookie-serialize-validate.md new file mode 100644 index 0000000000..099d42ce84 --- /dev/null +++ b/.changeset/cookie-serialize-validate.md @@ -0,0 +1,6 @@ +--- +"better-auth": patch +"@better-auth/electron": patch +--- + +Cookie values containing characters outside the bare cookie-octet range (such as `;`, `"`, or `\`) are now percent-encoded into the `Cookie` header. They were previously dropped on re-serialization, which could break flows that store structured values in cookies. diff --git a/packages/better-auth/src/cookies/cookie-utils.ts b/packages/better-auth/src/cookies/cookie-utils.ts index b05cad1fd9..1be8d3e179 100644 --- a/packages/better-auth/src/cookies/cookie-utils.ts +++ b/packages/better-auth/src/cookies/cookie-utils.ts @@ -1,4 +1,5 @@ function tryDecode(str: string): string { + if (str.indexOf("%") === -1) return str; try { return decodeURIComponent(str); } catch { @@ -101,13 +102,13 @@ export function parseSetCookieHeader( const [nameValue, ...attributes] = parts; const [name, ...valueParts] = (nameValue || "").split("="); - const value = valueParts.join("="); + const value = unquoteCookieValue(valueParts.join("=")); - if (!name || value === undefined) { + if (!name) { return; } - const decodedValue = value.includes("%") ? tryDecode(value) : value; + const decodedValue = tryDecode(value); const attrObj: CookieAttributes = { value: decodedValue }; attributes.forEach((attribute) => { @@ -178,7 +179,7 @@ export function toCookieOptions( * * @see https://datatracker.ietf.org/doc/html/rfc7230#section-3.2.6 */ -const cookieNameRegex = /^[\w!#$%&'*.^`|~+-]+$/; +export const cookieNameRegex = /^[\w!#$%&'*.^`|~+-]+$/; /** * Cookie-value char set per RFC 6265 §4.1.1, plus space and comma. @@ -188,6 +189,18 @@ const cookieNameRegex = /^[\w!#$%&'*.^`|~+-]+$/; */ const cookieValueRegex = /^[ !#-:<-[\]-~]*$/; +/** + * Strip surrounding double-quotes per RFC 6265 §4.1.1 quoted-string form. + * + * @see https://datatracker.ietf.org/doc/html/rfc6265#section-4.1.1 + */ +function unquoteCookieValue(value: string): string { + if (value.length < 2 || !value.startsWith('"') || !value.endsWith('"')) { + return value; + } + return value.slice(1, -1); +} + /** * Trim leading/trailing OWS (space / horizontal tab) per RFC 7230 §3.2.3. * Narrower than `String.prototype.trim()`, which strips CR/LF and other @@ -225,12 +238,10 @@ export function parseCookies(cookie: string): Map { const eq = chunk.indexOf("="); if (eq === -1) continue; const key = trimOWS(chunk.slice(0, eq)); - let val = trimOWS(chunk.slice(eq + 1)); - if (val.length >= 2 && val[0] === '"' && val[val.length - 1] === '"') { - val = val.slice(1, -1); - } + const val = unquoteCookieValue(trimOWS(chunk.slice(eq + 1))); + // Validate wire-format octets, then decode to the semantic value. if (cookieNameRegex.test(key) && cookieValueRegex.test(val)) { - cookieMap.set(key, val); + cookieMap.set(key, tryDecode(val)); } } return cookieMap; @@ -250,10 +261,14 @@ export function setRequestCookie( value: string, ): void { const cookieMap = parseCookies(headers.get("cookie") || ""); - cookieMap.set(name, value); + if (cookieNameRegex.test(name)) { + cookieMap.set(name, value); + } headers.set( "cookie", - Array.from(cookieMap, ([k, v]) => `${k}=${v}`).join("; "), + Array.from(cookieMap, ([k, v]) => `${k}=${encodeURIComponent(v)}`).join( + "; ", + ), ); } @@ -272,12 +287,16 @@ export function applySetCookies( const cookieMap = parseCookies(target.get("cookie") || ""); for (const setCookie of setCookieValues) { for (const [name, attr] of parseSetCookieHeader(setCookie)) { - cookieMap.set(name, attr.value); + if (cookieNameRegex.test(name)) { + cookieMap.set(name, attr.value); + } } } target.set( "cookie", - Array.from(cookieMap, ([k, v]) => `${k}=${v}`).join("; "), + Array.from(cookieMap, ([k, v]) => `${k}=${encodeURIComponent(v)}`).join( + "; ", + ), ); } diff --git a/packages/better-auth/src/cookies/cookies.test.ts b/packages/better-auth/src/cookies/cookies.test.ts index a2287d2d50..08e8cc1ba4 100644 --- a/packages/better-auth/src/cookies/cookies.test.ts +++ b/packages/better-auth/src/cookies/cookies.test.ts @@ -411,6 +411,18 @@ describe("cookie-utils setRequestCookie", () => { "valid=1; locale=en; better-auth.session_token=abc", ); }); + + it("percent-encodes reserved cookie-octet bytes when serializing", () => { + const headers = new Headers({ cookie: "locale=en" }); + setRequestCookie(headers, "session", "foo;bar=baz"); + expect(headers.get("cookie")).toBe("locale=en; session=foo%3Bbar%3Dbaz"); + }); + + it("treats input as semantic and percent-encodes literal double-quotes", () => { + const headers = new Headers(); + setRequestCookie(headers, "token", '"abc"'); + expect(headers.get("cookie")).toBe("token=%22abc%22"); + }); }); describe("getSessionCookie", async () => { @@ -1574,6 +1586,18 @@ describe("applySetCookies", () => { applySetCookies(headers, ["a=new; Path=/"]); expect(headers.get("cookie")).toBe("a=new; b=keep"); }); + + it("re-encodes Set-Cookie values containing reserved bytes on wire join", () => { + const headers = new Headers({ cookie: "session=safe" }); + applySetCookies(headers, ["pref=foo%3Bbar=hello; Path=/"]); + expect(headers.get("cookie")).toBe("session=safe; pref=foo%3Bbar%3Dhello"); + }); + + it("strips RFC 6265 quoted-string wrapping from Set-Cookie values", () => { + const headers = new Headers(); + applySetCookies(headers, ['token="abc"; Path=/']); + expect(headers.get("cookie")).toBe("token=abc"); + }); }); describe("expireCookie", () => { diff --git a/packages/better-auth/src/plugins/bearer/index.ts b/packages/better-auth/src/plugins/bearer/index.ts index d345301245..30b91dcae6 100644 --- a/packages/better-auth/src/plugins/bearer/index.ts +++ b/packages/better-auth/src/plugins/bearer/index.ts @@ -70,21 +70,18 @@ export const bearer = (options?: BearerOptions | undefined) => { return; } - let signedToken: string; let decodedToken: string; if (token.includes(".")) { - const isEncoded = token.includes("%"); - signedToken = isEncoded ? token : encodeURIComponent(token); - decodedToken = isEncoded ? tryDecode(token) : token; + decodedToken = token.includes("%") ? tryDecode(token) : token; } else { if (options?.requireSignature) { return; } - signedToken = ( + const signed = ( await serializeSignedCookie("", token, c.context.secret) ).replace("=", ""); - decodedToken = tryDecode(signedToken); + decodedToken = tryDecode(signed); } try { const isValid = await createHMAC( @@ -109,7 +106,7 @@ export const bearer = (options?: BearerOptions | undefined) => { setRequestCookie( headers, c.context.authCookies.sessionToken.name, - signedToken, + decodedToken, ); return { context: { diff --git a/packages/electron/src/cookies.ts b/packages/electron/src/cookies.ts index 3a53c4df7e..c60b8b10ca 100644 --- a/packages/electron/src/cookies.ts +++ b/packages/electron/src/cookies.ts @@ -1,4 +1,4 @@ -import { parseSetCookieHeader } from "better-auth/cookies"; +import { cookieNameRegex, parseSetCookieHeader } from "better-auth/cookies"; interface StoredCookie { value: string; @@ -40,13 +40,13 @@ export function getCookie(cookie: string) { try { parsed = JSON.parse(cookie) as Record; } catch (_e) {} - const toSend = Object.entries(parsed).reduce((acc, [key, value]) => { - if (value.expires && new Date(value.expires) < new Date()) { - return acc; - } - return `${acc}; ${key}=${value.value}`; - }, ""); - return toSend; + const pairs: string[] = []; + for (const [key, value] of Object.entries(parsed)) { + if (value.expires && new Date(value.expires) < new Date()) continue; + if (!cookieNameRegex.test(key)) continue; + pairs.push(`${key}=${encodeURIComponent(value.value)}`); + } + return pairs.join("; "); } /** diff --git a/packages/electron/test/electron.test.ts b/packages/electron/test/electron.test.ts index 7911ced3d8..abe615452c 100644 --- a/packages/electron/test/electron.test.ts +++ b/packages/electron/test/electron.test.ts @@ -10,6 +10,7 @@ import { getMigrations } from "better-auth/db/migration"; import { beforeEach, describe, expect, vi } from "vitest"; import { authenticate, kElectron } from "../src/authenticate"; import { electronClient } from "../src/client"; +import { getCookie } from "../src/cookies"; import { ELECTRON_ERROR_CODES } from "../src/error-codes"; import { electron } from "../src/index"; import { fetchUserImage, normalizeUserOutput } from "../src/user"; @@ -2223,3 +2224,43 @@ describe("Electron", () => { }); }); }); + +describe("cookies getCookie", () => { + it("serializes stored cookies into a Cookie header string", () => { + const stored = JSON.stringify({ + "better-auth.session_token": { value: "abc", expires: null }, + }); + expect(getCookie(stored)).toBe("better-auth.session_token=abc"); + }); + + it("joins multiple stored cookies with `; ` without a leading separator", () => { + const stored = JSON.stringify({ + a: { value: "1", expires: null }, + b: { value: "2", expires: null }, + }); + expect(getCookie(stored)).toBe("a=1; b=2"); + }); + + it("percent-encodes stored values containing reserved cookie-octet bytes", () => { + const stored = JSON.stringify({ + session: { value: "safe", expires: null }, + pref: { value: "foo;bar=baz", expires: null }, + }); + expect(getCookie(stored)).toBe("session=safe; pref=foo%3Bbar%3Dbaz"); + }); + + it("skips stored entries whose name violates the cookie-name token", () => { + const stored = JSON.stringify({ + session: { value: "safe", expires: null }, + "bad name": { value: "x", expires: null }, + }); + expect(getCookie(stored)).toBe("session=safe"); + }); + + it("skips expired entries", () => { + const stored = JSON.stringify({ + session: { value: "abc", expires: new Date(0).toISOString() }, + }); + expect(getCookie(stored)).toBe(""); + }); +});