diff --git a/.cargo/config.toml b/.cargo/config.toml index e0acd1cd4..e9b0e1391 100644 --- a/.cargo/config.toml +++ b/.cargo/config.toml @@ -32,3 +32,10 @@ SOCKET_NO_CONFIG = "1" # tests opt back in with SOCKET_NO_UPDATE_CHECK=0 plus a wiremock # SOCKET_UPDATE_BASE_URL. SOCKET_NO_UPDATE_CHECK = "1" + +# No test child (and no dev `cargo run`) may POST telemetry to the real +# public proxy: an unauthenticated run sends its events to +# patches-api.socket.dev, and most spawning suites point no proxy URL at a +# mock. Suites that assert telemetry events opt back in with +# SOCKET_TELEMETRY_DISABLED=0 (or remove it) plus a wiremock endpoint. +SOCKET_TELEMETRY_DISABLED = "1" diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8038c9bd6..f4f8067ea 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -990,14 +990,14 @@ jobs: # setup-node step, no version change. # # `npm_required` turns the npm suites' "npm not installed" soft-skip - # into a hard failure. Not on Windows: `Command::new("npm")` cannot - # resolve `npm.cmd` there, so that leg still skips (a known gap; see - # docs/testing/npm-compatibility.md). + # into a hard failure, on every OS: the suite resolves npm through + # PATHEXT, so Windows finds the `npm.cmd` shim instead of skipping. - os: ubuntu-latest suite: e2e_redirect_npm_build npm_required: '1' - os: windows-latest suite: e2e_redirect_npm_build + npm_required: '1' - os: ubuntu-latest suite: e2e_redirect_rush_sim - os: windows-latest @@ -1252,6 +1252,7 @@ jobs: env: SOCKET_NO_CONFIG: '1' SOCKET_NO_UPDATE_CHECK: '1' + SOCKET_TELEMETRY_DISABLED: '1' # e2e-full runs these same steps over its rows. steps: &e2e-steps - name: Checkout @@ -1617,14 +1618,15 @@ jobs: SOCKET_PATCH_SBT_E2E_VERSION: ${{ matrix.sbt }} E2E_SUITE: ${{ matrix.suite }} E2E_TEST_FILTER: ${{ matrix.test_filter || '--ignored' }} - E2E_JVM_TOOL: ${{ matrix.jvm_tool }} E2E_ALLOW_EMPTY: ${{ matrix.allow_empty }} shell: bash # Runs from the package root with CARGO_MANIFEST_DIR set, as # `cargo test` would. `suite` may name several binaries; an # `allow_empty` row skips the ones whose test file has not landed. - # Every suite a Gradle leg runs must run at least one test (per - # suite, so one suite's tests never hide another's empty filter). + # Every suite a leg runs must run at least one test (per suite, so + # one suite's tests never hide another's empty filter): a renamed + # module, a dropped `#[ignore]` under the default `--ignored` + # selector or a stale `test_filter` would otherwise pass as 0/0. run: | set -uo pipefail exe='' @@ -1644,8 +1646,8 @@ jobs: continue fi passed=$(sed -n 's/^test result: .* \([0-9][0-9]*\) passed;.*/\1/p' "$log" | head -n 1) - if [ "$E2E_JVM_TOOL" = gradle ] && [ "${passed:-0}" = 0 ]; then - echo "::error::$suite ran no test in this Gradle leg ($E2E_TEST_FILTER)" + if [ "${passed:-0}" = 0 ]; then + echo "::error::$suite ran no test in this leg (filter: $E2E_TEST_FILTER). Fix the row's suite or test_filter so it selects the tests it is meant to run." status=1 fi done @@ -1750,6 +1752,7 @@ jobs: env: SOCKET_NO_CONFIG: '1' SOCKET_NO_UPDATE_CHECK: '1' + SOCKET_TELEMETRY_DISABLED: '1' # e2e-full runs these same steps over its rows. steps: *e2e-steps @@ -1798,6 +1801,7 @@ jobs: env: SOCKET_NO_CONFIG: '1' SOCKET_NO_UPDATE_CHECK: '1' + SOCKET_TELEMETRY_DISABLED: '1' steps: *e2e-steps # ---------------------------------------------------------------------- @@ -2026,6 +2030,7 @@ jobs: env: SOCKET_NO_CONFIG: '1' SOCKET_NO_UPDATE_CHECK: '1' + SOCKET_TELEMETRY_DISABLED: '1' strategy: fail-fast: false matrix: @@ -2098,6 +2103,7 @@ jobs: env: SOCKET_NO_CONFIG: '1' SOCKET_NO_UPDATE_CHECK: '1' + SOCKET_TELEMETRY_DISABLED: '1' strategy: fail-fast: false matrix: @@ -2118,6 +2124,7 @@ jobs: env: SOCKET_NO_CONFIG: '1' SOCKET_NO_UPDATE_CHECK: '1' + SOCKET_TELEMETRY_DISABLED: '1' strategy: fail-fast: false matrix: diff --git a/.github/workflows/composer-compatibility.yml b/.github/workflows/composer-compatibility.yml index 4741fef4c..9c9f9ea98 100644 --- a/.github/workflows/composer-compatibility.yml +++ b/.github/workflows/composer-compatibility.yml @@ -78,6 +78,7 @@ concurrency: env: SOCKET_NO_CONFIG: '1' SOCKET_NO_UPDATE_CHECK: '1' + SOCKET_TELEMETRY_DISABLED: '1' jobs: native: diff --git a/.github/workflows/go-compatibility.yml b/.github/workflows/go-compatibility.yml index eea8dbbf2..6251cd386 100644 --- a/.github/workflows/go-compatibility.yml +++ b/.github/workflows/go-compatibility.yml @@ -36,6 +36,7 @@ concurrency: env: SOCKET_NO_CONFIG: '1' SOCKET_NO_UPDATE_CHECK: '1' + SOCKET_TELEMETRY_DISABLED: '1' jobs: go: diff --git a/.github/workflows/gradle-compatibility.yml b/.github/workflows/gradle-compatibility.yml index 0f3944966..8ebf4842b 100644 --- a/.github/workflows/gradle-compatibility.yml +++ b/.github/workflows/gradle-compatibility.yml @@ -95,6 +95,7 @@ concurrency: env: SOCKET_NO_CONFIG: '1' SOCKET_NO_UPDATE_CHECK: '1' + SOCKET_TELEMETRY_DISABLED: '1' jobs: build: diff --git a/.github/workflows/npm-compatibility.yml b/.github/workflows/npm-compatibility.yml index 64f83abe2..b792902da 100644 --- a/.github/workflows/npm-compatibility.yml +++ b/.github/workflows/npm-compatibility.yml @@ -119,6 +119,7 @@ jobs: SOCKET_PATCH_NPM_E2E_REQUIRED: '1' SOCKET_NO_CONFIG: '1' SOCKET_NO_UPDATE_CHECK: '1' + SOCKET_TELEMETRY_DISABLED: '1' run: | chmod +x target/debug/socket-patch target/debug/e2e_redirect_npm_build target/debug/e2e_vendor_npm_build target/debug/e2e_redirect_npm_build --include-ignored --test-threads 4 diff --git a/.github/workflows/pdm-compatibility.yml b/.github/workflows/pdm-compatibility.yml index fcec586f6..c286fcaa9 100644 --- a/.github/workflows/pdm-compatibility.yml +++ b/.github/workflows/pdm-compatibility.yml @@ -57,6 +57,7 @@ concurrency: env: SOCKET_NO_CONFIG: '1' SOCKET_NO_UPDATE_CHECK: '1' + SOCKET_TELEMETRY_DISABLED: '1' PDM_CHECK_UPDATE: 'false' jobs: diff --git a/.github/workflows/pipenv-compatibility.yml b/.github/workflows/pipenv-compatibility.yml index 000f746c5..4664f6c02 100644 --- a/.github/workflows/pipenv-compatibility.yml +++ b/.github/workflows/pipenv-compatibility.yml @@ -85,6 +85,7 @@ jobs: env: SOCKET_NO_CONFIG: '1' SOCKET_NO_UPDATE_CHECK: '1' + SOCKET_TELEMETRY_DISABLED: '1' # The runners carry whatever patch release uv ships; the harness # only needs one 3.8 and one 3.12. BACKTEST_PY38: '3.8' diff --git a/.github/workflows/pnpm-compatibility.yml b/.github/workflows/pnpm-compatibility.yml index 6abe6c81e..b00288934 100644 --- a/.github/workflows/pnpm-compatibility.yml +++ b/.github/workflows/pnpm-compatibility.yml @@ -120,6 +120,7 @@ jobs: SOCKET_PATCH_PNPM_E2E_REQUIRED: '1' SOCKET_NO_CONFIG: '1' SOCKET_NO_UPDATE_CHECK: '1' + SOCKET_TELEMETRY_DISABLED: '1' run: | chmod +x bin/socket-patch bin/pnpm-e2e bin/pnpm-vendor-e2e export SOCKET_PATCH_PNPM_E2E_SOCKET_BIN="$PWD/bin/socket-patch" diff --git a/.github/workflows/poetry-compatibility.yml b/.github/workflows/poetry-compatibility.yml index abcae8402..cf38394ab 100644 --- a/.github/workflows/poetry-compatibility.yml +++ b/.github/workflows/poetry-compatibility.yml @@ -50,6 +50,7 @@ concurrency: env: SOCKET_NO_CONFIG: '1' SOCKET_NO_UPDATE_CHECK: '1' + SOCKET_TELEMETRY_DISABLED: '1' jobs: build: diff --git a/.github/workflows/sbt-compatibility.yml b/.github/workflows/sbt-compatibility.yml index 0c2edb03f..76eb5edc9 100644 --- a/.github/workflows/sbt-compatibility.yml +++ b/.github/workflows/sbt-compatibility.yml @@ -66,6 +66,7 @@ concurrency: env: SOCKET_NO_CONFIG: '1' SOCKET_NO_UPDATE_CHECK: '1' + SOCKET_TELEMETRY_DISABLED: '1' jobs: image: diff --git a/crates/socket-patch-cli/tests/cli_parse_apply.rs b/crates/socket-patch-cli/tests/cli_parse_apply.rs index f68ae72d5..642c70db3 100644 --- a/crates/socket-patch-cli/tests/cli_parse_apply.rs +++ b/crates/socket-patch-cli/tests/cli_parse_apply.rs @@ -8,17 +8,21 @@ use std::path::PathBuf; -use clap::Parser; use socket_patch_cli::commands::apply::ApplyArgs; -use socket_patch_cli::{Cli, Commands}; +use socket_patch_cli::Commands; + +#[path = "common/hermetic.rs"] +mod hermetic; + +use hermetic::try_parse; /// Parse `socket-patch apply ` and return the inner `ApplyArgs`. /// Panics if parsing fails or yields a non-`Apply` subcommand — tests for -/// the failure path call `Cli::try_parse_from` directly. +/// the failure path call [`try_parse`] directly. fn parse_apply(extra: &[&str]) -> ApplyArgs { let mut argv: Vec<&str> = vec!["socket-patch", "apply"]; argv.extend_from_slice(extra); - let cli = Cli::try_parse_from(&argv).expect("parse"); + let cli = try_parse(&argv).expect("parse"); match cli.command { Commands::Apply(a) => a, _ => panic!("expected Apply"), @@ -305,7 +309,7 @@ fn no_telemetry_long() { /// the trailing token is rejected as an unknown argument. #[test] fn bare_bool_does_not_consume_next_token() { - match Cli::try_parse_from(["socket-patch", "apply", "--force", "stray"]) { + match try_parse(&["socket-patch", "apply", "--force", "stray"]) { Ok(_) => panic!("`--force stray` must reject the stray positional"), Err(err) => assert_eq!(err.kind(), clap::error::ErrorKind::UnknownArgument), } @@ -563,7 +567,7 @@ fn download_mode_values_are_not_normalized() { /// expectation to assert an `InvalidValue` error. #[test] fn download_mode_invalid_value_is_only_caught_at_runtime() { - match Cli::try_parse_from(["socket-patch", "apply", "--download-mode", "totally-bogus"]) { + match try_parse(&["socket-patch", "apply", "--download-mode", "totally-bogus"]) { Ok(cli) => match cli.command { Commands::Apply(a) => assert_eq!( a.common.download_mode, "totally-bogus", @@ -588,7 +592,7 @@ fn download_mode_invalid_value_is_only_caught_at_runtime() { fn unknown_flag_fails_with_unknown_argument() { // `Cli` doesn't implement `Debug`, so we can't use `.expect_err()` — // match the Result by hand. - match Cli::try_parse_from(["socket-patch", "apply", "--unknown-flag"]) { + match try_parse(&["socket-patch", "apply", "--unknown-flag"]) { Ok(_) => panic!("--unknown-flag must be rejected"), Err(err) => { assert_eq!(err.kind(), clap::error::ErrorKind::UnknownArgument); diff --git a/crates/socket-patch-cli/tests/cli_parse_rollback.rs b/crates/socket-patch-cli/tests/cli_parse_rollback.rs index f1590292e..8e4b7dcde 100644 --- a/crates/socket-patch-cli/tests/cli_parse_rollback.rs +++ b/crates/socket-patch-cli/tests/cli_parse_rollback.rs @@ -7,15 +7,19 @@ //! these tests is a breaking change and requires a MAJOR bump per //! `crates/socket-patch-cli/CLI_CONTRACT.md`. -use clap::Parser; use socket_patch_cli::commands::rollback::RollbackArgs; -use socket_patch_cli::{Cli, Commands}; +use socket_patch_cli::Commands; use std::path::PathBuf; +#[path = "common/hermetic.rs"] +mod hermetic; + +use hermetic::try_parse; + fn parse_rollback(extra: &[&str]) -> RollbackArgs { let mut argv = vec!["socket-patch", "rollback"]; argv.extend_from_slice(extra); - let cli = Cli::try_parse_from(&argv).expect("parse"); + let cli = try_parse(&argv).expect("parse"); match cli.command { Commands::Rollback(a) => a, _ => panic!("expected Rollback"), @@ -402,7 +406,7 @@ fn preserve_state_does_not_consume_next_token() { #[test] fn unknown_flag_fails() { - let err = match Cli::try_parse_from(["socket-patch", "rollback", "--unknown-flag"]) { + let err = match try_parse(&["socket-patch", "rollback", "--unknown-flag"]) { Ok(_) => panic!("expected parse failure"), Err(e) => e, }; @@ -411,7 +415,7 @@ fn unknown_flag_fails() { #[test] fn removed_one_off_flag_is_unknown() { - let err = match Cli::try_parse_from(["socket-patch", "rollback", "--one-off"]) { + let err = match try_parse(&["socket-patch", "rollback", "--one-off"]) { Ok(_) => panic!("expected parse error for the v5-removed --one-off"), Err(e) => e, }; diff --git a/crates/socket-patch-cli/tests/common/hermetic.rs b/crates/socket-patch-cli/tests/common/hermetic.rs index 31822f99c..12c67c3ec 100644 --- a/crates/socket-patch-cli/tests/common/hermetic.rs +++ b/crates/socket-patch-cli/tests/common/hermetic.rs @@ -48,9 +48,10 @@ const HOSTILE_SEEDS: &[(&str, &str)] = &[ /// A `Command` for `bin` with the hermetic `SOCKET_*` environment: the /// hostile seeds scrubbed, every other ambient `SOCKET_*` removed (removing -/// `SOCKET_API_TOKEN` also forces the public proxy), and the two opt-outs -/// forced on. Callers add args, cwd and their own env afterwards; caller env -/// lands last, so explicit injections survive the scrub. +/// `SOCKET_API_TOKEN` also forces the public proxy), and the three opt-outs +/// (config layer, update notifier, telemetry) forced on. Callers add args, +/// cwd and their own env afterwards; caller env lands last, so explicit +/// injections survive the scrub. pub fn command(bin: &Path) -> Command { let mut cmd = Command::new(bin); for (k, v) in HOSTILE_SEEDS { @@ -72,6 +73,12 @@ pub fn command(bin: &Path) -> Command { // this force-set is the layer that holds there. Notifier tests opt back // in via caller env (which lands last). cmd.env("SOCKET_NO_UPDATE_CHECK", "1"); + // And for telemetry: an unauthenticated child POSTs its events to the + // real public proxy unless a suite points it at a mock. The workspace + // `[env]` default can be overridden by the developer's shell, so force + // it here too. Telemetry suites opt back in via caller env with + // `SOCKET_TELEMETRY_DISABLED=0` and a wiremock endpoint. + cmd.env("SOCKET_TELEMETRY_DISABLED", "1"); cmd } @@ -97,6 +104,35 @@ pub fn scrub_socket_vars(cmd: &mut Command) { } } +/// The in-process half, for test binaries that only parse argv with clap: +/// clap reads every env-bound `SOCKET_*` flag at parse time, so an ambient +/// value (or a `.cargo/config.toml` `[env]` default such as +/// `SOCKET_TELEMETRY_DISABLED=1`) changes what a parse yields. Removes every +/// `SOCKET_*` var from this process, once; later calls wait for the first. +/// Nothing is restored, so call it only from binaries where no test needs a +/// `SOCKET_*` var, and call it before every parse so no parse reads the +/// environment while it is being changed. +pub fn scrub_process_socket_env() { + static ONCE: std::sync::Once = std::sync::Once::new(); + ONCE.call_once(|| { + for (key, _) in std::env::vars_os() { + if key.to_string_lossy().starts_with("SOCKET_") { + std::env::remove_var(&key); + } + } + }); +} + +/// `Cli::try_parse_from` with the ambient `SOCKET_*` environment removed +/// first (see [`scrub_process_socket_env`]): clap reads the env-bound flags +/// at parse time, so without it the workspace `SOCKET_TELEMETRY_DISABLED=1` +/// default (or a developer's shell) would set `--no-telemetry` and friends +/// in every parse. +pub fn try_parse(argv: &[&str]) -> Result { + scrub_process_socket_env(); + ::try_parse_from(argv) +} + /// Opt-in scrubs for the ambient config of the tools a suite drives. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum Extra { diff --git a/crates/socket-patch-cli/tests/npm_e2e_common/mod.rs b/crates/socket-patch-cli/tests/npm_e2e_common/mod.rs index 60c2fc028..03796292f 100644 --- a/crates/socket-patch-cli/tests/npm_e2e_common/mod.rs +++ b/crates/socket-patch-cli/tests/npm_e2e_common/mod.rs @@ -62,7 +62,16 @@ fn env_set(name: &str) -> Option { /// The npm executable under test. pub fn npm_program() -> OsString { - env_set(BIN_ENV).unwrap_or_else(|| "npm".into()) + env_set(BIN_ENV).unwrap_or_else(path_npm) +} + +/// `npm` from `PATH`, resolved the way the CLI resolves its own tools +/// (`PATHEXT` on Windows). A bare `Command::new("npm")` cannot spawn the +/// `npm.cmd` shim Windows installs, so without this every Windows run +/// skipped as "npm not installed". +fn path_npm() -> OsString { + socket_patch_core::utils::process::resolve_tool("npm") + .map_or_else(|| "npm".into(), PathBuf::into_os_string) } /// `SOCKET_PATCH_NPM_E2E_REQUIRED` is set (CI matrix legs). @@ -137,7 +146,7 @@ pub fn npm_version() -> Option { /// `SOCKET_PATCH_NPM_E2E_LOCK_WRITER_BIN`, else `npm` from `PATH` when it /// is not the npm under test and reports major >= 7. pub fn modern_npm_writer() -> Option { - let candidate = env_set("SOCKET_PATCH_NPM_E2E_LOCK_WRITER_BIN").unwrap_or_else(|| "npm".into()); + let candidate = env_set("SOCKET_PATCH_NPM_E2E_LOCK_WRITER_BIN").unwrap_or_else(path_npm); let probe = tempfile::tempdir().unwrap(); let out = npm_command_for(&candidate, probe.path()) .arg("--version") diff --git a/crates/socket-patch-cli/tests/spawn_env_hygiene.rs b/crates/socket-patch-cli/tests/spawn_env_hygiene.rs index 057700a3b..2447d2091 100644 --- a/crates/socket-patch-cli/tests/spawn_env_hygiene.rs +++ b/crates/socket-patch-cli/tests/spawn_env_hygiene.rs @@ -272,6 +272,137 @@ fn command_scrubs_ambient_socket_vars_and_forces_the_opt_outs() { ); } +/// B69: a test child must never POST telemetry to the real public proxy. +/// An ambient opt-in (`SOCKET_TELEMETRY_DISABLED=0`, or a developer shell +/// that removed the `.cargo/config.toml` default) must not reach the child; +/// a suite that asserts telemetry sets its own value after `command`. +#[test] +#[serial] +fn command_forces_telemetry_off() { + for ambient in ["0", ""] { + with_ambient(&[("SOCKET_TELEMETRY_DISABLED", ambient)], || { + let env = effective_env(&hermetic::command(Path::new("socket-patch"))); + assert_eq!( + get(&env, "SOCKET_TELEMETRY_DISABLED"), + Some("1"), + "ambient SOCKET_TELEMETRY_DISABLED={ambient:?} must not re-enable telemetry" + ); + }); + } + let mut cmd = hermetic::command(Path::new("socket-patch")); + cmd.env("SOCKET_TELEMETRY_DISABLED", "0"); + assert_eq!( + get(&effective_env(&cmd), "SOCKET_TELEMETRY_DISABLED"), + Some("0"), + "a telemetry suite's own opt-in lands last" + ); +} + +/// B69: every process `cargo test` / `cargo run` starts (and every child it +/// spawns without a scrub) inherits the three opt-outs from the workspace +/// `.cargo/config.toml` `[env]` table. +#[test] +fn cargo_config_env_carries_the_opt_outs() { + let config = Path::new(env!("CARGO_MANIFEST_DIR")).join("../../.cargo/config.toml"); + let text = std::fs::read_to_string(&config) + .unwrap_or_else(|e| panic!("read {}: {e}", config.display())) + .replace("\r\n", "\n"); + let env_table = text + .split("\n[env]\n") + .nth(1) + .unwrap_or_else(|| panic!("{} has no [env] table", config.display())); + let env_table = env_table.split("\n[").next().unwrap_or(env_table); + for var in [ + "SOCKET_NO_CONFIG", + "SOCKET_NO_UPDATE_CHECK", + "SOCKET_TELEMETRY_DISABLED", + ] { + assert!( + env_table + .lines() + .any(|line| line.trim() == format!("{var} = \"1\"")), + "{} [env] must set {var} = \"1\"", + config.display() + ); + } +} + +/// B69: CI legs that run the prebuilt test binaries directly (not through +/// cargo) do not get the `[env]` table, so their workflow env blocks copy it +/// by hand. Any env block that copies one opt-out must copy all three. +#[test] +fn workflow_env_copies_carry_every_opt_out() { + const OPT_OUTS: [&str; 3] = [ + "SOCKET_NO_CONFIG", + "SOCKET_NO_UPDATE_CHECK", + "SOCKET_TELEMETRY_DISABLED", + ]; + let dir = Path::new(env!("CARGO_MANIFEST_DIR")).join("../../.github/workflows"); + let mut entries: Vec<_> = std::fs::read_dir(&dir) + .unwrap_or_else(|e| panic!("read {}: {e}", dir.display())) + .map(|e| e.expect("workflow dir entry").path()) + .filter(|p| p.extension().is_some_and(|x| x == "yml" || x == "yaml")) + .collect(); + entries.sort(); + let mut copies = 0; + let mut missing = Vec::new(); + for path in entries { + let text = std::fs::read_to_string(&path) + .unwrap_or_else(|e| panic!("read {}: {e}", path.display())) + .replace("\r\n", "\n"); + let lines: Vec<&str> = text.lines().collect(); + // An env block is a run of `KEY: value` lines at one indent; find + // every run that sets any opt-out and check it sets them all. + let mut i = 0; + while i < lines.len() { + let line = lines[i]; + let Some(var) = OPT_OUTS + .iter() + .find(|v| line.trim_start().starts_with(&format!("{v}:"))) + else { + i += 1; + continue; + }; + let indent = line.len() - line.trim_start().len(); + let same_block = |l: &&str| { + let t = l.trim_start(); + l.len() - t.len() == indent && !t.is_empty() && !t.starts_with('-') + }; + let mut start = i; + while start > 0 && same_block(&lines[start - 1]) { + start -= 1; + } + let mut end = i; + while end + 1 < lines.len() && same_block(&lines[end + 1]) { + end += 1; + } + let block = &lines[start..=end]; + copies += 1; + for want in OPT_OUTS { + let set = block.iter().any(|l| l.trim() == format!("{want}: '1'")); + if !set { + missing.push(format!( + "{}:{} (block with {var}) lacks {want}: '1'", + path.display(), + start + 1 + )); + } + } + i = end + 1; + } + } + assert!( + copies > 0, + "found no workflow env copy of the [env] opt-outs" + ); + assert!( + missing.is_empty(), + "workflow env blocks that copy .cargo/config.toml [env] must set all \ + three opt-outs:\n{}", + missing.join("\n") + ); +} + #[test] #[serial] fn command_never_leaks_its_hostile_seeds() { @@ -460,8 +591,10 @@ fn no_new_private_scrub_socket_env_copies() { .collect(); assert!( unexpected.is_empty(), - "spawn through `common/hermetic.rs` (hermetic::command + scrub_extra) \ - instead of a private scrub_socket_env: {unexpected:?}" + "these test files define a private scrub_socket_env: {unexpected:?}. \ + Spawn through `tests/common/hermetic.rs` instead (hermetic::command \ + for the binary, scrub_socket_vars + scrub_extra for package-manager \ + children). Do not add them to PENDING_SCRUB_COPIES." ); let stale: Vec<&&str> = PENDING_SCRUB_COPIES .iter() @@ -469,8 +602,10 @@ fn no_new_private_scrub_socket_env_copies() { .collect(); assert!( stale.is_empty(), - "these files no longer carry a scrub_socket_env copy; drop them from \ - PENDING_SCRUB_COPIES: {stale:?}" + "these files no longer carry a scrub_socket_env copy: {stale:?}. Delete \ + them from PENDING_SCRUB_COPIES in \ + crates/socket-patch-cli/tests/spawn_env_hygiene.rs (another PR may \ + have migrated them; rebase and drop the entries)." ); } @@ -487,8 +622,11 @@ fn no_new_bare_binary_spawns() { .collect(); assert!( unexpected.is_empty(), - "spawn the binary through `hermetic::command` / `common::run*`, not a \ - bare Command::new: {unexpected:?}" + "these test files spawn the binary with a bare Command::new: \ + {unexpected:?}. Use `hermetic::command(&bin)` / \ + `hermetic::binary_command()` or `common::run*` (tests/common/) so the \ + child gets the hermetic SOCKET_* environment. Do not add them to \ + PENDING_RAW_SPAWNS." ); let stale: Vec<&&str> = PENDING_RAW_SPAWNS .iter() @@ -496,7 +634,9 @@ fn no_new_bare_binary_spawns() { .collect(); assert!( stale.is_empty(), - "these files no longer spawn the binary bare; drop them from \ - PENDING_RAW_SPAWNS: {stale:?}" + "these files no longer spawn the binary bare: {stale:?}. Delete them \ + from PENDING_RAW_SPAWNS in \ + crates/socket-patch-cli/tests/spawn_env_hygiene.rs (another PR may \ + have migrated them; rebase and drop the entries)." ); } diff --git a/crates/socket-patch-core/src/utils/digest.rs b/crates/socket-patch-core/src/utils/digest.rs index 630adefa1..0ef654449 100644 --- a/crates/socket-patch-core/src/utils/digest.rs +++ b/crates/socket-patch-core/src/utils/digest.rs @@ -194,9 +194,31 @@ mod tests { } } inline.sort(); - assert_eq!( - inline, PENDING_INLINE_DIGESTS, - "production files computing digests inline differ from the pending list" - ); + let new: Vec<&String> = inline + .iter() + .filter(|f| !PENDING_INLINE_DIGESTS.contains(&f.as_str())) + .collect(); + let stale: Vec<&&str> = PENDING_INLINE_DIGESTS + .iter() + .filter(|f| !inline.iter().any(|g| g == *f)) + .collect(); + let mut problems = Vec::new(); + if !new.is_empty() { + problems.push(format!( + "these files compute a digest inline: {new:?}. Call the `*_of` \ + helpers in crates/socket-patch-core/src/utils/digest.rs instead \ + (sha256_hex_of, sha1_hex_of, sha512_base64_of, sha512_sri_of). \ + Do not add them to PENDING_INLINE_DIGESTS." + )); + } + if !stale.is_empty() { + problems.push(format!( + "these files no longer compute a digest inline: {stale:?}. Delete \ + them from PENDING_INLINE_DIGESTS in \ + crates/socket-patch-core/src/utils/digest.rs (another PR may \ + have moved them onto the helpers; rebase and drop the entries)." + )); + } + assert!(problems.is_empty(), "{}", problems.join("\n")); } }