Skip to content

Commit 514e618

Browse files
pgondhi987pgondhi987
committed
fix: guard mcp http redirects
1 parent 4e57526 commit 514e618

5 files changed

Lines changed: 283 additions & 170 deletions

File tree

‎src/agents/mcp-http-fetch.test.ts‎

Lines changed: 99 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { afterEach, beforeEach, describe, expect, it } from "vitest";
1+
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
22
import { TEST_UNDICI_RUNTIME_DEPS_KEY } from "../infra/net/undici-runtime.js";
33
import {
44
buildMcpHttpFetch,
@@ -8,17 +8,31 @@ import {
88
} from "./mcp-http-fetch.js";
99

1010
const testGlobal = globalThis as Record<string, unknown>;
11+
const { lookupMock } = vi.hoisted(() => ({
12+
lookupMock: vi.fn(),
13+
}));
14+
15+
vi.mock("node:dns/promises", () => ({
16+
lookup: lookupMock,
17+
}));
1118

1219
class TestAgent {
1320
constructor(readonly options: unknown) {}
1421
}
1522

16-
function TestEnvHttpProxyAgent(): undefined {
17-
return undefined;
23+
class TestEnvHttpProxyAgent {
24+
constructor(readonly options: unknown) {}
1825
}
1926

20-
function TestProxyAgent(): undefined {
21-
return undefined;
27+
class TestProxyAgent {
28+
constructor(readonly options: unknown) {}
29+
}
30+
31+
function redirectResponse(location: string, status = 302): Response {
32+
return new Response(null, {
33+
status,
34+
headers: { location },
35+
});
2236
}
2337

2438
function getDispatcher(init: unknown): unknown {
@@ -28,6 +42,15 @@ function getDispatcher(init: unknown): unknown {
2842
return (init as { dispatcher?: unknown }).dispatcher;
2943
}
3044

45+
function getDispatcherConnectOptions(init: unknown): Record<string, unknown> | undefined {
46+
const dispatcher = getDispatcher(init);
47+
if (!(dispatcher instanceof TestAgent)) {
48+
return undefined;
49+
}
50+
const options = dispatcher.options as { connect?: Record<string, unknown> };
51+
return options.connect;
52+
}
53+
3154
describe("MCP HTTP fetch helpers", () => {
3255
const fetchCalls: Array<{
3356
url: string | URL | Request;
@@ -36,6 +59,16 @@ describe("MCP HTTP fetch helpers", () => {
3659

3760
beforeEach(() => {
3861
fetchCalls.length = 0;
62+
vi.stubEnv("HTTP_PROXY", "");
63+
vi.stubEnv("HTTPS_PROXY", "");
64+
vi.stubEnv("ALL_PROXY", "");
65+
vi.stubEnv("http_proxy", "");
66+
vi.stubEnv("https_proxy", "");
67+
vi.stubEnv("all_proxy", "");
68+
vi.stubEnv("NO_PROXY", "");
69+
vi.stubEnv("no_proxy", "");
70+
lookupMock.mockReset();
71+
lookupMock.mockResolvedValue([{ address: "93.184.216.34", family: 4 }]);
3972
testGlobal[TEST_UNDICI_RUNTIME_DEPS_KEY] = {
4073
Agent: TestAgent,
4174
EnvHttpProxyAgent: TestEnvHttpProxyAgent,
@@ -48,6 +81,7 @@ describe("MCP HTTP fetch helpers", () => {
4881
});
4982

5083
afterEach(() => {
84+
vi.unstubAllEnvs();
5185
delete testGlobal[TEST_UNDICI_RUNTIME_DEPS_KEY];
5286
});
5387

@@ -61,7 +95,66 @@ describe("MCP HTTP fetch helpers", () => {
6195
await fetch("https://auth.example.com/token");
6296

6397
expect(getDispatcher(fetchCalls[0]?.init)).toBeInstanceOf(TestAgent);
64-
expect(getDispatcher(fetchCalls[1]?.init)).toBeUndefined();
98+
expect(getDispatcherConnectOptions(fetchCalls[0]?.init)).toMatchObject({
99+
rejectUnauthorized: false,
100+
});
101+
expect(getDispatcher(fetchCalls[1]?.init)).toBeInstanceOf(TestAgent);
102+
expect(
103+
getDispatcherConnectOptions(fetchCalls[1]?.init)?.["rejectUnauthorized"],
104+
).toBeUndefined();
105+
});
106+
107+
it("uses configured env proxy for ordinary MCP HTTP requests", async () => {
108+
vi.stubEnv("https_proxy", "http://proxy.example:8080");
109+
const fetch = buildMcpHttpFetch({
110+
resourceUrl: "https://mcp.example.com/mcp",
111+
});
112+
113+
await fetch("https://mcp.example.com/token");
114+
115+
expect(getDispatcher(fetchCalls[0]?.init)).toBeInstanceOf(TestEnvHttpProxyAgent);
116+
expect(lookupMock).not.toHaveBeenCalled();
117+
});
118+
119+
it("keeps same-origin TLS overrides ahead of configured env proxy", async () => {
120+
vi.stubEnv("https_proxy", "http://proxy.example:8080");
121+
const fetch = buildMcpHttpFetch({
122+
sslVerify: false,
123+
resourceUrl: "https://mcp.example.com/mcp",
124+
});
125+
126+
await fetch("https://mcp.example.com/token");
127+
await fetch("https://auth.example.com/token");
128+
129+
expect(getDispatcher(fetchCalls[0]?.init)).toBeInstanceOf(TestAgent);
130+
expect(getDispatcherConnectOptions(fetchCalls[0]?.init)).toMatchObject({
131+
rejectUnauthorized: false,
132+
});
133+
expect(getDispatcher(fetchCalls[1]?.init)).toBeInstanceOf(TestEnvHttpProxyAgent);
134+
});
135+
136+
it("uses configured env proxy for redirected targets after a NO_PROXY first hop", async () => {
137+
vi.stubEnv("https_proxy", "http://proxy.example:8080");
138+
vi.stubEnv("no_proxy", "mcp.example.com");
139+
testGlobal[TEST_UNDICI_RUNTIME_DEPS_KEY] = {
140+
Agent: TestAgent,
141+
EnvHttpProxyAgent: TestEnvHttpProxyAgent,
142+
ProxyAgent: TestProxyAgent,
143+
fetch: async (url: string | URL | Request, init?: unknown) => {
144+
fetchCalls.push({ url, init });
145+
return fetchCalls.length === 1
146+
? redirectResponse("https://auth.example.com/token")
147+
: new Response("ok");
148+
},
149+
};
150+
const fetch = buildMcpHttpFetch({
151+
resourceUrl: "https://mcp.example.com/mcp",
152+
});
153+
154+
await fetch("https://mcp.example.com/token");
155+
156+
expect(getDispatcher(fetchCalls[0]?.init)).toBeInstanceOf(TestAgent);
157+
expect(getDispatcher(fetchCalls[1]?.init)).toBeInstanceOf(TestEnvHttpProxyAgent);
65158
});
66159

67160
it("removes static Authorization headers for OAuth-backed runtime requests", () => {

‎src/agents/mcp-http-fetch.ts‎

Lines changed: 125 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
import fs from "node:fs";
22
import type { FetchLike } from "@modelcontextprotocol/sdk/shared/transport.js";
3+
import { fetchWithSsrFGuard } from "../infra/net/fetch-guard.js";
4+
import {
5+
ssrfPolicyFromHttpBaseUrlAllowedOrigin,
6+
type PinnedDispatcherPolicy,
7+
} from "../infra/net/ssrf.js";
38
import { loadUndiciRuntimeDeps } from "../infra/net/undici-runtime.js";
49

510
/** MCP SDK-compatible fetch function type. */
@@ -12,6 +17,98 @@ export const fetchWithUndici: FetchLike = async (url, init) =>
1217
init as Parameters<ReturnType<typeof loadUndiciRuntimeDeps>["fetch"]>[1],
1318
)) as unknown as Response;
1419

20+
const fetchWithUndiciGuard = async (
21+
input: RequestInfo | URL,
22+
init?: RequestInit,
23+
): Promise<Response> => await fetchWithUndici(input instanceof Request ? input.url : input, init);
24+
25+
const MCP_HTTP_MAX_REDIRECTS = 20;
26+
const managedMcpResponseCleanupRegistry = new FinalizationRegistry<{
27+
finalize: () => Promise<void>;
28+
}>((held) => {
29+
void held.finalize();
30+
});
31+
32+
function resolveFetchRequest(input: RequestInfo | URL, init?: RequestInit) {
33+
if (input instanceof Request) {
34+
const request = new Request(input, init);
35+
const body = request.body ?? undefined;
36+
return {
37+
url: request.url,
38+
init: {
39+
method: request.method,
40+
headers: request.headers,
41+
body,
42+
redirect: request.redirect,
43+
signal: request.signal,
44+
...(body ? ({ duplex: "half" } as const) : {}),
45+
} satisfies RequestInit & { duplex?: "half" },
46+
};
47+
}
48+
return {
49+
url: input instanceof URL ? input.toString() : input,
50+
init,
51+
};
52+
}
53+
54+
function buildManagedMcpResponse(
55+
response: Response,
56+
release: () => Promise<void>,
57+
refreshTimeout?: () => void,
58+
): Response {
59+
if (!response.body) {
60+
void release();
61+
return response;
62+
}
63+
64+
const source = response.body;
65+
let reader: ReadableStreamDefaultReader<Uint8Array> | undefined;
66+
let released = false;
67+
const cleanupRegistrationToken = {};
68+
const finalize = async () => {
69+
if (released) {
70+
return;
71+
}
72+
released = true;
73+
managedMcpResponseCleanupRegistry.unregister(cleanupRegistrationToken);
74+
await reader?.cancel().catch(() => undefined);
75+
await release().catch(() => undefined);
76+
};
77+
const wrappedBody = new ReadableStream<Uint8Array>({
78+
start() {
79+
reader = source.getReader();
80+
},
81+
async pull(controller) {
82+
try {
83+
const chunk = await reader?.read();
84+
if (!chunk || chunk.done) {
85+
controller.close();
86+
await finalize();
87+
return;
88+
}
89+
refreshTimeout?.();
90+
controller.enqueue(chunk.value);
91+
} catch (error) {
92+
controller.error(error);
93+
await finalize();
94+
}
95+
},
96+
async cancel(reason) {
97+
try {
98+
await reader?.cancel(reason);
99+
} finally {
100+
await finalize();
101+
}
102+
},
103+
});
104+
managedMcpResponseCleanupRegistry.register(wrappedBody, { finalize }, cleanupRegistrationToken);
105+
return new Response(wrappedBody, {
106+
status: response.status,
107+
statusText: response.statusText,
108+
headers: response.headers,
109+
});
110+
}
111+
15112
/** Builds an MCP fetch function with optional TLS/client-cert dispatcher support. */
16113
export function buildMcpHttpFetch(params: {
17114
sslVerify?: boolean;
@@ -21,32 +118,39 @@ export function buildMcpHttpFetch(params: {
21118
}): FetchLike {
22119
const needsCustomDispatcher =
23120
params.sslVerify === false || Boolean(params.clientCert || params.clientKey);
24-
if (!needsCustomDispatcher) {
25-
return fetchWithUndici;
26-
}
27121
const scopedOrigin = params.resourceUrl ? new URL(params.resourceUrl).origin : undefined;
122+
const policy = params.resourceUrl
123+
? ssrfPolicyFromHttpBaseUrlAllowedOrigin(params.resourceUrl)
124+
: undefined;
28125

29-
const buildDispatcher = () => {
30-
const { Agent } = loadUndiciRuntimeDeps();
31-
return new Agent({
32-
connect: {
33-
...(params.sslVerify === false ? { rejectUnauthorized: false } : {}),
34-
...(params.clientCert ? { cert: fs.readFileSync(params.clientCert, "utf-8") } : {}),
35-
...(params.clientKey ? { key: fs.readFileSync(params.clientKey, "utf-8") } : {}),
36-
},
37-
});
126+
let customConnect: Record<string, unknown> | undefined;
127+
const resolveCustomDispatcherPolicy = (url: URL): PinnedDispatcherPolicy | undefined => {
128+
if (!needsCustomDispatcher || !scopedOrigin || url.origin !== scopedOrigin) {
129+
return undefined;
130+
}
131+
customConnect ??= {
132+
...(params.sslVerify === false ? { rejectUnauthorized: false } : {}),
133+
...(params.clientCert ? { cert: fs.readFileSync(params.clientCert, "utf-8") } : {}),
134+
...(params.clientKey ? { key: fs.readFileSync(params.clientKey, "utf-8") } : {}),
135+
};
136+
return { mode: "direct", connect: customConnect };
38137
};
39138

40-
let dispatcher: unknown;
41139
return async (url, init) => {
42-
if (scopedOrigin && new URL(url).origin !== scopedOrigin) {
43-
return fetchWithUndici(url, init);
44-
}
45-
dispatcher ??= buildDispatcher();
46-
return (await loadUndiciRuntimeDeps().fetch(url, {
47-
...(init as RequestInit),
48-
dispatcher,
49-
} as Parameters<ReturnType<typeof loadUndiciRuntimeDeps>["fetch"]>[1])) as unknown as Response;
140+
const request = resolveFetchRequest(url, init);
141+
const guardedFetchOptions = {
142+
url: request.url,
143+
init: request.init,
144+
fetchImpl: fetchWithUndiciGuard,
145+
maxRedirects: MCP_HTTP_MAX_REDIRECTS,
146+
allowCrossOriginUnsafeRedirectReplay: true,
147+
auditContext: "mcp-http",
148+
useEnvProxyForEligibleUrls: true,
149+
...(policy ? { policy } : {}),
150+
...(needsCustomDispatcher ? { resolveDispatcherPolicy: resolveCustomDispatcherPolicy } : {}),
151+
};
152+
const guarded = await fetchWithSsrFGuard(guardedFetchOptions);
153+
return buildManagedMcpResponse(guarded.response, guarded.release, guarded.refreshTimeout);
50154
};
51155
}
52156

0 commit comments

Comments
 (0)