Sandbox: honor bind mounts in file tools

src/agents/pi-tools.read.ts

@@ -269,23 +269,6 @@ export function wrapToolWorkspaceRootGuard(tool: AnyAgentTool, root: string): An
   };
 }
 
-function wrapSandboxPathGuard(tool: AnyAgentTool, root: string): AnyAgentTool {
-  return {
-    ...tool,
-    execute: async (toolCallId, args, signal, onUpdate) => {
-      const normalized = normalizeToolParams(args);
-      const record =
-        normalized ??
-        (args && typeof args === "object" ? (args as Record<string, unknown>) : undefined);
-      const filePath = record?.path;
-      if (typeof filePath === "string" && filePath.trim()) {
-        await assertSandboxPath({ filePath, cwd: root, root });
-      }
-      return tool.execute(toolCallId, normalized ?? args, signal, onUpdate);
-    },
-  };
-}
-
 type SandboxToolParams = {
   root: string;
   bridge: SandboxFsBridge;
@@ -295,27 +278,21 @@ export function createSandboxedReadTool(params: SandboxToolParams) {
   const base = createReadTool(params.root, {
     operations: createSandboxReadOperations(params),
   }) as unknown as AnyAgentTool;
-  return wrapSandboxPathGuard(createOpenClawReadTool(base), params.root);
+  return createOpenClawReadTool(base);
 }
 
 export function createSandboxedWriteTool(params: SandboxToolParams) {
   const base = createWriteTool(params.root, {
     operations: createSandboxWriteOperations(params),
   }) as unknown as AnyAgentTool;
-  return wrapSandboxPathGuard(
-    wrapToolParamNormalization(base, CLAUDE_PARAM_GROUPS.write),
-    params.root,
-  );
+  return wrapToolParamNormalization(base, CLAUDE_PARAM_GROUPS.write);
 }
 
 export function createSandboxedEditTool(params: SandboxToolParams) {
   const base = createEditTool(params.root, {
     operations: createSandboxEditOperations(params),
   }) as unknown as AnyAgentTool;
-  return wrapSandboxPathGuard(
-    wrapToolParamNormalization(base, CLAUDE_PARAM_GROUPS.edit),
-    params.root,
-  );
+  return wrapToolParamNormalization(base, CLAUDE_PARAM_GROUPS.edit);
 }
 
 export function createOpenClawReadTool(base: AnyAgentTool): AnyAgentTool {

src/agents/sandbox/fs-bridge.test.ts

@@ -10,34 +10,37 @@ import { createSandboxFsBridge } from "./fs-bridge.js";
 
 const mockedExecDockerRaw = vi.mocked(execDockerRaw);
 
-const sandbox: SandboxContext = {
-  enabled: true,
-  sessionKey: "sandbox:test",
-  workspaceDir: "/tmp/workspace",
-  agentWorkspaceDir: "/tmp/workspace",
-  workspaceAccess: "rw",
-  containerName: "moltbot-sbx-test",
-  containerWorkdir: "/workspace",
-  docker: {
-    image: "moltbot-sandbox:bookworm-slim",
-    containerPrefix: "moltbot-sbx-",
-    network: "none",
-    user: "1000:1000",
-    workdir: "/workspace",
-    readOnlyRoot: false,
-    tmpfs: [],
-    capDrop: [],
-    seccompProfile: "",
-    apparmorProfile: "",
-    setupCommand: "",
-    binds: [],
-    dns: [],
-    extraHosts: [],
-    pidsLimit: 0,
-  },
-  tools: { allow: ["*"], deny: [] },
-  browserAllowHostControl: false,
-};
+function createSandbox(overrides?: Partial<SandboxContext>): SandboxContext {
+  return {
+    enabled: true,
+    sessionKey: "sandbox:test",
+    workspaceDir: "/tmp/workspace",
+    agentWorkspaceDir: "/tmp/workspace",
+    workspaceAccess: "rw",
+    containerName: "moltbot-sbx-test",
+    containerWorkdir: "/workspace",
+    docker: {
+      image: "moltbot-sandbox:bookworm-slim",
+      containerPrefix: "moltbot-sbx-",
+      network: "none",
+      user: "1000:1000",
+      workdir: "/workspace",
+      readOnlyRoot: false,
+      tmpfs: [],
+      capDrop: [],
+      seccompProfile: "",
+      apparmorProfile: "",
+      setupCommand: "",
+      binds: [],
+      dns: [],
+      extraHosts: [],
+      pidsLimit: 0,
+    },
+    tools: { allow: ["*"], deny: [] },
+    browserAllowHostControl: false,
+    ...overrides,
+  };
+}
 
 describe("sandbox fs bridge shell compatibility", () => {
   beforeEach(() => {
@@ -67,7 +70,7 @@ describe("sandbox fs bridge shell compatibility", () => {
   });
 
   it("uses POSIX-safe shell prologue in all bridge commands", async () => {
-    const bridge = createSandboxFsBridge({ sandbox });
+    const bridge = createSandboxFsBridge({ sandbox: createSandbox() });
 
     await bridge.readFile({ filePath: "a.txt" });
     await bridge.writeFile({ filePath: "b.txt", data: "hello" });
@@ -85,4 +88,37 @@ describe("sandbox fs bridge shell compatibility", () => {
     expect(scripts.every((script) => script.includes("set -eu;"))).toBe(true);
     expect(scripts.some((script) => script.includes("pipefail"))).toBe(false);
   });
+
+  it("resolves bind-mounted absolute container paths for reads", async () => {
+    const sandbox = createSandbox({
+      docker: {
+        ...createSandbox().docker,
+        binds: ["/tmp/workspace-two:/workspace-two:ro"],
+      },
+    });
+    const bridge = createSandboxFsBridge({ sandbox });
+
+    await bridge.readFile({ filePath: "/workspace-two/README.md" });
+
+    const args = mockedExecDockerRaw.mock.calls.at(-1)?.[0] ?? [];
+    expect(args).toEqual(
+      expect.arrayContaining(["moltbot-sbx-test", "sh", "-c", 'set -eu; cat -- "$1"']),
+    );
+    expect(args.at(-1)).toBe("/workspace-two/README.md");
+  });
+
+  it("blocks writes into read-only bind mounts", async () => {
+    const sandbox = createSandbox({
+      docker: {
+        ...createSandbox().docker,
+        binds: ["/tmp/workspace-two:/workspace-two:ro"],
+      },
+    });
+    const bridge = createSandboxFsBridge({ sandbox });
+
+    await expect(
+      bridge.writeFile({ filePath: "/workspace-two/new.txt", data: "hello" }),
+    ).rejects.toThrow(/read-only/);
+    expect(mockedExecDockerRaw).not.toHaveBeenCalled();
+  });
 });

src/agents/sandbox/fs-bridge.ts

@@ -1,7 +1,10 @@
-import path from "node:path";
 import type { SandboxContext, SandboxWorkspaceAccess } from "./types.js";
-import { resolveSandboxPath } from "../sandbox-paths.js";
 import { execDockerRaw, type ExecDockerRawResult } from "./docker.js";
+import {
+  buildSandboxFsMounts,
+  resolveSandboxFsPathWithMounts,
+  type SandboxResolvedFsPath,
+} from "./fs-paths.js";
 
 type RunCommandOptions = {
   args?: string[];
@@ -55,17 +58,20 @@ export function createSandboxFsBridge(params: { sandbox: SandboxContext }): Sand
 
 class SandboxFsBridgeImpl implements SandboxFsBridge {
   private readonly sandbox: SandboxContext;
+  private readonly mounts: ReturnType<typeof buildSandboxFsMounts>;
 
   constructor(sandbox: SandboxContext) {
     this.sandbox = sandbox;
+    this.mounts = buildSandboxFsMounts(sandbox);
   }
 
   resolvePath(params: { filePath: string; cwd?: string }): SandboxResolvedPath {
-    return resolveSandboxFsPath({
-      sandbox: this.sandbox,
-      filePath: params.filePath,
-      cwd: params.cwd,
-    });
+    const target = this.resolveResolvedPath(params);
+    return {
+      hostPath: target.hostPath,
+      relativePath: target.relativePath,
+      containerPath: target.containerPath,
+    };
   }
 
   async readFile(params: {
@@ -73,7 +79,7 @@ class SandboxFsBridgeImpl implements SandboxFsBridge {
     cwd?: string;
     signal?: AbortSignal;
   }): Promise<Buffer> {
-    const target = this.resolvePath(params);
+    const target = this.resolveResolvedPath(params);
     const result = await this.runCommand('set -eu; cat -- "$1"', {
       args: [target.containerPath],
       signal: params.signal,
@@ -89,8 +95,8 @@ class SandboxFsBridgeImpl implements SandboxFsBridge {
     mkdir?: boolean;
     signal?: AbortSignal;
   }): Promise<void> {
-    this.ensureWriteAccess("write files");
-    const target = this.resolvePath(params);
+    const target = this.resolveResolvedPath(params);
+    this.ensureWriteAccess(target, "write files");
     const buffer = Buffer.isBuffer(params.data)
       ? params.data
       : Buffer.from(params.data, params.encoding ?? "utf8");
@@ -106,8 +112,8 @@ class SandboxFsBridgeImpl implements SandboxFsBridge {
   }
 
   async mkdirp(params: { filePath: string; cwd?: string; signal?: AbortSignal }): Promise<void> {
-    this.ensureWriteAccess("create directories");
-    const target = this.resolvePath(params);
+    const target = this.resolveResolvedPath(params);
+    this.ensureWriteAccess(target, "create directories");
     await this.runCommand('set -eu; mkdir -p -- "$1"', {
       args: [target.containerPath],
       signal: params.signal,
@@ -121,8 +127,8 @@ class SandboxFsBridgeImpl implements SandboxFsBridge {
     force?: boolean;
     signal?: AbortSignal;
   }): Promise<void> {
-    this.ensureWriteAccess("remove files");
-    const target = this.resolvePath(params);
+    const target = this.resolveResolvedPath(params);
+    this.ensureWriteAccess(target, "remove files");
     const flags = [params.force === false ? "" : "-f", params.recursive ? "-r" : ""].filter(
       Boolean,
     );
@@ -139,9 +145,10 @@ class SandboxFsBridgeImpl implements SandboxFsBridge {
     cwd?: string;
     signal?: AbortSignal;
   }): Promise<void> {
-    this.ensureWriteAccess("rename files");
-    const from = this.resolvePath({ filePath: params.from, cwd: params.cwd });
-    const to = this.resolvePath({ filePath: params.to, cwd: params.cwd });
+    const from = this.resolveResolvedPath({ filePath: params.from, cwd: params.cwd });
+    const to = this.resolveResolvedPath({ filePath: params.to, cwd: params.cwd });
+    this.ensureWriteAccess(from, "rename files");
+    this.ensureWriteAccess(to, "rename files");
     await this.runCommand(
       'set -eu; dir=$(dirname -- "$2"); if [ "$dir" != "." ]; then mkdir -p -- "$dir"; fi; mv -- "$1" "$2"',
       {
@@ -156,7 +163,7 @@ class SandboxFsBridgeImpl implements SandboxFsBridge {
     cwd?: string;
     signal?: AbortSignal;
   }): Promise<SandboxFsStat | null> {
-    const target = this.resolvePath(params);
+    const target = this.resolveResolvedPath(params);
     const result = await this.runCommand('set -eu; stat -c "%F|%s|%Y" -- "$1"', {
       args: [target.containerPath],
       signal: params.signal,
@@ -204,44 +211,27 @@ class SandboxFsBridgeImpl implements SandboxFsBridge {
     });
   }
 
-  private ensureWriteAccess(action: string) {
-    if (!allowsWrites(this.sandbox.workspaceAccess)) {
-      throw new Error(
-        `Sandbox workspace (${this.sandbox.workspaceAccess}) does not allow ${action}.`,
-      );
+  private ensureWriteAccess(target: SandboxResolvedFsPath, action: string) {
+    if (!allowsWrites(this.sandbox.workspaceAccess) || !target.writable) {
+      throw new Error(`Sandbox path is read-only; cannot ${action}: ${target.containerPath}`);
     }
   }
+
+  private resolveResolvedPath(params: { filePath: string; cwd?: string }): SandboxResolvedFsPath {
+    return resolveSandboxFsPathWithMounts({
+      filePath: params.filePath,
+      cwd: params.cwd ?? this.sandbox.workspaceDir,
+      defaultWorkspaceRoot: this.sandbox.workspaceDir,
+      defaultContainerRoot: this.sandbox.containerWorkdir,
+      mounts: this.mounts,
+    });
+  }
 }
 
 function allowsWrites(access: SandboxWorkspaceAccess): boolean {
   return access === "rw";
 }
 
-function resolveSandboxFsPath(params: {
-  sandbox: SandboxContext;
-  filePath: string;
-  cwd?: string;
-}): SandboxResolvedPath {
-  const root = params.sandbox.workspaceDir;
-  const cwd = params.cwd ?? root;
-  const { resolved, relative } = resolveSandboxPath({
-    filePath: params.filePath,
-    cwd,
-    root,
-  });
-  const normalizedRelative = relative
-    ? relative.split(path.sep).filter(Boolean).join(path.posix.sep)
-    : "";
-  const containerPath = normalizedRelative
-    ? path.posix.join(params.sandbox.containerWorkdir, normalizedRelative)
-    : params.sandbox.containerWorkdir;
-  return {
-    hostPath: resolved,
-    relativePath: normalizedRelative,
-    containerPath,
-  };
-}
-
 function coerceStatType(typeRaw?: string): "file" | "directory" | "other" {
   if (!typeRaw) {
     return "other";