Repository navigation
fix(collect): read only the agent's own regular files, capped in size and time - #607
Open
earakely-scale wants to merge 4 commits into
Open
earakely-scale wants to merge 4 commits into
earakely-scale wants to merge 4 commits into
Conversation
… and time A manifest entry is read only under base_path, and its name is stored as a key with no separators or dot segments. On a local sandbox's host, which is this machine, a file is read only where it resolves inside the sandbox's work dir or base_path; sandboxes declare that folder as app_dir_on_this_machine. Only regular files are read, links followed, up to the step's max_bytes (16 GiB by default). The sandbox sends at most the size it reported, the decoder refuses more, and a read past its deadline is abandoned. Modal exec output streams instead of being read whole, and the CUA path applies the same checks. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Each collected key becomes a clean relative member name: backslashes separate, drive prefixes, leading separators and '.' segments are dropped, and '..' climbs no higher than the archive root. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…n, time the whole read Under /app only the sandbox's work dir counts as a root: an agent could swap a folder there for a link out. A base_path outside /app still counts, resolved. Two manifest names that clean to one key get "key (2)" and on. The deadline covers exec too, which some providers run to completion. Manifest paths are matched against base_path normalized. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ranslates stat's %F names the file type in the system's language, so on a host with a non-English locale every file would have been refused. %f prints the raw mode, checked with stat.S_ISREG. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This branch has not been deployed
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.
Why
collect_artifactscopies what an agent leaves behind onto the operator's side. Three flaws let the agent choose more than its own deliverables:manifest_step_id) the paths come from the agent's own structured output. Every absolute path was read, insidebase_pathor not.sandbox_namealone) runs on the operator's machine. So a manifest naming~/.ssh/id_rsaor~/.aws/credentialscopied that file into the run's object store.base_pathdid the same.../../.bashrcbecame an archive entry that climbs out of the folder it's extracted into, holding bytes the agent wrote.base64 < pathuntil the sandbox closed the stream, and its size was checked only afterwards./dev/zeroor an 8 TiB sparse file streamed onto the operator's disk without end, and a FIFO hung the step.What changes
base_pathonce normalized. Any other entry is skipped with a warning..or..segments, or the file's name if nothing is left.../../.bashrcis stored as.bashrc, andlogs/run.jsonaslogs_run.json. Names that clean to the same key getkey (2),key (3)and so on.base_pathnormalized.readlink -f) inside the sandbox's work dir, or insidebase_pathwhen the task names a folder outside/app. It reads the resolved path. Anything else, a link out included, is a hard failure for that file.app_dir_on_this_machine: the folder on this machine its host commands see as/app. It's None by default, and the local sandbox's work dir. Collect asks the sandbox for it rather than checking its class.ON_THIS_MACHINEdoesn't answer the question, because a smol VM sandbox sets it but runs its commands inside a VM.base_pathoutside/appstays readable, because a run_code script on a local VM sandbox writes to host folders the task chooses, and a task author can already run commands on that host. Under/apponly the work dir counts, since the agent could swap a folder there for a link.stat -L -c %f:%s, and the raw mode bits it prints are checked withstat.S_ISREG;%F's type names are translated by locale. Anything but a regular file is refused before it's read: a device, FIFO, directory or socket, or a link to one.max_bytesis refused too.max_bytesdefaults to 16 GiB, andto_dict/from_dictcarry it.head -c SIZE < path | base64, so it sends no more than the size it reported, and the decoder refuses any byte past that size.execincluded, is abandoned once it outlasts 300 s plus one second per MiB. Some providers run the whole command insideexec._ModalStreamAdapteris now async-iterable. Modal and Modal VM output is decoded to disk as it arrives instead of being read whole. Providers whose SDK returns the output whole are still bounded by the cap.[ -f ], links followed). A file overmax_bytesis refused before the first read..segments are dropped;..climbs at most to the archive root;artifact.Compatibility
base_path: they used to be collected, keyed by their file name; now they're skipped. A task whose agent reports deliverables in several folders needs abase_paththat holds them all./:logs/run.jsonis now keyedlogs_run.json.base_pathis no longer followed.max_bytesset on its step. The default sits well above the files collected in practice.headin the sandbox: the read now also runshead, from the same coreutils that already providebase64.Tests
base_path, or climbing out with.., is skipped;max_bytesis a hard failure and is never read;max_bytesround-trips;/dev/zero, a FIFO or a directory, and a file over the cap is refused before any read;base_pathis read;base_pathunder/appthat the agent swapped for a link to a home folder reaches nothing outside the work dir;base_pathis matched normalized;execthat never returns is held to the deadline;../../x,/abs/../../y, backslashes and drive prefixes.report.pdfis a link to/dev/zerobase_pathenumeratedmax_bytes; the rest collected/app/artifact/../..statwithout-Lsized the link itself../../.bashrc,..\..\evil.sh.bashrc,evil.sh../../x,/abs/../../y../../.bashrc,..\..\evil.sh,../../x,abs/../../y.bashrc,evil.sh,x,ypython:3.12-slimsandbox, collected throughModalSandbox's exec adapter:/dev/zero, a FIFO and an 8 TiB sparse file were each refused in 0.4 s.Not covered here
🤖 Generated with Claude Code


Confidence Score: 5/5
The PR appears safe to merge, with no new actionable issues found.Summary
collect_artifactslimits which files agents can deliver and bounds their reads.Diagram
%%{init: {'theme': 'neutral'}}%% flowchart LR A[Choose files] --> B[Check paths and names] B --> C[Check regular file and size] C --> D[Read within size and time limits] D --> E[Upload and register artifacts] E --> F[Download with safe archive names]Reviews (7) · Last reviewed commit: "fix(collect): tell a regular file by its..." · Reviewed by Greptile