#16509: Fix sandbox path validation rejecting Docker bind mount paths
docker
agents
stale
size: S
trusted-contributor
Cluster:
Sandbox Path Management Fixes
## 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
#3907: fix(sandbox): use absolute /bin/sh path + add allowedReadPaths config
by pvoo · 2026-01-29
88.9%
#11820: fix(sandbox): remap container paths in sandboxed file tools
by steflsd · 2026-02-08
86.3%
#21665: fix(sandbox): add /home and /Users to bind-mount denylist
by AI-Reviewer-QS · 2026-02-20
84.2%
#16929: fix(security): block access to sensitive directories from within sa...
by CornBrother0x · 2026-02-15
83.0%
#19344: fix(sandbox): allow writes when workspaceAccess is 'none'
by mingming099 · 2026-02-17
82.9%
#17402: fix:sandbox path issue
by luckylhb90 · 2026-02-15
82.3%
#16922: fix: remove incorrect sandbox file tool guidance
by carrotRakko · 2026-02-15
82.3%
#13873: fix(sandbox): prevent Windows PATH from poisoning docker exec
by alessandrorodi · 2026-02-11
81.7%
#4226: Fix/sandbox containerworkdir rw access
by ozgur-polat · 2026-01-29
81.7%
#20991: fix(sandbox): fall back to gateway UID:GID when no user is configur...
by cluster2600 · 2026-02-19
81.4%