fix(sandbox): make tool decisions host-authoritative - #329
Open
Jake Present (jakepresent) wants to merge 3 commits into
Open
fix(sandbox): make tool decisions host-authoritative#329Jake 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 18:44
Two hardening fixes on the trust boundary this PR introduces. RemoteActionMediator defaulted an untracked executor to real_executed=True. That value drives execution_status in the trusted ledger, which is the strongest claim in the evidence contract: whether a consequential action actually ran. An executor that never proved it executed should not be reported as having executed. Not reachable today (AgentHooksToolHost always supplies a tracking executor), but it is a fail-open default on the exact boundary this PR exists to close. The run_turn fail-closed gate re-derived 'did the host mediate this turn' by string-matching evidence_source inside a rendered evidence message. That value is known structurally at drain time, so matching it back out couples a security gate to a presentation detail: a later change to the evidence shape could disable the gate with every test still green. Record the count of drained host ledger rows instead. Also fixes an existing test that passed a bare lambda where production passes a tracking executor; it now mirrors the real call shape. Verified by sabotage: restoring the fail-open default fails the new execution-claim test, and both breaking the gate and dropping the counter fail the new gate test. 1346 passed, 23 skipped, 840 subtests outside the viewer suite. The 29 viewer failures are the pre-existing missing viewer/node_modules gap and reproduce identically on the unmodified PR head.
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.
Problem
The stock sandbox currently trusts the evaluated target to choose pass/mock/block and return its own action evidence. Read-only policy files do not make that evidence authoritative: target code can omit or forge endpoint events, and the writable target ledger is not an independent record.
Approach
Add an explicit
target.host_action_mediation: truepath for stock containers:AgentHooksToolHostthroughRemoteActionMediatorover the existing narrow relay;The evidence contract is intentionally split:
target_reportedandresult_authoritative: false;Existing endpoint setups and containers that do not opt in keep the current local/target-reported path.
Review hardening
Adversarial review probes found and this head fixes:
Tests
135 passed, 4 skipped186 passed1391 passed, 24 skipped, 840 subtests passedlookup_customerandsend_message;target_reported;host_mediator;A fresh stock-image build was blocked locally by an external
files.pythonhosted.orgTLS handshake failure while pip fetched build requirements. The real-Docker regression is included intests/test_sandbox_runtime_docker.py; it should run in the dedicated Docker workflow from #328 once this stack is rebased onto it.Stack and landing
This PR is intentionally stacked on #327 because stable ASSERT case identity is part of the host ledger contract. After #327 merges, rebase this branch onto
mainand change the base.Related parallel fixes:
Recommended landing order: #328, #326, #327, then this PR with the Docker workflow rerun against its exact head.
Deliberate follow-up
This PR does not move consequential real tool implementations and credentials behind a host-side adapter. That stronger boundary would make passed execution independently authoritative, but it changes the customer onboarding/tool-packaging contract and should be reviewed as a separate product and architecture extension.