From 9b65523d69797a778cd439ad5ee74d9ed1b16738 Mon Sep 17 00:00:00 2001 From: Tim Smart Date: Thu, 30 Jul 2026 11:31:47 +1200 Subject: [PATCH] Harden cookie attribute validation --- .changeset/eff-210-cookie-validation.md | 5 ++ packages/effect/src/unstable/http/Cookies.ts | 52 +++++++++++------ .../effect/test/unstable/http/Cookies.test.ts | 58 ++++++++++++++++++- 3 files changed, 94 insertions(+), 21 deletions(-) create mode 100644 .changeset/eff-210-cookie-validation.md diff --git a/.changeset/eff-210-cookie-validation.md b/.changeset/eff-210-cookie-validation.md new file mode 100644 index 00000000000..7050ceb33a6 --- /dev/null +++ b/.changeset/eff-210-cookie-validation.md @@ -0,0 +1,5 @@ +--- +"effect": patch +--- + +Validate cookie names, domains, and paths before constructing or serializing cookies. diff --git a/packages/effect/src/unstable/http/Cookies.ts b/packages/effect/src/unstable/http/Cookies.ts index f8307972424..a22a9b0ea71 100644 --- a/packages/effect/src/unstable/http/Cookies.ts +++ b/packages/effect/src/unstable/http/Cookies.ts @@ -426,6 +426,10 @@ export const isEmpty = (self: Cookies): boolean => Record.isEmptyRecord(self.coo // oxlint-disable-next-line no-control-regex const fieldContentRegExp = /^[\u0009\u0020-\u007e\u0080-\u00ff]+$/ +const cookieNameRegExp = /^[!#$%&'*+\-.^_`|~0-9A-Za-z]+$/ +// oxlint-disable-next-line no-control-regex +const cookieDomainRegExp = /^[\u0009\u0020-\u003a\u003c-\u007e\u0080-\u00ff]+$/ +const cookiePathRegExp = /^[\u0020-\u003a\u003c-\u007e]+$/ const CookieProto = { [CookieTypeId]: CookieTypeId, @@ -455,26 +459,10 @@ export function makeCookie( value: string, options?: Cookie["options"] | undefined ): Result.Result { - if (!fieldContentRegExp.test(name)) { - return Result.fail(CookiesError.fromReason("InvalidCookieName")) - } const encodedValue = encodeURIComponent(value) - if (encodedValue && !fieldContentRegExp.test(encodedValue)) { - return Result.fail(CookiesError.fromReason("InvalidCookieValue")) - } - - if (options !== undefined) { - if (options.domain !== undefined && !fieldContentRegExp.test(options.domain)) { - return Result.fail(CookiesError.fromReason("InvalidCookieDomain")) - } - - if (options.path !== undefined && !fieldContentRegExp.test(options.path)) { - return Result.fail(CookiesError.fromReason("InvalidCookiePath")) - } - - if (options.maxAge !== undefined && !Duration.isFinite(Duration.fromInputUnsafe(options.maxAge))) { - return Result.fail(CookiesError.fromReason("CookieInfinityMaxAge")) - } + const error = validateCookie(name, encodedValue, options) + if (error !== undefined) { + return Result.fail(error) } return Result.succeed(Object.assign(Object.create(CookieProto), { @@ -485,6 +473,28 @@ export function makeCookie( })) } +function validateCookie( + name: string, + encodedValue: string, + options: Cookie["options"] | undefined +): CookiesError | undefined { + if (!cookieNameRegExp.test(name)) { + return CookiesError.fromReason("InvalidCookieName") + } + if (encodedValue && !fieldContentRegExp.test(encodedValue)) { + return CookiesError.fromReason("InvalidCookieValue") + } + if (options?.domain !== undefined && !cookieDomainRegExp.test(options.domain)) { + return CookiesError.fromReason("InvalidCookieDomain") + } + if (options?.path !== undefined && !cookiePathRegExp.test(options.path)) { + return CookiesError.fromReason("InvalidCookiePath") + } + if (options?.maxAge !== undefined && !Duration.isFinite(Duration.fromInputUnsafe(options.maxAge))) { + return CookiesError.fromReason("CookieInfinityMaxAge") + } +} + /** * Create a new cookie, throwing an error if invalid * @@ -779,6 +789,10 @@ export const setAllUnsafe: { * @since 4.0.0 */ export function serializeCookie(self: Cookie): string { + const error = validateCookie(self.name, self.valueEncoded, self.options) + if (error !== undefined) { + throw error + } let str = self.name + "=" + self.valueEncoded if (self.options === undefined) { diff --git a/packages/effect/test/unstable/http/Cookies.test.ts b/packages/effect/test/unstable/http/Cookies.test.ts index 16e6d00cafa..96a82fd1a49 100644 --- a/packages/effect/test/unstable/http/Cookies.test.ts +++ b/packages/effect/test/unstable/http/Cookies.test.ts @@ -1,12 +1,66 @@ import { assert, describe, it } from "@effect/vitest" import { assertNone, assertSome, deepStrictEqual } from "@effect/vitest/utils" -import { Schema } from "effect" +import { Result, Schema } from "effect" import * as Option from "effect/Option" import { TestSchema } from "effect/testing" import { Cookies } from "effect/unstable/http" -import { assertSuccess } from "../../utils/assert.ts" +import { assertFailure, assertSuccess } from "../../utils/assert.ts" describe("Cookies", () => { + describe("makeCookie", () => { + it("rejects cookie attribute delimiters in names, domains, and paths", () => { + assertFailure( + Cookies.makeCookie("a; Domain=evil.com; b", "token"), + Cookies.CookiesError.fromReason("InvalidCookieName") + ) + assertFailure( + Cookies.makeCookie("session", "token", { domain: "legit.com; Domain=.parent.tld" }), + Cookies.CookiesError.fromReason("InvalidCookieDomain") + ) + assertFailure( + Cookies.makeCookie("session", "token", { path: "/; HttpOnly" }), + Cookies.CookiesError.fromReason("InvalidCookiePath") + ) + }) + + it("accepts RFC 6265 token names and legitimate domains and paths", () => { + for (const domain of ["sub.example.com", ".sub.example.com"]) { + const cookie = Result.getOrThrow( + Cookies.makeCookie("!#$%&'*+-.^_`|~", "token", { + domain, + path: "/some-path_with~chars/%20" + }) + ) + + assert.strictEqual( + Cookies.serializeCookie(cookie), + `!#$%&'*+-.^_\`|~=token; Domain=${domain}; Path=/some-path_with~chars/%20` + ) + } + }) + }) + + describe("toSetCookieHeaders", () => { + const invalidCookie = { + name: "session", + value: "token", + valueEncoded: "token", + options: { domain: "legit.com; Domain=.evil.com" } + } as unknown as Cookies.Cookie + + it("rejects invalid cookies supplied through fromIterable", () => { + const cookies = Cookies.fromIterable([invalidCookie]) + + assert.throws(() => Cookies.toSetCookieHeaders(cookies), /InvalidCookieDomain/) + }) + + it("rejects invalid cookies supplied through setCookie", () => { + const cookies = Cookies.setCookie(Cookies.empty, invalidCookie) + + assert.throws(() => Cookies.toSetCookieHeaders(cookies), /InvalidCookieDomain/) + }) + }) + it("expireCookie returns a Result with an expired Set-Cookie value", () => { assertSuccess( Cookies.expireCookie(Cookies.empty, "session", { path: "/", secure: true }),