Skip to content

Commit 678f42e

Browse files
mikolalysenkoclaude
andcommitted
Merge origin/main into fix/cargo-members-liveness-origin
Brings #2018 (never patch a user-patched pnpm/yarn/vlt dependency). Conflict in CLI_CONTRACT.md "Unattested references": kept main's patchedDependencies wording and its new pnpm/Yarn/patch-package paragraph, and this branch's two Cargo cases (count Eight -> Ten). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2 parents f1eb764 + 6d17873 commit 678f42e

19 files changed

Lines changed: 2821 additions & 289 deletions

File tree

‎crates/socket-patch-cli/CLI_CONTRACT.md‎

Lines changed: 39 additions & 5 deletions
Large diffs are not rendered by default.

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -870,6 +870,23 @@ async fn run_redirect_selected_untracked(
870870
);
871871
}
872872

873+
// The engine's user-patch gate (pnpm, yarn, patch-package), which the
874+
// in-memory engine applies over its own walk. Here the consent gate
875+
// already walked the project and left such packages out unless
876+
// `--allow-patch-conflicts` (which lifts this one too), so only what
877+
// its walk does not read is added: the Rush locks' (common and every
878+
// subspace's) and a root `shrinkwrap.yaml`'s patched entries (the
879+
// in-memory engine reads them too). Each such package is a skip event,
880+
// as the consent gate reports it.
881+
// Its warnings join the run's (as the consent gate's and the in-memory
882+
// engine's do), after the record warnings.
883+
let mut user_patch_warnings: Vec<socket_patch_core::patch::redirect::RewriteWarning> =
884+
Vec::new();
885+
if !common.allow_patch_conflicts {
886+
user_patch_warnings =
887+
engine::withhold_user_patched_by_supplement(view, &mut candidates, &mut skipped).await;
888+
}
889+
873890
// A NEW row the lockfiles already pin on the patch server these
874891
// references name (not a configured one, so the recorded view's
875892
// discovery could not see it) is ALREADY: decided here, before the lock
@@ -922,7 +939,17 @@ async fn run_redirect_selected_untracked(
922939

923940
// Re-read project patch declarations under the write lock. A patch added
924941
// during API selection must not be lost in a takeover or lock rewrite.
942+
// The supplement's Rush and `shrinkwrap.yaml` locks are re-read too
943+
// (the consent gate's walk below doesn't read them): a patch recorded
944+
// there while this run waited for the lock withholds its package as
945+
// the first read would have.
925946
if !common.allow_patch_conflicts {
947+
if lock.is_some() {
948+
let late =
949+
engine::withhold_user_patched_by_supplement(view, &mut candidates, &mut skipped)
950+
.await;
951+
user_patch_warnings.extend(late);
952+
}
926953
if let Err(error) =
927954
crate::commands::user_patches::check(common, candidates.iter().map(&sel_purl)).await
928955
{
@@ -1696,6 +1723,7 @@ async fn run_redirect_selected_untracked(
16961723
// record, package-manager, stale-install, takeover and prune warnings.
16971724
let mut engine_warnings = rewrite.warnings.clone();
16981725
engine_warnings.extend(record_warnings);
1726+
engine_warnings.extend(user_patch_warnings);
16991727
engine_warnings.extend(done.rush_warnings.iter().cloned());
17001728
engine_warnings.extend(done.pnpm_warnings.iter().cloned());
17011729
engine_warnings.extend(done.npm_warnings.iter().cloned());

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

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
//! The CLI consent gate. --yes, --force, UUID selection and socket.yml do
22
//! not grant permission to combine a Socket patch with a project's own patch.
33
4-
use socket_patch_core::patch::user_patches::UserPatchIndex;
4+
use socket_patch_core::patch::user_patches::{ReadScope, UserPatchIndex};
55

66
use crate::args::GlobalArgs;
77
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
2525
}
2626

2727
pub(crate) async fn inspect(common: &GlobalArgs) -> UserPatchIndex {
28-
if common.is_global() {
29-
// A local checkout does not govern a separately selected global tree.
30-
UserPatchIndex::default()
31-
} else {
32-
UserPatchIndex::discover(&common.cwd).await
28+
match common.read_scope() {
29+
// A local checkout does not govern a separately selected global
30+
// tree, so a global run inspects no project (it would read one
31+
// with no repository bound, `ReadScope::Global`).
32+
ReadScope::Global => UserPatchIndex::default(),
33+
scope @ ReadScope::Workspace => UserPatchIndex::discover_scoped(&common.cwd, scope).await,
3334
}
3435
}
3536

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

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -247,8 +247,9 @@ const NOTE_CARGO_PATCH_BYPASSED: &str = "vex_cargo_patch_bypassed";
247247
/// `pnpm-lock.<branch>.yaml` exists, which pnpm installs from instead when
248248
/// `gitBranchLockfile` is on; its presence withholds (`vex::Unattested`).
249249
const NOTE_PNPM_GIT_BRANCH_LOCKFILE: &str = "vex_pnpm_git_branch_lockfile";
250-
/// Omission tag and note: an npm or Bun pin's package is one the project
251-
/// patches itself (`patchedDependencies`, `vex::Unattested`).
250+
/// Omission tag and note: an npm-family pin's package is one the project
251+
/// patches itself (npm / Bun / pnpm `patchedDependencies`, a yarn `patch:`
252+
/// descriptor, a patch-package patch; `vex::Unattested`).
252253
const NOTE_NPM_PATCHED_DEPENDENCY: &str = "vex_npm_patched_dependency";
253254
/// Omission tag and note: a vendored sbt pin the build's local resolution
254255
/// evidence does not verify, or under a build-wide blocker
@@ -330,10 +331,12 @@ fn unattested_note(kind: UnattestedKind) -> (&'static str, &'static str) {
330331
),
331332
UnattestedKind::UserPatchedDependency => (
332333
NOTE_NPM_PATCHED_DEPENDENCY,
333-
"not attested while the project patches the package itself (fold the Socket fix \
334-
into your own patch, or remove the `patchedDependencies` entry and re-run); an \
335-
npm pin an earlier hosted run wrote fails `npm ci` with `EPATCHFAILED` until \
336-
`socket-patch rollback <uuid>` restores the registry entry",
334+
"not attested while the project patches the package itself, even when it was \
335+
wired with `--allow-patch-conflicts` (fold the Socket fix into your own patch, or \
336+
remove your patch: the `patchedDependencies` entry, the yarn `patch:` descriptor \
337+
or the patch-package file, and re-run); an npm pin an earlier hosted run wrote \
338+
fails `npm ci` with `EPATCHFAILED` until `socket-patch rollback <uuid>` restores \
339+
the registry entry",
337340
),
338341
UnattestedKind::SbtResolutionUnverified => (
339342
NOTE_SBT_RESOLUTION_UNVERIFIED,

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

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -769,7 +769,10 @@ fn two_uuids_for_one_package_are_a_conflict() {
769769
/// A user `yarn patch` on top of Socket's hosted patch: yarn keeps the
770770
/// hosted base entry and adds a `patch:` entry wrapping its locator. The
771771
/// `patch:` entry is not Socket's (silently skipped); the base entry is,
772-
/// and the installed tree (base + the user's patch) decides.
772+
/// but yarn applies the user's diff over the Socket bytes, so the install
773+
/// never runs the Socket patch alone: the reference is omitted as
774+
/// `vex_npm_patched_dependency` whatever the user's patch touched, and is
775+
/// attested again once the user's patch is removed.
773776
#[test]
774777
fn user_yarn_patch_on_top_of_hosted() {
775778
let tmp = tempfile::tempdir().unwrap();
@@ -817,8 +820,13 @@ fn user_yarn_patch_on_top_of_hosted() {
817820
b"user-patched readme\n",
818821
);
819822
let out = run_vex(&binary(), cwd, &VexRun::online(&api));
820-
attested(&out, LP, Marker::Redirected, "user patch elsewhere");
821-
// The user's patch rewrote the Socket-patched file: not attested.
823+
omitted(
824+
&out,
825+
LP,
826+
"vex_npm_patched_dependency",
827+
"user patch elsewhere",
828+
);
829+
// The user's patch rewrote the Socket-patched file: still omitted.
822830
install_at(
823831
cwd,
824832
"node_modules/left-pad",
@@ -827,7 +835,20 @@ fn user_yarn_patch_on_top_of_hosted() {
827835
b"/* user yarn patch */\nmodule.exports = 1;\n",
828836
);
829837
let out = run_vex(&binary(), cwd, &VexRun::online(&api));
830-
omitted(&out, LP, "hash_mismatch", "user patch rewrote the file");
838+
omitted(
839+
&out,
840+
LP,
841+
"vex_npm_patched_dependency",
842+
"user patch rewrote the file",
843+
);
844+
// The user drops their patch: only the hosted base entry is left, and
845+
// the Socket-patched tree is attested.
846+
project(cwd, serde_json::json!({ "left-pad": "1.3.0" }), None);
847+
put(cwd, "yarn.lock", lp_hosted_lock(&url).as_bytes());
848+
install_at(cwd, "node_modules/left-pad", "left-pad", "1.3.0", PATCHED);
849+
std::fs::remove_file(cwd.join("node_modules/left-pad/README.md")).unwrap();
850+
let out = run_vex(&binary(), cwd, &VexRun::online(&api));
851+
attested(&out, LP, Marker::Redirected, "user patch removed");
831852
}
832853

833854
// ── spoofing ─────────────────────────────────────────────────────────────

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

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2192,3 +2192,125 @@ async fn issue_1630_berry_exec_and_git_copies_are_named() {
21922192
}
21932193
}
21942194
}
2195+
2196+
/// REGRESSION: the in-memory engine (no consent gate in front of it, unlike
2197+
/// the CLI) pinned a package the project patches itself with pnpm
2198+
/// (`patchedDependencies`, in `pnpm-workspace.yaml`, `package.json`'s
2199+
/// `pnpm` field or the lock), yarn (a `patch:` descriptor in
2200+
/// `package.json`, or the locator `yarn.lock` resolves) or patch-package,
2201+
/// so the install stacked the user's diff on the hosted
2202+
/// patch. Such a candidate is now withheld from every rewriter, skipped
2203+
/// `user_patch_conflict` with a warning, and its lock left untouched;
2204+
/// another package's patch withholds nothing.
2205+
#[tokio::test]
2206+
async fn user_patched_npm_packages_are_never_pinned() {
2207+
let berry_wrapper =
2208+
"\"left-pad\": \"patch:left-pad@npm%3A^1.3.0#./.yarn/patches/left-pad.patch\"";
2209+
let cases: Vec<(&str, &str, String, &str)> = vec![
2210+
(
2211+
"redirect/npm/pnpm/basic",
2212+
"pnpm-workspace.yaml",
2213+
"patchedDependencies:\n left-pad@1.3.0: patches/left-pad@1.3.0.patch\n".into(),
2214+
"pnpm-lock.yaml",
2215+
),
2216+
(
2217+
"redirect/npm/pnpm/basic",
2218+
"package.json",
2219+
r#"{"pnpm":{"patchedDependencies":{"left-pad":"patches/left-pad.patch"}}}"#.into(),
2220+
"pnpm-lock.yaml",
2221+
),
2222+
(
2223+
"redirect/npm/yarn-berry/basic",
2224+
"package.json",
2225+
String::from_utf8(
2226+
fixture_files(&fixtures_root().join("redirect/npm/yarn-berry/basic/input"))
2227+
["package.json"]
2228+
.clone(),
2229+
)
2230+
.unwrap()
2231+
.replacen("\"left-pad\": \"^1.3.0\"", berry_wrapper, 1),
2232+
"yarn.lock",
2233+
),
2234+
(
2235+
"redirect/npm/yarn-berry/basic",
2236+
"yarn.lock",
2237+
format!(
2238+
"{}\n\"left-pad@patch:left-pad@npm%3A1.3.0#./.yarn/patches/lp.patch\":\n \
2239+
version: 1.3.0\n resolution: \"left-pad@patch:left-pad@npm%3A1.3.0#./.yarn/\
2240+
patches/lp.patch::version=1.3.0&hash=abc123\"\n languageName: node\n \
2241+
linkType: hard\n",
2242+
String::from_utf8(
2243+
fixture_files(&fixtures_root().join("redirect/npm/yarn-berry/basic/input"))
2244+
["yarn.lock"]
2245+
.clone()
2246+
)
2247+
.unwrap()
2248+
),
2249+
"yarn.lock",
2250+
),
2251+
(
2252+
NPM_FIXTURE,
2253+
"patches/left-pad+1.3.0.patch",
2254+
"diff\n".into(),
2255+
"package-lock.json",
2256+
),
2257+
];
2258+
for (fixture, file, text, lock) in cases {
2259+
let dir = fixtures_root().join(fixture);
2260+
let server = MockServer::start().await;
2261+
mount_api(
2262+
&server,
2263+
&patches_from_overrides(&dir.join("overrides.json"), None),
2264+
)
2265+
.await;
2266+
let files = fixture_files(&dir.join("input"));
2267+
let control = run_engine(&server, build_input(&files, &[], options(false))).await;
2268+
assert_eq!(
2269+
control.projects[0].redirected.len(),
2270+
1,
2271+
"{fixture}: control {:?}",
2272+
control.projects[0].skipped
2273+
);
2274+
2275+
let mut files = files;
2276+
files.insert(file.to_string(), text.clone().into_bytes());
2277+
let output = run_engine(&server, build_input(&files, &[], options(false))).await;
2278+
let project = &output.projects[0];
2279+
assert!(project.error.is_none(), "{fixture}: {:?}", project.error);
2280+
assert!(
2281+
project.redirected.is_empty(),
2282+
"{fixture} {file}: {:?}",
2283+
project.redirected
2284+
);
2285+
let skip = project
2286+
.skipped
2287+
.iter()
2288+
.find(|s| s.reason == "user_patch_conflict")
2289+
.unwrap_or_else(|| panic!("{fixture} {file}: {:?}", project.skipped));
2290+
assert_eq!(skip.purl, "pkg:npm/left-pad@1.3.0");
2291+
assert!(
2292+
skip.detail.as_deref().is_some_and(|d| d.contains(file)),
2293+
"{fixture}: {:?}",
2294+
skip.detail
2295+
);
2296+
let warnings = project.redirect["warnings"].as_array().unwrap();
2297+
assert!(
2298+
warnings.iter().any(|w| w["code"] == "user_patch_conflict"),
2299+
"{fixture}: {warnings:?}"
2300+
);
2301+
assert!(
2302+
output.changed_files.iter().all(|f| f.path != lock),
2303+
"{fixture}: {lock} must stay untouched"
2304+
);
2305+
2306+
// Another package's patch withholds nothing.
2307+
let mut files = fixture_files(&dir.join("input"));
2308+
files.insert(
2309+
"pnpm-workspace.yaml".into(),
2310+
b"patchedDependencies:\n is-number@7.0.0: patches/is-number.patch\n".to_vec(),
2311+
);
2312+
files.insert("patches/is-number+7.0.0.patch".into(), b"diff\n".to_vec());
2313+
let output = run_engine(&server, build_input(&files, &[], options(false))).await;
2314+
assert_eq!(output.projects[0].redirected.len(), 1, "{fixture}: other");
2315+
}
2316+
}

0 commit comments

Comments
 (0)