From efc7c38fbb352e464e731cc50f1d9df02b6712ff Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Wed, 7 Oct 2026 12:25:59 -0400 Subject: [PATCH 1/2] Only warn berry migration risk when the classic lock holds a pin An offline-mirror refusal leaves the matched classic entries untouched, but the matched entry still set any_pinned, so a first hosted run that wrote nothing reported redirect_yarn_classic_berry_migration_risk next to redirect_yarn_classic_offline_mirror. Count a refused entry as pinned only when an earlier run already wrote its hosted URL into the block. Reported by Cursor Bugbot on #917. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../src/patch/redirect/mod.rs | 39 ++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/crates/socket-patch-core/src/patch/redirect/mod.rs b/crates/socket-patch-core/src/patch/redirect/mod.rs index a2b825411..2a9170321 100644 --- a/crates/socket-patch-core/src/patch/redirect/mod.rs +++ b/crates/socket-patch-core/src/patch/redirect/mod.rs @@ -3519,6 +3519,7 @@ fn rewrite_yarn_classic( Regex::new(&(String::from(r#"\n {2}version ""#) + ®ex::escape(&dep.version) + "\"")) .expect("version regex from the escaped version is valid"); let mut matched_any = false; + let mut pinned_any = false; let mut alias_skipped = false; let mut copy_skipped = false; for (i, block) in blocks.iter_mut().enumerate() { @@ -3606,8 +3607,12 @@ fn rewrite_yarn_classic( result .refused_yarn_classic_uuids .insert(dep.patch_uuid.clone()); + // The refusal leaves this entry as it was, so it carries a + // hosted pin only if an earlier run already wrote one. + pinned_any |= block.contains(dep.artifact_url.as_str()); continue; } + pinned_any = true; let frag = dep .integrity .sha1 @@ -3664,7 +3669,7 @@ fn rewrite_yarn_classic( detail: format!("no yarn.lock entry resolving {fname}@{}", dep.version), }); } - any_pinned |= matched_any; + any_pinned |= pinned_any; } // Yarn 2+ (berry) migrates a classic lock on install and re-resolves // every entry from the registry, dropping the hosted pins this lock now @@ -10129,6 +10134,38 @@ mod tests { assert_eq!(berry_risk_count(&r), 0, "{:?}", r.warnings); } + /// An offline-mirror refusal writes no pin, so it must not also claim + /// the lock carries hosted pins; a pin an earlier run already wrote is + /// still at risk and still warned about. + #[test] + fn yarn_classic_berry_risk_follows_pins_under_offline_mirror_refusal() { + let lp = npm_override( + "left-pad", + "1.3.0", + "http://p.test/lp.tgz", + "sha512-PATCHED==", + ); + let mut files = classic_files(Some(r#"{"name":"p"}"#)); + let mut first = RewriteResult::default(); + rewrite_yarn_classic(&files, std::slice::from_ref(&lp), &mut first); + let pinned_lock = first.files["yarn.lock"].clone(); + + files.insert( + YARNRC_REL.to_string(), + "yarn-offline-mirror ./mirror\n".to_string(), + ); + let mut r = RewriteResult::default(); + rewrite_yarn_classic(&files, std::slice::from_ref(&lp), &mut r); + let codes: Vec<&str> = r.warnings.iter().map(|w| w.code.as_str()).collect(); + assert_eq!(codes, ["redirect_yarn_classic_offline_mirror"]); + + files.insert("yarn.lock".into(), pinned_lock); + let mut r = RewriteResult::default(); + rewrite_yarn_classic(&files, std::slice::from_ref(&lp), &mut r); + assert!(r.files.is_empty(), "{:?}", r.files); + assert_eq!(berry_risk_count(&r), 1, "{:?}", r.warnings); + } + /// A CRLF classic lock (Windows `core.autocrlf` checkout) must rewrite /// the TARGET entry, not whichever entry happens to come first, and every /// untouched line must keep its CRLF ending byte-exactly (see the CRLF From f0062e501ccbf015feb95cf2f8379ebdfe73b16f Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Wed, 7 Oct 2026 15:10:09 -0400 Subject: [PATCH 2/2] Count same-server hosted pins from earlier runs as berry migration risk Under an offline-mirror refusal the prior-pin check matched only this run's exact artifact URL, so a lock still holding a hosted pin from an earlier grant token or patch uuid on the same patch server stayed silent about the berry migration risk. Reuse berry_hosted_pin_is_ours on the entry's resolved URL (fragment stripped) so any same-origin hosted pin naming the package version counts, while a user's own mirror does not. Reported by Cursor Bugbot on #1077. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../src/patch/redirect/mod.rs | 37 ++++++++++++++++++- 1 file changed, 35 insertions(+), 2 deletions(-) diff --git a/crates/socket-patch-core/src/patch/redirect/mod.rs b/crates/socket-patch-core/src/patch/redirect/mod.rs index 393bcc74b..fde7e87b3 100644 --- a/crates/socket-patch-core/src/patch/redirect/mod.rs +++ b/crates/socket-patch-core/src/patch/redirect/mod.rs @@ -3608,8 +3608,17 @@ fn rewrite_yarn_classic( .refused_yarn_classic_uuids .insert(dep.patch_uuid.clone()); // The refusal leaves this entry as it was, so it carries a - // hosted pin only if an earlier run already wrote one. - pinned_any |= block.contains(dep.artifact_url.as_str()); + // hosted pin only if an earlier run already wrote one — this + // run's URL, or one on the same patch server from an earlier + // grant token or patch uuid. + pinned_any |= resolved_re.find(block).is_some_and(|m| { + let url = m + .as_str() + .trim_start_matches("\n resolved \"") + .trim_end_matches('"'); + let url = url.split_once('#').map_or(url, |(u, _)| u); + berry_hosted_pin_is_ours(url, &fname, Some(&dep.version), &dep.artifact_url) + }); continue; } pinned_any = true; @@ -10162,6 +10171,30 @@ mod tests { rewrite_yarn_classic(&files, std::slice::from_ref(&lp), &mut r); assert!(r.files.is_empty(), "{:?}", r.files); assert_eq!(berry_risk_count(&r), 1, "{:?}", r.warnings); + + // A pin from an earlier grant token or uuid on the same patch server + // is still lost on a berry migrate, so it is still warned about; a + // same-named tarball on another origin (a user mirror) is not ours. + let host = "https://patch.test/patch/npm/left-pad/1.3.0"; + let current = npm_override( + "left-pad", + "1.3.0", + &format!("{host}/tok-new/uuid-new/left-pad-1.3.0.tgz"), + "sha512-PATCHED==", + ); + for (old_url, expect) in [ + (format!("{host}/tok-old/uuid-old/left-pad-1.3.0.tgz"), 1), + ("https://mirror.test/left-pad-1.3.0.tgz".to_string(), 0), + ] { + let pinned = classic_lock_two_entries().replace( + "https://registry.yarnpkg.com/left-pad/-/left-pad-1.3.0.tgz", + &old_url, + ); + files.insert("yarn.lock".into(), pinned); + let mut r = RewriteResult::default(); + rewrite_yarn_classic(&files, std::slice::from_ref(¤t), &mut r); + assert_eq!(berry_risk_count(&r), expect, "{old_url}: {:?}", r.warnings); + } } /// A CRLF classic lock (Windows `core.autocrlf` checkout) must rewrite