fix(sandbox): pin fs bridge readfile handles

CHANGELOG.md

@@ -742,7 +742,7 @@ Docs: https://docs.openclaw.ai
 - Slack/download-file scoping: thread/channel-aware `download-file` actions now propagate optional scope context and reject downloads when Slack metadata definitively shows the file is outside the requested channel/thread, while preserving legacy behavior when share metadata is unavailable.
 - Security/Sandbox media reads: eliminate sandbox media TOCTOU symlink-retarget escapes by enforcing root-scoped boundary-safe reads at attachment/image load time and consolidating shared safe-read helpers across sandbox media callsites. This ships in the next npm release. Thanks @tdjackey for reporting.
 - Security/Sandbox media staging: block destination symlink escapes in `stageSandboxMedia` by replacing direct destination copies with root-scoped safe writes for both local and SCP-staged attachments, preventing out-of-workspace file overwrite through `media/inbound` alias traversal. This ships in the next npm release (`2026.3.2`). Thanks @tdjackey for reporting.
-- Security/Sandbox fs bridge: harden sandbox `mkdirp`, `remove`, and `rename` operations by anchoring filesystem changes to verified canonical parent directories plus basenames instead of passing mutable full path strings to `mkdir -p`, `rm`, and `mv`, reducing parent-directory symlink-rebind TOCTOU exposure in sandbox file operations. This ships in the next npm release. Thanks @tdjackey for reporting.
+- Security/Sandbox fs bridge: harden sandbox `readFile`, `mkdirp`, `remove`, and `rename` operations by pinning reads to boundary-opened file descriptors and anchoring filesystem changes to verified canonical parent directories plus basenames instead of passing mutable full path strings to `mkdir -p`, `rm`, and `mv`, reducing TOCTOU race exposure in sandbox file operations. This ships in the next npm release. Thanks @tdjackey for reporting.
 - Node host/service auth env: include `OPENCLAW_GATEWAY_TOKEN` in `openclaw node install` service environments (with `CLAWDBOT_GATEWAY_TOKEN` compatibility fallback) so installed node services keep remote gateway token auth across restart/reboot. Fixes #31041. Thanks @OneStepAt4time for reporting, @byungsker, @liuxiaopai-ai, and @vincentkoc.
 - Security/Subagents sandbox inheritance: block sandboxed sessions from spawning cross-agent subagents that would run unsandboxed, preventing runtime sandbox downgrade via `sessions_spawn agentId`. Thanks @tdjackey for reporting.
 - Security/Workspace safe writes: harden `writeFileWithinRoot` against symlink-retarget TOCTOU races by opening existing files without truncation, creating missing files with exclusive create, deferring truncation until post-open identity+boundary validation, and removing out-of-root create artifacts on blocked races; added regression tests for truncate/create race paths. This ships in the next npm release (`2026.3.2`). Thanks @tdjackey for reporting.

src/agents/sandbox/fs-bridge-command-plans.ts

@@ -10,14 +10,6 @@ export type SandboxFsCommandPlan = {
   allowFailure?: boolean;
 };
 
