fix(mcp): harden stdio env filtering

eleqtrizit · eleqtrizit · commit c440454b5b6d · 2026-06-09T19:10:48.000Z
diff --git a/src/agents/mcp-config-shared.ts b/src/agents/mcp-config-shared.ts
@@ -4,7 +4,43 @@
  * MCP transport setup uses these functions to normalize loose JSON config into
  * string records/arrays while dropping unsafe host environment variables.
  */
-import { isDangerousHostEnvVarName } from "../infra/host-env-security.js";
+import {
+  isDangerousHostEnvVarName,
+  isDangerousHostInheritedEnvVarName,
+  normalizeEnvVarKey,
+} from "../infra/host-env-security.js";
+
+const MCP_EXPLICIT_CREDENTIAL_ENV_KEYS = new Set([
+  // Explicit MCP server credentials are operator-configured auth inputs, not
+  // inherited host config pivots. If policy adds credential keys, add only
+  // direct credentials here; keep loader/search/config pivots blocked.
+  "AMQP_URL",
+  "AWS_ACCESS_KEY_ID",
+  "AWS_SECRET_ACCESS_KEY",
+  "AWS_SECURITY_TOKEN",
+  "AWS_SESSION_TOKEN",
+  "AZURE_CLIENT_ID",
+  "AZURE_CLIENT_SECRET",
+  "DATABASE_URL",
+  "GH_TOKEN",
+  "GITHUB_TOKEN",
+  "GITLAB_TOKEN",
+  "MONGODB_URI",
+  "NODE_AUTH_TOKEN",
+  "NPM_TOKEN",
+  "REDIS_URL",
+]);
+
+function isDangerousMcpStdioEnvVarName(rawKey: string): boolean {
+  if (isDangerousHostEnvVarName(rawKey)) {
+    return true;
+  }
+  const key = normalizeEnvVarKey(rawKey);
+  if (!key || MCP_EXPLICIT_CREDENTIAL_ENV_KEYS.has(key.toUpperCase())) {
+    return false;
+  }
+  return isDangerousHostInheritedEnvVarName(key);
+}
 
 /** Returns whether a value is a plain MCP config record. */
 export function isMcpConfigRecord(value: unknown): value is Record<string, unknown> {
@@ -63,7 +99,7 @@ export function toMcpEnvRecord(
   return toMcpFilteredStringRecord(value, {
     ...options,
     preserveEmptyWhenKeysDropped: true,
-    shouldDropKey: (key) => isDangerousHostEnvVarName(key),
+    shouldDropKey: (key) => isDangerousMcpStdioEnvVarName(key),
   });
 }
 
diff --git a/src/agents/mcp-transport-config.test.ts b/src/agents/mcp-transport-config.test.ts
@@ -49,8 +49,9 @@ describe("resolveMcpTransportConfig", () => {
   });
 
   it("drops dangerous env overrides from stdio config", () => {
-    // Stdio env is executable process input. Block loader/shell hook variables
-    // while preserving ordinary provider tokens and scalar env values.
+    // Stdio env is inherited executable process input. Block loader/shell hook
+    // variables and child-process config pivots while preserving explicit MCP
+    // credentials and ordinary scalar env values.
     const resolved = resolveMcpTransportConfig("probe", {
       command: "node",
       env: {
@@ -62,6 +63,8 @@ describe("resolveMcpTransportConfig", () => {
         NODE_OPTIONS: "--require=./evil.js",
         LD_PRELOAD: "/tmp/pwn.so",
         BASH_ENV: "/tmp/pwn.sh",
+        ANSIBLE_CONFIG: "/tmp/evil-ansible.cfg",
+        TF_CLI_CONFIG_FILE: "/tmp/evil-terraform.rc",
       },
     });
 
@@ -92,6 +95,12 @@ describe("resolveMcpTransportConfig", () => {
     expect(logWarn).toHaveBeenCalledWith(
       'bundle-mcp: server "probe": env "BASH_ENV" is blocked for stdio startup safety and was ignored.',
     );
+    expect(logWarn).toHaveBeenCalledWith(
+      'bundle-mcp: server "probe": env "ANSIBLE_CONFIG" is blocked for stdio startup safety and was ignored.',
+    );
+    expect(logWarn).toHaveBeenCalledWith(
+      'bundle-mcp: server "probe": env "TF_CLI_CONFIG_FILE" is blocked for stdio startup safety and was ignored.',
+    );
   });
 
   it("uses an explicit empty stdio env when all configured env keys are blocked", () => {
