From ad40d1dbe71f9e840215334d2e129734975ac647 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Thu, 5 Nov 2026 13:23:55 -0600 Subject: [PATCH 1/5] Plan pnpm rewires with one core for both modes Add formats/pnpm/plan.rs, the planning core hosted and vendored mode share. They now differ only in where a rewired entry points: * Target::check_lock: one lock-version table. Hosted takes every lock but pnpm 1's early shrinkwrap. Vendored takes 5.4 / 6.0 / 9.0 and routes each dialect from it. This replaces unsupported_early_shrinkwrap, check_v9_lock_version and the legacy check_lock_version. * match_key: one lookup of how a key names name@version, used by the hosted lookup, the hosted "vendored?" hint, and both vendored dialects' target, split-entry and rekey checks. It finds the registry entry with its peer suffix, or a vendored rekey read with origin::vendor_ref plus the artifact leaf. * Target::repoints: the all-or-nothing gate's per-instance predicate. Hosted splices any parsable peer context; vendored rekeys the plain entry only. Within the vendored planner, the v9 and legacy dialects now share the dep-pair revert (DepPair), the block revert (revert_block, re-sorting legacy's bare file: rekey), target_blocks, and dep_entries. dep_entries also serves the importer edit, the 6.0 root-dep edit, the ref gate and the lock index. The ref gate's index and scan paths run one set of predicates. Behavior changes, each pinned by a test: * N15: the vendored planner and reverter edit a CRLF pnpm-lock.yaml or pnpm-workspace.yaml like its LF twin and write it back CRLF. Only a file mixing CRLF and LF is still refused (vendor_lockfile_crlf_unsupported). * N5 (vendored planner): a sibling checkout's ../.socket/vendor rekey is no longer taken for this project's entry. * The hosted "vendored" hint now judges a rekey by its artifact leaf, as the vendored planner does, instead of the entry's version: line. N16 is kept as a target-specific refusal: no lock version beyond 5.4 / 6.0 / 9.0 has a captured fixture that proves the vendored rekey. Co-Authored-By: Claude Opus 5.5 --- .../tests/in_process_vendor_pnpm_takeover.rs | 59 +- .../src/formats/pnpm/grammar.rs | 32 - .../src/formats/pnpm/hosted.rs | 86 +- .../socket-patch-core/src/formats/pnpm/mod.rs | 86 +- .../src/formats/pnpm/plan.rs | 201 +++ .../src/patch/redirect/mod.rs | 22 + crates/socket-patch-core/src/vendor/mod.rs | 15 +- .../src/vendor/npm_common.rs | 2 +- .../socket-patch-core/src/vendor/pnpm_lock.rs | 1145 +++++++++-------- .../src/vendor/pnpm_lock_legacy.rs | 366 ++---- 10 files changed, 1007 insertions(+), 1007 deletions(-) create mode 100644 crates/socket-patch-core/src/formats/pnpm/plan.rs diff --git a/crates/socket-patch-cli/tests/in_process_vendor_pnpm_takeover.rs b/crates/socket-patch-cli/tests/in_process_vendor_pnpm_takeover.rs index 7d4bcc49d..60ff577bd 100644 --- a/crates/socket-patch-cli/tests/in_process_vendor_pnpm_takeover.rs +++ b/crates/socket-patch-cli/tests/in_process_vendor_pnpm_takeover.rs @@ -1,7 +1,7 @@ //! Hermetic hosted → vendored takeover tests through the built binary for //! pnpm projects whose shape the pnpm vendored backend refuses although //! hosted mode accepts it (#853): a `catalog:` dependency -//! (`vendor_lock_entry_unsupported`), a CRLF `pnpm-lock.yaml` +//! (`vendor_lock_entry_unsupported`), a `pnpm-lock.yaml` mixing CRLF and LF //! (`vendor_lockfile_crlf_unsupported`) and a conflicting user override //! (a range) in `pnpm-workspace.yaml` (`vendor_override_conflict`). //! @@ -54,6 +54,8 @@ enum Shape { Catalog, /// A plain dependency whose lock is converted to CRLF after hosting. Crlf, + /// A plain dependency whose lock mixes CRLF and LF after hosting. + MixedEol, /// A plain dependency plus a conflicting user /// `overrides: { left-pad: ^1.3.0 }` in pnpm-workspace.yaml. Override, @@ -90,7 +92,7 @@ fn write_pnpm_project(root: &Path, shape: Shape) { Shape::Catalog => format!("packages:\n - .\ncatalog:\n {NAME}: {VERSION}\n"), Shape::Override => format!("overrides:\n {NAME}: ^{VERSION}\n"), Shape::ExactPin => format!("overrides:\n {NAME}: {VERSION}\n"), - Shape::Crlf | Shape::Plain | Shape::TwoDocument => String::new(), + Shape::Crlf | Shape::MixedEol | Shape::Plain | Shape::TwoDocument => String::new(), }; if !workspace.is_empty() { std::fs::write(root.join("pnpm-workspace.yaml"), workspace).unwrap(); @@ -117,7 +119,7 @@ fn write_pnpm_project(root: &Path, shape: Shape) { )), Shape::Override => lock.push_str(&format!("overrides:\n {NAME}: ^{VERSION}\n\n")), Shape::ExactPin => lock.push_str(&format!("overrides:\n {NAME}: {VERSION}\n\n")), - Shape::Crlf | Shape::Plain | Shape::TwoDocument => {} + Shape::Crlf | Shape::MixedEol | Shape::Plain | Shape::TwoDocument => {} } let specifier = match shape { Shape::Catalog => "'catalog:'".to_string(), @@ -305,7 +307,8 @@ fn snapshot(root: &Path) -> Vec<(&'static str, Option>)> { /// The hosted project: pristine lock, then a real `scan --mode hosted` /// (and, for [`Shape::Crlf`], the lock converted to CRLF as a -/// `core.autocrlf` checkout would leave it). Returns the hosted wiring. +/// `core.autocrlf` checkout would leave it; for [`Shape::MixedEol`], its +/// first line only). Returns the hosted wiring. fn host_project(root: &Path, api: &str, shape: Shape) -> Vec<(&'static str, Option>)> { write_pnpm_project(root, shape); let (code, env) = run_mode(root, api, "scan", "hosted", &[]); @@ -316,8 +319,10 @@ fn host_project(root: &Path, api: &str, shape: Shape) -> Vec<(&'static str, Opti lock.contains(HOSTED_URL), "hosted mode must pin the {shape:?} lock entry:\n{lock}\n{env:#}" ); - if matches!(shape, Shape::Crlf) { - std::fs::write(&lock_path, lock.replace('\n', "\r\n")).unwrap(); + match shape { + Shape::Crlf => std::fs::write(&lock_path, lock.replace('\n', "\r\n")).unwrap(), + Shape::MixedEol => std::fs::write(&lock_path, lock.replacen('\n', "\r\n", 1)).unwrap(), + _ => {} } snapshot(root) } @@ -402,17 +407,21 @@ async fn get_vendored_over_hosted_pnpm_catalog_dep_keeps_the_hosted_pin() { refused_takeover_keeps_hosted_pin(Shape::Catalog, "get", "vendor_lock_entry_unsupported").await; } -/// #853: a CRLF `pnpm-lock.yaml` (a `core.autocrlf` checkout). +/// #853: a `pnpm-lock.yaml` mixing CRLF and LF. (A CRLF lock, a +/// `core.autocrlf` checkout, takes over since lock-model 2b: see +/// [`scan_vendored_over_hosted_pnpm_crlf_lock_takes_over_and_stays_crlf`].) #[tokio::test(flavor = "multi_thread")] -async fn scan_vendored_over_hosted_pnpm_crlf_lock_keeps_the_hosted_pin() { - refused_takeover_keeps_hosted_pin(Shape::Crlf, "scan", "vendor_lockfile_crlf_unsupported") +async fn scan_vendored_over_hosted_pnpm_mixed_eol_lock_keeps_the_hosted_pin() { + refused_takeover_keeps_hosted_pin(Shape::MixedEol, "scan", "vendor_lockfile_crlf_unsupported") .await; } -/// #853: `get --mode vendored` over a CRLF lock (a project-level refusal). +/// #853: `get --mode vendored` over a mixed-ending lock (a project-level +/// refusal). #[tokio::test(flavor = "multi_thread")] -async fn get_vendored_over_hosted_pnpm_crlf_lock_keeps_the_hosted_pin() { - refused_takeover_keeps_hosted_pin(Shape::Crlf, "get", "vendor_lockfile_crlf_unsupported").await; +async fn get_vendored_over_hosted_pnpm_mixed_eol_lock_keeps_the_hosted_pin() { + refused_takeover_keeps_hosted_pin(Shape::MixedEol, "get", "vendor_lockfile_crlf_unsupported") + .await; } /// #853: a conflicting user override (a range) in `pnpm-workspace.yaml`. @@ -522,6 +531,32 @@ async fn scan_vendored_over_hosted_pnpm_plain_dep_still_takes_over() { ); } +/// Lock-model 2b (divergence N15): a CRLF `pnpm-lock.yaml` (a +/// `core.autocrlf` checkout) takes over like its LF twin, and the vendored +/// lock keeps its CRLF endings. It used to be refused +/// (`vendor_lockfile_crlf_unsupported`) and keep the hosted pin. +#[tokio::test(flavor = "multi_thread")] +async fn scan_vendored_over_hosted_pnpm_crlf_lock_takes_over_and_stays_crlf() { + let server = MockServer::start().await; + mock_api(&server).await; + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + host_project(root, &server.uri(), Shape::Crlf); + + let (exit, env) = run_mode(root, &server.uri(), "scan", "vendored", &[]); + assert_eq!(exit, 0, "the CRLF takeover must succeed: {env:#}"); + let lock = std::fs::read_to_string(root.join("pnpm-lock.yaml")).unwrap(); + assert!(!lock.contains(HOSTED_URL), "{lock}"); + assert!( + lock.contains(&format!(".socket/vendor/npm/{UUID}/")), + "the lock must point at the vendored artifact:\n{lock}" + ); + assert!( + !lock.replace("\r\n", "").contains('\n'), + "the lock keeps its CRLF endings: {lock:?}" + ); +} + /// #854: a user exact pin equal to the vendored version in /// `pnpm-workspace.yaml` (the pnpm 10.5+/11/12 override map) is taken over /// like the same pin in package.json: the workspace value becomes the diff --git a/crates/socket-patch-core/src/formats/pnpm/grammar.rs b/crates/socket-patch-core/src/formats/pnpm/grammar.rs index 3ba09bf18..9f7ce6e4e 100644 --- a/crates/socket-patch-core/src/formats/pnpm/grammar.rs +++ b/crates/socket-patch-core/src/formats/pnpm/grammar.rs @@ -13,21 +13,6 @@ use crate::formats::text::strip_bom; use super::lines::ENV_DOC_IMPORTER_FIELDS; -/// Early pnpm 1 writes shrinkwrapVersion 3 without a minor version and -/// unconditionally drops registry tarball URLs on install. Its frozen flag -/// cannot preserve this redirect (verified with pnpm 1.0.0). -pub(crate) fn unsupported_early_shrinkwrap(content: &str) -> bool { - let content = strip_bom(content); - let version = content - .lines() - .find_map(|line| line.strip_prefix("shrinkwrapVersion:")); - let minor = content - .lines() - .find_map(|line| line.strip_prefix("shrinkwrapMinorVersion:")); - version.is_some_and(|v| v.trim().trim_matches(['\'', '"']) == "3") - && !minor.is_some_and(|v| v.trim().parse::().is_ok_and(|v| v > 0)) -} - pub(crate) struct Entry<'a> { pub key: &'a str, pub body: &'a str, @@ -270,23 +255,6 @@ pub(crate) fn suffix<'a>(key: &'a str, name: &str, version: &str) -> Option<&'a Some(suffix) } -pub(crate) fn supported_suffix(suffix: &str) -> bool { - if suffix.is_empty() || suffix.starts_with('_') { - return true; - } - let mut depth = 0usize; - for c in suffix.chars() { - match c { - '(' => depth += 1, - ')' if depth > 0 => depth -= 1, - ')' => return false, - _ if depth == 0 => return false, - _ => {} - } - } - depth == 0 -} - pub(crate) struct Resolution<'a> { pub range: Range, pub fields: Vec<(&'a str, &'a str)>, diff --git a/crates/socket-patch-core/src/formats/pnpm/hosted.rs b/crates/socket-patch-core/src/formats/pnpm/hosted.rs index 84f2524e2..14f8e05bb 100644 --- a/crates/socket-patch-core/src/formats/pnpm/hosted.rs +++ b/crates/socket-patch-core/src/formats/pnpm/hosted.rs @@ -11,6 +11,7 @@ use serde_json::Value; use crate::patch::redirect::{full_name, DepOverride, FileEdit, RewriteResult, RewriteWarning}; use super::grammar as pnpm; +use super::plan::{match_key, KeyMatch, Target}; /// Whether `entry` resolves to exactly `artifact_url` — the per-instance /// residual-gate predicate. @@ -42,11 +43,8 @@ struct PnpmLockState<'f> { /// Entry indices sorted by normalized (unquoted, `/`-stripped) key. sorted: Vec, pending: Vec<(std::ops::Range, String)>, - spliced: std::collections::HashSet, + spliced: HashSet, changed: bool, - /// [`vendored_packages`] of `text`, computed on the first unmatched dep - /// and dropped when [`PnpmLockState::materialize`] replaces `text`. - vendored: Option>, } impl<'f> PnpmLockState<'f> { @@ -54,13 +52,12 @@ impl<'f> PnpmLockState<'f> { let mut state = PnpmLockState { path, text: Cow::Borrowed(text), - early_shrinkwrap: pnpm::unsupported_early_shrinkwrap(text), + early_shrinkwrap: Target::Hosted.check_lock(text).is_err(), entries: Vec::new(), sorted: Vec::new(), pending: Vec::new(), spliced: Default::default(), changed: false, - vendored: None, }; state.reindex(); state @@ -84,17 +81,6 @@ impl<'f> PnpmLockState<'f> { self.sorted = sorted; } - /// Whether the lock vendors `fname@version` ([`vendored_packages`]). - /// Read from `text` without folding `pending` in: a resolution splice - /// never touches a packages key or a `name:` / `version:` line (see - /// the type docs), so the answer is the post-splice lock's. - fn vendors(&mut self, fname: &str, version: &str) -> bool { - let text: &str = &self.text; - self.vendored - .get_or_insert_with(|| vendored_packages(text)) - .contains(&(fname.to_string(), version.to_string())) - } - fn entry(&self, i: usize) -> pnpm::Entry<'_> { let (key, body) = &self.entries[i]; pnpm::Entry { @@ -111,20 +97,32 @@ impl<'f> PnpmLockState<'f> { } /// Entries whose key names `fname@version` (any suffix), in file order — - /// the same set a full [`pnpm::suffix`] scan of the logical lock yields. + /// the same set a full [`match_key`] scan of the logical lock yields. fn hits(&mut self, fname: &str, version: &str) -> Vec { - let hits = self.lookup(fname, version); + let hits = self.lookup(fname, version, false); if hits.iter().any(|i| self.spliced.contains(i)) { self.materialize(); - return self.lookup(fname, version); + return self.lookup(fname, version, false); } hits } - fn lookup(&self, fname: &str, version: &str) -> Vec { + /// Whether a vendored rewire rekeyed `fname@version` + /// ([`KeyMatch::Rekeyed`]). Read from `text` without folding `pending` + /// in: a resolution splice never touches a key (see the type docs). + fn vendors(&self, fname: &str, version: &str) -> bool { + !self.lookup(fname, version, true).is_empty() + } + + /// The entries [`match_key`] matches as a registry instance or, with + /// `rekeyed`, as a vendored rekey, found by their key prefixes. + fn lookup(&self, fname: &str, version: &str, rekeyed: bool) -> Vec { + let prefixes = match rekeyed { + false => [format!("{fname}@{version}"), format!("{fname}/{version}")], + true => [format!("{fname}@file:"), "file:".to_string()], + }; let mut out = Vec::new(); - for sep in ['@', '/'] { - let prefix = format!("{fname}{sep}{version}"); + for prefix in &prefixes { let start = self .sorted .partition_point(|&i| self.norm_key(i) < prefix.as_str()); @@ -137,7 +135,11 @@ impl<'f> PnpmLockState<'f> { } out.sort_unstable(); out.dedup(); - out.retain(|&i| pnpm::suffix(self.entry(i).key, fname, version).is_some()); + out.retain(|&i| match match_key(self.entry(i).key, fname, version) { + Some(KeyMatch::Registry { .. }) => !rekeyed, + Some(KeyMatch::Rekeyed) => rekeyed, + None => false, + }); out } @@ -162,7 +164,6 @@ impl<'f> PnpmLockState<'f> { out.push_str(&self.text[cursor..]); self.text = Cow::Owned(out); self.spliced.clear(); - self.vendored = None; self.reindex(); #[cfg(debug_assertions)] debug_assert_eq!( @@ -268,7 +269,7 @@ pub(crate) fn plan_hosted( let entry = lock.entry(i); let suffix = pnpm::suffix(entry.key, &fname, &dep.version) .expect("hits only holds entries naming this dep"); - let resolution = if pnpm::supported_suffix(suffix) { + let resolution = if Target::Hosted.repoints(suffix) { pnpm::resolution(&entry) } else { None @@ -359,9 +360,7 @@ pub(crate) fn plan_hosted( // mode switch. Fail-closed either way: nothing is rewritten for the // dep. if !matched_any { - let vendored = locks - .iter_mut() - .any(|lock| lock.vendors(&fname, &dep.version)); + let vendored = locks.iter().any(|lock| lock.vendors(&fname, &dep.version)); if vendored { result.warnings.push(RewriteWarning { code: "redirect_pnpm_entry_vendored".into(), @@ -390,35 +389,6 @@ pub(crate) fn plan_hosted( } } -/// The `(name, version)` of every packages entry of `text` the vendor -/// backends rekeyed: v9's `@file:` key or legacy's bare -/// `file:` key with a `name:` line, each with its `version:` -/// line, whose path is this project's vendored npm artifact -/// ([`crate::origin::vendor_ref`]: root-anchored, canonical uuid, -/// traversal-safe leaf) — the one rule lockfile discovery reads vendored -/// pnpm keys with. A user's own `file:` dep of the same name, a -/// `../.socket/vendor/…` path from another checkout, and an `overrides:` -/// line alone (pnpm mirrors it from package.json whether or not anything -/// matches) are not vendoring. -fn vendored_packages(text: &str) -> HashSet<(String, String)> { - use super::{classify_pnpm_key, PnpmKey}; - let lock = super::PnpmLock::parse_keys(text); - lock.packages() - .iter() - .filter_map(|package| { - let (name, path) = match classify_pnpm_key(package.key) { - PnpmKey::V9File { name, path } => (name.trim_start_matches('/'), path), - PnpmKey::LegacyFile { path } => (pnpm::entry_field(&package.entry, "name")?, path), - PnpmKey::Registry { .. } | PnpmKey::Other => return None, - }; - let version = pnpm::entry_field(&package.entry, "version")?; - crate::origin::vendor_ref(path) - .filter(|v| v.eco == "npm") - .map(|_| (name.to_string(), version.to_string())) - }) - .collect() -} - /// Test-only reference for the residual gate: every instance of this exact /// name@version in `content` that does not resolve to `artifact_url`. /// Production judges the same predicate inline, per instance, on each diff --git a/crates/socket-patch-core/src/formats/pnpm/mod.rs b/crates/socket-patch-core/src/formats/pnpm/mod.rs index 2df9b643f..b94801dfb 100644 --- a/crates/socket-patch-core/src/formats/pnpm/mod.rs +++ b/crates/socket-patch-core/src/formats/pnpm/mod.rs @@ -28,6 +28,7 @@ pub(crate) mod grammar; pub(crate) mod hosted; pub(crate) mod lines; pub(crate) mod modules_yaml; +pub(crate) mod plan; mod version; pub(crate) mod workspace; @@ -286,28 +287,28 @@ fn head_lock_version(text: &str) -> Option { /// looping them back to "re-lock with pnpm >= 9" would hand them the lock /// they have. pub fn sniff_lock_grammar(text: &str) -> Result { - match head_lock_version(text).as_deref() { + let version = head_lock_version(text); + match version.as_deref() { Some("9.0") => Ok(PnpmLockGrammar::V9), Some("5.4") => Ok(PnpmLockGrammar::V54), Some("6.0") => Ok(PnpmLockGrammar::V60), - Some(v) => { - let major = v.split('.').next().and_then(|m| m.parse::().ok()); + v => { + let major = v.and_then(|v| v.split('.').next()?.parse::().ok()); + let v = v.map_or("no lockfileVersion in its head".into(), |v| { + format!("lockfileVersion {v}") + }); Err(match major { - Some(m) if m < 9 => format!( - "{PNPM_LOCK} has lockfileVersion {v}; supported versions are 5.4 \ - (pnpm 7), 6.0 (pnpm 8), and 9.0 (pnpm >= 9) — re-lock with pnpm >= 9" - ), - _ => format!( - "{PNPM_LOCK} has lockfileVersion {v}; this socket-patch build supports \ + Some(m) if m >= 9 => format!( + "{PNPM_LOCK} has {v}; this socket-patch build supports \ lockfileVersions 5.4, 6.0, and 9.0 — re-lock with a pnpm release that \ emits one of them, or update socket-patch" ), + _ => format!( + "{PNPM_LOCK} has {v}; supported versions are 5.4 (pnpm 7), 6.0 \ + (pnpm 8), and 9.0 (pnpm >= 9) — re-lock with pnpm >= 9" + ), }) } - None => Err(format!( - "{PNPM_LOCK} has no lockfileVersion in its head; supported versions are 5.4, \ - 6.0, and 9.0 — re-lock with pnpm >= 9" - )), } } @@ -357,43 +358,6 @@ pub fn is_shrinkwrap_lock(text: &str) -> bool { .any(|line| line.starts_with("shrinkwrapVersion:")) } -/// The lockfileVersion the v9 vendored planner splices. -const V9_LOCK_VERSION: &str = "9.0"; - -/// `lockfileVersion: '9.0'` head check (accept pnpm's single quotes plus -/// double-quoted/bare spellings) — the v9 vendored planner's own guard. -/// The flavor router sniffs with [`sniff_lock_grammar`] instead, whose -/// allowlist also routes the legacy 5.4/6.0 grammars to their planner; -/// this check only fires if a non-9.0 lock reaches the v9 planner directly. -pub(crate) fn check_v9_lock_version(text: &str) -> Result<(), String> { - match head_lock_version(text) { - Some(v) if v == V9_LOCK_VERSION => Ok(()), - Some(v) => { - // The remedy must point the right way: 5.x (pnpm 7) / 6.x - // (pnpm 8) locks predate the v9 grammar and upgrading pnpm - // re-locks them, but a HIGHER version means the user's pnpm - // already outgrew this build — telling them "re-lock with - // pnpm >= 9" would loop them back to the lock they have. - let major = v.split('.').next().and_then(|m| m.parse::().ok()); - Err(match major { - Some(m) if m < 9 => format!( - "{PNPM_LOCK} has lockfileVersion {v}; only {V9_LOCK_VERSION} is \ - supported — re-lock with pnpm >= 9" - ), - _ => format!( - "{PNPM_LOCK} has lockfileVersion {v}; this socket-patch build supports \ - lockfileVersion {V9_LOCK_VERSION} — re-lock with a pnpm release \ - that emits it, or update socket-patch" - ), - }) - } - None => Err(format!( - "{PNPM_LOCK} has no lockfileVersion in its head; only \ - {V9_LOCK_VERSION} is supported — re-lock with pnpm >= 9" - )), - } -} - // ── the model ── /// One pnpm lock, parsed once (see the module docs). @@ -1046,11 +1010,6 @@ mod tests { may_need_store_flag(plain), "{plain}" ); - assert_eq!( - check_v9_lock_version(&bom), - check_v9_lock_version(plain), - "{plain}" - ); let keys = |text: &str| { PnpmLock::parse(text).entries().map(|e| { e.into_iter() @@ -1067,9 +1026,9 @@ mod tests { assert_eq!(lock_version_major(&format!("\u{feff}{v9}")), Some(9)); assert!(may_need_store_flag(&format!("\u{feff}{v52}"))); assert!(may_need_store_flag(&format!("\u{feff}{shrinkwrap}"))); - assert!(grammar::unsupported_early_shrinkwrap( - "\u{feff}shrinkwrapVersion: 3\n" - )); + assert!(plan::Target::Hosted + .check_lock("\u{feff}shrinkwrapVersion: 3\n") + .is_err()); // Exactly one BOM is encoding; a second one is content, as for pnpm. assert!(!is_pnpm_lock_text( "\u{feff}\u{feff}lockfileVersion: '9.0'\n" @@ -1146,14 +1105,15 @@ mod architecture_tests { ( "vendor/pnpm_lock.rs", 3, - "v9 vendored planner: preflight and splices over line blocks \ - (`lock_has_target_package`, `edit_packages`, `LockIndex`)", + "vendored planner (both dialects): the shared lookup and rekey \ + over line blocks (`lock_has_target_package`, `edit_packages`, \ + `LockIndex`), keys read with `plan::match_key`", ), ( "vendor/pnpm_lock_legacy.rs", - 6, - "5.4 / 6.0 vendored planner: preflight, splices and revert over \ - line blocks", + 4, + "5.4 / 6.0 dialect: its ref gate, sorted rekey, dep-ref edit and \ + revert over line blocks", ), ]; diff --git a/crates/socket-patch-core/src/formats/pnpm/plan.rs b/crates/socket-patch-core/src/formats/pnpm/plan.rs new file mode 100644 index 000000000..a2fa56c1d --- /dev/null +++ b/crates/socket-patch-core/src/formats/pnpm/plan.rs @@ -0,0 +1,201 @@ +//! The pnpm planning core both modes share. Hosted ([`super::hosted`]) and +//! vendored (`vendor::pnpm_lock`, `vendor::pnpm_lock_legacy`) mode differ +//! only in where a rewired entry points ([`Target`]); this module owns what +//! they agree on: +//! +//! * which lock versions each target's edit has fixtures for +//! ([`Target::check_lock`]); +//! * how a lock key names a patched `name@version` ([`match_key`]): the +//! registry entry with its peer suffix, or the entry a vendored rewire +//! already rekeyed; +//! * the all-or-nothing gate: an instance the target's edit cannot repoint +//! ([`Target::repoints`]) refuses the release in every lock, so no lock +//! is left half-rewired. +//! +//! The edits themselves stay with the targets: hosted splices each +//! instance's `resolution:`; vendored rekeys the entry to its `file:` +//! artifact and mirrors the override pnpm cross-checks. + +use super::grammar; +use super::{sniff_lock_grammar, PnpmLockGrammar}; +use crate::constants::npm_family::PNPM_LOCK; +use crate::formats::text::strip_bom; +use crate::vendor::npm_common::tgz_rel_leaf; + +/// Where a rewired entry points. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum Target { + /// A Socket-hosted tarball URL (`scan --mode hosted`): the instance's + /// `resolution:` is spliced in place. + Hosted, + /// A committed `.socket/vendor/npm//` tarball (`vendor`): + /// the entry is rekeyed to `file:`. + Vendored, +} + +impl Target { + /// Whether this target's edit can rewrite a lock with `text`'s head, + /// and in which grammar (`None`: the hosted splice reads every + /// grammar the same way). Hosted takes every lock but pnpm 1's early + /// shrinkwrap (shrinkwrapVersion 3 without a minor version), whose + /// installers drop registry tarball URLs whatever the frozen flag says + /// (verified with pnpm 1.0.0). Vendored takes the grammars its rekey was + /// captured against (5.4, 6.0, 9.0): another version's `file:` override + /// shape has no pnpm-blessed fixture, so it is refused, + /// target-specifically (divergences N16). + pub(crate) fn check_lock(self, text: &str) -> Result, String> { + if self == Target::Vendored { + return sniff_lock_grammar(text).map(Some); + } + let field = |name: &str| strip_bom(text).lines().find_map(|l| l.strip_prefix(name)); + let early = field("shrinkwrapVersion:") + .is_some_and(|v| v.trim().trim_matches(['\'', '"']) == "3") + && !field("shrinkwrapMinorVersion:") + .is_some_and(|v| v.trim().parse::().is_ok_and(|v| v > 0)); + match early { + true => Err(format!( + "{PNPM_LOCK} uses early pnpm 1 shrinkwrapVersion 3 without a supported \ + minor version" + )), + false => Ok(None), + } + } + + /// Whether this target's edit repoints a registry instance whose key + /// carries `suffix` after `name@version`. Hosted splices every peer + /// context it can parse (pnpm 5's `_peer@1` or balanced `(peer@1)` + /// groups); vendored rekeys the plain entry only, since a peer-suffixed + /// rekey has no fixture and would dangle. + pub(crate) fn repoints(self, suffix: &str) -> bool { + if self == Target::Vendored || suffix.is_empty() || suffix.starts_with('_') { + return suffix.is_empty() || self == Target::Hosted; + } + let mut depth = 0usize; + for c in suffix.chars() { + match c { + '(' => depth += 1, + ')' if depth > 0 => depth -= 1, + _ if depth == 0 => return false, + _ => {} + } + } + depth == 0 + } +} +/// How one `packages:` / `snapshots:` key names a patched release. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum KeyMatch<'k> { + /// The registry entry, with whatever follows `name@version` (empty, or + /// a peer suffix such as `(react@18.2.0)` / `_react@18.2.0`). + Registry { suffix: &'k str }, + /// The entry a vendored rewire rekeyed: v9's `name@file:` or + /// legacy's bare `file:`, where the artifact is this + /// project's vendored tarball of this release + /// ([`crate::origin::vendor_ref`]: root-anchored, so a sibling + /// checkout's `../.socket/vendor/…` is not ours; any uuid). + Rekeyed, +} + +/// How `key` names `name@version`, if it does (see [`KeyMatch`]). Keys in +/// every grammar's spelling (`name@1.0.0`, `/name@1.0.0`, `/name/1.0.0`, +/// quoted) read alike. +pub(crate) fn match_key<'k>(key: &'k str, name: &str, version: &str) -> Option> { + if let Some(suffix) = grammar::suffix(key, name, version) { + return Some(KeyMatch::Registry { suffix }); + } + let key = grammar::unquote(key); + let path = key + .strip_prefix(name) + .and_then(|rest| rest.strip_prefix("@file:")) + .or_else(|| key.strip_prefix("file:"))?; + // The leaf binding matters: a project can vendor one package at several + // versions, and an edit must never take a sibling version's entry. + crate::origin::vendor_ref(path) + .filter(|v| v.eco == "npm" && v.leaf == tgz_rel_leaf(name, version)) + .map(|_| KeyMatch::Rekeyed) +} + +#[cfg(test)] +mod tests { + use super::*; + + const ART: &str = ".socket/vendor/npm/0b8d2c3e-4f5a-4b6c-8d7e-9f0a1b2c3d4e/left-pad-1.3.0.tgz"; + + #[test] + fn keys_of_every_grammar_name_the_release() { + for key in [ + "left-pad@1.3.0", + "/left-pad@1.3.0", + "/left-pad/1.3.0", + "'left-pad@1.3.0'", + ] { + assert_eq!( + match_key(key, "left-pad", "1.3.0"), + Some(KeyMatch::Registry { suffix: "" }), + "{key}" + ); + } + assert_eq!( + match_key("left-pad@1.3.0(react@18.2.0)", "left-pad", "1.3.0"), + Some(KeyMatch::Registry { + suffix: "(react@18.2.0)" + }) + ); + assert_eq!(match_key("left-pad@1.3.01", "left-pad", "1.3.0"), None); + assert_eq!( + match_key(&format!("left-pad@file:{ART}"), "left-pad", "1.3.0"), + Some(KeyMatch::Rekeyed) + ); + assert_eq!( + match_key(&format!("file:{ART}"), "left-pad", "1.3.0"), + Some(KeyMatch::Rekeyed) + ); + // A sibling version's artifact, another package's key, and a + // sibling checkout's path are not this release's rekey. + assert_eq!(match_key(&format!("file:{ART}"), "left-pad", "1.3.1"), None); + assert_eq!( + match_key(&format!("is-odd@file:{ART}"), "left-pad", "1.3.0"), + None + ); + assert_eq!( + match_key(&format!("left-pad@file:../{ART}"), "left-pad", "1.3.0"), + None + ); + } + + #[test] + fn each_target_repoints_what_its_edit_has_fixtures_for() { + assert!(Target::Hosted.repoints("")); + assert!(Target::Hosted.repoints("(react@18.2.0)")); + assert!(Target::Hosted.repoints("_react@18.2.0")); + assert!(!Target::Hosted.repoints("(react@18.2.0")); + assert!(Target::Vendored.repoints("")); + assert!(!Target::Vendored.repoints("(react@18.2.0)")); + assert!(!Target::Vendored.repoints("_react@18.2.0")); + } + + /// Divergence N16: hosted reads every lock but early shrinkwrap, while + /// vendored keeps its refusal for every version its rekey was not + /// captured against (target-specific). + #[test] + fn lock_versions_each_target_rewrites() { + for (head, vendored) in [ + ("lockfileVersion: '9.0'\n", Some(PnpmLockGrammar::V9)), + ("lockfileVersion: '6.0'\n", Some(PnpmLockGrammar::V60)), + ("lockfileVersion: 5.4\n", Some(PnpmLockGrammar::V54)), + ("lockfileVersion: 5.3\n", None), + ("lockfileVersion: 5.1\n", None), + ("lockfileVersion: '6.1'\n", None), + ("lockfileVersion: '10.0'\n", None), + ("shrinkwrapVersion: 3\nshrinkwrapMinorVersion: 9\n", None), + ] { + assert_eq!(Target::Hosted.check_lock(head), Ok(None), "{head}"); + assert_eq!( + Target::Vendored.check_lock(head).ok().flatten(), + vendored, + "{head}" + ); + } + assert!(Target::Hosted.check_lock("shrinkwrapVersion: 3\n").is_err()); + } +} diff --git a/crates/socket-patch-core/src/patch/redirect/mod.rs b/crates/socket-patch-core/src/patch/redirect/mod.rs index 0201fbb31..165450ab4 100644 --- a/crates/socket-patch-core/src/patch/redirect/mod.rs +++ b/crates/socket-patch-core/src/patch/redirect/mod.rs @@ -28332,6 +28332,28 @@ packages: .replace("version: 1.3.0", "version: 1.2.0"), "redirect_pnpm_entry_not_found", ), + // Lock-model 2b: the hint reads the key with the planning core's + // `match_key`, the rule the vendored planner rekeys by, so the + // artifact leaf names the release and the `version:` line is not + // consulted. Before, a rekey without a `version:` line was "not + // found" and one whose leaf names another release but whose + // `version:` line matched was "vendored". + ( + "a rekey without a version line", + v9(&format!(".socket/vendor/npm/{uuid}/left-pad-1.3.0.tgz")) + .replace(" version: 1.3.0\n", ""), + "redirect_pnpm_entry_vendored", + ), + ( + "another release's artifact under this version line", + v9(&format!(".socket/vendor/npm/{uuid}/left-pad-1.2.0.tgz")), + "redirect_pnpm_entry_not_found", + ), + ( + "a sibling checkout's artifact", + v9(&format!("../.socket/vendor/npm/{uuid}/left-pad-1.3.0.tgz")), + "redirect_pnpm_entry_not_found", + ), ] { assert_eq!(codes(&lock), vec![want], "{case}"); } diff --git a/crates/socket-patch-core/src/vendor/mod.rs b/crates/socket-patch-core/src/vendor/mod.rs index dd732ca7f..df2e1fd7b 100644 --- a/crates/socket-patch-core/src/vendor/mod.rs +++ b/crates/socket-patch-core/src/vendor/mod.rs @@ -989,8 +989,8 @@ pub async fn lock_text_refusals( /// deletes; a gate that depended on it would make this under-predict, and /// the wet run still refuses. Yarn's restores rewrite key and checksum /// text, and legacy pnpm 7/8 locks (5.4 / 6.0) are not lock-text gated at -/// all, so neither is predicted here: a CRLF legacy lock, an override -/// conflict or an unsupported legacy entry previews `would_vendor` while +/// all, so neither is predicted here: a mixed line-ending legacy lock, an +/// override conflict or an unsupported legacy entry previews `would_vendor` while /// the wet takeover refuses it (keeping the hosted pin, #963) — a known /// preview gap the contract names. pub async fn pnpm_takeover_lock_text_refusals( @@ -2236,10 +2236,15 @@ mod pnpm_takeover_lock_text_refusal_tests { tmp } + /// Divergences N15: a CRLF hosted lock (an autocrlf checkout) takes over + /// like its LF twin; only a lock mixing CRLF and LF is refused. #[tokio::test] - async fn crlf_hosted_pnpm_lock_is_refused() { + async fn only_a_mixed_line_ending_hosted_pnpm_lock_is_refused() { let tmp = project("pnpm-lock.yaml", &HOSTED_V9.replace('\n', "\r\n")); let refused = pnpm_takeover_lock_text_refusals(tmp.path(), &[(PURL, UUID)]).await; + assert!(refused.is_empty(), "{refused:?}"); + let tmp = project("pnpm-lock.yaml", &HOSTED_V9.replacen('\n', "\r\n", 1)); + let refused = pnpm_takeover_lock_text_refusals(tmp.path(), &[(PURL, UUID)]).await; assert_eq!( refused.get(PURL).map(|(code, _)| *code), Some("vendor_lockfile_crlf_unsupported"), @@ -2266,8 +2271,8 @@ mod pnpm_takeover_lock_text_refusal_tests { } /// A known preview gap (CLI_CONTRACT, #853): a legacy pnpm 7/8 lock - /// (5.4 / 6.0) is not lock-text gated, so even a CRLF one the wet - /// takeover refuses (`vendor_lockfile_crlf_unsupported`) previews + /// (5.4 / 6.0) is not lock-text gated, so even one the wet takeover + /// refuses (a mixed line-ending lock, `vendor_lockfile_crlf_unsupported`) previews /// `would_vendor`. The wet run's refusal keeps the hosted pin (#963). #[tokio::test] async fn legacy_pnpm_project_is_out_of_scope() { diff --git a/crates/socket-patch-core/src/vendor/npm_common.rs b/crates/socket-patch-core/src/vendor/npm_common.rs index 48de7a329..0e477052c 100644 --- a/crates/socket-patch-core/src/vendor/npm_common.rs +++ b/crates/socket-patch-core/src/vendor/npm_common.rs @@ -659,7 +659,7 @@ pub(crate) fn is_safe_npm_name(name: &str) -> bool { /// The artifact path under the uuid dir: `[@scope/]-.tgz`, /// with the scope kept as a real subdirectory. -pub(super) fn tgz_rel_leaf(name: &str, version: &str) -> String { +pub(crate) fn tgz_rel_leaf(name: &str, version: &str) -> String { format!("{}.tgz", pkg_rel_leaf(name, version)) } diff --git a/crates/socket-patch-core/src/vendor/pnpm_lock.rs b/crates/socket-patch-core/src/vendor/pnpm_lock.rs index 7547e780d..d80b04d29 100644 --- a/crates/socket-patch-core/src/vendor/pnpm_lock.rs +++ b/crates/socket-patch-core/src/vendor/pnpm_lock.rs @@ -69,6 +69,7 @@ use super::npm_common::{ use super::parse_memo::ParseMemo; use super::path::parse_vendor_path; use super::pnpm_build_approvals as build_approvals; +use super::pnpm_lock_legacy::swap_block_sorted; use super::revert::{self, KeepPolicy}; use super::source::PackageSource; use super::state::{PnpmMeta, VendorEntry, WiringAction, WiringRecord}; @@ -78,10 +79,12 @@ use crate::formats::pnpm::lines::{ indent_of, next_block, parse_key_line, section_bounds, split_lines, split_project_document, unquote_value, yaml_key, yaml_key_like, YamlBlock, }; +use crate::formats::pnpm::plan::{match_key, KeyMatch, Target}; use crate::formats::pnpm::workspace; -use crate::formats::pnpm::{check_v9_lock_version as check_lock_version, vendored_npm_uuids}; +use crate::formats::pnpm::{vendored_npm_uuids, PnpmLockGrammar}; const PACKAGE_JSON: &str = "package.json"; +use crate::utils::line_endings::{to_lf, LineEndings}; use crate::utils::pnpm_workspace::{governing_workspace_file, PNPM_WORKSPACE}; /// Refusal code for a workspace member whose pnpm settings live in an @@ -173,6 +176,23 @@ impl PnpmDialect { fn allows_revert_file(self, file: &str) -> bool { REVERT_ALLOWLIST.contains(&file) && (self == Self::V9 || file != PNPM_WORKSPACE) } + + /// The grammar of a lock this dialect rewrites: one the vendored target + /// takes ([`Target::check_lock`]) and this dialect splices. + pub(super) fn check_lock(self, text: &str) -> Result { + let grammar = Target::Vendored + .check_lock(text)? + .unwrap_or(PnpmLockGrammar::V9); + match (self, grammar) { + (Self::V9, PnpmLockGrammar::V9) + | (Self::Legacy, PnpmLockGrammar::V54 | PnpmLockGrammar::V60) => Ok(grammar), + _ => Err(format!( + "{PNPM_LOCK} is a {} lock, which the {} planner does not write", + grammar.describe(), + self.flavor() + )), + } + } } pub(super) async fn vendor_pnpm_dialect( @@ -301,7 +321,9 @@ impl NpmLockBackend for PnpmBackend { mut pkg, mut lock, lock_prefix, + lock_eol, ws_text, + ws_eol, }, effective_key, } = plan; @@ -395,15 +417,18 @@ impl NpmLockBackend for PnpmBackend { .render(&pkg) .map_err(|e| format!("cannot serialize {PACKAGE_JSON}: {e}"))?; // An env document ahead of the project lock is carried over verbatim. + // Each file is written back in its own line endings (`lf_text`). let lock_out = format!("{lock_prefix}{}", lock.lines().join("\n")); + let ws_out = ws_edit.new_text.as_deref().map(|t| ws_eol.restore(t)); + let ws_was = ws_text.as_deref().map(|t| ws_eol.restore(t)); commit_surfaces( project_root, pkg_changed.then_some(new_pkg_bytes.as_slice()), &pkg_bytes, - ws_edit.new_text.as_deref().map(str::as_bytes), - ws_text.as_deref().map(str::as_bytes), + ws_out.as_deref().map(str::as_bytes), + ws_was.as_deref().map(str::as_bytes), ws_edit.created_file, - lock_changed.then_some(lock_out.as_bytes()), + lock_changed.then_some(lock_eol.restore(&lock_out).as_bytes()), ) .await?; if lock_changed { @@ -459,7 +484,11 @@ struct PnpmProject { lock: ProjectLock, /// Everything ahead of the project document, written back verbatim. lock_prefix: String, + /// The line endings the lock and the workspace file are written back + /// with; their texts here are LF ([`lf_text`]). + lock_eol: LineEndings, ws_text: Option, + ws_eol: LineEndings, } enum ProjectLock { @@ -564,13 +593,7 @@ async fn read_project( ))); } }; - let grammar = match dialect { - PnpmDialect::V9 => { - check_lock_version(&lock_text).map(|()| crate::formats::pnpm::PnpmLockGrammar::V9) - } - PnpmDialect::Legacy => super::pnpm_lock_legacy::check_lock_version(&lock_text), - }; - let grammar = match grammar { + let grammar = match dialect.check_lock(&lock_text) { Ok(grammar) => grammar, Err(detail) => { return Err(Box::new(refused( @@ -579,23 +602,7 @@ async fn read_project( ))); } }; - // CRLF line endings (Windows autocrlf checkouts) break every structural - // probe below — `split_lines` keeps the trailing `\r`, so section headers - // like `packages:` never match — which would otherwise surface as a - // misleading "no packages entry … run `pnpm install`" refusal. pnpm - // itself tolerates CRLF, so name the real cause and fail closed before - // any probe runs. - if lock_text.contains('\r') { - return Err(Box::new(refused( - "vendor_lockfile_crlf_unsupported", - format!( - "{PNPM_LOCK} has CRLF line endings, which this rewriter cannot edit \ - byte-faithfully — normalize the file to LF (re-run `pnpm install`, \ - or add `pnpm-lock.yaml text eol=lf` to .gitattributes and re-checkout) \ - and retry" - ), - ))); - } + let (lock_text, lock_eol) = lf_text(PNPM_LOCK, lock_text)?; if dialect == PnpmDialect::Legacy { return Ok(PnpmProject { pkg_bytes, @@ -606,7 +613,9 @@ async fn read_project( grammar, )?), lock_prefix: String::new(), + lock_eol, ws_text: None, + ws_eol: LineEndings::None, }); } let (lock_prefix, project_text) = match split_v9_lock(&lock_text) { @@ -636,21 +645,20 @@ async fn read_project( // encoding, permissions, a FIFO) must refuse — treating it as absent // would route into the create path, which OVERWRITES the user's // workspace definition with the root-only scaffold. - let ws_text: Option = - match read_regular_to_string(&project_root.join(PNPM_WORKSPACE)).await { - Ok(text) => Some(text), - Err(e) if e.kind() == std::io::ErrorKind::NotFound => None, - Err(e) => { - return Err(Box::new(refused( - "vendor_lockfile_missing", - format!( - "cannot read {PNPM_WORKSPACE}: {e} — vendoring mirrors the override \ + let (ws_text, ws_eol) = match read_regular_to_string(&project_root.join(PNPM_WORKSPACE)).await { + Ok(text) => lf_text(PNPM_WORKSPACE, text).map(|(t, eol)| (Some(t), eol))?, + Err(e) if e.kind() == std::io::ErrorKind::NotFound => (None, LineEndings::None), + Err(e) => { + return Err(Box::new(refused( + "vendor_lockfile_missing", + format!( + "cannot read {PNPM_WORKSPACE}: {e} — vendoring mirrors the override \ into it (pnpm >= 11 reads overrides from pnpm-workspace.yaml); fix \ the file and retry" - ), - ))); - } - }; + ), + ))); + } + }; // A workspace member with its own lock (`sharedWorkspaceLockfile: // false`) and no workspace file of its own: pnpm reads `overrides:` // only from the workspace root's file, so the create path below would @@ -677,28 +685,36 @@ async fn read_project( ))); } } - // Same CRLF posture as the lock above: the workspace splices match exact - // LF lines, so a CRLF file dodges the conflict checks and the edit - // appends a DUPLICATE `overrides:` section — a duplicated mapping key - // pnpm refuses to parse. Fail closed naming the real cause. - if ws_text.as_deref().is_some_and(|t| t.contains('\r')) { - return Err(Box::new(refused( - "vendor_lockfile_crlf_unsupported", - format!( - "{PNPM_WORKSPACE} has CRLF line endings, which this rewriter cannot edit \ - byte-faithfully — normalize the file to LF and retry" - ), - ))); - } Ok(PnpmProject { pkg_bytes, pkg, lock: ProjectLock::V9(lines), lock_prefix, + lock_eol, ws_text, + ws_eol, }) } +/// A pnpm file's text with LF line endings, and the endings it is written +/// back with. A CRLF file (a Windows autocrlf checkout) is read and edited +/// like its LF twin and written back CRLF, as hosted mode's byte splices +/// keep it (divergences N15). A file that mixes endings is refused: no one +/// ending writes it back byte-faithfully. +fn lf_text(file: &str, text: String) -> Result<(String, LineEndings), Box> { + match LineEndings::of(&text) { + LineEndings::Mixed => Err(Box::new(refused( + "vendor_lockfile_crlf_unsupported", + format!( + "{file} mixes CRLF and LF line endings, which this rewriter cannot write \ + back byte-faithfully — normalize the file to one line ending and retry" + ), + ))), + LineEndings::Crlf => Ok((text.replace("\r\n", "\n"), LineEndings::Crlf)), + eol => Ok((text, eol)), + } +} + /// `(prefix, project document)` of a v9 lock ([`split_project_document`]), /// the project document's own head version-checked. `Err` names why the /// lock's documents are not pnpm's layout. @@ -706,7 +722,7 @@ fn split_v9_lock(text: &str) -> Result<(&str, &str), String> { let (prefix, project) = split_project_document(text).map_err(|why| format!("{PNPM_LOCK} {why}"))?; if !prefix.is_empty() { - check_lock_version(project).map_err(|e| { + PnpmDialect::V9.check_lock(project).map_err(|e| { format!("the project document of {PNPM_LOCK} fails its version check: {e}") })?; } @@ -770,7 +786,7 @@ fn preflight_package( { return Err(Box::new(refused("vendor_override_conflict", detail))); } - if !lock_has_target_package_in(lines, name, version) { + if !lock_has_target_package(lines, name, version) { return Err(Box::new(refused( "vendor_lock_entry_not_found", format!( @@ -834,7 +850,7 @@ pub async fn pnpm_entry_in_use(entry: &VendorEntry, project_root: &Path) -> Opti let text = read_regular_to_string(&project_root.join(PNPM_LOCK)) .await .ok()?; - if check_lock_version(&text).is_err() { + if PnpmDialect::V9.check_lock(&text).is_err() { return None; } // Every `packages:`/`snapshots:` block key resolving into @@ -1008,6 +1024,7 @@ pub(super) async fn revert_pnpm_dialect( // file degrades to a warning and the artifact removal still proceeds). let mut lock_lines: Option> = None; let mut lock_prefix = String::new(); + let mut lock_eol = LineEndings::None; if touches_lock { let read = match lock_text { Some(read) => read, @@ -1015,18 +1032,21 @@ pub(super) async fn revert_pnpm_dialect( }; match read { // The wiring lives in the project document; an env document - // ahead of it is written back verbatim. - Ok(text) => match dialect { - PnpmDialect::V9 => { - let (prefix, project) = match split_v9_lock(&text) { + // ahead of it is written back verbatim, in the lock's own line + // endings (N15). + Ok(text) => { + lock_eol = LineEndings::of(&text); + let text = to_lf(&text); + let (prefix, project) = match dialect { + PnpmDialect::V9 => match split_v9_lock(&text) { Ok(split) => split, Err(why) => return RevertOutcome::failed(why), - }; - lock_prefix = prefix.to_string(); - lock_lines = Some(split_lines(project)); - } - PnpmDialect::Legacy => lock_lines = Some(split_lines(&text)), - }, + }, + PnpmDialect::Legacy => ("", &*text), + }; + lock_prefix = prefix.to_string(); + lock_lines = Some(split_lines(project)); + } Err(e) if e.kind() == std::io::ErrorKind::NotFound => { outcome.warnings.push(VendorWarning::new( "vendor_lockfile_missing", @@ -1126,7 +1146,9 @@ pub(super) async fn revert_pnpm_dialect( if let Some(lines) = &lock_lines { if let Err(e) = atomic_write_bytes_preserving_mode( &project_root.join(PNPM_LOCK), - format!("{lock_prefix}{}", lines.join("\n")).as_bytes(), + lock_eol + .restore(&format!("{lock_prefix}{}", lines.join("\n"))) + .as_bytes(), ) .await { @@ -1231,18 +1253,10 @@ async fn revert_workspace( } Err(e) => return Err(format!("cannot read {PNPM_WORKSPACE}: {e}")), }; - // CRLF blindness fails CLOSED: the splices below match exact LF lines, - // so a CRLF-converted file would read an Added override as "already - // converged" and let the artifact it still points at be removed. Drift, - // not convergence — the keep gate holds the artifact until the user - // normalizes the file. - if text.contains('\r') { - warnings.push(drifted(format!( - "{PNPM_WORKSPACE} has CRLF line endings the revert splices cannot edit; \ - left alone — normalize the file to LF and re-run `vendor --revert`" - ))); - return Ok(()); - } + // A CRLF file (an autocrlf checkout of what vendoring wrote) is read + // like its LF twin and written back CRLF (N15). + let eol = LineEndings::of(&text); + let text = to_lf(&text).into_owned(); // Fast path: a file we created that is still byte-identical to the // scaffold we wrote → delete it (byte-restore to "no file"). @@ -1278,7 +1292,7 @@ async fn revert_workspace( } dirty |= build_approvals::revert_ws(&mut lines, approvals); if dirty { - atomic_write_bytes_preserving_mode(&path, lines.join("\n").as_bytes()) + atomic_write_bytes_preserving_mode(&path, eol.restore(&lines.join("\n")).as_bytes()) .await .map_err(|e| format!("cannot write {PNPM_WORKSPACE}: {e}"))?; } @@ -1405,12 +1419,9 @@ impl EditCtx<'_> { vendor_value_is_for(value, self.name, self.version) } - /// Is `key` our rekeyed `name@file:` packages/snapshots key for - /// THIS name@version (any uuid)? - fn is_ours_key(&self, key: &str) -> bool { - key.strip_prefix(self.name) - .and_then(|rest| rest.strip_prefix("@file:")) - .is_some_and(|rest| self.is_ours(rest)) + /// [`target_key`] of a packages/snapshots key for this release. + pub(super) fn target_key(&self, key: &str) -> Option { + target_key(key, self.name, self.version) } /// The per-importer `specifier:` spelling: re-relativized for nested @@ -1678,56 +1689,26 @@ pub(super) fn check_lock_override( Ok(()) } -/// Pre-flight: does the lock have a packages entry vendoring can target — -/// the registry `name@version` key, or our own rekeyed `name@file:` key -/// (the in-sync / stale-uuid re-run)? The ours-probe binds to THIS -/// version's leaf — a sibling version's vendored entry is not a target -/// (see `vendor_value_is_for`). -fn lock_has_target_package(lines: &[String], name: &str, version: &str) -> bool { - let Some((start, end)) = section_bounds(lines, "packages") else { - return false; - }; - let reg_key = format!("{name}@{version}"); - let ours_prefix = format!("{name}@file:"); - let mut i = start + 1; - while let Some(block) = next_block(lines, i, end) { - if block.key == reg_key { - return true; - } - if let Some(rest) = block.key.strip_prefix(&ours_prefix) { - if vendor_value_is_for(rest, name, version) { - return true; - } - } - i = block.end; +/// Whether a packages/snapshots `key` is one the vendored edit targets for +/// `name@version` ([`match_key`]): `Some(false)` for the registry entry the +/// rekey repoints ([`Target::repoints`]: no peer suffix), `Some(true)` for a +/// rekey of this release already made (the in-sync / stale-uuid re-run), +/// `None` for every other key. The rekey probe binds to THIS version's leaf +/// and to this project's `.socket/vendor/`. +pub(super) fn target_key(key: &str, name: &str, version: &str) -> Option { + match match_key(key, name, version)? { + KeyMatch::Registry { suffix } => Target::Vendored.repoints(suffix).then_some(false), + KeyMatch::Rekeyed => Some(true), } - false } -/// [`lock_has_target_package`] over the backend's lock lines: answered from -/// the section index while the lines are the memoized split (the -/// candidate blocks are exactly those whose key is the registry key or -/// starts with `name@file:`, in lock order), else by the scan. -fn lock_has_target_package_in(lines: &LockLines, name: &str, version: &str) -> bool { - let Some(index) = lines.index() else { - return lock_has_target_package(lines, name, version); - }; - if index.packages.bounds.is_none() { - return false; - } - let reg_key = format!("{name}@{version}"); - let ours_prefix = format!("{name}@file:"); - index - .packages - .candidates(®_key, name) - .into_iter() - .any(|block| { - block.key == reg_key - || block - .key - .strip_prefix(&ours_prefix) - .is_some_and(|rest| vendor_value_is_for(rest, name, version)) - }) +/// Pre-flight: does the lock have a packages entry vendoring can target +/// ([`target_key`])? From the section index while the lines are the +/// memoized split, else by the scan ([`matching_blocks`]). +pub(super) fn lock_has_target_package(lines: &L, name: &str, version: &str) -> bool { + lines.bounds("packages").is_some_and(|(start, end)| { + !matching_blocks(lines, "packages", start, end, name, version).is_empty() + }) } /// Pre-flight fail-closed guard against reference forms the surgery does @@ -1821,60 +1802,37 @@ fn check_rewritable_refs_with( i = catalog.end; } } - if let Some(index) = index { - for candidate in index.snapshot_ref_candidates(®_key, name, version) { - match candidate { - SnapshotCandidate::Key(block) => { - let key = &index.snapshots.blocks[block].key; - if key.starts_with(&key_peer_prefix) { - return refuse("a peer-suffixed snapshot key", key); - } - } - SnapshotCandidate::Body(line) => { - let Some((dep, _repr, rest)) = parse_key_line(&lines[line], 6) else { - continue; - }; - let rest = unquote_value(rest); - if rest == reg_key || rest.starts_with(&key_peer_prefix) { - return refuse("an aliased snapshot reference", rest); - } - if dep == name && rest.starts_with(&val_peer_prefix) { - return refuse("a peer-suffixed snapshot reference", rest); - } - } + // The positions the checks below read: from the index, the earliest + // position each refusal can hold at; by the scan, every one. + let scanned; + let (snapshot_refs, importer_refs): (Box + '_>, _) = + match index { + Some(index) => ( + Box::new( + index + .snapshot_ref_candidates(®_key, name, version) + .into_iter(), + ), + index.importer_ref_candidates(®_key, name, version), + ), + None => { + scanned = importer_deps(lines); + ( + Box::new(snapshot_positions(lines)), + scanned.iter().collect(), + ) } - } - for entry in index.importer_ref_candidates(®_key, name, version) { - let dep = entry.dep.as_str(); - if let Some(v) = entry.ver.as_deref().map(unquote_value) { - if v == reg_key || v.starts_with(&key_peer_prefix) { - return refuse("an aliased importer version", v); - } - if dep == name && v.starts_with(&val_peer_prefix) { - return refuse("a peer-suffixed importer version", v); - } - if dep == name && v == version { - if let Some(s) = entry.spec.as_deref() { - if unquote_value(s).starts_with("catalog:") { - return refuse_catalog( - &format!("importer `{}` specifier", entry.importer), - s, - ); - } - } + }; + for candidate in snapshot_refs { + match candidate { + SnapshotCandidate::Key(header) => { + let key = parse_key_line(&lines[header], 2).map_or("", |(key, ..)| key); + if key.starts_with(&key_peer_prefix) { + return refuse("a peer-suffixed snapshot key", key); } } - } - return Ok(()); - } - if let Some((start, end)) = section_bounds(lines, "snapshots") { - let mut i = start + 1; - while let Some(block) = next_block(lines, i, end) { - if block.key.starts_with(&key_peer_prefix) { - return refuse("a peer-suffixed snapshot key", &block.key); - } - for line in &lines[block.header + 1..block.end] { - let Some((dep, _repr, rest)) = parse_key_line(line, 6) else { + SnapshotCandidate::Body(line) => { + let Some((dep, _repr, rest)) = parse_key_line(&lines[line], 6) else { continue; }; let rest = unquote_value(rest); @@ -1885,52 +1843,73 @@ fn check_rewritable_refs_with( return refuse("a peer-suffixed snapshot reference", rest); } } - i = block.end; } } - if let Some((start, end)) = section_bounds(lines, "importers") { - let mut i = start + 1; - while let Some(importer) = next_block(lines, i, end) { - let mut k = importer.header + 1; - while k < importer.end { - let Some((dep, _repr, rest)) = parse_key_line(&lines[k], 6) else { - k += 1; - continue; - }; - if !rest.is_empty() { - k += 1; - continue; - } - let (spec, ver, f) = dep_field_lines(lines, k + 1, importer.end, 8); - if let Some((_, v)) = &ver { - let v = unquote_value(v); - if v == reg_key || v.starts_with(&key_peer_prefix) { - return refuse("an aliased importer version", v); - } - if dep == name && v.starts_with(&val_peer_prefix) { - return refuse("a peer-suffixed importer version", v); - } - if dep == name && v == version { - if let Some((_, s)) = spec { - // pnpm spells the default catalog `'catalog:'` - // (quoted) and named ones `catalog:` (bare). - if unquote_value(&s).starts_with("catalog:") { - return refuse_catalog( - &format!("importer `{}` specifier", importer.key), - &s, - ); - } - } - } - } - k = f; + for entry in importer_refs { + let dep = entry.dep.as_str(); + let Some(v) = entry.ver.as_deref().map(unquote_value) else { + continue; + }; + if v == reg_key || v.starts_with(&key_peer_prefix) { + return refuse("an aliased importer version", v); + } + if dep == name && v.starts_with(&val_peer_prefix) { + return refuse("a peer-suffixed importer version", v); + } + // pnpm spells the default catalog `'catalog:'` (quoted) and named + // ones `catalog:` (bare). + if let Some(s) = entry + .spec + .as_deref() + .filter(|_| dep == name && v == version) + { + if unquote_value(s).starts_with("catalog:") { + return refuse_catalog(&format!("importer `{}` specifier", entry.importer), s); } - i = importer.end; } } Ok(()) } +/// Every snapshot block header and body line, in lock order: the scan's +/// [`SnapshotCandidate`]s. +fn snapshot_positions(lines: &[String]) -> impl Iterator + '_ { + let blocks = section_bounds(lines, "snapshots").into_iter(); + blocks + .flat_map(move |(start, end)| { + std::iter::successors(next_block(lines, start + 1, end), move |b| { + next_block(lines, b.end, end) + }) + }) + .flat_map(|b| { + std::iter::once(SnapshotCandidate::Key(b.header)) + .chain((b.header + 1..b.end).map(SnapshotCandidate::Body)) + }) +} + +/// Every importer's [`DepEntry`]s, in lock order. +fn importer_deps(lines: &[String]) -> Vec { + let Some((start, end)) = section_bounds(lines, "importers") else { + return Vec::new(); + }; + let mut out = Vec::new(); + let mut i = start + 1; + while let Some(importer) = next_block(lines, i, end) { + out.extend( + dep_entries(lines, importer.header + 1, importer.end, 6) + .into_iter() + .map(|e| ImporterDep { + importer: importer.key.clone(), + dep: e.dep, + spec: e.spec.map(|(_, s)| s), + ver: e.ver.map(|(_, v)| v), + }), + ); + i = importer.end; + } + out +} + // ───────────────────────── package.json override ───────────────────────── /// Add/refresh `pnpm.overrides[@] = file:` on the @@ -2390,30 +2369,50 @@ pub(super) fn overrides_record( } } -/// Locate a dep entry's `specifier:`/`version:` field lines at `indent` -/// (4 for legacy root deps, 8 for modern importers). Returns their values plus -/// the index of the first non-field line. -#[allow(clippy::type_complexity)] -pub(super) fn dep_field_lines( +/// One dependency entry with nested `specifier:` / `version:` fields (each +/// with its line): a v9 importer dep (key at 6, fields at 8) or a legacy +/// 6.0 root dep (2 / 4). +pub(super) struct DepEntry { + pub(super) dep: String, + pub(super) spec: Option<(usize, String)>, + pub(super) ver: Option<(usize, String)>, +} + +/// Every [`DepEntry`] keyed at `indent` in `[start, end)`, in order. A key +/// with an inline value is not one; unknown fields are skipped. +pub(super) fn dep_entries( lines: &[String], - mut f: usize, + start: usize, end: usize, indent: usize, -) -> (Option<(usize, String)>, Option<(usize, String)>, usize) { - let mut spec = None; - let mut ver = None; - while f < end { - let Some((field, _repr, fval)) = parse_key_line(&lines[f], indent) else { - break; +) -> Vec { + let mut out = Vec::new(); + let mut k = start; + while k < end { + let Some((dep, _repr, "")) = parse_key_line(&lines[k], indent) else { + k += 1; + continue; }; - match field { - "specifier" => spec = Some((f, fval.to_string())), - "version" => ver = Some((f, fval.to_string())), - _ => {} + let mut entry = DepEntry { + dep: dep.to_string(), + spec: None, + ver: None, + }; + k += 1; + while let Some((field, _repr, value)) = lines[..end] + .get(k) + .and_then(|l| parse_key_line(l, indent + 2)) + { + match field { + "specifier" => entry.spec = Some((k, value.to_string())), + "version" => entry.ver = Some((k, value.to_string())), + _ => {} + } + k += 1; } - f += 1; + out.push(entry); } - (spec, ver, f) + out } /// Edit 2: every importer's dep entry for the exact `name@version` — @@ -2430,92 +2429,40 @@ fn edit_importers( let mut changed = false; let mut i = start + 1; while let Some(importer) = next_block(lines.read(), i, end) { - let importer_key = importer.key.clone(); + let importer_key = importer.key; // Dep entries sit at 6-space indent under the 4-space dep-type // headers; their fields at 8. - let mut k = importer.header + 1; - while k < importer.end { - let Some((dep, _repr, rest)) = parse_key_line(&lines.read()[k], 6) else { - k += 1; + for entry in dep_entries(lines.read(), importer.header + 1, importer.end, 6) { + let (Some((si, old_spec)), Some((vi, old_ver))) = (entry.spec, entry.ver) else { continue; }; - if dep != ctx.name || !rest.is_empty() { - k += 1; + let target = old_ver == ctx.version || (old_ver != ctx.spec && ctx.is_ours(&old_ver)); + if entry.dep != ctx.name || !target { continue; } - let (spec_idx, ver_idx, f) = dep_field_lines(lines.read(), k + 1, importer.end, 8); - if let (Some((si, old_spec)), Some((vi, old_ver))) = (spec_idx, ver_idx) { - let target = - old_ver == ctx.version || (old_ver != ctx.spec && ctx.is_ours(&old_ver)); - if target { - let was_ours = ctx.is_ours(&old_ver); - let importer_spec = ctx.spec_for_importer(&importer_key); - // Built before the splices below: `dep` borrows `lines[k]`. - let record = WiringRecord { - file: PNPM_LOCK.to_string(), - kind: KIND_LOCK_IMPORTER_DEP.to_string(), - action: WiringAction::Rewritten, - key: Some(format!("{importer_key}|{dep}")), - original: if was_ours { - None - } else { - Some(serde_json::json!({ - "specifier": old_spec, - "version": old_ver, - })) - }, - new: Some(serde_json::json!({ - "specifier": importer_spec, - "version": ctx.spec, - })), - }; - let lines = lines.write(); - lines[si] = format!(" specifier: {importer_spec}"); - lines[vi] = format!(" version: {}", ctx.spec); - wiring.push(record); - changed = true; - } - } - k = f; + let importer_spec = ctx.spec_for_importer(&importer_key); + wiring.push(WiringRecord { + file: PNPM_LOCK.to_string(), + kind: KIND_LOCK_IMPORTER_DEP.to_string(), + action: WiringAction::Rewritten, + key: Some(format!("{importer_key}|{}", entry.dep)), + original: (!ctx.is_ours(&old_ver)) + .then(|| serde_json::json!({ "specifier": old_spec, "version": old_ver })), + new: Some(serde_json::json!({ + "specifier": importer_spec, + "version": ctx.spec, + })), + }); + let lines = lines.write(); + lines[si] = format!(" specifier: {importer_spec}"); + lines[vi] = format!(" version: {}", ctx.spec); + changed = true; } i = importer.end; } Ok(changed) } -/// Fail closed on a half-drifted lock: when a `[start, end)` section -/// carries BOTH the registry-keyed entry and a socket file:-keyed entry -/// for this package, a rekey would splice a DUPLICATE mapping key (pnpm -/// refuses to parse those) and surgery cannot decide which block carries -/// the truth. -/// -/// `matches` is the section's [`matching_blocks`]: every other block is -/// neither, so it cannot change the answer. -fn check_no_split_entry( - matches: &[YamlBlock], - ctx: &EditCtx<'_>, - section: &str, -) -> Result<(), String> { - let reg_key = ctx.reg_key(); - let mut has_registry = false; - let mut has_ours = false; - for block in matches { - if block.key == reg_key { - has_registry = true; - } else if ctx.is_ours_key(&block.key) { - has_ours = true; - } - } - if has_registry && has_ours { - return Err(format!( - "{section} section carries BOTH `{reg_key}` and a `{}@file:…` entry (a \ - half-edited lock); run `pnpm install` to re-resolve it, then re-vendor", - ctx.name - )); - } - Ok(()) -} - /// Edit 3: rekey the `packages:` entry and rewrite its body — /// `resolution: {integrity: , tarball: }`, a `version:` line /// inserted after it, `deprecated:` dropped, everything else verbatim. @@ -2524,16 +2471,15 @@ fn edit_packages( ctx: &EditCtx<'_>, wiring: &mut Vec, ) -> Result { - let (start, end) = lines.bounds("packages").ok_or("no packages: section")?; + let bounds = lines.bounds("packages").ok_or("no packages: section")?; let reg_key = ctx.reg_key(); let new_key = ctx.new_key(); - let matches = matching_blocks(lines, "packages", start, end, ctx); - check_no_split_entry(&matches, ctx, "packages")?; + let matches = target_blocks(lines, "packages", bounds, ctx.name, ctx.version)?; // The first registry-or-ours block is the one rewritten (every path // below returns). if let Some(block) = matches.into_iter().next() { - let is_ours_key = ctx.is_ours_key(&block.key); + let is_ours_key = ctx.target_key(&block.key) == Some(true); let original_lines: Vec = lines.read()[block.header..block.end].to_vec(); let expected_resolution = format!( " resolution: {{integrity: {}, tarball: {}}}", @@ -2605,17 +2551,16 @@ fn edit_snapshot_rekey( ctx: &EditCtx<'_>, wiring: &mut Vec, ) -> Result { - let Some((start, end)) = lines.bounds("snapshots") else { + let Some(bounds) = lines.bounds("snapshots") else { return Ok(false); // a lock without snapshots has nothing to rekey }; let reg_key = ctx.reg_key(); let new_key = ctx.new_key(); - let matches = matching_blocks(lines, "snapshots", start, end, ctx); - check_no_split_entry(&matches, ctx, "snapshots")?; + let matches = target_blocks(lines, "snapshots", bounds, ctx.name, ctx.version)?; // The first registry-or-ours block decides (every path below returns). if let Some(block) = matches.into_iter().next() { - let is_ours_key = ctx.is_ours_key(&block.key); + let is_ours_key = ctx.target_key(&block.key) == Some(true); if block.key == new_key { return Ok(false); // in sync } @@ -2725,24 +2670,23 @@ fn edit_snapshot_refs( Ok(changed) } -/// The blocks of section `name` (`[start, end)`, its current bounds) whose -/// key is the registry key `name@version` or our `name@file:` key for this -/// version, in lock order — the only blocks the packages/snapshots edits -/// act on. From the index while the lines are the memoized split (its -/// candidates are a superset, filtered by the same predicate), else by the -/// scan. +/// The blocks of `section` (`[start, end)`, its current bounds) whose key is +/// a [`target_key`] of `name@version`, in lock order — the only blocks the +/// packages/snapshots edits act on. From the index while the lines are the +/// memoized split (its candidates are a superset, filtered by the same +/// predicate), else by the scan. fn matching_blocks( lines: &L, - name: &str, + section: &str, start: usize, end: usize, - ctx: &EditCtx<'_>, + name: &str, + version: &str, ) -> Vec { - let reg_key = ctx.reg_key(); - let keep = |key: &str| key == reg_key || ctx.is_ours_key(key); - if let Some(section) = lines.index().and_then(|index| index.section(name)) { + let keep = |key: &str| target_key(key, name, version).is_some(); + if let Some(section) = lines.index().and_then(|index| index.section(section)) { return section - .candidates(®_key, ctx.name) + .candidates(name, version) .into_iter() .filter(|block| keep(&block.key)) .map(IndexedBlock::to_block) @@ -2760,6 +2704,32 @@ fn matching_blocks( found } +/// [`matching_blocks`], failing closed on a half-drifted lock: when the +/// section carries BOTH the registry entry and a rekey of it, a rekey would +/// splice a DUPLICATE mapping key (pnpm refuses to parse those) and surgery +/// cannot decide which block carries the truth. +pub(super) fn target_blocks( + lines: &L, + section: &str, + (start, end): (usize, usize), + name: &str, + version: &str, +) -> Result, String> { + let matches = matching_blocks(lines, section, start, end, name, version); + let rekeyed = |want| move |b: &&YamlBlock| target_key(&b.key, name, version) == Some(want); + match ( + matches.iter().find(rekeyed(false)), + matches.iter().any(|b| rekeyed(true)(&b)), + ) { + (Some(registry), true) => Err(format!( + "{section} section carries BOTH `{}` and a vendored `file:` rekey of it (a \ + half-edited lock); run `pnpm install` to re-resolve it, then re-vendor", + registry.key + )), + _ => Ok(matches), + } +} + // ─────────────────────── memoized split + section index ──────────────────── /// The run's `pnpm-lock.yaml` split. A vendored run reads the lock once per @@ -2839,7 +2809,7 @@ impl std::ops::Deref for LockLines { /// Read / write access to lock lines for the edits, which run both on the /// backend's [`LockLines`] and on a plain `Vec` (unit tests). `index` is /// `Some` only while the lines are exactly the ones it was built from. -trait EditLines { +pub(super) trait EditLines { fn read(&self) -> &Vec; fn write(&mut self) -> &mut Vec; fn index(&self) -> Option<&LockIndex> { @@ -2922,8 +2892,9 @@ impl IndexedBlock { /// The blocks of one top-level section, in lock order, as the scans walk /// them ([`section_bounds`] + [`next_block`]), with two lookups: blocks by -/// exact key, and blocks by the key text before each `@file:` in it (so -/// `name` finds every key starting `name@file:`). +/// key (one leading `/` dropped, as [`match_key`] reads it), and blocks by +/// the key text before each `@file:` in it (so `name` finds every key +/// starting `name@file:`, and `""` every bare `file:` key). #[derive(Default)] struct SectionIndex { bounds: Option<(usize, usize)>, @@ -2945,11 +2916,12 @@ impl SectionIndex { while let Some(block) = next_block(lines, i, end) { i = block.end; let at = index.blocks.len(); - index.by_key.entry(block.key.clone()).or_default().push(at); - for (p, _) in block.key.match_indices("@file:") { + let key = block.key.strip_prefix('/').unwrap_or(&block.key); + index.by_key.entry(key.to_string()).or_default().push(at); + for (p, _) in format!("@{}", block.key).match_indices("@file:") { index .by_file_prefix - .entry(block.key[..p].to_string()) + .entry(block.key[..p.saturating_sub(1)].to_string()) .or_default() .push(at); } @@ -2964,14 +2936,18 @@ impl SectionIndex { index } - /// Every block whose key is `reg_key` or starts with `name@file:`, in - /// lock order. - fn candidates(&self, reg_key: &str, name: &str) -> Vec<&IndexedBlock> { - let mut at: Vec = self - .by_key - .get(reg_key) - .into_iter() - .chain(self.by_file_prefix.get(name)) + /// Every block whose key may name `name@version` without a suffix + /// ([`match_key`]): its registry key in either separator, or a + /// `name@file:` / bare `file:` key, in lock order. + fn candidates(&self, name: &str, version: &str) -> Vec<&IndexedBlock> { + let mut at: Vec = [format!("{name}@{version}"), format!("{name}/{version}")] + .iter() + .filter_map(|key| self.by_key.get(key)) + .chain( + [name, ""] + .iter() + .filter_map(|n| self.by_file_prefix.get(*n)), + ) .flatten() .copied() .collect(); @@ -2992,7 +2968,7 @@ struct ImporterDep { } /// A position the snapshot scan of [`check_rewritable_refs`] may refuse -/// at: a block header (by block) or a body line. +/// at: a block header or a body line (by line). enum SnapshotCandidate { Key(usize), Body(usize), @@ -3004,7 +2980,7 @@ enum SnapshotCandidate { /// its package up instead of scanning; `*_paren` maps key on the text /// before each `(` (so `x` finds everything starting `x(`). #[derive(Default)] -struct LockIndex { +pub(super) struct LockIndex { /// First line index of every column-0 line, and all column-0 lines in /// order: [`section_bounds`] for any section. headers: HashMap, @@ -3086,53 +3062,33 @@ impl LockIndex { } } - if let Some((start, end)) = section_bounds(lines, "importers") { - let mut i = start + 1; - while let Some(importer) = next_block(lines, i, end) { - let mut k = importer.header + 1; - while k < importer.end { - let Some((dep, _repr, rest)) = parse_key_line(&lines[k], 6) else { - k += 1; - continue; - }; - if !rest.is_empty() { - k += 1; - continue; - } - let (spec, ver, f) = dep_field_lines(lines, k + 1, importer.end, 8); - if let Some((_, v)) = &ver { - let at = index.importer_deps.len(); - let v = unquote_value(v); - index.first_importer_ver.entry(v.to_string()).or_insert(at); - for prefix in paren_prefixes(v) { - index - .first_importer_ver_paren - .entry(prefix.to_string()) - .or_insert(at); - index - .first_importer_dep_ver_paren - .entry((dep.to_string(), prefix.to_string())) - .or_insert(at); - } - if spec - .as_ref() - .is_some_and(|(_, s)| unquote_value(s).starts_with("catalog:")) - { - index - .first_importer_catalog - .entry((dep.to_string(), v.to_string())) - .or_insert(at); - } - index.importer_deps.push(ImporterDep { - importer: importer.key.clone(), - dep: dep.to_string(), - spec: spec.map(|(_, s)| s), - ver: ver.map(|(_, v)| v), - }); - } - k = f; - } - i = importer.end; + index.importer_deps = importer_deps(lines); + index.importer_deps.retain(|entry| entry.ver.is_some()); + for (at, entry) in index.importer_deps.iter().enumerate() { + let (dep, v) = ( + &entry.dep, + unquote_value(entry.ver.as_deref().unwrap_or("")), + ); + index.first_importer_ver.entry(v.to_string()).or_insert(at); + for prefix in paren_prefixes(v) { + index + .first_importer_ver_paren + .entry(prefix.to_string()) + .or_insert(at); + index + .first_importer_dep_ver_paren + .entry((dep.clone(), prefix.to_string())) + .or_insert(at); + } + if entry + .spec + .as_deref() + .is_some_and(|s| unquote_value(s).starts_with("catalog:")) + { + index + .first_importer_catalog + .entry((dep.clone(), v.to_string())) + .or_insert(at); } } index @@ -3166,10 +3122,8 @@ impl LockIndex { ) -> Vec { let mut found: Vec<(usize, SnapshotCandidate)> = Vec::new(); if let Some(&block) = self.first_snapshot_key_paren.get(reg_key) { - found.push(( - self.snapshots.blocks[block].header, - SnapshotCandidate::Key(block), - )); + let header = self.snapshots.blocks[block].header; + found.push((header, SnapshotCandidate::Key(header))); } let body = [ self.first_snapshot_rest.get(reg_key), @@ -3413,84 +3367,106 @@ fn revert_importer_dep( )); return; }; + let pair = DepPair { + rec, + key, + dep, + entry_uuid, + label: "importer dep", + }; let mut i = start + 1; while let Some(importer) = next_block(lines, i, end) { - if importer.key != importer_key { - i = importer.end; - continue; + if importer.key == importer_key { + return pair.restore(lines, importer.header + 1, importer.end, 6, dirty, warnings); } - let mut k = importer.header + 1; - while k < importer.end { - let Some((d, _repr, rest)) = parse_key_line(&lines[k], 6) else { - k += 1; - continue; - }; - if d != dep || !rest.is_empty() { - k += 1; - continue; - } - let (spec_idx, ver_idx, _) = dep_field_lines(lines, k + 1, importer.end, 8); - let (Some((si, live_spec)), Some((vi, live_ver))) = (spec_idx, ver_idx) else { - break; - }; - // ALREADY CONVERGED: both fields already equal the recorded - // pre-vendor original — an earlier partial revert (or the - // user, by hand) already restored this record. Not drift: - // stay silent so the drift-skip keep gate can converge. - if let Some(original) = rec.original.as_ref() { - if original.get("specifier").and_then(Value::as_str) == Some(live_spec.as_str()) - && original.get("version").and_then(Value::as_str) == Some(live_ver.as_str()) - { - return; - } - } - let new_ver = rec - .new - .as_ref() - .and_then(|n| n.get("version")) - .and_then(Value::as_str); - let ours = Some(live_ver.as_str()) == new_ver - || parse_vendor_path(&live_ver) - .is_some_and(|p| p.eco == "npm" && p.uuid == entry_uuid); - if !ours { - warnings.push(drifted(format!( - "importer dep `{key}` was re-resolved since vendoring ({live_ver}); left alone" - ))); - return; - } - let Some(original) = rec.original.as_ref() else { - warnings.push(drifted(format!( - "importer dep `{key}` has no recorded pre-vendor original; left as-is \ - (re-run `pnpm install` to re-resolve it)" - ))); - return; - }; - let (Some(orig_spec), Some(orig_ver)) = ( - original.get("specifier").and_then(Value::as_str), - original.get("version").and_then(Value::as_str), - ) else { - warnings.push(drifted(format!( - "importer dep `{key}` original is malformed" - ))); - return; - }; - lines[si] = format!(" specifier: {orig_spec}"); - lines[vi] = format!(" version: {orig_ver}"); - *dirty = true; + i = importer.end; + } + pair.removed(warnings); +} + +/// One recorded `specifier:` / `version:` pair to restore: a v9 importer +/// dep or a legacy 6.0 root dep. +pub(super) struct DepPair<'a> { + pub(super) rec: &'a WiringRecord, + pub(super) key: &'a str, + pub(super) dep: &'a str, + pub(super) entry_uuid: &'a str, + /// Warning prefix (`importer dep` / `root dep`). + pub(super) label: &'a str, +} + +impl DepPair<'_> { + fn removed(&self, warnings: &mut Vec) { + warnings.push(removed(format!( + "{} `{}` no longer exists; nothing to restore", + self.label, self.key + ))); + } + + /// Restore the pair of the `dep` entry at `indent` in `[start, end)` + /// ([`dep_entries`]); a missing entry is "removed". + pub(super) fn restore( + &self, + lines: &mut [String], + start: usize, + end: usize, + indent: usize, + dirty: &mut bool, + warnings: &mut Vec, + ) { + let (rec, key, label) = (self.rec, self.key, self.label); + let entry = dep_entries(lines, start, end, indent) + .into_iter() + .find(|e| e.dep == self.dep); + let Some((Some((si, live_spec)), Some((vi, live_ver)))) = entry.map(|e| (e.spec, e.ver)) + else { + return self.removed(warnings); + }; + // ALREADY CONVERGED: both fields already equal the recorded + // pre-vendor original — an earlier partial revert (or the user, by + // hand) already restored this record. Not drift: stay silent so the + // drift-skip keep gate can converge. + fn field<'v>(v: Option<&'v Value>, f: &str) -> Option<&'v str> { + v.and_then(|v| v.get(f)).and_then(Value::as_str) + } + let original = rec.original.as_ref(); + if original.is_some() + && field(original, "specifier") == Some(&live_spec) + && field(original, "version") == Some(&live_ver) + { return; } - break; + let ours = field(rec.new.as_ref(), "version") == Some(&live_ver) + || parse_vendor_path(&live_ver) + .is_some_and(|p| p.eco == "npm" && p.uuid == self.entry_uuid); + if !ours { + return warnings.push(drifted(format!( + "{label} `{key}` was re-resolved since vendoring ({live_ver}); left alone" + ))); + } + if original.is_none() { + return warnings.push(drifted(format!( + "{label} `{key}` has no recorded pre-vendor original; left as-is \ + (re-run `pnpm install` to re-resolve it)" + ))); + } + let (Some(orig_spec), Some(orig_ver)) = + (field(original, "specifier"), field(original, "version")) + else { + return warnings.push(drifted(format!("{label} `{key}` original is malformed"))); + }; + let pad = " ".repeat(indent + 2); + lines[si] = format!("{pad}specifier: {orig_spec}"); + lines[vi] = format!("{pad}version: {orig_ver}"); + *dirty = true; } - warnings.push(removed(format!( - "importer dep `{key}` no longer exists; nothing to restore" - ))); } /// Restore a rekeyed packages/snapshots block: locate the block by the NEW /// key (from `rec.new`'s first line), verify ownership, splice the original /// lines back — merged with any dependency ref ANOTHER entry rewrote inside /// the block since (#830, [`merge_live_dep_refs`]). -fn revert_block( +pub(super) fn revert_block( lines: &mut Vec, rec: &WiringRecord, key: &str, @@ -3525,11 +3501,12 @@ fn revert_block( continue; } // Ours iff the live block is exactly what we wrote, or its key still - // points into OUR uuid dir (a re-serialized but unmoved entry). + // points into OUR uuid dir (a re-serialized but unmoved entry): v9's + // `name@file:` or legacy's bare `file:`. let live: Vec = lines[block.header..block.end].to_vec(); - let key_is_ours = new_key.rsplit_once("@file:").is_some_and(|(_, p)| { - parse_vendor_path(p).is_some_and(|v| v.eco == "npm" && v.uuid == entry_uuid) - }); + let path = new_key.rsplit_once("@file:").map_or(new_key, |(_, p)| p); + let key_is_ours = + parse_vendor_path(path).is_some_and(|v| v.eco == "npm" && v.uuid == entry_uuid); if live != new_lines && !key_is_ours { warnings.push(drifted(format!( "{section} entry `{new_key}` was changed since vendoring; left alone" @@ -3544,7 +3521,25 @@ fn revert_block( return; }; let restored = merge_live_dep_refs(&original, &new_lines, &live); - lines.splice(block.header..block.end, restored); + // Legacy's bare `file:` rekey moved the block across the `/` vs + // `file:` sort boundary, so it goes back to its byte-sorted place + // (byte-identity with the pre-vendor lock); v9 rekeys in place. + if !new_key.starts_with("file:") { + lines.splice(block.header..block.end, restored); + } else if let Some(orig_key) = original.first().and_then(|l| parse_key_line(l, 2)) { + let orig_key = orig_key.0.to_string(); + if swap_block_sorted(lines, section, new_key, &orig_key, &restored).is_err() { + warnings.push(drifted(format!( + "{section} entry `{new_key}` vanished mid-restore; left alone" + ))); + return; + } + } else { + warnings.push(drifted(format!( + "{section} entry `{key}` original is malformed" + ))); + return; + } *dirty = true; return; } @@ -5702,28 +5697,44 @@ snapshots: assert!(matches!(fx.vendor(false).await, VendorOutcome::Done { .. })); } - /// A CRLF pnpm-lock.yaml (Windows autocrlf checkout) passes the version - /// sniff (`str::lines` strips `\r`) but breaks every structural probe — - /// the refusal must name the line endings as the cause, not the - /// misleading "no packages entry … run `pnpm install`". + /// Divergences N15: a CRLF pnpm-lock.yaml (a Windows autocrlf checkout) + /// is wired like its LF twin and keeps its CRLF endings, as hosted + /// mode's byte splices keep them, and the revert restores the CRLF + /// bytes. Vendoring used to refuse it (`vendor_lockfile_crlf_unsupported`). #[tokio::test] - async fn crlf_lock_refuses_naming_line_endings() { + async fn crlf_lock_is_wired_like_its_lf_twin_and_stays_crlf() { let crlf_lock = P1_BEFORE_LOCK.replace('\n', "\r\n"); let fx = fixture_with(P1_BEFORE_PKG, &crlf_lock).await; + let (result, entry, _) = expect_done(fx.vendor(false).await); + assert!(result.success, "{:?}", result.error); + let wired = P1_AFTER_LOCK.replace(SPIKE_INTEGRITY, &fx.actual_integrity().await); + assert_eq!(fx.read(PNPM_LOCK).await, wired.replace('\n', "\r\n")); + assert_eq!(fx.read(PACKAGE_JSON).await, P1_AFTER_PKG); + + let outcome = revert_pnpm(&entry.unwrap(), fx.root(), false).await; + assert!(outcome.success, "{:?}", outcome.error); + assert!(outcome.warnings.is_empty(), "{:?}", outcome.warnings); + assert_eq!(fx.read(PNPM_LOCK).await, crlf_lock, "CRLF bytes restored"); + assert_eq!(fx.read(PACKAGE_JSON).await, P1_BEFORE_PKG); + assert!(!fx.root().join(".socket/vendor/npm").join(UUID).exists()); + } + + /// N15's one remaining refusal: a lock mixing CRLF and LF has no single + /// ending to write back, so it is refused naming the line endings (not + /// the misleading "no packages entry … run `pnpm install`"). + #[tokio::test] + async fn mixed_line_ending_lock_refuses_naming_line_endings() { + let mixed = P1_BEFORE_LOCK.replacen('\n', "\r\n", 1); + let fx = fixture_with(P1_BEFORE_PKG, &mixed).await; let detail = expect_refused(fx.vendor(false).await, "vendor_lockfile_crlf_unsupported"); - assert!(detail.contains("CRLF"), "{detail}"); - assert!( - !detail.contains("make sure the package is installed"), - "no misleading install remedy: {detail}" - ); - assert_eq!(fx.read(PNPM_LOCK).await, crlf_lock, "lock untouched"); + assert!(detail.contains("mixes CRLF and LF"), "{detail}"); + assert_eq!(fx.read(PNPM_LOCK).await, mixed, "lock untouched"); assert_eq!(fx.read(PACKAGE_JSON).await, P1_BEFORE_PKG, "pkg untouched"); assert!( !fx.root().join(".socket/vendor").exists(), "refusal stages no artifact" ); } - #[tokio::test] async fn dry_run_writes_nothing() { let fx = fixture_with(P1_BEFORE_PKG, P1_BEFORE_LOCK).await; @@ -6361,7 +6372,9 @@ snapshots: fn lock_version_remedy_is_version_aware() { use crate::formats::pnpm::{sniff_lock_grammar, PnpmLockGrammar}; - assert!(check_lock_version("lockfileVersion: '9.0'\n").is_ok()); + assert!(PnpmDialect::V9 + .check_lock("lockfileVersion: '9.0'\n") + .is_ok()); assert_eq!( sniff_lock_grammar("lockfileVersion: '9.0'\n"), Ok(PnpmLockGrammar::V9) @@ -6381,7 +6394,7 @@ snapshots: // …while the v9 BACKEND's own guard still holds them out (they can // only reach `vendor_pnpm` through a router bug). for head in ["lockfileVersion: 5.4\n", "lockfileVersion: '6.0'\n"] { - assert!(check_lock_version(head).is_err(), "{head}"); + assert!(PnpmDialect::V9.check_lock(head).is_err(), "{head}"); } // Too old for the allowlist (pnpm 6's 5.3): upgrade pnpm. @@ -6395,14 +6408,14 @@ snapshots: assert!(err.contains("update socket-patch"), "{err}"); assert!(!err.contains("re-lock with pnpm >= 9"), "{err}"); // The v9 backend's guard agrees on the direction. - let err = check_lock_version(head).unwrap_err(); + let err = PnpmDialect::V9.check_lock(head).unwrap_err(); assert!(err.contains("update socket-patch"), "{err}"); } // No version in the head: unrecognizably old — upgrade advice stands. let err = sniff_lock_grammar("importers:\n").unwrap_err(); assert!(err.contains("re-lock with pnpm >= 9"), "{err}"); - assert!(check_lock_version("importers:\n").is_err()); + assert!(PnpmDialect::V9.check_lock("importers:\n").is_err()); } #[test] @@ -7256,69 +7269,66 @@ snapshots: assert_eq!(fx.read(PNPM_LOCK).await, P1_BEFORE_LOCK, "lock untouched"); } - /// A CRLF pnpm-workspace.yaml (Windows autocrlf checkout) dodges every - /// structural probe — the conflict checks miss the existing `overrides:` - /// section and the edit appends a DUPLICATE one, which pnpm rejects as a - /// duplicated mapping key (bricking every pnpm command). Same fail-closed - /// posture as the lock's CRLF refusal. + /// N15 for pnpm-workspace.yaml: a CRLF file (Windows autocrlf checkout) + /// is read like its LF twin, so the existing `overrides:` section is found + /// and extended (never a DUPLICATE section, which pnpm rejects), and the + /// file keeps its CRLF endings. Vendoring used to refuse the file. #[tokio::test] - async fn crlf_workspace_file_refuses_instead_of_duplicating_overrides() { + async fn crlf_workspace_file_gains_the_override_in_its_own_section() { let fx = fixture_with(P1_BEFORE_PKG, P1_BEFORE_LOCK).await; let crlf_ws = "packages:\r\n - 'packages/*'\r\noverrides:\r\n other-pkg: 2.0.0\r\n"; write_ws(&fx, crlf_ws).await; - let detail = expect_refused(fx.vendor(false).await, "vendor_lockfile_crlf_unsupported"); - assert!(detail.contains(PNPM_WORKSPACE), "{detail}"); - assert!(detail.contains("CRLF"), "{detail}"); + let (result, entry, _) = expect_done(fx.vendor(false).await); + assert!(result.success, "{:?}", result.error); + let ws = fx.read(PNPM_WORKSPACE).await; + assert_eq!(ws.matches("overrides:").count(), 1, "{ws:?}"); + assert!(ws.contains(" other-pkg: 2.0.0\r\n"), "{ws:?}"); + assert!( + ws.contains(&format!("left-pad@1.3.0: file:{}\r\n", fx.rel_tgz())), + "{ws:?}" + ); + assert!(!ws.replace("\r\n", "").contains('\n'), "stays CRLF: {ws:?}"); + + let outcome = revert_pnpm(&entry.unwrap(), fx.root(), false).await; + assert!(outcome.success, "{:?}", outcome.error); assert_eq!( fx.read(PNPM_WORKSPACE).await, crlf_ws, - "workspace untouched" + "CRLF bytes restored" ); - assert_eq!(fx.read(PNPM_LOCK).await, P1_BEFORE_LOCK, "lock untouched"); - assert_eq!(fx.read(PACKAGE_JSON).await, P1_BEFORE_PKG, "pkg untouched"); } - /// The revert twin of the CRLF blindness: a workspace file converted to - /// CRLF since vendoring makes the Added override read as "already - /// converged" (the section probe misses it), so revert would remove the - /// artifact while the live override still points at it — pnpm >= 11 then - /// resolves through a deleted tarball. Must be a drift-keep instead. + /// The revert twin: a project vendored with LF files and then checked + /// out with autocrlf (every text file CRLF) reverts cleanly. The created + /// workspace scaffold reads as the scaffold and is deleted, the lock is + /// restored in CRLF, and the artifact goes. Before N15 the CRLF workspace + /// file was a drift-keep that held the artifact until the user + /// normalized it. #[tokio::test] - async fn crlf_workspace_revert_is_a_drift_keep_not_silent_convergence() { + async fn crlf_checkout_of_a_vendored_project_reverts_cleanly() { let fx = fixture_with(P1_BEFORE_PKG, P1_BEFORE_LOCK).await; let (_, entry, _) = expect_done(fx.vendor(false).await); let entry = entry.unwrap(); - let crlf_ws = fx.read(PNPM_WORKSPACE).await.replace('\n', "\r\n"); - tokio::fs::write(fx.root().join(PNPM_WORKSPACE), &crlf_ws) - .await - .unwrap(); + for file in [PNPM_WORKSPACE, PNPM_LOCK] { + let crlf = fx.read(file).await.replace('\n', "\r\n"); + tokio::fs::write(fx.root().join(file), crlf).await.unwrap(); + } let outcome = revert_pnpm(&entry, fx.root(), false).await; assert!(outcome.success, "{:?}", outcome.error); - assert!( - outcome - .warnings - .iter() - .any(|w| w.code == "vendor_lock_entry_drifted" && w.detail.contains(PNPM_WORKSPACE)), - "CRLF workspace must surface as drift: {:?}", - outcome.warnings - ); - assert!( - outcome.kept_artifact, - "the still-referenced artifact must be kept" - ); - assert!( - fx.root() - .join(format!(".socket/vendor/npm/{UUID}")) - .exists(), - "artifact dir survives the drift-keep" - ); + assert!(outcome.warnings.is_empty(), "{:?}", outcome.warnings); + assert!(!outcome.kept_artifact); + assert!(!fx.root().join(PNPM_WORKSPACE).exists(), "scaffold deleted"); assert_eq!( - fx.read(PNPM_WORKSPACE).await, - crlf_ws, - "CRLF file left alone" + fx.read(PNPM_LOCK).await, + P1_BEFORE_LOCK.replace('\n', "\r\n"), + "lock restored, still CRLF" ); + assert!(!fx + .root() + .join(format!(".socket/vendor/npm/{UUID}")) + .exists()); } /// A CRLF lock (a Windows autocrlf checkout) must never read as @@ -9774,19 +9784,60 @@ snapshots: assert_eq!(check_rewritable_refs(&lines, "left-pad", "1.3.0"), Ok(())); } + #[test] + fn a_sibling_checkouts_rekey_is_not_a_target() { + // Lock-model 2b (divergence N5, vendored planner): the planner reads + // its own rekeyed keys with the planning core's `match_key` + // (`origin::vendor_ref`, root-anchored), as hosted mode and discovery + // do. Before, `parse_vendor_path` found the anchor anywhere, so a + // sibling checkout's `../.socket/vendor/…` rekey was "ours" and got + // rekeyed into this project's artifact. + let art = format!(".socket/vendor/npm/{UUID}/left-pad-1.3.0.tgz"); + let target = |key: &str| target_key(key, "left-pad", "1.3.0"); + assert_eq!(target(&format!("left-pad@file:{art}")), Some(true)); + assert_eq!(target(&format!("file:{art}")), Some(true), "legacy rekey"); + assert_eq!(target("left-pad@1.3.0"), Some(false)); + assert_eq!( + target("/left-pad/1.3.0"), + Some(false), + "legacy registry key" + ); + assert_eq!(target("left-pad@1.3.0(react@18.2.0)"), None, "peer context"); + let sibling = format!("left-pad@file:../{art}"); + assert_eq!(target(&sibling), None); + let lock = split_lines(&format!( + "lockfileVersion: '9.0'\n\npackages:\n\n {sibling}:\n resolution: \ + {{integrity: sha512-x, tarball: file:../{art}}}\n version: 1.3.0\n" + )); + assert!(!lock_has_target_package(&lock, "left-pad", "1.3.0")); + } + /// A dep entry carrying a field that is neither `specifier` nor /// `version` (hand-edited or future-pnpm) is skipped, not misparsed: - /// both known fields are still found and the cursor lands past the - /// whole field run. + /// both known fields are still found and the entry ends at the first + /// non-field line. #[test] - fn dep_field_lines_ignores_unknown_fields() { + fn dep_entries_ignore_unknown_fields() { let lines = split_lines( - " specifier: ^1.0.0\n engines: whatever\n version: 1.0.0\n other:", + " dep:\n specifier: ^1.0.0\n engines: whatever\n version: 1.0.0\n other:\n inline: 1.0.0", + ); + let entries = dep_entries(&lines, 0, lines.len(), 6); + let got: Vec<_> = entries + .iter() + .map(|e| (e.dep.as_str(), e.spec.clone(), e.ver.clone())) + .collect(); + assert_eq!( + got, + vec![ + ( + "dep", + Some((1, "^1.0.0".to_string())), + Some((3, "1.0.0".to_string())) + ), + ("other", None, None), + ], + "fields end at the first non-field line; an inline value is no entry" ); - let (spec, ver, f) = dep_field_lines(&lines, 0, lines.len(), 8); - assert_eq!(spec, Some((0, "^1.0.0".to_string()))); - assert_eq!(ver, Some((2, "1.0.0".to_string()))); - assert_eq!(f, 3, "cursor stops at the first non-field line"); } /// The importer/snapshot edits are no-ops on a lock without their @@ -10122,7 +10173,7 @@ snapshots: let peer_suffixed = P1_BEFORE_LOCK.replace(" left-pad@1.3.0: {}", " left-pad@1.3.0(react@18.2.0): {}"); let absent = P1_BEFORE_LOCK.replace("left-pad@1.3.0", "left-pad@1.3.1"); - let crlf = P1_BEFORE_LOCK.replace('\n', "\r\n"); + let mixed = P1_BEFORE_LOCK.replacen('\n', "\r\n", 1); let legacy = P1_BEFORE_LOCK.replace("lockfileVersion: '9.0'", "lockfileVersion: '6.0'"); let cases: [(&str, &str, &str, &str); 6] = [ ( @@ -10150,9 +10201,9 @@ snapshots: "vendor_lock_entry_not_found", ), ( - "CRLF lock", + "mixed line-ending lock", P1_BEFORE_PKG, - &crlf, + &mixed, "vendor_lockfile_crlf_unsupported", ), ( @@ -10428,7 +10479,7 @@ snapshots: "seed {seed} refs {name}@{version}\n{text}" ); assert_eq!( - lock_has_target_package_in(&shared(&lines), name, version), + lock_has_target_package(&shared(&lines), name, version), lock_has_target_package(&lines, name, version), "seed {seed} target {name}@{version}\n{text}" ); @@ -10519,7 +10570,9 @@ snapshots: P1_BEFORE_LOCK, ))))), lock_prefix: String::new(), + lock_eol: LineEndings::Lf, ws_text: None, + ws_eol: LineEndings::None, }; let doc_of = |p: &PnpmProject| match &p.lock { ProjectLock::V9(LockLines::Shared(doc)) => Arc::clone(doc), @@ -10593,8 +10646,8 @@ snapshots: let ProjectLock::V9(lines) = &second.lock else { unreachable!() }; - assert!(!lock_has_target_package_in(lines, "left-pad", "1.3.0")); - assert!(lock_has_target_package_in(lines, "right-pad", "1.3.0")); + assert!(!lock_has_target_package(lines, "left-pad", "1.3.0")); + assert!(lock_has_target_package(lines, "right-pad", "1.3.0")); } /// pnpm-workspace.yaml spellings and document shapes the overrides surgery diff --git a/crates/socket-patch-core/src/vendor/pnpm_lock_legacy.rs b/crates/socket-patch-core/src/vendor/pnpm_lock_legacy.rs index 549524546..09fc48634 100644 --- a/crates/socket-patch-core/src/vendor/pnpm_lock_legacy.rs +++ b/crates/socket-patch-core/src/vendor/pnpm_lock_legacy.rs @@ -64,9 +64,9 @@ use crate::utils::fs::read_regular_to_string; use super::common::refused; use super::path::parse_vendor_path; use super::pnpm_lock::{ - dep_field_lines, drifted, lines_value, merge_live_dep_refs, ref_record_blocks, removed, - revert_overrides_line, same_modulo_dep_refs, value_lines, vendor_value_is_for, DepRef, EditCtx, - PnpmDialect, KIND_LOCK_OVERRIDES, KIND_LOCK_PACKAGE, + dep_entries, drifted, lines_value, lock_has_target_package, ref_record_blocks, removed, + revert_block, revert_overrides_line, target_blocks, target_key, vendor_value_is_for, DepPair, + DepRef, EditCtx, PnpmDialect, KIND_LOCK_OVERRIDES, KIND_LOCK_PACKAGE, }; use super::source::PackageSource; use super::state::{VendorEntry, WiringAction, WiringRecord}; @@ -76,7 +76,7 @@ use crate::formats::pnpm::lines::{ next_block, parse_key_line, section_bounds, split_lines, unquote_value, yaml_key, yaml_key_like, yaml_value, }; -use crate::formats::pnpm::{sniff_lock_grammar, PnpmLock, PnpmLockGrammar}; +use crate::formats::pnpm::{PnpmLock, PnpmLockGrammar}; /// The [`VendorEntry::flavor`] string legacy wirings are stamped with. /// Distinct from the v9 backend's `"pnpm"` so an older binary (which has no @@ -230,15 +230,6 @@ pub(super) struct LegacyLock { abs_root: String, } -pub(super) fn check_lock_version(text: &str) -> Result { - match sniff_lock_grammar(text) { - Ok(PnpmLockGrammar::V9) => Err(format!( - "{PNPM_LOCK} is a lockfileVersion 9.0 lock; not a legacy grammar" - )), - result => result, - } -} - pub(super) fn read_lock( lock_text: &str, project_root: &Path, @@ -310,7 +301,7 @@ pub(super) fn preflight_package( if let Err(detail) = check_rewritable_refs(&lock.lines, &ctx) { return Err(Box::new(refused("vendor_lock_entry_unsupported", detail))); } - if !lock_has_target_package(&lock.lines, &ctx) { + if !lock_has_target_package(&lock.lines, name, version) { return Err(Box::new(refused( "vendor_lock_entry_not_found", format!( @@ -341,10 +332,7 @@ pub async fn pnpm_legacy_entry_in_use(entry: &VendorEntry, project_root: &Path) let text = read_regular_to_string(&project_root.join(PNPM_LOCK)) .await .ok()?; - match sniff_lock_grammar(&text) { - Ok(PnpmLockGrammar::V54 | PnpmLockGrammar::V60) => {} - _ => return None, - } + PnpmDialect::Legacy.check_lock(&text).ok()?; Some(PnpmLock::parse(&text).vendored_in_use(&entry.uuid)) } @@ -399,24 +387,6 @@ impl LegacyLock { // ─────────────────────────── pre-flight checks ─────────────────────────── -/// Does the lock have a packages entry vendoring can target — the grammar's -/// registry key, or our rekeyed `file:` key (the in-sync / stale-uuid -/// re-run)? -fn lock_has_target_package(lines: &[String], ctx: &Ctx<'_>) -> bool { - let Some((start, end)) = section_bounds(lines, "packages") else { - return false; - }; - let reg_key = ctx.reg_key(); - let mut i = start + 1; - while let Some(block) = next_block(lines, i, end) { - if block.key == reg_key || ctx.is_ours(&block.key) { - return true; - } - i = block.end; - } - false -} - /// Fail-closed guard against legacy reference forms the surgery does not /// rewrite: PEER-SUFFIXED dep paths (`/name/version_peer…` in v5.4, /// `/name@version(peer…)` in v6.0) and ALIASED references (a dep whose @@ -436,29 +406,20 @@ fn check_rewritable_refs(lines: &[String], ctx: &Ctx<'_>) -> Result<(), String> ctx.name, ctx.version )) }; - // Root dep maps (v5.4 bare values; v6.0 version: fields). + // Root dep maps: v5.4 bare values, v6.0 nested `version:` fields. for section in ROOT_DEP_SECTIONS { let Some((start, end)) = section_bounds(lines, section) else { continue; }; - let mut k = start + 1; - while k < end { - let Some((dep, _repr, rest)) = parse_key_line(&lines[k], 2) else { - k += 1; - continue; - }; - let value = if rest.is_empty() { - // v6.0 nested shape: read the version: field. - let (_, ver, f) = dep_field_lines(lines, k + 1, end, 4); - k = f; - match ver { - Some((_, v)) => v, - None => continue, - } - } else { - k += 1; - rest.to_string() - }; + let bare = lines[start + 1..end] + .iter() + .filter_map(|line| parse_key_line(line, 2)) + .filter(|(_, _, value)| !value.is_empty()) + .map(|(dep, _, value)| (dep.to_string(), value.to_string())); + let nested = dep_entries(lines, start + 1, end, 2) + .into_iter() + .filter_map(|entry| Some((entry.dep, entry.ver?.1))); + for (dep, value) in bare.chain(nested) { if value == reg_key || value.starts_with(&key_peer_prefix) { return refuse("an aliased root dependency", &value); } @@ -617,52 +578,32 @@ fn edit_root_deps_v60( let Some((start, end)) = section_bounds(lines, section) else { continue; }; - let mut k = start + 1; - while k < end { - let Some((dep, _repr, rest)) = parse_key_line(&lines[k], 2) else { - k += 1; + for entry in dep_entries(lines, start + 1, end, 2) { + let (Some((si, old_spec)), Some((vi, old_ver))) = (entry.spec, entry.ver) else { continue; }; - if dep != ctx.name || !rest.is_empty() { - k += 1; + if entry.dep != ctx.name || !(old_ver == ctx.version || ctx.is_ours(&old_ver)) { continue; } - let (spec_f, ver_f, f) = dep_field_lines(lines, k + 1, end, 4); - if let (Some((si, old_spec)), Some((vi, old_ver))) = (spec_f, ver_f) { - let target = old_ver == ctx.version || ctx.is_ours(&old_ver); - if target { - hit = true; - if old_ver == ctx.spec && old_spec == abs_yaml { - k = f; - continue; // in sync - } - let was_ours = ctx.is_ours(&old_ver); - // Built before the splices below: `dep` borrows `lines[k]`. - let record = WiringRecord { - file: PNPM_LOCK.to_string(), - kind: KIND_LOCK_ROOT_DEP_PAIR.to_string(), - action: WiringAction::Rewritten, - key: Some(format!("{section}|{dep}")), - original: if was_ours { - None - } else { - Some(serde_json::json!({ - "specifier": old_spec, - "version": old_ver, - })) - }, - new: Some(serde_json::json!({ - "specifier": ctx.abs_spec, - "version": ctx.spec, - })), - }; - lines[si] = format!(" specifier: {abs_yaml}"); - lines[vi] = format!(" version: {}", ctx.spec); - wiring.push(record); - changed = true; - } + hit = true; + if old_ver == ctx.spec && old_spec == abs_yaml { + continue; // in sync } - k = f; + wiring.push(WiringRecord { + file: PNPM_LOCK.to_string(), + kind: KIND_LOCK_ROOT_DEP_PAIR.to_string(), + action: WiringAction::Rewritten, + key: Some(format!("{section}|{}", entry.dep)), + original: (!ctx.is_ours(&old_ver)) + .then(|| serde_json::json!({ "specifier": old_spec, "version": old_ver })), + new: Some(serde_json::json!({ + "specifier": ctx.abs_spec, + "version": ctx.spec, + })), + }); + lines[si] = format!(" specifier: {abs_yaml}"); + lines[vi] = format!(" version: {}", ctx.spec); + changed = true; } } Ok((changed, hit)) @@ -677,37 +618,14 @@ fn edit_packages( ctx: &Ctx<'_>, wiring: &mut Vec, ) -> Result { - let (start, end) = section_bounds(lines, "packages").ok_or("no packages: section")?; + let bounds = section_bounds(lines, "packages").ok_or("no packages: section")?; let reg_key = ctx.reg_key(); let new_key = ctx.new_key(); - - // Fail closed on a half-drifted lock carrying BOTH spellings. - let mut has_registry = false; - let mut has_ours = false; - let mut j = start + 1; - while let Some(block) = next_block(lines, j, end) { - if block.key == reg_key { - has_registry = true; - } else if ctx.is_ours(&block.key) { - has_ours = true; - } - j = block.end; - } - if has_registry && has_ours { - return Err(format!( - "packages section carries BOTH `{reg_key}` and a `file:…` entry (a \ - half-edited lock); run `pnpm install` to re-resolve it, then re-vendor" - )); - } - - let mut i = start + 1; - while let Some(block) = next_block(lines, i, end) { - let is_registry = block.key == reg_key; - let is_ours_key = ctx.is_ours(&block.key); - if !is_registry && !is_ours_key { - i = block.end; - continue; - } + let matches = target_blocks(&*lines, "packages", bounds, ctx.name, ctx.version)?; + // The first registry-or-ours block is the one rewritten (every path + // below returns). + if let Some(block) = matches.into_iter().next() { + let is_ours_key = target_key(&block.key, ctx.name, ctx.version) == Some(true); let original_lines: Vec = lines[block.header..block.end].to_vec(); let expected_resolution = format!( " resolution: {{integrity: {}, tarball: {}}}", @@ -844,7 +762,7 @@ fn edit_pkg_dep_refs( /// sort before `file:`-keys). pnpm sorts package keys with the `sort-keys` /// default compare (JS code-unit order), which plain Rust `str` ordering /// matches for these ASCII keys. -fn swap_block_sorted( +pub(super) fn swap_block_sorted( lines: &mut Vec, section: &str, old_key: &str, @@ -940,7 +858,7 @@ pub(super) fn revert_lock_record( KIND_LOCK_ROOT_DEP_PAIR => { revert_root_dep_pair(lines, rec, key, entry_uuid, dirty, warnings) } - KIND_LOCK_PACKAGE => revert_package_block(lines, rec, key, entry_uuid, dirty, warnings), + KIND_LOCK_PACKAGE => revert_block(lines, rec, key, "packages", entry_uuid, dirty, warnings), KIND_LOCK_PKG_DEP_REF => revert_pkg_dep_ref(lines, rec, key, entry_uuid, dirty, warnings), other => warnings.push(drifted(format!( "unknown wiring kind `{other}` for `{key}`; left alone" @@ -1030,156 +948,14 @@ fn revert_root_dep_pair( ))); return; }; - let mut k = start + 1; - while k < end { - let Some((d, _repr, rest)) = parse_key_line(&lines[k], 2) else { - k += 1; - continue; - }; - if d != dep || !rest.is_empty() { - k += 1; - continue; - } - let (spec_f, ver_f, _) = dep_field_lines(lines, k + 1, end, 4); - let (Some((si, live_spec)), Some((vi, live_ver))) = (spec_f, ver_f) else { - break; - }; - // ALREADY CONVERGED: both fields already equal the recorded - // pre-vendor original — an earlier partial revert (or the user, by - // hand) already restored this record. Not drift: stay silent so - // the drift-skip keep gate can converge. - if let Some(original) = rec.original.as_ref() { - if original.get("specifier").and_then(Value::as_str) == Some(live_spec.as_str()) - && original.get("version").and_then(Value::as_str) == Some(live_ver.as_str()) - { - return; - } - } - let new_ver = rec - .new - .as_ref() - .and_then(|n| n.get("version")) - .and_then(Value::as_str); - let ours = Some(live_ver.as_str()) == new_ver - || parse_vendor_path(&live_ver).is_some_and(|p| p.eco == "npm" && p.uuid == entry_uuid); - if !ours { - warnings.push(drifted(format!( - "root dep `{key}` was re-resolved since vendoring ({live_ver}); left alone" - ))); - return; - } - let Some(original) = rec.original.as_ref() else { - warnings.push(drifted(format!( - "root dep `{key}` has no recorded pre-vendor original; left as-is \ - (re-run `pnpm install` to re-resolve it)" - ))); - return; - }; - let (Some(orig_spec), Some(orig_ver)) = ( - original.get("specifier").and_then(Value::as_str), - original.get("version").and_then(Value::as_str), - ) else { - warnings.push(drifted(format!("root dep `{key}` original is malformed"))); - return; - }; - lines[si] = format!(" specifier: {orig_spec}"); - lines[vi] = format!(" version: {orig_ver}"); - *dirty = true; - return; - } - warnings.push(removed(format!( - "root dep `{key}` no longer exists; nothing to restore" - ))); -} - -/// Restore the rekeyed packages block: locate it by the NEW `file:` key, -/// verify ownership, then reinsert the ORIGINAL block at its byte-sorted -/// position (the rekey moved it across the `/` vs `file:` sort boundary, so -/// an in-place splice would restore it out of order and break byte-identity -/// with the pre-vendor lock) — merged with any dependency ref ANOTHER entry -/// rewrote inside the block since (#830, [`merge_live_dep_refs`]). -fn revert_package_block( - lines: &mut Vec, - rec: &WiringRecord, - key: &str, - entry_uuid: &str, - dirty: &mut bool, - warnings: &mut Vec, -) { - let Some(new_lines) = rec.new.as_ref().and_then(value_lines) else { - warnings.push(drifted(format!( - "record for `{key}` has no `new` fragment; left alone" - ))); - return; - }; - let Some((new_key, _repr, _rest)) = new_lines.first().and_then(|l| parse_key_line(l, 2)) else { - warnings.push(drifted(format!( - "record for `{key}` has a malformed fragment" - ))); - return; - }; - let Some((start, end)) = section_bounds(lines, "packages") else { - warnings.push(drifted(format!( - "packages section is gone; `{key}` not restored" - ))); - return; + let pair = DepPair { + rec, + key, + dep, + entry_uuid, + label: "root dep", }; - let mut i = start + 1; - while let Some(block) = next_block(lines, i, end) { - if block.key != new_key { - i = block.end; - continue; - } - let live: Vec = lines[block.header..block.end].to_vec(); - let key_is_ours = - parse_vendor_path(new_key).is_some_and(|p| p.eco == "npm" && p.uuid == entry_uuid); - if live != new_lines && !key_is_ours { - warnings.push(drifted(format!( - "packages entry `{new_key}` was changed since vendoring; left alone" - ))); - return; - } - let Some(original) = rec.original.as_ref().and_then(value_lines) else { - warnings.push(drifted(format!( - "packages entry `{key}` has no recorded pre-vendor original; left as-is \ - (re-run `pnpm install` to re-resolve it)" - ))); - return; - }; - let Some((orig_key, _r, _v)) = original.first().and_then(|l| parse_key_line(l, 2)) else { - warnings.push(drifted(format!( - "packages entry `{key}` original is malformed" - ))); - return; - }; - let restored = merge_live_dep_refs(&original, &new_lines, &live); - if swap_block_sorted(lines, "packages", new_key, orig_key, &restored).is_err() { - warnings.push(drifted(format!( - "packages entry `{new_key}` vanished mid-restore; left alone" - ))); - return; - } - *dirty = true; - return; - } - // ALREADY CONVERGED: an earlier partial revert restored this record — - // the restore rekeys the block back to its pre-vendor key, so the - // recorded `file:` key no longer matches while the original block is - // live (up to the dependency refs the merge kept). Not drift: stay - // silent so the drift-skip keep gate can converge instead of keeping - // the artifacts forever. - if let Some(orig) = rec.original.as_ref().and_then(value_lines) { - let mut j = start + 1; - while let Some(block) = next_block(lines, j, end) { - if same_modulo_dep_refs(&lines[block.header..block.end], &orig) { - return; - } - j = block.end; - } - } - warnings.push(removed(format!( - "packages entry `{new_key}` no longer exists; nothing to restore" - ))); + pair.restore(lines, start + 1, end, 2, dirty, warnings); } /// Restore one dependent packages block's dep ref to this entry. Keyed by @@ -2622,14 +2398,21 @@ packages: assert_eq!(fx.read(PNPM_LOCK).await, lock, "refusal writes nothing"); } - /// CRLF and non-allowlisted lock versions refuse with the same codes - /// and byte-untouched files as the v9 backend. + /// Divergences N15 on a legacy lock: a CRLF 5.4 lock is wired like its + /// LF twin (the captured pnpm 7 shape) and stays CRLF; vendoring used to + /// refuse it. Non-allowlisted lock versions still refuse, with + /// byte-untouched files, as the v9 backend does (N16, target-specific). #[tokio::test] - async fn crlf_and_foreign_versions_refuse() { + async fn crlf_is_wired_and_foreign_versions_refuse() { let crlf = T7_BEFORE_LOCK.replace('\n', "\r\n"); let fx = fixture_with(T_BEFORE_PKG, &crlf).await; - expect_refused(fx.vendor(false).await, "vendor_lockfile_crlf_unsupported"); - assert_eq!(fx.read(PNPM_LOCK).await, crlf, "refusal writes nothing"); + let (result, entry, _) = expect_done(fx.vendor(false).await); + assert!(result.success, "{:?}", result.error); + let wired = fx.expected_lock(T7_AFTER_LOCK).await; + assert_eq!(fx.read(PNPM_LOCK).await, wired.replace('\n', "\r\n")); + let outcome = revert_pnpm_legacy(&entry.unwrap(), fx.root(), false).await; + assert!(outcome.success, "{:?}", outcome.error); + assert_eq!(fx.read(PNPM_LOCK).await, crlf, "CRLF bytes restored"); // pnpm 6's 5.3 is NOT allowlisted. let old = T7_BEFORE_LOCK.replace("lockfileVersion: 5.4", "lockfileVersion: 5.3"); @@ -2858,8 +2641,8 @@ packages: ); } - /// Wiring records left alone during a WIRED revert (here: a - /// CRLF-converted lock whose LF-exact probes all miss) must KEEP the + /// Wiring records left alone during a WIRED revert (here: a root dep + /// re-resolved by hand since vendoring) must KEEP the /// artifact dir — the lock still resolves through the tarball, and /// deleting it bricks every subsequent install (the lossiness guard, /// present in the v9/npm/bun backends). @@ -2868,8 +2651,12 @@ packages: let fx = fixture_with(T_BEFORE_PKG, T7_BEFORE_LOCK).await; let (_, entry, _) = expect_done(fx.vendor(false).await); let entry = entry.unwrap(); - let crlf = fx.read(PNPM_LOCK).await.replace('\n', "\r\n"); - tokio::fs::write(fx.root().join(PNPM_LOCK), &crlf) + let drifted = fx.read(PNPM_LOCK).await.replacen( + &format!(" left-pad: file:{}\n", fx.rel_tgz()), + " left-pad: 1.3.1\n", + 1, + ); + tokio::fs::write(fx.root().join(PNPM_LOCK), &drifted) .await .unwrap(); @@ -2890,12 +2677,11 @@ packages: ); assert!( fx.root().join(fx.rel_tgz()).exists(), - "the CRLF lock still resolves through the tarball; deleting it bricks installs" + "the lock still resolves through the tarball; deleting it bricks installs" ); - assert_eq!( - fx.read(PNPM_LOCK).await, - crlf, - "the drifted lock is left alone" + assert!( + fx.read(PNPM_LOCK).await.contains(" left-pad: 1.3.1\n"), + "the drifted line is left alone" ); } From 4a84f2da4ef3cdf6e3b18a5bfe36b1cbb57bf4e7 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Thu, 5 Nov 2026 13:23:55 -0600 Subject: [PATCH 2/5] Mark N5, N15 and N16 for the pnpm planner merge; rebudget N5's vendored-planner part and N15 are resolved in PR 2b. N16 stays target-specific for pnpm vendored mode. scripts/loc-budget.py --lower: lock-readers 31207 -> 31201, lock-writers 88314 -> 88093. Co-Authored-By: Claude Opus 5.5 --- docs/design/lock-model-divergences.md | 6 +++--- scripts/loc-budget.json | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/design/lock-model-divergences.md b/docs/design/lock-model-divergences.md index a25f67760..c77e7e7f3 100644 --- a/docs/design/lock-model-divergences.md +++ b/docs/design/lock-model-divergences.md @@ -83,7 +83,7 @@ follow from the install-tree decision. | N2 | npm user patches (#711) and `bun patch` (#367) | Hosted skips that one dependency and records it as refused. Vendored refuses the whole npm v4 lock, or the one bun dependency. Discovery never reads `patchedDependencies` / `"patched"` and attests a pre-#711 hosted pin. | Hosted's per-dependency skip, in discovery too | false attestation | | N3 | A workspace member's stray npm/bun/vlt lock | Discovery treats it as absent. Hosted and vendored refuse. The inventory reads it as the project's lock. | The inventory ignores it | | | N4 | pnpm 11's env-lockfile document | See [Decisions](lock-model-unification.md#decisions-under-the-hosted-rule). `main_document` takes the text after the last `---`, so a crafted multi-document lock chooses its own "project". | Its own tree; documents identified by content, not position. **Resolved in PR 2a**: every reader reads both documents through `PnpmLock`, each entry tagged by document; discovery records carry the env document's install tree (`TreePart::PnpmEnv`) beside the real lock path, and that tree neither contests nor is contested by the project document or another lock; the env document is the FIRST document when its importers carry only `configDependencies` / `packageManagerDependencies` (pnpm's own position rule), and every other document is read as the project. | | -| N5 | What counts as Socket wiring | Discovery and the unwind use the allowlist. The inventory records hosted URLs on any host. `parse_vendor_path` accepts `.socket/vendor/` anywhere, so a `../.socket/vendor/…` entry from a sibling checkout is "ours" to the vendored writer and reverter. The hosted pnpm "vendored?" check is a substring heuristic. | The `origin` classifier everywhere. pnpm part **resolved in PR 2a**: the hosted pnpm "vendored?" check reads the rekeyed packages entry with `origin::vendor_ref`. | revert could rewrite another project's wiring | +| N5 | What counts as Socket wiring | Discovery and the unwind use the allowlist. The inventory records hosted URLs on any host. `parse_vendor_path` accepts `.socket/vendor/` anywhere, so a `../.socket/vendor/…` entry from a sibling checkout is "ours" to the vendored writer and reverter. The hosted pnpm "vendored?" check is a substring heuristic. | The `origin` classifier everywhere. pnpm part **resolved in PR 2a**: the hosted pnpm "vendored?" check reads the rekeyed packages entry with `origin::vendor_ref`. **PR 2b**: both modes read pnpm keys with one rule, `formats::pnpm::plan::match_key` (`origin::vendor_ref` plus the artifact leaf), so the vendored planner no longer takes a sibling checkout's rekey for its own. The vendored reverter still finds its wiring with `parse_vendor_path` (values such as pnpm 7/8's absolute specifier need it). | revert could rewrite another project's wiring | | N6 | `npm-shrinkwrap.json` and `package-lock.json` | Hosted, vendored and discovery read both, with a contest. The inventory reads the shrinkwrap only. | Both, everywhere | | | N7 | npm lockfileVersion | Hosted rewrites v1 and v4. Vendored accepts v2/v3 only. The inventory ignores v1. Discovery reads v1 and v2+. | Hosted, once vendored `file:` wiring under npm 6 and npm 12.1 v4 has fixtures | | | N8 | npm `file:` directory link identity (#1481) | Hosted and discovery give a versionless link its source directory's `name@version`. Vendored matches the entry's own version, wiring the registry copies with no warning. | Hosted | silent vendored success | @@ -93,8 +93,8 @@ follow from the install-tree decision. | N12 | Rush common and subspace locks | Hosted and discovery always read them. The inventory reads them only when there is no root lock (#2003 aligns the listing itself). Vendored refuses Rush. | Always read. Vendored's refusal stays. **Resolved in PR 2a** (the inventory reads them whenever `rush.json` exists). | target-specific (vendored) | | N13 | Plug'n'Play | Vendored refuses. The hosted berry preflight has no PnP gate. The inventory returns an error, so `scan` sees nothing in a project hosted pinned. | The inventory reads PnP locks. Vendored keeps its refusal. | target-specific (vendored) | | N14 | pnpm `lockfile=false` | Hosted refuses. Discovery withholds hosted refs, keeps vendored ones. Vendored proceeds. The inventory ignores the setting. | The modes stay as they are; the inventory follows discovery. **Resolved in PR 2a**: under the setting (not under Rush), the inventory keeps a Socket-hosted pnpm entry but drops its integrity, as discovery withholds the lock pin. | target-specific | -| N15 | CRLF `pnpm-lock.yaml` / `pnpm-workspace.yaml` | Vendored refuses; the rest handle it | Handle it everywhere | | -| N16 | Lock versions | pnpm: vendored accepts only 5.4/6.0/9.0; hosted any but early shrinkwrap. vlt: vendored refuses a lock with no version; hosted rewrites and warns. | Hosted | | +| N15 | CRLF `pnpm-lock.yaml` / `pnpm-workspace.yaml` | Vendored refuses; the rest handle it | Handle it everywhere. **Resolved in PR 2b**: the vendored planner and reverter edit a CRLF lock or workspace file like its LF twin and write it back CRLF. Only a file mixing CRLF and LF is still refused (`vendor_lockfile_crlf_unsupported`), since no single ending writes it back byte-faithfully. | | +| N16 | Lock versions | pnpm: vendored accepts only 5.4/6.0/9.0; hosted any but early shrinkwrap. vlt: vendored refuses a lock with no version; hosted rewrites and warns. | Hosted. **pnpm, PR 2b**: one table, `plan::Target::check_lock`. Vendored keeps its 5.4/6.0/9.0 range: no other version has a captured pnpm `file:` override shape that proves its rekey, so the other versions stay refused, target-specifically. vlt is unchanged. | target-specific (pnpm vendored) | | N17 | Bun workspaces, yarn classic offline mirror | Refused differently by the two modes | Stays | target-specific | | N18 | An unparseable npm lock | Hosted warns and skips. Vendored refuses the primary lock. Discovery diagnoses. The inventory returns nothing silently. | Discovery's diagnostic, surfaced by every consumer | | | N19 | yarn.lock with a stray top-level line | Discovery rejects the file; hosted and the inventory scan its blocks | Discovery's rule | | diff --git a/scripts/loc-budget.json b/scripts/loc-budget.json index b31d39de7..8ffa1aab4 100644 --- a/scripts/loc-budget.json +++ b/scripts/loc-budget.json @@ -17,7 +17,7 @@ "crates/socket-patch-core/src/utils/pipenv.rs", "crates/socket-patch-core/src/crawlers/deno_crawler.rs" ], - "max": 31207 + "max": 31201 }, "lock-writers": { "paths": [ @@ -30,7 +30,7 @@ "crates/socket-patch-core/src/vendor/npm_origin.rs", "crates/socket-patch-core/src/vendor/test_support" ], - "max": 88314 + "max": 88093 } } } From 56392e18261c4fe8c5c5e047b6cfa273ed65be73 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Thu, 5 Nov 2026 14:37:39 -0600 Subject: [PATCH 3/5] Shadow a shrinkwrap.yaml beside a pnpm-lock.yaml (N11) Owner decision on divergence N11: pnpm never reads a pnpm <= 2 shrinkwrap.yaml it has a pnpm-lock.yaml for, so that file is shadowed. The planning core's planned_locks leaves it out, so hosted mode never rewrites it and it no longer takes part in the all-or-nothing gate. Before, an entry the splice cannot repoint, or pnpm 1's early-shrinkwrap head, refused the dependency in every lock. Discovery keeps sweeping it for stale Socket identities, and vendored mode never reads it. A shrinkwrap.yaml that is the only pnpm lock (nested ones are shadowed only by their own sibling) is rewritten as before. No redirect golden changes. Co-Authored-By: Claude Opus 5.5 --- crates/socket-patch-cli/CLI_CONTRACT.md | 2 +- .../src/formats/pnpm/hosted.rs | 37 ++++------ .../src/formats/pnpm/plan.rs | 54 ++++++++++++-- .../src/patch/redirect/mod.rs | 70 +++++++++++++++++++ docs/design/lock-model-divergences.md | 4 +- docs/ecosystems.md | 3 +- 6 files changed, 135 insertions(+), 35 deletions(-) diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index a3ed909d6..3f491176e 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -272,7 +272,7 @@ For a **9.0 root lock**, the CLI ensures `pnpm-workspace.yaml` carries `trustLoc **pnpm per-branch locks (#556)**: with `gitBranchLockfile` on (a top-level `gitBranchLockfile: true` in the root `pnpm-workspace.yaml`; when the YAML leaves the key unset, `git-branch-lockfile=true` in the root `.npmrc`, as pnpm 10 and older read it; on disk also `pnpm_config_git_branch_lockfile` (pnpm 11 and newer) or `npm_config_git_branch_lockfile` (pnpm 10 and older; either when the major is unknown, read as for the `lockfile` setting above) set to `true`; a workspace member with no `pnpm-workspace.yaml` of its own reads the governing ancestor's file and the `.npmrc` beside it), pnpm installs a git branch from its own `pnpm-lock..yaml` whenever that lock exists, in every directory it installs a lock from: the root, and under `sharedWorkspaceLockfile: false` each member's (#492). When the setting is on AND at least one such `pnpm-lock..yaml` exists (at the root, or in a member that installs from its own lock), hosted mode leaves every root pnpm lock (`pnpm-lock.yaml`, `shrinkwrap.yaml`) and `pnpm-workspace.yaml` untouched (every member lock too), confirms no npm pin from them and warns `redirect_pnpm_git_branch_lockfile` (in place of `redirect_pnpm_no_lockfile` when the branch lock is the only one); the detail names the setting and the branch lock(s), and the remedy is to turn `gitBranchLockfile` off and fold the branch locks into `pnpm-lock.yaml` with `pnpm install --merge-git-branch-lockfiles`, then re-run. A vendored→hosted takeover of a pnpm-vendored purl is refused with the same code (the vendored wiring and ledger stay byte-for-byte). Vendored mode refuses the same projects with `vendor_pnpm_git_branch_lockfile`, before any write and in the pre-flight. The setting with no branch lock (main, or a branch whose dependencies never changed) installs from `pnpm-lock.yaml`, which both modes wire as usual; a stray branch lock with the setting off is ignored. The in-memory engine fetches root `pnpm-lock.*.yaml` names presence-only and refuses the same way. -`redirect_pnpm_no_lockfile` names pnpm when installer markers exist without a lock; `redirect_pnpm_entry_vendored` identifies a vendored entry instead of reporting it missing. Supported `shrinkwrap.yaml` files are writable lockfiles, not read-only markers. +`redirect_pnpm_no_lockfile` names pnpm when installer markers exist without a lock; `redirect_pnpm_entry_vendored` identifies a vendored entry instead of reporting it missing. Supported `shrinkwrap.yaml` files are writable lockfiles, not read-only markers, when no `pnpm-lock.yaml` sits beside them. pnpm never reads a `shrinkwrap.yaml` it has a `pnpm-lock.yaml` for, so that one is shadowed: hosted mode never rewrites it and leaves it out of the all-or-nothing gate, so an entry it cannot repoint (or pnpm 1's early shrinkwrap head) refuses nothing, and `vex` only sweeps it for stale Socket identities. Vendored mode never writes it either. **vlt hosted-mode contract**: `scan` / `get --mode hosted` rewrite, in `vlt-lock.json`, every default-registry node of a granted `name@version` (the `''` / `npm` segment or a URL segment equal to the lock's scalar `registry`, both DepID grammars, every peer and modifier variant): slot [2] becomes the granted sha512 and slot [3] the hosted URL (appended to a 3-tuple); the DepID, flags and trailing slots, the line ending and every other byte stay. `options` is never edited and `vlt.json` is only read. A lock with another `lockfileVersion` (decided on the raw JSON token), a BOM, a non-object body or a `nodes` section outside vlt's one-node-per-line layout refuses the whole lock (`redirect_vlt_lock_unsupported`). **Confirmation**: vlt drives when its install state (`node_modules/.vlt-lock.json` or `node_modules/.vlt/`) is present or no other npm-family lock is; then only `vlt-lock.json` confirms a uuid. Otherwise every lock is rewritten, `redirect_vlt_sibling_lockfiles` warns, and the other locks' rules confirm, including a dep `vlt-lock.json` merely does not wire (`redirect_vlt_entry_not_found`, `redirect_vlt_entry_vendored`, or `redirect_vlt_custom_registry_skipped` alone). Whichever lock drives, a dep the vlt rewriter refuses (`redirect_vlt_missing_sha512`, `redirect_vlt_unsupported_lock_key`) is never confirmed by any lock, although a sibling lock may already carry its rewritten URL. **Artifact preflight**: before any takeover or write (dry runs included), each granted artifact with a default-registry instance is fetched once as vlt fetches it and must verify, else the dep is withheld (`redirect_vlt_artifact_unverifiable`, see the tag table). **Heal**: stale installed copies of Socket-owned nodes are removed so the next `vlt install` extracts the patched bytes, and `rollback` / `remove` do the same for the registry bytes (`--no-vlt-install-cleanup` keeps them; optional dependencies' copies are always kept); `redirect_vlt_reinstall_required` says what happened and what to run. The same-run `--vex` never attests a vlt package whose installed copy is stale or unchecked, whose lock a vlt release may ignore (`redirect_vlt_lockfile_version_missing`, `redirect_vlt_old_lockfile_ignored`, `redirect_vlt_scalar_registry_ignored`), or which also resolves from a non-default registry (`redirect_vlt_custom_registry_skipped`). `vlt.json` or vlt install state without `vlt-lock.json` warns `redirect_vlt_no_lockfile` instead of `redirect_npm_no_lockfile`. `rollback` / `remove` restore each hosted node's slots [2] and [3] from the version document of the registry the node resolves against (slot [3] is its `dist.tarball`, as vlt writes it; `upstream_registry_fallback` when that registry can't be read), following the lock's own slot-[3] convention (see "Hosted unwind coverage"). Tested releases: `docs/testing/vlt-compatibility.md`. diff --git a/crates/socket-patch-core/src/formats/pnpm/hosted.rs b/crates/socket-patch-core/src/formats/pnpm/hosted.rs index 14f8e05bb..e16f3979d 100644 --- a/crates/socket-patch-core/src/formats/pnpm/hosted.rs +++ b/crates/socket-patch-core/src/formats/pnpm/hosted.rs @@ -11,7 +11,7 @@ use serde_json::Value; use crate::patch::redirect::{full_name, DepOverride, FileEdit, RewriteResult, RewriteWarning}; use super::grammar as pnpm; -use super::plan::{match_key, KeyMatch, Target}; +use super::plan::{match_key, planned_locks, KeyMatch, Target}; /// Whether `entry` resolves to exactly `artifact_url` — the per-instance /// residual-gate predicate. @@ -107,13 +107,6 @@ impl<'f> PnpmLockState<'f> { hits } - /// Whether a vendored rewire rekeyed `fname@version` - /// ([`KeyMatch::Rekeyed`]). Read from `text` without folding `pending` - /// in: a resolution splice never touches a key (see the type docs). - fn vendors(&self, fname: &str, version: &str) -> bool { - !self.lookup(fname, version, true).is_empty() - } - /// The entries [`match_key`] matches as a registry instance or, with /// `rekeyed`, as a vendored rekey, found by their key prefixes. fn lookup(&self, fname: &str, version: &str, rekeyed: bool) -> Vec { @@ -135,10 +128,9 @@ impl<'f> PnpmLockState<'f> { } out.sort_unstable(); out.dedup(); - out.retain(|&i| match match_key(self.entry(i).key, fname, version) { - Some(KeyMatch::Registry { .. }) => !rekeyed, - Some(KeyMatch::Rekeyed) => rekeyed, - None => false, + out.retain(|&i| { + match_key(self.entry(i).key, fname, version) + .is_some_and(|m| (m == KeyMatch::Rekeyed) == rekeyed) }); out } @@ -192,17 +184,10 @@ pub(crate) fn plan_hosted( let npm: Vec<&DepOverride> = overrides.iter().filter(|o| o.ecosystem == "npm").collect(); // A pnpm lock lives at the project root or at any nested path (e.g. Rush // repos keep them under `common/config/rush/`); every such files-map key - // is rewritten under the same grammar. Deterministic order: BTreeMap - // iterates keys sorted, so goldens are stable across every lock in the set. - let lock_keys: Vec<&String> = files - .keys() - .filter(|k| { - matches!( - k.rsplit('/').next(), - Some("pnpm-lock.yaml" | "shrinkwrap.yaml") - ) - }) - .collect(); + // is rewritten under the same grammar, except a `shrinkwrap.yaml` a + // `pnpm-lock.yaml` shadows ([`planned_locks`]). Deterministic order: + // BTreeMap iterates keys sorted, so goldens are stable across every lock. + let lock_keys = planned_locks(&files.keys().collect::>()); if npm.is_empty() || lock_keys.is_empty() { return; } @@ -360,7 +345,11 @@ pub(crate) fn plan_hosted( // mode switch. Fail-closed either way: nothing is rewritten for the // dep. if !matched_any { - let vendored = locks.iter().any(|lock| lock.vendors(&fname, &dep.version)); + // A rekey ([`KeyMatch::Rekeyed`]) is read from `text` without + // folding `pending` in: a resolution splice never touches a key. + let vendored = locks + .iter() + .any(|lock| !lock.lookup(&fname, &dep.version, true).is_empty()); if vendored { result.warnings.push(RewriteWarning { code: "redirect_pnpm_entry_vendored".into(), diff --git a/crates/socket-patch-core/src/formats/pnpm/plan.rs b/crates/socket-patch-core/src/formats/pnpm/plan.rs index a2fa56c1d..54361ecb2 100644 --- a/crates/socket-patch-core/src/formats/pnpm/plan.rs +++ b/crates/socket-patch-core/src/formats/pnpm/plan.rs @@ -18,7 +18,6 @@ use super::grammar; use super::{sniff_lock_grammar, PnpmLockGrammar}; -use crate::constants::npm_family::PNPM_LOCK; use crate::formats::text::strip_bom; use crate::vendor::npm_common::tgz_rel_leaf; @@ -52,13 +51,10 @@ impl Target { .is_some_and(|v| v.trim().trim_matches(['\'', '"']) == "3") && !field("shrinkwrapMinorVersion:") .is_some_and(|v| v.trim().parse::().is_ok_and(|v| v > 0)); - match early { - true => Err(format!( - "{PNPM_LOCK} uses early pnpm 1 shrinkwrapVersion 3 without a supported \ - minor version" - )), - false => Ok(None), + if early { + return Err("early pnpm 1 shrinkwrapVersion 3 without a minor version".into()); } + Ok(None) } /// Whether this target's edit repoints a registry instance whose key @@ -82,6 +78,29 @@ impl Target { depth == 0 } } +/// The pnpm locks among `keys` (root-relative paths) a planner rewrites, in +/// their order: every `pnpm-lock.yaml`, and a pnpm <= 2 `shrinkwrap.yaml` +/// only when no `pnpm-lock.yaml` sits beside it. pnpm never reads a +/// `shrinkwrap.yaml` it has a `pnpm-lock.yaml` for, so that one is +/// SHADOWED (owner decision on divergence N11): never rewritten, never part +/// of the all-or-nothing gate (debris in it cannot refuse a dependency), +/// and only swept for stale Socket identities by lockfile discovery. +pub(crate) fn planned_locks<'k>(keys: &[&'k String]) -> Vec<&'k String> { + // `dir` keeps its trailing `/` (empty at the root). + let shadowed = |key: &str| { + key.strip_suffix("shrinkwrap.yaml").is_some_and(|dir| { + keys.iter() + .any(|k| k.strip_suffix("pnpm-lock.yaml") == Some(dir)) + }) + }; + let planned = |key: &&String| match key.rsplit('/').next() { + Some("pnpm-lock.yaml") => true, + Some("shrinkwrap.yaml") => !shadowed(key), + _ => false, + }; + keys.iter().copied().filter(planned).collect() +} + /// How one `packages:` / `snapshots:` key names a patched release. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(crate) enum KeyMatch<'k> { @@ -163,6 +182,27 @@ mod tests { ); } + /// N11 (owner decision): a `shrinkwrap.yaml` with a `pnpm-lock.yaml` + /// beside it is shadowed; one without is planned. + #[test] + fn a_pnpm_lock_shadows_its_sibling_shrinkwrap() { + let keys: Vec = [ + "a/pnpm-lock.yaml", + "a/shrinkwrap.yaml", + "b/shrinkwrap.yaml", + "package.json", + "pnpm-lock.yaml", + "shrinkwrap.yaml", + ] + .map(String::from) + .into(); + let refs: Vec<&String> = keys.iter().collect(); + assert_eq!( + planned_locks(&refs), + ["a/pnpm-lock.yaml", "b/shrinkwrap.yaml", "pnpm-lock.yaml"] + ); + } + #[test] fn each_target_repoints_what_its_edit_has_fixtures_for() { assert!(Target::Hosted.repoints("")); diff --git a/crates/socket-patch-core/src/patch/redirect/mod.rs b/crates/socket-patch-core/src/patch/redirect/mod.rs index 165450ab4..fbac8d8db 100644 --- a/crates/socket-patch-core/src/patch/redirect/mod.rs +++ b/crates/socket-patch-core/src/patch/redirect/mod.rs @@ -28359,6 +28359,76 @@ packages: } } + /// Divergence N11 (owner decision, lock-model 2b): a pnpm <= 2 + /// `shrinkwrap.yaml` beside a `pnpm-lock.yaml` is shadowed. pnpm reads + /// only the `pnpm-lock.yaml`, so the hosted planner never rewrites the + /// shrinkwrap and leaves it out of the all-or-nothing gate: debris in it + /// (a key the splice cannot repoint, or pnpm 1's early shrinkwrap head) + /// no longer refuses the dependency in every lock. A `shrinkwrap.yaml` + /// that is the only pnpm lock is still rewritten. + #[test] + fn pnpm_shrinkwrap_beside_a_pnpm_lock_is_shadowed() { + let url = "http://patch.test/left-pad-1.3.0.tgz"; + let ovr = npm_override("left-pad", "1.3.0", url, "sha512-PATCHED=="); + let lock = "lockfileVersion: '9.0'\n\npackages:\n\n left-pad@1.3.0:\n \ + resolution: {integrity: sha512-OLD==}\n"; + let shrinkwrap = |head: &str, key: &str| { + format!( + "{head}\n\npackages:\n\n {key}:\n resolution: {{integrity: sha512-OLD==}}\n" + ) + }; + let supported = "shrinkwrapVersion: 3\nshrinkwrapMinorVersion: 9"; + let run = |files: &[(&str, String)]| { + let files: BTreeMap = files + .iter() + .map(|(k, v)| (k.to_string(), v.clone())) + .collect(); + rewrite_registry_redirect(&files, std::slice::from_ref(&ovr)) + }; + for (case, shrink) in [ + ("a plain entry", shrinkwrap(supported, "/left-pad/1.3.0")), + ( + "an unparseable key", + shrinkwrap(supported, "/left-pad/1.3.0)("), + ), + ( + "an early shrinkwrap", + shrinkwrap("shrinkwrapVersion: 3", "/left-pad/1.3.0"), + ), + ] { + let r = run(&[ + ("pnpm-lock.yaml", lock.to_string()), + ("shrinkwrap.yaml", shrink), + ]); + assert_eq!( + r.files.keys().collect::>(), + ["pnpm-lock.yaml"], + "{case}: only the governing lock is written: {:?}", + r.warnings + ); + assert!(r.files["pnpm-lock.yaml"].contains(url), "{case}"); + assert!(r.warnings.is_empty(), "{case}: {:?}", r.warnings); + assert!(r.refused_pnpm_uuids.is_empty(), "{case}"); + assert!(r.edits.iter().all(|e| e.path == "pnpm-lock.yaml"), "{case}"); + } + // A nested `shrinkwrap.yaml` is shadowed only by its own sibling. + let r = run(&[ + ("pnpm-lock.yaml", lock.to_string()), + ( + "sub/shrinkwrap.yaml", + shrinkwrap(supported, "/left-pad/1.3.0"), + ), + ]); + assert!( + r.files.contains_key("sub/shrinkwrap.yaml"), + "{:?}", + r.warnings + ); + // The only pnpm lock: rewritten as before. + let r = run(&[("shrinkwrap.yaml", shrinkwrap(supported, "/left-pad/1.3.0"))]); + assert!(r.files["shrinkwrap.yaml"].contains(url), "{:?}", r.warnings); + } + /// A classic-lock block whose first line is not a `key:` line /// (hand-mangled) is skipped without derailing the rewrite of the real /// entry — and survives byte-identically. diff --git a/docs/design/lock-model-divergences.md b/docs/design/lock-model-divergences.md index c77e7e7f3..00af98b9b 100644 --- a/docs/design/lock-model-divergences.md +++ b/docs/design/lock-model-divergences.md @@ -31,7 +31,7 @@ follow from the install-tree decision. | J12 | scala-cli | Guidance only (`patch/redirect/scala_guidance.rs`) | Back-port: hosted wires scala-cli through the merged planner (a same-GAV Socket repository instead of a suffixed version). | | P3 | Is a shadowed `pdm.lock` rewritten? | Only when it governs (`pdm_drives`, ~564) | Rewrite it like every other tree, per the install-tree decision. | | P4 | Are `-r` includes of `requirements.txt` rewritten? | Root file only (`patch/redirect/requirements.rs` ~208) | Follow includes, per the install-tree decision. | -| N11 | Is a `shrinkwrap.yaml` beside `pnpm-lock.yaml` rewritten? | Yes; its all-or-nothing gate covers both files | **Pending.** Proposed: treat it as shadowed (swept for stale identities, never written). pnpm never reads it, and an unparseable debris key in it can refuse the dependency in every lock. | +| N11 | Is a `shrinkwrap.yaml` beside `pnpm-lock.yaml` rewritten? | Yes; its all-or-nothing gate covers both files | **Resolved (owner: shadowed) in PR 2b.** Swept for stale identities, never written, and out of the all-or-nothing gate (`formats::pnpm::plan::planned_locks`). pnpm never reads it, and an unparseable debris key in it used to refuse the dependency in every lock. A `shrinkwrap.yaml` that is the only pnpm lock is rewritten as before. | ## JVM @@ -89,7 +89,7 @@ follow from the install-tree decision. | N8 | npm `file:` directory link identity (#1481) | Hosted and discovery give a versionless link its source directory's `name@version`. Vendored matches the entry's own version, wiring the registry copies with no warning. | Hosted | silent vendored success | | N9 | yarn classic blocks reached only through an alias | Hosted skips them. Vendored rewrites them. Discovery counts an alias block as an install of the real package. | Back-port: hosted rewrites alias blocks (vendored supports what hosted lacks) | | | N10 | Locks of several npm-family package managers in one project | Hosted rewrites every family; discovery reads every family with the contest. The inventory and vendored pick one family (vlt > bun > pnpm > yarn > npm). | Union (separate trees) | | -| N11 | `shrinkwrap.yaml` beside `pnpm-lock.yaml` | Hosted reads and splices it, and its all-or-nothing gate covers both. Discovery sweeps it as ignored. The inventory reads it only as a fallback. Vendored never reads it. | Pending owner decision (see above) | needs owner | +| N11 | `shrinkwrap.yaml` beside `pnpm-lock.yaml` | Hosted reads and splices it, and its all-or-nothing gate covers both. Discovery sweeps it as ignored. The inventory reads it only as a fallback. Vendored never reads it. | **Resolved (owner: shadowed) in PR 2b**: hosted never rewrites it and leaves it out of the gate; discovery keeps sweeping it; vendored never reads it. A lone `shrinkwrap.yaml` keeps today's behavior. | | | N12 | Rush common and subspace locks | Hosted and discovery always read them. The inventory reads them only when there is no root lock (#2003 aligns the listing itself). Vendored refuses Rush. | Always read. Vendored's refusal stays. **Resolved in PR 2a** (the inventory reads them whenever `rush.json` exists). | target-specific (vendored) | | N13 | Plug'n'Play | Vendored refuses. The hosted berry preflight has no PnP gate. The inventory returns an error, so `scan` sees nothing in a project hosted pinned. | The inventory reads PnP locks. Vendored keeps its refusal. | target-specific (vendored) | | N14 | pnpm `lockfile=false` | Hosted refuses. Discovery withholds hosted refs, keeps vendored ones. Vendored proceeds. The inventory ignores the setting. | The modes stay as they are; the inventory follows discovery. **Resolved in PR 2a**: under the setting (not under Rush), the inventory keeps a Socket-hosted pnpm entry but drops its integrity, as discovery withholds the lock pin. | target-specific | diff --git a/docs/ecosystems.md b/docs/ecosystems.md index b03dc6665..9e7d42b11 100644 --- a/docs/ecosystems.md +++ b/docs/ecosystems.md @@ -171,7 +171,8 @@ Each Legacy format has an upgrade path and an undo path. Both work in v5: from its legacy mirror, alias nodes included) and rewires both locks in npm 12's dual-lock state. Majors 6–12 are measured in [npm compatibility](testing/npm-compatibility.md). -- **pnpm** — hosted rewriting supports legacy `shrinkwrap.yaml` (pnpm 1/2), +- **pnpm** — hosted rewriting supports legacy `shrinkwrap.yaml` (pnpm 1/2; never + rewritten beside a `pnpm-lock.yaml`, which pnpm reads instead), lockfileVersion 5.x (pnpm 3–7), 6.0 (pnpm 8), and 9.0 (pnpm 9–12). Early pnpm 1 locks with shrinkwrapVersion 3 and no positive minor version are refused because those installers discard hosted URLs; the tested From cc3ad0c95a1de1b3191f7701a63c5ca26395bc3e Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Thu, 5 Nov 2026 23:16:36 -0600 Subject: [PATCH 4/5] Address the code review of the pnpm planner merge - Read every vendored pnpm text file through one reader, read_pnpm_text. It normalizes CRLF and rejects mixed endings. The revert now reads a CRLF two-document lock: it used to fail on `---\r`. A lock or workspace file that mixes CRLF and LF now fails the revert before any write, as vendoring refuses it; the revert used to rewrite it all-LF. - N11: a project that pins pnpm 1 / 2 (install record or package.json packageManager) reads only shrinkwrap.yaml, so there the stray pnpm-lock.yaml is the shadowed one. Hosted (planned_locks) and discovery share governing_pnpm_lock. The pin reader moves to utils::pnpm_workspace::pinned_pnpm_major, which pnpm_major_from reuses. - sniff_lock_grammar: an unparseable lockfileVersion major keeps the update-socket-patch remedy. - Perf: SectionIndex::build no longer formats a string per block. dep_entries is lazy and borrowed, so the non-index ref gate stops at the first refusal. matching_blocks classifies each block once, and target_block returns it with its rekey flag. - Target::check_lock is split into hosted_refuses_lock and the vendored dialect's sniff_lock_grammar check, with no dead default. Target::repoints is an explicit match per target. Budget, against the base: lock-readers 31206 (base 31207), lock-writers 88115 (base 88314). Co-Authored-By: Claude Opus 5.5 --- crates/socket-patch-cli/CLI_CONTRACT.md | 2 +- .../src/formats/pnpm/hosted.rs | 19 +- .../socket-patch-core/src/formats/pnpm/mod.rs | 15 +- .../src/formats/pnpm/plan.rs | 225 ++++--- .../src/patch/redirect/mod.rs | 20 + .../src/utils/pnpm_workspace.rs | 44 +- .../socket-patch-core/src/vendor/pnpm_lock.rs | 620 ++++++++++-------- .../src/vendor/pnpm_lock_legacy.rs | 30 +- .../socket-patch-core/src/vex/discover/npm.rs | 48 +- docs/design/lock-model-divergences.md | 4 +- docs/ecosystems.md | 3 +- scripts/loc-budget.json | 4 +- 12 files changed, 618 insertions(+), 416 deletions(-) diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index 3f491176e..4b7c6ec51 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -272,7 +272,7 @@ For a **9.0 root lock**, the CLI ensures `pnpm-workspace.yaml` carries `trustLoc **pnpm per-branch locks (#556)**: with `gitBranchLockfile` on (a top-level `gitBranchLockfile: true` in the root `pnpm-workspace.yaml`; when the YAML leaves the key unset, `git-branch-lockfile=true` in the root `.npmrc`, as pnpm 10 and older read it; on disk also `pnpm_config_git_branch_lockfile` (pnpm 11 and newer) or `npm_config_git_branch_lockfile` (pnpm 10 and older; either when the major is unknown, read as for the `lockfile` setting above) set to `true`; a workspace member with no `pnpm-workspace.yaml` of its own reads the governing ancestor's file and the `.npmrc` beside it), pnpm installs a git branch from its own `pnpm-lock..yaml` whenever that lock exists, in every directory it installs a lock from: the root, and under `sharedWorkspaceLockfile: false` each member's (#492). When the setting is on AND at least one such `pnpm-lock..yaml` exists (at the root, or in a member that installs from its own lock), hosted mode leaves every root pnpm lock (`pnpm-lock.yaml`, `shrinkwrap.yaml`) and `pnpm-workspace.yaml` untouched (every member lock too), confirms no npm pin from them and warns `redirect_pnpm_git_branch_lockfile` (in place of `redirect_pnpm_no_lockfile` when the branch lock is the only one); the detail names the setting and the branch lock(s), and the remedy is to turn `gitBranchLockfile` off and fold the branch locks into `pnpm-lock.yaml` with `pnpm install --merge-git-branch-lockfiles`, then re-run. A vendored→hosted takeover of a pnpm-vendored purl is refused with the same code (the vendored wiring and ledger stay byte-for-byte). Vendored mode refuses the same projects with `vendor_pnpm_git_branch_lockfile`, before any write and in the pre-flight. The setting with no branch lock (main, or a branch whose dependencies never changed) installs from `pnpm-lock.yaml`, which both modes wire as usual; a stray branch lock with the setting off is ignored. The in-memory engine fetches root `pnpm-lock.*.yaml` names presence-only and refuses the same way. -`redirect_pnpm_no_lockfile` names pnpm when installer markers exist without a lock; `redirect_pnpm_entry_vendored` identifies a vendored entry instead of reporting it missing. Supported `shrinkwrap.yaml` files are writable lockfiles, not read-only markers, when no `pnpm-lock.yaml` sits beside them. pnpm never reads a `shrinkwrap.yaml` it has a `pnpm-lock.yaml` for, so that one is shadowed: hosted mode never rewrites it and leaves it out of the all-or-nothing gate, so an entry it cannot repoint (or pnpm 1's early shrinkwrap head) refuses nothing, and `vex` only sweeps it for stale Socket identities. Vendored mode never writes it either. +`redirect_pnpm_no_lockfile` names pnpm when installer markers exist without a lock; `redirect_pnpm_entry_vendored` identifies a vendored entry instead of reporting it missing. Supported `shrinkwrap.yaml` files are writable lockfiles, not read-only markers, when no `pnpm-lock.yaml` sits beside them. pnpm >= 3 never reads a `shrinkwrap.yaml` it has a `pnpm-lock.yaml` for, so that one is shadowed (unless the project pins pnpm 1 or 2 in `node_modules/.modules.yaml` or package.json's `packageManager`: those read only `shrinkwrap.yaml`, so there the stray `pnpm-lock.yaml` is the shadowed one): hosted mode never rewrites it and leaves it out of the all-or-nothing gate, so an entry it cannot repoint (or pnpm 1's early shrinkwrap head) refuses nothing, and `vex` only sweeps it for stale Socket identities. Vendored mode never writes it either. **vlt hosted-mode contract**: `scan` / `get --mode hosted` rewrite, in `vlt-lock.json`, every default-registry node of a granted `name@version` (the `''` / `npm` segment or a URL segment equal to the lock's scalar `registry`, both DepID grammars, every peer and modifier variant): slot [2] becomes the granted sha512 and slot [3] the hosted URL (appended to a 3-tuple); the DepID, flags and trailing slots, the line ending and every other byte stay. `options` is never edited and `vlt.json` is only read. A lock with another `lockfileVersion` (decided on the raw JSON token), a BOM, a non-object body or a `nodes` section outside vlt's one-node-per-line layout refuses the whole lock (`redirect_vlt_lock_unsupported`). **Confirmation**: vlt drives when its install state (`node_modules/.vlt-lock.json` or `node_modules/.vlt/`) is present or no other npm-family lock is; then only `vlt-lock.json` confirms a uuid. Otherwise every lock is rewritten, `redirect_vlt_sibling_lockfiles` warns, and the other locks' rules confirm, including a dep `vlt-lock.json` merely does not wire (`redirect_vlt_entry_not_found`, `redirect_vlt_entry_vendored`, or `redirect_vlt_custom_registry_skipped` alone). Whichever lock drives, a dep the vlt rewriter refuses (`redirect_vlt_missing_sha512`, `redirect_vlt_unsupported_lock_key`) is never confirmed by any lock, although a sibling lock may already carry its rewritten URL. **Artifact preflight**: before any takeover or write (dry runs included), each granted artifact with a default-registry instance is fetched once as vlt fetches it and must verify, else the dep is withheld (`redirect_vlt_artifact_unverifiable`, see the tag table). **Heal**: stale installed copies of Socket-owned nodes are removed so the next `vlt install` extracts the patched bytes, and `rollback` / `remove` do the same for the registry bytes (`--no-vlt-install-cleanup` keeps them; optional dependencies' copies are always kept); `redirect_vlt_reinstall_required` says what happened and what to run. The same-run `--vex` never attests a vlt package whose installed copy is stale or unchecked, whose lock a vlt release may ignore (`redirect_vlt_lockfile_version_missing`, `redirect_vlt_old_lockfile_ignored`, `redirect_vlt_scalar_registry_ignored`), or which also resolves from a non-default registry (`redirect_vlt_custom_registry_skipped`). `vlt.json` or vlt install state without `vlt-lock.json` warns `redirect_vlt_no_lockfile` instead of `redirect_npm_no_lockfile`. `rollback` / `remove` restore each hosted node's slots [2] and [3] from the version document of the registry the node resolves against (slot [3] is its `dist.tarball`, as vlt writes it; `upstream_registry_fallback` when that registry can't be read), following the lock's own slot-[3] convention (see "Hosted unwind coverage"). Tested releases: `docs/testing/vlt-compatibility.md`. diff --git a/crates/socket-patch-core/src/formats/pnpm/hosted.rs b/crates/socket-patch-core/src/formats/pnpm/hosted.rs index e16f3979d..54908d9dc 100644 --- a/crates/socket-patch-core/src/formats/pnpm/hosted.rs +++ b/crates/socket-patch-core/src/formats/pnpm/hosted.rs @@ -11,7 +11,7 @@ use serde_json::Value; use crate::patch::redirect::{full_name, DepOverride, FileEdit, RewriteResult, RewriteWarning}; use super::grammar as pnpm; -use super::plan::{match_key, planned_locks, KeyMatch, Target}; +use super::plan::{hosted_refuses_lock, match_key, planned_locks, KeyMatch, Target}; /// Whether `entry` resolves to exactly `artifact_url` — the per-instance /// residual-gate predicate. @@ -52,7 +52,7 @@ impl<'f> PnpmLockState<'f> { let mut state = PnpmLockState { path, text: Cow::Borrowed(text), - early_shrinkwrap: Target::Hosted.check_lock(text).is_err(), + early_shrinkwrap: hosted_refuses_lock(text), entries: Vec::new(), sorted: Vec::new(), pending: Vec::new(), @@ -119,12 +119,8 @@ impl<'f> PnpmLockState<'f> { let start = self .sorted .partition_point(|&i| self.norm_key(i) < prefix.as_str()); - out.extend( - self.sorted[start..] - .iter() - .take_while(|&&i| self.norm_key(i).starts_with(prefix.as_str())) - .copied(), - ); + let rest = self.sorted[start..].iter(); + out.extend(rest.take_while(|&&i| self.norm_key(i).starts_with(prefix.as_str()))); } out.sort_unstable(); out.dedup(); @@ -184,10 +180,11 @@ pub(crate) fn plan_hosted( let npm: Vec<&DepOverride> = overrides.iter().filter(|o| o.ecosystem == "npm").collect(); // A pnpm lock lives at the project root or at any nested path (e.g. Rush // repos keep them under `common/config/rush/`); every such files-map key - // is rewritten under the same grammar, except a `shrinkwrap.yaml` a - // `pnpm-lock.yaml` shadows ([`planned_locks`]). Deterministic order: + // is rewritten under the same grammar, except the one of a + // `pnpm-lock.yaml` / `shrinkwrap.yaml` pair pnpm never reads + // ([`planned_locks`]). Deterministic order: // BTreeMap iterates keys sorted, so goldens are stable across every lock. - let lock_keys = planned_locks(&files.keys().collect::>()); + let lock_keys = planned_locks(files); if npm.is_empty() || lock_keys.is_empty() { return; } diff --git a/crates/socket-patch-core/src/formats/pnpm/mod.rs b/crates/socket-patch-core/src/formats/pnpm/mod.rs index b94801dfb..45aca1c30 100644 --- a/crates/socket-patch-core/src/formats/pnpm/mod.rs +++ b/crates/socket-patch-core/src/formats/pnpm/mod.rs @@ -293,17 +293,20 @@ pub fn sniff_lock_grammar(text: &str) -> Result { Some("5.4") => Ok(PnpmLockGrammar::V54), Some("6.0") => Ok(PnpmLockGrammar::V60), v => { - let major = v.and_then(|v| v.split('.').next()?.parse::().ok()); + // An old (or missing) version is fixed by upgrading pnpm; a newer or + // unreadable one means pnpm outgrew this build. + let major = v.map(|v| v.split('.').next().and_then(|m| m.parse::().ok())); + let old = matches!(major, None | Some(Some(..9))); let v = v.map_or("no lockfileVersion in its head".into(), |v| { format!("lockfileVersion {v}") }); - Err(match major { - Some(m) if m >= 9 => format!( + Err(match old { + false => format!( "{PNPM_LOCK} has {v}; this socket-patch build supports \ lockfileVersions 5.4, 6.0, and 9.0 — re-lock with a pnpm release that \ emits one of them, or update socket-patch" ), - _ => format!( + true => format!( "{PNPM_LOCK} has {v}; supported versions are 5.4 (pnpm 7), 6.0 \ (pnpm 8), and 9.0 (pnpm >= 9) — re-lock with pnpm >= 9" ), @@ -1026,9 +1029,7 @@ mod tests { assert_eq!(lock_version_major(&format!("\u{feff}{v9}")), Some(9)); assert!(may_need_store_flag(&format!("\u{feff}{v52}"))); assert!(may_need_store_flag(&format!("\u{feff}{shrinkwrap}"))); - assert!(plan::Target::Hosted - .check_lock("\u{feff}shrinkwrapVersion: 3\n") - .is_err()); + assert!(plan::hosted_refuses_lock("\u{feff}shrinkwrapVersion: 3\n")); // Exactly one BOM is encoding; a second one is content, as for pnpm. assert!(!is_pnpm_lock_text( "\u{feff}\u{feff}lockfileVersion: '9.0'\n" diff --git a/crates/socket-patch-core/src/formats/pnpm/plan.rs b/crates/socket-patch-core/src/formats/pnpm/plan.rs index 54361ecb2..34d5fb051 100644 --- a/crates/socket-patch-core/src/formats/pnpm/plan.rs +++ b/crates/socket-patch-core/src/formats/pnpm/plan.rs @@ -4,7 +4,9 @@ //! they agree on: //! //! * which lock versions each target's edit has fixtures for -//! ([`Target::check_lock`]); +//! ([`hosted_refuses_lock`]; vendored takes the grammars its rekey was +//! captured against, [`super::sniff_lock_grammar`], and refuses the rest, +//! target-specifically: divergences N16); //! * how a lock key names a patched `name@version` ([`match_key`]): the //! registry entry with its peer suffix, or the entry a vendored rewire //! already rekeyed; @@ -17,8 +19,12 @@ //! artifact and mirrors the override pnpm cross-checks. use super::grammar; -use super::{sniff_lock_grammar, PnpmLockGrammar}; + +use std::collections::BTreeMap; + +use crate::constants::npm_family::{PNPM_LOCK, PNPM_SHRINKWRAP_LEGACY}; use crate::formats::text::strip_bom; +use crate::utils::pnpm_workspace::pinned_pnpm_major; use crate::vendor::npm_common::tgz_rel_leaf; /// Where a rewired entry points. @@ -32,73 +38,77 @@ pub(crate) enum Target { Vendored, } -impl Target { - /// Whether this target's edit can rewrite a lock with `text`'s head, - /// and in which grammar (`None`: the hosted splice reads every - /// grammar the same way). Hosted takes every lock but pnpm 1's early - /// shrinkwrap (shrinkwrapVersion 3 without a minor version), whose - /// installers drop registry tarball URLs whatever the frozen flag says - /// (verified with pnpm 1.0.0). Vendored takes the grammars its rekey was - /// captured against (5.4, 6.0, 9.0): another version's `file:` override - /// shape has no pnpm-blessed fixture, so it is refused, - /// target-specifically (divergences N16). - pub(crate) fn check_lock(self, text: &str) -> Result, String> { - if self == Target::Vendored { - return sniff_lock_grammar(text).map(Some); - } - let field = |name: &str| strip_bom(text).lines().find_map(|l| l.strip_prefix(name)); - let early = field("shrinkwrapVersion:") - .is_some_and(|v| v.trim().trim_matches(['\'', '"']) == "3") - && !field("shrinkwrapMinorVersion:") - .is_some_and(|v| v.trim().parse::().is_ok_and(|v| v > 0)); - if early { - return Err("early pnpm 1 shrinkwrapVersion 3 without a minor version".into()); - } - Ok(None) - } +/// Whether the hosted splice refuses a lock: pnpm 1's early shrinkwrap +/// (shrinkwrapVersion 3 without a minor version), whose installers drop +/// registry tarball URLs whatever the frozen flag says (verified with pnpm +/// 1.0.0). Hosted takes every other lock version (divergences N16). +pub(crate) fn hosted_refuses_lock(text: &str) -> bool { + let field = |name: &str| strip_bom(text).lines().find_map(|l| l.strip_prefix(name)); + field("shrinkwrapVersion:").is_some_and(|v| v.trim().trim_matches(['\'', '"']) == "3") + && !field("shrinkwrapMinorVersion:") + .is_some_and(|v| v.trim().parse::().is_ok_and(|v| v > 0)) +} +impl Target { /// Whether this target's edit repoints a registry instance whose key - /// carries `suffix` after `name@version`. Hosted splices every peer - /// context it can parse (pnpm 5's `_peer@1` or balanced `(peer@1)` - /// groups); vendored rekeys the plain entry only, since a peer-suffixed - /// rekey has no fixture and would dangle. + /// carries `suffix` after `name@version`. pub(crate) fn repoints(self, suffix: &str) -> bool { - if self == Target::Vendored || suffix.is_empty() || suffix.starts_with('_') { - return suffix.is_empty() || self == Target::Hosted; - } - let mut depth = 0usize; - for c in suffix.chars() { - match c { - '(' => depth += 1, - ')' if depth > 0 => depth -= 1, - _ if depth == 0 => return false, - _ => {} + match self { + // The rekey moves the plain entry only: a peer-suffixed rekey + // has no fixture and would dangle. + Target::Vendored => suffix.is_empty(), + // The splice repoints every peer context it can parse: none, + // pnpm 5's `_peer@1`, or balanced `(peer@1)` groups. + Target::Hosted if suffix.is_empty() || suffix.starts_with('_') => true, + Target::Hosted => { + let mut depth = 0usize; + for c in suffix.chars() { + match c { + '(' => depth += 1, + ')' if depth > 0 => depth -= 1, + _ if depth == 0 => return false, + _ => {} + } + } + depth == 0 } } - depth == 0 } } -/// The pnpm locks among `keys` (root-relative paths) a planner rewrites, in -/// their order: every `pnpm-lock.yaml`, and a pnpm <= 2 `shrinkwrap.yaml` -/// only when no `pnpm-lock.yaml` sits beside it. pnpm never reads a -/// `shrinkwrap.yaml` it has a `pnpm-lock.yaml` for, so that one is -/// SHADOWED (owner decision on divergence N11): never rewritten, never part -/// of the all-or-nothing gate (debris in it cannot refuse a dependency), -/// and only swept for stale Socket identities by lockfile discovery. -pub(crate) fn planned_locks<'k>(keys: &[&'k String]) -> Vec<&'k String> { - // `dir` keeps its trailing `/` (empty at the root). - let shadowed = |key: &str| { - key.strip_suffix("shrinkwrap.yaml").is_some_and(|dir| { - keys.iter() - .any(|k| k.strip_suffix("pnpm-lock.yaml") == Some(dir)) - }) - }; - let planned = |key: &&String| match key.rsplit('/').next() { - Some("pnpm-lock.yaml") => true, - Some("shrinkwrap.yaml") => !shadowed(key), - _ => false, + +/// The lock pnpm reads in a directory holding `pnpm-lock.yaml` (`lock`) +/// and/or a pnpm <= 2 `shrinkwrap.yaml` (`shrinkwrap`), given the pnpm major +/// the project pins ([`pinned_pnpm_major`]); the other one is SHADOWED +/// (owner decision on divergence N11): never rewritten, never part of the +/// all-or-nothing gate (debris in it cannot refuse a dependency), only swept +/// for stale Socket identities by lockfile discovery. pnpm 1 / 2 read only +/// `shrinkwrap.yaml`; pnpm >= 3 read only `pnpm-lock.yaml`. With no pin, +/// `pnpm-lock.yaml` governs (pnpm <= 2 never writes one). +pub(crate) fn governing_pnpm_lock( + lock: bool, + shrinkwrap: bool, + pinned_major: Option, +) -> Option<&'static str> { + let pnpm_1_or_2 = pinned_major.is_some_and(|major| major <= 2); + let governs = shrinkwrap && (!lock || pnpm_1_or_2); + (governs.then_some(PNPM_SHRINKWRAP_LEGACY)).or(lock.then_some(PNPM_LOCK)) +} + +/// The pnpm locks of a hosted run's `files` (root-relative paths) a planner +/// rewrites, in key order: per directory, the [`governing_pnpm_lock`], read +/// with the evidence `files` holds for that directory. +pub(crate) fn planned_locks(files: &BTreeMap) -> Vec<&String> { + let planned = |key: &&String| { + let leaf = key.rsplit('/').next().unwrap_or(key); + // `dir` keeps its trailing `/` (empty at the root). + let in_dir = |rel: &str| format!("{}{rel}", &key[..key.len() - leaf.len()]); + let pinned = pinned_pnpm_major(|rel| files.get(&in_dir(rel)).cloned()); + let has = |rel: &str| files.contains_key(&in_dir(rel)); + matches!(leaf, PNPM_LOCK | PNPM_SHRINKWRAP_LEGACY) + && governing_pnpm_lock(has(PNPM_LOCK), has(PNPM_SHRINKWRAP_LEGACY), pinned) + == Some(leaf) }; - keys.iter().copied().filter(planned).collect() + files.keys().filter(planned).collect() } /// How one `packages:` / `snapshots:` key names a patched release. @@ -136,6 +146,7 @@ pub(crate) fn match_key<'k>(key: &'k str, name: &str, version: &str) -> Option = [ - "a/pnpm-lock.yaml", - "a/shrinkwrap.yaml", - "b/shrinkwrap.yaml", - "package.json", - "pnpm-lock.yaml", - "shrinkwrap.yaml", - ] - .map(String::from) - .into(); - let refs: Vec<&String> = keys.iter().collect(); + fn the_pinned_pnpm_major_picks_the_governing_lock() { + // No pin: `pnpm-lock.yaml` governs a pair, and each lone lock itself. + assert_eq!(governing_pnpm_lock(true, true, None), Some(PNPM_LOCK)); assert_eq!( - planned_locks(&refs), - ["a/pnpm-lock.yaml", "b/shrinkwrap.yaml", "pnpm-lock.yaml"] + governing_pnpm_lock(false, true, None), + Some(PNPM_SHRINKWRAP_LEGACY) + ); + assert_eq!(governing_pnpm_lock(false, false, Some(2)), None); + // pnpm 1 / 2 read only `shrinkwrap.yaml`; pnpm >= 3 only the lock. + assert_eq!( + governing_pnpm_lock(true, true, Some(2)), + Some(PNPM_SHRINKWRAP_LEGACY) + ); + assert_eq!(governing_pnpm_lock(true, true, Some(3)), Some(PNPM_LOCK)); + assert_eq!(governing_pnpm_lock(true, false, Some(1)), Some(PNPM_LOCK)); + + let files = |pin: Option<&str>| { + let mut files: BTreeMap = [ + "a/pnpm-lock.yaml", + "a/shrinkwrap.yaml", + "b/shrinkwrap.yaml", + "pnpm-lock.yaml", + "shrinkwrap.yaml", + ] + .into_iter() + .map(|k| (k.to_string(), String::new())) + .collect(); + if let Some(pin) = pin { + let pkg = format!(r#"{{"packageManager": "pnpm@{pin}"}}"#); + files.insert("package.json".into(), pkg); + } + files + }; + let planned = |files: &BTreeMap| { + planned_locks(files) + .into_iter() + .cloned() + .collect::>() + }; + // No evidence, or pnpm >= 3: each `pnpm-lock.yaml` shadows its + // sibling shrinkwrap. + for pin in [None, Some("9.15.0")] { + assert_eq!( + planned(&files(pin)), + ["a/pnpm-lock.yaml", "b/shrinkwrap.yaml", "pnpm-lock.yaml"], + "{pin:?}" + ); + } + // pnpm 2 pinned at the root: there `shrinkwrap.yaml` governs and the + // stray `pnpm-lock.yaml` is shadowed; `a/` has no evidence of its own. + assert_eq!( + planned(&files(Some("2.25.7"))), + ["a/pnpm-lock.yaml", "b/shrinkwrap.yaml", "shrinkwrap.yaml"] + ); + // The install record outranks package.json. + let mut record = files(Some("9.15.0")); + record.insert( + "node_modules/.modules.yaml".into(), + "packageManager: pnpm@2.25.7\n".into(), + ); + assert_eq!( + planned(&record).last().map(String::as_str), + Some("shrinkwrap.yaml") ); } @@ -229,13 +290,9 @@ mod tests { ("lockfileVersion: '10.0'\n", None), ("shrinkwrapVersion: 3\nshrinkwrapMinorVersion: 9\n", None), ] { - assert_eq!(Target::Hosted.check_lock(head), Ok(None), "{head}"); - assert_eq!( - Target::Vendored.check_lock(head).ok().flatten(), - vendored, - "{head}" - ); + assert!(!hosted_refuses_lock(head), "{head}"); + assert_eq!(sniff_lock_grammar(head).ok(), vendored, "{head}"); } - assert!(Target::Hosted.check_lock("shrinkwrapVersion: 3\n").is_err()); + assert!(hosted_refuses_lock("shrinkwrapVersion: 3\n")); } } diff --git a/crates/socket-patch-core/src/patch/redirect/mod.rs b/crates/socket-patch-core/src/patch/redirect/mod.rs index fbac8d8db..d96c18c77 100644 --- a/crates/socket-patch-core/src/patch/redirect/mod.rs +++ b/crates/socket-patch-core/src/patch/redirect/mod.rs @@ -28424,6 +28424,26 @@ packages: "{:?}", r.warnings ); + // A project pinning pnpm 2 installs from `shrinkwrap.yaml` (review + // finding): it is rewritten and the stray `pnpm-lock.yaml` is the + // shadowed one, so debris in it refuses nothing. + let pnpm2 = r#"{"packageManager": "pnpm@2.25.7"}"#.to_string(); + let r = run(&[ + ("package.json", pnpm2), + ( + "pnpm-lock.yaml", + lock.replace("left-pad@1.3.0:", "left-pad@1.3.0)(:"), + ), + ("shrinkwrap.yaml", shrinkwrap(supported, "/left-pad/1.3.0")), + ]); + assert_eq!( + r.files.keys().collect::>(), + ["shrinkwrap.yaml"], + "{:?}", + r.warnings + ); + assert!(r.files["shrinkwrap.yaml"].contains(url)); + assert!(r.refused_pnpm_uuids.is_empty(), "{:?}", r.warnings); // The only pnpm lock: rewritten as before. let r = run(&[("shrinkwrap.yaml", shrinkwrap(supported, "/left-pad/1.3.0"))]); assert!(r.files["shrinkwrap.yaml"].contains(url), "{:?}", r.warnings); diff --git a/crates/socket-patch-core/src/utils/pnpm_workspace.rs b/crates/socket-patch-core/src/utils/pnpm_workspace.rs index 8671daab2..483ed46f6 100644 --- a/crates/socket-patch-core/src/utils/pnpm_workspace.rs +++ b/crates/socket-patch-core/src/utils/pnpm_workspace.rs @@ -890,36 +890,54 @@ fn lockfile_off(setting: impl Fn(&str, &str) -> Option) -> Option) -> Option { + let texts = read_texts(view, &[MODULES_YAML, "package.json", PNPM_LOCK]).await; + pnpm_major_from(|rel| texts.get(rel).cloned()) +} + +/// The pnpm major the project at `view` pins ([`pinned_pnpm_major`]). +pub(crate) async fn pinned_pnpm_major_in(view: &ProjectView<'_>) -> Option { + let texts = read_texts(view, &[MODULES_YAML, "package.json"]).await; + pinned_pnpm_major(|rel| texts.get(rel).cloned()) +} + +/// The readable files of `rels` under `view`, by path. +async fn read_texts<'r>(view: &ProjectView<'_>, rels: &[&'r str]) -> BTreeMap<&'r str, String> { let mut texts = BTreeMap::new(); - for rel in [MODULES_YAML, "package.json", PNPM_LOCK] { + for &rel in rels { if let Ok(text) = view.read_text(rel).await { texts.insert(rel, text); } } - pnpm_major_from(|rel| texts.get(rel).cloned()) + texts } /// The installed pnpm's record of the project. pub(crate) const MODULES_YAML: &str = "node_modules/.modules.yaml"; -/// The pnpm major whose settings reading applies to a project whose files -/// `read` returns by project-relative path: the one that installed it -/// ([`MODULES_YAML`]), else the one package.json pins, else what the root -/// lock's grammar implies (`pnpm_settings_major`). -pub(crate) fn pnpm_major_from(read: impl Fn(&str) -> Option) -> Option { - use crate::patch::redirect::upstream::npm::{ - modules_yaml_pnpm_major, package_json_pnpm_major, pnpm_settings_major, - }; - let pinned = read(MODULES_YAML) +/// The pnpm major a project pins, from its files as `read` returns them by +/// project-relative path: the one that installed it ([`MODULES_YAML`]), +/// else the one package.json's `packageManager` names. +pub(crate) fn pinned_pnpm_major(read: impl Fn(&str) -> Option) -> Option { + use crate::patch::redirect::upstream::npm::{modules_yaml_pnpm_major, package_json_pnpm_major}; + let modules = read(MODULES_YAML); + modules .as_deref() .and_then(modules_yaml_pnpm_major) .or_else(|| { read("package.json") .as_deref() .and_then(package_json_pnpm_major) - }); + }) +} + +/// The pnpm major whose settings reading applies to a project whose files +/// `read` returns by project-relative path: the one it pins +/// ([`pinned_pnpm_major`]), else what the root lock's grammar implies +/// (`pnpm_settings_major`). +pub(crate) fn pnpm_major_from(read: impl Fn(&str) -> Option) -> Option { + let pinned = pinned_pnpm_major(&read); let lock = read(PNPM_LOCK).unwrap_or_default(); - pnpm_settings_major(&lock, pinned) + crate::patch::redirect::upstream::npm::pnpm_settings_major(&lock, pinned) } /// What pnpm does with its `lockfile` setting off, for the refusals and diff --git a/crates/socket-patch-core/src/vendor/pnpm_lock.rs b/crates/socket-patch-core/src/vendor/pnpm_lock.rs index d80b04d29..9d032904a 100644 --- a/crates/socket-patch-core/src/vendor/pnpm_lock.rs +++ b/crates/socket-patch-core/src/vendor/pnpm_lock.rs @@ -80,11 +80,12 @@ use crate::formats::pnpm::lines::{ unquote_value, yaml_key, yaml_key_like, YamlBlock, }; use crate::formats::pnpm::plan::{match_key, KeyMatch, Target}; +use crate::formats::pnpm::sniff_lock_grammar; use crate::formats::pnpm::workspace; use crate::formats::pnpm::{vendored_npm_uuids, PnpmLockGrammar}; const PACKAGE_JSON: &str = "package.json"; -use crate::utils::line_endings::{to_lf, LineEndings}; +use crate::utils::line_endings::LineEndings; use crate::utils::pnpm_workspace::{governing_workspace_file, PNPM_WORKSPACE}; /// Refusal code for a workspace member whose pnpm settings live in an @@ -178,11 +179,10 @@ impl PnpmDialect { } /// The grammar of a lock this dialect rewrites: one the vendored target - /// takes ([`Target::check_lock`]) and this dialect splices. + /// rewrites ([`sniff_lock_grammar`]: 5.4, 6.0 and 9.0, the grammars its + /// rekey was captured against; N16) and this dialect splices. pub(super) fn check_lock(self, text: &str) -> Result { - let grammar = Target::Vendored - .check_lock(text)? - .unwrap_or(PnpmLockGrammar::V9); + let grammar = sniff_lock_grammar(text)?; match (self, grammar) { (Self::V9, PnpmLockGrammar::V9) | (Self::Legacy, PnpmLockGrammar::V54 | PnpmLockGrammar::V60) => Ok(grammar), @@ -417,7 +417,7 @@ impl NpmLockBackend for PnpmBackend { .render(&pkg) .map_err(|e| format!("cannot serialize {PACKAGE_JSON}: {e}"))?; // An env document ahead of the project lock is carried over verbatim. - // Each file is written back in its own line endings (`lf_text`). + // Each file is written back in its own line endings (`read_pnpm_text`). let lock_out = format!("{lock_prefix}{}", lock.lines().join("\n")); let ws_out = ws_edit.new_text.as_deref().map(|t| ws_eol.restore(t)); let ws_was = ws_text.as_deref().map(|t| ws_eol.restore(t)); @@ -485,7 +485,7 @@ struct PnpmProject { /// Everything ahead of the project document, written back verbatim. lock_prefix: String, /// The line endings the lock and the workspace file are written back - /// with; their texts here are LF ([`lf_text`]). + /// with; their texts here are LF ([`read_pnpm_text`]). lock_eol: LineEndings, ws_text: Option, ws_eol: LineEndings, @@ -584,15 +584,9 @@ async fn read_project( { return Err(Box::new(refused(refusal.0, refusal.1))); } - let lock_text = match read_regular_to_string(&project_root.join(PNPM_LOCK)).await { - Ok(text) => text, - Err(e) => { - return Err(Box::new(refused( - "vendor_lockfile_missing", - format!("cannot read {PNPM_LOCK}: {e} — run `pnpm install` first"), - ))); - } - }; + let (lock_text, lock_eol) = read_pnpm_text(project_root, PNPM_LOCK) + .await + .map_err(|e| read_refusal(PNPM_LOCK, e, "run `pnpm install` first"))?; let grammar = match dialect.check_lock(&lock_text) { Ok(grammar) => grammar, Err(detail) => { @@ -602,7 +596,6 @@ async fn read_project( ))); } }; - let (lock_text, lock_eol) = lf_text(PNPM_LOCK, lock_text)?; if dialect == PnpmDialect::Legacy { return Ok(PnpmProject { pkg_bytes, @@ -645,18 +638,16 @@ async fn read_project( // encoding, permissions, a FIFO) must refuse — treating it as absent // would route into the create path, which OVERWRITES the user's // workspace definition with the root-only scaffold. - let (ws_text, ws_eol) = match read_regular_to_string(&project_root.join(PNPM_WORKSPACE)).await { - Ok(text) => lf_text(PNPM_WORKSPACE, text).map(|(t, eol)| (Some(t), eol))?, + let (ws_text, ws_eol) = match read_pnpm_text(project_root, PNPM_WORKSPACE).await { + Ok((text, eol)) => (Some(text), eol), Err(e) if e.kind() == std::io::ErrorKind::NotFound => (None, LineEndings::None), Err(e) => { - return Err(Box::new(refused( - "vendor_lockfile_missing", - format!( - "cannot read {PNPM_WORKSPACE}: {e} — vendoring mirrors the override \ - into it (pnpm >= 11 reads overrides from pnpm-workspace.yaml); fix \ - the file and retry" - ), - ))); + return Err(read_refusal( + PNPM_WORKSPACE, + e, + "vendoring mirrors the override into it (pnpm >= 11 reads overrides from \ + pnpm-workspace.yaml); fix the file and retry", + )) } }; // A workspace member with its own lock (`sharedWorkspaceLockfile: @@ -696,25 +687,57 @@ async fn read_project( }) } -/// A pnpm file's text with LF line endings, and the endings it is written -/// back with. A CRLF file (a Windows autocrlf checkout) is read and edited -/// like its LF twin and written back CRLF, as hosted mode's byte splices -/// keep it (divergences N15). A file that mixes endings is refused: no one -/// ending writes it back byte-faithfully. -fn lf_text(file: &str, text: String) -> Result<(String, LineEndings), Box> { +/// A pnpm file mixing CRLF and LF (see [`read_pnpm_text`]). +#[derive(Debug)] +struct MixedEndings(&'static str); + +impl std::fmt::Display for MixedEndings { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!( + f, + "{} mixes CRLF and LF line endings, which this rewriter cannot write back \ + byte-faithfully — normalize the file to one line ending and retry", + self.0 + ) + } +} + +impl std::error::Error for MixedEndings {} + +/// Read `file` under `root` as every vendored pnpm reader does (vendoring, +/// its pre-flight and the revert): its text with LF line endings, and the +/// endings it is written back with. A CRLF file (a Windows autocrlf +/// checkout) is edited like its LF twin and written back CRLF, as hosted +/// mode's byte splices keep it (divergences N15). A file that mixes endings +/// is an error ([`is_mixed`]): no one ending writes it back byte-faithfully, +/// so vendoring refuses it and the revert fails before any write. +async fn read_pnpm_text(root: &Path, file: &'static str) -> std::io::Result<(String, LineEndings)> { + let text = read_regular_to_string(&root.join(file)).await?; match LineEndings::of(&text) { - LineEndings::Mixed => Err(Box::new(refused( - "vendor_lockfile_crlf_unsupported", - format!( - "{file} mixes CRLF and LF line endings, which this rewriter cannot write \ - back byte-faithfully — normalize the file to one line ending and retry" - ), - ))), + LineEndings::Mixed => Err(std::io::Error::other(MixedEndings(file))), LineEndings::Crlf => Ok((text.replace("\r\n", "\n"), LineEndings::Crlf)), eol => Ok((text, eol)), } } +/// Whether a [`read_pnpm_text`] error is a mixed-ending file. +fn is_mixed(e: &std::io::Error) -> bool { + e.get_ref().is_some_and(|inner| inner.is::()) +} + +/// The vendoring refusal for a [`read_pnpm_text`] error other than a +/// missing file: `vendor_lockfile_crlf_unsupported` for mixed endings, else +/// `vendor_lockfile_missing` with `remedy`. +fn read_refusal(file: &str, e: std::io::Error, remedy: &str) -> Box { + Box::new(match is_mixed(&e) { + true => refused("vendor_lockfile_crlf_unsupported", e.to_string()), + false => refused( + "vendor_lockfile_missing", + format!("cannot read {file}: {e} — {remedy}"), + ), + }) +} + /// `(prefix, project document)` of a v9 lock ([`split_project_document`]), /// the project document's own head version-checked. `Err` names why the /// lock's documents are not pnpm's layout. @@ -968,30 +991,58 @@ pub(super) async fn revert_pnpm_dialect( return blocked; } } - // A v9 lock whose documents are not pnpm's layout cannot be reverted - // without guessing which document holds the wiring: fail before any - // surface is touched (the dry run too, so the preview never advertises - // a revert the wet run refuses) instead of reverting package.json and - // the workspace file around a lock left wired to the artifact. - let mut lock_text: Option> = None; - if dialect == PnpmDialect::V9 - && entry + // Read the lock and the workspace file the records touch up front, as + // vendoring reads them ([`read_pnpm_text`]), and fail before any surface + // is touched (the dry run too, so the preview never advertises a revert + // the wet run refuses) when one mixes CRLF and LF, or when a v9 lock's + // documents are not pnpm's layout: neither can be reverted without + // guessing, and reverting package.json and the workspace file around a + // lock left wired to the artifact would desync them. + let touches = |kind: fn(&str) -> bool| { + entry .wiring .iter() - .any(|r| r.file != PACKAGE_JSON && dialect.allows_revert_file(&r.file)) + .any(|r| dialect.allows_revert_file(&r.file) && kind(&r.file)) + }; + let read = |file| read_pnpm_text(project_root, file); + let lock_read = match touches(|f| f != PACKAGE_JSON) { + true => Some(read(PNPM_LOCK).await), + false => None, + }; + let ws_read = match touches(|f| f == PNPM_WORKSPACE) { + true => Some(read(PNPM_WORKSPACE).await), + false => None, + }; + for e in [&lock_read, &ws_read] + .into_iter() + .flatten() + .filter_map(|r| r.as_ref().err()) { - let read = read_regular_to_string(&project_root.join(PNPM_LOCK)).await; - if let Ok(text) = &read { - if let Err(why) = split_v9_lock(text) { - return RevertOutcome::failed(format!( - "{why}, so the vendored wiring of {} cannot be located; nothing was \ - reverted — restore the vendored {PNPM_LOCK} (or re-lock it with pnpm) \ - and re-run the revert", - entry.base_purl - )); - } + if is_mixed(e) { + return RevertOutcome::failed(format!("{e}; nothing was reverted")); } - lock_text = Some(read); + } + let mut lock_prefix = String::new(); + let mut lock_lines: Option> = None; + if let Some(Ok((text, _))) = &lock_read { + // The wiring lives in the project document; an env document ahead + // of it is written back verbatim. + let (prefix, project) = match dialect { + PnpmDialect::V9 => match split_v9_lock(text) { + Ok(split) => split, + Err(why) => { + return RevertOutcome::failed(format!( + "{why}, so the vendored wiring of {} cannot be located; nothing was \ + reverted — restore the vendored {PNPM_LOCK} (or re-lock it with \ + pnpm) and re-run the revert", + entry.base_purl + )) + } + }, + PnpmDialect::Legacy => ("", text.as_str()), + }; + lock_prefix = prefix.to_string(); + lock_lines = Some(split_lines(project)); } if dry_run { return RevertOutcome::ok(); @@ -1001,7 +1052,6 @@ pub(super) async fn revert_pnpm_dialect( // Partition by file through the allowlist (fail-closed skip+warning on // anything else — see REVERT_ALLOWLIST's security note). let mut touches_pkg = false; - let mut touches_lock = false; for rec in &entry.wiring { if !dialect.allows_revert_file(&rec.file) { outcome.warnings.push(VendorWarning::new( @@ -1013,49 +1063,23 @@ pub(super) async fn revert_pnpm_dialect( )); continue; } - if rec.file == PACKAGE_JSON { - touches_pkg = true; - } else { - touches_lock = true; - } + touches_pkg |= rec.file == PACKAGE_JSON; } - // Load both surfaces up front (fail-closed on unparseable; a missing - // file degrades to a warning and the artifact removal still proceeds). - let mut lock_lines: Option> = None; - let mut lock_prefix = String::new(); - let mut lock_eol = LineEndings::None; - if touches_lock { - let read = match lock_text { - Some(read) => read, - None => read_regular_to_string(&project_root.join(PNPM_LOCK)).await, - }; - match read { - // The wiring lives in the project document; an env document - // ahead of it is written back verbatim, in the lock's own line - // endings (N15). - Ok(text) => { - lock_eol = LineEndings::of(&text); - let text = to_lf(&text); - let (prefix, project) = match dialect { - PnpmDialect::V9 => match split_v9_lock(&text) { - Ok(split) => split, - Err(why) => return RevertOutcome::failed(why), - }, - PnpmDialect::Legacy => ("", &*text), - }; - lock_prefix = prefix.to_string(); - lock_lines = Some(split_lines(project)); - } - Err(e) if e.kind() == std::io::ErrorKind::NotFound => { - outcome.warnings.push(VendorWarning::new( - "vendor_lockfile_missing", - format!("{PNPM_LOCK} is missing; lock fragments cannot be restored"), - )); - } - Err(e) => return RevertOutcome::failed(format!("cannot read {PNPM_LOCK}: {e}")), + // A missing lock degrades to a warning and the artifact removal still + // proceeds; an unreadable one fails. + let lock_eol = match lock_read { + Some(Ok((_, eol))) => eol, + Some(Err(e)) if e.kind() == std::io::ErrorKind::NotFound => { + outcome.warnings.push(VendorWarning::new( + "vendor_lockfile_missing", + format!("{PNPM_LOCK} is missing; lock fragments cannot be restored"), + )); + LineEndings::None } - } + Some(Err(e)) => return RevertOutcome::failed(format!("cannot read {PNPM_LOCK}: {e}")), + None => LineEndings::None, + }; let mut pkg_state: Option<(Value, JsonLayout)> = None; if touches_pkg { match read_regular_to_bytes(&project_root.join(PACKAGE_JSON)).await { @@ -1186,9 +1210,10 @@ pub(super) async fn revert_pnpm_dialect( && build_approvals::is_ws_kind(&r.kind) }) .collect(); - if ws_rec.is_some() || !ws_approvals.is_empty() { + if let Some(read) = ws_read.filter(|_| ws_rec.is_some() || !ws_approvals.is_empty()) { if let Err(e) = revert_workspace( project_root, + read, ws_rec, &ws_approvals, entry, @@ -1217,9 +1242,11 @@ pub(super) async fn revert_pnpm_dialect( /// out of a file we edited (restoring the taken-over value, and dropping an /// `overrides:` section we created once it empties). Drift ⇒ warning, left /// alone. Build approvals vendoring added (`approvals`) are taken out too. -/// `Err` is a genuine write failure. +/// `read` is the file as [`read_pnpm_text`] read it (LF, with the endings +/// it is written back in). `Err` is a genuine read or write failure. async fn revert_workspace( project_root: &Path, + read: std::io::Result<(String, LineEndings)>, rec: Option<&WiringRecord>, approvals: &[&WiringRecord], entry: &VendorEntry, @@ -1235,8 +1262,8 @@ async fn revert_workspace( }; let entry_uuid = entry.uuid.as_str(); let path = project_root.join(PNPM_WORKSPACE); - let text = match read_regular_to_string(&path).await { - Ok(t) => t, + let (text, eol) = match read { + Ok(read) => read, Err(e) if e.kind() == std::io::ErrorKind::NotFound => { // ALREADY CONVERGED: for an Added override (no recorded // original) the reverted state is "no override" — a missing @@ -1253,10 +1280,6 @@ async fn revert_workspace( } Err(e) => return Err(format!("cannot read {PNPM_WORKSPACE}: {e}")), }; - // A CRLF file (an autocrlf checkout of what vendoring wrote) is read - // like its LF twin and written back CRLF (N15). - let eol = LineEndings::of(&text); - let text = to_lf(&text).into_owned(); // Fast path: a file we created that is still byte-identical to the // scaffold we wrote → delete it (byte-restore to "no file"). @@ -1419,11 +1442,6 @@ impl EditCtx<'_> { vendor_value_is_for(value, self.name, self.version) } - /// [`target_key`] of a packages/snapshots key for this release. - pub(super) fn target_key(&self, key: &str) -> Option { - target_key(key, self.name, self.version) - } - /// The per-importer `specifier:` spelling: re-relativized for nested /// importers, root-relative for `.` (spike P7). fn spec_for_importer(&self, importer: &str) -> String { @@ -1803,26 +1821,28 @@ fn check_rewritable_refs_with( } } // The positions the checks below read: from the index, the earliest - // position each refusal can hold at; by the scan, every one. - let scanned; - let (snapshot_refs, importer_refs): (Box + '_>, _) = - match index { - Some(index) => ( - Box::new( - index - .snapshot_ref_candidates(®_key, name, version) - .into_iter(), - ), - index.importer_ref_candidates(®_key, name, version), + // position each refusal can hold at; by the scan, every one, read + // lazily so the scan stops at the first refusal. + type Refs<'a, T> = Box + 'a>; + let (snapshot_refs, importer_refs): (Refs, Refs) = match index { + Some(index) => ( + Box::new( + index + .snapshot_ref_candidates(®_key, name, version) + .into_iter(), ), - None => { - scanned = importer_deps(lines); - ( - Box::new(snapshot_positions(lines)), - scanned.iter().collect(), - ) - } - }; + Box::new( + index + .importer_ref_candidates(®_key, name, version) + .into_iter() + .map(ImporterDep::as_ref), + ), + ), + None => ( + Box::new(snapshot_positions(lines)), + Box::new(importer_refs(lines)), + ), + }; for candidate in snapshot_refs { match candidate { SnapshotCandidate::Key(header) => { @@ -1846,23 +1866,18 @@ fn check_rewritable_refs_with( } } for entry in importer_refs { - let dep = entry.dep.as_str(); - let Some(v) = entry.ver.as_deref().map(unquote_value) else { + let Some(v) = entry.ver.map(unquote_value) else { continue; }; if v == reg_key || v.starts_with(&key_peer_prefix) { return refuse("an aliased importer version", v); } - if dep == name && v.starts_with(&val_peer_prefix) { + if entry.dep == name && v.starts_with(&val_peer_prefix) { return refuse("a peer-suffixed importer version", v); } // pnpm spells the default catalog `'catalog:'` (quoted) and named // ones `catalog:` (bare). - if let Some(s) = entry - .spec - .as_deref() - .filter(|_| dep == name && v == version) - { + if let Some(s) = entry.spec.filter(|_| entry.dep == name && v == version) { if unquote_value(s).starts_with("catalog:") { return refuse_catalog(&format!("importer `{}` specifier", entry.importer), s); } @@ -1887,27 +1902,33 @@ fn snapshot_positions(lines: &[String]) -> impl Iterator Vec { - let Some((start, end)) = section_bounds(lines, "importers") else { - return Vec::new(); - }; - let mut out = Vec::new(); - let mut i = start + 1; - while let Some(importer) = next_block(lines, i, end) { - out.extend( - dep_entries(lines, importer.header + 1, importer.end, 6) - .into_iter() - .map(|e| ImporterDep { - importer: importer.key.clone(), - dep: e.dep, - spec: e.spec.map(|(_, s)| s), - ver: e.ver.map(|(_, v)| v), - }), - ); - i = importer.end; - } - out +/// One importer dep entry as the ref gate reads it ([`ImporterDep`], +/// borrowed). +struct ImporterRef<'a> { + importer: &'a str, + dep: &'a str, + spec: Option<&'a str>, + ver: Option<&'a str>, +} + +/// Every importer's [`DepEntry`]s, in lock order, read lazily. +fn importer_refs(lines: &[String]) -> impl Iterator> { + let blocks = section_bounds(lines, "importers").into_iter(); + blocks + .flat_map(move |(start, end)| { + std::iter::successors(next_block(lines, start + 1, end), move |b| { + next_block(lines, b.end, end) + }) + }) + .flat_map(move |importer| { + let key = parse_key_line(&lines[importer.header], 2).map_or("", |(key, ..)| key); + dep_entries(lines, importer.header + 1, importer.end, 6).map(move |e| ImporterRef { + importer: key, + dep: e.dep, + spec: e.spec.map(|(_, s)| s), + ver: e.ver.map(|(_, v)| v), + }) + }) } // ───────────────────────── package.json override ───────────────────────── @@ -2371,48 +2392,58 @@ pub(super) fn overrides_record( /// One dependency entry with nested `specifier:` / `version:` fields (each /// with its line): a v9 importer dep (key at 6, fields at 8) or a legacy -/// 6.0 root dep (2 / 4). -pub(super) struct DepEntry { - pub(super) dep: String, - pub(super) spec: Option<(usize, String)>, - pub(super) ver: Option<(usize, String)>, +/// 6.0 root dep (2 / 4). Borrowed from the lines. +pub(super) struct DepEntry<'l> { + pub(super) dep: &'l str, + pub(super) spec: Option<(usize, &'l str)>, + pub(super) ver: Option<(usize, &'l str)>, } -/// Every [`DepEntry`] keyed at `indent` in `[start, end)`, in order. A key -/// with an inline value is not one; unknown fields are skipped. +impl DepEntry<'_> { + /// `(specifier line, specifier, version line, version)`, owned so the + /// caller can rewrite the lines; `None` unless both fields are present. + pub(super) fn pair(&self) -> Option<(usize, String, usize, String)> { + let ((si, spec), (vi, ver)) = (self.spec?, self.ver?); + Some((si, spec.to_string(), vi, ver.to_string())) + } +} + +/// Every [`DepEntry`] keyed at `indent` in `[start, end)`, in order, read +/// lazily. A key with an inline value is not one; unknown fields are +/// skipped. pub(super) fn dep_entries( lines: &[String], start: usize, end: usize, indent: usize, -) -> Vec { - let mut out = Vec::new(); +) -> impl Iterator> { let mut k = start; - while k < end { - let Some((dep, _repr, "")) = parse_key_line(&lines[k], indent) else { + std::iter::from_fn(move || { + while k < end { k += 1; - continue; - }; - let mut entry = DepEntry { - dep: dep.to_string(), - spec: None, - ver: None, - }; - k += 1; - while let Some((field, _repr, value)) = lines[..end] - .get(k) - .and_then(|l| parse_key_line(l, indent + 2)) - { - match field { - "specifier" => entry.spec = Some((k, value.to_string())), - "version" => entry.ver = Some((k, value.to_string())), - _ => {} + let Some((dep, _repr, "")) = parse_key_line(&lines[k - 1], indent) else { + continue; + }; + let mut entry = DepEntry { + dep, + spec: None, + ver: None, + }; + while let Some((field, _repr, value)) = lines[..end] + .get(k) + .and_then(|l| parse_key_line(l, indent + 2)) + { + match field { + "specifier" => entry.spec = Some((k, value)), + "version" => entry.ver = Some((k, value)), + _ => {} + } + k += 1; } - k += 1; + return Some(entry); } - out.push(entry); - } - out + None + }) } /// Edit 2: every importer's dep entry for the exact `name@version` — @@ -2432,12 +2463,13 @@ fn edit_importers( let importer_key = importer.key; // Dep entries sit at 6-space indent under the 4-space dep-type // headers; their fields at 8. - for entry in dep_entries(lines.read(), importer.header + 1, importer.end, 6) { - let (Some((si, old_spec)), Some((vi, old_ver))) = (entry.spec, entry.ver) else { - continue; - }; + let pairs: Vec<_> = dep_entries(lines.read(), importer.header + 1, importer.end, 6) + .filter(|entry| entry.dep == ctx.name) + .filter_map(|entry| entry.pair()) + .collect(); + for (si, old_spec, vi, old_ver) in pairs { let target = old_ver == ctx.version || (old_ver != ctx.spec && ctx.is_ours(&old_ver)); - if entry.dep != ctx.name || !target { + if !target { continue; } let importer_spec = ctx.spec_for_importer(&importer_key); @@ -2445,7 +2477,7 @@ fn edit_importers( file: PNPM_LOCK.to_string(), kind: KIND_LOCK_IMPORTER_DEP.to_string(), action: WiringAction::Rewritten, - key: Some(format!("{importer_key}|{}", entry.dep)), + key: Some(format!("{importer_key}|{}", ctx.name)), original: (!ctx.is_ours(&old_ver)) .then(|| serde_json::json!({ "specifier": old_spec, "version": old_ver })), new: Some(serde_json::json!({ @@ -2474,12 +2506,11 @@ fn edit_packages( let bounds = lines.bounds("packages").ok_or("no packages: section")?; let reg_key = ctx.reg_key(); let new_key = ctx.new_key(); - let matches = target_blocks(lines, "packages", bounds, ctx.name, ctx.version)?; - // The first registry-or-ours block is the one rewritten (every path // below returns). - if let Some(block) = matches.into_iter().next() { - let is_ours_key = ctx.target_key(&block.key) == Some(true); + if let Some((block, is_ours_key)) = + target_block(lines, "packages", bounds, ctx.name, ctx.version)? + { let original_lines: Vec = lines.read()[block.header..block.end].to_vec(); let expected_resolution = format!( " resolution: {{integrity: {}, tarball: {}}}", @@ -2556,11 +2587,10 @@ fn edit_snapshot_rekey( }; let reg_key = ctx.reg_key(); let new_key = ctx.new_key(); - let matches = target_blocks(lines, "snapshots", bounds, ctx.name, ctx.version)?; - // The first registry-or-ours block decides (every path below returns). - if let Some(block) = matches.into_iter().next() { - let is_ours_key = ctx.target_key(&block.key) == Some(true); + if let Some((block, is_ours_key)) = + target_block(lines, "snapshots", bounds, ctx.name, ctx.version)? + { if block.key == new_key { return Ok(false); // in sync } @@ -2671,10 +2701,11 @@ fn edit_snapshot_refs( } /// The blocks of `section` (`[start, end)`, its current bounds) whose key is -/// a [`target_key`] of `name@version`, in lock order — the only blocks the -/// packages/snapshots edits act on. From the index while the lines are the -/// memoized split (its candidates are a superset, filtered by the same -/// predicate), else by the scan. +/// a [`target_key`] of `name@version`, each with that classification (`true`: +/// a rekey), in lock order — the only blocks the packages/snapshots edits +/// act on. From the index while the lines are the memoized split (its +/// candidates are a superset, filtered by the same predicate), else by the +/// scan. fn matching_blocks( lines: &L, section: &str, @@ -2682,52 +2713,49 @@ fn matching_blocks( end: usize, name: &str, version: &str, -) -> Vec { - let keep = |key: &str| target_key(key, name, version).is_some(); +) -> Vec<(YamlBlock, bool)> { + let classify = |block: YamlBlock| Some((target_key(&block.key, name, version)?, block)); if let Some(section) = lines.index().and_then(|index| index.section(section)) { return section .candidates(name, version) .into_iter() - .filter(|block| keep(&block.key)) - .map(IndexedBlock::to_block) + .filter_map(|block| classify(block.to_block())) + .map(|(rekeyed, block)| (block, rekeyed)) .collect(); } let lines = lines.read(); - let mut found = Vec::new(); - let mut i = start + 1; - while let Some(block) = next_block(lines, i, end) { - i = block.end; - if keep(&block.key) { - found.push(block); - } - } - found + let blocks = std::iter::successors(next_block(lines, start + 1, end), |b| { + next_block(lines, b.end, end) + }); + blocks + .filter_map(classify) + .map(|(rekeyed, block)| (block, rekeyed)) + .collect() } -/// [`matching_blocks`], failing closed on a half-drifted lock: when the -/// section carries BOTH the registry entry and a rekey of it, a rekey would -/// splice a DUPLICATE mapping key (pnpm refuses to parse those) and surgery -/// cannot decide which block carries the truth. -pub(super) fn target_blocks( +/// The block the packages/snapshots edit rewrites: the first +/// [`matching_blocks`] entry, with whether it is a rekey already made. +/// Fails closed on a half-drifted lock: when the section carries BOTH the +/// registry entry and a rekey of it, a rekey would splice a DUPLICATE +/// mapping key (pnpm refuses to parse those) and surgery cannot decide +/// which block carries the truth. +pub(super) fn target_block( lines: &L, section: &str, (start, end): (usize, usize), name: &str, version: &str, -) -> Result, String> { +) -> Result, String> { let matches = matching_blocks(lines, section, start, end, name, version); - let rekeyed = |want| move |b: &&YamlBlock| target_key(&b.key, name, version) == Some(want); - match ( - matches.iter().find(rekeyed(false)), - matches.iter().any(|b| rekeyed(true)(&b)), - ) { - (Some(registry), true) => Err(format!( + let registry = matches.iter().find(|(_, rekeyed)| !rekeyed); + if let Some((registry, _)) = registry.filter(|_| matches.iter().any(|(_, rekeyed)| *rekeyed)) { + return Err(format!( "{section} section carries BOTH `{}` and a vendored `file:` rekey of it (a \ half-edited lock); run `pnpm install` to re-resolve it, then re-vendor", registry.key - )), - _ => Ok(matches), + )); } + Ok(matches.into_iter().next()) } // ─────────────────────── memoized split + section index ──────────────────── @@ -2918,12 +2946,12 @@ impl SectionIndex { let at = index.blocks.len(); let key = block.key.strip_prefix('/').unwrap_or(&block.key); index.by_key.entry(key.to_string()).or_default().push(at); - for (p, _) in format!("@{}", block.key).match_indices("@file:") { - index - .by_file_prefix - .entry(block.key[..p.saturating_sub(1)].to_string()) - .or_default() - .push(at); + // A bare `file:` key files under `""`, a `name@file:` one under `name`. + let bare = block.key.starts_with("file:").then_some(0); + let named = block.key.match_indices("@file:").map(|(p, _)| p); + for p in bare.into_iter().chain(named) { + let prefix = block.key[..p].to_string(); + index.by_file_prefix.entry(prefix).or_default().push(at); } index.blocks.push(IndexedBlock { header: block.header, @@ -2967,6 +2995,17 @@ struct ImporterDep { ver: Option, } +impl ImporterDep { + fn as_ref(&self) -> ImporterRef<'_> { + ImporterRef { + importer: &self.importer, + dep: &self.dep, + spec: self.spec.as_deref(), + ver: self.ver.as_deref(), + } + } +} + /// A position the snapshot scan of [`check_rewritable_refs`] may refuse /// at: a block header or a body line (by line). enum SnapshotCandidate { @@ -3062,8 +3101,15 @@ impl LockIndex { } } - index.importer_deps = importer_deps(lines); - index.importer_deps.retain(|entry| entry.ver.is_some()); + index.importer_deps = importer_refs(lines) + .filter(|entry| entry.ver.is_some()) + .map(|e| ImporterDep { + importer: e.importer.to_string(), + dep: e.dep.to_string(), + spec: e.spec.map(str::to_string), + ver: e.ver.map(str::to_string), + }) + .collect(); for (at, entry) in index.importer_deps.iter().enumerate() { let (dep, v) = ( &entry.dep, @@ -3415,9 +3461,7 @@ impl DepPair<'_> { warnings: &mut Vec, ) { let (rec, key, label) = (self.rec, self.key, self.label); - let entry = dep_entries(lines, start, end, indent) - .into_iter() - .find(|e| e.dep == self.dep); + let entry = dep_entries(lines, start, end, indent).find(|e| e.dep == self.dep); let Some((Some((si, live_spec)), Some((vi, live_ver)))) = entry.map(|e| (e.spec, e.ver)) else { return self.removed(warnings); @@ -3431,13 +3475,13 @@ impl DepPair<'_> { } let original = rec.original.as_ref(); if original.is_some() - && field(original, "specifier") == Some(&live_spec) - && field(original, "version") == Some(&live_ver) + && field(original, "specifier") == Some(live_spec) + && field(original, "version") == Some(live_ver) { return; } - let ours = field(rec.new.as_ref(), "version") == Some(&live_ver) - || parse_vendor_path(&live_ver) + let ours = field(rec.new.as_ref(), "version") == Some(live_ver) + || parse_vendor_path(live_ver) .is_some_and(|p| p.eco == "npm" && p.uuid == self.entry_uuid); if !ours { return warnings.push(drifted(format!( @@ -5999,6 +6043,57 @@ snapshots: format!("---\n{env}\n---\n{project}") } + /// N15 on pnpm 11's two-document lock: vendored, then checked out with + /// autocrlf (`---\r` separators), the lock reverts to its CRLF bytes. + /// Review finding: the revert's document check read the raw CRLF text, + /// refused `---\r` as a marker line, and failed every such revert. + #[tokio::test] + async fn crlf_two_document_lock_reverts() { + let before = two_doc(ENV_DOC_CONFIG_DEP, P1_BEFORE_LOCK); + let fx = fixture_with(P1_BEFORE_PKG, &before).await; + let (_, entry, _) = expect_done(fx.vendor(false).await); + let crlf = fx.read(PNPM_LOCK).await.replace('\n', "\r\n"); + tokio::fs::write(fx.root().join(PNPM_LOCK), &crlf) + .await + .unwrap(); + let outcome = revert_pnpm(&entry.unwrap(), fx.root(), false).await; + assert!(outcome.success, "{:?}", outcome.error); + assert_eq!(fx.read(PNPM_LOCK).await, before.replace('\n', "\r\n")); + } + + /// Review finding: a revert met a lock or workspace file that MIXES CRLF + /// and LF and wrote it back all-LF. It now fails before any write, dry + /// run included, as vendoring refuses such a file: every surface and the + /// artifact stay as they are. + #[tokio::test] + async fn mixed_line_ending_revert_fails_before_any_write() { + for file in [PNPM_LOCK, PNPM_WORKSPACE] { + let fx = fixture_with(P1_BEFORE_PKG, P1_BEFORE_LOCK).await; + let (_, entry, _) = expect_done(fx.vendor(false).await); + let entry = entry.unwrap(); + let mixed = fx.read(file).await.replacen('\n', "\r\n", 1); + tokio::fs::write(fx.root().join(file), &mixed) + .await + .unwrap(); + let pkg = fx.read(PACKAGE_JSON).await; + for dry_run in [true, false] { + let outcome = revert_pnpm(&entry, fx.root(), dry_run).await; + assert!(!outcome.success, "{file}"); + let error = outcome.error.unwrap_or_default(); + assert!( + error.contains(&format!("{file} mixes CRLF and LF")), + "{error}" + ); + } + assert_eq!(fx.read(file).await, mixed, "{file} left alone"); + assert_eq!(fx.read(PACKAGE_JSON).await, pkg, "{file}: nothing reverted"); + assert!( + fx.root().join(fx.rel_tgz()).exists(), + "{file}: artifact kept" + ); + } + } + /// #466: vendoring a two-document lock edits the PROJECT document only — /// byte-for-byte the single-document oracle — and leaves the env /// document untouched, even when a config dependency shares the @@ -6403,7 +6498,12 @@ snapshots: // Future versions: the fix is an allowlisted-emitting pnpm or a // newer build, never "pnpm >= 9" (the user's pnpm already is). - for head in ["lockfileVersion: '9.1'\n", "lockfileVersion: '10.0'\n"] { + // An unparseable major (review finding) is not "old" either. + for head in [ + "lockfileVersion: '9.1'\n", + "lockfileVersion: '10.0'\n", + "lockfileVersion: 'v10'\n", + ] { let err = sniff_lock_grammar(head).unwrap_err(); assert!(err.contains("update socket-patch"), "{err}"); assert!(!err.contains("re-lock with pnpm >= 9"), "{err}"); @@ -9821,19 +9921,13 @@ snapshots: let lines = split_lines( " dep:\n specifier: ^1.0.0\n engines: whatever\n version: 1.0.0\n other:\n inline: 1.0.0", ); - let entries = dep_entries(&lines, 0, lines.len(), 6); - let got: Vec<_> = entries - .iter() - .map(|e| (e.dep.as_str(), e.spec.clone(), e.ver.clone())) + let got: Vec<_> = dep_entries(&lines, 0, lines.len(), 6) + .map(|e| (e.dep, e.spec, e.ver)) .collect(); assert_eq!( got, vec![ - ( - "dep", - Some((1, "^1.0.0".to_string())), - Some((3, "1.0.0".to_string())) - ), + ("dep", Some((1, "^1.0.0")), Some((3, "1.0.0"))), ("other", None, None), ], "fields end at the first non-field line; an inline value is no entry" diff --git a/crates/socket-patch-core/src/vendor/pnpm_lock_legacy.rs b/crates/socket-patch-core/src/vendor/pnpm_lock_legacy.rs index 09fc48634..1c1dafb3e 100644 --- a/crates/socket-patch-core/src/vendor/pnpm_lock_legacy.rs +++ b/crates/socket-patch-core/src/vendor/pnpm_lock_legacy.rs @@ -65,8 +65,8 @@ use super::common::refused; use super::path::parse_vendor_path; use super::pnpm_lock::{ dep_entries, drifted, lines_value, lock_has_target_package, ref_record_blocks, removed, - revert_block, revert_overrides_line, target_blocks, target_key, vendor_value_is_for, DepPair, - DepRef, EditCtx, PnpmDialect, KIND_LOCK_OVERRIDES, KIND_LOCK_PACKAGE, + revert_block, revert_overrides_line, target_block, vendor_value_is_for, DepPair, DepRef, + EditCtx, PnpmDialect, KIND_LOCK_OVERRIDES, KIND_LOCK_PACKAGE, }; use super::source::PackageSource; use super::state::{VendorEntry, WiringAction, WiringRecord}; @@ -415,16 +415,15 @@ fn check_rewritable_refs(lines: &[String], ctx: &Ctx<'_>) -> Result<(), String> .iter() .filter_map(|line| parse_key_line(line, 2)) .filter(|(_, _, value)| !value.is_empty()) - .map(|(dep, _, value)| (dep.to_string(), value.to_string())); + .map(|(dep, _, value)| (dep, value)); let nested = dep_entries(lines, start + 1, end, 2) - .into_iter() .filter_map(|entry| Some((entry.dep, entry.ver?.1))); for (dep, value) in bare.chain(nested) { if value == reg_key || value.starts_with(&key_peer_prefix) { - return refuse("an aliased root dependency", &value); + return refuse("an aliased root dependency", value); } if dep == ctx.name && value.starts_with(&val_peer_prefix) { - return refuse("a peer-suffixed root dependency", &value); + return refuse("a peer-suffixed root dependency", value); } } } @@ -578,11 +577,12 @@ fn edit_root_deps_v60( let Some((start, end)) = section_bounds(lines, section) else { continue; }; - for entry in dep_entries(lines, start + 1, end, 2) { - let (Some((si, old_spec)), Some((vi, old_ver))) = (entry.spec, entry.ver) else { - continue; - }; - if entry.dep != ctx.name || !(old_ver == ctx.version || ctx.is_ours(&old_ver)) { + let pairs: Vec<_> = dep_entries(lines, start + 1, end, 2) + .filter(|entry| entry.dep == ctx.name) + .filter_map(|entry| entry.pair()) + .collect(); + for (si, old_spec, vi, old_ver) in pairs { + if !(old_ver == ctx.version || ctx.is_ours(&old_ver)) { continue; } hit = true; @@ -593,7 +593,7 @@ fn edit_root_deps_v60( file: PNPM_LOCK.to_string(), kind: KIND_LOCK_ROOT_DEP_PAIR.to_string(), action: WiringAction::Rewritten, - key: Some(format!("{section}|{}", entry.dep)), + key: Some(format!("{section}|{}", ctx.name)), original: (!ctx.is_ours(&old_ver)) .then(|| serde_json::json!({ "specifier": old_spec, "version": old_ver })), new: Some(serde_json::json!({ @@ -621,11 +621,11 @@ fn edit_packages( let bounds = section_bounds(lines, "packages").ok_or("no packages: section")?; let reg_key = ctx.reg_key(); let new_key = ctx.new_key(); - let matches = target_blocks(&*lines, "packages", bounds, ctx.name, ctx.version)?; // The first registry-or-ours block is the one rewritten (every path // below returns). - if let Some(block) = matches.into_iter().next() { - let is_ours_key = target_key(&block.key, ctx.name, ctx.version) == Some(true); + if let Some((block, is_ours_key)) = + target_block(&*lines, "packages", bounds, ctx.name, ctx.version)? + { let original_lines: Vec = lines[block.header..block.end].to_vec(); let expected_resolution = format!( " resolution: {{integrity: {}, tarball: {}}}", diff --git a/crates/socket-patch-core/src/vex/discover/npm.rs b/crates/socket-patch-core/src/vex/discover/npm.rs index 96d63c64e..b759d13a9 100644 --- a/crates/socket-patch-core/src/vex/discover/npm.rs +++ b/crates/socket-patch-core/src/vex/discover/npm.rs @@ -58,6 +58,7 @@ use super::{ DIAG_LOCKFILE_UNPARSEABLE, DIAG_REF_INVALID, DIAG_REF_UNATTRIBUTABLE, }; use crate::constants::npm_family::{NPM_LOCKS, PNPM_LOCK, PNPM_SHRINKWRAP_LEGACY}; +use crate::formats::pnpm::plan::governing_pnpm_lock; use crate::formats::pnpm::{ classify_pnpm_key, entry_bundled, entry_field, pnpm_registry_key, Bundled, PnpmDocument, PnpmKey, PnpmLock, PnpmPackage, @@ -627,14 +628,13 @@ fn entry_ref( /// ## Files /// /// * `pnpm-lock.yaml` at the root. -/// * `shrinkwrap.yaml` (pnpm 1 / 2) ONLY when there is no `pnpm-lock.yaml`: -/// pnpm >= 3 never reads it (it migrates it to `pnpm-lock.yaml` once), and -/// pnpm <= 2 never writes `pnpm-lock.yaml`, so a pair means one of them is -/// migration debris — the same call `lock_inventory` makes (it reads -/// shrinkwrap.yaml only on `vendor_lockfile_missing`). The hosted rewriter -/// edits both when both exist, so the live one is covered either way; a -/// stale shrinkwrap left carrying a Socket url must not attest from the -/// lockfile basis after `pnpm-lock.yaml` was reverted. +/// * `shrinkwrap.yaml` (pnpm 1 / 2) ONLY when there is no `pnpm-lock.yaml`, +/// or the project pins pnpm 1 / 2: pnpm >= 3 never reads it (it migrates +/// it to `pnpm-lock.yaml` once), and pnpm <= 2 reads nothing else, so a +/// pair means one of them is migration debris. The choice is the hosted +/// planner's (`formats::pnpm::plan::governing_pnpm_lock`, N11): the +/// other file is never rewritten, and a stale Socket url left in it must +/// not attest from the lockfile basis. /// * Rush (`rush.json` present): `common/config/rush/pnpm-lock.yaml` and every /// `common/config/subspaces//pnpm-lock.yaml` — the only non-root /// locks in scope, exactly the set `scan --mode hosted` collects and @@ -701,15 +701,20 @@ async fn extract_pnpm(ctx: &DiscoverCtx<'_>, out: &mut Discovery) { crate::utils::pnpm_workspace::lockfile_disabled(&ctx.view, false).await }; let disabled = disabled.as_deref(); - let root_lock = ctx.exists(PNPM_LOCK).await; - if root_lock { - extract_pnpm_lock(ctx, PNPM_LOCK, disabled, out).await; - // pnpm never reads the debris shrinkwrap: the patches it still names - // are recognized as NOT wired (module docs, rule 11), so a ledger - // record its stale Socket url alone keeps "mentioned" is dead. - ctx.recognize_ignored(PNPM_SHRINKWRAP_LEGACY).await; - } else { - extract_pnpm_lock(ctx, PNPM_SHRINKWRAP_LEGACY, disabled, out).await; + // The lock pnpm reads, as the hosted planner picks it + // (`formats::pnpm::plan::governing_pnpm_lock`, N11). pnpm never reads + // the other one: the patches it still names are recognized as NOT + // wired (module docs, rule 11), so a ledger record its stale Socket + // url alone keeps "mentioned" is dead. + let pinned = crate::utils::pnpm_workspace::pinned_pnpm_major_in(&ctx.view).await; + let (lock, shrinkwrap) = (PNPM_LOCK, PNPM_SHRINKWRAP_LEGACY); + let both = (ctx.exists(lock).await, ctx.exists(shrinkwrap).await); + if let Some(governing) = governing_pnpm_lock(both.0, both.1, pinned) { + extract_pnpm_lock(ctx, governing, disabled, out).await; + if both == (true, true) { + let shadowed = if governing == lock { shrinkwrap } else { lock }; + ctx.recognize_ignored(shadowed).await; + } } if rush { // The common lock, then each subspace's, sorted for deterministic @@ -3574,6 +3579,15 @@ mod tests { out.recognized_files(UUID_A, WiringMode::Hosted), vec![std::path::Path::new("shrinkwrap.yaml")] ); + // N11 review finding: a project that pins pnpm 1 / 2 installs from + // `shrinkwrap.yaml`, so there it is the lock that attests and the + // stray `pnpm-lock.yaml` is the shadowed one, as the hosted planner + // picks them (`plan::governing_pnpm_lock`). + with_lock.write("package.json", r#"{"packageManager": "pnpm@2.25.7"}"#); + assert_refs( + &run(&with_lock).await, + &[("pkg:npm/left-pad@1.3.0", UUID_A, WiringMode::Hosted)], + ); } /// `--patch-server-url` deployments count; the same URL without the diff --git a/docs/design/lock-model-divergences.md b/docs/design/lock-model-divergences.md index 00af98b9b..83bed89fa 100644 --- a/docs/design/lock-model-divergences.md +++ b/docs/design/lock-model-divergences.md @@ -31,7 +31,7 @@ follow from the install-tree decision. | J12 | scala-cli | Guidance only (`patch/redirect/scala_guidance.rs`) | Back-port: hosted wires scala-cli through the merged planner (a same-GAV Socket repository instead of a suffixed version). | | P3 | Is a shadowed `pdm.lock` rewritten? | Only when it governs (`pdm_drives`, ~564) | Rewrite it like every other tree, per the install-tree decision. | | P4 | Are `-r` includes of `requirements.txt` rewritten? | Root file only (`patch/redirect/requirements.rs` ~208) | Follow includes, per the install-tree decision. | -| N11 | Is a `shrinkwrap.yaml` beside `pnpm-lock.yaml` rewritten? | Yes; its all-or-nothing gate covers both files | **Resolved (owner: shadowed) in PR 2b.** Swept for stale identities, never written, and out of the all-or-nothing gate (`formats::pnpm::plan::planned_locks`). pnpm never reads it, and an unparseable debris key in it used to refuse the dependency in every lock. A `shrinkwrap.yaml` that is the only pnpm lock is rewritten as before. | +| N11 | Is a `shrinkwrap.yaml` beside `pnpm-lock.yaml` rewritten? | Yes; its all-or-nothing gate covers both files | **Resolved (owner: shadowed) in PR 2b.** Swept for stale identities, never written, and out of the all-or-nothing gate (`formats::pnpm::plan::governing_pnpm_lock`, which hosted and discovery share). When the project pins pnpm 1 / 2 (install record or package.json `packageManager`), which reads only `shrinkwrap.yaml`, the stray `pnpm-lock.yaml` is the shadowed one. pnpm never reads it, and an unparseable debris key in it used to refuse the dependency in every lock. A `shrinkwrap.yaml` that is the only pnpm lock is rewritten as before. | ## JVM @@ -89,7 +89,7 @@ follow from the install-tree decision. | N8 | npm `file:` directory link identity (#1481) | Hosted and discovery give a versionless link its source directory's `name@version`. Vendored matches the entry's own version, wiring the registry copies with no warning. | Hosted | silent vendored success | | N9 | yarn classic blocks reached only through an alias | Hosted skips them. Vendored rewrites them. Discovery counts an alias block as an install of the real package. | Back-port: hosted rewrites alias blocks (vendored supports what hosted lacks) | | | N10 | Locks of several npm-family package managers in one project | Hosted rewrites every family; discovery reads every family with the contest. The inventory and vendored pick one family (vlt > bun > pnpm > yarn > npm). | Union (separate trees) | | -| N11 | `shrinkwrap.yaml` beside `pnpm-lock.yaml` | Hosted reads and splices it, and its all-or-nothing gate covers both. Discovery sweeps it as ignored. The inventory reads it only as a fallback. Vendored never reads it. | **Resolved (owner: shadowed) in PR 2b**: hosted never rewrites it and leaves it out of the gate; discovery keeps sweeping it; vendored never reads it. A lone `shrinkwrap.yaml` keeps today's behavior. | | +| N11 | `shrinkwrap.yaml` beside `pnpm-lock.yaml` | Hosted reads and splices it, and its all-or-nothing gate covers both. Discovery sweeps it as ignored. The inventory reads it only as a fallback. Vendored never reads it. | **Resolved (owner: shadowed) in PR 2b**: hosted never rewrites it and leaves it out of the gate; discovery keeps sweeping it; vendored never reads it. A pnpm 1 / 2 pin flips the pair: `shrinkwrap.yaml` governs, `pnpm-lock.yaml` is shadowed. A lone `shrinkwrap.yaml` keeps today's behavior. | | | N12 | Rush common and subspace locks | Hosted and discovery always read them. The inventory reads them only when there is no root lock (#2003 aligns the listing itself). Vendored refuses Rush. | Always read. Vendored's refusal stays. **Resolved in PR 2a** (the inventory reads them whenever `rush.json` exists). | target-specific (vendored) | | N13 | Plug'n'Play | Vendored refuses. The hosted berry preflight has no PnP gate. The inventory returns an error, so `scan` sees nothing in a project hosted pinned. | The inventory reads PnP locks. Vendored keeps its refusal. | target-specific (vendored) | | N14 | pnpm `lockfile=false` | Hosted refuses. Discovery withholds hosted refs, keeps vendored ones. Vendored proceeds. The inventory ignores the setting. | The modes stay as they are; the inventory follows discovery. **Resolved in PR 2a**: under the setting (not under Rush), the inventory keeps a Socket-hosted pnpm entry but drops its integrity, as discovery withholds the lock pin. | target-specific | diff --git a/docs/ecosystems.md b/docs/ecosystems.md index 9e7d42b11..9a3d1f688 100644 --- a/docs/ecosystems.md +++ b/docs/ecosystems.md @@ -172,7 +172,8 @@ Each Legacy format has an upgrade path and an undo path. Both work in v5: 12's dual-lock state. Majors 6–12 are measured in [npm compatibility](testing/npm-compatibility.md). - **pnpm** — hosted rewriting supports legacy `shrinkwrap.yaml` (pnpm 1/2; never - rewritten beside a `pnpm-lock.yaml`, which pnpm reads instead), + rewritten beside a `pnpm-lock.yaml`, which pnpm >= 3 reads instead; a + project pinning pnpm 1/2 keeps `shrinkwrap.yaml` and leaves the stray lock), lockfileVersion 5.x (pnpm 3–7), 6.0 (pnpm 8), and 9.0 (pnpm 9–12). Early pnpm 1 locks with shrinkwrapVersion 3 and no positive minor version are refused because those installers discard hosted URLs; the tested diff --git a/scripts/loc-budget.json b/scripts/loc-budget.json index 8ffa1aab4..f5ebc49a3 100644 --- a/scripts/loc-budget.json +++ b/scripts/loc-budget.json @@ -17,7 +17,7 @@ "crates/socket-patch-core/src/utils/pipenv.rs", "crates/socket-patch-core/src/crawlers/deno_crawler.rs" ], - "max": 31201 + "max": 31206 }, "lock-writers": { "paths": [ @@ -30,7 +30,7 @@ "crates/socket-patch-core/src/vendor/npm_origin.rs", "crates/socket-patch-core/src/vendor/test_support" ], - "max": 88093 + "max": 88115 } } } From e7df82058ea24126708f363adc7b6b7dc33f57c4 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Thu, 5 Nov 2026 23:34:40 -0600 Subject: [PATCH 5/5] Refuse vendoring a pnpm 1 / 2 project beside its shrinkwrap.yaml Vendored mode now reads the same plan::governing_pnpm_lock decision as hosted mode and discovery. When the project pins pnpm 1 or 2 (install record, else package.json packageManager), pnpm installs from shrinkwrap.yaml and ignores pnpm-lock.yaml. Vendored mode does not write the shrinkwrap grammar, so it now refuses with vendor_lockfile_version_unsupported before any write, instead of wiring the stray pnpm-lock.yaml. With no pin, nothing changes. Documented in CLI_CONTRACT.md and the N11 divergence row. The mixed-ending read error is now an io::ErrorKind::Unsupported error instead of a custom type, which keeps lock-writers within budget (88113). Co-Authored-By: Claude Opus 5.5 --- crates/socket-patch-cli/CLI_CONTRACT.md | 2 +- .../socket-patch-core/src/vendor/pnpm_lock.rs | 98 ++++++++++++------- docs/design/lock-model-divergences.md | 2 +- scripts/loc-budget.json | 2 +- 4 files changed, 66 insertions(+), 38 deletions(-) diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index 4b7c6ec51..f133269e6 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -272,7 +272,7 @@ For a **9.0 root lock**, the CLI ensures `pnpm-workspace.yaml` carries `trustLoc **pnpm per-branch locks (#556)**: with `gitBranchLockfile` on (a top-level `gitBranchLockfile: true` in the root `pnpm-workspace.yaml`; when the YAML leaves the key unset, `git-branch-lockfile=true` in the root `.npmrc`, as pnpm 10 and older read it; on disk also `pnpm_config_git_branch_lockfile` (pnpm 11 and newer) or `npm_config_git_branch_lockfile` (pnpm 10 and older; either when the major is unknown, read as for the `lockfile` setting above) set to `true`; a workspace member with no `pnpm-workspace.yaml` of its own reads the governing ancestor's file and the `.npmrc` beside it), pnpm installs a git branch from its own `pnpm-lock..yaml` whenever that lock exists, in every directory it installs a lock from: the root, and under `sharedWorkspaceLockfile: false` each member's (#492). When the setting is on AND at least one such `pnpm-lock..yaml` exists (at the root, or in a member that installs from its own lock), hosted mode leaves every root pnpm lock (`pnpm-lock.yaml`, `shrinkwrap.yaml`) and `pnpm-workspace.yaml` untouched (every member lock too), confirms no npm pin from them and warns `redirect_pnpm_git_branch_lockfile` (in place of `redirect_pnpm_no_lockfile` when the branch lock is the only one); the detail names the setting and the branch lock(s), and the remedy is to turn `gitBranchLockfile` off and fold the branch locks into `pnpm-lock.yaml` with `pnpm install --merge-git-branch-lockfiles`, then re-run. A vendored→hosted takeover of a pnpm-vendored purl is refused with the same code (the vendored wiring and ledger stay byte-for-byte). Vendored mode refuses the same projects with `vendor_pnpm_git_branch_lockfile`, before any write and in the pre-flight. The setting with no branch lock (main, or a branch whose dependencies never changed) installs from `pnpm-lock.yaml`, which both modes wire as usual; a stray branch lock with the setting off is ignored. The in-memory engine fetches root `pnpm-lock.*.yaml` names presence-only and refuses the same way. -`redirect_pnpm_no_lockfile` names pnpm when installer markers exist without a lock; `redirect_pnpm_entry_vendored` identifies a vendored entry instead of reporting it missing. Supported `shrinkwrap.yaml` files are writable lockfiles, not read-only markers, when no `pnpm-lock.yaml` sits beside them. pnpm >= 3 never reads a `shrinkwrap.yaml` it has a `pnpm-lock.yaml` for, so that one is shadowed (unless the project pins pnpm 1 or 2 in `node_modules/.modules.yaml` or package.json's `packageManager`: those read only `shrinkwrap.yaml`, so there the stray `pnpm-lock.yaml` is the shadowed one): hosted mode never rewrites it and leaves it out of the all-or-nothing gate, so an entry it cannot repoint (or pnpm 1's early shrinkwrap head) refuses nothing, and `vex` only sweeps it for stale Socket identities. Vendored mode never writes it either. +`redirect_pnpm_no_lockfile` names pnpm when installer markers exist without a lock; `redirect_pnpm_entry_vendored` identifies a vendored entry instead of reporting it missing. Supported `shrinkwrap.yaml` files are writable lockfiles, not read-only markers, when no `pnpm-lock.yaml` sits beside them. pnpm >= 3 never reads a `shrinkwrap.yaml` it has a `pnpm-lock.yaml` for, so that one is shadowed (unless the project pins pnpm 1 or 2 in `node_modules/.modules.yaml` or package.json's `packageManager`: those read only `shrinkwrap.yaml`, so there the stray `pnpm-lock.yaml` is the shadowed one): hosted mode never rewrites it and leaves it out of the all-or-nothing gate, so an entry it cannot repoint (or pnpm 1's early shrinkwrap head) refuses nothing, and `vex` only sweeps it for stale Socket identities. Vendored mode never writes it either, and refuses a project that pins pnpm 1 or 2 beside a `shrinkwrap.yaml` (`vendor_lockfile_version_unsupported`: it does not write the shrinkwrap grammar, and the stray `pnpm-lock.yaml` would patch nothing pnpm installs). **vlt hosted-mode contract**: `scan` / `get --mode hosted` rewrite, in `vlt-lock.json`, every default-registry node of a granted `name@version` (the `''` / `npm` segment or a URL segment equal to the lock's scalar `registry`, both DepID grammars, every peer and modifier variant): slot [2] becomes the granted sha512 and slot [3] the hosted URL (appended to a 3-tuple); the DepID, flags and trailing slots, the line ending and every other byte stay. `options` is never edited and `vlt.json` is only read. A lock with another `lockfileVersion` (decided on the raw JSON token), a BOM, a non-object body or a `nodes` section outside vlt's one-node-per-line layout refuses the whole lock (`redirect_vlt_lock_unsupported`). **Confirmation**: vlt drives when its install state (`node_modules/.vlt-lock.json` or `node_modules/.vlt/`) is present or no other npm-family lock is; then only `vlt-lock.json` confirms a uuid. Otherwise every lock is rewritten, `redirect_vlt_sibling_lockfiles` warns, and the other locks' rules confirm, including a dep `vlt-lock.json` merely does not wire (`redirect_vlt_entry_not_found`, `redirect_vlt_entry_vendored`, or `redirect_vlt_custom_registry_skipped` alone). Whichever lock drives, a dep the vlt rewriter refuses (`redirect_vlt_missing_sha512`, `redirect_vlt_unsupported_lock_key`) is never confirmed by any lock, although a sibling lock may already carry its rewritten URL. **Artifact preflight**: before any takeover or write (dry runs included), each granted artifact with a default-registry instance is fetched once as vlt fetches it and must verify, else the dep is withheld (`redirect_vlt_artifact_unverifiable`, see the tag table). **Heal**: stale installed copies of Socket-owned nodes are removed so the next `vlt install` extracts the patched bytes, and `rollback` / `remove` do the same for the registry bytes (`--no-vlt-install-cleanup` keeps them; optional dependencies' copies are always kept); `redirect_vlt_reinstall_required` says what happened and what to run. The same-run `--vex` never attests a vlt package whose installed copy is stale or unchecked, whose lock a vlt release may ignore (`redirect_vlt_lockfile_version_missing`, `redirect_vlt_old_lockfile_ignored`, `redirect_vlt_scalar_registry_ignored`), or which also resolves from a non-default registry (`redirect_vlt_custom_registry_skipped`). `vlt.json` or vlt install state without `vlt-lock.json` warns `redirect_vlt_no_lockfile` instead of `redirect_npm_no_lockfile`. `rollback` / `remove` restore each hosted node's slots [2] and [3] from the version document of the registry the node resolves against (slot [3] is its `dist.tarball`, as vlt writes it; `upstream_registry_fallback` when that registry can't be read), following the lock's own slot-[3] convention (see "Hosted unwind coverage"). Tested releases: `docs/testing/vlt-compatibility.md`. diff --git a/crates/socket-patch-core/src/vendor/pnpm_lock.rs b/crates/socket-patch-core/src/vendor/pnpm_lock.rs index 9d032904a..4d616350d 100644 --- a/crates/socket-patch-core/src/vendor/pnpm_lock.rs +++ b/crates/socket-patch-core/src/vendor/pnpm_lock.rs @@ -74,12 +74,12 @@ use super::revert::{self, KeepPolicy}; use super::source::PackageSource; use super::state::{PnpmMeta, VendorEntry, WiringAction, WiringRecord}; use super::{RevertOpts, RevertOutcome, VendorOutcome, VendorWarning}; -use crate::constants::npm_family::PNPM_LOCK; +use crate::constants::npm_family::{PNPM_LOCK, PNPM_SHRINKWRAP_LEGACY}; use crate::formats::pnpm::lines::{ indent_of, next_block, parse_key_line, section_bounds, split_lines, split_project_document, unquote_value, yaml_key, yaml_key_like, YamlBlock, }; -use crate::formats::pnpm::plan::{match_key, KeyMatch, Target}; +use crate::formats::pnpm::plan::{governing_pnpm_lock, match_key, KeyMatch, Target}; use crate::formats::pnpm::sniff_lock_grammar; use crate::formats::pnpm::workspace; use crate::formats::pnpm::{vendored_npm_uuids, PnpmLockGrammar}; @@ -577,13 +577,24 @@ async fn read_project( ))); } }; - if let Some(refusal) = git_branch_lock_refusal( - &crate::vendor::lock_inventory::ProjectView::Disk(project_root), - ) - .await - { + let view = crate::vendor::lock_inventory::ProjectView::Disk(project_root); + if let Some(refusal) = git_branch_lock_refusal(&view).await { return Err(Box::new(refused(refusal.0, refusal.1))); } + // pnpm 1 / 2 read only `shrinkwrap.yaml`, whose grammar the rekey has no + // fixture for: there `pnpm-lock.yaml` is stray, and wiring it would + // patch nothing pnpm installs (N11, the choice hosted and discovery make). + let shrinkwrap = view.exists_no_follow(PNPM_SHRINKWRAP_LEGACY).await; + let pinned = crate::utils::pnpm_workspace::pinned_pnpm_major_in(&view).await; + if governing_pnpm_lock(true, shrinkwrap, pinned) == Some(PNPM_SHRINKWRAP_LEGACY) { + let why = "the project pins pnpm 1 / 2, which installs from shrinkwrap.yaml and ignores \ + pnpm-lock.yaml; vendored mode does not write that grammar — upgrade pnpm \ + and re-lock, or use `--mode hosted`"; + return Err(Box::new(refused( + "vendor_lockfile_version_unsupported", + why, + ))); + } let (lock_text, lock_eol) = read_pnpm_text(project_root, PNPM_LOCK) .await .map_err(|e| read_refusal(PNPM_LOCK, e, "run `pnpm install` first"))?; @@ -687,49 +698,34 @@ async fn read_project( }) } -/// A pnpm file mixing CRLF and LF (see [`read_pnpm_text`]). -#[derive(Debug)] -struct MixedEndings(&'static str); - -impl std::fmt::Display for MixedEndings { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!( - f, - "{} mixes CRLF and LF line endings, which this rewriter cannot write back \ - byte-faithfully — normalize the file to one line ending and retry", - self.0 - ) - } -} - -impl std::error::Error for MixedEndings {} - /// Read `file` under `root` as every vendored pnpm reader does (vendoring, /// its pre-flight and the revert): its text with LF line endings, and the /// endings it is written back with. A CRLF file (a Windows autocrlf /// checkout) is edited like its LF twin and written back CRLF, as hosted /// mode's byte splices keep it (divergences N15). A file that mixes endings -/// is an error ([`is_mixed`]): no one ending writes it back byte-faithfully, -/// so vendoring refuses it and the revert fails before any write. -async fn read_pnpm_text(root: &Path, file: &'static str) -> std::io::Result<(String, LineEndings)> { +/// is an `Unsupported` error, which no read of a regular file returns: no +/// one ending writes it back byte-faithfully, so vendoring refuses it and +/// the revert fails before any write. +async fn read_pnpm_text(root: &Path, file: &str) -> std::io::Result<(String, LineEndings)> { let text = read_regular_to_string(&root.join(file)).await?; match LineEndings::of(&text) { - LineEndings::Mixed => Err(std::io::Error::other(MixedEndings(file))), + LineEndings::Mixed => Err(std::io::Error::new( + std::io::ErrorKind::Unsupported, + format!( + "{file} mixes CRLF and LF line endings, which this rewriter cannot write back \ + byte-faithfully — normalize the file to one line ending and retry" + ), + )), LineEndings::Crlf => Ok((text.replace("\r\n", "\n"), LineEndings::Crlf)), eol => Ok((text, eol)), } } -/// Whether a [`read_pnpm_text`] error is a mixed-ending file. -fn is_mixed(e: &std::io::Error) -> bool { - e.get_ref().is_some_and(|inner| inner.is::()) -} - /// The vendoring refusal for a [`read_pnpm_text`] error other than a /// missing file: `vendor_lockfile_crlf_unsupported` for mixed endings, else /// `vendor_lockfile_missing` with `remedy`. fn read_refusal(file: &str, e: std::io::Error, remedy: &str) -> Box { - Box::new(match is_mixed(&e) { + Box::new(match e.kind() == std::io::ErrorKind::Unsupported { true => refused("vendor_lockfile_crlf_unsupported", e.to_string()), false => refused( "vendor_lockfile_missing", @@ -1018,7 +1014,7 @@ pub(super) async fn revert_pnpm_dialect( .flatten() .filter_map(|r| r.as_ref().err()) { - if is_mixed(e) { + if e.kind() == std::io::ErrorKind::Unsupported { return RevertOutcome::failed(format!("{e}; nothing was reverted")); } } @@ -6043,6 +6039,38 @@ snapshots: format!("---\n{env}\n---\n{project}") } + /// N11: a project that pins pnpm 1 / 2 installs from `shrinkwrap.yaml`, + /// so its `pnpm-lock.yaml` is stray. Vendoring refuses it, with nothing + /// written, instead of wiring a lock pnpm never reads, which is the + /// choice hosted mode and discovery make (`plan::governing_pnpm_lock`). + /// With no pin, the `pnpm-lock.yaml` governs and is vendored as before. + #[tokio::test] + async fn a_pnpm_2_pin_beside_a_shrinkwrap_refuses() { + let shrinkwrap = "shrinkwrapVersion: 3\nshrinkwrapMinorVersion: 9\n"; + let pinned = P1_BEFORE_PKG.replacen('{', "{\n \"packageManager\": \"pnpm@2.25.7\",", 1); + let fx = fixture_with(&pinned, P1_BEFORE_LOCK).await; + tokio::fs::write(fx.root().join("shrinkwrap.yaml"), shrinkwrap) + .await + .unwrap(); + let detail = expect_refused( + fx.vendor(false).await, + "vendor_lockfile_version_unsupported", + ); + assert!( + detail.contains("pnpm 1 / 2") && detail.contains("shrinkwrap.yaml"), + "{detail}" + ); + assert_eq!(fx.read(PNPM_LOCK).await, P1_BEFORE_LOCK, "lock untouched"); + assert_eq!(fx.read(PACKAGE_JSON).await, pinned, "pkg untouched"); + assert!(!fx.root().join(".socket/vendor").exists()); + + let fx = fixture_with(P1_BEFORE_PKG, P1_BEFORE_LOCK).await; + tokio::fs::write(fx.root().join("shrinkwrap.yaml"), shrinkwrap) + .await + .unwrap(); + assert!(matches!(fx.vendor(false).await, VendorOutcome::Done { .. })); + } + /// N15 on pnpm 11's two-document lock: vendored, then checked out with /// autocrlf (`---\r` separators), the lock reverts to its CRLF bytes. /// Review finding: the revert's document check read the raw CRLF text, diff --git a/docs/design/lock-model-divergences.md b/docs/design/lock-model-divergences.md index 83bed89fa..c9e5d9ca4 100644 --- a/docs/design/lock-model-divergences.md +++ b/docs/design/lock-model-divergences.md @@ -89,7 +89,7 @@ follow from the install-tree decision. | N8 | npm `file:` directory link identity (#1481) | Hosted and discovery give a versionless link its source directory's `name@version`. Vendored matches the entry's own version, wiring the registry copies with no warning. | Hosted | silent vendored success | | N9 | yarn classic blocks reached only through an alias | Hosted skips them. Vendored rewrites them. Discovery counts an alias block as an install of the real package. | Back-port: hosted rewrites alias blocks (vendored supports what hosted lacks) | | | N10 | Locks of several npm-family package managers in one project | Hosted rewrites every family; discovery reads every family with the contest. The inventory and vendored pick one family (vlt > bun > pnpm > yarn > npm). | Union (separate trees) | | -| N11 | `shrinkwrap.yaml` beside `pnpm-lock.yaml` | Hosted reads and splices it, and its all-or-nothing gate covers both. Discovery sweeps it as ignored. The inventory reads it only as a fallback. Vendored never reads it. | **Resolved (owner: shadowed) in PR 2b**: hosted never rewrites it and leaves it out of the gate; discovery keeps sweeping it; vendored never reads it. A pnpm 1 / 2 pin flips the pair: `shrinkwrap.yaml` governs, `pnpm-lock.yaml` is shadowed. A lone `shrinkwrap.yaml` keeps today's behavior. | | +| N11 | `shrinkwrap.yaml` beside `pnpm-lock.yaml` | Hosted reads and splices it, and its all-or-nothing gate covers both. Discovery sweeps it as ignored. The inventory reads it only as a fallback. Vendored never reads it. | **Resolved (owner: shadowed) in PR 2b**: hosted never rewrites it and leaves it out of the gate; discovery keeps sweeping it; vendored never reads it. A pnpm 1 / 2 pin flips the pair: `shrinkwrap.yaml` governs, `pnpm-lock.yaml` is shadowed, and vendored mode refuses the project (`vendor_lockfile_version_unsupported`). A lone `shrinkwrap.yaml` keeps today's behavior. | | | N12 | Rush common and subspace locks | Hosted and discovery always read them. The inventory reads them only when there is no root lock (#2003 aligns the listing itself). Vendored refuses Rush. | Always read. Vendored's refusal stays. **Resolved in PR 2a** (the inventory reads them whenever `rush.json` exists). | target-specific (vendored) | | N13 | Plug'n'Play | Vendored refuses. The hosted berry preflight has no PnP gate. The inventory returns an error, so `scan` sees nothing in a project hosted pinned. | The inventory reads PnP locks. Vendored keeps its refusal. | target-specific (vendored) | | N14 | pnpm `lockfile=false` | Hosted refuses. Discovery withholds hosted refs, keeps vendored ones. Vendored proceeds. The inventory ignores the setting. | The modes stay as they are; the inventory follows discovery. **Resolved in PR 2a**: under the setting (not under Rush), the inventory keeps a Socket-hosted pnpm entry but drops its integrity, as discovery withholds the lock pin. | target-specific | diff --git a/scripts/loc-budget.json b/scripts/loc-budget.json index f5ebc49a3..ecd7d7a83 100644 --- a/scripts/loc-budget.json +++ b/scripts/loc-budget.json @@ -30,7 +30,7 @@ "crates/socket-patch-core/src/vendor/npm_origin.rs", "crates/socket-patch-core/src/vendor/test_support" ], - "max": 88115 + "max": 88113 } } }