fix(mcp): harden auth disk boundary and guard refresh-vs-reset race

Author: TheGreatAxios

src/mcp/auth-store.ts

@@ -77,11 +77,28 @@ function isEnoent(err: unknown): boolean {
   );
 }
 
+function isAuthRecord(value: unknown): value is Record<string, unknown> {
+  return typeof value === "object" && value !== null && !Array.isArray(value);
+}
+
+export function isValidAuthTokens(value: unknown): value is OAuthTokens {
+  return isAuthRecord(value) && typeof value.access_token === "string";
+}
+
 function parseAuthState(raw: string): MCPAuthState | undefined {
   try {
     const parsed: unknown = JSON.parse(raw);
-    if (typeof parsed === "object" && parsed !== null)
-      return parsed as MCPAuthState;
+    // The discriminator is explicit shape, not "parses to object": arrays,
+    // null, and primitives are corrupt. Tokens crossing from disk into memory
+    // must be absent (a missing key is an explicit logout write) or an object
+    // carrying a string access_token; anything else reads as unreadable so
+    // the caller keeps its in-memory mirror and never stores the malformed
+    // value.
+    if (!isAuthRecord(parsed)) return undefined;
+    const state = parsed as MCPAuthState;
+    if (state.tokens !== undefined && !isValidAuthTokens(state.tokens))
+      return undefined;
+    return state;
   } catch {
     // A corrupt auth file should not wedge the session; treat it as no state and
     // let a fresh authorization overwrite it.

src/mcp/oauth-provider.test.ts

@@ -738,6 +738,169 @@ describe("createOAuthProvider", () => {
     expect((await syncValue(provider.tokens()))?.access_token).toBe("tok");
   });
 
+  test("keeps the mirror when the auth file holds a JSON array", 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 saveClient(provider, clientInfo(1));
+    await provider.saveTokens({ access_token: "tok", token_type: "bearer" });
+
+    await writeFile(authFilePath(linear, home), "[]");
+
+    expect((await syncValue(provider.tokens()))?.access_token).toBe("tok");
+    expect((await syncValue(provider.clientInformation()))?.client_id).toBe(
+      "client-on-1",
+    );
+  });
+
+  test("treats malformed disk tokens as unreadable and never throws", async () => {
+    const malformed = [
+      `{"tokens":null}`,
+      `{"tokens":"tok-a"}`,
+      `{"tokens":{}}`,
+      `{"tokens":[]}`,
+    ];
+    for (const raw of malformed) {
+      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 writeFile(authFilePath(linear, home), raw);
+
+      expect((await syncValue(provider.tokens()))?.access_token).toBe("tok");
+      expect(await syncValue(provider.clientInformation())).toBeUndefined();
+    }
+  });
+
+  test("never throws when disk holds an other-port client with null tokens", async () => {
+    const home = await tempHome();
+    const provider = await createOAuthProvider({
+      serverName: "linear",
+      serverURL: linear.serverURL,
+      redirectUrl: "http://127.0.0.1:62000/callback",
+      onAuthURL: () => undefined,
+      home,
+    });
+    await saveClient(provider, clientInfo(62000));
+    await provider.saveTokens({
+      access_token: "tok-a",
+      token_type: "bearer",
+      expires_in: 3600,
+      refresh_token: "ref-a",
+    });
+
+    await writeFile(
+      authFilePath(linear, home),
+      JSON.stringify({
+        clientInformation: clientInfo(60435),
+        tokens: null,
+      }),
+    );
+
+    expect((await syncValue(provider.tokens()))?.access_token).toBe("tok-a");
+    expect((await syncValue(provider.clientInformation()))?.client_id).toBe(
+      "client-on-62000",
+    );
+  });
+
+  test("a mid-refresh sibling logout is not resurrected by the refresh write", async () => {
+    const home = await tempHome();
+    let releaseRefresh!: () => void;
+    const refreshGate = new Promise<void>((resolve) => {
+      releaseRefresh = resolve;
+    });
+    let refreshPosted!: () => void;
+    const refreshPostedGate = new Promise<void>((resolve) => {
+      refreshPosted = resolve;
+    });
+    const fetchFn = async (
+      url: string | URL,
+      init?: RequestInit,
+    ): Promise<Response> => {
+      const href = String(url);
+      if (init?.method === "POST") {
+        refreshPosted();
+        await refreshGate;
+        return new Response(
+          JSON.stringify({
+            access_token: "resurrected",
+            token_type: "bearer",
+            expires_in: 3600,
+            refresh_token: "ref-b",
+          }),
+          { status: 200, headers: { "content-type": "application/json" } },
+        );
+      }
+      if (href.includes("oauth-protected-resource")) {
+        return new Response(
+          JSON.stringify({
+            resource: "https://mcp.linear.app/mcp",
+            authorization_servers: ["https://mcp.linear.app"],
+          }),
+          { status: 200, headers: { "content-type": "application/json" } },
+        );
+      }
+      if (
+        href.includes("oauth-authorization-server") ||
+        href.includes("openid-configuration")
+      ) {
+        return new Response(
+          JSON.stringify({
+            issuer: "https://mcp.linear.app",
+            authorization_endpoint: "https://mcp.linear.app/authorize",
+            token_endpoint: "https://mcp.linear.app/oauth/token",
+            response_types_supported: ["code"],
+          }),
+          { status: 200, headers: { "content-type": "application/json" } },
+        );
+      }
+      return new Response(null, { status: 404 });
+    };
+
+    const a = await createOAuthProvider({
+      serverName: "linear",
+      serverURL: linear.serverURL,
+      redirectUrl: "http://127.0.0.1:62000/callback",
+      onAuthURL: () => undefined,
+      home,
+      fetchFn,
+    });
+    await saveClient(a, clientInfo(62000));
+    await a.saveTokens({
+      access_token: "stale",
+      token_type: "bearer",
+      expires_in: 1,
+      refresh_token: "ref-a",
+    });
+    const b = await createOAuthProvider({
+      serverName: "linear",
+      serverURL: linear.serverURL,
+      redirectUrl: "http://127.0.0.1:62000/callback",
+      onAuthURL: () => undefined,
+      home,
+    });
+
+    const pending = a.refreshToken("ref-a");
+    await refreshPostedGate;
+    await b.resetAuthorization();
+    releaseRefresh();
+    await pending;
+
+    expect((await loadAuthState(linear, home)).tokens).toBeUndefined();
+    expect(await syncValue(a.tokens())).toBeUndefined();
+  });
+
   test("keeps the mirror through an unreadable auth file and recovers once readable", async () => {
     const home = await tempHome();
     const provider = await createOAuthProvider({

src/mcp/oauth-provider.ts

@@ -20,6 +20,7 @@ import type {
 import type { FetchLike } from "@modelcontextprotocol/sdk/shared/transport.js";
 import {
   authFilePath,
+  isValidAuthTokens,
   tryLoadAuthStateSync,
   updateAuthState,
   type MCPAuthIdentity,
@@ -45,9 +46,9 @@ function redirectUrisInclude(
   info: OAuthClientInformationFull | undefined,
   redirectUrl: string,
 ): boolean {
-  const uris = info?.redirect_uris;
-  if (uris === undefined || uris.length === 0) return true;
-  return uris.includes(redirectUrl);
+  const uris: unknown = info?.redirect_uris;
+  if (!Array.isArray(uris) || uris.length === 0) return true;
+  return (uris as unknown[]).includes(redirectUrl);
 }
 
 function isAbortError(err: unknown): boolean {
@@ -77,12 +78,20 @@ function shouldAdoptClient(
   next: MCPAuthState,
   redirectUrl: string,
 ): boolean {
-  if (next.clientInformation === undefined) return false;
+  // Malformed disk must never throw or poison the mirror: adoption requires a
+  // client object and valid tokens. next.tokens is boundary-validated upstream,
+  // but the guards below keep this total even if that contract is bypassed.
+  if (
+    typeof next.clientInformation !== "object" ||
+    next.clientInformation === null ||
+    Array.isArray(next.clientInformation)
+  )
+    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 &&
+    isValidAuthTokens(next.tokens) &&
     next.tokens.access_token !== stored.tokens?.access_token
   );
 }
@@ -264,6 +273,16 @@ export async function createOAuthProvider(
     },
     refreshToken: async (refreshToken: string): Promise<OAuthTokens> => {
       try {
+        // Snapshot the durable access token before the network round-trip. A
+        // sibling resetAuthorization (logout) or rotation that lands mid-refresh
+        // must win over this stale grant: the post-refresh write re-checks disk
+        // and drops the refreshed tokens instead of resurrecting revoked ones.
+        // There is no server-side revocation, so the durable file is the only
+        // source of truth for "logged out". Sync read: refreshToken callers may
+        // busy-wait on microtasks alone, which an async disk read could never
+        // resolve behind.
+        const preRefreshAccess = tryLoadAuthStateSync(identity, home)?.tokens
+          ?.access_token;
         const fetchFn = opts.fetchFn;
         if (
           authorizationServerMetadata === undefined ||
@@ -312,9 +331,19 @@ export async function createOAuthProvider(
           resource: resourceURL,
           ...(fetchFn === undefined ? {} : { fetchFn }),
         });
+        let refreshSuperseded = false;
         await apply((state) => {
+          if (state.tokens?.access_token !== preRefreshAccess) {
+            refreshSuperseded = true;
+            return;
+          }
           state.tokens = tokens;
         });
+        if (refreshSuperseded) {
+          const disk = tryLoadAuthStateSync(identity, home);
+          if (disk?.tokens !== undefined) stored.tokens = disk.tokens;
+          else delete stored.tokens;
+        }
         return tokens;
       } catch (err) {
         if (isAbortError(err) || err instanceof UnauthorizedError) throw err;