fix(sandbox): honor explicit bind mounts over workspace defaults
Co-authored-by: tasaankaeris <tasaankaeris@users.noreply.github.com>
This commit is contained in:
@@ -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)
|
||||
|
||||
@@ -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}`);
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -263,6 +263,7 @@ export function buildSandboxCreateArgs(params: {
|
||||
createdAtMs?: number;
|
||||
labels?: Record<string, string>;
|
||||
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);
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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<string>();
|
||||
const deduped: SandboxFsMount[] = [];
|
||||
|
||||
Reference in New Issue
Block a user