From 0a5821a8117f4e118c147557dd208cb1831c1d54 Mon Sep 17 00:00:00 2001 From: Hasan FLeyah Date: Mon, 2 Feb 2026 02:36:24 +0300 Subject: [PATCH] fix(security): enforce strict environment variable validation in exec tool (#4896) --- src/agents/bash-tools.exec.path.test.ts | 15 +++---- src/agents/bash-tools.exec.ts | 54 +++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 7 deletions(-) diff --git a/src/agents/bash-tools.exec.path.test.ts b/src/agents/bash-tools.exec.path.test.ts index 4b2e7d964..33e922ce1 100644 --- a/src/agents/bash-tools.exec.path.test.ts +++ b/src/agents/bash-tools.exec.path.test.ts @@ -86,7 +86,7 @@ describe("exec PATH login shell merge", () => { expect(shellPathMock).toHaveBeenCalledTimes(1); }); - it("skips login-shell PATH when env.PATH is provided", async () => { + it("throws security violation when env.PATH is provided", async () => { if (isWin) { return; } @@ -98,13 +98,14 @@ describe("exec PATH login shell merge", () => { shellPathMock.mockClear(); const tool = createExecTool({ host: "gateway", security: "full", ask: "off" }); - const result = await tool.execute("call1", { - command: "echo $PATH", - env: { PATH: "/explicit/bin" }, - }); - const entries = normalizePathEntries(result.content.find((c) => c.type === "text")?.text); - expect(entries).toEqual(["/explicit/bin"]); + await expect( + tool.execute("call1", { + command: "echo $PATH", + env: { PATH: "/explicit/bin" }, + }), + ).rejects.toThrow(/Security Violation: Custom 'PATH' variable is forbidden/); + expect(shellPathMock).not.toHaveBeenCalled(); }); }); diff --git a/src/agents/bash-tools.exec.ts b/src/agents/bash-tools.exec.ts index 2f8b026ac..f510045b5 100644 --- a/src/agents/bash-tools.exec.ts +++ b/src/agents/bash-tools.exec.ts @@ -56,6 +56,49 @@ import { getShellConfig, sanitizeBinaryOutput } from "./shell-utils.js"; import { callGatewayTool } from "./tools/gateway.js"; import { listNodes, resolveNodeIdFromList } from "./tools/nodes-utils.js"; +// Security: Blocklist of environment variables that could alter execution flow +// or inject code when running on non-sandboxed hosts (Gateway/Node). +const DANGEROUS_HOST_ENV_VARS = new Set([ + "LD_PRELOAD", + "LD_LIBRARY_PATH", + "LD_AUDIT", + "DYLD_INSERT_LIBRARIES", + "DYLD_LIBRARY_PATH", + "NODE_OPTIONS", + "NODE_PATH", + "PYTHONPATH", + "PYTHONHOME", + "RUBYLIB", + "PERL5LIB", + "BASH_ENV", + "ENV", + "GCONV_PATH", + "IFS", + "SSLKEYLOGFILE", +]); + +// Centralized sanitization helper. +// Throws an error if dangerous variables or PATH modifications are detected on the host. +function validateHostEnv(env: Record): void { + for (const key of Object.keys(env)) { + const upperKey = key.toUpperCase(); + + // 1. Block known dangerous variables (Fail Closed) + if (DANGEROUS_HOST_ENV_VARS.has(upperKey)) { + throw new Error( + `Security Violation: Environment variable '${key}' is forbidden during host execution.`, + ); + } + + // 2. Strictly block PATH modification on host + // Allowing custom PATH on the gateway/node can lead to binary hijacking. + if (upperKey === "PATH") { + throw new Error( + "Security Violation: Custom 'PATH' variable is forbidden during host execution.", + ); + } + } +} const DEFAULT_MAX_OUTPUT = clampNumber( readEnvInt("PI_BASH_MAX_OUTPUT_CHARS"), 200_000, @@ -916,7 +959,15 @@ export function createExecTool( } const baseEnv = coerceEnv(process.env); + + // Logic: Sandbox gets raw env. Host (gateway/node) must pass validation. + // We validate BEFORE merging to prevent any dangerous vars from entering the stream. + if (host !== "sandbox" && params.env) { + validateHostEnv(params.env); + } + const mergedEnv = params.env ? { ...baseEnv, ...params.env } : baseEnv; + const env = sandbox ? buildSandboxEnv({ defaultPath: DEFAULT_PATH, @@ -925,6 +976,7 @@ export function createExecTool( containerWorkdir: containerWorkdir ?? sandbox.containerWorkdir, }) : mergedEnv; + if (!sandbox && host === "gateway" && !params.env?.PATH) { const shellPath = getShellPathFromLoginShell({ env: process.env, @@ -976,7 +1028,9 @@ export function createExecTool( ); } const argv = buildNodeShellCommand(params.command, nodeInfo?.platform); + const nodeEnv = params.env ? { ...params.env } : undefined; + if (nodeEnv) { applyPathPrepend(nodeEnv, defaultPathPrepend, { requireExisting: true }); }