Skip to content

CI: hermetic spawns in scan/policy/network suites (#823) - #1515

Merged
Mikola Lysenko (mikolalysenko) merged 3 commits into
mainfrom
agent/fix-ci-hermetic-scan-policy-network
Oct 14, 2026
Merged

Mikola Lysenko (mikolalysenko) merged 3 commits into
mainfrom
agent/fix-ci-hermetic-scan-policy-network

Conversation

@mikolalysenko

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

Copy link
Copy Markdown
Collaborator

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

Refs #823 (slice 16: scan, socket.yml policy and apply-network suites). Uses Refs, so #823 stays open for the remaining PENDING_RAW_SPAWNS files.

Summary

e2e_scan.rs, e2e_socket_yml_policy.rs and apply/apply_network.rs now spawn socket-patch through tests/common/hermetic.rs and leave PENDING_RAW_SPAWNS (12 → 9 on main; 6 once #1510 also lands, and the two edits don't touch adjacent lines). The ratchet in spawn_env_hygiene.rs keeps them hermetic. Test-only; no product code and no workflow changes.

Root cause

Each file started the CLI with a bare Command::new and its own SOCKET_* handling, which had drifted from the shared builder:

  • apply_network removed only SOCKET_API_TOKEN in 8 of its 9 spawns. On main, a shell that exports SOCKET_DRY_RUN=true SOCKET_ECOSYSTEMS=cargo fails 9 of its tests.
  • All three sweeps let an ambient SOCKET_NO_CONFIG=0 through, so a developer's real socket login could reach the child, and none of them forced the update check off.
  • The policy suite's fixture includes a gem but never scrubbed BUNDLE_*. It now uses Extra::Venv + Extra::Bundle.
  • e2e_scan's npm children now drop ambient SOCKET_* too.

Two coverage fixes so the move doesn't weaken guards (from /code-review):

  • offline_apply_with_token_makes_zero_network_requests: the builder forces telemetry off, which would hide a telemetry POST under --offline. The test sets SOCKET_TELEMETRY_DISABLED=0. Checked by removing is_offline_env() from is_telemetry_disabled: the test fails with mock server saw: ["POST /patch/telemetry"].
  • apply_uses_locally_cached_blob_without_fetching pinned only SOCKET_API_URL to a dead port, but with no token the child uses the public proxy. It now pins SOCKET_PROXY_URL as well.

Test evidence

  • Red, without the conversions (only the PENDING_RAW_SPAWNS removal) and with a hostile shell (SOCKET_DRY_RUN=true SOCKET_ECOSYSTEMS=cargo SOCKET_ORG_SLUG=hostile SOCKET_NO_CONFIG=0): spawn_env_hygiene fails no_new_bare_binary_spawns for all 3 files, and apply fails 9 apply_network tests.
  • Green under the same hostile shell (plus VIRTUAL_ENV=/nonexistent): apply 135/135, e2e_socket_yml_policy 26/26, spawn_env_hygiene 16/16, e2e_scan non-ignored 20/20.
  • e2e_scan's 11 #[ignore]d live-API tests need patches-api.socket.dev, which the agent sandbox can't reach, so they weren't run. CI doesn't run them on PRs either.
  • cargo fmt --all -- --check and cargo clippy --locked --workspace --all-features -- -D warnings are clean. The test-target clippy notes are all pre-existing in prebuilt_common.

/code-review high

Fixed: the telemetry guard, the proxy pin, Extra::Venv in the pypi test, and the e2e_scan comment. Not fixed:

CI cost

No workflow changes. A lean PR run is the same before and after (~28 Linux jobs, ~8 min).

🤖 Generated with Claude Code

https://claude.ai/code/session_01NSWn8iEQppkpg6sqyjhEx6


Generated by Claude Code

Assisted-by: Claude Code:claude-opus-5-5
e2e_scan, e2e_socket_yml_policy and apply/apply_network started
socket-patch with a bare Command::new and their own SOCKET_* sweeps.
apply_network only removed SOCKET_API_TOKEN in most tests, so a
developer shell exporting SOCKET_DRY_RUN or SOCKET_ECOSYSTEMS failed 9
of its tests. All three sweeps let an ambient SOCKET_NO_CONFIG=0
through, so a real `socket login` could reach the child, and none
forced the update check off.

All three now spawn through hermetic::command. The policy suite also
scrubs Bundler env, since its fixture includes a gem. The npm children
in e2e_scan drop ambient SOCKET_* too. The files leave
PENDING_RAW_SPAWNS, so the spawn_env_hygiene ratchet keeps them
hermetic.

Refs #823

Assisted-by: Claude Code:claude-opus-5-5
The shared builder forces telemetry off and removes the API token. The
offline airgap test then no longer showed that --offline alone keeps
telemetry away from the mock. It turns telemetry back on, so it fails
again if offline runs ever POST telemetry (checked by removing the
offline term from is_telemetry_disabled).

The cached-blob test only pointed SOCKET_API_URL at a dead port. With
no token the child uses the public proxy, so a regressed fetch could
reach the real proxy and still pass. Pin SOCKET_PROXY_URL too. The
pypi test uses Extra::Venv instead of a hand-written VIRTUAL_ENV
removal.

Refs #823

Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko
Mikola Lysenko (mikolalysenko) marked this pull request as ready for review October 14, 2026 02:05
@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 88407b9. Configure here.

@mikolalysenko Mikola Lysenko (mikolalysenko) added the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label Oct 14, 2026
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Ready for review at 88407b95577d5e9d19677bedd68fba060200dd58.


Generated by Claude Code

Merged via the queue into main with commit a5ddcf7 Oct 14, 2026
86 checks passed
@mikolalysenko
Mikola Lysenko (mikolalysenko) deleted the agent/fix-ci-hermetic-scan-policy-network branch October 14, 2026 03:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants