refactor(sandbox): share bind parsing and host-path policy checks

src/agents/sandbox/bind-spec.test.ts

@@ -0,0 +1,29 @@
+import { describe, expect, it } from "vitest";
+import { splitSandboxBindSpec } from "./bind-spec.js";
+
+describe("splitSandboxBindSpec", () => {
+  it("splits POSIX bind specs with and without mode", () => {
+    expect(splitSandboxBindSpec("/tmp/a:/workspace-a:ro")).toEqual({
+      host: "/tmp/a",
+      container: "/workspace-a",
+      options: "ro",
+    });
+    expect(splitSandboxBindSpec("/tmp/b:/workspace-b")).toEqual({
+      host: "/tmp/b",
+      container: "/workspace-b",
+      options: "",
+    });
+  });
+
+  it("preserves Windows drive-letter host paths", () => {
+    expect(splitSandboxBindSpec("C:\\Users\\kai\\workspace:/workspace:ro")).toEqual({
+      host: "C:\\Users\\kai\\workspace",
+      container: "/workspace",
+      options: "ro",
+    });
+  });
+
+  it("returns null when no host/container separator exists", () => {
+    expect(splitSandboxBindSpec("/tmp/no-separator")).toBeNull();
+  });
+});

src/agents/sandbox/bind-spec.ts

@@ -0,0 +1,34 @@
+type SplitBindSpec = {
+  host: string;
+  container: string;
+  options: string;
+};
+
+export function splitSandboxBindSpec(spec: string): SplitBindSpec | null {
+  const separator = getHostContainerSeparatorIndex(spec);
+  if (separator === -1) {
+    return null;
+  }
+
+  const host = spec.slice(0, separator);
+  const rest = spec.slice(separator + 1);
+  const optionsStart = rest.indexOf(":");
+  if (optionsStart === -1) {
+    return { host, container: rest, options: "" };
+  }
+  return {
+    host,
+    container: rest.slice(0, optionsStart),
+    options: rest.slice(optionsStart + 1),
+  };
+}
+
+function getHostContainerSeparatorIndex(spec: string): number {
+  const hasDriveLetterPrefix = /^[A-Za-z]:[\\/]/.test(spec);
+  for (let i = hasDriveLetterPrefix ? 2 : 0; i < spec.length; i += 1) {
+    if (spec[i] === ":") {
+      return i;
+    }
+  }
+  return -1;
+}

src/agents/sandbox/fs-paths.ts

@@ -1,6 +1,8 @@
 import path from "node:path";
 import { resolveSandboxInputPath, resolveSandboxPath } from "../sandbox-paths.js";
+import { splitSandboxBindSpec } from "./bind-spec.js";
 import { SANDBOX_AGENT_WORKSPACE_MOUNT } from "./constants.js";
