← Back to PRs

#16509: Fix sandbox path validation rejecting Docker bind mount paths

by Clawborn open 2026-02-14 20:20 View on GitHub →
docker agents stale size: S trusted-contributor
## Summary Fixes #16379 Sandboxed agents with Docker bind mounts (`docker.binds`) could not use `read`, `write`, or `edit` tools on files within bound directories. The path validation in `resolveSandboxPath` only checked whether a file path was within the workspace root, rejecting all bind mount host paths with `Path escapes sandbox root`. ## Root Cause `resolveSandboxPath()` validates that resolved file paths are relative to `params.root` (the workspace directory). Docker bind mount host paths (e.g. `/root/.openclaw/workspace-two`) are outside the workspace root, so they always fail validation — even though the container has legitimate access via the bind mount. ## Changes - **`sandbox-paths.ts`**: Add optional `allowedPaths` parameter to `resolveSandboxPath` and `assertSandboxPath`. When a path escapes the root, check if it falls within any allowed path before throwing. - **`sandbox/fs-bridge.ts`**: Parse bind mount host paths from `docker.binds` config and pass them to `resolveSandboxPath`. - **`pi-tools.read.ts`**: Thread `allowedPaths` through `wrapSandboxPathGuard` and `SandboxToolParams`. - **`pi-tools.ts`**: Extract bind mount host paths from sandbox context and pass to sandboxed tool constructors. - **`sandbox-paths.test.ts`**: Unit tests covering allowed paths, exact match, rejection of paths outside both root and allowed paths, and path traversal attacks. ## Testing - 6 new unit tests for `resolveSandboxPath` with `allowedPaths` — all passing - Path traversal through allowed paths is correctly rejected - Backward compatible: existing behavior unchanged when `allowedPaths` is not provided <!-- greptile_comment --> <h3>Greptile Summary</h3> This PR adds `allowedPaths` support to the sandbox path validation so that Docker bind mount host paths are not rejected as sandbox escapes. The approach is sound: `resolveSandboxPath` now checks if an out-of-root path falls within any allowed path (derived from `docker.binds`), with proper traversal attack protection. However, two issues need to be addressed before this is safe to merge: - **Symlink check targets wrong directory**: `assertSandboxPath` always passes `params.root` (workspace root) as the base for `assertNoSymlink`, even when the path was matched via an allowed path. This means symlink protection doesn't actually cover bind-mounted directories — symlinks within those directories won't be detected. - **Container path mapping is incorrect for bind mounts**: `resolveSandboxFsPath` computes `containerPath` by joining `containerWorkdir` with the relative path, but for bind mounts the container-side mount point differs from the workspace workdir. All Docker exec operations (read, write, stat, etc.) will target the wrong file inside the container. - The bind mount parsing and new tests are well-structured, with good coverage of edge cases including path traversal and Windows drive letters. <h3>Confidence Score: 2/5</h3> - This PR has two logic bugs — a security gap in symlink checking and incorrect container path mapping — that should be fixed before merging. - The path validation logic itself is correct and properly rejects traversal attacks. However, the symlink check uses the wrong base directory for allowed paths (security regression), and the container path computation for bind-mounted files will cause Docker operations to target wrong files. Both issues affect the core functionality this PR is trying to enable. - Pay close attention to `src/agents/sandbox-paths.ts` (symlink check base directory) and `src/agents/sandbox/fs-bridge.ts` (container path mapping for bind mounts). <sub>Last reviewed commit: f35b909</sub> <!-- greptile_other_comments_section --> <!-- /greptile_comment -->

Most Similar PRs