Fix sandbox path validation rejecting Docker bind mount paths#16509
Fix sandbox path validation rejecting Docker bind mount paths#16509Clawborn wants to merge 3 commits intoopenclaw:mainfrom
Conversation
resolveSandboxPath only checked if file paths were within the workspace root directory. For sandboxed agents with Docker bind mounts configured via docker.binds, legitimate paths to bound directories were rejected with 'Path escapes sandbox root' because the host paths of bind mounts fell outside the workspace root. Add allowedPaths parameter to resolveSandboxPath and assertSandboxPath that accepts additional valid path prefixes. Parse Docker bind mount host paths from the sandbox config and pass them through the tool creation chain (pi-tools -> sandbox path guard -> path validation). Fixes openclaw#16379
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
src/agents/sandbox/fs-bridge.ts
Outdated
| const parts = bind.split(":"); | ||
| // Format: hostPath:containerPath[:options] | ||
| return parts.length >= 2 ? parts[0] : ""; |
There was a problem hiding this comment.
parsing Windows bind mount host paths with drive letters (e.g., C:\path:/container:ro) would incorrectly split on the drive colon, extracting C instead of C:\path as the host path
on Windows, Docker bind mounts with drive letters like C:\Users\me:/data:ro would split into ["C", "\Users\me", "/data", "ro"], causing parts[0] to return only C instead of the full host path C:\Users\me
| const parts = bind.split(":"); | |
| // Format: hostPath:containerPath[:options] | |
| return parts.length >= 2 ? parts[0] : ""; | |
| const parts = bind.split(":"); | |
| // Format: hostPath:containerPath[:options] | |
| // Handle Windows paths with drive letters (e.g., C:\path) | |
| if (parts.length >= 3 && parts[0].match(/^[A-Za-z]$/)) { | |
| return parts[0] + ":" + parts[1]; | |
| } | |
| return parts.length >= 2 ? parts[0] : ""; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/agents/sandbox/fs-bridge.ts
Line: 229:231
Comment:
parsing Windows bind mount host paths with drive letters (e.g., `C:\path:/container:ro`) would incorrectly split on the drive colon, extracting `C` instead of `C:\path` as the host path
on Windows, Docker bind mounts with drive letters like `C:\Users\me:/data:ro` would split into `["C", "\Users\me", "/data", "ro"]`, causing `parts[0]` to return only `C` instead of the full host path `C:\Users\me`
```suggestion
const parts = bind.split(":");
// Format: hostPath:containerPath[:options]
// Handle Windows paths with drive letters (e.g., C:\path)
if (parts.length >= 3 && parts[0].match(/^[A-Za-z]$/)) {
return parts[0] + ":" + parts[1];
}
return parts.length >= 2 ? parts[0] : "";
```
How can I resolve this? If you propose a fix, please make it concise.
openclaw-barnacle
bot
added
docker
thewilloftheshadow
force-pushed
the
main
branch
from
bfc1ccb to
f92900f
Compare
Additional Comments (2)
When await assertNoSymlink(resolved.relative, path.resolve(params.root));For example, if the file is To fix this, you need to also return the matched allowed-path root from Prompt To Fix With AIThis is a comment left during a code review.
Path: src/agents/sandbox-paths.ts
Line: 69:78
Comment:
**Symlink check uses wrong base directory for allowed paths**
When `resolveSandboxPath` matches an allowed path, it returns `relative` as the path relative to that *allowed* directory. But `assertSandboxPath` always passes `params.root` (the workspace root) as the base for `assertNoSymlink`:
```typescript
await assertNoSymlink(resolved.relative, path.resolve(params.root));
```
For example, if the file is `/data/workspace-two/MEMORY.md` resolved via allowed path `/data/workspace-two`, `assertNoSymlink("MEMORY.md", "/data/workspace-one")` will check `/data/workspace-one/MEMORY.md` for symlinks — not the actual file being accessed. This means symlink-based sandbox escapes within bind-mounted directories would go undetected.
To fix this, you need to also return the matched allowed-path root from `resolveSandboxPath` and use it as the base for symlink checking:
```
const base = resolved.matchedAllowedPath ?? path.resolve(params.root);
await assertNoSymlink(resolved.relative, base);
```
How can I resolve this? If you propose a fix, please make it concise.
When a file resolves via an allowed (bind mount) path, For example, with bind
All Docker exec operations ( Prompt To Fix With AIThis is a comment left during a code review.
Path: src/agents/sandbox/fs-bridge.ts
Line: 270:275
Comment:
**Container path is wrong for bind-mounted files**
When a file resolves via an allowed (bind mount) path, `relative` is relative to the *host-side* bind mount path. The `containerPath` is then computed as `containerWorkdir + relative`, but for bind mounts the container-side mount point is different from `containerWorkdir`.
For example, with bind `"/host/data:/container/data:ro"` and file `/host/data/file.txt`:
- `relative` = `"file.txt"`
- `containerPath` = `"/workspace/file.txt"` (wrong — should be `"/container/data/file.txt"`)
All Docker exec operations (`cat`, `stat`, `mkdir`, etc.) will target the wrong path inside the container. You need to map the host bind path to its corresponding container mount point and use that instead of `containerWorkdir` when the path was matched via an allowed path.
How can I resolve this? If you propose a fix, please make it concise. |
|
This pull request has been automatically marked as stale due to inactivity. |
openclaw-barnacle
bot
added
stale
3 participants
Summary
Fixes #16379
Sandboxed agents with Docker bind mounts (
docker.binds) could not useread,write, oredittools on files within bound directories. The path validation inresolveSandboxPathonly checked whether a file path was within the workspace root, rejecting all bind mount host paths withPath escapes sandbox root.Root Cause
resolveSandboxPath()validates that resolved file paths are relative toparams.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 optionalallowedPathsparameter toresolveSandboxPathandassertSandboxPath. 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 fromdocker.bindsconfig and pass them toresolveSandboxPath.pi-tools.read.ts: ThreadallowedPathsthroughwrapSandboxPathGuardandSandboxToolParams.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
resolveSandboxPathwithallowedPaths— all passingallowedPathsis not providedGreptile Summary
This PR adds
allowedPathssupport to the sandbox path validation so that Docker bind mount host paths are not rejected as sandbox escapes. The approach is sound:resolveSandboxPathnow checks if an out-of-root path falls within any allowed path (derived fromdocker.binds), with proper traversal attack protection.However, two issues need to be addressed before this is safe to merge:
assertSandboxPathalways passesparams.root(workspace root) as the base forassertNoSymlink, 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.resolveSandboxFsPathcomputescontainerPathby joiningcontainerWorkdirwith 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.Confidence Score: 2/5
src/agents/sandbox-paths.ts(symlink check base directory) andsrc/agents/sandbox/fs-bridge.ts(container path mapping for bind mounts).Last reviewed commit: f35b909