+import { resolveSandboxHostPathViaExistingAncestor } from "./host-paths.js";
 import type { SandboxContext } from "./types.js";
 
 export type SandboxFsMount = {
@@ -23,19 +25,13 @@ type ParsedBindMount = {
   writable: boolean;
 };
 
-type SplitBindSpec = {
-  host: string;
-  container: string;
-  options: string;
-};
-
 export function parseSandboxBindMount(spec: string): ParsedBindMount | null {
   const trimmed = spec.trim();
   if (!trimmed) {
     return null;
   }
 
-  const parsed = splitBindSpec(trimmed);
+  const parsed = splitSandboxBindSpec(trimmed);
   if (!parsed) {
     return null;
   }
@@ -60,35 +56,6 @@ export function parseSandboxBindMount(spec: string): ParsedBindMount | null {
   };
 }
 
-function splitBindSpec(spec: string): SplitBindSpec | null {
-  const separator = getHostContainerSeparatorIndex(spec);
-  if (separator === -1) {
-    return null;
-  }
-
-  const host = spec.slice(0, separator);
-  const rest = spec.slice(separator + 1);
-  const optionsStart = rest.indexOf(":");
-  if (optionsStart === -1) {
-    return { host, container: rest, options: "" };
-  }
-  return {
-    host,
-    container: rest.slice(0, optionsStart),
-    options: rest.slice(optionsStart + 1),
-  };
-}
-
-function getHostContainerSeparatorIndex(spec: string): number {
-  const hasDriveLetterPrefix = /^[A-Za-z]:[\\/]/.test(spec);
-  for (let i = hasDriveLetterPrefix ? 2 : 0; i < spec.length; i += 1) {
-    if (spec[i] === ":") {
-      return i;
-    }
-  }
-  return -1;
-}
-
 export function buildSandboxFsMounts(sandbox: SandboxContext): SandboxFsMount[] {
   const mounts: SandboxFsMount[] = [
     {
@@ -259,7 +226,9 @@ function isPathInsidePosix(root: string, target: string): boolean {
 }
 
 function isPathInsideHost(root: string, target: string): boolean {
-  const rel = path.relative(root, target);
+  const canonicalRoot = resolveSandboxHostPathViaExistingAncestor(path.resolve(root));
+  const canonicalTarget = resolveSandboxHostPathViaExistingAncestor(path.resolve(target));
+  const rel = path.relative(canonicalRoot, canonicalTarget);
   if (!rel) {
     return true;
   }

src/agents/sandbox/host-paths.test.ts

@@ -0,0 +1,38 @@
+import { mkdtempSync, mkdirSync, realpathSync, symlinkSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import { describe, expect, it } from "vitest";
+import {
+  normalizeSandboxHostPath,
+  resolveSandboxHostPathViaExistingAncestor,
+} from "./host-paths.js";
+
+describe("normalizeSandboxHostPath", () => {
+  it("normalizes dot segments and strips trailing slash", () => {
+    expect(normalizeSandboxHostPath("/tmp/a/../b//")).toBe("/tmp/b");
+  });
+});
+
+describe("resolveSandboxHostPathViaExistingAncestor", () => {
+  it("keeps non-absolute paths unchanged", () => {
+    expect(resolveSandboxHostPathViaExistingAncestor("relative/path")).toBe("relative/path");
+  });
+
+  it("resolves symlink parents when the final leaf does not exist", () => {
+    if (process.platform === "win32") {
+      return;
+    }
+
+    const root = mkdtempSync(join(tmpdir(), "openclaw-host-paths-"));
+    const workspace = join(root, "workspace");
+    const outside = join(root, "outside");
+    mkdirSync(workspace, { recursive: true });
+    mkdirSync(outside, { recursive: true });
+    const link = join(workspace, "alias-out");
+    symlinkSync(outside, link);
+
+    const unresolved = join(link, "missing-leaf");
+    const resolved = resolveSandboxHostPathViaExistingAncestor(unresolved);
+    expect(resolved).toBe(join(realpathSync.native(outside), "missing-leaf"));
+  });
+});

src/agents/sandbox/host-paths.ts

@@ -0,0 +1,47 @@
+import { existsSync, realpathSync } from "node:fs";
+import { posix } from "node:path";
+
+/**
+ * Normalize a POSIX host path: resolve `.`, `..`, collapse `//`, strip trailing `/`.
+ */
+export function normalizeSandboxHostPath(raw: string): string {
+  const trimmed = raw.trim();
+  return posix.normalize(trimmed).replace(/\/+$/, "") || "/";
+}
+
+/**
+ * Resolve a path through the deepest existing ancestor so parent symlinks are honored
+ * even when the final source leaf does not exist yet.
+ */
+export function resolveSandboxHostPathViaExistingAncestor(sourcePath: string): string {
+  if (!sourcePath.startsWith("/")) {
+    return sourcePath;
+  }
+
+  const normalized = normalizeSandboxHostPath(sourcePath);
+  let current = normalized;
+  const missingSegments: string[] = [];
+
+  while (current !== "/" && !existsSync(current)) {
+    missingSegments.unshift(posix.basename(current));
+    const parent = posix.dirname(current);
+    if (parent === current) {
+      break;
+    }
+    current = parent;
+  }
+
+  if (!existsSync(current)) {
+    return normalized;
+  }
+
+  try {
+    const resolvedAncestor = normalizeSandboxHostPath(realpathSync.native(current));
+    if (missingSegments.length === 0) {
+      return resolvedAncestor;
+    }
+    return normalizeSandboxHostPath(posix.join(resolvedAncestor, ...missingSegments));
+  } catch {
+    return normalized;
+  }
+}

src/agents/sandbox/validate-sandbox-security.ts

@@ -5,9 +5,12 @@
  * Enforced at runtime when creating sandbox containers.
  */
 
-import { existsSync, realpathSync } from "node:fs";
+import { splitSandboxBindSpec } from "./bind-spec.js";
-import { posix } from "node:path";
 import { SANDBOX_AGENT_WORKSPACE_MOUNT } from "./constants.js";
+import {
+  normalizeSandboxHostPath,
+  resolveSandboxHostPathViaExistingAncestor,
+} from "./host-paths.js";
 
 // Targeted denylist: host paths that should never be exposed inside sandbox containers.
 // Exported for reuse in security audit collectors.
@@ -53,20 +56,11 @@ type ParsedBindSpec = {
 
 function parseBindSpec(bind: string): ParsedBindSpec {
   const trimmed = bind.trim();
-  const firstColon = trimmed.indexOf(":");
+  const parsed = splitSandboxBindSpec(trimmed);
-  if (firstColon <= 0) {
+  if (!parsed) {
     return { source: trimmed, target: "" };
   }
-  const source = trimmed.slice(0, firstColon);
+  return { source: parsed.host, target: parsed.container };
-  const rest = trimmed.slice(firstColon + 1);
-  const secondColon = rest.indexOf(":");
-  if (secondColon === -1) {
-    return { source, target: rest };
-  }
-  return {
-    source,
-    target: rest.slice(0, secondColon),
-  };
 }
 
 /**
@@ -85,8 +79,7 @@ export function parseBindTargetPath(bind: string): string {
  * Normalize a POSIX path: resolve `.`, `..`, collapse `//`, strip trailing `/`.
  */
 export function normalizeHostPath(raw: string): string {
-  const trimmed = raw.trim();
+  return normalizeSandboxHostPath(raw);
-  return posix.normalize(trimmed).replace(/\/+$/, "") || "/";
 }
 
 /**
@@ -119,41 +112,6 @@ export function getBlockedReasonForSourcePath(sourceNormalized: string): Blocked
   return null;
 }
 
-function resolvePathViaExistingAncestor(sourcePath: string): string {
-  if (!sourcePath.startsWith("/")) {
-    return sourcePath;
-  }
-
-  const normalized = normalizeHostPath(sourcePath);
-  let current = normalized;
-  const missingSegments: string[] = [];
-
-  // Resolve through the deepest existing ancestor so symlink parents are honored
-  // even when the final source leaf does not exist yet.
-  while (current !== "/" && !existsSync(current)) {
-    missingSegments.unshift(posix.basename(current));
-    const parent = posix.dirname(current);
-    if (parent === current) {
-      break;
-    }
-    current = parent;
-  }
-
-  if (!existsSync(current)) {
-    return normalized;
-  }
-
-  try {
-    const resolvedAncestor = normalizeHostPath(realpathSync.native(current));
-    if (missingSegments.length === 0) {
-      return resolvedAncestor;
-    }
-    return normalizeHostPath(posix.join(resolvedAncestor, ...missingSegments));
-  } catch {
-    return normalized;
-  }
-}
-
 function normalizeAllowedRoots(roots: string[] | undefined): string[] {
   if (!roots?.length) {
     return [];
@@ -165,7 +123,7 @@ function normalizeAllowedRoots(roots: string[] | undefined): string[] {
   const expanded = new Set<string>();
   for (const root of normalized) {
     expanded.add(root);
-    const real = resolvePathViaExistingAncestor(root);
+    const real = resolveSandboxHostPathViaExistingAncestor(root);
     if (real !== root) {
       expanded.add(real);
     }
@@ -217,6 +175,25 @@ function getReservedTargetReason(bind: string): BlockedBindReason | null {
   return null;
 }
 
+function enforceSourcePathPolicy(params: {
+  bind: string;
+  sourcePath: string;
+  allowedRoots: string[];
+  allowSourcesOutsideAllowedRoots: boolean;
+}): void {
+  const blockedReason = getBlockedReasonForSourcePath(params.sourcePath);
+  if (blockedReason) {
+    throw formatBindBlockedError({ bind: params.bind, reason: blockedReason });
+  }
+  if (params.allowSourcesOutsideAllowedRoots) {
+    return;
+  }
+  const allowedReason = getOutsideAllowedRootsReason(params.sourcePath, params.allowedRoots);
+  if (allowedReason) {
+    throw formatBindBlockedError({ bind: params.bind, reason: allowedReason });
+  }
+}
+
 function formatBindBlockedError(params: { bind: string; reason: BlockedBindReason }): Error {
   if (params.reason.kind === "non_absolute") {
     return new Error(
@@ -281,26 +258,21 @@ export function validateBindMounts(
 
     const sourceRaw = parseBindSourcePath(bind);
     const sourceNormalized = normalizeHostPath(sourceRaw);
-
+    enforceSourcePathPolicy({
-    if (!options?.allowSourcesOutsideAllowedRoots) {
+      bind,
-      const allowedReason = getOutsideAllowedRootsReason(sourceNormalized, allowedRoots);
+      sourcePath: sourceNormalized,
-      if (allowedReason) {
+      allowedRoots,
-        throw formatBindBlockedError({ bind, reason: allowedReason });
+      allowSourcesOutsideAllowedRoots: options?.allowSourcesOutsideAllowedRoots === true,
-      }
+    });
-    }
 
     // Symlink escape hardening: resolve through existing ancestors and re-check.
-    const sourceCanonical = resolvePathViaExistingAncestor(sourceNormalized);
+    const sourceCanonical = resolveSandboxHostPathViaExistingAncestor(sourceNormalized);
-    const reason = getBlockedReasonForSourcePath(sourceCanonical);
+    enforceSourcePathPolicy({
-    if (reason) {
+      bind,
-      throw formatBindBlockedError({ bind, reason });
+      sourcePath: sourceCanonical,
-    }
+      allowedRoots,
-    if (!options?.allowSourcesOutsideAllowedRoots) {
+      allowSourcesOutsideAllowedRoots: options?.allowSourcesOutsideAllowedRoots === true,
-      const allowedReason = getOutsideAllowedRootsReason(sourceCanonical, allowedRoots);
+    });
-      if (allowedReason) {
-        throw formatBindBlockedError({ bind, reason: allowedReason });
-      }
-    }
   }
 }