Never patch a pnpm, yarn or vlt dependency the user patched

crates/socket-patch-cli/src/commands/scan/hosted.rs

@@ -870,6 +870,23 @@ async fn run_redirect_selected_untracked(
         );
     }
 
+    // The engine's user-patch gate (pnpm, yarn, patch-package), which the
+    // in-memory engine applies over its own walk. Here the consent gate
+    // already walked the project and left such packages out unless
+    // `--allow-patch-conflicts` (which lifts this one too), so only what
+    // its walk does not read is added: the Rush locks' (common and every
+    // subspace's) and a root `shrinkwrap.yaml`'s patched entries (the
+    // in-memory engine reads them too). Each such package is a skip event,
+    // as the consent gate reports it.
+    // Its warnings join the run's (as the consent gate's and the in-memory
+    // engine's do), after the record warnings.
+    let mut user_patch_warnings: Vec<socket_patch_core::patch::redirect::RewriteWarning> =
+        Vec::new();
+    if !common.allow_patch_conflicts {
+        user_patch_warnings =
+            engine::withhold_user_patched_by_supplement(view, &mut candidates, &mut skipped).await;
+    }
+
     // A NEW row the lockfiles already pin on the patch server these
     // references name (not a configured one, so the recorded view's
     // discovery could not see it) is ALREADY: decided here, before the lock
