fix: restore docker binds and PATH in sandbox exec (#873)
Thanks @akonyer. Co-authored-by: Aaron Konyer <aaronk@gomodular.ca>
This commit is contained in:
@@ -11,6 +11,8 @@
|
||||
### Fixes
|
||||
- Embedded runner: suppress raw API error payloads from replies. (#924) — thanks @grp06.
|
||||
- Auth: normalize Claude Code CLI profile mode to oauth and auto-migrate config. (#855) — thanks @sebslight.
|
||||
- Sandbox: restore `docker.binds` config validation for custom bind mounts. (#873) — thanks @akonyer.
|
||||
- Sandbox: preserve configured PATH for `docker exec` so custom tools remain available. (#873) — thanks @akonyer.
|
||||
|
||||
## 2026.1.14
|
||||
|
||||
|
||||
@@ -434,3 +434,7 @@ Example:
|
||||
- Container not running: it will auto-create per session on demand.
|
||||
- Permission errors in sandbox: set `docker.user` to a UID:GID that matches your
|
||||
mounted workspace ownership (or chown the workspace folder).
|
||||
- Custom tools not found: Clawdbot runs commands with `sh -lc` (login shell), which
|
||||
sources `/etc/profile` and may reset PATH. Set `docker.env.PATH` to prepend your
|
||||
custom tool paths (e.g., `/custom/bin:/usr/local/share/npm-global/bin`), or add
|
||||
a script under `/etc/profile.d/` in your Dockerfile.
|
||||
|
||||
@@ -60,7 +60,12 @@ export function buildDockerExecArgs(params: {
|
||||
for (const [key, value] of Object.entries(params.env)) {
|
||||
args.push("-e", `${key}=${value}`);
|
||||
}
|
||||
args.push(params.containerName, "sh", "-lc", params.command);
|
||||
// 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"; ` : "";
|
||||
args.push(params.containerName, "sh", "-lc", `${pathExport}${params.command}`);
|
||||
return args;
|
||||
}
|
||||
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { afterEach, beforeEach, describe, expect, it } from "vitest";
|
||||
import { resetProcessRegistryForTests } from "./bash-process-registry.js";
|
||||
import { createExecTool, createProcessTool, execTool, processTool } from "./bash-tools.js";
|
||||
import { buildDockerExecArgs } from "./bash-tools.shared.js";
|
||||
import { sanitizeBinaryOutput } from "./shell-utils.js";
|
||||
|
||||
const isWin = process.platform === "win32";
|
||||
@@ -239,3 +240,73 @@ describe("exec tool backgrounding", () => {
|
||||
expect(pollB.details.status).toBe("failed");
|
||||
});
|
||||
});
|
||||
|
||||
describe("buildDockerExecArgs", () => {
|
||||
it("prepends custom PATH after login shell sourcing to preserve both custom and system tools", () => {
|
||||
const args = buildDockerExecArgs({
|
||||
containerName: "test-container",
|
||||
command: "echo hello",
|
||||
env: {
|
||||
PATH: "/custom/bin:/usr/local/bin:/usr/bin",
|
||||
HOME: "/home/user",
|
||||
},
|
||||
tty: false,
|
||||
});
|
||||
|
||||
const commandArg = args[args.length - 1];
|
||||
expect(commandArg).toContain('export PATH="/custom/bin:/usr/local/bin:/usr/bin:$PATH"');
|
||||
expect(commandArg).toContain("echo hello");
|
||||
expect(commandArg).toBe('export PATH="/custom/bin:/usr/local/bin:/usr/bin:$PATH"; echo hello');
|
||||
});
|
||||
|
||||
it("does not add PATH export when PATH is not in env", () => {
|
||||
const args = buildDockerExecArgs({
|
||||
containerName: "test-container",
|
||||
command: "echo hello",
|
||||
env: {
|
||||
HOME: "/home/user",
|
||||
},
|
||||
tty: false,
|
||||
});
|
||||
|
||||
const commandArg = args[args.length - 1];
|
||||
expect(commandArg).toBe("echo hello");
|
||||
expect(commandArg).not.toContain("export PATH");
|
||||
});
|
||||
|
||||
it("includes workdir flag when specified", () => {
|
||||
const args = buildDockerExecArgs({
|
||||
containerName: "test-container",
|
||||
command: "pwd",
|
||||
workdir: "/workspace",
|
||||
env: { HOME: "/home/user" },
|
||||
tty: false,
|
||||
});
|
||||
|
||||
expect(args).toContain("-w");
|
||||
expect(args).toContain("/workspace");
|
||||
});
|
||||
|
||||
it("uses login shell for consistent environment", () => {
|
||||
const args = buildDockerExecArgs({
|
||||
containerName: "test-container",
|
||||
command: "echo test",
|
||||
env: { HOME: "/home/user" },
|
||||
tty: false,
|
||||
});
|
||||
|
||||
expect(args).toContain("sh");
|
||||
expect(args).toContain("-lc");
|
||||
});
|
||||
|
||||
it("includes tty flag when requested", () => {
|
||||
const args = buildDockerExecArgs({
|
||||
containerName: "test-container",
|
||||
command: "bash",
|
||||
env: { HOME: "/home/user" },
|
||||
tty: true,
|
||||
});
|
||||
|
||||
expect(args).toContain("-t");
|
||||
});
|
||||
});
|
||||
|
||||
60
src/config/config.sandbox-docker.test.ts
Normal file
60
src/config/config.sandbox-docker.test.ts
Normal file
@@ -0,0 +1,60 @@
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
|
||||
describe("sandbox docker config", () => {
|
||||
it("accepts binds array in sandbox.docker config", async () => {
|
||||
vi.resetModules();
|
||||
const { validateConfigObject } = await import("./config.js");
|
||||
const res = validateConfigObject({
|
||||
agents: {
|
||||
defaults: {
|
||||
sandbox: {
|
||||
docker: {
|
||||
binds: [
|
||||
"/var/run/docker.sock:/var/run/docker.sock",
|
||||
"/home/user/source:/source:rw",
|
||||
],
|
||||
},
|
||||
},
|
||||
},
|
||||
list: [
|
||||
{
|
||||
id: "main",
|
||||
sandbox: {
|
||||
docker: {
|
||||
image: "custom-sandbox:latest",
|
||||
binds: ["/home/user/projects:/projects:ro"],
|
||||
},
|
||||
},
|
||||
},
|
||||
],
|
||||
},
|
||||
});
|
||||
expect(res.ok).toBe(true);
|
||||
if (res.ok) {
|
||||
expect(res.config.agents?.defaults?.sandbox?.docker?.binds).toEqual([
|
||||
"/var/run/docker.sock:/var/run/docker.sock",
|
||||
"/home/user/source:/source:rw",
|
||||
]);
|
||||
expect(res.config.agents?.list?.[0]?.sandbox?.docker?.binds).toEqual([
|
||||
"/home/user/projects:/projects:ro",
|
||||
]);
|
||||
}
|
||||
});
|
||||
|
||||
it("rejects non-string values in binds array", async () => {
|
||||
vi.resetModules();
|
||||
const { validateConfigObject } = await import("./config.js");
|
||||
const res = validateConfigObject({
|
||||
agents: {
|
||||
defaults: {
|
||||
sandbox: {
|
||||
docker: {
|
||||
binds: [123, "/valid/path:/path"],
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
});
|
||||
expect(res.ok).toBe(false);
|
||||
});
|
||||
});
|
||||
@@ -77,6 +77,7 @@ export const SandboxDockerSchema = z
|
||||
apparmorProfile: z.string().optional(),
|
||||
dns: z.array(z.string()).optional(),
|
||||
extraHosts: z.array(z.string()).optional(),
|
||||
binds: z.array(z.string()).optional(),
|
||||
})
|
||||
.optional();
|
||||
|
||||
|
||||
Reference in New Issue
Block a user