#23112: fix(sandbox): propagate docker.user to all exec commands
agents
size: S
Cluster:
Sandbox Enhancements and Fixes
## Summary
Fixes #20979 (partially — addresses the propagation gap; see #20991 for the fallback default).
When `agents.defaults.sandbox.docker.user` is configured, the value was only used by `buildSandboxCreateArgs` for the initial `docker create`. All subsequent `docker exec` calls — FS bridge (Read/Write/Edit tools), bash exec runtime, pi-tools sandbox config, and the container setup command — ran as root (no `-u` flag). With `capDrop: ["ALL"]`, root lacks `DAC_OVERRIDE` and gets "Permission denied" on the workspace directory owned by the configured user.
This PR propagates `docker.user` through all exec paths so every `docker exec` includes `-u <user>`.
## Root Cause
`docker.user` was partially wired: `buildSandboxCreateArgs` passed `--user` to `docker create`, but four other exec call sites constructed their own `docker exec` args without consulting `docker.user`:
1. **`fs-bridge.ts`** — Read/Write/Edit/apply_patch tools
2. **`bash-tools.exec-runtime.ts`** — Bash tool runtime
3. **`bash-tools.shared.ts`** (`buildDockerExecArgs`) — shared exec arg builder
4. **`docker.ts`** (`createSandboxContainer`) — setup command
## Changes
**`src/agents/bash-tools.shared.ts`** — Add `user?: string` to `buildDockerExecArgs` params and insert `-u` flag:
```diff
export function buildDockerExecArgs(params: {
...
tty: boolean;
+ user?: string;
}) {
const args = ["exec", "-i"];
+ if (params.user) {
+ args.push("-u", params.user);
+ }
if (params.tty) {
```
**`src/agents/bash-tools.exec-runtime.ts`** — Pass `sandbox.user` through:
```diff
tty: opts.usePty,
+ user: opts.sandbox.user,
```
**`src/agents/sandbox/fs-bridge.ts`** — Insert `-u` before container name:
```diff
- const dockerArgs = ["exec", "-i", this.sandbox.containerName, "sh", "-c", script, ...];
+ const dockerArgs = ["exec", "-i"];
+ if (this.sandbox.docker.user) {
+ dockerArgs.push("-u", this.sandbox.docker.user);
+ }
+ dockerArgs.push(this.sandbox.containerName, "sh", "-c", script, ...);
```
**`src/agents/sandbox/docker.ts`** — Add `-u` to setup command exec:
```diff
- await execDocker(["exec", "-i", name, "sh", "-lc", cfg.setupCommand]);
+ const setupArgs = ["exec", "-i"];
+ if (cfg.user) {
+ setupArgs.push("-u", cfg.user);
+ }
+ setupArgs.push(name, "sh", "-lc", cfg.setupCommand);
+ await execDocker(setupArgs);
```
**`src/agents/pi-tools.ts`** — Pass `docker.user` to `BashSandboxConfig`:
```diff
env: sandbox.docker.env,
+ user: sandbox.docker.user,
```
**`src/agents/bash-tools.shared.ts`** (type) — Add `user` to `BashSandboxConfig`:
```diff
export type BashSandboxConfig = {
...
env?: Record<string, string>;
+ user?: string;
};
```
## Relationship to #20991
This PR and #20991 are complementary:
| Aspect | This PR | PR #20991 |
|--------|---------|-----------|
| Problem | `docker.user` configured but not propagated to exec | `docker.user` not configured at all |
| Fix | Propagate configured user to all exec paths | Fall back to gateway UID:GID |
| Scope | 5 source files + 2 test files | Different files (auto-detection logic) |
Both are needed: #20991 provides a sensible default when no user is configured, this PR ensures the configured value actually reaches all exec commands.
## Test Plan
- [x] 6 new unit tests for `buildDockerExecArgs` user flag (`bash-tools.shared.test.ts`)
- [x] 3 new unit tests for `buildSandboxCreateArgs` user flag (`docker.test.ts`)
- [x] All existing sandbox tests pass (no regressions)
- [x] `oxlint` — 0 errors
- [x] `oxfmt` — clean
- [x] `tsgo` — clean
Fixes #20979
Related: #20991, #17941, #16379
<!-- greptile_comment -->
<h3>Greptile Summary</h3>
This PR systematically propagates the configured `agents.defaults.sandbox.docker.user` value to all `docker exec` commands, fixing a critical permission issue when `capDrop: ["ALL"]` is set.
**Key changes:**
- Modified `buildDockerExecArgs` in `bash-tools.shared.ts` to accept and insert `-u` flag
- Updated all four exec call sites (fs-bridge, bash exec runtime, pi-tools config, setup command) to pass `docker.user`
- Added comprehensive unit tests for both `buildDockerExecArgs` and `buildSandboxCreateArgs`
**Impact:**
The fix resolves permission denied errors that occurred because root (default exec user) lacks `DAC_OVERRIDE` capability when `capDrop: ["ALL"]` is configured, preventing access to workspace directories owned by the configured user. All docker exec calls now run as the same user specified in the container creation.
<h3>Confidence Score: 5/5</h3>
- This PR is safe to merge with minimal risk
- The implementation is thorough, well-tested, and addresses a clear security/permission bug. All four exec call sites have been systematically updated with the same pattern. The changes are type-safe (optional `user?: string`), backward-compatible (gracefully handles undefined/empty values), and include comprehensive unit tests covering edge cases (undefined, empty string, flag ordering). No breaking changes or risky refactoring.
- No files require special attention
<sub>Last reviewed commit: cf9a7db</sub>
<!-- greptile_other_comments_section -->
<!-- /greptile_comment -->
Most Similar PRs
#20991: fix(sandbox): fall back to gateway UID:GID when no user is configur...
by cluster2600 · 2026-02-19
88.1%
#4226: Fix/sandbox containerworkdir rw access
by ozgur-polat · 2026-01-29
79.0%
#16509: Fix sandbox path validation rejecting Docker bind mount paths
by Clawborn · 2026-02-14
78.4%
#16922: fix: remove incorrect sandbox file tool guidance
by carrotRakko · 2026-02-15
77.8%
#19344: fix(sandbox): allow writes when workspaceAccess is 'none'
by mingming099 · 2026-02-17
77.7%
#3907: fix(sandbox): use absolute /bin/sh path + add allowedReadPaths config
by pvoo · 2026-01-29
77.6%
#13873: fix(sandbox): prevent Windows PATH from poisoning docker exec
by alessandrorodi · 2026-02-11
77.3%
#8161: fix(sandbox): block dangerous environment variables from Docker con...
by yubrew · 2026-02-03
76.5%
#20477: fix(cron): prevent sandbox config clobbering in hook/cron agent path
by olyashok · 2026-02-19
75.9%
#11820: fix(sandbox): remap container paths in sandboxed file tools
by steflsd · 2026-02-08
75.6%