Keep the PKCE code verifier in the cookie rather than the state URL

[devin-ai-integration]

Summary

The PKCE code verifier was serialized into the OAuth state parameter, so it round-tripped through the authorization response URL. This change keeps the verifier in the cookie only and reconstructs it on callback, so it no longer travels in the URL.

Please review before merging.

[devin-ai-integration]

Original prompt from Linear User

Please work on ticket "AuthKit React Router OAuth callback: PKCE code verifier sealed into the state URL parameter with self-satisfiable double-submit check — leaked callback URL yields full account takeover" (SEC-1309)

@playbook:playbook-b588614117c7477a9b9729928385384f

[devin-ai-integration]

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

• Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
• Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

• Disable automatic comment, CI, and merge conflict monitoring

[linear-code]

SEC-1309

[greptile-apps]

Greptile Summary

This PR keeps the PKCE verifier out of the OAuth state URL. The main changes are:

• Seals URL state and the verifier cookie separately.
• Binds both payloads with a shared nonce.
• Validates each payload with a dedicated schema.
• Adds tests for state replay and cookie attributes.

Confidence Score: 4/5

Mixed-version callbacks can fail during a rolling deployment.

• The nonce binding blocks the reported replay path.
• A login started on a new instance returns 500 if its callback reaches an old instance.
• The failure lasts until old callback instances are drained.

src/get-authorization-url.ts and src/authkit-callback-route.ts

Important Files Changed

Filename	Overview
src/get-authorization-url.ts	Separates state from the verifier cookie, but the new wire format can fail against an old callback instance.
src/authkit-callback-route.ts	Unseals both payloads independently and checks their shared nonce before exchanging the code.
src/pkce.ts	Adds dedicated state and verifier decoding helpers and supports an explicit cookie value.
src/interfaces.ts	Splits the combined state schema into public state metadata and a secret verifier-cookie payload.
src/authkit-callback-route.spec.ts	Adds coverage that rejects URL state replayed as the verifier cookie.
src/get-authorization-url.spec.ts	Checks verifier secrecy, nonce binding, distinct sealed values, and cookie attributes.

Sequence Diagram

sequenceDiagram
    participant B as Browser
    participant N as New instance
    participant O as OAuth provider
    participant L as Old instance
    B->>N: Start authorization
    N-->>B: state S and cookie V
    B->>O: Authorize with state S
    O-->>B: Callback with code and state S
    B->>L: Send state S and cookie V
    L->>L: "Require S == V"
    L-->>B: 500 OAuth state mismatch

Loading

Comments Outside Diff (1)

1. src/get-authorization-url.ts, line 50-55 (link)

   Mixed Versions Reject Valid Callbacks

   During a rolling deployment, a new instance can issue distinct state and verifier-cookie values before the callback reaches an old instance. The old callback requires those values to be identical, so it returns a 500 for a valid login until every callback instance is updated.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: src/get-authorization-url.ts
   Line: 50-55
   
   Comment:
   **Mixed Versions Reject Valid Callbacks**
   
   During a rolling deployment, a new instance can issue distinct state and verifier-cookie values before the callback reaches an old instance. The old callback requires those values to be identical, so it returns a 500 for a valid login until every callback instance is updated.
   
   How can I resolve this? If you propose a fix, please make it concise.

Prompt To Fix All With AI

Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
src/get-authorization-url.ts:50-55
**Mixed Versions Reject Valid Callbacks**

During a rolling deployment, a new instance can issue distinct state and verifier-cookie values before the callback reaches an old instance. The old callback requires those values to be identical, so it returns a 500 for a valid login until every callback instance is updated.

_{Reviews (1): Last reviewed commit: "Keep PKCE code verifier out of the OAuth..." | Re-trigger Greptile}

[devin-ai-integration]

Re: Greptile's P1 "Mixed Versions Reject Valid Callbacks" (src/get-authorization-url.ts:50-55) — acknowledged, but I don't think it should change this PR.

This is a transient rolling-deploy artifact, not a correctness bug in the new code, and it's inherent to any change of the state/cookie wire format:

• It's one-directional and self-healing. Only a flow started on a new instance whose callback lands on an old instance fails (old code requires state == cookie), and only until old callback instances drain. The reverse (old-instance flow → new-instance callback) still succeeds: the old sealed value carries { nonce, codeVerifier, ... }, so getVerifierFromPKCECookieValue recovers the verifier and the shared-nonce check passes. Affected in-flight logins during the brief window just retry.
• The only way to avoid it is to keep the new callback accepting the old state == cookie format during rollout — which is exactly the self-satisfiable double-submit path this PR removes. Preserving it would re-open the account-takeover vector (SEC-1309) for the duration of every deploy, a worse trade than a short window of retryable 500s.

So I'm intentionally leaving this as-is. If zero login disruption during rollout is a hard requirement, the right lever is deploy-level (drain / blue-green, or a temporary dual-read window that is explicitly not the vulnerable equality check), which is out of scope for this security fix. Flagging for the human reviewer to decide.