Repository navigation
fix(sandbox): bake safe.directory into image and surface chown failures - #83
Merged
Merged
Conversation
Sandboxes on production hosts hit `fatal: detected dubious ownership` on `git pull` whenever the bind-mounted workspace inherited a uid other than the container's `temps` user (uid 1000) — common when the host-side temps server runs as root or when userns-remap is enabled on the Docker daemon. Two changes: 1. Dockerfile generator now runs `git config --system --add safe.directory /home/temps/workspace` while still root, so Git inside the sandbox trusts the workspace regardless of stat owner. Belt-and-suspenders against userns-remap and against the post-start chown failing. 2. Replaced three `let _ = start_exec(...)` sites that silently swallowed exec failures (home chown, work-dir chown, AI-CLI restore) 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 if they fail (was: silent broken sandbox); the AI-CLI restore stays best-effort but its exit code is logged.
… fix Adds a Fixed entry under [Unreleased] for PR #83 covering the two behavior changes: - `safe.directory` baked into sandbox image to avoid `git pull` dubious-ownership failures under userns-remap or root-owned mounts. - New `run_root_exec` helper surfacing chown / AI-CLI restore exec failures that were previously swallowed by `let _ = start_exec(...)`.
2 of 5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
git config --system --add safe.directory /home/temps/workspaceto the sandbox Dockerfile generator sogit pullinside a workspace doesn't fail withfatal: detected dubious ownershipwhen the bind-mounted host workspace comes in owned by a uid other than the container'stempsuser (uid 1000) — common when the temps server runs as root or when userns-remap is enabled on the host Docker daemon.let _ = start_exec(...)post-start sites inDockerSandboxProvider::create_sandbox(home chown, work-dir chown, AI-CLI restore) with a newrun_root_exechelper that drains the output stream, inspects the exit code, and logs stderr on non-zero. The two chown steps now fail sandbox creation if they fail, rather than leaving the user with a silently broken workspace; the AI-CLI restore stays best-effort but its exit code is logged.Why
Production sandboxes were hitting
git pullfailures inside the workspace terminal:Investigation showed
/home/temps/workspacewas still owned by0:0(root) inside the container despite the post-start chown logic — the chown exec was failing silently because of the swallowedlet _ =. This change both prevents the symptom (Git trusts the workspace either way) and surfaces the underlying cause (chown failures now fail sandbox creation).Deploy notes
The
safe.directorybake-in only takes effect after sandbox images are rebuilt and pushed:The
run_root_execchange ships with the nexttempsserver release.Test plan
cargo check --lib -p temps-agentspassescargo test --lib -p temps-agents --no-runcompiles testsgit pullin the workspace — confirm no dubious-ownership error