[JacobStephens2]

Summary

The release summary Session's sandbox used to bind every host toolchain root (~/.cargo, ~/.rustup, ~/.nvm and their env overrides) and the skills folder. That broke Spec #2054's promise of no host filesystem read access. It is now sealed. Other Sessions keep today's binds.

 pub(crate) struct Request<'a> {
-    pub skills_dir: &'a Path,
+    pub host: Host<'a>,   // Tools { skills_dir } | Sealed
 }

 prepare(request)
-  bind skills_dir; toolchains_into(home)
+  match request.host
+    Tools { skills_dir } => bind skills_dir; toolchains_into(home)   // Trials, Graders, Process sessions
+    Sealed               => node_into(harness binary)                // release summary
   ...
-  conceal_toolchain_credentials
+  if Tools: conceal_toolchain_credentials

node_into reads the Harness's #! line:

• #!/abs/node: binds that one file read-only at that path.
• #!/usr/bin/env node: binds only the resolved host node at its canonical path, with its directory first on PATH.
• Anything else (native binary, other interpreter): binds nothing.

If node can't be resolved, prepare fails with Unavailable, so the existing fallback (patch plus GitHub generated notes) applies. The binds are never widened.

ADR 0013's exception and the manual are unchanged, as the issue's Decision asks.

Evidence

Execution evidence

Cause: src/release/summary.rs write built its sandbox with sandbox::prepare, which always called toolchains_into. That bound every existing toolchain root read-only, so the summary Harness could read anything under ~/.nvm.

• Before (at ea4bd94f, with the issue's test added): cargo test --locked --test release -- release_summary_sandbox_binds_no_host_toolchain_directories

  thread 'native::release_summary_sandbox_binds_no_host_toolchain_directories' panicked at tests/release/native.rs:1581:5:
  summary Session read a host toolchain file: ... Host file readable ...
  test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 148 filtered out
  

  After: the same command:

  test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 148 filtered out; finished in 0.44s
  

• npm-installed Harness: cargo test --locked --test release -- native::release_summary_ runs both release tests:

  test native::release_summary_sandbox_binds_only_node_for_an_npm_installed_harness ... ok
  test native::release_summary_sandbox_binds_no_host_toolchain_directories ... ok
  test result: ok. 2 passed; 0 failed
  

  If write is switched back to the old binds (Host::Tools), both tests fail.
• Unit tests: cargo test --locked --bin thirdshift sandbox::tests gives 30 passed. This includes a_harness_names_node_only_in_its_interpreter_line and a_sealed_harness_binds_its_node_alone_or_fails; the second covers an unresolvable node failing with needs node.
• Affected tests: scripts/affected-tests main selected the full suite.
  • On the first commit: PASS cargo test (774.60s).
  • After the review fixes: two unrelated tests failed intermittently, each in a different run, and pass on rerun:
    • experiments release::native_release_waits_for_leak_decisions_before_any_push failed with "Experiment is already running … lock acquisition failed"; it then passed 3 of 3 times.
    • process findings::fixing_dispatches_each_reproduced_finding_and_keeps_unreproduced_findings_private passed 2 of 3 times. It runs through sandbox::for_trial, which keeps the unchanged Host::Tools path.
  • Every other target in cargo test --locked --no-fail-fast passed.

Visual evidence

None: no UI changes.

Merge Danger

Door: two-way

Reverting restores the old binds.

Blast Radius: release-summaries

Only the release summary Session changes. Its worst case is a Harness whose node can't be resolved: release summaries then fall back to patch plus GitHub generated notes instead of a written summary. Trials, Graders and Process sessions use Host::Tools and are unchanged.

Unaddressed findings

Standards

• ADR 0013 Exception (agy release summaries can still read local files #653) doesn't mention the node bind. Declined. The issue's Decision (Release summary sandbox can read host toolchain directories #2230) asks for exactly this bind and says: "Keep the ADR 0013 exception and the manual's wording as they are, since the code now matches them." The Day shift has already made the call, so no new issue was filed.

Spec

None.

Review coverage: both reviewers read every changed file (src/release/summary.rs, src/sandbox.rs, tests/release/native.rs).

Closes #2230

Built with claude 2.1.314 · opus · default effort

Security review outcome

No unaddressed introduced findings.