From 8215c754117bbde30793bb1ec426986aee9ebc14 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 6 Oct 2026 13:28:02 +0000 Subject: [PATCH 1/4] Start fix for #935, #938, #939 Assisted-by: Claude Code:claude-opus-5-5 From 918a9ce76c76de770c0c9c9aeb5cb5fff7c62334 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 6 Oct 2026 13:48:14 +0000 Subject: [PATCH 2/4] Stop VEX attesting beside an unpatched lock copy A lockfile-only `vex` attested a package as not_affected while the same lock also installed an unpatched copy of that exact name@version: - pnpm: a `file:` directory or tarball copy (#935) - yarn classic: a registry block left beside the Socket block, e.g. after `yarn add -W left-pad --exact` (#938) - yarn berry: a `file:` / url copy locked under another dependency name, which the `resolutions` pin never reaches (#939) The cross-lock contest only weighs OTHER locks, and each extractor wrote its own same-lock rule (npm and yarn classic git only). Discovery now has one shared same-lock rule: extractors record an unpatched copy and every ref of the same name@version in that lock is dropped with a patched_ref_unattributable diagnostic naming the copy. Yarn classic's git-copy filter moves onto it. The berry and pnpm extractors read the copy's real package from its package.json (directory or tarball) or from the registry tarball url. Assisted-by: Claude Code:claude-opus-5-5 --- .../socket-patch-core/src/vex/discover/mod.rs | 112 ++++++ .../socket-patch-core/src/vex/discover/npm.rs | 184 +++++++++- .../src/vex/discover/testing/golden.rs | 24 +- .../src/vex/discover/yarn.rs | 347 ++++++++++++++++-- .../vex-discover-golden/redirect-npm.json | 50 ++- 5 files changed, 673 insertions(+), 44 deletions(-) diff --git a/crates/socket-patch-core/src/vex/discover/mod.rs b/crates/socket-patch-core/src/vex/discover/mod.rs index d43715c5d..7cc525045 100644 --- a/crates/socket-patch-core/src/vex/discover/mod.rs +++ b/crates/socket-patch-core/src/vex/discover/mod.rs @@ -404,6 +404,23 @@ pub struct ResolvedElsewhere { pub file: PathBuf, } +/// A lock entry that installs its OWN copy of a package — a `file:` +/// directory or tarball, a user url, git, a registry block no Socket rewrite +/// reached — beside a Socket wiring of the same `name@version` in the SAME +/// lock (see [`Discovery::unpatched_copy`]). The package manager installs +/// that copy too (or instead), so the lock's wiring is never attested. +#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)] +pub struct UnpatchedCopy { + /// Canonical base purl ([`canonical_base_purl`]). + pub purl: String, + /// Root-relative lock file. + pub file: PathBuf, + /// The lock entry's key, as the lock spells it. + pub key: String, + /// How that entry installs, completing "lock entry `` …". + pub how: String, +} + /// A ref discovery emits (so rollback, remove and list find the wiring) /// that must not be attested: the files show a build that resolves the /// package from somewhere the pin does not reach. Today: a Gradle lock @@ -437,6 +454,9 @@ pub struct Discovery { /// non-Socket source — the evidence the cross-lock contest /// ([`discover_patched_refs_with`]) weighs against another lock's ref. pub elsewhere: Vec, + /// Same-lock copies that contest that lock's own refs + /// ([`Discovery::unpatched_copy`]). + pub unpatched_copies: Vec, /// Refs in `refs` whose wiring a build bypasses ([`Unattested`]). pub unattested: Vec, } @@ -576,6 +596,70 @@ impl Discovery { } } + /// Record that lock `file`'s entry `key` installs its own copy of `purl` + /// (`None` is ignored), `how` saying from where. The cross-lock contest + /// only weighs OTHER locks (a lock that wires a package is never its + /// own contester there), so this is the one same-lock rule every + /// extractor shares (#935, #938, #939): the copy is also + /// [`Discovery::resolved_elsewhere`] evidence, and + /// [`Discovery::contest_within_locks`] drops every ref of the same + /// `name@version` in the same file. + pub(crate) fn unpatched_copy( + &mut self, + file: &str, + purl: Option, + key: &str, + how: &str, + ) { + let Some(purl) = purl else { + return; + }; + self.resolved_elsewhere(file, Some(purl.clone())); + let copy = UnpatchedCopy { + purl: canonical_base_purl(&purl), + file: PathBuf::from(file), + key: key.to_string(), + how: how.to_string(), + }; + if !self.unpatched_copies.contains(©) { + self.unpatched_copies.push(copy); + } + } + + /// Drop every ref whose OWN lock also installs an unpatched copy of the + /// same `name@version` ([`Discovery::unpatched_copy`]): the build ships + /// that copy whatever the wiring does, so the ref is diagnosed + /// ([`DIAG_REF_UNATTRIBUTABLE`], naming the entry) and not emitted. Its + /// uuid stays recognized (rule 11). + fn contest_within_locks(&mut self) { + if self.unpatched_copies.is_empty() { + return; + } + let refs = std::mem::take(&mut self.refs); + for r in refs { + let copy = self + .unpatched_copies + .iter() + .find(|c| c.purl == r.purl && c.file == r.source_file) + .cloned(); + match copy { + Some(c) => { + let file = r.source_file.to_string_lossy().into_owned(); + self.diag( + DIAG_REF_UNATTRIBUTABLE, + &file, + format!( + "{file}: {} is wired to Socket patch {} but lock entry `{}` {}; \ + that copy stays UNPATCHED and nothing is attested", + r.purl, r.uuid, c.key, c.how + ), + ); + } + None => self.refs.push(r), + } + } + } + /// Drop every ref that ANOTHER lock contests: a lock that resolves the /// same package at the same version from a non-Socket source /// ([`Discovery::resolved_elsewhere`]) while wiring it to no patch @@ -699,6 +783,8 @@ impl Discovery { fn finalize(&mut self) { self.elsewhere.sort(); self.elsewhere.dedup(); + self.unpatched_copies.sort(); + self.unpatched_copies.dedup(); self.unattested.sort(); self.unattested.dedup(); self.refs.sort_by(|a, b| { @@ -770,6 +856,7 @@ async fn discover_with_ctx(ctx: DiscoverCtx<'_>) -> Discovery { gradle::extract(&ctx, &mut out).await; nuget::extract(&ctx, &mut out).await; deno::extract(&ctx, &mut out).await; + out.contest_within_locks(); out.contest_across_locks(); out.recognized.extend(ctx.take_recognized()); out.finalize(); @@ -913,6 +1000,12 @@ impl<'a> DiscoverCtx<'a> { self.view.read_text(rel).await.ok() } + /// Bytes twin of [`DiscoverCtx::read_advisory_text`] (a user's `file:` + /// tarball, read only to name the package it holds). + pub(crate) async fn read_advisory_bytes(&self, rel: &str) -> Option> { + self.view.read_bytes(rel).await.ok() + } + /// Bytes twin of [`DiscoverCtx::read_text`] (JSON and binary locks). A /// binary lock is swept through its lossy UTF-8 view: string pools store /// resolutions verbatim, and a stale string an older patch generation @@ -2066,6 +2159,23 @@ pub(crate) mod testing { /// the patch uuid after it. pub(crate) const TOKEN: &str = "11111111-2222-4333-8444-555555555555"; + /// A minimal npm tarball (`package/package.json` naming + /// `name@version`) — a user's `file:` tarball copy. + pub(crate) fn npm_tgz(name: &str, version: &str) -> Vec { + let manifest = format!(r#"{{"name":"{name}","version":"{version}"}}"#); + let mut tar = tar::Builder::new(flate2::write::GzEncoder::new( + Vec::new(), + flate2::Compression::default(), + )); + let mut header = tar::Header::new_gnu(); + header.set_size(manifest.len() as u64); + header.set_mode(0o644); + header.set_cksum(); + tar.append_data(&mut header, "package/package.json", manifest.as_bytes()) + .unwrap(); + tar.into_inner().unwrap().finish().unwrap() + } + /// Production artifact-URL shape on Socket's patch server /// (`…/patch//////`). pub(crate) fn hosted_url( @@ -2163,6 +2273,8 @@ pub(crate) mod testing { let ctx = self.ctx(); let mut out = Discovery::default(); extract(&ctx, &mut out).await; + // Same-lock copies contest within one extractor's own locks. + out.contest_within_locks(); let swept = ctx.take_recognized(); assert_recognition_covers_refs(&out, &swept, "the ctx sweep", Some(self.root())); out.recognized.extend(swept); diff --git a/crates/socket-patch-core/src/vex/discover/npm.rs b/crates/socket-patch-core/src/vex/discover/npm.rs index 289b2bf61..c130efa95 100644 --- a/crates/socket-patch-core/src/vex/discover/npm.rs +++ b/crates/socket-patch-core/src/vex/discover/npm.rs @@ -572,9 +572,119 @@ async fn extract_pnpm_lock(ctx: &DiscoverCtx<'_>, file: &str, out: &mut Discover ); return; } + let mut copies: Vec = Vec::new(); for package in lock.packages() { - pnpm_entry_ref(ctx, file, package, out); + pnpm_entry_ref(ctx, file, package, &mut copies, out); } + record_pnpm_file_copies(ctx, file, copies, out).await; +} + +/// A pnpm `packages:` entry installed from a user's `file:` directory or +/// tarball, awaiting [`record_pnpm_file_copies`]. +struct PnpmFileCopy { + key: String, + /// The package name (the v9 key's, or a legacy entry's `name:` field). + name: Option, + /// The entry's `version:` field (always there for a tarball). + version: Option, + /// The `file:` path, relative to the lock's directory. + path: String, + directory: bool, +} + +/// Record each [`PnpmFileCopy`] as an unpatched copy of its package (#935): +/// pnpm installs a `file:` directory or tarball from the user's own bytes, +/// and no override or tarball rewire of the registry entry reaches it. A +/// directory entry carries no version, so it is read from the directory's +/// `package.json` (a legacy entry's name too); one whose package cannot be +/// read is left alone. +async fn record_pnpm_file_copies( + ctx: &DiscoverCtx<'_>, + file: &str, + copies: Vec, + out: &mut Discovery, +) { + let lock_dir = std::path::Path::new(file) + .parent() + .map(|d| d.to_string_lossy().replace('\\', "/")) + .unwrap_or_default(); + for copy in copies { + let (mut name, mut version) = (copy.name, copy.version); + if name.is_none() || version.is_none() { + let manifest: Option = + match crate::utils::cargo_workspace::normalize_rel(&lock_dir, ©.path) { + Some(rel) if copy.directory => { + let manifest = if rel.is_empty() { + "package.json".to_string() + } else { + format!("{rel}/package.json") + }; + ctx.read_advisory_text(&manifest).await.and_then(|t| { + serde_json::from_str(t.trim_start_matches('\u{feff}')).ok() + }) + } + Some(rel) => match ctx.read_advisory_bytes(&rel).await { + Some(bytes) => tokio::task::spawn_blocking(move || { + let map = + crate::patch::package::read_archive_bytes_to_map(&bytes).ok()?; + serde_json::from_slice::(map.get("package.json")?).ok() + }) + .await + .ok() + .flatten(), + None => None, + }, + None => None, + }; + let field = |k: &str| { + manifest + .as_ref() + .and_then(|m| m.get(k)) + .and_then(Value::as_str) + .map(str::to_string) + }; + name = name.or_else(|| field("name")); + version = version.or_else(|| field("version")); + } + let (Some(name), Some(version)) = (name, version) else { + continue; + }; + let what = if copy.directory { + "directory" + } else { + "tarball" + }; + out.unpatched_copy( + file, + npm_purl(&name, &version), + ©.key, + &format!( + "installs it from the user's file: {what} {:?}, which no Socket wiring \ + reaches", + copy.path + ), + ); + } +} + +/// The [`PnpmFileCopy`] of a `file:`-keyed entry, `None` for any other key. +fn pnpm_file_copy(package: &PnpmPackage<'_>, directory: bool) -> Option { + let (name, path) = match classify_pnpm_key(package.key) { + PnpmKey::V9File { name, path } => (Some(name.to_string()), path), + PnpmKey::LegacyFile { path } => ( + entry_field(&package.entry, "name").map(str::to_string), + path, + ), + PnpmKey::Registry { .. } | PnpmKey::Other => return None, + }; + let path = path.strip_prefix("file:").unwrap_or(path).to_string(); + Some(PnpmFileCopy { + key: package.key.to_string(), + name, + version: entry_field(&package.entry, "version").map(str::to_string), + path, + directory, + }) } /// Classify one `packages:` entry and push its ref, if any. @@ -582,6 +692,7 @@ fn pnpm_entry_ref( ctx: &DiscoverCtx<'_>, file: &str, package: &PnpmPackage<'_>, + copies: &mut Vec, out: &mut Discovery, ) { let key = package.key; @@ -601,6 +712,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)); + copies.extend(pnpm_file_copy(package, true)); return; }; let integrity = resolution @@ -641,8 +753,10 @@ fn pnpm_entry_ref( true, )); } else { - // A registry-keyed entry fetching some other tarball. + // A registry-keyed entry fetching some other tarball, or a user's + // `file:` tarball. out.resolved_elsewhere(file, pnpm_registry_key_purl(key)); + copies.extend(pnpm_file_copy(package, false)); } } @@ -1979,6 +2093,72 @@ mod tests { assert!(out.diagnostics.is_empty(), "{:#?}", out.diagnostics); } + /// #935: pnpm installs a `file:` directory or `file:` tarball copy of + /// the wired name@version from the user's own bytes, so a hosted pin of + /// the registry entry in the SAME lock is not attested: the ref is + /// dropped with a diagnostic naming the copy (v9 keys and pnpm 8's + /// legacy `file:` keys alike). Controls: the wiring alone is a ref, and + /// a `file:` copy of ANOTHER version does not contest it. + #[tokio::test] + async fn issue_935_same_lock_file_copy_contests_the_pnpm_ref() { + 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 lock = |extra: &str| format!("lockfileVersion: '9.0'\n\npackages:\n\n{wired}{extra}"); + let dir_v9 = " left-pad@file:forks/left-pad:\n \ + resolution: {directory: forks/left-pad, type: directory}\n\n"; + let tgz_v9 = " left-pad@file:forks/left-pad-1.3.0.tgz:\n \ + resolution: {integrity: sha512-UPSTREAM==, tarball: file:forks/left-pad-1.3.0.tgz}\n \ + version: 1.3.0\n\n"; + let dir_legacy = " file:forks/left-pad:\n \ + resolution: {directory: forks/left-pad, type: directory}\n \ + name: left-pad\n version: 1.3.0\n\n"; + + let control = Project::new(); + control.write("pnpm-lock.yaml", lock("")); + assert_refs( + &run(&control).await, + &[("pkg:npm/left-pad@1.3.0", UUID_A, WiringMode::Hosted)], + ); + + for (case, extra, fork_version) in [ + ("v9 file: directory", dir_v9, "1.3.0"), + ("v9 file: tarball", tgz_v9, "1.3.0"), + ("legacy file: directory", dir_legacy, "1.3.0"), + ] { + let p = Project::new(); + p.write("pnpm-lock.yaml", lock(extra)); + p.write( + "forks/left-pad/package.json", + format!(r#"{{"name":"left-pad","version":"{fork_version}"}}"#), + ); + p.write("forks/left-pad-1.3.0.tgz", npm_tgz("left-pad", "1.3.0")); + let out = run(&p).await; + assert!(out.refs.is_empty(), "{case}: {:#?}", out.refs); + assert!( + out.diagnostics + .iter() + .any(|d| d.code == DIAG_REF_UNATTRIBUTABLE + && d.detail.contains("forks/left-pad") + && d.detail.contains("UNPATCHED")), + "{case}: {:#?}", + out.diagnostics + ); + } + + // A `file:` directory holding ANOTHER version is not a copy of it. + let p = Project::new(); + p.write("pnpm-lock.yaml", lock(dir_v9)); + p.write( + "forks/left-pad/package.json", + r#"{"name":"left-pad","version":"2.0.0"}"#, + ); + assert_refs( + &run(&p).await, + &[("pkg:npm/left-pad@1.3.0", UUID_A, WiringMode::Hosted)], + ); + } + /// The committed golden (TS backend output — what a depscan PR leaves). #[tokio::test] async fn pnpm_golden_hosted_fixture() { 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 7c73c6761..92542b27f 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,8 @@ use std::path::{Path, PathBuf}; use serde_json::{json, Map, Value}; use crate::vex::discover::{ - Diag, Discovery, PatchedRef, Recognized, ResolvedElsewhere, Unattested, UnlockedPin, WiringMode, + Diag, Discovery, PatchedRef, Recognized, ResolvedElsewhere, Unattested, UnlockedPin, + UnpatchedCopy, WiringMode, }; /// Set to `1` to (re)write the goldens instead of comparing against them. @@ -119,6 +120,7 @@ fn render(out: &Discovery, root: &Path) -> Value { recognized, unlocked_pins, elsewhere, + unpatched_copies, unattested, } = out; let refs: Vec = refs @@ -225,6 +227,26 @@ fn render(out: &Discovery, root: &Path) -> Value { .collect::>() .into(); } + if !unpatched_copies.is_empty() { + rendered["unpatched_copies"] = unpatched_copies + .iter() + .map(|c| { + let UnpatchedCopy { + purl, + file, + key, + how, + } = c; + json!({ + "purl": purl, + "file": path_str(file), + "key": key, + "how": normalize(how, &roots), + }) + }) + .collect::>() + .into(); + } rendered } diff --git a/crates/socket-patch-core/src/vex/discover/yarn.rs b/crates/socket-patch-core/src/vex/discover/yarn.rs index b6763b88f..6aefe69e5 100644 --- a/crates/socket-patch-core/src/vex/discover/yarn.rs +++ b/crates/socket-patch-core/src/vex/discover/yarn.rs @@ -138,44 +138,21 @@ fn stray_top_level_line(text: &str) -> Option<&str> { // ── classic ────────────────────────────────────────────────────────────── fn extract_classic(ctx: &DiscoverCtx<'_>, entries: Vec, out: &mut Discovery) { - // (purl, key) of every live git-fetched block: that copy installs the - // git bytes, so no wiring of the same package in this lock is attested. - let mut git_copies: Vec<(String, String)> = Vec::new(); for entry in entries { if entry.live && !entry.patterns.is_empty() { - classic_block(ctx, &entry, &mut git_copies, out); - } - } - if git_copies.is_empty() { - return; - } - let refs = std::mem::take(&mut out.refs); - for r in refs { - let git_key = (r.source_file == std::path::Path::new(YARN_LOCK)) - .then(|| git_copies.iter().find(|(purl, _)| *purl == r.purl)) - .flatten(); - match git_key { - Some((_, key)) => out.diag( - DIAG_REF_UNATTRIBUTABLE, - YARN_LOCK, - format!( - "{YARN_LOCK}: {} is wired to a Socket patch but lock entry `{key}` \ - installs from git, which yarn fetches from the git source rather than \ - a tarball; that copy stays UNPATCHED and nothing is attested", - r.purl - ), - ), - None => out.refs.push(r), + classic_block(ctx, &entry, out); } } } -fn classic_block( - ctx: &DiscoverCtx<'_>, - entry: &YarnEntry, - git_copies: &mut Vec<(String, String)>, - out: &mut Discovery, -) { +/// Classify one live classic block. A block of a package that yarn +/// installs from anything but a Socket wiring — git (#363) or a registry / +/// url tarball (#938) — is an unpatched copy of that `name@version` +/// ([`Discovery::unpatched_copy`]): yarn 1 installs ONE copy per +/// `name@version`, and which block it takes depends on which pattern it +/// resolves first, so no wiring of the same version in this lock is +/// attested beside it. +fn classic_block(ctx: &DiscoverCtx<'_>, entry: &YarnEntry, out: &mut Discovery) { let YarnEntry { block, patterns, .. } = entry; @@ -207,10 +184,12 @@ fn classic_block( ), ); } - if let Some(purl) = purl { - out.resolved_elsewhere(YARN_LOCK, Some(purl.clone())); - git_copies.push((purl, block.key.clone())); - } + out.unpatched_copy( + YARN_LOCK, + purl, + &block.key, + "installs from git, which yarn fetches from the git source rather than a tarball", + ); return; } let Some(resolved) = resolved else { @@ -228,13 +207,23 @@ fn classic_block( let names: Vec> = names.into_iter().collect(); let Some(wiring) = classify(ctx, resolved, YARN_LOCK, &block.key, out) else { // Not Socket's (a rejected Socket spelling was diagnosed instead): - // evidence against another lock's wiring of the same package. + // an unpatched copy of the package, which contests a wiring of the + // same version in this lock and in any other. if let ([Some(name)], Some(version), false) = ( names.as_slice(), classic_field(&block.lines, "version"), root_anchored_spelling(resolved), ) { - out.resolved_elsewhere(YARN_LOCK, npm_purl(name, version)); + out.unpatched_copy( + YARN_LOCK, + npm_purl(name, version), + &block.key, + &format!( + "installs it from {resolved:?}, not a Socket patch (yarn 1 \ + installs one copy per name@version, from whichever block it resolves \ + first)" + ), + ); } return; }; @@ -296,6 +285,7 @@ struct BerryHostedKeyed { async fn extract_berry(ctx: &DiscoverCtx<'_>, lock: BerryLock, out: &mut Discovery) { let mut vendored: Vec = Vec::new(); let mut hosted_keyed: Vec = Vec::new(); + let mut copies: Vec = Vec::new(); for entry in lock.entries.iter().filter(|e| e.live) { berry_block( ctx, @@ -303,9 +293,11 @@ async fn extract_berry(ctx: &DiscoverCtx<'_>, lock: BerryLock, out: &mut Discove lock.cache_key.as_deref(), &mut vendored, &mut hosted_keyed, + &mut copies, out, ); } + record_berry_copies(ctx, copies, out).await; confirm_berry_hosted_keyed(ctx, hosted_keyed, out).await; confirm_berry_vendored(ctx, vendored, out).await; } @@ -383,6 +375,7 @@ fn berry_block( cache_key: Option<&str>, vendored: &mut Vec, hosted_keyed: &mut Vec, + copies: &mut Vec, out: &mut Discovery, ) { let block = &entry.block; @@ -410,6 +403,17 @@ fn berry_block( if let (Some(v), false) = (locator_version, root_anchored_spelling(spec)) { let version = berry_field(&block.lines, "version").unwrap_or(v); out.resolved_elsewhere(YARN_LOCK, npm_purl(name, version)); + } else if locator_version.is_none() && !root_anchored_spelling(spec) { + // A user's `file:` / url copy: yarn keys it by the DEPENDENCY + // name (`lp2@file:…`), so which package it installs is read + // from the copy itself ([`record_berry_copies`], #939). + if let Some(version) = berry_field(&block.lines, "version") { + copies.push(BerryCopy { + key: block.key.clone(), + reference: reference.to_string(), + version: version.to_string(), + }); + } } return; }; @@ -692,6 +696,146 @@ fn emit( } } +/// A berry lock entry that installs a user's `file:` tarball / directory or +/// url tarball (not a Socket wiring), awaiting [`record_berry_copies`]. +struct BerryCopy { + key: String, + /// The locator's reference (`file:[#…][::…]` or a url). + reference: String, + version: String, +} + +/// Record each [`BerryCopy`] as an unpatched copy of the package it really +/// installs (#939). yarn keys a `file:` / url dependency by the name the +/// depender gave it, so `"lp2": "file:left-pad-1.3.0.tgz"` locks as `lp2@…` +/// while `node_modules/lp2` IS left-pad@1.3.0, and no `resolutions` pin of +/// `left-pad` reaches it. The real name comes from the copy itself: the +/// registry tarball path of a url (`…//-/-.tgz`), the +/// `package.json` inside a `file:` tarball, or a `file:` directory's +/// `package.json`. A copy whose name cannot be read is left alone. +async fn record_berry_copies(ctx: &DiscoverCtx<'_>, copies: Vec, out: &mut Discovery) { + for copy in copies { + let (name, how) = if let Some(path) = copy.reference.strip_prefix("file:") { + let path = path + .split(['#', ':']) + .next() + .unwrap_or_default() + .to_string(); + let Some(rel) = berry_file_copy_path(©.reference, &path) else { + continue; + }; + let name = if is_tarball_leaf(&rel) { + match ctx.read_advisory_bytes(&rel).await { + Some(bytes) => { + tokio::task::spawn_blocking(move || tarball_package_name(&bytes)) + .await + .ok() + .flatten() + } + None => None, + } + } else { + let manifest = if rel.is_empty() { + PACKAGE_JSON.to_string() + } else { + format!("{rel}/{PACKAGE_JSON}") + }; + match ctx.read_advisory_text(&manifest).await { + Some(text) => manifest_name(text.as_bytes()), + None => None, + } + }; + (name, format!("installs it from the user's file:{path}")) + } else { + let url = copy.reference.split(['#']).next().unwrap_or_default(); + ( + registry_tarball_name(url, ©.version), + format!("installs it from {url:?}"), + ) + }; + let Some(name) = name else { + continue; + }; + out.unpatched_copy( + YARN_LOCK, + npm_purl(&name, ©.version), + ©.key, + &format!( + "{how}, which no Socket wiring of {name} reaches (yarn keys it by the \ + dependency name)" + ), + ); + } +} + +/// The root-relative path a berry `file:` reference names: relative to the +/// workspace in its `locator=` binding (`b@workspace:packages/b`), or to +/// the root when it has none. `None` when it leaves the root. +fn berry_file_copy_path(reference: &str, path: &str) -> Option { + let workspace = reference + .split_once("::") + .and_then(|(_, bindings)| bindings.split('&').find_map(|b| b.strip_prefix("locator="))) + .map(crate::utils::purl::percent_decode_purl_component) + .and_then(|locator| { + locator + .split_once("@workspace:") + .map(|(_, ws)| ws.to_string()) + }) + .unwrap_or_default(); + let workspace = if workspace == "." { + String::new() + } else { + workspace + }; + crate::utils::cargo_workspace::normalize_rel(&workspace, path) +} + +fn is_tarball_leaf(path: &str) -> bool { + path.ends_with(".tgz") || path.ends_with(".tar.gz") +} + +/// `name` of a `package.json`. +fn manifest_name(bytes: &[u8]) -> Option { + let bytes = bytes.strip_prefix(b"\xef\xbb\xbf").unwrap_or(bytes); + serde_json::from_slice::(bytes) + .ok()? + .get("name")? + .as_str() + .map(str::to_string) +} + +/// `name` of the `package.json` inside an npm tarball. +fn tarball_package_name(bytes: &[u8]) -> Option { + let map = crate::patch::package::read_archive_bytes_to_map(bytes).ok()?; + manifest_name(map.get(PACKAGE_JSON)?) +} + +/// The package an npm registry tarball url serves, from its +/// `//-/-.tgz` path (`` may be `@scope/leaf`, +/// its `@` / `/` possibly percent-encoded); `None` for any other shape. +fn registry_tarball_name(url: &str, version: &str) -> Option { + let path = url.split_once("://").map_or(url, |(_, rest)| rest); + let path = path.split(['?']).next()?; + let (before, file) = path.rsplit_once("/-/")?; + let mut segs: Vec = before + .split('/') + .skip(1) // the host + .map(|seg| crate::utils::purl::percent_decode_purl_component(seg).into_owned()) + .collect(); + let leaf_name = segs.pop()?; + let (scope, leaf_name) = match leaf_name.split_once('/') { + Some((scope, leaf)) => (Some(scope.to_string()), leaf.to_string()), + None => (segs.pop().filter(|s| s.starts_with('@')), leaf_name), + }; + if file != format!("{leaf_name}-{version}.tgz") { + return None; + } + Some(match scope { + Some(scope) => format!("{scope}/{leaf_name}"), + None => leaf_name, + }) +} + #[cfg(test)] mod tests { use super::super::testing::*; @@ -1131,6 +1275,133 @@ mod tests { } } + /// #938: a registry block of the wired name@version beside the Socket + /// block in the SAME yarn.lock (e.g. after `yarn add -W left-pad + /// --exact`): yarn 1 installs one copy per name@version, from whichever + /// block it resolves first, so the wiring is not attested and the + /// diagnostic names the registry block. Control: another version's + /// registry block does not contest it. + #[tokio::test] + async fn issue_938_classic_registry_block_of_the_same_version_contests_the_ref() { + let lp = hosted_url("npm", "left-pad", "1.3.0", UUID_A, "left-pad-1.3.0.tgz"); + let wired = classic_block("left-pad@^1.3.0", "1.3.0", &lp, Some(SRI)); + let registry = |version: &str| { + classic_block( + &format!("left-pad@{version}"), + version, + &format!("https://registry.yarnpkg.com/left-pad/-/left-pad-{version}.tgz#5b8a"), + Some("sha512-UPSTREAM=="), + ) + }; + let p = Project::new(); + p.write("yarn.lock", classic(&[registry("1.3.0"), wired.clone()])); + let out = run(&p).await; + assert!(out.refs.is_empty(), "{:#?}", out.refs); + assert!( + out.diagnostics + .iter() + .any(|d| d.code == DIAG_REF_UNATTRIBUTABLE + && d.detail.contains("left-pad@1.3.0") + && d.detail.contains("registry.yarnpkg.com")), + "{:#?}", + out.diagnostics + ); + + let p = Project::new(); + p.write("yarn.lock", classic(&[registry("1.2.0"), wired])); + assert_refs( + &run(&p).await, + &[("pkg:npm/left-pad@1.3.0", UUID_A, WiringMode::Hosted)], + ); + } + + /// #939: yarn berry keys a `file:` tarball / directory or url dependency + /// by the DEPENDENCY name (`lp2@file:…`), so a copy of the wired + /// left-pad@1.3.0 under another name escapes the `resolutions` pin and + /// installs unpatched. Its package is read from the copy itself (the + /// tarball's or directory's `package.json`, the registry url's path) and + /// it contests the wiring in the same lock. Control: a copy holding + /// another package is not one. + #[tokio::test] + async fn issue_939_berry_other_name_copy_contests_the_ref() { + let ws = "b%40workspace%3Apackages%2Fb"; + let wired = berry_vendored_block("left-pad", "1.3.0", UUID_A); + let copies = [ + ( + "file: tarball", + berry_block( + &format!("lp2@file:../../forks/left-pad-1.3.0.tgz::locator={ws}"), + "1.3.0", + &format!( + "lp2@file:../../forks/left-pad-1.3.0.tgz#../../forks/left-pad-1.3.0.tgz\ + ::hash=5c8e4c&locator={ws}" + ), + None, + ), + ), + ( + "file: directory", + berry_block( + &format!("lp2@file:../../forks/left-pad::locator={ws}"), + "1.3.0", + &format!("lp2@file:../../forks/left-pad#../../forks/left-pad::hash=1a2b3c&locator={ws}"), + None, + ), + ), + ( + "registry tarball url", + berry_block( + "lp2@https://registry.npmjs.org/left-pad/-/left-pad-1.3.0.tgz", + "1.3.0", + "lp2@https://registry.npmjs.org/left-pad/-/left-pad-1.3.0.tgz", + None, + ), + ), + ]; + let project = |blocks: &[String], fork: &str| { + let p = Project::new(); + p.write("yarn.lock", berry(blocks)); + p.write( + "package.json", + package_json_with_resolutions(serde_json::json!({ + "left-pad": format!("file:./.socket/vendor/npm/{UUID_A}/left-pad-1.3.0.tgz"), + })), + ); + p.write("forks/left-pad-1.3.0.tgz", npm_tgz(fork, "1.3.0")); + p.write( + "forks/left-pad/package.json", + format!(r#"{{"name":"{fork}","version":"1.3.0"}}"#), + ); + p + }; + assert_refs( + &run(&project(std::slice::from_ref(&wired), "left-pad")).await, + &[("pkg:npm/left-pad@1.3.0", UUID_A, WiringMode::Vendored)], + ); + for (case, copy) in &copies { + let out = run(&project(&[wired.clone(), copy.clone()], "left-pad")).await; + assert!(out.refs.is_empty(), "{case}: {:#?}", out.refs); + assert!( + out.diagnostics + .iter() + .any(|d| d.code == DIAG_REF_UNATTRIBUTABLE + && d.detail.contains("lp2@") + && d.detail.contains("UNPATCHED")), + "{case}: {:#?}", + out.diagnostics + ); + } + // The `file:` copies hold another package: not a copy of left-pad. + for (case, copy) in &copies[..2] { + let out = run(&project(&[wired.clone(), copy.clone()], "other-pkg")).await; + assert_refs( + &out, + &[("pkg:npm/left-pad@1.3.0", UUID_A, WiringMode::Vendored)], + ); + assert!(out.refs.len() == 1, "{case}"); + } + } + /// The vendored berry pair exactly as `vendor::yarn_berry_lock` writes it /// — spike B3's lock entry plus the root `package.json` `resolutions` /// value — for a plain and a scoped package. diff --git a/crates/socket-patch-core/tests/fixtures/vex-discover-golden/redirect-npm.json b/crates/socket-patch-core/tests/fixtures/vex-discover-golden/redirect-npm.json index 372837d60..e06e3fdb3 100644 --- a/crates/socket-patch-core/tests/fixtures/vex-discover-golden/redirect-npm.json +++ b/crates/socket-patch-core/tests/fixtures/vex-discover-golden/redirect-npm.json @@ -6859,7 +6859,21 @@ "file": "yarn.lock" } ], - "live_claims": [] + "live_claims": [], + "unpatched_copies": [ + { + "purl": "pkg:npm/left-pad@1.3.0", + "file": "yarn.lock", + "key": "\"safe-pad@npm:left-pad@^1.3.0\"", + "how": "installs it from \"https://registry.yarnpkg.com/left-pad/-/left-pad-1.3.0.tgz#5b8a3a7765dfe001261dde915ec1972a5f1bb07e\", not a Socket patch (yarn 1 installs one copy per name@version, from whichever block it resolves first)" + }, + { + "purl": "pkg:npm/totally-other@1.3.0", + "file": "yarn.lock", + "key": "\"left-pad@npm:totally-other@^1.3.0\"", + "how": "installs it from \"https://registry.yarnpkg.com/totally-other/-/totally-other-1.3.0.tgz#aaaabbbbccccddddeeeeffff0000111122223333\", not a Socket patch (yarn 1 installs one copy per name@version, from whichever block it resolves first)" + } + ] }, "redirect/npm/yarn-classic/basic/expected": { "refs": [ @@ -6909,7 +6923,15 @@ "file": "yarn.lock" } ], - "live_claims": [] + "live_claims": [], + "unpatched_copies": [ + { + "purl": "pkg:npm/left-pad@1.3.0", + "file": "yarn.lock", + "key": "left-pad@1.3.0", + "how": "installs it from \"https://registry.yarnpkg.com/left-pad/-/left-pad-1.3.0.tgz#5b8a3a7765dfe001261dde915ec1972a5f1bb07e\", not a Socket patch (yarn 1 installs one copy per name@version, from whichever block it resolves first)" + } + ] }, "redirect/npm/yarn-classic/crlf/expected": { "refs": [ @@ -6951,6 +6973,14 @@ "uuid": "22222222-2222-2222-2222-222222222222", "purl": "pkg:npm/left-pad@1.3.0" } + ], + "unpatched_copies": [ + { + "purl": "pkg:npm/abbrev@1.1.1", + "file": "yarn.lock", + "key": "abbrev@^1.0.0", + "how": "installs it from \"https://registry.yarnpkg.com/abbrev/-/abbrev-1.1.1.tgz#f8f2c887ad10bf67f634f005b6987fed3179aac8\", not a Socket patch (yarn 1 installs one copy per name@version, from whichever block it resolves first)" + } ] }, "redirect/npm/yarn-classic/crlf/input": { @@ -6968,6 +6998,20 @@ "file": "yarn.lock" } ], - "live_claims": [] + "live_claims": [], + "unpatched_copies": [ + { + "purl": "pkg:npm/abbrev@1.1.1", + "file": "yarn.lock", + "key": "abbrev@^1.0.0", + "how": "installs it from \"https://registry.yarnpkg.com/abbrev/-/abbrev-1.1.1.tgz#f8f2c887ad10bf67f634f005b6987fed3179aac8\", not a Socket patch (yarn 1 installs one copy per name@version, from whichever block it resolves first)" + }, + { + "purl": "pkg:npm/left-pad@1.3.0", + "file": "yarn.lock", + "key": "left-pad@1.3.0", + "how": "installs it from \"https://registry.yarnpkg.com/left-pad/-/left-pad-1.3.0.tgz#5b8a3a7765dfe001261dde915ec1972a5f1bb07e\", not a Socket patch (yarn 1 installs one copy per name@version, from whichever block it resolves first)" + } + ] } } From 7eda8d888b1c16c3ba8f7e6ce0e121f01f6b160d Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 5 Oct 2026 18:24:21 +0000 Subject: [PATCH 3/4] Route Gradle digests through utils::digest main has failed socket-patch-core's lib tests since Gradle support (#646) and the digest helpers (#865) both landed. The guard test production_digests_go_through_the_helpers flags three files #646 added that still hash inline: crawlers/gradle_cache.rs, patch/jvm_jar.rs and patch/sidecars/maven.rs. That breaks test, test-release and coverage on every open PR. Each inline sha1/sha256 call now goes through sha1_hex_of or sha256_hex_of, which compute the same lowercase hex. Behaviour is unchanged. Assisted-by: Claude Code:claude-opus-5-5 (cherry picked from commit 659ac2c24e5c5904e743b4bc98ea4645da2ed6a1) --- crates/socket-patch-core/src/crawlers/gradle_cache.rs | 9 ++++----- crates/socket-patch-core/src/patch/jvm_jar.rs | 7 ++----- crates/socket-patch-core/src/patch/sidecars/maven.rs | 4 +--- 3 files changed, 7 insertions(+), 13 deletions(-) diff --git a/crates/socket-patch-core/src/crawlers/gradle_cache.rs b/crates/socket-patch-core/src/crawlers/gradle_cache.rs index ef295ee27..afd7c4fba 100644 --- a/crates/socket-patch-core/src/crawlers/gradle_cache.rs +++ b/crates/socket-patch-core/src/crawlers/gradle_cache.rs @@ -70,8 +70,7 @@ pub fn hash_eq(dir_name: &str, sha1_hex: &str) -> bool { /// Whether `bytes` are the pristine download Gradle stored in the hash /// directory `dir_name` (their sha1 names it). pub fn pristine(dir_name: &str, bytes: &[u8]) -> bool { - use sha1::{Digest, Sha1}; - hash_eq(dir_name, &hex::encode(Sha1::digest(bytes))) + hash_eq(dir_name, &crate::utils::digest::sha1_hex_of(bytes)) } /// Whether `path` is a version directory of a `files-2.1` tree @@ -432,8 +431,6 @@ impl DerivedIndex { /// The [`DerivedCopies`] of the jar `jar_leaf` whose pristine bytes /// hash to `pristine_sha1`. pub fn query(&self, jar_leaf: &str, pristine_sha1: &str) -> DerivedCopies { - use sha1::{Digest, Sha1}; - let instrumented = format!("instrumented-{jar_leaf}"); let mut out = DerivedCopies { incomplete: self.incomplete, @@ -460,7 +457,9 @@ impl DerivedIndex { out.stale.push(path.clone()); } else if name == jar_leaf || name == instrumented { match crate::utils::fs::read_regular_to_bytes_sync(path) { - Ok(bytes) if hash_eq(&hex::encode(Sha1::digest(&bytes)), pristine_sha1) => { + Ok(bytes) + if hash_eq(&crate::utils::digest::sha1_hex_of(&bytes), pristine_sha1) => + { out.stale.push(path.clone()) } Ok(_) => out.unknown.push(path.clone()), diff --git a/crates/socket-patch-core/src/patch/jvm_jar.rs b/crates/socket-patch-core/src/patch/jvm_jar.rs index 82d679406..f38a84403 100644 --- a/crates/socket-patch-core/src/patch/jvm_jar.rs +++ b/crates/socket-patch-core/src/patch/jvm_jar.rs @@ -25,8 +25,6 @@ use std::collections::HashMap; use std::path::{Path, PathBuf}; -use sha1::Digest as _; - use crate::crawlers::gradle_cache; use crate::hash::git_sha256::compute_git_sha256_from_bytes; use crate::manifest::schema::PatchFileInfo; @@ -353,12 +351,11 @@ fn unpatched_members( } fn sha256_hex(bytes: &[u8]) -> String { - use sha2::Digest as _; - hex::encode(sha2::Sha256::digest(bytes)) + crate::utils::digest::sha256_hex_of(bytes) } fn sha1_hex(bytes: &[u8]) -> String { - hex::encode(sha1::Sha1::digest(bytes)) + crate::utils::digest::sha1_hex_of(bytes) } /// `/jvm-originals/.jar`. diff --git a/crates/socket-patch-core/src/patch/sidecars/maven.rs b/crates/socket-patch-core/src/patch/sidecars/maven.rs index f2f5a2466..8798bfce6 100644 --- a/crates/socket-patch-core/src/patch/sidecars/maven.rs +++ b/crates/socket-patch-core/src/patch/sidecars/maven.rs @@ -17,8 +17,6 @@ use std::path::{Path, PathBuf}; -use sha1::Digest as _; - use super::{ SidecarAdvisory, SidecarAdvisoryCode, SidecarError, SidecarFile, SidecarFileAction, SidecarPayload, SidecarSeverity, @@ -44,7 +42,7 @@ impl Algo { fn digest(self, bytes: &[u8]) -> String { match self { - Algo::Sha1 => hex::encode(sha1::Sha1::digest(bytes)), + Algo::Sha1 => crate::utils::digest::sha1_hex_of(bytes), Algo::Md5 => hex::encode(md5(bytes)), } } From 980b7b628641e7195adfbc37609b978d766937a4 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 6 Oct 2026 14:10:37 +0000 Subject: [PATCH 4/4] Drop quadratic dedup of same-lock copies Every non-Socket yarn classic block is now recorded as a possible unpatched copy, and each record scanned the whole list for a duplicate first. On a 3000-package lock that made hosted scans and rescans about 17% slower in the scan benchmark. The list is already sorted and deduplicated once when discovery finishes, so the per-record scan goes. Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-core/src/vex/discover/mod.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/crates/socket-patch-core/src/vex/discover/mod.rs b/crates/socket-patch-core/src/vex/discover/mod.rs index 7cc525045..b07db48b0 100644 --- a/crates/socket-patch-core/src/vex/discover/mod.rs +++ b/crates/socket-patch-core/src/vex/discover/mod.rs @@ -615,15 +615,14 @@ impl Discovery { return; }; self.resolved_elsewhere(file, Some(purl.clone())); - let copy = UnpatchedCopy { + // Deduplicated once, in `finalize`: a per-push scan is quadratic + // over a lock with thousands of registry blocks. + self.unpatched_copies.push(UnpatchedCopy { purl: canonical_base_purl(&purl), file: PathBuf::from(file), key: key.to_string(), how: how.to_string(), - }; - if !self.unpatched_copies.contains(©) { - self.unpatched_copies.push(copy); - } + }); } /// Drop every ref whose OWN lock also installs an unpatched copy of the