Skip to content

Commit 657fce2

Browse files
Fix yarn portal copies read as no package (#1579) (#1586)
* Start fix for #1579 Assisted-by: Claude Code:claude-opus-5-5 * Flag yarn portal copies that can't be identified A yarn berry portal: or link: copy is locked at the placeholder version 0.0.0-use.local, so only its package.json says what it installs. Two shapes were dropped as "no package copy", so hosted and vendored scans reported a clean success and every VEX path attested not_affected while yarn linked unpatched code: - a portal: target with no package.json (a .gitkeep placeholder a fetch step fills): yarn refuses a manifest-less portal, so the directory on disk is not what gets installed. Only a link: to a plain directory is exempt now. - a manifest with no name or no version. A nameless one may be any package; one naming a package but no version may be any version of that package, and contests only that package. The copy now records whether it is a portal or a link, and the vendored, hosted and vex paths share the decision. Fixes #1579 Assisted-by: Claude Code:claude-opus-5-5 * Name versionless yarn copies by their package A portal:/link: copy whose package.json names a package but no version now gets a vendoring warning that says so, instead of "whose package can't be read". An empty or invalid name in that manifest names no package, so the copy is treated as possibly any package rather than as a copy of a package called "". Refs #1579 Assisted-by: Claude Code:claude-opus-5-5 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 15cbde4 commit 657fce2

7 files changed

Lines changed: 346 additions & 73 deletions

File tree

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

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1984,3 +1984,36 @@ async fn issue_1561_berry_missing_relative_portal_and_link_copies_are_named() {
19841984
assert!(details.is_empty(), "{protocol}: {details:?}");
19851985
}
19861986
}
1987+
1988+
/// #1579: a `portal:` target holding only a `.gitkeep` placeholder (yarn
1989+
/// refuses a manifest-less portal, so it is not what gets installed), and a
1990+
/// linked manifest with no `name`, may be any package; one naming left-pad
1991+
/// with no version may be left-pad. The hosted scan pins the registry entry
1992+
/// and names each as possibly unpatched; it reported a clean success.
1993+
/// Controls: a `link:` to the placeholder (plain assets) and a versionless
1994+
/// manifest naming another package are no copy of left-pad.
1995+
#[tokio::test]
1996+
async fn issue_1579_berry_portal_placeholder_and_versionless_copies_are_named() {
1997+
const UUID: &str = "15791579-1579-4579-8579-157915791579";
1998+
for protocol in ["portal", "link"] {
1999+
let spec = format!("{protocol}:./vend/lp");
2000+
for (file, text, named) in [
2001+
("vend/lp/.gitkeep", "", protocol == "portal"),
2002+
("vend/lp/package.json", r#"{"version":"1.3.0"}"#, true),
2003+
("vend/lp/package.json", r#"{"name":"left-pad"}"#, true),
2004+
("vend/lp/package.json", r#"{"name":"other-pkg"}"#, false),
2005+
] {
2006+
let details = berry_linked_copy_details(UUID, &spec, &[(file, text)]).await;
2007+
if !named {
2008+
assert!(details.is_empty(), "{protocol} {text:?}: {details:?}");
2009+
continue;
2010+
}
2011+
assert_eq!(details.len(), 1, "{protocol} {text:?}: {details:?}");
2012+
assert!(
2013+
details[0].contains(&format!("lp2@{spec}::")) && details[0].contains("UNPATCHED"),
2014+
"{protocol} {text:?}: {}",
2015+
details[0]
2016+
);
2017+
}
2018+
}
2019+
}

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

Lines changed: 81 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,21 @@ pub(crate) enum CopyRef {
2626
/// `node_modules`: the lock records the placeholder version
2727
/// `0.0.0-use.local`, so the package's name AND version are its
2828
/// `package.json`'s.
29-
Linked(String),
29+
Linked(String, LinkKind),
3030
/// A url tarball (`#` fragment dropped): the package is the registry
3131
/// package its `/<name>/-/<leaf>-<version>.tgz` path names.
3232
Url(String),
3333
}
3434

35+
/// Which berry protocol a [`CopyRef::Linked`] copy came from. yarn installs
36+
/// a `link:` to any directory, a package or plain assets, but refuses a
37+
/// `portal:` whose target has no `package.json` (#1579).
38+
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
39+
pub(crate) enum LinkKind {
40+
Portal,
41+
Link,
42+
}
43+
3544
impl CopyRef {
3645
/// The root-relative file to read for [`CopyRef::identity`]: the tarball
3746
/// itself, or the directory's `package.json`. `None` for a url, which
@@ -42,14 +51,14 @@ impl CopyRef {
4251
/// UNC path) a network share. Such a copy is never read, so its package
4352
/// stays unknown (#1516).
4453
pub(crate) fn read_path(&self) -> Option<String> {
45-
if let CopyRef::Tarball(rel) | CopyRef::Directory(rel) | CopyRef::Linked(rel) = self {
54+
if let CopyRef::Tarball(rel) | CopyRef::Directory(rel) | CopyRef::Linked(rel, _) = self {
4655
if crate::utils::relpath::is_anchored(rel) {
4756
return None;
4857
}
4958
}
5059
match self {
5160
CopyRef::Tarball(rel) => Some(rel.clone()),
52-
CopyRef::Directory(dir) | CopyRef::Linked(dir) => Some(if dir.is_empty() {
61+
CopyRef::Directory(dir) | CopyRef::Linked(dir, _) => Some(if dir.is_empty() {
5362
"package.json".to_string()
5463
} else {
5564
format!("{dir}/package.json")
@@ -70,7 +79,7 @@ impl CopyRef {
7079
let name = match self {
7180
CopyRef::Tarball(_) => tarball_manifest_name(bytes?)?,
7281
CopyRef::Directory(_) => manifest_name(bytes?)?,
73-
CopyRef::Linked(_) => {
82+
CopyRef::Linked(..) => {
7483
let manifest = parse_manifest(bytes?)?;
7584
let name = manifest.get("name")?.as_str()?.to_string();
7685
let version = manifest.get("version")?.as_str()?.to_string();
@@ -81,6 +90,20 @@ impl CopyRef {
8190
Some((name, lock_version.to_string()))
8291
}
8392

93+
/// The package a [`CopyRef::Linked`] copy's manifest names when it
94+
/// names no version, so [`CopyRef::identity`] can't say: the copy may
95+
/// be any version of that package, and of no other (#1579). `None` for
96+
/// any other copy, or a manifest that names no package (an empty or
97+
/// invalid npm name names none, so the copy may be any package).
98+
pub(crate) fn linked_name(&self, bytes: Option<&[u8]>) -> Option<String> {
99+
match self {
100+
CopyRef::Linked(..) => {
101+
manifest_name(bytes?).filter(|n| crate::vendor::is_safe_npm_name(n))
102+
}
103+
_ => None,
104+
}
105+
}
106+
84107
/// Where the copy installs from, for a warning or diagnostic: "installs
85108
/// it from …" / "links it to …". A url is redacted, since the phrase
86109
/// may be published (a VEX document, a JSON envelope).
@@ -89,7 +112,7 @@ impl CopyRef {
89112
CopyRef::Tarball(rel) | CopyRef::Directory(rel) => {
90113
format!("installs it from the user's file:{rel}")
91114
}
92-
CopyRef::Linked(rel) => {
115+
CopyRef::Linked(rel, _) => {
93116
let rel = if rel.is_empty() { "." } else { rel };
94117
format!("links it to the user's directory {rel} (portal:/link:)")
95118
}
@@ -142,12 +165,11 @@ pub(crate) fn berry_copy_ref(reference: &str) -> Option<CopyRef> {
142165
));
143166
}
144167
let (path, linked) = if let Some(path) = reference.strip_prefix("file:") {
145-
(path, false)
146-
} else if let Some(path) = reference
147-
.strip_prefix("portal:")
148-
.or_else(|| reference.strip_prefix("link:"))
149-
{
150-
(path, true)
168+
(path, None)
169+
} else if let Some(path) = reference.strip_prefix("portal:") {
170+
(path, Some(LinkKind::Portal))
171+
} else if let Some(path) = reference.strip_prefix("link:") {
172+
(path, Some(LinkKind::Link))
151173
} else {
152174
return None;
153175
};
@@ -156,8 +178,8 @@ pub(crate) fn berry_copy_ref(reference: &str) -> Option<CopyRef> {
156178
let path = path.split("::").next().unwrap_or_default();
157179
let path = path.split('#').next().unwrap_or_default();
158180
let rel = berry_copy_path(reference, path)?;
159-
Some(if linked {
160-
CopyRef::Linked(rel)
181+
Some(if let Some(kind) = linked {
182+
CopyRef::Linked(rel, kind)
161183
} else if is_tarball_path(path) {
162184
CopyRef::Tarball(rel)
163185
} else {
@@ -212,13 +234,6 @@ fn copy_path(base: &str, path: &str) -> Option<String> {
212234
))
213235
}
214236

215-
/// Whether `bytes` parse as a JSON `package.json` (any content): a copy
216-
/// whose manifest doesn't (a Git LFS pointer, an encrypted blob) can't be
217-
/// read for its package (#1561).
218-
pub(crate) fn is_json_manifest(bytes: &[u8]) -> bool {
219-
parse_manifest(bytes).is_some()
220-
}
221-
222237
fn parse_manifest(bytes: &[u8]) -> Option<serde_json::Value> {
223238
let bytes = crate::formats::text::strip_bom_bytes(bytes);
224239
serde_json::from_slice::<serde_json::Value>(bytes).ok()
@@ -320,11 +335,11 @@ mod tests {
320335
);
321336
assert_eq!(
322337
berry_copy_ref(&format!("portal:../lp{ws}")),
323-
Some(CopyRef::Linked("packages/lp".into()))
338+
Some(CopyRef::Linked("packages/lp".into(), LinkKind::Portal))
324339
);
325340
assert_eq!(
326341
berry_copy_ref("link:./vend/lp::locator=app%40workspace%3A."),
327-
Some(CopyRef::Linked("vend/lp".into()))
342+
Some(CopyRef::Linked("vend/lp".into(), LinkKind::Link))
328343
);
329344
assert_eq!(berry_copy_ref("npm:1.3.0"), None);
330345
assert_eq!(berry_copy_ref("workspace:packages/b"), None);
@@ -335,27 +350,43 @@ mod tests {
335350
#[test]
336351
fn absolute_linked_refs_keep_the_path_and_are_unread() {
337352
let ws = "::locator=app%40workspace%3A.";
338-
for (reference, path) in [
339-
(format!("portal:/home/me/lp{ws}"), "/home/me/lp"),
340-
(format!("link:/home/me/lp{ws}"), "/home/me/lp"),
341-
(format!("portal:C:/Users/me/lp{ws}"), "C:/Users/me/lp"),
353+
for (reference, path, kind) in [
354+
(
355+
format!("portal:/home/me/lp{ws}"),
356+
"/home/me/lp",
357+
LinkKind::Portal,
358+
),
359+
(
360+
format!("link:/home/me/lp{ws}"),
361+
"/home/me/lp",
362+
LinkKind::Link,
363+
),
364+
(
365+
format!("portal:C:/Users/me/lp{ws}"),
366+
"C:/Users/me/lp",
367+
LinkKind::Portal,
368+
),
342369
] {
343370
assert_eq!(
344371
berry_copy_ref(&reference),
345-
Some(CopyRef::Linked(path.into())),
372+
Some(CopyRef::Linked(path.into(), kind)),
346373
"{reference}"
347374
);
348375
}
349-
assert!(CopyRef::Linked("/home/me/lp".into()).read_path().is_none());
376+
assert!(CopyRef::Linked("/home/me/lp".into(), LinkKind::Portal)
377+
.read_path()
378+
.is_none());
350379
assert_eq!(
351-
CopyRef::Linked("vend/lp".into()).read_path().as_deref(),
380+
CopyRef::Linked("vend/lp".into(), LinkKind::Link)
381+
.read_path()
382+
.as_deref(),
352383
Some("vend/lp/package.json")
353384
);
354385
}
355386

356387
#[test]
357388
fn linked_identity_reads_name_and_version_from_the_manifest() {
358-
let linked = CopyRef::Linked("vend/lp".into());
389+
let linked = CopyRef::Linked("vend/lp".into(), LinkKind::Portal);
359390
assert_eq!(linked.read_path().as_deref(), Some("vend/lp/package.json"));
360391
let manifest = br#"{"name":"left-pad","version":"1.3.0"}"#;
361392
assert_eq!(
@@ -373,4 +404,24 @@ mod tests {
373404
);
374405
assert_eq!(dir.identity("1.3.0", None), None);
375406
}
407+
408+
/// #1579: a linked manifest naming a package but no version names that
409+
/// package; an empty or invalid name names none.
410+
#[test]
411+
fn linked_name_reads_a_valid_name_only() {
412+
let linked = CopyRef::Linked("vend/lp".into(), LinkKind::Portal);
413+
assert_eq!(
414+
linked
415+
.linked_name(Some(br#"{"name":"left-pad"}"#))
416+
.as_deref(),
417+
Some("left-pad")
418+
);
419+
for manifest in [&br#"{"name":""}"#[..], br#"{"name":"../x"}"#, br#"{}"#] {
420+
assert_eq!(linked.linked_name(Some(manifest)), None);
421+
}
422+
assert_eq!(
423+
CopyRef::Directory("lp".into()).linked_name(Some(br#"{"name":"left-pad"}"#)),
424+
None
425+
);
426+
}
376427
}

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

Lines changed: 72 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -275,11 +275,18 @@ impl NpmLockBackend for YarnBerryBackend {
275275
dependency name, from {origin}; the name-keyed resolutions entry \
276276
vendoring writes never reaches it, so that copy stays UNPATCHED"
277277
)
278+
} else if copy.versionless {
279+
format!(
280+
"{YARN_LOCK} entry `{key}` installs {name} under another dependency \
281+
name, from {origin}, whose package.json names no version, so it may \
282+
be {name}@{version}; the name-keyed resolutions entry vendoring \
283+
writes never reaches it, so if it is, that copy stays UNPATCHED"
284+
)
278285
} else {
279286
// An unidentified `portal:` / `link:` copy has no
280287
// version to name (#1540, #1561).
281288
let what =
282-
if matches!(copy.source, crate::formats::yarn::copy::CopyRef::Linked(_)) {
289+
if matches!(copy.source, crate::formats::yarn::copy::CopyRef::Linked(..)) {
283290
"a package".to_string()
284291
} else {
285292
format!("version {version}")
@@ -2569,12 +2576,13 @@ __metadata:
25692576
/// absolute one (#1540): named as a possible unpatched copy, while the
25702577
/// registry entry still vendors. It was dropped silently. Control: a
25712578
/// `link:` to an existing plain directory with no `package.json` is no
2572-
/// package copy and stays unnamed.
2579+
/// package copy and stays unnamed; a `portal:` to one is named, since
2580+
/// yarn refuses a manifest-less portal (#1579).
25732581
#[tokio::test]
25742582
async fn issue_1561_missing_relative_portal_and_link_targets_are_named() {
25752583
for protocol in ["portal", "link"] {
25762584
for target in ["missing", "empty", "lfs-pointer", "plain-dir"] {
2577-
let plain_dir = target == "plain-dir";
2585+
let plain_dir = target == "plain-dir" && protocol == "link";
25782586
let key = format!("lp2@{protocol}:./vend/lp::locator=vendor-spike%40workspace%3A.");
25792587
let block = format!(
25802588
"\n\"{key}\":\n version: 0.0.0-use.local\n resolution: \"{key}\"\n \
@@ -2626,6 +2634,67 @@ __metadata:
26262634
}
26272635
}
26282636

2637+
/// #1579: a `portal:` target with no `package.json` (a `.gitkeep`
2638+
/// placeholder; yarn refuses a manifest-less portal, so it is not what
2639+
/// gets installed) and a linked manifest with no `name` are unknown, and
2640+
/// one naming left-pad with no version may be left-pad: each is named as
2641+
/// a possible unpatched copy while the registry entry still vendors.
2642+
/// Controls: a `link:` to a placeholder directory (plain assets) and a
2643+
/// versionless manifest naming another package stay unnamed.
2644+
#[tokio::test]
2645+
async fn issue_1579_portal_placeholder_and_versionless_linked_manifests_are_named() {
2646+
for protocol in ["portal", "link"] {
2647+
for (target, named) in [
2648+
("gitkeep", protocol == "portal"),
2649+
("no-name", true),
2650+
("no-version-left-pad", true),
2651+
("no-version-other", false),
2652+
] {
2653+
let key = format!("lp2@{protocol}:./vend/lp::locator=vendor-spike%40workspace%3A.");
2654+
let block = format!(
2655+
"\n\"{key}\":\n version: 0.0.0-use.local\n resolution: \"{key}\"\n \
2656+
languageName: node\n linkType: soft\n"
2657+
);
2658+
let lock = format!("{B3_BEFORE_LOCK}{block}");
2659+
let fx = fixture_with(B3_BEFORE_PKG, &lock).await;
2660+
let dir = fx.tmp.path().join("vend/lp");
2661+
tokio::fs::create_dir_all(&dir).await.unwrap();
2662+
let (file, text) = match target {
2663+
"gitkeep" => (".gitkeep", ""),
2664+
"no-name" => ("package.json", r#"{"version":"1.3.0"}"#),
2665+
"no-version-left-pad" => ("package.json", r#"{"name":"left-pad"}"#),
2666+
_ => ("package.json", r#"{"name":"other-pkg"}"#),
2667+
};
2668+
tokio::fs::write(dir.join(file), text).await.unwrap();
2669+
2670+
let (result, entry, warnings) = expect_done(fx.vendor(false).await);
2671+
assert!(result.success, "{protocol}/{target}: {:?}", result.error);
2672+
assert!(entry.is_some(), "{protocol}/{target}: registry vendors");
2673+
let copies: Vec<&VendorWarning> = warnings
2674+
.iter()
2675+
.filter(|w| w.code == "vendor_copy_unpatched")
2676+
.collect();
2677+
if !named {
2678+
assert!(copies.is_empty(), "{protocol}/{target}: {warnings:?}");
2679+
continue;
2680+
}
2681+
assert_eq!(copies.len(), 1, "{protocol}/{target}: {warnings:?}");
2682+
let (what, why) = if target == "no-version-left-pad" {
2683+
("installs left-pad under", "names no version")
2684+
} else {
2685+
("installs a package under", "can't be read")
2686+
};
2687+
assert!(
2688+
copies[0].detail.contains(&format!("`\"{key}\"`"))
2689+
&& copies[0].detail.contains(what)
2690+
&& copies[0].detail.contains(why),
2691+
"{protocol}/{target}: {}",
2692+
copies[0].detail
2693+
);
2694+
}
2695+
}
2696+
}
2697+
26292698
/// #939 (review): when left-pad@1.3.0 is locked ONLY under another
26302699
/// dependency name, vendoring refuses `vendor_lock_entry_not_rewritable`
26312700
/// naming that copy, in the loop and in the download plan alike, not

0 commit comments

Comments
 (0)