From a35fc4c9571865eecbd1dc1eb686496a1f81bce7 Mon Sep 17 00:00:00 2001 From: Sawyer Date: Tue, 8 Sep 2026 21:52:59 -0700 Subject: [PATCH 1/5] Re-read OAuth state from disk in the provider's sync getters The MCP OAuth provider snapshotted auth state once at construction, so a session that re-authenticated never published its fresh tokens to sibling sessions: they kept serving a stale in-memory mirror and needlessly triggered the browser flow after a 401. The SDK requires tokens(), clientInformation(), and codeVerifier() to be synchronous, so the getters now guard with statSync and only readFileSync the auth file (via a new loadAuthStateSync that mirrors loadAuthState's parsing) when its mtime or size changed. A vanished or unreadable file keeps the in-memory mirror instead of throwing from a sync getter. --- src/mcp/auth-store.ts | 26 +++++++++++++++++- src/mcp/oauth-provider.test.ts | 48 +++++++++++++++++++++++++++++++++- src/mcp/oauth-provider.ts | 44 +++++++++++++++++++++++++++---- 3 files changed, 111 insertions(+), 7 deletions(-) diff --git a/src/mcp/auth-store.ts b/src/mcp/auth-store.ts index 2b53e1433..cbd333fa4 100644 --- a/src/mcp/auth-store.ts +++ b/src/mcp/auth-store.ts @@ -1,5 +1,6 @@ import { createHash } from "node:crypto"; import { mkdir, readFile, rename, unlink, writeFile } from "node:fs/promises"; +import { readFileSync } from "node:fs"; import { homedir } from "node:os"; import { dirname, join } from "node:path"; import type { @@ -41,7 +42,7 @@ export function normalizeMCPServerURL(serverURL: string): string { return url.toString(); } -function authFilePath(identity: MCPAuthIdentity, home: string): string { +export function authFilePath(identity: MCPAuthIdentity, home: string = homedir()): string { const normalizedURL = normalizeMCPServerURL(identity.serverURL); const digest = createHash("sha256") .update(JSON.stringify([identity.serverName, normalizedURL])) @@ -49,6 +50,29 @@ function authFilePath(identity: MCPAuthIdentity, home: string): string { return join(mcpAuthDir(home), `${serverDisplaySlug(identity.serverName)}-${digest}.json`); } +// Synchronous mirror of loadAuthState for the SDK's sync getters (tokens(), +// clientInformation(), codeVerifier()), which cannot await disk I/O. A missing or +// corrupt file yields empty state, matching loadAuthState's tolerance. +export function loadAuthStateSync( + identity: MCPAuthIdentity, + home: string = homedir(), +): MCPAuthState { + let raw: string; + try { + raw = readFileSync(authFilePath(identity, home), "utf8"); + } catch { + return {}; + } + try { + const parsed: unknown = JSON.parse(raw); + if (typeof parsed === "object" && parsed !== null) return parsed as MCPAuthState; + } catch { + // A corrupt auth file should not wedge the session; treat it as no state and + // let a fresh authorization overwrite it. + } + return {}; +} + export async function loadAuthState( identity: MCPAuthIdentity, home: string = homedir(), diff --git a/src/mcp/oauth-provider.test.ts b/src/mcp/oauth-provider.test.ts index 9e8d8bc1d..6be4f90e1 100644 --- a/src/mcp/oauth-provider.test.ts +++ b/src/mcp/oauth-provider.test.ts @@ -2,7 +2,7 @@ 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 { loadAuthState, saveAuthState } from "./auth-store.js"; +import { loadAuthState, saveAuthState, deleteAuthState } from "./auth-store.js"; import { createOAuthProvider } from "./oauth-provider.js"; async function tempHome(): Promise { @@ -208,6 +208,52 @@ describe("createOAuthProvider", () => { expect(await readFile(join(dir, ".json"), "utf8")).toBe(legacy); }); + test("propagates tokens saved by one provider to an existing sibling provider", async () => { + const home = await tempHome(); + const a = await createOAuthProvider({ + serverName: "linear", + serverURL: linear.serverURL, + redirectUrl: "http://127.0.0.1:1/callback", + onAuthURL: () => undefined, + home, + }); + const b = await createOAuthProvider({ + serverName: "linear", + serverURL: linear.serverURL, + redirectUrl: "http://127.0.0.1:1/callback", + onAuthURL: () => undefined, + home, + }); + expect(await syncValue(b.tokens())).toBeUndefined(); + + await a.saveTokens({ + access_token: "fresh", + token_type: "bearer", + expires_in: 3600, + refresh_token: "fresh-refresh", + }); + + expect((await syncValue(b.tokens()))?.access_token).toBe("fresh"); + }); + + test("sync getters fall back to the in-memory mirror when the auth file disappears", async () => { + const home = await tempHome(); + const provider = await createOAuthProvider({ + serverName: "linear", + serverURL: linear.serverURL, + redirectUrl: "http://127.0.0.1:1/callback", + onAuthURL: () => undefined, + home, + }); + await provider.saveTokens({ access_token: "tok", token_type: "bearer" }); + expect((await syncValue(provider.tokens()))?.access_token).toBe("tok"); + + await deleteAuthState(linear, home); + + expect((await syncValue(provider.tokens()))?.access_token).toBe("tok"); + expect(await syncValue(provider.clientInformation())).toBeUndefined(); + }); + test("does not delete scoped state whose filename stem is another provider name", async () => { const home = await tempHome(); const dir = join(home, ".corbits", "mcp-auth"); diff --git a/src/mcp/oauth-provider.ts b/src/mcp/oauth-provider.ts index 4519f27d1..6f36a1b18 100644 --- a/src/mcp/oauth-provider.ts +++ b/src/mcp/oauth-provider.ts @@ -5,7 +5,15 @@ import type { OAuthClientMetadata, OAuthTokens, } from "@modelcontextprotocol/sdk/shared/auth.js"; -import { updateAuthState, type MCPAuthIdentity, type MCPAuthState } from "./auth-store.js"; +import { + authFilePath, + loadAuthStateSync, + updateAuthState, + type MCPAuthIdentity, + type MCPAuthState, +} from "./auth-store.js"; +import { statSync } from "node:fs"; +import { homedir } from "node:os"; import { MCP_CLIENT_NAME } from "../branding.js"; export interface OAuthProviderOptions { @@ -54,22 +62,45 @@ export async function createOAuthProvider( serverName: opts.serverName, serverURL: opts.serverURL, }; + const home = opts.home ?? homedir(); // Load + scrub stale DCR under the per-file chain so concurrent providers see - // the same cleaned state. Mutations always re-read disk; this in-memory mirror - // only serves the SDK's sync getters (tokens / clientInformation / codeVerifier). + // the same cleaned state. Mutations always re-read disk; the in-memory mirror + // serves the SDK's sync getters, refreshed from disk when the auth file + // changes so tokens saved by another session are picked up immediately. const stored: MCPAuthState = await updateAuthState( identity, (state) => { dropStaleClientRegistration(state, opts.redirectUrl); }, - opts.home, + home, ); const apply = async (mutator: (state: MCPAuthState) => void): Promise => { - const next = await updateAuthState(identity, mutator, opts.home); + const next = await updateAuthState(identity, mutator, home); replaceStored(stored, next); }; + // Cheap staleness guard: statSync per getter, sync read only when the file's + // mtime or size changed. A vanished file keeps the mirror rather than throwing. + const authPath = authFilePath(identity, home); + let seenStamp: string | undefined; + const refreshFromDisk = (): void => { + let stamp: string | undefined; + try { + const stat = statSync(authPath); + stamp = `${String(stat.mtimeMs)}:${String(stat.size)}`; + } catch { + // File gone (or unreadable): keep serving the in-memory mirror rather + // than throwing from a sync getter. + if (seenStamp === undefined) return; + seenStamp = undefined; + return; + } + if (stamp === seenStamp) return; + seenStamp = stamp; + replaceStored(stored, loadAuthStateSync(identity, home)); + }; + let oauthState: string | undefined; return { get redirectUrl(): string { @@ -89,6 +120,7 @@ export async function createOAuthProvider( }; }, clientInformation(): OAuthClientInformationMixed | undefined { + refreshFromDisk(); return stored.clientInformation; }, saveClientInformation(info: OAuthClientInformationMixed): Promise { @@ -97,6 +129,7 @@ export async function createOAuthProvider( }); }, tokens(): OAuthTokens | undefined { + refreshFromDisk(); return stored.tokens; }, saveTokens(tokens: OAuthTokens): Promise { @@ -115,6 +148,7 @@ export async function createOAuthProvider( }); }, codeVerifier(): string { + refreshFromDisk(); if (stored.codeVerifier === undefined) throw new Error("No PKCE code verifier saved for this authorization."); return stored.codeVerifier; From 755903b7bd9783bbd003c01723a64efcc64d7278 Mon Sep 17 00:00:00 2001 From: Sawyer Date: Tue, 8 Sep 2026 21:53:03 -0700 Subject: [PATCH 2/5] Retry OAuth disk refresh after a failed read The refresh guard recorded the file stamp before reading it, so a persistently unreadable auth file (e.g. chmod 000) wiped the in-memory mirror with empty state and never retried. The stamp is now committed only after a successful read, leaving it stale so the next getter call retries. loadAuthStateSync also now matches loadAuthState's error contract: missing (ENOENT) or corrupt files yield empty state, while other read errors propagate to the caller instead of being swallowed. --- src/mcp/auth-store.ts | 17 +++++++++++++---- src/mcp/oauth-provider.test.ts | 27 +++++++++++++++++++++++++-- src/mcp/oauth-provider.ts | 10 +++++++++- 3 files changed, 47 insertions(+), 7 deletions(-) diff --git a/src/mcp/auth-store.ts b/src/mcp/auth-store.ts index cbd333fa4..fa5a4f3e9 100644 --- a/src/mcp/auth-store.ts +++ b/src/mcp/auth-store.ts @@ -51,8 +51,9 @@ export function authFilePath(identity: MCPAuthIdentity, home: string = homedir() } // Synchronous mirror of loadAuthState for the SDK's sync getters (tokens(), -// clientInformation(), codeVerifier()), which cannot await disk I/O. A missing or -// corrupt file yields empty state, matching loadAuthState's tolerance. +// clientInformation(), codeVerifier()), which cannot await disk I/O. Tolerates +// a missing (ENOENT) or corrupt file with empty state, matching loadAuthState; +// other read errors propagate to the caller. export function loadAuthStateSync( identity: MCPAuthIdentity, home: string = homedir(), @@ -60,8 +61,16 @@ export function loadAuthStateSync( let raw: string; try { raw = readFileSync(authFilePath(identity, home), "utf8"); - } catch { - return {}; + } catch (err) { + if ( + typeof err === "object" && + err !== null && + "code" in err && + (err as { code?: unknown }).code === "ENOENT" + ) { + return {}; + } + throw err; } try { const parsed: unknown = JSON.parse(raw); diff --git a/src/mcp/oauth-provider.test.ts b/src/mcp/oauth-provider.test.ts index 6be4f90e1..daacc3fc5 100644 --- a/src/mcp/oauth-provider.test.ts +++ b/src/mcp/oauth-provider.test.ts @@ -1,8 +1,8 @@ import { describe, expect, test } from "bun:test"; -import { mkdir, mkdtemp, readFile, writeFile } from "node:fs/promises"; +import { chmod, appendFile, mkdir, mkdtemp, readFile, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { loadAuthState, saveAuthState, deleteAuthState } from "./auth-store.js"; +import { authFilePath, loadAuthState, saveAuthState, deleteAuthState } from "./auth-store.js"; import { createOAuthProvider } from "./oauth-provider.js"; async function tempHome(): Promise { @@ -254,6 +254,29 @@ describe("createOAuthProvider", () => { expect(await syncValue(provider.clientInformation())).toBeUndefined(); }); + test("keeps the mirror through an unreadable auth file and recovers once readable", async () => { + const home = await tempHome(); + const provider = await createOAuthProvider({ + serverName: "linear", + serverURL: linear.serverURL, + redirectUrl: "http://127.0.0.1:1/callback", + onAuthURL: () => undefined, + home, + }); + await provider.saveTokens({ access_token: "tok", token_type: "bearer" }); + + // Force a stat change so the mtime guard actually attempts the read. + const path = authFilePath(linear, home); + await appendFile(path, " "); + await chmod(path, 0o000); + expect((await syncValue(provider.tokens()))?.access_token).toBe("tok"); + expect((await syncValue(provider.tokens()))?.access_token).toBe("tok"); + + await chmod(path, 0o600); + await saveAuthState(linear, { tokens: { access_token: "fresh", token_type: "bearer" } }, home); + expect((await syncValue(provider.tokens()))?.access_token).toBe("fresh"); + }); + test("does not delete scoped state whose filename stem is another provider name", async () => { const home = await tempHome(); const dir = join(home, ".corbits", "mcp-auth"); diff --git a/src/mcp/oauth-provider.ts b/src/mcp/oauth-provider.ts index 6f36a1b18..922c696d2 100644 --- a/src/mcp/oauth-provider.ts +++ b/src/mcp/oauth-provider.ts @@ -97,8 +97,16 @@ export async function createOAuthProvider( return; } if (stamp === seenStamp) return; + let next: MCPAuthState; + try { + next = loadAuthStateSync(identity, home); + } catch { + // Unreadable rather than missing: leave the stamp stale so the next + // getter call retries instead of pinning an empty state over the mirror. + return; + } seenStamp = stamp; - replaceStored(stored, loadAuthStateSync(identity, home)); + replaceStored(stored, next); }; let oauthState: string | undefined; From a41fda5c76b42c55232e3214c48f1e5eda4b028c Mon Sep 17 00:00:00 2001 From: Sawyer Date: Wed, 9 Sep 2026 10:46:44 -0700 Subject: [PATCH 3/5] Keep in-progress OAuth PKCE and client registration local A different-port sibling writing PKCE or DCR was replacing this session's live verifier and redirect. Failed disk reads also cleared good in-memory tokens because empty-on-corrupt was used as cache invalidation. --- src/mcp/auth-store.ts | 109 ++++++++++++++++++--------------- src/mcp/oauth-provider.test.ts | 106 +++++++++++++++++++++++++++++--- src/mcp/oauth-provider.ts | 82 +++++++++++-------------- 3 files changed, 194 insertions(+), 103 deletions(-) diff --git a/src/mcp/auth-store.ts b/src/mcp/auth-store.ts index fa5a4f3e9..37207ff92 100644 --- a/src/mcp/auth-store.ts +++ b/src/mcp/auth-store.ts @@ -50,28 +50,16 @@ export function authFilePath(identity: MCPAuthIdentity, home: string = homedir() return join(mcpAuthDir(home), `${serverDisplaySlug(identity.serverName)}-${digest}.json`); } -// Synchronous mirror of loadAuthState for the SDK's sync getters (tokens(), -// clientInformation(), codeVerifier()), which cannot await disk I/O. Tolerates -// a missing (ENOENT) or corrupt file with empty state, matching loadAuthState; -// other read errors propagate to the caller. -export function loadAuthStateSync( - identity: MCPAuthIdentity, - home: string = homedir(), -): MCPAuthState { - let raw: string; - try { - raw = readFileSync(authFilePath(identity, home), "utf8"); - } catch (err) { - if ( - typeof err === "object" && - err !== null && - "code" in err && - (err as { code?: unknown }).code === "ENOENT" - ) { - return {}; - } - throw err; - } +function isEnoent(err: unknown): boolean { + return ( + typeof err === "object" && + err !== null && + "code" in err && + (err as { code?: unknown }).code === "ENOENT" + ); +} + +function parseAuthState(raw: string): MCPAuthState | undefined { try { const parsed: unknown = JSON.parse(raw); if (typeof parsed === "object" && parsed !== null) return parsed as MCPAuthState; @@ -79,35 +67,63 @@ export function loadAuthStateSync( // A corrupt auth file should not wedge the session; treat it as no state and // let a fresh authorization overwrite it. } - return {}; + return undefined; } -export async function loadAuthState( - identity: MCPAuthIdentity, - home: string = homedir(), -): Promise { - let raw: string; +function stateFromRaw(raw: string | undefined): MCPAuthState { + if (raw === undefined) return {}; + return parseAuthState(raw) ?? {}; +} + +function readAuthFileSync(path: string): string | undefined { try { - raw = await readFile(authFilePath(identity, home), "utf8"); + return readFileSync(path, "utf8"); } catch (err) { - if ( - typeof err === "object" && - err !== null && - "code" in err && - (err as { code?: unknown }).code === "ENOENT" - ) { - return {}; - } + if (isEnoent(err)) return undefined; throw err; } +} + +async function readAuthFile(path: string): Promise { try { - const parsed = JSON.parse(raw); - if (typeof parsed === "object" && parsed !== null) return parsed as MCPAuthState; + return await readFile(path, "utf8"); + } catch (err) { + if (isEnoent(err)) return undefined; + throw err; + } +} + +// Synchronous connect-contract load. Tolerates a missing (ENOENT) or corrupt +// file with empty state, matching loadAuthState; other read errors propagate. +export function loadAuthStateSync( + identity: MCPAuthIdentity, + home: string = homedir(), +): MCPAuthState { + return stateFromRaw(readAuthFileSync(authFilePath(identity, home))); +} + +export async function loadAuthState( + identity: MCPAuthIdentity, + home: string = homedir(), +): Promise { + return stateFromRaw(await readAuthFile(authFilePath(identity, home))); +} + +// Cache refresh for a live provider: missing, unreadable, or corrupt files +// return undefined so the caller keeps its in-memory mirror. Empty-on-corrupt +// is loadAuthState's connect contract, not cache invalidation. +export function tryLoadAuthStateSync( + identity: MCPAuthIdentity, + home: string = homedir(), +): MCPAuthState | undefined { + let raw: string | undefined; + try { + raw = readAuthFileSync(authFilePath(identity, home)); } catch { - // A corrupt auth file should not wedge the session; treat it as no state and - // let a fresh authorization overwrite it. + return undefined; } - return {}; + if (raw === undefined) return undefined; + return parseAuthState(raw); } // pid alone is not unique per call — concurrent saves in one process must not @@ -207,14 +223,7 @@ async function unlinkAuthFile(path: string): Promise { try { await unlink(path); } catch (err) { - if ( - typeof err === "object" && - err !== null && - "code" in err && - (err as { code?: unknown }).code === "ENOENT" - ) { - return; - } + if (isEnoent(err)) return; throw err; } } diff --git a/src/mcp/oauth-provider.test.ts b/src/mcp/oauth-provider.test.ts index daacc3fc5..b8748d6ce 100644 --- a/src/mcp/oauth-provider.test.ts +++ b/src/mcp/oauth-provider.test.ts @@ -1,5 +1,6 @@ import { describe, expect, test } from "bun:test"; -import { chmod, appendFile, mkdir, mkdtemp, readFile, writeFile } from "node:fs/promises"; +import { readFileSync } from "node:fs"; +import { chmod, mkdir, mkdtemp, readFile, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { authFilePath, loadAuthState, saveAuthState, deleteAuthState } from "./auth-store.js"; @@ -25,6 +26,15 @@ async function syncValue(value: T | Promise): Promise { return await value; } +async function saveClient( + provider: Awaited>, + info: ReturnType, +): Promise { + const save = provider.saveClientInformation; + if (save === undefined) throw new Error("saveClientInformation is required"); + await save(info); +} + describe("createOAuthProvider", () => { test("drops stale DCR client when redirect port changed and no tokens exist", async () => { const home = await tempHome(); @@ -236,6 +246,60 @@ describe("createOAuthProvider", () => { expect((await syncValue(b.tokens()))?.access_token).toBe("fresh"); }); + test("does not replace an in-progress PKCE verifier from a different-port sibling", async () => { + const home = await tempHome(); + const a = await createOAuthProvider({ + serverName: "linear", + serverURL: linear.serverURL, + redirectUrl: "http://127.0.0.1:62000/callback", + onAuthURL: () => undefined, + home, + }); + await saveClient(a, clientInfo(62000)); + await a.saveCodeVerifier("pkce-a"); + + const b = await createOAuthProvider({ + serverName: "linear", + serverURL: linear.serverURL, + redirectUrl: "http://127.0.0.1:60435/callback", + onAuthURL: () => undefined, + home, + }); + expect(a.codeVerifier()).toBe("pkce-a"); + + await b.saveCodeVerifier("pkce-b"); + expect(a.codeVerifier()).toBe("pkce-a"); + expect(b.codeVerifier()).toBe("pkce-b"); + }); + + test("does not adopt a different-port sibling DCR client without tokens", async () => { + const home = await tempHome(); + const a = await createOAuthProvider({ + serverName: "linear", + serverURL: linear.serverURL, + redirectUrl: "http://127.0.0.1:62000/callback", + onAuthURL: () => undefined, + home, + }); + await saveClient(a, clientInfo(62000)); + + const b = await createOAuthProvider({ + serverName: "linear", + serverURL: linear.serverURL, + redirectUrl: "http://127.0.0.1:60435/callback", + onAuthURL: () => undefined, + home, + }); + expect((await syncValue(a.clientInformation()))?.client_id).toBe("client-on-62000"); + + await saveClient(b, clientInfo(60435)); + const info = await syncValue(a.clientInformation()); + expect(info?.client_id).toBe("client-on-62000"); + expect(info && "redirect_uris" in info ? info.redirect_uris : undefined).toEqual([ + "http://127.0.0.1:62000/callback", + ]); + }); + test("sync getters fall back to the in-memory mirror when the auth file disappears", async () => { const home = await tempHome(); const provider = await createOAuthProvider({ @@ -254,7 +318,7 @@ describe("createOAuthProvider", () => { expect(await syncValue(provider.clientInformation())).toBeUndefined(); }); - test("keeps the mirror through an unreadable auth file and recovers once readable", async () => { + test("keeps live tokens when the auth file is corrupt", async () => { const home = await tempHome(); const provider = await createOAuthProvider({ serverName: "linear", @@ -265,14 +329,42 @@ describe("createOAuthProvider", () => { }); await provider.saveTokens({ access_token: "tok", token_type: "bearer" }); - // Force a stat change so the mtime guard actually attempts the read. const path = authFilePath(linear, home); - await appendFile(path, " "); - await chmod(path, 0o000); - expect((await syncValue(provider.tokens()))?.access_token).toBe("tok"); + await writeFile(path, "{not-json"); expect((await syncValue(provider.tokens()))?.access_token).toBe("tok"); + }); + + test("keeps the mirror through an unreadable auth file and recovers once readable", async () => { + const home = await tempHome(); + const provider = await createOAuthProvider({ + serverName: "linear", + serverURL: linear.serverURL, + redirectUrl: "http://127.0.0.1:1/callback", + onAuthURL: () => undefined, + home, + }); + await provider.saveTokens({ access_token: "tok", token_type: "bearer" }); - await chmod(path, 0o600); + const path = authFilePath(linear, home); + await chmod(path, 0o000); + try { + try { + readFileSync(path, "utf8"); + // Owner-read still succeeds (root or platforms that ignore mode) — skip. + return; + } catch (err) { + expect( + typeof err === "object" && + err !== null && + "code" in err && + (err as { code?: unknown }).code === "EACCES", + ).toBe(true); + } + expect((await syncValue(provider.tokens()))?.access_token).toBe("tok"); + expect((await syncValue(provider.tokens()))?.access_token).toBe("tok"); + } finally { + await chmod(path, 0o600); + } await saveAuthState(linear, { tokens: { access_token: "fresh", token_type: "bearer" } }, home); expect((await syncValue(provider.tokens()))?.access_token).toBe("fresh"); }); diff --git a/src/mcp/oauth-provider.ts b/src/mcp/oauth-provider.ts index 922c696d2..0e7e6f05c 100644 --- a/src/mcp/oauth-provider.ts +++ b/src/mcp/oauth-provider.ts @@ -1,3 +1,4 @@ +import { homedir } from "node:os"; import type { OAuthClientProvider } from "@modelcontextprotocol/sdk/client/auth.js"; import type { OAuthClientInformationFull, @@ -6,14 +7,11 @@ import type { OAuthTokens, } from "@modelcontextprotocol/sdk/shared/auth.js"; import { - authFilePath, - loadAuthStateSync, + tryLoadAuthStateSync, updateAuthState, type MCPAuthIdentity, type MCPAuthState, } from "./auth-store.js"; -import { statSync } from "node:fs"; -import { homedir } from "node:os"; import { MCP_CLIENT_NAME } from "../branding.js"; export interface OAuthProviderOptions { @@ -46,13 +44,22 @@ function dropStaleClientRegistration(state: MCPAuthState, redirectUrl: string): delete state.codeVerifier; } -function replaceStored(stored: MCPAuthState, next: MCPAuthState): void { - delete stored.clientInformation; - delete stored.tokens; - delete stored.codeVerifier; - if (next.clientInformation !== undefined) stored.clientInformation = next.clientInformation; +function shouldAdoptClient(stored: MCPAuthState, next: MCPAuthState, redirectUrl: string): boolean { + if (next.clientInformation === undefined) return false; + if (redirectUrisInclude(next.clientInformation, redirectUrl)) return true; + // Other-port DCR is a sibling's in-progress registration unless they also + // published new tokens (completed re-auth). + return next.tokens !== undefined && next.tokens.access_token !== stored.tokens?.access_token; +} + +function assignTokens(stored: MCPAuthState, next: MCPAuthState): void { if (next.tokens !== undefined) stored.tokens = next.tokens; - if (next.codeVerifier !== undefined) stored.codeVerifier = next.codeVerifier; + else delete stored.tokens; +} + +function assignClient(stored: MCPAuthState, next: MCPAuthState): void { + if (next.clientInformation !== undefined) stored.clientInformation = next.clientInformation; + else delete stored.clientInformation; } export async function createOAuthProvider( @@ -64,9 +71,10 @@ export async function createOAuthProvider( }; const home = opts.home ?? homedir(); // Load + scrub stale DCR under the per-file chain so concurrent providers see - // the same cleaned state. Mutations always re-read disk; the in-memory mirror - // serves the SDK's sync getters, refreshed from disk when the auth file - // changes so tokens saved by another session are picked up immediately. + // the same cleaned state. Mutations always re-read disk; tokens and matching + // DCR are observed from disk so a sibling session's completed auth is picked + // up. PKCE stays instance-local after this snapshot — a different-port sibling + // must not clobber an in-progress verifier. const stored: MCPAuthState = await updateAuthState( identity, (state) => { @@ -77,36 +85,16 @@ export async function createOAuthProvider( const apply = async (mutator: (state: MCPAuthState) => void): Promise => { const next = await updateAuthState(identity, mutator, home); - replaceStored(stored, next); + assignTokens(stored, next); + assignClient(stored, next); }; - // Cheap staleness guard: statSync per getter, sync read only when the file's - // mtime or size changed. A vanished file keeps the mirror rather than throwing. - const authPath = authFilePath(identity, home); - let seenStamp: string | undefined; - const refreshFromDisk = (): void => { - let stamp: string | undefined; - try { - const stat = statSync(authPath); - stamp = `${String(stat.mtimeMs)}:${String(stat.size)}`; - } catch { - // File gone (or unreadable): keep serving the in-memory mirror rather - // than throwing from a sync getter. - if (seenStamp === undefined) return; - seenStamp = undefined; - return; - } - if (stamp === seenStamp) return; - let next: MCPAuthState; - try { - next = loadAuthStateSync(identity, home); - } catch { - // Unreadable rather than missing: leave the stamp stale so the next - // getter call retries instead of pinning an empty state over the mirror. - return; - } - seenStamp = stamp; - replaceStored(stored, next); + const refreshDurableFromDisk = (): void => { + const next = tryLoadAuthStateSync(identity, home); + if (next === undefined) return; + const adoptClient = shouldAdoptClient(stored, next, opts.redirectUrl); + assignTokens(stored, next); + if (adoptClient) assignClient(stored, next); }; let oauthState: string | undefined; @@ -128,16 +116,17 @@ export async function createOAuthProvider( }; }, clientInformation(): OAuthClientInformationMixed | undefined { - refreshFromDisk(); + refreshDurableFromDisk(); return stored.clientInformation; }, saveClientInformation(info: OAuthClientInformationMixed): Promise { + stored.clientInformation = info as OAuthClientInformationFull; return apply((state) => { state.clientInformation = info as OAuthClientInformationFull; }); }, tokens(): OAuthTokens | undefined { - refreshFromDisk(); + refreshDurableFromDisk(); return stored.tokens; }, saveTokens(tokens: OAuthTokens): Promise { @@ -151,19 +140,19 @@ export async function createOAuthProvider( opts.onAuthURL(opts.serverName, authorizationUrl.toString()); }, saveCodeVerifier(codeVerifier: string): Promise { + stored.codeVerifier = codeVerifier; return apply((state) => { state.codeVerifier = codeVerifier; }); }, codeVerifier(): string { - refreshFromDisk(); if (stored.codeVerifier === undefined) throw new Error("No PKCE code verifier saved for this authorization."); return stored.codeVerifier; }, - resetAuthorization(): Promise { + async resetAuthorization(): Promise { oauthState = undefined; - return apply((state) => { + await apply((state) => { delete state.tokens; delete state.codeVerifier; // Next browser flow needs a client registered for *this* loopback port. @@ -171,6 +160,7 @@ export async function createOAuthProvider( delete state.clientInformation; } }); + delete stored.codeVerifier; }, }; } From e6cfdc0a4351774af0790b814803e0721a0021fe Mon Sep 17 00:00:00 2001 From: Sawyer Date: Wed, 9 Sep 2026 13:19:06 -0700 Subject: [PATCH 4/5] Keep matching OAuth DCR across sibling saveTokens apply() assigned disk client after every mutation, so a different-port sibling saveClient then this session's saveTokens replaced live DCR. Mutations now rewrite this session's matching client onto disk; getters still adopt completed sibling tokens. --- src/mcp/oauth-provider.test.ts | 131 ++++++++++++++++++++++++++++++++- src/mcp/oauth-provider.ts | 50 ++++++++++++- 2 files changed, 178 insertions(+), 3 deletions(-) diff --git a/src/mcp/oauth-provider.test.ts b/src/mcp/oauth-provider.test.ts index b8748d6ce..7ea6b1e4a 100644 --- a/src/mcp/oauth-provider.test.ts +++ b/src/mcp/oauth-provider.test.ts @@ -1,4 +1,5 @@ -import { describe, expect, test } from "bun:test"; +import { describe, expect, spyOn, test } from "bun:test"; +import * as fs from "node:fs"; import { readFileSync } from "node:fs"; import { chmod, mkdir, mkdtemp, readFile, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; @@ -300,6 +301,112 @@ describe("createOAuthProvider", () => { ]); }); + test("saveTokens after a different-port sibling construct keeps this session's DCR", async () => { + const home = await tempHome(); + const a = await createOAuthProvider({ + serverName: "linear", + serverURL: linear.serverURL, + redirectUrl: "http://127.0.0.1:62000/callback", + onAuthURL: () => undefined, + home, + }); + await saveClient(a, clientInfo(62000)); + await a.saveCodeVerifier("pkce-a"); + + const b = await createOAuthProvider({ + serverName: "linear", + serverURL: linear.serverURL, + redirectUrl: "http://127.0.0.1:60435/callback", + onAuthURL: () => undefined, + home, + }); + expect(await syncValue(b.clientInformation())).toBeUndefined(); + + await a.saveTokens({ + access_token: "tok-a", + token_type: "bearer", + expires_in: 3600, + refresh_token: "ref-a", + }); + + const info = await syncValue(a.clientInformation()); + expect(info?.client_id).toBe("client-on-62000"); + expect(info && "redirect_uris" in info ? info.redirect_uris : undefined).toEqual([ + "http://127.0.0.1:62000/callback", + ]); + const disk = await loadAuthState(linear, home); + expect(disk.clientInformation?.client_id).toBe("client-on-62000"); + expect(disk.clientInformation?.redirect_uris).toEqual(["http://127.0.0.1:62000/callback"]); + }); + + test("saveTokens after a different-port sibling saveClient keeps this session's DCR", async () => { + const home = await tempHome(); + const a = await createOAuthProvider({ + serverName: "linear", + serverURL: linear.serverURL, + redirectUrl: "http://127.0.0.1:62000/callback", + onAuthURL: () => undefined, + home, + }); + await saveClient(a, clientInfo(62000)); + await a.saveCodeVerifier("pkce-a"); + + const b = await createOAuthProvider({ + serverName: "linear", + serverURL: linear.serverURL, + redirectUrl: "http://127.0.0.1:60435/callback", + onAuthURL: () => undefined, + home, + }); + await saveClient(b, clientInfo(60435)); + + await a.saveTokens({ + access_token: "tok-a", + token_type: "bearer", + expires_in: 3600, + refresh_token: "ref-a", + }); + + const info = await syncValue(a.clientInformation()); + expect(info?.client_id).toBe("client-on-62000"); + expect(info && "redirect_uris" in info ? info.redirect_uris : undefined).toEqual([ + "http://127.0.0.1:62000/callback", + ]); + const disk = await loadAuthState(linear, home); + expect(disk.clientInformation?.client_id).toBe("client-on-62000"); + expect(disk.clientInformation?.redirect_uris).toEqual(["http://127.0.0.1:62000/callback"]); + expect(disk.tokens?.access_token).toBe("tok-a"); + expect((await syncValue(a.tokens()))?.access_token).toBe("tok-a"); + }); + + test("saveCodeVerifier after a different-port sibling saveClient does not adopt that client", async () => { + const home = await tempHome(); + const a = await createOAuthProvider({ + serverName: "linear", + serverURL: linear.serverURL, + redirectUrl: "http://127.0.0.1:62000/callback", + onAuthURL: () => undefined, + home, + }); + await saveClient(a, clientInfo(62000)); + + const b = await createOAuthProvider({ + serverName: "linear", + serverURL: linear.serverURL, + redirectUrl: "http://127.0.0.1:60435/callback", + onAuthURL: () => undefined, + home, + }); + await saveClient(b, clientInfo(60435)); + await a.saveCodeVerifier("pkce-a"); + + const info = await syncValue(a.clientInformation()); + expect(info?.client_id).toBe("client-on-62000"); + expect(info && "redirect_uris" in info ? info.redirect_uris : undefined).toEqual([ + "http://127.0.0.1:62000/callback", + ]); + }); + test("sync getters fall back to the in-memory mirror when the auth file disappears", async () => { const home = await tempHome(); const provider = await createOAuthProvider({ @@ -369,6 +476,28 @@ describe("createOAuthProvider", () => { expect((await syncValue(provider.tokens()))?.access_token).toBe("fresh"); }); + test("sync getters skip reading the auth file when mtime and size are unchanged", async () => { + const home = await tempHome(); + const provider = await createOAuthProvider({ + serverName: "linear", + serverURL: linear.serverURL, + redirectUrl: "http://127.0.0.1:1/callback", + onAuthURL: () => undefined, + home, + }); + await provider.saveTokens({ access_token: "tok", token_type: "bearer" }); + expect((await syncValue(provider.tokens()))?.access_token).toBe("tok"); + + const read = spyOn(fs, "readFileSync"); + try { + expect((await syncValue(provider.tokens()))?.access_token).toBe("tok"); + expect(await syncValue(provider.clientInformation())).toBeUndefined(); + expect(read).not.toHaveBeenCalled(); + } finally { + read.mockRestore(); + } + }); + test("does not delete scoped state whose filename stem is another provider name", async () => { const home = await tempHome(); const dir = join(home, ".corbits", "mcp-auth"); diff --git a/src/mcp/oauth-provider.ts b/src/mcp/oauth-provider.ts index 0e7e6f05c..7c58b2b1a 100644 --- a/src/mcp/oauth-provider.ts +++ b/src/mcp/oauth-provider.ts @@ -1,3 +1,4 @@ +import { statSync } from "node:fs"; import { homedir } from "node:os"; import type { OAuthClientProvider } from "@modelcontextprotocol/sdk/client/auth.js"; import type { @@ -7,6 +8,7 @@ import type { OAuthTokens, } from "@modelcontextprotocol/sdk/shared/auth.js"; import { + authFilePath, tryLoadAuthStateSync, updateAuthState, type MCPAuthIdentity, @@ -62,6 +64,25 @@ function assignClient(stored: MCPAuthState, next: MCPAuthState): void { else delete stored.clientInformation; } +function matchingLiveClient( + stored: MCPAuthState, + redirectUrl: string, +): OAuthClientInformationFull | undefined { + const live = stored.clientInformation; + if (live === undefined || !redirectUrisInclude(live, redirectUrl)) return undefined; + return live; +} + +function persistMatchingLiveClient( + stored: MCPAuthState, + next: MCPAuthState, + redirectUrl: string, +): void { + const live = matchingLiveClient(stored, redirectUrl); + if (live === undefined) return; + next.clientInformation = live; +} + export async function createOAuthProvider( opts: OAuthProviderOptions, ): Promise { @@ -84,14 +105,39 @@ export async function createOAuthProvider( ); const apply = async (mutator: (state: MCPAuthState) => void): Promise => { - const next = await updateAuthState(identity, mutator, home); + const next = await updateAuthState( + identity, + (state) => { + mutator(state); + persistMatchingLiveClient(stored, state, opts.redirectUrl); + }, + home, + ); assignTokens(stored, next); - assignClient(stored, next); + if (matchingLiveClient(stored, opts.redirectUrl) === undefined) { + assignClient(stored, next); + } }; + // Cheap staleness guard: statSync per getter, sync read only when the file's + // mtime or size changed. Stamp commits only after a successful read so a + // failed/unreadable file is retried on the next getter call. + const authPath = authFilePath(identity, home); + let seenStamp: string | undefined; const refreshDurableFromDisk = (): void => { + let stamp: string | undefined; + try { + const stat = statSync(authPath); + stamp = `${String(stat.mtimeMs)}:${String(stat.size)}`; + } catch { + if (seenStamp === undefined) return; + seenStamp = undefined; + return; + } + if (stamp === seenStamp) return; const next = tryLoadAuthStateSync(identity, home); if (next === undefined) return; + seenStamp = stamp; const adoptClient = shouldAdoptClient(stored, next, opts.redirectUrl); assignTokens(stored, next); if (adoptClient) assignClient(stored, next); From 0c8cc4def8eb2d0ca84f98315a5e0b46dcf0344c Mon Sep 17 00:00:00 2001 From: Sawyer Date: Wed, 9 Sep 2026 15:24:43 -0700 Subject: [PATCH 5/5] Cover idle getter token adoption and same-port DCR rotation Idle sibling tokens must still land on a session that already has matching live DCR, without going through apply. Same-port saveClient must replace this session's client even after a sibling wrote a different port. --- src/mcp/oauth-provider.test.ts | 72 ++++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/src/mcp/oauth-provider.test.ts b/src/mcp/oauth-provider.test.ts index 7ea6b1e4a..0696f8c06 100644 --- a/src/mcp/oauth-provider.test.ts +++ b/src/mcp/oauth-provider.test.ts @@ -407,6 +407,78 @@ describe("createOAuthProvider", () => { ]); }); + test("idle tokens getter adopts a sibling's completed auth without rewriting matching DCR", async () => { + const home = await tempHome(); + const a = await createOAuthProvider({ + serverName: "linear", + serverURL: linear.serverURL, + redirectUrl: "http://127.0.0.1:62000/callback", + onAuthURL: () => undefined, + home, + }); + await saveClient(a, clientInfo(62000)); + await a.saveTokens({ + access_token: "tok-a", + token_type: "bearer", + expires_in: 3600, + refresh_token: "ref-a", + }); + + const b = await createOAuthProvider({ + serverName: "linear", + serverURL: linear.serverURL, + redirectUrl: "http://127.0.0.1:60435/callback", + onAuthURL: () => undefined, + home, + }); + await saveClient(b, clientInfo(60435)); + await b.saveTokens({ + access_token: "tok-b", + token_type: "bearer", + expires_in: 3600, + refresh_token: "ref-b", + }); + + expect((await syncValue(a.tokens()))?.access_token).toBe("tok-b"); + const disk = await loadAuthState(linear, home); + expect(disk.tokens?.access_token).toBe("tok-b"); + expect(disk.clientInformation?.client_id).toBe("client-on-60435"); + expect(disk.clientInformation?.redirect_uris).toEqual(["http://127.0.0.1:60435/callback"]); + }); + + test("same-port DCR rotation after a different-port sibling saveClient keeps the new client", async () => { + const home = await tempHome(); + const v1 = { ...clientInfo(62000), client_id: "client-on-62000-v1" }; + const v2 = { ...clientInfo(62000), client_id: "client-on-62000-v2" }; + const a = await createOAuthProvider({ + serverName: "linear", + serverURL: linear.serverURL, + redirectUrl: "http://127.0.0.1:62000/callback", + onAuthURL: () => undefined, + home, + }); + await saveClient(a, v1); + + const b = await createOAuthProvider({ + serverName: "linear", + serverURL: linear.serverURL, + redirectUrl: "http://127.0.0.1:60435/callback", + onAuthURL: () => undefined, + home, + }); + await saveClient(b, clientInfo(60435)); + await saveClient(a, v2); + + const info = await syncValue(a.clientInformation()); + expect(info?.client_id).toBe("client-on-62000-v2"); + expect(info && "redirect_uris" in info ? info.redirect_uris : undefined).toEqual([ + "http://127.0.0.1:62000/callback", + ]); + const disk = await loadAuthState(linear, home); + expect(disk.clientInformation?.client_id).toBe("client-on-62000-v2"); + expect(disk.clientInformation?.redirect_uris).toEqual(["http://127.0.0.1:62000/callback"]); + }); + test("sync getters fall back to the in-memory mirror when the auth file disappears", async () => { const home = await tempHome(); const provider = await createOAuthProvider({