From b69623501989872efdb57c8b9b514caf85b2a049 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Wed, 4 Nov 2026 10:49:22 -0600 Subject: [PATCH 1/6] Read every pnpm lock document, the env lockfile as its own tree PnpmLock is now the one pnpm entry walk, over every document of the lock. Each entry is tagged with its document (PnpmDocument::Project or Env), and documents are told apart by content: in a multi-document lock, one whose importers carry only configDependencies / packageManagerDependencies is pnpm 11+'s env lockfile, every other document is a project one. grammar::main_document (the text after the last `---`) is gone, so a crafted lock can no longer pick its own "project" and hide the rest (lock-model divergence N4). Behavior changes (owner decision: the env document counts and is its own install tree): - The inventory (PnpmLock::entries), PnpmLock::resolves, the legacy vendored in-use check (vendored_in_use) and InstalledPackages read the env document too, as the hosted planner and the v9 in-use check already did. env_document_is_not_in_the_inventory pinned #1949's skip; it becomes env_document_is_read_as_its_own_tree. - Discovery emits refs for Socket wiring in the env document. Its records are keyed ` (env lockfile)` while the contests run, so the project document's registry entries, file: copies and bundled copies never contest its refs (and the reverse), its wiring does not exempt the project document from contesting another lock, and it is skipped by the cross-lock contest both ways (config dependencies install apart from every node_modules). finalize maps the key back to the lock path. Pinned by pnpm_env_document_is_its_own_install_tree. - A lock with several non-env documents reads all of them as project documents (before: only the last). Pinned by documents_are_identified_by_content_not_position. InstalledPackages::from_lock_text is rebuilt on PnpmLock (keeping its last-wins name:/version: rule and its None cases). pnpm writes the current lockfile (node_modules/.pnpm/lock.yaml) as the project lock alone and installs config dependencies outside the virtual store, so an env document is not expected there; if one appears its entries keep their store entries, the side that never hides an installed package. The vendored set is computed lazily (OnceLock), so the crawler does not pay for it. lock_text_has_member_importers, pnpm_settings_major and the hosted unwind's tarball evidence read documents by content too (the latter through PnpmLock, project documents only, as before). Co-Authored-By: Claude Opus 5.5 --- .../src/formats/pnpm/grammar.rs | 89 +++++- .../src/formats/pnpm/lines.rs | 3 +- .../socket-patch-core/src/formats/pnpm/mod.rs | 256 ++++++++++++------ .../src/formats/pnpm/workspace.rs | 12 +- .../src/patch/redirect/upstream/npm.rs | 13 +- .../src/vendor/lock_inventory/tests.rs | 3 +- .../socket-patch-core/src/vex/discover/mod.rs | 85 +++++- .../socket-patch-core/src/vex/discover/npm.rs | 161 ++++++++++- 8 files changed, 513 insertions(+), 109 deletions(-) diff --git a/crates/socket-patch-core/src/formats/pnpm/grammar.rs b/crates/socket-patch-core/src/formats/pnpm/grammar.rs index 6030b57bf..10a7a57fc 100644 --- a/crates/socket-patch-core/src/formats/pnpm/grammar.rs +++ b/crates/socket-patch-core/src/formats/pnpm/grammar.rs @@ -9,6 +9,8 @@ use std::ops::Range; 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). @@ -88,24 +90,93 @@ pub(crate) fn is_pnpm_lock_text(text: &str) -> bool { .any(|line| line.starts_with("lockfileVersion:") || line.starts_with("shrinkwrapVersion:")) } -/// The main document of a pnpm lock: the text after its last column-0 -/// `---` marker, else all of it. pnpm 11+ writes the env lockfile -/// (`configDependencies` / `packageManagerDependencies`, whose resolutions -/// never carry `tarball:`) as a first document ahead of the project lock. -pub(crate) fn main_document(content: &str) -> &str { +/// Which document of a pnpm lock a span belongs to ([`documents`]). +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] +pub enum PnpmDocument { + /// The project lock: what `pnpm install` links into `node_modules`. + Project, + /// pnpm 11+'s env lockfile: the `configDependencies` / + /// `packageManagerDependencies` pnpm installs into its config store + /// and its own package-manager install, not into `node_modules`. Its + /// own install tree (docs/design/lock-model-unification.md, Decisions). + Env, +} + +/// The documents of a pnpm lock, in file order: the byte range of each +/// (separator lines excluded, a leading BOM inside the first) and what it +/// is. A text without a column-0 `---` line is one project document. A +/// blank document (the empty span before a leading `---`) is dropped. +/// +/// pnpm >= 11 writes the env lockfile as a first document ahead of the +/// project lock. A document is identified by its CONTENT, not by its +/// position: in a lock of two or more documents, one whose `importers:` +/// carry only [`ENV_DOC_IMPORTER_FIELDS`] (at least one) is +/// [`PnpmDocument::Env`]; every other document is +/// [`PnpmDocument::Project`]. So a crafted lock cannot hide entries from a +/// reader by adding documents: each one is read, as the tree its content +/// says it is. CRLF reads like LF. +pub(crate) fn documents(content: &str) -> Vec<(Range, PnpmDocument)> { + let mut spans = Vec::new(); let mut start = 0; let mut offset = 0; - for line in content.split_inclusive('\n') { - offset += line.len(); + for (i, line) in content.split_inclusive('\n').enumerate() { let text = line.trim_end_matches(['\r', '\n']); + let text = if i == 0 { strip_bom(text) } else { text }; if text .strip_prefix("---") .is_some_and(|rest| rest.is_empty() || rest.starts_with([' ', '\t'])) { - start = offset; + spans.push(start..offset); + start = offset + line.len(); + } + offset += line.len(); + } + spans.push(start..content.len()); + spans.retain(|span| { + strip_bom(&content[span.clone()]).lines().any(|line| { + let line = line.trim(); + !line.is_empty() && !line.starts_with('#') + }) + }); + if spans.is_empty() { + return vec![(0..content.len(), PnpmDocument::Project)]; + } + let several = spans.len() > 1; + spans + .into_iter() + .map(|span| { + let kind = if several && is_env_document(&content[span.clone()]) { + PnpmDocument::Env + } else { + PnpmDocument::Project + }; + (span, kind) + }) + .collect() +} + +/// Whether one document's `importers:` carry only the env lockfile's +/// fields ([`ENV_DOC_IMPORTER_FIELDS`]), at least one of them. +fn is_env_document(doc: &str) -> bool { + let mut in_importers = false; + let mut env_fields = 0usize; + for line in strip_bom(doc).split('\n') { + let line = line.strip_suffix('\r').unwrap_or(line); + if !line.is_empty() && !line.starts_with([' ', '#']) { + in_importers = line == "importers:"; + continue; + } + if !in_importers { + continue; + } + if let Some((field, _, _)) = super::lines::parse_key_line(line, 4) { + if !ENV_DOC_IMPORTER_FIELDS.contains(&field) { + return false; + } + env_fields += 1; } } - &content[start..] + env_fields > 0 } /// The value of the entry-level (four-space) `field:` line of a packages diff --git a/crates/socket-patch-core/src/formats/pnpm/lines.rs b/crates/socket-patch-core/src/formats/pnpm/lines.rs index 0badfcb72..3899f56ea 100644 --- a/crates/socket-patch-core/src/formats/pnpm/lines.rs +++ b/crates/socket-patch-core/src/formats/pnpm/lines.rs @@ -26,7 +26,8 @@ pub(crate) fn section_bounds(lines: &[String], name: &str) -> Option<(usize, usi /// The importer dependency fields pnpm writes into the env document of a /// two-document lock. -const ENV_DOC_IMPORTER_FIELDS: [&str; 2] = ["configDependencies", "packageManagerDependencies"]; +pub(crate) const ENV_DOC_IMPORTER_FIELDS: [&str; 2] = + ["configDependencies", "packageManagerDependencies"]; /// Split a lock into `(prefix, project document)`, `prefix + project == /// text`. pnpm >= 11 writes `pnpm-lock.yaml` as two YAML documents when the diff --git a/crates/socket-patch-core/src/formats/pnpm/mod.rs b/crates/socket-patch-core/src/formats/pnpm/mod.rs index 1b06aba9e..feb05405b 100644 --- a/crates/socket-patch-core/src/formats/pnpm/mod.rs +++ b/crates/socket-patch-core/src/formats/pnpm/mod.rs @@ -33,12 +33,14 @@ pub(crate) mod workspace; pub use version::{matches_file_id_build_keys, root_only_workspace_breaks_add}; +pub use grammar::PnpmDocument; pub(crate) use grammar::{ entry_bundled, entry_field, is_pnpm_lock_text, Bundled, Entry, Resolution, }; pub(crate) use hosted::plan_hosted; use std::collections::HashSet; +use std::sync::OnceLock; use crate::constants::npm_family::PNPM_LOCK; use crate::formats::text::strip_bom; @@ -58,6 +60,9 @@ pub(crate) struct PnpmPackage<'a> { /// The entry's `resolution:` map; `None` when it has none or the /// grammar refuses it (duplicate keys, nested values, aliases). pub(crate) resolution: Option>, + /// The lock document the entry is in ([`grammar::documents`]): the + /// project lock or pnpm 11+'s env lockfile, each its own install tree. + pub(crate) document: PnpmDocument, } impl<'a> PnpmPackage<'a> { @@ -78,16 +83,22 @@ impl<'a> PnpmPackage<'a> { } } -/// Every `packages:` entry of a pnpm lock text, in lock order — the ONE -/// entry walk the inventory, lockfile discovery and repair share. Every -/// entry is returned (registry, rekeyed vendored, hosted, directory, git); -/// each consumer applies its own key and resolution rules. -pub(crate) fn pnpm_packages(text: &str) -> Vec> { +/// Every `packages:` entry of a pnpm lock text, in lock order, each tagged +/// with its document — the ONE entry walk every reader shares (through +/// [`PnpmLock::packages`]). Every entry is returned (registry, rekeyed +/// vendored, hosted, directory, git); each consumer applies its own key +/// and resolution rules. +fn pnpm_packages(text: &str) -> Vec> { + let documents = grammar::documents(text); grammar::entries(text) .into_iter() .map(|entry| PnpmPackage { key: grammar::unquote(entry.key.trim()), resolution: grammar::resolution(&entry), + document: documents + .iter() + .find(|(span, _)| span.contains(&entry.offset)) + .map_or(PnpmDocument::Project, |(_, kind)| *kind), entry, }) .collect() @@ -368,16 +379,21 @@ pub(crate) fn check_v9_lock_version(text: &str) -> Result<(), String> { // ── the model ── /// One pnpm lock, parsed once (see the module docs). +/// +/// Every document is read. pnpm 11+ writes its env lockfile +/// (`configDependencies`, `packageManagerDependencies`) as a first +/// document; its entries are installed too (into pnpm's config store, not +/// `node_modules`), so they are part of the lock, tagged +/// [`PnpmDocument::Env`]: a reader that keeps install trees apart +/// (lockfile discovery's contests) reads the env document as a tree of its +/// own ([`PnpmPackage::document`]). pub struct PnpmLock<'t> { text: &'t str, - /// The project document ([`grammar::main_document`]): pnpm 11+ writes - /// its env lockfile (`configDependencies`, `packageManagerDependencies`) - /// as a first document whose packages the install never links (#1949). - doc: &'t str, - /// Entries of the project document only, with offsets into `text`. + /// Every document's entries, with offsets into `text`. packages: Vec>, - /// [`vendored_npm_uuids`] of the project document. - vendored: HashSet, + /// [`vendored_npm_uuids`] of the whole lock, computed on first use (the + /// store crawler never asks). + vendored: OnceLock>, } /// One `packages:` entry whose resolution names a tarball — a candidate @@ -392,18 +408,10 @@ pub(crate) struct PnpmTarballRef<'p> { impl<'t> PnpmLock<'t> { /// Parse a lock text (any content: a non-pnpm text has no entries). pub fn parse(text: &'t str) -> Self { - let doc = grammar::main_document(strip_bom(text)); - // `doc` is a suffix of `text`: parse the whole text so offsets stay - // relative to it, and keep the entries that start in `doc`. - let doc_start = text.len() - doc.len(); PnpmLock { text, - doc, - packages: pnpm_packages(text) - .into_iter() - .filter(|p| p.entry.offset >= doc_start) - .collect(), - vendored: vendored_npm_uuids(doc), + packages: pnpm_packages(text), + vendored: OnceLock::new(), } } @@ -416,23 +424,40 @@ impl<'t> PnpmLock<'t> { is_pnpm_lock_text(self.text) } - /// Every `packages:` entry, in lock order. + /// Every `packages:` entry of every document, in lock order, each + /// tagged with its [`PnpmDocument`]. pub(crate) fn packages(&self) -> &[PnpmPackage<'t>] { &self.packages } + /// The entries of one document kind, in lock order. + pub(crate) fn packages_of( + &self, + document: PnpmDocument, + ) -> impl Iterator> { + self.packages.iter().filter(move |p| p.document == document) + } + + /// The document kinds the lock has entries in, project first. + pub(crate) fn documents(&self) -> Vec { + let mut kinds: Vec = self.packages.iter().map(|p| p.document).collect(); + kinds.sort(); + kinds.dedup(); + kinds + } + fn has_packages_section(&self) -> bool { - self.doc + self.text .lines() .any(|l| l.trim_end_matches('\r') == "packages:") } /// The registry inventory: one entry per plain-registry packages key - /// (every key generation, [`pnpm_registry_key`]), flow and block - /// resolutions, CRLF included. A resolution the grammar refuses leaves - /// the entry listed without a verifier; our own vendored tarballs are - /// not registry dependencies and are dropped. `None` when the lock has - /// no `packages:` section. + /// (every key generation, [`pnpm_registry_key`]) of every document, + /// flow and block resolutions, CRLF included. A resolution the grammar + /// refuses leaves the entry listed without a verifier; our own vendored + /// tarballs are not registry dependencies and are dropped. `None` when + /// the lock has no `packages:` section. pub fn entries(&self) -> Option> { if !self.has_packages_section() { return None; @@ -504,7 +529,9 @@ impl<'t> PnpmLock<'t> { /// package.json even when nothing matches it). CRLF locks read like LF /// ones. pub fn vendored_in_use(&self, uuid: &str) -> bool { - self.vendored.contains(uuid) + self.vendored + .get_or_init(|| vendored_npm_uuids(self.text)) + .contains(uuid) } } @@ -524,54 +551,40 @@ pub(crate) struct InstalledPackages { impl InstalledPackages { /// Read a current lockfile in any lock generation (v5.4 `/name/1.0.0`, /// v6 `/name@1.0.0`, v9 `name@1.0.0` keys; a non-registry entry by its - /// `name:` / `version:` fields, or a v9 key's name). `None` when the - /// text is not a pnpm lock, or holds an entry whose package cannot be - /// named — the caller must then keep every store entry. + /// `name:` / `version:` fields, the last of each winning, or a v9 key's + /// name), through [`PnpmLock`]'s entry walk. `None` when the text is not + /// a pnpm lock, or holds an entry whose package cannot be named — the + /// caller must then keep every store entry. + /// + /// Every document counts. pnpm writes the current lockfile as the + /// project lock alone (the env lockfile lives only in `pnpm-lock.yaml`, + /// and config dependencies install into `node_modules/.pnpm-config`, not + /// the virtual store), so a second document is not expected here; if + /// one appears, its entries keep their store entries too, the side that + /// never hides an installed package. pub(crate) fn from_lock_text(text: &str) -> Option { if !grammar::is_pnpm_lock_text(text) { return None; } - // One line walk (the scan reads this on every run, for every - // package of the install): two-space `packages:` keys and their - // four-space `name:` / `version:` fields, nothing else. + let lock = PnpmLock::parse(text); let mut out = Self::default(); - let mut entries = 0usize; - let mut in_packages = false; - let mut current: Option<(&str, Option<&str>, Option<&str>)> = None; - let mut finish = |entry: Option<(&str, Option<&str>, Option<&str>)>, out: &mut Self| { - let Some((key, field_name, field_version)) = entry else { - return Some(()); - }; - entries += 1; - out.insert_entry(key, field_name, field_version) - }; - for raw in grammar::main_document(strip_bom(text)).split('\n') { - let line = raw.strip_suffix('\r').unwrap_or(raw); - if !line.is_empty() && !line.starts_with([' ', '#']) { - finish(current.take(), &mut out)?; - in_packages = line == "packages:"; - continue; - } - if !in_packages { - continue; - } - if let Some((key, _, _)) = lines::parse_key_line(line, 2) { - finish(current.take(), &mut out)?; - current = Some((key, None, None)); - } else if let (Some(entry), Some((field, _, value))) = - (current.as_mut(), lines::parse_key_line(line, 4)) - { - let value = Some(grammar::unquote(value.trim())).filter(|v| !v.is_empty()); - match field { - "name" => entry.1 = value, - "version" => entry.2 = value, - _ => {} + for package in lock.packages() { + let (mut name, mut version) = (None, None); + for line in package.entry.body.split('\n') { + let line = line.strip_suffix('\r').unwrap_or(line); + if let Some((field, _, value)) = lines::parse_key_line(line, 4) { + let value = Some(grammar::unquote(value.trim())).filter(|v| !v.is_empty()); + match field { + "name" => name = value, + "version" => version = value, + _ => {} + } } } + out.insert_entry(package.key, name, version)?; } - finish(current.take(), &mut out)?; // A lock listing no package at all says nothing about the store. - (entries > 0).then_some(out) + (!lock.packages().is_empty()).then_some(out) } /// Record one `packages:` entry; `None` when its package cannot be @@ -768,12 +781,15 @@ mod tests { assert!(!PnpmLock::parse(importers).resolves("left-pad", "1.3.0")); } - /// #1949: a pnpm 11+ lock's env document (config dependencies the - /// install never links) is not part of the inventory: `entries()`, - /// `resolves()`, the vendored uuids and `InstalledPackages` all read - /// the project document alone, and agree with each other. + /// pnpm 11+'s env document is part of the lock (owner decision, + /// lock-model divergence N4; this reverses #1949's skip): pnpm installs + /// its config dependencies, and the hosted planner and the v9 vendored + /// in-use check already read it. `entries()`, `resolves()`, the + /// vendored uuids and `InstalledPackages` all read both documents and + /// agree with each other; every entry is tagged with its document, so + /// discovery keeps the env document a tree of its own. #[test] - fn env_document_is_not_in_the_inventory() { + fn env_document_is_read_as_its_own_tree() { let env = format!( "---\nlockfileVersion: '9.0'\n\nimporters:\n\n .:\n configDependencies:\n cfg-dep:\n specifier: 2.0.0\n version: 2.0.0\n\npackages:\n\n cfg-dep@2.0.0:\n resolution: {{integrity: sha512-c}}\n\n b@file:.socket/vendor/npm/{UUID}/b-1.0.0.tgz:\n resolution: {{integrity: sha512-x, tarball: file:.socket/vendor/npm/{UUID}/b-1.0.0.tgz}}\n version: 1.0.0\n\n" ); @@ -792,17 +808,44 @@ mod tests { .collect(); assert_eq!( names, - [("left-pad".to_string(), "1.3.0".to_string())], + [ + ("cfg-dep".to_string(), "2.0.0".to_string()), + ("left-pad".to_string(), "1.3.0".to_string()) + ], "{text:?}" ); assert!(lock.resolves("left-pad", "1.3.0")); - assert!(!lock.resolves("cfg-dep", "2.0.0")); - assert!(!lock.vendored_in_use(UUID)); - assert!(lock.packages().iter().all(|p| p.entry.offset >= env.len())); + assert!(lock.resolves("cfg-dep", "2.0.0")); + assert!(lock.vendored_in_use(UUID)); + let tagged: Vec<_> = lock + .packages() + .iter() + .map(|p| (p.key, p.document)) + .collect(); + assert_eq!( + tagged, + [ + ("cfg-dep@2.0.0", PnpmDocument::Env), + ( + format!("b@file:.socket/vendor/npm/{UUID}/b-1.0.0.tgz").as_str(), + PnpmDocument::Env + ), + ("left-pad@1.3.0", PnpmDocument::Project), + ], + "{text:?}" + ); + assert_eq!(lock.documents(), [PnpmDocument::Project, PnpmDocument::Env]); let installed = InstalledPackages::from_lock_text(&text).unwrap(); assert!(installed.contains("left-pad", "1.3.0")); - assert!(!installed.contains("cfg-dep", "2.0.0")); + assert!(installed.contains("cfg-dep", "2.0.0")); + assert!(installed.contains("b", "1.0.0")); } + // One document: the project, whatever its importers say. + let lone = env.trim_start_matches("---\n"); + assert!(PnpmLock::parse(lone) + .packages() + .iter() + .all(|p| p.document == PnpmDocument::Project)); // Offsets still index the whole text. let text = format!("{env}{project}"); let lock = PnpmLock::parse(&text); @@ -813,6 +856,61 @@ mod tests { ); } + /// N4: documents are told apart by content, not position. Before, the + /// readers took the text after the LAST `---` as the project, so a + /// lock with an extra document chose its own "project" and hid the + /// rest; now every document is read, as the tree its importers say. + #[test] + fn documents_are_identified_by_content_not_position() { + let env = "lockfileVersion: '9.0'\n\nimporters:\n\n .:\n configDependencies:\n cfg-dep:\n specifier: 2.0.0\n version: 2.0.0\n\npackages:\n\n cfg-dep@2.0.0:\n resolution: {integrity: sha512-c}\n"; + let project = |dep: &str| { + format!( + "lockfileVersion: '9.0'\n\nimporters:\n\n .:\n dependencies:\n {dep}:\n specifier: 1.0.0\n version: 1.0.0\n\npackages:\n\n {dep}@1.0.0:\n resolution: {{integrity: sha512-x}}\n" + ) + }; + let kinds = |text: &str| -> Vec<(String, PnpmDocument)> { + PnpmLock::parse(text) + .packages() + .iter() + .map(|p| (p.key.to_string(), p.document)) + .collect() + }; + let own = |pairs: &[(&str, PnpmDocument)]| -> Vec<(String, PnpmDocument)> { + pairs.iter().map(|(k, d)| (k.to_string(), *d)).collect() + }; + use PnpmDocument::{Env, Project}; + // The env document after the project still reads as the env tree. + let swapped = format!("---\n{}---\n{env}", project("a")); + assert_eq!( + kinds(&swapped), + own(&[("a@1.0.0", Project), ("cfg-dep@2.0.0", Env)]) + ); + // A first document that is not the env document is a project one, + // and its entries are no longer skipped. + let two = format!("---\n{}---\n{}", project("a"), project("b")); + assert_eq!( + kinds(&two), + own(&[("a@1.0.0", Project), ("b@1.0.0", Project)]) + ); + // A third document is read too, CRLF and a BOM included. + let three = format!("---\n{env}---\n{}---\n{}", project("a"), project("b")); + for text in [ + three.clone(), + three.replace('\n', "\r\n"), + format!("\u{feff}{three}"), + ] { + assert_eq!( + kinds(&text), + own(&[ + ("cfg-dep@2.0.0", Env), + ("a@1.0.0", Project), + ("b@1.0.0", Project) + ]), + "{text:?}" + ); + } + } + #[test] fn vendored_in_use_reads_v9_and_legacy_keys_and_crlf() { let v9 = format!( diff --git a/crates/socket-patch-core/src/formats/pnpm/workspace.rs b/crates/socket-patch-core/src/formats/pnpm/workspace.rs index 5c0d12b7f..63b83c9fd 100644 --- a/crates/socket-patch-core/src/formats/pnpm/workspace.rs +++ b/crates/socket-patch-core/src/formats/pnpm/workspace.rs @@ -202,11 +202,17 @@ pub(crate) fn lock_has_member_importers>(lines: &[S]) -> bool { } /// [`lock_has_member_importers`] of a whole `pnpm-lock.yaml` text: its -/// project document alone ([`super::grammar::main_document`]), so importers +/// project documents alone ([`super::grammar::documents`]), so importers /// of a pnpm 11+ env document never count. pub(crate) fn lock_text_has_member_importers(text: &str) -> bool { - let doc = super::grammar::main_document(strip_bom(text)); - lock_has_member_importers(&doc.split('\n').collect::>()) + use super::grammar::{documents, PnpmDocument}; + documents(text) + .into_iter() + .filter(|(_, kind)| *kind == PnpmDocument::Project) + .any(|(span, _)| { + let doc = strip_bom(&text[span]); + lock_has_member_importers(&doc.split('\n').collect::>()) + }) } /// `text` with the contiguous run of lines `block` removed, when that diff --git a/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs b/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs index 1bb703280..ef0031a0e 100644 --- a/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs +++ b/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs @@ -1276,15 +1276,16 @@ fn pnpm_registry_is_own(registry: Option<&str>) -> bool { /// env lockfile document ahead of the project lock (only pnpm >= 11 /// writes one, so its `.npmrc` pnpm settings are ignored). pub(crate) fn pnpm_settings_major(text: &str, pm_major: Option) -> Option { - use crate::formats::pnpm::grammar::{is_pnpm_lock_text, main_document}; + use crate::formats::pnpm::grammar::{documents, PnpmDocument}; use crate::formats::pnpm::lock_version_major; let legacy_lock = lock_version_major(text).is_some_and(|major| major < 9) || text .lines() .any(|line| line.starts_with("shrinkwrapVersion:")); - let main = main_document(text); - let env_document = is_pnpm_lock_text(&text[..text.len() - main.len()]); + let env_document = documents(text) + .iter() + .any(|(_, kind)| *kind == PnpmDocument::Env); pm_major .or(legacy_lock.then_some(8)) .or(env_document.then_some(11)) @@ -1504,17 +1505,17 @@ fn pnpm_include_tarball( /// URL can still appear with it off, under a second registry). With no /// bare one, a tarball pnpm could have derived proves it on. `None`: no /// such resolution (an unconventional URL is recorded either way). Only -/// the main document counts: pnpm 11+'s env lockfile document records its +/// the project documents count: pnpm 11+'s env lockfile document records its /// resolutions bare whatever the setting. fn pnpm_lock_tarball_evidence( text: &str, registry: impl Fn(&str) -> Option, is_hosted: impl Fn(&str) -> bool, ) -> Option { - use crate::formats::pnpm::{classify_pnpm_key, grammar::main_document, pnpm_packages, PnpmKey}; + use crate::formats::pnpm::{classify_pnpm_key, PnpmDocument, PnpmKey, PnpmLock}; let mut derived = false; - for package in pnpm_packages(main_document(text)) { + for package in PnpmLock::parse(text).packages_of(PnpmDocument::Project) { let PnpmKey::Registry { name, version } = classify_pnpm_key(package.key) else { continue; }; diff --git a/crates/socket-patch-core/src/vendor/lock_inventory/tests.rs b/crates/socket-patch-core/src/vendor/lock_inventory/tests.rs index bd14058d0..21651261d 100644 --- a/crates/socket-patch-core/src/vendor/lock_inventory/tests.rs +++ b/crates/socket-patch-core/src/vendor/lock_inventory/tests.rs @@ -2888,7 +2888,8 @@ fn pnpm_resolution_tokens_cover_maps_the_grammar_refuses() { let lock = format!( "lockfileVersion: '9.0'\n\npackages:\n\n x@1.0.0:\n resolution:\n tarball: {url}\n nested:\n a: b\n\n y@1.0.0:\n resolution: {{integrity: sha512-a}}\n resolution: {{tarball: '{url}'}}\n\n z@1.0.0:\n resolution: {{integrity: sha512-z,\n tarball: \"{url}\"}}\n\n ok@1.0.0:\n resolution: {{integrity: sha512-ok}}\n" ); - let packages = crate::formats::pnpm::pnpm_packages(&lock); + let parsed = crate::formats::pnpm::PnpmLock::parse(&lock); + let packages = parsed.packages(); let by_key = |key: &str| packages.iter().find(|p| p.key == key).unwrap(); for key in ["x@1.0.0", "y@1.0.0", "z@1.0.0"] { let package = by_key(key); diff --git a/crates/socket-patch-core/src/vex/discover/mod.rs b/crates/socket-patch-core/src/vex/discover/mod.rs index 5c08758c3..427eceecf 100644 --- a/crates/socket-patch-core/src/vex/discover/mod.rs +++ b/crates/socket-patch-core/src/vex/discover/mod.rs @@ -244,6 +244,36 @@ pub const DIAG_REF_INVALID: &str = "patched_ref_invalid"; /// dependency version, a nuget source with no package mapping). pub const DIAG_REF_UNATTRIBUTABLE: &str = "patched_ref_unattributable"; +/// The label [`pnpm_document_tree`] appends to a pnpm lock's path to key +/// its env lockfile document as an install tree of its own. No file +/// discovery reads ends in it (every lock path ends in `.yaml`). +pub(crate) const PNPM_ENV_TREE_SUFFIX: &str = " (env lockfile)"; + +/// The install-tree key of one document of the pnpm lock at root-relative +/// `file`. pnpm 11+'s env lockfile document (config dependencies, the +/// pinned package manager) installs apart from the project's +/// `node_modules`, so it is its own tree (owner decision, lock-model +/// divergence N4): its records carry ` (env lockfile)` as their +/// file. That keeps its refs and the project document's entries apart in +/// the same-lock and bundled-copy contests ([`Discovery::contest_within_locks`], +/// [`UnwiredCopy`] scopes), keeps its wiring from exempting the project +/// document in [`Discovery::contest_across_locks`], and lets that contest +/// skip it both ways: config dependencies install apart from every +/// `node_modules`, so no other lock decides them and they decide no other +/// lock's ref. Within the env document the same-lock rules apply. The +/// project document keeps `file` itself. [`Discovery::finalize`] maps +/// every key back to `file`, so consumers only ever see real paths; detail +/// texts keep the label. +pub(crate) fn pnpm_document_tree( + file: &str, + document: crate::formats::pnpm::PnpmDocument, +) -> String { + match document { + crate::formats::pnpm::PnpmDocument::Project => file.to_string(), + crate::formats::pnpm::PnpmDocument::Env => format!("{file}{PNPM_ENV_TREE_SUFFIX}"), + } +} + /// One non-fatal discovery finding. The CLI surfaces each as a run warning /// (`code` is the stable routing tag, `file` the root-relative source). #[derive(Debug, Clone, PartialEq, Eq)] @@ -1168,7 +1198,9 @@ impl Discovery { /// 11). This generalizes the npm extractor's shrinkwrap/package-lock /// rule to every pair of locks. PEP 723 script locks (`*.py.lock`) /// neither contest nor are contested: each is scoped to its own script's - /// install. Files of one install tree ([`Discovery::install_trees`]) are + /// install. Neither is a pnpm env lockfile document + /// ([`pnpm_document_tree`]): pnpm installs its config dependencies into + /// its config store whatever else runs. Files of one install tree ([`Discovery::install_trees`]) are /// one lock here: pip resolves a direct reference and a compatible `==` /// pin of the same version in `requirements.txt` and its `-r` includes /// to the reference (#1086). @@ -1193,7 +1225,7 @@ impl Discovery { let mut contested = Vec::new(); let refs = std::mem::take(&mut self.refs); for r in refs { - let other = (!is_script_lock(&r.source_file)) + let other = (!is_script_lock(&r.source_file) && !is_pnpm_env_tree(&r.source_file)) .then(|| { let tree = self.tree_of(&r.source_file); // Only a PyPI ref can match another spelling of its @@ -1212,9 +1244,13 @@ impl Discovery { .find(|e| { let other = self.tree_of(&e.file); // A tree that wires the release itself, in any - // spelling, is no contester. + // spelling, is no contester. A pnpm env + // lockfile installs config dependencies apart + // from every `node_modules`: it contests no + // other tree. e.covers(&r.purl) && other != tree + && !is_pnpm_env_tree(&e.file) && !wiring.get(other).is_some_and(|purls| { purls.contains(&r.purl) || (pypi @@ -1270,6 +1306,41 @@ impl Discovery { } } + /// Map every [`pnpm_document_tree`] key back to its real lock file once + /// the contests that needed the trees apart have run: consumers read, + /// match and display `source_file` / `file` as root-relative paths. + /// Detail texts keep the key's label, which names the document. + fn resolve_document_trees(&mut self) { + fn real(path: &mut PathBuf) { + if let Some(file) = path + .to_str() + .and_then(|p| p.strip_suffix(PNPM_ENV_TREE_SUFFIX)) + { + *path = PathBuf::from(file); + } + } + for r in self + .refs + .iter_mut() + .chain(&mut self.shadowed) + .chain(&mut self.rewirable) + { + real(&mut r.source_file); + } + self.elsewhere.iter_mut().for_each(|e| real(&mut e.file)); + self.unpatched_copies + .iter_mut() + .for_each(|c| real(&mut c.file)); + self.unattested.iter_mut().for_each(|u| real(&mut u.file)); + for c in &mut self.contested { + real(&mut c.file); + real(&mut c.other); + } + self.diagnostics.iter_mut().for_each(|d| real(&mut d.file)); + self.recognized.iter_mut().for_each(|r| real(&mut r.file)); + self.withheld.iter_mut().for_each(|r| real(&mut r.file)); + } + /// The install tree `file` belongs to: its tree's root file, or itself. fn tree_of<'a>(&'a self, file: &'a Path) -> &'a Path { self.install_trees @@ -1425,6 +1496,7 @@ impl Discovery { } fn finalize(&mut self) { + self.resolve_document_trees(); self.elsewhere.sort(); self.elsewhere.dedup(); self.unpatched_copies.sort(); @@ -1605,6 +1677,13 @@ fn is_script_lock(file: &Path) -> bool { crate::utils::python_lock::is_script_lock_name(&file.to_string_lossy()) } +/// Whether `file` is a [`pnpm_document_tree`] key of a pnpm env lockfile +/// document (before [`Discovery::finalize`] maps it back). +fn is_pnpm_env_tree(file: &Path) -> bool { + file.to_str() + .is_some_and(|f| f.ends_with(PNPM_ENV_TREE_SUFFIX)) +} + /// What every extractor receives: the project view plus the hosted-origin /// allowlist, with the guarded-read and identity helpers bolted on. pub(crate) struct DiscoverCtx<'a> { diff --git a/crates/socket-patch-core/src/vex/discover/npm.rs b/crates/socket-patch-core/src/vex/discover/npm.rs index 3193d7f80..567050efa 100644 --- a/crates/socket-patch-core/src/vex/discover/npm.rs +++ b/crates/socket-patch-core/src/vex/discover/npm.rs @@ -53,14 +53,14 @@ use std::path::PathBuf; use serde_json::Value; use super::{ - npm_purl, npm_vendored_tarball_names, parse_json, vendor_ref, CopyTarget, DiscoverCtx, - Discovery, LocateOpts, Located, PatchedRef, UnattestedKind, UnwiredCopy, VendorRef, - DIAG_LOCKFILE_UNPARSEABLE, DIAG_REF_INVALID, DIAG_REF_UNATTRIBUTABLE, + npm_purl, npm_vendored_tarball_names, parse_json, pnpm_document_tree, vendor_ref, CopyTarget, + DiscoverCtx, Discovery, LocateOpts, Located, PatchedRef, UnattestedKind, UnwiredCopy, + VendorRef, 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::{ - classify_pnpm_key, entry_bundled, entry_field, pnpm_registry_key, Bundled, PnpmKey, PnpmLock, - PnpmPackage, + classify_pnpm_key, entry_bundled, entry_field, pnpm_registry_key, Bundled, PnpmDocument, + PnpmKey, PnpmLock, PnpmPackage, }; use crate::formats::text::strip_bom; use crate::utils::digest::is_sri_pin; @@ -642,6 +642,12 @@ fn entry_ref( /// exists, like the rewriter (no precedence between files — rule 10). /// Vendoring refuses Rush (`npm_flavor`), so these only carry hosted refs. /// +/// Every document of a lock is read. pnpm 11+'s env lockfile document +/// (config dependencies, the pinned package manager; identified by its +/// importers, [`PnpmLock::packages`]) is an install tree of its own +/// ([`pnpm_document_tree`]): its refs are emitted, and they never contest +/// or are contested by the project document or another lock. +/// /// ## Entries /// /// Only `packages:` entries are read — the section pnpm resolves and fetches @@ -748,10 +754,29 @@ async fn extract_pnpm_lock( ); return; } + // Each document is its own install tree: pnpm 11+'s env lockfile is + // read under its own tree key ([`pnpm_document_tree`]), so its refs and + // the project document's entries never contest each other. + for document in lock.documents() { + let tree = pnpm_document_tree(file, document); + extract_pnpm_document(ctx, &tree, &lock, document, disabled, out).await; + } +} + +/// The refs and copies of one document of a pnpm lock, recorded under the +/// document's install-tree key `file` (see [`extract_pnpm_lock`]). +async fn extract_pnpm_document( + ctx: &DiscoverCtx<'_>, + file: &str, + lock: &PnpmLock<'_>, + document: PnpmDocument, + disabled: Option<&str>, + out: &mut Discovery, +) { let mut copies: Vec = Vec::new(); let mut bundles: Vec<(&str, Bundled<'_>)> = Vec::new(); let mut withheld: BTreeSet = BTreeSet::new(); - for package in lock.packages() { + for package in lock.packages_of(document) { let withhold = disabled.map(|_| &mut withheld); pnpm_entry_ref(ctx, file, package, &mut copies, withhold, out); bundles.extend(entry_bundled(&package.entry).map(|b| (package.key, b))); @@ -925,7 +950,8 @@ pub(crate) async fn pnpm_lock_file_copies( let ctx = DiscoverCtx::with_origins(root, &[]); let lock = PnpmLock::parse(text); let mut copies: Vec = Vec::new(); - for package in lock.packages() { + // The vendored backend wires the project document alone. + for package in lock.packages_of(PnpmDocument::Project) { // As `pnpm_entry_ref` classifies them: an entry without a tarball // is a directory copy; one with a tarball is a copy unless it is a // Socket wiring (or one discovery rejects as such). @@ -3091,6 +3117,127 @@ mod tests { assert_eq!(out.unattested.len(), 1, "vendored: {:#?}", out.unattested); } + /// pnpm 11+'s env lockfile document is read as an install tree of its + /// own (owner decision, lock-model divergence N4). Before, discovery + /// skipped it (#1949), so a Socket wiring of a config dependency was + /// no ref although the hosted planner writes it. Now it is a ref; the + /// project document's registry entry, `file:` copy and bundled copy of + /// the same release never contest it (and the reverse); its wiring does + /// not exempt the project document from contesting another lock; and + /// it contests no other lock's ref. + #[tokio::test] + async fn pnpm_env_document_is_its_own_install_tree() { + let url = hosted_url("npm", "left-pad", "1.3.0", UUID_A, "left-pad-1.3.0.tgz"); + let wired = + format!(" left-pad@1.3.0:\n resolution: {{integrity: {SRI}, tarball: {url}}}\n\n"); + let registry = " left-pad@1.3.0:\n resolution: {integrity: sha512-UPSTREAM==}\n\n"; + let env = |packages: &str| { + format!( + "---\nlockfileVersion: '9.0'\n\nimporters:\n\n .:\n configDependencies:\n \ + left-pad:\n specifier: 1.3.0\n version: 1.3.0\n\npackages:\n\n{packages}" + ) + }; + let project = |packages: &str| { + format!( + "---\nlockfileVersion: '9.0'\n\nimporters:\n\n .:\n dependencies:\n \ + left-pad:\n specifier: 1.3.0\n version: 1.3.0\n\npackages:\n\n{packages}" + ) + }; + let hosted = [("pkg:npm/left-pad@1.3.0", UUID_A, WiringMode::Hosted)]; + let file_copy = " left-pad@file:forks/left-pad:\n \ + resolution: {directory: forks/left-pad, type: directory}\n\n"; + let bundle = " host-pkg@1.0.0:\n resolution: {integrity: sha512-HOST==}\n \ + bundledDependencies: true\n\n"; + + // Wired in the env document, the project resolving the same release + // from the registry, from a `file:` copy, or bundling it: a ref. + for (case, project_packages) in [ + ("registry", registry.to_string()), + ("file: copy", format!("{registry}{file_copy}")), + ("bundled copy", format!("{registry}{bundle}")), + ] { + for crlf in [false, true] { + let p = Project::new(); + let text = format!("{}{}", env(&wired), project(&project_packages)); + let text = if crlf { + text.replace('\n', "\r\n") + } else { + text + }; + p.write("pnpm-lock.yaml", text); + p.write( + "forks/left-pad/package.json", + r#"{"name":"left-pad","version":"1.3.0"}"#, + ); + let out = p.discover().await; + assert_refs(&out, &hosted); + assert_eq!( + out.refs[0].source_file, + std::path::Path::new("pnpm-lock.yaml") + ); + assert!(out.unattested.is_empty(), "{case}: {:#?}", out.unattested); + assert!(out.shadowed.is_empty(), "{case}: {:#?}", out.shadowed); + assert!(out.contested.is_empty(), "{case}: {:#?}", out.contested); + assert_eq!( + out.hosted_claim("pkg:npm/left-pad@1.3.0", UUID_A), + Some(true), + "{case}" + ); + } + } + + // Wired in the project document, the env document resolving the + // same release from the registry: a ref. + let p = Project::new(); + p.write( + "pnpm-lock.yaml", + format!("{}{}", env(registry), project(&wired)), + ); + let out = p.discover().await; + assert_refs(&out, &hosted); + assert!(out.contested.is_empty(), "{:#?}", out.contested); + + // The env document's registry entry contests no other lock's ref. + let npm_lock = format!( + r#"{{"name":"m","lockfileVersion":3,"packages":{{"":{{"name":"m"}}, + "node_modules/left-pad":{{"version":"1.3.0","resolved":"{url}","integrity":"{SRI}"}}}}}}"# + ); + let p = Project::new(); + p.write("package-lock.json", &npm_lock); + p.write( + "pnpm-lock.yaml", + format!("{}{}", env(registry), project("")), + ); + let out = p.discover().await; + assert_refs(&out, &hosted); + assert!(out.contested.is_empty(), "{:#?}", out.contested); + + // The env document's wiring does not exempt the project document, + // whose registry entry still contests package-lock.json's ref; the + // env document's own ref stands. + let p = Project::new(); + p.write("package-lock.json", &npm_lock); + p.write( + "pnpm-lock.yaml", + format!("{}{}", env(&wired), project(registry)), + ); + let out = p.discover().await; + assert_refs(&out, &hosted); + assert_eq!( + out.refs[0].source_file, + std::path::Path::new("pnpm-lock.yaml") + ); + assert_eq!(out.contested.len(), 1, "{:#?}", out.contested); + let c = &out.contested[0]; + assert_eq!( + (c.file.as_path(), c.other.as_path()), + ( + std::path::Path::new("package-lock.json"), + std::path::Path::new("pnpm-lock.yaml") + ) + ); + } + /// The pnpm bundled mark stays in its own lock: a same-version ref in /// another lock (a `package-lock.json` twin) is neither marked nor /// contested by it — the pnpm lock does not record the bundled version, From 182e3b72343c532a4d67e8b4447c164cb101abe3 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Wed, 4 Nov 2026 10:52:37 -0600 Subject: [PATCH 2/6] Ask the origin classifier whether a pnpm dep is vendored The hosted pnpm planner names a dep it cannot find "vendored" instead of "not found" when the lock carries the vendor backend's wiring for it. It decided that with a substring test over every line: any line starting `@file:.socket/vendor/`, or an `@:` line containing both `file:` and `.socket/vendor/`. It now reads the lock's packages entries through PnpmLock and asks crate::origin::vendor_ref (pure path grammar, no I/O, no host policy, so allowed in formats/): a v9 `@file:` key, or a legacy bare `file:` key whose `name:` line names the dep, with `` this project's vendored npm artifact. That is the rule discovery reads vendored pnpm keys with (lock-model divergence N5, pnpm part). Behavior changes, pinned by pnpm_vendored_diagnosis_reads_the_rekeyed_entry_through_origin (the backends' real v9 and legacy shapes stay "vendored", per the existing tests): - A `file:` path without a canonical uuid dir, or under another ecosystem's vendor dir, is redirect_pnpm_entry_not_found now. - An `overrides:` line alone, with no rekeyed packages entry behind it, is redirect_pnpm_entry_not_found now: pnpm mirrors overrides from package.json whether or not anything matches (the vendored in-use check already treats it that way). The depscan-sized equivalence fixture's vendored entry had no uuid dir, a shape the backend never writes; it now carries one, and its golden is re-blessed (both digests change because the input text changed). Co-Authored-By: Claude Opus 5.5 --- .../src/formats/pnpm/hosted.rs | 61 +++++++++--------- .../src/patch/redirect/mod.rs | 62 +++++++++++++++++++ .../patch/redirect/pnpm_equivalence_tests.rs | 2 +- .../equivalence/pnpm_depscan_sized.golden | 2 +- 4 files changed, 92 insertions(+), 35 deletions(-) diff --git a/crates/socket-patch-core/src/formats/pnpm/hosted.rs b/crates/socket-patch-core/src/formats/pnpm/hosted.rs index c491bd22c..a99f0585f 100644 --- a/crates/socket-patch-core/src/formats/pnpm/hosted.rs +++ b/crates/socket-patch-core/src/formats/pnpm/hosted.rs @@ -338,44 +338,18 @@ pub(crate) fn plan_hosted( // is named as such: `socket-patch vendor` removes the registry // resolution this grammar looks for (v9 respells the packages key // `@file:.socket/vendor/…`; v5/v6 rekey it to a bare `file:` - // key but keep the `@: file:…` overrides line), so - // the generic not-locked wording would send users on a wild-goose - // `pnpm install` when the real path is a mode switch. Fail-closed - // either way: nothing is rewritten for the dep. + // key with a `name:` line), so the generic not-locked wording would + // send users on a wild-goose `pnpm install` when the real path is a + // mode switch. Fail-closed either way: nothing is rewritten for the + // dep. if !matched_any { - let v9_vendored_key = format!("{fname}@file:"); - let override_key = format!("{fname}@{}", dep.version); // Scanned over the post-splice text, so fold pending splices in. for lock in locks.iter_mut() { lock.materialize(); } - let vendored = locks.iter().any(|lock| { - lock.text.lines().any(|line| { - let t = line.trim_start(); - let t = t.strip_prefix('\'').unwrap_or(t); - // v9 packages/snapshots key (leading `/` in v6 spelling). - // The vendor backend always writes the RELATIVE - // `file:.socket/vendor/…` spelling here, so anchoring on - // it keeps a user's own `file:` dep of the same name - // from being misreported as vendored. - let key = t.strip_prefix('/').unwrap_or(t); - if key - .strip_prefix(&v9_vendored_key) - .is_some_and(|rest| rest.starts_with(".socket/vendor/")) - { - return true; - } - // overrides / root-dep line: `@: file:…` - // (pnpm <=8 absolutizes the value, so only the - // `.socket/vendor/` tail is stable enough to match). - t.strip_prefix(&override_key) - .map(|rest| rest.strip_prefix('\'').unwrap_or(rest)) - .and_then(|rest| rest.strip_prefix(':')) - .is_some_and(|rest| { - rest.contains("file:") && rest.contains(".socket/vendor/") - }) - }) - }); + let vendored = locks + .iter() + .any(|lock| vendors_package(&super::PnpmLock::parse(&lock.text), &fname)); if vendored { result.warnings.push(RewriteWarning { code: "redirect_pnpm_entry_vendored".into(), @@ -404,6 +378,27 @@ pub(crate) fn plan_hosted( } } +/// Whether `lock` has a packages entry the vendor backends rekeyed for +/// package `fname`: v9's `@file:` key or legacy's bare +/// `file:` key with a `name:` 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 vendors_package(lock: &super::PnpmLock<'_>, fname: &str) -> bool { + use super::{classify_pnpm_key, PnpmKey}; + lock.packages().iter().any(|package| { + let (name, path) = match classify_pnpm_key(package.key) { + PnpmKey::V9File { name, path } => (Some(name.trim_start_matches('/')), path), + PnpmKey::LegacyFile { path } => (pnpm::entry_field(&package.entry, "name"), path), + PnpmKey::Registry { .. } | PnpmKey::Other => return false, + }; + name == Some(fname) && crate::origin::vendor_ref(path).is_some_and(|v| v.eco == "npm") + }) +} + /// 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/patch/redirect/mod.rs b/crates/socket-patch-core/src/patch/redirect/mod.rs index 706b6eb21..4fbee0155 100644 --- a/crates/socket-patch-core/src/patch/redirect/mod.rs +++ b/crates/socket-patch-core/src/patch/redirect/mod.rs @@ -28267,6 +28267,68 @@ packages: ); } + /// Lock-model divergence N5 (pnpm): "is this dep vendored?" reads the + /// rekeyed packages entry with the origin classifier + /// (`origin::vendor_ref`), the rule discovery reads vendored pnpm keys + /// with, instead of a substring test over every line. A path that is + /// not this project's vendored npm artifact (no canonical uuid dir, + /// another ecosystem's dir) and an `overrides:` line with no rekeyed + /// entry behind it are the generic entry-not-found now; the backends' + /// real v9 and legacy shapes stay vendored (see + /// `pnpm_vendored_entry_is_named_vendored_not_entry_not_found`). + #[test] + fn pnpm_vendored_diagnosis_reads_the_rekeyed_entry_through_origin() { + let ovr = npm_override( + "left-pad", + "1.3.0", + "http://patch.test/left-pad-1.3.0.tgz", + "sha512-PATCHED==", + ); + let uuid = "1a2b3c4d-5e6f-4a1b-8c2d-0123456789ab"; + let v9 = |path: &str| { + format!( + "lockfileVersion: '9.0'\n\npackages:\n\n left-pad@file:{path}:\n \ + resolution: {{integrity: sha512-VENDORED==, tarball: file:{path}}}\n \ + version: 1.3.0\n" + ) + }; + let legacy_overrides_only = format!( + "lockfileVersion: '6.0'\n\noverrides:\n left-pad@1.3.0: \ + file:/abs/project/.socket/vendor/npm/{uuid}/left-pad-1.3.0.tgz\n\npackages:\n\n \ + /other@1.0.0:\n resolution: {{integrity: sha512-OTHER==}}\n dev: false\n" + ); + let codes = |lock: &str| { + let files = BTreeMap::from([("pnpm-lock.yaml".to_string(), lock.to_string())]); + let r = rewrite_registry_redirect(&files, std::slice::from_ref(&ovr)); + assert!(r.files.is_empty() && r.edits.is_empty(), "{lock}"); + r.warnings.into_iter().map(|w| w.code).collect::>() + }; + for (case, lock, want) in [ + ( + "the backend's v9 shape", + v9(&format!(".socket/vendor/npm/{uuid}/left-pad-1.3.0.tgz")), + "redirect_pnpm_entry_vendored", + ), + ( + "no uuid dir", + v9(".socket/vendor/npm/left-pad-1.3.0.tgz"), + "redirect_pnpm_entry_not_found", + ), + ( + "another ecosystem's dir", + v9(&format!(".socket/vendor/cargo/{uuid}/left-pad-1.3.0.tgz")), + "redirect_pnpm_entry_not_found", + ), + ( + "an overrides line alone", + legacy_overrides_only, + "redirect_pnpm_entry_not_found", + ), + ] { + assert_eq!(codes(&lock), vec![want], "{case}"); + } + } + /// 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/crates/socket-patch-core/src/patch/redirect/pnpm_equivalence_tests.rs b/crates/socket-patch-core/src/patch/redirect/pnpm_equivalence_tests.rs index 2d026a253..7ce3cc6ea 100644 --- a/crates/socket-patch-core/src/patch/redirect/pnpm_equivalence_tests.rs +++ b/crates/socket-patch-core/src/patch/redirect/pnpm_equivalence_tests.rs @@ -232,7 +232,7 @@ fn indexed_pnpm_rewrite_matches_golden_on_depscan_sized_lock_set() { }; let root_extra = vec![ format!( - " vendored-pkg@file:.socket/vendor/npm/vendored-pkg-1.0.0.tgz:\n resolution: {{tarball: file:.socket/vendor/npm/vendored-pkg-1.0.0.tgz}}\n" + " vendored-pkg@file:.socket/vendor/npm/0a1b2c3d-4e5f-4a6b-8c7d-8e9f0a1b2c3d/vendored-pkg-1.0.0.tgz:\n resolution: {{tarball: file:.socket/vendor/npm/0a1b2c3d-4e5f-4a6b-8c7d-8e9f0a1b2c3d/vendored-pkg-1.0.0.tgz}}\n" ), // A second copy of an entry that already points at the hosted // artifact (a re-run): rebuilt == original, no edit. diff --git a/crates/socket-patch-core/tests/equivalence/pnpm_depscan_sized.golden b/crates/socket-patch-core/tests/equivalence/pnpm_depscan_sized.golden index 8b8c62706..29f1efde0 100644 --- a/crates/socket-patch-core/tests/equivalence/pnpm_depscan_sized.golden +++ b/crates/socket-patch-core/tests/equivalence/pnpm_depscan_sized.golden @@ -1,3 +1,3 @@ # A depscan-sized seeded pnpm-lock.yaml set + shuffled overrides. # -0 782731128c8cd746 f3e10497dfc8fec6 +0 342d79c926a00044 2cf6f312234e59ca From 38d5a3cf9ff5392704bbd1e36a1a2346022bb74b Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Wed, 4 Nov 2026 10:55:42 -0600 Subject: [PATCH 3/6] Read Rush locks always and follow lockfile=false in the inventory Two npm-family inventory rules now follow lockfile discovery (and so what hosted mode pins): - Rush's common and subspace pnpm locks are separate install trees, read whenever rush.json is present, beside whatever lock the flavor router chose. Before, the inventory read them only as a fallback when the root had no lock (lock-model divergence N12). The listing itself is unchanged (#2003 aligns it separately). root_pnpm_lock_wins_over_ rush_fallback pinned the old shadowing; it becomes rush_locks_are_read_beside_the_root_lock. - pnpm's `lockfile` setting off (lockfile_disabled, not under Rush, as in discovery): pnpm neither reads nor writes the root or member locks. Discovery keeps a hosted ref from such a lock but withholds its lock pin; the inventory now keeps the entry (its coordinates and hosted url, which ledger liveness reads) and drops its integrity, so vendor's fetch and repair never verify against a pin the install does not use. Registry entries keep their integrity: a registry release's hash verifies its content whether or not pnpm reads the lock (N14). Pinned by pnpm_lockfile_off_withholds_hosted_pins. "Hosted" here is the default Socket host (origin::hosted_patch_uuid with no configured origins): the inventory has no per-run origin list yet; the install-tree PR moves it onto the single classifier. Co-Authored-By: Claude Opus 5.5 --- .../src/vendor/lock_inventory/npm_family.rs | 73 +++++++++++++-- .../src/vendor/lock_inventory/tests.rs | 92 ++++++++++++++++++- 2 files changed, 151 insertions(+), 14 deletions(-) diff --git a/crates/socket-patch-core/src/vendor/lock_inventory/npm_family.rs b/crates/socket-patch-core/src/vendor/lock_inventory/npm_family.rs index 208934cf1..1dabe294f 100644 --- a/crates/socket-patch-core/src/vendor/lock_inventory/npm_family.rs +++ b/crates/socket-patch-core/src/vendor/lock_inventory/npm_family.rs @@ -8,6 +8,8 @@ use std::path::Path; use crate::constants::npm_family::{BUN_LOCK, PNPM_SHRINKWRAP_LEGACY}; use crate::formats::governing_locks::{npm_governing_family, npm_lock_files, NpmLockFamily}; use crate::formats::yarn::{grammar as yarn_grammar, YarnLockGrammar}; +use crate::origin::hosted_patch_uuid; +use crate::utils::pnpm_workspace::lockfile_disabled; use crate::utils::purl::npm_purl; use crate::vendor::npm_flavor::NpmLockFlavor; @@ -20,7 +22,7 @@ use super::pnpm::{ use super::view::ProjectView; use super::vlt::inventory_vlt_in; use super::yarn::{inventory_yarn_berry_in, inventory_yarn_classic_in}; -use super::{dedup_prefer_integrity, LockfileEntry, UnsupportedNpmLayout}; +use super::{dedup_prefer_integrity, LockIntegrity, LockfileEntry, UnsupportedNpmLayout}; use crate::vendor::npm_flavor::detect_npm_lock_flavor_in; // ── registry view ── @@ -41,8 +43,8 @@ use crate::vendor::npm_flavor::detect_npm_lock_flavor_in; /// failing that, the workspace members' own locks. A pnpm workspace with /// `sharedWorkspaceLockfile: false` reads those beside its root lock /// ([`inventory_pnpm_member_locks_in`], #492). -/// Any remaining probe failure falls back to Rush's common lock when -/// `rush.json` is present. +/// Rush's common and subspace locks are read beside whatever this picks +/// whenever `rush.json` is present ([`inventory_npm_lock_raw_in`]). #[cfg(test)] pub(crate) async fn inventory_npm_lock( project_root: &Path, @@ -61,8 +63,64 @@ pub(super) async fn inventory_npm_lock_in( /// [`inventory_npm_lock`] before its collapse: every /// guarded instance ([`super::inventory_project_every_lock`]). +/// +/// Two rules follow lockfile discovery (`vex::discover::npm`'s +/// `extract_pnpm`), which reads what hosted mode pins: +/// +/// * Rush's common and subspace locks are separate install trees, read +/// whenever `rush.json` is present, beside whatever the router chose +/// (before: only when the root had no lock; lock-model divergence N12). +/// * pnpm's `lockfile` setting off ([`lockfile_disabled`], not under Rush, +/// which runs pnpm with settings of its own): pnpm neither reads nor +/// writes the root or member locks, so a Socket-hosted pin in them is no +/// lockfile basis. Discovery keeps such a ref but withholds its lock pin; +/// here the entry stays listed (what it names is still the dependency +/// set, and its hosted url still shows liveness the wiring) with its +/// integrity dropped, so nothing verifies or fetches against the pin +/// (N14). Registry entries keep theirs: a registry release's integrity +/// is its content verifier whether or not pnpm reads the lock. pub(super) async fn inventory_npm_lock_raw_in( view: &ProjectView<'_>, +) -> Result)>, UnsupportedNpmLayout> { + let mut found = inventory_npm_router_lock_raw_in(view).await?; + let rush = view.exists("rush.json").await; + if let Some((NpmLockFlavor::Pnpm | NpmLockFlavor::PnpmLegacy, entries)) = &mut found { + if !rush && lockfile_disabled(view, false).await.is_some() { + withhold_hosted_pins(entries); + } + } + let rush = guard_npm(inventory_rush_pnpm_locks_in(view).await); + if rush.is_empty() { + return Ok(found); + } + Ok(Some(match found { + Some((flavor, mut entries)) => { + entries.extend(rush); + (flavor, entries) + } + None => (NpmLockFlavor::Pnpm, rush), + })) +} + +/// Drop the integrity of every entry resolving from Socket's patch server +/// (see [`inventory_npm_lock_raw_in`]). +fn withhold_hosted_pins(entries: &mut [LockfileEntry]) { + for entry in entries { + if entry + .resolved + .as_deref() + .and_then(|url| hosted_patch_uuid(url, &[])) + .is_some() + { + entry.integrity = LockIntegrity::None; + } + } +} + +/// The lock the npm flavor router picks, with its fallbacks (see +/// [`inventory_npm_lock`]); Rush's locks are added by the caller. +async fn inventory_npm_router_lock_raw_in( + view: &ProjectView<'_>, ) -> Result)>, UnsupportedNpmLayout> { let (flavor, _warnings) = match detect_npm_lock_flavor_in(view).await { Ok(found) => found, @@ -144,13 +202,8 @@ pub(super) async fn inventory_npm_lock_raw_in( return Ok(Some((NpmLockFlavor::Pnpm, guard_npm(members)))); } } - // Rush monorepos have no root package.json/lock pair; their - // single pnpm source-of-truth lives under common/config/rush/. - // The flavor probe (root-relative) can't see it, so fall back - // explicitly when the root lock is absent but rush.json is - // present. - let rush = inventory_rush_pnpm_locks_in(view).await; - return Ok((!rush.is_empty()).then(|| (NpmLockFlavor::Pnpm, guard_npm(rush)))); + // Rush's locks are read by the caller, whatever the probe says. + return Ok(None); } }; let raw = match flavor { diff --git a/crates/socket-patch-core/src/vendor/lock_inventory/tests.rs b/crates/socket-patch-core/src/vendor/lock_inventory/tests.rs index 21651261d..6ef23cd40 100644 --- a/crates/socket-patch-core/src/vendor/lock_inventory/tests.rs +++ b/crates/socket-patch-core/src/vendor/lock_inventory/tests.rs @@ -756,10 +756,12 @@ async fn rush_json_without_any_lock_yields_none() { assert!(inventory_npm_lock(tmp.path()).await.unwrap().is_none()); } +/// Lock-model divergence N12: Rush's locks are their own install trees, +/// read whenever `rush.json` is present — beside a root lock too, as +/// hosted mode and lockfile discovery read them. Before, a root lock +/// shadowed them (this test pinned "the root lock wins"). #[tokio::test] -async fn root_pnpm_lock_wins_over_rush_fallback() { - // A plain pnpm project that also happens to carry a stray rush.json - // must route through the normal root-lock path, never the fallback. +async fn rush_locks_are_read_beside_the_root_lock() { let tmp = tempfile::tempdir().unwrap(); write(tmp.path(), "rush.json", r#"{"rushVersion":"5.0.0"}"#).await; write(tmp.path(), "pnpm-lock.yaml", PNPM_LOCK).await; @@ -779,9 +781,91 @@ packages: let (flavor, entries) = inventory_npm_lock(tmp.path()).await.unwrap().unwrap(); assert_eq!(flavor, NpmLockFlavor::Pnpm); assert!(entries.iter().any(|e| e.name == "left-pad")); + assert!( + entries.iter().any(|e| e.name == "only-in-common"), + "the Rush common lock is read beside the root lock: {entries:?}" + ); + + // Beside another family's lock too. + tokio::fs::remove_file(tmp.path().join("pnpm-lock.yaml")) + .await + .unwrap(); + write(tmp.path(), "package-lock.json", PACKAGE_LOCK).await; + let (flavor, entries) = inventory_npm_lock(tmp.path()).await.unwrap().unwrap(); + assert_eq!(flavor, NpmLockFlavor::PackageLock); + assert!( + entries.iter().any(|e| e.name == "only-in-common"), + "{entries:?}" + ); + + // Without rush.json they are not Rush's. + tokio::fs::remove_file(tmp.path().join("rush.json")) + .await + .unwrap(); + let (_, entries) = inventory_npm_lock(tmp.path()).await.unwrap().unwrap(); assert!( !entries.iter().any(|e| e.name == "only-in-common"), - "the root lock must win; the rush fallback must not run: {entries:?}" + "{entries:?}" + ); +} + +/// Lock-model divergence N14: with pnpm's `lockfile` setting off, pnpm +/// never reads the lock, so a Socket-hosted pin in it is no lockfile +/// basis. Lockfile discovery keeps the ref and withholds its pin; the +/// inventory keeps the entry (and its hosted url) and drops its +/// integrity, so nothing fetches or verifies against it. Registry entries +/// keep theirs. Before, the inventory ignored the setting. Rush runs pnpm +/// with settings of its own, so its locks are untouched (as in discovery). +#[tokio::test] +async fn pnpm_lockfile_off_withholds_hosted_pins() { + let uuid = "1a2b3c4d-5e6f-4a1b-8c2d-0123456789ab"; + let url = + format!("https://patch.socket.dev/patch/npm/left-pad/1.3.0/tok/{uuid}/left-pad-1.3.0.tgz"); + let lock = format!( + "lockfileVersion: '9.0'\n\npackages:\n\n left-pad@1.3.0:\n \ + resolution: {{integrity: sha512-PATCHED==, tarball: {url}}}\n\n \ + right-pad@2.0.0:\n resolution: {{integrity: sha512-UPSTREAM==}}\n" + ); + let integrity = |entries: &[LockfileEntry], name: &str| entry(entries, name).integrity.clone(); + for (setting, withheld) in [ + (None, false), + (Some((".npmrc", "lockfile=false\n")), true), + (Some(("pnpm-workspace.yaml", "lockfile: false\n")), true), + (Some((".npmrc", "lockfile=true\n")), false), + ] { + let tmp = tempfile::tempdir().unwrap(); + write(tmp.path(), "pnpm-lock.yaml", &lock).await; + if let Some((file, text)) = setting { + write(tmp.path(), file, text).await; + } + let (_, entries) = inventory_npm_lock(tmp.path()).await.unwrap().unwrap(); + let left = entry(&entries, "left-pad"); + assert_eq!(left.resolved.as_deref(), Some(url.as_str()), "{setting:?}"); + assert_eq!( + integrity(&entries, "left-pad"), + if withheld { + LockIntegrity::None + } else { + LockIntegrity::Sri("sha512-PATCHED==".into()) + }, + "{setting:?}" + ); + assert_eq!( + integrity(&entries, "right-pad"), + LockIntegrity::Sri("sha512-UPSTREAM==".into()), + "{setting:?}" + ); + } + + // Rush: the setting is not Rush's, so its lock keeps the pin. + let tmp = tempfile::tempdir().unwrap(); + write(tmp.path(), "rush.json", r#"{"rushVersion":"5.0.0"}"#).await; + write(tmp.path(), ".npmrc", "lockfile=false\n").await; + write_nested(tmp.path(), "common/config/rush/pnpm-lock.yaml", &lock).await; + let (_, entries) = inventory_npm_lock(tmp.path()).await.unwrap().unwrap(); + assert_eq!( + integrity(&entries, "left-pad"), + LockIntegrity::Sri("sha512-PATCHED==".into()) ); } From f63ee08c9a0f6862c4f1e39fefffd338c26459bb Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Wed, 4 Nov 2026 10:58:11 -0600 Subject: [PATCH 4/6] Guard pnpm entry reads with an architecture test Outside formats/pnpm, pnpm lock entries are read through PnpmLock (one walk, every document tagged). A new test in formats/pnpm/mod.rs fails on any production use of grammar::entries / pnpm_packages or a hand-rolled `packages:` line walk elsewhere, except the writers that splice by byte range or line position, each allowlisted with a label and a use count: - patch/redirect/upstream/npm.rs (2): the hosted unwind's resolution splices (restore_pnpm_locks); - vendor/pnpm_lock.rs (3): the v9 vendored planner's preflight and line splices; - vendor/pnpm_lock_legacy.rs (6): the 5.4 / 6.0 vendored planner's preflight, splices and revert. Co-Authored-By: Claude Opus 5.5 --- .../src/formats/pnpm/grammar.rs | 8 +- .../socket-patch-core/src/formats/pnpm/mod.rs | 121 ++++++++++++++++++ .../socket-patch-core/src/vex/discover/mod.rs | 4 +- 3 files changed, 128 insertions(+), 5 deletions(-) diff --git a/crates/socket-patch-core/src/formats/pnpm/grammar.rs b/crates/socket-patch-core/src/formats/pnpm/grammar.rs index 10a7a57fc..0e6ae2baf 100644 --- a/crates/socket-patch-core/src/formats/pnpm/grammar.rs +++ b/crates/socket-patch-core/src/formats/pnpm/grammar.rs @@ -1,9 +1,11 @@ //! The `packages:` entry grammar: read pnpm's package blocks without //! reserializing unrelated YAML. The old shrinkwrap and lockfile 5.1/5.2 //! formats use block resolutions; newer locks use flow mappings. Peer -//! suffixes are identities, not part of the version. The hosted planner -//! ([`super::hosted`]), the lock inventory and lockfile discovery all read -//! entries through this one walk. +//! suffixes are identities, not part of the version. Every reader reads +//! entries through [`super::PnpmLock`], which wraps this walk; the writers +//! that splice byte ranges (the hosted planner, [`super::hosted`], and the +//! hosted unwind) call it directly (see `architecture_tests` in +//! `formats/pnpm/mod.rs`). use std::ops::Range; diff --git a/crates/socket-patch-core/src/formats/pnpm/mod.rs b/crates/socket-patch-core/src/formats/pnpm/mod.rs index feb05405b..95f21ef42 100644 --- a/crates/socket-patch-core/src/formats/pnpm/mod.rs +++ b/crates/socket-patch-core/src/formats/pnpm/mod.rs @@ -1065,3 +1065,124 @@ mod tests { assert!(!may_need_store_flag("packages: {}\n")); } } + +/// ARCHITECTURE GUARD: outside `formats/pnpm`, pnpm lock entries are READ +/// through [`PnpmLock`] (one walk, every document tagged), never through +/// the grammar's raw entry walk or a hand-rolled `packages:` line walk. +/// Writers that splice by byte range or line position keep their own +/// walk; each such use is allowlisted below with a label and a count, so +/// a new reader in the same file still fails. +#[cfg(test)] +mod architecture_tests { + use std::path::{Path, PathBuf}; + + /// `(root-relative file, uses, why)`. + const WRITERS: &[(&str, usize, &str)] = &[ + ( + "patch/redirect/upstream/npm.rs", + 2, + "hosted unwind (`restore_pnpm_locks`): splices each pin's \ + resolution byte range back to the registry spelling", + ), + ( + "vendor/pnpm_lock.rs", + 3, + "v9 vendored planner: preflight and splices over line blocks \ + (`lock_has_target_package`, `edit_packages`, `LockIndex`)", + ), + ( + "vendor/pnpm_lock_legacy.rs", + 6, + "5.4 / 6.0 vendored planner: preflight, splices and revert over \ + line blocks", + ), + ]; + + /// Whether one production line reads pnpm entries outside the model. + fn raw_walk(line: &str) -> bool { + let code = line.trim_start(); + if code.starts_with("//") { + return false; + } + [ + "grammar::entries(", + "pnpm::entries(", + "pnpm_grammar::entries(", + "pnpm_packages(", + "== \"packages:\"", + ] + .iter() + .any(|needle| code.contains(needle)) + || (code.contains("\"packages\")") + && (code.contains("bounds(") || code.contains("SectionIndex::build("))) + } + + /// `text` up to its test module. + fn production(text: &str) -> &str { + ["#[cfg(test)]\nmod ", "#[cfg(test)]\npub(crate) mod "] + .iter() + .filter_map(|marker| text.find(marker)) + .min() + .map_or(text, |at| &text[..at]) + } + + fn rs_files(dir: &Path, out: &mut Vec) { + for entry in std::fs::read_dir(dir).expect("read src dir") { + let path = entry.expect("dir entry").path(); + if path.is_dir() { + rs_files(&path, out); + } else if path.extension().is_some_and(|e| e == "rs") { + out.push(path); + } + } + } + + #[test] + fn pnpm_entries_are_read_through_the_model() { + let src = Path::new(env!("CARGO_MANIFEST_DIR")).join("src"); + let mut files = Vec::new(); + rs_files(&src, &mut files); + let mut found: Vec<(String, usize)> = Vec::new(); + for path in files { + let rel = path + .strip_prefix(&src) + .unwrap() + .to_string_lossy() + .replace('\\', "/"); + if rel.starts_with("formats/pnpm/") || rel.ends_with("tests.rs") { + continue; + } + let text = std::fs::read_to_string(&path) + .unwrap() + .replace("\r\n", "\n"); + let uses = production(&text).lines().filter(|l| raw_walk(l)).count(); + if uses > 0 { + found.push((rel, uses)); + } + } + found.sort(); + let mut allowed: Vec<(String, usize)> = WRITERS + .iter() + .map(|(rel, uses, _)| (rel.to_string(), *uses)) + .collect(); + allowed.sort(); + assert_eq!( + found, allowed, + "pnpm lock entries read outside formats/pnpm: read them through \ + PnpmLock::packages (a reader), or allowlist a writer that needs \ + byte or line positions in WRITERS with its reason" + ); + } + + #[test] + fn raw_walk_needles() { + assert!(raw_walk(" for entry in pnpm::entries(&text) {")); + assert!(raw_walk(" in_packages = line == \"packages:\";")); + assert!(raw_walk( + " let (s, e) = section_bounds(lines, \"packages\").unwrap();" + )); + assert!(!raw_walk(" // pnpm::entries(&text)")); + assert!(!raw_walk(" lock.get(\"packages\")")); + assert!(!raw_walk(" for p in PnpmLock::parse(t).packages() {")); + } +} diff --git a/crates/socket-patch-core/src/vex/discover/mod.rs b/crates/socket-patch-core/src/vex/discover/mod.rs index 427eceecf..48e9c2a6f 100644 --- a/crates/socket-patch-core/src/vex/discover/mod.rs +++ b/crates/socket-patch-core/src/vex/discover/mod.rs @@ -30,7 +30,7 @@ //! lockfile supplement, the vendored fetch inventory) read each format //! through ONE shared reader that yields EVERY entry, Socket-owned ones //! included: the npm-family entry models in `lock_inventory` -//! (`npm_lock_nodes`, `pnpm::pnpm_packages`, `yarn::classic_entries` / +//! (`npm_lock_nodes`, `PnpmLock::packages`, `yarn::classic_entries` / //! `berry_entries`, `BunLockb::parse_packages`, `vlt::vlt_lock_model`) and, //! for the other formats, the readers the writers own (`cargo_lock` / //! `cargo_config`, `go_mod_edit` / `go_sum_edit`, `formats::gem`, @@ -4621,7 +4621,7 @@ mod tests { /// ARCHITECTURE GUARD (rule 12): the npm-family extractors read their /// locks through the entry models the lock inventory shares - /// (`classic_entries` / `berry_entries`, `pnpm_packages`, + /// (`classic_entries` / `berry_entries`, `PnpmLock::packages`, /// `bun_text_entries`, `BunLockb::parse_packages`), never through the /// grammar primitives those models wrap, nor a second parser of the /// same lock (a JSON read of `bun.lock`) — so discovery and the From 09fa5a9e50da46d98d37e0720d729de8efd12d02 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Wed, 4 Nov 2026 11:02:21 -0600 Subject: [PATCH 5/6] Mark N4, N5 (pnpm), N12 and N14 resolved; rebudget docs/design/lock-model-divergences.md records what PR 2a resolved. Budget: lock-readers 30,976 -> 31,117 (+141), raised by hand: - vex/discover/mod.rs +46: the env document's tree key (pnpm_document_tree), mapping it back to the lock path in finalize, and skipping it in the cross-lock contest; - formats/pnpm/grammar.rs +51: the content-based document split (documents, is_env_document) replacing main_document; - vendor/lock_inventory/npm_family.rs +35: Rush locks beside the routed lock and the lockfile=false pin withholding; - vex/discover/npm.rs +13: one extraction pass per document; - formats/pnpm/workspace.rs +6, lines.rs +1; - formats/pnpm/mod.rs -4 (InstalledPackages on PnpmLock instead of its own line walk), formats/pnpm/hosted.rs -7 (the vendored check). lock-writers 88,314 -> 88,313 (--lower). Co-Authored-By: Claude Opus 5.5 --- .../socket-patch-core/src/patch/redirect/upstream/npm.rs | 4 +--- docs/design/lock-model-divergences.md | 8 ++++---- scripts/loc-budget.json | 4 ++-- 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs b/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs index ef0031a0e..9250b6b6b 100644 --- a/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs +++ b/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs @@ -1283,9 +1283,7 @@ pub(crate) fn pnpm_settings_major(text: &str, pm_major: Option) -> Option (env lockfile)`, which neither contests nor is contested by the project document or another lock; a document is the env one when its importers carry only `configDependencies` / `packageManagerDependencies`. | | +| 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 | | 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 | | 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 | -| 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. | target-specific (vendored) | +| 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 | target-specific | +| 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 | | | N17 | Bun workspaces, yarn classic offline mirror | Refused differently by the two modes | Stays | target-specific | diff --git a/scripts/loc-budget.json b/scripts/loc-budget.json index efdb902f1..c73793400 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": 30976 + "max": 31117 }, "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": 88313 } } } From 613ca3aae6b4b593789831698b185474960a99e7 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Wed, 4 Nov 2026 11:47:02 -0600 Subject: [PATCH 6/6] Address code review of the pnpm reader PR 1. The pnpm architecture guard cut each file at its first `#[cfg(test)] mod`, so a file declaring an out-of-line test module near the top (crawlers/npm_crawler.rs, patch/redirect/mod.rs, vendor/mod.rs) went mostly unchecked. It now drops only the gated items: a `mod x;` line, or an inline item through its closing brace, counted outside comments and string, raw string and char literals (which span lines). production_drops_only_test_gated_items pins it, including late production code of those three files. 2. The env document must be the FIRST document as well as env-shaped, as pnpm and split_project_document read it. An env-shaped later document is a project one, so it cannot skip the cross-lock contest, and pnpm_settings_major no longer reports 11 for it. documents_are_identified_by_position_and_content (was ..._by_content_not_position) and pnpm_include_tarball_ignores_the_env_ lockfile_document pin it. 3. The hosted "vendored?" check compares the version too: another version vendored is redirect_pnpm_entry_not_found. The depscan-sized equivalence fixture's vendored entry gains the `version:` line the backend always writes; golden re-blessed. 4. LockfileEntry gains `pnpm_env`, and dedup_prefer_integrity never lets an env-document instance displace a project one (project first, then a verifier). env_document_instances_never_displace_project_instances. 5. Entries are tagged by their KEY line's offset, so an empty-bodied last key of a document keeps its document. an_entry_with_an_empty_body_keeps_its_document. 6. No pseudo path: discovery records carry an explicit TreePart (File or PnpmEnv) beside the real file (PatchedRef, ResolvedElsewhere, UnpatchedCopy, UnwiredCopy scope); the contests compare it, and the finalize remap is gone. An env ref the same file's project document also wires (same purl, uuid, mode) is dropped in finalize, so the two never both survive, whatever their pins. 7. InstalledPackages walks keys only (PnpmLock::parse_keys, no resolution parse) on the store crawler's hot path. 8. The hosted planner builds each lock's vendored (name, version) set once, from the unspliced text (a splice never touches a key or a name:/version: line), dropped when materialize replaces the text. 9. Rush locks are no flavor's: the routed lock keeps its flavor alone (inventory_npm_lock), and inventory_npm_trees_raw_in unions it with Rush's trees; rush.json is stat'd once through the view (the Rush reader no longer re-checks it), no bool/Vec shadowing. Budget: lock-readers 31,117 -> 31,207 (+90: TreePart and its helpers +30 in vex/discover/mod.rs, PnpmTree +22 in vex/discover/npm.rs, the key-only walk +16 in formats/pnpm/mod.rs, the vendored cache +13 in formats/pnpm/hosted.rs, `pnpm_env: false` in 21 LockfileEntry literals, golden render +7, minus 15 in the inventory's npm_family/pnpm/mod). lock-writers 88,313 -> 88,314 (+1: hosted/engine.rs compares the part). Co-Authored-By: Claude Opus 5.5 --- .../src/commands/scan/discovery.rs | 1 + .../src/formats/cargo/mod.rs | 1 + .../src/formats/composer/mod.rs | 1 + .../socket-patch-core/src/formats/gem/mod.rs | 1 + .../src/formats/pnpm/grammar.rs | 23 +- .../src/formats/pnpm/hosted.rs | 66 ++-- .../socket-patch-core/src/formats/pnpm/mod.rs | 284 ++++++++++++++++-- crates/socket-patch-core/src/hosted/engine.rs | 1 + .../src/patch/redirect/mod.rs | 8 + .../patch/redirect/pnpm_equivalence_tests.rs | 2 +- .../src/patch/redirect/upstream/npm.rs | 7 + .../src/vendor/lock_inventory/golang.rs | 1 + .../src/vendor/lock_inventory/mod.rs | 36 ++- .../src/vendor/lock_inventory/npm_family.rs | 97 +++--- .../src/vendor/lock_inventory/pnpm.rs | 17 +- .../src/vendor/lock_inventory/pypi.rs | 6 + .../src/vendor/lock_inventory/recover.rs | 6 + .../src/vendor/lock_inventory/tests.rs | 74 ++++- .../socket-patch-core/src/vex/discover/mod.rs | 204 +++++++------ .../socket-patch-core/src/vex/discover/npm.rs | 138 ++++++--- .../src/vex/discover/testing/golden.rs | 16 +- .../equivalence/pnpm_depscan_sized.golden | 2 +- docs/design/lock-model-divergences.md | 2 +- scripts/loc-budget.json | 4 +- 24 files changed, 717 insertions(+), 281 deletions(-) diff --git a/crates/socket-patch-cli/src/commands/scan/discovery.rs b/crates/socket-patch-cli/src/commands/scan/discovery.rs index 0562e26ee..fd27bc50d 100644 --- a/crates/socket-patch-cli/src/commands/scan/discovery.rs +++ b/crates/socket-patch-cli/src/commands/scan/discovery.rs @@ -722,6 +722,7 @@ mod tests { resolved: None, integrity: LockIntegrity::None, source_kind: SourceKind::Unspecified, + pnpm_env: false, } } diff --git a/crates/socket-patch-core/src/formats/cargo/mod.rs b/crates/socket-patch-core/src/formats/cargo/mod.rs index 081b7f230..459533241 100644 --- a/crates/socket-patch-core/src/formats/cargo/mod.rs +++ b/crates/socket-patch-core/src/formats/cargo/mod.rs @@ -493,6 +493,7 @@ impl CargoLock { out.push(LockfileEntry { ecosystem: "cargo", source_kind, + pnpm_env: false, purl, name: pkg.name.clone(), version, diff --git a/crates/socket-patch-core/src/formats/composer/mod.rs b/crates/socket-patch-core/src/formats/composer/mod.rs index 6e3f8f3e9..25ebb6e85 100644 --- a/crates/socket-patch-core/src/formats/composer/mod.rs +++ b/crates/socket-patch-core/src/formats/composer/mod.rs @@ -183,6 +183,7 @@ impl<'a> ComposerLock<'a> { out.push(LockfileEntry { ecosystem: "composer", source_kind: SourceKind::Unspecified, + pnpm_env: false, name, version, purl, diff --git a/crates/socket-patch-core/src/formats/gem/mod.rs b/crates/socket-patch-core/src/formats/gem/mod.rs index e041581ba..09e40090c 100644 --- a/crates/socket-patch-core/src/formats/gem/mod.rs +++ b/crates/socket-patch-core/src/formats/gem/mod.rs @@ -290,6 +290,7 @@ impl<'t> GemfileLock<'t> { out.push(LockfileEntry { ecosystem: "gem", source_kind: SourceKind::Unspecified, + pnpm_env: false, purl, resolved, name: name.to_string(), diff --git a/crates/socket-patch-core/src/formats/pnpm/grammar.rs b/crates/socket-patch-core/src/formats/pnpm/grammar.rs index 0e6ae2baf..3ba09bf18 100644 --- a/crates/socket-patch-core/src/formats/pnpm/grammar.rs +++ b/crates/socket-patch-core/src/formats/pnpm/grammar.rs @@ -109,14 +109,16 @@ pub enum PnpmDocument { /// is. A text without a column-0 `---` line is one project document. A /// blank document (the empty span before a leading `---`) is dropped. /// -/// pnpm >= 11 writes the env lockfile as a first document ahead of the -/// project lock. A document is identified by its CONTENT, not by its -/// position: in a lock of two or more documents, one whose `importers:` -/// carry only [`ENV_DOC_IMPORTER_FIELDS`] (at least one) is -/// [`PnpmDocument::Env`]; every other document is -/// [`PnpmDocument::Project`]. So a crafted lock cannot hide entries from a -/// reader by adding documents: each one is read, as the tree its content -/// says it is. CRLF reads like LF. +/// pnpm >= 11 writes the env lockfile as the FIRST document, ahead of the +/// project lock, and reads it from there (as the vendored planners' +/// `lines::split_project_document` does). So the env document is told by +/// position AND content: in a lock of two or more documents, the first one +/// is [`PnpmDocument::Env`] when its `importers:` carry only +/// [`ENV_DOC_IMPORTER_FIELDS`] (at least one). Every other document is +/// [`PnpmDocument::Project`], an env-shaped later one included: no +/// document can opt out of the project tree's contests, and a crafted lock +/// cannot hide entries by adding documents, since each one is read. CRLF +/// reads like LF. pub(crate) fn documents(content: &str) -> Vec<(Range, PnpmDocument)> { let mut spans = Vec::new(); let mut start = 0; @@ -146,8 +148,9 @@ pub(crate) fn documents(content: &str) -> Vec<(Range, PnpmDocument)> { let several = spans.len() > 1; spans .into_iter() - .map(|span| { - let kind = if several && is_env_document(&content[span.clone()]) { + .enumerate() + .map(|(i, span)| { + let kind = if several && i == 0 && is_env_document(&content[span.clone()]) { PnpmDocument::Env } else { PnpmDocument::Project diff --git a/crates/socket-patch-core/src/formats/pnpm/hosted.rs b/crates/socket-patch-core/src/formats/pnpm/hosted.rs index a99f0585f..84f2524e2 100644 --- a/crates/socket-patch-core/src/formats/pnpm/hosted.rs +++ b/crates/socket-patch-core/src/formats/pnpm/hosted.rs @@ -4,7 +4,7 @@ //! rewriter's pnpm leg). use std::borrow::Cow; -use std::collections::BTreeMap; +use std::collections::{BTreeMap, HashSet}; use serde_json::Value; @@ -44,6 +44,9 @@ struct PnpmLockState<'f> { pending: Vec<(std::ops::Range, String)>, spliced: std::collections::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> { @@ -57,6 +60,7 @@ impl<'f> PnpmLockState<'f> { pending: Vec::new(), spliced: Default::default(), changed: false, + vendored: None, }; state.reindex(); state @@ -80,6 +84,17 @@ 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 { @@ -147,6 +162,7 @@ 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!( @@ -343,13 +359,9 @@ pub(crate) fn plan_hosted( // mode switch. Fail-closed either way: nothing is rewritten for the // dep. if !matched_any { - // Scanned over the post-splice text, so fold pending splices in. - for lock in locks.iter_mut() { - lock.materialize(); - } let vendored = locks - .iter() - .any(|lock| vendors_package(&super::PnpmLock::parse(&lock.text), &fname)); + .iter_mut() + .any(|lock| lock.vendors(&fname, &dep.version)); if vendored { result.warnings.push(RewriteWarning { code: "redirect_pnpm_entry_vendored".into(), @@ -378,25 +390,33 @@ pub(crate) fn plan_hosted( } } -/// Whether `lock` has a packages entry the vendor backends rekeyed for -/// package `fname`: v9's `@file:` key or legacy's bare -/// `file:` key with a `name:` 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:` +/// 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 vendors_package(lock: &super::PnpmLock<'_>, fname: &str) -> bool { +fn vendored_packages(text: &str) -> HashSet<(String, String)> { use super::{classify_pnpm_key, PnpmKey}; - lock.packages().iter().any(|package| { - let (name, path) = match classify_pnpm_key(package.key) { - PnpmKey::V9File { name, path } => (Some(name.trim_start_matches('/')), path), - PnpmKey::LegacyFile { path } => (pnpm::entry_field(&package.entry, "name"), path), - PnpmKey::Registry { .. } | PnpmKey::Other => return false, - }; - name == Some(fname) && crate::origin::vendor_ref(path).is_some_and(|v| v.eco == "npm") - }) + 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 diff --git a/crates/socket-patch-core/src/formats/pnpm/mod.rs b/crates/socket-patch-core/src/formats/pnpm/mod.rs index 95f21ef42..2df9b643f 100644 --- a/crates/socket-patch-core/src/formats/pnpm/mod.rs +++ b/crates/socket-patch-core/src/formats/pnpm/mod.rs @@ -88,29 +88,47 @@ impl<'a> PnpmPackage<'a> { /// [`PnpmLock::packages`]). Every entry is returned (registry, rekeyed /// vendored, hosted, directory, git); each consumer applies its own key /// and resolution rules. -fn pnpm_packages(text: &str) -> Vec> { +fn pnpm_packages(text: &str, resolve: Resolve) -> Vec> { let documents = grammar::documents(text); grammar::entries(text) .into_iter() - .map(|entry| PnpmPackage { - key: grammar::unquote(entry.key.trim()), - resolution: grammar::resolution(&entry), - document: documents - .iter() - .find(|(span, _)| span.contains(&entry.offset)) - .map_or(PnpmDocument::Project, |(_, kind)| *kind), - entry, + .map(|entry| { + // Tag by the KEY line: an entry with an empty body (its key the + // last line of a document) has its body offset on the next + // document's `---` line, or at the end of the text. + let key_at = entry.key.as_ptr() as usize - text.as_ptr() as usize; + PnpmPackage { + key: grammar::unquote(entry.key.trim()), + resolution: match resolve { + Resolve::Yes => grammar::resolution(&entry), + Resolve::No => None, + }, + document: documents + .iter() + .find(|(span, _)| span.contains(&key_at)) + .map_or(PnpmDocument::Project, |(_, kind)| *kind), + entry, + } }) .collect() } +/// Whether [`pnpm_packages`] parses each entry's `resolution:` map. +#[derive(Clone, Copy)] +enum Resolve { + Yes, + /// Keys and bodies only: the store crawler's hot path + /// ([`InstalledPackages::from_lock_text`]) never reads a resolution. + No, +} + /// The `(integrity, tarball)` of one recorded `packages:` block — a /// vendored planner's wiring fragment (its key line and body, as /// recorded), each unquoted; `None` unless the fragment is exactly one /// entry with a flat resolution. pub(crate) fn fragment_resolution(block: &[String]) -> Option<(Option, Option)> { let text = format!("packages:\n{}\n", block.join("\n")); - let packages = pnpm_packages(&text); + let packages = pnpm_packages(&text, Resolve::Yes); let [package] = packages.as_slice() else { return None; }; @@ -410,7 +428,18 @@ impl<'t> PnpmLock<'t> { pub fn parse(text: &'t str) -> Self { PnpmLock { text, - packages: pnpm_packages(text), + packages: pnpm_packages(text, Resolve::Yes), + vendored: OnceLock::new(), + } + } + + /// [`PnpmLock::parse`] without the resolution maps (every + /// [`PnpmPackage::resolution`] is `None`), for a reader of keys and + /// entry fields only. + fn parse_keys(text: &'t str) -> Self { + PnpmLock { + text, + packages: pnpm_packages(text, Resolve::No), vendored: OnceLock::new(), } } @@ -478,12 +507,10 @@ impl<'t> PnpmLock<'t> { if tarball.is_some_and(|t| parse_vendor_path(t).is_some()) { continue; } - out.push(LockfileEntry::npm( - name, - version, - tarball.and_then(http_url), - integrity, - )); + let mut entry = + LockfileEntry::npm(name, version, tarball.and_then(http_url), integrity); + entry.pnpm_env = package.document == PnpmDocument::Env; + out.push(entry); } Some(out) } @@ -566,7 +593,9 @@ impl InstalledPackages { if !grammar::is_pnpm_lock_text(text) { return None; } - let lock = PnpmLock::parse(text); + // Keys and `name:` / `version:` lines only: the scan reads this on + // every run, for every package of the install. + let lock = PnpmLock::parse_keys(text); let mut out = Self::default(); for package in lock.packages() { let (mut name, mut version) = (None, None); @@ -856,12 +885,15 @@ mod tests { ); } - /// N4: documents are told apart by content, not position. Before, the - /// readers took the text after the LAST `---` as the project, so a - /// lock with an extra document chose its own "project" and hid the - /// rest; now every document is read, as the tree its importers say. + /// N4: before, the readers took the text after the LAST `---` as the + /// project, so a lock with an extra document chose its own "project" + /// and hid the rest; now every document is read. The env document is + /// the FIRST one, when its importers say so, as pnpm and the vendored + /// planners' `split_project_document` read it: an env-shaped LATER + /// document is a project one, so it cannot opt out of the project + /// tree's contests (code-review finding 2). #[test] - fn documents_are_identified_by_content_not_position() { + fn documents_are_identified_by_position_and_content() { let env = "lockfileVersion: '9.0'\n\nimporters:\n\n .:\n configDependencies:\n cfg-dep:\n specifier: 2.0.0\n version: 2.0.0\n\npackages:\n\n cfg-dep@2.0.0:\n resolution: {integrity: sha512-c}\n"; let project = |dep: &str| { format!( @@ -879,12 +911,13 @@ mod tests { pairs.iter().map(|(k, d)| (k.to_string(), *d)).collect() }; use PnpmDocument::{Env, Project}; - // The env document after the project still reads as the env tree. + // An env-shaped document after the project is a project one. let swapped = format!("---\n{}---\n{env}", project("a")); assert_eq!( kinds(&swapped), - own(&[("a@1.0.0", Project), ("cfg-dep@2.0.0", Env)]) + own(&[("a@1.0.0", Project), ("cfg-dep@2.0.0", Project)]) ); + assert!(!PnpmLock::parse(&swapped).documents().contains(&Env)); // A first document that is not the env document is a project one, // and its entries are no longer skipped. let two = format!("---\n{}---\n{}", project("a"), project("b")); @@ -911,6 +944,32 @@ mod tests { } } + /// An entry is tagged by its KEY line (code-review finding 5): a key + /// whose body is empty — the last line of the env document, right + /// before `---` — has its body offset on the separator, in no document. + #[test] + fn an_entry_with_an_empty_body_keeps_its_document() { + let env = "---\nlockfileVersion: '9.0'\n\nimporters:\n\n .:\n configDependencies:\n cfg-dep:\n specifier: 2.0.0\n version: 2.0.0\n\npackages:\n\n cfg-dep@2.0.0:\n"; + let project = "---\nlockfileVersion: '9.0'\n\npackages:\n\n a@1.0.0:\n resolution: {integrity: sha512-x}\n\n b@1.0.0:"; + let text = format!("{env}{project}"); + for text in [text.clone(), text.replace('\n', "\r\n")] { + let tagged: Vec<_> = PnpmLock::parse(&text) + .packages() + .iter() + .map(|p| (p.key, p.document)) + .collect(); + assert_eq!( + tagged, + [ + ("cfg-dep@2.0.0", PnpmDocument::Env), + ("a@1.0.0", PnpmDocument::Project), + ("b@1.0.0", PnpmDocument::Project), + ], + "{text:?}" + ); + } + } + #[test] fn vendored_in_use_reads_v9_and_legacy_keys_and_crlf() { let v9 = format!( @@ -1117,13 +1176,134 @@ mod architecture_tests { && (code.contains("bounds(") || code.contains("SectionIndex::build("))) } - /// `text` up to its test module. - fn production(text: &str) -> &str { - ["#[cfg(test)]\nmod ", "#[cfg(test)]\npub(crate) mod "] - .iter() - .filter_map(|marker| text.find(marker)) - .min() - .map_or(text, |at| &text[..at]) + /// The lines of `text` outside `#[cfg(test)]` / `#[cfg(all(test, …))]` + /// items: an out-of-line `mod name;` declaration drops that line only, + /// an inline item (`mod tests { … }`, a helper `fn`) drops through its + /// closing brace, so production code after either is still scanned. + fn production(text: &str) -> Vec<&str> { + let lines: Vec<&str> = text.split('\n').collect(); + let deltas = brace_deltas(text); + let mut out = Vec::new(); + let mut i = 0; + while i < lines.len() { + let attr = lines[i].trim_start(); + if !(attr.starts_with("#[cfg(test)]") || attr.starts_with("#[cfg(all(test")) { + out.push(lines[i]); + i += 1; + continue; + } + // Skip the gated item's other attributes, then the item. + i += 1; + while i < lines.len() && lines[i].trim_start().starts_with("#[") { + i += 1; + } + let mut depth = 0i64; + let mut opened = false; + while i < lines.len() { + let (open, close) = deltas[i]; + let line = lines[i]; + i += 1; + depth += open - close; + opened |= open > 0; + if (!opened && line.trim_end().ends_with(';')) || (opened && depth <= 0) { + break; + } + } + } + out + } + + /// Per line of `text` (split on `\n`), the `{` and `}` in code: outside + /// comments and string, raw string and char literals, which may span + /// lines. + fn brace_deltas(text: &str) -> Vec<(i64, i64)> { + let chars: Vec = text.chars().collect(); + let mut out = vec![(0i64, 0i64)]; + let mut i = 0; + let ident = |c: char| c.is_alphanumeric() || c == '_'; + // Advance past `n` chars, opening a new line entry at each `\n`. + let step = |out: &mut Vec<(i64, i64)>, i: &mut usize, n: usize| { + for _ in 0..n { + if chars.get(*i) == Some(&'\n') { + out.push((0, 0)); + } + *i += 1; + } + }; + while i < chars.len() { + let c = chars[i]; + let next = chars.get(i + 1).copied(); + if c == '/' && next == Some('/') { + while i < chars.len() && chars[i] != '\n' { + i += 1; + } + } else if c == '/' && next == Some('*') { + let mut depth = 0; + while i < chars.len() { + if chars[i] == '/' && chars.get(i + 1) == Some(&'*') { + depth += 1; + step(&mut out, &mut i, 2); + } else if chars[i] == '*' && chars.get(i + 1) == Some(&'/') { + depth -= 1; + step(&mut out, &mut i, 2); + if depth == 0 { + break; + } + } else { + step(&mut out, &mut i, 1); + } + } + } else if c == 'r' + && matches!(next, Some('"' | '#')) + && (i == 0 || !ident(chars[i - 1]) || chars[i - 1] == 'b') + { + let hashes = chars[i + 1..].iter().take_while(|&&h| h == '#').count(); + if chars.get(i + 1 + hashes) != Some(&'"') { + step(&mut out, &mut i, 1); + continue; + } + step(&mut out, &mut i, 2 + hashes); + while i < chars.len() { + let closes = chars[i] == '"' + && chars[i + 1..] + .iter() + .take(hashes) + .filter(|&&h| h == '#') + .count() + == hashes; + if closes { + step(&mut out, &mut i, 1 + hashes); + break; + } + step(&mut out, &mut i, 1); + } + } else if c == '"' { + step(&mut out, &mut i, 1); + while i < chars.len() && chars[i] != '"' { + let n = if chars[i] == '\\' { 2 } else { 1 }; + step(&mut out, &mut i, n); + } + step(&mut out, &mut i, 1); + } else if c == '\'' && next == Some('\\') { + // An escaped char literal (`'\''`, `'\u{7b}'`). + step(&mut out, &mut i, 2); + while i < chars.len() && chars[i] != '\'' { + step(&mut out, &mut i, 1); + } + step(&mut out, &mut i, 1); + } else if c == '\'' && chars.get(i + 2) == Some(&'\'') { + step(&mut out, &mut i, 3); + } else { + let last = out.last_mut().expect("one entry per line"); + match c { + '{' => last.0 += 1, + '}' => last.1 += 1, + _ => {} + } + step(&mut out, &mut i, 1); + } + } + out } fn rs_files(dir: &Path, out: &mut Vec) { @@ -1155,7 +1335,10 @@ mod architecture_tests { let text = std::fs::read_to_string(&path) .unwrap() .replace("\r\n", "\n"); - let uses = production(&text).lines().filter(|l| raw_walk(l)).count(); + let uses = production(&text) + .into_iter() + .filter(|l| raw_walk(l)) + .count(); if uses > 0 { found.push((rel, uses)); } @@ -1185,4 +1368,39 @@ mod architecture_tests { assert!(!raw_walk(" lock.get(\"packages\")")); assert!(!raw_walk(" for p in PnpmLock::parse(t).packages() {")); } + + /// Code-review finding 1: an out-of-line test module declared near the + /// top of a file (`crawlers/npm_crawler.rs`, `patch/redirect/mod.rs`, + /// `vendor/mod.rs` do) used to end the scan there, so a raw walk deep + /// in the file went unseen. Only the gated items are dropped now. + #[test] + fn production_drops_only_test_gated_items() { + let src = "use a;\n#[cfg(test)]\nmod oracle;\n#[cfg(all(test, unix))]\n#[path = \"x.rs\"]\nmod x_tests;\n\ + #[cfg(test)]\nfn helper() {\n let s = \"}\";\n let c = '{';\n if true { pnpm::entries(t); }\n}\n\ + fn reader<'a>(t: &'a str) {\n for e in pnpm::entries(t) {}\n}\n\ + #[cfg(test)]\nmod tests {\n fn t() { pnpm::entries(\"{\"); }\n \ + const S: &str = \"a {\n{ b\";\n const R: &str = r#\"{\"{\n\"#;\n /* { */\n}\n\ + fn after() { let x = line == \"packages:\"; }\n"; + let kept = production(src); + assert_eq!(kept.iter().filter(|l| raw_walk(l)).count(), 2, "{kept:#?}"); + assert!(kept.iter().all(|l| !l.contains("mod oracle"))); + assert!(kept.iter().any(|l| l.contains("fn after()"))); + assert!(kept.iter().all(|l| !l.contains("fn helper"))); + // The real files that declare test modules early keep their late + // production code. + let src = Path::new(env!("CARGO_MANIFEST_DIR")).join("src"); + for (rel, late) in [ + ("crawlers/npm_crawler.rs", "fn is_safe_npm_component("), + ("patch/redirect/mod.rs", "fn rewrite_golang_target("), + ("vendor/mod.rs", "pub async fn vendored_purl_keys("), + ] { + let text = std::fs::read_to_string(src.join(rel)) + .unwrap() + .replace("\r\n", "\n"); + assert!( + production(&text).iter().any(|l| l.starts_with(late)), + "{rel}: production code before its test module was not scanned" + ); + } + } } diff --git a/crates/socket-patch-core/src/hosted/engine.rs b/crates/socket-patch-core/src/hosted/engine.rs index 09901663b..1fb9c4c41 100644 --- a/crates/socket-patch-core/src/hosted/engine.rs +++ b/crates/socket-patch-core/src/hosted/engine.rs @@ -2194,6 +2194,7 @@ async fn unattributed_pins( for c in discovery.unpatched_copies.iter().filter(|c| { c.purl == r.purl && c.file == r.source_file + && c.part == r.part && matches!( c.file.file_name().and_then(|n| n.to_str()), Some("pnpm-lock.yaml" | "shrinkwrap.yaml" | "yarn.lock") diff --git a/crates/socket-patch-core/src/patch/redirect/mod.rs b/crates/socket-patch-core/src/patch/redirect/mod.rs index 4fbee0155..0201fbb31 100644 --- a/crates/socket-patch-core/src/patch/redirect/mod.rs +++ b/crates/socket-patch-core/src/patch/redirect/mod.rs @@ -28324,6 +28324,14 @@ packages: legacy_overrides_only, "redirect_pnpm_entry_not_found", ), + // Code-review finding 3: another version vendored is not this + // one (the dep's version is simply not locked). + ( + "another version vendored", + v9(&format!(".socket/vendor/npm/{uuid}/left-pad-1.2.0.tgz")) + .replace("version: 1.3.0", "version: 1.2.0"), + "redirect_pnpm_entry_not_found", + ), ] { assert_eq!(codes(&lock), vec![want], "{case}"); } diff --git a/crates/socket-patch-core/src/patch/redirect/pnpm_equivalence_tests.rs b/crates/socket-patch-core/src/patch/redirect/pnpm_equivalence_tests.rs index 7ce3cc6ea..8a00efcb7 100644 --- a/crates/socket-patch-core/src/patch/redirect/pnpm_equivalence_tests.rs +++ b/crates/socket-patch-core/src/patch/redirect/pnpm_equivalence_tests.rs @@ -232,7 +232,7 @@ fn indexed_pnpm_rewrite_matches_golden_on_depscan_sized_lock_set() { }; let root_extra = vec![ format!( - " vendored-pkg@file:.socket/vendor/npm/0a1b2c3d-4e5f-4a6b-8c7d-8e9f0a1b2c3d/vendored-pkg-1.0.0.tgz:\n resolution: {{tarball: file:.socket/vendor/npm/0a1b2c3d-4e5f-4a6b-8c7d-8e9f0a1b2c3d/vendored-pkg-1.0.0.tgz}}\n" + " vendored-pkg@file:.socket/vendor/npm/0a1b2c3d-4e5f-4a6b-8c7d-8e9f0a1b2c3d/vendored-pkg-1.0.0.tgz:\n resolution: {{tarball: file:.socket/vendor/npm/0a1b2c3d-4e5f-4a6b-8c7d-8e9f0a1b2c3d/vendored-pkg-1.0.0.tgz}}\n version: 1.0.0\n" ), // A second copy of an entry that already points at the hosted // artifact (a re-run): rebuilt == original, no edit. diff --git a/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs b/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs index 9250b6b6b..7b68418c8 100644 --- a/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs +++ b/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs @@ -3802,6 +3802,13 @@ mod tests { // A lone leading `---` marker is no env document. let marker_only = format!("---\n{}", lock("")); assert!(include(&marker_only, None, Some(RC_ON), None)); + // Neither is an env-shaped document AFTER the project one: pnpm + // writes the env document first (code-review finding 2). + let env_last = format!("---\n{}{env}", lock("")); + assert_eq!(super::pnpm_settings_major(&pinned_only, None), Some(11)); + assert_eq!(super::pnpm_settings_major(&env_last, None), None); + // Its bare entry is project evidence: the setting is off. + assert!(!include(&env_last, None, Some(RC_ON), None)); // A bare entry in the main document still proves it off. let bare = " b@2.0.0:\n resolution: {integrity: sha512-B==}\n"; assert!(!include( diff --git a/crates/socket-patch-core/src/vendor/lock_inventory/golang.rs b/crates/socket-patch-core/src/vendor/lock_inventory/golang.rs index 25636f85d..f95d3a65f 100644 --- a/crates/socket-patch-core/src/vendor/lock_inventory/golang.rs +++ b/crates/socket-patch-core/src/vendor/lock_inventory/golang.rs @@ -43,6 +43,7 @@ pub(super) async fn inventory_go_sum_raw_in(view: &ProjectView<'_>) -> Option = Vec::new(); let mut unsupported: Vec = Vec::new(); - let npm = if every { - npm_family::inventory_npm_lock_raw_in(view).await - } else { - npm_family::inventory_npm_lock_in(view).await - }; - match npm { - Ok(Some((_, entries))) => out.extend(entries), - Ok(None) => {} + match npm_family::inventory_npm_trees_raw_in(view).await { + Ok(entries) if every => out.extend(entries), + Ok(entries) => out.extend(dedup_prefer_integrity(entries)), Err(diag) => unsupported.push(diag), } unsupported.extend(gem::unsupported_gem_layout_in(view).await); @@ -349,20 +350,23 @@ async fn union_views_in( (out, unsupported) } -/// Collapse duplicate (name, version) instances, preferring one that -/// carries a verifier. Each view applies it ONCE, at its public boundary, -/// over its `*_raw` body (collapsing is associative over concatenation — -/// the first verifier-bearing instance wins, else the first — so a union of -/// raw sub-views collapses to what collapsing each sub-view first would). +/// Collapse duplicate (name, version) instances, preferring a project +/// instance over a pnpm env-lockfile one ([`LockfileEntry::pnpm_env`]: a +/// config dependency never stands in for the project's own copy, whose +/// `resolved` url ledger liveness reads), then one that carries a +/// verifier. Each view applies it ONCE, at its public boundary, over its +/// `*_raw` body (collapsing is associative over concatenation — the first +/// most-preferred instance wins — so a union of raw sub-views collapses to +/// what collapsing each sub-view first would). fn dedup_prefer_integrity(raw: Vec) -> Vec { + let rank = |e: &LockfileEntry| (!e.pnpm_env, e.integrity != LockIntegrity::None); let mut seen: HashMap<(String, String), usize> = HashMap::new(); let mut out: Vec = Vec::new(); for entry in raw { let key = (entry.name.clone(), entry.version.clone()); match seen.get(&key) { Some(&i) => { - if out[i].integrity == LockIntegrity::None && entry.integrity != LockIntegrity::None - { + if rank(&entry) > rank(&out[i]) { out[i] = entry; } } diff --git a/crates/socket-patch-core/src/vendor/lock_inventory/npm_family.rs b/crates/socket-patch-core/src/vendor/lock_inventory/npm_family.rs index 1dabe294f..f8bc82c6a 100644 --- a/crates/socket-patch-core/src/vendor/lock_inventory/npm_family.rs +++ b/crates/socket-patch-core/src/vendor/lock_inventory/npm_family.rs @@ -14,6 +14,8 @@ use crate::utils::purl::npm_purl; use crate::vendor::npm_flavor::NpmLockFlavor; use super::bun::{bun_text_lock_drives, inventory_bun_binary_in, inventory_bun_in}; +#[cfg(test)] +use super::dedup_prefer_integrity; use super::npm::inventory_package_lock_in; use super::pnpm::{ inventory_pnpm_lock_in, inventory_pnpm_lock_rel_in, inventory_pnpm_member_locks_in, @@ -22,7 +24,7 @@ use super::pnpm::{ use super::view::ProjectView; use super::vlt::inventory_vlt_in; use super::yarn::{inventory_yarn_berry_in, inventory_yarn_classic_in}; -use super::{dedup_prefer_integrity, LockIntegrity, LockfileEntry, UnsupportedNpmLayout}; +use super::{LockIntegrity, LockfileEntry, UnsupportedNpmLayout}; use crate::vendor::npm_flavor::detect_npm_lock_flavor_in; // ── registry view ── @@ -42,64 +44,73 @@ use crate::vendor::npm_flavor::detect_npm_lock_flavor_in; /// the pnpm <=2-era `shrinkwrap.yaml` (same v5 grammar, older filename) or, /// failing that, the workspace members' own locks. A pnpm workspace with /// `sharedWorkspaceLockfile: false` reads those beside its root lock -/// ([`inventory_pnpm_member_locks_in`], #492). -/// Rush's common and subspace locks are read beside whatever this picks -/// whenever `rush.json` is present ([`inventory_npm_lock_raw_in`]). +/// ([`inventory_pnpm_member_locks_in`], #492). The flavor names that +/// routed lock; Rush's locks, which no flavor routes, are read beside it by +/// [`inventory_npm_trees_raw_in`]. #[cfg(test)] pub(crate) async fn inventory_npm_lock( project_root: &Path, ) -> Result)>, UnsupportedNpmLayout> { - inventory_npm_lock_in(&ProjectView::Disk(project_root)).await + let view = ProjectView::Disk(project_root); + let rush = view.exists("rush.json").await; + inventory_npm_lock_raw_in(&view, rush) + .await + .map(|found| found.map(|(flavor, raw)| (flavor, dedup_prefer_integrity(raw)))) } -/// [`inventory_npm_lock`] over a [`ProjectView`]. -pub(super) async fn inventory_npm_lock_in( - view: &ProjectView<'_>, -) -> Result)>, UnsupportedNpmLayout> { - inventory_npm_lock_raw_in(view) +/// Every npm-family tree the inventory reads, collapsed: what +/// [`super::inventory_project`] lists for npm. +#[cfg(test)] +pub(crate) async fn inventory_npm_trees( + project_root: &Path, +) -> Result, UnsupportedNpmLayout> { + inventory_npm_trees_raw_in(&ProjectView::Disk(project_root)) .await - .map(|found| found.map(|(flavor, raw)| (flavor, dedup_prefer_integrity(raw)))) + .map(dedup_prefer_integrity) } -/// [`inventory_npm_lock`] before its collapse: every -/// guarded instance ([`super::inventory_project_every_lock`]). -/// -/// Two rules follow lockfile discovery (`vex::discover::npm`'s -/// `extract_pnpm`), which reads what hosted mode pins: -/// -/// * Rush's common and subspace locks are separate install trees, read -/// whenever `rush.json` is present, beside whatever the router chose -/// (before: only when the root had no lock; lock-model divergence N12). -/// * pnpm's `lockfile` setting off ([`lockfile_disabled`], not under Rush, -/// which runs pnpm with settings of its own): pnpm neither reads nor -/// writes the root or member locks, so a Socket-hosted pin in them is no -/// lockfile basis. Discovery keeps such a ref but withholds its lock pin; -/// here the entry stays listed (what it names is still the dependency -/// set, and its hosted url still shows liveness the wiring) with its -/// integrity dropped, so nothing verifies or fetches against the pin -/// (N14). Registry entries keep theirs: a registry release's integrity -/// is its content verifier whether or not pnpm reads the lock. -pub(super) async fn inventory_npm_lock_raw_in( +/// Every guarded npm-family instance, uncollapsed: the routed lock +/// ([`inventory_npm_lock`]) and, whenever `rush.json` is present, Rush's +/// common and subspace locks, separate install trees beside it as hosted +/// mode and lockfile discovery read them (before: only when the root had +/// no lock; lock-model divergence N12). +pub(super) async fn inventory_npm_trees_raw_in( view: &ProjectView<'_>, +) -> Result, UnsupportedNpmLayout> { + let rush = view.exists("rush.json").await; + let mut out = inventory_npm_lock_raw_in(view, rush) + .await? + .map(|(_, entries)| entries) + .unwrap_or_default(); + if rush { + out.extend(guard_npm(inventory_rush_pnpm_locks_in(view).await)); + } + Ok(out) +} + +/// The routed lock's guarded instances ([`inventory_npm_lock`] before its +/// collapse). With pnpm's `lockfile` setting off ([`lockfile_disabled`]; +/// not under Rush, `rush` being whether `rush.json` exists, which runs +/// pnpm with settings of its own, as in lockfile discovery's +/// `extract_pnpm`), pnpm neither reads nor writes the root or member +/// locks, so a Socket-hosted pin in them is no lockfile basis. Discovery +/// keeps such a ref but withholds its lock pin; here the entry stays +/// listed (what it names is still the dependency set, and its hosted url +/// still shows ledger liveness the wiring) with its integrity dropped, so +/// nothing verifies or fetches against the pin (N14). Registry entries +/// keep theirs: a registry release's integrity is its content verifier +/// whether or not pnpm reads the lock. +async fn inventory_npm_lock_raw_in( + view: &ProjectView<'_>, + rush: bool, ) -> Result)>, UnsupportedNpmLayout> { let mut found = inventory_npm_router_lock_raw_in(view).await?; - let rush = view.exists("rush.json").await; if let Some((NpmLockFlavor::Pnpm | NpmLockFlavor::PnpmLegacy, entries)) = &mut found { if !rush && lockfile_disabled(view, false).await.is_some() { withhold_hosted_pins(entries); } } - let rush = guard_npm(inventory_rush_pnpm_locks_in(view).await); - if rush.is_empty() { - return Ok(found); - } - Ok(Some(match found { - Some((flavor, mut entries)) => { - entries.extend(rush); - (flavor, entries) - } - None => (NpmLockFlavor::Pnpm, rush), - })) + Ok(found) } /// Drop the integrity of every entry resolving from Socket's patch server @@ -118,7 +129,7 @@ fn withhold_hosted_pins(entries: &mut [LockfileEntry]) { } /// The lock the npm flavor router picks, with its fallbacks (see -/// [`inventory_npm_lock`]); Rush's locks are added by the caller. +/// [`inventory_npm_lock`]). async fn inventory_npm_router_lock_raw_in( view: &ProjectView<'_>, ) -> Result)>, UnsupportedNpmLayout> { diff --git a/crates/socket-patch-core/src/vendor/lock_inventory/pnpm.rs b/crates/socket-patch-core/src/vendor/lock_inventory/pnpm.rs index 9efe7dfd7..ea0c64927 100644 --- a/crates/socket-patch-core/src/vendor/lock_inventory/pnpm.rs +++ b/crates/socket-patch-core/src/vendor/lock_inventory/pnpm.rs @@ -102,11 +102,11 @@ fn pnpm_lock_text_inventory(text: &str) -> Option> { /// `common/config/subspaces//pnpm-lock.yaml`. `rush install` copies /// the source lock into common/temp and runs pnpm there. /// -/// Only called (via [`inventory_npm_lock`]) when there is NO root lock but -/// `rush.json` is present, so it never shadows a plain pnpm project. The +/// Called (via `npm_family::inventory_npm_trees_raw_in`) only when +/// `rush.json` is present, which that caller checks once through the +/// view; the locks are read beside whatever lock the router picks. The /// subspace directory is read sorted for deterministic output. Missing -/// files/dirs are skipped fail-soft; the caller drops the whole result when -/// it comes back empty. +/// files/dirs are skipped fail-soft. pub(super) async fn inventory_rush_pnpm_locks_in(view: &ProjectView<'_>) -> Vec { let project = match view { ProjectView::Disk(_) | ProjectView::Snapshot(_) => { @@ -115,9 +115,6 @@ pub(super) async fn inventory_rush_pnpm_locks_in(view: &ProjectView<'_>) -> Vec< } ProjectView::Memory(project) => *project, }; - if !project.contains("rush.json") { - return Vec::new(); - } let mut out = Vec::new(); if let Some(entries) = inventory_pnpm_lock_rel_in(view, RUSH_COMMON_LOCK_REL).await { out.extend(entries); @@ -135,12 +132,6 @@ pub(super) async fn inventory_rush_pnpm_locks_in(view: &ProjectView<'_>) -> Vec< } async fn inventory_rush_pnpm_locks(project_root: &Path) -> Vec { - if tokio::fs::metadata(project_root.join("rush.json")) - .await - .is_err() - { - return Vec::new(); - } let mut out = Vec::new(); // The single source-of-truth lock. diff --git a/crates/socket-patch-core/src/vendor/lock_inventory/pypi.rs b/crates/socket-patch-core/src/vendor/lock_inventory/pypi.rs index b1822066a..0280a0ad9 100644 --- a/crates/socket-patch-core/src/vendor/lock_inventory/pypi.rs +++ b/crates/socket-patch-core/src/vendor/lock_inventory/pypi.rs @@ -375,6 +375,7 @@ pub(super) fn python_lock_inventory(text: &str) -> Option> { out.push(LockfileEntry { ecosystem: "pypi", source_kind: SourceKind::Unspecified, + pnpm_env: false, purl, name, version: version.to_string(), @@ -416,6 +417,7 @@ async fn inventory_poetry_lock(view: &ProjectView<'_>) -> Option) -> Option) -> Option) -> Option .map(|(name, version, purl, _)| LockfileEntry { ecosystem: "pypi", source_kind: SourceKind::Unspecified, + pnpm_env: false, purl, name, version, @@ -722,6 +727,7 @@ async fn inventory_requirements_txt(view: &ProjectView<'_>) -> Option, integrity: LockIntegrity| LockfileEntry { ecosystem: "npm", source_kind: SourceKind::Unspecified, + pnpm_env: false, purl: format!("pkg:npm/{name}@{version}"), name: name.to_string(), version: version.to_string(), diff --git a/crates/socket-patch-core/src/vendor/lock_inventory/tests.rs b/crates/socket-patch-core/src/vendor/lock_inventory/tests.rs index 6ef23cd40..68f264cac 100644 --- a/crates/socket-patch-core/src/vendor/lock_inventory/tests.rs +++ b/crates/socket-patch-core/src/vendor/lock_inventory/tests.rs @@ -741,8 +741,9 @@ packages: ) .await; - let (flavor, entries) = inventory_npm_lock(tmp.path()).await.unwrap().unwrap(); - assert_eq!(flavor, NpmLockFlavor::Pnpm); + // No lock is routed (Rush's are no flavor's), and both are read. + assert!(inventory_npm_lock(tmp.path()).await.unwrap().is_none()); + let entries = inventory_npm_trees(tmp.path()).await.unwrap(); // Union across the common lock and the subspace lock. assert_eq!(entry(&entries, "left-pad").version, "1.3.0"); assert_eq!(entry(&entries, "only-in-subspace").version, "9.9.9"); @@ -754,6 +755,7 @@ async fn rush_json_without_any_lock_yields_none() { let tmp = tempfile::tempdir().unwrap(); write(tmp.path(), "rush.json", r#"{"rushVersion":"5.0.0"}"#).await; assert!(inventory_npm_lock(tmp.path()).await.unwrap().is_none()); + assert!(inventory_npm_trees(tmp.path()).await.unwrap().is_empty()); } /// Lock-model divergence N12: Rush's locks are their own install trees, @@ -778,8 +780,12 @@ packages: ) .await; - let (flavor, entries) = inventory_npm_lock(tmp.path()).await.unwrap().unwrap(); + // The routed lock keeps its flavor and holds only its own entries + // (code-review finding 9: Rush's are no flavor's). + let (flavor, routed) = inventory_npm_lock(tmp.path()).await.unwrap().unwrap(); assert_eq!(flavor, NpmLockFlavor::Pnpm); + assert!(!routed.iter().any(|e| e.name == "only-in-common")); + let entries = inventory_npm_trees(tmp.path()).await.unwrap(); assert!(entries.iter().any(|e| e.name == "left-pad")); assert!( entries.iter().any(|e| e.name == "only-in-common"), @@ -791,24 +797,77 @@ packages: .await .unwrap(); write(tmp.path(), "package-lock.json", PACKAGE_LOCK).await; - let (flavor, entries) = inventory_npm_lock(tmp.path()).await.unwrap().unwrap(); + let (flavor, routed) = inventory_npm_lock(tmp.path()).await.unwrap().unwrap(); assert_eq!(flavor, NpmLockFlavor::PackageLock); + assert!(!routed.iter().any(|e| e.name == "only-in-common")); + let entries = inventory_npm_trees(tmp.path()).await.unwrap(); assert!( entries.iter().any(|e| e.name == "only-in-common"), "{entries:?}" ); + // The project union lists them too. + assert!(inventory_project(tmp.path()) + .await + .iter() + .any(|e| e.name == "only-in-common")); // Without rush.json they are not Rush's. tokio::fs::remove_file(tmp.path().join("rush.json")) .await .unwrap(); - let (_, entries) = inventory_npm_lock(tmp.path()).await.unwrap().unwrap(); + let entries = inventory_npm_trees(tmp.path()).await.unwrap(); assert!( !entries.iter().any(|e| e.name == "only-in-common"), "{entries:?}" ); } +/// Code-review finding 4: the env document's registry instance of a +/// release comes first in the lock and may carry the only verifier (the +/// project's hosted instance loses its pin under `lockfile=false`), but a +/// config dependency never stands in for the project's own copy: the +/// collapsed view keeps the project's hosted instance, and the +/// every-instance view (ledger liveness) keeps both. +#[tokio::test] +async fn env_document_instances_never_displace_project_instances() { + let uuid = "1a2b3c4d-5e6f-4a1b-8c2d-0123456789ab"; + let url = + format!("https://patch.socket.dev/patch/npm/left-pad/1.3.0/tok/{uuid}/left-pad-1.3.0.tgz"); + let lock = format!( + "---\nlockfileVersion: '9.0'\n\nimporters:\n\n .:\n configDependencies:\n \ + left-pad:\n specifier: 1.3.0\n version: 1.3.0\n\npackages:\n\n \ + left-pad@1.3.0:\n resolution: {{integrity: sha512-UPSTREAM==}}\n\n---\n\ + lockfileVersion: '9.0'\n\npackages:\n\n left-pad@1.3.0:\n \ + resolution: {{integrity: sha512-PATCHED==, tarball: {url}}}\n" + ); + for lockfile_off in [false, true] { + let tmp = tempfile::tempdir().unwrap(); + write(tmp.path(), "pnpm-lock.yaml", &lock).await; + if lockfile_off { + write(tmp.path(), ".npmrc", "lockfile=false\n").await; + } + let (_, entries) = inventory_npm_lock(tmp.path()).await.unwrap().unwrap(); + let left = entry(&entries, "left-pad"); + assert_eq!( + left.resolved.as_deref(), + Some(url.as_str()), + "{lockfile_off}" + ); + assert!(!left.pnpm_env, "{lockfile_off}"); + let every: Vec<_> = inventory_project_every_lock(tmp.path()) + .await + .into_iter() + .filter(|e| e.name == "left-pad") + .map(|e| (e.pnpm_env, e.resolved)) + .collect(); + assert_eq!( + every, + [(true, None), (false, Some(url.clone()))], + "{lockfile_off}" + ); + } +} + /// Lock-model divergence N14: with pnpm's `lockfile` setting off, pnpm /// never reads the lock, so a Socket-hosted pin in it is no lockfile /// basis. Lockfile discovery keeps the ref and withholds its pin; the @@ -862,7 +921,7 @@ async fn pnpm_lockfile_off_withholds_hosted_pins() { write(tmp.path(), "rush.json", r#"{"rushVersion":"5.0.0"}"#).await; write(tmp.path(), ".npmrc", "lockfile=false\n").await; write_nested(tmp.path(), "common/config/rush/pnpm-lock.yaml", &lock).await; - let (_, entries) = inventory_npm_lock(tmp.path()).await.unwrap().unwrap(); + let entries = inventory_npm_trees(tmp.path()).await.unwrap(); assert_eq!( integrity(&entries, "left-pad"), LockIntegrity::Sri("sha512-PATCHED==".into()) @@ -1382,6 +1441,7 @@ async fn lookup_matches_cargo_and_golang_purls() { LockfileEntry { ecosystem: "cargo", source_kind: SourceKind::Unspecified, + pnpm_env: false, name: "serde".into(), version: "1.0.200".into(), purl: "pkg:cargo/serde@1.0.200".into(), @@ -1391,6 +1451,7 @@ async fn lookup_matches_cargo_and_golang_purls() { LockfileEntry { ecosystem: "golang", source_kind: SourceKind::Unspecified, + pnpm_env: false, name: "github.com/x/y".into(), version: "v1.0.0".into(), purl: "pkg:golang/github.com/x/y@v1.0.0".into(), @@ -3777,6 +3838,7 @@ async fn lookup_matches_by_purl_identity() { let entry = |ecosystem: &'static str, name: &str, version: &str| LockfileEntry { ecosystem, source_kind: SourceKind::Unspecified, + pnpm_env: false, name: name.into(), version: version.into(), purl: format!("pkg:{ecosystem}/{name}@{version}"), diff --git a/crates/socket-patch-core/src/vex/discover/mod.rs b/crates/socket-patch-core/src/vex/discover/mod.rs index 48e9c2a6f..f798ca957 100644 --- a/crates/socket-patch-core/src/vex/discover/mod.rs +++ b/crates/socket-patch-core/src/vex/discover/mod.rs @@ -182,7 +182,7 @@ //! `vlt-lock.json`), so discovery and the inventory see one entry walk //! per format (an architecture test in this module enforces it). -use std::collections::{BTreeMap, BTreeSet, HashMap}; +use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet}; use std::path::{Path, PathBuf}; use std::sync::Mutex; @@ -244,33 +244,32 @@ pub const DIAG_REF_INVALID: &str = "patched_ref_invalid"; /// dependency version, a nuget source with no package mapping). pub const DIAG_REF_UNATTRIBUTABLE: &str = "patched_ref_unattributable"; -/// The label [`pnpm_document_tree`] appends to a pnpm lock's path to key -/// its env lockfile document as an install tree of its own. No file -/// discovery reads ends in it (every lock path ends in `.yaml`). -pub(crate) const PNPM_ENV_TREE_SUFFIX: &str = " (env lockfile)"; - -/// The install-tree key of one document of the pnpm lock at root-relative -/// `file`. pnpm 11+'s env lockfile document (config dependencies, the -/// pinned package manager) installs apart from the project's -/// `node_modules`, so it is its own tree (owner decision, lock-model -/// divergence N4): its records carry ` (env lockfile)` as their -/// file. That keeps its refs and the project document's entries apart in -/// the same-lock and bundled-copy contests ([`Discovery::contest_within_locks`], -/// [`UnwiredCopy`] scopes), keeps its wiring from exempting the project -/// document in [`Discovery::contest_across_locks`], and lets that contest -/// skip it both ways: config dependencies install apart from every -/// `node_modules`, so no other lock decides them and they decide no other -/// lock's ref. Within the env document the same-lock rules apply. The -/// project document keeps `file` itself. [`Discovery::finalize`] maps -/// every key back to `file`, so consumers only ever see real paths; detail -/// texts keep the label. -pub(crate) fn pnpm_document_tree( - file: &str, - document: crate::formats::pnpm::PnpmDocument, -) -> String { - match document { - crate::formats::pnpm::PnpmDocument::Project => file.to_string(), - crate::formats::pnpm::PnpmDocument::Env => format!("{file}{PNPM_ENV_TREE_SUFFIX}"), +/// Which install tree of its file a record belongs to: the install-tree +/// key is the file (its [`Discovery::install_trees`] root) plus this part. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, PartialOrd, Ord, Hash)] +pub enum TreePart { + /// The file's own tree (every format but the one below). + #[default] + File, + /// pnpm 11+'s env lockfile document (config dependencies, the pinned + /// package manager), which installs apart from the project's + /// `node_modules`: its own tree (owner decision, lock-model divergence + /// N4). Its refs and the project document's entries of the same lock + /// never contest each other ([`Discovery::contest_within_locks`], + /// [`UnwiredCopy`] scopes), its wiring never exempts the project + /// document in [`Discovery::contest_across_locks`], and that contest + /// skips it both ways: no other lock decides a config dependency, and a + /// config dependency decides no other lock's ref. Within the document + /// the same-lock rules apply. + PnpmEnv, +} + +impl From for TreePart { + fn from(document: crate::formats::pnpm::PnpmDocument) -> Self { + match document { + crate::formats::pnpm::PnpmDocument::Project => TreePart::File, + crate::formats::pnpm::PnpmDocument::Env => TreePart::PnpmEnv, + } } } @@ -328,6 +327,8 @@ pub struct PatchedRef { /// package from, whose dependency sub-maps decide which dependencies /// install beside it (#1538). pub yarn_classic_block: Option, + /// The install tree of `source_file` the wiring is in ([`TreePart`]). + pub part: TreePart, } /// A hosted yarn classic pin's lock block, as lock-only VEX reads it @@ -386,9 +387,16 @@ impl PatchedRef { integrity_required, url: url.map(str::to_string), yarn_classic_block: None, + part: TreePart::File, } } + /// This ref, in install tree `part` of its file. + pub(crate) fn in_part(mut self, part: TreePart) -> Self { + self.part = part; + self + } + /// A vendored ref for the artifact `vref` names. pub fn vendored( purl: String, @@ -406,6 +414,7 @@ impl PatchedRef { integrity_required: false, url: None, yarn_classic_block: None, + part: TreePart::File, } } @@ -507,6 +516,8 @@ pub struct ResolvedElsewhere { /// which pip compares as a string /// ([`Discovery::resolved_elsewhere_as_spelled`]). pub as_spelled: bool, + /// The install tree of `file` the entry is in ([`TreePart`]). + pub part: TreePart, } impl ResolvedElsewhere { @@ -578,6 +589,8 @@ pub struct UnpatchedCopy { /// What makes the build patch that copy, when it is not the usual /// "depend on the registry release instead" (a bundled copy, #758). pub remedy: Option, + /// The install tree of `file` the entry is in ([`TreePart`]). + pub part: TreePart, } /// A lock entry that installs SOME package (at `version`, when known) under @@ -684,8 +697,9 @@ pub enum CopyTarget { /// extractor has run ([`Discovery::unattest_unwired_copies`]). #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)] pub struct UnwiredCopy { - /// Only refs of this root-relative file; `None` = refs of any file. - pub(crate) scope: Option, + /// Only refs of this root-relative file, in this tree of it; `None` = + /// refs of any file. + pub(crate) scope: Option<(PathBuf, TreePart)>, pub(crate) target: CopyTarget, /// Root-relative file that shows the copy. pub(crate) file: PathBuf, @@ -696,7 +710,11 @@ pub struct UnwiredCopy { impl UnwiredCopy { fn covers(&self, r: &PatchedRef) -> bool { - if self.scope.as_ref().is_some_and(|f| *f != r.source_file) { + if self + .scope + .as_ref() + .is_some_and(|(f, part)| *f != r.source_file || *part != r.part) + { return false; } match &self.target { @@ -1004,11 +1022,22 @@ impl Discovery { /// lock's size (#993), and every reader before then only asks whether /// some entry matches. pub(crate) fn resolved_elsewhere(&mut self, file: &str, purl: Option) { + self.resolved_elsewhere_in(file, TreePart::File, purl); + } + + /// [`Discovery::resolved_elsewhere`] in install tree `part` of `file`. + pub(crate) fn resolved_elsewhere_in( + &mut self, + file: &str, + part: TreePart, + purl: Option, + ) { let Some(purl) = purl else { return }; self.elsewhere.push(ResolvedElsewhere { purl: canonical_base_purl(&purl), file: PathBuf::from(file), as_spelled: false, + part, }); } @@ -1021,6 +1050,7 @@ impl Discovery { purl: canonical_base_purl(&purl), file: PathBuf::from(file), as_spelled: true, + part: TreePart::File, }); } @@ -1040,6 +1070,7 @@ impl Discovery { purl, file: PathBuf::from(file), as_spelled: false, + part: TreePart::File, }); } @@ -1057,11 +1088,23 @@ impl Discovery { purl: Option, key: &str, how: &str, + ) { + self.unpatched_copy_in(file, TreePart::File, purl, key, how); + } + + /// [`Discovery::unpatched_copy`] in install tree `part` of `file`. + pub(crate) fn unpatched_copy_in( + &mut self, + file: &str, + part: TreePart, + purl: Option, + key: &str, + how: &str, ) { let Some(purl) = purl else { return; }; - self.resolved_elsewhere(file, Some(purl.clone())); + self.resolved_elsewhere_in(file, part, Some(purl.clone())); // Deduplicated once, in `finalize`: a per-push scan is quadratic // over a lock with thousands of registry blocks. self.unpatched_copies.push(UnpatchedCopy { @@ -1070,6 +1113,7 @@ impl Discovery { key: key.to_string(), how: how.to_string(), remedy: None, + part, }); } @@ -1121,7 +1165,9 @@ impl Discovery { let purls: BTreeSet<&str> = self .refs .iter() - .filter(|r| r.source_file == u.file) + // A file-level copy: no extractor records one in a pnpm + // env lockfile, whose refs are a tree of their own. + .filter(|r| r.source_file == u.file && r.part == TreePart::File) .filter(|r| match &u.version { Some(v) => r.purl.rsplit_once('@').is_some_and(|(_, rv)| rv == v), None => true, @@ -1140,6 +1186,7 @@ impl Discovery { key: u.key.clone(), how: u.how.clone(), remedy: None, + part: TreePart::File, }); } } @@ -1162,7 +1209,7 @@ impl Discovery { let copy = self .unpatched_copies .iter() - .find(|c| c.purl == r.purl && c.file == r.source_file) + .find(|c| c.purl == r.purl && c.file == r.source_file && c.part == r.part) .cloned(); match copy { Some(c) => { @@ -1199,15 +1246,16 @@ impl Discovery { /// rule to every pair of locks. PEP 723 script locks (`*.py.lock`) /// neither contest nor are contested: each is scoped to its own script's /// install. Neither is a pnpm env lockfile document - /// ([`pnpm_document_tree`]): pnpm installs its config dependencies into - /// its config store whatever else runs. Files of one install tree ([`Discovery::install_trees`]) are - /// one lock here: pip resolves a direct reference and a compatible `==` - /// pin of the same version in `requirements.txt` and its `-r` includes - /// to the reference (#1086). + /// ([`TreePart::PnpmEnv`]): pnpm installs its config dependencies into + /// its config store whatever else runs, and its wiring exempts no + /// other tree. Files of one install tree ([`Discovery::install_trees`]) + /// are one lock here: pip resolves a direct reference and a compatible + /// `==` pin of the same version in `requirements.txt` and its `-r` + /// includes to the reference (#1086). fn contest_across_locks(&mut self) { // The purls each install tree wires. let mut wiring: BTreeMap> = BTreeMap::new(); - for r in &self.refs { + for r in self.refs.iter().filter(|r| r.part == TreePart::File) { wiring .entry(self.tree_of(&r.source_file).to_path_buf()) .or_default() @@ -1225,7 +1273,7 @@ impl Discovery { let mut contested = Vec::new(); let refs = std::mem::take(&mut self.refs); for r in refs { - let other = (!is_script_lock(&r.source_file) && !is_pnpm_env_tree(&r.source_file)) + let other = (!is_script_lock(&r.source_file) && r.part == TreePart::File) .then(|| { let tree = self.tree_of(&r.source_file); // Only a PyPI ref can match another spelling of its @@ -1250,7 +1298,7 @@ impl Discovery { // other tree. e.covers(&r.purl) && other != tree - && !is_pnpm_env_tree(&e.file) + && e.part == TreePart::File && !wiring.get(other).is_some_and(|purls| { purls.contains(&r.purl) || (pypi @@ -1306,39 +1354,33 @@ impl Discovery { } } - /// Map every [`pnpm_document_tree`] key back to its real lock file once - /// the contests that needed the trees apart have run: consumers read, - /// match and display `source_file` / `file` as root-relative paths. - /// Detail texts keep the key's label, which names the document. - fn resolve_document_trees(&mut self) { - fn real(path: &mut PathBuf) { - if let Some(file) = path - .to_str() - .and_then(|p| p.strip_suffix(PNPM_ENV_TREE_SUFFIX)) - { - *path = PathBuf::from(file); - } - } - for r in self - .refs - .iter_mut() - .chain(&mut self.shadowed) - .chain(&mut self.rewirable) - { - real(&mut r.source_file); - } - self.elsewhere.iter_mut().for_each(|e| real(&mut e.file)); - self.unpatched_copies - .iter_mut() - .for_each(|c| real(&mut c.file)); - self.unattested.iter_mut().for_each(|u| real(&mut u.file)); - for c in &mut self.contested { - real(&mut c.file); - real(&mut c.other); - } - self.diagnostics.iter_mut().for_each(|d| real(&mut d.file)); - self.recognized.iter_mut().for_each(|r| real(&mut r.file)); - self.withheld.iter_mut().for_each(|r| real(&mut r.file)); + /// Drop a pnpm env-lockfile ref ([`TreePart::PnpmEnv`]) that the same + /// file's project document also wires (the same `(purl, uuid, mode)`): + /// one wiring of the package in that file, whose project instance is + /// the one an installed `node_modules` copy is verified against, even + /// when the two pins differ. + fn drop_env_twins(refs: &mut Vec) { + let project: HashSet<(PathBuf, String, String, WiringMode)> = refs + .iter() + .filter(|r| r.part == TreePart::File) + .map(|r| { + ( + r.source_file.clone(), + r.purl.clone(), + r.uuid.clone(), + r.mode, + ) + }) + .collect(); + refs.retain(|r| { + r.part == TreePart::File + || !project.contains(&( + r.source_file.clone(), + r.purl.clone(), + r.uuid.clone(), + r.mode, + )) + }); } /// The install tree `file` belongs to: its tree's root file, or itself. @@ -1496,7 +1538,6 @@ impl Discovery { } fn finalize(&mut self) { - self.resolve_document_trees(); self.elsewhere.sort(); self.elsewhere.dedup(); self.unpatched_copies.sort(); @@ -1506,12 +1547,14 @@ impl Discovery { self.contested.sort(); self.contested.dedup(); for refs in [&mut self.refs, &mut self.shadowed, &mut self.rewirable] { + Self::drop_env_twins(refs); refs.sort_by(|a, b| { - (&a.source_file, &a.purl, &a.uuid, a.mode).cmp(&( + (&a.source_file, &a.purl, &a.uuid, a.mode, a.part).cmp(&( &b.source_file, &b.purl, &b.uuid, b.mode, + b.part, )) }); refs.dedup(); @@ -1677,13 +1720,6 @@ fn is_script_lock(file: &Path) -> bool { crate::utils::python_lock::is_script_lock_name(&file.to_string_lossy()) } -/// Whether `file` is a [`pnpm_document_tree`] key of a pnpm env lockfile -/// document (before [`Discovery::finalize`] maps it back). -fn is_pnpm_env_tree(file: &Path) -> bool { - file.to_str() - .is_some_and(|f| f.ends_with(PNPM_ENV_TREE_SUFFIX)) -} - /// What every extractor receives: the project view plus the hosted-origin /// allowlist, with the guarded-read and identity helpers bolted on. pub(crate) struct DiscoverCtx<'a> { diff --git a/crates/socket-patch-core/src/vex/discover/npm.rs b/crates/socket-patch-core/src/vex/discover/npm.rs index 567050efa..96d63c64e 100644 --- a/crates/socket-patch-core/src/vex/discover/npm.rs +++ b/crates/socket-patch-core/src/vex/discover/npm.rs @@ -53,9 +53,9 @@ use std::path::PathBuf; use serde_json::Value; use super::{ - npm_purl, npm_vendored_tarball_names, parse_json, pnpm_document_tree, vendor_ref, CopyTarget, - DiscoverCtx, Discovery, LocateOpts, Located, PatchedRef, UnattestedKind, UnwiredCopy, - VendorRef, DIAG_LOCKFILE_UNPARSEABLE, DIAG_REF_INVALID, DIAG_REF_UNATTRIBUTABLE, + npm_purl, npm_vendored_tarball_names, parse_json, vendor_ref, CopyTarget, DiscoverCtx, + Discovery, LocateOpts, Located, PatchedRef, TreePart, UnattestedKind, UnwiredCopy, VendorRef, + 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::{ @@ -645,7 +645,7 @@ fn entry_ref( /// Every document of a lock is read. pnpm 11+'s env lockfile document /// (config dependencies, the pinned package manager; identified by its /// importers, [`PnpmLock::packages`]) is an install tree of its own -/// ([`pnpm_document_tree`]): its refs are emitted, and they never contest +/// ([`TreePart::PnpmEnv`]): its refs are emitted, and they never contest /// or are contested by the project document or another lock. /// /// ## Entries @@ -755,19 +755,39 @@ async fn extract_pnpm_lock( return; } // Each document is its own install tree: pnpm 11+'s env lockfile is - // read under its own tree key ([`pnpm_document_tree`]), so its refs and - // the project document's entries never contest each other. + // read as [`TreePart::PnpmEnv`], so its refs and the project document's + // entries never contest each other. for document in lock.documents() { - let tree = pnpm_document_tree(file, document); + let tree = PnpmTree::of(file, document); extract_pnpm_document(ctx, &tree, &lock, document, disabled, out).await; } } -/// The refs and copies of one document of a pnpm lock, recorded under the -/// document's install-tree key `file` (see [`extract_pnpm_lock`]). +/// One install tree of a pnpm lock: the lock's root-relative `file`, the +/// [`TreePart`] its records carry, and the `label` its diagnostics name it +/// by (` (env lockfile)` for the env document). +struct PnpmTree<'a> { + file: &'a str, + part: TreePart, + label: String, +} + +impl<'a> PnpmTree<'a> { + fn of(file: &'a str, document: PnpmDocument) -> Self { + let part = TreePart::from(document); + let label = match part { + TreePart::File => file.to_string(), + TreePart::PnpmEnv => format!("{file} (env lockfile)"), + }; + PnpmTree { file, part, label } + } +} + +/// The refs and copies of one document of a pnpm lock, recorded in its +/// install tree (see [`extract_pnpm_lock`]). async fn extract_pnpm_document( ctx: &DiscoverCtx<'_>, - file: &str, + tree: &PnpmTree<'_>, lock: &PnpmLock<'_>, document: PnpmDocument, disabled: Option<&str>, @@ -778,17 +798,18 @@ async fn extract_pnpm_document( let mut withheld: BTreeSet = BTreeSet::new(); for package in lock.packages_of(document) { let withhold = disabled.map(|_| &mut withheld); - pnpm_entry_ref(ctx, file, package, &mut copies, withhold, out); + pnpm_entry_ref(ctx, tree, package, &mut copies, withhold, out); bundles.extend(entry_bundled(&package.entry).map(|b| (package.key, b))); } - record_pnpm_file_copies(ctx, file, copies, out).await; - record_pnpm_bundled_copies(file, bundles, out); + record_pnpm_file_copies(ctx, tree, copies, out).await; + record_pnpm_bundled_copies(tree, bundles, out); + let label = &tree.label; if let (Some(source), false) = (disabled, withheld.is_empty()) { out.diag( DIAG_REF_UNATTRIBUTABLE, - file, + tree.file, format!( - "{file}: {}, so its hosted pins of {} are not attested from the lock alone, \ + "{label}: {}, so its hosted pins of {} are not attested from the lock alone, \ only from an installed tree whose every copy verifies; \ `socket-patch scan --mode vendored` wires them without a lock", crate::utils::pnpm_workspace::describe_lockfile_disabled(source), @@ -814,7 +835,12 @@ async fn extract_pnpm_document( /// stay refs: the wiring is intact and no rewire could clear the bundled /// copy, so the ledgers' liveness gates (`vendor --check`, `scan`) keep /// seeing them live, and the copy is no cross-lock evidence either. -fn record_pnpm_bundled_copies(file: &str, bundles: Vec<(&str, Bundled<'_>)>, out: &mut Discovery) { +fn record_pnpm_bundled_copies( + tree: &PnpmTree<'_>, + bundles: Vec<(&str, Bundled<'_>)>, + out: &mut Discovery, +) { + let (file, label) = (tree.file, &tree.label); for (parent, bundled) in bundles { let (targets, what) = match bundled { Bundled::All => ( @@ -831,11 +857,11 @@ fn record_pnpm_bundled_copies(file: &str, bundles: Vec<(&str, Bundled<'_>)>, out }; for target in targets { out.unwired_copy(UnwiredCopy { - scope: Some(PathBuf::from(file)), + scope: Some((PathBuf::from(file), tree.part)), target, file: PathBuf::from(file), detail: format!( - "{file} entry `{parent}` {what}, which pnpm unpacks from that package's \ + "{label} entry `{parent}` {what}, which pnpm unpacks from that package's \ own tarball without locking its version, so no Socket wiring reaches it" ), kind: UnattestedKind::BundledCopy, @@ -865,11 +891,11 @@ struct PnpmFileCopy { /// read is left alone. async fn record_pnpm_file_copies( ctx: &DiscoverCtx<'_>, - file: &str, + tree: &PnpmTree<'_>, copies: Vec, out: &mut Discovery, ) { - let lock_dir = std::path::Path::new(file) + let lock_dir = std::path::Path::new(tree.file) .parent() .map(|d| d.to_string_lossy().replace('\\', "/")) .unwrap_or_default(); @@ -919,8 +945,9 @@ async fn record_pnpm_file_copies( } else { "tarball" }; - out.unpatched_copy( - file, + out.unpatched_copy_in( + tree.file, + tree.part, npm_purl(&name, &version), ©.key, &format!( @@ -977,7 +1004,8 @@ pub(crate) async fn pnpm_lock_file_copies( })); } let mut scratch = Discovery::default(); - record_pnpm_file_copies(&ctx, file, copies, &mut scratch).await; + let tree = PnpmTree::of(file, PnpmDocument::Project); + record_pnpm_file_copies(&ctx, &tree, copies, &mut scratch).await; let purl = crate::utils::purl_key::canonical_base_purl(&purl); scratch .unpatched_copies @@ -1009,12 +1037,13 @@ fn pnpm_file_copy(package: &PnpmPackage<'_>, directory: bool) -> Option, - file: &str, + tree: &PnpmTree<'_>, package: &PnpmPackage<'_>, copies: &mut Vec, withheld: Option<&mut BTreeSet>, out: &mut Discovery, ) { + let (file, label) = (tree.file, &tree.label); let key = package.key; let Some(resolution) = &package.resolution else { if resolution_is_socket_shaped(ctx, package) { @@ -1022,7 +1051,7 @@ fn pnpm_entry_ref( DIAG_REF_INVALID, file, format!( - "{file}: packages entry {key:?} references a Socket patch through a \ + "{label}: packages entry {key:?} references a Socket patch through a \ resolution that is not a flat pnpm resolution map; it is ignored" ), ); @@ -1031,7 +1060,7 @@ fn pnpm_entry_ref( }; let Some(tarball) = resolution.tarball() else { // A plain registry entry (integrity only) or a directory/git dep. - out.resolved_elsewhere(file, pnpm_registry_key_purl(key)); + out.resolved_elsewhere_in(file, tree.part, pnpm_registry_key_purl(key)); copies.extend(pnpm_file_copy(package, true)); return; }; @@ -1041,14 +1070,14 @@ fn pnpm_entry_ref( .map(|sri| LockIntegrity::Sri(sri.to_string())); let located = ctx.locate(tarball, LocateOpts::LITERAL_CHECKED); if let Some(vref) = located.vendored { - pnpm_vendored_ref(file, package, tarball, &vref, integrity, out); + pnpm_vendored_ref(tree, package, tarball, &vref, integrity, out); } else if located.decorated_leaf { // pnpm fetches the literal file (`#` / `::` included). out.diag( DIAG_REF_INVALID, file, format!( - "{file}: packages entry {key:?} (tarball {tarball:?}) is not a literal \ + "{label}: packages entry {key:?} (tarball {tarball:?}) is not a literal \ .socket/vendor/npm// path; it is ignored" ), ); @@ -1058,7 +1087,7 @@ fn pnpm_entry_ref( DIAG_REF_INVALID, file, format!( - "{file}: packages entry {key:?} resolves from the Socket patch server but \ + "{label}: packages entry {key:?} resolves from the Socket patch server but \ its key is not a registry name@version" ), ); @@ -1072,25 +1101,20 @@ fn pnpm_entry_ref( } None => integrity, }; - out.push(PatchedRef::hosted( - purl, - uuid, - file, - Some(tarball), - integrity, - true, - )); + out.push( + PatchedRef::hosted(purl, uuid, file, Some(tarball), integrity, true).in_part(tree.part), + ); } else { // A registry-keyed entry fetching some other tarball, or a user's // `file:` tarball. - out.resolved_elsewhere(file, pnpm_registry_key_purl(key)); + out.resolved_elsewhere_in(file, tree.part, pnpm_registry_key_purl(key)); copies.extend(pnpm_file_copy(package, false)); } } /// A vendored packages entry (see [`extract_pnpm`] for the key shapes). fn pnpm_vendored_ref( - file: &str, + tree: &PnpmTree<'_>, package: &PnpmPackage<'_>, tarball: &str, vref: &VendorRef, @@ -1101,8 +1125,11 @@ fn pnpm_vendored_ref( let invalid = |out: &mut Discovery, why: &str| { out.diag( DIAG_REF_INVALID, - file, - format!("{file}: packages entry {key:?} (tarball {tarball:?}) {why}; it is ignored"), + tree.file, + format!( + "{}: packages entry {key:?} (tarball {tarball:?}) {why}; it is ignored", + tree.label + ), ); }; // (name, the key's own `file:` path if the key was rekeyed). @@ -1113,7 +1140,7 @@ fn pnpm_vendored_ref( PnpmKey::V9File { name, path } => (Some(name), Some(path)), // Registry-form key with a hand-pointed `file:` tarball. PnpmKey::Registry { name, version } => { - return finish_vendored(file, name, version, vref, integrity, invalid, out) + return finish_vendored(tree, name, version, vref, integrity, invalid, out) } PnpmKey::Other => return invalid(out, "has no registry name@version key"), }; @@ -1128,11 +1155,11 @@ fn pnpm_vendored_ref( let Some(version) = entry_field(&package.entry, "version") else { return invalid(out, "has no (single) `version:` line"); }; - finish_vendored(file, name, version, vref, integrity, invalid, out) + finish_vendored(tree, name, version, vref, integrity, invalid, out) } fn finish_vendored( - file: &str, + tree: &PnpmTree<'_>, name: &str, version: &str, vref: &VendorRef, @@ -1149,7 +1176,7 @@ fn finish_vendored( &format!("names {purl} but its tarball is not that package's vendored npm tarball"), ); } - out.push(PatchedRef::vendored(purl, vref, file, integrity)); + out.push(PatchedRef::vendored(purl, vref, tree.file, integrity).in_part(tree.part)); } /// [`pnpm_registry_key`] (the lock inventory's key grammar) as a validated @@ -3171,10 +3198,13 @@ mod tests { ); let out = p.discover().await; assert_refs(&out, &hosted); + // The real lock path, with the tree carried beside it + // (code-review finding 6: no pseudo path). assert_eq!( out.refs[0].source_file, std::path::Path::new("pnpm-lock.yaml") ); + assert_eq!(out.refs[0].part, TreePart::PnpmEnv, "{case}"); assert!(out.unattested.is_empty(), "{case}: {:#?}", out.unattested); assert!(out.shadowed.is_empty(), "{case}: {:#?}", out.shadowed); assert!(out.contested.is_empty(), "{case}: {:#?}", out.contested); @@ -3195,8 +3225,26 @@ mod tests { ); let out = p.discover().await; assert_refs(&out, &hosted); + assert_eq!(out.refs[0].part, TreePart::File); assert!(out.contested.is_empty(), "{:#?}", out.contested); + // Wired in both documents, the pins differing: ONE ref, the project + // document's (code-review finding 6: no twin refs of one file). + let env_wired = wired.replace(SRI, "sha512-ENVPIN=="); + let p = Project::new(); + p.write( + "pnpm-lock.yaml", + format!("{}{}", env(&env_wired), project(&wired)), + ); + let out = p.discover().await; + assert_refs(&out, &hosted); + assert_eq!(out.refs.len(), 1, "{:#?}", out.refs); + assert_eq!(out.refs[0].part, TreePart::File); + assert_eq!( + out.refs[0].locked_integrity, + Some(LockIntegrity::Sri(SRI.into())) + ); + // The env document's registry entry contests no other lock's ref. let npm_lock = format!( r#"{{"name":"m","lockfileVersion":3,"packages":{{"":{{"name":"m"}}, diff --git a/crates/socket-patch-core/src/vex/discover/testing/golden.rs b/crates/socket-patch-core/src/vex/discover/testing/golden.rs index 54509be0e..589d75633 100644 --- a/crates/socket-patch-core/src/vex/discover/testing/golden.rs +++ b/crates/socket-patch-core/src/vex/discover/testing/golden.rs @@ -37,7 +37,7 @@ use std::path::{Path, PathBuf}; use serde_json::{json, Map, Value}; use crate::vex::discover::{ - ContestedRef, Diag, Discovery, PatchedRef, Recognized, ResolvedElsewhere, Unattested, + ContestedRef, Diag, Discovery, PatchedRef, Recognized, ResolvedElsewhere, TreePart, Unattested, UnlockedPin, UnpatchedCopy, WiringMode, }; @@ -150,8 +150,9 @@ fn render(out: &Discovery, root: &Path) -> Value { url, // The block's lines; the lock itself is the golden's input. yarn_classic_block: _, + part, } = r; - json!({ + let mut rendered = json!({ "purl": purl, "uuid": uuid, "mode": mode(*m), @@ -161,7 +162,13 @@ fn render(out: &Discovery, root: &Path) -> Value { "integrity_required": integrity_required, "url": url, "lockfile_basis_ok": r.lockfile_basis_ok(), - }) + }); + // Only off the default, so the goldens of single-tree files stay + // byte-identical. + if *part != TreePart::File { + rendered["part"] = json!(format!("{part:?}")); + } + rendered }; let refs: Vec = refs.iter().map(render_ref).collect(); let diagnostics: Vec = diagnostics @@ -255,6 +262,8 @@ fn render(out: &Discovery, root: &Path) -> Value { key, how, remedy, + // A pnpm env-lockfile copy contests only its own tree. + part: _, } = c; let mut copy = json!({ "purl": purl, @@ -301,6 +310,7 @@ fn render(out: &Discovery, root: &Path) -> Value { key, how, remedy, + part: _, } = c; let mut copy = json!({ "purl": purl, diff --git a/crates/socket-patch-core/tests/equivalence/pnpm_depscan_sized.golden b/crates/socket-patch-core/tests/equivalence/pnpm_depscan_sized.golden index 29f1efde0..5b9361677 100644 --- a/crates/socket-patch-core/tests/equivalence/pnpm_depscan_sized.golden +++ b/crates/socket-patch-core/tests/equivalence/pnpm_depscan_sized.golden @@ -1,3 +1,3 @@ # A depscan-sized seeded pnpm-lock.yaml set + shuffled overrides. # -0 342d79c926a00044 2cf6f312234e59ca +0 b120df27c737dd90 1ab3b36d03d3b3f8 diff --git a/docs/design/lock-model-divergences.md b/docs/design/lock-model-divergences.md index 9f486b460..a25f67760 100644 --- a/docs/design/lock-model-divergences.md +++ b/docs/design/lock-model-divergences.md @@ -82,7 +82,7 @@ follow from the install-tree decision. | N1 | pnpm `gitBranchLockfile` with a branch lock present: is the root lock the install tree? | Hosted withholds the root locks and refuses (`hosted/engine.rs` ~930). Vendored refuses. Discovery and the inventory never check the setting. | Hosted | false attestation | | 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 keys the env document as the tree ` (env lockfile)`, which neither contests nor is contested by the project document or another lock; a document is the env one when its importers carry only `configDependencies` / `packageManagerDependencies`. | | +| 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 | | 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 | | diff --git a/scripts/loc-budget.json b/scripts/loc-budget.json index c73793400..b31d39de7 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": 31117 + "max": 31207 }, "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": 88313 + "max": 88314 } } }