fix(sandbox): keep egress evidence outside target mount - #326
Open
Jake Present (jakepresent) wants to merge 3 commits into
Open
fix(sandbox): keep egress evidence outside target mount#326Jake Present (jakepresent) wants to merge 3 commits into
Jake Present (jakepresent) wants to merge 3 commits into
Conversation
Jake Present (jakepresent)
requested review from
Aaron Aspinwall (AaronAspinwall123),
Chang Liu (changliu2) and
tangym
as code owners
August 20, 2026 05:05
The host-only property was asserted two ways, and neither covered the
general case on the ungated path:
- the unit test pinned the audit path to one hardcoded tmp_path layout,
so it proves the current caller's arrangement but not the invariant;
- the general mount check lives in test_sandbox_runtime_docker.py, which
is ASSERT_RUN_DOCKER_TESTS-gated and skipped in normal CI.
Parse the real -v arguments and assert the ledger is not inside any of
them. Verified this catches a leak the existing assertions miss: adding a
mount of output_dir.parent leaves the audit path unchanged (so the
equality assert still passes) while placing the ledger inside a target
mount, and only this check fails.
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.
Problem
The stock sandbox wrote host-proxy network evidence under the same directory mounted read/write into the evaluated target. A target could overwrite or delete
egress.jsonlbefore ASSERT consumed it.Fix
audit/directoryoutput/directory/sandbox/output/egress.jsonlcannot alter the host ledgerThis PR fixes network-ledger mutability only. Moving tool-action capture out of the evaluated process is a separate architectural change.
Verification
out/egress.jsonlinstead of host-onlyaudit/egress.jsonl).74 passedacross focused sandbox runtime/example/hostile-input tests.1370 passed, 24 skipped, 840 subtests passedacrosstests/after installing the viewer's locked dependencies./sandbox/output, while the host ledger remained unchanged and its directory was absent from target mounts.Risk
Low and scoped to the stock container runtime's temporary-directory layout. Both directories remain owned by the same per-case temporary session and are removed together during session cleanup.