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 6030b57bf..3ba09bf18 100644 --- a/crates/socket-patch-core/src/formats/pnpm/grammar.rs +++ b/crates/socket-patch-core/src/formats/pnpm/grammar.rs @@ -1,14 +1,18 @@ //! 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; 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 +92,96 @@ 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 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; 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() + .enumerate() + .map(|(i, span)| { + let kind = if several && i == 0 && 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/hosted.rs b/crates/socket-patch-core/src/formats/pnpm/hosted.rs index c491bd22c..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!( @@ -338,44 +354,14 @@ 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_mut() + .any(|lock| lock.vendors(&fname, &dep.version)); if vendored { result.warnings.push(RewriteWarning { code: "redirect_pnpm_entry_vendored".into(), @@ -404,6 +390,35 @@ pub(crate) fn plan_hosted( } } +/// The `(name, version)` of every packages entry of `text` the vendor +/// backends rekeyed: v9's `@file:` key or legacy's bare +/// `file:` key with a `name:` line, each with its `version:` +/// line, whose path is this project's vendored npm artifact +/// ([`crate::origin::vendor_ref`]: root-anchored, canonical uuid, +/// traversal-safe leaf) — the one rule lockfile discovery reads vendored +/// pnpm keys with. A user's own `file:` dep of the same name, a +/// `../.socket/vendor/…` path from another checkout, and an `overrides:` +/// line alone (pnpm mirrors it from package.json whether or not anything +/// matches) are not vendoring. +fn vendored_packages(text: &str) -> HashSet<(String, String)> { + use super::{classify_pnpm_key, PnpmKey}; + let lock = super::PnpmLock::parse_keys(text); + lock.packages() + .iter() + .filter_map(|package| { + let (name, path) = match classify_pnpm_key(package.key) { + PnpmKey::V9File { name, path } => (name.trim_start_matches('/'), path), + PnpmKey::LegacyFile { path } => (pnpm::entry_field(&package.entry, "name")?, path), + PnpmKey::Registry { .. } | PnpmKey::Other => return None, + }; + let version = pnpm::entry_field(&package.entry, "version")?; + crate::origin::vendor_ref(path) + .filter(|v| v.eco == "npm") + .map(|_| (name.to_string(), version.to_string())) + }) + .collect() +} + /// Test-only reference for the residual gate: every instance of this exact /// name@version in `content` that does not resolve to `artifact_url`. /// Production judges the same predicate inline, per instance, on each diff --git a/crates/socket-patch-core/src/formats/pnpm/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..2df9b643f 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,28 +83,52 @@ 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, 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), - 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; }; @@ -368,16 +397,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 +426,21 @@ 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, 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(), } } @@ -416,23 +453,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; @@ -453,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) } @@ -504,7 +556,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 +578,42 @@ 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. + // 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(); - 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 +810,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 +837,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 +885,91 @@ mod tests { ); } + /// 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_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!( + "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}; + // 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", 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")); + 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:?}" + ); + } + } + + /// 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!( @@ -967,3 +1124,283 @@ 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("))) + } + + /// 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) { + 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) + .into_iter() + .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() {")); + } + + /// 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/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/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 f28fa9f14..b740642ca 100644 --- a/crates/socket-patch-core/src/patch/redirect/mod.rs +++ b/crates/socket-patch-core/src/patch/redirect/mod.rs @@ -28265,6 +28265,76 @@ 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", + ), + // 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}"); + } + } + /// 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..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/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 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 1bb703280..7b68418c8 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,14 @@ 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(|(_, k)| *k == PnpmDocument::Env); pm_major .or(legacy_lock.then_some(8)) .or(env_document.then_some(11)) @@ -1504,17 +1503,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; }; @@ -3803,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 208934cf1..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 @@ -8,10 +8,14 @@ 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; 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, @@ -20,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, LockfileEntry, UnsupportedNpmLayout}; +use super::{LockIntegrity, LockfileEntry, UnsupportedNpmLayout}; use crate::vendor::npm_flavor::detect_npm_lock_flavor_in; // ── registry view ── @@ -40,28 +44,93 @@ 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). -/// Any remaining probe failure falls back to Rush's common lock when -/// `rush.json` is present. +/// ([`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)))) +} + +/// 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(dedup_prefer_integrity) +} + +/// 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) } -/// [`inventory_npm_lock`] over a [`ProjectView`]. -pub(super) async fn inventory_npm_lock_in( +/// 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> { - inventory_npm_lock_raw_in(view) - .await - .map(|found| found.map(|(flavor, raw)| (flavor, dedup_prefer_integrity(raw)))) + let mut found = inventory_npm_router_lock_raw_in(view).await?; + if let Some((NpmLockFlavor::Pnpm | NpmLockFlavor::PnpmLegacy, entries)) = &mut found { + if !rush && lockfile_disabled(view, false).await.is_some() { + withhold_hosted_pins(entries); + } + } + Ok(found) +} + +/// 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; + } + } } -/// [`inventory_npm_lock`] before its collapse: every -/// guarded instance ([`super::inventory_project_every_lock`]). -pub(super) async fn inventory_npm_lock_raw_in( +/// The lock the npm flavor router picks, with its fallbacks (see +/// [`inventory_npm_lock`]). +async fn inventory_npm_router_lock_raw_in( view: &ProjectView<'_>, ) -> Result)>, UnsupportedNpmLayout> { let (flavor, _warnings) = match detect_npm_lock_flavor_in(view).await { @@ -144,13 +213,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/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 2d9636f7f..be5994858 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 bd14058d0..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,12 +755,15 @@ 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, +/// 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; @@ -776,12 +780,151 @@ 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"), + "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, 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_trees(tmp.path()).await.unwrap(); assert!( !entries.iter().any(|e| e.name == "only-in-common"), - "the root lock must win; the rush fallback must not run: {entries:?}" + "{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 +/// 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_trees(tmp.path()).await.unwrap(); + assert_eq!( + integrity(&entries, "left-pad"), + LockIntegrity::Sri("sha512-PATCHED==".into()) ); } @@ -1298,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(), @@ -1307,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(), @@ -2888,7 +3033,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); @@ -3692,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 fcefec8b5..fb708f70a 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`, @@ -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; @@ -249,6 +249,35 @@ 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"; +/// 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, + } + } +} + /// 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)] @@ -303,6 +332,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 @@ -361,9 +392,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, @@ -381,6 +419,7 @@ impl PatchedRef { integrity_required: false, url: None, yarn_classic_block: None, + part: TreePart::File, } } @@ -482,6 +521,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 { @@ -553,6 +594,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 @@ -659,8 +702,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, @@ -671,7 +715,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 { @@ -979,11 +1027,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, }); } @@ -996,6 +1055,7 @@ impl Discovery { purl: canonical_base_purl(&purl), file: PathBuf::from(file), as_spelled: true, + part: TreePart::File, }); } @@ -1015,6 +1075,7 @@ impl Discovery { purl, file: PathBuf::from(file), as_spelled: false, + part: TreePart::File, }); } @@ -1032,11 +1093,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 { @@ -1045,6 +1118,7 @@ impl Discovery { key: key.to_string(), how: how.to_string(), remedy: None, + part, }); } @@ -1096,7 +1170,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, @@ -1115,6 +1191,7 @@ impl Discovery { key: u.key.clone(), how: u.how.clone(), remedy: None, + part: TreePart::File, }); } } @@ -1137,7 +1214,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) => { @@ -1173,14 +1250,17 @@ 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 - /// 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). + /// install. Neither is a pnpm env lockfile document + /// ([`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() @@ -1198,7 +1278,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) && r.part == TreePart::File) .then(|| { let tree = self.tree_of(&r.source_file); // Only a PyPI ref can match another spelling of its @@ -1217,9 +1297,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 + && e.part == TreePart::File && !wiring.get(other).is_some_and(|purls| { purls.contains(&r.purl) || (pypi @@ -1275,6 +1359,35 @@ impl Discovery { } } + /// 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. fn tree_of<'a>(&'a self, file: &'a Path) -> &'a Path { self.install_trees @@ -1439,12 +1552,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(); @@ -4567,7 +4682,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 diff --git a/crates/socket-patch-core/src/vex/discover/npm.rs b/crates/socket-patch-core/src/vex/discover/npm.rs index 3193d7f80..96d63c64e 100644 --- a/crates/socket-patch-core/src/vex/discover/npm.rs +++ b/crates/socket-patch-core/src/vex/discover/npm.rs @@ -54,13 +54,13 @@ 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, + 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::{ - 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 +/// ([`TreePart::PnpmEnv`]): 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,22 +754,62 @@ async fn extract_pnpm_lock( ); return; } + // Each document is its own install tree: pnpm 11+'s env lockfile is + // read as [`TreePart::PnpmEnv`], so its refs and the project document's + // entries never contest each other. + for document in lock.documents() { + let tree = PnpmTree::of(file, document); + extract_pnpm_document(ctx, &tree, &lock, document, disabled, out).await; + } +} + +/// 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<'_>, + tree: &PnpmTree<'_>, + 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); + 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), @@ -789,7 +835,12 @@ async fn extract_pnpm_lock( /// 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 => ( @@ -806,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, @@ -840,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(); @@ -894,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!( @@ -925,7 +977,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). @@ -951,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 @@ -983,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) { @@ -996,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" ), ); @@ -1005,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; }; @@ -1015,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" ), ); @@ -1032,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" ), ); @@ -1046,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, @@ -1075,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). @@ -1087,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"), }; @@ -1102,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, @@ -1123,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 @@ -3091,6 +3144,148 @@ 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); + // 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); + 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_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"}}, + "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, 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 8b8c62706..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 782731128c8cd746 f3e10497dfc8fec6 +0 b120df27c737dd90 1ab3b36d03d3b3f8 diff --git a/docs/design/lock-model-divergences.md b/docs/design/lock-model-divergences.md index 2a2003e2b..f6cb67c08 100644 --- a/docs/design/lock-model-divergences.md +++ b/docs/design/lock-model-divergences.md @@ -84,17 +84,17 @@ follow from the install-tree decision. N11 was decided on 2026-11-05. | 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. Discovery withholds pnpm root and member-lock attestations whenever any `pnpm-lock..yaml` exists, whatever the settings or environment say (owner, 2026-11-05; #2010 (draft)). | 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, for every package manager's user patches (owner decision; #2010 (draft) for npm and bun, #2018 (draft) for pnpm, yarn and vlt). The override is the existing `--allow-patch-conflicts`; `vex` never attests under a user patch, even with it. | 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. Fixed in #2009 (draft). | | -| 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. The vendored writers, reverters and GC claim only this project's vendor paths in #2025 (draft); the pnpm readers and planner follow in #2009 / #2029 (draft). | revert could rewrite another project's wiring | +| 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 (#2009)**: 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. The vendored writers, reverters and GC claim only this project's vendor paths in #2025 (draft). pnpm part **resolved in PR 2a (#2009)**: the hosted pnpm "vendored?" check reads the rekeyed packages entry with `origin::vendor_ref`; the planner follows in #2029 (draft). | 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. | Shadowed (owner, 2026-11-05; see above). Fixed in #2029 (draft). | | -| 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. Fixed in #2009 (draft). | 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 (#2009)** (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. Fixed in #2009 (draft). | 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 (#2009)**: 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 272b903fa..7df1b640f 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": 30544 + "max": 30767 }, "lock-writers": { "paths": [