fix(sandbox): harden fs bridge path checks and bind mount policy

src/agents/sandbox/fs-bridge.ts

@@ -1,8 +1,12 @@
+import fs from "node:fs/promises";
+import path from "node:path";
+import { isNotFoundPathError, isPathInside } from "../../infra/path-guards.js";
 import { execDockerRaw, type ExecDockerRawResult } from "./docker.js";
 import {
   buildSandboxFsMounts,
   resolveSandboxFsPathWithMounts,
   type SandboxResolvedFsPath,
+  type SandboxFsMount,
 } from "./fs-paths.js";
 import type { SandboxContext, SandboxWorkspaceAccess } from "./types.js";
 
@@ -13,6 +17,12 @@ type RunCommandOptions = {
   signal?: AbortSignal;
 };
 
+type PathSafetyOptions = {
+  action: string;
+  allowFinalSymlink?: boolean;
+  requireWritable?: boolean;
+};
+
 export type SandboxResolvedPath = {
   hostPath: string;
   relativePath: string;
@@ -59,10 +69,14 @@ export function createSandboxFsBridge(params: { sandbox: SandboxContext }): Sand
 class SandboxFsBridgeImpl implements SandboxFsBridge {
   private readonly sandbox: SandboxContext;
   private readonly mounts: ReturnType<typeof buildSandboxFsMounts>;
+  private readonly mountsByContainer: ReturnType<typeof buildSandboxFsMounts>;
 
   constructor(sandbox: SandboxContext) {
     this.sandbox = sandbox;
     this.mounts = buildSandboxFsMounts(sandbox);
+    this.mountsByContainer = [...this.mounts].toSorted(
+      (a, b) => b.containerRoot.length - a.containerRoot.length,
+    );
   }
 
   resolvePath(params: { filePath: string; cwd?: string }): SandboxResolvedPath {
@@ -80,6 +94,7 @@ class SandboxFsBridgeImpl implements SandboxFsBridge {
     signal?: AbortSignal;
   }): Promise<Buffer> {
     const target = this.resolveResolvedPath(params);
+    await this.assertPathSafety(target, { action: "read files" });
     const result = await this.runCommand('set -eu; cat -- "$1"', {
       args: [target.containerPath],
       signal: params.signal,
@@ -97,6 +112,7 @@ class SandboxFsBridgeImpl implements SandboxFsBridge {
   }): Promise<void> {
     const target = this.resolveResolvedPath(params);
     this.ensureWriteAccess(target, "write files");
+    await this.assertPathSafety(target, { action: "write files", requireWritable: true });
     const buffer = Buffer.isBuffer(params.data)
       ? params.data
       : Buffer.from(params.data, params.encoding ?? "utf8");
@@ -114,6 +130,7 @@ class SandboxFsBridgeImpl implements SandboxFsBridge {
   async mkdirp(params: { filePath: string; cwd?: string; signal?: AbortSignal }): Promise<void> {
     const target = this.resolveResolvedPath(params);
     this.ensureWriteAccess(target, "create directories");
+    await this.assertPathSafety(target, { action: "create directories", requireWritable: true });
     await this.runCommand('set -eu; mkdir -p -- "$1"', {
       args: [target.containerPath],
       signal: params.signal,
@@ -129,6 +146,11 @@ class SandboxFsBridgeImpl implements SandboxFsBridge {
   }): Promise<void> {
     const target = this.resolveResolvedPath(params);
     this.ensureWriteAccess(target, "remove files");
+    await this.assertPathSafety(target, {
+      action: "remove files",
+      requireWritable: true,
+      allowFinalSymlink: true,
+    });
     const flags = [params.force === false ? "" : "-f", params.recursive ? "-r" : ""].filter(
       Boolean,
     );
@@ -149,6 +171,15 @@ class SandboxFsBridgeImpl implements SandboxFsBridge {
     const to = this.resolveResolvedPath({ filePath: params.to, cwd: params.cwd });
     this.ensureWriteAccess(from, "rename files");
     this.ensureWriteAccess(to, "rename files");
+    await this.assertPathSafety(from, {
+      action: "rename files",
+      requireWritable: true,
+      allowFinalSymlink: true,
+    });
+    await this.assertPathSafety(to, {
+      action: "rename files",
+      requireWritable: true,
+    });
     await this.runCommand(
       'set -eu; dir=$(dirname -- "$2"); if [ "$dir" != "." ]; then mkdir -p -- "$dir"; fi; mv -- "$1" "$2"',
       {
@@ -164,6 +195,7 @@ class SandboxFsBridgeImpl implements SandboxFsBridge {
     signal?: AbortSignal;
   }): Promise<SandboxFsStat | null> {
     const target = this.resolveResolvedPath(params);
+    await this.assertPathSafety(target, { action: "stat files" });
     const result = await this.runCommand('set -eu; stat -c "%F|%s|%Y" -- "$1"', {
       args: [target.containerPath],
       signal: params.signal,
@@ -211,6 +243,79 @@ class SandboxFsBridgeImpl implements SandboxFsBridge {
     });
   }
 
+  private 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}`,
+      );
+    }
+
+    await assertNoHostSymlinkEscape({
+      absolutePath: target.hostPath,
+      rootPath: lexicalMount.hostRoot,
+      allowFinalSymlink: options.allowFinalSymlink === true,
+    });
+
+    const canonicalContainerPath = await this.resolveCanonicalContainerPath({
+      containerPath: target.containerPath,
+      allowFinalSymlink: options.allowFinalSymlink === true,
+    });
+    const canonicalMount = this.resolveMountByContainerPath(canonicalContainerPath);
+    if (!canonicalMount) {
+      throw new Error(
+        `Sandbox path escapes allowed mounts; cannot ${options.action}: ${target.containerPath}`,
+      );
+    }
+    if (options.requireWritable && !canonicalMount.writable) {
+      throw new Error(
+        `Sandbox path is read-only; cannot ${options.action}: ${target.containerPath}`,
+      );
+    }
+  }
+
+  private resolveMountByContainerPath(containerPath: string): SandboxFsMount | null {
+    const normalized = normalizeContainerPath(containerPath);
+    for (const mount of this.mountsByContainer) {
+      if (isPathInsidePosix(normalizeContainerPath(mount.containerRoot), normalized)) {
+        return mount;
+      }
+    }
+    return null;
+  }
+
+  private async resolveCanonicalContainerPath(params: {
+    containerPath: string;
+    allowFinalSymlink: boolean;
+  }): Promise<string> {
+    const script = [
+      "set -eu",
+      'target="$1"',
+      'allow_final="$2"',
+      'suffix=""',
+      'probe="$target"',
+      'if [ "$allow_final" = "1" ] && [ -L "$target" ]; then probe=$(dirname -- "$target"); fi',
+      'cursor="$probe"',
+      'while [ ! -e "$cursor" ] && [ ! -L "$cursor" ]; do',
+      '  parent=$(dirname -- "$cursor")',
+      '  if [ "$parent" = "$cursor" ]; then break; fi',
+      '  base=$(basename -- "$cursor")',
+      '  suffix="/$base$suffix"',
+      '  cursor="$parent"',
+      "done",
+      'canonical=$(readlink -f -- "$cursor")',
+      'printf "%s%s\\n" "$canonical" "$suffix"',
+    ].join("; ");
+    const result = await this.runCommand(script, {
+      args: [params.containerPath, params.allowFinalSymlink ? "1" : "0"],
+    });
+    const canonical = result.stdout.toString("utf8").trim();
+    if (!canonical.startsWith("/")) {
+      throw new Error(`Failed to resolve canonical sandbox path: ${params.containerPath}`);
+    }
+    return normalizeContainerPath(canonical);
+  }
+
   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}`);
@@ -245,3 +350,65 @@ function coerceStatType(typeRaw?: string): "file" | "directory" | "other" {
   }
   return "other";
 }
+
+function normalizeContainerPath(value: string): string {
+  const normalized = path.posix.normalize(value);
+  return normalized === "." ? "/" : normalized;
+}
+
+function isPathInsidePosix(root: string, target: string): boolean {
+  if (root === "/") {
+    return true;
+  }
+  return target === root || target.startsWith(`${root}/`);
+}
+
+async function assertNoHostSymlinkEscape(params: {
+  absolutePath: string;
+  rootPath: string;
+  allowFinalSymlink: boolean;
+}): Promise<void> {
+  const root = path.resolve(params.rootPath);
+  const target = path.resolve(params.absolutePath);
+  if (!isPathInside(root, target)) {
+    throw new Error(`Sandbox path escapes mount root (${root}): ${params.absolutePath}`);
+  }
+  const relative = path.relative(root, target);
+  if (!relative) {
+    return;
+  }
+  const rootReal = await tryRealpath(root);
+  const parts = relative.split(path.sep).filter(Boolean);
+  let current = root;
+  for (let idx = 0; idx < parts.length; idx += 1) {
+    current = path.join(current, parts[idx] ?? "");
+    const isLast = idx === parts.length - 1;
+    try {
+      const stat = await fs.lstat(current);
+      if (!stat.isSymbolicLink()) {
+        continue;
+      }
+      if (params.allowFinalSymlink && isLast) {
+        return;
+      }
+      const symlinkTarget = await tryRealpath(current);
+      if (!isPathInside(rootReal, symlinkTarget)) {
+        throw new Error(`Symlink escapes sandbox mount root (${rootReal}): ${current}`);
+      }
+      current = symlinkTarget;
+    } catch (error) {
+      if (isNotFoundPathError(error)) {
+        return;
+      }
+      throw error;
+    }
+  }
+}
+
+async function tryRealpath(value: string): Promise<string> {
+  try {
+    return await fs.realpath(value);
+  } catch {
+    return path.resolve(value);
+  }
+}