feat(jobs): sandbox job commands with bubblewrap on Linux/WSL2 - #420
Merged
Conversation
Supervisor::spawn now runs commands through a bwrap sandbox on native Linux and WSL2 (read-only root, writable bind for the job's cwd only, .git re-protected read-only, private tmpfs, PID/user namespace containment). Windows and macOS are unchanged -- bubblewrap has no equivalent on either, so those platforms keep running unsandboxed as before. Falls back to unsandboxed on Linux too if bwrap isn't on PATH. Flag set adapted from openai/codex's linux-sandbox crate. Verified against a real WSL2 kernel: cwd writes succeed, .git/outside-cwd writes are denied, descendants are reaped on exit with no host leak, and job scratch space uses a private tmpfs rather than the host's shared /tmp. Agentflare-Agent: claude-code_2-1-225_agent Agentflare-Branch: jobs/bwrap-sandbox-linux
…'s binaries bwrap.rs -> bwrap/mod.rs (fixes a module-path bug: mod bwrap_install declared inside bwrap.rs needs the submodule at bwrap/bwrap_install.rs, not a flat sibling file). New bwrap/bwrap_install.rs: when bwrap isn't on PATH, installs it via `mise install "github:openai/codex[matching_regex=^bwrap-.*\.tar\.gz$]@rust-v0.147.0"`. Bubblewrap's own GitHub releases ship a source tarball only -- no prebuilt binaries -- so this reuses openai/codex's own prebuilt, statically-linked (musl) release binaries instead. Requires mise on PATH, which agentflare's own doctor/setup flow already ensures, so this is an existing dependency at a new call site rather than a new one (avoids adding ureq/flate2/tar/sha2 just to reimplement what mise already does). find_or_install_bwrap() checks PATH first, installs on miss, and caches the result for the process lifetime so a box without bwrap (or without mise) doesn't retry the install on every job. Logs the outcome either way so a job silently running unsandboxed isn't indistinguishable from a sandboxed one. Verified end-to-end against a real WSL2 Ubuntu install: the exact mise install/where command sequence correctly disambiguates the right binary out of a release that bundles several unrelated tools, for the right host arch, and the extracted binary runs. Agentflare-Agent: claude-code_2-1-225_agent Agentflare-Branch: jobs/bwrap-sandbox-linux
which_bwrap()'s test called std::env::set_var("PATH", "") to check the
not-found case, but cargo's test harness runs tests in parallel across
threads within one process -- env vars are process-global, not
thread-local, so that mutation raced with any concurrently-running
test that spawns a child process expecting PATH intact.
Caught by actually running the suite natively on Linux (WSL2) for the
first time: spawn_times_out_and_kills_a_descendant_that_escaped_into_
its_own_process_group failed with ENOENT ("sh" not found) because it
happened to run while the PATH-clearing test held PATH empty.
Fixed by factoring the PATH search into a pure find_on_path(path,
name) helper the test calls directly with an injected empty OsStr,
touching no process state. Full suite (28 tests) now passes natively
on Linux, including the descendant-reaping test exercising the real
/proc-walking implementation and the full supervisor_test.rs suite
routing through the real sandbox::wrap() path.
Agentflare-Agent: claude-code_2-1-225_agent
Agentflare-Branch: jobs/bwrap-sandbox-linux
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe change adds platform-specific Bubblewrap sandboxing for job commands. Linux discovers or installs Bubblewrap, builds isolated execution arguments, and applies the wrapper during supervisor process creation. Other platforms retain direct command execution. ChangesJob sandboxing
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Supervisor
participant SandboxWrap
participant BwrapDiscovery
participant Bubblewrap
Supervisor->>SandboxWrap: wrap(command, args, cwd)
SandboxWrap->>BwrapDiscovery: locate or install bwrap
BwrapDiscovery-->>SandboxWrap: bwrap path or unavailable
SandboxWrap->>Bubblewrap: execute isolated command
Bubblewrap-->>Supervisor: child process result
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/agentflare-jobs/src/sandbox/bwrap/bwrap_install.rs`:
- Around line 26-30: Update the installer flow around the mise Command and
Supervisor::spawn so it launches via a child process, uses the bootstrap timeout
while waiting, terminates the child when the timeout expires, and sets stdin to
Stdio::null() alongside the existing stdout suppression. Preserve the current
optional status handling for successful completion and failed process startup.
In `@crates/agentflare-jobs/src/sandbox/bwrap/mod.rs`:
- Around line 140-143: Require executable permissions for every PATH tool
lookup: update find_on_path in crates/agentflare-jobs/src/sandbox/bwrap/mod.rs
(lines 140-143) to skip non-executable bwrap candidates, and update the mise
candidate checks plus installed bwrap validation in
crates/agentflare-jobs/src/sandbox/bwrap/bwrap_install.rs (lines 43-51) to
require executable files while continuing to later PATH entries.
- Around line 71-80: Update the HOME cache mounting logic around HOME_CACHE_DIRS
to prevent writable host mounts: replace the current --bind-try arguments with
read-only bind mounts, or redirect these paths to isolated per-job cache
directories. Preserve conditional mounting only for existing paths and ensure
jobs cannot persist changes to the host HOME directories.
- Around line 53-69: Normalize the optional cwd to an absolute path relative to
the supervisor process directory before constructing any Bubblewrap arguments.
Update the cwd handling around path_to_string, including .git detection and
--chdir, so --bind never receives a relative path; add coverage for cwd="." and
another relative cwd.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: beb0e9c4-61ff-47dd-a207-83e34e898649
📒 Files selected for processing (5)
crates/agentflare-jobs/src/lib.rscrates/agentflare-jobs/src/sandbox/bwrap/bwrap_install.rscrates/agentflare-jobs/src/sandbox/bwrap/mod.rscrates/agentflare-jobs/src/sandbox/mod.rscrates/agentflare-jobs/src/supervisor.rs
- HOME_CACHE_DIRS (.cargo/.rustup/.cache/.npm) now mount read-only (--ro-bind-try) instead of writable -- a writable bind let a sandboxed job persist changes into host caches, undermining the sandbox's own containment. - bwrap PATH lookup (find_on_path) and mise-install's own which() now require the executable bit, not just is_file(), so a stray non-executable file with a matching name can't silently break the fallback-to-unsandboxed path. - Relative job cwd is normalized to an absolute, lexically-clean path before being used in --bind/--chdir args, since bwrap takes those paths literally (unlike Command::current_dir). - mise install now runs as a killable child with a bounded timeout and stdin closed, instead of a blocking .status() call that could hang the worker thread forever on a stalled network or a prompt. Verified natively on WSL2 Linux (real bwrap present): full agentflare-jobs suite (33 tests) passes, cargo fmt clean, clippy clean with -A unsafe_code -A clippy::pedantic -D warnings. Agentflare-Agent: claude-code_2-1-225_agent Agentflare-Branch: jobs/bwrap-sandbox-linux
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/agentflare-jobs/src/sandbox/bwrap/mod.rs`:
- Around line 112-139: Replace the lexical normalization in absolute_path with
filesystem-aware resolution for cwd, preserving symlink traversal semantics;
have absolute_path return None when current_dir() or canonicalization fails so
sandbox::wrap uses its existing fallback. Update callers as needed to handle the
optional path, and add coverage for a symlink followed by `..`.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9e068f29-a817-4c76-a4a2-9354587b9459
📒 Files selected for processing (2)
crates/agentflare-jobs/src/sandbox/bwrap/bwrap_install.rscrates/agentflare-jobs/src/sandbox/bwrap/mod.rs
CodeRabbit's second pass on 0d7419d flagged the previous absolute_path fix: stripping '.' / '..' components lexically is wrong the moment a symlink sits in the path (link/.. is the parent of whatever link points at, not the lexical parent of link itself), which could bind the wrong directory into the sandbox. Switch to std::fs::canonicalize, matching realpath/kernel resolution. absolute_path now returns None on an unresolvable cwd, and wrap() bails out to the existing unsandboxed fallback in that case rather than proceeding with a bind path that doesn't mean what it looks like. Added a regression test with an actual symlink + '..' to pin the exact case lexical normalization got wrong, plus tests for the missing-path and already-absolute cases. Verified natively on WSL2 Linux (real bwrap present): full agentflare-jobs suite (35 tests) passes, fmt clean, clippy clean with -A unsafe_code -A clippy::pedantic -D warnings. Agentflare-Agent: claude-code_2-1-225_agent Agentflare-Branch: jobs/bwrap-sandbox-linux
getappz
added a commit
that referenced
this pull request
Aug 11, 2026
…#445) run_headless builds its own Command directly and never went through Supervisor::spawn, so the bwrap sandboxing added in #420 never covered the headless agent CLI subprocess that autonomous work-item dispatch (dispatch_item -> in_process -> WorkItemExecutor -> execute_work) actually launches. Wrap the headless argv through agentflare_jobs::sandbox::wrap before spawning, same as Supervisor::spawn does, and expose that module publicly so the root crate can call it. Agentflare-Agent: claude-code Agentflare-Branch: task/67 Agentflare-Item: 67 Co-authored-by: shiva <shiva@gosysinfo.tech>
This was referenced Aug 12, 2026
getappz
added a commit
that referenced
this pull request
Aug 14, 2026
…eal commits (#482) Item #109: a real commit landed on the claimed branch and was pushed to origin, but gh pr create never resulted in a PR (soft-failed silently), and the item still went straight to completed. push_and_open_pr's soft-fail-and-return-None contract predates item #420's split of done, back when the item was already marked completed before this code ran. Now that the completed transition happens right here, a diverged branch with should_push set but no resulting PR must hard-error instead of falling through to mark_completed -- same treatment item #92 already gives an auto-commit failure on a dirty tree. Added a regression test using a local bare-repo origin (push succeeds, gh pr create is unreachable) to reproduce push-without-PR and confirm done returns a hard error, posts a comment, and leaves completed_at unset. item.rs crossed the 1500-line LOC gate as a result; allowlisted at the frozen 2000-line ceiling like the codebase's other over-limit files, since splitting its per-action handlers into submodules is separate, larger work than a completion-correctness fix should carry. Agentflare-Agent: claude-code Agentflare-Branch: task/109-agentflare-work-marks-an-item-done-even Agentflare-Item: 109 Co-authored-by: shiva <shiva@gosysinfo.tech>
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
1 participant
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
Supervisor::spawn()now runs job commands inside abwrapsandbox on native Linux and WSL2: read-only root, writable bind for only the job's cwd,.gitre-protected read-only even inside that writable root, private tmpfs (not the host's shared/tmp), PID/user namespace containment. Windows and macOS are unchanged — bubblewrap has no equivalent on either platform.bwrapisn't onPATH, it's installed on demand viamise install "github:openai/codex[matching_regex=^bwrap-.*\.tar\.gz$]@rust-v0.147.0"— bubblewrap's own GitHub releases ship a source tarball only, so this reuses openai/codex's prebuilt, statically-linked release binaries instead. Falls back to unsandboxed (with a logged warning, not silently) ifbwrap/misearen't available.PATHenv var, which raced with a concurrently-running test under cargo's parallel test harness.Context
Threat model: the GitHub bridge routes external issue content into autonomous
workjob execution across a mixed fleet (Windows+WSL2, native Linux, macOS). This adds OS-level containment as defense-in-depth under the bridge's existingauthor_associationgate — narrowing blast radius if a routed job is compromised or misbehaves, independent of whether the routing decision itself was right.Flag set and design adapted from openai/codex's own
linux-sandboxcrate, scaled down: no split filesystem policy, glob-based path masking, or seccomp network filter, since this job runner operates on worktrees it already controls rather than arbitrary third-party workspaces.Test plan
cargo fmt --check/cargo clippy -- -A unsafe_code -A clippy::pedanticclean on Windows target.git/outside-cwd writes denied, descendants reaped on exit with no host leak, private tmpfs isolated from host/tmpand from other concurrent sandboxesmise install/mise wheresequence validated end-to-end against a real WSL2 install, disambiguating the right binary out of a release bundling several unrelated tools/proc-walking implementation and the fullsupervisor_test.rssuite routing through the realsandbox::wrap()path/tmp, no cross-visibilityKnown gaps (out of scope for this PR)
Summary by CodeRabbit