Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions backend/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions backend/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
"multer": "^1.4.5-lts.2",
"pdfjs-dist": "^4.10.38",
"resend": "^4.5.1",
"undici": "^6.27.0",
"xlsx": "https://cdn.sheetjs.com/xlsx-0.20.3/xlsx-0.20.3.tgz",
"zod": "^3.25.76"
},
Expand Down
128 changes: 128 additions & 0 deletions backend/src/lib/mcp/__tests__/client.ssrf.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { Agent } from "undici";

// Mock DNS resolution so the SSRF guard is exercised deterministically without
// touching the network. `lookupMock` is hoisted so the vi.mock factory can
// reference it.
const { lookupMock } = vi.hoisted(() => ({ lookupMock: vi.fn() }));
vi.mock("dns/promises", () => ({
default: { lookup: lookupMock },
}));

import { guardedFetch, validateRemoteMcpUrl } from "../client";

function resolvesTo(...addresses: string[]) {
lookupMock.mockResolvedValue(
addresses.map((address) => ({
address,
family: address.includes(":") ? 6 : 4,
})),
);
}

beforeEach(() => {
lookupMock.mockReset();
});

afterEach(() => {
vi.restoreAllMocks();
});

describe("validateRemoteMcpUrl", () => {
it("rejects non-HTTPS URLs", async () => {
await expect(validateRemoteMcpUrl("http://example.com/")).rejects.toThrow(
/HTTPS/,
);
});

it("rejects invalid URLs", async () => {
await expect(validateRemoteMcpUrl("not a url")).rejects.toThrow(
/valid URL/,
);
});

it("rejects localhost and metadata hosts without a DNS lookup", async () => {
for (const host of [
"https://localhost/",
"https://foo.localhost/",
"https://metadata.google.internal/",
"https://instance-data/",
]) {
await expect(validateRemoteMcpUrl(host), host).rejects.toThrow(
/blocked host/,
);
}
expect(lookupMock).not.toHaveBeenCalled();
});

it("rejects private IPv4/IPv6 literals without a DNS lookup", async () => {
for (const host of [
"https://127.0.0.1/",
"https://10.0.0.1/",
"https://169.254.169.254/",
"https://[::1]/",
"https://[fd00::1]/",
]) {
await expect(validateRemoteMcpUrl(host), host).rejects.toThrow(
/blocked network address/,
);
}
expect(lookupMock).not.toHaveBeenCalled();
});

it("rejects a hostname that resolves to a private address", async () => {
resolvesTo("10.0.0.5");
await expect(
validateRemoteMcpUrl("https://rebind.example.com/"),
).rejects.toThrow(/blocked network address/);
});

it("rejects when ANY resolved address is private (mixed record set)", async () => {
resolvesTo("93.184.216.34", "192.168.1.1");
await expect(
validateRemoteMcpUrl("https://mixed.example.com/"),
).rejects.toThrow(/blocked network address/);
});

it("accepts a public host and strips credentials/hash", async () => {
resolvesTo("93.184.216.34");
const out = await validateRemoteMcpUrl(
"https://user:secret@public.example.com/path?q=1#frag",
);
expect(out).toBe("https://public.example.com/path?q=1");
expect(out).not.toContain("secret");
expect(out).not.toContain("frag");
});
});

