fix: execute sandboxed file ops inside containers (#4026)

Merged via /review-pr -> /prepare-pr -> /merge-pr.

Prepared head SHA: 795ec6aa2f
Co-authored-by: davidbors-snyk <240482518+davidbors-snyk@users.noreply.github.com>
Co-authored-by: steipete <58493+steipete@users.noreply.github.com>
Reviewed-by: @steipete

src/agents/sandbox/context.ts

@@ -9,6 +9,7 @@ import { DEFAULT_AGENT_WORKSPACE_DIR } from "../workspace.js";
 import { ensureSandboxBrowser } from "./browser.js";
 import { resolveSandboxConfigForAgent } from "./config.js";
 import { ensureSandboxContainer } from "./docker.js";
+import { createSandboxFsBridge } from "./fs-bridge.js";
 import { maybePruneSandboxes } from "./prune.js";
 import { resolveSandboxRuntimeStatus } from "./runtime-status.js";
 import { resolveSandboxScopeKey, resolveSandboxWorkspaceDir } from "./shared.js";
@@ -83,7 +84,7 @@ export async function resolveSandboxContext(params: {
     evaluateEnabled,
   });
 
-  return {
+  const sandboxContext: SandboxContext = {
     enabled: true,
     sessionKey: rawSessionKey,
     workspaceDir,
@@ -96,6 +97,10 @@ export async function resolveSandboxContext(params: {
     browserAllowHostControl: cfg.browser.allowHostControl,
     browser: browser ?? undefined,
   };
+
+  sandboxContext.fsBridge = createSandboxFsBridge({ sandbox: sandboxContext });
+
+  return sandboxContext;
 }
 
 export async function ensureSandboxWorkspaceForSession(params: {

src/agents/sandbox/docker.ts

@@ -1,5 +1,109 @@
 import { spawn } from "node:child_process";
-import type { SandboxConfig, SandboxDockerConfig, SandboxWorkspaceAccess } from "./types.js";
+
+type ExecDockerRawOptions = {
+  allowFailure?: boolean;
+  input?: Buffer | string;
+  signal?: AbortSignal;
+};
+
+export type ExecDockerRawResult = {
+  stdout: Buffer;
+  stderr: Buffer;
+  code: number;
+};
+
+type ExecDockerRawError = Error & {
+  code: number;
+  stdout: Buffer;
+  stderr: Buffer;
+};
+
+function createAbortError(): Error {
+  const err = new Error("Aborted");
+  err.name = "AbortError";
+  return err;
+}
+
+export function execDockerRaw(
+  args: string[],
+  opts?: ExecDockerRawOptions,
+): Promise<ExecDockerRawResult> {
+  return new Promise<ExecDockerRawResult>((resolve, reject) => {
+    const child = spawn("docker", args, {
+      stdio: ["pipe", "pipe", "pipe"],
+    });
+    const stdoutChunks: Buffer[] = [];
+    const stderrChunks: Buffer[] = [];
+    let aborted = false;
+
+    const signal = opts?.signal;
+    const handleAbort = () => {
+      if (aborted) {
+        return;
+      }
+      aborted = true;
+      child.kill("SIGTERM");
+    };
+    if (signal) {
+      if (signal.aborted) {
+        handleAbort();
+      } else {
+        signal.addEventListener("abort", handleAbort);
+      }
+    }
+
+    child.stdout?.on("data", (chunk) => {
+      stdoutChunks.push(Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk));
+    });
+    child.stderr?.on("data", (chunk) => {
+      stderrChunks.push(Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk));
+    });
+
+    child.on("error", (error) => {
+      if (signal) {
+        signal.removeEventListener("abort", handleAbort);
+      }
+      reject(error);
+    });
+
+    child.on("close", (code) => {
+      if (signal) {
+        signal.removeEventListener("abort", handleAbort);
+      }
+      const stdout = Buffer.concat(stdoutChunks);
+      const stderr = Buffer.concat(stderrChunks);
+      if (aborted || signal?.aborted) {
+        reject(createAbortError());
+        return;
+      }
+      const exitCode = code ?? 0;
+      if (exitCode !== 0 && !opts?.allowFailure) {
+        const message = stderr.length > 0 ? stderr.toString("utf8").trim() : "";
+        const error: ExecDockerRawError = Object.assign(
+          new Error(message || `docker ${args.join(" ")} failed`),
+          {
+            code: exitCode,
+            stdout,
+            stderr,
+          },
+        );
+        reject(error);
+        return;
+      }
+      resolve({ stdout, stderr, code: exitCode });
+    });
+
+    const stdin = child.stdin;
+    if (stdin) {
+      if (opts?.input !== undefined) {
+        stdin.end(opts.input);
+      } else {
+        stdin.end();
+      }
+    }
+  });
+}
+
 import { formatCliCommand } from "../../cli/command-format.js";
 import { defaultRuntime } from "../../runtime.js";
 import { computeSandboxConfigHash } from "./config-hash.js";
@@ -9,28 +113,15 @@ import { resolveSandboxAgentId, resolveSandboxScopeKey, slugifySessionKey } from
 
 const HOT_CONTAINER_WINDOW_MS = 5 * 60 * 1000;
 
-export function execDocker(args: string[], opts?: { allowFailure?: boolean }) {
-  return new Promise<{ stdout: string; stderr: string; code: number }>((resolve, reject) => {
-    const child = spawn("docker", args, {
-      stdio: ["ignore", "pipe", "pipe"],
-    });
-    let stdout = "";
-    let stderr = "";
-    child.stdout?.on("data", (chunk) => {
-      stdout += chunk.toString();
-    });
-    child.stderr?.on("data", (chunk) => {
-      stderr += chunk.toString();
-    });
-    child.on("close", (code) => {
-      const exitCode = code ?? 0;
-      if (exitCode !== 0 && !opts?.allowFailure) {
-        reject(new Error(stderr.trim() || `docker ${args.join(" ")} failed`));
-        return;
-      }
-      resolve({ stdout, stderr, code: exitCode });
-    });
-  });
+export type ExecDockerOptions = ExecDockerRawOptions;
+
+export async function execDocker(args: string[], opts?: ExecDockerOptions) {
+  const result = await execDockerRaw(args, opts);
+  return {
+    stdout: result.stdout.toString("utf8"),
+    stderr: result.stderr.toString("utf8"),
+    code: result.code,
+  };
 }
 
 export async function readDockerPort(containerName: string, port: number) {
@@ -195,9 +286,7 @@ export function buildSandboxCreateArgs(params: {
   if (typeof params.cfg.cpus === "number" && params.cfg.cpus > 0) {
     args.push("--cpus", String(params.cfg.cpus));
   }
-  for (const [name, value] of Object.entries(params.cfg.ulimits ?? {}) as Array<
-    [string, string | number | { soft?: number; hard?: number }]
-  >) {
+  for (const [name, value] of Object.entries(params.cfg.ulimits ?? {})) {
     const formatted = formatUlimitValue(name, value);
     if (formatted) {
       args.push("--ulimit", formatted);

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

@@ -0,0 +1,88 @@
+import { beforeEach, describe, expect, it, vi } from "vitest";
+
+vi.mock("./docker.js", () => ({
+  execDockerRaw: vi.fn(),
+}));
+
+import type { SandboxContext } from "./types.js";
+import { execDockerRaw } from "./docker.js";
+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,
+};
+
+describe("sandbox fs bridge shell compatibility", () => {
+  beforeEach(() => {
+    mockedExecDockerRaw.mockReset();
+    mockedExecDockerRaw.mockImplementation(async (args) => {
+      const script = args[5] ?? "";
+      if (script.includes('stat -c "%F|%s|%Y"')) {
+        return {
+          stdout: Buffer.from("regular file|1|2"),
+          stderr: Buffer.alloc(0),
+          code: 0,
+        };
+      }
+      if (script.includes('cat -- "$1"')) {
+        return {
+          stdout: Buffer.from("content"),
+          stderr: Buffer.alloc(0),
+          code: 0,
+        };
+      }
+      return {
+        stdout: Buffer.alloc(0),
+        stderr: Buffer.alloc(0),
+        code: 0,
+      };
+    });
+  });
+
+  it("uses POSIX-safe shell prologue in all bridge commands", async () => {
+    const bridge = createSandboxFsBridge({ sandbox });
+
+    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" });
+
+    expect(mockedExecDockerRaw).toHaveBeenCalled();
+
+    const scripts = mockedExecDockerRaw.mock.calls.map(([args]) => args[5] ?? "");
+    const executables = mockedExecDockerRaw.mock.calls.map(([args]) => args[3] ?? "");
+
+    expect(executables.every((shell) => shell === "sh")).toBe(true);
+    expect(scripts.every((script) => script.includes("set -eu;"))).toBe(true);
+    expect(scripts.some((script) => script.includes("pipefail"))).toBe(false);
+  });
+});

src/agents/sandbox/fs-bridge.ts

@@ -0,0 +1,257 @@
+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";
+
+type RunCommandOptions = {
+  args?: string[];
+  stdin?: Buffer | string;
+  allowFailure?: boolean;
+  signal?: AbortSignal;
+};
+
+export type SandboxResolvedPath = {
+  hostPath: string;
+  relativePath: string;
+  containerPath: string;
+};
+
+export type SandboxFsStat = {
+  type: "file" | "directory" | "other";
+  size: number;
+  mtimeMs: number;
+};
+
+export type SandboxFsBridge = {
+  resolvePath(params: { filePath: string; cwd?: string }): SandboxResolvedPath;
+  readFile(params: { filePath: string; cwd?: string; signal?: AbortSignal }): Promise<Buffer>;
+  writeFile(params: {
+    filePath: string;
+    cwd?: string;
+    data: Buffer | string;
+    encoding?: BufferEncoding;
+    mkdir?: boolean;
+    signal?: AbortSignal;
+  }): Promise<void>;
+  mkdirp(params: { filePath: string; cwd?: string; signal?: AbortSignal }): Promise<void>;
+  remove(params: {
+    filePath: string;
+    cwd?: string;
+    recursive?: boolean;
+    force?: boolean;
+    signal?: AbortSignal;
+  }): Promise<void>;
+  rename(params: { from: string; to: string; cwd?: string; signal?: AbortSignal }): Promise<void>;
+  stat(params: {
+    filePath: string;
+    cwd?: string;
+    signal?: AbortSignal;
+  }): Promise<SandboxFsStat | null>;
+};
+
+export function createSandboxFsBridge(params: { sandbox: SandboxContext }): SandboxFsBridge {
+  return new SandboxFsBridgeImpl(params.sandbox);
+}
+
+class SandboxFsBridgeImpl implements SandboxFsBridge {
+  private readonly sandbox: SandboxContext;
+
+  constructor(sandbox: SandboxContext) {
+    this.sandbox = sandbox;
+  }
+
+  resolvePath(params: { filePath: string; cwd?: string }): SandboxResolvedPath {
+    return resolveSandboxFsPath({
+      sandbox: this.sandbox,
+      filePath: params.filePath,
+      cwd: params.cwd,
+    });
+  }
+
+  async readFile(params: {
+    filePath: string;
+    cwd?: string;
+    signal?: AbortSignal;
+  }): Promise<Buffer> {
+    const target = this.resolvePath(params);
+    const result = await this.runCommand('set -eu; cat -- "$1"', {
+      args: [target.containerPath],
+      signal: params.signal,
+    });
+    return result.stdout;
+  }
+
+  async writeFile(params: {
+    filePath: string;
+    cwd?: string;
+    data: Buffer | string;
+    encoding?: BufferEncoding;
+    mkdir?: boolean;
+    signal?: AbortSignal;
+  }): Promise<void> {
+    this.ensureWriteAccess("write files");
+    const target = this.resolvePath(params);
+    const buffer = Buffer.isBuffer(params.data)
+      ? params.data
+      : Buffer.from(params.data, params.encoding ?? "utf8");
+    const script =
+      params.mkdir === false
+        ? 'set -eu; cat >"$1"'
+        : 'set -eu; dir=$(dirname -- "$1"); if [ "$dir" != "." ]; then mkdir -p -- "$dir"; fi; cat >"$1"';
+    await this.runCommand(script, {
+      args: [target.containerPath],
+      stdin: buffer,
+      signal: params.signal,
+    });
+  }
+
+  async mkdirp(params: { filePath: string; cwd?: string; signal?: AbortSignal }): Promise<void> {
+    this.ensureWriteAccess("create directories");
+    const target = this.resolvePath(params);
+    await this.runCommand('set -eu; mkdir -p -- "$1"', {
+      args: [target.containerPath],
+      signal: params.signal,
+    });
+  }
+
+  async remove(params: {
+    filePath: string;
+    cwd?: string;
+    recursive?: boolean;
+    force?: boolean;
+    signal?: AbortSignal;
+  }): Promise<void> {
+    this.ensureWriteAccess("remove files");
+    const target = this.resolvePath(params);
+    const flags = [params.force === false ? "" : "-f", params.recursive ? "-r" : ""].filter(
+      Boolean,
+    );
+    const rmCommand = flags.length > 0 ? `rm ${flags.join(" ")}` : "rm";
+    await this.runCommand(`set -eu; ${rmCommand} -- "$1"`, {
+      args: [target.containerPath],
+      signal: params.signal,
+    });
+  }
+
+  async rename(params: {
+    from: string;
+    to: string;
+    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 });
+    await this.runCommand(
+      'set -eu; dir=$(dirname -- "$2"); if [ "$dir" != "." ]; then mkdir -p -- "$dir"; fi; mv -- "$1" "$2"',
+      {
+        args: [from.containerPath, to.containerPath],
+        signal: params.signal,
+      },
+    );
+  }
+
+  async stat(params: {
+    filePath: string;
+    cwd?: string;
+    signal?: AbortSignal;
+  }): Promise<SandboxFsStat | null> {
+    const target = this.resolvePath(params);
+    const result = await this.runCommand('set -eu; stat -c "%F|%s|%Y" -- "$1"', {
+      args: [target.containerPath],
+      signal: params.signal,
+      allowFailure: true,
+    });
+    if (result.code !== 0) {
+      const stderr = result.stderr.toString("utf8");
+      if (stderr.includes("No such file or directory")) {
+        return null;
+      }
+      const message = stderr.trim() || `stat failed with code ${result.code}`;
+      throw new Error(`stat failed for ${target.containerPath}: ${message}`);
+    }
+    const text = result.stdout.toString("utf8").trim();
+    const [typeRaw, sizeRaw, mtimeRaw] = text.split("|");
+    const size = Number.parseInt(sizeRaw ?? "0", 10);
+    const mtime = Number.parseInt(mtimeRaw ?? "0", 10) * 1000;
+    return {
+      type: coerceStatType(typeRaw),
+      size: Number.isFinite(size) ? size : 0,
+      mtimeMs: Number.isFinite(mtime) ? mtime : 0,
+    };
+  }
+
+  private async runCommand(
+    script: string,
+    options: RunCommandOptions = {},
+  ): Promise<ExecDockerRawResult> {
+    const dockerArgs = [
+      "exec",
+      "-i",
+      this.sandbox.containerName,
+      "sh",
+      "-c",
+      script,
+      "moltbot-sandbox-fs",
+    ];
+    if (options.args?.length) {
+      dockerArgs.push(...options.args);
+    }
+    return execDockerRaw(dockerArgs, {
+      input: options.stdin,
+      allowFailure: options.allowFailure,
+      signal: options.signal,
+    });
+  }
+
+  private ensureWriteAccess(action: string) {
+    if (!allowsWrites(this.sandbox.workspaceAccess)) {
+      throw new Error(
+        `Sandbox workspace (${this.sandbox.workspaceAccess}) does not allow ${action}.`,
+      );
+    }
+  }
+}
+
+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";
+  }
+  const normalized = typeRaw.trim().toLowerCase();
+  if (normalized.includes("directory")) {
+    return "directory";
+  }
+  if (normalized.includes("file")) {
+    return "file";
+  }
+  return "other";
+}

src/agents/sandbox/types.ts

@@ -1,3 +1,4 @@
+import type { SandboxFsBridge } from "./fs-bridge.js";
 import type { SandboxDockerConfig } from "./types.docker.js";
 
 export type { SandboxDockerConfig } from "./types.docker.js";
@@ -77,6 +78,7 @@ export type SandboxContext = {
   tools: SandboxToolPolicy;
   browserAllowHostControl: boolean;
   browser?: SandboxBrowserContext;
+  fsBridge?: SandboxFsBridge;
 };
 
 export type SandboxWorkspaceInfo = {