From 6f895eb831e20b9a79bd0cb00c0c697e8fe69994 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 22 Feb 2026 20:31:40 +0100 Subject: [PATCH] fix(sandbox): honor explicit bind mounts over workspace defaults Co-authored-by: tasaankaeris --- CHANGELOG.md | 1 + src/agents/sandbox/browser.ts | 6 ++ .../docker.config-hash-recreate.test.ts | 58 ++++++++++++++++++- src/agents/sandbox/docker.ts | 14 ++++- src/agents/sandbox/fs-paths.test.ts | 20 +++++++ src/agents/sandbox/fs-paths.ts | 34 +++++++++-- 6 files changed, 126 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fdd138fc9..95dd2800b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,7 @@ Docs: https://docs.openclaw.ai - Slack/Extension: forward `message read` `threadId` to `readMessages` and use delivery-context `threadId` as outbound `thread_ts` fallback so extension replies/reads stay in the correct Slack thread. (#22216, #22485, #23836) Thanks @vincentkoc, @lan17 and @dorukardahan. - Channels/Group policy: fail closed when `groupPolicy: "allowlist"` is set without explicit `groups`, honor account-level `groupPolicy` overrides, and enforce `groupPolicy: "disabled"` as a hard group block. (#22215) Thanks @etereo. - Sandbox/Media: map container workspace paths (`/workspace/...` and `file:///workspace/...`) back to the host sandbox root for outbound media validation, preventing false deny errors for sandbox-generated local media. (#23083) Thanks @echo931. +- Sandbox/Docker: apply custom bind mounts after workspace mounts and prioritize bind-source resolution on overlapping paths, so explicit workspace binds are no longer ignored. (#22669) Thanks @tasaankaeris. - Config/Memory: allow `"mistral"` in `agents.defaults.memorySearch.provider` and `agents.defaults.memorySearch.fallback` schema validation. (#14934) Thanks @ThomsenDrake. - Security/Feishu: enforce ID-only allowlist matching for DM/group sender authorization, normalize Feishu ID prefixes during checks, and ignore mutable display names so display-name collisions cannot satisfy allowlist entries. This ships in the next npm release. Thanks @jiseoung for reporting. - Feishu/Commands: in group chats, command authorization now falls back to top-level `channels.feishu.allowFrom` when per-group `allowFrom` is not set, so `/command` no longer gets blocked by an unintended empty allowlist. (#23756) diff --git a/src/agents/sandbox/browser.ts b/src/agents/sandbox/browser.ts index e4b16880b..0c49e6323 100644 --- a/src/agents/sandbox/browser.ts +++ b/src/agents/sandbox/browser.ts @@ -227,6 +227,7 @@ export async function ensureSandboxBrowser(params: { "openclaw.browserConfigEpoch": SANDBOX_BROWSER_SECURITY_HASH_EPOCH, }, configHash: expectedHash, + includeBinds: false, }); const mainMountSuffix = params.cfg.workspaceAccess === "ro" && params.workspaceDir === params.agentWorkspaceDir @@ -240,6 +241,11 @@ export async function ensureSandboxBrowser(params: { `${params.agentWorkspaceDir}:${SANDBOX_AGENT_WORKSPACE_MOUNT}${agentMountSuffix}`, ); } + if (browserDockerCfg.binds?.length) { + for (const bind of browserDockerCfg.binds) { + args.push("-v", bind); + } + } args.push("-p", `127.0.0.1::${params.cfg.browser.cdpPort}`); if (noVncEnabled) { args.push("-p", `127.0.0.1::${params.cfg.browser.noVncPort}`); diff --git a/src/agents/sandbox/docker.config-hash-recreate.test.ts b/src/agents/sandbox/docker.config-hash-recreate.test.ts index d8c7778b1..08155c305 100644 --- a/src/agents/sandbox/docker.config-hash-recreate.test.ts +++ b/src/agents/sandbox/docker.config-hash-recreate.test.ts @@ -83,7 +83,7 @@ vi.mock("node:child_process", async (importOriginal) => { }; }); -function createSandboxConfig(dns: string[]): SandboxConfig { +function createSandboxConfig(dns: string[], binds?: string[]): SandboxConfig { return { mode: "all", scope: "shared", @@ -100,7 +100,7 @@ function createSandboxConfig(dns: string[]): SandboxConfig { env: { LANG: "C.UTF-8" }, dns, extraHosts: ["host.docker.internal:host-gateway"], - binds: ["/tmp/workspace:/workspace:rw"], + binds: binds ?? ["/tmp/workspace:/workspace:rw"], }, browser: { enabled: false, @@ -189,4 +189,58 @@ describe("ensureSandboxContainer config-hash recreation", () => { }), ); }); + + it("applies custom binds after workspace mounts so overlapping binds can override", async () => { + const workspaceDir = "/tmp/workspace"; + const cfg = createSandboxConfig( + ["1.1.1.1"], + ["/tmp/workspace-shared/USER.md:/workspace/USER.md:ro"], + ); + const expectedHash = computeSandboxConfigHash({ + docker: cfg.docker, + workspaceAccess: cfg.workspaceAccess, + workspaceDir, + agentWorkspaceDir: workspaceDir, + }); + + spawnState.inspectRunning = false; + spawnState.labelHash = "stale-hash"; + registryMocks.readRegistry.mockResolvedValue({ + entries: [ + { + containerName: "oc-test-shared", + sessionKey: "shared", + createdAtMs: 1, + lastUsedAtMs: 0, + image: cfg.docker.image, + configHash: "stale-hash", + }, + ], + }); + + await ensureSandboxContainer({ + sessionKey: "agent:main:session-1", + workspaceDir, + agentWorkspaceDir: workspaceDir, + cfg, + }); + + const createCall = spawnState.calls.find( + (call) => call.command === "docker" && call.args[0] === "create", + ); + expect(createCall).toBeDefined(); + expect(createCall?.args).toContain(`openclaw.configHash=${expectedHash}`); + + const bindArgs: string[] = []; + const args = createCall?.args ?? []; + for (let i = 0; i < args.length; i += 1) { + if (args[i] === "-v" && typeof args[i + 1] === "string") { + bindArgs.push(args[i + 1]); + } + } + const workspaceMountIdx = bindArgs.indexOf("/tmp/workspace:/workspace"); + const customMountIdx = bindArgs.indexOf("/tmp/workspace-shared/USER.md:/workspace/USER.md:ro"); + expect(workspaceMountIdx).toBeGreaterThanOrEqual(0); + expect(customMountIdx).toBeGreaterThan(workspaceMountIdx); + }); }); diff --git a/src/agents/sandbox/docker.ts b/src/agents/sandbox/docker.ts index 9204b8dd6..6f6769fa3 100644 --- a/src/agents/sandbox/docker.ts +++ b/src/agents/sandbox/docker.ts @@ -263,6 +263,7 @@ export function buildSandboxCreateArgs(params: { createdAtMs?: number; labels?: Record; configHash?: string; + includeBinds?: boolean; }) { // Runtime security validation: blocks dangerous bind mounts, network modes, and profiles. validateSandboxSecurity(params.cfg); @@ -342,7 +343,7 @@ export function buildSandboxCreateArgs(params: { args.push("--ulimit", formatted); } } - if (params.cfg.binds?.length) { + if (params.includeBinds !== false && params.cfg.binds?.length) { for (const bind of params.cfg.binds) { args.push("-v", bind); } @@ -350,6 +351,15 @@ export function buildSandboxCreateArgs(params: { return args; } +function appendCustomBinds(args: string[], cfg: SandboxDockerConfig): void { + if (!cfg.binds?.length) { + return; + } + for (const bind of cfg.binds) { + args.push("-v", bind); + } +} + async function createSandboxContainer(params: { name: string; cfg: SandboxDockerConfig; @@ -367,6 +377,7 @@ async function createSandboxContainer(params: { cfg, scopeKey, configHash: params.configHash, + includeBinds: false, }); args.push("--workdir", cfg.workdir); const mainMountSuffix = @@ -379,6 +390,7 @@ async function createSandboxContainer(params: { `${params.agentWorkspaceDir}:${SANDBOX_AGENT_WORKSPACE_MOUNT}${agentMountSuffix}`, ); } + appendCustomBinds(args, cfg); args.push(cfg.image, "sleep", "infinity"); await execDocker(args); diff --git a/src/agents/sandbox/fs-paths.test.ts b/src/agents/sandbox/fs-paths.test.ts index 52261863a..c6d223236 100644 --- a/src/agents/sandbox/fs-paths.test.ts +++ b/src/agents/sandbox/fs-paths.test.ts @@ -102,4 +102,24 @@ describe("resolveSandboxFsPathWithMounts", () => { }), ).toThrow(/Path escapes sandbox root/); }); + + it("prefers custom bind mounts over default workspace mount at /workspace", () => { + const sandbox = createSandbox({ + docker: { + ...createSandbox().docker, + binds: ["/tmp/override:/workspace:ro"], + }, + }); + const mounts = buildSandboxFsMounts(sandbox); + const resolved = resolveSandboxFsPathWithMounts({ + filePath: "/workspace/docs/AGENTS.md", + cwd: sandbox.workspaceDir, + defaultWorkspaceRoot: sandbox.workspaceDir, + defaultContainerRoot: sandbox.containerWorkdir, + mounts, + }); + + expect(resolved.hostPath).toBe(path.join(path.resolve("/tmp/override"), "docs", "AGENTS.md")); + expect(resolved.writable).toBe(false); + }); }); diff --git a/src/agents/sandbox/fs-paths.ts b/src/agents/sandbox/fs-paths.ts index 018fcac07..11b5d7120 100644 --- a/src/agents/sandbox/fs-paths.ts +++ b/src/agents/sandbox/fs-paths.ts @@ -134,10 +134,8 @@ export function resolveSandboxFsPathWithMounts(params: { defaultContainerRoot: string; mounts: SandboxFsMount[]; }): SandboxResolvedFsPath { - const mountsByContainer = [...params.mounts].toSorted( - (a, b) => b.containerRoot.length - a.containerRoot.length, - ); - const mountsByHost = [...params.mounts].toSorted((a, b) => b.hostRoot.length - a.hostRoot.length); + const mountsByContainer = [...params.mounts].toSorted(compareMountsByContainerPath); + const mountsByHost = [...params.mounts].toSorted(compareMountsByHostPath); const input = params.filePath; const inputPosix = normalizePosixInput(input); @@ -192,6 +190,34 @@ export function resolveSandboxFsPathWithMounts(params: { throw new Error(`Path escapes sandbox root (${params.defaultWorkspaceRoot}): ${input}`); } +function compareMountsByContainerPath(a: SandboxFsMount, b: SandboxFsMount): number { + const byLength = b.containerRoot.length - a.containerRoot.length; + if (byLength !== 0) { + return byLength; + } + // Keep resolver ordering aligned with docker mount precedence: custom binds can + // intentionally shadow default workspace mounts at the same container path. + return mountSourcePriority(b.source) - mountSourcePriority(a.source); +} + +function compareMountsByHostPath(a: SandboxFsMount, b: SandboxFsMount): number { + const byLength = b.hostRoot.length - a.hostRoot.length; + if (byLength !== 0) { + return byLength; + } + return mountSourcePriority(b.source) - mountSourcePriority(a.source); +} + +function mountSourcePriority(source: SandboxFsMount["source"]): number { + if (source === "bind") { + return 2; + } + if (source === "agent") { + return 1; + } + return 0; +} + function dedupeMounts(mounts: SandboxFsMount[]): SandboxFsMount[] { const seen = new Set(); const deduped: SandboxFsMount[] = [];