From ea09474bd8fabedc19b36baed5c08023778a2021 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 20:22:59 +0000 Subject: [PATCH 1/6] Start fix for #962 Assisted-by: Claude Code:claude-opus-5-5 From fb33dbea22a33d9b18b7cc81ce2e941a24737ba6 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 20:29:54 +0000 Subject: [PATCH 2/6] Refuse berry rollback under a user yarn patch Hosted yarn berry rollback and remove restored the pinned package's registry entry even when the user had run `yarn patch` on it. The user's patch: entry kept naming the Socket tarball URL, so every `yarn install --immutable` failed (YN0028) and a plain install put the Socket tarball back, while rollback reported success. The unwind now refuses such a pin and writes nothing, telling the user to drop their patch: resolution first and re-create it afterwards. Refs #962 Assisted-by: Claude Code:claude-opus-5-5 --- .../tests/in_process_redirect.rs | 91 +++++++++++++++++++ .../src/patch/redirect/upstream/npm.rs | 48 ++++++++++ 2 files changed, 139 insertions(+) diff --git a/crates/socket-patch-cli/tests/in_process_redirect.rs b/crates/socket-patch-cli/tests/in_process_redirect.rs index 36244519f..447ed9e9f 100644 --- a/crates/socket-patch-cli/tests/in_process_redirect.rs +++ b/crates/socket-patch-cli/tests/in_process_redirect.rs @@ -1139,6 +1139,97 @@ async fn yarn_berry_catalog_dependency_is_pinned_and_rolled_back() { ); } +/// #962: the user ran `yarn patch` / `yarn patch-commit -s` over a hosted +/// pin, so a `patch:` lock entry and a `resolutions` descriptor wrap the +/// hosted tarball URL. Restoring the base entry to its `npm:` descriptor +/// would leave that wrapper naming a URL no entry resolves any more: every +/// `yarn install --immutable` then fails (YN0028), and a plain install +/// re-pins the Socket tarball. `rollback` and `remove` must refuse the pin +/// and write nothing. +#[tokio::test] +#[serial] +async fn yarn_berry_rollback_refuses_a_pin_wrapped_by_a_user_yarn_patch() { + for command in ["rollback", "remove"] { + let server = MockServer::start().await; + mock_discovery(&server).await; + let hosted_url = HOSTED_URL.replace("http://patch.test", &server.uri()); + mock_reference_with_berry_url(&server, &hosted_url).await; + mock_view(&server).await; + let tarball = upstream_tarball(); + mock_npm_registry( + &server, + &vlt_hosted_common::sha512_sri(&tarball), + Some(tarball), + ) + .await; + + let tmp = tempfile::tempdir().unwrap(); + write_berry_project(tmp.path()); + let env = run_redirect_subprocess_with( + tmp.path(), + &server.uri(), + &["--patch-server-url", &server.uri()], + ); + assert_eq!(vlt_hosted_common::redirected(&env), 1, "{env:#}"); + + // What `yarn patch-commit -s` writes over the hosted pin: a + // `resolutions` entry routing the tarball descriptor to a `patch:` + // descriptor (the inner one with yarn's `%3A` escapes), the lock + // entry it resolves to, and the workspace dependency through it. + let wrapped = format!( + "patch:{NAME}@{}#~/.yarn/patches/{NAME}-http-7f4bf328bd.patch", + hosted_url.replace(':', "%3A") + ); + let pkg_path = tmp.path().join("package.json"); + let mut pkg: serde_json::Value = + serde_json::from_str(&std::fs::read_to_string(&pkg_path).unwrap()).unwrap(); + pkg["resolutions"][format!("{NAME}@{hosted_url}")] = serde_json::json!(wrapped); + std::fs::write( + &pkg_path, + serde_json::to_string_pretty(&pkg).unwrap() + "\n", + ) + .unwrap(); + let lock_path = tmp.path().join("yarn.lock"); + let lock = std::fs::read_to_string(&lock_path).unwrap().replace( + &format!(" {NAME}: \"npm:^{VERSION}\"\n"), + &format!(" {NAME}: \"{wrapped}\"\n"), + ) + &format!( + "\n\"{NAME}@{wrapped}\":\n version: {VERSION}\n \ + resolution: \"{NAME}@{wrapped}::version={VERSION}&hash=0d6d28\"\n \ + checksum: 10c0/{}\n languageName: node\n linkType: hard\n", + "4".repeat(128) + ); + std::fs::write(&lock_path, &lock).unwrap(); + let pkg_before = std::fs::read_to_string(&pkg_path).unwrap(); + + let (code, env) = hosted_unwind_json(tmp.path(), &server, &server.uri(), &[command, PURL]); + assert_eq!( + code, + Some(1), + "{command} must refuse the wrapped pin: {env:#}" + ); + assert_eq!( + rollback_json::hosted_reverted(&env), + serde_json::json!([]), + "{command}: {env:#}" + ); + assert!( + env.to_string().contains("yarn patch"), + "{command}: the refusal names the user's `yarn patch`: {env:#}" + ); + assert_eq!( + std::fs::read_to_string(&lock_path).unwrap(), + lock, + "{command} leaves yarn.lock untouched" + ); + assert_eq!( + std::fs::read_to_string(&pkg_path).unwrap(), + pkg_before, + "{command} leaves package.json untouched" + ); + } +} + /// #404 upgrade path: a lock pinned by an earlier release carries the old /// `npm:::__archiveUrl=` resolution, which makes yarn's npm fetcher /// send registry auth to the patch host. `rollback` must still recognize and diff --git a/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs b/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs index 2bd805f4f..97db5882f 100644 --- a/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs +++ b/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs @@ -727,6 +727,14 @@ async fn restore_berry( let mut hits: Vec = Vec::new(); // What the lock still declares, read once for the URL-keyed hits. let mut declared: Option> = None; + // Hosted pins a user `yarn patch` wraps (#962): restoring the base + // entry would leave the `patch:` descriptor naming a URL no entry + // resolves, which every `yarn install --immutable` rejects (YN0028). + let wrapped: BTreeSet = [Some(raw), pkg_text.as_deref()] + .into_iter() + .flatten() + .flat_map(|text| yarn_patch_wrapped_uuids(text, ctx)) + .collect(); for (i, block) in blocks.iter().enumerate() { let lines = stanza_lines(block); let Some(resolution) = berry_field(&lines, "resolution").map(str::to_string) else { @@ -766,6 +774,19 @@ async fn restore_berry( ); continue; }; + if wrapped.contains(&uuid) { + result.refuse( + &uuid, + format!( + "{name}@{version}: a `patch:` entry from `yarn patch` still wraps the \ + hosted tarball in {rel}, and restoring the registry entry under it would \ + leave a lock `yarn install --immutable` rejects — remove the `patch:` \ + resolution from {pkg_rel}, run `yarn install`, roll back, then re-run \ + `yarn patch` on the registry version" + ), + ); + continue; + } // An entry keyed by the tarball descriptor itself takes back the // descriptors its `resolutions` selectors route to that URL. let keyed_by_url = matches!( @@ -1041,6 +1062,33 @@ async fn restore_berry( view.write(rel, doc.render(&moved)); } +/// The hosted patch uuids a yarn `patch:` descriptor in `text` (a berry +/// lock or a root `package.json`) wraps. `yarn patch-commit` escapes the +/// inner descriptor (`patch:@http%3A//host/…#`); a lock +/// pinned before 5.0 has the wrapped `npm:…::__archiveUrl=` binding instead. +fn yarn_patch_wrapped_uuids(text: &str, ctx: &Ctx<'_>) -> Vec { + use crate::formats::yarn::patterns::split_pattern; + use crate::utils::purl::percent_decode_purl_component; + + text.match_indices("patch:") + .filter_map(|(at, tag)| { + let rest = &text[at + tag.len()..]; + let end = rest + .find(|c: char| { + matches!(c, '#' | '"' | '\'' | ',' | '\n' | '\r') || c.is_whitespace() + }) + .unwrap_or(rest.len()); + let inner = percent_decode_purl_component(&rest[..end]).into_owned(); + let (_, reference) = split_pattern(&inner)?; + let url = match reference.split_once("::__archiveUrl=") { + Some((_, binding)) => binding.split('&').next().unwrap_or(binding), + None => reference, + }; + ctx.hosted_uuid(url) + }) + .collect() +} + /// Retire the leftover hosted `resolutions` selectors of a yarn berry root /// `package.json` (#1203): `yarn remove` / `yarn up` deleted the lock entry /// a hosted pin keyed by its tarball URL but left its selector, which now From 54be1923226e64ad7f89a2c7b5bbfcdfff922547 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 20:39:21 +0000 Subject: [PATCH 3/6] Refuse vendored berry revert under yarn patch A vendored yarn berry revert over a package the user had `yarn patch`ed rewrote yarn.lock and package.json before refusing to delete the artifact, leaving a lock `yarn install --immutable` rejects. It now refuses before writing either file, on a dry run too, with the same remedy as the hosted rollback. The patch: scanner both unwinds use lives in formats::yarn::patterns. Fixes #962 Assisted-by: Claude Code:claude-opus-5-5 --- .../src/formats/yarn/patterns.rs | 41 +++++++ .../src/patch/redirect/upstream/npm.rs | 24 ++-- .../src/vendor/yarn_berry_lock.rs | 106 ++++++++++++++++++ 3 files changed, 155 insertions(+), 16 deletions(-) diff --git a/crates/socket-patch-core/src/formats/yarn/patterns.rs b/crates/socket-patch-core/src/formats/yarn/patterns.rs index 85a9ef3b4..ee2565ba0 100644 --- a/crates/socket-patch-core/src/formats/yarn/patterns.rs +++ b/crates/socket-patch-core/src/formats/yarn/patterns.rs @@ -114,6 +114,31 @@ pub(crate) fn classic_key_real_name(patterns: &[String]) -> Option<&str> { names.all(|n| n == Some(first)).then_some(first) } +/// The descriptors every berry `patch:` protocol wraps in `text` (a lock or +/// a `package.json`), percent-decoded once: `patch:left-pad@http%3A//h/x.tgz +/// #~/.yarn/patches/x.patch` wraps `left-pad@http://h/x.tgz`. The patch +/// path after `#` and any `::version=…` binding are dropped. +pub(crate) fn patch_wrapped_descriptors(text: &str) -> Vec { + text.match_indices("patch:") + // The protocol opens a range (`name@patch:`) or a quoted value, + // never the middle of a word: `x.patch::version=…` is no wrapper. + .filter(|&(at, _)| { + text[..at] + .chars() + .next_back() + .is_none_or(|c| matches!(c, '@' | '"' | '\'') || c.is_whitespace()) + }) + .map(|(at, tag)| { + let rest = &text[at + tag.len()..]; + let end = rest + .find(|c: char| matches!(c, '#' | '"' | '\'' | ',') || c.is_whitespace()) + .unwrap_or(rest.len()); + crate::utils::purl::percent_decode_purl_component(&rest[..end]).into_owned() + }) + .filter(|inner| !inner.is_empty()) + .collect() +} + /// A classic `resolved` value split at its first `#`: the url before it, /// and the fragment as a lowercase sha1 when it is 40 hex digits (either /// case) — the legacy tarball verifier yarn v1 enforces when no @@ -195,6 +220,22 @@ pub(crate) fn berry_npm_alias_target(range: &str) -> Option<&str> { mod tests { use super::*; + /// #962: the descriptor a `patch:` wraps, as `yarn patch-commit` + /// spells it in a lock key, a `resolution:` and a `package.json` value. + #[test] + fn patch_wrapped_descriptors_decode_what_yarn_patch_wraps() { + let lock = "\"left-pad@patch:left-pad@http%3A//h%3A1/x.tgz#~/.yarn/patches/a.patch\":\n \ + resolution: \"left-pad@patch:left-pad@npm%3A1.3.0#~/.yarn/patches/a.patch\ + ::version=1.3.0&hash=0d6d28\"\n"; + assert_eq!( + patch_wrapped_descriptors(lock), + vec!["left-pad@http://h:1/x.tgz", "left-pad@npm:1.3.0"] + ); + let pkg = r#"{ "resolutions": { "a@npm:1": "patch:a@file%3A./v/a.tgz#./p.patch" } }"#; + assert_eq!(patch_wrapped_descriptors(pkg), vec!["a@file:./v/a.tgz"]); + assert!(patch_wrapped_descriptors("\"left-pad@npm:1.3.0\":\n").is_empty()); + } + /// #1271: the classic grammar keeps an empty range (`left-pad@` from /// `"left-pad": ""`); the strict berry grammar still rejects it. #[test] diff --git a/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs b/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs index 97db5882f..94b244beb 100644 --- a/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs +++ b/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs @@ -1063,23 +1063,15 @@ async fn restore_berry( } /// The hosted patch uuids a yarn `patch:` descriptor in `text` (a berry -/// lock or a root `package.json`) wraps. `yarn patch-commit` escapes the -/// inner descriptor (`patch:@http%3A//host/…#`); a lock -/// pinned before 5.0 has the wrapped `npm:…::__archiveUrl=` binding instead. +/// lock or a root `package.json`) wraps: a tarball-URL pin, or the +/// `npm:…::__archiveUrl=` binding of a lock pinned before 5.0. fn yarn_patch_wrapped_uuids(text: &str, ctx: &Ctx<'_>) -> Vec { - use crate::formats::yarn::patterns::split_pattern; - use crate::utils::purl::percent_decode_purl_component; - - text.match_indices("patch:") - .filter_map(|(at, tag)| { - let rest = &text[at + tag.len()..]; - let end = rest - .find(|c: char| { - matches!(c, '#' | '"' | '\'' | ',' | '\n' | '\r') || c.is_whitespace() - }) - .unwrap_or(rest.len()); - let inner = percent_decode_purl_component(&rest[..end]).into_owned(); - let (_, reference) = split_pattern(&inner)?; + use crate::formats::yarn::patterns::{patch_wrapped_descriptors, split_pattern}; + + patch_wrapped_descriptors(text) + .iter() + .filter_map(|inner| { + let (_, reference) = split_pattern(inner)?; let url = match reference.split_once("::__archiveUrl=") { Some((_, binding)) => binding.split('&').next().unwrap_or(binding), None => reference, diff --git a/crates/socket-patch-core/src/vendor/yarn_berry_lock.rs b/crates/socket-patch-core/src/vendor/yarn_berry_lock.rs index 2d8633693..354b8156f 100644 --- a/crates/socket-patch-core/src/vendor/yarn_berry_lock.rs +++ b/crates/socket-patch-core/src/vendor/yarn_berry_lock.rs @@ -841,6 +841,28 @@ pub async fn revert_yarn_berry_opts( } } } + // A user `yarn patch` over the vendored tarball (#962): restoring the + // recorded entry under its `patch:` wrapper leaves a lock every + // `yarn install --immutable` rejects, so refuse before either file is + // written (and before the dry-run return, so a preview agrees). + if let Some(wired) = user_patch_wrapping(project_root, &entry.uuid).await { + let detail = format!( + "refusing to revert {uuid_dir_rel}: a `patch:` entry from `yarn patch` in {wired} \ + still wraps the vendored tarball, and restoring the registry entry under it would \ + leave a lock `yarn install --immutable` rejects — remove the `patch:` resolution \ + from {PACKAGE_JSON}, run `yarn install`, re-run `vendor --revert`, then re-run \ + `yarn patch` on the registry version" + ); + return RevertOutcome { + success: false, + warnings: vec![VendorWarning::new( + "vendor_lock_still_wired_revert_blocked", + detail.clone(), + )], + error: Some(detail), + kept_artifact: false, + }; + } if dry_run { return RevertOutcome::ok(); } @@ -1102,6 +1124,26 @@ pub async fn revert_yarn_berry_opts( // ───────────────────────────── revert internals ───────────────────────────── +/// The first wired file (yarn.lock, then package.json) in which a user +/// `patch:` descriptor wraps the vendored artifact of `uuid` (#962). An +/// unreadable or missing file wraps nothing here: the wired revert's own +/// reads report it. +async fn user_patch_wrapping(project_root: &Path, uuid: &str) -> Option<&'static str> { + let needle = uuid.to_ascii_lowercase(); + for wired in [YARN_LOCK, PACKAGE_JSON] { + let Ok(text) = read_regular_to_string(&project_root.join(wired)).await else { + continue; + }; + if crate::formats::yarn::patterns::patch_wrapped_descriptors(&text) + .iter() + .any(|inner| inner.to_ascii_lowercase().contains(&needle)) + { + return Some(wired); + } + } + None +} + /// Put back an entry vendoring pruned (see [`KIND_LOCK_ENTRY_PRUNED`]): /// the rewritten shared entry returns to its original lines, a removed one /// is inserted where yarn sorts it. Already restored is silent; anything @@ -3944,6 +3986,70 @@ __metadata: ); } + /// #962: the user ran `yarn patch` / `yarn patch-commit -s` over the + /// vendored package, so a `patch:` lock entry and `resolutions` value + /// wrap the vendored tarball. Restoring the recorded entry would leave + /// that wrapper naming a descriptor no entry resolves: every + /// `yarn install --immutable` rejects the lock (YN0028). The revert must + /// refuse BEFORE writing either file, on a dry run too. + #[tokio::test] + async fn revert_refuses_before_writing_when_a_user_yarn_patch_wraps_the_artifact() { + let fx = fixture().await; + let (_, entry, _) = expect_done(fx.vendor(false).await); + let entry = entry.unwrap(); + + let text = tokio::fs::read_to_string(fx.lock_path()).await.unwrap(); + let resolution = text + .lines() + .find_map(|l| l.strip_prefix(" resolution: \"left-pad@")) + .and_then(|r| r.strip_suffix('"')) + .expect("the vendored lock must carry our resolution line") + .to_string(); + let wrapped = format!( + "patch:left-pad@{}#~/.yarn/patches/left-pad-file-0d6d28.patch", + resolution.replace(':', "%3A") + ); + let lock = format!( + "{text}\n\"left-pad@{wrapped}\":\n version: 1.3.0\n \ + resolution: \"left-pad@{wrapped}::version=1.3.0&hash=0d6d28\"\n \ + languageName: node\n linkType: hard\n" + ); + tokio::fs::write(fx.lock_path(), &lock).await.unwrap(); + let pkg_text = tokio::fs::read_to_string(fx.pkg_path()).await.unwrap(); + let mut pkg: Value = serde_json::from_str(&pkg_text).unwrap(); + pkg["resolutions"][format!("left-pad@{resolution}")] = Value::String(wrapped); + let pkg_text = serde_json::to_string_pretty(&pkg).unwrap() + "\n"; + tokio::fs::write(fx.pkg_path(), &pkg_text).await.unwrap(); + + for dry_run in [true, false] { + let outcome = revert_yarn_berry(&entry, fx.root(), dry_run).await; + assert!( + !outcome.success, + "dry_run={dry_run}: must refuse under a user `yarn patch`: {:?}", + outcome.warnings + ); + assert!( + outcome.warnings.iter().any(|w| { + w.code == "vendor_lock_still_wired_revert_blocked" + && w.detail.contains("yarn patch") + }), + "dry_run={dry_run}: {:?}", + outcome.warnings + ); + assert_eq!( + tokio::fs::read_to_string(fx.lock_path()).await.unwrap(), + lock, + "dry_run={dry_run}: yarn.lock untouched" + ); + assert_eq!( + tokio::fs::read_to_string(fx.pkg_path()).await.unwrap(), + pkg_text, + "dry_run={dry_run}: package.json untouched" + ); + assert!(fx.tgz_path().exists(), "the artifact must survive"); + } + } + /// The user re-keys our resolutions entry (`"left-pad"` → /// `"left-pad@npm:1.3.0"`) without changing its `file:` value. The /// recorded key then reads as already-converged (Added record, key From 3d30d589f358c5995d116baab8410275c21a6556 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 20:41:24 +0000 Subject: [PATCH 4/6] Document the berry unwind refusal under yarn patch Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-cli/CLI_CONTRACT.md | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index f210b750e..9e96d7919 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -187,7 +187,14 @@ A missing referenced diff does not remove that protection. reversal (`rollback`, `remove`, `vendor --revert`) keep their existing behavior. `repair` may restore Socket-owned artifacts; any re-vendoring uses the same consent gate. Existing Socket pins are not automatically unwound when a user - adds a patch: inspect them and explicitly roll back if necessary. + adds a patch: inspect them and explicitly roll back if necessary. One + exception: a Yarn Berry hosted pin or vendored tarball that a user `patch:` + descriptor wraps (`yarn patch` run over the Socket pin) cannot be unwound + under it, since the wrapper would name a descriptor no lock entry resolves. + `rollback`, `remove` and `vendor --revert` refuse that package before + writing (hosted: a refused pin, exit 1; vendored: + `vendor_lock_still_wired_revert_blocked`, dry runs included) and ask for the + `patch:` resolution to be dropped first. Detection covers root and declared npm workspace manifests, ancestor workspace settings, and the following evidence: From 025b4ca93cbcd7d07c93e4a78cc5d0fa37a18901 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 20:43:56 +0000 Subject: [PATCH 5/6] Match yarn 4's real patch-commit output in tests yarn patch-commit -s rewrites the dependency itself, not resolutions; the remedies now say to put the registry range back in its place. Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-cli/CLI_CONTRACT.md | 2 +- .../tests/in_process_redirect.rs | 32 +++++++++++-------- .../src/patch/redirect/upstream/npm.rs | 6 ++-- .../src/vendor/yarn_berry_lock.rs | 11 ++++--- 4 files changed, 29 insertions(+), 22 deletions(-) diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index 9e96d7919..aefb4b8f2 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -194,7 +194,7 @@ A missing referenced diff does not remove that protection. `rollback`, `remove` and `vendor --revert` refuse that package before writing (hosted: a refused pin, exit 1; vendored: `vendor_lock_still_wired_revert_blocked`, dry runs included) and ask for the - `patch:` resolution to be dropped first. + `patch:` descriptor to be replaced by the registry range first. Detection covers root and declared npm workspace manifests, ancestor workspace settings, and the following evidence: diff --git a/crates/socket-patch-cli/tests/in_process_redirect.rs b/crates/socket-patch-cli/tests/in_process_redirect.rs index 447ed9e9f..152fefb22 100644 --- a/crates/socket-patch-cli/tests/in_process_redirect.rs +++ b/crates/socket-patch-cli/tests/in_process_redirect.rs @@ -1172,33 +1172,39 @@ async fn yarn_berry_rollback_refuses_a_pin_wrapped_by_a_user_yarn_patch() { ); assert_eq!(vlt_hosted_common::redirected(&env), 1, "{env:#}"); - // What `yarn patch-commit -s` writes over the hosted pin: a - // `resolutions` entry routing the tarball descriptor to a `patch:` - // descriptor (the inner one with yarn's `%3A` escapes), the lock - // entry it resolves to, and the workspace dependency through it. + // What yarn 4.18 `yarn patch-commit -s` writes over the hosted pin: + // the dependency becomes a `patch:` descriptor wrapping the tarball + // URL (with yarn's `%3A` escapes), locked by a `patch:` entry next to + // the hosted one, which stays (the `resolutions` selector is kept). let wrapped = format!( - "patch:{NAME}@{}#~/.yarn/patches/{NAME}-http-7f4bf328bd.patch", + "patch:{NAME}@{}#~/.yarn/patches/{NAME}-http-735c2bb24a.patch", hosted_url.replace(':', "%3A") ); let pkg_path = tmp.path().join("package.json"); let mut pkg: serde_json::Value = serde_json::from_str(&std::fs::read_to_string(&pkg_path).unwrap()).unwrap(); - pkg["resolutions"][format!("{NAME}@{hosted_url}")] = serde_json::json!(wrapped); + pkg["dependencies"][NAME] = serde_json::json!(wrapped); std::fs::write( &pkg_path, serde_json::to_string_pretty(&pkg).unwrap() + "\n", ) .unwrap(); let lock_path = tmp.path().join("yarn.lock"); - let lock = std::fs::read_to_string(&lock_path).unwrap().replace( - &format!(" {NAME}: \"npm:^{VERSION}\"\n"), - &format!(" {NAME}: \"{wrapped}\"\n"), - ) + &format!( - "\n\"{NAME}@{wrapped}\":\n version: {VERSION}\n \ - resolution: \"{NAME}@{wrapped}::version={VERSION}&hash=0d6d28\"\n \ - checksum: 10c0/{}\n languageName: node\n linkType: hard\n", + let pinned = std::fs::read_to_string(&lock_path).unwrap(); + let patched_entry = format!( + "\"{NAME}@{wrapped}\":\n version: {VERSION}\n \ + resolution: \"{NAME}@{wrapped}::version={VERSION}&hash=e516cc\"\n \ + checksum: 10c0/{}\n languageName: node\n linkType: hard\n\n", "4".repeat(128) ); + let workspace = "\"consumer@workspace:.\":"; + assert!(pinned.contains(workspace), "{pinned}"); + let lock = pinned + .replace( + &format!(" {NAME}: \"npm:^{VERSION}\"\n"), + &format!(" {NAME}: \"{wrapped}\"\n"), + ) + .replace(workspace, &format!("{patched_entry}{workspace}")); std::fs::write(&lock_path, &lock).unwrap(); let pkg_before = std::fs::read_to_string(&pkg_path).unwrap(); diff --git a/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs b/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs index 94b244beb..eaa2fdc9b 100644 --- a/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs +++ b/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs @@ -780,9 +780,9 @@ async fn restore_berry( format!( "{name}@{version}: a `patch:` entry from `yarn patch` still wraps the \ hosted tarball in {rel}, and restoring the registry entry under it would \ - leave a lock `yarn install --immutable` rejects — remove the `patch:` \ - resolution from {pkg_rel}, run `yarn install`, roll back, then re-run \ - `yarn patch` on the registry version" + leave a lock `yarn install --immutable` rejects — put the registry range \ + back in place of the `patch:` descriptor in {pkg_rel}, run `yarn install`, \ + roll back, then re-run `yarn patch` on the registry version" ), ); continue; diff --git a/crates/socket-patch-core/src/vendor/yarn_berry_lock.rs b/crates/socket-patch-core/src/vendor/yarn_berry_lock.rs index 354b8156f..edd93adb6 100644 --- a/crates/socket-patch-core/src/vendor/yarn_berry_lock.rs +++ b/crates/socket-patch-core/src/vendor/yarn_berry_lock.rs @@ -849,9 +849,9 @@ pub async fn revert_yarn_berry_opts( let detail = format!( "refusing to revert {uuid_dir_rel}: a `patch:` entry from `yarn patch` in {wired} \ still wraps the vendored tarball, and restoring the registry entry under it would \ - leave a lock `yarn install --immutable` rejects — remove the `patch:` resolution \ - from {PACKAGE_JSON}, run `yarn install`, re-run `vendor --revert`, then re-run \ - `yarn patch` on the registry version" + leave a lock `yarn install --immutable` rejects — put the registry range back in \ + place of the `patch:` descriptor in {PACKAGE_JSON}, run `yarn install`, re-run \ + `vendor --revert`, then re-run `yarn patch` on the registry version" ); return RevertOutcome { success: false, @@ -3987,7 +3987,7 @@ __metadata: } /// #962: the user ran `yarn patch` / `yarn patch-commit -s` over the - /// vendored package, so a `patch:` lock entry and `resolutions` value + /// vendored package, so a `patch:` lock entry and dependency descriptor /// wrap the vendored tarball. Restoring the recorded entry would leave /// that wrapper naming a descriptor no entry resolves: every /// `yarn install --immutable` rejects the lock (YN0028). The revert must @@ -4017,7 +4017,8 @@ __metadata: tokio::fs::write(fx.lock_path(), &lock).await.unwrap(); let pkg_text = tokio::fs::read_to_string(fx.pkg_path()).await.unwrap(); let mut pkg: Value = serde_json::from_str(&pkg_text).unwrap(); - pkg["resolutions"][format!("left-pad@{resolution}")] = Value::String(wrapped); + // yarn 4.18 `patch-commit -s` rewrites the dependency itself. + pkg["dependencies"]["left-pad"] = Value::String(wrapped); let pkg_text = serde_json::to_string_pretty(&pkg).unwrap() + "\n"; tokio::fs::write(fx.pkg_path(), &pkg_text).await.unwrap(); From f041f521893072c3094f872bb2598abaa97a4308 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 20:58:53 +0000 Subject: [PATCH 6/6] Share one yarn patch: layer walker The unwind guards now use the same reader as the user-patch consent gate: yarn's builtin compatibility wrappers no longer block a rollback, and a user patch layered over one is still caught. The remedy no longer assumes the patch: descriptor is in the root package.json. Assisted-by: Claude Code:claude-opus-5-5 --- .../src/formats/yarn/patterns.rs | 103 ++++++++++++++---- .../src/patch/redirect/upstream/npm.rs | 42 +++---- .../src/patch/user_patches.rs | 43 +------- .../src/vendor/yarn_berry_lock.rs | 11 +- 4 files changed, 119 insertions(+), 80 deletions(-) diff --git a/crates/socket-patch-core/src/formats/yarn/patterns.rs b/crates/socket-patch-core/src/formats/yarn/patterns.rs index ee2565ba0..71bf24327 100644 --- a/crates/socket-patch-core/src/formats/yarn/patterns.rs +++ b/crates/socket-patch-core/src/formats/yarn/patterns.rs @@ -114,28 +114,81 @@ pub(crate) fn classic_key_real_name(patterns: &[String]) -> Option<&str> { names.all(|n| n == Some(first)).then_some(first) } -/// The descriptors every berry `patch:` protocol wraps in `text` (a lock or -/// a `package.json`), percent-decoded once: `patch:left-pad@http%3A//h/x.tgz -/// #~/.yarn/patches/x.patch` wraps `left-pad@http://h/x.tgz`. The patch -/// path after `#` and any `::version=…` binding are dropped. -pub(crate) fn patch_wrapped_descriptors(text: &str) -> Vec { +/// What a berry `patch:` reference wraps, every layer peeled. +#[derive(Debug, PartialEq, Eq)] +pub(crate) struct PatchLayers { + /// The innermost source, percent-decoded (`left-pad@npm:1.3.0`, + /// `left-pad@http://host/x.tgz`). + pub source: String, + /// Some layer applies a patch outside yarn's builtin compatibility + /// namespace (`yarn patch`, or a plugin's own builtin). + pub user_patch: bool, +} + +/// Peel a berry `patch:` reference (`patch:…` or `name@patch:…`) layer by +/// layer: a compatibility patch can wrap a user patch, or vice versa, and +/// both still belong to the original package. `Ok(None)` when `spec` is no +/// patch reference; `Err` names a reference too odd to read. +pub(crate) fn patch_layers(spec: &str) -> Result, &'static str> { + use crate::utils::purl::percent_decode_purl_component; + + if !spec.starts_with("patch:") && !spec.contains("@patch:") { + return Ok(None); + } + let mut source = spec.to_owned(); + let mut user_patch = false; + for _ in 0..16 { + let Some(inner) = source + .strip_prefix("patch:") + .or_else(|| source.split_once("@patch:").map(|(_, rest)| rest)) + else { + return Ok(Some(PatchLayers { source, user_patch })); + }; + let Some((original, patches)) = inner.split_once('#') else { + return Err("unrecognized patch: reference"); + }; + // Only Yarn's compatibility namespace is exempt. Plugins can + // supply their own builtin patches through getBuiltinPatch. + let patches = percent_decode_purl_component(patches); + user_patch |= !patches + .split("::") + .next() + .unwrap_or(&patches) + .split('&') + .all(|p| { + let p = p.rsplit('!').next().unwrap_or(p).trim_start_matches('~'); + p.starts_with("builtin') + }); + source = percent_decode_purl_component(original).into_owned(); + } + Err("patch locator nesting exceeds inspection limit") +} + +/// The sources every user `patch:` reference in `text` (a berry lock or a +/// `package.json`) wraps, innermost layer first decoded +/// ([`patch_layers`]): `patch:left-pad@http%3A//h/x.tgz#~/.yarn/patches/ +/// x.patch` wraps `left-pad@http://h/x.tgz`. Yarn's builtin compatibility +/// wrappers are not the user's and are left out. +pub(crate) fn user_patch_sources(text: &str) -> Vec { text.match_indices("patch:") - // The protocol opens a range (`name@patch:`) or a quoted value, - // never the middle of a word: `x.patch::version=…` is no wrapper. + // The protocol opens a value (`"patch:`) or a range (`name@patch:`), + // never the middle of a word: `x.patch::version=…` is no reference. .filter(|&(at, _)| { text[..at] .chars() .next_back() .is_none_or(|c| matches!(c, '@' | '"' | '\'') || c.is_whitespace()) }) - .map(|(at, tag)| { - let rest = &text[at + tag.len()..]; + .filter_map(|(at, _)| { + let rest = &text[at..]; let end = rest - .find(|c: char| matches!(c, '#' | '"' | '\'' | ',') || c.is_whitespace()) + .find(|c: char| matches!(c, '"' | '\'' | ',') || c.is_whitespace()) .unwrap_or(rest.len()); - crate::utils::purl::percent_decode_purl_component(&rest[..end]).into_owned() + match patch_layers(&rest[..end]) { + Ok(Some(layers)) if layers.user_patch => Some(layers.source), + _ => None, + } }) - .filter(|inner| !inner.is_empty()) .collect() } @@ -220,20 +273,32 @@ pub(crate) fn berry_npm_alias_target(range: &str) -> Option<&str> { mod tests { use super::*; - /// #962: the descriptor a `patch:` wraps, as `yarn patch-commit` - /// spells it in a lock key, a `resolution:` and a `package.json` value. + /// #962: the source a user `patch:` wraps, as `yarn patch-commit` + /// spells it in a lock key, a `resolution:` and a `package.json` value; + /// builtin compatibility wrappers are not the user's. #[test] - fn patch_wrapped_descriptors_decode_what_yarn_patch_wraps() { + fn user_patch_sources_peel_what_yarn_patch_wraps() { let lock = "\"left-pad@patch:left-pad@http%3A//h%3A1/x.tgz#~/.yarn/patches/a.patch\":\n \ resolution: \"left-pad@patch:left-pad@npm%3A1.3.0#~/.yarn/patches/a.patch\ ::version=1.3.0&hash=0d6d28\"\n"; assert_eq!( - patch_wrapped_descriptors(lock), + user_patch_sources(lock), vec!["left-pad@http://h:1/x.tgz", "left-pad@npm:1.3.0"] ); - let pkg = r#"{ "resolutions": { "a@npm:1": "patch:a@file%3A./v/a.tgz#./p.patch" } }"#; - assert_eq!(patch_wrapped_descriptors(pkg), vec!["a@file:./v/a.tgz"]); - assert!(patch_wrapped_descriptors("\"left-pad@npm:1.3.0\":\n").is_empty()); + let pkg = r#"{ "dependencies": { "a": "patch:a@file%3A./v/a.tgz#./p.patch" } }"#; + assert_eq!(user_patch_sources(pkg), vec!["a@file:./v/a.tgz"]); + // A user patch over yarn's own compat wrapper: both layers peeled. + let nested = "\"typescript@patch:typescript@patch%3Atypescript@http%253A//h/t.tgz\ + %23optional!builtin%3Ccompat/typescript%3E#~/.yarn/patches/t.patch\":"; + assert_eq!( + user_patch_sources(nested), + vec!["typescript@http://h/t.tgz"] + ); + // The compat wrapper alone is not a user patch. + let builtin = + "\"resolve@patch:resolve@http%3A//h/r.tgz#optional!builtin\":"; + assert!(user_patch_sources(builtin).is_empty()); + assert!(user_patch_sources("\"left-pad@npm:1.3.0\":\n").is_empty()); } /// #1271: the classic grammar keeps an empty range (`left-pad@` from diff --git a/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs b/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs index eaa2fdc9b..3f4b47d3f 100644 --- a/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs +++ b/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs @@ -747,11 +747,7 @@ async fn restore_berry( continue; }; let url_pin = reference.starts_with("https://") || reference.starts_with("http://"); - let archive = if url_pin { - reference - } else if let Some((_, binding)) = reference.split_once("::__archiveUrl=") { - binding.split('&').next().unwrap_or(binding) - } else { + let Some(archive) = berry_hosted_archive(reference) else { continue; }; let Some(uuid) = ctx.hosted_uuid(archive) else { @@ -781,8 +777,9 @@ async fn restore_berry( "{name}@{version}: a `patch:` entry from `yarn patch` still wraps the \ hosted tarball in {rel}, and restoring the registry entry under it would \ leave a lock `yarn install --immutable` rejects — put the registry range \ - back in place of the `patch:` descriptor in {pkg_rel}, run `yarn install`, \ - roll back, then re-run `yarn patch` on the registry version" + back in place of the `patch:` descriptor in the package.json that declares \ + it, run `yarn install`, roll back, then re-run `yarn patch` on the registry \ + version" ), ); continue; @@ -1062,21 +1059,28 @@ async fn restore_berry( view.write(rel, doc.render(&moved)); } -/// The hosted patch uuids a yarn `patch:` descriptor in `text` (a berry -/// lock or a root `package.json`) wraps: a tarball-URL pin, or the -/// `npm:…::__archiveUrl=` binding of a lock pinned before 5.0. +/// The tarball URL a berry hosted pin's locator reference names: the +/// tarball-URL locator itself, or the percent-encoded `::__archiveUrl=` +/// binding of an `npm:` locator pinned by releases up to 5.0 (#404). +fn berry_hosted_archive(reference: &str) -> Option<&str> { + if reference.starts_with("https://") || reference.starts_with("http://") { + Some(reference) + } else { + let (_, binding) = reference.split_once("::__archiveUrl=")?; + Some(binding.split('&').next().unwrap_or(binding)) + } +} + +/// The hosted patch uuids a user `patch:` reference in `text` (a berry +/// lock or a root `package.json`) wraps (#962). fn yarn_patch_wrapped_uuids(text: &str, ctx: &Ctx<'_>) -> Vec { - use crate::formats::yarn::patterns::{patch_wrapped_descriptors, split_pattern}; + use crate::formats::yarn::patterns::{split_pattern, user_patch_sources}; - patch_wrapped_descriptors(text) + user_patch_sources(text) .iter() - .filter_map(|inner| { - let (_, reference) = split_pattern(inner)?; - let url = match reference.split_once("::__archiveUrl=") { - Some((_, binding)) => binding.split('&').next().unwrap_or(binding), - None => reference, - }; - ctx.hosted_uuid(url) + .filter_map(|source| { + let (_, reference) = split_pattern(source)?; + ctx.hosted_uuid(berry_hosted_archive(reference)?) }) .collect() } diff --git a/crates/socket-patch-core/src/patch/user_patches.rs b/crates/socket-patch-core/src/patch/user_patches.rs index 955ec67e6..0ee7c9d2d 100644 --- a/crates/socket-patch-core/src/patch/user_patches.rs +++ b/crates/socket-patch-core/src/patch/user_patches.rs @@ -14,7 +14,7 @@ use serde_json::Value; use crate::formats::text::strip_bom; use crate::utils::fs::read_regular_to_string; use crate::utils::npm_range::npm_range_admits; -use crate::utils::purl::{normalize_purl, percent_decode_purl_component, strip_purl_qualifiers}; +use crate::utils::purl::{normalize_purl, strip_purl_qualifiers}; use crate::utils::workspace_globs::{workspaces_include_as, WorkspacesDialect}; #[derive(Default, Debug)] @@ -474,45 +474,14 @@ impl UserPatchIndex { if !spec.starts_with("patch:") { return; } - let mut source = spec.to_owned(); - let mut user_patch = false; - // Inspect every layer: a compatibility patch can wrap a user patch, - // or vice versa. Both still belong to the original package. - for _ in 0..16 { - if let Some(inner) = source - .strip_prefix("patch:") - .or_else(|| source.split_once("@patch:").map(|(_, rest)| rest)) - { - let Some((original, patches)) = inner.split_once('#') else { - self.unknown("npm", path, "unrecognized patch: reference"); - return; - }; - // Only Yarn's compatibility namespace is exempt. Plugins can - // supply their own builtin patches through getBuiltinPatch. - let patches = percent_decode_purl_component(patches); - user_patch |= !patches - .split("::") - .next() - .unwrap_or(&patches) - .split('&') - .all(|p| { - let p = p.rsplit('!').next().unwrap_or(p).trim_start_matches('~'); - p.starts_with("builtin') - }); - source = percent_decode_purl_component(original).into_owned(); - continue; - } - if user_patch { - let (name, version) = locator_target(&source); + match crate::formats::yarn::patterns::patch_layers(spec) { + Ok(Some(layers)) if layers.user_patch => { + let (name, version) = locator_target(&layers.source); self.add("npm", name, version, path); } - return; + Ok(_) => {} + Err(reason) => self.unknown("npm", path, reason), } - self.unknown( - "npm", - path, - "patch locator nesting exceeds inspection limit", - ); } async fn patch_package(&mut self, dir: &Path, value: &Value) { diff --git a/crates/socket-patch-core/src/vendor/yarn_berry_lock.rs b/crates/socket-patch-core/src/vendor/yarn_berry_lock.rs index edd93adb6..f2ae19f3b 100644 --- a/crates/socket-patch-core/src/vendor/yarn_berry_lock.rs +++ b/crates/socket-patch-core/src/vendor/yarn_berry_lock.rs @@ -850,8 +850,9 @@ pub async fn revert_yarn_berry_opts( "refusing to revert {uuid_dir_rel}: a `patch:` entry from `yarn patch` in {wired} \ still wraps the vendored tarball, and restoring the registry entry under it would \ leave a lock `yarn install --immutable` rejects — put the registry range back in \ - place of the `patch:` descriptor in {PACKAGE_JSON}, run `yarn install`, re-run \ - `vendor --revert`, then re-run `yarn patch` on the registry version" + place of the `patch:` descriptor in the {PACKAGE_JSON} that declares it, run \ + `yarn install`, re-run `vendor --revert`, then re-run `yarn patch` on the \ + registry version" ); return RevertOutcome { success: false, @@ -1125,7 +1126,7 @@ pub async fn revert_yarn_berry_opts( // ───────────────────────────── revert internals ───────────────────────────── /// The first wired file (yarn.lock, then package.json) in which a user -/// `patch:` descriptor wraps the vendored artifact of `uuid` (#962). An +/// `patch:` reference wraps the vendored artifact of `uuid` (#962). An /// unreadable or missing file wraps nothing here: the wired revert's own /// reads report it. async fn user_patch_wrapping(project_root: &Path, uuid: &str) -> Option<&'static str> { @@ -1134,9 +1135,9 @@ async fn user_patch_wrapping(project_root: &Path, uuid: &str) -> Option<&'static let Ok(text) = read_regular_to_string(&project_root.join(wired)).await else { continue; }; - if crate::formats::yarn::patterns::patch_wrapped_descriptors(&text) + if crate::formats::yarn::patterns::user_patch_sources(&text) .iter() - .any(|inner| inner.to_ascii_lowercase().contains(&needle)) + .any(|source| source.to_ascii_lowercase().contains(&needle)) { return Some(wired); }