-export function buildReadFilePlan(target: SandboxResolvedFsPath): SandboxFsCommandPlan {
-  return {
-    checks: [{ target, options: { action: "read files" } }],
-    script: 'set -eu; cat -- "$1"',
-    args: [target.containerPath],
-  };
-}
-
 export function buildWriteCommitPlan(
   target: SandboxResolvedFsPath,
   tempPath: string,

src/agents/sandbox/fs-bridge-path-safety.ts

@@ -1,6 +1,6 @@
 import fs from "node:fs";
 import path from "node:path";
-import { openBoundaryFile } from "../../infra/boundary-file-read.js";
+import { openBoundaryFile, type BoundaryFileOpenResult } from "../../infra/boundary-file-read.js";
 import type { PathAliasPolicy } from "../../infra/path-alias-guards.js";
 import type { SafeOpenSyncAllowedType } from "../../infra/safe-open-sync.js";
 import type { SandboxResolvedFsPath, SandboxFsMount } from "./fs-paths.js";
@@ -49,13 +49,40 @@ export class SandboxFsPathGuard {
   }
 
   async assertPathSafety(target: SandboxResolvedFsPath, options: PathSafetyOptions) {
-    const lexicalMount = this.resolveMountByContainerPath(target.containerPath);
-    if (!lexicalMount) {
-      throw new Error(
-        `Sandbox path escapes allowed mounts; cannot ${options.action}: ${target.containerPath}`,
-      );
-    }
+    const lexicalMount = this.resolveRequiredMount(target.containerPath, options.action);
+    await this.assertPathSafetyWithinMount(target, options, lexicalMount);
+  }
 
+  async openReadableFile(
+    target: SandboxResolvedFsPath,
+  ): Promise<BoundaryFileOpenResult & { ok: true }> {
+    const lexicalMount = this.resolveRequiredMount(target.containerPath, "read files");
+    const opened = await openBoundaryFile({
+      absolutePath: target.hostPath,
+      rootPath: lexicalMount.hostRoot,
+      boundaryLabel: "sandbox mount root",
+    });
+    if (!opened.ok) {
+      throw opened.error instanceof Error
+        ? opened.error
+        : new Error(`Sandbox boundary checks failed; cannot read files: ${target.containerPath}`);
+    }
+    return opened;
+  }
+
+  private resolveRequiredMount(containerPath: string, action: string): SandboxFsMount {
+    const lexicalMount = this.resolveMountByContainerPath(containerPath);
+    if (!lexicalMount) {
+      throw new Error(`Sandbox path escapes allowed mounts; cannot ${action}: ${containerPath}`);
+    }
+    return lexicalMount;
+  }
+
+  private async assertPathSafetyWithinMount(
+    target: SandboxResolvedFsPath,
+    options: PathSafetyOptions,
+    lexicalMount: SandboxFsMount,
+  ) {
     const guarded = await openBoundaryFile({
       absolutePath: target.hostPath,
       rootPath: lexicalMount.hostRoot,

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

@@ -7,7 +7,6 @@ import {
   createSandboxFsBridge,
   expectMkdirpAllowsExistingDirectory,
   getScriptsFromCalls,
-  installDockerReadMock,
   installFsBridgeTestHarness,
   mockedExecDockerRaw,
   withTempDir,
@@ -109,11 +108,9 @@ describe("sandbox fs bridge boundary validation", () => {
     });
   });
 
-  it("rejects container-canonicalized paths outside allowed mounts", async () => {
-    installDockerReadMock({ canonicalPath: "/etc/passwd" });
-
+  it("rejects missing files before any docker read command runs", async () => {
     const bridge = createSandboxFsBridge({ sandbox: createSandbox() });
-    await expect(bridge.readFile({ filePath: "a.txt" })).rejects.toThrow(/escapes allowed mounts/i);
+    await expect(bridge.readFile({ filePath: "a.txt" })).rejects.toThrow(/ENOENT|no such file/i);
     const scripts = getScriptsFromCalls();
     expect(scripts.some((script) => script.includes('cat -- "$1"'))).toBe(false);
   });

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

@@ -1,41 +1,54 @@
+import fs from "node:fs/promises";
+import path from "node:path";
 import { describe, expect, it } from "vitest";
 import {
   createSandbox,
   createSandboxFsBridge,
-  findCallByScriptFragment,
-  getDockerPathArg,
   getScriptsFromCalls,
   installFsBridgeTestHarness,
   mockedExecDockerRaw,
+  withTempDir,
 } from "./fs-bridge.test-helpers.js";
 
 describe("sandbox fs bridge shell compatibility", () => {
   installFsBridgeTestHarness();
 
   it("uses POSIX-safe shell prologue in all bridge commands", async () => {
-    const bridge = createSandboxFsBridge({ sandbox: createSandbox() });
+    await withTempDir("openclaw-fs-bridge-shell-", async (stateDir) => {
+      const workspaceDir = path.join(stateDir, "workspace");
+      await fs.mkdir(workspaceDir, { recursive: true });
+      await fs.writeFile(path.join(workspaceDir, "a.txt"), "hello");
+      await fs.writeFile(path.join(workspaceDir, "b.txt"), "bye");
 
-    await bridge.readFile({ filePath: "a.txt" });
-    await bridge.writeFile({ filePath: "b.txt", data: "hello" });
-    await bridge.mkdirp({ filePath: "nested" });
-    await bridge.remove({ filePath: "b.txt" });
-    await bridge.rename({ from: "a.txt", to: "c.txt" });
-    await bridge.stat({ filePath: "c.txt" });
+      const bridge = createSandboxFsBridge({
+        sandbox: createSandbox({
+          workspaceDir,
+          agentWorkspaceDir: workspaceDir,
+        }),
+      });
 
-    expect(mockedExecDockerRaw).toHaveBeenCalled();
+      await bridge.readFile({ filePath: "a.txt" });
+      await bridge.writeFile({ filePath: "b.txt", data: "hello" });
+      await bridge.mkdirp({ filePath: "nested" });
+      await bridge.remove({ filePath: "b.txt" });
+      await bridge.rename({ from: "a.txt", to: "c.txt" });
+      await bridge.stat({ filePath: "c.txt" });
 
-    const scripts = getScriptsFromCalls();
-    const executables = mockedExecDockerRaw.mock.calls.map(([args]) => args[3] ?? "");
+      expect(mockedExecDockerRaw).toHaveBeenCalled();
 
-    expect(executables.every((shell) => shell === "sh")).toBe(true);
-    expect(scripts.every((script) => /set -eu[;\n]/.test(script))).toBe(true);
-    expect(scripts.some((script) => script.includes("pipefail"))).toBe(false);
+      const scripts = getScriptsFromCalls();
+      const executables = mockedExecDockerRaw.mock.calls.map(([args]) => args[3] ?? "");
+
+      expect(executables.every((shell) => shell === "sh")).toBe(true);
+      expect(scripts.every((script) => /set -eu[;\n]/.test(script))).toBe(true);
+      expect(scripts.some((script) => script.includes("pipefail"))).toBe(false);
+    });
   });
 
   it("resolveCanonicalContainerPath script is valid POSIX sh (no do; token)", async () => {
     const bridge = createSandboxFsBridge({ sandbox: createSandbox() });
 
-    await bridge.readFile({ filePath: "a.txt" });
+    await bridge.mkdirp({ filePath: "nested" });
 
     const scripts = getScriptsFromCalls();
     const canonicalScript = scripts.find((script) => script.includes("allow_final"));
@@ -45,44 +58,69 @@ describe("sandbox fs bridge shell compatibility", () => {
   });
 
   it("reads inbound media-style filenames with triple-dash ids", async () => {
-    const bridge = createSandboxFsBridge({ sandbox: createSandbox() });
-    const inboundPath = "media/inbound/file_1095---f00a04a2-99a0-4d98-99b0-dfe61c5a4198.ogg";
+    await withTempDir("openclaw-fs-bridge-read-", async (stateDir) => {
+      const workspaceDir = path.join(stateDir, "workspace");
+      const inboundPath = "media/inbound/file_1095---f00a04a2-99a0-4d98-99b0-dfe61c5a4198.ogg";
+      await fs.mkdir(path.join(workspaceDir, "media", "inbound"), { recursive: true });
+      await fs.writeFile(path.join(workspaceDir, inboundPath), "voice");
 
-    await bridge.readFile({ filePath: inboundPath });
+      const bridge = createSandboxFsBridge({
+        sandbox: createSandbox({
+          workspaceDir,
+          agentWorkspaceDir: workspaceDir,
+        }),
+      });
 
-    const readCall = findCallByScriptFragment('cat -- "$1"');
-    expect(readCall).toBeDefined();
-    const readPath = readCall ? getDockerPathArg(readCall[0]) : "";
-    expect(readPath).toContain("file_1095---");
+      await expect(bridge.readFile({ filePath: inboundPath })).resolves.toEqual(
+        Buffer.from("voice"),
+      );
+      expect(mockedExecDockerRaw).not.toHaveBeenCalled();
+    });
   });
 
   it("resolves dash-leading basenames into absolute container paths", async () => {
-    const bridge = createSandboxFsBridge({ sandbox: createSandbox() });
+    await withTempDir("openclaw-fs-bridge-read-", async (stateDir) => {
+      const workspaceDir = path.join(stateDir, "workspace");
+      await fs.mkdir(workspaceDir, { recursive: true });
+      await fs.writeFile(path.join(workspaceDir, "--leading.txt"), "dash");
 
-    await bridge.readFile({ filePath: "--leading.txt" });
+      const bridge = createSandboxFsBridge({
+        sandbox: createSandbox({
+          workspaceDir,
+          agentWorkspaceDir: workspaceDir,
+        }),
+      });
 
-    const readCall = findCallByScriptFragment('cat -- "$1"');
-    expect(readCall).toBeDefined();
-    const readPath = readCall ? getDockerPathArg(readCall[0]) : "";
-    expect(readPath).toBe("/workspace/--leading.txt");
+      await expect(bridge.readFile({ filePath: "--leading.txt" })).resolves.toEqual(
+        Buffer.from("dash"),
+      );
+      expect(mockedExecDockerRaw).not.toHaveBeenCalled();
+    });
   });
 
   it("resolves bind-mounted absolute container paths for reads", async () => {
-    const sandbox = createSandbox({
-      docker: {
-        ...createSandbox().docker,
-        binds: ["/tmp/workspace-two:/workspace-two:ro"],
-      },
+    await withTempDir("openclaw-fs-bridge-bind-read-", async (stateDir) => {
+      const workspaceDir = path.join(stateDir, "workspace");
+      const bindRoot = path.join(stateDir, "workspace-two");
+      await fs.mkdir(workspaceDir, { recursive: true });
+      await fs.mkdir(bindRoot, { recursive: true });
+      await fs.writeFile(path.join(bindRoot, "README.md"), "bind-read");
+
+      const sandbox = createSandbox({
+        workspaceDir,
+        agentWorkspaceDir: workspaceDir,
+        docker: {
+          ...createSandbox().docker,
+          binds: [`${bindRoot}:/workspace-two:ro`],
+        },
+      });
+      const bridge = createSandboxFsBridge({ sandbox });
+
+      await expect(bridge.readFile({ filePath: "/workspace-two/README.md" })).resolves.toEqual(
+        Buffer.from("bind-read"),
+      );
+      expect(mockedExecDockerRaw).not.toHaveBeenCalled();
     });
-    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(getDockerPathArg(args)).toBe("/workspace-two/README.md");
   });
 
   it("writes via temp file + atomic rename (never direct truncation)", async () => {

src/agents/sandbox/fs-bridge.ts

@@ -1,7 +1,7 @@
+import fs from "node:fs";
 import { execDockerRaw, type ExecDockerRawResult } from "./docker.js";
 import {
   buildMkdirpPlan,
-  buildReadFilePlan,
   buildRemovePlan,
   buildRenamePlan,
   buildStatPlan,
@@ -99,8 +99,12 @@ class SandboxFsBridgeImpl implements SandboxFsBridge {
     signal?: AbortSignal;
   }): Promise<Buffer> {
     const target = this.resolveResolvedPath(params);
-    const result = await this.runPlannedCommand(buildReadFilePlan(target), params.signal);
-    return result.stdout;
+    const opened = await this.pathGuard.openReadableFile(target);
+    try {
+      return fs.readFileSync(opened.fd);
+    } finally {
+      fs.closeSync(opened.fd);
+    }
   }
 
   async writeFile(params: {