Skip to content

Commit 23fa8d4

Browse files
CI: hermetic spawns in npm/Bun suites (#823) (#1510)
* Start fix for #823 slice 15 Assisted-by: Claude Code:claude-opus-5-5 * Spawn npm/Bun test CLIs hermetically The npm and Bun suites started socket-patch with a bare Command::new and their own SOCKET_* scrubs, which had drifted from tests/common/hermetic.rs. e2e_npm and the Bun redirect suite never forced the config layer or update notifier off, so a developer's `socket login` or an ambient SOCKET_NO_UPDATE_CHECK=0 reached the child; the in-process Bun vendor FIFO helper skipped the stand-in HOME the rest of that file's spawns get through common::run_with_env. All three now start from hermetic::command. The Bun redirect suite keeps its shared bun/npm registry scrub through scrub_ambient_bun_tool_env, the variant that leaves the builder's SOCKET_* opt-outs alone, and the FIFO helper adds jvm_env::isolate_cli to match run_with_env. e2e_npm keeps its partial cache pins so the global auto-discovery test still sees real prefixes. The files leave PENDING_RAW_SPAWNS (15 -> 12), so the ratchet keeps them hermetic. Refs #823 Assisted-by: Claude Code:claude-opus-5-5 * Keep real HOME and SOCKET_* off npm/Bun kids From /code-review high: the Bun redirect suite ran its bun fixture installs under the cache sandbox's home but the CLI under the real one, so a registry or scope in the developer's ~/.npmrc would make the two disagree. The CLI now gets the stand-in HOME (jvm_env::isolate_cli), as run_with_env gives every other suite. Its --no-telemetry flag goes: hermetic::command already forces telemetry off, and the flag would override a caller opting back in. e2e_npm's npm installs now drop ambient SOCKET_* vars too (hermetic::scrub_socket_vars), like the package-manager children of the other converted suites. Refs #823 Assisted-by: Claude Code:claude-opus-5-5 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 81dd140 commit 23fa8d4

4 files changed

Lines changed: 25 additions & 43 deletions

File tree

‎crates/socket-patch-cli/tests/e2e_npm.rs‎

Lines changed: 8 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
1616
#[path = "common/mod.rs"]
1717
mod common;
18-
use common::{binary, git_sha256};
18+
use common::{binary, git_sha256, hermetic};
1919

2020
use std::path::Path;
2121
use std::process::{Command, Output};
@@ -58,24 +58,12 @@ fn git_sha256_file(path: &Path) -> String {
5858
/// Run the CLI binary with the given args, setting `cwd` as the working dir.
5959
/// Returns `(exit_code, stdout, stderr)`.
6060
fn run(cwd: &Path, args: &[&str]) -> (i32, String, String) {
61-
let mut cmd = Command::new(binary());
61+
// The hermetic `SOCKET_*` environment: no ambient value (SOCKET_DRY_RUN,
62+
// SOCKET_GLOBAL_PREFIX, ...) changes what these tests exercise, no token
63+
// (so the public proxy), and the config layer, update notifier and
64+
// telemetry forced off.
65+
let mut cmd = hermetic::command(&binary());
6266
cmd.args(args).current_dir(cwd);
63-
// The binary binds a wide `SOCKET_*` env surface (SOCKET_CWD,
64-
// SOCKET_DRY_RUN, SOCKET_STRICT, SOCKET_ECOSYSTEMS, SOCKET_GLOBAL_PREFIX,
65-
// ...). An ambient value silently changes what these tests exercise —
66-
// SOCKET_DRY_RUN=true turns every real apply into a no-op, and
67-
// SOCKET_GLOBAL_PREFIX flips commands into global mode, aiming mutations
68-
// at the host's *real* global node_modules. Scrub the whole prefix so
69-
// only the flags each test passes are in effect; removing
70-
// SOCKET_API_TOKEN also forces the public proxy (free-tier). Telemetry
71-
// opt-outs are deliberately kept so an opted-out dev stays opted out.
72-
for (key, _) in std::env::vars_os() {
73-
let name = key.to_string_lossy();
74-
if name.starts_with("SOCKET_") && !name.contains("TELEMETRY") && name != "SOCKET_NO_CONFIG"
75-
{
76-
cmd.env_remove(&key);
77-
}
78-
}
7967
// Partial cache isolation only — the global auto-discovery test below
8068
// needs the binary's `npm root -g` / `yarn global dir` / `pnpm root -g`
8169
// probes to resolve the REAL prefixes, which come out of $HOME, so a full
@@ -108,6 +96,7 @@ fn assert_run_ok(cwd: &Path, args: &[&str], context: &str) -> (String, String) {
10896
fn npm_run(cwd: &Path, args: &[&str]) {
10997
let mut cmd = Command::new("npm");
11098
cmd.args(args).current_dir(cwd);
99+
hermetic::scrub_socket_vars(&mut cmd);
111100
cache_env::isolate(&mut cmd);
112101
let out = cmd.output().expect("failed to run npm");
113102
assert!(
@@ -357,6 +346,7 @@ fn test_npm_global_lifecycle() {
357346
]);
358347
// `--prefix` is a flag, so it still decides where the package lands; the
359348
// sandbox only moves the download cache off the caller's home.
349+
hermetic::scrub_socket_vars(&mut cmd);
360350
cache_env::isolate(&mut cmd);
361351
let out = cmd.output().expect("failed to run npm install -g");
362352
assert!(

‎crates/socket-patch-cli/tests/e2e_redirect_bun_build.rs‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@
9292
9393
#[path = "common/mod.rs"]
9494
mod common;
95-
use common::binary;
95+
use common::{binary, hermetic};
9696

9797
use std::path::{Path, PathBuf};
9898
use std::process::{Command, Output, Stdio};
@@ -300,17 +300,23 @@ fn bun_env(cwd: &Path, args: &[&str], cache_dir: &Path, env: &[(String, String)]
300300
cmd.output().expect("failed to run bun")
301301
}
302302

303-
/// The real binary with `--no-telemetry` appended: nothing in this suite
304-
/// should ever post a telemetry event, mocked API or not.
303+
/// The real binary. `hermetic::command` forces telemetry off: nothing in
304+
/// this suite should ever post a telemetry event, mocked API or not.
305305
fn run_socket(cwd: &Path, args: &[&str]) -> (i32, String, String) {
306306
run_socket_env(cwd, args, &[])
307307
}
308308

309-
/// [`run_socket`] with extra env applied after the scrub.
309+
/// [`run_socket`] with extra env applied after the scrub: the hermetic
310+
/// `SOCKET_*` environment (`hermetic::command`), the bun suites' shared
311+
/// scrub of every other ambient bun / npm registry setting, and a stand-in
312+
/// `HOME` (`jvm_env::isolate_cli`). The `bun` fixture children run under the
313+
/// cache sandbox's home, so the CLI must not read the developer's real
314+
/// `~/.npmrc` either: a registry or scope there would make the two disagree.
310315
fn run_socket_env(cwd: &Path, args: &[&str], env: &[(String, String)]) -> (i32, String, String) {
311-
let mut cmd = Command::new(binary());
312-
cmd.args(args).arg("--no-telemetry").current_dir(cwd);
313-
cache_env::scrub_ambient_bun_env(&mut cmd);
316+
let mut cmd = hermetic::command(&binary());
317+
cmd.args(args).current_dir(cwd);
318+
cache_env::scrub_ambient_bun_tool_env(&mut cmd);
319+
common::jvm_env::isolate_cli(&mut cmd);
314320
for (k, v) in env {
315321
cmd.env(k, v);
316322
}

‎crates/socket-patch-cli/tests/spawn_env_hygiene.rs‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,15 +32,12 @@ const PENDING_RAW_SPAWNS: &[&str] = &[
3232
"apply/apply_network.rs",
3333
"e2e_hosted_production.rs",
3434
"e2e_maven.rs",
35-
"e2e_npm.rs",
36-
"e2e_redirect_bun_build.rs",
3735
"e2e_redirect_cargo_shapes.rs",
3836
"e2e_scan.rs",
3937
"e2e_socket_yml_policy.rs",
4038
"e2e_vendored_production.rs",
4139
"e2e_vex_vendor.rs",
4240
"rollback/rollback_multicopy_blob_gate.rs",
43-
"vendor/in_process_vendor_bun.rs",
4441
];
4542

4643
/// The environment `cmd`'s child would see: the parent environment with

‎crates/socket-patch-cli/tests/vendor/in_process_vendor_bun.rs‎

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1619,21 +1619,10 @@ async fn scan_vendored_from_workspace_member_cwd_fetches_then_engine_refuses_loc
16191619
fn run_with_deadline(root: &Path, argv: &[&str], deadline: Duration) -> (i32, String, String) {
16201620
let out_path = root.join("child.stdout");
16211621
let err_path = root.join("child.stderr");
1622-
let mut cmd = Command::new(common::binary());
1623-
cmd.args(argv).current_dir(root);
1624-
for (key, _) in std::env::vars_os() {
1625-
let name = key.to_string_lossy();
1626-
if name.starts_with("SOCKET_")
1627-
&& !name.contains("TELEMETRY")
1628-
&& name != "SOCKET_NO_CONFIG"
1629-
&& name != "SOCKET_NO_UPDATE_CHECK"
1630-
{
1631-
cmd.env_remove(&key);
1632-
}
1633-
}
1634-
cmd.env("SOCKET_NO_CONFIG", "1")
1635-
.env("SOCKET_NO_UPDATE_CHECK", "1")
1636-
.env("SOCKET_TELEMETRY_DISABLED", "1")
1622+
let mut cmd = common::hermetic_command(&common::binary());
1623+
common::jvm_env::isolate_cli(&mut cmd);
1624+
cmd.args(argv)
1625+
.current_dir(root)
16371626
.stdin(Stdio::null())
16381627
.stdout(std::fs::File::create(&out_path).unwrap())
16391628
.stderr(std::fs::File::create(&err_path).unwrap());

0 commit comments

Comments
 (0)