fix(exec): prevent PATH injection in docker sandbox
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
@@ -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", () => {
|
||||
|
||||
Reference in New Issue
Block a user