Skip to content

fix(sandbox): bake safe.directory into image and surface chown failures #83

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dviejokfs merged 2 commits into from
May 10, 2026
Merged

fix(sandbox): bake safe.directory into image and surface chown failures #83

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
48aa093
fix(sandbox): bake safe.directory into image and surface chown failures
dviejokfs May 10, 2026
1aab324
docs(changelog): add entry for sandbox safe.directory + run_root_exec…
dviejokfs May 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Fixed
- **Sandbox `git pull` failed with `fatal: detected dubious ownership`**: `/home/temps/workspace` came up owned by uid `0:0` inside sandbox containers when the host-side `temps` server ran as root or the Docker daemon used userns-remap, so Git refused to operate on the bind-mount. The Dockerfile generator now bakes `git config --system --add safe.directory /home/temps/workspace` into the sandbox image while still root, so Git trusts the workspace regardless of stat owner — belt-and-suspenders against userns-remap and against post-start chown failures. Takes effect after sandbox images are rebuilt and pushed (`SANDBOX_CHANNEL=beta ./scripts/build-sandbox-images.sh`, then promote to `stable`). (#83)
- **Silent sandbox-creation failures from swallowed exec errors**: `DockerSandboxProvider::create_sandbox` had three `let _ = start_exec(...)` post-start sites (home `chown`, work-dir `chown`, AI-CLI restore) that discarded both the exit code and stderr — so a chown that failed against a userns-remapped container left users with a "successfully created" sandbox that then exploded the first time Git or any uid-sensitive tool ran. Replaced with a new `run_root_exec` helper that drains the output stream, inspects the exit code, and logs stderr on non-zero. The two chown steps now fail sandbox creation visibly instead of leaving a silently broken workspace; the AI-CLI restore stays best-effort but its exit code is logged. (#83)

## [0.1.0-beta.8] - 2026-05-10

### Fixed
Expand Down
236 changes: 141 additions & 95 deletions crates/temps-agents/src/sandbox/docker.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,17 @@ RUN groupadd -g 1100 git-users \
# bypasses that restriction since the host's docker daemon doesn't
# inherit the no-new-privileges flag.
RUN mkdir -p {work_dir} && chown {chown} {work_dir}
# Mark the workspace as a trusted Git directory regardless of stat owner.
# Without this, `git pull` inside the sandbox fails with "dubious ownership"
# whenever the bind-mounted host work_dir comes in owned by a uid other
# than the container's `temps` user (uid 1000) — which happens whenever the
# host-side temps server runs as root, or when userns-remap is enabled on
# the host Docker daemon. Both situations are normal on production hosts.
# The post-start `chown -R` is the primary fix for filesystem semantics,
# but Git's safety check is independent of permissions, so we belt-and-
# suspenders it here. Scoped to /home/temps/workspace (not '*') so we
# don't blanket-trust every repo a user might mount or clone elsewhere.
RUN git config --system --add safe.directory {work_dir}
# /run/temps-pty holds one Unix socket per terminal tab (one per {{kind,tab}}
# pair). dtach creates these sockets on first attach; subsequent reconnects
# find the existing socket and re-attach instead of respawning the CLI. The
Expand Down Expand Up @@ -519,6 +530,94 @@ impl DockerSandboxProvider {
Self { docker, config }
}

/// Run a command as root inside a freshly-started sandbox container,
/// drain its output, and return its exit code. Used for the post-start
/// fix-up steps (chown of home + work dir, AI-CLI restore) where we
/// need to know whether the command actually succeeded — earlier
/// versions used `let _ = start_exec(...)` and silently masked
/// failures, leaving sandboxes with root-owned workspaces that broke
/// `git pull` ("dubious ownership") and writes from the temps user.
async fn run_root_exec(
&self,
container_id: &str,
run_id: i32,
label: &str,
cmd: Vec<String>,
) -> Result<i64, AgentError> {
let exec = self
.docker
.create_exec(
container_id,
bollard::models::ExecConfig {
user: Some("0:0".to_string()),
cmd: Some(cmd),
attach_stdout: Some(true),
attach_stderr: Some(true),
..Default::default()
},
)
.await
.map_err(|e| AgentError::SandboxCreationFailed {
run_id,
provider: "docker".to_string(),
reason: format!("Failed to create {} exec: {}", label, e),
})?;

let output = self
.docker
.start_exec(
&exec.id,
Some(bollard::exec::StartExecOptions {
detach: false,
..Default::default()
}),
)
.await
.map_err(|e| AgentError::SandboxCreationFailed {
run_id,
provider: "docker".to_string(),
reason: format!("Failed to start {} exec: {}", label, e),
})?;

// Drain the output stream so the exec actually runs to completion;
// bollard tears the exec down when the stream is dropped, which
// can race with the underlying command on slow hosts.
let mut stderr_tail = String::new();
if let StartExecResults::Attached { mut output, .. } = output {
while let Some(chunk) = output.next().await {
if let Ok(LogOutput::StdErr { message }) = chunk {
let text = String::from_utf8_lossy(&message);
stderr_tail.push_str(&text);
}
}
}

let exit_code = self
.docker
.inspect_exec(&exec.id)
.await
.ok()
.and_then(|i| i.exit_code)
.unwrap_or(-1);

if exit_code != 0 {
let trimmed = stderr_tail.trim();
tracing::error!(
"Sandbox {} step '{}' exited {} (stderr: {})",
container_id,
label,
exit_code,
if trimmed.is_empty() {
"<empty>"
} else {
trimmed
}
);
}

Ok(exit_code)
}

/// Build the sandbox image if it doesn't exist.
/// For preset runtimes, generates a Dockerfile dynamically.
/// For custom images, assumes the image is already available (pull or pre-built).
Expand Down Expand Up @@ -1210,41 +1309,18 @@ impl SandboxProvider for DockerSandboxProvider {
// earlier image builds may be owned by a different uid entirely.
// Running chown as root (not USER {sandbox-user}) normalizes it
// every start.
{
let exec = self
.docker
.create_exec(
&container.id,
bollard::models::ExecConfig {
user: Some("0:0".to_string()),
cmd: Some(vec![
"chown".to_string(),
"-R".to_string(),
SANDBOX_CHOWN.to_string(),
SANDBOX_HOME.to_string(),
]),
attach_stdout: Some(true),
attach_stderr: Some(true),
..Default::default()
},
)
.await
.map_err(|e| AgentError::SandboxCreationFailed {
run_id: config.run_id,
provider: "docker".to_string(),
reason: format!("Failed to create chown exec: {}", e),
})?;
let _ = self
.docker
.start_exec(
&exec.id,
Some(bollard::exec::StartExecOptions {
detach: false,
..Default::default()
}),
)
.await;
}
self.run_root_exec(
&container.id,
config.run_id,
"chown-home",
vec![
"chown".to_string(),
"-R".to_string(),
SANDBOX_CHOWN.to_string(),
SANDBOX_HOME.to_string(),
],
)
.await?;

// Fix work-dir ownership: the bind-mounted host directory carries
// its host-side uid into the container, regardless of the image's
Expand All @@ -1254,41 +1330,18 @@ impl SandboxProvider for DockerSandboxProvider {
// Inside the container the sandbox user then can't write — TUIs
// fail, dev servers can't open lockfiles, etc. Mirror the home-dir
// fix: recursively chown as root, every start.
{
let exec = self
.docker
.create_exec(
&container.id,
bollard::models::ExecConfig {
user: Some("0:0".to_string()),
cmd: Some(vec![
"chown".to_string(),
"-R".to_string(),
SANDBOX_CHOWN.to_string(),
CONTAINER_WORK_DIR.to_string(),
]),
attach_stdout: Some(true),
attach_stderr: Some(true),
..Default::default()
},
)
.await
.map_err(|e| AgentError::SandboxCreationFailed {
run_id: config.run_id,
provider: "docker".to_string(),
reason: format!("Failed to create workspace chown exec: {}", e),
})?;
let _ = self
.docker
.start_exec(
&exec.id,
Some(bollard::exec::StartExecOptions {
detach: false,
..Default::default()
}),
)
.await;
}
self.run_root_exec(
&container.id,
config.run_id,
"chown-work",
vec![
"chown".to_string(),
"-R".to_string(),
SANDBOX_CHOWN.to_string(),
CONTAINER_WORK_DIR.to_string(),
],
)
.await?;

// Ensure AI CLIs are present in the home volume. Named volumes
// persist across image rebuilds and mask the image's home dir,
Expand Down Expand Up @@ -1323,34 +1376,27 @@ impl SandboxProvider for DockerSandboxProvider {
chown = SANDBOX_CHOWN,
user = SANDBOX_USER,
);
let exec = self
.docker
.create_exec(
// Best-effort: a missing AI-CLI restore shouldn't fail the
// whole sandbox creation (the bind-mount + chown above are
// already what unblocks `git pull` and the sandbox terminal),
// so we log the exit code via run_root_exec but ignore
// non-zero results. The helper itself emits a tracing::error
// line so the failure is still visible in logs.
if let Err(e) = self
.run_root_exec(
&container.id,
bollard::models::ExecConfig {
user: Some("0:0".to_string()),
cmd: Some(vec!["sh".to_string(), "-c".to_string(), restore_script]),
attach_stdout: Some(true),
attach_stderr: Some(true),
..Default::default()
},
config.run_id,
"ai-cli-restore",
vec!["sh".to_string(), "-c".to_string(), restore_script],
)
.await
.map_err(|e| AgentError::SandboxCreationFailed {
run_id: config.run_id,
provider: "docker".to_string(),
reason: format!("Failed to create claude-fix exec: {}", e),
})?;
let _ = self
.docker
.start_exec(
&exec.id,
Some(bollard::exec::StartExecOptions {
detach: false,
..Default::default()
}),
)
.await;
{
tracing::warn!(
"Sandbox {} ai-cli-restore step failed to launch: {} — continuing",
container.id,
e
);
}
}

tracing::info!(
Expand Down
Loading