Skip to content

Commit 3e773a0

Browse files
TheGreatAxiosTheGreatAxios
committed
fix(mcp): pin streamable HTTP redirects to same-origin
Pin redirectPolicy so OAuth discovery and token traffic cannot follow a cross-origin redirect if the SDK default changes.
1 parent 25998b8 commit 3e773a0

3 files changed

Lines changed: 180 additions & 41 deletions

File tree

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

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -188,10 +188,13 @@ describe("HTTP MCP auth policy", () => {
188188
});
189189
expect(callbackStarts).toBe(0);
190190
expect(providerCreates).toBe(0);
191-
// No signal and no provider: leave redirectPolicy unset so the SDK
192-
// default `same-origin` applies. Do not pin `follow`.
191+
// No signal and no provider: same-origin is pinned so the SDK default
192+
// can never widen to `follow`. Do not pin `follow`.
193193
expect(transportOptions).toEqual([
194-
{ reconnectionOptions: MCP_HTTP_RECONNECTION_OPTIONS },
194+
{
195+
redirectPolicy: "same-origin",
196+
reconnectionOptions: MCP_HTTP_RECONNECTION_OPTIONS,
197+
},
195198
]);
196199
});
197200

@@ -268,6 +271,7 @@ describe("HTTP MCP auth policy", () => {
268271
if (!result.ok) return;
269272
expect(transportOptions).toEqual([
270273
{
274+
redirectPolicy: "same-origin",
271275
reconnectionOptions: MCP_HTTP_RECONNECTION_OPTIONS,
272276
authProvider,
273277
requestInit: { signal: expect.any(AbortSignal) },
@@ -305,6 +309,7 @@ describe("HTTP MCP auth policy", () => {
305309
expect(providerServerURL).toBe("https://custom.example/mcp?mode=full");
306310
expect(transportOptions).toEqual([
307311
{
312+
redirectPolicy: "same-origin",
308313
reconnectionOptions: MCP_HTTP_RECONNECTION_OPTIONS,
309314
authProvider,
310315
requestInit: { signal: expect.any(AbortSignal) },

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

Lines changed: 166 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -10,44 +10,65 @@ import { createOAuthProvider } from "./oauth-provider.js";
1010
const SERVER_URL = "https://mcp.example/mcp";
1111
const OFF_ORIGIN = "https://evil.example/collect";
1212

13+
const RESOURCE_METADATA = {
14+
resource: SERVER_URL,
15+
authorization_servers: ["https://mcp.example"],
16+
};
17+
18+
const AUTH_SERVER_METADATA = {
19+
issuer: "https://mcp.example",
20+
authorization_endpoint: "https://mcp.example/authorize",
21+
token_endpoint: "https://mcp.example/oauth/token",
22+
response_types_supported: ["code"],
23+
};
24+
1325
function json(body: unknown): Response {
1426
return new Response(JSON.stringify(body), {
1527
status: 200,
1628
headers: { "content-type": "application/json" },
1729
});
1830
}
1931

32+
function redirectTo(location: string): Response {
33+
return new Response(null, { status: 308, headers: { location } });
34+
}
35+
36+
async function saveTestProvider(home: string) {
37+
const identity = { serverName: "example", serverURL: SERVER_URL };
38+
await saveAuthState(
39+
identity,
40+
{
41+
clientInformation: {
42+
client_id: "client",
43+
redirect_uris: ["http://127.0.0.1:1/callback"],
44+
client_id_issued_at: 1,
45+
token_endpoint_auth_method: "none",
46+
grant_types: ["authorization_code", "refresh_token"],
47+
response_types: ["code"],
48+
client_name: "interchange-code",
49+
},
50+
},
51+
home,
52+
);
53+
const provider = await createOAuthProvider({
54+
serverName: identity.serverName,
55+
serverURL: identity.serverURL,
56+
redirectUrl: "http://127.0.0.1:1/callback",
57+
onAuthURL: () => undefined,
58+
home,
59+
});
60+
await provider.saveCodeVerifier("verifier");
61+
return provider;
62+
}
63+
2064
describe("HTTP MCP redirect policy", () => {
2165
test("discovery and token fetches do not follow a cross-origin redirect", async () => {
2266
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");
67+
const provider = await saveTestProvider(home);
4768

4869
const abort = new AbortController();
4970
const options = streamableHTTPTransportOptions(provider, abort.signal);
50-
expect(options).not.toHaveProperty("redirectPolicy");
71+
expect(options).toMatchObject({ redirectPolicy: "same-origin" });
5172

5273
const requested: string[] = [];
5374
const fetchFn = async (
@@ -65,18 +86,10 @@ describe("HTTP MCP redirect policy", () => {
6586
target.pathname === "/oauth/token";
6687
if (!crossOrigin) {
6788
if (target.pathname === "/.well-known/oauth-protected-resource") {
68-
return json({
69-
resource: SERVER_URL,
70-
authorization_servers: ["https://mcp.example"],
71-
});
89+
return json(RESOURCE_METADATA);
7290
}
7391
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-
});
92+
return json(AUTH_SERVER_METADATA);
8093
}
8194
return new Response(null, { status: 404 });
8295
}
@@ -106,4 +119,122 @@ describe("HTTP MCP redirect policy", () => {
106119
const tokenPost = requested.find((href) => href.includes("/oauth/token"));
107120
expect(tokenPost?.startsWith("https://mcp.example/")).toBe(true);
108121
});
122+
123+
test("same-origin discovery and token redirects are followed (CL-10758)", async () => {
124+
const home = await mkdtemp(join(tmpdir(), "mcp-redirect-"));
125+
const provider = await saveTestProvider(home);
126+
127+
const abort = new AbortController();
128+
const options = streamableHTTPTransportOptions(provider, abort.signal);
129+
expect(options).toMatchObject({ redirectPolicy: "same-origin" });
130+
131+
const requested: string[] = [];
132+
const fetchFn = async (
133+
url: string | URL,
134+
init?: RequestInit,
135+
): Promise<Response> => {
136+
const href = String(url);
137+
requested.push(href);
138+
const target = new URL(href);
139+
// The stub only answers manual redirects: the SDK follows same-origin
140+
// chains itself and never lets fetch follow across origins.
141+
if (init?.redirect !== "manual")
142+
throw new Error(`expected redirect manual, got ${init?.redirect}`);
143+
if (target.hostname !== "mcp.example")
144+
return new Response("collected", { status: 200 });
145+
if (
146+
target.pathname === "/.well-known/oauth-protected-resource/mcp" ||
147+
target.pathname === "/.well-known/oauth-protected-resource"
148+
) {
149+
return redirectTo(
150+
"https://mcp.example/.well-known/oauth-protected-resource/mcp-2",
151+
);
152+
}
153+
if (target.pathname === "/.well-known/oauth-protected-resource/mcp-2") {
154+
return json(RESOURCE_METADATA);
155+
}
156+
if (target.pathname === "/.well-known/oauth-authorization-server") {
157+
return json(AUTH_SERVER_METADATA);
158+
}
159+
if (target.pathname === "/oauth/token") {
160+
return redirectTo("https://mcp.example/oauth/token-final");
161+
}
162+
if (target.pathname === "/oauth/token-final") {
163+
return json({ access_token: "access", token_type: "Bearer" });
164+
}
165+
return new Response(null, { status: 404 });
166+
};
167+
168+
const transport = new StreamableHTTPClientTransport(new URL(SERVER_URL), {
169+
...options,
170+
fetch: fetchFn,
171+
});
172+
// finishAuth resolves (void) once the code exchange completes; the
173+
// redirect targets below prove the same-origin hops were followed.
174+
await expect(transport.finishAuth("auth-code")).resolves.toBeUndefined();
175+
176+
expect(requested.some((href) => href.includes("evil.example"))).toBe(false);
177+
expect(
178+
requested.some((href) =>
179+
href.includes("/.well-known/oauth-protected-resource/mcp-2"),
180+
),
181+
).toBe(true);
182+
expect(requested.some((href) => href.includes("/oauth/token-final"))).toBe(
183+
true,
184+
);
185+
});
186+
187+
test("a same-origin token redirect chain past the SDK cap fails bounded (CL-10758)", async () => {
188+
const home = await mkdtemp(join(tmpdir(), "mcp-redirect-"));
189+
const provider = await saveTestProvider(home);
190+
191+
const abort = new AbortController();
192+
const options = streamableHTTPTransportOptions(provider, abort.signal);
193+
194+
const requested: string[] = [];
195+
const fetchFn = async (
196+
url: string | URL,
197+
init?: RequestInit,
198+
): Promise<Response> => {
199+
const href = String(url);
200+
requested.push(href);
201+
const target = new URL(href);
202+
if (init?.redirect !== "manual")
203+
throw new Error(`expected redirect manual, got ${init?.redirect}`);
204+
if (
205+
target.pathname === "/.well-known/oauth-protected-resource/mcp" ||
206+
target.pathname === "/.well-known/oauth-protected-resource"
207+
) {
208+
return json(RESOURCE_METADATA);
209+
}
210+
if (target.pathname === "/.well-known/oauth-authorization-server") {
211+
return json(AUTH_SERVER_METADATA);
212+
}
213+
const hop = /^\/oauth\/token(?:-(\d+))?$/.exec(target.pathname);
214+
if (hop !== null) {
215+
const depth = hop[1] === undefined ? 0 : Number(hop[1]);
216+
// Eight same-origin hops with a 200 past the end: the SDK must stop
217+
// following before it gets there.
218+
if (depth < 8)
219+
return redirectTo(`https://mcp.example/oauth/token-${depth + 1}`);
220+
return json({ access_token: "access", token_type: "Bearer" });
221+
}
222+
return new Response(null, { status: 404 });
223+
};
224+
225+
const transport = new StreamableHTTPClientTransport(new URL(SERVER_URL), {
226+
...options,
227+
fetch: fetchFn,
228+
});
229+
await expect(transport.finishAuth("auth-code")).rejects.toThrow();
230+
231+
expect(requested.some((href) => href.includes("evil.example"))).toBe(false);
232+
const tokenHits = requested.filter((href) => href.includes("/oauth/token"));
233+
// SDK MAX_REDIRECTS (5) plus the initial POST: the 200 at depth 8 is
234+
// never reached, and the chain cannot loop forever.
235+
expect(tokenHits).toHaveLength(6);
236+
expect(
237+
tokenHits.every((href) => href.startsWith("https://mcp.example/")),
238+
).toBe(true);
239+
});
109240
});

‎src/mcp/client.ts‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -410,10 +410,13 @@ export function streamableHTTPTransportOptions(
410410
authProvider: CorbitsOAuthProvider | undefined,
411411
signal: AbortSignal | undefined,
412412
) {
413-
// Leave redirectPolicy unset. SDK 1.32 defaults to `same-origin`; `follow`
414-
// also wraps OAuth discovery and token requests, so a 307/308 can post
415-
// refresh tokens or auth codes to another host.
413+
// Pin same-origin explicitly rather than relying on the SDK default:
414+
// `follow` also wraps OAuth discovery and token requests, so a 307/308
415+
// could post refresh tokens or auth codes to another host. Aligned with
416+
// the ^1.32.1 SDK range in package.json, where `same-origin` follows
417+
// within-origin chains (capped) for discovery, token, and SSE traffic.
416418
return {
419+
redirectPolicy: "same-origin" as const,
417420
reconnectionOptions: MCP_HTTP_RECONNECTION_OPTIONS,
418421
...(authProvider === undefined ? {} : { authProvider }),
419422
...(signal === undefined

0 commit comments

Comments
 (0)