Skip to content

feat: bound pending PKCE verifier cookies with eviction#42

Merged
nicknisi merged 3 commits into
mainfrom
feat/pkce-verifier-cookie-eviction
Jun 15, 2026
Merged

feat: bound pending PKCE verifier cookies with eviction#42
nicknisi merged 3 commits into
mainfrom
feat/pkce-verifier-cookie-eviction

Conversation

@nicknisi

Copy link
Copy Markdown
Member

Problem

Each authorization-URL call (createSignIn / createSignUp / createAuthorization) mints a uniquely-named wos-auth-verifier-<hash> cookie. The per-flow naming is deliberate — it lets concurrent sign-ins from multiple tabs coexist without clobbering each other (added in 0.5.x). The trade-off: because the name is derived from the sealed state (which embeds a fresh nonce + code verifier per call), cookies never overwrite. Generating URLs without navigating to them — e.g. prefetching getSignInUrl() in a route loader for display — leaves an orphan verifier cookie on every call. They accumulate until their 10-minute TTL, eventually bloating the Cookie request header into an HTTP 431.

This is the shared half of the fix for the consumer issue workos/authkit-tanstack-start#76.

Approach

Mirror what authkit-nextjs already does (src/pkce.ts, MAX_PKCE_COOKIES = 5): tolerate a small number of concurrent verifier cookies, then bound the pile-up by evicting stale ones. Kept as a pure, I/O-free primitive so any adapter can reuse it — the framework owns reading the request and emitting Set-Cookie.

New API

  • selectStalePKCEVerifierCookieNames(cookieNames, { keep, max? }) — given the request's cookie names and the verifier just minted (keep), returns the stale verifier names to delete once the total would exceed max (default 5). Policy is all-but-newest: content-hashed names carry no ordering, so "keep the newest N" isn't expressible — this matches the browser's own drop-at-limit behavior.
  • isPKCEVerifierCookieName(name) and DEFAULT_MAX_PENDING_PKCE_COOKIES.
  • AuthService.clearPendingVerifierByName(response, { cookieName, redirectUri? }) — clears a verifier by explicit name. clearPendingVerifier (which takes a sealed state) can't help here because the hashed name can't be reversed to a state; it now delegates to this method.

All three symbols are exported from the package index.

Consumer wiring (for context, not in this PR)

The TanStack Start adapter calls these right after writing a new verifier: parse the incoming Cookie header, selectStalePKCEVerifierCookieNames(names, { keep: result.cookieName }), then clearPendingVerifierByName each stale name through its existing pending-Set-Cookie channel — best-effort, so a cleanup failure never breaks URL generation.

Tests

  • src/core/pkce/eviction.spec.ts — 11 cases (within-cap no-op, over-cap evict-all-others, never evicts keep, ignores non-verifier cookies, custom max, prefix-boundary guard).
  • src/service/AuthService.spec.tsclearPendingVerifierByName clears by name + clearPendingVerifier delegates with the same options.
  • Full suite: 269 passing. format:check, lint, typecheck clean.

Note for reviewers

Typed as feat: because it adds public API surface (→ minor bump under release-please). If you'd rather this land as a patch so existing ^0.5.x consumers pick it up on install without a manual dep bump, retitle to fix:. The eviction cap is currently the hardcoded DEFAULT_MAX_PENDING_PKCE_COOKIES (matching authkit-nextjs); happy to thread it through as an option if you want it configurable.

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

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Devin Review found 1 potential issue.

Open in Devin Review

Comment thread src/core/pkce/eviction.spec.ts Outdated
@nicknisi nicknisi requested a review from gjtorikian June 15, 2026 19:46
@greptile-apps

greptile-apps Bot commented Jun 15, 2026

Copy link
Copy Markdown

Greptile Summary

This PR introduces a PKCE verifier cookie eviction mechanism to prevent Cookie header bloat (HTTP 431) caused by callers that generate authorization URLs without navigating to them. It mirrors the approach already in authkit-nextjs.

  • New pure utility (src/core/pkce/eviction.ts): exports isPKCEVerifierCookieName, selectStalePKCEVerifierCookieNames (all-or-nothing eviction once total exceeds max=5), and DEFAULT_MAX_PENDING_PKCE_COOKIES — I/O-free so any adapter can reuse them.
  • AuthService.clearPendingVerifierByName: new public method that clears a verifier by explicit cookie name (without needing the sealed state); clearPendingVerifier now delegates to it. Includes an isPKCEVerifierCookieName guard against clearing unrelated cookies (added from a prior review thread).
  • Tests: 11 eviction unit tests + 3 AuthService integration tests; all 269 passing.

Confidence Score: 5/5

Safe to merge — the change is a pure additive utility with no modifications to existing auth flows beyond the delegation refactor in clearPendingVerifier.

The eviction logic is I/O-free and well-isolated; clearPendingVerifier's delegation to clearPendingVerifierByName is semantically equivalent to the previous implementation; the isPKCEVerifierCookieName guard makes clearPendingVerifierByName fail-closed. No regressions to existing cookie handling are introduced.

No files require special attention. The only open suggestion is adding a guard on the keep parameter in selectStalePKCEVerifierCookieNames, which is a defensive hardening rather than a current defect.

Important Files Changed

Filename Overview
src/core/pkce/eviction.ts New pure utility — isPKCEVerifierCookieName, selectStalePKCEVerifierCookieNames, DEFAULT_MAX_PENDING_PKCE_COOKIES. Logic is sound; the all-or-nothing eviction policy is clearly documented and correctly implemented.
src/core/pkce/eviction.spec.ts 11 tests covering no-op, over-cap, never-evict-keep, non-verifier filtering, custom max, partial over-cap, and the default constant. Good coverage including the boundary guard test added from a previous review thread.
src/service/AuthService.ts clearPendingVerifier now delegates to new clearPendingVerifierByName, which adds an isPKCEVerifierCookieName guard (addressed from prior review). Delegation is safe since getPKCECookieNameForState always produces a valid verifier name.
src/service/AuthService.spec.ts Three new tests for clearPendingVerifierByName: clear-by-name, rejection of non-PKCE names, and delegation proof via spy. Correct and complete.
src/index.ts Exports three new public symbols from eviction.ts. No unintended re-exports or missing entries.

Sequence Diagram

sequenceDiagram
    participant Adapter as Framework Adapter
    participant AS as AuthService
    participant Eviction as eviction.ts
    participant Storage as CookieStorage

    Adapter->>AS: createSignIn(request)
    AS-->>Adapter: url, cookieName, Set-Cookie verifier

    Adapter->>Adapter: parse incoming Cookie header names
    Adapter->>Eviction: selectStalePKCEVerifierCookieNames(names, keep cookieName)
    Eviction-->>Adapter: staleNames array

    loop for each staleName
        Adapter->>AS: clearPendingVerifierByName(response, staleName)
        AS->>AS: isPKCEVerifierCookieName guard
        AS->>Storage: clearCookie(response, staleName, options)
        Storage-->>Adapter: Set-Cookie delete header
    end
Loading

Reviews (3): Last reviewed commit: "Refactor clearPendingVerifierByName test..." | Re-trigger Greptile

Comment thread src/service/AuthService.ts
Comment thread src/core/pkce/eviction.spec.ts
- 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 <nick.nisi@workos.com>
Comment thread src/service/AuthService.spec.ts Outdated
Co-Authored-By: nick.nisi@workos.com <nick.nisi@workos.com>
@nicknisi nicknisi merged commit c49e54d into main Jun 15, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants