Skip to content

Commit 75e49ae

Browse files
Fix yarn berry absolute portal:/link: copies (#1540) (#1551)
* Start fix for #1540 Assisted-by: Claude Code:claude-opus-5-5 * Block VEX for unread yarn portal:/link: copies A yarn berry portal: or link: dependency with an absolute path (e.g. "lp2": "portal:/home/me/left-pad") links an unread directory into node_modules. Its lock version is a placeholder, so it may be any version of any package. Socket-patch dropped it, so hosted and vendored scans reported success with no copy warning and `vex` attested not_affected while that copy installed unpatched. Such a copy is now recorded as an unidentified copy of any version: hosted scans warn redirect_copy_unpatched, vendored scans warn vendor_copy_unpatched, and `vex` withholds every package the lock wires, as it already does for an unread file: copy at its version. Fixes #1540 Assisted-by: Claude Code:claude-opus-5-5 * Test hosted scan names absolute portal copies Covers #1540 at the CLI level: a hosted yarn berry scan with an absolute portal: or link: dependency must warn redirect_copy_unpatched instead of reporting a clean success. Assisted-by: Claude Code:claude-opus-5-5 * Only unread portal copies count as any version Review follow-up for #1540. Treat a portal:/link: copy as possibly any package only when it is never read (an absolute path). A relative link: to a plain directory with no package.json is read, holds no package, and stays ignored as before, so it no longer blocks VEX for the whole lock. Also keep a Windows drive path (portal:C:/...) intact when parsing berry references, word the vendored warning for an unread portal copy without a made-up version, and simplify the expansion of unidentified copies. Assisted-by: Claude Code:claude-opus-5-5 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 1279a9a commit 75e49ae

7 files changed

Lines changed: 294 additions & 50 deletions

File tree

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

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1879,3 +1879,65 @@ async fn issue_939_berry_renamed_copy_is_named() {
18791879
}
18801880
}
18811881
}
1882+
1883+
/// #1540: a berry `portal:` / `link:` copy with an ABSOLUTE path is never
1884+
/// read (#1516), and its lock version is the placeholder
1885+
/// `0.0.0-use.local`, so it may be any version of any package. The hosted
1886+
/// scan still pins the registry entry, and names that copy as possibly
1887+
/// unpatched (`redirect_copy_unpatched`) whatever it links to; it used to
1888+
/// report a clean success while `node_modules/lp2` held unpatched bytes.
1889+
#[tokio::test]
1890+
async fn issue_1540_berry_absolute_portal_and_link_copies_are_named() {
1891+
const UUID: &str = "15401540-1540-4540-8540-154015401540";
1892+
let ws = "app%40workspace%3A.";
1893+
for protocol in ["portal", "link"] {
1894+
let spec = format!("{protocol}:/home/me/shared/left-pad");
1895+
let key = format!("lp2@{spec}::locator={ws}");
1896+
let server = MockServer::start().await;
1897+
mount_left_pad_grant(&server, UUID).await;
1898+
let mut files = BTreeMap::new();
1899+
files.insert(
1900+
"package.json".to_string(),
1901+
format!(
1902+
r#"{{"name":"app","version":"1.0.0","dependencies":{{"left-pad":"^1.3.0","lp2":"{spec}"}}}}"#
1903+
)
1904+
.into_bytes(),
1905+
);
1906+
files.insert(
1907+
"yarn.lock".to_string(),
1908+
format!(
1909+
"__metadata:\n version: 8\n cacheKey: 10c0\n\n\
1910+
\"app@workspace:.\":\n version: 0.0.0-use.local\n \
1911+
resolution: \"app@workspace:.\"\n dependencies:\n \
1912+
left-pad: \"npm:^1.3.0\"\n lp2: \"{spec}\"\n \
1913+
languageName: unknown\n linkType: soft\n\n\
1914+
\"left-pad@npm:^1.3.0\":\n version: 1.3.0\n \
1915+
resolution: \"left-pad@npm:1.3.0\"\n checksum: 10c0/{}\n \
1916+
languageName: node\n linkType: hard\n\n\
1917+
\"{key}\":\n version: 0.0.0-use.local\n resolution: \"{key}\"\n \
1918+
languageName: node\n linkType: soft\n",
1919+
"1".repeat(128)
1920+
)
1921+
.into_bytes(),
1922+
);
1923+
let output = run_engine(&server, build_input(&files, &[], options(false))).await;
1924+
let project = &output.projects[0];
1925+
assert!(project.error.is_none(), "{protocol}: {:?}", project.error);
1926+
assert_eq!(
1927+
project.redirected.len(),
1928+
1,
1929+
"{protocol}: {:?} {:#}",
1930+
project.skipped,
1931+
project.redirect
1932+
);
1933+
let details = copy_unpatched_details(project);
1934+
assert_eq!(details.len(), 1, "{protocol}: {:#}", project.redirect);
1935+
assert!(
1936+
details[0].contains(key.as_str())
1937+
&& details[0].contains("can't be read")
1938+
&& details[0].contains("UNPATCHED"),
1939+
"{protocol}: {}",
1940+
details[0]
1941+
);
1942+
}
1943+
}

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

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,13 @@ impl CopyRef {
5858
}
5959
}
6060

61+
/// A `portal:` / `link:` copy that is never read ([`CopyRef::read_path`]
62+
/// is `None`: an absolute path). Its lock version is the placeholder
63+
/// `0.0.0-use.local`, so it may be any version of any package (#1540).
64+
pub(crate) fn is_unread_linked(&self) -> bool {
65+
matches!(self, CopyRef::Linked(_)) && self.read_path().is_none()
66+
}
67+
6168
/// The `(name, version)` this copy installs, given the lock entry's
6269
/// `version` and the bytes read at [`CopyRef::read_path`]. `None` when
6370
/// the copy does not say (an unreadable or nameless manifest, a
@@ -151,7 +158,10 @@ pub(crate) fn berry_copy_ref(reference: &str) -> Option<CopyRef> {
151158
} else {
152159
return None;
153160
};
154-
let path = path.split(['#', ':']).next().unwrap_or_default();
161+
// Cut the `::locator=…` bindings and any `#` selector; a single `:` is
162+
// part of the path (a Windows drive, `portal:C:/Users/me/lp`).
163+
let path = path.split("::").next().unwrap_or_default();
164+
let path = path.split('#').next().unwrap_or_default();
155165
let rel = berry_copy_path(reference, path)?;
156166
Some(if linked {
157167
CopyRef::Linked(rel)
@@ -320,6 +330,29 @@ mod tests {
320330
assert_eq!(berry_copy_ref("workspace:packages/b"), None);
321331
}
322332

333+
/// #1540: an absolute `portal:` / `link:` path keeps its whole spelling,
334+
/// a Windows drive's `:` included, and is never read.
335+
#[test]
336+
fn absolute_linked_refs_keep_the_path_and_are_unread() {
337+
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"),
342+
] {
343+
assert_eq!(
344+
berry_copy_ref(&reference),
345+
Some(CopyRef::Linked(path.into())),
346+
"{reference}"
347+
);
348+
}
349+
let unix = CopyRef::Linked("/home/me/lp".into());
350+
assert!(unix.read_path().is_none() && unix.is_unread_linked());
351+
let relative = CopyRef::Linked("vend/lp".into());
352+
assert!(!relative.is_unread_linked());
353+
assert!(!CopyRef::Directory("/home/me/lp".into()).is_unread_linked());
354+
}
355+
323356
#[test]
324357
fn linked_identity_reads_name_and_version_from_the_manifest() {
325358
let linked = CopyRef::Linked("vend/lp".into());

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

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -276,8 +276,15 @@ impl NpmLockBackend for YarnBerryBackend {
276276
vendoring writes never reaches it, so that copy stays UNPATCHED"
277277
)
278278
} else {
279+
// An unread `portal:` / `link:` copy has no version
280+
// to name (#1540).
281+
let what = if copy.source.is_unread_linked() {
282+
"a package".to_string()
283+
} else {
284+
format!("version {version}")
285+
};
279286
format!(
280-
"{YARN_LOCK} entry `{key}` installs version {version} under another \
287+
"{YARN_LOCK} entry `{key}` installs {what} under another \
281288
dependency name, from {origin}, whose package can't be read, so it \
282289
may be {name}@{version}; the name-keyed resolutions entry vendoring \
283290
writes never reaches it, so if it is, that copy stays UNPATCHED"
@@ -2509,6 +2516,51 @@ __metadata:
25092516
}
25102517
}
25112518

2519+
/// #1540: a `portal:` / `link:` copy with an ABSOLUTE path is never read
2520+
/// (#1516), so its package and version are unknown. It is named as a
2521+
/// possible unpatched copy (`vendor_copy_unpatched`, unidentified
2522+
/// wording) whatever its target holds, never taken as proof the package
2523+
/// is installed, and the registry entry still vendors. It was dropped
2524+
/// silently.
2525+
#[tokio::test]
2526+
async fn issue_1540_absolute_portal_and_link_copies_are_named() {
2527+
for protocol in ["portal", "link"] {
2528+
for fork in ["left-pad", "other-pkg"] {
2529+
let ext = tempfile::tempdir().unwrap();
2530+
tokio::fs::write(
2531+
ext.path().join("package.json"),
2532+
format!(r#"{{"name":"{fork}","version":"1.3.0"}}"#),
2533+
)
2534+
.await
2535+
.unwrap();
2536+
let abs = ext.path().to_string_lossy().replace('\\', "/");
2537+
let key = format!("lp2@{protocol}:{abs}::locator=vendor-spike%40workspace%3A.");
2538+
let block = format!(
2539+
"\n\"{key}\":\n version: 0.0.0-use.local\n resolution: \"{key}\"\n \
2540+
languageName: node\n linkType: soft\n"
2541+
);
2542+
let lock = format!("{B3_BEFORE_LOCK}{block}");
2543+
let fx = fixture_with(B3_BEFORE_PKG, &lock).await;
2544+
2545+
let (result, entry, warnings) = expect_done(fx.vendor(false).await);
2546+
assert!(result.success, "{protocol}/{fork}: {:?}", result.error);
2547+
assert!(entry.is_some(), "{protocol}/{fork}: registry entry vendors");
2548+
let copies: Vec<&VendorWarning> = warnings
2549+
.iter()
2550+
.filter(|w| w.code == "vendor_copy_unpatched")
2551+
.collect();
2552+
assert_eq!(copies.len(), 1, "{protocol}/{fork}: {warnings:?}");
2553+
assert!(
2554+
copies[0].detail.contains(&format!("`\"{key}\"`"))
2555+
&& copies[0].detail.contains("installs a package under")
2556+
&& copies[0].detail.contains("can't be read"),
2557+
"{protocol}/{fork}: {}",
2558+
copies[0].detail
2559+
);
2560+
}
2561+
}
2562+
}
2563+
25122564
/// #939 (review): when left-pad@1.3.0 is locked ONLY under another
25132565
/// dependency name, vendoring refuses `vendor_lock_entry_not_rewritable`
25142566
/// naming that copy, in the loop and in the download plan alike, not

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

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -30,19 +30,21 @@ pub(crate) struct OtherNameCopy {
3030
pub(crate) source: CopyRef,
3131
/// The copy itself says it is `name@version`. `false` for a copy of
3232
/// that version whose package cannot be read (a missing directory, a
33-
/// tarball with no readable `package.json`, a non-registry url): it may
34-
/// be `name`, so it is named, but never taken as proof that `name` is
35-
/// installed (#1516).
33+
/// tarball with no readable `package.json`, a non-registry url, or an
34+
/// unread `portal:` / `link:` directory of any version): it may be
35+
/// `name`, so it is named, but never taken as proof that `name` is
36+
/// installed (#1516, #1540).
3637
pub(crate) identified: bool,
3738
}
3839

3940
/// Every live entry of `lock_text` (either grammar) that installs
4041
/// `name@version` under another dependency name, in lock order. A copy of
4142
/// that version whose package cannot be read is kept, unidentified: it
4243
/// may hold `name`, and leaving it out would let the scan and `vex` vouch
43-
/// for a lock that installs it unpatched (#1516). A `portal:` / `link:`
44-
/// copy whose manifest cannot be read is left out: its version is that
45-
/// manifest's, so nothing ties it to `version`.
44+
/// for a lock that installs it unpatched (#1516). So is a `portal:` /
45+
/// `link:` copy that is never read (an absolute path): its version is its
46+
/// manifest's, so it may be `version` too (#1540). One read and found to
47+
/// hold no versioned package (a `link:` to a plain directory) is left out.
4648
pub(crate) async fn other_name_copies(
4749
project_root: &Path,
4850
lock_text: &str,
@@ -108,7 +110,7 @@ pub(crate) async fn other_name_copies(
108110
let identified = match copy_identity(&source, &lock_version, bytes).await {
109111
Some((n, v)) if n == name && v == version => true,
110112
Some(_) => continue,
111-
None if matches!(source, CopyRef::Linked(_)) => continue,
113+
None if matches!(source, CopyRef::Linked(_)) && !source.is_unread_linked() => continue,
112114
None => false,
113115
};
114116
out.push(OtherNameCopy {

‎crates/socket-patch-core/src/vex/discover/mod.rs‎

Lines changed: 31 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -476,15 +476,17 @@ pub struct UnpatchedCopy {
476476
pub how: String,
477477
}
478478

479-
/// A lock entry that installs SOME package at `version` under another
480-
/// dependency name, from a copy whose package can't be read
481-
/// ([`Discovery::unidentified_copy`], #1516).
479+
/// A lock entry that installs SOME package (at `version`, when known) under
480+
/// another dependency name, from a copy whose package can't be read
481+
/// ([`Discovery::unidentified_copy`], #1516, #1540).
482482
#[derive(Debug, Clone, PartialEq, Eq)]
483483
pub struct UnidentifiedCopy {
484484
/// Root-relative lock file.
485485
pub file: PathBuf,
486-
/// The lock entry's version.
487-
pub version: String,
486+
/// The lock entry's version; `None` when the lock does not say (a yarn
487+
/// berry `portal:` / `link:` copy, whose version is its unread
488+
/// manifest's), so it may be any version (#1540).
489+
pub version: Option<String>,
488490
/// The lock entry's key, as the lock spells it.
489491
pub key: String,
490492
/// How that entry installs, completing "lock entry `<key>` …".
@@ -933,39 +935,41 @@ impl Discovery {
933935
/// directory, a tarball with no readable `package.json`). It may be any
934936
/// package at that version, so [`Discovery::contest_within_locks`]
935937
/// takes it as an unpatched copy of every package `file` wires at
936-
/// `version`: vouching for one would rest on a copy nobody read
937-
/// (#1516). Matched there, once every extractor has pushed its refs.
938-
pub(crate) fn unidentified_copy(&mut self, file: &str, version: &str, key: &str, how: &str) {
938+
/// `version` (at any version when `version` is `None`, #1540): vouching
939+
/// for one would rest on a copy nobody read (#1516). Matched there,
940+
/// once every extractor has pushed its refs.
941+
pub(crate) fn unidentified_copy(
942+
&mut self,
943+
file: &str,
944+
version: Option<&str>,
945+
key: &str,
946+
how: &str,
947+
) {
939948
self.unidentified_copies.push(UnidentifiedCopy {
940949
file: PathBuf::from(file),
941-
version: version.to_string(),
950+
version: version.map(str::to_string),
942951
key: key.to_string(),
943952
how: how.to_string(),
944953
});
945954
}
946955

947956
/// Fold [`Discovery::unidentified_copies`] into `unpatched_copies`: one
948-
/// copy per ref of the same file at the same version.
957+
/// copy per ref of the same file at the same version (at any version
958+
/// for a copy with none).
949959
fn expand_unidentified_copies(&mut self) {
950-
if self.unidentified_copies.is_empty() {
951-
return;
952-
}
953-
let mut wired: BTreeMap<(&Path, &str), BTreeSet<&str>> = BTreeMap::new();
954-
for r in &self.refs {
955-
if let Some((_, version)) = r.purl.rsplit_once('@') {
956-
wired
957-
.entry((r.source_file.as_path(), version))
958-
.or_default()
959-
.insert(r.purl.as_str());
960-
}
961-
}
962960
let mut expanded = Vec::new();
963961
for u in &self.unidentified_copies {
964-
for purl in wired
965-
.get(&(u.file.as_path(), u.version.as_str()))
966-
.into_iter()
967-
.flatten()
968-
{
962+
let purls: BTreeSet<&str> = self
963+
.refs
964+
.iter()
965+
.filter(|r| r.source_file == u.file)
966+
.filter(|r| match &u.version {
967+
Some(v) => r.purl.rsplit_once('@').is_some_and(|(_, rv)| rv == v),
968+
None => true,
969+
})
970+
.map(|r| r.purl.as_str())
971+
.collect();
972+
for purl in purls {
969973
expanded.push(UnpatchedCopy {
970974
purl: purl.to_string(),
971975
file: u.file.clone(),

0 commit comments

Comments
 (0)