From c9e29bb27e4c8dbb4c54df43897adb64b77278a7 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Fri, 14 Aug 2026 07:05:55 -0700 Subject: [PATCH] fix(vendor): carry berry lock original across patch updates Re-vendoring a yarn berry project under a new patch uuid dropped the pre-vendor registry lock entry from the ledger: the berry lock wiring key is the file: locator key, which embeds the uuid, so the carry-forward in persist_vendor_entry never matched the record being replaced. A later `vendor --revert` then deleted the artifact dir but left the dangling file: entry in yarn.lock, breaking `yarn install --immutable` until a plain install re-resolved it. Wiring identity is now uuid-agnostic (wiring_key_matches normalizes the embedded .socket/vendor// level), so revert restores the pristine registry entry byte-for-byte; old ledgers parse and match unchanged. Also: post-pack wiring failures in the yarn berry and classic backends now unwind the freshly created .socket/vendor uuid dir (previously orphaned with no ledger entry, so revert could never remove it), and berry vendoring surfaces npm-alias descriptors of the patched package - a loud vendor_alias_entry_skipped warning when a plain entry also vendors, and an alias-aware refusal detail when the alias is the only consumer - instead of silently leaving that copy on the unpatched bytes. Co-authored-by: Claude Fable 5 --- .../socket-patch-cli/src/commands/vendor.rs | 19 +- .../tests/in_process_vendor.rs | 131 ++++++++++ .../src/vendor/npm_common.rs | 28 +++ crates/socket-patch-core/src/vendor/path.rs | 85 +++++++ .../src/vendor/yarn_berry_lock.rs | 225 ++++++++++++++++-- .../src/vendor/yarn_classic_lock.rs | 76 +++++- 6 files changed, 532 insertions(+), 32 deletions(-) diff --git a/crates/socket-patch-cli/src/commands/vendor.rs b/crates/socket-patch-cli/src/commands/vendor.rs index e95638ad..62d69114 100644 --- a/crates/socket-patch-cli/src/commands/vendor.rs +++ b/crates/socket-patch-cli/src/commands/vendor.rs @@ -536,14 +536,21 @@ pub(crate) async fn persist_vendor_entry( // carry it forward by wiring identity, or a // later `--revert` can only shrug // (`vendor_lock_entry_drifted`) instead of - // restoring the registry fragment. + // restoring the registry fragment. Identity is + // uuid-agnostic (`wiring_key_matches`): berry's + // lock key embeds the vendored path, so the + // uuid change that CAUSED the re-vendor changes + // the key too. for rec in &mut entry.wiring { if rec.action == vendor::state::WiringAction::Rewritten && rec.original.is_none() { - if let Some(prev_rec) = prev - .wiring - .iter() - .find(|p| p.file == rec.file && p.kind == rec.kind && p.key == rec.key) - { + if let Some(prev_rec) = prev.wiring.iter().find(|p| { + p.file == rec.file + && p.kind == rec.kind + && match (p.key.as_deref(), rec.key.as_deref()) { + (Some(a), Some(b)) => vendor::path::wiring_key_matches(a, b), + (a, b) => a == b, + } + }) { rec.original = prev_rec.original.clone(); } } diff --git a/crates/socket-patch-cli/tests/in_process_vendor.rs b/crates/socket-patch-cli/tests/in_process_vendor.rs index 2b263cce..2695dc8b 100644 --- a/crates/socket-patch-cli/tests/in_process_vendor.rs +++ b/crates/socket-patch-cli/tests/in_process_vendor.rs @@ -685,6 +685,137 @@ async fn revendor_new_uuid_cleans_stale_artifact_and_still_reverts() { assert!(!fx.vendor_dir().exists(), "vendor tree fully pruned"); } +// ───────────────────────────────────────────────────────────────────── +// 8d. re-vendor under a new patch uuid — yarn berry +// ───────────────────────────────────────────────────────────────────── + +/// The yarn-berry variant of 8c. Berry's lock wiring key is the `file:` +/// locator key, which EMBEDS the patch uuid — so the carry-forward that +/// keeps the pre-vendor registry fragment across a patch update must match +/// wiring records uuid-agnostically. Without that, the re-vendored entry +/// keeps `original: null`, and a later `--revert` deletes the artifact dir +/// while leaving the dangling `file:` lock entry in place (a permanently +/// broken `yarn install --immutable`). +#[tokio::test] +async fn revendor_new_uuid_carries_original_forward_yarn_berry() { + const UUID2: &str = "0a1b2c3d-4e5f-6a7b-8c9d-0e1f2a3b4c5d"; + const BERRY_PKG: &str = r#"{ + "name": "berry-fixture", + "version": "1.0.0", + "private": true, + "dependencies": { + "left-pad": "1.3.0" + } +} +"#; + let berry_lock: String = format!( + "# This file is generated by running \"yarn install\" inside your project.\n\ + # Manual changes might be lost - proceed with caution!\n\n\ + __metadata:\n version: 8\n cacheKey: 10c0\n\n\ + \"left-pad@npm:1.3.0\":\n version: 1.3.0\n \ + resolution: \"left-pad@npm:1.3.0\"\n checksum: 10c0/{}\n \ + languageName: node\n linkType: hard\n\n\ + \"berry-fixture@workspace:.\":\n version: 0.0.0-use.local\n \ + resolution: \"berry-fixture@workspace:.\"\n dependencies:\n \ + left-pad: \"npm:1.3.0\"\n languageName: unknown\n linkType: soft\n", + "3".repeat(128) + ); + + // Berry project fixture: package.json + berry yarn.lock + .yarnrc.yml + + // installed copy + offline manifest/blob (same staging as npm_fixture). + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + let pkg = root.join("node_modules/left-pad"); + std::fs::create_dir_all(&pkg).unwrap(); + std::fs::write( + pkg.join("package.json"), + br#"{"name":"left-pad","version":"1.3.0"}"#, + ) + .unwrap(); + std::fs::write(pkg.join("index.js"), ORIG_INDEX).unwrap(); + std::fs::write(root.join("package.json"), BERRY_PKG).unwrap(); + std::fs::write(root.join("yarn.lock"), &berry_lock).unwrap(); + std::fs::write( + root.join(".yarnrc.yml"), + "nodeLinker: node-modules\nenableGlobalCache: true\n", + ) + .unwrap(); + let before_hash = compute_git_sha256_from_bytes(ORIG_INDEX); + let after_hash = compute_git_sha256_from_bytes(PATCHED_INDEX); + let manifest = json!({ "patches": { PURL: patch_record(&before_hash, &after_hash) } }); + std::fs::create_dir_all(root.join(".socket/blobs")).unwrap(); + std::fs::write( + root.join(".socket/manifest.json"), + serde_json::to_vec_pretty(&manifest).unwrap(), + ) + .unwrap(); + std::fs::write(root.join(".socket/blobs").join(&after_hash), PATCHED_INDEX).unwrap(); + + // First vendor (uuid A): the ledger's lock record carries the verbatim + // pre-vendor registry entry. + assert_eq!(vendor_run(vendor_args(root)).await, 0, "first vendor"); + let state_path = root.join(".socket/vendor/state.json"); + let lock_wiring = |state: &Value| -> Value { + state["entries"][PURL]["wiring"] + .as_array() + .expect("wiring array") + .iter() + .find(|w| w["kind"] == "yarn_berry_lock_entry") + .expect("berry lock wiring record") + .clone() + }; + let state: Value = serde_json::from_slice(&std::fs::read(&state_path).unwrap()).unwrap(); + let registry_original = lock_wiring(&state)["original"].clone(); + assert_eq!( + registry_original[0], "\"left-pad@npm:1.3.0\":", + "first vendor records the registry entry: {registry_original:#}" + ); + + // The manifest record moves to a newer patch uuid. + let mut manifest: Value = + serde_json::from_slice(&std::fs::read(root.join(".socket/manifest.json")).unwrap()) + .unwrap(); + manifest["patches"][PURL]["uuid"] = json!(UUID2); + std::fs::write( + root.join(".socket/manifest.json"), + serde_json::to_vec_pretty(&manifest).unwrap(), + ) + .unwrap(); + + let (code, env) = vendor_cli(root, &[]); + assert_eq!(code, 0, "re-vendor must succeed: {env:#}"); + let state: Value = serde_json::from_slice(&std::fs::read(&state_path).unwrap()).unwrap(); + assert_eq!(state["entries"][PURL]["uuid"], UUID2); + assert!( + !root.join(format!(".socket/vendor/npm/{UUID}")).exists(), + "old uuid dir swept" + ); + // THE regression: the re-vendored lock record (whose key now embeds + // uuid B) must have regained the registry original recorded under the + // uuid-A key. + assert_eq!( + lock_wiring(&state)["original"], + registry_original, + "the pre-vendor registry fragment must survive the uuid change: {state:#}" + ); + + // And revert must restore BOTH files byte-for-byte — pre-fix it leaves + // a `file:` lock entry pointing at the deleted uuid-B artifact. + let (code, env) = vendor_cli(root, &["--revert"]); + assert_eq!(code, 0, "revert after re-vendor: {env:#}"); + assert_eq!( + std::fs::read_to_string(root.join("yarn.lock")).unwrap(), + berry_lock, + "yarn.lock restored to the pristine registry entry byte-for-byte" + ); + assert_eq!( + std::fs::read_to_string(root.join("package.json")).unwrap(), + BERRY_PKG, + "package.json restored (resolutions table dropped)" + ); + assert!(!root.join(".socket/vendor").exists(), "vendor tree pruned"); +} + // ───────────────────────────────────────────────────────────────────── // 9. offline with no local source // ───────────────────────────────────────────────────────────────────── diff --git a/crates/socket-patch-core/src/vendor/npm_common.rs b/crates/socket-patch-core/src/vendor/npm_common.rs index 6235ec59..f8e9a8b9 100644 --- a/crates/socket-patch-core/src/vendor/npm_common.rs +++ b/crates/socket-patch-core/src/vendor/npm_common.rs @@ -575,6 +575,34 @@ pub(super) fn done_failure(purl: &str, error: String) -> VendorOutcome { done(failed_result(purl, Path::new(""), error), None, Vec::new()) } +/// [`done_failure`] for a wiring failure AFTER the shared pipeline packed +/// the artifact into `/.socket/vendor///`: unless the +/// uuid dir already existed before this run (a same-uuid re-vendor may still +/// be referenced by live wiring), best-effort remove it — no ledger entry is +/// ever persisted for a failed wiring, so `--revert` could never clean it up +/// and the module contract ("a failure leaves the project byte-untouched") +/// would be broken by an orphaned, possibly defective artifact dir. Empty +/// parent dirs are pruned non-recursively (a sibling artifact keeps them). +pub(super) async fn done_failure_unstage( + purl: &str, + error: String, + project_root: &Path, + uuid_dir_rel: &str, + uuid_dir_preexisted: bool, +) -> VendorOutcome { + if !uuid_dir_preexisted { + let uuid_dir = project_root.join(uuid_dir_rel); + let _ = remove_tree(&uuid_dir).await; + if let Some(eco_dir) = uuid_dir.parent() { + let _ = tokio::fs::remove_dir(eco_dir).await; + if let Some(vendor_dir) = eco_dir.parent() { + let _ = tokio::fs::remove_dir(vendor_dir).await; + } + } + } + done_failure(purl, error) +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/socket-patch-core/src/vendor/path.rs b/crates/socket-patch-core/src/vendor/path.rs index bdb4ded1..d5402ead 100644 --- a/crates/socket-patch-core/src/vendor/path.rs +++ b/crates/socket-patch-core/src/vendor/path.rs @@ -113,6 +113,55 @@ pub fn parse_vendor_path(s: &str) -> Option { }) } +/// Do two wiring-record keys name the same lockfile fragment across a patch +/// update? Byte-equal keys always match. Keys that embed a +/// `.socket/vendor///` path (yarn berry's `file:` locator lock +/// key) additionally match when they are equal with every uuid level +/// normalized away — updating a patch changes the uuid, which changes the +/// embedded path, but the record still names the same lock entry. The +/// re-vendor carry-forward relies on this to keep the pre-vendor `original` +/// across patch updates; keys without a recognizable vendored path only ever +/// match byte-equal. +pub fn wiring_key_matches(a: &str, b: &str) -> bool { + if a == b { + return true; + } + match (normalize_vendor_uuids(a), normalize_vendor_uuids(b)) { + (Some(na), Some(nb)) => na == nb, + _ => false, + } +} + +/// Replace every embedded `.socket/vendor//` uuid level with `*`; +/// `None` when the string embeds no recognizable vendored path. +fn normalize_vendor_uuids(s: &str) -> Option { + let anchor = format!("{VENDOR_DIR}/"); + let mut out = String::new(); + let mut rest = s; + let mut replaced = false; + while let Some(idx) = rest.find(&anchor) { + let head_end = idx + anchor.len(); + out.push_str(&rest[..head_end]); + rest = &rest[head_end..]; + let Some((eco, tail)) = rest.split_once('/') else { + break; + }; + if !ECOSYSTEM_DIRS.contains(&eco) { + continue; + } + let uuid_end = tail.find('/').unwrap_or(tail.len()); + if !is_canonical_uuid(&tail[..uuid_end]) { + continue; + } + out.push_str(eco); + out.push_str("/*"); + rest = &tail[uuid_end..]; + replaced = true; + } + out.push_str(rest); + replaced.then_some(out) +} + /// Split a `-` leaf at the version boundary: the version is /// the suffix after the LAST `-` that is immediately followed by a digit /// (versions always start with a digit; names may contain digit-bearing @@ -448,6 +497,42 @@ mod tests { assert!(parse_vendor_path(&format!("x.socket/vendor/npm/{UUID}/y.tgz")).is_none()); } + /// The re-vendor carry-forward matches wiring keys ACROSS a patch-uuid + /// change when the key embeds the vendored path (berry's `file:` locator + /// key), and only byte-equal otherwise. + #[test] + fn wiring_key_matching_is_uuid_agnostic_for_vendored_paths() { + const UUID2: &str = "0a1b2c3d-4e5f-6a7b-8c9d-0e1f2a3b4c5d"; + let berry_key = |uuid: &str| { + format!( + "\"left-pad@file:./.socket/vendor/npm/{uuid}/left-pad-1.3.0.tgz\ + ::locator=vendor-spike%40workspace%3A.\"" + ) + }; + // Same key always matches; uuid-only difference matches too. + assert!(wiring_key_matches(&berry_key(UUID), &berry_key(UUID))); + assert!(wiring_key_matches(&berry_key(UUID), &berry_key(UUID2))); + // A different package leaf (other name/version) never matches. + assert!(!wiring_key_matches( + &berry_key(UUID), + &format!( + "\"is-odd@file:./.socket/vendor/npm/{UUID2}/is-odd-1.0.0.tgz\ + ::locator=vendor-spike%40workspace%3A.\"" + ) + )); + // Keys with no vendored path only match byte-equal (classic block + // keys, npm lock paths, bare names). + assert!(wiring_key_matches("left-pad@^1.3.0", "left-pad@^1.3.0")); + assert!(!wiring_key_matches("left-pad@^1.3.0", "left-pad@~1.3.0")); + // A vendored-path key never matches a non-vendored one. + assert!(!wiring_key_matches(&berry_key(UUID), "left-pad@^1.3.0")); + // Non-canonical uuid levels are not normalized (fail-closed). + assert!(!wiring_key_matches( + ".socket/vendor/npm/not-a-uuid/x.tgz", + &format!(".socket/vendor/npm/{UUID}/x.tgz") + )); + } + #[test] fn leaf_round_trips() { // npm, incl. scoped and digit-bearing names + prerelease versions. diff --git a/crates/socket-patch-core/src/vendor/yarn_berry_lock.rs b/crates/socket-patch-core/src/vendor/yarn_berry_lock.rs index e64a66a3..d0170327 100644 --- a/crates/socket-patch-core/src/vendor/yarn_berry_lock.rs +++ b/crates/socket-patch-core/src/vendor/yarn_berry_lock.rs @@ -42,15 +42,15 @@ use crate::utils::uri::encode_uri_component; use super::berry_zip::berry_cache_checksum_10c0; use super::common::{already_patched_result, detect_eol, detect_indent, refused, serialize_json}; use super::npm_common::{ - done_failure, guard_coordinates, guard_revert_uuid_dir, stage_patch_pack, tgz_rel_leaf, + done_failure_unstage, guard_coordinates, guard_revert_uuid_dir, stage_patch_pack, tgz_rel_leaf, }; use super::path::parse_vendor_path; use super::state::{ write_marker, VendorArtifact, VendorEntry, VendorMarker, WiringAction, WiringRecord, }; use super::yarn_classic_lock::{ - body_field_line, lines_to_json, read_yarn_lock, replace_block, revert_recorded_block, - scan_blocks, split_key_patterns, split_pattern, LockBlock, + body_field_line, lines_to_json, pattern_real_name, read_yarn_lock, replace_block, + revert_recorded_block, scan_blocks, split_key_patterns, split_pattern, LockBlock, }; use super::{RevertOutcome, VendorOutcome, VendorWarning}; @@ -229,9 +229,39 @@ pub async fn vendor_yarn_berry( } // ── 6. The single replaceable lock entry ────────────────────────────── - let (target, target_is_ours) = match scan_berry_target(&blocks, name, version) { - Ok(Some((idx, is_ours))) => (&blocks[idx], is_ours), - Ok(None) => { + let scan = match scan_berry_target(&blocks, name, version) { + Ok(scan) => scan, + Err((code, detail)) => return refused(code, detail), + }; + // An `alias@npm:@…` descriptor consumes the patched package under + // a different ident; the bare-name resolutions entry vendoring writes + // can never move it, so that copy keeps installing the UNPATCHED bytes. + // Surface every such entry loudly instead of silently part-patching. + for key in &scan.alias_keys { + warnings.push(VendorWarning::new( + "vendor_alias_entry_skipped", + format!( + "{YARN_LOCK} entry `{key}` consumes {name}@{version} through an npm: alias; \ + the bare-name resolutions entry vendoring writes cannot move aliased \ + descriptors, so that copy keeps installing the UNPATCHED registry bytes" + ), + )); + } + let (target, target_is_ours) = match scan.target { + Some((idx, is_ours)) => (&blocks[idx], is_ours), + None => { + if !scan.alias_keys.is_empty() { + return refused( + "vendor_lock_entry_not_found", + format!( + "{YARN_LOCK} resolves {name}@{version} only through npm: alias \ + descriptors ({}); berry resolutions are name-keyed and cannot \ + reach aliased descriptors, so vendoring cannot rewire this \ + project's copy", + scan.alias_keys.join(", ") + ), + ); + } return refused( "vendor_lock_entry_not_found", format!( @@ -240,7 +270,6 @@ pub async fn vendor_yarn_berry( ), ); } - Err((code, detail)) => return refused(code, detail), }; let patches_manifest = record .files @@ -248,6 +277,12 @@ pub async fn vendor_yarn_berry( .any(|k| normalize_file_path(k) == "package.json"); // ── 7. Stage → patch → pack (shared flavor-agnostic pipeline) ───────── + // A wiring failure past this point must unwind the uuid dir staging is + // about to create — but never one that already existed (a same-uuid + // re-vendor's dir may still be referenced by live wiring). + let uuid_dir_preexisted = tokio::fs::metadata(project_root.join(&uuid_dir_rel)) + .await + .is_ok(); let (staged, result) = match stage_patch_pack( purl, installed_dir, @@ -279,7 +314,16 @@ pub async fn vendor_yarn_berry( // ── 8. Berry identity facts of the packed tarball ───────────────────── let tgz_bytes = match tokio::fs::read(&dest).await { Ok(b) => b, - Err(e) => return done_failure(purl, format!("cannot re-read the packed tarball: {e}")), + Err(e) => { + return done_failure_unstage( + purl, + format!("cannot re-read the packed tarball: {e}"), + project_root, + &uuid_dir_rel, + uuid_dir_preexisted, + ) + .await + } }; let tgz_sha512 = hex::encode(Sha512::digest(&tgz_bytes)); // `hash=` — the first 6 hex chars of sha512(tgz): the lock-committed @@ -288,10 +332,14 @@ pub async fn vendor_yarn_berry( let checksum = match berry_cache_checksum_10c0(&tgz_bytes, name) { Ok(c) => c, Err(e) => { - return done_failure( + return done_failure_unstage( purl, format!("cannot compute the berry cache checksum for {name}: {e}"), + project_root, + &uuid_dir_rel, + uuid_dir_preexisted, ) + .await } }; @@ -345,14 +393,30 @@ pub async fn vendor_yarn_berry( .entry("resolutions".to_string()) .or_insert_with(|| Value::Object(serde_json::Map::new())); let Some(res_obj) = res.as_object_mut() else { - return done_failure(purl, "resolutions table vanished mid-edit".to_string()); + return done_failure_unstage( + purl, + "resolutions table vanished mid-edit".to_string(), + project_root, + &uuid_dir_rel, + uuid_dir_preexisted, + ) + .await; }; res_obj.insert(name.to_string(), Value::String(spec.clone())); } let pkg_indent = detect_indent(&String::from_utf8_lossy(&pkg_bytes)); let new_pkg_bytes = match serialize_json(&new_pkg, &pkg_indent) { Ok(b) => b, - Err(e) => return done_failure(purl, format!("cannot serialize {PACKAGE_JSON}: {e}")), + Err(e) => { + return done_failure_unstage( + purl, + format!("cannot serialize {PACKAGE_JSON}: {e}"), + project_root, + &uuid_dir_rel, + uuid_dir_preexisted, + ) + .await + } }; let new_lock_text = replace_block(&lock_text, target, &new_lines, detect_eol(&lock_text)); if let Err(e) = commit_pair( @@ -363,7 +427,8 @@ pub async fn vendor_yarn_berry( ) .await { - return done_failure(purl, e); + return done_failure_unstage(purl, e, project_root, &uuid_dir_rel, uuid_dir_preexisted) + .await; } // ── 12. Marker + ledger entry ───────────────────────────────────────── @@ -667,17 +732,31 @@ async fn commit_pair( Ok(()) } -/// Find the one replaceable entry for `name@version` — `(index into blocks, -/// is_ours)`, where `is_ours` means the entry is already one of our `file:` -/// entries (stale uuid or current) — refusing fail-closed on anything a -/// bare-name resolutions entry would also move (other versions of the name, -/// non-npm protocols, ambiguous duplicates). +/// The result of [`scan_berry_target`]: the one replaceable entry (when +/// present) plus every alias-descriptor entry a bare-name resolutions entry +/// cannot reach. +struct BerryTargetScan { + /// `(index into blocks, is_ours)`, where `is_ours` means the entry is + /// already one of our `file:` entries (stale uuid or current). + target: Option<(usize, bool)>, + /// Lock keys of `alias@npm:@…` entries resolving the patched + /// version — semantically out of reach for a name-keyed resolutions + /// entry, so the caller must surface them instead of silently skipping. + alias_keys: Vec, +} + +/// Find the one replaceable entry for `name@version` — refusing fail-closed +/// on anything a bare-name resolutions entry would also move (other versions +/// of the name, non-npm protocols, ambiguous duplicates) — and collect the +/// npm-alias descriptor entries of the same package that vendoring can never +/// rewire. fn scan_berry_target( blocks: &[LockBlock], name: &str, version: &str, -) -> Result, (&'static str, String)> { +) -> Result { let mut found: Vec<(usize, bool)> = Vec::new(); + let mut alias_keys: Vec = Vec::new(); for (idx, block) in blocks.iter().enumerate() { if block.key == "__metadata" { continue; @@ -688,6 +767,13 @@ fn scan_berry_target( continue; // not a descriptor key we understand; not ours to touch } if !parsed.iter().any(|(n, _)| *n == name) { + // `alias@npm:@…` descriptors carry the real name inside + // the range; a name-keyed resolutions entry cannot move them. + if berry_field(&block.lines, "version") == Some(version) + && patterns.iter().any(|p| pattern_real_name(p) == Some(name)) + { + alias_keys.push(block.key.clone()); + } continue; } if !parsed.iter().all(|(n, _)| *n == name) { @@ -735,17 +821,19 @@ fn scan_berry_target( )); } } - match found.len() { - 0 => Ok(None), - 1 => Ok(found.into_iter().next()), - _ => Err(( + if found.len() > 1 { + return Err(( "vendor_override_conflict", format!( "multiple yarn.lock entries resolve {name}@{version}; refusing the \ ambiguous rewrite" ), - )), + )); } + Ok(BerryTargetScan { + target: found.into_iter().next(), + alias_keys, + }) } /// Body sections of a lock entry that are NOT the five scalar fields we own @@ -1304,6 +1392,97 @@ __metadata: fx.assert_untouched().await; } + /// An `alias@npm:left-pad@…` descriptor consumes the patched package + /// under a different ident; the name-keyed resolutions entry can never + /// move it, so vendoring must warn loudly about the unpatched copy + /// instead of silently part-patching. + #[tokio::test] + async fn alias_descriptor_entry_warns_and_stays_untouched() { + const ALIAS_BLOCK: &str = "\"safe-pad@npm:left-pad@1.3.0\":\n version: 1.3.0\n resolution: \"left-pad@npm:1.3.0\"\n checksum: 10c0/aa\n languageName: node\n linkType: hard\n"; + let lock = format!("{B3_BEFORE_LOCK}\n{ALIAS_BLOCK}"); + let fx = fixture_with(B3_BEFORE_PKG, &lock).await; + + let (result, entry, warnings) = expect_done(fx.vendor(false).await); + assert!(result.success, "{:?}", result.error); + assert!(entry.is_some(), "the plain entry still vendors"); + let warning = warnings + .iter() + .find(|w| w.code == "vendor_alias_entry_skipped") + .unwrap_or_else(|| panic!("expected the alias skip warning: {warnings:?}")); + assert!( + warning.detail.contains("safe-pad@npm:left-pad@1.3.0"), + "names the alias entry: {}", + warning.detail + ); + + let text = tokio::fs::read_to_string(fx.lock_path()).await.unwrap(); + assert!( + text.contains("left-pad@file:./"), + "plain entry rewired: {text}" + ); + assert!( + text.contains(ALIAS_BLOCK), + "alias entry byte-untouched: {text}" + ); + + // An alias of ANOTHER version is out of the patch's scope: no noise. + let other = ALIAS_BLOCK.replace("1.3.0", "1.2.0"); + let lock = format!("{B3_BEFORE_LOCK}\n{other}"); + let fx = fixture_with(B3_BEFORE_PKG, &lock).await; + let (result, _, warnings) = expect_done(fx.vendor(false).await); + assert!(result.success, "{:?}", result.error); + assert!( + !warnings + .iter() + .any(|w| w.code == "vendor_alias_entry_skipped"), + "{warnings:?}" + ); + } + + /// When the ONLY entry for the patched version is an alias descriptor, + /// the refusal must say so — the generic "make sure the package is + /// installed" detail would send the user to a `yarn install` that + /// changes nothing. + #[tokio::test] + async fn alias_only_lock_refuses_with_alias_detail() { + let lock = B3_BEFORE_LOCK.replace( + "\"left-pad@npm:1.3.0\":", + "\"safe-pad@npm:left-pad@1.3.0\":", + ); + let fx = fixture_with(B3_BEFORE_PKG, &lock).await; + let detail = expect_refused(fx.vendor(false).await, "vendor_lock_entry_not_found"); + assert!(detail.contains("alias"), "{detail}"); + assert!(detail.contains("safe-pad@npm:left-pad@1.3.0"), "{detail}"); + fx.assert_untouched().await; + } + + /// A wiring failure AFTER the tarball is packed (here: the fail-closed + /// berry cache checksum refusing a non-ASCII filename) must unwind the + /// freshly created uuid dir — no ledger entry exists for it, so + /// `--revert` could never clean it up and the user would commit an + /// unwired artifact. + #[tokio::test] + async fn post_pack_wiring_failure_unwinds_the_staged_artifact() { + let fx = fixture().await; + tokio::fs::write(fx.installed().join("café.js"), b"x") + .await + .unwrap(); + + let (result, entry, _) = expect_done(fx.vendor(false).await); + assert!(!result.success, "non-ASCII filename must fail the wiring"); + assert!( + result + .error + .as_deref() + .unwrap_or("") + .contains("berry cache checksum"), + "{:?}", + result.error + ); + assert!(entry.is_none()); + fx.assert_untouched().await; + } + #[tokio::test] async fn rerun_is_in_sync_and_byte_stable() { let fx = fixture().await; diff --git a/crates/socket-patch-core/src/vendor/yarn_classic_lock.rs b/crates/socket-patch-core/src/vendor/yarn_classic_lock.rs index bad8d269..9f3fd62a 100644 --- a/crates/socket-patch-core/src/vendor/yarn_classic_lock.rs +++ b/crates/socket-patch-core/src/vendor/yarn_classic_lock.rs @@ -31,7 +31,9 @@ use crate::patch::copy_tree::remove_tree; use crate::utils::fs::atomic_write_bytes_preserving_mode; use super::common::{already_patched_result, detect_eol, refused}; -use super::npm_common::{done_failure, guard_coordinates, guard_revert_uuid_dir, stage_patch_pack}; +use super::npm_common::{ + done_failure_unstage, guard_coordinates, guard_revert_uuid_dir, stage_patch_pack, +}; use super::path::parse_vendor_path; use super::state::{ write_marker, VendorArtifact, VendorEntry, VendorMarker, WiringAction, WiringRecord, @@ -113,6 +115,12 @@ pub async fn vendor_yarn_classic( } // ── 4–7. Stage → patch → pack (shared flavor-agnostic pipeline) ─────── + // A wiring failure past this point must unwind the uuid dir staging is + // about to create — but never one that already existed (a same-uuid + // re-vendor's dir may still be referenced by live wiring). + let uuid_dir_preexisted = tokio::fs::metadata(project_root.join(&uuid_dir_rel)) + .await + .is_ok(); let (staged, result) = match stage_patch_pack( purl, installed_dir, @@ -153,7 +161,14 @@ pub async fn vendor_yarn_classic( let edit = { let blocks = scan_blocks(&new_text); let Some(block) = blocks.iter().find(|b| &b.key == key) else { - return done_failure(purl, format!("lock block `{key}` vanished mid-rewrite")); + return done_failure_unstage( + purl, + format!("lock block `{key}` vanished mid-rewrite"), + project_root, + &uuid_dir_rel, + uuid_dir_preexisted, + ) + .await; }; let new_lines = rewrite_classic_block( &block.lines, @@ -213,7 +228,14 @@ pub async fn vendor_yarn_classic( } if let Err(e) = atomic_write_bytes_preserving_mode(&lock_path, new_text.as_bytes()).await { - return done_failure(purl, format!("cannot write {YARN_LOCK}: {e}")); + return done_failure_unstage( + purl, + format!("cannot write {YARN_LOCK}: {e}"), + project_root, + &uuid_dir_rel, + uuid_dir_preexisted, + ) + .await; } // ── 9. Marker + ledger entry ────────────────────────────────────────── @@ -1267,6 +1289,54 @@ left-pad@^1.3.0: ); } + /// A lock-write failure AFTER the tarball is packed must unwind the + /// freshly created uuid dir — no ledger entry exists for it, so + /// `--revert` could never clean it up and the user would commit an + /// unwired artifact (contract: a failure leaves the project + /// byte-untouched). + #[cfg(unix)] + #[tokio::test] + async fn lock_write_failure_unwinds_the_staged_artifact() { + use std::os::unix::fs::PermissionsExt; + let fx = fixture_with_lock(Y2_BEFORE).await; + // Pre-create the eco level so staging only creates the uuid dir. + tokio::fs::create_dir_all(fx.root().join(".socket/vendor/npm")) + .await + .unwrap(); + // A read-only project root: yarn.lock still reads and the tarball + // still packs (into the writable .socket/ subtree), but the atomic + // lock write (temp file in the root) fails. + let orig_mode = tokio::fs::metadata(fx.root()).await.unwrap().permissions(); + tokio::fs::set_permissions(fx.root(), std::fs::Permissions::from_mode(0o555)) + .await + .unwrap(); + let outcome = fx.vendor(false).await; + tokio::fs::set_permissions(fx.root(), orig_mode) + .await + .unwrap(); + + let (result, entry, _) = expect_done(outcome); + assert!(!result.success, "the lock write must fail"); + assert!( + result + .error + .as_deref() + .unwrap_or("") + .contains("cannot write yarn.lock"), + "{:?}", + result.error + ); + assert!(entry.is_none()); + assert!( + !fx.root().join(".socket/vendor").exists(), + "the staged uuid dir (and its empty parents) must be unwound" + ); + assert_eq!( + tokio::fs::read(fx.lock_path()).await.unwrap(), + fx.lock_bytes + ); + } + #[tokio::test] async fn berry_lock_and_missing_lock_are_refused() { let fx = fixture_with_lock("__metadata:\n version: 8\n cacheKey: 10c0\n").await;