Skip to content

Commit f1bb9ff

Browse files
committed
fix(mcp): leave SDK redirect policy at same-origin
`redirectPolicy: "follow"` made SDK 1.32.1 follow 307/308 on OAuth discovery and token requests, so refresh tokens and auth codes could be posted off-origin. Omit the override and keep the SDK default.
1 parent 3ee164e commit f1bb9ff

3 files changed

Lines changed: 116 additions & 12 deletions

File tree

‎src/mcp/client-auth-policy.test.ts‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -184,9 +184,9 @@ describe("HTTP MCP auth policy", () => {
184184
});
185185
expect(callbackStarts).toBe(0);
186186
expect(providerCreates).toBe(0);
187-
// No signal and no provider: still pins the operator-configured-host
188-
// redirect policy rather than the SDK 1.32 same-origin default.
189-
expect(transportOptions).toEqual([{ redirectPolicy: "follow" }]);
187+
// No signal and no provider: leave redirectPolicy unset so the SDK
188+
// default `same-origin` applies. Do not pin `follow`.
189+
expect(transportOptions).toEqual([{}]);
190190
});
191191

192192
test("closes a bound OAuth callback server when provider setup fails", async () => {
@@ -265,7 +265,6 @@ describe("HTTP MCP auth policy", () => {
265265
authProvider,
266266
requestInit: { signal: expect.any(AbortSignal) },
267267
fetch: expect.any(Function),
268-
redirectPolicy: "follow",
269268
},
270269
]);
271270

@@ -302,7 +301,6 @@ describe("HTTP MCP auth policy", () => {
302301
authProvider,
303302
requestInit: { signal: expect.any(AbortSignal) },
304303
fetch: expect.any(Function),
305-
redirectPolicy: "follow",
306304
},
307305
]);
308306
});
Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
import { mkdtemp } from "node:fs/promises";
2+
import { tmpdir } from "node:os";
3+
import { join } from "node:path";
4+
import { StreamableHTTPClientTransport } from "@modelcontextprotocol/sdk/client/streamableHttp.js";
5+
import { describe, expect, test } from "bun:test";
6+
import { saveAuthState } from "./auth-store.js";
7+
import { streamableHTTPTransportOptions } from "./client.js";
8+
import { createOAuthProvider } from "./oauth-provider.js";
9+
10+
const SERVER_URL = "https://mcp.example/mcp";
11+
const OFF_ORIGIN = "https://evil.example/collect";
12+
13+
function json(body: unknown): Response {
14+
return new Response(JSON.stringify(body), {
15+
status: 200,
16+
headers: { "content-type": "application/json" },
17+
});
18+
}
19+
20+
describe("HTTP MCP redirect policy", () => {
21+
test("discovery and token fetches do not follow a cross-origin redirect", async () => {
22+
const home = await mkdtemp(join(tmpdir(), "mcp-redirect-"));
23+
const identity = { serverName: "example", serverURL: SERVER_URL };
24+
await saveAuthState(
25+
identity,
26+
{
27+
clientInformation: {
28+
client_id: "client",
29+
redirect_uris: ["http://127.0.0.1:1/callback"],
30+
client_id_issued_at: 1,
31+
token_endpoint_auth_method: "none",
32+
grant_types: ["authorization_code", "refresh_token"],
33+
response_types: ["code"],
34+
client_name: "interchange-code",
35+
},
36+
},
37+
home,
38+
);
39+
const provider = await createOAuthProvider({
40+
serverName: identity.serverName,
41+
serverURL: identity.serverURL,
42+
redirectUrl: "http://127.0.0.1:1/callback",
43+
onAuthURL: () => undefined,
44+
home,
45+
});
46+
await provider.saveCodeVerifier("verifier");
47+
48+
const abort = new AbortController();
49+
const options = streamableHTTPTransportOptions(provider, abort.signal);
50+
expect(options).not.toHaveProperty("redirectPolicy");
51+
52+
const requested: string[] = [];
53+
const fetchFn = async (
54+
url: string | URL,
55+
init?: RequestInit,
56+
): Promise<Response> => {
57+
const href = String(url);
58+
requested.push(href);
59+
const target = new URL(href);
60+
if (target.hostname === "evil.example") {
61+
return new Response("collected", { status: 200 });
62+
}
63+
const crossOrigin =
64+
target.pathname === "/.well-known/oauth-protected-resource/mcp" ||
65+
target.pathname === "/oauth/token";
66+
if (!crossOrigin) {
67+
if (target.pathname === "/.well-known/oauth-protected-resource") {
68+
return json({
69+
resource: SERVER_URL,
70+
authorization_servers: ["https://mcp.example"],
71+
});
72+
}
73+
if (target.pathname === "/.well-known/oauth-authorization-server") {
74+
return json({
75+
issuer: "https://mcp.example",
76+
authorization_endpoint: "https://mcp.example/authorize",
77+
token_endpoint: "https://mcp.example/oauth/token",
78+
response_types_supported: ["code"],
79+
});
80+
}
81+
return new Response(null, { status: 404 });
82+
}
83+
const redirect = new Response(null, {
84+
status: 308,
85+
headers: { location: OFF_ORIGIN },
86+
});
87+
const mode = init?.redirect ?? "follow";
88+
if (mode === "error") throw new TypeError("redirect mode is error");
89+
if (mode === "manual") return redirect;
90+
return fetchFn(OFF_ORIGIN, init);
91+
};
92+
93+
const transport = new StreamableHTTPClientTransport(new URL(SERVER_URL), {
94+
...options,
95+
fetch: fetchFn,
96+
});
97+
await expect(transport.finishAuth("auth-code")).rejects.toThrow();
98+
99+
expect(requested.some((href) => href.includes("evil.example"))).toBe(false);
100+
expect(
101+
requested.some((href) =>
102+
href.includes("/.well-known/oauth-protected-resource/mcp"),
103+
),
104+
).toBe(true);
105+
expect(requested.some((href) => href.includes("/oauth/token"))).toBe(true);
106+
const tokenPost = requested.find((href) => href.includes("/oauth/token"));
107+
expect(tokenPost?.startsWith("https://mcp.example/")).toBe(true);
108+
});
109+
});

‎src/mcp/client.ts‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -405,21 +405,18 @@ export function fetchWithConnectAbort(
405405
};
406406
}
407407

408-
function streamableHTTPTransportOptions(
408+
export function streamableHTTPTransportOptions(
409409
authProvider: CorbitsOAuthProvider | undefined,
410410
signal: AbortSignal | undefined,
411411
) {
412-
// SDK 1.32 flips the client transports' default redirect handling to
413-
// same-origin-only. MCP endpoint URLs are operator-configured (user
414-
// settings, or trust-on-first-use local servers), so trust redirects from
415-
// the chosen origin: keep the pre-1.32 behavior of letting fetch follow
416-
// them rather than failing on a legit cross-origin hop.
412+
// Leave redirectPolicy unset. SDK 1.32 defaults to `same-origin`; `follow`
413+
// also wraps OAuth discovery and token requests, so a 307/308 can post
414+
// refresh tokens or auth codes to another host.
417415
return {
418416
...(authProvider === undefined ? {} : { authProvider }),
419417
...(signal === undefined
420418
? {}
421419
: { requestInit: { signal }, fetch: fetchWithConnectAbort(signal) }),
422-
redirectPolicy: "follow" as const,
423420
};
424421
}
425422

0 commit comments

Comments
 (0)