fix(mcp): keep PKCE verifier in provider memory instead of disk

src/mcp/add-server.test.ts

@@ -291,7 +291,11 @@ describe("persistMCPServerEnabled", () => {
         mcpServers: [linearHTTP],
       }),
     );
-    await saveAuthState(linearAuth, { codeVerifier: "keep-me" }, home);
+    await saveAuthState(
+      linearAuth,
+      { tokens: { access_token: "keep-me", token_type: "bearer" } },
+      home,
+    );
     const writer = createGlobalSettingsWriter(path);
 
     expect(
@@ -306,7 +310,7 @@ describe("persistMCPServerEnabled", () => {
       showPromptCost: true,
       mcpServers: [{ ...linearHTTP, enabled: false }],
     });
-    expect((await loadAuthState(linearAuth, home)).codeVerifier).toBe(
+    expect((await loadAuthState(linearAuth, home)).tokens?.access_token).toBe(
       "keep-me",
     );
 
@@ -388,7 +392,11 @@ describe("persistMCPServerRemoved", () => {
         ],
       }),
     );
-    await saveAuthState(linearAuth, { codeVerifier: "linear-secret" }, home);
+    await saveAuthState(
+      linearAuth,
+      { tokens: { access_token: "linear-secret", token_type: "bearer" } },
+      home,
+    );
     const writer = createGlobalSettingsWriter(path);
 
     const result = await persistMCPServerRemoved(writer, "linear", home);
