Repository navigation
fix(sandbox): stage and read files through one owned path - #378
Merged
Merged
Conversation
sudoTomas
force-pushed
the
fix/sandbox-file-writes
branch
2 times, most recently
from
November 5, 2026 17:27
1e91aef to
1871189
Compare
embry0 moved files into and out of a job's sandbox with docker cp at fixed paths. docker cp runs in the daemon as root and follows a link at the path it is given, so those paths were not a safe place for embry0 to write or read as root; and a staged file kept the orchestrator's uid and a 0600 mode, so a delivery only worked while the orchestrator and the agent shared a uid (#361). Add embry0/execution/sandbox_files.py as the single place that moves bytes across the sandbox boundary. Copy in (put_bytes_for_reader): write the bytes as root, from stdin, into a new directory under /run/embry0/drop that only root may write, so nothing in the sandbox can pre-place the destination. The file is created O_EXCL and no-follow and left root-owned and readable through its other bits; the sandbox drops every capability, so root cannot chown it, and a chowned file would be agent-writable. A caller that must land it at an agent path copies it there as the agent with cp --remove- destination, which never follows a link at the destination. Copy out (read_regular_file): read as the agent, not root. The agent only ever gets bytes it could already read, so a path that resolves to a file it cannot read — however the parent directories are arranged, and even if a parent is swapped mid-read — yields nothing new. The read is O_NOFOLLOW|O_NONBLOCK (a FIFO or device never blocks it), requires a regular file, and refuses a file past the caller's byte limit; the orchestrator enforces the limit too, reading the pipe in chunks and killing the read once it carries more, so another agent process writing into it cannot make embry0 buffer an unbounded amount. Each refusal has a distinct reason for permission, link, not-regular, too-large and missing. Every helper runs python3 -I with the working directory /. The deliverables listing is read as raw, capped, NUL-delimited bytes; every path is refused unless it stays under the deliverables directory, and a name with a control character is skipped; the per-file and total byte caps are enforced by the reader. Each drop is removed once its reader is done. copy_into / copy_bytes_into / copy_bytes_from are removed; run_cmd_bytes returns a read's raw bytes; a guard test refuses a docker exec as anyone but the agent, docker cp and the archive API outside a short per-function allowlist. No image rebuild: the changed code runs only in the orchestrator, and agent_runner imports sandbox_files lazily so the sandbox runner's import surface is unchanged. Fixes: #361
sudoTomas
force-pushed
the
fix/sandbox-file-writes
branch
from
November 5, 2026 18:21
1871189 to
8a13939
Compare
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
Move every file embry0 puts into or reads out of a job's sandbox through one module,
embry0.execution.sandbox_files, instead ofdocker cpat fixed paths. A copy in is written as root into a fresh root-only directory; a copy out is read as the agent. Fixes #361.Motivation — Intent + Problem
embry0 stages files into a sandbox (a delivery's file, a resumed session, an http-context file) and reads files back out (a session blob, a boot screenshot, a job's deliverables). It did this with
docker cpat fixed paths.docker cpruns in the daemon as root and follows a link at the path it is given, so a copy-in could overwrite a file the agent cannot write and a copy-out could read one it cannot read.0600mode, so a delivery only worked while the orchestrator and the agent shared a uid.cap_drop: ALLroot has noDAC_OVERRIDE, and the CLI writes its session file0600inside0700agent-owned directories — so reading as root failed, every resume fell back to the bounded text replay, and0600deliverables were dropped. This revision reads as the agent, which also closes the copy-out attack surface.How it works
put_bytes_for_reader. Writes the bytes as root, from stdin, into a freshly made/run/embry0/drop/<random>/that only root may write, createdO_EXCL | O_NOFOLLOW. The file is left root-owned and readable through its other bits — the sandbox drops every capability, so root cannotchownit, and a chowned file would be agent-writable. A caller that must land it at an agent path copies it there as the agent withcp --remove-destination(place_as_reader_script), which never follows a link at the destination.read_regular_file. Reads as the agent, not root: the agent only ever gets bytes it could already read, so a path that resolves to a file it cannot read — however the parent directories are arranged, and even if a parent is swapped mid-read — yields nothing new. The read opensO_RDONLY | O_NOFOLLOW | O_NONBLOCK(no final-link following; a FIFO or device returns at once instead of blocking), requires a regular file byfstat, and refuses a file past the caller's byte limit — enforced again on the orchestrator:run_cmd_bytesreads the pipe in chunks, never asks it for more than the limit plus 64 bytes, and kills the exec past that, so another agent process writing into the reader's stdout cannot make embry0 buffer an unbounded amount. It raises a distinct reason for permission-denied, link, not-regular, too-large and missing. Every helper runspython3 -Iwith the working directory/.discard_drop, which acts only on a normalised path of the exact shapeDROP_ROOT/<32 hex>/<name>); the http body is not stored twice.DockerClient.copy_into/copy_bytes_into/copy_bytes_fromare removed;run_cmd_bytesreturns a read's raw bytes. The guard test enforces the three invariants below.Pattern provenance
N/A — originated in this PR (the root-owned-drop + stdin write mirror the git-config staging added in #353).
Verification
Automated
-n 4 --fail-on-warnings, Docker unreachable: on642c002e18529 passed, 21 skipped, 0 failed; rebased onto current mainb3457a8c18608 passed, 21 skipped, 0 failedruff check .andruff format --check .on the whole tree: cleanmypy embry0: no issues in 652 source filesorigin/main; this round's tests against the previous head — the orchestrator-side cap (a fake process streaming 8 MiB past a 1 KiB cap, and a real one), the read's slack and post-check, the flooded stderr, the exact-shapediscard_drop, the control-character and trailing-space names, the capped listing, and the real-container collector all fail there.too_large, the listing uncapped,discard_dropwithout its token check / withoutnormpath/ back to a prefix check, control characters allowed, the listing stripped, the under-directory check off; and against the real container,-printfor-print0, a listing split on newlines, control characters allowed, and a read as root. Earlier rounds:DROP_ROOTunder/tmp, a write withoutO_EXCLor following a link, copy steps or copy-out as root, a read following a link / blocking on a FIFO / skipping the regular-file check / dropping the limit.user=/reader=other than"agent", auser/readerkey in a dict literal (what**carries), and any-u/-u<x>/--user/--user=/--privilegedargv element or shell substring are refused;docker cp(incl.docker container cp) and the archive API are refused everywhere; theruntime/docker.pyexemption ischown_volume_cmdonly. Of a 26-form battery (the reviewer's 11 misses among them) the previous guard caught 13 and this one catches 26; every guard detector is mutation-killed.Container e2e (
tests/integration/test_sandbox_files_e2e.py,requires_docker, runs in CI's Integration job; production flags--cap-drop=ALL --security-opt=no-new-privileges, container default user the agent)0600file an unprivileged agent wrote in a0700directory (the session-file and private-deliverable shape) is read back out — root cannot read it, the agent can._collect_artifacts: over a deliverables directory holding a private report, a name ending in a space, a newline-named directory, a symlink to/root, a FIFO, and a newline-named path whose part after the newline names an agent-readable file outside the directory (reachable only through a symlinked directoryfindnever descends), the collector uploads exactly the report and the trailing-space file — never the root canary or the outside file — and nothing blocks.read_regular_filerefuses the planted paths directly: through the linkpermission_denied, at the linklink, at the FIFOnot_regular(at once).python:3.12with anagentuser, so the test needs no embry0 image.Deploy notes
agent_runner.py,docker_client.pyandembry0/agents/are copied into the sandbox image (Dockerfile.sandbox), but the code paths this PR changes run only in the orchestrator, so a stale sandbox image is harmless.agent_runnerimportssandbox_fileslazily, so the sandbox runner's import surface is unchanged (tests/unit/sandbox/test_runner_import_surface.pystill passes). A standalone deployment that builds its own sandbox image rebuilds on its next start with no behaviour change.MAX_ARTIFACT_TOTAL_BYTESandMAX_LISTING_BYTES(deliverables) andMAX_SCREENSHOT_BYTES. New failure codeERR_SANDBOX_COPY_OUT_REFUSED.Fixes #361