describe("guardedFetch", () => {
it("throws and never calls fetch when the URL fails validation", async () => {
resolvesTo("10.0.0.5");
const fetchSpy = vi.spyOn(globalThis, "fetch");
await expect(
guardedFetch("https://rebind.example.com/"),
).rejects.toThrow(/blocked network address/);
expect(fetchSpy).not.toHaveBeenCalled();
});

it("pins the connection (undici dispatcher) and disables redirects for public hosts", async () => {
resolvesTo("93.184.216.34");
const fetchSpy = vi
.spyOn(globalThis, "fetch")
.mockResolvedValue(new Response("ok", { status: 200 }));

const res = await guardedFetch("https://public.example.com/x", {
method: "GET",
});
expect(res.status).toBe(200);
expect(fetchSpy).toHaveBeenCalledTimes(1);

const init = fetchSpy.mock.calls[0][1] as RequestInit & {
dispatcher?: unknown;
};
expect(init.redirect).toBe("manual");
expect(init.dispatcher).toBeInstanceOf(Agent);
// Original request options are preserved.
expect(init.method).toBe("GET");
});
});
101 changes: 63 additions & 38 deletions backend/src/lib/mcp/client.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import crypto from "crypto";
import dns from "dns/promises";
import net from "net";
import { Agent } from "undici";
import { isBlockedIp } from "../privateIp";
import {
BLOCKED_METADATA_HOSTS,
HEADER_NAME_RE,
Expand Down Expand Up @@ -223,41 +225,8 @@ export function toConnectorSummary(
};
}

function isPrivateIpv4(ip: string) {
const parts = ip.split(".").map((part) => Number.parseInt(part, 10));
if (parts.length !== 4 || parts.some((part) => !Number.isFinite(part))) {
return true;
}
const [a, b] = parts;
return (
a === 0 ||
a === 10 ||
a === 127 ||
(a === 100 && b >= 64 && b <= 127) ||
(a === 169 && b === 254) ||
(a === 172 && b >= 16 && b <= 31) ||
(a === 192 && b === 168) ||
(a === 192 && b === 0) ||
(a === 198 && (b === 18 || b === 19)) ||
a >= 224
);
}

function isPrivateIpv6(ip: string) {
const normalized = ip.toLowerCase();
if (normalized === "::1" || normalized === "::") return true;
if (normalized.startsWith("fc") || normalized.startsWith("fd")) return true;
if (/^fe[89ab]:/.test(normalized)) return true;
const ipv4Tail = normalized.match(/::ffff:(\d+\.\d+\.\d+\.\d+)$/);
return ipv4Tail ? isPrivateIpv4(ipv4Tail[1]) : false;
}

function isBlockedIp(ip: string) {
const family = net.isIP(ip);
if (family === 4) return isPrivateIpv4(ip);
if (family === 6) return isPrivateIpv6(ip);
return true;
}
// Private/reserved IP classification lives in lib/privateIp.ts so every
// guarded egress check reuses the exact same ranges.

export async function validateRemoteMcpUrl(rawUrl: string): Promise<string> {
let url: URL;
Expand All @@ -282,9 +251,17 @@ export async function validateRemoteMcpUrl(rawUrl: string): Promise<string> {
throw new Error("MCP server URL points to a blocked host.");
}

const literalFamily = net.isIP(hostname);
// URL.hostname wraps IPv6 literals in brackets ("[::1]"), which net.isIP
// does not recognize. Strip them so an IPv6 literal is classified by the
// private-IP guard rather than falling through to a DNS lookup that would
// treat the bracketed form as an (unresolvable) hostname.
const literalHost =
hostname.startsWith("[") && hostname.endsWith("]")
? hostname.slice(1, -1)
: hostname;
const literalFamily = net.isIP(literalHost);
const addresses = literalFamily
? [{ address: hostname }]
? [{ address: literalHost }]
: await dns.lookup(hostname, { all: true, verbatim: true });
if (!addresses.length || addresses.some(({ address }) => isBlockedIp(address))) {
throw new Error("MCP server URL resolves to a blocked network address.");
Expand Down Expand Up @@ -352,6 +329,50 @@ export function authConfigPatch(config: McpConnectorAuthConfig): Record<string,
});
}

