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

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

@@ -887,6 +887,20 @@ async fn run_redirect_selected_untracked(
         &vlt_preflight.withheld_everywhere,
         &mut skipped,
     );
+    // The engine's own user-patch gate (pnpm, yarn, patch-package), which
+    // the in-memory engine applies too. The consent gate above already
+    // left such packages out unless `--allow-patch-conflicts`, which lifts
+    // this one as well; it still covers evidence that gate does not read
+    // (a Rush lock's patched entries). Each such package is a skip event,
+    // as the consent gate reports it.
+    let _ = engine::withhold_user_patched(
+        &view,
+        &std::collections::BTreeMap::new(),
+        &mut candidates,
+        &mut skipped,
+        common.allow_patch_conflicts,
+    )
+    .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

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

@@ -235,8 +235,9 @@ 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`).
+/// 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";
 
 /// The omission tag of an [`Unattested`](socket_patch_core::vex::Unattested)
@@ -282,8 +283,10 @@ 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)",
+            "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)",
         ),
     }
 }

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

@@ -2202,3 +2202,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");
+    }
+}

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

@@ -3354,3 +3354,75 @@ async fn issue_1268_member_lock_still_hosted_keeps_trust_without_warning() {
         "the workspace file is left as it was"
     );
 }
+
+/// A package the project patches itself with pnpm's `patchedDependencies`
+/// is never pinned by default (the consent gate skips it), and only the
+/// explicit `--allow-patch-conflicts` override pins it. pnpm then applies
+/// the user's diff on top of the hosted tarball, so the install never runs
+/// the Socket patch alone: lockfile discovery keeps the pin as a ref (so
+/// `rollback` restores it) but marks it unattested, and `vex` omits it.
+#[tokio::test]
+#[serial]
+async fn hosted_pins_a_pnpm_patched_dependency_only_with_explicit_consent() {
+    use socket_patch_core::vex::discover::{
+        discover_patched_refs_with, DiscoverOptions, UnattestedKind,
+    };
+
+    let server = MockServer::start().await;
+    mock_discovery(&server).await;
+    mock_reference(&server).await;
+
+    let tmp = tempfile::tempdir().unwrap();
+    write_pnpm_project(tmp.path());
+    let workspace = format!(
+        "packages:\n  - '.'\npatchedDependencies:\n  {NAME}@{VERSION}: patches/{NAME}.patch\n"
+    );
+    std::fs::write(tmp.path().join("pnpm-workspace.yaml"), &workspace).unwrap();
+    let pristine = std::fs::read_to_string(tmp.path().join("pnpm-lock.yaml")).unwrap();
+
+    // Default: skipped, nothing written.
+    let code = run(hosted_args(tmp.path(), server.uri())).await;
+    assert_eq!(code, 0, "a user-patch skip alone exits 0");
+    assert_eq!(
+        std::fs::read_to_string(tmp.path().join("pnpm-lock.yaml")).unwrap(),
+        pristine,
+        "the user-patched package is left on its registry entry"
+    );
+
+    // Explicit consent: pinned.
+    let mut args = hosted_args(tmp.path(), server.uri());
+    args.common.allow_patch_conflicts = true;
+    let code = run(args).await;
+    assert_eq!(code, 0, "the consented run succeeds");
+    let lock = std::fs::read_to_string(tmp.path().join("pnpm-lock.yaml")).unwrap();
+    assert!(
+        lock.contains(&format!("tarball: {HOSTED_URL}")),
+        "--allow-patch-conflicts pins the package; got:\n{lock}"
+    );
+
+    // ...but the pin is never attested while pnpm stacks the user's patch.
+    let out = discover_patched_refs_with(
+        tmp.path(),
+        &DiscoverOptions {
+            patch_server_origins: vec!["http://patch.test".to_string()],
+        },
+    )
+    .await;
+    assert!(
+        out.refs.iter().any(|r| r.purl == PURL && r.uuid == UUID),
+        "the pin stays a ref: {:?}",
+        out.refs
+    );
+    let marked: Vec<_> = out
+        .unattested
+        .iter()
+        .filter(|u| u.kind == UnattestedKind::UserPatchedDependency)
+        .map(|u| (u.purl.as_str(), u.uuid.as_str()))
+        .collect();
+    assert_eq!(marked, [(PURL, UUID)], "{:#?}", out.unattested);
+    assert!(
+        out.unattested[0].detail.contains("pnpm-workspace.yaml"),
+        "{}",
+        out.unattested[0].detail
+    );
+}

crates/socket-patch-core/src/hosted/engine.rs

@@ -337,6 +337,105 @@ pub fn withhold_everywhere(
     candidates.retain(|c| !withheld_everywhere.contains_key(&c.dep.patch_uuid));
 }
 
+/// Skip reason of an npm candidate withheld because the project patches
+/// that package itself ([`withhold_user_patched`]): the code the CLI's
+/// consent gate reports for the same packages.
+pub const USER_PATCH_CONFLICT: &str = "user_patch_conflict";
+
+/// The pnpm and yarn locks [`withhold_user_patched`] reads when `files`
+/// does not already hold them.
+const USER_PATCH_LOCKS: [&str; 4] = [
+    "pnpm-lock.yaml",
+    "shrinkwrap.yaml",
+    "yarn.lock",
+    crate::constants::npm_family::RUSH_COMMON_LOCK_REL,
+];
+
+/// Withhold from every rewriter each npm candidate whose package the
+/// project patches itself with pnpm (`patchedDependencies`), yarn (a
+/// `patch:` descriptor) or patch-package, recording each as skipped
+/// ([`USER_PATCH_CONFLICT`]). The package manager applies the user's
+/// diff on top of whatever tarball the lock names, so a hosted pin either
+/// fails the install once the diff no longer applies or installs bytes
+/// that are neither the user's package nor the Socket patch. The
+/// evidence is [`UserPatchIndex::npm_family_in`], which `vex` discovery
+/// reads too, over `files` (the candidate texts already read: a member's
+/// own pnpm lock) and the root pnpm, yarn and Rush locks. npm's and Bun's
+/// own `patchedDependencies` are not read here: their rewriters skip such
+/// a package per resolution (#711, #367). `allow` is the caller's explicit
+/// consent (the CLI's `--allow-patch-conflicts`): nothing is withheld.
+/// Returns one [`USER_PATCH_CONFLICT`] warning per withheld candidate,
+/// with the skip's detail.
+///
+/// [`UserPatchIndex::npm_family_in`]: crate::patch::user_patches::UserPatchIndex::npm_family_in
+pub async fn withhold_user_patched(
+    view: &ProjectView<'_>,
+    files: &BTreeMap<String, String>,
+    candidates: &mut Vec<Candidate>,
+    skipped: &mut Vec<SkippedPatch>,
+    allow: bool,
+) -> Vec<RewriteWarning> {
+    let mut warnings = Vec::new();
+    if allow || !candidates.iter().any(|c| c.dep.ecosystem == "npm") {
+        return warnings;
+    }
+    let mut read: Vec<(String, String)> = Vec::new();
+    for rel in USER_PATCH_LOCKS {
+        if files.contains_key(rel) {
+            continue;
+        }
+        if let Ok(text) = view.read_text(rel).await {
+            read.push((rel.to_string(), text));
+        }
+    }
+    let locks: Vec<(&str, &str)> = files
+        .iter()
+        .chain(read.iter().map(|(rel, text)| (rel, text)))
+        .filter(|(rel, _)| {
+            matches!(
+                rel.rsplit('/').next(),
+                Some("pnpm-lock.yaml" | "shrinkwrap.yaml" | "yarn.lock")
+            )
+        })
+        .map(|(rel, text)| (rel.as_str(), text.as_str()))
+        .collect();
+    let index = crate::patch::user_patches::UserPatchIndex::npm_family_in(view, &locks).await;
+    if index.is_empty() {
+        return warnings;
+    }
+    candidates.retain(|c| {
+        if c.dep.ecosystem != "npm" {
+            return true;
+        }
+        let sources = index.conflicts(&c.purl);
+        if sources.is_empty() {
+            return true;
+        }
+        let detail = user_patched_detail(&c.purl, &sources);
+        warnings.push(warning(USER_PATCH_CONFLICT, detail.clone()));
+        skipped.push(SkippedPatch {
+            detail: Some(detail),
+            ..SkippedPatch::new(&c.purl, &c.dep.patch_uuid, USER_PATCH_CONFLICT)
+        });
+        false
+    });
+    warnings
+}
+
+/// [`withhold_user_patched`]'s skip detail.
+fn user_patched_detail(purl: &str, sources: &[String]) -> String {
+    format!(
+        "{purl}: {} declares a patch the project applies to this package itself (pnpm \
+         `patchedDependencies`, a yarn `patch:` descriptor, or a patch-package patch); the \
+         package manager applies it on top of whatever tarball the lock names, so pinning the \
+         hosted patch would stack the two (or fail every install once the user's diff no \
+         longer applies). It is left unchanged and stays without the Socket patch: fold the \
+         Socket fix into your own patch, or remove the user patch and re-run (the CLI's \
+         explicit `--allow-patch-conflicts` wires it anyway)",
+        sources.join(", ")
+    )
+}
+
 /// The project's candidate files as the rewriters read them.
 #[derive(Debug, Clone, Default)]
 pub struct CandidateFiles {

crates/socket-patch-core/src/hosted/memory/stages.rs

@@ -207,6 +207,12 @@ pub(crate) async fn plan(
     } else {
         engine::read_candidate_files(&view, &unreadable, &candidates).await
     };
+    // A package the project patches itself (pnpm, yarn, patch-package) is
+    // never wired: the in-memory engine has no consent knob.
+    pre_warnings.extend(
+        engine::withhold_user_patched(&view, &read.files, &mut candidates, &mut skipped, false)
+            .await,
+    );
     let wheels = engine::wheel_targets(&candidates, &read.files)
         .into_iter()
         .map(|(dep, sha256)| (dep.artifact_url.clone(), sha256.to_string()))