Skip to content

Commit 54be192

Browse files
committed
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
1 parent fb33dbe commit 54be192

3 files changed

Lines changed: 155 additions & 16 deletions

File tree

‎crates/socket-patch-core/src/formats/yarn/patterns.rs‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,31 @@ pub(crate) fn classic_key_real_name(patterns: &[String]) -> Option<&str> {
114114
names.all(|n| n == Some(first)).then_some(first)
115115
}
116116

117+
/// The descriptors every berry `patch:` protocol wraps in `text` (a lock or
118+
/// a `package.json`), percent-decoded once: `patch:left-pad@http%3A//h/x.tgz
119+
/// #~/.yarn/patches/x.patch` wraps `left-pad@http://h/x.tgz`. The patch
120+
/// path after `#` and any `::version=…` binding are dropped.
121+
pub(crate) fn patch_wrapped_descriptors(text: &str) -> Vec<String> {
122+
text.match_indices("patch:")
123+
// The protocol opens a range (`name@patch:`) or a quoted value,
124+
// never the middle of a word: `x.patch::version=…` is no wrapper.
125+
.filter(|&(at, _)| {
126+
text[..at]
127+
.chars()
128+
.next_back()
129+
.is_none_or(|c| matches!(c, '@' | '"' | '\'') || c.is_whitespace())
130+
})
131+
.map(|(at, tag)| {
132+
let rest = &text[at + tag.len()..];
133+
let end = rest
134+
.find(|c: char| matches!(c, '#' | '"' | '\'' | ',') || c.is_whitespace())
135+
.unwrap_or(rest.len());
136+
crate::utils::purl::percent_decode_purl_component(&rest[..end]).into_owned()
137+
})
138+
.filter(|inner| !inner.is_empty())
139+
.collect()
140+
}
141+
117142
/// A classic `resolved` value split at its first `#`: the url before it,
118143
/// and the fragment as a lowercase sha1 when it is 40 hex digits (either
119144
/// 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> {
195220
mod tests {
196221
use super::*;
197222

223+
/// #962: the descriptor a `patch:` wraps, as `yarn patch-commit`
224+
/// spells it in a lock key, a `resolution:` and a `package.json` value.
225+
#[test]
226+
fn patch_wrapped_descriptors_decode_what_yarn_patch_wraps() {
227+
let lock = "\"left-pad@patch:left-pad@http%3A//h%3A1/x.tgz#~/.yarn/patches/a.patch\":\n \
228+
resolution: \"left-pad@patch:left-pad@npm%3A1.3.0#~/.yarn/patches/a.patch\
229+
::version=1.3.0&hash=0d6d28\"\n";
230+
assert_eq!(
231+
patch_wrapped_descriptors(lock),
232+
vec!["left-pad@http://h:1/x.tgz", "left-pad@npm:1.3.0"]
233+
);
234+
let pkg = r#"{ "resolutions": { "a@npm:1": "patch:a@file%3A./v/a.tgz#./p.patch" } }"#;
235+
assert_eq!(patch_wrapped_descriptors(pkg), vec!["a@file:./v/a.tgz"]);
236+
assert!(patch_wrapped_descriptors("\"left-pad@npm:1.3.0\":\n").is_empty());
237+
}
238+
198239
/// #1271: the classic grammar keeps an empty range (`left-pad@` from
199240
/// `"left-pad": ""`); the strict berry grammar still rejects it.
200241
#[test]

‎crates/socket-patch-core/src/patch/redirect/upstream/npm.rs‎

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1063,23 +1063,15 @@ async fn restore_berry(
10631063
}
10641064

10651065
/// The hosted patch uuids a yarn `patch:` descriptor in `text` (a berry
1066-
/// lock or a root `package.json`) wraps. `yarn patch-commit` escapes the
1067-
/// inner descriptor (`patch:<name>@http%3A//host/…#<patch path>`); a lock
1068-
/// pinned before 5.0 has the wrapped `npm:…::__archiveUrl=` binding instead.
1066+
/// lock or a root `package.json`) wraps: a tarball-URL pin, or the
1067+
/// `npm:…::__archiveUrl=` binding of a lock pinned before 5.0.
10691068
fn yarn_patch_wrapped_uuids(text: &str, ctx: &Ctx<'_>) -> Vec<String> {
1070-
use crate::formats::yarn::patterns::split_pattern;
1071-
use crate::utils::purl::percent_decode_purl_component;
1072-
1073-
text.match_indices("patch:")
1074-
.filter_map(|(at, tag)| {
1075-
let rest = &text[at + tag.len()..];
1076-
let end = rest
1077-
.find(|c: char| {
1078-
matches!(c, '#' | '"' | '\'' | ',' | '\n' | '\r') || c.is_whitespace()
1079-
})
1080-
.unwrap_or(rest.len());
1081-
let inner = percent_decode_purl_component(&rest[..end]).into_owned();
1082-
let (_, reference) = split_pattern(&inner)?;
1069+
use crate::formats::yarn::patterns::{patch_wrapped_descriptors, split_pattern};
1070+
1071+
patch_wrapped_descriptors(text)
1072+
.iter()
1073+
.filter_map(|inner| {
1074+
let (_, reference) = split_pattern(inner)?;
10831075
let url = match reference.split_once("::__archiveUrl=") {
10841076
Some((_, binding)) => binding.split('&').next().unwrap_or(binding),
10851077
None => reference,

‎crates/socket-patch-core/src/vendor/yarn_berry_lock.rs‎

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -841,6 +841,28 @@ pub async fn revert_yarn_berry_opts(
841841
}
842842
}
843843
}
844+
// A user `yarn patch` over the vendored tarball (#962): restoring the
845+
// recorded entry under its `patch:` wrapper leaves a lock every
846+
// `yarn install --immutable` rejects, so refuse before either file is
847+
// written (and before the dry-run return, so a preview agrees).
848+
if let Some(wired) = user_patch_wrapping(project_root, &entry.uuid).await {
849+
let detail = format!(
850+
"refusing to revert {uuid_dir_rel}: a `patch:` entry from `yarn patch` in {wired} \
851+
still wraps the vendored tarball, and restoring the registry entry under it would \
852+
leave a lock `yarn install --immutable` rejects — remove the `patch:` resolution \
853+
from {PACKAGE_JSON}, run `yarn install`, re-run `vendor --revert`, then re-run \
854+
`yarn patch` on the registry version"
855+
);
856+
return RevertOutcome {
857+
success: false,
858+
warnings: vec![VendorWarning::new(
859+
"vendor_lock_still_wired_revert_blocked",
860+
detail.clone(),
861+
)],
862+
error: Some(detail),
863+
kept_artifact: false,
864+
};
865+
}
844866
if dry_run {
845867
return RevertOutcome::ok();
846868
}
@@ -1102,6 +1124,26 @@ pub async fn revert_yarn_berry_opts(
11021124

11031125
// ───────────────────────────── revert internals ─────────────────────────────
11041126

1127+
/// The first wired file (yarn.lock, then package.json) in which a user
1128+
/// `patch:` descriptor wraps the vendored artifact of `uuid` (#962). An
1129+
/// unreadable or missing file wraps nothing here: the wired revert's own
1130+
/// reads report it.
1131+
async fn user_patch_wrapping(project_root: &Path, uuid: &str) -> Option<&'static str> {
1132+
let needle = uuid.to_ascii_lowercase();
1133+
for wired in [YARN_LOCK, PACKAGE_JSON] {
1134+
let Ok(text) = read_regular_to_string(&project_root.join(wired)).await else {
1135+
continue;
1136+
};
1137+
if crate::formats::yarn::patterns::patch_wrapped_descriptors(&text)
1138+
.iter()
1139+
.any(|inner| inner.to_ascii_lowercase().contains(&needle))
1140+
{
1141+
return Some(wired);
1142+
}
1143+
}
1144+
None
1145+
}
1146+
11051147
/// Put back an entry vendoring pruned (see [`KIND_LOCK_ENTRY_PRUNED`]):
11061148
/// the rewritten shared entry returns to its original lines, a removed one
11071149
/// is inserted where yarn sorts it. Already restored is silent; anything
@@ -3944,6 +3986,70 @@ __metadata:
39443986
);
39453987
}
39463988

3989+
/// #962: the user ran `yarn patch` / `yarn patch-commit -s` over the
3990+
/// vendored package, so a `patch:` lock entry and `resolutions` value
3991+
/// wrap the vendored tarball. Restoring the recorded entry would leave
3992+
/// that wrapper naming a descriptor no entry resolves: every
3993+
/// `yarn install --immutable` rejects the lock (YN0028). The revert must
3994+
/// refuse BEFORE writing either file, on a dry run too.
3995+
#[tokio::test]
3996+
async fn revert_refuses_before_writing_when_a_user_yarn_patch_wraps_the_artifact() {
3997+
let fx = fixture().await;
3998+
let (_, entry, _) = expect_done(fx.vendor(false).await);
3999+
let entry = entry.unwrap();
4000+
4001+
let text = tokio::fs::read_to_string(fx.lock_path()).await.unwrap();
4002+
let resolution = text
4003+
.lines()
4004+
.find_map(|l| l.strip_prefix(" resolution: \"left-pad@"))
4005+
.and_then(|r| r.strip_suffix('"'))
4006+
.expect("the vendored lock must carry our resolution line")
4007+
.to_string();
4008+
let wrapped = format!(
4009+
"patch:left-pad@{}#~/.yarn/patches/left-pad-file-0d6d28.patch",
4010+
resolution.replace(':', "%3A")
4011+
);
4012+
let lock = format!(
4013+
"{text}\n\"left-pad@{wrapped}\":\n version: 1.3.0\n \
4014+
resolution: \"left-pad@{wrapped}::version=1.3.0&hash=0d6d28\"\n \
4015+
languageName: node\n linkType: hard\n"
4016+
);
4017+
tokio::fs::write(fx.lock_path(), &lock).await.unwrap();
4018+
let pkg_text = tokio::fs::read_to_string(fx.pkg_path()).await.unwrap();
4019+
let mut pkg: Value = serde_json::from_str(&pkg_text).unwrap();
4020+
pkg["resolutions"][format!("left-pad@{resolution}")] = Value::String(wrapped);
4021+
let pkg_text = serde_json::to_string_pretty(&pkg).unwrap() + "\n";
4022+
tokio::fs::write(fx.pkg_path(), &pkg_text).await.unwrap();
4023+
4024+
for dry_run in [true, false] {
4025+
let outcome = revert_yarn_berry(&entry, fx.root(), dry_run).await;
4026+
assert!(
4027+
!outcome.success,
4028+
"dry_run={dry_run}: must refuse under a user `yarn patch`: {:?}",
4029+
outcome.warnings
4030+
);
4031+
assert!(
4032+
outcome.warnings.iter().any(|w| {
4033+
w.code == "vendor_lock_still_wired_revert_blocked"
4034+
&& w.detail.contains("yarn patch")
4035+
}),
4036+
"dry_run={dry_run}: {:?}",
4037+
outcome.warnings
4038+
);
4039+
assert_eq!(
4040+
tokio::fs::read_to_string(fx.lock_path()).await.unwrap(),
4041+
lock,
4042+
"dry_run={dry_run}: yarn.lock untouched"
4043+
);
4044+
assert_eq!(
4045+
tokio::fs::read_to_string(fx.pkg_path()).await.unwrap(),
4046+
pkg_text,
4047+
"dry_run={dry_run}: package.json untouched"
4048+
);
4049+
assert!(fx.tgz_path().exists(), "the artifact must survive");
4050+
}
4051+
}
4052+
39474053
/// The user re-keys our resolutions entry (`"left-pad"` →
39484054
/// `"left-pad@npm:1.3.0"`) without changing its `file:` value. The
39494055
/// recorded key then reads as already-converged (Added record, key

0 commit comments

Comments
 (0)