From 1f6349cbea11952bf6344ce4b82f19b67e49f84b Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 7 Oct 2026 00:23:27 +0000 Subject: [PATCH 1/4] Start fix for #900 Assisted-by: Claude Code:claude-opus-5-5 From d38360df68c1af96292b90d901c7fd71a506a900 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 7 Oct 2026 00:35:51 +0000 Subject: [PATCH 2/4] Name the real cause when vendor --check fails `vendor --check` reported every dead vendored entry as "no lockfile or config references .socket/vendor/... any more; re-run `socket-patch vendor`". In two common cases that was false and the remedy did nothing, so the CI gate stayed red for good: - Another lock (e.g. package-lock.json beside a wired yarn.lock or bun.lock) resolves the same version from the registry. The check now says the wiring is contested, names both locks, and says to delete the lock the project does not install from. - The dependency left the lock (`npm uninstall` or an upgrade). The check now says the dependency was removed and points at `socket-patch scan --mode vendored --prune`, the command that reverts the entry. This matches scan's own hint. Discovery now keeps the refs it drops as contested, so callers can name the contesting lock. `vex`'s vendor_unwired phrase no longer claims nothing wires the artifact when the cause is a contest or a removed dependency. Fixes #900 Assisted-by: Claude Code:claude-opus-5-5 --- .../socket-patch-cli/src/commands/vendor.rs | 55 ++++++++++++- crates/socket-patch-cli/src/commands/vex.rs | 3 +- .../tests/in_process_vendor.rs | 78 +++++++++++++++++++ .../socket-patch-core/src/vex/discover/mod.rs | 63 +++++++++++++++ .../src/vex/discover/testing/golden.rs | 26 ++++++- .../vex-discover-golden/redirect-npm.json | 9 +++ 6 files changed, 228 insertions(+), 6 deletions(-) diff --git a/crates/socket-patch-cli/src/commands/vendor.rs b/crates/socket-patch-cli/src/commands/vendor.rs index 0289bf946..cfa5b7592 100644 --- a/crates/socket-patch-cli/src/commands/vendor.rs +++ b/crates/socket-patch-cli/src/commands/vendor.rs @@ -278,6 +278,56 @@ pub(crate) async fn dispatch_revert_one_opts( } } +/// The `vendor --check` failure for a ledger entry the liveness rule +/// ([`Discovery::vendor_entry_live`]) calls dead, naming WHY so the remedy +/// works: +/// +/// * another lock contests the wiring (`package-lock.json` resolving the +/// same version from the registry beside a wired `yarn.lock`): name both +/// locks; re-vendoring changes nothing; +/// * the dependency left the lock (upgraded or uninstalled): the in-use +/// probe the prune GC reverts by says so and no lock resolves the +/// package any more, so `scan --prune` is the fix, as `scan`'s own +/// `vendor_ledger_entry_unwired` hint says; +/// * otherwise a relock dropped the reference while the package stayed. +async fn unwired_check_failure( + discovery: &socket_patch_core::vex::discover::Discovery, + root: &Path, + key: &str, + entry: &VendorEntry, +) -> String { + let dir = format!(".socket/vendor/{}/{}", entry.ecosystem, entry.uuid); + if let Some(c) = discovery.vendored_contest(&entry.base_purl, &entry.uuid) { + return format!( + "wiring contested: {} wires {dir}, but {} resolves the same version from \ + elsewhere (not a Socket patch), so an install driven by {} gets the unpatched \ + package; delete whichever of the two locks the project does not install from \ + (re-vendoring changes nothing while both resolve it)", + c.file.display(), + c.other.display(), + c.other.display(), + ); + } + // Only the npm-family and Python extractors record every lock entry + // (`resolved_elsewhere`), so only there does "no lock resolves it" + // prove the dependency is gone rather than unreadable. + if matches!(entry.ecosystem.as_str(), "npm" | "pypi") + && !discovery.resolves_package(&entry.base_purl) + && dispatch_in_use_one(entry, root).await == Some(false) + { + return format!( + "dependency removed: no lockfile resolves {} any more (it was upgraded or \ + uninstalled), so nothing installs {dir}; run `socket-patch scan --mode vendored \ + --prune` to revert the vendored entry", + strip_purl_qualifiers(key) + ); + } + format!( + "wiring missing: no lockfile or config references {dir} any more, so a fresh install \ + gets the unpatched package; re-run `socket-patch vendor` to rewire it" + ) +} + /// Is this vendored entry still consumed by its project's lockfile /// dependency graph? `None` = cannot determine — callers must keep the /// entry (fail-safe): ecosystems other than npm, cargo and pypi (whose @@ -1039,10 +1089,7 @@ async fn run_check(args: &VendorArgs) -> i32 { // intact; a fresh install is then unpatched. Same rule as // `vex`'s `vendor_unwired`. if !discovery.vendor_entry_live(root, entry).await { - failure = Some(format!( - "wiring missing: no lockfile or config references .socket/vendor/{}/{} any more, so a fresh install gets the unpatched package; re-run `socket-patch vendor` to rewire it", - entry.ecosystem, entry.uuid - )); + failure = Some(unwired_check_failure(discovery, root, key, entry).await); } } if vendor::jvm::apply::upstream_unverified(entry) { diff --git a/crates/socket-patch-cli/src/commands/vex.rs b/crates/socket-patch-cli/src/commands/vex.rs index 09b37c485..35e9f9a8b 100644 --- a/crates/socket-patch-cli/src/commands/vex.rs +++ b/crates/socket-patch-cli/src/commands/vex.rs @@ -1465,7 +1465,8 @@ fn omission_phrase(reason: &str) -> &'static str { } VENDOR_UNWIRED => { "the vendor ledger records its artifact, but no lockfile or config wires it to this \ - package any more" + package in a way the build is sure to install (the wiring was dropped, another lock \ + resolves the same version from elsewhere, or the dependency was removed)" } REDIRECT_UNWIRED => { "the hosted ledger records it, but no lockfile wires its hosted patch to this \ diff --git a/crates/socket-patch-cli/tests/in_process_vendor.rs b/crates/socket-patch-cli/tests/in_process_vendor.rs index 3c51f2992..a81eea705 100644 --- a/crates/socket-patch-cli/tests/in_process_vendor.rs +++ b/crates/socket-patch-cli/tests/in_process_vendor.rs @@ -4079,6 +4079,84 @@ async fn vendor_check_fails_when_lock_no_longer_wires_artifact() { ); } +/// REGRESSION (#900, `npm uninstall` trigger): once the dependency leaves +/// the lock, `vendor --check` said "no lockfile or config references …, +/// so a fresh install gets the unpatched package; re-run `socket-patch +/// vendor`", but a fresh install gets no package at all and `vendor` is a +/// no-op. It must say the dependency was removed and point at the command +/// that reverts the entry (`scan --prune`, as `scan`'s own hint does). +#[tokio::test] +async fn vendor_check_names_removed_dependency_and_prune_remedy() { + let fx = npm_fixture(); + assert_eq!(vendor_run(vendor_args(fx.root())).await, 0, "vendor"); + + // `npm uninstall left-pad`: the lock no longer has the package at all. + let mut lock = serde_json::to_vec_pretty(&json!({ + "name": "fixture", + "version": "1.0.0", + "lockfileVersion": 3, + "requires": true, + "packages": { "": { "name": "fixture", "version": "1.0.0" } } + })) + .unwrap(); + lock.push(b'\n'); + std::fs::write(fx.lock_path(), &lock).unwrap(); + + let (code, env) = vendor_cli(fx.root(), &["--check"]); + assert_eq!(code, 1, "{env:#}"); + let event = find_event(&env, "failed", Some("vendor_check_failed")); + let reason = event["reason"].as_str().unwrap_or_default(); + assert!(reason.contains("dependency removed"), "{env:#}"); + assert!( + reason.contains("socket-patch scan --mode vendored --prune"), + "{env:#}" + ); + assert!( + !reason.contains("re-run `socket-patch vendor`") + && !reason.contains("gets the unpatched package"), + "no no-op remedy, no false claim: {env:#}" + ); +} + +/// REGRESSION (#900): a second npm-family lock resolving the same version +/// from the registry contests the vendored wiring. `vendor --check` must +/// fail (an install from that lock is unpatched) but name the contesting +/// lock, not claim that no lockfile references the artifact and send the +/// user to `socket-patch vendor`, which changes nothing. +#[tokio::test] +async fn vendor_check_names_contesting_lock() { + let fx = npm_fixture(); + assert_eq!(vendor_run(vendor_args(fx.root())).await, 0, "vendor"); + let (code, env) = vendor_cli(fx.root(), &["--check"]); + assert_eq!(code, 0, "{env:#}"); + + std::fs::write( + fx.root().join("yarn.lock"), + format!( + "# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.\n\ + # yarn lockfile v1\n\n\n\ + left-pad@^1.3.0:\n version \"1.3.0\"\n resolved \"{REG_RESOLVED}\"\n \ + integrity {REG_INTEGRITY}\n" + ), + ) + .unwrap(); + + let (code, env) = vendor_cli(fx.root(), &["--check"]); + assert_eq!(code, 1, "an install from yarn.lock is unpatched: {env:#}"); + let event = find_event(&env, "failed", Some("vendor_check_failed")); + let reason = event["reason"].as_str().unwrap_or_default(); + assert!(reason.contains("wiring contested"), "{env:#}"); + assert!( + reason.contains("package-lock.json") && reason.contains("yarn.lock"), + "names both locks: {env:#}" + ); + assert!( + !reason.contains("no lockfile or config references") + && !reason.contains("re-run `socket-patch vendor`"), + "no false claim, no no-op remedy: {env:#}" + ); +} + /// Manifest-less VEX over the committed state of an in-process npm /// `vendor` (the in-process twin of `e2e_vendor_npm_build`'s tail): the /// committed tarball is the evidence, so the checkout attests `(vendored)` diff --git a/crates/socket-patch-core/src/vex/discover/mod.rs b/crates/socket-patch-core/src/vex/discover/mod.rs index d43715c5d..36b7a3094 100644 --- a/crates/socket-patch-core/src/vex/discover/mod.rs +++ b/crates/socket-patch-core/src/vex/discover/mod.rs @@ -404,6 +404,22 @@ pub struct ResolvedElsewhere { pub file: PathBuf, } +/// A ref another lock contests ([`Discovery::contest_across_locks`]): it +/// was dropped from `refs` and diagnosed [`DIAG_REF_UNATTRIBUTABLE`]. Kept +/// so a ledger reader can name the contesting lock instead of reporting the +/// wiring as gone ([`Discovery::vendored_contest`]). +#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)] +pub struct ContestedRef { + /// Canonical base purl ([`canonical_base_purl`]). + pub purl: String, + pub uuid: String, + pub mode: WiringMode, + /// Root-relative lock that wires the patch. + pub file: PathBuf, + /// Root-relative lock that resolves the same version from elsewhere. + pub other: PathBuf, +} + /// 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 @@ -439,6 +455,8 @@ pub struct Discovery { pub elsewhere: Vec, /// Refs in `refs` whose wiring a build bypasses ([`Unattested`]). pub unattested: Vec, + /// Refs dropped because another lock contests them ([`ContestedRef`]). + pub contested: Vec, } impl Discovery { @@ -614,6 +632,13 @@ impl Discovery { } } for (r, other) in contested { + self.contested.push(ContestedRef { + purl: r.purl.clone(), + uuid: r.uuid.clone(), + mode: r.mode, + file: r.source_file.clone(), + other: other.clone(), + }); let file = r.source_file.to_string_lossy().into_owned(); self.diag( DIAG_REF_UNATTRIBUTABLE, @@ -677,6 +702,28 @@ impl Discovery { }) } + /// The cross-lock contest that killed a VENDORED ledger claim, if any: + /// a ref wiring `purl` to `uuid` that [`Discovery::contest_across_locks`] + /// dropped because another lock resolves the same version from + /// elsewhere. Lets a reader of a dead claim name both locks instead of + /// saying nothing wires the artifact. + pub fn vendored_contest(&self, purl: &str, uuid: &str) -> Option<&ContestedRef> { + let key = canonical_base_purl(purl); + self.contested.iter().find(|c| { + c.uuid == uuid && c.mode == WiringMode::Vendored && same_package(&c.purl, &key) + }) + } + + /// Whether any lock discovery read resolves `purl` (any spelling) at + /// all: wired to a Socket patch, contested, or from elsewhere + /// ([`Discovery::resolved_elsewhere`]). + pub fn resolves_package(&self, purl: &str) -> bool { + let key = canonical_base_purl(purl); + self.refs.iter().any(|r| same_package(&r.purl, &key)) + || self.contested.iter().any(|c| same_package(&c.purl, &key)) + || self.elsewhere.iter().any(|e| same_package(&e.purl, &key)) + } + fn recognize(&mut self, uuid: &str, mode: WiringMode, file: &str) { self.recognized.push(Recognized { uuid: uuid.to_string(), @@ -701,6 +748,8 @@ impl Discovery { self.elsewhere.dedup(); self.unattested.sort(); self.unattested.dedup(); + self.contested.sort(); + self.contested.dedup(); self.refs.sort_by(|a, b| { (&a.source_file, &a.purl, &a.uuid, a.mode).cmp(&( &b.source_file, @@ -3826,7 +3875,21 @@ mod tests { assert!(out.recognizes(uuid, mode), "{name}"); if mode == WiringMode::Hosted { assert_eq!(out.hosted_claim(purl, uuid), Some(false), "{name}"); + assert!(out.vendored_contest(purl, uuid).is_none(), "{name}"); + } else { + // #900: the dead vendored claim names both locks. + assert_eq!( + out.vendored_claim(purl, uuid, &format!(".socket/vendor/x/{uuid}")), + Some(false), + "{name}" + ); + let c = out.vendored_contest(purl, uuid).expect(name); + assert_eq!(c.file, std::path::Path::new(files[0].0), "{name}"); + assert_eq!(c.other, std::path::Path::new(files[1].0), "{name}"); + assert!(out.vendored_contest(purl, UUID_A).is_none(), "{name}"); } + assert!(out.resolves_package(purl), "{name}"); + assert!(!out.resolves_package("pkg:npm/unrelated@1.0.0"), "{name}"); // Without the contesting lock the same wiring is a ref. let alone = Project::new(); 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..22d2e40e7 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, + ContestedRef, Diag, Discovery, PatchedRef, Recognized, ResolvedElsewhere, Unattested, + UnlockedPin, WiringMode, }; /// Set to `1` to (re)write the goldens instead of comparing against them. @@ -120,6 +121,7 @@ fn render(out: &Discovery, root: &Path) -> Value { unlocked_pins, elsewhere, unattested, + contested, } = out; let refs: Vec = refs .iter() @@ -225,6 +227,28 @@ fn render(out: &Discovery, root: &Path) -> Value { .collect::>() .into(); } + if !contested.is_empty() { + rendered["contested"] = contested + .iter() + .map(|c| { + let ContestedRef { + purl, + uuid, + mode: m, + file, + other, + } = c; + json!({ + "purl": purl, + "uuid": uuid, + "mode": mode(*m), + "file": path_str(file), + "other": path_str(other), + }) + }) + .collect::>() + .into(); + } rendered } 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..d769824af 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 @@ -6066,6 +6066,15 @@ "uuid": "11111111-1111-4111-8111-111111111111", "purl": "pkg:npm/left-pad@1.3.0" } + ], + "contested": [ + { + "purl": "pkg:npm/ms@2.1.3", + "uuid": "22222222-2222-4222-8222-222222222222", + "mode": "hosted", + "file": "package-lock.json", + "other": "vlt-lock.json" + } ] }, "redirect/npm/vlt/sibling-package-lock-vlt-installed/input": { From bd5bf653fe241db13993074cb1a31eef72e78d59 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 7 Oct 2026 00:36:40 +0000 Subject: [PATCH 3/4] Document vendor --check cause-specific reasons Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-cli/CLI_CONTRACT.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index dff38f2c8..420830926 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -1666,7 +1666,11 @@ with `vendor_check_ok`; drift emits `failed` with `vendor_check_failed`, a wiring: an entry whose lockfile or config no longer references its `.socket/vendor/` artifact (for example after `pipenv lock`, `uv lock` or `npm install` re-resolved it) fails by the same liveness rule as `vex`'s -`vendor_unwired`. For a package-lock entry, drift also includes a +`vendor_unwired`. The reason names the cause: another lock resolving the same +version from elsewhere (`wiring contested`, naming both locks; delete the one +the project does not install from), or, for npm and PyPI, a dependency no lock +resolves any more (`dependency removed`; `scan --mode vendored --prune` reverts +the entry). For a package-lock entry, drift also includes a `package-lock.json` / `npm-shrinkwrap.json` entry for the vendored `name@version` that `vendor` would rewire but that does not resolve to the vendored artifact (#588); the reason names that entry. Missing ledger entries fail with From c831b630b22960df7c5777ffb9affbf1ba5eb49d Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 5 Oct 2026 18:24:21 +0000 Subject: [PATCH 4/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)), } }