Skip to content

CI: hermetic spawns in vlt test commons (#823) - #1488

Merged
Mikola Lysenko (mikolalysenko) merged 3 commits into
mainfrom
agent/fix-ci-hermetic-vlt-commons
Oct 13, 2026
Merged

Mikola Lysenko (mikolalysenko) merged 3 commits into
mainfrom
agent/fix-ci-hermetic-vlt-commons

Conversation

@mikolalysenko

@mikolalysenko Mikola Lysenko (mikolalysenko) commented Oct 13, 2026 •

Copy link
Copy Markdown
Collaborator

LLM Description written by Claude Code:claude-opus-5-5

Refs #823 (slice 10 of the hermetic-spawn migration). The issue stays open for the 29 remaining PENDING_RAW_SPAWNS entries (26 once #1484 lands too).

Root cause

The three vlt test commons each started the socket-patch child with a bare Command::new and a private SOCKET_* sweep that had drifted from tests/common/hermetic.rs:

  • vlt_e2e_common::socket_cmd relied on cache_env::scrub_ambient_vlt_env, which sweeps the parent's SOCKET_* once but doesn't remove the flag-bound vars by name, so one set in-process after the sweep would reach the child, and it had no hostile seeds to catch a dropped scrub.
  • vlt_hosted_common::scrubbed_cli kept every ambient SOCKET_*TELEMETRY* var and never forced SOCKET_TELEMETRY_DISABLED=1, so a developer shell with SOCKET_TELEMETRY_DISABLED=0 let the unauthenticated child POST telemetry.
  • vlt_vendor_common::cli carried a hand-written list of five extra names to remove.

Change

  • socket_cmd, scrubbed_cli and cli now start from hermetic::command / hermetic::binary_command and keep only their own scrubs (vlt / npm / VIRTUAL_ENV for the e2e helper, VLT_* for the vendor helper, proxies for all three).
  • New cache_env::scrub_ambient_vlt_tool_env: the vlt scrub minus SOCKET_*, the counterpart of the existing scrub_ambient_bun_tool_env. Running the full vlt scrub after the hermetic builder would remove the opt-outs it forces on; the tool variant leaves SOCKET_* to the builder. It has a self-test.
  • The three files leave PENDING_RAW_SPAWNS (42 -> 39 on main), so the ratchet in spawn_env_hygiene.rs keeps them hermetic. The entries aren't adjacent to the ones CI: hermetic spawns in vendor build tests (#823) #1484 removes, so the two PRs merge cleanly in either order.

Files: vlt_{e2e,hosted,vendor}_common/mod.rs, common/cache_env.rs, spawn_env_hygiene.rs.

CI cost

Test code only, no workflow edits. A lean PR run keeps the same job count (~29 Linux jobs) and job-minutes (~31 job-min, ~8 min wall) before and after.

Test evidence (local)

  • Red: with the three list entries removed and the helpers reverted, spawn_env_hygiene::no_new_bare_binary_spawns fails. Green with the change.
  • cargo fmt --all -- --check: clean. cargo clippy --locked --workspace --all-features -- -D warnings (the CI command): clean. cargo check --tests -p socket-patch-cli --all-features: clean (every test target compiles).
  • The 28 test targets that include a vlt common, plus spawn_env_hygiene and e2e_vendor_bun_build (a cache_env includer): 2019 passed, 8 failed, 135 ignored. The same 8 fail on unmodified origin/main in this container. They're chmod-based write-failure tests (*_state_write_failure_*, *_write_failures_*, *_unremovable, *_cleanup_failure_*) that can't fail as root, and the sandbox runs as uid 0.
  • I couldn't run the real-vlt vlt_pinned_matrix legs locally: their registry fixture fetches from npmjs directly, and the sandbox only allows proxied egress. Lean PR CI runs e2e_redirect_vlt_build on vlt 1.2.0, which drives socket_cmd.

/code-review high

  • Fixed: the original version re-applied FORCED_OPT_OUTS after the full vlt scrub, which was fragile. It now uses the new SOCKET_*-free tool scrub, and the is_ambient_vlt_var doc says which children take which scrub.
  • Not fixed here (pre-existing, out of scope for a migration slice):
    • vlt_vendor_common::cli sweeps only VLT_*, not npm_config_* / VIRTUAL_ENV like the e2e helper. That file doesn't include cache_env.
    • A vlt hook that calls socket-patch through the npx shim (VltRun::with_shims) wouldn't get the hermetic env. It has no callers today.
    • The proxy sweep is now repeated in three helpers. It could become an Extra::Proxy in a later slice.
    • vlt_hosted_common / vlt_vendor_common include hermetic.rs privately, the same way they already include oracle.rs / prebuilt_common, because their includers don't all have it at the crate root.

🤖 Generated with Claude Code

https://claude.ai/code/session_01KfnzRvB2CbCgiSWbcKJWPD


Generated by Claude Code


Note

Low Risk
Changes are limited to integration-test spawn hygiene; no production CLI or runtime behavior is modified.

Overview
Test-only migration (hermetic-spawn #823): vlt integration helpers now spawn socket-patch through the shared hermetic::command / hermetic::binary_command instead of Command::new plus per-file SOCKET_* sweeps that had drifted from tests/common/hermetic.rs.

vlt_e2e_common::socket_cmd, vlt_hosted_common::scrubbed_cli, and vlt_vendor_common::cli keep only their tool-specific scrubs (vlt/npm/proxy). cache_env::scrub_ambient_vlt_tool_env is the vlt ambient scrub without SOCKET_*, so a full vlt scrub after the hermetic builder does not strip forced telemetry/config opt-outs; docs and a self-test cover the split. Hosted/vendor commons include hermetic.rs locally like other shared test modules.

spawn_env_hygiene: the three vlt common paths are removed from PENDING_RAW_SPAWNS (42 → 39 pending bare spawns).

Reviewed by Cursor Bugbot for commit 9fed156. Configure here.

Assisted-by: Claude Code:claude-opus-5-5
The vlt e2e, hosted and vendor test helpers each started socket-patch
with a bare Command::new and their own SOCKET_* sweep. Those copies
had drifted from tests/common/hermetic.rs: none removed flag-bound
vars set in-process after the sweep, none used the hostile seeds that
catch a dropped scrub, and the hosted helper left an ambient
SOCKET_TELEMETRY_DISABLED=0 in place.

All three now build on hermetic::command and keep only their own
vlt and proxy scrubs. They leave PENDING_RAW_SPAWNS, so the ratchet
keeps them hermetic.

Refs #823

Assisted-by: Claude Code:claude-opus-5-5
The shared vlt scrub also removes SOCKET_* vars, so running it on a
socket-patch child built by the hermetic builder would turn telemetry
and the update check back on. Add a vlt scrub that leaves SOCKET_* to
the builder, as the bun suites already have, and use it for the vlt
e2e CLI child instead of re-applying the opt-outs by hand.

Refs #823

Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko
Mikola Lysenko (mikolalysenko) marked this pull request as ready for review October 13, 2026 15:32
@mikolalysenko Mikola Lysenko (mikolalysenko) added ci-perf CI / merge-queue performance finding (profiler routine) and removed ci-perf CI / merge-queue performance finding (profiler routine) labels Oct 13, 2026
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 9fed156. Configure here.

Merged via the queue into main with commit 49847ef Oct 13, 2026
86 checks passed
@mikolalysenko
Mikola Lysenko (mikolalysenko) deleted the agent/fix-ci-hermetic-vlt-commons branch October 13, 2026 16:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants