#3907: fix(sandbox): use absolute /bin/sh path + add allowedReadPaths config

## Context / Why this PR We hit two sandbox pain points in production usage: ### 1) Intermittent sandbox exec failures ("sh not found") **Observed error** (Docker/runc): ``` OCI runtime exec failed: exec failed: unable to start container process: exec: "sh": executable file not found in $PATH ``` This happened in a long-running nix2container-based sandbox container (no restarts), and then worked fine when retried later — a classic intermittent failure. **Root cause:** Our sandbox exec code used the bare executable `"sh"` when calling `docker exec`. That requires the OCI runtime to resolve `sh` via PATH. In nix2container images, PATH/executable resolution can be brittle (symlinked shells, minimal roots, etc.) and we saw runc fail resolution intermittently. **Why this fix:** Use an **absolute** shell path (`/bin/sh`) so the runtime does not need to search PATH at all. `/bin/sh` is present in our sandbox images (symlinked to bash). This is a minimal, robust fix that matches how most container tooling avoids PATH lookup for critical entrypoints. ### 2) Read tool cannot access bind-mounted paths outside workspace root We bind-mount extra paths into the sandbox (e.g. shared skills directories) but the host-side path validation blocks reads if the requested path is outside the configured workspace root. This forces workarounds like copying bind-mounted content into the workspace, which is slow and error-prone. **Why this fix:** Introduce an explicit allowlist `allowedReadPaths` so we can safely permit reads from specific additional directories (typically bind mounts) without weakening the sandbox boundary. --- ## What this PR changes ### Fix: use `/bin/sh` for sandbox docker exec - `src/agents/bash-tools.shared.ts`: `docker exec … sh -lc …` → `docker exec … /bin/sh -lc …` - `src/agents/sandbox/docker.ts`: same fix for `setupCommand` ### Feature: `allowedReadPaths` for bind mount access - `src/agents/sandbox/types.docker.ts`: add `allowedReadPaths?: string[]` - `src/agents/sandbox/config.ts`: merge arrays from global + agent config (same as `binds`) - `src/agents/sandbox-paths.ts`: - `resolveSandboxPath` now accepts `allowedPaths?: string[]` - Path is valid if within workspace root **OR** any allowed path - Returns `{ base }` so symlink checks apply correctly to the matched subtree - New tests: `src/agents/sandbox-paths.test.ts` --- ## Example config ```json { "sandbox": { "docker": { "binds": ["/host/path/skills:/workspace/.skills:ro"], "allowedReadPaths": ["/workspace/.skills"] } } } ``` --- ## Design Note: Why separate `allowedReadPaths` instead of auto-deriving from `binds`? We intentionally keep these as separate config options: - **`binds`** = "what is available inside the container" (container-side visibility) - **`allowedReadPaths`** = "what host-side path validation should permit" (host-side security gate) Auto-deriving allowed paths from bind mounts would be convenient but risky: someone might mount a path for container-internal use (e.g. a cache directory) without intending to expose it to host-side read validation. Keeping them separate ensures explicit opt-in for each security boundary. --- ## Testing ✅ 27 related tests pass: - `src/agents/sandbox-paths.test.ts` — new coverage for allowedPaths (7 tests) - `src/agents/bash-tools.test.ts` — updated expectation for /bin/sh - `src/agents/sandbox/tool-policy.test.ts` --- ## Safety - `allowedReadPaths` is explicit opt-in — does not broaden access arbitrarily - Symlink protection still enforced within allowed paths - `/bin/sh` is conservative — avoids PATH resolution entirely <!-- greptile_comment --> <h2>Greptile Overview</h2> <h3>Greptile Summary</h3> This PR hardens sandbox command execution by switching `docker exec … sh -lc` to `docker exec … /bin/sh -lc` (avoids OCI PATH resolution issues), and introduces an `allowedReadPaths` concept intended to permit read access to specific bind-mounted directories outside the workspace root. It also updates `resolveSandboxPath`/`assertSandboxPath` to support allowlisted bases and adds unit tests for the new path resolution behavior. Main concerns are around integration: `allowedReadPaths` is currently only added to config/types and `sandbox-paths.ts`, but the sandboxed tool wrappers that enforce path access (`assertSandboxPath`) don’t pass these allowed paths yet, so the feature may not work as intended. There’s also a scope-merging inconsistency where per-agent `allowedReadPaths` can leak into `shared` scope despite other docker overrides being ignored. <h3>Confidence Score: 3/5</h3> - This PR is mostly safe, but the new allowlist feature appears partially integrated and may not work as intended (and may subtly affect shared-scope behavior). - The /bin/sh change is straightforward and well-scoped. However, `allowedReadPaths` is not yet propagated to the main path-guard call sites that enforce sandbox file access, so the advertised functionality likely won’t take effect. Additionally, config merging includes per-agent allowed paths under shared scope, which is inconsistent with other shared-scope rules and could broaden access unexpectedly. - src/agents/pi-tools.read.ts, src/agents/sandbox/config.ts <!-- greptile_other_comments_section --> <!-- /greptile_comment -->