diff --git a/crates/socket-patch-cli/src/commands/apply.rs b/crates/socket-patch-cli/src/commands/apply.rs index 03816859..96fc577e 100644 --- a/crates/socket-patch-cli/src/commands/apply.rs +++ b/crates/socket-patch-cli/src/commands/apply.rs @@ -69,12 +69,20 @@ async fn ensure_blobs_for_mismatches( args: &ApplyArgs, manifest: &PatchManifest, all_packages: &HashMap, + vendored_purls: &HashSet, staged: &mut StagedSources, ) { if args.common.strict && !args.force { return; // strict fails on mismatch — nothing to fetch } - let needed = mismatch_blob_gaps(manifest, all_packages, &staged.blobs, args.force).await; + let needed = mismatch_blob_gaps( + manifest, + all_packages, + vendored_purls, + &staged.blobs, + args.force, + ) + .await; if needed.is_empty() { return; } @@ -125,10 +133,17 @@ async fn ensure_blobs_for_mismatches( /// qualifier-stripped key, and probe only the variants the apply loop /// will actually attempt (its representative-file installed-distribution /// gate, bypassed by `--force`) so a skipped sibling variant's files -/// don't trigger spurious fetches or `--offline` warnings. +/// don't trigger spurious fetches or `--offline` warnings. An UNQUALIFIED +/// singleton base is always attempted (the mismatch-policy fall-through), +/// so its mismatched files are probed unconditionally; a QUALIFIED +/// singleton keeps the gate, mirroring the apply loop. Vendor-owned bases +/// are skipped outright: the apply loop never attempts them (their +/// results are synthesized up front), so their drifted files must not +/// queue fetches either. async fn mismatch_blob_gaps( manifest: &PatchManifest, all_packages: &HashMap, + vendored_purls: &HashSet, blobs_path: &Path, force: bool, ) -> HashSet { @@ -136,11 +151,27 @@ async fn mismatch_blob_gaps( for (purl, pkg_path) in all_packages { let variant_eco = Ecosystem::from_purl(purl).is_some_and(|e| e.supports_release_variants()); let stripped = strip_purl_qualifiers(purl); - for (key, record) in &manifest.patches { - if key != purl && strip_purl_qualifiers(key) != stripped { - continue; - } - if variant_eco && !force { + let records: Vec<(&String, &PatchRecord)> = manifest + .patches + .iter() + .filter(|(key, _)| *key == purl || strip_purl_qualifiers(key) == stripped) + .collect(); + if vendored_purls.contains(purl.as_str()) + || vendored_purls.contains(stripped) + || records + .iter() + .any(|(key, _)| vendored_purls.contains(key.as_str())) + { + continue; + } + let gated = variant_eco + && !force + && (records.len() > 1 + || records + .first() + .is_some_and(|(key, _)| key.as_str() != stripped)); + for (_, record) in records { + if gated { if let Some((file_name, file_info)) = representative_file(&record.files) { let status = verify_file_patch(pkg_path, file_name, file_info) .await @@ -1068,7 +1099,7 @@ async fn apply_patches_inner( } // Apply patches - ensure_blobs_for_mismatches(args, &manifest, &all_packages, &mut staged).await; + ensure_blobs_for_mismatches(args, &manifest, &all_packages, &vendored_purls, &mut staged).await; let sources = staged.as_patch_sources(); let policy = mismatch_policy(args.force, args.common.strict); let mut has_errors = false; @@ -1133,7 +1164,27 @@ async fn apply_patches_inner( // variant's distribution isn't the one on disk, so skip it — // attempting it would only produce a spurious failure. // Mirrors `select_installed_variants`, used by rollback/get. - if !args.force { + // + // Exempt only an UNQUALIFIED singleton (the common bare + // `pkg:gem/name@ver` manifest key): it names no + // distribution, so a mismatch there is locally-modified + // bytes on the only candidate — exactly what the default + // mismatch policy covers (warn + apply the full verified + // patched content; `--strict` refuses). Gating it made that + // documented policy unreachable for gem/pypi/maven, so it + // falls through to `apply_package_patch`, whose + // `MismatchPolicy` handles it like the npm branch below. + // A QUALIFIED singleton (`?platform=`…) stays gated: it + // names one specific distribution, and — because the + // crawler drops the installed dir's platform suffix (see + // ruby_crawler's `parse_dir_name_version`) — this hash + // check is the ONLY thing resolving whether that + // distribution is the one on disk. Falling through would + // let a lone x86_64-linux record silently overwrite a + // darwin install in the Bundler plugin's `--silent` + // auto-apply, where the warn half of warn-and-apply is + // invisible. + if !args.force && (variants.len() > 1 || variants[0] != base_purl) { let first_status = match representative_file(&patch.files) { Some((file_name, file_info)) => Some( verify_file_patch(pkg_path, file_name, file_info) @@ -1656,7 +1707,8 @@ mod tests { let mut all_packages = HashMap::new(); all_packages.insert("pkg:pypi/foo@1.0.0".to_string(), pkg.clone()); - let needed = mismatch_blob_gaps(&manifest, &all_packages, &blobs, false).await; + let needed = + mismatch_blob_gaps(&manifest, &all_packages, &HashSet::new(), &blobs, false).await; assert_eq!( needed, HashSet::from(["3".repeat(64)]), @@ -1668,10 +1720,15 @@ mod tests { /// installed distribution (its representative file mismatches) is /// skipped by the apply loop, so its blobs must not be queued — that /// would mean spurious downloads and spurious `--offline` "will fail - /// to apply" warnings on every run. Under `--force` every variant IS - /// attempted, so then its blob must be queued. + /// to apply" warnings on every run. An unqualified singleton is + /// always attempted (see the singleton test below), so the group + /// carries an installed wheel sibling alongside the non-installed + /// sdist. Under `--force` every variant IS attempted, so then its + /// blob must be queued. #[tokio::test] async fn mismatch_blob_gaps_skips_non_installed_variant_unless_forced() { + use socket_patch_core::hash::git_sha256::compute_git_sha256_from_bytes; + let dir = tempfile::tempdir().unwrap(); let pkg = dir.path().join("pkg"); tokio::fs::create_dir_all(&pkg).await.unwrap(); @@ -1681,28 +1738,54 @@ mod tests { let blobs = dir.path().join("blobs"); tokio::fs::create_dir_all(&blobs).await.unwrap(); - // The sdist variant's only file has a different base than the + // Installed wheel variant: representative matches the on-disk + // bytes (Ready — no mismatch, so nothing to queue for it). + let mut wheel_files = HashMap::new(); + wheel_files.insert( + "aaa.py".to_string(), + PatchFileInfo { + before_hash: compute_git_sha256_from_bytes(b"pristine\n"), + after_hash: "1".repeat(64), + }, + ); + let mut manifest = manifest_with_record( + "pkg:pypi/foo@1.0.0?artifact_id=foo-1.0.0-py3-none-any.whl", + wheel_files, + ); + // The sdist sibling's only file has a different base than the // on-disk bytes: representative mismatch → not installed. - let mut files = HashMap::new(); - files.insert( + let mut sdist_files = HashMap::new(); + sdist_files.insert( "aaa.py".to_string(), PatchFileInfo { before_hash: "4".repeat(64), after_hash: "5".repeat(64), }, ); - let manifest = - manifest_with_record("pkg:pypi/foo@1.0.0?artifact_id=foo-1.0.0.tar.gz", files); + manifest.patches.insert( + "pkg:pypi/foo@1.0.0?artifact_id=foo-1.0.0.tar.gz".to_string(), + PatchRecord { + uuid: "22222222-2222-4222-8222-222222222222".to_string(), + exported_at: "2024-01-01T00:00:00Z".to_string(), + files: sdist_files, + vulnerabilities: HashMap::new(), + description: "fixture".to_string(), + license: "MIT".to_string(), + tier: "free".to_string(), + }, + ); let mut all_packages = HashMap::new(); all_packages.insert("pkg:pypi/foo@1.0.0".to_string(), pkg.clone()); - let needed = mismatch_blob_gaps(&manifest, &all_packages, &blobs, false).await; + let needed = + mismatch_blob_gaps(&manifest, &all_packages, &HashSet::new(), &blobs, false).await; assert!( needed.is_empty(), - "a non-installed variant is never attempted, so its blobs must not be queued: {needed:?}" + "a non-installed sibling variant is never attempted, so its blobs must not be queued: {needed:?}" ); - let needed = mismatch_blob_gaps(&manifest, &all_packages, &blobs, true).await; + let needed = + mismatch_blob_gaps(&manifest, &all_packages, &HashSet::new(), &blobs, true).await; assert_eq!( needed, HashSet::from(["5".repeat(64)]), @@ -1710,6 +1793,145 @@ mod tests { ); } + /// An UNQUALIFIED singleton release-variant base is always attempted + /// by the apply loop (the mismatch-policy fall-through: it names no + /// distribution, so a mismatch means locally-modified bytes), so its + /// mismatched file's afterHash blob must be queued even though the + /// representative file mismatches — otherwise the default Warn policy + /// has no bytes to overwrite with under `--download-mode diff` and + /// the apply fails instead of warn-overwriting. + #[tokio::test] + async fn mismatch_blob_gaps_singleton_mismatch_queued() { + let dir = tempfile::tempdir().unwrap(); + let pkg = dir.path().join("pkg"); + tokio::fs::create_dir_all(&pkg).await.unwrap(); + tokio::fs::write(pkg.join("aaa.rb"), b"locally modified\n") + .await + .unwrap(); + let blobs = dir.path().join("blobs"); + tokio::fs::create_dir_all(&blobs).await.unwrap(); + + let mut files = HashMap::new(); + files.insert( + "aaa.rb".to_string(), + PatchFileInfo { + before_hash: "4".repeat(64), + after_hash: "5".repeat(64), + }, + ); + let manifest = manifest_with_record("pkg:gem/foo@1.0.0", files); + let mut all_packages = HashMap::new(); + all_packages.insert("pkg:gem/foo@1.0.0".to_string(), pkg.clone()); + + let needed = + mismatch_blob_gaps(&manifest, &all_packages, &HashSet::new(), &blobs, false).await; + assert_eq!( + needed, + HashSet::from(["5".repeat(64)]), + "a singleton base falls through to the mismatch policy, so its blob is needed" + ); + } + + /// A QUALIFIED singleton (`?platform=`…) keeps the + /// installed-distribution gate — it names one specific distribution, + /// and the apply loop skips it when the representative file + /// mismatches (the crawler drops the gem dir's platform suffix, so + /// this hash check is the only platform resolution). Its blobs must + /// not be queued: that would mean spurious downloads and spurious + /// `--offline` warnings for a variant the loop never attempts. Under + /// `--force` it IS attempted, so then the blob is needed. + #[tokio::test] + async fn mismatch_blob_gaps_qualified_singleton_gated_unless_forced() { + let dir = tempfile::tempdir().unwrap(); + let pkg = dir.path().join("pkg"); + tokio::fs::create_dir_all(&pkg).await.unwrap(); + tokio::fs::write(pkg.join("aaa.rb"), b"darwin bytes\n") + .await + .unwrap(); + let blobs = dir.path().join("blobs"); + tokio::fs::create_dir_all(&blobs).await.unwrap(); + + let mut files = HashMap::new(); + files.insert( + "aaa.rb".to_string(), + PatchFileInfo { + before_hash: "4".repeat(64), + after_hash: "5".repeat(64), + }, + ); + let manifest = manifest_with_record("pkg:gem/foo@1.0.0?platform=x86_64-linux", files); + let mut all_packages = HashMap::new(); + all_packages.insert("pkg:gem/foo@1.0.0".to_string(), pkg.clone()); + + let needed = + mismatch_blob_gaps(&manifest, &all_packages, &HashSet::new(), &blobs, false).await; + assert!( + needed.is_empty(), + "a qualified singleton whose distribution is not on disk is never attempted, \ + so its blobs must not be queued: {needed:?}" + ); + + let needed = + mismatch_blob_gaps(&manifest, &all_packages, &HashSet::new(), &blobs, true).await; + assert_eq!( + needed, + HashSet::from(["5".repeat(64)]), + "--force attempts the qualified singleton, so the mismatch blob is needed" + ); + } + + /// A vendor-owned base is unconditionally skipped by the apply loop + /// (its result is synthesized up front), so its drifted installed + /// files must not queue blobs — that meant a spurious "Downloading N + /// full patched blob(s)" fetch online and a spurious "will fail to + /// apply" warning under `--offline` for a package apply never + /// touches. The same fixture queues without the vendor claim + /// (anti-vacuity: the mismatch is real). + #[tokio::test] + async fn mismatch_blob_gaps_vendored_base_never_queued() { + let dir = tempfile::tempdir().unwrap(); + let pkg = dir.path().join("pkg"); + tokio::fs::create_dir_all(&pkg).await.unwrap(); + tokio::fs::write(pkg.join("aaa.rb"), b"drifted\n") + .await + .unwrap(); + let blobs = dir.path().join("blobs"); + tokio::fs::create_dir_all(&blobs).await.unwrap(); + + let mut files = HashMap::new(); + files.insert( + "aaa.rb".to_string(), + PatchFileInfo { + before_hash: "4".repeat(64), + after_hash: "5".repeat(64), + }, + ); + let manifest = manifest_with_record("pkg:gem/foo@1.0.0", files); + let mut all_packages = HashMap::new(); + all_packages.insert("pkg:gem/foo@1.0.0".to_string(), pkg.clone()); + + let needed = + mismatch_blob_gaps(&manifest, &all_packages, &HashSet::new(), &blobs, false).await; + assert_eq!( + needed, + HashSet::from(["5".repeat(64)]), + "without a vendor claim the drifted singleton must queue (fixture sanity)" + ); + + let vendored = HashSet::from(["pkg:gem/foo@1.0.0".to_string()]); + let needed = mismatch_blob_gaps(&manifest, &all_packages, &vendored, &blobs, false).await; + assert!( + needed.is_empty(), + "a vendor-owned base is never attempted, so its blobs must not be queued: {needed:?}" + ); + + let needed = mismatch_blob_gaps(&manifest, &all_packages, &vendored, &blobs, true).await; + assert!( + needed.is_empty(), + "--force does not override vendor ownership in the apply loop, so nothing is queued: {needed:?}" + ); + } + /// Exact-key (npm-shaped) probing keeps working: unqualified manifest /// keys match the crawled purl directly, with no installed-variant /// gate (the npm branch always attempts). @@ -1736,7 +1958,8 @@ mod tests { let mut all_packages = HashMap::new(); all_packages.insert("pkg:npm/foo@1.0.0".to_string(), pkg.clone()); - let needed = mismatch_blob_gaps(&manifest, &all_packages, &blobs, false).await; + let needed = + mismatch_blob_gaps(&manifest, &all_packages, &HashSet::new(), &blobs, false).await; assert_eq!(needed, HashSet::from(["7".repeat(64)])); } diff --git a/crates/socket-patch-cli/src/commands/rollback.rs b/crates/socket-patch-cli/src/commands/rollback.rs index b96deb36..e5198229 100644 --- a/crates/socket-patch-cli/src/commands/rollback.rs +++ b/crates/socket-patch-cli/src/commands/rollback.rs @@ -552,11 +552,120 @@ async fn rollback_patches_inner( .patches .retain(|purl, _| in_scope.contains(purl)); - // Check for missing beforeHash blobs. Local-redirect PURLs (local-mode go) - // are excluded: their rollback just drops the project-local redirect + copy - // and reads no blobs, so a missing before-blob must not block an offline - // redirect rollback. - let gate_manifest = exclude_local_redirects(&scoped_manifest, &args.common); + let crawler_options = CrawlerOptions { + cwd: args.common.cwd.clone(), + global: args.common.global, + global_prefix: args.common.global_prefix.clone(), + }; + + let all_packages = find_packages_for_rollback( + &partitioned, + &crawler_options, + args.common.silent || args.common.json, + ) + .await; + + // Local-redirect rollback (local-mode go) drops a project-local redirect + // and reads nothing out of the ecosystem's package store, so — unlike an + // in-place restore — it must NOT depend on the crawler finding the package + // there. A directory `replace` makes go skip downloading the replaced + // module entirely, so a clone of a repo that committed `go.mod` + + // `.socket/go-patches/` + `.socket/manifest.json` (the documented golang + // workflow) has no module-cache copy for discovery to find. Without this + // fallback the redirect silently survived the rollback: `rollback` + // reported success while the build kept linking the patched copy, and + // `remove` (which delegates here) then deleted the manifest record, + // leaving an active patch nothing tracks. Scoped to `scoped_manifest` so + // `--ecosystems` still applies. + let undiscovered_redirects: Vec = scoped_manifest + .patches + .keys() + .filter(|purl| is_local_redirect(purl, &args.common) && !all_packages.contains_key(*purl)) + .cloned() + .collect(); + + // Group discovered packages by base PURL. A release-variant + // `package@version` (PyPI/RubyGems/Maven) may have several variants + // in the manifest that `merge_qualified` resolves to the same + // installed package dir. Rolling back a variant that is *not* present + // on disk would HashMismatch and report a spurious failure, so — + // mirroring apply — we collapse each group to the variant(s) whose + // hashes actually match the installed bytes. PyPI/RubyGems yield one + // such variant; Maven's coexisting classifier jars may yield several. + let mut groups: HashMap> = HashMap::new(); + for (purl, pkg_path) in &all_packages { + groups + .entry(strip_purl_qualifiers(purl).to_string()) + .or_default() + .push((purl, pkg_path)); + } + + // Resolve which variant(s) each base PURL will actually roll back, + // BEFORE the before-blob gate below, so the gate covers only them. + let mut rollback_targets: Vec<(&String, &PathBuf)> = Vec::new(); + for (_base, entries) in groups { + let to_rollback: Vec<(&String, &PathBuf)> = if entries.len() == 1 { + entries + } else { + // All variants in a group resolve to the same installed path. + let pkg_path = entries[0].1; + let candidates: Vec<(&str, &HashMap)> = entries + .iter() + .filter_map(|(purl, _)| { + filtered_manifest + .patches + .get(*purl) + .map(|p| (purl.as_str(), &p.files)) + }) + .collect(); + let matched = select_installed_variants(pkg_path, &candidates).await; + if matched.is_empty() { + // No variant matches the installed distribution (e.g. a + // locally-modified file). Fall back to attempting every + // variant so the per-file verification surfaces the + // mismatch rather than silently skipping the package. + entries + } else { + let winners: HashSet = matched + .iter() + .map(|&i| candidates[i].0.to_string()) + .collect(); + entries + .into_iter() + .filter(|(p, _)| winners.contains(*p)) + .collect() + } + }; + rollback_targets.extend(to_rollback); + } + + // Check for missing beforeHash blobs — AFTER discovery and variant + // narrowing, so a broad manifest's sibling variants that resolved to + // the same installed package but were narrowed away (they describe a + // distribution that is not on disk) don't gate the run: an + // unfetchable sibling before-blob used to abort the WHOLE rollback + // (`--offline`: wholesale; online: on any download failure) even + // though that variant was never going to be attempted. In-scope + // purls the crawler could NOT resolve keep the fail-closed gate + // (their blobs are still fetched up front). Local-redirect PURLs + // (local-mode go) are excluded as before: their rollback just drops + // the project-local redirect + copy and reads no blobs, so a missing + // before-blob must not block an offline redirect rollback. + let attempted_purls: HashSet<&str> = rollback_targets.iter().map(|(p, _)| p.as_str()).collect(); + let gate_manifest = exclude_local_redirects( + &PatchManifest { + patches: scoped_manifest + .patches + .iter() + .filter(|(purl, _)| { + attempted_purls.contains(purl.as_str()) || !all_packages.contains_key(*purl) + }) + .map(|(k, v)| (k.clone(), v.clone())) + .collect(), + setup: None, + }, + &args.common, + ); // `--dry-run`: verification needs real blob content for an accurate // preview, but the preview must not leave new files in the committable @@ -639,38 +748,6 @@ async fn rollback_patches_inner( } } - let crawler_options = CrawlerOptions { - cwd: args.common.cwd.clone(), - global: args.common.global, - global_prefix: args.common.global_prefix.clone(), - }; - - let all_packages = find_packages_for_rollback( - &partitioned, - &crawler_options, - args.common.silent || args.common.json, - ) - .await; - - // Local-redirect rollback (local-mode go) drops a project-local redirect - // and reads nothing out of the ecosystem's package store, so — unlike an - // in-place restore — it must NOT depend on the crawler finding the package - // there. A directory `replace` makes go skip downloading the replaced - // module entirely, so a clone of a repo that committed `go.mod` + - // `.socket/go-patches/` + `.socket/manifest.json` (the documented golang - // workflow) has no module-cache copy for discovery to find. Without this - // fallback the redirect silently survived the rollback: `rollback` - // reported success while the build kept linking the patched copy, and - // `remove` (which delegates here) then deleted the manifest record, - // leaving an active patch nothing tracks. Scoped to `scoped_manifest` so - // `--ecosystems` still applies. - let undiscovered_redirects: Vec = scoped_manifest - .patches - .keys() - .filter(|purl| is_local_redirect(purl, &args.common) && !all_packages.contains_key(*purl)) - .cloned() - .collect(); - if all_packages.is_empty() && undiscovered_redirects.is_empty() { if !args.common.silent && !args.common.json { println!("No packages found that match patches to rollback"); @@ -678,99 +755,47 @@ async fn rollback_patches_inner( return Ok((true, Vec::new(), vendored_skipped)); } - // Group discovered packages by base PURL. A release-variant - // `package@version` (PyPI/RubyGems/Maven) may have several variants - // in the manifest that `merge_qualified` resolves to the same - // installed package dir. Rolling back a variant that is *not* present - // on disk would HashMismatch and report a spurious failure, so — - // mirroring apply — we collapse each group to the variant(s) whose - // hashes actually match the installed bytes. PyPI/RubyGems yield one - // such variant; Maven's coexisting classifier jars may yield several. - let mut groups: HashMap> = HashMap::new(); - for (purl, pkg_path) in &all_packages { - groups - .entry(strip_purl_qualifiers(purl).to_string()) - .or_default() - .push((purl, pkg_path)); - } - // Rollback patches let mut results: Vec = Vec::new(); let mut has_errors = false; - for (_base, entries) in groups { - // Resolve which variant(s) to roll back for this base PURL. - let to_rollback: Vec<(&String, &PathBuf)> = if entries.len() == 1 { - entries - } else { - // All variants in a group resolve to the same installed path. - let pkg_path = entries[0].1; - let candidates: Vec<(&str, &HashMap)> = entries - .iter() - .filter_map(|(purl, _)| { - filtered_manifest - .patches - .get(*purl) - .map(|p| (purl.as_str(), &p.files)) - }) - .collect(); - let matched = select_installed_variants(pkg_path, &candidates).await; - if matched.is_empty() { - // No variant matches the installed distribution (e.g. a - // locally-modified file). Fall back to attempting every - // variant so the per-file verification surfaces the - // mismatch rather than silently skipping the package. - entries - } else { - let winners: HashSet = matched - .iter() - .map(|&i| candidates[i].0.to_string()) - .collect(); - entries - .into_iter() - .filter(|(p, _)| winners.contains(*p)) - .collect() - } + for (purl, pkg_path) in rollback_targets { + let patch = match filtered_manifest.patches.get(purl) { + Some(p) => p, + None => continue, }; - for (purl, pkg_path) in to_rollback { - let patch = match filtered_manifest.patches.get(purl) { - Some(p) => p, - None => continue, - }; - - // Local go drops the project-local `replace`-redirect; everything - // else — npm/pypi/gem and cargo (vendored or registry cache) — - // restores in place from before-blobs. - let result = match try_rollback_local_go(purl, pkg_path, patch, &args.common).await { - Some(r) => r, - None => { - rollback_package_patch( - purl, - pkg_path, - &patch.files, - &blobs_path, - args.common.dry_run, - ) - .await - } - }; + // Local go drops the project-local `replace`-redirect; everything + // else — npm/pypi/gem and cargo (vendored or registry cache) — + // restores in place from before-blobs. + let result = match try_rollback_local_go(purl, pkg_path, patch, &args.common).await { + Some(r) => r, + None => { + rollback_package_patch( + purl, + pkg_path, + &patch.files, + &blobs_path, + args.common.dry_run, + ) + .await + } + }; - if !result.success { - has_errors = true; - // Errors print even under --silent ("errors only", never - // "nothing"): with the summary muted, this line is the - // silent run's only failure diagnostic. - if !args.common.json { - eprintln!( - "Failed to rollback {}: {}", - purl, - result.error.as_deref().unwrap_or("unknown error") - ); - } + if !result.success { + has_errors = true; + // Errors print even under --silent ("errors only", never + // "nothing"): with the summary muted, this line is the + // silent run's only failure diagnostic. + if !args.common.json { + eprintln!( + "Failed to rollback {}: {}", + purl, + result.error.as_deref().unwrap_or("unknown error") + ); } - results.push(result); } + results.push(result); } // Redirects the crawler never saw (see `undiscovered_redirects` above): diff --git a/crates/socket-patch-cli/tests/cli_gem_variant_mismatch_policy.rs b/crates/socket-patch-cli/tests/cli_gem_variant_mismatch_policy.rs new file mode 100644 index 00000000..25adf4f0 --- /dev/null +++ b/crates/socket-patch-cli/tests/cli_gem_variant_mismatch_policy.rs @@ -0,0 +1,434 @@ +//! Mismatch-policy contract for release-variant (gem) packages. +//! +//! For npm, a locally-modified file is handled by the documented default +//! mismatch policy: warn + apply the full verified patched content, +//! `--strict` refuses, `--force` also skips missing files (see +//! `apply_network.rs::apply_hash_mismatch_default_warns_and_applies_strict_fails`). +//! Release-variant ecosystems (gem/pypi/maven) route through the variant +//! loop instead, whose installed-distribution gate used to skip ANY +//! variant whose representative file mismatched — making the default +//! policy unreachable for them: an UNQUALIFIED SINGLETON base (one bare +//! manifest record for the `package@version`, the common case) with a +//! locally-modified file failed with "no matching variant found" instead +//! of warn-overwriting. +//! +//! Behaviors pinned (all offline, real binary, synthetic gem trees): +//! * unqualified singleton + locally-modified file: default apply warns +//! (`content_mismatch_overwritten`) AND applies the full afterHash +//! bytes; `--strict` refuses (file untouched, exit 1); `--force` +//! keeps applying as before. +//! * QUALIFIED singleton (`?platform=`…): UNCHANGED — it names one +//! specific distribution and the representative-hash gate is the only +//! platform resolution (the crawler drops the gem dir's platform +//! suffix), so a wrong-platform install still fails closed with +//! "no matching variant found" and the file stays untouched. +//! * multi-variant base: UNCHANGED — only the installed variant is +//! applied; the mismatched sibling is skipped, never warn-overwritten +//! (a sibling mismatch means "different distribution", not "locally +//! modified"). When NO variant matches, the base still fails with +//! "no matching variant found" and the file stays untouched. + +use std::path::{Path, PathBuf}; +use std::process::Command; + +use sha2::{Digest, Sha256}; + +const SINGLETON_PURL: &str = "pkg:gem/rack@3.1.0"; +const UUID_SINGLETON: &str = "31313131-3131-4131-8131-313131313131"; + +const PRISTINE: &[u8] = b"module Rack\n VERSION = '3.1.0'\nend\n"; +const LOCAL: &[u8] = b"module Rack\n VERSION = '3.1.0'\nend\n# local tweak\n"; +const PATCH_MARKER: &[u8] = b"\n# SOCKET-SINGLETON-PATCH\n"; + +const MULTI_NAME: &str = "nokogiri"; +const MULTI_VERSION: &str = "1.16.5"; +const UUID_LINUX: &str = "41414141-4141-4141-8141-414141414141"; +const UUID_DARWIN: &str = "42424242-4242-4242-8242-424242424242"; + +const LINUX_PRISTINE: &[u8] = b"module Nokogiri\n VERSION = '1.16.5'\nend\n"; +const LINUX_MARKER: &[u8] = b"\n# SOCKET-LINUX-PATCH\n"; +const DARWIN_BEFORE: &[u8] = b"# nokogiri.rb from the arm64-darwin gem\n"; +const DARWIN_MARKER: &[u8] = b"\n# DARWIN-MARKER\n"; + +fn binary() -> PathBuf { + env!("CARGO_BIN_EXE_socket-patch").into() +} + +/// Git-SHA256: SHA256("blob \0" ++ content). +fn git_sha256(content: &[u8]) -> String { + let header = format!("blob {}\0", content.len()); + let mut hasher = Sha256::new(); + hasher.update(header.as_bytes()); + hasher.update(content); + hex::encode(hasher.finalize()) +} + +fn with_marker(base: &[u8], marker: &[u8]) -> Vec { + let mut v = base.to_vec(); + v.extend_from_slice(marker); + v +} + +/// Run `socket-patch apply --offline --ecosystems gem ` in `cwd` +/// with the ambient `SOCKET_*` environment scrubbed (prefix scrub, keeping +/// `SOCKET_NO_CONFIG`) so only the argv decides behavior, and telemetry +/// disabled. +fn run_apply(cwd: &Path, extra: &[&str]) -> (i32, String, String) { + let mut cmd = Command::new(binary()); + cmd.arg("apply") + .args(["--offline", "--ecosystems", "gem"]) + .args(extra) + .current_dir(cwd); + for (key, _) in std::env::vars_os() { + if key.to_string_lossy().starts_with("SOCKET_") + && key.to_string_lossy() != "SOCKET_NO_CONFIG" + { + cmd.env_remove(&key); + } + } + cmd.env("SOCKET_TELEMETRY_DISABLED", "1"); + let out = cmd.output().expect("run socket-patch apply"); + ( + out.status.code().unwrap_or(-1), + String::from_utf8_lossy(&out.stdout).to_string(), + String::from_utf8_lossy(&out.stderr).to_string(), + ) +} + +/// Synthesize an installed gem under the cwd's vendor/bundle tree (what +/// the ruby crawler scans in local mode; a real `gem install` produces +/// exactly this `gems/-[-]` leaf — verified +/// against RubyGems on ruby 3.4). Returns the patchable file path. +fn install_gem(cwd: &Path, leaf: &str, file_rel: &str, contents: &[u8]) -> PathBuf { + let gem_dir = cwd + .join("vendor") + .join("bundle") + .join("ruby") + .join("3.4.0") + .join("gems") + .join(leaf); + let file = gem_dir.join(file_rel); + std::fs::create_dir_all(file.parent().unwrap()).expect("create gem dir"); + std::fs::write(&file, contents).expect("write gem file"); + file +} + +fn patch_record(uuid: &str, file: &str, before_hash: &str, after_hash: &str) -> serde_json::Value { + serde_json::json!({ + "uuid": uuid, + "exportedAt": "2024-01-01T00:00:00Z", + "files": { file: { "beforeHash": before_hash, "afterHash": after_hash } }, + "vulnerabilities": {}, + "description": "gem variant mismatch fixture", + "license": "MIT", + "tier": "free" + }) +} + +fn write_socket_dir(cwd: &Path, patches: serde_json::Value, blobs: &[(&str, &[u8])]) { + let socket = cwd.join(".socket"); + let blobs_dir = socket.join("blobs"); + std::fs::create_dir_all(&blobs_dir).expect("create .socket/blobs"); + std::fs::write( + socket.join("manifest.json"), + serde_json::to_vec_pretty(&serde_json::json!({ "patches": patches })).unwrap(), + ) + .expect("write manifest"); + for (hash, content) in blobs { + std::fs::write(blobs_dir.join(hash), content).expect("write blob"); + } +} + +/// Singleton fixture: one bare-PURL gem record whose only file was +/// locally modified on disk (matches NEITHER beforeHash nor afterHash); +/// the afterHash blob is staged so the offline apply has the full +/// patched bytes available. Returns the on-disk file path and the exact +/// patched bytes a correct warn-overwrite must produce. +fn singleton_fixture(cwd: &Path) -> (PathBuf, Vec) { + let patched = with_marker(PRISTINE, PATCH_MARKER); + let file = install_gem(cwd, "rack-3.1.0", "lib/rack.rb", LOCAL); + write_socket_dir( + cwd, + serde_json::json!({ + SINGLETON_PURL: patch_record( + UUID_SINGLETON, + "lib/rack.rb", + &git_sha256(PRISTINE), + &git_sha256(&patched), + ) + }), + &[(&git_sha256(&patched), patched.as_slice())], + ); + // Fixture sanity: the on-disk bytes must match neither hash, or the + // mismatch path under test is never taken. + assert_ne!(git_sha256(LOCAL), git_sha256(PRISTINE)); + assert_ne!(git_sha256(LOCAL), git_sha256(&patched)); + (file, patched) +} + +/// Default policy: the singleton's local modification is overwritten with +/// the full verified patched content and surfaced as a warning — the same +/// npm contract, previously unreachable for gem (the run failed with +/// "no matching variant found" and left the file untouched). +#[test] +fn singleton_mismatch_default_warns_and_applies() { + let tmp = tempfile::tempdir().expect("tempdir"); + let (file, patched) = singleton_fixture(tmp.path()); + + let (code, _stdout, stderr) = run_apply(tmp.path(), &[]); + assert_eq!( + code, 0, + "default mismatch on a singleton variant is a warning, not an error; stderr={stderr}" + ); + assert_eq!( + std::fs::read(&file).expect("read patched file"), + patched, + "the file must carry exactly the verified patched bytes" + ); + assert!( + stderr.contains("content_mismatch_overwritten"), + "the overwrite must be surfaced as the npm-family mismatch warning; stderr={stderr}" + ); + assert!( + stderr.contains(SINGLETON_PURL), + "the warning must name the package; stderr={stderr}" + ); + assert!( + stderr.contains("applied the full verified patched content"), + "the warning must say what happened to the file; stderr={stderr}" + ); + + // JSON envelope twin: `applied` event for the purl plus the per-file + // warning event — the same shape apply_network pins for npm. + let tmp = tempfile::tempdir().expect("tempdir"); + let (file, patched) = singleton_fixture(tmp.path()); + let (code, stdout, _stderr) = run_apply(tmp.path(), &["--json"]); + assert_eq!(code, 0, "json run must also succeed; stdout={stdout}"); + assert_eq!(std::fs::read(&file).expect("read patched file"), patched); + let v: serde_json::Value = serde_json::from_str(stdout.trim()).expect("valid JSON envelope"); + assert_eq!(v["status"], "success", "{v:#}"); + let events = v["events"].as_array().expect("events array"); + assert!( + events + .iter() + .any(|e| e["action"] == "applied" && e["purl"] == SINGLETON_PURL), + "singleton must be reported applied: {events:?}" + ); + assert!( + events + .iter() + .any(|e| e["errorCode"] == "content_mismatch_overwritten"), + "the overwrite must ride as a warning event: {events:?}" + ); +} + +/// `--strict` restores the fail-closed contract for the singleton: exit 1 +/// with the per-file hash-mismatch error and the file byte-identical. +#[test] +fn singleton_mismatch_strict_refuses() { + let tmp = tempfile::tempdir().expect("tempdir"); + let (file, _patched) = singleton_fixture(tmp.path()); + + let (code, _stdout, stderr) = run_apply(tmp.path(), &["--strict"]); + assert_eq!( + code, 1, + "--strict must refuse the mismatch; stderr={stderr}" + ); + assert_eq!( + std::fs::read(&file).expect("read file"), + LOCAL, + "--strict must not modify the file" + ); + assert!( + stderr.contains(&format!("Failed to patch {SINGLETON_PURL}")), + "--strict must fail the package with a per-package error; stderr={stderr}" + ); + assert!( + stderr.contains("File hash does not match expected value"), + "--strict must name the actual refusal (the hash mismatch), not a \ + generic no-matching-variant miss; stderr={stderr}" + ); + assert!( + !stderr.contains("content_mismatch_overwritten"), + "--strict must not claim an overwrite happened; stderr={stderr}" + ); +} + +/// `--force` behavior is unchanged by the singleton fall-through: it +/// bypassed the gate before and still applies the full patched bytes. +#[test] +fn singleton_mismatch_force_still_applies() { + let tmp = tempfile::tempdir().expect("tempdir"); + let (file, patched) = singleton_fixture(tmp.path()); + + let (code, _stdout, stderr) = run_apply(tmp.path(), &["--force"]); + assert_eq!(code, 0, "--force must keep applying; stderr={stderr}"); + assert_eq!( + std::fs::read(&file).expect("read patched file"), + patched, + "--force must overwrite with the verified patched bytes" + ); +} + +fn multi_purl(platform: &str) -> String { + format!("pkg:gem/{MULTI_NAME}@{MULTI_VERSION}?platform={platform}") +} + +/// A QUALIFIED singleton keeps the fail-closed gate: a lone +/// `?platform=x86_64-linux` record names one specific distribution, and +/// the representative-hash check is the ONLY thing resolving whether that +/// distribution is the one on disk (the crawler drops the gem dir's +/// platform suffix). With an arm64-darwin gem installed, the default +/// apply must NOT overwrite it with the linux variant's bytes — the +/// Bundler plugin auto-applies with `--silent`, where the warn half of +/// warn-and-apply is invisible, and a later rollback would restore the +/// LINUX before-bytes onto the darwin install. It fails exactly like a +/// multi-variant base with no matching variant. +#[test] +fn qualified_singleton_wrong_platform_fails_closed() { + let tmp = tempfile::tempdir().expect("tempdir"); + let linux_patched = with_marker(LINUX_PRISTINE, LINUX_MARKER); + // On disk: the arm64-darwin gem, whose bytes match NEITHER of the + // linux record's hashes. + let file = install_gem( + tmp.path(), + &format!("{MULTI_NAME}-{MULTI_VERSION}-arm64-darwin"), + &format!("lib/{MULTI_NAME}.rb"), + DARWIN_BEFORE, + ); + write_socket_dir( + tmp.path(), + serde_json::json!({ + multi_purl("x86_64-linux"): patch_record( + UUID_LINUX, + "lib/nokogiri.rb", + &git_sha256(LINUX_PRISTINE), + &git_sha256(&linux_patched), + ), + }), + &[(&git_sha256(&linux_patched), linux_patched.as_slice())], + ); + // Fixture sanity: the darwin bytes must match neither hash, or the + // wrong-platform path under test is never taken. + assert_ne!(git_sha256(DARWIN_BEFORE), git_sha256(LINUX_PRISTINE)); + assert_ne!(git_sha256(DARWIN_BEFORE), git_sha256(&linux_patched)); + + let (code, _stdout, stderr) = run_apply(tmp.path(), &[]); + assert_eq!( + code, 1, + "a qualified singleton whose distribution is not on disk must fail closed; stderr={stderr}" + ); + assert!( + stderr.contains("no matching variant found"), + "the failure must stay the no-matching-variant error; stderr={stderr}" + ); + assert!( + !stderr.contains("content_mismatch_overwritten"), + "a wrong-distribution record must never be surfaced as a \ + local-modification overwrite; stderr={stderr}" + ); + assert_eq!( + std::fs::read(&file).expect("read file"), + DARWIN_BEFORE, + "the installed distribution's bytes must be untouched" + ); +} + +/// Multi-variant fixture: the x86_64-linux variant is installed (its +/// beforeHash matches the on-disk bytes unless `local_bytes` overrides +/// them); the arm64-darwin sibling describes a DIFFERENT distribution +/// whose beforeHash can never match. Both afterHash blobs are staged. +fn multi_variant_fixture(cwd: &Path, on_disk: &[u8]) -> (PathBuf, Vec, Vec) { + let linux_patched = with_marker(LINUX_PRISTINE, LINUX_MARKER); + let darwin_patched = with_marker(DARWIN_BEFORE, DARWIN_MARKER); + let file = install_gem( + cwd, + &format!("{MULTI_NAME}-{MULTI_VERSION}-x86_64-linux"), + &format!("lib/{MULTI_NAME}.rb"), + on_disk, + ); + write_socket_dir( + cwd, + serde_json::json!({ + multi_purl("x86_64-linux"): patch_record( + UUID_LINUX, + "lib/nokogiri.rb", + &git_sha256(LINUX_PRISTINE), + &git_sha256(&linux_patched), + ), + multi_purl("arm64-darwin"): patch_record( + UUID_DARWIN, + "lib/nokogiri.rb", + &git_sha256(DARWIN_BEFORE), + &git_sha256(&darwin_patched), + ), + }), + &[ + (&git_sha256(&linux_patched), linux_patched.as_slice()), + (&git_sha256(&darwin_patched), darwin_patched.as_slice()), + ], + ); + (file, linux_patched, darwin_patched) +} + +/// Over-broadening guard: a MULTI-variant base keeps the current +/// behavior. The installed variant applies; the mismatched sibling means +/// "different distribution" and is skipped — its bytes must never be +/// warn-overwritten onto the installed gem, and no mismatch warning may +/// fire. +#[test] +fn multi_variant_mismatched_sibling_is_skipped_not_overwritten() { + let tmp = tempfile::tempdir().expect("tempdir"); + let (file, linux_patched, _darwin_patched) = multi_variant_fixture(tmp.path(), LINUX_PRISTINE); + + let (code, _stdout, stderr) = run_apply(tmp.path(), &[]); + assert_eq!( + code, 0, + "the installed variant must apply cleanly; stderr={stderr}" + ); + let after = std::fs::read(&file).expect("read patched file"); + assert_eq!( + after, linux_patched, + "the file must carry exactly the installed platform's patched bytes" + ); + assert!( + !after + .windows(DARWIN_MARKER.len()) + .any(|w| w == DARWIN_MARKER), + "the sibling distribution's bytes must never be written" + ); + assert!( + !stderr.contains("content_mismatch_overwritten"), + "a sibling-variant mismatch is a skip, not an overwrite warning; stderr={stderr}" + ); +} + +/// Second guard: when NO variant of a multi-variant base matches the +/// on-disk bytes (locally-modified file), the base still fails with the +/// no-matching-variant error and the file stays untouched — the +/// mismatch-policy fall-through is singleton-only. +#[test] +fn multi_variant_none_matching_still_fails_closed() { + let local = with_marker(LINUX_PRISTINE, b"# local tweak\n"); + let tmp = tempfile::tempdir().expect("tempdir"); + let (file, linux_patched, darwin_patched) = multi_variant_fixture(tmp.path(), &local); + assert_ne!(git_sha256(&local), git_sha256(LINUX_PRISTINE)); + assert_ne!(git_sha256(&local), git_sha256(DARWIN_BEFORE)); + + let (code, _stdout, stderr) = run_apply(tmp.path(), &[]); + assert_eq!( + code, 1, + "a multi-variant base with no matching variant must keep failing; stderr={stderr}" + ); + assert!( + stderr.contains("no matching variant found"), + "the failure must stay the no-matching-variant error; stderr={stderr}" + ); + let after = std::fs::read(&file).expect("read file"); + assert_eq!( + after, local, + "no variant matched, so nothing may be written" + ); + assert_ne!(after, linux_patched); + assert_ne!(after, darwin_patched); +} diff --git a/crates/socket-patch-cli/tests/in_process_gem_multi_platform.rs b/crates/socket-patch-cli/tests/in_process_gem_multi_platform.rs index 69aa5d55..244ee6b3 100644 --- a/crates/socket-patch-cli/tests/in_process_gem_multi_platform.rs +++ b/crates/socket-patch-cli/tests/in_process_gem_multi_platform.rs @@ -18,6 +18,10 @@ //! * `remove ` over a broad manifest removes ALL platform //! variants and rolls back the file without spurious failure. //! * `rollback` (no id) over a broad manifest exits 0. +//! * `rollback`/`remove` over a broad manifest succeed even when the +//! UNINSTALLED sibling variant's before-blob is missing and +//! unfetchable — the gate covers only the narrowed (installed) +//! variant, online and `--offline`. use std::path::{Path, PathBuf}; @@ -500,6 +504,213 @@ async fn remove_base_purl_clears_all_platforms_and_rolls_back() { ); } +/// Delete the uninstalled (darwin) sibling's cached before-blob, +/// returning its hash. Asserts the blob was actually cached by the broad +/// scan first — without that, the "unfetchable sibling blob" scenario +/// below would pass vacuously (nothing missing, nothing gated). +fn delete_darwin_before_blob(cwd: &Path) -> String { + let hash = git_sha256(DARWIN_BEFORE_BYTES); + let blob = cwd.join(".socket").join("blobs").join(&hash); + assert!( + blob.exists(), + "broad scan must have cached the darwin before-blob at {}", + blob.display() + ); + std::fs::remove_file(&blob).expect("delete darwin before-blob"); + hash +} + +fn rollback_args(cwd: &Path, api_url: String, offline: bool) -> RollbackArgs { + RollbackArgs { + identifier: None, + common: socket_patch_cli::args::GlobalArgs { + cwd: cwd.to_path_buf(), + org: Some(ORG.to_string()), + api_url: Some(api_url), + api_token: Some("fake".to_string()), + json: true, + offline, + ecosystems: Some(vec!["gem".to_string()]), + ..socket_patch_cli::args::GlobalArgs::default() + }, + one_off: false, + } +} + +/// A broad manifest's UNINSTALLED sibling variant resolves to the same +/// installed dir but is narrowed away by `select_installed_variants`, so +/// its before-blob is never read. With that blob missing and unfetchable +/// (the mock serves no blob endpoint — every fetch 404s), rollback must +/// still restore the installed variant. Before the fix the before-blob +/// gate ran over the whole ecosystem-scoped manifest BEFORE narrowing, so +/// the sibling's 404 aborted the entire rollback ("1 blob(s) could not be +/// downloaded. Cannot rollback.") with the installed file left patched. +#[tokio::test] +#[serial] +async fn rollback_succeeds_when_uninstalled_sibling_before_blob_unfetchable() { + let tmp = tempfile::tempdir().expect("tempdir"); + let (gem_file, server) = fixture(tmp.path()).await; + + assert_eq!( + scan_run(scan_args(tmp.path(), server.uri(), true)).await, + 0, + "broad scan+apply must exit 0 before rollback" + ); + assert_eq!( + read_file(&gem_file), + patched_bytes(), + "gem must be patched before rollback" + ); + let darwin_hash = delete_darwin_before_blob(tmp.path()); + + let code = rollback_run(rollback_args(tmp.path(), server.uri(), false)).await; + assert_eq!( + code, 0, + "an unfetchable before-blob for the narrowed-away sibling variant must not abort rollback" + ); + assert_eq!( + read_file(&gem_file), + ORIGINAL_BYTES, + "the installed variant must be restored to exactly its original bytes" + ); + // The sibling never gates, so its blob must not even be requested. + let reqs = recorded(&server).await; + assert!( + !reqs + .iter() + .any(|r| r.url.path().contains(darwin_hash.as_str())), + "rollback must not fetch the narrowed-away sibling's before-blob; paths={:?}", + reqs.iter() + .map(|r| r.url.path().to_string()) + .collect::>() + ); + // Rollback is not remove: both variants stay recorded. + let mut keys = manifest_keys(tmp.path()); + keys.sort(); + let mut expected = vec![qualified(PLATFORM_INSTALLED), qualified(PLATFORM_OTHER)]; + expected.sort(); + assert_eq!( + keys, expected, + "rollback must leave both variants in the manifest" + ); +} + +/// The `--offline` twin: with only the INSTALLED variant's blobs cached, +/// an offline rollback of the broad manifest must succeed without any +/// blob fetch. Before the fix the pre-narrowing gate counted the missing +/// sibling blob and bailed ("blob(s) are missing and --offline mode is +/// enabled") before restoring anything. +#[tokio::test] +#[serial] +async fn rollback_offline_succeeds_with_only_installed_variant_blobs_cached() { + let tmp = tempfile::tempdir().expect("tempdir"); + let (gem_file, server) = fixture(tmp.path()).await; + + assert_eq!( + scan_run(scan_args(tmp.path(), server.uri(), true)).await, + 0, + "broad scan+apply must exit 0 before rollback" + ); + assert_eq!( + read_file(&gem_file), + patched_bytes(), + "gem must be patched before rollback" + ); + delete_darwin_before_blob(tmp.path()); + + // `rollback --offline` mirrors the flag into SOCKET_OFFLINE for the + // process (apply_env_toggles); save/restore it so this #[serial] + // binary's later tests don't inherit an offline environment. + let prev_offline = std::env::var("SOCKET_OFFLINE").ok(); + let code = rollback_run(rollback_args(tmp.path(), server.uri(), true)).await; + match prev_offline { + Some(v) => std::env::set_var("SOCKET_OFFLINE", v), + None => std::env::remove_var("SOCKET_OFFLINE"), + } + + assert_eq!( + code, 0, + "--offline rollback needs only the installed variant's cached blobs" + ); + assert_eq!( + read_file(&gem_file), + ORIGINAL_BYTES, + "the installed variant must be restored to exactly its original bytes" + ); + // No blob endpoint may be hit at all: everything needed was cached. + let reqs = recorded(&server).await; + assert!( + !reqs.iter().any(|r| r.url.path().contains("/patches/blob/")), + "--offline rollback must not fetch any blob; paths={:?}", + reqs.iter() + .map(|r| r.url.path().to_string()) + .collect::>() + ); +} + +/// `remove ` delegates to the same rollback path: with the +/// uninstalled sibling's before-blob missing and unfetchable (404), the +/// remove must still roll back the installed variant and clear BOTH +/// manifest records. Before the fix it aborted with `rollback_failed`, +/// leaving the file patched and the manifest intact. +#[tokio::test] +#[serial] +async fn remove_succeeds_when_uninstalled_sibling_before_blob_unfetchable() { + let tmp = tempfile::tempdir().expect("tempdir"); + let (gem_file, server) = fixture(tmp.path()).await; + + assert_eq!( + scan_run(scan_args(tmp.path(), server.uri(), true)).await, + 0, + "broad scan+apply must exit 0 before remove" + ); + assert_eq!( + read_file(&gem_file), + patched_bytes(), + "gem must be patched before remove" + ); + let darwin_hash = delete_darwin_before_blob(tmp.path()); + + let code = remove_run(RemoveArgs { + identifier: base_purl(), + common: socket_patch_cli::args::GlobalArgs { + cwd: tmp.path().to_path_buf(), + org: Some(ORG.to_string()), + api_url: Some(server.uri()), + api_token: Some("fake".to_string()), + json: true, + yes: true, + ecosystems: Some(vec!["gem".to_string()]), + ..socket_patch_cli::args::GlobalArgs::default() + }, + skip_rollback: false, + }) + .await; + assert_eq!( + code, 0, + "an unfetchable before-blob for the narrowed-away sibling variant must not abort remove" + ); + assert!( + manifest_keys(tmp.path()).is_empty(), + "remove must clear every platform variant from the manifest" + ); + assert_eq!( + read_file(&gem_file), + ORIGINAL_BYTES, + "remove must roll the gem file back to exactly its original bytes" + ); + let reqs = recorded(&server).await; + assert!( + !reqs + .iter() + .any(|r| r.url.path().contains(darwin_hash.as_str())), + "remove must not fetch the narrowed-away sibling's before-blob; paths={:?}", + reqs.iter() + .map(|r| r.url.path().to_string()) + .collect::>() + ); +} + #[tokio::test] #[serial] async fn rollback_all_over_broad_manifest_succeeds() { diff --git a/crates/socket-patch-cli/tests/rollback_invariants.rs b/crates/socket-patch-cli/tests/rollback_invariants.rs index fb9fbbda..3f4cf1b8 100644 --- a/crates/socket-patch-cli/tests/rollback_invariants.rs +++ b/crates/socket-patch-cli/tests/rollback_invariants.rs @@ -272,7 +272,8 @@ fn rollback_offline_with_missing_before_blob_partial_failure() { assert_eq!(v["dryRun"], false, "not a dry-run"); // Known design gap (see memory `apply-invariants-test-hardened`): the // offline missing-blob bail returns a *contentless* partial_failure — it - // aborts before crawling, so `failed` stays 0 and `results` is empty even + // aborts after discovery but before the rollback loop produces any + // per-package results, so `failed` stays 0 and `results` is empty even // though the run did not succeed. Pin that exact shape so the bail can't // silently morph into either a real failure count or a spurious success. assert_eq!(