diff --git a/bun.lock b/bun.lock index 77562cdcf..56a1902fe 100644 --- a/bun.lock +++ b/bun.lock @@ -29,7 +29,7 @@ "@intx/tools-posix": "workspace:*", "@intx/types": "workspace:*", "@intx/workflow-host": "0.3.0", - "@modelcontextprotocol/sdk": "^1.29.0", + "@modelcontextprotocol/sdk": "^1.32.1", "@opentui/core": "0.6.4", "arktype": "catalog:", "isomorphic-git": "catalog:", @@ -392,7 +392,7 @@ "@logtape/logtape": ["@logtape/logtape@2.3.3", "", {}, "sha512-H8AqgCq6M6Wu97FVPyfE5UT4w6H+Wdt9iGzLKPgtJRI/jrG9HbzzNUw9JL1LarkZD8xeKx9FqKfcLaXr0hGJuw=="], - "@modelcontextprotocol/sdk": ["@modelcontextprotocol/sdk@1.30.0", "", { "dependencies": { "@hono/node-server": "^1.19.9 || ^2.0.5", "ajv": "^8.17.1", "ajv-formats": "^3.0.1", "content-type": "^1.0.5", "cors": "^2.8.5", "cross-spawn": "^7.0.5", "eventsource": "^3.0.2", "eventsource-parser": "^3.0.0", "express": "^5.2.1", "express-rate-limit": "^8.2.1", "hono": "^4.11.4", "jose": "^6.1.3", "json-schema-typed": "^8.0.2", "pkce-challenge": "^5.0.0", "raw-body": "^3.0.0", "zod": "^3.25 || ^4.0", "zod-to-json-schema": "^3.25.1" }, "peerDependencies": { "@cfworker/json-schema": "^4.1.1" }, "optionalPeers": ["@cfworker/json-schema"] }, "sha512-xKd8OIzlqNzcqcNumGAa6g+PW2kjD5vrpcKOnfldAUPP3j7lnqMPwlTXQm8gF+UwH72z0lqaRbjr9hqGz0eITA=="], + "@modelcontextprotocol/sdk": ["@modelcontextprotocol/sdk@1.32.1", "", { "dependencies": { "@hono/node-server": "^1.19.9 || ^2.0.5", "ajv": "^8.17.1", "ajv-formats": "^3.0.1", "content-type": "^1.0.5", "cors": "^2.8.5", "cross-spawn": "^7.0.5", "eventsource": "^3.0.2", "eventsource-parser": "^3.0.0", "express": "^5.2.1", "express-rate-limit": "^8.2.1", "hono": "^4.11.4", "jose": "^6.1.3", "json-schema-typed": "^8.0.2", "pkce-challenge": "^5.0.0", "raw-body": "^3.0.0", "zod": "^3.25 || ^4.0", "zod-to-json-schema": "^3.25.1" }, "peerDependencies": { "@cfworker/json-schema": "^4.1.1" }, "optionalPeers": ["@cfworker/json-schema"] }, "sha512-2DdE+SJDtzLEEWzY1ZjY7Q+VcPhcV1KisD3zI4u0XZyktsjHum1mwbMI+JaulUBi2OZk+KJAi2uPXzxichPkdw=="], "@napi-rs/wasm-runtime": ["@napi-rs/wasm-runtime@1.2.5", "", { "dependencies": { "@tybys/wasm-util": "^0.10.3" }, "peerDependencies": { "@emnapi/core": "^1.7.1 || ^2.0.0-alpha.4", "@emnapi/runtime": "^1.7.1 || ^2.0.0-alpha.4" } }, "sha512-HshSvXzmBxNzqejd3k/KemgQ3hdREUYFRRrgxOZ+gPljDIsO2SDZrVZMd4Z/HsxudQbE62OVyVFE8z9BCurfjA=="], @@ -1062,8 +1062,6 @@ "@intx/workflow-deploy/@intx/workflow": ["@intx/workflow@0.4.0", "", { "dependencies": { "@intx/agent": "0.4.0", "@intx/inference": "0.4.0", "@intx/types": "0.4.0", "arktype": "^2.1.29" } }, "sha512-F5Is/lyKb4/WguAVYMDXwtI4ddUvqXbg34rcM6hPUOqupJHpbbD9+ZK/6TDfvbCiEgginbTVwMmNj6Fyg8vU+Q=="], - "@modelcontextprotocol/sdk/zod": ["zod@4.5.4", "", {}, "sha512-sC95tT5iHHH9gtpj6A81kh+NEaRAUFN+qlUPDUbRfOMvNf5QCBqsb3WgvnpVtK5Y+4UfA6KqufotuTvMGiTlsA=="], - "@npmcli/agent/lru-cache": ["lru-cache@11.5.2", "", {}, "sha512-4pfM1Ff0x50o0tQwb5ucw/RzNyD0/YJME6IVcStalZuMWxdt3sR3huStTtxz4PUmvZfRguvDejasvQ2kifR11g=="], "better-auth/zod": ["zod@4.5.4", "", {}, "sha512-sC95tT5iHHH9gtpj6A81kh+NEaRAUFN+qlUPDUbRfOMvNf5QCBqsb3WgvnpVtK5Y+4UfA6KqufotuTvMGiTlsA=="], diff --git a/package.json b/package.json index 713c411d7..f6e8313ee 100644 --- a/package.json +++ b/package.json @@ -105,7 +105,7 @@ "@intx/tools-posix": "workspace:*", "@intx/types": "workspace:*", "@intx/workflow-host": "0.3.0", - "@modelcontextprotocol/sdk": "^1.29.0", + "@modelcontextprotocol/sdk": "^1.32.1", "@opentui/core": "0.6.4", "arktype": "catalog:", "isomorphic-git": "catalog:", diff --git a/src/mcp/auth-store.test.ts b/src/mcp/auth-store.test.ts index 27141a516..1ca8817bf 100644 --- a/src/mcp/auth-store.test.ts +++ b/src/mcp/auth-store.test.ts @@ -208,6 +208,52 @@ describe("mcp auth-store", () => { ); }); + test("persists the SDK's nested issuer stamps unchanged", async () => { + const home = await tempHome(); + await saveAuthState( + linear, + { + clientInformation: { + client_id: "c1", + redirect_uris: ["http://127.0.0.1:1/callback"], + issuer: "https://as.example", + }, + tokens: { + access_token: "tok", + token_type: "bearer", + issuer: "https://as.example", + }, + }, + home, + ); + const final = await loadAuthState(linear, home); + expect(final.tokens?.access_token).toBe("tok"); + expect(final.tokens?.issuer).toBe("https://as.example"); + expect(final.clientInformation?.client_id).toBe("c1"); + expect(final.clientInformation?.issuer).toBe("https://as.example"); + }); + + test("loads legacy credentials saved before issuer stamps", async () => { + const home = await tempHome(); + // Pre-1.31 shape: no binding yet; the SDK consumes these as-is and + // re-stamps on first successful use, so loading must not require the key + // or invent one. + await saveAuthState( + linear, + { + tokens: { + access_token: "tok", + token_type: "bearer", + refresh_token: "r", + }, + }, + home, + ); + const final = await loadAuthState(linear, home); + expect(final.tokens?.access_token).toBe("tok"); + expect(final.tokens?.issuer).toBeUndefined(); + }); + test("ignores legacy name-only auth state without modifying it", async () => { const home = await tempHome(); const dir = join(home, ".corbits", "mcp-auth"); diff --git a/src/mcp/auth-store.ts b/src/mcp/auth-store.ts index 16f9982c6..9aa56276b 100644 --- a/src/mcp/auth-store.ts +++ b/src/mcp/auth-store.ts @@ -25,6 +25,16 @@ const logger = getLogger([LOG_NAMESPACE_ROOT, "mcp", "auth-store"]); // Per-server OAuth state. The durable pair — registered client info and // tokens — reaches disk; the PKCE verifier stays instance-local in the // provider. Legacy codeVerifier keys are stripped on write. +// +// Issuer binding (SDK >= 1.31): tokens and client information carry a nested +// `issuer`, the authorization server they belong to; both are persisted +// unchanged in that nested shape, not as separate fields. Stores saved before +// the stamp have no `issuer` key and still load — `issuer` is optional on +// both — and the SDK consumes unstamped credentials as-is, re-stamping them +// on the next successful auth or refresh. Stamping at read time is +// deliberately not done: the persisted epoch predates binding, so the +// authorization server cannot be reconstructed without discovery and a wrong +// guess would make the SDK discard valid credentials. export interface MCPAuthState { clientInformation?: OAuthClientInformationFull; tokens?: OAuthTokens; diff --git a/src/mcp/client-auth-policy.test.ts b/src/mcp/client-auth-policy.test.ts index f3303ddd3..0d71fc5ce 100644 --- a/src/mcp/client-auth-policy.test.ts +++ b/src/mcp/client-auth-policy.test.ts @@ -184,7 +184,9 @@ describe("HTTP MCP auth policy", () => { }); expect(callbackStarts).toBe(0); expect(providerCreates).toBe(0); - expect(transportOptions).toEqual([undefined]); + // No signal and no provider: leave redirectPolicy unset so the SDK + // default `same-origin` applies. Do not pin `follow`. + expect(transportOptions).toEqual([{}]); }); test("closes a bound OAuth callback server when provider setup fails", async () => { diff --git a/src/mcp/client-redirect-policy.test.ts b/src/mcp/client-redirect-policy.test.ts new file mode 100644 index 000000000..540e606f1 --- /dev/null +++ b/src/mcp/client-redirect-policy.test.ts @@ -0,0 +1,109 @@ +import { mkdtemp } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { StreamableHTTPClientTransport } from "@modelcontextprotocol/sdk/client/streamableHttp.js"; +import { describe, expect, test } from "bun:test"; +import { saveAuthState } from "./auth-store.js"; +import { streamableHTTPTransportOptions } from "./client.js"; +import { createOAuthProvider } from "./oauth-provider.js"; + +const SERVER_URL = "https://mcp.example/mcp"; +const OFF_ORIGIN = "https://evil.example/collect"; + +function json(body: unknown): Response { + return new Response(JSON.stringify(body), { + status: 200, + headers: { "content-type": "application/json" }, + }); +} + +describe("HTTP MCP redirect policy", () => { + test("discovery and token fetches do not follow a cross-origin redirect", async () => { + const home = await mkdtemp(join(tmpdir(), "mcp-redirect-")); + const identity = { serverName: "example", serverURL: SERVER_URL }; + await saveAuthState( + identity, + { + clientInformation: { + client_id: "client", + redirect_uris: ["http://127.0.0.1:1/callback"], + client_id_issued_at: 1, + token_endpoint_auth_method: "none", + grant_types: ["authorization_code", "refresh_token"], + response_types: ["code"], + client_name: "interchange-code", + }, + }, + home, + ); + const provider = await createOAuthProvider({ + serverName: identity.serverName, + serverURL: identity.serverURL, + redirectUrl: "http://127.0.0.1:1/callback", + onAuthURL: () => undefined, + home, + }); + await provider.saveCodeVerifier("verifier"); + + const abort = new AbortController(); + const options = streamableHTTPTransportOptions(provider, abort.signal); + expect(options).not.toHaveProperty("redirectPolicy"); + + const requested: string[] = []; + const fetchFn = async ( + url: string | URL, + init?: RequestInit, + ): Promise => { + const href = String(url); + requested.push(href); + const target = new URL(href); + if (target.hostname === "evil.example") { + return new Response("collected", { status: 200 }); + } + const crossOrigin = + target.pathname === "/.well-known/oauth-protected-resource/mcp" || + target.pathname === "/oauth/token"; + if (!crossOrigin) { + if (target.pathname === "/.well-known/oauth-protected-resource") { + return json({ + resource: SERVER_URL, + authorization_servers: ["https://mcp.example"], + }); + } + if (target.pathname === "/.well-known/oauth-authorization-server") { + return json({ + issuer: "https://mcp.example", + authorization_endpoint: "https://mcp.example/authorize", + token_endpoint: "https://mcp.example/oauth/token", + response_types_supported: ["code"], + }); + } + return new Response(null, { status: 404 }); + } + const redirect = new Response(null, { + status: 308, + headers: { location: OFF_ORIGIN }, + }); + const mode = init?.redirect ?? "follow"; + if (mode === "error") throw new TypeError("redirect mode is error"); + if (mode === "manual") return redirect; + return fetchFn(OFF_ORIGIN, init); + }; + + const transport = new StreamableHTTPClientTransport(new URL(SERVER_URL), { + ...options, + fetch: fetchFn, + }); + await expect(transport.finishAuth("auth-code")).rejects.toThrow(); + + expect(requested.some((href) => href.includes("evil.example"))).toBe(false); + expect( + requested.some((href) => + href.includes("/.well-known/oauth-protected-resource/mcp"), + ), + ).toBe(true); + expect(requested.some((href) => href.includes("/oauth/token"))).toBe(true); + const tokenPost = requested.find((href) => href.includes("/oauth/token")); + expect(tokenPost?.startsWith("https://mcp.example/")).toBe(true); + }); +}); diff --git a/src/mcp/client.ts b/src/mcp/client.ts index d7f5098e5..254c49e19 100644 --- a/src/mcp/client.ts +++ b/src/mcp/client.ts @@ -405,11 +405,13 @@ export function fetchWithConnectAbort( }; } -function streamableHTTPTransportOptions( +export function streamableHTTPTransportOptions( authProvider: CorbitsOAuthProvider | undefined, signal: AbortSignal | undefined, ) { - if (authProvider === undefined && signal === undefined) return undefined; + // Leave redirectPolicy unset. SDK 1.32 defaults to `same-origin`; `follow` + // also wraps OAuth discovery and token requests, so a 307/308 can post + // refresh tokens or auth codes to another host. return { ...(authProvider === undefined ? {} : { authProvider }), ...(signal === undefined diff --git a/src/mcp/oauth-provider.test.ts b/src/mcp/oauth-provider.test.ts index acc40e98f..362b303df 100644 --- a/src/mcp/oauth-provider.test.ts +++ b/src/mcp/oauth-provider.test.ts @@ -767,7 +767,7 @@ describe("createOAuthProvider", () => { ).toBe("scoped-secret"); }); - test("refreshToken posts grant_type=refresh_token with a resource and persists tokens", async () => { + test("refreshToken posts grant_type=refresh_token with a resource and persists issuer-bound tokens", async () => { const home = await tempHome(); await saveAuthState( linear, @@ -807,6 +807,7 @@ describe("createOAuthProvider", () => { const tokens = await provider.refreshToken("refresh-me"); expect(tokens.access_token).toBe("fresh-access"); + expect(tokens.issuer).toBe("https://mcp.linear.app"); expect(tokenBodies).toHaveLength(1); const body = tokenBodies[0] ?? ""; expect(body).toContain("grant_type=refresh_token"); @@ -820,6 +821,9 @@ describe("createOAuthProvider", () => { expect((await loadAuthState(linear, home)).tokens?.access_token).toBe( "fresh-access", ); + expect((await loadAuthState(linear, home)).tokens?.issuer).toBe( + "https://mcp.linear.app", + ); }); test("refreshToken wraps failures as UnauthorizedError", async () => { diff --git a/src/mcp/oauth-provider.ts b/src/mcp/oauth-provider.ts index e2733d6db..91a7332f0 100644 --- a/src/mcp/oauth-provider.ts +++ b/src/mcp/oauth-provider.ts @@ -312,10 +312,19 @@ export async function createOAuthProvider( resource: resourceURL, ...(fetchFn === undefined ? {} : { fetchFn }), }); + // SDK >= 1.31 binds stored credentials to their authorization server + // via an `issuer` stamp; the transport's auth() stamps its own saves, + // so refresh here must too — unstamped tokens would re-trigger the + // SDK's stored-tokens-without-issuer deprecation warning on the next + // auth() and stay unbound. + const boundTokens: OAuthTokens = { + ...tokens, + issuer: authorizationServerUrl, + }; await apply((state) => { - state.tokens = tokens; + state.tokens = boundTokens; }); - return tokens; + return boundTokens; } catch (err) { if (isAbortError(err) || err instanceof UnauthorizedError) throw err; throw new UnauthorizedError(