From f75a9a44845bdfba1ba378928cf29b42f488d1c8 Mon Sep 17 00:00:00 2001 From: Nick Nisi Date: Mon, 15 Jun 2026 14:43:10 -0500 Subject: [PATCH 1/3] feat: bound pending PKCE verifier cookies with eviction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Each authorization-URL call mints a uniquely-named `wos-auth-verifier-*` cookie. Per-flow naming exists so concurrent sign-ins (e.g. multiple tabs) don't clobber each other, but the cost is that cookies never overwrite: generating URLs without navigating to them — e.g. prefetching `getSignInUrl()` in a route loader — accumulates orphan verifier cookies until their 10-minute TTL, eventually bloating the `Cookie` request header into an HTTP 431. Add a framework-agnostic eviction primitive, mirroring authkit-nextjs: - `selectStalePKCEVerifierCookieNames(names, { keep, max })`: pure, I/O-free. Given the request's cookie names and the verifier just minted, returns the stale verifier names to delete once the total would exceed `max` (default 5). All-but-newest, since content-hashed names carry no ordering. - `isPKCEVerifierCookieName()` and `DEFAULT_MAX_PENDING_PKCE_COOKIES`. - `AuthService.clearPendingVerifierByName()`: clear a verifier by explicit name (the hashed name can't be reversed to a sealed `state`). `clearPendingVerifier` now delegates to it. Adapters call these after writing a new verifier to GC stale ones through their existing Set-Cookie channel; this PR is the shared half of the fix. Refs workos/authkit-tanstack-start#76 --- src/core/pkce/eviction.spec.ts | 80 +++++++++++++++++++++++++++++++++ src/core/pkce/eviction.ts | 77 +++++++++++++++++++++++++++++++ src/index.ts | 5 +++ src/service/AuthService.spec.ts | 42 +++++++++++++++++ src/service/AuthService.ts | 29 +++++++++++- 5 files changed, 231 insertions(+), 2 deletions(-) create mode 100644 src/core/pkce/eviction.spec.ts create mode 100644 src/core/pkce/eviction.ts diff --git a/src/core/pkce/eviction.spec.ts b/src/core/pkce/eviction.spec.ts new file mode 100644 index 0000000..c66e346 --- /dev/null +++ b/src/core/pkce/eviction.spec.ts @@ -0,0 +1,80 @@ +import { describe, it, expect } from 'vitest'; +import { + DEFAULT_MAX_PENDING_PKCE_COOKIES, + isPKCEVerifierCookieName, + selectStalePKCEVerifierCookieNames, +} from './eviction.js'; +import { PKCE_COOKIE_PREFIX, getPKCECookieNameForState } from './cookieName.js'; + +const verifier = (state: string) => getPKCECookieNameForState(state); + +describe('isPKCEVerifierCookieName', () => { + it('matches the per-flow hashed name', () => { + expect(isPKCEVerifierCookieName(verifier('abc'))).toBe(true); + }); + + it('matches the legacy unsuffixed name', () => { + expect(isPKCEVerifierCookieName(PKCE_COOKIE_PREFIX)).toBe(true); + }); + + it('rejects unrelated cookies', () => { + expect(isPKCEVerifierCookieName('wos-session')).toBe(false); + expect(isPKCEVerifierCookieName('foo')).toBe(false); + }); + + it('rejects names that share the prefix without the hyphen boundary', () => { + // Guards against `wos-auth-verifierEVIL` being treated as a verifier. + expect(isPKCEVerifierCookieName(`${PKCE_COOKIE_PREFIX}EVIL`)).toBe(false); + }); +}); + +describe('selectStalePKCEVerifierCookieNames', () => { + const keep = verifier('new-flow'); + + it('evicts nothing when the new cookie is the only verifier', () => { + expect(selectStalePKCEVerifierCookieNames([], { keep })).toEqual([]); + }); + + it('evicts nothing while total verifier count stays within the cap', () => { + // 4 existing + the new one = 5 = DEFAULT cap. + const existing = ['a', 'b', 'c', 'd'].map(verifier); + expect(selectStalePKCEVerifierCookieNames(existing, { keep })).toEqual([]); + }); + + it('evicts every other verifier once the new cookie tips it over the cap', () => { + // 5 existing + the new one = 6 > 5. + const existing = ['a', 'b', 'c', 'd', 'e'].map(verifier); + const stale = selectStalePKCEVerifierCookieNames(existing, { keep }); + expect(new Set(stale)).toEqual(new Set(existing)); + expect(stale).not.toContain(keep); + }); + + it('never evicts the cookie being kept, even if it is already on the request', () => { + const existing = ['a', 'b', 'c', 'd', keep]; + // existing verifiers other than keep = 4; +1 new (keep, already counted) → within cap. + expect(selectStalePKCEVerifierCookieNames(existing, { keep })).toEqual([]); + }); + + it('ignores non-verifier cookies when counting', () => { + const existing = [ + 'wos-session', + 'theme', + ...['a', 'b', 'c', 'd'].map(verifier), + ]; + expect(selectStalePKCEVerifierCookieNames(existing, { keep })).toEqual([]); + }); + + it('honors a custom max', () => { + const existing = ['a', 'b'].map(verifier); + // 2 existing + 1 new = 3 > max:2 → evict both existing. + const stale = selectStalePKCEVerifierCookieNames(existing, { + keep, + max: 2, + }); + expect(new Set(stale)).toEqual(new Set(existing)); + }); + + it('exposes a sane default cap', () => { + expect(DEFAULT_MAX_PENDING_PKCE_COOKIES).toBe(5); + }); +}); diff --git a/src/core/pkce/eviction.ts b/src/core/pkce/eviction.ts new file mode 100644 index 0000000..2df4a55 --- /dev/null +++ b/src/core/pkce/eviction.ts @@ -0,0 +1,77 @@ +import { PKCE_COOKIE_PREFIX } from './cookieName.js'; + +/** + * Default ceiling on simultaneous pending PKCE verifier cookies before + * eviction kicks in. Mirrors `authkit-nextjs`'s `MAX_PKCE_COOKIES`. + * + * A handful of concurrent flows is normal — e.g. several tabs each + * mid-sign-in, each holding its own verifier. Eviction only triggers + * once accumulation risks an oversized `Cookie` request header + * (HTTP 431), which happens when an integration generates authorization + * URLs it never navigates to (prefetching `getSignInUrl()` in a loader). + */ +export const DEFAULT_MAX_PENDING_PKCE_COOKIES = 5; + +/** + * True when `name` is a PKCE verifier cookie — either the per-flow hashed + * name (`wos-auth-verifier-`) or the legacy unsuffixed name. + * + * The hyphen boundary is required so a lookalike like + * `wos-auth-verifierXYZ` is NOT treated as a verifier. + */ +export function isPKCEVerifierCookieName(name: string): boolean { + return ( + name === PKCE_COOKIE_PREFIX || name.startsWith(`${PKCE_COOKIE_PREFIX}-`) + ); +} + +/** + * Decide which stale PKCE verifier cookies to evict on the request that + * mints `keep`. + * + * The per-flow cookie naming (each `getSignInUrl()`/`getSignUpUrl()` call + * derives a unique name from its sealed state) means cookies never + * overwrite — they accumulate until their 10-minute TTL. Generating URLs + * without navigating to them (e.g. prefetching in a loader) can pile up + * enough verifier cookies to exceed the server's request-header limit and + * surface as HTTP 431. + * + * Policy (matches `authkit-nextjs`): tolerate up to `max` simultaneous + * verifier cookies. Once the cookie being created this request would push + * the total past `max`, evict *every* other verifier — keeping only the + * one just minted. Because cookie names are content hashes with no + * embedded ordering, "keep the newest N" is not expressible; all-but-newest + * is the only well-defined bounded policy, and it mirrors the browser's own + * drop-oldest behavior at the cookie-jar limit. + * + * Pure and I/O-free: callers supply the incoming cookie names and emit the + * delete `Set-Cookie` headers for the returned names themselves. + * + * @param cookieNames - Names of all cookies on the incoming request. + * @param keep - Name of the verifier cookie being written this request; + * never included in the result. + * @param max - Maximum simultaneous verifier cookies tolerated before + * eviction. Defaults to {@link DEFAULT_MAX_PENDING_PKCE_COOKIES}. + * @returns Verifier cookie names to delete. Empty when within budget. + */ +export function selectStalePKCEVerifierCookieNames( + cookieNames: Iterable, + { + keep, + max = DEFAULT_MAX_PENDING_PKCE_COOKIES, + }: { keep: string; max?: number }, +): string[] { + const others = new Set(); + for (const name of cookieNames) { + if (name !== keep && isPKCEVerifierCookieName(name)) { + others.add(name); + } + } + + // +1 accounts for `keep`, which may not be on the incoming request yet. + if (others.size + 1 <= max) { + return []; + } + + return [...others]; +} diff --git a/src/index.ts b/src/index.ts index f0fa4e8..7f5d58c 100644 --- a/src/index.ts +++ b/src/index.ts @@ -44,6 +44,11 @@ export { PKCE_COOKIE_PREFIX, getPKCECookieNameForState, } from './core/pkce/cookieName.js'; +export { + DEFAULT_MAX_PENDING_PKCE_COOKIES, + isPKCEVerifierCookieName, + selectStalePKCEVerifierCookieNames, +} from './core/pkce/eviction.js'; // ============================================ // Encryption Fallback diff --git a/src/service/AuthService.spec.ts b/src/service/AuthService.spec.ts index c69146c..3bb226c 100644 --- a/src/service/AuthService.spec.ts +++ b/src/service/AuthService.spec.ts @@ -407,6 +407,48 @@ describe('AuthService', () => { }); }); + describe('clearPendingVerifierByName()', () => { + it('clears the named cookie without needing the sealed state', async () => { + const realStorage = makeStorage(); + const realService = new AuthService( + mockConfig as any, + realStorage as any, + makeClient() as any, + sessionEncryption, + ); + + const staleName = 'wos-auth-verifier-deadbeef'; + const result = await realService.clearPendingVerifierByName(undefined, { + cookieName: staleName, + }); + + expect(realStorage.lastClearOptions.get(staleName)?.path).toBe('/'); + expect(result.headers?.['Set-Cookie']).toContain(`${staleName}=`); + }); + + it('is what clearPendingVerifier delegates to (same options)', async () => { + const realStorage = makeStorage(); + const realService = new AuthService( + mockConfig as any, + realStorage as any, + makeClient() as any, + sessionEncryption, + ); + + const { cookieName } = await realService.createSignIn(undefined); + const sealedState = realStorage.cookies.get(cookieName)!; + + const clearCookieSpy = vi.spyOn(realStorage, 'clearCookie'); + await realService.clearPendingVerifier(undefined, { state: sealedState }); + + expect(clearCookieSpy).toHaveBeenCalledWith( + undefined, + cookieName, + expect.any(Object), + ); + }); + }); + describe('getWorkOS()', () => { it('returns WorkOS client', () => { const result = service.getWorkOS(); diff --git a/src/service/AuthService.ts b/src/service/AuthService.ts index 44b994f..c140459 100644 --- a/src/service/AuthService.ts +++ b/src/service/AuthService.ts @@ -283,10 +283,35 @@ export class AuthService { response: TResponse | undefined, options: { state: string; redirectUri?: string }, ): Promise<{ response?: TResponse; headers?: HeadersBag }> { - const cookieName = getPKCECookieNameForState(options.state); + return this.clearPendingVerifierByName(response, { + cookieName: getPKCECookieNameForState(options.state), + redirectUri: options.redirectUri, + }); + } + + /** + * Emit a `Set-Cookie` header that clears a PKCE verifier cookie by its + * explicit name. + * + * Unlike {@link clearPendingVerifier}, this does not derive the name from + * a sealed `state` — it takes the cookie name directly. Use it to evict + * stale verifier cookies discovered by enumerating the request's cookies + * (e.g. via {@link selectStalePKCEVerifierCookieNames}), where the + * originating `state` is unknown and unrecoverable from the hashed name. + * + * The delete is computed with the same (name, path, domain) tuple the + * browser uses to match a cookie for replacement, so it clears the + * target regardless of the `redirectUri` the flow was created with. Pass + * `options.redirectUri` only when you need the `secure` attribute to match + * a per-request override. + */ + async clearPendingVerifierByName( + response: TResponse | undefined, + options: { cookieName: string; redirectUri?: string }, + ): Promise<{ response?: TResponse; headers?: HeadersBag }> { return this.storage.clearCookie( response, - cookieName, + options.cookieName, getPKCECookieOptions(this.config, options.redirectUri), ); } From b31acfd661aaab9f23b75d0a50b0a366efa72052 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Mon, 15 Jun 2026 19:49:36 +0000 Subject: [PATCH 2/3] Fix review bugs: verifier test, PKCE cookie guard, over-cap test - Fix vacuous test: use verifier()-hashed names instead of raw strings in 'never evicts the cookie being kept' so isPKCEVerifierCookieName actually filters them - Add isPKCEVerifierCookieName guard to clearPendingVerifierByName to reject non-PKCE cookie names - Add partial over-cap eviction test (max=3, 3 existing + 1 new) to make the all-or-nothing cliff explicit Co-Authored-By: nick.nisi@workos.com --- src/core/pkce/eviction.spec.ts | 13 ++++++++++++- src/service/AuthService.spec.ts | 16 ++++++++++++++++ src/service/AuthService.ts | 6 ++++++ 3 files changed, 34 insertions(+), 1 deletion(-) diff --git a/src/core/pkce/eviction.spec.ts b/src/core/pkce/eviction.spec.ts index c66e346..35c7464 100644 --- a/src/core/pkce/eviction.spec.ts +++ b/src/core/pkce/eviction.spec.ts @@ -50,7 +50,7 @@ describe('selectStalePKCEVerifierCookieNames', () => { }); it('never evicts the cookie being kept, even if it is already on the request', () => { - const existing = ['a', 'b', 'c', 'd', keep]; + const existing = [...['a', 'b', 'c', 'd'].map(verifier), keep]; // existing verifiers other than keep = 4; +1 new (keep, already counted) → within cap. expect(selectStalePKCEVerifierCookieNames(existing, { keep })).toEqual([]); }); @@ -74,6 +74,17 @@ describe('selectStalePKCEVerifierCookieNames', () => { expect(new Set(stale)).toEqual(new Set(existing)); }); + it('evicts all others even when only slightly over the cap (partial over-cap)', () => { + // max=3, 3 existing + 1 new = 4 > 3 → evicts all 3 existing (not just the surplus). + const existing = ['a', 'b', 'c'].map(verifier); + const stale = selectStalePKCEVerifierCookieNames(existing, { + keep, + max: 3, + }); + expect(new Set(stale)).toEqual(new Set(existing)); + expect(stale).not.toContain(keep); + }); + it('exposes a sane default cap', () => { expect(DEFAULT_MAX_PENDING_PKCE_COOKIES).toBe(5); }); diff --git a/src/service/AuthService.spec.ts b/src/service/AuthService.spec.ts index 3bb226c..740706d 100644 --- a/src/service/AuthService.spec.ts +++ b/src/service/AuthService.spec.ts @@ -426,6 +426,22 @@ describe('AuthService', () => { expect(result.headers?.['Set-Cookie']).toContain(`${staleName}=`); }); + it('rejects non-PKCE cookie names', async () => { + const realStorage = makeStorage(); + const realService = new AuthService( + mockConfig as any, + realStorage as any, + makeClient() as any, + sessionEncryption, + ); + + await expect( + realService.clearPendingVerifierByName(undefined, { + cookieName: 'wos-session', + }), + ).rejects.toThrow('Refusing to clear non-PKCE cookie "wos-session"'); + }); + it('is what clearPendingVerifier delegates to (same options)', async () => { const realStorage = makeStorage(); const realService = new AuthService( diff --git a/src/service/AuthService.ts b/src/service/AuthService.ts index c140459..a97b799 100644 --- a/src/service/AuthService.ts +++ b/src/service/AuthService.ts @@ -3,6 +3,7 @@ import { AuthKitCore } from '../core/AuthKitCore.js'; import type { AuthKitConfig } from '../core/config/types.js'; import { getPKCECookieNameForState } from '../core/pkce/cookieName.js'; import { getPKCECookieOptions } from '../core/pkce/cookieOptions.js'; +import { isPKCEVerifierCookieName } from '../core/pkce/eviction.js'; import { AuthOperations } from '../operations/AuthOperations.js'; import type { AuthResult, @@ -309,6 +310,11 @@ export class AuthService { response: TResponse | undefined, options: { cookieName: string; redirectUri?: string }, ): Promise<{ response?: TResponse; headers?: HeadersBag }> { + if (!isPKCEVerifierCookieName(options.cookieName)) { + throw new Error( + `Refusing to clear non-PKCE cookie "${options.cookieName}"`, + ); + } return this.storage.clearCookie( response, options.cookieName, From 41425c1529f0e6d454882a95191fb31e6fb49721 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Mon, 15 Jun 2026 20:32:39 +0000 Subject: [PATCH 3/3] Refactor clearPendingVerifierByName tests to use beforeEach Co-Authored-By: nick.nisi@workos.com --- src/service/AuthService.spec.ts | 27 ++++++++------------------- 1 file changed, 8 insertions(+), 19 deletions(-) diff --git a/src/service/AuthService.spec.ts b/src/service/AuthService.spec.ts index 740706d..457bb43 100644 --- a/src/service/AuthService.spec.ts +++ b/src/service/AuthService.spec.ts @@ -408,15 +408,20 @@ describe('AuthService', () => { }); describe('clearPendingVerifierByName()', () => { - it('clears the named cookie without needing the sealed state', async () => { - const realStorage = makeStorage(); - const realService = new AuthService( + let realStorage: ReturnType; + let realService: InstanceType; + + beforeEach(() => { + realStorage = makeStorage(); + realService = new AuthService( mockConfig as any, realStorage as any, makeClient() as any, sessionEncryption, ); + }); + it('clears the named cookie without needing the sealed state', async () => { const staleName = 'wos-auth-verifier-deadbeef'; const result = await realService.clearPendingVerifierByName(undefined, { cookieName: staleName, @@ -427,14 +432,6 @@ describe('AuthService', () => { }); it('rejects non-PKCE cookie names', async () => { - const realStorage = makeStorage(); - const realService = new AuthService( - mockConfig as any, - realStorage as any, - makeClient() as any, - sessionEncryption, - ); - await expect( realService.clearPendingVerifierByName(undefined, { cookieName: 'wos-session', @@ -443,14 +440,6 @@ describe('AuthService', () => { }); it('is what clearPendingVerifier delegates to (same options)', async () => { - const realStorage = makeStorage(); - const realService = new AuthService( - mockConfig as any, - realStorage as any, - makeClient() as any, - sessionEncryption, - ); - const { cookieName } = await realService.createSignIn(undefined); const sealedState = realStorage.cookies.get(cookieName)!;