Skip to content

fix(core): harden MCP client transport setup #1389

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
zcxGGmu wants to merge 4 commits into
base: main
Choose a base branch
from
Open

fix(core): harden MCP client transport setup #1389

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f4b16f8
fix(core): harden MCP client transport setup
MaskerFather Aug 1, 2026
e624db1
fix(core): block mapped IPv6 MCP hosts
MaskerFather Aug 1, 2026
1ed799f
fix: harden MCP private host detection
MaskerFather Aug 2, 2026
d441a80
chore: add changeset for MCP hardening
MaskerFather Aug 4, 2026
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
65 changes: 63 additions & 2 deletions packages/core/src/mcp/client/index.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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";
Expand All @@ -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", () => ({
Expand Down Expand Up @@ -154,6 +162,59 @@ 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", () => {
expect(
() =>
new MCPClient({
clientInfo: mockClientInfo,
server: { type: "streamable-http", url: "http://127.0.0.1:8080/mcp" },
}),
).toThrow("MCP server URL host is not allowed");

expect(
() =>
new MCPClient({
clientInfo: mockClientInfo,
server: { type: "http", url: "https://localhost/mcp" },
}),
).toThrow("MCP server URL host is not allowed");
});

it("should throw an error for unsupported server config", () => {
expect(() => {
new MCPClient({
Expand Down
94 changes: 89 additions & 5 deletions packages/core/src/mcp/client/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,90 @@ 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<string, string>): Record<string, string> {
const safeDefaultEnvironment: Record<string, string> = {};
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, "");
if (!normalized || normalized === "localhost" || normalized.endsWith(".localhost")) {
return true;
}

return isPrivateIPv4(normalized) || isPrivateIPv6(normalized);
}
Comment on lines +84 to +94

@coderabbitai coderabbitai Bot Aug 1, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

Does the WHATWG URL parser strip a trailing dot from non-numeric domain hostnames?

💡 Result:

No, the WHATWG URL parser does not strip a trailing dot from a domain hostname [1][2]. The parser preserves the trailing dot if it is present in the input [1][3]. In the WHATWG URL Standard, the hostname is parsed and maintained as a string, and explicit logic exists to handle the trailing dot separately when performing operations such as determining the public suffix or the registrable domain [4][3]. For example, when calculating these properties, the standard explicitly defines a "trailingDot" variable that captures the dot if it exists at the end of the host, allowing algorithms to account for it while treating the rest of the host according to established rules [4][3]. This behavior contrasts with some other specifications or implementations that may normalize hostnames by removing trailing dots [5][2]. Because the WHATWG parser keeps the trailing dot, it is possible for a URL with a trailing dot (e.g., example.com.) to be treated as distinct from one without it (e.g., example.com) in some contexts, such as string-based security or filtering mechanisms [5][2]. The standard itself notes that while certificate comparison requires a host equivalence check that ignores the trailing dot, the URL parser does not enforce such restrictions on the host component [4][3][6].

Citations:

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Locate and inspect the target file and relevant implementation/imports.
if [ -f packages/core/src/mcp/client/index.ts ]; then
  echo "== outline =="
  ast-grep outline packages/core/src/mcp/client/index.ts --view expanded || true
  echo
  echo "== target lines =="
  sed -n '1,140p' packages/core/src/mcp/client/index.ts | nl -ba
else
  echo "target file not found"
  fd -a 'index.ts' . | sed -n '1,20p'
fi

echo
echo "== isLocalOrPrivateHost usages =="
rg -n "isLocalOrPrivateHost|privateIPv4|privateIPv6|isPrivateIPv" packages/core/src/mcp packages -S || true

echo
echo "== DNS resolution / connector usage mentions =="
rg -n "lookup|family|createServer|createClient|http\.|https\.|net\.|Agent|RequestInit|initTimeout|timeout|ssrf|mcp.*/client|serverTransport|ClientTransport" packages/core/src packages -S | sed -n '1,160p'

Repository: VoltAgent/voltagent

Length of output: 2525

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== target lines =="
sed -n '1,130p' packages/core/src/mcp/client/index.ts

echo
echo "== connect and HTTP/SSE construction excerpts =="
sed -n '130,420p' packages/core/src/mcp/client/index.ts

echo
echo "== isLocalOrPrivateHost usages =="
rg -n "isLocalOrPrivateHost|privateIPv4|privateIPv6|isPrivateIPv" . -S || true

echo
echo "== DNS resolution / connector usage mentions =="
rg -n "lookup|family|createServer|createClient|http\.|https\.|net\.|Agent|RequestInit|initTimeout|timeout|ssrf|mcp.*/client|serverTransport|ClientTransport|fetch\\(" packages/core/src packages -S 2>/dev/null | sed -n '1,240p'

Repository: VoltAgent/voltagent

Length of output: 36592

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Node WHATWG URL hostname behavior =="
node - <<'JS'
for (const raw of ["http://example.com", "http://example.com.", "http://localhost", "http://localhost.", "http://[::1]", "http://[::1]."]) {
  const url = new URL(raw)
  console.log(`${raw} -> protocol=${url.protocol}; host=${url.host}; hostname=${url.hostname}; hostEndsWithDot=${url.host.endsWith(".")}; hostnameEndsWithDot=${url.hostname.endsWith(".")}`)
}
JS

echo
echo "== MCP package version / lockfiles =="
fd -a 'package.json' packages/core packages/sdk | while read -r f; do
  echo "-- $f"
  sed -n '1,140p' "$f"
done

echo
echo "== lockfile mentions of mcp sdk =="
rg -n "`@modelcontextprotocol/sdk`|modelcontextprotocol" package.json pnpm-lock.yaml yarn.lock packages -S 2>/dev/null | sed -n '1,220p'

Repository: VoltAgent/voltagent

Length of output: 1490

Guard DNS and trailing-dot variants before connecting.

isLocalOrPrivateHost() checks the URL hostname string once before transport construction. Add connection-time IP validation or a custom resolver/agent policy to close rebind/short-TTl DNS bypasses. Also normalize/truncate trailing dots on hostnames before comparison, because new URL("...://localhost.") preserves hostname as localhost..

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/core/src/mcp/client/index.ts` around lines 83 - 90, Harden
isLocalOrPrivateHost and the connection path so DNS-resolved addresses are
validated at connection time, preventing rebinding or short-TTL DNS from
bypassing private-host checks; use the existing resolver/agent policy where
available rather than relying only on the initial URL hostname. Normalize
hostnames by removing trailing dots before localhost and IP comparisons, while
preserving bracket handling and private IPv4/IPv6 detection.


function isPrivateIPv4(hostname: string): boolean {
const octets = hostname.split(".");
if (octets.length !== 4) {
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
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 isPrivateIPv6(hostname: string): boolean {
return (
hostname === "::1" ||
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
hostname.startsWith("fc") ||
hostname.startsWith("fd") ||
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
Outdated
hostname.startsWith("fe80:") ||
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
Outdated
hostname === "0:0:0:0:0:0:0:1"
);
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

/**
* Client for interacting with Model Context Protocol (MCP) servers.
* Wraps the official MCP SDK client to provide a higher-level interface.
Expand Down Expand Up @@ -159,19 +243,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,
});
Expand All @@ -181,7 +265,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(
Expand Down Expand Up @@ -292,7 +376,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,
});
Expand Down