Merge e229faf850 into da71eaebd2

src/agents/bash-tools.shared.ts

@@ -72,7 +72,8 @@ export function buildDockerExecArgs(params: {
   const pathExport = hasCustomPath
     ? 'export PATH="${OPENCLAW_PREPEND_PATH}:$PATH"; unset OPENCLAW_PREPEND_PATH; '
     : "";
-  args.push(params.containerName, "sh", "-lc", `${pathExport}${params.command}`);
+  // Use absolute path /bin/sh to avoid OCI runtime PATH resolution issues in nix containers
+  args.push(params.containerName, "/bin/sh", "-lc", `${pathExport}${params.command}`);
   return args;
 }
 

src/agents/bash-tools.test.ts

@@ -399,7 +399,7 @@ describe("buildDockerExecArgs", () => {
       tty: false,
     });
 
-    expect(args).toContain("sh");
+    expect(args).toContain("/bin/sh");
     expect(args).toContain("-lc");
   });
 

src/agents/sandbox-paths.test.ts

@@ -0,0 +1,89 @@
+import { describe, expect, it } from "vitest";
+import { resolveSandboxPath, assertSandboxPath } from "./sandbox-paths.js";
+import path from "node:path";
+
+describe("resolveSandboxPath", () => {
+  const root = "/workspace";
+  const cwd = "/workspace";
+
+  it("allows paths within root", () => {
+    const result = resolveSandboxPath({
+      filePath: "subdir/file.txt",
+      cwd,
+      root,
+    });
+    expect(result.resolved).toBe("/workspace/subdir/file.txt");
+    expect(result.relative).toBe("subdir/file.txt");
+    expect(result.base).toBe("/workspace");
+  });
+
+  it("rejects paths escaping root without allowedPaths", () => {
+    expect(() =>
+      resolveSandboxPath({
+        filePath: "/other/path/file.txt",
+        cwd,
+        root,
+      }),
+    ).toThrow(/escapes sandbox root/);
+  });
+
+  it("allows paths in allowedPaths when they escape root", () => {
+    // /external/skills is outside /workspace, but allowed via allowedPaths
+    const result = resolveSandboxPath({
+      filePath: "/external/skills/tameson/SKILL.md",
+      cwd,
+      root,
+      allowedPaths: ["/external/skills"],
+    });
+    expect(result.resolved).toBe("/external/skills/tameson/SKILL.md");
+    expect(result.relative).toBe("tameson/SKILL.md");
+    expect(result.base).toBe("/external/skills");
+  });
+
+  it("still rejects paths not in root or allowedPaths", () => {
+    expect(() =>
+      resolveSandboxPath({
+        filePath: "/etc/passwd",
+        cwd,
+        root,
+        allowedPaths: ["/external/skills"],
+      }),
+    ).toThrow(/escapes sandbox root/);
+  });
+
+  it("allows exact match of allowedPath", () => {
+    const result = resolveSandboxPath({
+      filePath: "/external/skills",
+      cwd,
+      root,
+      allowedPaths: ["/external/skills"],
+    });
+    expect(result.resolved).toBe("/external/skills");
+    expect(result.relative).toBe("");
+    expect(result.base).toBe("/external/skills");
+  });
+
+  it("prefers root over allowedPaths for paths inside root", () => {
+    // Path is inside root - should use root as base, not allowedPaths
+    const result = resolveSandboxPath({
+      filePath: "/workspace/file.txt",
+      cwd,
+      root,
+      allowedPaths: ["/workspace"], // redundant, but shouldn't matter
+    });
+    expect(result.base).toBe("/workspace");
+    expect(result.relative).toBe("file.txt");
+  });
+
+  it("handles multiple allowedPaths", () => {
+    const result = resolveSandboxPath({
+      filePath: "/data/files/test.txt",
+      cwd,
+      root,
+      allowedPaths: ["/external/skills", "/data/files"],
+    });
+    expect(result.resolved).toBe("/data/files/test.txt");
+    expect(result.relative).toBe("test.txt");
+    expect(result.base).toBe("/data/files");
+  });
+});

src/agents/sandbox-paths.ts

@@ -25,25 +25,53 @@ function resolveToCwd(filePath: string, cwd: string): string {
   return path.resolve(cwd, expanded);
 }
 
