diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index d9895a2c..a4347f6a 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -1126,6 +1126,8 @@ Every `--json` invocation emits a single JSON object that follows the **unified } ``` +`files[].path` is the manifest file key (`package/index.js`). A pnpm or vlt package installed as more than one peer-variant store copy is patched (and rolled back) in every copy; a file of a copy other than the one the package resolved to is listed under that copy's on-disk path (`…/.pnpm/foo@1.0.0_react@18.3.1/node_modules/foo/index.js`). So a run that wrote only such a copy reports `applied` with that file, not `already_patched`, and rollback's `filesRolledBack` / `filesVerified` carry the same paths (it counts the package in `rolledBack`, not `alreadyOriginal`). + `details` is intentionally schemaless — different subcommands attach different keys. Consumers MUST treat unknown keys as best-effort metadata and must not break on absence. ### `PatchAction` vocabulary diff --git a/crates/socket-patch-cli/src/commands/vex_consumed.rs b/crates/socket-patch-cli/src/commands/vex_consumed.rs index b57d475f..cb0c6802 100644 --- a/crates/socket-patch-cli/src/commands/vex_consumed.rs +++ b/crates/socket-patch-cli/src/commands/vex_consumed.rs @@ -715,8 +715,11 @@ mod tests { None, ) .await; - assert_eq!(installed_again, installed); - let (paths, calls) = tracked_npm_hosted(&common, &installed_again).await; + // Since #605 the name-keyed resolver probes bundled trees itself, so + // it already returns the aliases and the nested store's peers. Feed + // the earlier, alias-free set to keep exercising alias expansion; + // the resolver's own set is checked against the same result below. + let (paths, calls) = tracked_npm_hosted(&common, &installed).await; assert_eq!(calls.len(), 1); let mut inputs = calls[0].clone(); inputs.sort(); @@ -738,6 +741,9 @@ mod tests { .len(), paths.len() ); + let (mut resolved, _) = tracked_npm_hosted(&common, &installed_again).await; + resolved.sort(); + assert_eq!(resolved, expected, "the resolver's own copy set"); } #[cfg(unix)] @@ -768,14 +774,19 @@ mod tests { None, ) .await; - assert!(installed.is_empty(), "{installed:?}"); - let (mut paths, calls) = tracked_npm_hosted(&common, &installed).await; + // Since #605 the name-keyed resolver reaches the alias and its + // sibling peers on its own. An alias-only set (what an alias-blind + // resolver returns) must still expand to the same copies. + let (mut paths, calls) = tracked_npm_hosted(&common, &HashMap::new()).await; assert_eq!(calls, vec![vec![alias.clone()]]); let mut expected = peers; expected.push(alias); paths.sort(); expected.sort(); assert_eq!(paths, expected); + let (mut resolved, _) = tracked_npm_hosted(&common, &installed).await; + resolved.sort(); + assert_eq!(resolved, expected, "the resolver's own copy set"); } #[cfg(unix)] diff --git a/crates/socket-patch-cli/tests/apply/in_process_npm_multicopy.rs b/crates/socket-patch-cli/tests/apply/in_process_npm_multicopy.rs index 2e97747d..724b6e55 100644 --- a/crates/socket-patch-cli/tests/apply/in_process_npm_multicopy.rs +++ b/crates/socket-patch-cli/tests/apply/in_process_npm_multicopy.rs @@ -398,6 +398,121 @@ fn apply_and_rollback_reach_both_transitive_only_vlt_store_copies() { assert_vlt_copies([&primary, &twin], false, "after rollback"); } +/// pnpm twin: one store entry per peer combination +/// (`.pnpm/dupvuln@1.0.0_react@18.2.0/` and `…_react@18.3.1/`), the +/// importer linking the first. Returns `(root, primary index.js, twin +/// index.js)`; the before and after blobs are both staged. +#[cfg(unix)] +fn build_pnpm_peer_variant_tree(tmp: &Path) -> (PathBuf, PathBuf, PathBuf) { + let name = "dupvuln"; + let purl = "pkg:npm/dupvuln@1.0.0"; + let original = b"module.exports = function(){ return 'VULNERABLE'; };\n"; + let mut patched = original.to_vec(); + patched.extend_from_slice(b"// SOCKET-PATCHED-MULTICOPY\n"); + std::fs::write( + tmp.join("package.json"), + r#"{ "name": "pnpm-root", "version": "0.0.0" }"#, + ) + .unwrap(); + let store = tmp.join("node_modules").join(".pnpm"); + let entry = |id: &str| store.join(id).join("node_modules").join(name); + let primary = write_copy( + &entry("dupvuln@1.0.0_react@18.2.0"), + name, + "1.0.0", + original, + ); + let twin = write_copy( + &entry("dupvuln@1.0.0_react@18.3.1"), + name, + "1.0.0", + original, + ); + std::os::unix::fs::symlink( + ".pnpm/dupvuln@1.0.0_react@18.2.0/node_modules/dupvuln", + tmp.join("node_modules").join(name), + ) + .unwrap(); + stage_manifest_and_blob( + tmp, + purl, + &git_sha256(original), + &git_sha256(&patched), + &patched, + ); + std::fs::write( + tmp.join(".socket").join("blobs").join(git_sha256(original)), + original, + ) + .unwrap(); + (tmp.to_path_buf(), primary, twin) +} + +/// The `dupvuln` events of an envelope, as `(action, errorCode)` pairs. +fn dupvuln_events(v: &serde_json::Value) -> Vec<(String, String)> { + v["events"] + .as_array() + .expect("events array") + .iter() + .filter(|e| e["purl"] == "pkg:npm/dupvuln@1.0.0") + .map(|e| { + ( + e["action"].as_str().unwrap_or_default().to_string(), + e["errorCode"].as_str().unwrap_or_default().to_string(), + ) + }) + .collect() +} + +/// #756: with the primary already patched and only a store twin +/// unpatched, apply writes the twin and must report the package as +/// applied (it used to say `already_patched` / `applied: 0`). Rollback is +/// the mirror: with the primary already original and only the twin +/// patched, it restores the twin and must count it as rolled back (it used +/// to say `rolledBack: 0` / `alreadyOriginal: 1`). Covered for both pnpm +/// and vlt store layouts, which share the fan-out. +#[cfg(unix)] +#[test] +fn apply_and_rollback_report_a_write_to_only_a_store_twin() { + for layout in ["pnpm", "vlt"] { + let tmp = tempfile::tempdir().unwrap(); + let (root, primary, twin) = if layout == "pnpm" { + build_pnpm_peer_variant_tree(tmp.path()) + } else { + build_vlt_peer_variant_tree(tmp.path(), true) + }; + let original = std::fs::read(&twin).unwrap(); + + let (code, v) = run_apply(&root); + assert_eq!(code, 0, "{layout}: first apply; envelope={v}"); + assert_vlt_copies([&primary, &twin], true, "after first apply"); + + // A re-created twin: upstream bytes back in the twin only. + std::fs::remove_file(&twin).unwrap(); + std::fs::write(&twin, &original).unwrap(); + let (code, v) = run_apply(&root); + assert_eq!(code, 0, "{layout}: heal apply; envelope={v}"); + assert_vlt_copies([&primary, &twin], true, "after heal apply"); + assert_eq!(v["summary"]["applied"], 1, "{layout}: envelope={v}"); + assert_eq!(v["summary"]["skipped"], 0, "{layout}: envelope={v}"); + assert_eq!( + dupvuln_events(&v), + vec![("applied".to_string(), String::new())], + "{layout}: envelope={v}" + ); + + // Upstream bytes back in the primary only: rollback restores the + // twin and must say so. + std::fs::remove_file(&primary).unwrap(); + std::fs::write(&primary, &original).unwrap(); + let (code, v) = run_rollback(&root); + assert_eq!(code, 0, "{layout}: rollback; envelope={v}"); + assert_vlt_copies([&primary, &twin], false, "after rollback"); + assert_eq!(v["rolledBack"], 1, "{layout}: envelope={v}"); + assert_eq!(v["alreadyOriginal"], 0, "{layout}: envelope={v}"); + } +} + /// #601: a copy bundled inside ANOTHER package's vlt or pnpm store entry /// (`.vlt/~npm~bundler@1.0.0/node_modules/bundler/node_modules/dupvuln`) /// is what that package loads, so apply must patch it even when the same diff --git a/crates/socket-patch-core/src/patch/apply.rs b/crates/socket-patch-core/src/patch/apply.rs index 51fbe8da..4cc3c660 100644 --- a/crates/socket-patch-core/src/patch/apply.rs +++ b/crates/socket-patch-core/src/patch/apply.rs @@ -725,10 +725,11 @@ async fn chown_blocking( /// the primary is AlreadyPatched, which is precisely the state a /// single-copy apply left behind (patched primary, vulnerable twin). A /// failed copy fails the whole result: claiming the CVE fixed while a -/// physical copy remains unpatched is the fail-open this closes. The -/// primary's per-file records are what the returned `ApplyResult` -/// carries (the envelope shape is unchanged); copies contribute only -/// success/error state. +/// physical copy remains unpatched is the fail-open this closes. Each +/// copy's per-file records are merged into the returned `ApplyResult` +/// with the file qualified by the copy's path, so a write to a twin alone +/// still reports the package as applied (see +/// [`store_copies`](crate::patch::store_copies)). pub async fn apply_package_patch( package_key: &str, pkg_path: &Path, @@ -738,50 +739,46 @@ pub async fn apply_package_patch( dry_run: bool, policy: MismatchPolicy, ) -> ApplyResult { - let mut result = - apply_package_patch_at(package_key, pkg_path, files, sources, uuid, dry_run, policy).await; - // Only npm purls can name pnpm or vlt store copies; everything else skips the - // (already cheap) discovery outright. - if result.success && package_key.starts_with("pkg:npm/") { - for copy in crate::crawlers::npm_crawler::find_store_peer_variant_copies(pkg_path).await { - let copy_result = - apply_package_patch_at(package_key, ©, files, sources, uuid, dry_run, policy) - .await; - fold_copy_result(&mut result, ©, copy_result); - } - } - result + crate::patch::store_copies::fan_out(package_key, pkg_path, |path| async move { + apply_package_patch_at(package_key, &path, files, sources, uuid, dry_run, policy).await + }) + .await } -/// Merge one pnpm or vlt store copy's result into the primary's. A failed -/// copy fails the whole result with a `store copy failed to patch: …` -/// note. A copy that patched fine but could not put file ownership back -/// (`success: true, error: Some(": patched, but ownership could not -/// be restored…")`) keeps `success` and appends that advisory verbatim (it -/// already names the copy's full file path), so the CLI's -/// `ownership_not_restored` warning sees every copy. Only the ownership -/// advisory is carried: the `--force` all-skipped note describes the copy -/// alone and would mislead on a primary that actually patched. -fn fold_copy_result(result: &mut ApplyResult, copy: &Path, copy_result: ApplyResult) { - let note = if copy_result.success { - match copy_result.error { - Some(advisory) if advisory.contains(OWNERSHIP_NOT_RESTORED_MARKER) => advisory, - _ => return, +impl crate::patch::store_copies::CopyFold for ApplyResult { + const VERB: &'static str = "patch"; + + fn success(&self) -> bool { + self.success + } + + fn mark_failed(&mut self) { + self.success = false; + } + + fn error_mut(&mut self) -> &mut Option { + &mut self.error + } + + fn extend_files(&mut self, copy: &mut Self, qualify: &dyn Fn(&str) -> String) { + for mut verified in copy.files_verified.drain(..) { + // A successful copy's NotFound is a `--force` skip: like its + // all-skipped note, it describes that copy alone and must not + // turn an already-patched primary into a no-op "applied". + if copy.success && verified.status == VerifyStatus::NotFound { + continue; + } + verified.file = qualify(&verified.file); + self.files_verified.push(verified); } - } else { - result.success = false; - format!( - "store copy {} failed to patch: {}", - copy.display(), - copy_result - .error - .unwrap_or_else(|| "unknown error".to_string()) - ) - }; - result.error = Some(match result.error.take() { - Some(prev) => format!("{prev}; {note}"), - None => note, - }); + for file in copy.files_patched.drain(..) { + let qualified = qualify(&file); + if let Some(via) = copy.applied_via.remove(&file) { + self.applied_via.insert(qualified.clone(), via); + } + self.files_patched.push(qualified); + } + } } /// The substring every ownership advisory carries (see @@ -3442,7 +3439,7 @@ mod tests { /// verbatim (it already names the copy's file path); a copy's `--force` /// all-skipped note is NOT carried (it describes the copy alone). #[test] - fn fold_copy_result_carries_ownership_advisories_and_failures() { + fn store_copy_fold_carries_ownership_advisories_and_failures() { let copy = Path::new("/store/pkg@1.0.0_peer"); let clean = || ApplyResult { package_key: "pkg:npm/a@1.0.0".to_string(), @@ -3456,14 +3453,14 @@ mod tests { }; let mut primary = clean(); - fold_copy_result(&mut primary, copy, clean()); + crate::patch::store_copies::fold(&mut primary, copy, clean()); assert!(primary.success && primary.error.is_none()); let advisory = format!( "/store/pkg@1.0.0_peer/index.js: patched, but {OWNERSHIP_NOT_RESTORED_MARKER} to uid 1 gid 2: EPERM" ); let mut primary = clean(); - fold_copy_result( + crate::patch::store_copies::fold( &mut primary, copy, ApplyResult { @@ -3475,7 +3472,7 @@ mod tests { assert_eq!(primary.error.as_deref(), Some(advisory.as_str())); let mut primary = clean(); - fold_copy_result( + crate::patch::store_copies::fold( &mut primary, copy, ApplyResult { @@ -3489,7 +3486,7 @@ mod tests { ); let mut primary = clean(); - fold_copy_result( + crate::patch::store_copies::fold( &mut primary, copy, ApplyResult { diff --git a/crates/socket-patch-core/src/patch/mod.rs b/crates/socket-patch-core/src/patch/mod.rs index b6f636cb..57becf6d 100644 --- a/crates/socket-patch-core/src/patch/mod.rs +++ b/crates/socket-patch-core/src/patch/mod.rs @@ -10,3 +10,4 @@ pub mod redirect; pub mod rollback; pub mod shared_store; pub mod sidecars; +pub(crate) mod store_copies; diff --git a/crates/socket-patch-core/src/patch/rollback.rs b/crates/socket-patch-core/src/patch/rollback.rs index 11ed4346..be15674b 100644 --- a/crates/socket-patch-core/src/patch/rollback.rs +++ b/crates/socket-patch-core/src/patch/rollback.rs @@ -339,8 +339,10 @@ pub fn cannot_rollback_error(file: &str, why: &str) -> String { /// dry-run (verify only, so a preview fails closed on a copy that cannot /// be rolled back). Apply materializes patch-ADDED files in every copy /// too, so the deletes must reach every copy as well. A failed copy fails -/// the whole result; the primary's per-file records are what the returned -/// `RollbackResult` carries. +/// the whole result; each copy's per-file records are merged into the +/// returned `RollbackResult` with the file qualified by the copy's path, so +/// a restore of a twin alone still counts as rolled back (see +/// [`store_copies`](crate::patch::store_copies)). pub async fn rollback_package_patch( package_key: &str, pkg_path: &Path, @@ -348,47 +350,35 @@ pub async fn rollback_package_patch( blobs_path: &Path, dry_run: bool, ) -> RollbackResult { - let mut result = - rollback_package_patch_at(package_key, pkg_path, files, blobs_path, dry_run).await; - // Only npm purls can name pnpm or vlt store copies; everything else skips the - // (already cheap) discovery outright. - if result.success && package_key.starts_with("pkg:npm/") { - for copy in crate::crawlers::npm_crawler::find_store_peer_variant_copies(pkg_path).await { - let copy_result = - rollback_package_patch_at(package_key, ©, files, blobs_path, dry_run).await; - fold_copy_result(&mut result, ©, copy_result); - } - } - result + crate::patch::store_copies::fan_out(package_key, pkg_path, |path| async move { + rollback_package_patch_at(package_key, &path, files, blobs_path, dry_run).await + }) + .await } -/// Merge one pnpm or vlt store copy's result into the primary's. A failed -/// copy fails the whole result with a `store copy failed to roll -/// back: …` note; a copy that restored fine but carries an advisory -/// (`success: true, error: Some(…)` — e.g. "…ownership could not be -/// restored…") keeps `success` and appends the advisory verbatim, so the -/// CLI's `ownership_not_restored` warning sees every copy, not just the -/// primary. The advisory already names the copy's full file path. -fn fold_copy_result(result: &mut RollbackResult, copy: &Path, copy_result: RollbackResult) { - let note = if copy_result.success { - match copy_result.error { - Some(advisory) => advisory, - None => return, +impl crate::patch::store_copies::CopyFold for RollbackResult { + const VERB: &'static str = "roll back"; + + fn success(&self) -> bool { + self.success + } + + fn mark_failed(&mut self) { + self.success = false; + } + + fn error_mut(&mut self) -> &mut Option { + &mut self.error + } + + fn extend_files(&mut self, copy: &mut Self, qualify: &dyn Fn(&str) -> String) { + for mut verified in copy.files_verified.drain(..) { + verified.file = qualify(&verified.file); + self.files_verified.push(verified); } - } else { - result.success = false; - format!( - "store copy {} failed to roll back: {}", - copy.display(), - copy_result - .error - .unwrap_or_else(|| "unknown error".to_string()) - ) - }; - result.error = Some(match result.error.take() { - Some(prev) => format!("{prev}; {note}"), - None => note, - }); + self.files_rolled_back + .extend(copy.files_rolled_back.drain(..).map(|file| qualify(&file))); + } } /// After deleting a patch-added file, undo the directories apply created @@ -2311,7 +2301,7 @@ mod tests { /// advisory verbatim (it already names the copy's file path); a clean copy /// changes nothing. #[test] - fn fold_copy_result_carries_advisories_and_failures() { + fn store_copy_fold_carries_advisories_and_failures() { let copy = Path::new("/store/pkg@1.0.0_peer"); let clean = || RollbackResult { package_key: "pkg:npm/a@1.0.0".to_string(), @@ -2324,13 +2314,13 @@ mod tests { }; let mut primary = clean(); - fold_copy_result(&mut primary, copy, clean()); + crate::patch::store_copies::fold(&mut primary, copy, clean()); assert!(primary.success && primary.error.is_none()); let advisory = "/store/pkg@1.0.0_peer/index.js: patched, but ownership could not be \ restored to uid 1 gid 2: EPERM"; let mut primary = clean(); - fold_copy_result( + crate::patch::store_copies::fold( &mut primary, copy, RollbackResult { @@ -2343,7 +2333,7 @@ mod tests { let mut primary = clean(); primary.error = Some("first".to_string()); - fold_copy_result( + crate::patch::store_copies::fold( &mut primary, copy, RollbackResult { @@ -2782,6 +2772,7 @@ mod tests { // that is the primary the resolver hands rollback. std::os::unix::fs::symlink(variants[0].join("foo"), nm.join("foo")).unwrap(); let primary = nm.join("foo"); + let twin_added = variants[1].join("foo").join("added.js"); let mut files = HashMap::new(); files.insert( @@ -2801,9 +2792,14 @@ mod tests { ) .await; assert!(result.success, "expected success: {:?}", result.error); + // The primary's delete under its manifest key, the twin's under its + // on-disk path (the store fan-out qualifies copy records). assert_eq!( result.files_rolled_back, - vec!["package/added.js".to_string()] + vec![ + "package/added.js".to_string(), + twin_added.display().to_string(), + ] ); for entry_nm in &variants { assert!( @@ -2834,6 +2830,11 @@ mod tests { .is_err(), "an already-original primary must still heal a patched twin" ); + // ...and report the heal (#756), not "already original". + assert_eq!( + result.files_rolled_back, + vec![twin_added.display().to_string()] + ); } /// #361 / #332: rollback in one project must not restore the original diff --git a/crates/socket-patch-core/src/patch/store_copies.rs b/crates/socket-patch-core/src/patch/store_copies.rs new file mode 100644 index 00000000..372dcfe7 --- /dev/null +++ b/crates/socket-patch-core/src/patch/store_copies.rs @@ -0,0 +1,332 @@ +//! One fan-out over the pnpm/vlt store peer-variant copies of an npm +//! package, shared by agent-mode apply and rollback. +//! +//! pnpm and vlt materialize a separate store copy per peer-dependency (or +//! vlt modifier) combination (`.pnpm/foo@1.0.0(react@17…)/` and +//! `…(react@18…)/`, or `.vlt/~npm~foo@1.0.0~peer.2/` and `~peer.3/`), all +//! real, runtime-loaded dirs, while the purl-keyed resolver hands the +//! engine exactly one primary path. [`fan_out`] runs the single-copy +//! engine on the primary and then on every other physical copy, folding +//! each copy into the primary's result with one rule for both directions: +//! +//! - A failed copy fails the whole result (`store copy failed to +//! : …`). Claiming the CVE fixed (or reverted) while a physical +//! copy is untouched is the fail-open this closes. +//! - A copy's per-file records are merged into the primary's, each file +//! qualified by the copy's on-disk path, so a write that landed only in +//! a twin is visible to the CLI's event classification and tallies. A +//! successful apply copy's `--force` skips (NotFound) are not carried, +//! for the same reason its all-skipped note is not. +//! - Of a successful copy's advisory, only the ownership note is carried +//! (it already names the copy's file). Any other success note (apply's +//! `--force` all-skipped note) describes the copy alone and would +//! mislead on the primary. + +use std::future::Future; +use std::path::{Path, PathBuf}; + +use crate::patch::apply::{normalize_file_path, OWNERSHIP_NOT_RESTORED_MARKER}; + +/// A single-copy engine result that [`fan_out`] can fold copies into. +pub(crate) trait CopyFold: Sized { + /// The verb in a failed copy's note ("patch", "roll back"). + const VERB: &'static str; + fn success(&self) -> bool; + fn mark_failed(&mut self); + fn error_mut(&mut self) -> &mut Option; + /// Move `copy_result`'s per-file records into `self`, renaming each + /// file with `qualify`. + fn extend_files(&mut self, copy_result: &mut Self, qualify: &dyn Fn(&str) -> String); +} + +/// Run `engine` on `pkg_path`, then (for a successful npm result) on every +/// other pnpm/vlt store copy of the same package, folding each copy in. +/// Copies are visited even when the primary was already done: that is the +/// state a single-copy run leaves behind (done primary, untouched twin). +pub(crate) async fn fan_out(package_key: &str, pkg_path: &Path, engine: F) -> R +where + R: CopyFold, + F: Fn(PathBuf) -> Fut, + Fut: Future, +{ + let mut result = engine(pkg_path.to_path_buf()).await; + // Only npm purls can name pnpm or vlt store copies; everything else + // skips the (already cheap) discovery outright. + if result.success() && package_key.starts_with("pkg:npm/") { + for copy in crate::crawlers::npm_crawler::find_store_peer_variant_copies(pkg_path).await { + let copy_result = engine(copy.clone()).await; + fold(&mut result, ©, copy_result); + } + } + result +} + +/// Merge one store copy's result into the primary's (see the module docs). +pub(crate) fn fold(result: &mut R, copy: &Path, mut copy_result: R) { + let qualify = |file: &str| copy.join(normalize_file_path(file)).display().to_string(); + result.extend_files(&mut copy_result, &qualify); + let note = if copy_result.success() { + match copy_result.error_mut().take() { + Some(advisory) if advisory.contains(OWNERSHIP_NOT_RESTORED_MARKER) => advisory, + _ => return, + } + } else { + result.mark_failed(); + format!( + "store copy {} failed to {}: {}", + copy.display(), + R::VERB, + copy_result + .error_mut() + .take() + .unwrap_or_else(|| "unknown error".to_string()) + ) + }; + let error = result.error_mut(); + *error = Some(match error.take() { + Some(prev) => format!("{prev}; {note}"), + None => note, + }); +} + +#[cfg(all(test, unix))] +mod regression_tests { + use std::collections::HashMap; + use std::path::{Path, PathBuf}; + + use crate::hash::git_sha256::compute_git_sha256_from_bytes; + use crate::manifest::schema::PatchFileInfo; + use crate::patch::apply::{apply_package_patch, MismatchPolicy, PatchSources, VerifyStatus}; + use crate::patch::rollback::{rollback_package_patch, VerifyRollbackStatus}; + + const ORIGINAL: &[u8] = b"module.exports = 'upstream';\n"; + const PATCHED: &[u8] = b"module.exports = 'patched';\n"; + + /// A pnpm store with two peer-variant copies of `foo@1.0.0`, the + /// importer's `node_modules/foo` linked to the first (the primary the + /// resolver hands apply/rollback). Returns `(root, primary, copies, + /// blobs, files)`. + async fn pnpm_twins( + primary_bytes: &[u8], + twin_bytes: &[u8], + ) -> ( + tempfile::TempDir, + PathBuf, + [PathBuf; 2], + PathBuf, + HashMap, + ) { + let root = tempfile::tempdir().unwrap(); + let nm = root.path().join("node_modules"); + let store = nm.join(".pnpm"); + let copies = [ + store.join("foo@1.0.0_react@18.2.0/node_modules/foo"), + store.join("foo@1.0.0_react@18.3.1/node_modules/foo"), + ]; + for (copy, bytes) in copies.iter().zip([primary_bytes, twin_bytes]) { + tokio::fs::create_dir_all(copy).await.unwrap(); + tokio::fs::write( + copy.join("package.json"), + r#"{"name":"foo","version":"1.0.0"}"#, + ) + .await + .unwrap(); + tokio::fs::write(copy.join("index.js"), bytes) + .await + .unwrap(); + } + std::os::unix::fs::symlink(&copies[0], nm.join("foo")).unwrap(); + let primary = nm.join("foo"); + + let blobs = root.path().join("blobs"); + tokio::fs::create_dir_all(&blobs).await.unwrap(); + let before_hash = compute_git_sha256_from_bytes(ORIGINAL); + let after_hash = compute_git_sha256_from_bytes(PATCHED); + tokio::fs::write(blobs.join(&before_hash), ORIGINAL) + .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, + }, + ); + (root, primary, copies, blobs, files) + } + + async fn read(path: &Path) -> Vec { + tokio::fs::read(path).await.unwrap() + } + + /// #756: the primary is already patched and only the twin is not. + /// Apply writes the twin, and the result must say so: the twin's file + /// is in `files_patched` (qualified by the copy path, with its + /// `applied_via`), and its verify record is `Ready`, so the CLI + /// classifies the package as applied, not `already_patched`. + #[tokio::test] + async fn apply_reports_a_write_to_an_unpatched_twin_copy() { + let (_root, primary, copies, blobs, files) = pnpm_twins(PATCHED, ORIGINAL).await; + let result = apply_package_patch( + "pkg:npm/foo@1.0.0", + &primary, + &files, + &PatchSources::blobs_only(&blobs), + None, + false, + MismatchPolicy::Warn, + ) + .await; + assert!(result.success, "{:?}", result.error); + assert_eq!(read(&copies[1].join("index.js")).await, PATCHED); + + let twin_file = copies[1].join("index.js").display().to_string(); + assert_eq!(result.files_patched, vec![twin_file.clone()]); + assert!(result.applied_via.contains_key(&twin_file)); + assert!( + result + .files_verified + .iter() + .any(|v| v.file == twin_file && v.status == VerifyStatus::Ready), + "the twin's verify record must be carried: {:?}", + result.files_verified + ); + assert!( + !result + .files_verified + .iter() + .all(|v| v.status == VerifyStatus::AlreadyPatched), + "a run that wrote a copy is not all-already-patched" + ); + } + + /// A `--force` twin that is missing the patched file skips it + /// (success, NotFound record, all-skipped note). That skip describes + /// the twin alone: over an already-patched primary the result must stay + /// all-already-patched, with no NotFound record and no note, or the CLI + /// would report an empty `applied` event instead of `already_patched`. + #[tokio::test] + async fn apply_force_skip_in_a_twin_keeps_an_already_patched_primary() { + let (_root, primary, copies, blobs, files) = pnpm_twins(PATCHED, ORIGINAL).await; + tokio::fs::remove_file(copies[1].join("index.js")) + .await + .unwrap(); + let result = apply_package_patch( + "pkg:npm/foo@1.0.0", + &primary, + &files, + &PatchSources::blobs_only(&blobs), + None, + false, + MismatchPolicy::Force, + ) + .await; + assert!(result.success, "{:?}", result.error); + assert!(result.error.is_none(), "{:?}", result.error); + assert!(result.files_patched.is_empty()); + assert!( + !result.files_verified.is_empty() + && result + .files_verified + .iter() + .all(|v| v.status == VerifyStatus::AlreadyPatched), + "{:?}", + result.files_verified + ); + } + + /// The dry-run twin of #756: a preview over a patched primary and an + /// unpatched twin must not classify as all-already-patched either. + #[tokio::test] + async fn apply_dry_run_carries_an_unpatched_twin_verify_record() { + let (_root, primary, copies, blobs, files) = pnpm_twins(PATCHED, ORIGINAL).await; + let result = apply_package_patch( + "pkg:npm/foo@1.0.0", + &primary, + &files, + &PatchSources::blobs_only(&blobs), + None, + true, + MismatchPolicy::Warn, + ) + .await; + assert!(result.success, "{:?}", result.error); + assert_eq!(read(&copies[1].join("index.js")).await, ORIGINAL); + assert!(result.files_patched.is_empty()); + assert!(result + .files_verified + .iter() + .any(|v| v.status == VerifyStatus::Ready)); + } + + /// Both copies already patched: still `already_patched` (every carried + /// record is AlreadyPatched, nothing written). + #[tokio::test] + async fn apply_over_patched_twins_stays_already_patched() { + let (_root, primary, _copies, blobs, files) = pnpm_twins(PATCHED, PATCHED).await; + let result = apply_package_patch( + "pkg:npm/foo@1.0.0", + &primary, + &files, + &PatchSources::blobs_only(&blobs), + None, + false, + MismatchPolicy::Warn, + ) + .await; + assert!(result.success, "{:?}", result.error); + assert!(result.files_patched.is_empty()); + assert_eq!(result.files_verified.len(), 2); + assert!(result + .files_verified + .iter() + .all(|v| v.status == VerifyStatus::AlreadyPatched)); + } + + /// #756 (rollback, from the issue's follow-up comment): the primary is + /// already original and only the twin is patched. Rollback restores the + /// twin and must report it in `files_rolled_back`, with a `Ready` verify + /// record, so the CLI counts it as rolled back. + #[tokio::test] + async fn rollback_reports_a_restore_of_a_patched_twin_copy() { + let (_root, primary, copies, blobs, files) = pnpm_twins(ORIGINAL, PATCHED).await; + let result = + rollback_package_patch("pkg:npm/foo@1.0.0", &primary, &files, &blobs, false).await; + assert!(result.success, "{:?}", result.error); + assert_eq!(read(&copies[1].join("index.js")).await, ORIGINAL); + + let twin_file = copies[1].join("index.js").display().to_string(); + assert_eq!(result.files_rolled_back, vec![twin_file.clone()]); + assert!( + result + .files_verified + .iter() + .any(|v| v.file == twin_file && v.status == VerifyRollbackStatus::Ready), + "the twin's verify record must be carried: {:?}", + result.files_verified + ); + assert!(!result + .files_verified + .iter() + .all(|v| v.status == VerifyRollbackStatus::AlreadyOriginal)); + } + + /// Both copies already original: still `already_original`. + #[tokio::test] + async fn rollback_over_original_twins_stays_already_original() { + let (_root, primary, _copies, blobs, files) = pnpm_twins(ORIGINAL, ORIGINAL).await; + let result = + rollback_package_patch("pkg:npm/foo@1.0.0", &primary, &files, &blobs, false).await; + assert!(result.success, "{:?}", result.error); + assert!(result.files_rolled_back.is_empty()); + assert_eq!(result.files_verified.len(), 2); + assert!(result + .files_verified + .iter() + .all(|v| v.status == VerifyRollbackStatus::AlreadyOriginal)); + } +}