GeezerVison is showing github.com — CI: hermetic spawns in vlt test commons (#823) by mikolalysenko · Pull Request #1488 · SocketDev/socket-patch
Conversation
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.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Assisted-by: Claude Code:claude-opus-5-5
This was referenced Oct 13, 2026
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
cursor[bot] reacted with thumbs up emoji
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
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.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Milestone
No milestone
Development
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
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.
|