@@ -922,7 +939,17 @@ async fn run_redirect_selected_untracked(
 
     // Re-read project patch declarations under the write lock. A patch added
     // during API selection must not be lost in a takeover or lock rewrite.
+    // The supplement's Rush and `shrinkwrap.yaml` locks are re-read too
+    // (the consent gate's walk below doesn't read them): a patch recorded
+    // there while this run waited for the lock withholds its package as
+    // the first read would have.
     if !common.allow_patch_conflicts {
+        if lock.is_some() {
+            let late =
+                engine::withhold_user_patched_by_supplement(view, &mut candidates, &mut skipped)
+                    .await;
+            user_patch_warnings.extend(late);
+        }
         if let Err(error) =
             crate::commands::user_patches::check(common, candidates.iter().map(&sel_purl)).await
         {
@@ -1696,6 +1723,7 @@ async fn run_redirect_selected_untracked(
     // record, package-manager, stale-install, takeover and prune warnings.
     let mut engine_warnings = rewrite.warnings.clone();
     engine_warnings.extend(record_warnings);
+    engine_warnings.extend(user_patch_warnings);
     engine_warnings.extend(done.rush_warnings.iter().cloned());
     engine_warnings.extend(done.pnpm_warnings.iter().cloned());
     engine_warnings.extend(done.npm_warnings.iter().cloned());

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

@@ -1,7 +1,7 @@
 //! The CLI consent gate. --yes, --force, UUID selection and socket.yml do
 //! not grant permission to combine a Socket patch with a project's own patch.
 
-use socket_patch_core::patch::user_patches::UserPatchIndex;
+use socket_patch_core::patch::user_patches::{ReadScope, UserPatchIndex};
 
 use crate::args::GlobalArgs;
 use crate::json_envelope::{Command, Envelope, EnvelopeError, PatchAction, PatchEvent};
@@ -25,11 +25,12 @@ fn any_detectable<'a>(common: &GlobalArgs, mut purls: impl Iterator<Item = &'a s
 }
 
 pub(crate) async fn inspect(common: &GlobalArgs) -> UserPatchIndex {
-    if common.is_global() {
-        // A local checkout does not govern a separately selected global tree.
-        UserPatchIndex::default()
-    } else {
-        UserPatchIndex::discover(&common.cwd).await
+    match common.read_scope() {
+        // A local checkout does not govern a separately selected global
+        // tree, so a global run inspects no project (it would read one
+        // with no repository bound, `ReadScope::Global`).
+        ReadScope::Global => UserPatchIndex::default(),
+        scope @ ReadScope::Workspace => UserPatchIndex::discover_scoped(&common.cwd, scope).await,
     }
 }
 

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

@@ -236,8 +236,9 @@ const NOTE_MAVEN_CLASSIFIER_UNPATCHED: &str = "vex_maven_classifier_unpatched";
 /// `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`).
+/// Omission tag and note: an npm-family pin's package is one the project
+/// patches itself (npm / Bun / pnpm `patchedDependencies`, a yarn `patch:`
+/// descriptor, a patch-package patch; `vex::Unattested`).
 const NOTE_NPM_PATCHED_DEPENDENCY: &str = "vex_npm_patched_dependency";
 /// Omission tag and note: a vendored sbt pin the build's local resolution
 /// evidence does not verify, or under a build-wide blocker
@@ -310,10 +311,12 @@ fn unattested_note(kind: UnattestedKind) -> (&'static str, &'static str) {
         ),
         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",
+            "not attested while the project patches the package itself, even when it was \
+             wired with `--allow-patch-conflicts` (fold the Socket fix into your own patch, or \
+             remove your patch: the `patchedDependencies` entry, the yarn `patch:` descriptor \
+             or the patch-package file, 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",
         ),
         UnattestedKind::SbtResolutionUnverified => (
             NOTE_SBT_RESOLUTION_UNVERIFIED,

crates/socket-patch-cli/tests/e2e_vex_lockfile/yarn_berry.rs

@@ -769,7 +769,10 @@ fn two_uuids_for_one_package_are_a_conflict() {
 /// A user `yarn patch` on top of Socket's hosted patch: yarn keeps the
 /// hosted base entry and adds a `patch:` entry wrapping its locator. The
 /// `patch:` entry is not Socket's (silently skipped); the base entry is,
-/// and the installed tree (base + the user's patch) decides.
+/// but yarn applies the user's diff over the Socket bytes, so the install
+/// never runs the Socket patch alone: the reference is omitted as
+/// `vex_npm_patched_dependency` whatever the user's patch touched, and is
+/// attested again once the user's patch is removed.
 #[test]
 fn user_yarn_patch_on_top_of_hosted() {
     let tmp = tempfile::tempdir().unwrap();
@@ -817,8 +820,13 @@ fn user_yarn_patch_on_top_of_hosted() {
         b"user-patched readme\n",
     );
     let out = run_vex(&binary(), cwd, &VexRun::online(&api));
-    attested(&out, LP, Marker::Redirected, "user patch elsewhere");
-    // The user's patch rewrote the Socket-patched file: not attested.
+    omitted(
+        &out,
+        LP,
+        "vex_npm_patched_dependency",
+        "user patch elsewhere",
+    );
+    // The user's patch rewrote the Socket-patched file: still omitted.
     install_at(
         cwd,
         "node_modules/left-pad",
@@ -827,7 +835,20 @@ fn user_yarn_patch_on_top_of_hosted() {
         b"/* user yarn patch */\nmodule.exports = 1;\n",
     );
     let out = run_vex(&binary(), cwd, &VexRun::online(&api));
-    omitted(&out, LP, "hash_mismatch", "user patch rewrote the file");
+    omitted(
+        &out,
+        LP,
+        "vex_npm_patched_dependency",
+        "user patch rewrote the file",
+    );
+    // The user drops their patch: only the hosted base entry is left, and
+    // the Socket-patched tree is attested.
+    project(cwd, serde_json::json!({ "left-pad": "1.3.0" }), None);
+    put(cwd, "yarn.lock", lp_hosted_lock(&url).as_bytes());
+    install_at(cwd, "node_modules/left-pad", "left-pad", "1.3.0", PATCHED);
+    std::fs::remove_file(cwd.join("node_modules/left-pad/README.md")).unwrap();
+    let out = run_vex(&binary(), cwd, &VexRun::online(&api));
+    attested(&out, LP, Marker::Redirected, "user patch removed");
 }
 
 // ── spoofing ─────────────────────────────────────────────────────────────

crates/socket-patch-cli/tests/hosted_memory_engine.rs

@@ -2192,3 +2192,125 @@ async fn issue_1630_berry_exec_and_git_copies_are_named() {
         }
     }
 }
+
+/// REGRESSION: the in-memory engine (no consent gate in front of it, unlike
+/// the CLI) pinned a package the project patches itself with pnpm
+/// (`patchedDependencies`, in `pnpm-workspace.yaml`, `package.json`'s
+/// `pnpm` field or the lock), yarn (a `patch:` descriptor in
+/// `package.json`, or the locator `yarn.lock` resolves) or patch-package,
+/// so the install stacked the user's diff on the hosted
+/// patch. Such a candidate is now withheld from every rewriter, skipped
+/// `user_patch_conflict` with a warning, and its lock left untouched;
+/// another package's patch withholds nothing.
+#[tokio::test]
+async fn user_patched_npm_packages_are_never_pinned() {
+    let berry_wrapper =
+        "\"left-pad\": \"patch:left-pad@npm%3A^1.3.0#./.yarn/patches/left-pad.patch\"";
+    let cases: Vec<(&str, &str, String, &str)> = vec![
+        (
+            "redirect/npm/pnpm/basic",
+            "pnpm-workspace.yaml",
+            "patchedDependencies:\n  left-pad@1.3.0: patches/left-pad@1.3.0.patch\n".into(),
+            "pnpm-lock.yaml",
+        ),
+        (
+            "redirect/npm/pnpm/basic",
+            "package.json",
+            r#"{"pnpm":{"patchedDependencies":{"left-pad":"patches/left-pad.patch"}}}"#.into(),
+            "pnpm-lock.yaml",
+        ),
+        (
+            "redirect/npm/yarn-berry/basic",
+            "package.json",
+            String::from_utf8(
+                fixture_files(&fixtures_root().join("redirect/npm/yarn-berry/basic/input"))
+                    ["package.json"]
+                    .clone(),
+            )
+            .unwrap()
+            .replacen("\"left-pad\": \"^1.3.0\"", berry_wrapper, 1),
+            "yarn.lock",
+        ),
+        (
+            "redirect/npm/yarn-berry/basic",
+            "yarn.lock",
+            format!(
+                "{}\n\"left-pad@patch:left-pad@npm%3A1.3.0#./.yarn/patches/lp.patch\":\n  \
+                 version: 1.3.0\n  resolution: \"left-pad@patch:left-pad@npm%3A1.3.0#./.yarn/\
+                 patches/lp.patch::version=1.3.0&hash=abc123\"\n  languageName: node\n  \
+                 linkType: hard\n",
+                String::from_utf8(
+                    fixture_files(&fixtures_root().join("redirect/npm/yarn-berry/basic/input"))
+                        ["yarn.lock"]
+                        .clone()
+                )
+                .unwrap()
+            ),
+            "yarn.lock",
+        ),
+        (
+            NPM_FIXTURE,
+            "patches/left-pad+1.3.0.patch",
+            "diff\n".into(),
+            "package-lock.json",
+        ),
+    ];
+    for (fixture, file, text, lock) in cases {
+        let dir = fixtures_root().join(fixture);
+        let server = MockServer::start().await;
+        mount_api(
+            &server,
+            &patches_from_overrides(&dir.join("overrides.json"), None),
+        )
+        .await;
+        let files = fixture_files(&dir.join("input"));
+        let control = run_engine(&server, build_input(&files, &[], options(false))).await;
+        assert_eq!(
+            control.projects[0].redirected.len(),
+            1,
+            "{fixture}: control {:?}",
+            control.projects[0].skipped
+        );
+
+        let mut files = files;
+        files.insert(file.to_string(), text.clone().into_bytes());
+        let output = run_engine(&server, build_input(&files, &[], options(false))).await;
+        let project = &output.projects[0];
+        assert!(project.error.is_none(), "{fixture}: {:?}", project.error);
+        assert!(
+            project.redirected.is_empty(),
+            "{fixture} {file}: {:?}",
+            project.redirected
+        );
+        let skip = project
+            .skipped
+            .iter()
+            .find(|s| s.reason == "user_patch_conflict")
+            .unwrap_or_else(|| panic!("{fixture} {file}: {:?}", project.skipped));
+        assert_eq!(skip.purl, "pkg:npm/left-pad@1.3.0");
+        assert!(
+            skip.detail.as_deref().is_some_and(|d| d.contains(file)),
+            "{fixture}: {:?}",
+            skip.detail
+        );
+        let warnings = project.redirect["warnings"].as_array().unwrap();
+        assert!(
+            warnings.iter().any(|w| w["code"] == "user_patch_conflict"),
+            "{fixture}: {warnings:?}"
+        );
+        assert!(
+            output.changed_files.iter().all(|f| f.path != lock),
+            "{fixture}: {lock} must stay untouched"
+        );
+
+        // Another package's patch withholds nothing.
+        let mut files = fixture_files(&dir.join("input"));
+        files.insert(
+            "pnpm-workspace.yaml".into(),
+            b"patchedDependencies:\n  is-number@7.0.0: patches/is-number.patch\n".to_vec(),
+        );
+        files.insert("patches/is-number+7.0.0.patch".into(), b"diff\n".to_vec());
+        let output = run_engine(&server, build_input(&files, &[], options(false))).await;
+        assert_eq!(output.projects[0].redirected.len(), 1, "{fixture}: other");
+    }
+}