// A per-request undici dispatcher whose DNS lookup runs the private-IP guard at
// the moment the socket is opened and returns ONLY validated addresses. Because
// undici connects to exactly what this lookup yields, the address we validate is
// the address we connect to — there is no second, unguarded resolution for an
// attacker to race (DNS-rebinding / TOCTOU). The URL hostname is untouched, so
// the Host header and TLS SNI still reflect the real host and HTTPS verifies
// normally against legitimate public servers.
function pinnedGuardAgent(): Agent {
return new Agent({
connect: {
lookup: (hostname, _options, callback) => {
dns.lookup(hostname, { all: true, verbatim: true })
.then((addresses) => {
if (
!addresses.length ||
addresses.some(({ address }) => isBlockedIp(address))
) {
callback(
new Error(
"MCP server URL resolves to a blocked network address.",
),
[],
);
return;
}
callback(null, addresses);
})
.catch((err: unknown) =>
callback(
err instanceof Error ? err : new Error(String(err)),
[],
),
);
},
},
});
}

// The single guarded egress helper for every outbound MCP request (connector
// transport, OAuth discovery/registration/refresh). It rejects non-HTTPS,
// credentialed, metadata-host and private-IP-literal URLs up front, pins the
// connection to a connect-time-validated address, and refuses to auto-follow
// redirects (`redirect: "manual"`) so a 3xx to an internal host cannot smuggle
// egress past the guard.
export async function guardedFetch(
input: Parameters<typeof fetch>[0],
init?: Parameters<typeof fetch>[1],
Expand All @@ -363,7 +384,11 @@ export async function guardedFetch(
? input.toString()
: input.url;
await validateRemoteMcpUrl(url);
return fetch(input, { ...init, redirect: "manual" });
return fetch(input, {
...init,
redirect: "manual",
dispatcher: pinnedGuardAgent(),
} as RequestInit);
}

export function base64Url(buffer: Buffer) {
Expand Down
21 changes: 11 additions & 10 deletions backend/src/lib/mcp/oauth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,10 @@ function parseWwwAuthenticate(value: string | null): string | null {
}

async function fetchJson(url: string, init?: RequestInit) {
await validateRemoteMcpUrl(url);
const response = await fetch(url, { ...init, redirect: "manual" });
// Route through the shared guarded egress helper so this call gets the same
// HTTPS-only / private-IP / connect-time-pinned / no-redirect protections as
// the connector transport (closes the raw-fetch SSRF gap in OAuth discovery).
const response = await guardedFetch(url, init);
if (!response.ok) {
throw new Error(`Failed to fetch OAuth metadata (${response.status}).`);
}
Expand All @@ -58,12 +60,14 @@ async function fetchJson(url: string, init?: RequestInit) {
}

async function discoverProtectedResourceMetadataUrl(serverUrl: string) {
// The MCP server URL is attacker-influenced, so both discovery probes go
// through the shared guarded egress helper rather than raw fetch (previously
// an unvalidated SSRF sink).
const attempts: Array<() => Promise<Response>> = [
() => fetch(serverUrl, { method: "GET", redirect: "manual" }),
() => guardedFetch(serverUrl, { method: "GET" }),
() =>
fetch(serverUrl, {
guardedFetch(serverUrl, {
method: "POST",
redirect: "manual",
headers: {
Accept: "application/json, text/event-stream",
"Content-Type": "application/json",
Expand Down Expand Up @@ -189,10 +193,8 @@ async function registerOAuthClient(
redirectUri: string,
) {
if (!metadata.registrationEndpoint) return null;
await validateRemoteMcpUrl(metadata.registrationEndpoint);
const response = await fetch(metadata.registrationEndpoint, {
const response = await guardedFetch(metadata.registrationEndpoint, {
method: "POST",
redirect: "manual",
headers: {
Accept: "application/json",
"Content-Type": "application/json",
Expand Down Expand Up @@ -329,8 +331,7 @@ async function refreshOAuthAccessToken(row: OAuthTokenRow, db: Db) {
});
if (clientSecret) body.set("client_secret", clientSecret);
if (row.resource) body.set("resource", row.resource);
await validateRemoteMcpUrl(row.token_endpoint);
const response = await fetch(row.token_endpoint, {
const response = await guardedFetch(row.token_endpoint, {
method: "POST",
headers: {
Accept: "application/json",
Expand Down
Loading