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

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

@@ -232,6 +232,12 @@ 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 `gitBranchLockfile`
+/// makes pnpm install from a `pnpm-lock.<branch>.yaml` (`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 +274,17 @@ 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 branch locks are in use; the wiring itself is intact, so \
+             re-running `scan` / `vendor` does not change this (turn gitBranchLockfile off and \
+             fold the branch locks in with `pnpm install --merge-git-branch-lockfiles`)",
+        ),
+        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)",
+        ),
     }
 }
 
@@ -418,7 +435,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 +1781,33 @@ 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"
+        );
+    }
+
     /// 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,70 @@ 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.
+#[derive(Debug, Default)]
+pub(crate) struct NpmUserPatches {
+    /// The root manifest's `patchedDependencies` keys.
+    keys: Vec<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),
+            locked: Vec::new(),
+        }
+    }
+
+    /// 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)
+        {
+            return Some(format!("package.json `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

crates/socket-patch-core/src/utils/pnpm_workspace.rs

@@ -775,6 +775,14 @@ pub fn is_git_branch_lock_name(name: &str) -> bool {
 /// on pnpm 11+, `npm_config_git_branch_lockfile` on pnpm 10 and older,
 /// see [`env_setting`]).
 pub async fn git_branch_locks(view: &ProjectView<'_>) -> Option<GitBranchLocks> {
+    git_branch_locks_in(view, true).await
+}
+
+/// [`git_branch_locks`], consulting the environment only with `env`: the
+/// hosted and vendored writers run in the install's environment, while
+/// `vex` discovery answers from the project's files alone (as it does for
+/// [`lockfile_disabled`]).
+pub async fn git_branch_locks_in(view: &ProjectView<'_>, env: bool) -> Option<GitBranchLocks> {
     let mut locks = branch_lock_names(view, "").await;
     let mut members_unlisted = None;
     match workspace_members(view).await {
@@ -808,7 +816,7 @@ pub async fn git_branch_locks(view: &ProjectView<'_>) -> Option<GitBranchLocks>
                 });
         }
     }
-    if setting.is_none() {
+    if setting.is_none() && env {
         setting = view_setting(view, None, None, KEYS.0, KEYS.1).await;
     }
     let setting = git_branch_on(setting)?;