fix(mcp): block dangerous stdio env overrides

This commit is contained in:
Devin Robison
2026-04-20 21:31:09 +00:00
committed by Peter Steinberger
parent 97534372f8
commit 62fa507189
3 changed files with 83 additions and 4 deletions

View File

@@ -1,16 +1,28 @@
import {
isDangerousHostEnvOverrideVarName,
isDangerousHostEnvVarName,
} from "../infra/host-env-security.js";
export function isMcpConfigRecord(value: unknown): value is Record<string, unknown> {
return value !== null && typeof value === "object" && !Array.isArray(value);
}
export function toMcpStringRecord(
function toMcpFilteredStringRecord(
value: unknown,
options?: { onDroppedEntry?: (key: string, value: unknown) => void },
options?: {
onDroppedEntry?: (key: string, value: unknown) => void;
shouldDropKey?: (key: string) => boolean;
},
): Record<string, string> | undefined {
if (!isMcpConfigRecord(value)) {
return undefined;
}
const entries = Object.entries(value)
.map(([key, entry]) => {
if (options?.shouldDropKey?.(key)) {
options?.onDroppedEntry?.(key, entry);
return null;
}
if (typeof entry === "string") {
return [key, entry] as const;
}
@@ -24,6 +36,24 @@ export function toMcpStringRecord(
return entries.length > 0 ? Object.fromEntries(entries) : undefined;
}
export function toMcpStringRecord(
value: unknown,
options?: { onDroppedEntry?: (key: string, value: unknown) => void },
): Record<string, string> | undefined {
return toMcpFilteredStringRecord(value, options);
}
export function toMcpEnvRecord(
value: unknown,
options?: { onDroppedEntry?: (key: string, value: unknown) => void },
): Record<string, string> | undefined {
return toMcpFilteredStringRecord(value, {
...options,
shouldDropKey: (key) =>
isDangerousHostEnvVarName(key) || isDangerousHostEnvOverrideVarName(key),
});
}
export function toMcpStringArray(value: unknown): string[] | undefined {
if (!Array.isArray(value)) {
return undefined;

View File

@@ -1,4 +1,4 @@
import { isMcpConfigRecord, toMcpStringArray, toMcpStringRecord } from "./mcp-config-shared.js";
import { isMcpConfigRecord, toMcpEnvRecord, toMcpStringArray } from "./mcp-config-shared.js";
type StdioMcpServerLaunchConfig = {
command: string;
@@ -35,7 +35,7 @@ export function resolveStdioMcpServerLaunchConfig(raw: unknown): StdioMcpServerL
config: {
command: raw.command,
args: toMcpStringArray(raw.args),
env: toMcpStringRecord(raw.env),
env: toMcpEnvRecord(raw.env),
cwd,
},
};

View File

@@ -18,6 +18,35 @@ describe("resolveMcpTransportConfig", () => {
});
});
it("drops dangerous env overrides from stdio config", () => {
const resolved = resolveMcpTransportConfig("probe", {
command: "node",
env: {
SAFE_VALUE: "ok",
PORT: 3000,
ENABLED: true,
NODE_OPTIONS: "--require=./evil.js",
LD_PRELOAD: "/tmp/pwn.so",
BASH_ENV: "/tmp/pwn.sh",
},
});
expect(resolved).toEqual({
kind: "stdio",
transportType: "stdio",
command: "node",
args: undefined,
env: {
SAFE_VALUE: "ok",
PORT: "3000",
ENABLED: "true",
},
cwd: undefined,
description: "node",
connectionTimeoutMs: 30_000,
});
});
it("resolves SSE config by default", () => {
const resolved = resolveMcpTransportConfig("probe", {
url: "https://mcp.example.com/sse",
@@ -40,6 +69,26 @@ describe("resolveMcpTransportConfig", () => {
});
});
it("keeps HTTP header parsing unchanged for env-like names", () => {
const resolved = resolveMcpTransportConfig("probe", {
url: "https://mcp.example.com/sse",
headers: {
NODE_OPTIONS: "allowed-header",
},
});
expect(resolved).toEqual({
kind: "http",
transportType: "sse",
url: "https://mcp.example.com/sse",
headers: {
NODE_OPTIONS: "allowed-header",
},
description: "https://mcp.example.com/sse",
connectionTimeoutMs: 30_000,
});
});
it("resolves explicit streamable HTTP config", () => {
const resolved = resolveMcpTransportConfig("probe", {
url: "https://mcp.example.com/http",