Stop attesting npm-family pins pnpm or a user patch overrides

crates/socket-patch-cli/src/commands/vex_sources.rs

@@ -232,6 +232,13 @@ const NOTE_MAVEN_REACTOR_ROOT: &str = "vex_maven_reactor_root";
 /// classifier copy at another version, which the pin does not reach
 /// (`vex::Unattested`).
 const NOTE_MAVEN_CLASSIFIER_UNPATCHED: &str = "vex_maven_classifier_unpatched";
+/// Omission tag and note: a pnpm lock pins the patch, but a
+/// `pnpm-lock.<branch>.yaml` exists, which pnpm installs from instead when
+/// `gitBranchLockfile` is on; its presence withholds (`vex::Unattested`).
+const NOTE_PNPM_GIT_BRANCH_LOCKFILE: &str = "vex_pnpm_git_branch_lockfile";
+/// Omission tag and note: an npm or Bun pin's package is one the project
+/// patches itself (`patchedDependencies`, `vex::Unattested`).
+const NOTE_NPM_PATCHED_DEPENDENCY: &str = "vex_npm_patched_dependency";
 
 /// The omission tag of an [`Unattested`](socket_patch_core::vex::Unattested)
 /// ref, and the remedy its note ends with.
@@ -268,6 +275,20 @@ fn unattested_note(kind: UnattestedKind) -> (&'static str, &'static str) {
             "not attested while that classifier copy resolves an unpatched version; the \
              wiring itself is intact, so re-running `scan` does not change this",
         ),
+        UnattestedKind::PnpmGitBranchLock => (
+            NOTE_PNPM_GIT_BRANCH_LOCKFILE,
+            "not attested while any branch lock exists, whatever gitBranchLockfile says; the \
+             wiring itself is intact, so re-running `scan` / `vendor` does not change this \
+             (fold the branch locks in with `pnpm install --merge-git-branch-lockfiles`, turn \
+             gitBranchLockfile off and delete the `pnpm-lock.<branch>.yaml` files)",
+        ),
+        UnattestedKind::UserPatchedDependency => (
+            NOTE_NPM_PATCHED_DEPENDENCY,
+            "not attested while the project patches the package itself (fold the Socket fix \
+             into your own patch, or remove the `patchedDependencies` entry and re-run); an \
+             npm pin an earlier hosted run wrote fails `npm ci` with `EPATCHFAILED` until \
+             `socket-patch rollback <uuid>` restores the registry entry",
+        ),
     }
 }
 
