fix(v1): enforce artifact transport boundaries - #2464
Open
xeophon wants to merge 1 commit into
Open
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Contributor
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — This production-path change hardens artifact transfer across the solver, host, and grading-runtime trust boundary while changing which archived filesystem entries are accepted. The resulting security and compatibility impact across Harbor and agentic judging warrants careful human review. You can add or adjust custom eligibility rules. Learn more. |
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.
Overview
This change makes grading artifact transport enforce the two boundaries its API already expresses: collection has a fixed host-transfer budget, and each archive may restore only regular files and directories beneath its declared artifact root.
The implementation stays inside the shared v1 artifact transport layer, so Harbor separate verification and isolated agentic judging receive the same behavior without provider-specific handling or interface changes.
Trust boundary
Artifact transport crosses three ownership domains:
The archive cannot be treated as trusted merely because framework code invoked tar. The solver controls the source filesystem and may leave processes running or alter runtime tooling. The host therefore treats the resulting bytes as untrusted input and validates them before allowing any grading-runtime mutation.
Bounded transfer
Collection now passes the remaining rollout artifact budget directly to Runtime.read through max_bytes.
This removes the separate size-probe and read sequence. The byte ceiling is enforced by the same operation that transfers bytes out of the runtime, so replacing or growing the temporary archive between two operations cannot turn a checked archive into an unbounded host read.
The existing aggregate budget remains unchanged: each collected archive consumes from MAX_ARTIFACT_BYTES, and the next archive receives only the remaining budget.
Archive preflight
Restore performs a complete host-side preflight of every collected root and archive before clearing, writing, or extracting anything in the grading runtime.
Declared roots must:
Every tar member must:
Malformed tar data is rejected. Symbolic links, hard links, devices, FIFOs, sockets, and other special member types are rejected as one explicit transport policy. This deliberately favors a small, auditable restoration contract over reproducing every filesystem feature across trust boundaries.
Restore ordering
All archives are validated before the first root is cleared. A rejected archive therefore leaves every grading-runtime root untouched, including roots belonging to otherwise valid archives earlier in declaration order.
Once preflight succeeds, restoration retains the existing ordering semantics:
Clearing all roots first remains important for overlapping declarations: a later nested root cannot delete content that an earlier archive has already restored.
macOS archive behavior
Archive creation sets COPYFILE_DISABLE=1. The macOS tar implementation otherwise adds AppleDouble sidecar entries next to a directory root, placing generated metadata outside the root that was declared for transport. Disabling those sidecars keeps macOS collection consistent with the same root-containment contract used by Linux runtimes.
The environment variable is harmless for tar implementations that do not implement AppleDouble metadata.
Compatibility and interfaces
The public Artifact, collect, restore, task-state, Harbor, and agentic-judge interfaces are unchanged. Archive storage remains an ordered mapping from absolute source root to tar bytes or an optional-missing marker.
The intentional behavioral restriction is that links and special files are no longer transportable artifacts. Tasks that need linked content must publish regular files and directories, or materialize the required target content during finalization.
Scope
The change is contained to verifiers/v1/utils/artifacts.py. It does not add dependencies, alter runtime providers, change artifact size limits, change grading lifecycle ownership, or introduce a second transport implementation.
Note
High Risk
Changes security-critical artifact handling across the solver–host–grading trust boundary; incorrect validation could allow path escape or unsafe extraction, while stricter rules may break tasks that relied on symlinks in artifacts.
Overview
Hardens v1 artifact collect and restore so untrusted tar bytes from the solver runtime cannot bypass size limits or write outside declared roots.
Collection passes the remaining rollout budget into
runtime.read(..., max_bytes=budget)instead of a separatewc -ccheck, closing a race where the archive could grow between probe and read. macOS collection setsCOPYFILE_DISABLE=1so AppleDouble sidecars are not packed beside directory roots.Restore runs host-side preflight (
_validate_restore) on every root and archive before any grading-runtimermor extract: roots must be absolute paths under/(not/itself); each tar member must stay under its root with no.., and only regular files and directories are allowed—links and special entries fail fast. Malformed archives surface asRuntimeError.Reviewed by Cursor Bugbot for commit 5f9b702. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Enforce artifact transport boundaries in v1 restore and
_tar_out_validate_restorein artifacts.py to host-side validate tar archives before extraction: rejects absolute member paths,..traversal, links/special files, and malformed or unreadable archives withRuntimeErrorrestore()now calls_validate_restoreon each root/archive pair before any destructive operation_tar_outprependsCOPYFILE_DISABLE=1to avoid AppleDouble sidecar files and replaces the separate size probe with a singleruntime.read(path, max_bytes=budget)callrestore()now rejects archives it previously accepted; callers passing archives with absolute paths, traversal entries, or non-regular/non-directory members will receiveRuntimeErrorbefore extractionMacroscope summarized 5f9b702.