-export function resolveSandboxPath(params: { filePath: string; cwd: string; root: string }): {
+export function resolveSandboxPath(params: {
+  filePath: string;
+  cwd: string;
+  root: string;
+  allowedPaths?: string[];
+}): {
   resolved: string;
   relative: string;
+  base: string;
 } {
   const resolved = resolveToCwd(params.filePath, params.cwd);
   const rootResolved = path.resolve(params.root);
   const relative = path.relative(rootResolved, resolved);
+
+  // Check if path is within the main root
   if (!relative || relative === "") {
-    return { resolved, relative: "" };
+    return { resolved, relative: "", base: rootResolved };
   }
-  if (relative.startsWith("..") || path.isAbsolute(relative)) {
-    throw new Error(`Path escapes sandbox root (${shortPath(rootResolved)}): ${params.filePath}`);
+  if (!relative.startsWith("..") && !path.isAbsolute(relative)) {
+    return { resolved, relative, base: rootResolved };
   }
-  return { resolved, relative };
+
+  // Path escapes main root - check allowedPaths
+  if (params.allowedPaths?.length) {
+    for (const allowedPath of params.allowedPaths) {
+      const allowedResolved = path.resolve(allowedPath);
+      const relativeToAllowed = path.relative(allowedResolved, resolved);
+      if (
+        relativeToAllowed === "" ||
+        (!relativeToAllowed.startsWith("..") && !path.isAbsolute(relativeToAllowed))
+      ) {
+        return { resolved, relative: relativeToAllowed, base: allowedResolved };
+      }
+    }
+  }
+
+  throw new Error(`Path escapes sandbox root (${shortPath(rootResolved)}): ${params.filePath}`);
 }
 
-export async function assertSandboxPath(params: { filePath: string; cwd: string; root: string }) {
+export async function assertSandboxPath(params: {
+  filePath: string;
+  cwd: string;
+  root: string;
+  allowedPaths?: string[];
+}) {
   const resolved = resolveSandboxPath(params);
-  await assertNoSymlink(resolved.relative, path.resolve(params.root));
+  await assertNoSymlink(resolved.relative, resolved.base);
   return resolved;
 }
 

src/agents/sandbox/config.ts

@@ -51,6 +51,10 @@ export function resolveSandboxDockerConfig(params: {
     : globalDocker?.ulimits;
 
   const binds = [...(globalDocker?.binds ?? []), ...(agentDocker?.binds ?? [])];
+  const allowedReadPaths = [
+    ...(globalDocker?.allowedReadPaths ?? []),
+    ...(agentDocker?.allowedReadPaths ?? []),
+  ];
 
   return {
     image: agentDocker?.image ?? globalDocker?.image ?? DEFAULT_SANDBOX_IMAGE,
@@ -76,6 +80,7 @@ export function resolveSandboxDockerConfig(params: {
     dns: agentDocker?.dns ?? globalDocker?.dns,
     extraHosts: agentDocker?.extraHosts ?? globalDocker?.extraHosts,
     binds: binds.length ? binds : undefined,
+    allowedReadPaths: allowedReadPaths.length ? allowedReadPaths : undefined,
   };
 }
 

src/agents/sandbox/docker.ts

@@ -203,7 +203,8 @@ async function createSandboxContainer(params: {
   await execDocker(["start", name]);
 
   if (cfg.setupCommand?.trim()) {
-    await execDocker(["exec", "-i", name, "sh", "-lc", cfg.setupCommand]);
+    // Use absolute path /bin/sh to avoid OCI runtime PATH resolution issues in nix containers
+    await execDocker(["exec", "-i", name, "/bin/sh", "-lc", cfg.setupCommand]);
   }
 }
 

src/agents/sandbox/types.docker.ts

@@ -19,4 +19,10 @@ export type SandboxDockerConfig = {
   dns?: string[];
   extraHosts?: string[];
   binds?: string[];
+  /**
+   * Additional paths (inside the container) that are allowed for read operations.
+   * Useful for bind-mounted paths outside the workspace root.
+   * Example: ["/workspace/.skills"] to allow reading from bind-mounted skills.
+   */
+  allowedReadPaths?: string[];
 };