From 9a09a845bb529d45cbf5b95b5c49ec743a3c66b6 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Oct 2026 17:52:26 +0000 Subject: [PATCH 1/3] Start fix for #1089 core covgap files Assisted-by: Claude Code:claude-opus-5-5 From 36254790e118c476cf8b371bbdf11cdc20cfbd90 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Oct 2026 18:02:24 +0000 Subject: [PATCH 2/3] CI: core covgap tests join their suites The core crate kept five coverage-gap test files, each its own test binary, with helpers mirrored from the suites they called companions. Each now lives in its named suite and shares that suite's helpers: - covgap_api_blob_fetcher -> blob_fetcher_edges_e2e (drops the mirrored client, manifest and dir-count helpers) - covgap_crawlers_npm_crawler and covgap_patch_apply -> crawler_npm_e2e, next to the twin-healing success tests - covgap_crawlers_composer_crawler -> crawler_composer_e2e, using its EnvVars restore guard and global_opts instead of private copies - covgap_update_state -> update_state_env (it blanks HOME, so it must stay its own process) No test is dropped and nothing is asserted differently. Four fewer test binaries to link per test leg; the Windows shard timings move with them. Refs #1089 Assisted-by: Claude Code:claude-opus-5-5 --- .../tests/blob_fetcher_edges_e2e.rs | 355 +++++++++- .../tests/covgap_api_blob_fetcher.rs | 441 ------------ .../tests/covgap_crawlers_composer_crawler.rs | 250 ------- .../tests/covgap_crawlers_npm_crawler.rs | 423 ------------ .../tests/covgap_patch_apply.rs | 244 ------- .../tests/crawler_composer_e2e.rs | 215 ++++++ .../tests/crawler_npm_e2e.rs | 652 +++++++++++++++++- ...ap_update_state.rs => update_state_env.rs} | 2 +- scripts/ci-test-durations.json | 12 +- 9 files changed, 1218 insertions(+), 1376 deletions(-) delete mode 100644 crates/socket-patch-core/tests/covgap_api_blob_fetcher.rs delete mode 100644 crates/socket-patch-core/tests/covgap_crawlers_composer_crawler.rs delete mode 100644 crates/socket-patch-core/tests/covgap_crawlers_npm_crawler.rs delete mode 100644 crates/socket-patch-core/tests/covgap_patch_apply.rs rename crates/socket-patch-core/tests/{covgap_update_state.rs => update_state_env.rs} (97%) diff --git a/crates/socket-patch-core/tests/blob_fetcher_edges_e2e.rs b/crates/socket-patch-core/tests/blob_fetcher_edges_e2e.rs index 2fbcb157e..62eef7bc8 100644 --- a/crates/socket-patch-core/tests/blob_fetcher_edges_e2e.rs +++ b/crates/socket-patch-core/tests/blob_fetcher_edges_e2e.rs @@ -5,13 +5,14 @@ //! loop). use socket_patch_core::api::blob_fetcher::{ - fetch_blobs_by_hash, fetch_missing_blobs, get_missing_blobs, + fetch_blobs_by_hash, fetch_missing_blobs, format_fetch_result, get_missing_blobs, OnProgress, }; use socket_patch_core::api::client::{ApiClient, ApiClientOptions}; use socket_patch_core::manifest::schema::{PatchFileInfo, PatchManifest, PatchRecord}; use std::collections::HashMap; use std::collections::HashSet; use std::path::Path; +use std::sync::{Arc, Mutex}; /// Build an `ApiClient` pointed at a closed port so any *actual* HTTP /// call fails fast (connection refused). The short-circuit tests rely @@ -61,8 +62,8 @@ fn manifest_with_after_hashes(after: &[&str]) -> PatchManifest { } } -/// Count the directory entries under `dir` (used to prove a short-circuit -/// did zero filesystem writes). +/// Count the directory entries under `dir` (proves a short-circuit or an +/// error path wrote nothing: no final entry, no `.socket-dl-*` stage). fn dir_entry_count(dir: &Path) -> usize { std::fs::read_dir(dir).unwrap().count() } @@ -635,3 +636,351 @@ async fn partly_created_cache_dirs_are_removed_on_failure() { "the .socket/ that create_dir_all made before failing must be removed" ); } + +// ── uncreatable cache directory → per-entry write failure ─────────── +// +// The target directory path is routed through a REGULAR FILE +// (`tmp/notadir/`), so the writer's on-demand `create_dir_all` fails +// with ENOTDIR on every platform, even as root. The presence probes that +// run first (`get_missing_blobs`) also fail to +// stat through the file, so everything is reported missing and every +// entry is fetched. Each download succeeds and hash-verifies; only the +// disk write fails — so the outcome is the ordinary per-entry +// "Failed to write ... to disk" arm, and the blocking file survives. + +/// `fetch_missing_blobs` when the blobs directory cannot be created: +/// every blob is fetched (the mocks pin one call each) and reported as a +/// per-blob disk-write failure; nothing is downloaded or skipped. +#[tokio::test] +async fn fetch_missing_blobs_uncreatable_blobs_dir_is_per_blob_write_failure() { + let tmp = tempfile::tempdir().unwrap(); + let notadir = tmp.path().join("notadir"); + std::fs::write(¬adir, b"a regular file, not a directory").unwrap(); + let blobs = notadir.join("blobs"); + + let c1 = b"first blob body"; + let c2 = b"second blob body"; + let h1 = compute_git_sha256_from_bytes(c1); + let h2 = compute_git_sha256_from_bytes(c2); + let server = MockServer::start().await; + for (hash, body) in [(&h1, c1.to_vec()), (&h2, c2.to_vec())] { + Mock::given(method("GET")) + .and(path_matcher(format!("/patch/blob/{hash}"))) + .respond_with(ResponseTemplate::new(200).set_body_bytes(body)) + .expect(1) + .mount(&server) + .await; + } + let manifest = manifest_with_after_hashes(&[&h1, &h2]); + let client = proxy_client(&server.uri()); + + let result = fetch_missing_blobs(&manifest, &blobs, &client, None).await; + assert_eq!(result.total, 2, "both missing blobs are accounted for"); + assert_eq!(result.failed, 2, "every entry fails on the dir error"); + assert_eq!(result.downloaded, 0); + assert_eq!(result.skipped, 0); + assert_eq!(result.results.len(), 2); + let seen: HashSet<&str> = result.results.iter().map(|r| r.hash.as_str()).collect(); + assert_eq!(seen, HashSet::from([h1.as_str(), h2.as_str()])); + for entry in &result.results { + assert!(!entry.success); + let err = entry.error.as_deref().unwrap(); + assert!( + err.contains("Failed to write blob to disk"), + "per-blob disk-write message expected: {err}" + ); + } + // The path through the file is untouched: still a regular file. + assert!(notadir.is_file(), "the blocking file must be left alone"); +} + +/// `fetch_blobs_by_hash` (the rollback-path beforeHash fetcher) with an +/// uncreatable blobs directory: the skip bookkeeping sees nothing present, +/// every hash is fetched, and each is a per-blob disk-write failure — +/// `total == failed == results.len()`, `downloaded == skipped == 0`. +#[tokio::test] +async fn fetch_blobs_by_hash_uncreatable_blobs_dir_is_per_blob_write_failure() { + let tmp = tempfile::tempdir().unwrap(); + let notadir = tmp.path().join("notadir"); + std::fs::write(¬adir, b"file blocking the path").unwrap(); + let blobs = notadir.join("blobs"); + + let c1 = b"rollback blob one"; + let c2 = b"rollback blob two"; + let h1 = compute_git_sha256_from_bytes(c1); + let h2 = compute_git_sha256_from_bytes(c2); + let server = MockServer::start().await; + for (hash, body) in [(&h1, c1.to_vec()), (&h2, c2.to_vec())] { + Mock::given(method("GET")) + .and(path_matcher(format!("/patch/blob/{hash}"))) + .respond_with(ResponseTemplate::new(200).set_body_bytes(body)) + .expect(1) + .mount(&server) + .await; + } + let hashes: HashSet = [h1.clone(), h2.clone()].into_iter().collect(); + let client = proxy_client(&server.uri()); + + let result = fetch_blobs_by_hash(&hashes, &blobs, &client, None).await; + assert_eq!(result.total, 2); + assert_eq!(result.failed, 2); + assert_eq!(result.results.len(), 2); + assert_eq!(result.downloaded, 0); + assert_eq!( + result.skipped, 0, + "nothing can be present under a path that is not a directory" + ); + let seen: HashSet<&str> = result.results.iter().map(|r| r.hash.as_str()).collect(); + assert_eq!(seen, HashSet::from([h1.as_str(), h2.as_str()])); + for entry in &result.results { + assert!(!entry.success); + assert!(entry + .error + .as_deref() + .unwrap() + .contains("Failed to write blob to disk")); + } + assert!(notadir.is_file(), "the blocking file must be left alone"); +} + +/// A fetch that lands nothing creates nothing: with every blob 404 the +/// blobs directory — and the `.socket/` above it — must not come into +/// existence. The dir is the writer's to create, on the first verified +/// download only. +#[tokio::test] +async fn fetch_missing_blobs_all_not_found_leaves_blobs_dir_absent() { + let tmp = tempfile::tempdir().unwrap(); + let socket = tmp.path().join(".socket"); + let blobs = socket.join("blobs"); + + let h1 = "a".repeat(64); + let h2 = "b".repeat(64); + let server = MockServer::start().await; + for hash in [&h1, &h2] { + Mock::given(method("GET")) + .and(path_matcher(format!("/patch/blob/{hash}"))) + .respond_with(ResponseTemplate::new(404)) + .expect(1) + .mount(&server) + .await; + } + let manifest = manifest_with_after_hashes(&[&h1, &h2]); + let client = proxy_client(&server.uri()); + + let result = fetch_missing_blobs(&manifest, &blobs, &client, None).await; + assert_eq!(result.total, 2); + assert_eq!(result.failed, 2); + assert_eq!(result.downloaded, 0); + for entry in &result.results { + assert!(!entry.success); + assert!(entry.error.as_deref().unwrap().contains("not found")); + } + assert!( + !blobs.exists() && !socket.exists(), + "a fetch that lands nothing must leave no .socket/blobs/ residue" + ); +} + +// ── Blob-loop progress callback ────────────────────────────────────── + +/// The blob-download loop invokes the progress callback once per blob +/// with `(hash, 1-based index, total)`, BEFORE the fetch attempt — the +/// closed-port client makes every fetch fail, yet the callbacks must +/// all have fired with the documented 1-based sequential indices. +#[tokio::test] +async fn fetch_missing_blobs_progress_callback_is_one_based_and_pre_fetch() { + let tmp = tempfile::tempdir().unwrap(); + let blobs = tmp.path().join("blobs"); + std::fs::create_dir(&blobs).unwrap(); + let h1 = "a".repeat(64); + let h2 = "b".repeat(64); + let manifest = manifest_with_after_hashes(&[&h1, &h2]); + let client = dummy_client(); + + let calls: Arc>> = Arc::new(Mutex::new(Vec::new())); + let calls_cb = calls.clone(); + let cb: OnProgress = Box::new(move |h: &str, idx: usize, total: usize| { + calls_cb.lock().unwrap().push((h.to_string(), idx, total)); + }); + + let result = fetch_missing_blobs(&manifest, &blobs, &client, Some(&cb)).await; + assert_eq!(result.total, 2); + assert_eq!(result.downloaded, 0, "closed-port client downloads nothing"); + assert_eq!(result.failed, 2); + + let recorded = calls.lock().unwrap().clone(); + assert_eq!( + recorded.len(), + 2, + "callback fires exactly once per blob even when every fetch fails \ + (i.e. it fires before the attempt): {recorded:?}" + ); + // 1-based, sequential indices with the full total each time. The hash + // order is HashSet-nondeterministic, so match hashes as a set. + assert_eq!(recorded[0].1, 1, "first callback index is 1, not 0"); + assert_eq!(recorded[1].1, 2); + assert!(recorded.iter().all(|(_, _, t)| *t == 2)); + let seen: HashSet<&str> = recorded.iter().map(|(h, _, _)| h.as_str()).collect(); + assert_eq!(seen, HashSet::from([h1.as_str(), h2.as_str()])); +} + +// ── Disk-write failure arms (download OK, atomic write fails) ──────── +// +// unix-only: the pre-created cache directory is chmod'd 0o555 (readable +// + traversable, not writable), so `create_dir_all` still succeeds on +// the existing dir and execution reaches the write. A probe write +// verifies the mode bits actually deny writes (root / CAP_DAC_OVERRIDE +// would make the test vacuous) — checked BEFORE mounting any +// `.expect(1)` mock so a skip cannot trip drop-verification. + +/// Returns true when writing into `dir` is denied. Removes its probe on +/// the (privileged) success path. +#[cfg(unix)] +fn write_into_dir_denied(dir: &Path) -> bool { + let probe = dir.join(".write-probe"); + match std::fs::write(&probe, b"") { + Ok(()) => { + let _ = std::fs::remove_file(&probe); + false + } + Err(_) => true, + } +} + +/// Content downloads and hash-verifies, but the blob cannot be written: +/// a per-blob "Failed to write blob to disk" failure, not a panic, and +/// nothing — no final file, no `.socket-dl-*` stage — lands in the dir. +#[cfg(unix)] +#[tokio::test] +async fn fetch_missing_blobs_disk_write_failure_is_per_blob_failure() { + use std::os::unix::fs::PermissionsExt; + + let tmp = tempfile::tempdir().unwrap(); + let blobs = tmp.path().join("blobs"); + std::fs::create_dir(&blobs).unwrap(); + std::fs::set_permissions(&blobs, std::fs::Permissions::from_mode(0o555)).unwrap(); + if !write_into_dir_denied(&blobs) { + eprintln!("skipping: directory mode bits do not deny writes here (root?)"); + return; + } + + let content = b"verified content that cannot be persisted"; + let hash = compute_git_sha256_from_bytes(content); + + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path_matcher(format!("/patch/blob/{hash}"))) + .respond_with(ResponseTemplate::new(200).set_body_bytes(content.to_vec())) + .expect(1) + .mount(&server) + .await; + + let manifest = manifest_with_after_hashes(&[&hash]); + let client = proxy_client(&server.uri()); + + let result = fetch_missing_blobs(&manifest, &blobs, &client, None).await; + assert_eq!(result.total, 1); + assert_eq!( + result.downloaded, 0, + "an unwritable blob is not 'downloaded'" + ); + assert_eq!(result.failed, 1); + assert_eq!(result.skipped, 0); + let err = result.results[0].error.as_deref().unwrap(); + assert!( + err.contains("Failed to write blob to disk"), + "disk-write arm message expected: {err}" + ); + // Integrity: nothing may sit at the content-addressed path (a later + // run's presence check would trust it), and no staging turd either — + // the stage was never created, which is the invariant callers need. + assert!(!blobs.join(&hash).exists()); + assert_eq!( + dir_entry_count(&blobs), + 0, + "no partial file, no stage litter" + ); + + // Restore so tempdir teardown can't mask a failure. + std::fs::set_permissions(&blobs, std::fs::Permissions::from_mode(0o755)).unwrap(); +} + +// ── Mixed-outcome aggregation ──────────────────────────────────────── + +/// One run combining all three `download_entries` arms: a good blob, a +/// 404, and a hash-mismatch. Counters aggregate per-arm, each entry +/// carries its own arm's error, only the good blob lands on disk, and +/// `format_fetch_result` renders the genuinely mixed result. +#[tokio::test] +async fn fetch_missing_blobs_mixed_outcomes_aggregate_and_format() { + let content_ok = b"the one genuine blob body"; + let h_ok = compute_git_sha256_from_bytes(content_ok); + let h_404 = "a".repeat(64); + let h_bad = compute_git_sha256_from_bytes(b"content the server will not send"); + let tampered = b"tampered payload".to_vec(); + assert_ne!(compute_git_sha256_from_bytes(&tampered), h_bad); + + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path_matcher(format!("/patch/blob/{h_ok}"))) + .respond_with(ResponseTemplate::new(200).set_body_bytes(content_ok.to_vec())) + .expect(1) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(path_matcher(format!("/patch/blob/{h_404}"))) + .respond_with(ResponseTemplate::new(404)) + .expect(1) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(path_matcher(format!("/patch/blob/{h_bad}"))) + .respond_with(ResponseTemplate::new(200).set_body_bytes(tampered)) + .expect(1) + .mount(&server) + .await; + + let tmp = tempfile::tempdir().unwrap(); + let blobs = tmp.path().join("blobs"); + std::fs::create_dir(&blobs).unwrap(); + let manifest = manifest_with_after_hashes(&[&h_ok, &h_404, &h_bad]); + let client = proxy_client(&server.uri()); + + let result = fetch_missing_blobs(&manifest, &blobs, &client, None).await; + assert_eq!(result.total, 3); + assert_eq!(result.downloaded, 1); + assert_eq!(result.failed, 2); + assert_eq!(result.skipped, 0); + assert_eq!(result.results.len(), 3); + + // HashSet iteration order is nondeterministic — match by hash. + let by_hash = |h: &str| { + result + .results + .iter() + .find(|r| r.hash == h) + .unwrap_or_else(|| panic!("{h} missing from results")) + }; + let ok = by_hash(&h_ok); + assert!(ok.success && ok.error.is_none()); + let nf = by_hash(&h_404); + assert!(!nf.success); + assert!(nf.error.as_deref().unwrap().contains("not found")); + let bad = by_hash(&h_bad); + assert!(!bad.success); + assert!(bad.error.as_deref().unwrap().contains("mismatch")); + + // Only the verified blob was persisted, byte-for-byte, nothing else. + assert_eq!(std::fs::read(blobs.join(&h_ok)).unwrap(), content_ok); + assert!(!blobs.join(&h_404).exists()); + assert!(!blobs.join(&h_bad).exists()); + assert_eq!(dir_entry_count(&blobs), 1); + + // End-to-end formatter exercise with a genuinely mixed result. + let rendered = format_fetch_result(&result); + assert!(rendered.contains("Downloaded 1 blob\n"), "{rendered}"); + assert!( + rendered.contains("Failed to download 2 blobs"), + "{rendered}" + ); +} diff --git a/crates/socket-patch-core/tests/covgap_api_blob_fetcher.rs b/crates/socket-patch-core/tests/covgap_api_blob_fetcher.rs deleted file mode 100644 index 7f2d4af0b..000000000 --- a/crates/socket-patch-core/tests/covgap_api_blob_fetcher.rs +++ /dev/null @@ -1,441 +0,0 @@ -//! Coverage-gap tests for `api::blob_fetcher`'s never-executed error -//! branches: -//! -//! * an uncreatable cache directory — the dir is created by the first -//! download inside `stream_cache_entry_atomic`, never up front, -//! so the failure is a per-entry "Failed to write ... to disk" and a -//! fetch that lands nothing leaves no `.socket/blobs/` husk — driven -//! cross-platform via ENOTDIR (the target directory is routed *through -//! a regular file*, which fails even as root, unlike permission tricks); -//! * the blob-download loop's progress callback — the diff-loop -//! twin is tested in `blob_fetcher_edges_e2e.rs`, this one never ran; -//! * the "Failed to write blob/archive to disk" arms where the download -//! succeeded but the atomic cache write failed (unix-only, read-only directory); -//! * mixed-outcome aggregation across `download_entries` arms — each arm -//! is individually covered by the sibling suite, but a single run -//! combining success + 404 + hash-mismatch never was. -//! -//! Helper fns are deliberate twins of `blob_fetcher_edges_e2e.rs`'s -//! (integration-test binaries cannot share code except via -//! `tests/common/`, and these are api-suite-specific). - -use socket_patch_core::api::blob_fetcher::{ - fetch_blobs_by_hash, fetch_missing_blobs, format_fetch_result, OnProgress, -}; -use socket_patch_core::api::client::{ApiClient, ApiClientOptions}; -use socket_patch_core::hash::git_sha256::compute_git_sha256_from_bytes; -use socket_patch_core::manifest::schema::{PatchFileInfo, PatchManifest, PatchRecord}; -use std::collections::{HashMap, HashSet}; -use std::path::Path; -use std::sync::{Arc, Mutex}; -use wiremock::matchers::{method, path as path_matcher}; -use wiremock::{Mock, MockServer, ResponseTemplate}; - -/// Closed-port client: any *actual* HTTP call fails fast (connection -/// refused), so a branch that is supposed to return before any I/O -/// shows up as a wrong error message / `failed` count if it regresses -/// into fetching. -fn dummy_client() -> ApiClient { - ApiClient::new(ApiClientOptions { - api_url: "http://127.0.0.1:1".to_string(), - api_token: None, - route: socket_patch_core::api::client::ApiRoute::Proxy, - }) -} - -/// Public-proxy client pointed at a mock server `base` (binary fetches -/// go to `/patch/blob/`). -fn proxy_client(base: &str) -> ApiClient { - ApiClient::new(ApiClientOptions { - api_url: base.to_string(), - api_token: None, - route: socket_patch_core::api::client::ApiRoute::Proxy, - }) -} - -/// Manifest with one patch whose files carry the given `afterHash`es. -fn manifest_with_after_hashes(after: &[&str]) -> PatchManifest { - let mut files = HashMap::new(); - for (i, h) in after.iter().enumerate() { - files.insert( - format!("package/file{i}.js"), - PatchFileInfo { - before_hash: format!("{:0>64}", format!("be{i}")), - after_hash: (*h).to_string(), - }, - ); - } - let mut patches = HashMap::new(); - patches.insert( - "pkg:npm/test@1.0.0".to_string(), - PatchRecord { - uuid: "11111111-1111-4111-8111-111111111111".to_string(), - exported_at: "2024-01-01T00:00:00Z".to_string(), - files, - vulnerabilities: HashMap::new(), - description: "test".to_string(), - license: "MIT".to_string(), - tier: "free".to_string(), - }, - ); - PatchManifest { - patches, - setup: None, - } -} - -/// Count the directory entries under `dir` (proves an error path wrote -/// nothing — neither a final entry nor `.socket-dl-*` staging litter). -fn dir_entry_count(dir: &Path) -> usize { - std::fs::read_dir(dir).unwrap().count() -} - -// ── uncreatable cache directory → per-entry write failure ─────────── -// -// The target directory path is routed through a REGULAR FILE -// (`tmp/notadir/`), so the writer's on-demand `create_dir_all` fails -// with ENOTDIR on every platform, even as root. The presence probes that -// run first (`get_missing_blobs`) also fail to -// stat through the file, so everything is reported missing and every -// entry is fetched. Each download succeeds and hash-verifies; only the -// disk write fails — so the outcome is the ordinary per-entry -// "Failed to write ... to disk" arm, and the blocking file survives. - -/// `fetch_missing_blobs` when the blobs directory cannot be created: -/// every blob is fetched (the mocks pin one call each) and reported as a -/// per-blob disk-write failure; nothing is downloaded or skipped. -#[tokio::test] -async fn fetch_missing_blobs_uncreatable_blobs_dir_is_per_blob_write_failure() { - let tmp = tempfile::tempdir().unwrap(); - let notadir = tmp.path().join("notadir"); - std::fs::write(¬adir, b"a regular file, not a directory").unwrap(); - let blobs = notadir.join("blobs"); - - let c1 = b"first blob body"; - let c2 = b"second blob body"; - let h1 = compute_git_sha256_from_bytes(c1); - let h2 = compute_git_sha256_from_bytes(c2); - let server = MockServer::start().await; - for (hash, body) in [(&h1, c1.to_vec()), (&h2, c2.to_vec())] { - Mock::given(method("GET")) - .and(path_matcher(format!("/patch/blob/{hash}"))) - .respond_with(ResponseTemplate::new(200).set_body_bytes(body)) - .expect(1) - .mount(&server) - .await; - } - let manifest = manifest_with_after_hashes(&[&h1, &h2]); - let client = proxy_client(&server.uri()); - - let result = fetch_missing_blobs(&manifest, &blobs, &client, None).await; - assert_eq!(result.total, 2, "both missing blobs are accounted for"); - assert_eq!(result.failed, 2, "every entry fails on the dir error"); - assert_eq!(result.downloaded, 0); - assert_eq!(result.skipped, 0); - assert_eq!(result.results.len(), 2); - let seen: HashSet<&str> = result.results.iter().map(|r| r.hash.as_str()).collect(); - assert_eq!(seen, HashSet::from([h1.as_str(), h2.as_str()])); - for entry in &result.results { - assert!(!entry.success); - let err = entry.error.as_deref().unwrap(); - assert!( - err.contains("Failed to write blob to disk"), - "per-blob disk-write message expected: {err}" - ); - } - // The path through the file is untouched: still a regular file. - assert!(notadir.is_file(), "the blocking file must be left alone"); -} - -/// `fetch_blobs_by_hash` (the rollback-path beforeHash fetcher) with an -/// uncreatable blobs directory: the skip bookkeeping sees nothing present, -/// every hash is fetched, and each is a per-blob disk-write failure — -/// `total == failed == results.len()`, `downloaded == skipped == 0`. -#[tokio::test] -async fn fetch_blobs_by_hash_uncreatable_blobs_dir_is_per_blob_write_failure() { - let tmp = tempfile::tempdir().unwrap(); - let notadir = tmp.path().join("notadir"); - std::fs::write(¬adir, b"file blocking the path").unwrap(); - let blobs = notadir.join("blobs"); - - let c1 = b"rollback blob one"; - let c2 = b"rollback blob two"; - let h1 = compute_git_sha256_from_bytes(c1); - let h2 = compute_git_sha256_from_bytes(c2); - let server = MockServer::start().await; - for (hash, body) in [(&h1, c1.to_vec()), (&h2, c2.to_vec())] { - Mock::given(method("GET")) - .and(path_matcher(format!("/patch/blob/{hash}"))) - .respond_with(ResponseTemplate::new(200).set_body_bytes(body)) - .expect(1) - .mount(&server) - .await; - } - let hashes: HashSet = [h1.clone(), h2.clone()].into_iter().collect(); - let client = proxy_client(&server.uri()); - - let result = fetch_blobs_by_hash(&hashes, &blobs, &client, None).await; - assert_eq!(result.total, 2); - assert_eq!(result.failed, 2); - assert_eq!(result.results.len(), 2); - assert_eq!(result.downloaded, 0); - assert_eq!( - result.skipped, 0, - "nothing can be present under a path that is not a directory" - ); - let seen: HashSet<&str> = result.results.iter().map(|r| r.hash.as_str()).collect(); - assert_eq!(seen, HashSet::from([h1.as_str(), h2.as_str()])); - for entry in &result.results { - assert!(!entry.success); - assert!(entry - .error - .as_deref() - .unwrap() - .contains("Failed to write blob to disk")); - } - assert!(notadir.is_file(), "the blocking file must be left alone"); -} - -/// A fetch that lands nothing creates nothing: with every blob 404 the -/// blobs directory — and the `.socket/` above it — must not come into -/// existence. The dir is the writer's to create, on the first verified -/// download only. -#[tokio::test] -async fn fetch_missing_blobs_all_not_found_leaves_blobs_dir_absent() { - let tmp = tempfile::tempdir().unwrap(); - let socket = tmp.path().join(".socket"); - let blobs = socket.join("blobs"); - - let h1 = "a".repeat(64); - let h2 = "b".repeat(64); - let server = MockServer::start().await; - for hash in [&h1, &h2] { - Mock::given(method("GET")) - .and(path_matcher(format!("/patch/blob/{hash}"))) - .respond_with(ResponseTemplate::new(404)) - .expect(1) - .mount(&server) - .await; - } - let manifest = manifest_with_after_hashes(&[&h1, &h2]); - let client = proxy_client(&server.uri()); - - let result = fetch_missing_blobs(&manifest, &blobs, &client, None).await; - assert_eq!(result.total, 2); - assert_eq!(result.failed, 2); - assert_eq!(result.downloaded, 0); - for entry in &result.results { - assert!(!entry.success); - assert!(entry.error.as_deref().unwrap().contains("not found")); - } - assert!( - !blobs.exists() && !socket.exists(), - "a fetch that lands nothing must leave no .socket/blobs/ residue" - ); -} - -// ── Blob-loop progress callback ────────────────────────────────────── - -/// The blob-download loop invokes the progress callback once per blob -/// with `(hash, 1-based index, total)`, BEFORE the fetch attempt — the -/// closed-port client makes every fetch fail, yet the callbacks must -/// all have fired with the documented 1-based sequential indices. -/// (The diff-loop twin is covered in `blob_fetcher_edges_e2e.rs`; this -/// loop's callback had never run under test.) -#[tokio::test] -async fn fetch_missing_blobs_progress_callback_is_one_based_and_pre_fetch() { - let tmp = tempfile::tempdir().unwrap(); - let blobs = tmp.path().join("blobs"); - std::fs::create_dir(&blobs).unwrap(); - let h1 = "a".repeat(64); - let h2 = "b".repeat(64); - let manifest = manifest_with_after_hashes(&[&h1, &h2]); - let client = dummy_client(); - - let calls: Arc>> = Arc::new(Mutex::new(Vec::new())); - let calls_cb = calls.clone(); - let cb: OnProgress = Box::new(move |h: &str, idx: usize, total: usize| { - calls_cb.lock().unwrap().push((h.to_string(), idx, total)); - }); - - let result = fetch_missing_blobs(&manifest, &blobs, &client, Some(&cb)).await; - assert_eq!(result.total, 2); - assert_eq!(result.downloaded, 0, "closed-port client downloads nothing"); - assert_eq!(result.failed, 2); - - let recorded = calls.lock().unwrap().clone(); - assert_eq!( - recorded.len(), - 2, - "callback fires exactly once per blob even when every fetch fails \ - (i.e. it fires before the attempt): {recorded:?}" - ); - // 1-based, sequential indices with the full total each time. The hash - // order is HashSet-nondeterministic, so match hashes as a set. - assert_eq!(recorded[0].1, 1, "first callback index is 1, not 0"); - assert_eq!(recorded[1].1, 2); - assert!(recorded.iter().all(|(_, _, t)| *t == 2)); - let seen: HashSet<&str> = recorded.iter().map(|(h, _, _)| h.as_str()).collect(); - assert_eq!(seen, HashSet::from([h1.as_str(), h2.as_str()])); -} - -// ── Disk-write failure arms (download OK, atomic write fails) ──────── -// -// unix-only: the pre-created cache directory is chmod'd 0o555 (readable -// + traversable, not writable), so `create_dir_all` still succeeds on -// the existing dir and execution reaches the write. A probe write -// verifies the mode bits actually deny writes (root / CAP_DAC_OVERRIDE -// would make the test vacuous) — checked BEFORE mounting any -// `.expect(1)` mock so a skip cannot trip drop-verification. - -/// Returns true when writing into `dir` is denied. Removes its probe on -/// the (privileged) success path. -#[cfg(unix)] -fn write_into_dir_denied(dir: &Path) -> bool { - let probe = dir.join(".write-probe"); - match std::fs::write(&probe, b"") { - Ok(()) => { - let _ = std::fs::remove_file(&probe); - false - } - Err(_) => true, - } -} - -/// Content downloads and hash-verifies, but the blob cannot be written: -/// a per-blob "Failed to write blob to disk" failure, not a panic, and -/// nothing — no final file, no `.socket-dl-*` stage — lands in the dir. -#[cfg(unix)] -#[tokio::test] -async fn fetch_missing_blobs_disk_write_failure_is_per_blob_failure() { - use std::os::unix::fs::PermissionsExt; - - let tmp = tempfile::tempdir().unwrap(); - let blobs = tmp.path().join("blobs"); - std::fs::create_dir(&blobs).unwrap(); - std::fs::set_permissions(&blobs, std::fs::Permissions::from_mode(0o555)).unwrap(); - if !write_into_dir_denied(&blobs) { - eprintln!("skipping: directory mode bits do not deny writes here (root?)"); - return; - } - - let content = b"verified content that cannot be persisted"; - let hash = compute_git_sha256_from_bytes(content); - - let server = MockServer::start().await; - Mock::given(method("GET")) - .and(path_matcher(format!("/patch/blob/{hash}"))) - .respond_with(ResponseTemplate::new(200).set_body_bytes(content.to_vec())) - .expect(1) - .mount(&server) - .await; - - let manifest = manifest_with_after_hashes(&[&hash]); - let client = proxy_client(&server.uri()); - - let result = fetch_missing_blobs(&manifest, &blobs, &client, None).await; - assert_eq!(result.total, 1); - assert_eq!( - result.downloaded, 0, - "an unwritable blob is not 'downloaded'" - ); - assert_eq!(result.failed, 1); - assert_eq!(result.skipped, 0); - let err = result.results[0].error.as_deref().unwrap(); - assert!( - err.contains("Failed to write blob to disk"), - "disk-write arm message expected: {err}" - ); - // Integrity: nothing may sit at the content-addressed path (a later - // run's presence check would trust it), and no staging turd either — - // the stage was never created, which is the invariant callers need. - assert!(!blobs.join(&hash).exists()); - assert_eq!( - dir_entry_count(&blobs), - 0, - "no partial file, no stage litter" - ); - - // Restore so tempdir teardown can't mask a failure. - std::fs::set_permissions(&blobs, std::fs::Permissions::from_mode(0o755)).unwrap(); -} - -// ── Mixed-outcome aggregation ──────────────────────────────────────── - -/// One run combining all three `download_entries` arms: a good blob, a -/// 404, and a hash-mismatch. Counters aggregate per-arm, each entry -/// carries its own arm's error, only the good blob lands on disk, and -/// `format_fetch_result` renders the genuinely mixed result. -#[tokio::test] -async fn fetch_missing_blobs_mixed_outcomes_aggregate_and_format() { - let content_ok = b"the one genuine blob body"; - let h_ok = compute_git_sha256_from_bytes(content_ok); - let h_404 = "a".repeat(64); - let h_bad = compute_git_sha256_from_bytes(b"content the server will not send"); - let tampered = b"tampered payload".to_vec(); - assert_ne!(compute_git_sha256_from_bytes(&tampered), h_bad); - - let server = MockServer::start().await; - Mock::given(method("GET")) - .and(path_matcher(format!("/patch/blob/{h_ok}"))) - .respond_with(ResponseTemplate::new(200).set_body_bytes(content_ok.to_vec())) - .expect(1) - .mount(&server) - .await; - Mock::given(method("GET")) - .and(path_matcher(format!("/patch/blob/{h_404}"))) - .respond_with(ResponseTemplate::new(404)) - .expect(1) - .mount(&server) - .await; - Mock::given(method("GET")) - .and(path_matcher(format!("/patch/blob/{h_bad}"))) - .respond_with(ResponseTemplate::new(200).set_body_bytes(tampered)) - .expect(1) - .mount(&server) - .await; - - let tmp = tempfile::tempdir().unwrap(); - let blobs = tmp.path().join("blobs"); - std::fs::create_dir(&blobs).unwrap(); - let manifest = manifest_with_after_hashes(&[&h_ok, &h_404, &h_bad]); - let client = proxy_client(&server.uri()); - - let result = fetch_missing_blobs(&manifest, &blobs, &client, None).await; - assert_eq!(result.total, 3); - assert_eq!(result.downloaded, 1); - assert_eq!(result.failed, 2); - assert_eq!(result.skipped, 0); - assert_eq!(result.results.len(), 3); - - // HashSet iteration order is nondeterministic — match by hash. - let by_hash = |h: &str| { - result - .results - .iter() - .find(|r| r.hash == h) - .unwrap_or_else(|| panic!("{h} missing from results")) - }; - let ok = by_hash(&h_ok); - assert!(ok.success && ok.error.is_none()); - let nf = by_hash(&h_404); - assert!(!nf.success); - assert!(nf.error.as_deref().unwrap().contains("not found")); - let bad = by_hash(&h_bad); - assert!(!bad.success); - assert!(bad.error.as_deref().unwrap().contains("mismatch")); - - // Only the verified blob was persisted, byte-for-byte, nothing else. - assert_eq!(std::fs::read(blobs.join(&h_ok)).unwrap(), content_ok); - assert!(!blobs.join(&h_404).exists()); - assert!(!blobs.join(&h_bad).exists()); - assert_eq!(dir_entry_count(&blobs), 1); - - // End-to-end formatter exercise with a genuinely mixed result. - let rendered = format_fetch_result(&result); - assert!(rendered.contains("Downloaded 1 blob\n"), "{rendered}"); - assert!( - rendered.contains("Failed to download 2 blobs"), - "{rendered}" - ); -} diff --git a/crates/socket-patch-core/tests/covgap_crawlers_composer_crawler.rs b/crates/socket-patch-core/tests/covgap_crawlers_composer_crawler.rs deleted file mode 100644 index db21a33fd..000000000 --- a/crates/socket-patch-core/tests/covgap_crawlers_composer_crawler.rs +++ /dev/null @@ -1,250 +0,0 @@ -//! Coverage-gap integration tests for `crawlers::composer_crawler`'s -//! global-home discovery chain: the fall-through when `COMPOSER_HOME` names -//! a nonexistent directory, and the `composer global config home` shell-out -//! success branches (driven by a fake `composer` shim on PATH — the real -//! production path on any machine with composer installed and COMPOSER_HOME -//! unset). Companion to `crawler_composer_e2e.rs`, whose serial env -//! stubbing conventions these tests mirror. - -use std::path::Path; - -use socket_patch_core::crawlers::types::CrawlerOptions; -use socket_patch_core::crawlers::ComposerCrawler; - -fn global_options_at(root: &Path) -> CrawlerOptions { - CrawlerOptions { - cwd: root.to_path_buf(), - global: true, - global_prefix: None, - } -} - -/// Saves an env var's prior value on construction and restores it on drop, -/// so a panicking assert can't leak a stubbed COMPOSER_HOME/HOME/PATH into -/// later serial tests in this binary. -struct EnvVarGuard { - key: &'static str, - prev: Option, -} - -impl EnvVarGuard { - fn set(key: &'static str, value: &std::ffi::OsStr) -> Self { - let prev = std::env::var_os(key); - std::env::set_var(key, value); - EnvVarGuard { key, prev } - } - - fn remove(key: &'static str) -> Self { - let prev = std::env::var_os(key); - std::env::remove_var(key); - EnvVarGuard { key, prev } - } -} - -impl Drop for EnvVarGuard { - fn drop(&mut self) { - match self.prev.take() { - Some(v) => std::env::set_var(self.key, v), - None => std::env::remove_var(self.key), - } - } -} - -/// A set-but-BROKEN `COMPOSER_HOME` (naming a nonexistent directory) must -/// fall through the discovery chain — composer CLI, then platform defaults — -/// rather than yield the bogus path or bail empty. -/// `get_vendor_paths_global_via_composer_home_env` in the companion suite -/// pins the env var's success arm; this pins the failure arm's ordering. -#[tokio::test] -#[serial_test::serial] -async fn get_vendor_paths_global_nonexistent_composer_home_falls_back() { - let tmp = tempfile::tempdir().unwrap(); - let dot_composer_vendor = tmp.path().join(".composer").join("vendor"); - tokio::fs::create_dir_all(&dot_composer_vendor) - .await - .unwrap(); - // PATH stubbed to a binary-free tempdir so `composer global config - // home` on a machine with composer installed can't short-circuit the - // fallback chain. - let empty_path = tempfile::tempdir().unwrap(); - let bogus_home = tmp.path().join("does-not-exist"); - - let _composer_home = EnvVarGuard::set("COMPOSER_HOME", bogus_home.as_os_str()); - let _home = EnvVarGuard::set("HOME", tmp.path().as_os_str()); - let _path = EnvVarGuard::set("PATH", empty_path.path().as_os_str()); - // Composer's other platform defaults (`%APPDATA%\Composer` on Windows, - // `$XDG_CONFIG_HOME/composer`) would outrank the HOME candidate here. - let _app_data = EnvVarGuard::remove("APPDATA"); - let _xdg = EnvVarGuard::remove("XDG_CONFIG_HOME"); - - let crawler = ComposerCrawler; - let paths = crawler - .get_vendor_paths(&global_options_at(tmp.path())) - .await - .unwrap(); - - assert_eq!( - paths, - vec![dot_composer_vendor], - "a set-but-nonexistent COMPOSER_HOME must fall through to the \ - HOME/.composer platform default, not yield the bogus path or empty" - ); -} - -/// Write an executable `composer` shim into `dir` that ignores its -/// arguments and echoes `echo_path` — a stand-in for `composer global -/// config home`. The shebang names /bin/sh by absolute path, so the script -/// executes even with PATH stripped down to the shim dir alone. -#[cfg(unix)] -fn write_composer_shim(dir: &Path, echo_path: &Path) { - use std::os::unix::fs::PermissionsExt; - let shim = dir.join("composer"); - std::fs::write( - &shim, - format!("#!/bin/sh\necho '{}'\n", echo_path.display()), - ) - .unwrap(); - std::fs::set_permissions(&shim, std::fs::Permissions::from_mode(0o755)).unwrap(); -} - -/// `composer global config home` answering an EXISTING directory is the -/// production path on any machine with composer installed and -/// COMPOSER_HOME unset: the CLI's answer must win over the HOME/.composer -/// platform default. -#[cfg(unix)] -#[tokio::test] -#[serial_test::serial] -async fn get_vendor_paths_global_composer_cli_answer_wins() { - let tmp = tempfile::tempdir().unwrap(); - // What the fake CLI reports as the composer home; its vendor/ exists. - let cli_home = tmp.path().join("cli-home"); - let cli_vendor = cli_home.join("vendor"); - tokio::fs::create_dir_all(&cli_vendor).await.unwrap(); - // HOME tripwire: if the CLI branch were skipped, discovery would land - // here instead and the equality assert below would catch it. - let home = tempfile::tempdir().unwrap(); - tokio::fs::create_dir_all(home.path().join(".composer").join("vendor")) - .await - .unwrap(); - let shim_dir = tempfile::tempdir().unwrap(); - write_composer_shim(shim_dir.path(), &cli_home); - - let _composer_home = EnvVarGuard::remove("COMPOSER_HOME"); - let _home = EnvVarGuard::set("HOME", home.path().as_os_str()); - let _path = EnvVarGuard::set("PATH", shim_dir.path().as_os_str()); - - let crawler = ComposerCrawler; - let paths = crawler - .get_vendor_paths(&global_options_at(tmp.path())) - .await - .unwrap(); - - assert_eq!( - paths, - vec![cli_vendor], - "the composer CLI's home answer must win over the HOME/.composer fallback" - ); -} - -/// The CLI answering a NONEXISTENT directory must fall through to the -/// platform defaults rather than yield the bogus path or bail empty. -#[cfg(unix)] -#[tokio::test] -#[serial_test::serial] -async fn get_vendor_paths_global_composer_cli_nonexistent_home_falls_back() { - let tmp = tempfile::tempdir().unwrap(); - let home = tempfile::tempdir().unwrap(); - let dot_composer_vendor = home.path().join(".composer").join("vendor"); - tokio::fs::create_dir_all(&dot_composer_vendor) - .await - .unwrap(); - // Each shim lives in its own tempdir; nothing rewrites a shim in place. - let shim_dir = tempfile::tempdir().unwrap(); - write_composer_shim(shim_dir.path(), &tmp.path().join("cli-home-missing")); - - let _composer_home = EnvVarGuard::remove("COMPOSER_HOME"); - let _home = EnvVarGuard::set("HOME", home.path().as_os_str()); - let _path = EnvVarGuard::set("PATH", shim_dir.path().as_os_str()); - // `$XDG_CONFIG_HOME/composer` would outrank the HOME candidate here. - let _xdg = EnvVarGuard::remove("XDG_CONFIG_HOME"); - - let crawler = ComposerCrawler; - let paths = crawler - .get_vendor_paths(&global_options_at(tmp.path())) - .await - .unwrap(); - - assert_eq!( - paths, - vec![dot_composer_vendor], - "a CLI answer naming a nonexistent home must fall through to HOME/.composer" - ); -} - -/// #586: on an XDG system Composer 2 looks for its home in the XDG config -/// dir first and only then in `~/.composer` (`Factory::getHomeDir`). A -/// leftover `~/.composer` (Composer 1 era, even an empty one) must not -/// hide the real global install under `~/.config/composer` when the -/// `composer` CLI can't be asked. -#[cfg(not(windows))] -#[tokio::test] -#[serial_test::serial] -async fn get_vendor_paths_global_xdg_home_outranks_stale_dot_composer() { - let home = tempfile::tempdir().unwrap(); - tokio::fs::create_dir_all(home.path().join(".composer")) - .await - .unwrap(); - let xdg_vendor = home.path().join(".config").join("composer").join("vendor"); - tokio::fs::create_dir_all(&xdg_vendor).await.unwrap(); - let empty_path = tempfile::tempdir().unwrap(); - - let _composer_home = EnvVarGuard::remove("COMPOSER_HOME"); - let _home = EnvVarGuard::set("HOME", home.path().as_os_str()); - let _path = EnvVarGuard::set("PATH", empty_path.path().as_os_str()); - let _xdg_config = EnvVarGuard::remove("XDG_CONFIG_HOME"); - // Any `XDG_*` variable makes Composer treat the system as XDG. - let _xdg_marker = EnvVarGuard::set("XDG_SESSION_TYPE", std::ffi::OsStr::new("tty")); - - let paths = ComposerCrawler - .get_vendor_paths(&global_options_at(home.path())) - .await - .unwrap(); - - assert_eq!( - paths, - vec![xdg_vendor], - "Composer 2 picks ~/.config/composer over ~/.composer on an XDG system" - ); -} - -/// The same layout with `XDG_CONFIG_HOME` set: Composer 2 uses -/// `$XDG_CONFIG_HOME/composer`, still ahead of a stale `~/.composer`. -#[cfg(not(windows))] -#[tokio::test] -#[serial_test::serial] -async fn get_vendor_paths_global_xdg_config_home_outranks_stale_dot_composer() { - let home = tempfile::tempdir().unwrap(); - tokio::fs::create_dir_all(home.path().join(".composer").join("vendor")) - .await - .unwrap(); - let xdg = tempfile::tempdir().unwrap(); - let xdg_vendor = xdg.path().join("composer").join("vendor"); - tokio::fs::create_dir_all(&xdg_vendor).await.unwrap(); - let empty_path = tempfile::tempdir().unwrap(); - - let _composer_home = EnvVarGuard::remove("COMPOSER_HOME"); - let _home = EnvVarGuard::set("HOME", home.path().as_os_str()); - let _path = EnvVarGuard::set("PATH", empty_path.path().as_os_str()); - let _xdg_config = EnvVarGuard::set("XDG_CONFIG_HOME", xdg.path().as_os_str()); - - let paths = ComposerCrawler - .get_vendor_paths(&global_options_at(home.path())) - .await - .unwrap(); - - assert_eq!( - paths, - vec![xdg_vendor], - "Composer 2 picks $XDG_CONFIG_HOME/composer over ~/.composer" - ); -} diff --git a/crates/socket-patch-core/tests/covgap_crawlers_npm_crawler.rs b/crates/socket-patch-core/tests/covgap_crawlers_npm_crawler.rs deleted file mode 100644 index 298999e55..000000000 --- a/crates/socket-patch-core/tests/covgap_crawlers_npm_crawler.rs +++ /dev/null @@ -1,423 +0,0 @@ -//! Coverage-gap integration tests for `crawlers::npm_crawler`: the -//! never-executed skip/fallback regions of the store walkers -//! (`collect_nested_node_modules`, `collect_nested_store_entries`, -//! `gather_scoped_packages`) and every reject/fallback gate of -//! `find_store_peer_variant_copies` (pnpm and vlt stores). Each test stages the real on-disk shape -//! that reaches its region and asserts resolver/scan OUTPUT, not just -//! survival. Companion to `crawler_npm_e2e.rs` (helpers mirrored from -//! there). - -use std::path::Path; - -use socket_patch_core::crawlers::npm_crawler::find_store_peer_variant_copies; -use socket_patch_core::crawlers::types::CrawlerOptions; -use socket_patch_core::crawlers::NpmCrawler; - -fn options_at(root: &Path) -> CrawlerOptions { - CrawlerOptions { - cwd: root.to_path_buf(), - global: false, - global_prefix: None, - } -} - -/// Stage a package inside node_modules. `name` may include a `@scope/` -/// prefix. -async fn stage_npm_pkg(node_modules: &Path, name: &str, version: &str) { - let pkg_dir = node_modules.join(name); - tokio::fs::create_dir_all(&pkg_dir).await.unwrap(); - let pkg_json = format!(r#"{{"name":"{name}","version":"{version}"}}"#); - tokio::fs::write(pkg_dir.join("package.json"), pkg_json) - .await - .unwrap(); -} - -// ── plain-FILE store markers at a node_modules root ──────────── - -/// A stray plain FILE named `.pnpm` or `.registry.npmjs.org` at a -/// `node_modules` root (trivially stageable; real trees carry such junk) -/// must be skipped by BOTH walks — the resolver's nested-nm BFS and the -/// scan pass — without erroring out and without being mistaken for a -/// virtual store. -#[tokio::test] -#[serial_test::parallel] -async fn find_by_purls_and_crawl_all_skip_plain_file_store_markers() { - let tmp = tempfile::tempdir().unwrap(); - let nm = tmp.path().join("node_modules"); - stage_npm_pkg(&nm, "foo", "1.0.0").await; - // Plain files where the store DIRS would live. - tokio::fs::write(nm.join(".pnpm"), b"junk").await.unwrap(); - tokio::fs::write(nm.join(".registry.npmjs.org"), b"junk") - .await - .unwrap(); - - // Resolver walk: foo resolves at the root, the marker files are - // skipped (no error, no phantom copies). - let crawler = NpmCrawler::new(); - let purls = vec!["pkg:npm/foo@1.0.0".to_string()]; - let result = crawler.find_by_purls(&nm, &purls).await.unwrap(); - assert_eq!(result.len(), 1, "only foo resolves; got {result:?}"); - let copies = result.get("pkg:npm/foo@1.0.0").unwrap(); - assert_eq!(copies.len(), 1, "exactly one physical copy of foo"); - assert_eq!(copies[0].path, nm.join("foo")); - - // Scan walk: exactly one package inventoried. - let packages = crawler.crawl_all(&options_at(tmp.path())).await; - assert_eq!( - packages.len(), - 1, - "marker files must not add or hide packages; got {:?}", - packages.iter().map(|p| p.purl.as_str()).collect::>() - ); - assert_eq!(packages[0].purl, "pkg:npm/foo@1.0.0"); - assert_eq!(packages[0].path, nm.join("foo")); -} - -// ── stray entries inside a `@scope` dir during the nested-nm BFS ─ - -/// The resolver's nested-node_modules walk descends `@scope/` dirs -/// looking for deeper `node_modules`. Hidden entries (`.DS_Store`-style -/// junk is ubiquitous inside scope dirs on macOS) and plain files inside -/// the `@scope` dir must be skipped without breaking or polluting the -/// walk — the target below the real scoped package must still resolve. -#[tokio::test] -#[serial_test::parallel] -async fn find_by_purls_ignores_stray_entries_in_scope_dirs() { - let tmp = tempfile::tempdir().unwrap(); - let nm = tmp.path().join("node_modules"); - stage_npm_pkg(&nm, "@scope/pkg", "1.0.0").await; - // The target lives ONLY inside the scoped package's nested - // node_modules, so resolving it forces the walk through @scope. - stage_npm_pkg(&nm.join("@scope/pkg/node_modules"), "leaf", "2.0.0").await; - // Stray entries inside the @scope dir itself: a hidden dir and a - // plain file. - tokio::fs::create_dir_all(nm.join("@scope/.cache")) - .await - .unwrap(); - tokio::fs::write(nm.join("@scope/notes.txt"), b"junk") - .await - .unwrap(); - - let crawler = NpmCrawler::new(); - let purls = vec![ - "pkg:npm/leaf@2.0.0".to_string(), - "pkg:npm/@scope/pkg@1.0.0".to_string(), - ]; - let result = crawler.find_by_purls(&nm, &purls).await.unwrap(); - - assert_eq!(result.len(), 2, "both packages resolve; got {result:?}"); - assert_eq!( - result.get("pkg:npm/leaf@2.0.0").unwrap()[0].path, - nm.join("@scope/pkg/node_modules/leaf"), - "leaf resolves only by descending @scope/pkg's nested node_modules" - ); - assert_eq!( - result.get("pkg:npm/@scope/pkg@1.0.0").unwrap()[0].path, - nm.join("@scope/pkg") - ); -} - -// ── scoped root-linked pnpm direct dep: dedup + no symlink descend ─ - -/// Scoped twin of the unscoped `identity_seen` dedup: a SCOPED pnpm -/// direct dep is root-linked (`node_modules/@scope/leaf` → store copy), -/// so the importer pass inventories it first and the store pass must -/// skip the redundant re-read — while STILL descending the store copy, -/// because a bundled dep is a real dir physically present only there. -/// The importer pass must inventory the symlinked scoped package without -/// descending through the symlink (descending would surface the bundled -/// dep at its symlink-relative path instead of its physical store home). -#[cfg(unix)] -#[tokio::test] -#[serial_test::parallel] -async fn crawl_all_dedups_scoped_root_linked_pnpm_direct_dep_and_skips_symlink_descend() { - use std::os::unix::fs::symlink; - - let tmp = tempfile::tempdir().unwrap(); - let nm = tmp.path().join("node_modules"); - let store = nm.join(".pnpm"); - - // Store copy of the scoped package… - stage_npm_pkg( - &store.join("@scope+leaf@2.0.0").join("node_modules"), - "@scope/leaf", - "2.0.0", - ) - .await; - let store_leaf = store.join("@scope+leaf@2.0.0/node_modules/@scope/leaf"); - // …with a bundled dep as a REAL dir below it (its only physical home). - stage_npm_pkg(&store_leaf.join("node_modules"), "bundled", "1.0.0").await; - - // Importer root: the scoped direct dep is a symlink into the store. - tokio::fs::create_dir_all(nm.join("@scope")).await.unwrap(); - symlink(&store_leaf, nm.join("@scope/leaf")).unwrap(); - - let crawler = NpmCrawler::new(); - let result = crawler.crawl_all(&options_at(tmp.path())).await; - - let purls: Vec<&str> = result.iter().map(|p| p.purl.as_str()).collect(); - assert_eq!( - result.len(), - 2, - "exactly @scope/leaf (once) + bundled; got {purls:?}" - ); - let leaf_copies: Vec<_> = result - .iter() - .filter(|p| p.purl == "pkg:npm/@scope/leaf@2.0.0") - .collect(); - assert_eq!( - leaf_copies.len(), - 1, - "the root-linked scoped dep must be inventoried exactly once" - ); - assert_eq!( - leaf_copies[0].path, - nm.join("@scope/leaf"), - "the importer-root path wins over the store copy" - ); - let bundled = result - .iter() - .find(|p| p.purl == "pkg:npm/bundled@1.0.0") - .unwrap_or_else(|| panic!("bundled dep must be inventoried; got {purls:?}")); - assert_eq!( - bundled.path, - store_leaf.join("node_modules/bundled"), - "bundled dep surfaces at its physical store home — reached by \ - descending the store entry (dedup skip must not stop the \ - descent), never through the importer symlink" - ); -} - -// ── find_store_peer_variant_copies: pnpm probe gates ──────────── - -/// All four reject/fallback gates of the peer-variant store probe, in one -/// staged store: -/// - an UNDECODABLE entry name stays probeable and its real matching copy -/// IS returned (a regression here silently skips a physical copy, -/// leaving a live vulnerable file after apply reports success); -/// - an entry whose candidate dir is absent is skipped; -/// - a candidate that is a SYMLINK to another entry's copy is skipped -/// (that copy is already yielded via its own entry — no double count); -/// - an imposter whose package.json disagrees with the decoded name is -/// rejected by the authority probe. -#[cfg(unix)] -#[tokio::test] -#[serial_test::parallel] -async fn find_store_peer_variant_copies_probe_gates() { - use std::os::unix::fs::symlink; - - let tmp = tempfile::tempdir().unwrap(); - let nm = tmp.path().join("node_modules"); - let store = nm.join(".pnpm"); - - // (a) The primary peer-variant copy. - stage_npm_pkg( - &store.join("foo@1.0.0(react@17.0.2)").join("node_modules"), - "foo", - "1.0.0", - ) - .await; - let primary = store.join("foo@1.0.0(react@17.0.2)/node_modules/foo"); - - // (b) A real twin under another peer combination — must be returned. - stage_npm_pkg( - &store.join("foo@1.0.0(react@18.2.0)").join("node_modules"), - "foo", - "1.0.0", - ) - .await; - let twin = store.join("foo@1.0.0(react@18.2.0)/node_modules/foo"); - - // (c) An UNDECODABLE entry name (`_` reads as a legacy peer suffix, - // so decode yields None) holding a real matching copy — must be - // returned: undecodable stays probeable. - stage_npm_pkg( - &store.join("foo_bar@1.0.0").join("node_modules"), - "foo", - "1.0.0", - ) - .await; - let undecodable_copy = store.join("foo_bar@1.0.0/node_modules/foo"); - - // (d) A matching-decoded entry whose node_modules holds no `foo` dir. - tokio::fs::create_dir_all(store.join("foo@1.0.0(vue@3.2.0)").join("node_modules")) - .await - .unwrap(); - - // (e) A matching-decoded entry whose `foo` is a SYMLINK to twin (b). - let sym_nm = store.join("foo@1.0.0(sym@1.0.0)").join("node_modules"); - tokio::fs::create_dir_all(&sym_nm).await.unwrap(); - symlink(&twin, sym_nm.join("foo")).unwrap(); - - // (f) An imposter: the entry NAME decodes to foo@1.0.0 (passes the - // advertisement filter) but the package.json inside says 1.0.1. - stage_npm_pkg( - &store.join("foo@1.0.0(baz@1.0.0)").join("node_modules"), - "foo", - "1.0.1", - ) - .await; - - let copies = find_store_peer_variant_copies(&primary).await; - - let got: std::collections::HashSet<_> = copies.iter().cloned().collect(); - let want: std::collections::HashSet<_> = [twin.clone(), undecodable_copy.clone()] - .into_iter() - .collect(); - assert_eq!( - got, want, - "exactly the real twin and the undecodable-entry copy — no \ - primary, no symlink duplicate, no absent candidate, no imposter" - ); -} - -/// Fail-safe gate: when the PRIMARY's package.json is unreadable there is -/// no safe way to identify twins, so NONE are reported — even though a -/// perfectly probeable twin exists in the store (the primary itself is -/// still handled by the caller). -#[tokio::test] -#[serial_test::parallel] -async fn find_store_peer_variant_copies_unreadable_primary_returns_empty() { - let tmp = tempfile::tempdir().unwrap(); - let nm = tmp.path().join("node_modules"); - let store = nm.join(".pnpm"); - - // Primary: a bare dir with NO package.json. - let primary = store.join("foo@1.0.0(react@17.0.2)/node_modules/foo"); - tokio::fs::create_dir_all(&primary).await.unwrap(); - - // A fully-formed twin that WOULD match if the primary were readable. - stage_npm_pkg( - &store.join("foo@1.0.0(react@18.2.0)").join("node_modules"), - "foo", - "1.0.0", - ) - .await; - - let copies = find_store_peer_variant_copies(&primary).await; - assert!( - copies.is_empty(), - "unreadable primary identity ⇒ no twins reported; got {copies:?}" - ); -} - -// ── find_store_peer_variant_copies: vlt probe gates ───────────── - -/// vlt's fan-out from a workspace member's link: the store lives beside -/// the ROOT `node_modules`, reachable only on the link's canonical chain. -/// Real copies whose DepID decodes to the primary's `name@version` are -/// returned (a `~peer.` twin); a git dependency of the same -/// `name@version` is never a variant (the resolver reports it as a primary -/// of its own), and a store entry reached through a link is not a store -/// entry. -#[cfg(unix)] -#[tokio::test] -#[serial_test::parallel] -async fn find_store_peer_variant_copies_vlt_member_link_and_git_copy() { - use std::os::unix::fs::symlink; - - let tmp = tempfile::tempdir().unwrap(); - let store = tmp.path().join("node_modules").join(".vlt"); - let entry = |id: &str| store.join(id).join("node_modules"); - stage_npm_pkg(&entry("~npm~foo@1.0.0~peer.2"), "foo", "1.0.0").await; - stage_npm_pkg(&entry("~npm~foo@1.0.0~peer.3"), "foo", "1.0.0").await; - stage_npm_pkg(&entry("git~github_cu+foo~v1.0.0"), "foo", "1.0.0").await; - let elsewhere = tempfile::tempdir().unwrap(); - stage_npm_pkg(&elsewhere.path().join("node_modules"), "foo", "1.0.0").await; - symlink(elsewhere.path(), store.join("~npm~foo@1.0.0~peer.4")).unwrap(); - - let member_nm = tmp.path().join("packages/a/node_modules"); - tokio::fs::create_dir_all(&member_nm).await.unwrap(); - symlink( - "../../../node_modules/.vlt/~npm~foo@1.0.0~peer.2/node_modules/foo", - member_nm.join("foo"), - ) - .unwrap(); - - let copies = find_store_peer_variant_copies(&member_nm.join("foo")).await; - assert_eq!( - copies - .iter() - .map(|p| std::fs::canonicalize(p).unwrap()) - .collect::>(), - vec![std::fs::canonicalize(entry("~npm~foo@1.0.0~peer.3").join("foo")).unwrap()], - "exactly the peer twin; got {copies:?}" - ); -} - -/// Fail-safe gate on the vlt side too: an unreadable primary identity -/// reports no twins. -#[tokio::test] -#[serial_test::parallel] -async fn find_store_peer_variant_copies_vlt_unreadable_primary_returns_empty() { - let tmp = tempfile::tempdir().unwrap(); - let store = tmp.path().join("node_modules").join(".vlt"); - let primary = store.join("~npm~foo@1.0.0~peer.2/node_modules/foo"); - tokio::fs::create_dir_all(&primary).await.unwrap(); - stage_npm_pkg( - &store.join("~npm~foo@1.0.0~peer.3").join("node_modules"), - "foo", - "1.0.0", - ) - .await; - assert!(find_store_peer_variant_copies(&primary).await.is_empty()); -} - -// ── pnpm<=3 legacy store: stray node_modules + depth-1 home ──── - -/// Two edge shapes of the nested legacy-store walk: -/// - a stray `node_modules` child inside the store host belongs to a -/// parent entry, never a name/version coordinate — a decoy package -/// staged below it must NOT be inventoried; -/// - a depth-1 package home (`//node_modules/`) has no -/// `/` in its host-relative path, so the raw component becomes an -/// undecodable-but-probed entry name and the package IS inventoried. -#[tokio::test] -#[serial_test::parallel] -async fn crawl_all_legacy_store_skips_stray_node_modules_and_probes_depth1_home() { - let tmp = tempfile::tempdir().unwrap(); - let nm = tmp.path().join("node_modules"); - let store = nm.join(".registry.npmjs.org"); - - // Normal legacy shape: //node_modules/. - stage_npm_pkg(&store.join("mkdirp/0.5.5/node_modules"), "mkdirp", "0.5.5").await; - // Depth-1 home: /node_modules/ (no version dir). - stage_npm_pkg(&store.join("directpkg/node_modules"), "directpkg", "3.0.0").await; - // Stray node_modules child of the host, shaped like a package home so - // a skip regression would surface the decoy in the inventory. - stage_npm_pkg( - &store.join("node_modules/decoy/9.9.9/node_modules"), - "decoy", - "9.9.9", - ) - .await; - - let crawler = NpmCrawler::new(); - let result = crawler.crawl_all(&options_at(tmp.path())).await; - - let purls: Vec<&str> = result.iter().map(|p| p.purl.as_str()).collect(); - assert_eq!( - result.len(), - 2, - "exactly mkdirp + directpkg, decoy stays out; got {purls:?}" - ); - let by_purl = |purl: &str| { - result - .iter() - .find(|p| p.purl == purl) - .unwrap_or_else(|| panic!("{purl} must be inventoried; got {purls:?}")) - }; - assert_eq!( - by_purl("pkg:npm/mkdirp@0.5.5").path, - store.join("mkdirp/0.5.5/node_modules/mkdirp") - ); - assert_eq!( - by_purl("pkg:npm/directpkg@3.0.0").path, - store.join("directpkg/node_modules/directpkg"), - "a depth-1 package home must be probed (undecodable entry name)" - ); - assert!( - !purls.contains(&"pkg:npm/decoy@9.9.9"), - "a stray node_modules child of the store host must be skipped" - ); -} diff --git a/crates/socket-patch-core/tests/covgap_patch_apply.rs b/crates/socket-patch-core/tests/covgap_patch_apply.rs deleted file mode 100644 index 83926de6e..000000000 --- a/crates/socket-patch-core/tests/covgap_patch_apply.rs +++ /dev/null @@ -1,244 +0,0 @@ -//! Coverage-gap integration tests for `patch::apply`: the pnpm and vlt -//! peer-variant copy FAILURE aggregation in `apply_package_patch`, and the -//! copy-on-write guard for hardlinked store files. -//! -//! The success half (patching/healing every twin) lives in -//! `crawler_npm_e2e.rs`; these exercise the fail-closed branch — a copy -//! that cannot be patched must flip the whole result to failure with a -//! "store copy ... failed to patch: ..." note, never report the CVE -//! fixed while a physical twin stays divergent. - -use std::collections::HashMap; -use std::path::{Path, PathBuf}; - -use socket_patch_core::hash::git_sha256::compute_git_sha256_from_bytes; -use socket_patch_core::manifest::schema::PatchFileInfo; -use socket_patch_core::patch::apply::{apply_package_patch, MismatchPolicy, PatchSources}; - -const ORIGINAL: &[u8] = b"module.exports = 'vulnerable';\n"; -const PATCHED: &[u8] = b"module.exports = 'fixed';\n"; - -/// Stage one store entry `//node_modules/foo` holding a -/// package.json and, when `index_content` is `Some`, an `index.js`. -/// Returns the staged package root. -async fn stage_store_entry(store: &Path, entry: &str, index_content: Option<&[u8]>) -> PathBuf { - let pkg = store.join(entry).join("node_modules").join("foo"); - tokio::fs::create_dir_all(&pkg).await.unwrap(); - tokio::fs::write( - pkg.join("package.json"), - r#"{"name":"foo","version":"1.0.0"}"#, - ) - .await - .unwrap(); - if let Some(content) = index_content { - tokio::fs::write(pkg.join("index.js"), content) - .await - .unwrap(); - } - pkg -} - -/// Shared apply invocation: blob-only sources staged under `root`, one -/// patched file `package/index.js` (ORIGINAL → PATCHED), Warn policy. -async fn apply_foo(root: &Path, primary: &Path) -> socket_patch_core::patch::apply::ApplyResult { - let before_hash = compute_git_sha256_from_bytes(ORIGINAL); - let after_hash = compute_git_sha256_from_bytes(PATCHED); - - let blobs = root.join("blobs"); - tokio::fs::create_dir_all(&blobs).await.unwrap(); - tokio::fs::write(blobs.join(&after_hash), PATCHED) - .await - .unwrap(); - - let mut files = HashMap::new(); - files.insert( - "package/index.js".to_string(), - PatchFileInfo { - before_hash, - after_hash, - }, - ); - let sources = PatchSources { - blobs_path: &blobs, - mem_blobs: None, - }; - apply_package_patch( - "pkg:npm/foo@1.0.0", - primary, - &files, - &sources, - false, - MismatchPolicy::Warn, - ) - .await -} - -/// Fail-closed invariant: after the primary store copy patches cleanly, a -/// peer-variant twin whose pre-existing file is MISSING (a hard error -/// under Warn) must flip the whole result to failure and surface the -/// aggregated "store copy ... failed to patch" note — never claim -/// the CVE fixed with a divergent twin left behind. -#[tokio::test] -#[serial_test::parallel] -async fn pnpm_twin_copy_failure_fails_whole_apply_fail_closed() { - let tmp = tempfile::tempdir().unwrap(); - let store = tmp.path().join("node_modules").join(".pnpm"); - - let primary = stage_store_entry(&store, "foo@1.0.0(react@17.0.2)", Some(ORIGINAL)).await; - // Twin advertises the same foo@1.0.0 but is missing its pre-existing - // index.js — the copy apply fails with NotFound. - let twin = stage_store_entry(&store, "foo@1.0.0(react@18.2.0)", None).await; - - let result = apply_foo(tmp.path(), &primary).await; - - assert!( - !result.success, - "a failed twin copy must fail the whole apply (got success; error: {:?})", - result.error - ); - let err = result.error.as_deref().expect("aggregated error present"); - assert!( - err.contains("store copy"), - "error must carry the store-copy note: {err}" - ); - assert!( - err.contains("react@18.2.0"), - "error must name the failing twin's path: {err}" - ); - assert!( - err.contains("File not found"), - "error must carry the copy's underlying failure: {err}" - ); - - // Fail-closed does NOT roll back the primary: its bytes stay patched - // and its per-file records are what the envelope carries. - assert_eq!( - tokio::fs::read(primary.join("index.js")).await.unwrap(), - PATCHED, - "primary copy stays patched" - ); - assert_eq!( - result.files_patched, - vec!["package/index.js".to_string()], - "the primary's per-file records are preserved" - ); - // The twin is still missing its file — nothing was fabricated there. - assert!(!twin.join("index.js").exists()); -} - -/// Two failing twins: the second note must be CONCATENATED onto the first -/// (`"...; store copy ..."`), naming both twin paths. -#[tokio::test] -#[serial_test::parallel] -async fn pnpm_multiple_twin_copy_failures_aggregate_with_semicolon() { - let tmp = tempfile::tempdir().unwrap(); - let store = tmp.path().join("node_modules").join(".pnpm"); - - let primary = stage_store_entry(&store, "foo@1.0.0(react@17.0.2)", Some(ORIGINAL)).await; - stage_store_entry(&store, "foo@1.0.0(react@18.2.0)", None).await; - stage_store_entry(&store, "foo@1.0.0(react@19.0.0)", None).await; - - let result = apply_foo(tmp.path(), &primary).await; - - assert!(!result.success, "two failed twins must fail the apply"); - let err = result.error.as_deref().expect("aggregated error present"); - assert!( - err.contains("; store copy"), - "second failure must be joined onto the first with '; ': {err}" - ); - assert!( - err.contains("react@18.2.0") && err.contains("react@19.0.0"), - "error must name BOTH failing twins: {err}" - ); - // Primary still patched; fail-closed, not rolled back. - assert_eq!( - tokio::fs::read(primary.join("index.js")).await.unwrap(), - PATCHED - ); -} - -/// The vlt twin of the fail-closed invariant: a `~peer.3` copy that cannot -/// be patched fails the whole apply, naming that copy. -#[tokio::test] -#[serial_test::parallel] -async fn vlt_twin_copy_failure_fails_whole_apply_fail_closed() { - let tmp = tempfile::tempdir().unwrap(); - let store = tmp.path().join("node_modules").join(".vlt"); - - let primary = stage_store_entry(&store, "~npm~foo@1.0.0~peer.2", Some(ORIGINAL)).await; - let twin = stage_store_entry(&store, "~npm~foo@1.0.0~peer.3", None).await; - - let result = apply_foo(tmp.path(), &primary).await; - - assert!(!result.success, "error: {:?}", result.error); - let err = result.error.as_deref().expect("aggregated error present"); - assert!( - err.starts_with("store copy ") && err.contains("~peer.3"), - "the note names the failing vlt copy: {err}" - ); - assert_eq!( - tokio::fs::read(primary.join("index.js")).await.unwrap(), - PATCHED - ); - assert!(!twin.join("index.js").exists()); -} - -/// Copy-on-write security invariant (vlt 1.2.0 hardlinks store files from -/// `/store/v1//` on Linux): patching a target that shares its -/// inode with the machine-wide cache must replace the target's directory -/// entry and never write through the shared inode. The cache link keeps -/// its bytes and its inode; the target ends up on a private inode. Any -/// in-place write (open for write, truncate, append, chmod before the -/// rename) fails this test. -#[tokio::test] -#[serial_test::parallel] -async fn apply_breaks_hardlink_to_shared_store_file() { - let tmp = tempfile::tempdir().unwrap(); - let store = tmp.path().join("node_modules").join(".vlt"); - let primary = stage_store_entry(&store, "~npm~foo@1.0.0", None).await; - let cache = tmp.path().join("cache/store/v1/0a1b2c/index.js"); - tokio::fs::create_dir_all(cache.parent().unwrap()) - .await - .unwrap(); - tokio::fs::write(&cache, ORIGINAL).await.unwrap(); - std::fs::hard_link(&cache, primary.join("index.js")).unwrap(); - #[cfg(unix)] - let (cache_ino, cache_mode) = { - use std::os::unix::fs::MetadataExt as _; - let meta = std::fs::metadata(&cache).unwrap(); - assert_eq!(meta.nlink(), 2, "precondition: the target is a hardlink"); - (meta.ino(), meta.mode()) - }; - - let result = apply_foo(tmp.path(), &primary).await; - assert!(result.success, "{:?}", result.error); - - assert_eq!( - tokio::fs::read(primary.join("index.js")).await.unwrap(), - PATCHED - ); - assert_eq!( - tokio::fs::read(&cache).await.unwrap(), - ORIGINAL, - "the shared store file must never be written through" - ); - #[cfg(unix)] - { - use std::os::unix::fs::MetadataExt as _; - let cache_meta = std::fs::metadata(&cache).unwrap(); - let target_meta = std::fs::metadata(primary.join("index.js")).unwrap(); - assert_eq!(cache_meta.ino(), cache_ino, "the cache keeps its inode"); - assert_eq!(cache_meta.mode(), cache_mode, "the cache keeps its mode"); - assert_eq!( - cache_meta.nlink(), - 1, - "the target no longer links the cache" - ); - assert_ne!( - target_meta.ino(), - cache_ino, - "the target is a private inode" - ); - assert_eq!(target_meta.nlink(), 1); - } -} diff --git a/crates/socket-patch-core/tests/crawler_composer_e2e.rs b/crates/socket-patch-core/tests/crawler_composer_e2e.rs index 1bb13c615..0996d17df 100644 --- a/crates/socket-patch-core/tests/crawler_composer_e2e.rs +++ b/crates/socket-patch-core/tests/crawler_composer_e2e.rs @@ -1574,3 +1574,218 @@ async fn user_level_vendor_dir_expands_home_and_env_placeholders() { .unwrap(); assert_eq!(paths, vec![shared]); } + +// ── global home discovery chain: broken COMPOSER_HOME, composer CLI ── +// +// Fall-through ordering of `get_vendor_paths` in global mode when +// COMPOSER_HOME names a missing directory, and the `composer global +// config home` shell-out (a fake `composer` shim on PATH). + +/// A set-but-BROKEN `COMPOSER_HOME` (naming a nonexistent directory) must +/// fall through the discovery chain — composer CLI, then platform defaults — +/// rather than yield the bogus path or bail empty. +/// `get_vendor_paths_global_via_composer_home_env` above +/// pins the env var's success arm; this pins the failure arm's ordering. +#[tokio::test] +#[serial_test::serial] +async fn get_vendor_paths_global_nonexistent_composer_home_falls_back() { + let tmp = tempfile::tempdir().unwrap(); + let dot_composer_vendor = tmp.path().join(".composer").join("vendor"); + tokio::fs::create_dir_all(&dot_composer_vendor) + .await + .unwrap(); + // PATH stubbed to a binary-free tempdir so `composer global config + // home` on a machine with composer installed can't short-circuit the + // fallback chain. + let empty_path = tempfile::tempdir().unwrap(); + let bogus_home = tmp.path().join("does-not-exist"); + + let _env = EnvVars::set(&[ + ("COMPOSER_HOME", Some(bogus_home.as_os_str())), + ("HOME", Some(tmp.path().as_os_str())), + ("PATH", Some(empty_path.path().as_os_str())), + // Composer's other platform defaults (`%APPDATA%\Composer` on Windows, + // `$XDG_CONFIG_HOME/composer`) would outrank the HOME candidate here. + ("APPDATA", None), + ("XDG_CONFIG_HOME", None), + ]); + + let crawler = ComposerCrawler; + let paths = crawler + .get_vendor_paths(&global_opts(tmp.path())) + .await + .unwrap(); + + assert_eq!( + paths, + vec![dot_composer_vendor], + "a set-but-nonexistent COMPOSER_HOME must fall through to the \ + HOME/.composer platform default, not yield the bogus path or empty" + ); +} + +/// Write an executable `composer` shim into `dir` that ignores its +/// arguments and echoes `echo_path` — a stand-in for `composer global +/// config home`. The shebang names /bin/sh by absolute path, so the script +/// executes even with PATH stripped down to the shim dir alone. +#[cfg(unix)] +fn write_composer_shim(dir: &Path, echo_path: &Path) { + use std::os::unix::fs::PermissionsExt; + let shim = dir.join("composer"); + std::fs::write( + &shim, + format!("#!/bin/sh\necho '{}'\n", echo_path.display()), + ) + .unwrap(); + std::fs::set_permissions(&shim, std::fs::Permissions::from_mode(0o755)).unwrap(); +} + +/// `composer global config home` answering an EXISTING directory is the +/// production path on any machine with composer installed and +/// COMPOSER_HOME unset: the CLI's answer must win over the HOME/.composer +/// platform default. +#[cfg(unix)] +#[tokio::test] +#[serial_test::serial] +async fn get_vendor_paths_global_composer_cli_answer_wins() { + let tmp = tempfile::tempdir().unwrap(); + // What the fake CLI reports as the composer home; its vendor/ exists. + let cli_home = tmp.path().join("cli-home"); + let cli_vendor = cli_home.join("vendor"); + tokio::fs::create_dir_all(&cli_vendor).await.unwrap(); + // HOME tripwire: if the CLI branch were skipped, discovery would land + // here instead and the equality assert below would catch it. + let home = tempfile::tempdir().unwrap(); + tokio::fs::create_dir_all(home.path().join(".composer").join("vendor")) + .await + .unwrap(); + let shim_dir = tempfile::tempdir().unwrap(); + write_composer_shim(shim_dir.path(), &cli_home); + + let _env = EnvVars::set(&[ + ("COMPOSER_HOME", None), + ("HOME", Some(home.path().as_os_str())), + ("PATH", Some(shim_dir.path().as_os_str())), + ]); + + let crawler = ComposerCrawler; + let paths = crawler + .get_vendor_paths(&global_opts(tmp.path())) + .await + .unwrap(); + + assert_eq!( + paths, + vec![cli_vendor], + "the composer CLI's home answer must win over the HOME/.composer fallback" + ); +} + +/// The CLI answering a NONEXISTENT directory must fall through to the +/// platform defaults rather than yield the bogus path or bail empty. +#[cfg(unix)] +#[tokio::test] +#[serial_test::serial] +async fn get_vendor_paths_global_composer_cli_nonexistent_home_falls_back() { + let tmp = tempfile::tempdir().unwrap(); + let home = tempfile::tempdir().unwrap(); + let dot_composer_vendor = home.path().join(".composer").join("vendor"); + tokio::fs::create_dir_all(&dot_composer_vendor) + .await + .unwrap(); + // Each shim lives in its own tempdir; nothing rewrites a shim in place. + let shim_dir = tempfile::tempdir().unwrap(); + write_composer_shim(shim_dir.path(), &tmp.path().join("cli-home-missing")); + + let _env = EnvVars::set(&[ + ("COMPOSER_HOME", None), + ("HOME", Some(home.path().as_os_str())), + ("PATH", Some(shim_dir.path().as_os_str())), + // `$XDG_CONFIG_HOME/composer` would outrank the HOME candidate here. + ("XDG_CONFIG_HOME", None), + ]); + + let crawler = ComposerCrawler; + let paths = crawler + .get_vendor_paths(&global_opts(tmp.path())) + .await + .unwrap(); + + assert_eq!( + paths, + vec![dot_composer_vendor], + "a CLI answer naming a nonexistent home must fall through to HOME/.composer" + ); +} + +/// #586: on an XDG system Composer 2 looks for its home in the XDG config +/// dir first and only then in `~/.composer` (`Factory::getHomeDir`). A +/// leftover `~/.composer` (Composer 1 era, even an empty one) must not +/// hide the real global install under `~/.config/composer` when the +/// `composer` CLI can't be asked. +#[cfg(not(windows))] +#[tokio::test] +#[serial_test::serial] +async fn get_vendor_paths_global_xdg_home_outranks_stale_dot_composer() { + let home = tempfile::tempdir().unwrap(); + tokio::fs::create_dir_all(home.path().join(".composer")) + .await + .unwrap(); + let xdg_vendor = home.path().join(".config").join("composer").join("vendor"); + tokio::fs::create_dir_all(&xdg_vendor).await.unwrap(); + let empty_path = tempfile::tempdir().unwrap(); + + let _env = EnvVars::set(&[ + ("COMPOSER_HOME", None), + ("HOME", Some(home.path().as_os_str())), + ("PATH", Some(empty_path.path().as_os_str())), + ("XDG_CONFIG_HOME", None), + // Any `XDG_*` variable makes Composer treat the system as XDG. + ("XDG_SESSION_TYPE", Some(std::ffi::OsStr::new("tty"))), + ]); + + let paths = ComposerCrawler + .get_vendor_paths(&global_opts(home.path())) + .await + .unwrap(); + + assert_eq!( + paths, + vec![xdg_vendor], + "Composer 2 picks ~/.config/composer over ~/.composer on an XDG system" + ); +} + +/// The same layout with `XDG_CONFIG_HOME` set: Composer 2 uses +/// `$XDG_CONFIG_HOME/composer`, still ahead of a stale `~/.composer`. +#[cfg(not(windows))] +#[tokio::test] +#[serial_test::serial] +async fn get_vendor_paths_global_xdg_config_home_outranks_stale_dot_composer() { + let home = tempfile::tempdir().unwrap(); + tokio::fs::create_dir_all(home.path().join(".composer").join("vendor")) + .await + .unwrap(); + let xdg = tempfile::tempdir().unwrap(); + let xdg_vendor = xdg.path().join("composer").join("vendor"); + tokio::fs::create_dir_all(&xdg_vendor).await.unwrap(); + let empty_path = tempfile::tempdir().unwrap(); + + let _env = EnvVars::set(&[ + ("COMPOSER_HOME", None), + ("HOME", Some(home.path().as_os_str())), + ("PATH", Some(empty_path.path().as_os_str())), + ("XDG_CONFIG_HOME", Some(xdg.path().as_os_str())), + ]); + + let paths = ComposerCrawler + .get_vendor_paths(&global_opts(home.path())) + .await + .unwrap(); + + assert_eq!( + paths, + vec![xdg_vendor], + "Composer 2 picks $XDG_CONFIG_HOME/composer over ~/.composer" + ); +} diff --git a/crates/socket-patch-core/tests/crawler_npm_e2e.rs b/crates/socket-patch-core/tests/crawler_npm_e2e.rs index affc134e5..03abb0b1c 100644 --- a/crates/socket-patch-core/tests/crawler_npm_e2e.rs +++ b/crates/socket-patch-core/tests/crawler_npm_e2e.rs @@ -3,17 +3,22 @@ //! read_package_json, find_by_purls scoped vs unscoped, crawl_all //! over a synthetic node_modules tree). -use std::path::Path; +use std::collections::HashMap; +use std::path::{Path, PathBuf}; use socket_patch_core::crawlers::npm_crawler::{ - build_npm_purl, bun_global_dir_from_env, get_bun_global_prefix, get_bun_global_prefix_with, - get_npm_global_prefix, get_npm_global_prefix_with, get_pnpm_global_prefix, - get_pnpm_global_prefix_with, get_yarn_global_prefix, get_yarn_global_prefix_with, - parse_bun_ls_global_output, parse_npm_root_output, parse_package_name, parse_pnpm_root_output, - parse_yarn_dir_output, read_package_json, resolve_bun_global_prefix_with, + build_npm_purl, bun_global_dir_from_env, find_store_peer_variant_copies, get_bun_global_prefix, + get_bun_global_prefix_with, get_npm_global_prefix, get_npm_global_prefix_with, + get_pnpm_global_prefix, get_pnpm_global_prefix_with, get_yarn_global_prefix, + get_yarn_global_prefix_with, parse_bun_ls_global_output, parse_npm_root_output, + parse_package_name, parse_pnpm_root_output, parse_yarn_dir_output, read_package_json, + resolve_bun_global_prefix_with, }; use socket_patch_core::crawlers::types::CrawlerOptions; use socket_patch_core::crawlers::NpmCrawler; +use socket_patch_core::hash::git_sha256::compute_git_sha256_from_bytes; +use socket_patch_core::manifest::schema::PatchFileInfo; +use socket_patch_core::patch::apply::{apply_package_patch, MismatchPolicy, PatchSources}; fn options_at(root: &Path) -> CrawlerOptions { CrawlerOptions { @@ -3504,3 +3509,638 @@ async fn apply_heals_unpatched_vlt_twin_when_primary_already_patched() { "an AlreadyPatched primary must not mask a still-vulnerable twin" ); } + +// ── store walkers: skip and fallback regions ───────────────────────── +// +// The skip/fallback regions of the store walkers +// (`collect_nested_node_modules`, `collect_nested_store_entries`, +// `gather_scoped_packages`) and every reject/fallback gate of +// `find_store_peer_variant_copies` (pnpm and vlt stores). Each test stages +// the on-disk shape that reaches its region and asserts resolver/scan +// output. + +// ── plain-FILE store markers at a node_modules root ──────────── + +/// A stray plain FILE named `.pnpm` or `.registry.npmjs.org` at a +/// `node_modules` root (trivially stageable; real trees carry such junk) +/// must be skipped by BOTH walks — the resolver's nested-nm BFS and the +/// scan pass — without erroring out and without being mistaken for a +/// virtual store. +#[tokio::test] +#[serial_test::parallel] +async fn find_by_purls_and_crawl_all_skip_plain_file_store_markers() { + let tmp = tempfile::tempdir().unwrap(); + let nm = tmp.path().join("node_modules"); + stage_npm_pkg(&nm, "foo", "1.0.0").await; + // Plain files where the store DIRS would live. + tokio::fs::write(nm.join(".pnpm"), b"junk").await.unwrap(); + tokio::fs::write(nm.join(".registry.npmjs.org"), b"junk") + .await + .unwrap(); + + // Resolver walk: foo resolves at the root, the marker files are + // skipped (no error, no phantom copies). + let crawler = NpmCrawler::new(); + let purls = vec!["pkg:npm/foo@1.0.0".to_string()]; + let result = crawler.find_by_purls(&nm, &purls).await.unwrap(); + assert_eq!(result.len(), 1, "only foo resolves; got {result:?}"); + let copies = result.get("pkg:npm/foo@1.0.0").unwrap(); + assert_eq!(copies.len(), 1, "exactly one physical copy of foo"); + assert_eq!(copies[0].path, nm.join("foo")); + + // Scan walk: exactly one package inventoried. + let packages = crawler.crawl_all(&options_at(tmp.path())).await; + assert_eq!( + packages.len(), + 1, + "marker files must not add or hide packages; got {:?}", + packages.iter().map(|p| p.purl.as_str()).collect::>() + ); + assert_eq!(packages[0].purl, "pkg:npm/foo@1.0.0"); + assert_eq!(packages[0].path, nm.join("foo")); +} + +// ── stray entries inside a `@scope` dir during the nested-nm BFS ─ + +/// The resolver's nested-node_modules walk descends `@scope/` dirs +/// looking for deeper `node_modules`. Hidden entries (`.DS_Store`-style +/// junk is ubiquitous inside scope dirs on macOS) and plain files inside +/// the `@scope` dir must be skipped without breaking or polluting the +/// walk — the target below the real scoped package must still resolve. +#[tokio::test] +#[serial_test::parallel] +async fn find_by_purls_ignores_stray_entries_in_scope_dirs() { + let tmp = tempfile::tempdir().unwrap(); + let nm = tmp.path().join("node_modules"); + stage_npm_pkg(&nm, "@scope/pkg", "1.0.0").await; + // The target lives ONLY inside the scoped package's nested + // node_modules, so resolving it forces the walk through @scope. + stage_npm_pkg(&nm.join("@scope/pkg/node_modules"), "leaf", "2.0.0").await; + // Stray entries inside the @scope dir itself: a hidden dir and a + // plain file. + tokio::fs::create_dir_all(nm.join("@scope/.cache")) + .await + .unwrap(); + tokio::fs::write(nm.join("@scope/notes.txt"), b"junk") + .await + .unwrap(); + + let crawler = NpmCrawler::new(); + let purls = vec![ + "pkg:npm/leaf@2.0.0".to_string(), + "pkg:npm/@scope/pkg@1.0.0".to_string(), + ]; + let result = crawler.find_by_purls(&nm, &purls).await.unwrap(); + + assert_eq!(result.len(), 2, "both packages resolve; got {result:?}"); + assert_eq!( + result.get("pkg:npm/leaf@2.0.0").unwrap()[0].path, + nm.join("@scope/pkg/node_modules/leaf"), + "leaf resolves only by descending @scope/pkg's nested node_modules" + ); + assert_eq!( + result.get("pkg:npm/@scope/pkg@1.0.0").unwrap()[0].path, + nm.join("@scope/pkg") + ); +} + +// ── scoped root-linked pnpm direct dep: dedup + no symlink descend ─ + +/// Scoped twin of the unscoped `identity_seen` dedup: a SCOPED pnpm +/// direct dep is root-linked (`node_modules/@scope/leaf` → store copy), +/// so the importer pass inventories it first and the store pass must +/// skip the redundant re-read — while STILL descending the store copy, +/// because a bundled dep is a real dir physically present only there. +/// The importer pass must inventory the symlinked scoped package without +/// descending through the symlink (descending would surface the bundled +/// dep at its symlink-relative path instead of its physical store home). +#[cfg(unix)] +#[tokio::test] +#[serial_test::parallel] +async fn crawl_all_dedups_scoped_root_linked_pnpm_direct_dep_and_skips_symlink_descend() { + use std::os::unix::fs::symlink; + + let tmp = tempfile::tempdir().unwrap(); + let nm = tmp.path().join("node_modules"); + let store = nm.join(".pnpm"); + + // Store copy of the scoped package… + stage_npm_pkg( + &store.join("@scope+leaf@2.0.0").join("node_modules"), + "@scope/leaf", + "2.0.0", + ) + .await; + let store_leaf = store.join("@scope+leaf@2.0.0/node_modules/@scope/leaf"); + // …with a bundled dep as a REAL dir below it (its only physical home). + stage_npm_pkg(&store_leaf.join("node_modules"), "bundled", "1.0.0").await; + + // Importer root: the scoped direct dep is a symlink into the store. + tokio::fs::create_dir_all(nm.join("@scope")).await.unwrap(); + symlink(&store_leaf, nm.join("@scope/leaf")).unwrap(); + + let crawler = NpmCrawler::new(); + let result = crawler.crawl_all(&options_at(tmp.path())).await; + + let purls: Vec<&str> = result.iter().map(|p| p.purl.as_str()).collect(); + assert_eq!( + result.len(), + 2, + "exactly @scope/leaf (once) + bundled; got {purls:?}" + ); + let leaf_copies: Vec<_> = result + .iter() + .filter(|p| p.purl == "pkg:npm/@scope/leaf@2.0.0") + .collect(); + assert_eq!( + leaf_copies.len(), + 1, + "the root-linked scoped dep must be inventoried exactly once" + ); + assert_eq!( + leaf_copies[0].path, + nm.join("@scope/leaf"), + "the importer-root path wins over the store copy" + ); + let bundled = result + .iter() + .find(|p| p.purl == "pkg:npm/bundled@1.0.0") + .unwrap_or_else(|| panic!("bundled dep must be inventoried; got {purls:?}")); + assert_eq!( + bundled.path, + store_leaf.join("node_modules/bundled"), + "bundled dep surfaces at its physical store home — reached by \ + descending the store entry (dedup skip must not stop the \ + descent), never through the importer symlink" + ); +} + +// ── find_store_peer_variant_copies: pnpm probe gates ──────────── + +/// All four reject/fallback gates of the peer-variant store probe, in one +/// staged store: +/// - an UNDECODABLE entry name stays probeable and its real matching copy +/// IS returned (a regression here silently skips a physical copy, +/// leaving a live vulnerable file after apply reports success); +/// - an entry whose candidate dir is absent is skipped; +/// - a candidate that is a SYMLINK to another entry's copy is skipped +/// (that copy is already yielded via its own entry — no double count); +/// - an imposter whose package.json disagrees with the decoded name is +/// rejected by the authority probe. +#[cfg(unix)] +#[tokio::test] +#[serial_test::parallel] +async fn find_store_peer_variant_copies_probe_gates() { + use std::os::unix::fs::symlink; + + let tmp = tempfile::tempdir().unwrap(); + let nm = tmp.path().join("node_modules"); + let store = nm.join(".pnpm"); + + // (a) The primary peer-variant copy. + stage_npm_pkg( + &store.join("foo@1.0.0(react@17.0.2)").join("node_modules"), + "foo", + "1.0.0", + ) + .await; + let primary = store.join("foo@1.0.0(react@17.0.2)/node_modules/foo"); + + // (b) A real twin under another peer combination — must be returned. + stage_npm_pkg( + &store.join("foo@1.0.0(react@18.2.0)").join("node_modules"), + "foo", + "1.0.0", + ) + .await; + let twin = store.join("foo@1.0.0(react@18.2.0)/node_modules/foo"); + + // (c) An UNDECODABLE entry name (`_` reads as a legacy peer suffix, + // so decode yields None) holding a real matching copy — must be + // returned: undecodable stays probeable. + stage_npm_pkg( + &store.join("foo_bar@1.0.0").join("node_modules"), + "foo", + "1.0.0", + ) + .await; + let undecodable_copy = store.join("foo_bar@1.0.0/node_modules/foo"); + + // (d) A matching-decoded entry whose node_modules holds no `foo` dir. + tokio::fs::create_dir_all(store.join("foo@1.0.0(vue@3.2.0)").join("node_modules")) + .await + .unwrap(); + + // (e) A matching-decoded entry whose `foo` is a SYMLINK to twin (b). + let sym_nm = store.join("foo@1.0.0(sym@1.0.0)").join("node_modules"); + tokio::fs::create_dir_all(&sym_nm).await.unwrap(); + symlink(&twin, sym_nm.join("foo")).unwrap(); + + // (f) An imposter: the entry NAME decodes to foo@1.0.0 (passes the + // advertisement filter) but the package.json inside says 1.0.1. + stage_npm_pkg( + &store.join("foo@1.0.0(baz@1.0.0)").join("node_modules"), + "foo", + "1.0.1", + ) + .await; + + let copies = find_store_peer_variant_copies(&primary).await; + + let got: std::collections::HashSet<_> = copies.iter().cloned().collect(); + let want: std::collections::HashSet<_> = [twin.clone(), undecodable_copy.clone()] + .into_iter() + .collect(); + assert_eq!( + got, want, + "exactly the real twin and the undecodable-entry copy — no \ + primary, no symlink duplicate, no absent candidate, no imposter" + ); +} + +/// Fail-safe gate: when the PRIMARY's package.json is unreadable there is +/// no safe way to identify twins, so NONE are reported — even though a +/// perfectly probeable twin exists in the store (the primary itself is +/// still handled by the caller). +#[tokio::test] +#[serial_test::parallel] +async fn find_store_peer_variant_copies_unreadable_primary_returns_empty() { + let tmp = tempfile::tempdir().unwrap(); + let nm = tmp.path().join("node_modules"); + let store = nm.join(".pnpm"); + + // Primary: a bare dir with NO package.json. + let primary = store.join("foo@1.0.0(react@17.0.2)/node_modules/foo"); + tokio::fs::create_dir_all(&primary).await.unwrap(); + + // A fully-formed twin that WOULD match if the primary were readable. + stage_npm_pkg( + &store.join("foo@1.0.0(react@18.2.0)").join("node_modules"), + "foo", + "1.0.0", + ) + .await; + + let copies = find_store_peer_variant_copies(&primary).await; + assert!( + copies.is_empty(), + "unreadable primary identity ⇒ no twins reported; got {copies:?}" + ); +} + +// ── find_store_peer_variant_copies: vlt probe gates ───────────── + +/// vlt's fan-out from a workspace member's link: the store lives beside +/// the ROOT `node_modules`, reachable only on the link's canonical chain. +/// Real copies whose DepID decodes to the primary's `name@version` are +/// returned (a `~peer.` twin); a git dependency of the same +/// `name@version` is never a variant (the resolver reports it as a primary +/// of its own), and a store entry reached through a link is not a store +/// entry. +#[cfg(unix)] +#[tokio::test] +#[serial_test::parallel] +async fn find_store_peer_variant_copies_vlt_member_link_and_git_copy() { + use std::os::unix::fs::symlink; + + let tmp = tempfile::tempdir().unwrap(); + let store = tmp.path().join("node_modules").join(".vlt"); + let entry = |id: &str| store.join(id).join("node_modules"); + stage_npm_pkg(&entry("~npm~foo@1.0.0~peer.2"), "foo", "1.0.0").await; + stage_npm_pkg(&entry("~npm~foo@1.0.0~peer.3"), "foo", "1.0.0").await; + stage_npm_pkg(&entry("git~github_cu+foo~v1.0.0"), "foo", "1.0.0").await; + let elsewhere = tempfile::tempdir().unwrap(); + stage_npm_pkg(&elsewhere.path().join("node_modules"), "foo", "1.0.0").await; + symlink(elsewhere.path(), store.join("~npm~foo@1.0.0~peer.4")).unwrap(); + + let member_nm = tmp.path().join("packages/a/node_modules"); + tokio::fs::create_dir_all(&member_nm).await.unwrap(); + symlink( + "../../../node_modules/.vlt/~npm~foo@1.0.0~peer.2/node_modules/foo", + member_nm.join("foo"), + ) + .unwrap(); + + let copies = find_store_peer_variant_copies(&member_nm.join("foo")).await; + assert_eq!( + copies + .iter() + .map(|p| std::fs::canonicalize(p).unwrap()) + .collect::>(), + vec![std::fs::canonicalize(entry("~npm~foo@1.0.0~peer.3").join("foo")).unwrap()], + "exactly the peer twin; got {copies:?}" + ); +} + +/// Fail-safe gate on the vlt side too: an unreadable primary identity +/// reports no twins. +#[tokio::test] +#[serial_test::parallel] +async fn find_store_peer_variant_copies_vlt_unreadable_primary_returns_empty() { + let tmp = tempfile::tempdir().unwrap(); + let store = tmp.path().join("node_modules").join(".vlt"); + let primary = store.join("~npm~foo@1.0.0~peer.2/node_modules/foo"); + tokio::fs::create_dir_all(&primary).await.unwrap(); + stage_npm_pkg( + &store.join("~npm~foo@1.0.0~peer.3").join("node_modules"), + "foo", + "1.0.0", + ) + .await; + assert!(find_store_peer_variant_copies(&primary).await.is_empty()); +} + +// ── pnpm<=3 legacy store: stray node_modules + depth-1 home ──── + +/// Two edge shapes of the nested legacy-store walk: +/// - a stray `node_modules` child inside the store host belongs to a +/// parent entry, never a name/version coordinate — a decoy package +/// staged below it must NOT be inventoried; +/// - a depth-1 package home (`//node_modules/`) has no +/// `/` in its host-relative path, so the raw component becomes an +/// undecodable-but-probed entry name and the package IS inventoried. +#[tokio::test] +#[serial_test::parallel] +async fn crawl_all_legacy_store_skips_stray_node_modules_and_probes_depth1_home() { + let tmp = tempfile::tempdir().unwrap(); + let nm = tmp.path().join("node_modules"); + let store = nm.join(".registry.npmjs.org"); + + // Normal legacy shape: //node_modules/. + stage_npm_pkg(&store.join("mkdirp/0.5.5/node_modules"), "mkdirp", "0.5.5").await; + // Depth-1 home: /node_modules/ (no version dir). + stage_npm_pkg(&store.join("directpkg/node_modules"), "directpkg", "3.0.0").await; + // Stray node_modules child of the host, shaped like a package home so + // a skip regression would surface the decoy in the inventory. + stage_npm_pkg( + &store.join("node_modules/decoy/9.9.9/node_modules"), + "decoy", + "9.9.9", + ) + .await; + + let crawler = NpmCrawler::new(); + let result = crawler.crawl_all(&options_at(tmp.path())).await; + + let purls: Vec<&str> = result.iter().map(|p| p.purl.as_str()).collect(); + assert_eq!( + result.len(), + 2, + "exactly mkdirp + directpkg, decoy stays out; got {purls:?}" + ); + let by_purl = |purl: &str| { + result + .iter() + .find(|p| p.purl == purl) + .unwrap_or_else(|| panic!("{purl} must be inventoried; got {purls:?}")) + }; + assert_eq!( + by_purl("pkg:npm/mkdirp@0.5.5").path, + store.join("mkdirp/0.5.5/node_modules/mkdirp") + ); + assert_eq!( + by_purl("pkg:npm/directpkg@3.0.0").path, + store.join("directpkg/node_modules/directpkg"), + "a depth-1 package home must be probed (undecodable entry name)" + ); + assert!( + !purls.contains(&"pkg:npm/decoy@9.9.9"), + "a stray node_modules child of the store host must be skipped" + ); +} + +// ── peer-variant twin copy failures in apply_package_patch ─────────── +// +// The fail-closed half of the twin-healing tests above: a store copy that +// cannot be patched must flip the whole result to failure with a "store +// copy ... failed to patch: ..." note, never report the CVE fixed while a +// physical twin stays divergent. Also the copy-on-write guard for +// hardlinked store files. + +const ORIGINAL: &[u8] = b"module.exports = 'vulnerable';\n"; +const PATCHED: &[u8] = b"module.exports = 'fixed';\n"; + +/// Stage one store entry `//node_modules/foo` holding a +/// package.json and, when `index_content` is `Some`, an `index.js`. +/// Returns the staged package root. +async fn stage_store_entry(store: &Path, entry: &str, index_content: Option<&[u8]>) -> PathBuf { + let pkg = store.join(entry).join("node_modules").join("foo"); + tokio::fs::create_dir_all(&pkg).await.unwrap(); + tokio::fs::write( + pkg.join("package.json"), + r#"{"name":"foo","version":"1.0.0"}"#, + ) + .await + .unwrap(); + if let Some(content) = index_content { + tokio::fs::write(pkg.join("index.js"), content) + .await + .unwrap(); + } + pkg +} + +/// Shared apply invocation: blob-only sources staged under `root`, one +/// patched file `package/index.js` (ORIGINAL → PATCHED), Warn policy. +async fn apply_foo(root: &Path, primary: &Path) -> socket_patch_core::patch::apply::ApplyResult { + let before_hash = compute_git_sha256_from_bytes(ORIGINAL); + let after_hash = compute_git_sha256_from_bytes(PATCHED); + + let blobs = root.join("blobs"); + tokio::fs::create_dir_all(&blobs).await.unwrap(); + tokio::fs::write(blobs.join(&after_hash), PATCHED) + .await + .unwrap(); + + let mut files = HashMap::new(); + files.insert( + "package/index.js".to_string(), + PatchFileInfo { + before_hash, + after_hash, + }, + ); + let sources = PatchSources { + blobs_path: &blobs, + mem_blobs: None, + }; + apply_package_patch( + "pkg:npm/foo@1.0.0", + primary, + &files, + &sources, + false, + MismatchPolicy::Warn, + ) + .await +} + +/// Fail-closed invariant: after the primary store copy patches cleanly, a +/// peer-variant twin whose pre-existing file is MISSING (a hard error +/// under Warn) must flip the whole result to failure and surface the +/// aggregated "store copy ... failed to patch" note — never claim +/// the CVE fixed with a divergent twin left behind. +#[tokio::test] +#[serial_test::parallel] +async fn pnpm_twin_copy_failure_fails_whole_apply_fail_closed() { + let tmp = tempfile::tempdir().unwrap(); + let store = tmp.path().join("node_modules").join(".pnpm"); + + let primary = stage_store_entry(&store, "foo@1.0.0(react@17.0.2)", Some(ORIGINAL)).await; + // Twin advertises the same foo@1.0.0 but is missing its pre-existing + // index.js — the copy apply fails with NotFound. + let twin = stage_store_entry(&store, "foo@1.0.0(react@18.2.0)", None).await; + + let result = apply_foo(tmp.path(), &primary).await; + + assert!( + !result.success, + "a failed twin copy must fail the whole apply (got success; error: {:?})", + result.error + ); + let err = result.error.as_deref().expect("aggregated error present"); + assert!( + err.contains("store copy"), + "error must carry the store-copy note: {err}" + ); + assert!( + err.contains("react@18.2.0"), + "error must name the failing twin's path: {err}" + ); + assert!( + err.contains("File not found"), + "error must carry the copy's underlying failure: {err}" + ); + + // Fail-closed does NOT roll back the primary: its bytes stay patched + // and its per-file records are what the envelope carries. + assert_eq!( + tokio::fs::read(primary.join("index.js")).await.unwrap(), + PATCHED, + "primary copy stays patched" + ); + assert_eq!( + result.files_patched, + vec!["package/index.js".to_string()], + "the primary's per-file records are preserved" + ); + // The twin is still missing its file — nothing was fabricated there. + assert!(!twin.join("index.js").exists()); +} + +/// Two failing twins: the second note must be CONCATENATED onto the first +/// (`"...; store copy ..."`), naming both twin paths. +#[tokio::test] +#[serial_test::parallel] +async fn pnpm_multiple_twin_copy_failures_aggregate_with_semicolon() { + let tmp = tempfile::tempdir().unwrap(); + let store = tmp.path().join("node_modules").join(".pnpm"); + + let primary = stage_store_entry(&store, "foo@1.0.0(react@17.0.2)", Some(ORIGINAL)).await; + stage_store_entry(&store, "foo@1.0.0(react@18.2.0)", None).await; + stage_store_entry(&store, "foo@1.0.0(react@19.0.0)", None).await; + + let result = apply_foo(tmp.path(), &primary).await; + + assert!(!result.success, "two failed twins must fail the apply"); + let err = result.error.as_deref().expect("aggregated error present"); + assert!( + err.contains("; store copy"), + "second failure must be joined onto the first with '; ': {err}" + ); + assert!( + err.contains("react@18.2.0") && err.contains("react@19.0.0"), + "error must name BOTH failing twins: {err}" + ); + // Primary still patched; fail-closed, not rolled back. + assert_eq!( + tokio::fs::read(primary.join("index.js")).await.unwrap(), + PATCHED + ); +} + +/// The vlt twin of the fail-closed invariant: a `~peer.3` copy that cannot +/// be patched fails the whole apply, naming that copy. +#[tokio::test] +#[serial_test::parallel] +async fn vlt_twin_copy_failure_fails_whole_apply_fail_closed() { + let tmp = tempfile::tempdir().unwrap(); + let store = tmp.path().join("node_modules").join(".vlt"); + + let primary = stage_store_entry(&store, "~npm~foo@1.0.0~peer.2", Some(ORIGINAL)).await; + let twin = stage_store_entry(&store, "~npm~foo@1.0.0~peer.3", None).await; + + let result = apply_foo(tmp.path(), &primary).await; + + assert!(!result.success, "error: {:?}", result.error); + let err = result.error.as_deref().expect("aggregated error present"); + assert!( + err.starts_with("store copy ") && err.contains("~peer.3"), + "the note names the failing vlt copy: {err}" + ); + assert_eq!( + tokio::fs::read(primary.join("index.js")).await.unwrap(), + PATCHED + ); + assert!(!twin.join("index.js").exists()); +} + +/// Copy-on-write security invariant (vlt 1.2.0 hardlinks store files from +/// `/store/v1//` on Linux): patching a target that shares its +/// inode with the machine-wide cache must replace the target's directory +/// entry and never write through the shared inode. The cache link keeps +/// its bytes and its inode; the target ends up on a private inode. Any +/// in-place write (open for write, truncate, append, chmod before the +/// rename) fails this test. +#[tokio::test] +#[serial_test::parallel] +async fn apply_breaks_hardlink_to_shared_store_file() { + let tmp = tempfile::tempdir().unwrap(); + let store = tmp.path().join("node_modules").join(".vlt"); + let primary = stage_store_entry(&store, "~npm~foo@1.0.0", None).await; + let cache = tmp.path().join("cache/store/v1/0a1b2c/index.js"); + tokio::fs::create_dir_all(cache.parent().unwrap()) + .await + .unwrap(); + tokio::fs::write(&cache, ORIGINAL).await.unwrap(); + std::fs::hard_link(&cache, primary.join("index.js")).unwrap(); + #[cfg(unix)] + let (cache_ino, cache_mode) = { + use std::os::unix::fs::MetadataExt as _; + let meta = std::fs::metadata(&cache).unwrap(); + assert_eq!(meta.nlink(), 2, "precondition: the target is a hardlink"); + (meta.ino(), meta.mode()) + }; + + let result = apply_foo(tmp.path(), &primary).await; + assert!(result.success, "{:?}", result.error); + + assert_eq!( + tokio::fs::read(primary.join("index.js")).await.unwrap(), + PATCHED + ); + assert_eq!( + tokio::fs::read(&cache).await.unwrap(), + ORIGINAL, + "the shared store file must never be written through" + ); + #[cfg(unix)] + { + use std::os::unix::fs::MetadataExt as _; + let cache_meta = std::fs::metadata(&cache).unwrap(); + let target_meta = std::fs::metadata(primary.join("index.js")).unwrap(); + assert_eq!(cache_meta.ino(), cache_ino, "the cache keeps its inode"); + assert_eq!(cache_meta.mode(), cache_mode, "the cache keeps its mode"); + assert_eq!( + cache_meta.nlink(), + 1, + "the target no longer links the cache" + ); + assert_ne!( + target_meta.ino(), + cache_ino, + "the target is a private inode" + ); + assert_eq!(target_meta.nlink(), 1); + } +} diff --git a/crates/socket-patch-core/tests/covgap_update_state.rs b/crates/socket-patch-core/tests/update_state_env.rs similarity index 97% rename from crates/socket-patch-core/tests/covgap_update_state.rs rename to crates/socket-patch-core/tests/update_state_env.rs index 57d39b223..173a2a779 100644 --- a/crates/socket-patch-core/tests/covgap_update_state.rs +++ b/crates/socket-patch-core/tests/update_state_env.rs @@ -1,4 +1,4 @@ -//! Coverage-gap integration test for `update::state`: the degenerate +//! Integration test for `update::state`: the degenerate //! "no resolvable state dir" environment — `SOCKET_UPDATE_STATE_DIR`, //! the platform cache vars, and the home vars all unset/empty (a //! stripped container). The documented contract is that callers diff --git a/scripts/ci-test-durations.json b/scripts/ci-test-durations.json index 2d3e637a9..6e3dc16f1 100644 --- a/scripts/ci-test-durations.json +++ b/scripts/ci-test-durations.json @@ -12,7 +12,7 @@ "api_timeout_e2e": 4.29, "apply": 14.53, "binary_fetch_error_classification_e2e": 0.01, - "blob_fetcher_edges_e2e": 2.06, + "blob_fetcher_edges_e2e": 6.1, "child_deadline_guard": 0.25, "cli": 7.69, "cli_apply_silent": 0.1, @@ -36,20 +36,15 @@ "cli_sigpipe": 0.0, "command_module_layering": 0.11, "contract_gradle_codes": 0.93, - "covgap_api_blob_fetcher": 4.04, - "covgap_crawlers_composer_crawler": 0.01, - "covgap_crawlers_npm_crawler": 0.04, - "covgap_patch_apply": 0.06, - "covgap_update_state": 0.01, "crawl_fd_limit_e2e": 0.0, "crawler_cargo_e2e": 0.06, - "crawler_composer_e2e": 0.11, + "crawler_composer_e2e": 0.12, "crawler_deno_e2e": 0.04, "crawler_go_e2e": 0.04, "crawler_gradle_e2e": 0.17, "crawler_maven_e2e": 0.07, "crawler_monorepo_gaps": 0.02, - "crawler_npm_e2e": 1.2, + "crawler_npm_e2e": 1.3, "crawler_nuget_e2e": 0.07, "crawler_python_e2e": 1.29, "crawler_ruby_e2e": 0.85, @@ -225,6 +220,7 @@ "spawn_env_hygiene": 0.39, "telemetry_helpers_e2e": 0.01, "update": 16.45, + "update_state_env": 0.01, "upstream_restore_golden": 8.1, "uv_hosted": 0.01, "vendor": 12.8, From 709b39147045df2788b62e682189488bef290bd8 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Oct 2026 18:13:22 +0000 Subject: [PATCH 3/3] CI: share npm/composer test fixtures Review follow-ups on the folded core suites: - crawler_npm_e2e: the moved apply tests now use the suite's one index.js patch fixture (ungated and renamed from the vlt-only helpers) and stage_npm_pkg, and the per-test imports that the file-level ones replaced are gone. - crawler_composer_e2e: the older global-home tests restore their env through EnvVars too, so a panic no longer leaks a stubbed HOME or PATH into later tests in the binary. - blob_fetcher_edges_e2e: the read-only blobs dir gets its write bit back through a drop guard, so a failed assert or the root skip no longer leaves an undeletable tempdir. Refs #1089 Assisted-by: Claude Code:claude-opus-5-5 --- .../tests/blob_fetcher_edges_e2e.rs | 12 +- .../tests/crawler_composer_e2e.rs | 234 +++++------------- .../tests/crawler_npm_e2e.rs | 133 +++------- 3 files changed, 106 insertions(+), 273 deletions(-) diff --git a/crates/socket-patch-core/tests/blob_fetcher_edges_e2e.rs b/crates/socket-patch-core/tests/blob_fetcher_edges_e2e.rs index 62eef7bc8..83fb0ce38 100644 --- a/crates/socket-patch-core/tests/blob_fetcher_edges_e2e.rs +++ b/crates/socket-patch-core/tests/blob_fetcher_edges_e2e.rs @@ -859,6 +859,15 @@ async fn fetch_missing_blobs_disk_write_failure_is_per_blob_failure() { let blobs = tmp.path().join("blobs"); std::fs::create_dir(&blobs).unwrap(); std::fs::set_permissions(&blobs, std::fs::Permissions::from_mode(0o555)).unwrap(); + // Put the write bit back however the test ends (skip, failed assert), + // so the tempdir can always remove what the fetch left behind. + struct RestoreMode<'a>(&'a Path); + impl Drop for RestoreMode<'_> { + fn drop(&mut self) { + let _ = std::fs::set_permissions(self.0, std::fs::Permissions::from_mode(0o755)); + } + } + let _restore = RestoreMode(&blobs); if !write_into_dir_denied(&blobs) { eprintln!("skipping: directory mode bits do not deny writes here (root?)"); return; @@ -900,9 +909,6 @@ async fn fetch_missing_blobs_disk_write_failure_is_per_blob_failure() { 0, "no partial file, no stage litter" ); - - // Restore so tempdir teardown can't mask a failure. - std::fs::set_permissions(&blobs, std::fs::Permissions::from_mode(0o755)).unwrap(); } // ── Mixed-outcome aggregation ──────────────────────────────────────── diff --git a/crates/socket-patch-core/tests/crawler_composer_e2e.rs b/crates/socket-patch-core/tests/crawler_composer_e2e.rs index 0996d17df..96695d365 100644 --- a/crates/socket-patch-core/tests/crawler_composer_e2e.rs +++ b/crates/socket-patch-core/tests/crawler_composer_e2e.rs @@ -371,21 +371,12 @@ async fn get_vendor_paths_global_via_composer_home_env() { let vendor = composer_home.join("vendor"); tokio::fs::create_dir_all(&vendor).await.unwrap(); - let prev_composer = std::env::var("COMPOSER_HOME").ok(); - std::env::set_var("COMPOSER_HOME", composer_home); + let _env = EnvVars::set(&[("COMPOSER_HOME", Some(composer_home.as_os_str()))]); - let crawler = ComposerCrawler; - let opts = CrawlerOptions { - cwd: tmp.path().to_path_buf(), - global: true, - global_prefix: None, - }; - let paths = crawler.get_vendor_paths(&opts).await.unwrap(); - - std::env::remove_var("COMPOSER_HOME"); - if let Some(v) = prev_composer { - std::env::set_var("COMPOSER_HOME", v); - } + let paths = ComposerCrawler + .get_vendor_paths(&global_opts(tmp.path())) + .await + .unwrap(); assert_eq!( paths, @@ -408,46 +399,20 @@ async fn get_vendor_paths_global_via_home_dot_composer_fallback() { // where composer is installed. let empty_path = tempfile::tempdir().unwrap(); - let prev_composer = std::env::var("COMPOSER_HOME").ok(); - let prev_home = std::env::var("HOME").ok(); - let prev_path = std::env::var("PATH").ok(); - std::env::remove_var("COMPOSER_HOME"); - std::env::set_var("HOME", tmp.path()); - std::env::set_var("PATH", empty_path.path()); - // Composer's other platform defaults (`%APPDATA%\Composer` on Windows, - // `$XDG_CONFIG_HOME/composer`) would outrank the HOME candidates here. - let prev_app_data = std::env::var_os("APPDATA"); - let prev_xdg = std::env::var_os("XDG_CONFIG_HOME"); - std::env::remove_var("APPDATA"); - std::env::remove_var("XDG_CONFIG_HOME"); - - let crawler = ComposerCrawler; - let opts = CrawlerOptions { - cwd: tmp.path().to_path_buf(), - global: true, - global_prefix: None, - }; - let paths = crawler.get_vendor_paths(&opts).await.unwrap(); + let _env = EnvVars::set(&[ + ("COMPOSER_HOME", None), + ("HOME", Some(tmp.path().as_os_str())), + ("PATH", Some(empty_path.path().as_os_str())), + // Composer's other platform defaults (`%APPDATA%\Composer` on Windows, + // `$XDG_CONFIG_HOME/composer`) would outrank the HOME candidates here. + ("APPDATA", None), + ("XDG_CONFIG_HOME", None), + ]); - if let Some(v) = prev_composer { - std::env::set_var("COMPOSER_HOME", v); - } - if let Some(v) = prev_home { - std::env::set_var("HOME", v); - } else { - std::env::remove_var("HOME"); - } - if let Some(v) = prev_path { - std::env::set_var("PATH", v); - } else { - std::env::remove_var("PATH"); - } - if let Some(v) = prev_app_data { - std::env::set_var("APPDATA", v); - } - if let Some(v) = prev_xdg { - std::env::set_var("XDG_CONFIG_HOME", v); - } + let paths = ComposerCrawler + .get_vendor_paths(&global_opts(tmp.path())) + .await + .unwrap(); assert_eq!( paths, @@ -472,46 +437,20 @@ async fn get_vendor_paths_global_via_home_xdg_config_composer_fallback() { tokio::fs::create_dir_all(&vendor).await.unwrap(); let empty_path = tempfile::tempdir().unwrap(); - let prev_composer = std::env::var("COMPOSER_HOME").ok(); - let prev_home = std::env::var("HOME").ok(); - let prev_path = std::env::var("PATH").ok(); - std::env::remove_var("COMPOSER_HOME"); - std::env::set_var("HOME", tmp.path()); - std::env::set_var("PATH", empty_path.path()); - // Composer's other platform defaults (`%APPDATA%\Composer` on Windows, - // `$XDG_CONFIG_HOME/composer`) would outrank the HOME candidates here. - let prev_app_data = std::env::var_os("APPDATA"); - let prev_xdg = std::env::var_os("XDG_CONFIG_HOME"); - std::env::remove_var("APPDATA"); - std::env::remove_var("XDG_CONFIG_HOME"); - - let crawler = ComposerCrawler; - let opts = CrawlerOptions { - cwd: tmp.path().to_path_buf(), - global: true, - global_prefix: None, - }; - let paths = crawler.get_vendor_paths(&opts).await.unwrap(); + let _env = EnvVars::set(&[ + ("COMPOSER_HOME", None), + ("HOME", Some(tmp.path().as_os_str())), + ("PATH", Some(empty_path.path().as_os_str())), + // Composer's other platform defaults (`%APPDATA%\Composer` on Windows, + // `$XDG_CONFIG_HOME/composer`) would outrank the HOME candidates here. + ("APPDATA", None), + ("XDG_CONFIG_HOME", None), + ]); - if let Some(v) = prev_composer { - std::env::set_var("COMPOSER_HOME", v); - } - if let Some(v) = prev_home { - std::env::set_var("HOME", v); - } else { - std::env::remove_var("HOME"); - } - if let Some(v) = prev_path { - std::env::set_var("PATH", v); - } else { - std::env::remove_var("PATH"); - } - if let Some(v) = prev_app_data { - std::env::set_var("APPDATA", v); - } - if let Some(v) = prev_xdg { - std::env::set_var("XDG_CONFIG_HOME", v); - } + let paths = ComposerCrawler + .get_vendor_paths(&global_opts(tmp.path())) + .await + .unwrap(); assert_eq!( paths, @@ -531,46 +470,20 @@ async fn get_vendor_paths_global_no_composer_no_home_layout_returns_empty() { let tmp = tempfile::tempdir().unwrap(); let empty_path = tempfile::tempdir().unwrap(); - let prev_composer = std::env::var("COMPOSER_HOME").ok(); - let prev_home = std::env::var("HOME").ok(); - let prev_path = std::env::var("PATH").ok(); - let prev_app_data = std::env::var_os("APPDATA"); - let prev_xdg = std::env::var_os("XDG_CONFIG_HOME"); - std::env::remove_var("APPDATA"); - std::env::remove_var("XDG_CONFIG_HOME"); - std::env::remove_var("COMPOSER_HOME"); - // HOME is set, but the temp HOME has no .composer / .config/composer. - std::env::set_var("HOME", tmp.path()); - // PATH stubbed so the composer CLI cannot be spawned. - std::env::set_var("PATH", empty_path.path()); - - let crawler = ComposerCrawler; - let opts = CrawlerOptions { - cwd: tmp.path().to_path_buf(), - global: true, - global_prefix: None, - }; - let paths = crawler.get_vendor_paths(&opts).await.unwrap(); + let _env = EnvVars::set(&[ + ("APPDATA", None), + ("XDG_CONFIG_HOME", None), + ("COMPOSER_HOME", None), + // HOME is set, but the temp HOME has no .composer / .config/composer. + ("HOME", Some(tmp.path().as_os_str())), + // PATH stubbed so the composer CLI cannot be spawned. + ("PATH", Some(empty_path.path().as_os_str())), + ]); - if let Some(v) = prev_composer { - std::env::set_var("COMPOSER_HOME", v); - } - if let Some(v) = prev_home { - std::env::set_var("HOME", v); - } else { - std::env::remove_var("HOME"); - } - if let Some(v) = prev_path { - std::env::set_var("PATH", v); - } else { - std::env::remove_var("PATH"); - } - if let Some(v) = prev_app_data { - std::env::set_var("APPDATA", v); - } - if let Some(v) = prev_xdg { - std::env::set_var("XDG_CONFIG_HOME", v); - } + let paths = ComposerCrawler + .get_vendor_paths(&global_opts(tmp.path())) + .await + .unwrap(); assert!( paths.is_empty(), @@ -594,53 +507,20 @@ async fn get_vendor_paths_global_empty_home_not_cwd_relative() { .unwrap(); let empty_path = tempfile::tempdir().unwrap(); - let prev_composer = std::env::var("COMPOSER_HOME").ok(); - let prev_home = std::env::var("HOME").ok(); - let prev_profile = std::env::var("USERPROFILE").ok(); - let prev_path = std::env::var("PATH").ok(); - let prev_app_data = std::env::var_os("APPDATA"); - let prev_xdg = std::env::var_os("XDG_CONFIG_HOME"); - std::env::remove_var("APPDATA"); - std::env::remove_var("XDG_CONFIG_HOME"); - std::env::remove_var("COMPOSER_HOME"); - std::env::set_var("HOME", ""); - std::env::set_var("USERPROFILE", ""); - std::env::set_var("PATH", empty_path.path()); - let cwd_guard = CwdGuard::enter(tmp.path()); - - let crawler = ComposerCrawler; - let opts = CrawlerOptions { - cwd: tmp.path().to_path_buf(), - global: true, - global_prefix: None, - }; - let paths = crawler.get_vendor_paths(&opts).await.unwrap(); + let _env = EnvVars::set(&[ + ("APPDATA", None), + ("XDG_CONFIG_HOME", None), + ("COMPOSER_HOME", None), + ("HOME", Some(std::ffi::OsStr::new(""))), + ("USERPROFILE", Some(std::ffi::OsStr::new(""))), + ("PATH", Some(empty_path.path().as_os_str())), + ]); + let _cwd = CwdGuard::enter(tmp.path()); - drop(cwd_guard); - if let Some(v) = prev_composer { - std::env::set_var("COMPOSER_HOME", v); - } - if let Some(v) = prev_home { - std::env::set_var("HOME", v); - } else { - std::env::remove_var("HOME"); - } - if let Some(v) = prev_profile { - std::env::set_var("USERPROFILE", v); - } else { - std::env::remove_var("USERPROFILE"); - } - if let Some(v) = prev_path { - std::env::set_var("PATH", v); - } else { - std::env::remove_var("PATH"); - } - if let Some(v) = prev_app_data { - std::env::set_var("APPDATA", v); - } - if let Some(v) = prev_xdg { - std::env::set_var("XDG_CONFIG_HOME", v); - } + let paths = ComposerCrawler + .get_vendor_paths(&global_opts(tmp.path())) + .await + .unwrap(); assert!( paths.is_empty(), diff --git a/crates/socket-patch-core/tests/crawler_npm_e2e.rs b/crates/socket-patch-core/tests/crawler_npm_e2e.rs index 03abb0b1c..6b733b5a8 100644 --- a/crates/socket-patch-core/tests/crawler_npm_e2e.rs +++ b/crates/socket-patch-core/tests/crawler_npm_e2e.rs @@ -2206,9 +2206,6 @@ async fn crawl_all_inventories_pnpm3_legacy_registry_store_exactly_once() { #[tokio::test] #[serial_test::parallel] async fn apply_and_rollback_reach_every_pnpm_peer_variant_copy() { - use socket_patch_core::hash::git_sha256::compute_git_sha256_from_bytes; - use socket_patch_core::manifest::schema::PatchFileInfo; - use socket_patch_core::patch::apply::{apply_package_patch, MismatchPolicy, PatchSources}; use socket_patch_core::patch::rollback::rollback_package_patch; use std::os::unix::fs::symlink; @@ -2253,7 +2250,7 @@ async fn apply_and_rollback_reach_every_pnpm_peer_variant_copy() { .await .unwrap(); - let mut files = std::collections::HashMap::new(); + let mut files = HashMap::new(); files.insert( "package/index.js".to_string(), PatchFileInfo { @@ -2337,9 +2334,6 @@ async fn apply_and_rollback_reach_every_pnpm_peer_variant_copy() { #[tokio::test] #[serial_test::parallel] async fn apply_heals_unpatched_pnpm_twin_when_primary_already_patched() { - use socket_patch_core::hash::git_sha256::compute_git_sha256_from_bytes; - use socket_patch_core::manifest::schema::PatchFileInfo; - use socket_patch_core::patch::apply::{apply_package_patch, MismatchPolicy, PatchSources}; use std::os::unix::fs::symlink; let tmp = tempfile::tempdir().unwrap(); @@ -2374,7 +2368,7 @@ async fn apply_heals_unpatched_pnpm_twin_when_primary_already_patched() { .await .unwrap(); - let mut files = std::collections::HashMap::new(); + let mut files = HashMap::new(); files.insert( "package/index.js".to_string(), PatchFileInfo { @@ -3109,8 +3103,6 @@ async fn find_by_purls_skips_vlt_dependency_links_and_importer_resolved_copies() #[tokio::test] #[serial_test::parallel] async fn find_by_purls_returns_same_version_git_copy_as_its_own_primary() { - use socket_patch_core::crawlers::npm_crawler::find_store_peer_variant_copies; - let tmp = tempfile::tempdir().unwrap(); let nm = tmp.path().join("node_modules"); let store = nm.join(".vlt"); @@ -3303,51 +3295,44 @@ async fn crawl_all_inventories_vlt_workspace_root_store_once() { ); } -/// The apply fixture shared by the vlt fan-out tests: one patched file -/// `package/index.js` and the blobs to apply and roll it back. -#[cfg(unix)] -struct VltPatch { - files: std::collections::HashMap, - blobs: std::path::PathBuf, +/// The apply fixture shared by the store fan-out tests: one patched file +/// `package/index.js` (ORIGINAL -> PATCHED) and the blobs to apply and +/// roll it back. +struct IndexJsPatch { + files: HashMap, + blobs: PathBuf, } -#[cfg(unix)] -const VLT_ORIGINAL: &[u8] = b"module.exports = 'vulnerable';\n"; -#[cfg(unix)] -const VLT_PATCHED: &[u8] = b"module.exports = 'fixed';\n"; - -#[cfg(unix)] -async fn vlt_patch(root: &Path) -> VltPatch { - use socket_patch_core::hash::git_sha256::compute_git_sha256_from_bytes; - use socket_patch_core::manifest::schema::PatchFileInfo; +const ORIGINAL: &[u8] = b"module.exports = 'vulnerable';\n"; +const PATCHED: &[u8] = b"module.exports = 'fixed';\n"; - let before_hash = compute_git_sha256_from_bytes(VLT_ORIGINAL); - let after_hash = compute_git_sha256_from_bytes(VLT_PATCHED); +async fn index_js_patch(root: &Path) -> IndexJsPatch { + let before_hash = compute_git_sha256_from_bytes(ORIGINAL); + let after_hash = compute_git_sha256_from_bytes(PATCHED); let blobs = root.join("blobs"); tokio::fs::create_dir_all(&blobs).await.unwrap(); - tokio::fs::write(blobs.join(&after_hash), VLT_PATCHED) + tokio::fs::write(blobs.join(&after_hash), PATCHED) .await .unwrap(); - tokio::fs::write(blobs.join(&before_hash), VLT_ORIGINAL) + tokio::fs::write(blobs.join(&before_hash), ORIGINAL) .await .unwrap(); - let files = std::collections::HashMap::from([( + let files = HashMap::from([( "package/index.js".to_string(), PatchFileInfo { before_hash, after_hash, }, )]); - VltPatch { files, blobs } + IndexJsPatch { files, blobs } } -#[cfg(unix)] -async fn vlt_apply( +/// Apply `patch` to `primary` and its store twins under the Warn policy. +async fn apply_index_js( purl: &str, primary: &Path, - patch: &VltPatch, + patch: &IndexJsPatch, ) -> socket_patch_core::patch::apply::ApplyResult { - use socket_patch_core::patch::apply::{apply_package_patch, MismatchPolicy, PatchSources}; let sources = PatchSources { blobs_path: &patch.blobs, mem_blobs: None, @@ -3435,26 +3420,26 @@ async fn apply_and_rollback_reach_every_vlt_variant_copy() { assert!(copies.len() >= 2, "{}: several copies", case.tree); let cas = tmp.path().join("cas-index.js"); - tokio::fs::write(&cas, VLT_ORIGINAL).await.unwrap(); + tokio::fs::write(&cas, ORIGINAL).await.unwrap(); for copy in &copies { std::fs::hard_link(&cas, copy.join("index.js")).unwrap(); } - let patch = vlt_patch(tmp.path()).await; + let patch = index_js_patch(tmp.path()).await; let primary = tmp.path().join(case.primary); assert!(primary.join("package.json").is_file(), "{}", case.tree); - let result = vlt_apply(case.purl, &primary, &patch).await; + let result = apply_index_js(case.purl, &primary, &patch).await; assert!(result.success, "{}: {:?}", case.tree, result.error); for copy in &copies { assert_eq!( tokio::fs::read(copy.join("index.js")).await.unwrap(), - VLT_PATCHED, + PATCHED, "{}: every store copy is patched ({})", case.tree, copy.display() ); } - assert_eq!(tokio::fs::read(&cas).await.unwrap(), VLT_ORIGINAL); + assert_eq!(tokio::fs::read(&cas).await.unwrap(), ORIGINAL); let rb = rollback_package_patch(case.purl, &primary, &patch.files, &patch.blobs, false).await; @@ -3462,13 +3447,13 @@ async fn apply_and_rollback_reach_every_vlt_variant_copy() { for copy in &copies { assert_eq!( tokio::fs::read(copy.join("index.js")).await.unwrap(), - VLT_ORIGINAL, + ORIGINAL, "{}: every store copy is restored ({})", case.tree, copy.display() ); } - assert_eq!(tokio::fs::read(&cas).await.unwrap(), VLT_ORIGINAL); + assert_eq!(tokio::fs::read(&cas).await.unwrap(), ORIGINAL); } } @@ -3487,15 +3472,15 @@ async fn apply_heals_unpatched_vlt_twin_when_primary_already_patched() { .join("~npm~use-sync-external-store@1.2.0~peer.2/node_modules/use-sync-external-store"); let twin = store .join("~npm~use-sync-external-store@1.2.0~peer.3/node_modules/use-sync-external-store"); - tokio::fs::write(primary_copy.join("index.js"), VLT_PATCHED) + tokio::fs::write(primary_copy.join("index.js"), PATCHED) .await .unwrap(); - tokio::fs::write(twin.join("index.js"), VLT_ORIGINAL) + tokio::fs::write(twin.join("index.js"), ORIGINAL) .await .unwrap(); - let patch = vlt_patch(tmp.path()).await; + let patch = index_js_patch(tmp.path()).await; - let result = vlt_apply( + let result = apply_index_js( "pkg:npm/use-sync-external-store@1.2.0", &tmp.path() .join("packages/a/node_modules/use-sync-external-store"), @@ -3505,7 +3490,7 @@ async fn apply_heals_unpatched_vlt_twin_when_primary_already_patched() { assert!(result.success, "{:?}", result.error); assert_eq!( tokio::fs::read(twin.join("index.js")).await.unwrap(), - VLT_PATCHED, + PATCHED, "an AlreadyPatched primary must not mask a still-vulnerable twin" ); } @@ -3917,21 +3902,13 @@ async fn crawl_all_legacy_store_skips_stray_node_modules_and_probes_depth1_home( // physical twin stays divergent. Also the copy-on-write guard for // hardlinked store files. -const ORIGINAL: &[u8] = b"module.exports = 'vulnerable';\n"; -const PATCHED: &[u8] = b"module.exports = 'fixed';\n"; - -/// Stage one store entry `//node_modules/foo` holding a -/// package.json and, when `index_content` is `Some`, an `index.js`. -/// Returns the staged package root. +/// Stage one store entry `//node_modules/foo` (foo@1.0.0) +/// with, when `index_content` is `Some`, an `index.js`. Returns the +/// staged package root. async fn stage_store_entry(store: &Path, entry: &str, index_content: Option<&[u8]>) -> PathBuf { - let pkg = store.join(entry).join("node_modules").join("foo"); - tokio::fs::create_dir_all(&pkg).await.unwrap(); - tokio::fs::write( - pkg.join("package.json"), - r#"{"name":"foo","version":"1.0.0"}"#, - ) - .await - .unwrap(); + let nm = store.join(entry).join("node_modules"); + stage_npm_pkg(&nm, "foo", "1.0.0").await; + let pkg = nm.join("foo"); if let Some(content) = index_content { tokio::fs::write(pkg.join("index.js"), content) .await @@ -3940,39 +3917,9 @@ async fn stage_store_entry(store: &Path, entry: &str, index_content: Option<&[u8 pkg } -/// Shared apply invocation: blob-only sources staged under `root`, one -/// patched file `package/index.js` (ORIGINAL → PATCHED), Warn policy. +/// Apply the shared `package/index.js` patch to foo@1.0.0 at `primary`. async fn apply_foo(root: &Path, primary: &Path) -> socket_patch_core::patch::apply::ApplyResult { - let before_hash = compute_git_sha256_from_bytes(ORIGINAL); - let after_hash = compute_git_sha256_from_bytes(PATCHED); - - let blobs = root.join("blobs"); - tokio::fs::create_dir_all(&blobs).await.unwrap(); - tokio::fs::write(blobs.join(&after_hash), PATCHED) - .await - .unwrap(); - - let mut files = HashMap::new(); - files.insert( - "package/index.js".to_string(), - PatchFileInfo { - before_hash, - after_hash, - }, - ); - let sources = PatchSources { - blobs_path: &blobs, - mem_blobs: None, - }; - apply_package_patch( - "pkg:npm/foo@1.0.0", - primary, - &files, - &sources, - false, - MismatchPolicy::Warn, - ) - .await + apply_index_js("pkg:npm/foo@1.0.0", primary, &index_js_patch(root).await).await } /// Fail-closed invariant: after the primary store copy patches cleanly, a