@@ -418,7 +439,9 @@ pub(crate) async fn plan(
     // Wired, but a build bypasses the pin (`Unattested`: a Gradle lock
     // above the hosted base resolves the newer upstream release, a pnpm
     // bundled copy, a deno.lock copy, an npm shrinkwrap with no
-    // package-lock.json twin, which npm >= 12 ignores): the ref keeps
+    // package-lock.json twin, which npm >= 12 ignores, a pnpm lock
+    // `gitBranchLockfile` bypasses, a package the project patches itself):
+    // the ref keeps
     // rollback, remove, list and the ledgers' liveness working, and the
     // patch is omitted.
     cands.retain(|c| {
@@ -1762,6 +1785,37 @@ mod tests {
         assert!(live.hosted.contains_key(PURL));
     }
 
+    /// Each npm-family bypass (#556 branch locks, #711 / #367 user patches)
+    /// is omitted under its own tag, distinct from every other kind's.
+    #[test]
+    fn npm_family_unattested_kinds_have_their_own_tags() {
+        let kinds = [
+            UnattestedKind::LockAboveBase,
+            UnattestedKind::BundledCopy,
+            UnattestedKind::DenoLock,
+            UnattestedKind::NpmShrinkwrapOnly,
+            UnattestedKind::MavenReactorRoot,
+            UnattestedKind::MavenClassifierUnpatched,
+            UnattestedKind::PnpmGitBranchLock,
+            UnattestedKind::UserPatchedDependency,
+        ];
+        let tags: std::collections::BTreeSet<&str> =
+            kinds.iter().map(|k| unattested_note(*k).0).collect();
+        assert_eq!(tags.len(), kinds.len(), "{tags:?}");
+        assert_eq!(
+            unattested_note(UnattestedKind::PnpmGitBranchLock).0,
+            "vex_pnpm_git_branch_lockfile"
+        );
+        assert_eq!(
+            unattested_note(UnattestedKind::UserPatchedDependency).0,
+            "vex_npm_patched_dependency"
+        );
+        // Review (#2010): a pre-#711 pin breaks `npm ci` (EPATCHFAILED)
+        // until it is rolled back, as hosted's warning says.
+        let remedy = unattested_note(UnattestedKind::UserPatchedDependency).1;
+        assert!(remedy.contains("socket-patch rollback"), "{remedy}");
+    }
+
     /// Ledger-only records with no wiring anywhere are gated, except the
     /// manifest-owned one, which falls back to agent-mode verification; an
     /// in-run confirmed purl is live by construction.

crates/socket-patch-core/src/patch/redirect/mod.rs

@@ -1302,52 +1302,22 @@ fn rewrite_npm_lock(
     // would still stack the user's diff on the hosted bytes. Each present
     // lock is parsed ONCE, here, and the parse is handed to its rewrite;
     // only a lock that spells `"patched"` at all is walked for the gate.
-    let user_patched = crate::vendor::bun_lock_text::patched_dependency_keys(
-        files.get("package.json").map(String::as_str),
-        None,
-    );
+    let mut user_patches =
+        NpmUserPatches::from_manifest(files.get("package.json").map(String::as_str));
     let parsed: Vec<(&str, Option<Value>)> = present
         .iter()
         .map(|lockfile| (*lockfile, parse_json_text(&files[*lockfile]).ok()))
         .collect();
-    let lock_patched: Vec<(String, String, &str, String)> = parsed
-        .iter()
-        .filter(|(lockfile, _)| files[*lockfile].contains("\"patched\""))
-        .filter_map(|(lockfile, lock)| Some((*lockfile, lock.as_ref()?)))
-        .flat_map(|(lockfile, lock)| {
-            npm_lock_entries(lock)
-                .into_iter()
-                .filter(|e| e.section == NpmLockSection::Packages && e.is_dependency())
-                .filter(|e| e.value.get("patched").is_some_and(|p| !p.is_null()))
-                .filter_map(|e| {
-                    let version = e.node.version?.to_string();
-                    Some((
-                        e.node.name.to_string(),
-                        version,
-                        lockfile,
-                        e.key.to_string(),
-                    ))
-                })
-                .collect::<Vec<_>>()
-        })
-        .collect();
+    for (lockfile, lock) in &parsed {
+        if let (Some(lock), true) = (lock, files[*lockfile].contains("\"patched\"")) {
+            user_patches.add_lock(lockfile, lock);
+        }
+    }
     let npm: Vec<&DepOverride> = npm
         .into_iter()
         .filter(|dep| {
             let fname = full_name(dep);
-            let source = if let Some(key) = crate::vendor::bun_lock_text::patched_dependency_key(
-                &user_patched,
-                &fname,
-                &dep.version,
-                &dep.artifact_url,
-            ) {
-                format!("package.json `patchedDependencies` has `{key}`")
-            } else if let Some((_, _, lockfile, key)) = lock_patched
-                .iter()
-                .find(|(name, version, _, _)| *name == fname && *version == dep.version)
-            {
-                format!("{lockfile} entry `{key}` carries npm's `patched` record")
-            } else {
+            let Some(source) = user_patches.source(&fname, &dep.version, &dep.artifact_url) else {
                 return true;
             };
             // A lock an earlier (pre-#711) hosted run already pinned keeps
@@ -1440,6 +1410,91 @@ pub fn npm_deno_lock_detail(packages: &[String], how: &str) -> String {
     )
 }
 
+/// The packages a project patches itself with npm 12's native `npm patch`
+/// (#711), as the hosted npm rewriter's gate reads them: a root
+/// `package.json` `patchedDependencies` key, or the non-null `patched`
+/// record npm writes on a lock's `packages` dependency entry (lockfileVersion
+/// 4) in any present npm lock. npm applies that diff to every install of the
+/// package, whichever lock names it. `vex` discovery reads the same
+/// evidence, so it never attests a pin hosted mode refuses, and reads Bun's
+/// (`bun patch`, #367: the same manifest key, or the copy `bun.lock`
+/// mirrors, [`NpmUserPatches::add_mirrored_keys`]) through it too.
+#[derive(Debug, Default)]
+pub(crate) struct NpmUserPatches {
+    /// The `patchedDependencies` keys: the root manifest's, then any only a
+    /// lock mirrors.
+    keys: Vec<String>,
+    /// `(key, lockfile)` of each key only a lock mirrors.
+    mirrored: Vec<(String, String)>,
+    /// `(name, version, lockfile, entry key)` of every lock entry carrying
+    /// a `patched` record.
+    locked: Vec<(String, String, String, String)>,
+}
+
+impl NpmUserPatches {
+    /// The root `package.json`'s `patchedDependencies` keys (none when
+    /// `manifest` is absent or unparseable).
+    pub(crate) fn from_manifest(manifest: Option<&str>) -> Self {
+        NpmUserPatches {
+            keys: crate::vendor::bun_lock_text::patched_dependency_keys(manifest, None),
+            ..Self::default()
+        }
+    }
+
+    /// Add the `patchedDependencies` keys the text lock `lockfile`
+    /// (`bun.lock`) mirrors at its top that the manifest does not have.
+    pub(crate) fn add_mirrored_keys(&mut self, lockfile: &str, lock: &str) {
+        for key in crate::vendor::bun_lock_text::patched_dependency_keys(None, Some(lock)) {
+            if !self.keys.contains(&key) {
+                self.mirrored.push((key.clone(), lockfile.to_string()));
+                self.keys.push(key);
+            }
+        }
+    }
+
+    /// Add the `patched` records of the parsed npm lock `lockfile`.
+    pub(crate) fn add_lock(&mut self, lockfile: &str, lock: &Value) {
+        let patched = npm_lock_entries(lock)
+            .into_iter()
+            .filter(|e| e.section == NpmLockSection::Packages && e.is_dependency())
+            .filter(|e| e.value.get("patched").is_some_and(|p| !p.is_null()))
+            .filter_map(|e| {
+                let version = e.node.version?.to_string();
+                Some((
+                    e.node.name.to_string(),
+                    version,
+                    lockfile.to_string(),
+                    e.key.to_string(),
+                ))
+            });
+        self.locked.extend(patched);
+    }
+
+    /// Why `name@version` counts as patched by the project itself, for a
+    /// warning or diagnostic, when it does. `target` is what the wiring
+    /// resolves the package to (the hosted URL, or the vendored tarball
+    /// path): a `patchedDependencies` key naming exactly it is no conflict
+    /// ([`crate::vendor::bun_lock_text::patched_dependency_key`]).
+    pub(crate) fn source(&self, name: &str, version: &str, target: &str) -> Option<String> {
+        if let Some(key) =
+            crate::vendor::bun_lock_text::patched_dependency_key(&self.keys, name, version, target)
+        {
+            let file = self
+                .mirrored
+                .iter()
+                .find(|(k, _)| k == key)
+                .map_or("package.json", |(_, lockfile)| lockfile.as_str());
+            return Some(format!("{file} `patchedDependencies` has `{key}`"));
+        }
+        self.locked
+            .iter()
+            .find(|(n, v, _, _)| n == name && v == version)
+            .map(|(_, _, lockfile, key)| {
+                format!("{lockfile} entry `{key}` carries npm's `patched` record")
+            })
+    }
+}
+
 /// Leave `dep` on its registry entry because the project patches it with
 /// npm 12's native `npm patch` (#711). npm extracts whatever tarball the
 /// lock names and then applies the user's diff, failing the whole install