← Back to PRs

#23112: fix(sandbox): propagate docker.user to all exec commands

by dashed open 2026-02-22 01:42 View on GitHub →
agents size: S
## 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