fix(core): OAuth callback listener acts only on this flow's state

clients/cli/README.md

@@ -524,7 +524,7 @@ The CLI runs the same loopback callback server as the TUI (`http://127.0.0.1:627
 
 1. Starts the callback listener on `--callback-url` (or `MCP_OAUTH_CALLBACK_URL`)
 2. Prints the authorization URL to stderr (OSC 8 hyperlink when stderr is a TTY) and **opens the default browser** when allowed. Default: open on a **stderr** TTY (so `2>&1 | tee` prints a plain URL into the log but does not auto-open); plain URL only when stderr is non-TTY. `MCP_AUTO_OPEN_ENABLED=false` never opens; `=true` forces open even on a non-TTY (same as the web launcher).
-3. Waits for the browser redirect, exchanges the code, and retries connect or the failed RPC
+3. Waits for the browser redirect, exchanges the code, and retries connect or the failed RPC. Only a redirect echoing this flow's OAuth `state` is acted on — any other request to the listener, an `?error=` response included, gets a 400 and is ignored, so another local process or a web page cannot cancel the sign-in or choose the error it reports; the wait continues until the matching redirect or the 15-minute timeout
 
 Interactive OAuth (connect-time or mid-RPC) requires a TTY on **stdin or stderr** (so `2>&1 | tee` still works — stdin stays a TTY), or `MCP_AUTO_OPEN_ENABLED=true`. When neither stdin nor stderr is a TTY and that env is unset (typical CI), the CLI fails fast with `auth_required` instead of waiting up to 15 minutes on the loopback callback. Use **`--stored-auth-only`** for non-interactive runs that should only consume the shared store.
 

clients/cli/__tests__/oauth-interactive.test.ts

@@ -39,7 +39,7 @@ const oauthTestStatePath = join(
 
 async function completeOAuthAuthorization(
   authorizationUrl: URL,
-): Promise<{ code: string; iss?: string }> {
+): Promise<{ code: string; iss?: string; state?: string }> {
   let response = await fetch(authorizationUrl.toString(), {
     redirect: "manual",
   });
@@ -73,21 +73,25 @@ async function completeOAuthAuthorization(
     throw new Error("Missing authorization code");
   }
   const iss = redirect.searchParams.get("iss") ?? undefined;
-  return iss ? { code, iss } : { code };
+  // The loopback listener only acts on a callback echoing the flow's state
+  // (#2847), exactly as a real browser redirect does.
+  const state = redirect.searchParams.get("state") ?? undefined;
+  return { code, ...(iss ? { iss } : {}), ...(state ? { state } : {}) };
 }
 
 function createAutoCompleteNavigation(
   redirectUrlProvider: MutableRedirectUrlProvider,
 ) {
   return new CallbackNavigation(async (url) => {
-    const { code, iss } = await completeOAuthAuthorization(url);
+    const { code, iss, state } = await completeOAuthAuthorization(url);
     const redirect = redirectUrlProvider.redirectUrl;
     if (!redirect) {
       throw new Error("redirectUrl not set");
     }
     const callback = new URL(redirect);
     callback.searchParams.set("code", code);
     if (iss) callback.searchParams.set("iss", iss);
+    if (state) callback.searchParams.set("state", state);
     await fetch(callback.href);
   });
 }

clients/web/src/test/core/auth/runner-interactive-oauth.test.ts

@@ -37,6 +37,7 @@ function mockClient(
 interface MockCallbackHandlers {
   onCallback?: OAuthCallbackServerStartOptions["onCallback"];
   onError?: OAuthCallbackServerStartOptions["onError"];
+  expectedState?: OAuthCallbackServerStartOptions["expectedState"];
 }
 
 function createMockCallbackServer(handlers: {
@@ -47,6 +48,7 @@ function createMockCallbackServer(handlers: {
       handlers.current = {
         onCallback: opts.onCallback,
         onError: opts.onError,
+        expectedState: opts.expectedState,
       };
       return {
         port: 6276,
@@ -336,6 +338,7 @@ describe("runRunnerInteractiveOAuth", () => {
         handlers.current.onError?.({
           error: "access_denied",
           error_description: "user cancelled",
+          source: "redirect",
         });
         return new URL("https://as.example/authorize");
       }),
@@ -352,7 +355,177 @@ describe("runRunnerInteractiveOAuth", () => {
         },
         createCallbackServer: () => createMockCallbackServer(handlers),
       }),
-    ).rejects.toThrow("user cancelled");
+    ).rejects.toThrow(
+      "OAuth authorization failed. The authorization server said: user cancelled (access_denied)",
+    );
+  });
+
+  it("hands the listener the state of the authorize URL authenticate() returns (#2847)", async () => {
+    const client = mockClient({
+      authenticate: vi.fn(async () => {
+        await simulateCallback(handlers.current);
+        return new URL("https://as.example/authorize?state=connect-state");
+      }),
+    });
+
+    await runRunnerInteractiveOAuth({
+      client,
+      redirectUrlProvider: { redirectUrl: "" },
+      callbackListen: {
+        hostname: "127.0.0.1",
+        port: 6276,
+        pathname: "/oauth/callback",
+      },
+      createCallbackServer: () => createMockCallbackServer(handlers),
+    });
+
+    await expect(handlers.current.expectedState).resolves.toBe("connect-state");
+  });
+
+  it("hands the listener the step-up authorize URL's state before navigating (#2847)", async () => {
+    let stateAtNavigation: string | undefined;
+    const client = mockClient({
+      beginInteractiveAuthorization: vi.fn(async () => {
+        stateAtNavigation = await handlers.current.expectedState;
+        await simulateCallback(handlers.current, "step-up-code");
+      }),
+    });
+
+    await runRunnerInteractiveOAuth({
+      client,
+      redirectUrlProvider: { redirectUrl: "" },
+      callbackListen: {
+        hostname: "127.0.0.1",
+        port: 6276,
+        pathname: "/oauth/callback",
+      },
+      authorizationUrl: new URL("https://as.example/authorize?state=step-up"),
+      createCallbackServer: () => createMockCallbackServer(handlers),
+    });
+
+    expect(stateAtNavigation).toBe("step-up");
+  });
+
+  it("resolves an undefined expected state for an authorize URL without one", async () => {
+    const client = mockClient({
+      authenticate: vi.fn(async () => {
+        await simulateCallback(handlers.current);
+        return new URL("https://as.example/authorize");
+      }),
+    });
+
+    await runRunnerInteractiveOAuth({
+      client,
+      redirectUrlProvider: { redirectUrl: "" },
+      callbackListen: {
+        hostname: "127.0.0.1",
+        port: 6276,
+        pathname: "/oauth/callback",
+      },
+      createCallbackServer: () => createMockCallbackServer(handlers),
+    });
+
+    await expect(handlers.current.expectedState).resolves.toBeUndefined();
+  });
+
+  it("leaves a listener-reported callback_error unlabeled — it is our failure, not the server's", async () => {
+    const client = mockClient({
+      authenticate: vi.fn(async () => {
+        handlers.current.onError?.({
+          error: "callback_error",
+          error_description: "exchange blew up",
+          source: "callback",
+        });
+        return new URL("https://as.example/authorize");
+      }),
+    });
+
+    const run = runRunnerInteractiveOAuth({
+      client,
+      redirectUrlProvider: { redirectUrl: "" },
+      callbackListen: {
+        hostname: "127.0.0.1",
+        port: 6276,
+        pathname: "/oauth/callback",
+      },
+      createCallbackServer: () => createMockCallbackServer(handlers),
+    });
+    await expect(run).rejects.toThrow("exchange blew up");
+    await expect(run).rejects.not.toThrow("authorization server said");
+  });
+
+  it("labels a redirect error even when it spells its code callback_error", async () => {
+    const client = mockClient({
+      authenticate: vi.fn(async () => {
+        handlers.current.onError?.({
+          error: "callback_error",
+          error_description: "Server-supplied diagnosis",
+          source: "redirect",
+        });
+        return new URL("https://as.example/authorize");
+      }),
+    });
+
+    await expect(
+      runRunnerInteractiveOAuth({
+        client,
+        redirectUrlProvider: { redirectUrl: "" },
+        callbackListen: {
+          hostname: "127.0.0.1",
+          port: 6276,
+          pathname: "/oauth/callback",
+        },
+        createCallbackServer: () => createMockCallbackServer(handlers),
+      }),
+    ).rejects.toThrow(
+      "The authorization server said: Server-supplied diagnosis (callback_error)",
+    );
+  });
+
+  it("reports a bare 'OAuth error' when the listener reports no error text at all", async () => {
+    const client = mockClient({
+      authenticate: vi.fn(async () => {
+        handlers.current.onError?.({ error: "", source: "redirect" });
+        return new URL("https://as.example/authorize");
+      }),
+    });
+
+    const run = runRunnerInteractiveOAuth({
+      client,
+      redirectUrlProvider: { redirectUrl: "" },
+      callbackListen: {
+        hostname: "127.0.0.1",
+        port: 6276,
+        pathname: "/oauth/callback",
+      },
+      createCallbackServer: () => createMockCallbackServer(handlers),
+    });
+    await expect(run).rejects.toThrow(/^OAuth error$/);
+  });
+
+  it("falls back to a generic message for a callback_error without a description", async () => {
+    const client = mockClient({
+      authenticate: vi.fn(async () => {
+        handlers.current.onError?.({
+          error: "callback_error",
+          source: "callback",
+        });
+        return new URL("https://as.example/authorize");
+      }),
+    });
+
+    await expect(
+      runRunnerInteractiveOAuth({
+        client,
+        redirectUrlProvider: { redirectUrl: "" },
+        callbackListen: {
+          hostname: "127.0.0.1",
+          port: 6276,
+          pathname: "/oauth/callback",
+        },
+        createCallbackServer: () => createMockCallbackServer(handlers),
+      }),
+    ).rejects.toThrow("OAuth callback failed");
   });
 
   it("times out when the browser callback never arrives", async () => {
@@ -443,7 +616,10 @@ describe("runRunnerInteractiveOAuth", () => {
     const redirectUrlProvider = { redirectUrl: "" };
     const client = mockClient({
       authenticate: vi.fn(async () => {
-        handlers.current.onError?.({ error: "access_denied" });
+        handlers.current.onError?.({
+          error: "access_denied",
+          source: "redirect",
+        });
         return new URL("https://as.example/authorize");
       }),
     });
@@ -459,7 +635,9 @@ describe("runRunnerInteractiveOAuth", () => {
         },
         createCallbackServer: () => createMockCallbackServer(handlers),
       }),
-    ).rejects.toThrow("access_denied");
+    ).rejects.toThrow(
+      "OAuth authorization failed. The authorization server said: access_denied",
+    );
   });
 
   it("defaults to createOAuthCallbackServer when none is provided", async () => {