Fix vendor --check unwired cause and remedy (#900)

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

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) {

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 \

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)`

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()),

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)
 }
 
 /// `<socket_dir>/jvm-originals/<sha256>.jar`.

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)),
         }
     }

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<ResolvedElsewhere>,
     /// Refs in `refs` whose wiring a build bypasses ([`Unattested`]).
     pub unattested: Vec<Unattested>,
+    /// Refs dropped because another lock contests them ([`ContestedRef`]).
+    pub contested: Vec<ContestedRef>,
 }
 
 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();