Skip to content

Commit 68547b1

Browse files
committed
Harden berry pin writes and refused locks
A patch server URL containing "$" was expanded as a regex capture reference when written into the berry lock entry, corrupting its resolution and checksum lines. The URL is now written literally. When the berry preflight refuses a lock (mixed line endings, an unsupported cacheKey), packages that lock pins are still decided by the berry rewriter, and left unconfirmed. Before, a URL from an earlier run in such a lock could confirm the package through the hosted text probe. Assisted-by: Claude Code:claude-opus-5-5
1 parent 3bc0d7e commit 68547b1

1 file changed

Lines changed: 111 additions & 5 deletions

File tree

  • crates/socket-patch-core/src/patch/redirect

‎crates/socket-patch-core/src/patch/redirect/mod.rs‎

Lines changed: 111 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ use std::borrow::Cow;
1919
use std::collections::BTreeMap;
2020
use std::sync::LazyLock;
2121

22-
use regex::Regex;
22+
use regex::{NoExpand, Regex};
2323
use serde::{Deserialize, Serialize};
2424
use serde_json::{json, Value};
2525

@@ -3389,6 +3389,15 @@ fn rewrite_yarn_berry(
33893389
preflight_yarn_berry_hosted(raw, files.get(".yarnrc.yml").map(String::as_str))
33903390
{
33913391
result.warnings.push(warning);
3392+
// Nothing is verified, so nothing is confirmed — but a dep this lock
3393+
// locks is still this rewriter's to decide: an earlier run's URL in
3394+
// the lock must not confirm it through the text probe.
3395+
let lf = to_lf(body);
3396+
for dep in &npm {
3397+
if berry_lock_locks(&lf, &full_name(dep), &dep.version) {
3398+
result.yarn_berry_uuids.insert(dep.patch_uuid.clone());
3399+
}
3400+
}
33923401
return;
33933402
}
33943403
let eol = LineEndings::of(body);
@@ -3731,21 +3740,27 @@ fn rewrite_yarn_berry(
37313740
let resolution = format!("{fname}@{}", dep.artifact_url);
37323741
let body_lines = block.split_once('\n').map(|(_, rest)| rest).unwrap_or("");
37333742
let mut rewritten = format!("\n{new_key}:\n{body_lines}");
3743+
// `NoExpand`: the URL is literal text, and a `$` in it (a patch
3744+
// server path) must never be read as a capture-group reference.
37343745
rewritten = resolution_re
3735-
.replace(&rewritten, format!("\n resolution: \"{resolution}\"").as_str())
3746+
.replace(
3747+
&rewritten,
3748+
NoExpand(&format!("\n resolution: \"{resolution}\"")),
3749+
)
37363750
.to_string();
37373751
match &checksum {
37383752
Some(checksum) if checksum_re.is_match(&rewritten) => {
37393753
rewritten = checksum_re
3740-
.replace(&rewritten, format!("\n checksum: {checksum}").as_str())
3754+
.replace(&rewritten, NoExpand(&format!("\n checksum: {checksum}")))
37413755
.to_string();
37423756
}
37433757
Some(checksum) => {
37443758
rewritten = resolution_re
37453759
.replace(
37463760
&rewritten,
3747-
format!("\n resolution: \"{resolution}\"\n checksum: {checksum}")
3748-
.as_str(),
3761+
NoExpand(&format!(
3762+
"\n resolution: \"{resolution}\"\n checksum: {checksum}"
3763+
)),
37493764
)
37503765
.to_string();
37513766
}
@@ -3840,6 +3855,25 @@ pub(crate) fn berry_entries_sorted(blocks: &[String]) -> bool {
38403855
keys.windows(2).all(|w| w[0] <= w[1])
38413856
}
38423857

3858+
/// Whether an LF-normalized berry lock holds an entry for `name` at
3859+
/// `version`, under any descriptor (npm, tarball, `patch:`, …).
3860+
fn berry_lock_locks(content: &str, name: &str, version: &str) -> bool {
3861+
use crate::vendor::yarn_classic_lock::{split_berry_key_patterns, split_pattern};
3862+
let version_line = format!("\n version: {version}\n");
3863+
content.split("\n\n").any(|block| {
3864+
let Some(key) = block.lines().next().and_then(|l| l.strip_suffix(':')) else {
3865+
return false;
3866+
};
3867+
if key.starts_with([' ', '\t', '#']) || key == "__metadata" {
3868+
return false;
3869+
}
3870+
format!("{block}\n").contains(&version_line)
3871+
&& split_berry_key_patterns(key)
3872+
.iter()
3873+
.any(|p| split_pattern(p).is_some_and(|(n, _)| n == name))
3874+
})
3875+
}
3876+
38433877
/// The root manifest the yarn berry hosted pin edits.
38443878
const BERRY_MANIFEST: &str = "package.json";
38453879

@@ -8214,6 +8248,78 @@ mod tests {
82148248
}
82158249
}
82168250

8251+
/// A berry lock the preflight refuses (here an unsupported cacheKey)
8252+
/// verifies nothing, so it confirms nothing — yet the deps it locks stay
8253+
/// the berry rewriter's: an earlier run's URL in the lock must not
8254+
/// confirm them through the hosted text probe.
8255+
#[test]
8256+
fn yarn_berry_preflight_refusal_owns_locked_deps_without_confirming() {
8257+
let checksum = format!("10c0/{}", "7".repeat(128));
8258+
let url = berry_hosted_url("left-pad", "left-pad", "1.3.0");
8259+
let ovr = berry_override("left-pad", "1.3.0", &url, &checksum);
8260+
let mut first = RewriteResult::default();
8261+
rewrite_yarn_berry(
8262+
&berry_files(berry_lock("10c0"), berry_manifest()),
8263+
std::slice::from_ref(&ovr),
8264+
&mut first,
8265+
);
8266+
let refused = berry_files(
8267+
first.files["yarn.lock"].replace("cacheKey: 10c0", "cacheKey: 8c0"),
8268+
berry_manifest(),
8269+
);
8270+
let unlocked = berry_override("right-pad", "1.0.0", &url, &checksum);
8271+
let mut r = RewriteResult::default();
8272+
rewrite_yarn_berry(&refused, &[ovr, unlocked], &mut r);
8273+
assert_eq!(r.warnings[0].code, "redirect_yarn_berry_cache_unsupported");
8274+
assert!(r.files.is_empty());
8275+
assert_eq!(
8276+
r.yarn_berry_uuids.iter().collect::<Vec<_>>(),
8277+
vec![BERRY_UUID],
8278+
"only the locked dep is owned"
8279+
);
8280+
assert!(r.confirmed_yarn_berry_uuids.is_empty());
8281+
}
8282+
8283+
/// The artifact URL is spliced as literal text: a `$` in it (legal in a
8284+
/// URL path) must not be expanded as a regex capture reference.
8285+
#[test]
8286+
fn yarn_berry_artifact_url_dollar_is_written_literally() {
8287+
let checksum = format!("10c0/{}", "7".repeat(128));
8288+
let url = "https://patch.socket.dev/patch/npm/left-pad/1.3.0/t$0k$1/u/left-pad-1.3.0.tgz";
8289+
let ovr = berry_override("left-pad", "1.3.0", url, &checksum);
8290+
let mut r = RewriteResult::default();
8291+
rewrite_yarn_berry(
8292+
&berry_files(berry_lock("10c0"), berry_manifest()),
8293+
std::slice::from_ref(&ovr),
8294+
&mut r,
8295+
);
8296+
assert!(r.warnings.is_empty(), "{:?}", r.warnings);
8297+
let out = &r.files["yarn.lock"];
8298+
assert!(
8299+
out.contains(&format!(
8300+
"\"left-pad@{url}\":\n version: 1.3.0\n resolution: \"left-pad@{url}\"\n \
8301+
checksum: {checksum}\n"
8302+
)),
8303+
"{out}"
8304+
);
8305+
// ... and the checksum-line insertion path, for a lock without one.
8306+
let no_checksum_line =
8307+
berry_lock("10c0").replace(&format!(" checksum: 10c0/{}\n", "3".repeat(128)), "");
8308+
let mut r = RewriteResult::default();
8309+
rewrite_yarn_berry(
8310+
&berry_files(no_checksum_line, berry_manifest()),
8311+
std::slice::from_ref(&ovr),
8312+
&mut r,
8313+
);
8314+
let out = &r.files["yarn.lock"];
8315+
assert!(
8316+
out.contains(&format!(
8317+
" resolution: \"left-pad@{url}\"\n checksum: {checksum}\n"
8318+
)),
8319+
"{out}"
8320+
);
8321+
}
8322+
82178323
/// A rescan whose grant lacks the `yarnBerry10c0` checksum still
82188324
/// confirms a pin an earlier run completed (the lock already holds the
82198325
/// checksum it was written with); an entry that would need the checksum

0 commit comments

Comments
 (0)