From 771f23d36b95ec2204cc9a0054045f5d8439ea75 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 27 Jan 2026 03:33:09 +0000 Subject: [PATCH] fix(exec): prevent PATH injection in docker sandbox --- docs/tools/exec.md | 3 ++- src/agents/bash-tools.shared.ts | 9 ++++++++- src/agents/bash-tools.test.ts | 25 +++++++++++++++++++++++-- 3 files changed, 33 insertions(+), 4 deletions(-) diff --git a/docs/tools/exec.md b/docs/tools/exec.md index 2524c3665..30771cf38 100644 --- a/docs/tools/exec.md +++ b/docs/tools/exec.md @@ -67,7 +67,8 @@ Example: - macOS: `/opt/homebrew/bin`, `/usr/local/bin`, `/usr/bin`, `/bin` - Linux: `/usr/local/bin`, `/usr/bin`, `/bin` - `host=sandbox`: runs `sh -lc` (login shell) inside the container, so `/etc/profile` may reset `PATH`. - Clawdbot prepends `env.PATH` after profile sourcing; `tools.exec.pathPrepend` applies here too. + Clawdbot prepends `env.PATH` after profile sourcing via an internal env var (no shell interpolation); + `tools.exec.pathPrepend` applies here too. - `host=node`: only env overrides you pass are sent to the node. `tools.exec.pathPrepend` only applies if the exec call already sets `env.PATH`. Headless node hosts accept `PATH` only when it prepends the node host PATH (no replacement). macOS nodes drop `PATH` overrides entirely. diff --git a/src/agents/bash-tools.shared.ts b/src/agents/bash-tools.shared.ts index bd09a18ff..aa4e5d000 100644 --- a/src/agents/bash-tools.shared.ts +++ b/src/agents/bash-tools.shared.ts @@ -60,11 +60,18 @@ export function buildDockerExecArgs(params: { for (const [key, value] of Object.entries(params.env)) { args.push("-e", `${key}=${value}`); } + const hasCustomPath = typeof params.env.PATH === "string" && params.env.PATH.length > 0; + if (hasCustomPath) { + // Avoid interpolating PATH into the shell command; pass it via env instead. + args.push("-e", `CLAWDBOT_PREPEND_PATH=${params.env.PATH}`); + } // Login shell (-l) sources /etc/profile which resets PATH to a minimal set, // overriding both Docker ENV and -e PATH=... environment variables. // Prepend custom PATH after profile sourcing to ensure custom tools are accessible // while preserving system paths that /etc/profile may have added. - const pathExport = params.env.PATH ? `export PATH="${params.env.PATH}:$PATH"; ` : ""; + const pathExport = hasCustomPath + ? 'export PATH="${CLAWDBOT_PREPEND_PATH}:$PATH"; unset CLAWDBOT_PREPEND_PATH; ' + : ""; args.push(params.containerName, "sh", "-lc", `${pathExport}${params.command}`); return args; } diff --git a/src/agents/bash-tools.test.ts b/src/agents/bash-tools.test.ts index 500b95f30..e5a008ca2 100644 --- a/src/agents/bash-tools.test.ts +++ b/src/agents/bash-tools.test.ts @@ -318,9 +318,30 @@ describe("buildDockerExecArgs", () => { }); const commandArg = args[args.length - 1]; - expect(commandArg).toContain('export PATH="/custom/bin:/usr/local/bin:/usr/bin:$PATH"'); + expect(args).toContain("CLAWDBOT_PREPEND_PATH=/custom/bin:/usr/local/bin:/usr/bin"); + expect(commandArg).toContain('export PATH="${CLAWDBOT_PREPEND_PATH}:$PATH"'); expect(commandArg).toContain("echo hello"); - expect(commandArg).toBe('export PATH="/custom/bin:/usr/local/bin:/usr/bin:$PATH"; echo hello'); + expect(commandArg).toBe( + 'export PATH="${CLAWDBOT_PREPEND_PATH}:$PATH"; unset CLAWDBOT_PREPEND_PATH; echo hello', + ); + }); + + it("does not interpolate PATH into the shell command", () => { + const injectedPath = "$(touch /tmp/clawdbot-path-injection)"; + const args = buildDockerExecArgs({ + containerName: "test-container", + command: "echo hello", + env: { + PATH: injectedPath, + HOME: "/home/user", + }, + tty: false, + }); + + const commandArg = args[args.length - 1]; + expect(args).toContain(`CLAWDBOT_PREPEND_PATH=${injectedPath}`); + expect(commandArg).not.toContain(injectedPath); + expect(commandArg).toContain("CLAWDBOT_PREPEND_PATH"); }); it("does not add PATH export when PATH is not in env", () => {