From ad40d1dbe71f9e840215334d2e129734975ac647 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Thu, 5 Nov 2026 13:23:55 -0600 Subject: [PATCH 1/7] 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/7] 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/7] 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/7] 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/7] 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 } } } From 64dfe54da26a67b0d7c77b83d891eef05f4a8ff3 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 7 Nov 2026 04:25:43 +0000 Subject: [PATCH 6/7] Test the pnpm takeover gate with a mixed-ending lock The refusals test merged in from main (#2015) expected a pure CRLF pnpm lock to be refused with vendor_lockfile_crlf_unsupported. This branch teaches the pnpm rewriter to wire pure CRLF locks byte-faithfully, so that refusal now applies only to locks that mix CRLF and LF. Give the test a mixed-ending lock so it still checks that a hosted pin's takeover meets the pnpm lock-text gates. Co-Authored-By: Claude --- crates/socket-patch-core/src/vendor/refusals.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/socket-patch-core/src/vendor/refusals.rs b/crates/socket-patch-core/src/vendor/refusals.rs index acd529e27..34be6f8b5 100644 --- a/crates/socket-patch-core/src/vendor/refusals.rs +++ b/crates/socket-patch-core/src/vendor/refusals.rs @@ -882,6 +882,8 @@ mod tests { /// A hosted pnpm pin's takeover meets the pnpm lock-text gates /// (#853); a ledger entry already at the selected uuid is in sync. + /// The lock mixes CRLF and LF endings, which the pnpm rewriter + /// cannot write back byte-faithfully (a pure CRLF lock is wired). #[tokio::test] async fn a_hosted_pin_meets_the_pnpm_gates_before_its_takeover() { let tmp = tempfile::tempdir().unwrap(); @@ -892,7 +894,7 @@ mod tests { .unwrap(); std::fs::write( tmp.path().join("pnpm-lock.yaml"), - HOSTED_V9.replace('\n', "\r\n"), + HOSTED_V9.replacen('\n', "\r\n", 1), ) .unwrap(); let pin = HostedPin { From 2b4475e67601971ca4ca3a61240e3cb57a9ffede Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 7 Nov 2026 09:16:34 +0000 Subject: [PATCH 7/7] Read package.json beside a root pnpm lock pair in hosted mode The hosted pnpm planner picks which of a root pnpm-lock.yaml / shrinkwrap.yaml pair to rewrite from the pnpm major the project pins: node_modules/.modules.yaml, else package.json's packageManager. The hosted engine only read package.json beside an npm or yarn lock, so on a fresh clone that pins pnpm 1 or 2 in packageManager (no install record) hosted rewrote the stray pnpm-lock.yaml while pnpm installs from shrinkwrap.yaml. Vendored mode and discovery read the pin from disk and already chose shrinkwrap.yaml. Read the manifest as advisory input whenever both root pnpm locks are present, so all three agree. Addresses Bugbot 4500674838 on #2029. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_011f9aePC6sLFkqRM2XPWq4P --- crates/socket-patch-core/src/hosted/engine.rs | 45 ++++++++++++++++++- 1 file changed, 43 insertions(+), 2 deletions(-) diff --git a/crates/socket-patch-core/src/hosted/engine.rs b/crates/socket-patch-core/src/hosted/engine.rs index 13ae7e4d4..d41b0412f 100644 --- a/crates/socket-patch-core/src/hosted/engine.rs +++ b/crates/socket-patch-core/src/hosted/engine.rs @@ -621,10 +621,17 @@ pub async fn read_candidate_files( // and yarn classic rewriters read it as advisory input only: no // rewriter edits it, so a link or an unreadable in-memory entry is left // out (the rewriters then keep their conservative reading) rather than - // refused. + // refused. Beside a root `pnpm-lock.yaml` / `shrinkwrap.yaml` pair its + // `packageManager` pin decides which of the two pnpm reads + // (`formats::pnpm::plan::planned_locks`), as vendored mode and + // discovery read it, when no `node_modules/.modules.yaml` records the + // installed major (a fresh clone). } else if candidates.iter().any(|c| c.dep.ecosystem == "npm") && (NPM_LOCKS.iter().any(|lock| out.files.contains_key(*lock)) - || out.files.contains_key("yarn.lock")) + || out.files.contains_key("yarn.lock") + || ROOT_PNPM_LOCKS + .iter() + .all(|lock| out.files.contains_key(*lock))) { let rel = crate::hosted::memory::select::NPM_MANIFEST_REL; if let Some(text) = read_advisory(view, unreadable, rel).await { @@ -4332,6 +4339,40 @@ mod tests { ); } + #[tokio::test] + async fn a_pnpm_lock_pair_reads_the_package_manager_pin() { + // pnpm 2 pinned in package.json, no install record: pnpm reads + // shrinkwrap.yaml, so the planner must see the pin to leave the + // stray pnpm-lock.yaml alone. + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + let manifest = r#"{"name":"app","packageManager":"pnpm@2.25.7"}"#; + std::fs::write(root.join("package.json"), manifest).unwrap(); + std::fs::write(root.join("pnpm-lock.yaml"), "lockfileVersion: 5.4\n").unwrap(); + std::fs::write( + root.join("shrinkwrap.yaml"), + "shrinkwrapMinorVersion: 9\nshrinkwrapVersion: 3\n", + ) + .unwrap(); + let candidates = vec![left_pad_candidate()]; + let read = + read_candidate_files(&ProjectView::Disk(root), &BTreeSet::new(), &candidates).await; + assert_eq!( + read.files.get("package.json").map(String::as_str), + Some(manifest) + ); + assert_eq!( + crate::formats::pnpm::plan::planned_locks(&read.files), + vec!["shrinkwrap.yaml"] + ); + + // One pnpm lock alone governs whatever the pin: no manifest read. + std::fs::remove_file(root.join("shrinkwrap.yaml")).unwrap(); + let read = + read_candidate_files(&ProjectView::Disk(root), &BTreeSet::new(), &candidates).await; + assert!(!read.files.contains_key("package.json")); + } + #[tokio::test] async fn issue_490_the_root_manifest_overrides_reach_the_npm_rewriter() { let redirected = |done: &Rewritten| {