Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
b4224f0
Stop attesting npm-family pins pnpm or a user patch overrides
mikolalysenko Nov 4, 2026
76bb103
Address /code-review findings on npm-family VEX discovery
mikolalysenko Nov 5, 2026
5df67ed
Withhold pnpm lock pins whenever a branch lock exists
mikolalysenko Nov 5, 2026
4d775b1
Never patch a pnpm, yarn or vlt dependency the user patched
mikolalysenko Nov 4, 2026
3f076f6
Read member patches and stop attesting Bun patches on the pin
mikolalysenko Nov 4, 2026
23dd236
Bound user-patch reads to the repository in workspace runs only
mikolalysenko Nov 5, 2026
93709da
Fix /code-review findings on the user-patch walk and selection
mikolalysenko Nov 6, 2026
ac92b08
Withhold pnpm pins when workspace members cannot be listed
claude Nov 6, 2026
ab96d63
Describe the pnpm branch-lock mark as discovery applies it
claude Nov 6, 2026
51da9f9
Merge remote-tracking branch 'origin/fix/npm-discovery-false-attestat…
claude Nov 6, 2026
9517b37
Keep the disk user-patch gate and walk as wide as before
claude Nov 6, 2026
941dc7a
Merge main into fix/npm-discovery-false-attestation
claude Nov 7, 2026
bae37f5
Merge remote-tracking branch 'origin/main' into pr2010
claude Nov 7, 2026
e8b0934
Merge origin/main into fix/npm-discovery-false-attestation
claude Nov 7, 2026
811f337
Merge fix/npm-discovery-false-attestation into fix/user-patched-deps-…
claude Nov 7, 2026
f665796
Merge remote-tracking branch 'origin/main' into fix/npm-discovery-fal…
claude Nov 7, 2026
85e9817
Merge fix/npm-discovery-false-attestation into fix/user-patched-deps-…
claude Nov 7, 2026
366c5e4
Read Rush subspace locks in the disk user-patch supplement
claude Nov 7, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 35 additions & 4 deletions crates/socket-patch-cli/CLI_CONTRACT.md

Large diffs are not rendered by default.

8 changes: 8 additions & 0 deletions crates/socket-patch-cli/src/args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -447,6 +447,14 @@ impl GlobalArgs {
self.global || self.global_prefix.is_some()
}

/// The read scope this run's mode fixes: a global run (`--global` /
/// `--global-prefix`) reads anywhere on the system; a project run never
/// reads a project file outside the git repository root. #2025's
/// `GlobalArgs::read_scope` has the same contract.
pub(crate) fn read_scope(&self) -> socket_patch_core::patch::user_patches::ReadScope {
socket_patch_core::patch::user_patches::ReadScope::of_global(self.is_global())
}

/// Whether `--ecosystems` selects `eco` (every ecosystem when unset or
/// empty). The names are validated at parse time, so this is an exact
/// match.
Expand Down
25 changes: 25 additions & 0 deletions crates/socket-patch-cli/src/commands/scan/hosted.rs
Original file line number Diff line number Diff line change
Expand Up @@ -870,6 +870,31 @@ 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.
if !common.allow_patch_conflicts {
let mut read: Vec<(String, String)> = Vec::new();
for rel in engine::user_patch_supplement_lock_rels(view).await {
if let Ok(text) = view.read_text(&rel).await {
read.push((rel, text));
}
}
if !read.is_empty() {
let locks: Vec<(&str, &str)> = read
.iter()
.map(|(rel, text)| (rel.as_str(), text.as_str()))
.collect();
let index = socket_patch_core::patch::user_patches::UserPatchIndex::from_locks(&locks);
let _ = engine::withhold_user_patched_by(&index, &mut candidates, &mut skipped);
}
}

// 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
Expand Down
13 changes: 7 additions & 6 deletions crates/socket-patch-cli/src/commands/user_patches.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand All @@ -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,
}
}

Expand Down
61 changes: 60 additions & 1 deletion crates/socket-patch-cli/src/commands/vex_sources.rs
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,14 @@ 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-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
/// (`vex::Unattested`, `UnattestedKind::SbtResolutionUnverified`).
Expand Down Expand Up @@ -292,6 +300,24 @@ 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 (or the members that could hold one \
cannot be listed), 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, or make the workspace's `packages` \
listable)",
),
UnattestedKind::UserPatchedDependency => (
NOTE_NPM_PATCHED_DEPENDENCY,
"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,
"not attested while that holds (the detail names the remedy, or that there is \
Expand Down Expand Up @@ -502,7 +528,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.
// A file-scoped mark (a Go replace the workspace keeps from the build)
Expand Down Expand Up @@ -1931,6 +1959,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}");
}

/// A hosted Go ref of `uuid` in `file`, and the Go workspace mark on it.
fn go_ref(purl: &str, uuid: &str, file: &str) -> PatchedRef {
PatchedRef::hosted(
Expand Down
122 changes: 122 additions & 0 deletions crates/socket-patch-cli/tests/hosted_memory_engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
}
Loading
Loading