diff --git a/.changeset/mcp-client-hardening.md b/.changeset/mcp-client-hardening.md new file mode 100644 index 000000000..868ade6ad --- /dev/null +++ b/.changeset/mcp-client-hardening.md @@ -0,0 +1,5 @@ +--- +"@voltagent/core": patch +--- + +Harden MCP client transport setup by filtering stdio environment variables and rejecting local, private, link-local, and reserved MCP server URL targets. diff --git a/packages/core/src/mcp/client/index.spec.ts b/packages/core/src/mcp/client/index.spec.ts index d99170b94..18c6e7dde 100644 --- a/packages/core/src/mcp/client/index.spec.ts +++ b/packages/core/src/mcp/client/index.spec.ts @@ -1,6 +1,9 @@ import { Client } from "@modelcontextprotocol/sdk/client/index.js"; import { SSEClientTransport } from "@modelcontextprotocol/sdk/client/sse.js"; -import { StdioClientTransport } from "@modelcontextprotocol/sdk/client/stdio.js"; +import { + StdioClientTransport, + getDefaultEnvironment, +} from "@modelcontextprotocol/sdk/client/stdio.js"; import { StreamableHTTPClientTransport } from "@modelcontextprotocol/sdk/client/streamableHttp.js"; import { beforeEach, describe, expect, it, vi } from "vitest"; import { convertJsonSchemaToZod } from "zod-from-json-schema"; @@ -23,7 +26,12 @@ vi.mock("@modelcontextprotocol/sdk/client/streamableHttp.js", () => ({ vi.mock("@modelcontextprotocol/sdk/client/stdio.js", () => ({ StdioClientTransport: vi.fn(), - getDefaultEnvironment: vi.fn().mockReturnValue({}), + getDefaultEnvironment: vi.fn().mockReturnValue({ + PATH: "/usr/bin", + HOME: "/home/test", + OPENAI_API_KEY: "secret-openai-key", + AWS_SECRET_ACCESS_KEY: "secret-aws-key", + }), })); vi.mock("zod-from-json-schema", () => ({ @@ -154,6 +162,89 @@ describe("MCPClient", () => { }); }); + it("should not pass sensitive parent environment variables to stdio servers", () => { + new MCPClient({ + clientInfo: mockClientInfo, + server: mockStdioServerConfig, + }); + + expect(getDefaultEnvironment).toHaveBeenCalled(); + const transportOptions = (StdioClientTransport as vi.Mock).mock.calls.at(-1)?.[0]; + expect(transportOptions.env).toMatchObject({ + PATH: "/usr/bin", + HOME: "/home/test", + TEST: "true", + }); + expect(transportOptions.env).not.toHaveProperty("OPENAI_API_KEY"); + expect(transportOptions.env).not.toHaveProperty("AWS_SECRET_ACCESS_KEY"); + }); + + it("should reject unsupported MCP server URL protocols", () => { + expect( + () => + new MCPClient({ + clientInfo: mockClientInfo, + server: { type: "http", url: "file:///etc/passwd" }, + }), + ).toThrow("Unsupported MCP server URL protocol"); + + expect( + () => + new MCPClient({ + clientInfo: mockClientInfo, + server: { type: "sse", url: "ftp://example.com/mcp" }, + }), + ).toThrow("Unsupported MCP server URL protocol"); + }); + + it("should reject MCP server URLs that target local or private addresses", () => { + const blockedUrls = [ + "http://127.0.0.1:8080/mcp", + "https://localhost/mcp", + "http://127.0.0.1.:8080/mcp", + "https://localhost./mcp", + "http://10.0.0.1/mcp", + "http://192.168.1.1/mcp", + "http://172.16.0.1/mcp", + "http://172.31.255.254/mcp", + "http://169.254.169.254/mcp", + "http://100.64.0.1/mcp", + "http://[::ffff:127.0.0.1]:8080/mcp", + "http://[::ffff:10.0.0.1]:8080/mcp", + "http://[::ffff:169.254.169.254]/mcp", + "http://[::ffff:192.168.0.1]:8080/mcp", + "http://[fc00::1]/mcp", + "http://[fd12::1]/mcp", + "http://[fe80::1]/mcp", + "http://[fe90::1]/mcp", + "http://[fea0::1]/mcp", + "http://[febf::1]/mcp", + ]; + const transportTypes = ["http", "sse", "streamable-http"] as const; + + for (const type of transportTypes) { + for (const url of blockedUrls) { + expect( + () => + new MCPClient({ + clientInfo: mockClientInfo, + server: { type, url }, + }), + ).toThrow("MCP server URL host is not allowed"); + } + } + }); + + it("should allow public hostnames that start with private IPv6 prefixes", () => { + expect( + () => + new MCPClient({ + clientInfo: mockClientInfo, + server: { type: "streamable-http", url: "https://fc-public.example.com/mcp" }, + }), + ).not.toThrow(); + }); + it("should throw an error for unsupported server config", () => { expect(() => { new MCPClient({ diff --git a/packages/core/src/mcp/client/index.ts b/packages/core/src/mcp/client/index.ts index 40bffeaa8..51533a063 100644 --- a/packages/core/src/mcp/client/index.ts +++ b/packages/core/src/mcp/client/index.ts @@ -1,3 +1,4 @@ +import { isIP } from "node:net"; import { Client } from "@modelcontextprotocol/sdk/client/index.js"; import { SSEClientTransport } from "@modelcontextprotocol/sdk/client/sse.js"; import { @@ -38,6 +39,118 @@ import type { import type { UserInputHandler } from "./user-input-bridge"; import { UserInputBridge } from "./user-input-bridge"; +const SAFE_STDIO_ENV_KEYS = new Set([ + "PATH", + "Path", + "HOME", + "USERPROFILE", + "APPDATA", + "LOCALAPPDATA", + "TMP", + "TEMP", + "SystemRoot", + "ComSpec", +]); + +function buildStdioEnvironment(explicitEnv?: Record): Record { + const safeDefaultEnvironment: Record = {}; + const defaultEnvironment = getDefaultEnvironment(); + + for (const [key, value] of Object.entries(defaultEnvironment)) { + if (SAFE_STDIO_ENV_KEYS.has(key)) { + safeDefaultEnvironment[key] = value; + } + } + + return { + ...safeDefaultEnvironment, + ...(explicitEnv || {}), + }; +} + +function createMCPServerUrl(rawUrl: string): URL { + const url = new URL(rawUrl); + if (url.protocol !== "http:" && url.protocol !== "https:") { + throw new Error(`Unsupported MCP server URL protocol: ${url.protocol}`); + } + + if (isLocalOrPrivateHost(url.hostname)) { + throw new Error(`MCP server URL host is not allowed: ${url.hostname}`); + } + + return url; +} + +function isLocalOrPrivateHost(hostname: string): boolean { + const normalized = hostname + .toLowerCase() + .replace(/^\[|\]$/g, "") + .replace(/\.+$/g, ""); + if (!normalized || normalized === "localhost" || normalized.endsWith(".localhost")) { + return true; + } + + return isPrivateIPv4(normalized) || isPrivateIPv6(normalized); +} + +function isPrivateIPv4(hostname: string): boolean { + const octets = hostname.split("."); + if (octets.length !== 4) { + return false; + } + + const values = octets.map((octet) => Number.parseInt(octet, 10)); + if (values.some((value, index) => !/^\d+$/.test(octets[index]) || value < 0 || value > 255)) { + return false; + } + + const [first, second] = values; + return ( + first === 0 || + first === 10 || + first === 127 || + (first === 169 && second === 254) || + (first === 172 && second >= 16 && second <= 31) || + (first === 192 && second === 168) || + (first === 100 && second >= 64 && second <= 127) + ); +} + +function getIPv4FromMappedIPv6(hostname: string): string | undefined { + const mappedMatch = hostname.match(/^::ffff:(?:(0):)?([0-9a-f]{1,4}):([0-9a-f]{1,4})$/i); + if (!mappedMatch) { + return undefined; + } + + const [, , highHex, lowHex] = mappedMatch; + const high = Number.parseInt(highHex, 16); + const low = Number.parseInt(lowHex, 16); + if (high > 0xffff || low > 0xffff) { + return undefined; + } + + return `${(high >> 8) & 0xff}.${high & 0xff}.${(low >> 8) & 0xff}.${low & 0xff}`; +} + +function isPrivateIPv6(hostname: string): boolean { + if (isIP(hostname) !== 6) { + return false; + } + + const mappedIPv4 = getIPv4FromMappedIPv6(hostname); + if (mappedIPv4) { + return isPrivateIPv4(mappedIPv4); + } + + const firstHextet = Number.parseInt(hostname.split(":")[0] || "0", 16); + return ( + hostname === "::1" || + hostname === "0:0:0:0:0:0:0:1" || + (firstHextet >= 0xfc00 && firstHextet <= 0xfdff) || + (firstHextet >= 0xfe80 && firstHextet <= 0xfebf) + ); +} + /** * Client for interacting with Model Context Protocol (MCP) servers. * Wraps the official MCP SDK client to provide a higher-level interface. @@ -159,19 +272,19 @@ export class MCPClient extends SimpleEventEmitter { if (this.isHTTPServer(config.server)) { // HTTP type: Try streamable HTTP first with SSE fallback - this.transport = new StreamableHTTPClientTransport(new URL(config.server.url), { + this.transport = new StreamableHTTPClientTransport(createMCPServerUrl(config.server.url), { requestInit: config.server.requestInit, }); this.shouldAttemptFallback = true; } else if (this.isSSEServer(config.server)) { // Explicit SSE transport - this.transport = new SSEClientTransport(new URL(config.server.url), { + this.transport = new SSEClientTransport(createMCPServerUrl(config.server.url), { requestInit: config.server.requestInit, eventSourceInit: config.server.eventSourceInit, }); } else if (this.isStreamableHTTPServer(config.server)) { // Explicit streamable HTTP transport (no fallback) - this.transport = new StreamableHTTPClientTransport(new URL(config.server.url), { + this.transport = new StreamableHTTPClientTransport(createMCPServerUrl(config.server.url), { requestInit: config.server.requestInit, sessionId: config.server.sessionId, }); @@ -181,7 +294,7 @@ export class MCPClient extends SimpleEventEmitter { command: config.server.command, args: config.server.args || [], cwd: config.server.cwd, - env: { ...getDefaultEnvironment(), ...(config.server.env || {}) }, + env: buildStdioEnvironment(config.server.env), }); } else { throw new Error( @@ -292,7 +405,7 @@ export class MCPClient extends SimpleEventEmitter { throw new Error("Invalid server config for SSE fallback"); } - this.transport = new SSEClientTransport(new URL(this.serverConfig.url), { + this.transport = new SSEClientTransport(createMCPServerUrl(this.serverConfig.url), { requestInit: this.serverConfig.requestInit, eventSourceInit: this.serverConfig.eventSourceInit, });