Fix VEX attesting beside an unpatched same-lock copy (#935, #938, #939)

crates/socket-patch-core/src/vex/discover/mod.rs

@@ -408,6 +408,23 @@ pub struct ResolvedElsewhere {
     pub file: PathBuf,
 }
 
+/// A lock entry that installs its OWN copy of a package — a `file:`
+/// directory or tarball, a user url, git, a registry block no Socket rewrite
+/// reached — beside a Socket wiring of the same `name@version` in the SAME
+/// lock (see [`Discovery::unpatched_copy`]). The package manager installs
+/// that copy too (or instead), so the lock's wiring is never attested.
+#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)]
+pub struct UnpatchedCopy {
+    /// Canonical base purl ([`canonical_base_purl`]).
+    pub purl: String,
+    /// Root-relative lock file.
+    pub file: PathBuf,
+    /// The lock entry's key, as the lock spells it.
+    pub key: String,
+    /// How that entry installs, completing "lock entry `<key>` …".
+    pub how: String,
+}
+
 /// 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
@@ -457,6 +474,9 @@ pub struct Discovery {
     /// non-Socket source — the evidence the cross-lock contest
     /// ([`discover_patched_refs_with`]) weighs against another lock's ref.
     pub elsewhere: Vec<ResolvedElsewhere>,
+    /// Same-lock copies that contest that lock's own refs
+    /// ([`Discovery::unpatched_copy`]).
+    pub unpatched_copies: Vec<UnpatchedCopy>,
     /// Refs in `refs` whose wiring a build bypasses ([`Unattested`]).
     pub unattested: Vec<Unattested>,
     /// Refs dropped because another lock contests them ([`ContestedRef`]).
@@ -598,6 +618,69 @@ impl Discovery {
         }
     }
 
+    /// Record that lock `file`'s entry `key` installs its own copy of `purl`
+    /// (`None` is ignored), `how` saying from where. The cross-lock contest
+    /// only weighs OTHER locks (a lock that wires a package is never its
+    /// own contester there), so this is the one same-lock rule every
+    /// extractor shares (#935, #938, #939): the copy is also
+    /// [`Discovery::resolved_elsewhere`] evidence, and
+    /// [`Discovery::contest_within_locks`] drops every ref of the same
+    /// `name@version` in the same file.
+    pub(crate) fn unpatched_copy(
+        &mut self,
+        file: &str,
+        purl: Option<String>,
+        key: &str,
+        how: &str,
+    ) {
+        let Some(purl) = purl else {
+            return;
+        };
+        self.resolved_elsewhere(file, Some(purl.clone()));
+        // Deduplicated once, in `finalize`: a per-push scan is quadratic
+        // over a lock with thousands of registry blocks.
+        self.unpatched_copies.push(UnpatchedCopy {
+            purl: canonical_base_purl(&purl),
+            file: PathBuf::from(file),
+            key: key.to_string(),
+            how: how.to_string(),
+        });
+    }
+
+    /// Drop every ref whose OWN lock also installs an unpatched copy of the
+    /// same `name@version` ([`Discovery::unpatched_copy`]): the build ships
+    /// that copy whatever the wiring does, so the ref is diagnosed
+    /// ([`DIAG_REF_UNATTRIBUTABLE`], naming the entry) and not emitted. Its
+    /// uuid stays recognized (rule 11).
+    fn contest_within_locks(&mut self) {
+        if self.unpatched_copies.is_empty() {
+            return;
+        }
+        let refs = std::mem::take(&mut self.refs);
+        for r in refs {
+            let copy = self
+                .unpatched_copies
+                .iter()
+                .find(|c| c.purl == r.purl && c.file == r.source_file)
+                .cloned();
+            match copy {
+                Some(c) => {
+                    let file = r.source_file.to_string_lossy().into_owned();
+                    self.diag(
+                        DIAG_REF_UNATTRIBUTABLE,
+                        &file,
+                        format!(
+                            "{file}: {} is wired to Socket patch {} but lock entry `{}` {}; \
+                             that copy stays UNPATCHED and nothing is attested",
+                            r.purl, r.uuid, c.key, c.how
+                        ),
+                    );
+                }
+                None => self.refs.push(r),
+            }
+        }
+    }
+
     /// Drop every ref that ANOTHER lock contests: a lock that resolves the
     /// same package at the same version from a non-Socket source
     /// ([`Discovery::resolved_elsewhere`]) while wiring it to no patch
@@ -750,6 +833,8 @@ impl Discovery {
     fn finalize(&mut self) {
         self.elsewhere.sort();
         self.elsewhere.dedup();
+        self.unpatched_copies.sort();
+        self.unpatched_copies.dedup();
         self.unattested.sort();
         self.unattested.dedup();
         self.contested.sort();
@@ -824,6 +909,7 @@ async fn discover_with_ctx(ctx: DiscoverCtx<'_>) -> Discovery {
     sbt::extract(&ctx, &mut out).await;
     nuget::extract(&ctx, &mut out).await;
     deno::extract(&ctx, &mut out).await;
+    out.contest_within_locks();
     out.contest_across_locks();
     out.recognized.extend(ctx.take_recognized());
     out.finalize();
@@ -967,6 +1053,12 @@ impl<'a> DiscoverCtx<'a> {
         self.view.read_text(rel).await.ok()
     }
 
+    /// Bytes twin of [`DiscoverCtx::read_advisory_text`] (a user's `file:`
+    /// tarball, read only to name the package it holds).
+    pub(crate) async fn read_advisory_bytes(&self, rel: &str) -> Option<Vec<u8>> {
+        self.view.read_bytes(rel).await.ok()
+    }
+
     /// Bytes twin of [`DiscoverCtx::read_text`] (JSON and binary locks). A
     /// binary lock is swept through its lossy UTF-8 view: string pools store
     /// resolutions verbatim, and a stale string an older patch generation
@@ -2120,6 +2212,23 @@ pub(crate) mod testing {
     /// the patch uuid after it.
     pub(crate) const TOKEN: &str = "11111111-2222-4333-8444-555555555555";
 
+    /// A minimal npm tarball (`package/package.json` naming
+    /// `name@version`) — a user's `file:` tarball copy.
+    pub(crate) fn npm_tgz(name: &str, version: &str) -> Vec<u8> {
+        let manifest = format!(r#"{{"name":"{name}","version":"{version}"}}"#);
+        let mut tar = tar::Builder::new(flate2::write::GzEncoder::new(
+            Vec::new(),
+            flate2::Compression::default(),
+        ));
+        let mut header = tar::Header::new_gnu();
+        header.set_size(manifest.len() as u64);
+        header.set_mode(0o644);
+        header.set_cksum();
+        tar.append_data(&mut header, "package/package.json", manifest.as_bytes())
+            .unwrap();
+        tar.into_inner().unwrap().finish().unwrap()
+    }
+
     /// Production artifact-URL shape on Socket's patch server
     /// (`…/patch/<eco>/<name>/<version>/<token>/<uuid>/<leaf>`).
     pub(crate) fn hosted_url(
@@ -2217,6 +2326,8 @@ pub(crate) mod testing {
             let ctx = self.ctx();
             let mut out = Discovery::default();
             extract(&ctx, &mut out).await;
+            // Same-lock copies contest within one extractor's own locks.
+            out.contest_within_locks();
             let swept = ctx.take_recognized();
             assert_recognition_covers_refs(&out, &swept, "the ctx sweep", Some(self.root()));
             out.recognized.extend(swept);

crates/socket-patch-core/src/vex/discover/npm.rs

@@ -572,16 +572,127 @@ async fn extract_pnpm_lock(ctx: &DiscoverCtx<'_>, file: &str, out: &mut Discover
         );
         return;
     }
+    let mut copies: Vec<PnpmFileCopy> = Vec::new();
     for package in lock.packages() {
-        pnpm_entry_ref(ctx, file, package, out);
+        pnpm_entry_ref(ctx, file, package, &mut copies, out);
     }
+    record_pnpm_file_copies(ctx, file, copies, out).await;
+}
+
+/// A pnpm `packages:` entry installed from a user's `file:` directory or
+/// tarball, awaiting [`record_pnpm_file_copies`].
+struct PnpmFileCopy {
+    key: String,
+    /// The package name (the v9 key's, or a legacy entry's `name:` field).
+    name: Option<String>,
+    /// The entry's `version:` field (always there for a tarball).
+    version: Option<String>,
+    /// The `file:` path, relative to the lock's directory.
+    path: String,
+    directory: bool,
+}
+
+/// Record each [`PnpmFileCopy`] as an unpatched copy of its package (#935):
+/// pnpm installs a `file:` directory or tarball from the user's own bytes,
+/// and no override or tarball rewire of the registry entry reaches it. A
+/// directory entry carries no version, so it is read from the directory's
+/// `package.json` (a legacy entry's name too); one whose package cannot be
+/// read is left alone.
+async fn record_pnpm_file_copies(
+    ctx: &DiscoverCtx<'_>,
+    file: &str,
+    copies: Vec<PnpmFileCopy>,
+    out: &mut Discovery,
+) {
+    let lock_dir = std::path::Path::new(file)
+        .parent()
+        .map(|d| d.to_string_lossy().replace('\\', "/"))
+        .unwrap_or_default();
+    for copy in copies {
+        let (mut name, mut version) = (copy.name, copy.version);
+        if name.is_none() || version.is_none() {
+            let manifest: Option<Value> =
+                match crate::utils::cargo_workspace::normalize_rel(&lock_dir, &copy.path) {
+                    Some(rel) if copy.directory => {
+                        let manifest = if rel.is_empty() {
+                            "package.json".to_string()
+                        } else {
+                            format!("{rel}/package.json")
+                        };
+                        ctx.read_advisory_text(&manifest).await.and_then(|t| {
+                            serde_json::from_str(t.trim_start_matches('\u{feff}')).ok()
+                        })
+                    }
+                    Some(rel) => match ctx.read_advisory_bytes(&rel).await {
+                        Some(bytes) => tokio::task::spawn_blocking(move || {
+                            let map =
+                                crate::patch::package::read_archive_bytes_to_map(&bytes).ok()?;
+                            serde_json::from_slice::<Value>(map.get("package.json")?).ok()
+                        })
+                        .await
+                        .ok()
+                        .flatten(),
+                        None => None,
+                    },
+                    None => None,
+                };
+            let field = |k: &str| {
+                manifest
+                    .as_ref()
+                    .and_then(|m| m.get(k))
+                    .and_then(Value::as_str)
+                    .map(str::to_string)
+            };
+            name = name.or_else(|| field("name"));
+            version = version.or_else(|| field("version"));
+        }
+        let (Some(name), Some(version)) = (name, version) else {
+            continue;
+        };
+        let what = if copy.directory {
+            "directory"
+        } else {
+            "tarball"
+        };
+        out.unpatched_copy(
+            file,
+            npm_purl(&name, &version),
+            &copy.key,
+            &format!(
+                "installs it from the user's file: {what} {:?}, which no Socket wiring \
+                 reaches",
+                copy.path
+            ),
+        );
+    }
+}
+
+/// The [`PnpmFileCopy`] of a `file:`-keyed entry, `None` for any other key.
+fn pnpm_file_copy(package: &PnpmPackage<'_>, directory: bool) -> Option<PnpmFileCopy> {
+    let (name, path) = match classify_pnpm_key(package.key) {
+        PnpmKey::V9File { name, path } => (Some(name.to_string()), path),
+        PnpmKey::LegacyFile { path } => (
+            entry_field(&package.entry, "name").map(str::to_string),
+            path,
+        ),
+        PnpmKey::Registry { .. } | PnpmKey::Other => return None,
+    };
+    let path = path.strip_prefix("file:").unwrap_or(path).to_string();
+    Some(PnpmFileCopy {
+        key: package.key.to_string(),
+        name,
+        version: entry_field(&package.entry, "version").map(str::to_string),
+        path,
+        directory,
+    })
 }
 
 /// Classify one `packages:` entry and push its ref, if any.
 fn pnpm_entry_ref(
     ctx: &DiscoverCtx<'_>,
     file: &str,
     package: &PnpmPackage<'_>,
+    copies: &mut Vec<PnpmFileCopy>,
     out: &mut Discovery,
 ) {
     let key = package.key;
@@ -601,6 +712,7 @@ fn pnpm_entry_ref(
     let Some(tarball) = resolution.tarball() else {
         // A plain registry entry (integrity only) or a directory/git dep.
         out.resolved_elsewhere(file, pnpm_registry_key_purl(key));
+        copies.extend(pnpm_file_copy(package, true));
         return;
     };
     let integrity = resolution
@@ -641,8 +753,10 @@ fn pnpm_entry_ref(
             true,
         ));
     } else {
-        // A registry-keyed entry fetching some other tarball.
+        // A registry-keyed entry fetching some other tarball, or a user's
+        // `file:` tarball.
         out.resolved_elsewhere(file, pnpm_registry_key_purl(key));
+        copies.extend(pnpm_file_copy(package, false));
     }
 }
 
@@ -1979,6 +2093,72 @@ mod tests {
         assert!(out.diagnostics.is_empty(), "{:#?}", out.diagnostics);
     }
 
+    /// #935: pnpm installs a `file:` directory or `file:` tarball copy of
+    /// the wired name@version from the user's own bytes, so a hosted pin of
+    /// the registry entry in the SAME lock is not attested: the ref is
+    /// dropped with a diagnostic naming the copy (v9 keys and pnpm 8's
+    /// legacy `file:` keys alike). Controls: the wiring alone is a ref, and
+    /// a `file:` copy of ANOTHER version does not contest it.
+    #[tokio::test]
+    async fn issue_935_same_lock_file_copy_contests_the_pnpm_ref() {
+        let url = hosted_url("npm", "left-pad", "1.3.0", UUID_A, "left-pad-1.3.0.tgz");
+        let wired =
+            format!("  left-pad@1.3.0:\n    resolution: {{integrity: {SRI}, tarball: {url}}}\n\n");
+        let lock = |extra: &str| format!("lockfileVersion: '9.0'\n\npackages:\n\n{wired}{extra}");
+        let dir_v9 = "  left-pad@file:forks/left-pad:\n    \
+                      resolution: {directory: forks/left-pad, type: directory}\n\n";
+        let tgz_v9 = "  left-pad@file:forks/left-pad-1.3.0.tgz:\n    \
+                      resolution: {integrity: sha512-UPSTREAM==, tarball: file:forks/left-pad-1.3.0.tgz}\n    \
+                      version: 1.3.0\n\n";
+        let dir_legacy = "  file:forks/left-pad:\n    \
+                          resolution: {directory: forks/left-pad, type: directory}\n    \
+                          name: left-pad\n    version: 1.3.0\n\n";
+
+        let control = Project::new();
+        control.write("pnpm-lock.yaml", lock(""));
+        assert_refs(
+            &run(&control).await,
+            &[("pkg:npm/left-pad@1.3.0", UUID_A, WiringMode::Hosted)],
+        );
+
+        for (case, extra, fork_version) in [
+            ("v9 file: directory", dir_v9, "1.3.0"),
+            ("v9 file: tarball", tgz_v9, "1.3.0"),
+            ("legacy file: directory", dir_legacy, "1.3.0"),
+        ] {
+            let p = Project::new();
+            p.write("pnpm-lock.yaml", lock(extra));
+            p.write(
+                "forks/left-pad/package.json",
+                format!(r#"{{"name":"left-pad","version":"{fork_version}"}}"#),
+            );
+            p.write("forks/left-pad-1.3.0.tgz", npm_tgz("left-pad", "1.3.0"));
+            let out = run(&p).await;
+            assert!(out.refs.is_empty(), "{case}: {:#?}", out.refs);
+            assert!(
+                out.diagnostics
+                    .iter()
+                    .any(|d| d.code == DIAG_REF_UNATTRIBUTABLE
+                        && d.detail.contains("forks/left-pad")
+                        && d.detail.contains("UNPATCHED")),
+                "{case}: {:#?}",
+                out.diagnostics
+            );
+        }
+
+        // A `file:` directory holding ANOTHER version is not a copy of it.
+        let p = Project::new();
+        p.write("pnpm-lock.yaml", lock(dir_v9));
+        p.write(
+            "forks/left-pad/package.json",
+            r#"{"name":"left-pad","version":"2.0.0"}"#,
+        );
+        assert_refs(
+            &run(&p).await,
+            &[("pkg:npm/left-pad@1.3.0", UUID_A, WiringMode::Hosted)],
+        );
+    }
+
     /// The committed golden (TS backend output — what a depscan PR leaves).
     #[tokio::test]
     async fn pnpm_golden_hosted_fixture() {