From e4193a278f7a7206cc24883dafc7abf5e137f01d Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 20:22:51 +0000 Subject: [PATCH 1/8] Start fix for #1538 Assisted-by: Claude Code:claude-opus-5-5 From f28e8f28a030cbaa0f09c516d89502b573948889 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 20:45:26 +0000 Subject: [PATCH 2/8] Check installed npm deps against declared range A hosted yarn classic pin written by v4 can install a patched package whose package.json now asks for a newer dependency range (is-number ^7.0.0) while the lock still installs the old one (6.0.0). vex and scan --vex only checked that node_modules/ existed, so they attested not_affected for a package that runs with the old dependency. The installed check now finds the copy node actually loads (the nearest node_modules/) and omits the patch with dependency_not_installed when that copy's version is outside the declared range. Ranges or versions the npm range reader can't decide, and non-registry specs, are still never evidence. The npm range reader moves from the yarn berry vendor backend to utils::npm_range so both callers share it. Refs #1538 Assisted-by: Claude Code:claude-opus-5-5 --- .../tests/e2e_vex_lockfile/yarn.rs | 11 ++ crates/socket-patch-core/src/utils/mod.rs | 1 + .../socket-patch-core/src/utils/npm_range.rs | 117 ++++++++++++++ .../src/vendor/yarn_berry_lock.rs | 110 +------------ crates/socket-patch-core/src/vex/verify.rs | 144 ++++++++++++++++-- 5 files changed, 261 insertions(+), 122 deletions(-) create mode 100644 crates/socket-patch-core/src/utils/npm_range.rs diff --git a/crates/socket-patch-cli/tests/e2e_vex_lockfile/yarn.rs b/crates/socket-patch-cli/tests/e2e_vex_lockfile/yarn.rs index 82dddf42f..5f4ad9e30 100644 --- a/crates/socket-patch-cli/tests/e2e_vex_lockfile/yarn.rs +++ b/crates/socket-patch-cli/tests/e2e_vex_lockfile/yarn.rs @@ -705,6 +705,17 @@ fn hosted_patch_adding_an_uninstalled_dependency_is_omitted() { let (code, env) = vex_json(cwd, &["--proxy-url", &server.uri()]); let cell = format!("{flavor:?} dependency installed"); assert_attested(cwd, code, &env, UUID, "redirected", &cell); + + // #1538: installed, but outside the range the patched package.json + // declares (the old dependency a pre-v5 pin's lock still installs). + put( + cwd, + "node_modules/is-odd/package.json", + br#"{"name":"is-odd","version":"2.0.0"}"#, + ); + let (code, env) = vex_json(cwd, &["--proxy-url", &server.uri()]); + let cell = format!("{flavor:?} dependency out of range"); + assert_omitted(cwd, code, &env, "dependency_not_installed", &cell); } drop(rt); } diff --git a/crates/socket-patch-core/src/utils/mod.rs b/crates/socket-patch-core/src/utils/mod.rs index f3d49935f..127f2c121 100644 --- a/crates/socket-patch-core/src/utils/mod.rs +++ b/crates/socket-patch-core/src/utils/mod.rs @@ -14,6 +14,7 @@ pub(crate) mod http; pub(crate) mod line_endings; pub mod lock_fragments; pub mod notice; +pub(crate) mod npm_range; pub(crate) mod package_manager; pub mod pdm_lock; pub(crate) mod pep440; diff --git a/crates/socket-patch-core/src/utils/npm_range.rs b/crates/socket-patch-core/src/utils/npm_range.rs new file mode 100644 index 000000000..049bbe7e6 --- /dev/null +++ b/crates/socket-patch-core/src/utils/npm_range.rs @@ -0,0 +1,117 @@ +//! npm semver ranges, read the way npm reads them, over the `semver` +//! crate (whose bare-version and partial-version rules differ from npm's). + +/// Whether the npm semver `range` admits `version`; `None` for a range +/// this reader does not decide (a dist-tag, a partial hyphen bound). +pub(crate) fn npm_range_admits(range: &str, version: &str) -> Option { + let version = semver::Version::parse(version).ok()?; + let mut admits = false; + for alternative in range.split("||") { + let comparators: Vec = match alternative.split_once(" - ") { + // `a - b` with full versions is `>=a <=b`. + Some((low, high)) => { + let full = |v: &str| { + let v = v.trim().trim_start_matches(['v', '=']); + semver::Version::parse(v).ok().map(|_| v.to_string()) + }; + vec![format!(">={}", full(low)?), format!("<={}", full(high)?)] + } + None => glue_operators(alternative) + .iter() + .map(|c| npm_comparator(c)) + .collect::>()?, + }; + let req = match comparators.is_empty() { + true => "*".to_string(), + false => comparators.join(", "), + }; + admits |= semver::VersionReq::parse(&req).ok()?.matches(&version); + } + Some(admits) +} + +/// The comparators of one npm range alternative, an operator npm allows +/// to stand apart (`>= 1.2.0`) joined to its version. +fn glue_operators(alternative: &str) -> Vec { + let mut out: Vec = Vec::new(); + let mut pending = String::new(); + for token in alternative.split_whitespace() { + if token + .chars() + .all(|c| matches!(c, '^' | '~' | '>' | '<' | '=')) + { + pending.push_str(token); + } else { + out.push(format!("{}{token}", std::mem::take(&mut pending))); + } + } + out +} + +/// One npm comparator in the `semver` crate's syntax, which differs for a +/// bare version: npm's `1.3.0` is exact and `1.3` means `~1.3`, where the +/// crate reads both as caret ranges. +fn npm_comparator(comparator: &str) -> Option { + let op_len = comparator + .find(|c: char| !matches!(c, '^' | '~' | '>' | '<' | '=')) + .unwrap_or(comparator.len()); + let (op, version) = comparator.split_at(op_len); + let version = version.strip_prefix('v').unwrap_or(version); + if !op.is_empty() { + return Some(format!("{op}{version}")); + } + // Prerelease and build tags ride on the patch part. + let core = version.split(['-', '+']).next().unwrap_or(version); + let parts: Vec<&str> = core.split('.').collect(); + let wildcard = |p: &&str| matches!(*p, "*" | "x" | "X"); + if parts.iter().any(wildcard) { + return Some(version.to_string()); + } + match parts.len() { + 3 => Some(format!("={version}")), + 1 | 2 => Some(format!("~{version}")), + _ => None, + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn npm_range_admits_reads_npm_semantics() { + for (range, version, want) in [ + ("1.3.0", "1.3.0", Some(true)), + ("1.3.0", "1.3.1", Some(false)), + ("^1.3.0", "1.9.0", Some(true)), + ("^1.3.0", "2.0.0", Some(false)), + ("~1.3.0", "1.3.5", Some(true)), + ("~1.3.0", "1.4.0", Some(false)), + ("1.3", "1.3.9", Some(true)), + ("1.3", "1.4.0", Some(false)), + ("1", "1.9.0", Some(true)), + ("1.x", "1.9.0", Some(true)), + ("*", "4.0.0", Some(true)), + ("", "4.0.0", Some(true)), + (">=1.2.0 <1.4.0", "1.3.0", Some(true)), + (">=1.2.0 <1.3.0", "1.3.0", Some(false)), + ("^2.0.0 || ^1.3.0", "1.3.0", Some(true)), + ("1.0.0 - 2.0.0", "1.3.0", Some(true)), + ("1.0.0 - 1.2.0", "1.3.0", Some(false)), + ("1.0 - 2", "1.3.0", None), + (">= 1.2.0", "1.3.0", Some(true)), + ("< 1.3.0", "1.3.0", Some(false)), + ("v1.3.0", "1.3.0", Some(true)), + ("=1.3.0", "1.3.0", Some(true)), + ("1.3.0-beta.1", "1.3.0-beta.1", Some(true)), + ("1.3.0-beta.1", "1.3.0", Some(false)), + ("latest", "1.3.0", None), + ] { + assert_eq!( + npm_range_admits(range, version), + want, + "{range} vs {version}" + ); + } + } +} diff --git a/crates/socket-patch-core/src/vendor/yarn_berry_lock.rs b/crates/socket-patch-core/src/vendor/yarn_berry_lock.rs index 84571be78..109569e61 100644 --- a/crates/socket-patch-core/src/vendor/yarn_berry_lock.rs +++ b/crates/socket-patch-core/src/vendor/yarn_berry_lock.rs @@ -61,6 +61,7 @@ use crate::utils::fs::{ atomic_write_bytes_preserving_mode, read_regular_to_bytes, read_regular_to_string, }; use crate::utils::line_endings::LineEndings; +use crate::utils::npm_range::npm_range_admits; use crate::utils::socket_dir::remove_tree_and_prune; use crate::utils::uri::encode_uri_component; @@ -1799,79 +1800,6 @@ fn plain_npm_range(range: &str) -> bool { !range.contains([':', '@', '/', '#']) } -/// Whether the npm semver `range` admits `version`; `None` for a range -/// this reader does not decide (a dist-tag, a partial hyphen bound). -fn npm_range_admits(range: &str, version: &str) -> Option { - let version = semver::Version::parse(version).ok()?; - let mut admits = false; - for alternative in range.split("||") { - let comparators: Vec = match alternative.split_once(" - ") { - // `a - b` with full versions is `>=a <=b`. - Some((low, high)) => { - let full = |v: &str| { - let v = v.trim().trim_start_matches(['v', '=']); - semver::Version::parse(v).ok().map(|_| v.to_string()) - }; - vec![format!(">={}", full(low)?), format!("<={}", full(high)?)] - } - None => glue_operators(alternative) - .iter() - .map(|c| npm_comparator(c)) - .collect::>()?, - }; - let req = match comparators.is_empty() { - true => "*".to_string(), - false => comparators.join(", "), - }; - admits |= semver::VersionReq::parse(&req).ok()?.matches(&version); - } - Some(admits) -} - -/// The comparators of one npm range alternative, an operator npm allows -/// to stand apart (`>= 1.2.0`) joined to its version. -fn glue_operators(alternative: &str) -> Vec { - let mut out: Vec = Vec::new(); - let mut pending = String::new(); - for token in alternative.split_whitespace() { - if token - .chars() - .all(|c| matches!(c, '^' | '~' | '>' | '<' | '=')) - { - pending.push_str(token); - } else { - out.push(format!("{}{token}", std::mem::take(&mut pending))); - } - } - out -} - -/// One npm comparator in the `semver` crate's syntax, which differs for a -/// bare version: npm's `1.3.0` is exact and `1.3` means `~1.3`, where the -/// crate reads both as caret ranges. -fn npm_comparator(comparator: &str) -> Option { - let op_len = comparator - .find(|c: char| !matches!(c, '^' | '~' | '>' | '<' | '=')) - .unwrap_or(comparator.len()); - let (op, version) = comparator.split_at(op_len); - let version = version.strip_prefix('v').unwrap_or(version); - if !op.is_empty() { - return Some(format!("{op}{version}")); - } - // Prerelease and build tags ride on the patch part. - let core = version.split(['-', '+']).next().unwrap_or(version); - let parts: Vec<&str> = core.split('.').collect(); - let wildcard = |p: &&str| matches!(*p, "*" | "x" | "X"); - if parts.iter().any(wildcard) { - return Some(version.to_string()); - } - match parts.len() { - 3 => Some(format!("={version}")), - 1 | 2 => Some(format!("~{version}")), - _ => None, - } -} - /// The root workspace's name: the lock's single-pattern `@workspace:.` /// entry (the key + resolution of our file: entry embed it). fn root_workspace_name(blocks: &[LockBlock]) -> Option { @@ -3462,42 +3390,6 @@ __metadata: assert_eq!(spell_checksum("abcd", true), "abcd"); } - #[test] - fn npm_range_admits_reads_npm_semantics() { - for (range, version, want) in [ - ("1.3.0", "1.3.0", Some(true)), - ("1.3.0", "1.3.1", Some(false)), - ("^1.3.0", "1.9.0", Some(true)), - ("^1.3.0", "2.0.0", Some(false)), - ("~1.3.0", "1.3.5", Some(true)), - ("~1.3.0", "1.4.0", Some(false)), - ("1.3", "1.3.9", Some(true)), - ("1.3", "1.4.0", Some(false)), - ("1", "1.9.0", Some(true)), - ("1.x", "1.9.0", Some(true)), - ("*", "4.0.0", Some(true)), - ("", "4.0.0", Some(true)), - (">=1.2.0 <1.4.0", "1.3.0", Some(true)), - (">=1.2.0 <1.3.0", "1.3.0", Some(false)), - ("^2.0.0 || ^1.3.0", "1.3.0", Some(true)), - ("1.0.0 - 2.0.0", "1.3.0", Some(true)), - ("1.0.0 - 1.2.0", "1.3.0", Some(false)), - ("1.0 - 2", "1.3.0", None), - (">= 1.2.0", "1.3.0", Some(true)), - ("< 1.3.0", "1.3.0", Some(false)), - ("v1.3.0", "1.3.0", Some(true)), - ("=1.3.0", "1.3.0", Some(true)), - ("1.3.0-beta.1", "1.3.0-beta.1", Some(true)), - ("1.3.0-beta.1", "1.3.0", Some(false)), - ("latest", "1.3.0", None), - ] { - assert_eq!( - npm_range_admits(range, version), - want, - "{range} vs {version}" - ); - } - } #[tokio::test] async fn revert_leaves_drifted_fragments_alone_with_warnings() { // Lock drift: the user re-resolved our entry back to the registry. diff --git a/crates/socket-patch-core/src/vex/verify.rs b/crates/socket-patch-core/src/vex/verify.rs index 958eb8158..189ff5518 100644 --- a/crates/socket-patch-core/src/vex/verify.rs +++ b/crates/socket-patch-core/src/vex/verify.rs @@ -18,6 +18,7 @@ use std::path::{Path, PathBuf}; use crate::crawlers::Ecosystem; use crate::manifest::schema::{PatchManifest, PatchRecord}; use crate::patch::apply::{verify_file_patch, VerifyStatus}; +use crate::utils::npm_range::npm_range_admits; use crate::vendor::state::{lookup_entry, VendorEntry}; use crate::vendor::verify::{ is_vlt_dir_entry, verify_vendored_patch_record, vlt_installed_copy_matches, @@ -509,9 +510,13 @@ fn touches_npm_manifest(record: &PatchRecord) -> bool { /// that node cannot resolve from it: no `node_modules/` in the copy /// or in any directory above its real path (node's lookup, which also /// finds a bundled, hoisted or pnpm/bun store-sibling copy behind a -/// root-linked symlink). Optional dependencies may be absent, and a copy -/// outside a `node_modules` tree (a Plug'n'Play install), an unreadable -/// manifest or a name that is not a safe package name is never evidence. +/// root-linked symlink), or the nearest such copy's version is outside the +/// range the manifest declares (a patch that changed a range the lock never +/// locked installs beside the old dependency, #1538). Optional dependencies +/// may be absent, and a copy outside a `node_modules` tree (a Plug'n'Play +/// install), an unreadable manifest, a name that is not a safe package +/// name, or a range or installed version this reader does not decide is +/// never evidence. async fn unresolved_npm_dependency(pkg_path: &Path) -> Option { let pkg_path = pkg_path.to_path_buf(); crate::utils::fs::run_blocking(move || { @@ -530,22 +535,50 @@ async fn unresolved_npm_dependency(pkg_path: &Path) -> Option { .and_then(serde_json::Value::as_object); pkg.get("dependencies")? .as_object()? - .keys() - .filter(|name| optional.is_none_or(|o| !o.contains_key(*name))) - .filter(|name| crate::vendor::npm_common::is_safe_npm_name(name)) - .find(|name| !node_resolves(&real, name)) - .cloned() + .iter() + .filter(|(name, _)| optional.is_none_or(|o| !o.contains_key(*name))) + .filter(|(name, _)| crate::vendor::npm_common::is_safe_npm_name(name)) + .find(|(name, spec)| match node_resolve(&real, name) { + None => true, + Some(copy) => spec + .as_str() + .and_then(declared_npm_range) + .is_some_and(|range| !copy_admitted(©, range)), + }) + .map(|(name, _)| name.clone()) }) .await } -/// Whether node resolves the package `name` from the package directory -/// `from`: `/node_modules/` exists for `from` or an ancestor -/// that is not itself a `node_modules` directory. -fn node_resolves(from: &Path, name: &str) -> bool { +/// The copy of the package `name` node resolves from the package directory +/// `from`: `/node_modules/` for the nearest of `from` and its +/// ancestors that is not itself a `node_modules` directory. +fn node_resolve(from: &Path, name: &str) -> Option { from.ancestors() .filter(|dir| dir.file_name().is_none_or(|n| n != "node_modules")) - .any(|dir| dir.join("node_modules").join(name).exists()) + .map(|dir| dir.join("node_modules").join(name)) + .find(|copy| copy.exists()) +} + +/// The registry semver range of a `dependencies` value: the value itself, +/// or an `npm:@` alias's range. `None` for any other spec (a +/// path, url, git or workspace dependency, a bare alias). +fn declared_npm_range(spec: &str) -> Option<&str> { + let range = match spec.strip_prefix("npm:") { + Some(target) => &target[target.get(1..)?.find('@')? + 2..], + None => spec, + }; + (!range.contains([':', '/', '#', '@'])).then_some(range.trim()) +} + +/// Whether the installed copy's `package.json` version is in `range`; a +/// version or range [`npm_range_admits`] does not decide admits it. +fn copy_admitted(copy: &Path, range: &str) -> bool { + let version = crate::utils::fs::read_regular_to_string_sync(©.join("package.json")) + .ok() + .and_then(|text| serde_json::from_str::(&text).ok()) + .and_then(|pkg| pkg.get("version")?.as_str().map(str::to_string)); + version.is_none_or(|v| npm_range_admits(range, &v) != Some(false)) } /// Whether `rest` (what follows a matched `from` prefix) starts a new name @@ -2006,6 +2039,91 @@ mod tests { } } + /// A patch that changes a dependency's range (#1538): the copy node + /// resolves (the NEAREST `node_modules/`) must satisfy the range + /// the patched `package.json` declares, not merely exist. + #[tokio::test] + async fn unresolved_npm_dependency_checks_the_declared_range() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + let put = |rel: &str, text: &str| { + let path = root.join(rel); + std::fs::create_dir_all(path.parent().unwrap()).unwrap(); + std::fs::write(path, text).unwrap(); + }; + let lp = root.join("node_modules/lp"); + put( + "node_modules/lp/package.json", + r#"{"name":"lp","dependencies":{"is-number":"^7.0.0"}}"#, + ); + + // The old dependency the lock still installs: not the declared range. + put( + "node_modules/is-number/package.json", + r#"{"name":"is-number","version":"6.0.0"}"#, + ); + assert_eq!( + unresolved_npm_dependency(&lp).await.as_deref(), + Some("is-number") + ); + // A nested copy that satisfies it is the one node loads. + put( + "node_modules/lp/node_modules/is-number/package.json", + r#"{"name":"is-number","version":"7.0.0"}"#, + ); + assert_eq!(unresolved_npm_dependency(&lp).await, None); + // ...and a nested copy that does not hides a hoisted one that does. + put( + "node_modules/is-number/package.json", + r#"{"name":"is-number","version":"7.1.0"}"#, + ); + put( + "node_modules/lp/node_modules/is-number/package.json", + r#"{"name":"is-number","version":"6.0.0"}"#, + ); + assert_eq!( + unresolved_npm_dependency(&lp).await.as_deref(), + Some("is-number") + ); + + // An `npm:` alias is checked against its target's range. + let alias = root.join("alias/node_modules/lp"); + put( + "alias/node_modules/lp/package.json", + r#"{"name":"lp","dependencies":{"num":"npm:is-number@^7.0.0"}}"#, + ); + put( + "alias/node_modules/num/package.json", + r#"{"name":"is-number","version":"6.0.0"}"#, + ); + assert_eq!( + unresolved_npm_dependency(&alias).await.as_deref(), + Some("num") + ); + put( + "alias/node_modules/num/package.json", + r#"{"name":"is-number","version":"7.0.0"}"#, + ); + assert_eq!(unresolved_npm_dependency(&alias).await, None); + + // A range this reader does not decide, a non-registry spec or a + // copy without a readable version is never evidence. + let other = root.join("other/node_modules/lp"); + put( + "other/node_modules/lp/package.json", + r#"{"name":"lp","dependencies":{"a":"latest","b":"file:../b", + "c":"github:o/c","d":"^2.0.0"}}"#, + ); + for dep in ["a", "b", "c"] { + put( + &format!("other/node_modules/{dep}/package.json"), + r#"{"version":"1.0.0"}"#, + ); + } + put("other/node_modules/d/package.json", r#"{"name":"d"}"#); + assert_eq!(unresolved_npm_dependency(&other).await, None); + } + fn hosted_ctx(purl: &str, copies: HostedCopies) -> VendorContext { VendorContext { hosted: [(purl.to_string(), copies)].into_iter().collect(), From 41855ffc2abf2ee739a23c232f0e256444cd7d3f Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 20:58:21 +0000 Subject: [PATCH 3/8] Check yarn classic lock deps in lock-only VEX On a checkout with only package.json and yarn.lock, vex attested a hosted yarn classic pin written by v4 even when the patched package adds a dependency or bumps a range the lock never locked. Yarn 1 installs the dependencies the pinned block's sub-maps name, so such a pin installs the patched package without what it now needs. Discovery now keeps the pinned block of each hosted classic pin. When a patch changes package.json and vex would attest it from the lock alone, it reads the patched package.json (.socket/blobs, else the patch API) and requires the block's dependency sub-maps to be exactly the patched ones, each locked. Otherwise the patch is omitted with dependency_not_installed, or dependency_manifest_unverifiable when the patched package.json can't be read. Fixes #1538 Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-cli/CLI_CONTRACT.md | 3 +- crates/socket-patch-cli/src/commands/vex.rs | 141 +++++++++++++++++- .../tests/e2e_vex_lockfile/yarn.rs | 100 +++++++++++++ .../socket-patch-core/src/vex/discover/mod.rs | 40 +++++ .../src/vex/discover/testing/golden.rs | 2 + .../src/vex/discover/yarn.rs | 44 ++++-- 6 files changed, 307 insertions(+), 23 deletions(-) diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index 695ee163a..584b1cd3a 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -1442,7 +1442,8 @@ command's integration tests check its output. | `vendor_ledger_missing` | `failed` (artifact-level: `uuid` + `details.{ecosystem,path}`, no purl) | repair (v5.0) and `vendor --check`: a lockfile references `.socket/vendor///` but the vendor ledger has no entry for it; repair no longer rebuilds ledger entries from lockfiles. Recovery: restore `.socket/vendor/state.json` from version control and re-run `repair`, or `git checkout -- ` and re-vendor. | | `vendor_wiring_unknown_revert_blocked` | `skipped` (beside the `failed`/`revert_failed` event) | vendor --revert: the ledger entry was reconstructed by a pre-v5 `repair` without wiring records and the live lockfile still resolves through the artifact — the revert refuses (fail-closed) instead of deleting a tarball the lock points at. Recovery: `socket-patch repair`, then restore the pre-vendor lock (or re-lock without the override) and re-run the revert. repair: an npm ledger entry whose `flavor` this release does not know (written by a newer socket-patch) is skipped, never health-checked or rebuilt, and the artifact, wiring and ledger stay as found (a lone `skipped` event; the run's exit is unaffected). Recovery: upgrade socket-patch. | | `stale_install` | `skipped` | vex (in-run `scan --mode hosted --vex`): a hosted stale-install probe found positively unpatched installed bytes, so the purl is omitted even under `--vex-no-verify` (see the gem / Python stale-install guards). | -| `dependency_not_installed` | `skipped` | vex: a hosted npm-family copy carries every patched file, but the patch changes its `package.json` and a dependency it declares (other than an optional one) does not resolve from the copy (no `node_modules/` in it or above its real path), for example because the lockfile never locked it (a yarn classic pin written before the served-manifest check, #1462). Omitted until the dependency is installed. | +| `dependency_not_installed` | `skipped` | vex: a hosted npm-family copy carries every patched file, but the patch changes its `package.json` and a dependency it declares (other than an optional one) does not resolve from the copy (no `node_modules/` in it or above its real path), for example because the lockfile never locked it (a yarn classic pin written before the served-manifest check, #1462), or the nearest copy that resolves is outside the range the patched `package.json` declares (#1538). Lock-only (no installed copy): a yarn classic pin whose block's `dependencies:` / `optionalDependencies:` sub-maps are not exactly the ones the patched `package.json` declares, each locked (#1538). Omitted until the dependency is installed or locked. | +| `dependency_manifest_unverifiable` | `skipped` | vex (lock-only): a yarn classic hosted pin whose patch changes `package.json`, and the patched `package.json` could be read neither from `.socket/blobs` nor from the patch API (`--offline`, a fetch failure), so the dependencies the lock installs with it cannot be checked (#1538). | | `record_unavailable` | `skipped` | vex (manifest-less): a lockfile-wired patch has no local record (manifest, this run's hosted records or a pre-v5 redirect ledger, vendor ledger) and none could be fetched — `--offline`, transport error, 404, or a refused (paid) patch. Omitted, never attested from the `socket-patch.vendor.json` marker. | | `record_mismatch` | `skipped` | vex (manifest-less): the record found for a wired patch names another package or another patch uuid than the wiring. | | `vendor_unwired` | `skipped` | vex: a vendor-ledger entry whose committed artifact no lockfile/config wires any more (reverted lock, leftover ledger or artifact). Applies under `--no-verify` too. | diff --git a/crates/socket-patch-cli/src/commands/vex.rs b/crates/socket-patch-cli/src/commands/vex.rs index 4cac3b911..160806f71 100644 --- a/crates/socket-patch-cli/src/commands/vex.rs +++ b/crates/socket-patch-cli/src/commands/vex.rs @@ -541,6 +541,110 @@ async fn hosted_gem_mirror_captured( captured } +/// Lock-only verdicts of hosted yarn classic pins (#1538): `purls` the +/// plan may attest from the lock alone, mapped to the omission tag of each +/// whose patch changes the package's `package.json` while the pinned +/// block's dependency sub-maps are not exactly the ones that manifest +/// declares, each locked. Yarn 1 installs what the lock names, so such a +/// pin (one an earlier release wrote, #1462) installs the patched package +/// without its patched dependencies: `dependency_not_installed`. A patched +/// `package.json` that can be read neither from `.socket/blobs` nor from +/// the patch API is `dependency_manifest_unverifiable`. +async fn yarn_classic_lock_dependency_gaps( + common: &GlobalArgs, + params: &VexBuildParams, + plan: &Plan, + purls: &[&str], +) -> HashMap { + let mut gaps = HashMap::new(); + for purl in purls { + let blocks: Vec<_> = plan + .hosted + .get(*purl) + .into_iter() + .flat_map(|wiring| &wiring.refs) + .filter_map(|r| r.yarn_classic_block.as_ref()) + .collect(); + let Some(after_hash) = plan.view.patches.get(*purl).and_then(|record| { + record + .files + .iter() + .find(|(key, _)| key.strip_prefix("package/").unwrap_or(key) == "package.json") + .map(|(_, info)| info.after_hash.as_str()) + }) else { + continue; + }; + if blocks.is_empty() { + continue; + } + let pkg = patched_blob(common, params, after_hash) + .await + .and_then(|bytes| serde_json::from_slice::(&bytes).ok()) + .filter(serde_json::Value::is_object); + let gap = match pkg { + None => DEPENDENCY_MANIFEST_UNVERIFIABLE, + Some(pkg) if blocks.iter().any(|b| !b.installs_dependencies_of(&pkg)) => { + DEPENDENCY_NOT_INSTALLED + } + Some(_) => continue, + }; + gaps.insert(purl.to_string(), gap); + } + gaps +} + +/// Omission tag: a hosted yarn classic pin attested from the lock alone +/// whose patch changes `package.json`, which could not be read to check +/// the dependencies the lock installs beside it (#1538). +const DEPENDENCY_MANIFEST_UNVERIFIABLE: &str = "dependency_manifest_unverifiable"; + +/// Largest patched `package.json` blob [`patched_blob`] reads. +const MAX_PATCHED_MANIFEST_BYTES: usize = 4 * 1024 * 1024; + +/// The patched file whose git-sha256 is `after_hash`: from the local +/// `.socket/blobs` store, else (unless `--offline`) from the patch API. +/// `None` when neither has it intact. +async fn patched_blob( + common: &GlobalArgs, + params: &VexBuildParams, + after_hash: &str, +) -> Option> { + use socket_patch_core::hash::git_sha256::compute_git_sha256_from_bytes; + let intact = |bytes: &Vec| { + bytes.len() <= MAX_PATCHED_MANIFEST_BYTES + && compute_git_sha256_from_bytes(bytes).eq_ignore_ascii_case(after_hash) + }; + let local = common.cwd.join(".socket/blobs").join(after_hash); + if let Some(bytes) = socket_patch_core::utils::fs::read_regular_to_bytes(&local) + .await + .ok() + .filter(intact) + { + return Some(bytes); + } + if common.offline { + return None; + } + let overrides = common.api_client_overrides(); + let client = params + .api_client + .get_or_init(|| async { + socket_patch_core::api::client::get_api_client_with_overrides(overrides) + .await + .0 + }) + .await; + let mut body = client.fetch_blob(after_hash).await.ok()??; + let mut bytes = Vec::new(); + while let Some(chunk) = body.chunk().await.ok()? { + bytes.extend_from_slice(chunk.as_ref()); + if bytes.len() > MAX_PATCHED_MANIFEST_BYTES { + return None; + } + } + Some(bytes).filter(intact) +} + /// Core VEX pipeline shared by the standalone `vex` command and the /// embedded `apply`/`vendor`/`scan` `--vex` paths: resolve the product, verify the /// plan's record view against disk (unless `no_verify`), build the OpenVEX @@ -738,17 +842,34 @@ async fn generate_vex( }; let hidden = |purl: &str| purl.starts_with("pkg:npm/") && (npm_store_hidden || pnp_unconsumed(purl)); - let mut lockfile_attested = Vec::new(); - outcome.failed.retain(|f| { - let excused = f.reason == "package_not_found" + let excusable = |f: &FailedPatch| { + f.reason == "package_not_found" && plan.lockfile_basis.contains(&f.purl) && !mirror_refused(&f.purl) && crawled(&f.purl) - && !hidden(&f.purl); - if excused { - lockfile_attested.push(f.purl.clone()); + && !hidden(&f.purl) + }; + // A yarn classic pin attests from the lock only when the lock + // installs the patched package's dependencies with it (#1538). + let excusable_purls: Vec<&str> = outcome + .failed + .iter() + .filter(|f| excusable(f)) + .map(|f| f.purl.as_str()) + .collect(); + let dependency_gaps = + yarn_classic_lock_dependency_gaps(common, params, &plan, &excusable_purls).await; + let mut lockfile_attested = Vec::new(); + outcome.failed.retain_mut(|f| { + if !excusable(f) { + return true; + } + if let Some(gap) = dependency_gaps.get(&f.purl) { + f.reason = gap.to_string(); + return true; } - !excused + lockfile_attested.push(f.purl.clone()); + false }); outcome.applied.extend(lockfile_attested); withhold_unpatched_jvm_copies( @@ -1786,6 +1907,11 @@ fn omission_phrase(reason: &str) -> &'static str { "the patched package.json declares a dependency that is not installed beside \ the package" } + DEPENDENCY_MANIFEST_UNVERIFIABLE => { + "the patch changes package.json, and the patched package.json could be read \ + neither from .socket/blobs nor from the patch API to check the dependencies the \ + lockfile installs with it" + } "mirror_overrides_source" => { "a Bundler mirror overrides the hosted source and installed bytes were not verified" } @@ -1992,6 +2118,7 @@ mod tests { "stale_install", "mirror_overrides_source", DEPENDENCY_NOT_INSTALLED, + DEPENDENCY_MANIFEST_UNVERIFIABLE, RECORD_UNAVAILABLE, RECORD_MISMATCH, VENDOR_UNWIRED, diff --git a/crates/socket-patch-cli/tests/e2e_vex_lockfile/yarn.rs b/crates/socket-patch-cli/tests/e2e_vex_lockfile/yarn.rs index 5f4ad9e30..6f89b206c 100644 --- a/crates/socket-patch-cli/tests/e2e_vex_lockfile/yarn.rs +++ b/crates/socket-patch-cli/tests/e2e_vex_lockfile/yarn.rs @@ -720,6 +720,106 @@ fn hosted_patch_adding_an_uninstalled_dependency_is_omitted() { drop(rt); } +/// #1538: lock-only VEX of a yarn classic hosted pin whose patch changes +/// `package.json`. yarn 1 installs the dependencies the pinned block's +/// sub-maps name, so a pin an earlier release wrote without rewriting them +/// (#1462) installs the patched package beside the old dependencies. +/// +/// | lock | patched blob | verdict | +/// |---|---|---| +/// | no sub-map, patch adds `is-odd ^3.0.0` | patch API | dependency_not_installed | +/// | sub-map `is-odd ^2.0.0`, patch says `^3.0.0` | patch API | dependency_not_installed | +/// | sub-map rewritten, `is-odd@^3.0.0` locked | patch API | attests | +/// | same | `.socket/blobs` only | attests | +/// | same | nowhere | dependency_manifest_unverifiable | +#[test] +fn hosted_classic_lock_only_checks_the_pinned_dependency_maps() { + use wiremock::matchers::{method, path}; + use wiremock::{Mock, ResponseTemplate}; + const MANIFEST: &str = "package/package.json"; + let pristine = format!( + r#"{{"name":"{NAME}","version":"{VERSION}","dependencies":{{"is-odd":"^2.0.0"}}}}"# + ); + let patched = format!( + r#"{{"name":"{NAME}","version":"{VERSION}","dependencies":{{"is-odd":"^3.0.0"}}}}"# + ); + let after_hash = compute_git_sha256_from_bytes(patched.as_bytes()); + let mut view = patch_view(UUID, PURL); + view["files"][MANIFEST] = serde_json::json!({ + "beforeHash": compute_git_sha256_from_bytes(pristine.as_bytes()), + "afterHash": after_hash, + }); + let (rt, server) = serve_patch_views(vec![(UUID, view)]); + let (rt_no_blob, server_no_blob) = serve_patch_views(vec![(UUID, { + let mut v = patch_view(UUID, PURL); + v["files"][MANIFEST] = serde_json::json!({ + "beforeHash": compute_git_sha256_from_bytes(pristine.as_bytes()), + "afterHash": after_hash, + }); + v + })]); + rt.block_on( + Mock::given(method("GET")) + .and(path(format!("/patch/blob/{after_hash}"))) + .respond_with(ResponseTemplate::new(200).set_body_bytes(patched.clone().into_bytes())) + .mount(&server), + ); + + let base = hosted_lock(Flavor::Classic, "https://patch.socket.dev", UUID); + let is_odd = |range: &str, version: &str| { + format!( + "\nis-odd@{range}:\n version \"{version}\"\n resolved \ + \"https://registry.yarnpkg.com/is-odd/-/is-odd-{version}.tgz#\ + 0123456789abcdef0123456789abcdef01234567\"\n integrity \ + sha512-aXNvZGRpc29kZGlzb2RkaXNvZGQ=\n" + ) + }; + let with_sub_map = |range: &str| format!("{base} dependencies:\n is-odd \"{range}\"\n"); + let cells = [ + ( + "added dependency", + base.clone() + &is_odd("^2.0.0", "2.0.0"), + ), + ( + "changed range", + with_sub_map("^2.0.0") + &is_odd("^2.0.0", "2.0.0"), + ), + ]; + for (cell, lock) in cells { + let tmp = tempfile::tempdir().unwrap(); + let cwd = tmp.path(); + hosted_checkout(cwd, Flavor::Classic); + put(cwd, "yarn.lock", lock.as_bytes()); + let (code, env) = vex_json(cwd, &["--proxy-url", &server.uri()]); + assert_omitted(cwd, code, &env, "dependency_not_installed", cell); + } + + // The v5 pin: the sub-map rewritten and the new descriptor locked. + let tmp = tempfile::tempdir().unwrap(); + let cwd = tmp.path(); + hosted_checkout(cwd, Flavor::Classic); + let lock = with_sub_map("^3.0.0") + &is_odd("^3.0.0", "3.0.1"); + put(cwd, "yarn.lock", lock.as_bytes()); + let (code, env) = vex_json(cwd, &["--proxy-url", &server.uri()]); + assert_attested(cwd, code, &env, UUID, "redirected", "rewritten sub-map"); + + // The patched manifest from nowhere: never assumed. + let (code, env) = vex_json(cwd, &["--proxy-url", &server_no_blob.uri()]); + let cell = "rewritten sub-map, no blob"; + assert_omitted(cwd, code, &env, "dependency_manifest_unverifiable", cell); + + // ...but read from the local blob store. + put( + cwd, + &format!(".socket/blobs/{after_hash}"), + patched.as_bytes(), + ); + let (code, env) = vex_json(cwd, &["--proxy-url", &server_no_blob.uri()]); + assert_attested(cwd, code, &env, UUID, "redirected", "local blob"); + drop(rt); + drop(rt_no_blob); +} + // ────────────────────────────────────────────────────────────────────── // VENDORED — lockfile only (cells a, b, e) // ────────────────────────────────────────────────────────────────────── diff --git a/crates/socket-patch-core/src/vex/discover/mod.rs b/crates/socket-patch-core/src/vex/discover/mod.rs index fafc90b54..963049218 100644 --- a/crates/socket-patch-core/src/vex/discover/mod.rs +++ b/crates/socket-patch-core/src/vex/discover/mod.rs @@ -281,6 +281,44 @@ pub struct PatchedRef { /// Hosted only: the Socket URL (artifact, registry index, or source url) /// the reference was recovered from, for diagnostics. pub url: Option, + /// Hosted yarn classic only: the pinned block yarn 1 installs the + /// package from, whose dependency sub-maps decide which dependencies + /// install beside it (#1538). + pub yarn_classic_block: Option, +} + +/// A hosted yarn classic pin's lock block, as lock-only VEX reads it +/// (#1538). Yarn 1 installs the dependencies the block's `dependencies:` / +/// `optionalDependencies:` sub-maps name, each through a block of its own, +/// not the ones the patched tarball's `package.json` declares: a pin an +/// earlier release wrote without rewriting the sub-maps (#1462) installs +/// the patched package beside the old dependencies. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ClassicPinnedBlock { + lines: Vec, + /// Every descriptor the sub-maps name keys a block of the lock. + sub_maps_locked: bool, +} + +impl ClassicPinnedBlock { + /// The block of `lines`, in a lock whose keys hold the patterns + /// `locked`. + pub(crate) fn new(lines: &[String], locked: &std::collections::HashSet<&str>) -> Self { + use crate::formats::yarn::classic_deps::block_dep_descriptors; + ClassicPinnedBlock { + lines: lines.to_vec(), + sub_maps_locked: block_dep_descriptors(lines) + .iter() + .all(|d| locked.contains(d.as_str())), + } + } + + /// Whether yarn 1 installs, from this block, exactly the dependency + /// maps the patched package's `package.json` (`pkg`) declares, each + /// one locked. + pub fn installs_dependencies_of(&self, pkg: &serde_json::Value) -> bool { + self.sub_maps_locked && crate::formats::yarn::classic_deps::dep_maps_match(&self.lines, pkg) + } } impl PatchedRef { @@ -303,6 +341,7 @@ impl PatchedRef { locked_integrity, integrity_required, url: url.map(str::to_string), + yarn_classic_block: None, } } @@ -322,6 +361,7 @@ impl PatchedRef { locked_integrity, integrity_required: false, url: None, + yarn_classic_block: None, } } 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 d08299347..d0ef62e6c 100644 --- a/crates/socket-patch-core/src/vex/discover/testing/golden.rs +++ b/crates/socket-patch-core/src/vex/discover/testing/golden.rs @@ -148,6 +148,8 @@ fn render(out: &Discovery, root: &Path) -> Value { locked_integrity, integrity_required, url, + // The block's lines; the lock itself is the golden's input. + yarn_classic_block: _, } = r; json!({ "purl": purl, diff --git a/crates/socket-patch-core/src/vex/discover/yarn.rs b/crates/socket-patch-core/src/vex/discover/yarn.rs index 7fedd9ae9..ab3cb49fe 100644 --- a/crates/socket-patch-core/src/vex/discover/yarn.rs +++ b/crates/socket-patch-core/src/vex/discover/yarn.rs @@ -86,8 +86,8 @@ use serde_json::Value; use super::{ npm_purl, npm_vendored_tarball_names, parse_json, root_anchored_spelling, vendor_ref_decorated, - DiscoverCtx, Discovery, LocateOpts, PatchedRef, VendorRef, Wired, DIAG_LOCKFILE_UNPARSEABLE, - DIAG_REF_INVALID, DIAG_REF_UNATTRIBUTABLE, + ClassicPinnedBlock, DiscoverCtx, Discovery, LocateOpts, PatchedRef, VendorRef, Wired, + DIAG_LOCKFILE_UNPARSEABLE, DIAG_REF_INVALID, DIAG_REF_UNATTRIBUTABLE, }; use crate::formats::yarn::blocks::{berry_field, classic_field}; use crate::formats::yarn::copy::{berry_copy_ref, classic_copy_ref, CopyRef}; @@ -142,9 +142,15 @@ fn stray_top_level_line(text: &str) -> Option<&str> { async fn extract_classic(ctx: &DiscoverCtx<'_>, entries: Vec, out: &mut Discovery) { let mut copies: Vec = Vec::new(); - for entry in entries { + // Every pattern a live block's key holds: what the lock resolves. + let locked: std::collections::HashSet<&str> = entries + .iter() + .filter(|e| e.live) + .flat_map(|e| e.patterns.iter().map(String::as_str)) + .collect(); + for entry in &entries { if entry.live && !entry.patterns.is_empty() { - classic_block(ctx, &entry, &mut copies, out); + classic_block(ctx, entry, &locked, &mut copies, out); } } record_copies(ctx, copies, out).await; @@ -186,6 +192,7 @@ fn classic_block_purl(entry: &YarnEntry) -> Option { fn classic_block( ctx: &DiscoverCtx<'_>, entry: &YarnEntry, + locked: &std::collections::HashSet<&str>, copies: &mut Vec, out: &mut Discovery, ) { @@ -305,7 +312,12 @@ fn classic_block( .filter(|sri| is_sri_pin(sri)) .map(|sri| LockIntegrity::Sri(sri.to_string())) .or_else(|| split_resolved_sha1(resolved).1.map(LockIntegrity::Sha1Hex)); - emit(name, version, resolved, wiring, integrity, &block.key, out); + // A hosted pin's sub-maps decide what installs beside it (#1538). + let pinned = + matches!(wiring, Wired::Hosted(_)).then(|| ClassicPinnedBlock::new(&block.lines, locked)); + emit( + name, version, resolved, wiring, integrity, &block.key, pinned, out, + ); } // ── berry ──────────────────────────────────────────────────────────────── @@ -467,6 +479,7 @@ async fn confirm_berry_hosted_keyed( entry.wiring, entry.integrity, &entry.key, + None, out, ); } else { @@ -631,7 +644,9 @@ fn berry_block( key: block.key.clone(), }); } - hosted => emit(name, version, spec, hosted, integrity, &block.key, out), + hosted => emit( + name, version, spec, hosted, integrity, &block.key, None, out, + ), } } @@ -804,7 +819,9 @@ fn checked_purl(name: &str, version: &str, key: &str, out: &mut Discovery) -> Op purl } -/// Push the ref for a classic block or a berry hosted entry. +/// Push the ref for a classic block or a berry hosted entry; `classic_block` +/// is a hosted classic pin's block ([`ClassicPinnedBlock`]). +#[allow(clippy::too_many_arguments)] fn emit( name: &str, version: &str, @@ -812,6 +829,7 @@ fn emit( wiring: Wired, integrity: Option, key: &str, + classic_block: Option, out: &mut Discovery, ) { match wiring { @@ -827,14 +845,10 @@ fn emit( } else { std::borrow::Cow::Borrowed(spec) }; - out.push(PatchedRef::hosted( - purl, - uuid, - YARN_LOCK, - Some(&url), - integrity, - true, - )); + out.push(PatchedRef { + yarn_classic_block: classic_block, + ..PatchedRef::hosted(purl, uuid, YARN_LOCK, Some(&url), integrity, true) + }); } Wired::Vendored(vref) => { let Some(purl) = vendored_purl(name, version, spec, &vref, key, out) else { From 0b42c38598ddae0d6b9f4e5b519aaa0d1d1c3b3b Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 21:17:37 +0000 Subject: [PATCH 4/8] Tighten #1538 checks after code review - Skip the installed range check for a dependency the project forces with npm overrides, pnpm.overrides or yarn resolutions: its version is the user's choice, not a sign the patch is missing. - Range-check installed optional dependencies too (absent ones are still fine), so installed and lock-only VEX agree. - Compare a yarn classic block's sub-maps with the patched package.json by value, so `foo latest` (how yarn writes it) matches. - Read the patched package.json blob from the manifest's .socket dir, accept only a 64-hex hash as its name, and check its size before reading it. - Share the package.json record-key rule and compute the lock-only candidates once. Refs #1538 Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-cli/CLI_CONTRACT.md | 2 +- crates/socket-patch-cli/src/commands/vex.rs | 64 ++++---- .../tests/e2e_vex_lockfile/yarn.rs | 11 +- .../src/formats/yarn/classic_deps.rs | 68 ++++++++ .../socket-patch-core/src/vex/discover/mod.rs | 3 +- crates/socket-patch-core/src/vex/verify.rs | 154 +++++++++++++++--- 6 files changed, 237 insertions(+), 65 deletions(-) diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index 584b1cd3a..6b34f6265 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -1442,7 +1442,7 @@ command's integration tests check its output. | `vendor_ledger_missing` | `failed` (artifact-level: `uuid` + `details.{ecosystem,path}`, no purl) | repair (v5.0) and `vendor --check`: a lockfile references `.socket/vendor///` but the vendor ledger has no entry for it; repair no longer rebuilds ledger entries from lockfiles. Recovery: restore `.socket/vendor/state.json` from version control and re-run `repair`, or `git checkout -- ` and re-vendor. | | `vendor_wiring_unknown_revert_blocked` | `skipped` (beside the `failed`/`revert_failed` event) | vendor --revert: the ledger entry was reconstructed by a pre-v5 `repair` without wiring records and the live lockfile still resolves through the artifact — the revert refuses (fail-closed) instead of deleting a tarball the lock points at. Recovery: `socket-patch repair`, then restore the pre-vendor lock (or re-lock without the override) and re-run the revert. repair: an npm ledger entry whose `flavor` this release does not know (written by a newer socket-patch) is skipped, never health-checked or rebuilt, and the artifact, wiring and ledger stay as found (a lone `skipped` event; the run's exit is unaffected). Recovery: upgrade socket-patch. | | `stale_install` | `skipped` | vex (in-run `scan --mode hosted --vex`): a hosted stale-install probe found positively unpatched installed bytes, so the purl is omitted even under `--vex-no-verify` (see the gem / Python stale-install guards). | -| `dependency_not_installed` | `skipped` | vex: a hosted npm-family copy carries every patched file, but the patch changes its `package.json` and a dependency it declares (other than an optional one) does not resolve from the copy (no `node_modules/` in it or above its real path), for example because the lockfile never locked it (a yarn classic pin written before the served-manifest check, #1462), or the nearest copy that resolves is outside the range the patched `package.json` declares (#1538). Lock-only (no installed copy): a yarn classic pin whose block's `dependencies:` / `optionalDependencies:` sub-maps are not exactly the ones the patched `package.json` declares, each locked (#1538). Omitted until the dependency is installed or locked. | +| `dependency_not_installed` | `skipped` | vex: a hosted npm-family copy carries every patched file, but the patch changes its `package.json` and a dependency it declares (other than an optional one) does not resolve from the copy (no `node_modules/` in it or above its real path), for example because the lockfile never locked it (a yarn classic pin written before the served-manifest check, #1462), or the nearest copy that resolves (an installed optional one included) is outside the range the patched `package.json` declares, unless the project forces that dependency with npm `overrides`, `pnpm.overrides` or yarn `resolutions` (#1538). Lock-only (no installed copy): a yarn classic pin whose block's `dependencies:` / `optionalDependencies:` sub-maps are not exactly the ones the patched `package.json` declares, each locked (#1538). Omitted until the dependency is installed or locked. | | `dependency_manifest_unverifiable` | `skipped` | vex (lock-only): a yarn classic hosted pin whose patch changes `package.json`, and the patched `package.json` could be read neither from `.socket/blobs` nor from the patch API (`--offline`, a fetch failure), so the dependencies the lock installs with it cannot be checked (#1538). | | `record_unavailable` | `skipped` | vex (manifest-less): a lockfile-wired patch has no local record (manifest, this run's hosted records or a pre-v5 redirect ledger, vendor ledger) and none could be fetched — `--offline`, transport error, 404, or a refused (paid) patch. Omitted, never attested from the `socket-patch.vendor.json` marker. | | `record_mismatch` | `skipped` | vex (manifest-less): the record found for a wired patch names another package or another patch uuid than the wiring. | diff --git a/crates/socket-patch-cli/src/commands/vex.rs b/crates/socket-patch-cli/src/commands/vex.rs index 160806f71..f3c399bff 100644 --- a/crates/socket-patch-cli/src/commands/vex.rs +++ b/crates/socket-patch-cli/src/commands/vex.rs @@ -16,14 +16,14 @@ //! * `--json` (requires `--output`): VEX JSON to file, envelope JSON //! to stdout. This is the CI integration shape. -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::path::{Path, PathBuf}; use clap::Args; use socket_patch_core::api::client::ApiClient; use socket_patch_core::manifest::schema::{PatchManifest, PatchRecord}; use socket_patch_core::telemetry::{track_vex_failed, track_vex_generated, TelemetryAuth}; -use socket_patch_core::vex::verify::DEPENDENCY_NOT_INSTALLED; +use socket_patch_core::vex::verify::{npm_manifest_after_hash, DEPENDENCY_NOT_INSTALLED}; use socket_patch_core::vex::{ build_document, detect_product, BuildOptions, Document, FailedPatch, VendorContext, VerifyOutcome, @@ -565,13 +565,12 @@ async fn yarn_classic_lock_dependency_gaps( .flat_map(|wiring| &wiring.refs) .filter_map(|r| r.yarn_classic_block.as_ref()) .collect(); - let Some(after_hash) = plan.view.patches.get(*purl).and_then(|record| { - record - .files - .iter() - .find(|(key, _)| key.strip_prefix("package/").unwrap_or(key) == "package.json") - .map(|(_, info)| info.after_hash.as_str()) - }) else { + let Some(after_hash) = plan + .view + .patches + .get(*purl) + .and_then(npm_manifest_after_hash) + else { continue; }; if blocks.is_empty() { @@ -610,17 +609,26 @@ async fn patched_blob( after_hash: &str, ) -> Option> { use socket_patch_core::hash::git_sha256::compute_git_sha256_from_bytes; + // A record's hash names a file in the blob store: never a path. + if after_hash.len() != 64 || !after_hash.bytes().all(|b| b.is_ascii_hexdigit()) { + return None; + } let intact = |bytes: &Vec| { bytes.len() <= MAX_PATCHED_MANIFEST_BYTES && compute_git_sha256_from_bytes(bytes).eq_ignore_ascii_case(after_hash) }; - let local = common.cwd.join(".socket/blobs").join(after_hash); - if let Some(bytes) = socket_patch_core::utils::fs::read_regular_to_bytes(&local) + let local = common.socket_dir().join("blobs").join(after_hash); + let small = tokio::fs::metadata(&local) .await - .ok() - .filter(intact) - { - return Some(bytes); + .is_ok_and(|meta| meta.len() <= MAX_PATCHED_MANIFEST_BYTES as u64); + if small { + if let Some(bytes) = socket_patch_core::utils::fs::read_regular_to_bytes(&local) + .await + .ok() + .filter(intact) + { + return Some(bytes); + } } if common.offline { return None; @@ -842,26 +850,26 @@ async fn generate_vex( }; let hidden = |purl: &str| purl.starts_with("pkg:npm/") && (npm_store_hidden || pnp_unconsumed(purl)); - let excusable = |f: &FailedPatch| { - f.reason == "package_not_found" - && plan.lockfile_basis.contains(&f.purl) - && !mirror_refused(&f.purl) - && crawled(&f.purl) - && !hidden(&f.purl) - }; - // A yarn classic pin attests from the lock only when the lock - // installs the patched package's dependencies with it (#1538). - let excusable_purls: Vec<&str> = outcome + let excusable: HashSet = outcome .failed .iter() - .filter(|f| excusable(f)) - .map(|f| f.purl.as_str()) + .filter(|f| { + f.reason == "package_not_found" + && plan.lockfile_basis.contains(&f.purl) + && !mirror_refused(&f.purl) + && crawled(&f.purl) + && !hidden(&f.purl) + }) + .map(|f| f.purl.clone()) .collect(); + // A yarn classic pin attests from the lock only when the lock + // installs the patched package's dependencies with it (#1538). + let excusable_purls: Vec<&str> = excusable.iter().map(String::as_str).collect(); let dependency_gaps = yarn_classic_lock_dependency_gaps(common, params, &plan, &excusable_purls).await; let mut lockfile_attested = Vec::new(); outcome.failed.retain_mut(|f| { - if !excusable(f) { + if f.reason != "package_not_found" || !excusable.contains(&f.purl) { return true; } if let Some(gap) = dependency_gaps.get(&f.purl) { diff --git a/crates/socket-patch-cli/tests/e2e_vex_lockfile/yarn.rs b/crates/socket-patch-cli/tests/e2e_vex_lockfile/yarn.rs index 6f89b206c..87bb1d8ef 100644 --- a/crates/socket-patch-cli/tests/e2e_vex_lockfile/yarn.rs +++ b/crates/socket-patch-cli/tests/e2e_vex_lockfile/yarn.rs @@ -749,15 +749,8 @@ fn hosted_classic_lock_only_checks_the_pinned_dependency_maps() { "beforeHash": compute_git_sha256_from_bytes(pristine.as_bytes()), "afterHash": after_hash, }); - let (rt, server) = serve_patch_views(vec![(UUID, view)]); - let (rt_no_blob, server_no_blob) = serve_patch_views(vec![(UUID, { - let mut v = patch_view(UUID, PURL); - v["files"][MANIFEST] = serde_json::json!({ - "beforeHash": compute_git_sha256_from_bytes(pristine.as_bytes()), - "afterHash": after_hash, - }); - v - })]); + let (rt, server) = serve_patch_views(vec![(UUID, view.clone())]); + let (rt_no_blob, server_no_blob) = serve_patch_views(vec![(UUID, view.clone())]); rt.block_on( Mock::given(method("GET")) .and(path(format!("/patch/blob/{after_hash}"))) diff --git a/crates/socket-patch-core/src/formats/yarn/classic_deps.rs b/crates/socket-patch-core/src/formats/yarn/classic_deps.rs index 0ae7fd482..149691851 100644 --- a/crates/socket-patch-core/src/formats/yarn/classic_deps.rs +++ b/crates/socket-patch-core/src/formats/yarn/classic_deps.rs @@ -160,6 +160,47 @@ pub(crate) fn dep_maps_match(lines: &[String], pkg: &Value) -> bool { with_manifest_dep_maps(lines, pkg) == lines } +/// Whether the sub-maps `lines` carries declare exactly the dependencies +/// `pkg` declares, read back however yarn 1 quoted them (`foo latest` +/// and `foo "latest"` alike): what yarn installs from the block is what +/// the manifest asks for. +pub(crate) fn dep_maps_declare(lines: &[String], pkg: &Value) -> bool { + block_dep_maps(lines) == manifest_dep_maps(pkg) +} + +/// A block's non-empty dependency sub-maps, each sorted by name, in +/// [`DEP_FIELDS`] order (the shape of [`manifest_dep_maps`]). +fn block_dep_maps(lines: &[String]) -> Vec<(&'static str, Vec<(String, String)>)> { + let mut maps: Vec<(&'static str, Vec<(String, String)>)> = Vec::new(); + let mut field: Option<&'static str> = None; + for line in lines.iter().skip(1) { + if let Some(rest) = body_field_line(line) { + field = DEP_FIELDS + .iter() + .copied() + .find(|f| rest.strip_suffix(':') == Some(*f)); + continue; + } + let Some(field) = field else { + continue; + }; + let (name, rest) = yarn_token(line.trim_start()); + let (range, _) = yarn_token(rest.trim_start()); + if name.is_empty() || range.is_empty() { + continue; + } + match maps.iter_mut().find(|(f, _)| *f == field) { + Some((_, deps)) => deps.push((name, range)), + None => maps.push((field, vec![(name, range)])), + } + } + for (_, deps) in &mut maps { + deps.sort_unstable(); + } + maps.sort_by_key(|(f, _)| DEP_FIELDS.iter().position(|d| d == f)); + maps +} + fn is_dep_map_header(rest: &str) -> bool { DEP_FIELDS.iter().any(|f| rest.strip_suffix(':') == Some(f)) } @@ -215,6 +256,33 @@ is-odd@3.0.1:\n version \"3.0.1\"\n dependencies:\n is-number \"^6.0.0\"\n" text.lines().map(str::to_string).collect() } + #[test] + fn dep_maps_declare_reads_yarns_quoting() { + let block = lines( + "foo@1.0.0:\n version \"1.0.0\"\n dependencies:\n bar latest\n \ + \"@s/baz\" \"^1.0.0\"\n optionalDependencies:\n opt \"^2.0.0\"\n", + ); + let pkg = serde_json::json!({ + "dependencies": {"@s/baz": "^1.0.0", "bar": "latest"}, + "optionalDependencies": {"opt": "^2.0.0"}, + }); + assert!(dep_maps_declare(&block, &pkg)); + let bumped = serde_json::json!({ + "dependencies": {"@s/baz": "^2.0.0", "bar": "latest"}, + "optionalDependencies": {"opt": "^2.0.0"}, + }); + assert!(!dep_maps_declare(&block, &bumped)); + let added = serde_json::json!({ + "dependencies": {"@s/baz": "^1.0.0", "bar": "latest", "qux": "1"}, + "optionalDependencies": {"opt": "^2.0.0"}, + }); + assert!(!dep_maps_declare(&block, &added)); + let moved = serde_json::json!({ + "dependencies": {"@s/baz": "^1.0.0", "bar": "latest", "opt": "^2.0.0"}, + }); + assert!(!dep_maps_declare(&block, &moved)); + } + #[test] fn unlocked_descriptors_names_only_the_unresolved_ones() { let blocks = scan_blocks(LOCK); diff --git a/crates/socket-patch-core/src/vex/discover/mod.rs b/crates/socket-patch-core/src/vex/discover/mod.rs index 963049218..894f8ed80 100644 --- a/crates/socket-patch-core/src/vex/discover/mod.rs +++ b/crates/socket-patch-core/src/vex/discover/mod.rs @@ -317,7 +317,8 @@ impl ClassicPinnedBlock { /// maps the patched package's `package.json` (`pkg`) declares, each /// one locked. pub fn installs_dependencies_of(&self, pkg: &serde_json::Value) -> bool { - self.sub_maps_locked && crate::formats::yarn::classic_deps::dep_maps_match(&self.lines, pkg) + self.sub_maps_locked + && crate::formats::yarn::classic_deps::dep_maps_declare(&self.lines, pkg) } } diff --git a/crates/socket-patch-core/src/vex/verify.rs b/crates/socket-patch-core/src/vex/verify.rs index 189ff5518..387342c51 100644 --- a/crates/socket-patch-core/src/vex/verify.rs +++ b/crates/socket-patch-core/src/vex/verify.rs @@ -500,56 +500,111 @@ pub const DEPENDENCY_NOT_INSTALLED: &str = "dependency_not_installed"; /// Whether an npm `record` patches the package's own `package.json`, the /// only way a patch changes the dependencies its package declares. fn touches_npm_manifest(record: &PatchRecord) -> bool { + npm_manifest_after_hash(record).is_some() +} + +/// The patched `package.json`'s afterHash when an npm `record` patches it. +pub fn npm_manifest_after_hash(record: &PatchRecord) -> Option<&str> { record .files - .keys() - .any(|key| key.strip_prefix("package/").unwrap_or(key) == "package.json") + .iter() + .find(|(key, _)| key.strip_prefix("package/").unwrap_or(key) == "package.json") + .map(|(_, info)| info.after_hash.as_str()) } -/// The first `dependencies` entry of the installed npm copy at `pkg_path` -/// that node cannot resolve from it: no `node_modules/` in the copy -/// or in any directory above its real path (node's lookup, which also -/// finds a bundled, hoisted or pnpm/bun store-sibling copy behind a -/// root-linked symlink), or the nearest such copy's version is outside the -/// range the manifest declares (a patch that changed a range the lock never -/// locked installs beside the old dependency, #1538). Optional dependencies -/// may be absent, and a copy outside a `node_modules` tree (a Plug'n'Play -/// install), an unreadable manifest, a name that is not a safe package -/// name, or a range or installed version this reader does not decide is -/// never evidence. +/// The first dependency of the installed npm copy at `pkg_path` that node +/// cannot resolve from it: no `node_modules/` in the copy or in any +/// directory above its real path (node's lookup, which also finds a +/// bundled, hoisted or pnpm/bun store-sibling copy behind a root-linked +/// symlink), or the nearest such copy's version is outside the range the +/// manifest declares (a patch that changed a range the lock never locked +/// installs beside the old dependency, #1538). An optional dependency may +/// be absent, but an installed one is range-checked too. A dependency the +/// project forces itself (npm `overrides`, pnpm `pnpm.overrides`, yarn +/// `resolutions` in the project's `package.json`) is not range-checked: +/// its version is the user's choice. A copy outside a `node_modules` tree +/// (a Plug'n'Play install), an unreadable manifest, a name that is not a +/// safe package name, or a range or installed version this reader does +/// not decide is never evidence. async fn unresolved_npm_dependency(pkg_path: &Path) -> Option { let pkg_path = pkg_path.to_path_buf(); crate::utils::fs::run_blocking(move || { - if !pkg_path + // The project whose `node_modules` holds the copy. + let project: PathBuf = pkg_path .components() - .any(|c| c.as_os_str() == "node_modules") - { + .take_while(|c| c.as_os_str() != "node_modules") + .collect(); + if project == pkg_path { return None; } let text = crate::utils::fs::read_regular_to_string_sync(&pkg_path.join("package.json")).ok()?; let pkg: serde_json::Value = serde_json::from_str(&text).ok()?; let real = std::fs::canonicalize(&pkg_path).ok()?; - let optional = pkg - .get("optionalDependencies") - .and_then(serde_json::Value::as_object); - pkg.get("dependencies")? - .as_object()? - .iter() + let map = |field: &str| pkg.get(field).and_then(serde_json::Value::as_object); + let optional = map("optionalDependencies"); + let forced = project_forced_names(&project); + let required = map("dependencies") + .into_iter() + .flatten() .filter(|(name, _)| optional.is_none_or(|o| !o.contains_key(*name))) - .filter(|(name, _)| crate::vendor::npm_common::is_safe_npm_name(name)) - .find(|(name, spec)| match node_resolve(&real, name) { - None => true, + .map(|entry| (entry, false)); + let optional = optional.into_iter().flatten().map(|entry| (entry, true)); + required + .chain(optional) + .filter(|((name, _), _)| crate::vendor::npm_common::is_safe_npm_name(name)) + .find(|((name, spec), optional)| match node_resolve(&real, name) { + None => !optional, + Some(_) if forced.iter().any(|key| forces(key, name)) => false, Some(copy) => spec .as_str() .and_then(declared_npm_range) .is_some_and(|range| !copy_admitted(©, range)), }) - .map(|(name, _)| name.clone()) + .map(|((name, _), _)| name.clone()) }) .await } +/// The keys of the version overrides the project's own `package.json` +/// declares: npm `overrides` (nested objects included), pnpm +/// `pnpm.overrides` and yarn `resolutions`. +fn project_forced_names(project: &Path) -> Vec { + fn keys(value: &serde_json::Value, out: &mut Vec) { + for (key, nested) in value.as_object().into_iter().flatten() { + out.push(key.clone()); + keys(nested, out); + } + } + let mut out = Vec::new(); + let Some(pkg) = crate::utils::fs::read_regular_to_string_sync(&project.join("package.json")) + .ok() + .and_then(|text| serde_json::from_str::(&text).ok()) + else { + return out; + }; + for value in [ + pkg.get("overrides"), + pkg.get("resolutions"), + pkg.pointer("/pnpm/overrides"), + ] + .into_iter() + .flatten() + { + keys(value, &mut out); + } + out +} + +/// Whether the override key `key` (`name`, `name@range`, `parent>name`, +/// `**/name`, `parent/name`) forces the package `name`. +fn forces(key: &str, name: &str) -> bool { + let tail = key.rsplit('>').next().unwrap_or(key); + std::iter::once(tail) + .chain(tail.match_indices('/').map(|(i, _)| &tail[i + 1..])) + .any(|c| c == name || c.strip_prefix(name).is_some_and(|r| r.starts_with('@'))) +} + /// The copy of the package `name` node resolves from the package directory /// `from`: `/node_modules/` for the nearest of `from` and its /// ancestors that is not itself a `node_modules` directory. @@ -2122,6 +2177,53 @@ mod tests { } put("other/node_modules/d/package.json", r#"{"name":"d"}"#); assert_eq!(unresolved_npm_dependency(&other).await, None); + + // An optional dependency may be absent, but an installed one must + // be in range. + let opt = root.join("opt/node_modules/lp"); + put( + "opt/node_modules/lp/package.json", + r#"{"name":"lp","optionalDependencies":{"fsevents":"^2.0.0"}}"#, + ); + assert_eq!(unresolved_npm_dependency(&opt).await, None); + put( + "opt/node_modules/fsevents/package.json", + r#"{"name":"fsevents","version":"1.2.0"}"#, + ); + assert_eq!( + unresolved_npm_dependency(&opt).await.as_deref(), + Some("fsevents") + ); + + // A version the project forces itself is the user's choice. + let forced = root.join("forced/node_modules/lp"); + put( + "forced/node_modules/lp/package.json", + r#"{"name":"lp","dependencies":{"minimist":"^0.2.0","@s/b":"^2.0.0"}}"#, + ); + put( + "forced/node_modules/minimist/package.json", + r#"{"name":"minimist","version":"1.2.8"}"#, + ); + put( + "forced/node_modules/@s/b/package.json", + r#"{"name":"@s/b","version":"1.0.0"}"#, + ); + assert_eq!( + unresolved_npm_dependency(&forced).await.as_deref(), + Some("minimist") + ); + for project in [ + r#"{"overrides":{"minimist":"1.2.8","@s/b":"1.0.0"}}"#, + r#"{"overrides":{"lp":{"minimist":"1.2.8"},"lp>@s/b":"1.0.0"}}"#, + r#"{"pnpm":{"overrides":{"minimist@<1":"1.2.8","lp>@s/b@^2":"1.0.0"}}}"#, + r#"{"resolutions":{"**/minimist":"1.2.8","lp/@s/b":"1.0.0"}}"#, + ] { + put("forced/package.json", project); + assert_eq!(unresolved_npm_dependency(&forced).await, None, "{project}"); + } + assert!(!forces("minimist-x", "minimist")); + assert!(!forces("lp>minimist-x@1", "minimist")); } fn hosted_ctx(purl: &str, copies: HostedCopies) -> VendorContext { From e8accadac3a8a1de26d28774324a3873e5de57e5 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 21:45:13 +0000 Subject: [PATCH 5/8] Match override keys by their last selector The installed range check skipped any dependency an override key merely mentioned: a nested npm override's parent (which only scopes the overrides under it) counted as forced, and splitting keys on every `/` made `@types/react` force `react`. Keys are now read with the yarn selector parser (scope-aware), after pnpm's `>`, and a nested npm override's parent counts only when its "." entry forces it. Refs #1538 Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-core/src/vex/verify.rs | 39 +++++++++++++++++----- 1 file changed, 31 insertions(+), 8 deletions(-) diff --git a/crates/socket-patch-core/src/vex/verify.rs b/crates/socket-patch-core/src/vex/verify.rs index 387342c51..7f2411ab7 100644 --- a/crates/socket-patch-core/src/vex/verify.rs +++ b/crates/socket-patch-core/src/vex/verify.rs @@ -567,12 +567,19 @@ async fn unresolved_npm_dependency(pkg_path: &Path) -> Option { } /// The keys of the version overrides the project's own `package.json` -/// declares: npm `overrides` (nested objects included), pnpm -/// `pnpm.overrides` and yarn `resolutions`. +/// declares: npm `overrides`, pnpm `pnpm.overrides` and yarn +/// `resolutions`. A nested npm override's parent key only scopes the +/// overrides under it, so it counts only when its `"."` entry forces the +/// parent itself. fn project_forced_names(project: &Path) -> Vec { fn keys(value: &serde_json::Value, out: &mut Vec) { for (key, nested) in value.as_object().into_iter().flatten() { - out.push(key.clone()); + if key == "." { + continue; + } + if nested.is_string() || nested.get(".").is_some() { + out.push(key.clone()); + } keys(nested, out); } } @@ -596,13 +603,12 @@ fn project_forced_names(project: &Path) -> Vec { out } -/// Whether the override key `key` (`name`, `name@range`, `parent>name`, -/// `**/name`, `parent/name`) forces the package `name`. +/// Whether the override key `key` (`name`, `name@range`, pnpm's +/// `parent>name`, yarn's `**/name` / `parent/@scope/name`) forces the +/// package `name`: the key's last selector names it. fn forces(key: &str, name: &str) -> bool { let tail = key.rsplit('>').next().unwrap_or(key); - std::iter::once(tail) - .chain(tail.match_indices('/').map(|(i, _)| &tail[i + 1..])) - .any(|c| c == name || c.strip_prefix(name).is_some_and(|r| r.starts_with('@'))) + crate::formats::yarn::patterns::resolution_selector_target(tail) == Some(name) } /// The copy of the package `name` node resolves from the package directory @@ -2224,6 +2230,23 @@ mod tests { } assert!(!forces("minimist-x", "minimist")); assert!(!forces("lp>minimist-x@1", "minimist")); + assert!(!forces("@types/minimist", "minimist")); + assert!(!forces("lp/@types/minimist@1", "minimist")); + assert!(forces("lp/@s/b@^2", "@s/b")); + // A nested override's parent only scopes it; `"."` forces it. + put( + "forced/package.json", + r#"{"overrides":{"minimist":{"x":"1.0.0"},"@types/minimist":"1"}}"#, + ); + assert_eq!( + unresolved_npm_dependency(&forced).await.as_deref(), + Some("minimist") + ); + put( + "forced/package.json", + r#"{"overrides":{"minimist":{".":"1.2.8"},"@s/b":"1.0.0"}}"#, + ); + assert_eq!(unresolved_npm_dependency(&forced).await, None); } fn hosted_ctx(purl: &str, copies: HostedCopies) -> VendorContext { From a12e919fe9798b5eaf601305460a2c71f45f5f0e Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 21:58:50 +0000 Subject: [PATCH 6/8] Ignore no-op npm overrides in the range check An npm override of "*" or "" (directly or as a nested override's "." entry) keeps the declared range, so it doesn't force the dependency's version and the installed copy is still range-checked. A "$name" reference pins the root's own spec and still counts as forced. Refs #1538 Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-core/src/vex/verify.rs | 32 ++++++++++++++++------ 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/crates/socket-patch-core/src/vex/verify.rs b/crates/socket-patch-core/src/vex/verify.rs index 7f2411ab7..21fde9d0a 100644 --- a/crates/socket-patch-core/src/vex/verify.rs +++ b/crates/socket-patch-core/src/vex/verify.rs @@ -568,16 +568,19 @@ async fn unresolved_npm_dependency(pkg_path: &Path) -> Option { /// The keys of the version overrides the project's own `package.json` /// declares: npm `overrides`, pnpm `pnpm.overrides` and yarn -/// `resolutions`. A nested npm override's parent key only scopes the -/// overrides under it, so it counts only when its `"."` entry forces the -/// parent itself. +/// `resolutions`, each forcing a version (not `*` or empty). A nested npm +/// override's parent key only scopes the overrides under it, so it counts +/// only when its `"."` entry forces the parent itself. fn project_forced_names(project: &Path) -> Vec { fn keys(value: &serde_json::Value, out: &mut Vec) { for (key, nested) in value.as_object().into_iter().flatten() { if key == "." { continue; } - if nested.is_string() || nested.get(".").is_some() { + // `*` and an empty value keep the declared range; `$name` + // pins the root's own spec, which is still the user's choice. + let pins = |v: &serde_json::Value| v.as_str().is_some_and(|v| !matches!(v, "" | "*")); + if pins(nested) || nested.get(".").is_some_and(pins) { out.push(key.clone()); } keys(nested, out); @@ -2242,11 +2245,24 @@ mod tests { unresolved_npm_dependency(&forced).await.as_deref(), Some("minimist") ); - put( - "forced/package.json", + for unforced in [ + r#"{"overrides":{"minimist":{".":"*"},"@s/b":"1.0.0"}}"#, + r#"{"overrides":{"minimist":"","@s/b":"1.0.0"}}"#, + ] { + put("forced/package.json", unforced); + assert_eq!( + unresolved_npm_dependency(&forced).await.as_deref(), + Some("minimist"), + "{unforced}" + ); + } + for forcing in [ r#"{"overrides":{"minimist":{".":"1.2.8"},"@s/b":"1.0.0"}}"#, - ); - assert_eq!(unresolved_npm_dependency(&forced).await, None); + r#"{"overrides":{"minimist":"$minimist","@s/b":"1.0.0"}}"#, + ] { + put("forced/package.json", forcing); + assert_eq!(unresolved_npm_dependency(&forced).await, None, "{forcing}"); + } } fn hosted_ctx(purl: &str, copies: HostedCopies) -> VendorContext { From 354d977e3e3c7b4bfc7592aed8f2b63fe3f1d839 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 22:13:38 +0000 Subject: [PATCH 7/8] Only count overrides that reach the patched copy An override scoped under another package (yarn "mkdirp/minimist", pnpm "mkdirp>minimist", npm {"mkdirp": {"minimist": ...}}) doesn't touch the patched package's own copy of the dependency, so it no longer skips the range check. Overrides are read as (parent, target) pairs and count only when unscoped or scoped under the patched package itself. A whitespace-padded "*" is a no-op like "*". Refs #1538 Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-core/src/vex/verify.rs | 130 +++++++++++++-------- 1 file changed, 84 insertions(+), 46 deletions(-) diff --git a/crates/socket-patch-core/src/vex/verify.rs b/crates/socket-patch-core/src/vex/verify.rs index 21fde9d0a..527b08748 100644 --- a/crates/socket-patch-core/src/vex/verify.rs +++ b/crates/socket-patch-core/src/vex/verify.rs @@ -543,7 +543,8 @@ async fn unresolved_npm_dependency(pkg_path: &Path) -> Option { let real = std::fs::canonicalize(&pkg_path).ok()?; let map = |field: &str| pkg.get(field).and_then(serde_json::Value::as_object); let optional = map("optionalDependencies"); - let forced = project_forced_names(&project); + let overrides = project_overrides(&project); + let owner = pkg.get("name").and_then(serde_json::Value::as_str); let required = map("dependencies") .into_iter() .flatten() @@ -555,7 +556,7 @@ async fn unresolved_npm_dependency(pkg_path: &Path) -> Option { .filter(|((name, _), _)| crate::vendor::npm_common::is_safe_npm_name(name)) .find(|((name, spec), optional)| match node_resolve(&real, name) { None => !optional, - Some(_) if forced.iter().any(|key| forces(key, name)) => false, + Some(_) if forces(&overrides, owner, name) => false, Some(copy) => spec .as_str() .and_then(declared_npm_range) @@ -566,24 +567,35 @@ async fn unresolved_npm_dependency(pkg_path: &Path) -> Option { .await } -/// The keys of the version overrides the project's own `package.json` -/// declares: npm `overrides`, pnpm `pnpm.overrides` and yarn -/// `resolutions`, each forcing a version (not `*` or empty). A nested npm -/// override's parent key only scopes the overrides under it, so it counts -/// only when its `"."` entry forces the parent itself. -fn project_forced_names(project: &Path) -> Vec { - fn keys(value: &serde_json::Value, out: &mut Vec) { +/// The version overrides the project's own `package.json` declares, as +/// `(parent, target)` package names: npm `overrides`, pnpm +/// `pnpm.overrides` and yarn `resolutions`, each forcing a version (not +/// `*` or empty). `parent` is the package an override is scoped under +/// (npm nesting, pnpm `parent>target`, yarn `parent/target`), `None` for +/// one that applies wherever the target installs (`target`, `**/target`). +/// A nested npm override's parent is itself forced only by its `"."`. +fn project_overrides(project: &Path) -> Vec<(Option, String)> { + use crate::formats::yarn::patterns::resolution_selector_target as target_of; + // `*` and an empty value keep the declared range; `$name` pins the + // root's own spec, which is still the user's choice. + fn pins(value: &serde_json::Value) -> bool { + value + .as_str() + .is_some_and(|v| !matches!(v.trim(), "" | "*")) + } + fn npm( + value: &serde_json::Value, + parent: Option<&str>, + out: &mut Vec<(Option, String)>, + ) { for (key, nested) in value.as_object().into_iter().flatten() { - if key == "." { + let Some(name) = (key != ".").then(|| target_of(key)).flatten() else { continue; - } - // `*` and an empty value keep the declared range; `$name` - // pins the root's own spec, which is still the user's choice. - let pins = |v: &serde_json::Value| v.as_str().is_some_and(|v| !matches!(v, "" | "*")); + }; if pins(nested) || nested.get(".").is_some_and(pins) { - out.push(key.clone()); + out.push((parent.map(str::to_string), name.to_string())); } - keys(nested, out); + npm(nested, Some(name), out); } } let mut out = Vec::new(); @@ -593,25 +605,57 @@ fn project_forced_names(project: &Path) -> Vec { else { return out; }; - for value in [ - pkg.get("overrides"), - pkg.get("resolutions"), - pkg.pointer("/pnpm/overrides"), - ] - .into_iter() - .flatten() - { - keys(value, &mut out); + if let Some(overrides) = pkg.get("overrides") { + npm(overrides, None, &mut out); + } + let selectors = |value: Option<&serde_json::Value>| { + value + .and_then(serde_json::Value::as_object) + .into_iter() + .flatten() + .filter(|(_, v)| pins(v)) + .map(|(k, _)| k.clone()) + .collect::>() + }; + for key in selectors(pkg.pointer("/pnpm/overrides")) { + let mut chain = key.rsplit('>'); + if let Some(target) = chain.next().and_then(target_of) { + let parent = chain.next().and_then(target_of).map(str::to_string); + out.push((parent, target.to_string())); + } + } + for key in selectors(pkg.get("resolutions")) { + let mut chain = yarn_selector_segments(&key).into_iter().rev(); + if let Some(target) = chain.next().and_then(target_of) { + let parent = chain.next().and_then(target_of).map(str::to_string); + out.push((parent, target.to_string())); + } + } + out +} + +/// A yarn `resolutions` selector's `/`-separated segments, a scope's own +/// separator kept inside its segment (`a/@s/b@1` → `a`, `@s/b@1`). +fn yarn_selector_segments(selector: &str) -> Vec<&str> { + let mut out = Vec::new(); + let mut start = 0; + for (i, _) in selector.match_indices('/') { + if !selector[start..i].starts_with('@') || selector[start..i].contains('/') { + out.push(&selector[start..i]); + start = i + 1; + } } + out.push(&selector[start..]); out } -/// Whether the override key `key` (`name`, `name@range`, pnpm's -/// `parent>name`, yarn's `**/name` / `parent/@scope/name`) forces the -/// package `name`: the key's last selector names it. -fn forces(key: &str, name: &str) -> bool { - let tail = key.rsplit('>').next().unwrap_or(key); - crate::formats::yarn::patterns::resolution_selector_target(tail) == Some(name) +/// Whether an override of `overrides` forces the dependency `name` of the +/// package `owner`: one that applies everywhere, or one scoped under +/// `owner` itself. +fn forces(overrides: &[(Option, String)], owner: Option<&str>, name: &str) -> bool { + overrides.iter().any(|(parent, target)| { + target == name && parent.as_deref().is_none_or(|p| Some(p) == owner) + }) } /// The copy of the package `name` node resolves from the package directory @@ -2224,29 +2268,23 @@ mod tests { ); for project in [ r#"{"overrides":{"minimist":"1.2.8","@s/b":"1.0.0"}}"#, - r#"{"overrides":{"lp":{"minimist":"1.2.8"},"lp>@s/b":"1.0.0"}}"#, + r#"{"overrides":{"lp":{"minimist":"1.2.8","@s/b":"1.0.0"}}}"#, r#"{"pnpm":{"overrides":{"minimist@<1":"1.2.8","lp>@s/b@^2":"1.0.0"}}}"#, r#"{"resolutions":{"**/minimist":"1.2.8","lp/@s/b":"1.0.0"}}"#, ] { put("forced/package.json", project); assert_eq!(unresolved_npm_dependency(&forced).await, None, "{project}"); } - assert!(!forces("minimist-x", "minimist")); - assert!(!forces("lp>minimist-x@1", "minimist")); - assert!(!forces("@types/minimist", "minimist")); - assert!(!forces("lp/@types/minimist@1", "minimist")); - assert!(forces("lp/@s/b@^2", "@s/b")); - // A nested override's parent only scopes it; `"."` forces it. - put( - "forced/package.json", - r#"{"overrides":{"minimist":{"x":"1.0.0"},"@types/minimist":"1"}}"#, - ); - assert_eq!( - unresolved_npm_dependency(&forced).await.as_deref(), - Some("minimist") - ); + // Not `minimist` of `lp`: another package, another parent's copy, + // a nested override's parent (only `"."` forces it), or a no-op. for unforced in [ + r#"{"overrides":{"minimist-x":"1","@types/minimist":"1","@s/b":"1.0.0"}}"#, + r#"{"pnpm":{"overrides":{"lp>minimist-x@1":"1","mkdirp>minimist":"1","@s/b":"1"}}}"#, + r#"{"resolutions":{"lp/@types/minimist@1":"1","mkdirp/minimist":"1","@s/b":"1"}}"#, + r#"{"overrides":{"mkdirp":{"minimist":"1.2.8"},"@s/b":"1.0.0"}}"#, + r#"{"overrides":{"minimist":{"x":"1.0.0"},"@s/b":"1.0.0"}}"#, r#"{"overrides":{"minimist":{".":"*"},"@s/b":"1.0.0"}}"#, + r#"{"overrides":{"minimist":" * ","@s/b":"1.0.0"}}"#, r#"{"overrides":{"minimist":"","@s/b":"1.0.0"}}"#, ] { put("forced/package.json", unforced); From 2734608eb70e749b623cd5fd0886d93f5c6d39bf Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 22:26:50 +0000 Subject: [PATCH 8/8] Scope parent/**/dep resolutions to the parent A yarn resolution "mkdirp/**/minimist" only reaches minimist under mkdirp, but the range check read the "**" as no parent and treated it as forcing minimist everywhere. The parent is now the nearest segment before the target that isn't "**"; only a leading "**/" applies everywhere. Refs #1538 Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-core/src/vex/verify.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/crates/socket-patch-core/src/vex/verify.rs b/crates/socket-patch-core/src/vex/verify.rs index 527b08748..45b485ba6 100644 --- a/crates/socket-patch-core/src/vex/verify.rs +++ b/crates/socket-patch-core/src/vex/verify.rs @@ -627,8 +627,10 @@ fn project_overrides(project: &Path) -> Vec<(Option, String)> { for key in selectors(pkg.get("resolutions")) { let mut chain = yarn_selector_segments(&key).into_iter().rev(); if let Some(target) = chain.next().and_then(target_of) { - let parent = chain.next().and_then(target_of).map(str::to_string); - out.push((parent, target.to_string())); + // `parent/**/target` is scoped under `parent` like + // `parent/target`; only a leading `**/` applies everywhere. + let parent = chain.find(|s| *s != "**").and_then(target_of); + out.push((parent.map(str::to_string), target.to_string())); } } out @@ -2271,6 +2273,7 @@ mod tests { r#"{"overrides":{"lp":{"minimist":"1.2.8","@s/b":"1.0.0"}}}"#, r#"{"pnpm":{"overrides":{"minimist@<1":"1.2.8","lp>@s/b@^2":"1.0.0"}}}"#, r#"{"resolutions":{"**/minimist":"1.2.8","lp/@s/b":"1.0.0"}}"#, + r#"{"resolutions":{"lp/**/minimist":"1.2.8","@s/b":"1.0.0"}}"#, ] { put("forced/package.json", project); assert_eq!(unresolved_npm_dependency(&forced).await, None, "{project}"); @@ -2281,6 +2284,7 @@ mod tests { r#"{"overrides":{"minimist-x":"1","@types/minimist":"1","@s/b":"1.0.0"}}"#, r#"{"pnpm":{"overrides":{"lp>minimist-x@1":"1","mkdirp>minimist":"1","@s/b":"1"}}}"#, r#"{"resolutions":{"lp/@types/minimist@1":"1","mkdirp/minimist":"1","@s/b":"1"}}"#, + r#"{"resolutions":{"mkdirp/**/minimist":"1","@s/b":"1"}}"#, r#"{"overrides":{"mkdirp":{"minimist":"1.2.8"},"@s/b":"1.0.0"}}"#, r#"{"overrides":{"minimist":{"x":"1.0.0"},"@s/b":"1.0.0"}}"#, r#"{"overrides":{"minimist":{".":"*"},"@s/b":"1.0.0"}}"#,