Skip to content
Merged
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
60 changes: 48 additions & 12 deletions src/connectors/mcp-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,10 +170,7 @@ async function executeStdioMcp(
}

const child = spawn(transport.command, transport.args ?? [], {
env: {
...process.env,
...resolveChildEnv(transport.env ?? {}),
},
env: buildChildEnv(transport.env ?? {}),
shell: false,
stdio: ["pipe", "pipe", "pipe"],
});
Expand Down Expand Up @@ -220,10 +217,7 @@ async function executeStdioMcpTool(
}

const child = spawn(transport.command, transport.args ?? [], {
env: {
...process.env,
...resolveChildEnv(transport.env ?? {}),
},
env: buildChildEnv(transport.env ?? {}),
shell: false,
stdio: ["pipe", "pipe", "pipe"],
});
Expand Down Expand Up @@ -256,10 +250,7 @@ async function listStdioMcpTools(
}

const child = spawn(transport.command, transport.args ?? [], {
env: {
...process.env,
...resolveChildEnv(transport.env ?? {}),
},
env: buildChildEnv(transport.env ?? {}),
shell: false,
stdio: ["pipe", "pipe", "pipe"],
});
Expand Down Expand Up @@ -734,6 +725,51 @@ function resolveChildEnv(
);
}

// Base environment variables an MCP subprocess may legitimately need to run
// (locating binaries, resolving the home dir, temp paths, terminal behavior).
// Deliberately excludes OpenWiki credentials so a spawned MCP server command
// cannot read the user's API keys and OAuth refresh tokens out of process.env.
const CHILD_ENV_ALLOWLIST = [
"PATH",
"HOME",
"HOMEPATH",
"HOMEDRIVE",
"USERPROFILE",
"TMPDIR",
"TEMP",
"TMP",
"LANG",
"LC_ALL",
"LC_CTYPE",
"TERM",
"TZ",
"SystemRoot",
"SYSTEMROOT",
"ComSpec",
"PATHEXT",
] as const;

// Builds the environment for a spawned stdio MCP subprocess: only an
// allow-listed set of safe base variables plus the credentials the transport
// explicitly declares via `transport.env`. The full process.env (which holds
// every provider API key and OAuth token) is never forwarded.
// Exported for tests to pin this security invariant.
export function buildChildEnv(
envRefs: Record<string, string>,
): Record<string, string> {
const base: Record<string, string> = {};
for (const key of CHILD_ENV_ALLOWLIST) {
const value = process.env[key];
if (typeof value === "string") {
base[key] = value;
}
}
return {
...base,
...resolveChildEnv(envRefs),
};
}

async function resolveHeaders(
headers: Record<string, string>,
): Promise<Record<string, string>> {
Expand Down
70 changes: 70 additions & 0 deletions test/mcp-client.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
import { afterEach, beforeEach, describe, expect, test } from "vitest";
import { buildChildEnv } from "../src/connectors/mcp-client.ts";

describe("buildChildEnv", () => {
const SECRET_KEYS = [
"ANTHROPIC_API_KEY",
"OPENAI_API_KEY",
"TAVILY_API_KEY",
"SLACK_CLIENT_SECRET",
"GMAIL_ACCESS_TOKEN",
"X_REFRESH_TOKEN",
];
const saved: Record<string, string | undefined> = {};

beforeEach(() => {
for (const key of [...SECRET_KEYS, "PATH", "MCP_SERVER_TOKEN"]) {
saved[key] = process.env[key];
}
for (const key of SECRET_KEYS) {
process.env[key] = `secret-value-for-${key}`;
}
process.env.PATH = "/usr/bin:/bin";
process.env.MCP_SERVER_TOKEN = "declared-token-123";
});

afterEach(() => {
for (const [key, value] of Object.entries(saved)) {
if (value === undefined) {
delete process.env[key];
} else {
process.env[key] = value;
}
}
});

test("does not forward OpenWiki credentials to the child env", () => {
const childEnv = buildChildEnv({});
for (const key of SECRET_KEYS) {
expect(childEnv).not.toHaveProperty(key);
}
// A random full-process.env secret must never leak by value either.
expect(Object.values(childEnv)).not.toContain(
"secret-value-for-ANTHROPIC_API_KEY",
);
});

test("passes through allow-listed base variables like PATH", () => {
const childEnv = buildChildEnv({});
expect(childEnv.PATH).toBe("/usr/bin:/bin");
});

test("resolves only the credentials the transport explicitly declares", () => {
const childEnv = buildChildEnv({ MCP_TOKEN: "${MCP_SERVER_TOKEN}" });
expect(childEnv.MCP_TOKEN).toBe("declared-token-123");
// The source var name itself is not exposed, only the mapped target var.
expect(childEnv).not.toHaveProperty("MCP_SERVER_TOKEN");
});

test("throws for an unresolvable declared reference", () => {
expect(() => buildChildEnv({ MCP_TOKEN: "${DOES_NOT_EXIST}" })).toThrow(
/DOES_NOT_EXIST is required/u,
);
});

test("rejects invalid child env key names", () => {
expect(() => buildChildEnv({ "bad-key": "${PATH}" })).toThrow(
/Invalid env var reference/u,
);
});
});