@@ -480,7 +488,11 @@ describe("persistMCPServerRemoved", () => {
       path,
       JSON.stringify({ providers: {}, mcpServers: [custom] }),
     );
-    await saveAuthState(identity, { codeVerifier: "custom-exa" }, home);
+    await saveAuthState(
+      identity,
+      { tokens: { access_token: "custom-exa", token_type: "bearer" } },
+      home,
+    );
     const writer = createGlobalSettingsWriter(path);
 
     expect(await persistMCPServerRemoved(writer, "exa", home)).toMatchObject({
@@ -501,7 +513,11 @@ describe("persistMCPServerRemoved", () => {
       path,
       JSON.stringify({ providers: {}, mcpServers: [linearHTTP] }),
     );
-    await saveAuthState(linearAuth, { codeVerifier: "linear-secret" }, home);
+    await saveAuthState(
+      linearAuth,
+      { tokens: { access_token: "linear-secret", token_type: "bearer" } },
+      home,
+    );
     const authDir = mcpAuthDir(home);
     await chmod(authDir, 0o000);
     const writer = createGlobalSettingsWriter(path);
@@ -538,7 +554,11 @@ describe("local MCP persist", () => {
       localPath,
       JSON.stringify({ provider: "a", mcpServers: [linearHTTP] }),
     );
-    await saveAuthState(linearAuth, { codeVerifier: "local-linear" }, home);
+    await saveAuthState(
+      linearAuth,
+      { tokens: { access_token: "local-linear", token_type: "bearer" } },
+      home,
+    );
     const writer = createLocalSettingsWriter(localPath);
 
     expect(
@@ -582,7 +602,11 @@ describe("local MCP persist", () => {
       path,
       JSON.stringify({ provider: "a", mcpServers: [linearHTTP] }),
     );
-    await saveAuthState(linearAuth, { codeVerifier: "local-linear" }, home);
+    await saveAuthState(
+      linearAuth,
+      { tokens: { access_token: "local-linear", token_type: "bearer" } },
+      home,
+    );
     const authDir = mcpAuthDir(home);
     await chmod(authDir, 0o000);
     const writer = createLocalSettingsWriter(path);

src/mcp/auth-store.test.ts

@@ -1,8 +1,9 @@
 import { describe, expect, test } from "bun:test";
 import { mkdir, mkdtemp, readFile, writeFile } from "node:fs/promises";
 import { tmpdir } from "node:os";
-import { join } from "node:path";
+import { dirname, join } from "node:path";
 import {
+  authFilePath,
   deleteAuthState,
   loadAuthState,
   saveAuthState,
@@ -33,9 +34,10 @@ describe("mcp auth-store", () => {
       home,
     );
 
-    // Reproduce the bug: one writer saves tokens while another overwrites with a
-    // fresh codeVerifier from a concurrent OAuth start. With full-snapshot
-    // persists, the verifier write wiped tokens; with updateAuthState, both land.
+    // Reproduce the bug: one writer saves tokens while another updates the
+    // client registration from a concurrent OAuth start. With full-snapshot
+    // persists, the registration write wiped tokens; with updateAuthState,
+    // both land.
     const writes = await Promise.all([
       updateAuthState(
         linear,
@@ -52,23 +54,28 @@ describe("mcp auth-store", () => {
       updateAuthState(
         linear,
         (state) => {
-          state.codeVerifier = "verifier-from-other-session";
+          state.clientInformation = {
+            client_id: "c1",
+            redirect_uris: ["http://127.0.0.1:1/callback"],
+            client_id_issued_at: 1,
+            client_name: "other-session",
+          };
         },
         home,
       ),
     ]);
 
     const final = await loadAuthState(linear, home);
     expect(final.tokens?.access_token).toBe("tok");
-    expect(final.codeVerifier).toBe("verifier-from-other-session");
+    expect(final.clientInformation?.client_name).toBe("other-session");
     expect(final.clientInformation?.client_id).toBe("c1");
     expect(
       writes[0].tokens?.access_token === "tok" ||
         writes[1].tokens?.access_token === "tok",
     ).toBe(true);
   });
 
-  test("overlapping updateAuthState from two processes keeps tokens and PKCE", async () => {
+  test("overlapping updateAuthState from two processes keeps tokens and client registration", async () => {
     const home = await tempHome();
     await saveAuthState(
       linear,
@@ -103,7 +110,12 @@ describe("mcp auth-store", () => {
               refresh_token: "ref",
             };
           } else {
-            state.codeVerifier = "verifier-from-other-session";
+            state.clientInformation = {
+              client_id: "c1",
+              redirect_uris: ["http://127.0.0.1:1/callback"],
+              client_id_issued_at: 1,
+              client_name: "other-session",
+            };
           }
         },
         home,
@@ -118,7 +130,7 @@ describe("mcp auth-store", () => {
         },
       ),
       Bun.spawn(
-        [process.execPath, "-e", script, "--", home, "verifier", barrier],
+        [process.execPath, "-e", script, "--", home, "client", barrier],
         {
           stdout: "ignore",
           stderr: "pipe",
@@ -135,27 +147,33 @@ describe("mcp auth-store", () => {
 
     const final = await loadAuthState(linear, home);
     expect(final.tokens?.access_token).toBe("tok");
-    expect(final.codeVerifier).toBe("verifier-from-other-session");
+    expect(final.clientInformation?.client_name).toBe("other-session");
     expect(final.clientInformation?.client_id).toBe("c1");
   });
 
   test("concurrent saveAuthState calls do not throw ENOENT on temp rename", async () => {
     const home = await tempHome();
     await Promise.all(
       Array.from({ length: 20 }, (_, i) =>
-        saveAuthState(linear, { codeVerifier: `v${String(i)}` }, home),
+        saveAuthState(
+          linear,
+          {
+            tokens: { access_token: `v${String(i)}`, token_type: "bearer" },
+          },
+          home,
+        ),
       ),
     );
     const final = await loadAuthState(linear, home);
-    expect(final.codeVerifier?.startsWith("v")).toBe(true);
+    expect(final.tokens?.access_token?.startsWith("v")).toBe(true);
     // No leftover temp files from failed renames.
     const dir = join(home, ".corbits", "mcp-auth");
     const files = await Array.fromAsync(
       new Bun.Glob("linear-*.json").scan(dir),
     );
     expect(files).toHaveLength(1);
     const raw = await readFile(join(dir, files[0] ?? "missing"), "utf8");
-    expect(JSON.parse(raw).codeVerifier).toBe(final.codeVerifier);
+    expect(JSON.parse(raw).tokens).toEqual(final.tokens);
   });
 
   test("scopes credentials to normalized endpoint identity", async () => {
@@ -175,13 +193,21 @@ describe("mcp auth-store", () => {
       serverURL: "https://ONE.example:443/mcp#ignored",
     };
 
-    await saveAuthState(originA, { codeVerifier: "only-a" }, home);
+    await saveAuthState(
+      originA,
+      { tokens: { access_token: "only-a", token_type: "bearer" } },
+      home,
+    );
 
-    expect((await loadAuthState(originA, home)).codeVerifier).toBe("only-a");
+    expect((await loadAuthState(originA, home)).tokens?.access_token).toBe(
+      "only-a",
+    );
     expect(await loadAuthState(originB, home)).toEqual({});
     expect(await loadAuthState(pathB, home)).toEqual({});
     expect(await loadAuthState(queryB, home)).toEqual({});
-    expect((await loadAuthState(equivalent, home)).codeVerifier).toBe("only-a");
+    expect((await loadAuthState(equivalent, home)).tokens?.access_token).toBe(
+      "only-a",
+    );
   });
 
   test("ignores legacy name-only auth state without modifying it", async () => {
@@ -204,14 +230,42 @@ describe("mcp auth-store", () => {
     });
   });
 
+  test("updateAuthState strips a legacy PKCE verifier instead of persisting it", async () => {
+    const home = await tempHome();
+    const path = authFilePath(linear, home);
+    await mkdir(dirname(path), { recursive: true });
+    await writeFile(
+      path,
+      JSON.stringify({
+        tokens: { access_token: "tok", token_type: "bearer" },
+        codeVerifier: "legacy-secret",
+      }),
+    );
+
+    const written = await updateAuthState(
+      linear,
+      (state) => {
+        state.tokens = { access_token: "tok2", token_type: "bearer" };
+      },
+      home,
+    );
+
+    expect(written.tokens?.access_token).toBe("tok2");
+    expect("codeVerifier" in written).toBe(false);
+    expect(await readFile(path, "utf8")).not.toContain("codeVerifier");
+    expect((await loadAuthState(linear, home)).tokens?.access_token).toBe(
+      "tok2",
+    );
+  });
+
   test("bounds the display slug without weakening scoped identity", async () => {
     const home = await tempHome();
     const dir = join(home, ".corbits", "mcp-auth");
     const prefixName = "a".repeat(48);
     const longName = `${prefixName}/long`;
     await saveAuthState(
       { serverName: longName, serverURL: "https://long.example/mcp" },
-      { codeVerifier: "scoped-secret" },
+      { tokens: { access_token: "scoped-secret", token_type: "bearer" } },
       home,
     );
 
@@ -225,8 +279,14 @@ describe("mcp auth-store", () => {
 
   test("deleteAuthState removes an existing file", async () => {
     const home = await tempHome();
-    await saveAuthState(linear, { codeVerifier: "secret" }, home);
-    expect((await loadAuthState(linear, home)).codeVerifier).toBe("secret");
+    await saveAuthState(
+      linear,
+      { tokens: { access_token: "secret", token_type: "bearer" } },
+      home,
+    );
+    expect((await loadAuthState(linear, home)).tokens?.access_token).toBe(
+      "secret",
+    );
 
     await deleteAuthState(linear, home);
     expect(await loadAuthState(linear, home)).toEqual({});
@@ -241,13 +301,17 @@ describe("mcp auth-store", () => {
 
   test("concurrent updateAuthState and deleteAuthState do not throw ENOENT on rename", async () => {
     const home = await tempHome();
-    await saveAuthState(linear, { codeVerifier: "v0" }, home);
+    await saveAuthState(
+      linear,
+      { tokens: { access_token: "v0", token_type: "bearer" } },
+      home,
+    );
 
     await Promise.all([
       updateAuthState(
         linear,
         (state) => {
-          state.codeVerifier = "v1";
+          state.tokens = { access_token: "v1", token_type: "bearer" };
         },
         home,
       ),
@@ -256,7 +320,7 @@ describe("mcp auth-store", () => {
 
     const final = await loadAuthState(linear, home);
     expect(
-      final.codeVerifier === undefined || final.codeVerifier === "v1",
+      final.tokens === undefined || final.tokens.access_token === "v1",
     ).toBe(true);
   });
 });

src/mcp/auth-store.ts

@@ -18,14 +18,14 @@ import type {
 } from "@modelcontextprotocol/sdk/shared/auth.js";
 import { SETTINGS_DIR_NAME } from "../branding.js";
 
-// Per-server OAuth state persisted between sessions. Holding the PKCE verifier is
-// necessary because authorization spans a process boundary (browser round-trip);
-// tokens and dynamically-registered client info let later sessions reconnect
-// without any user interaction.
+// Per-server OAuth state persisted between sessions. Only the durable pair —
+// dynamically-registered client info and tokens — reaches disk; the PKCE
+// verifier is instance-local transient memory in the OAuth provider, since the
+// browser flow starts and finishes inside one provider episode. Legacy files
+// may still carry a codeVerifier key, which is stripped on write.
 export interface MCPAuthState {
   clientInformation?: OAuthClientInformationFull;
   tokens?: OAuthTokens;
-  codeVerifier?: string;
 }
 
 export interface MCPAuthIdentity {
@@ -67,7 +67,7 @@ export function authFilePath(
   );
 }
 
-function isEnoent(err: unknown): boolean {
+export function isEnoent(err: unknown): boolean {
   return (
     typeof err === "object" &&
     err !== null &&
@@ -150,7 +150,7 @@ let tmpWriteCounter = 0;
 
 // Serialize read-modify-write per auth file so two OAuth provider instances for
 // the same server cannot clobber each other's fields (classic lost-update: one
-// session's saveCodeVerifier wiping another's just-written tokens).
+// session's saveTokens wiping another's just-saved client registration).
 const updateChains = new Map<string, Promise<unknown>>();
 
 const LOCK_STALE_MS = 5_000;
@@ -220,10 +220,22 @@ function enqueueAuthFileOp<T>(path: string, op: () => Promise<T>): Promise<T> {
   return run;
 }
 
+// Legacy auth files may carry a codeVerifier key from when the PKCE verifier
+// flowed through disk. The verifier is instance-local transient memory now —
+// strip the key so it never reaches disk again.
+function withoutLegacyVerifier(state: MCPAuthState): MCPAuthState {
+  if (!("codeVerifier" in state)) return state;
+  const copy: Record<string, unknown> = { ...state };
+  delete copy.codeVerifier;
+  return copy as MCPAuthState;
+}
+
 async function writeAuthFile(path: string, state: MCPAuthState): Promise<void> {
   await mkdir(dirname(path), { recursive: true, mode: 0o700 });
   const tmp = `${path}.${process.pid}.${(tmpWriteCounter += 1)}.tmp`;
-  await writeFile(tmp, JSON.stringify(state, null, 2), { mode: 0o600 });
+  await writeFile(tmp, JSON.stringify(withoutLegacyVerifier(state), null, 2), {
+    mode: 0o600,
+  });
   await rename(tmp, path);
 }
 
@@ -250,8 +262,9 @@ export async function updateAuthState(
   return enqueueAuthFileOp(path, async () => {
     const state = await loadAuthState(identity, home);
     mutator(state);
-    await writeAuthFile(path, state);
-    return state;
+    const durable = withoutLegacyVerifier(state);
+    await writeAuthFile(path, durable);
+    return durable;
   });
 }