diff --git a/crates/socket-patch-cli/tests/hosted_memory_engine.rs b/crates/socket-patch-cli/tests/hosted_memory_engine.rs index 6af8fa388..e42ff3958 100644 --- a/crates/socket-patch-cli/tests/hosted_memory_engine.rs +++ b/crates/socket-patch-cli/tests/hosted_memory_engine.rs @@ -1984,3 +1984,36 @@ async fn issue_1561_berry_missing_relative_portal_and_link_copies_are_named() { assert!(details.is_empty(), "{protocol}: {details:?}"); } } + +/// #1579: a `portal:` target holding only a `.gitkeep` placeholder (yarn +/// refuses a manifest-less portal, so it is not what gets installed), and a +/// linked manifest with no `name`, may be any package; one naming left-pad +/// with no version may be left-pad. The hosted scan pins the registry entry +/// and names each as possibly unpatched; it reported a clean success. +/// Controls: a `link:` to the placeholder (plain assets) and a versionless +/// manifest naming another package are no copy of left-pad. +#[tokio::test] +async fn issue_1579_berry_portal_placeholder_and_versionless_copies_are_named() { + const UUID: &str = "15791579-1579-4579-8579-157915791579"; + for protocol in ["portal", "link"] { + let spec = format!("{protocol}:./vend/lp"); + for (file, text, named) in [ + ("vend/lp/.gitkeep", "", protocol == "portal"), + ("vend/lp/package.json", r#"{"version":"1.3.0"}"#, true), + ("vend/lp/package.json", r#"{"name":"left-pad"}"#, true), + ("vend/lp/package.json", r#"{"name":"other-pkg"}"#, false), + ] { + let details = berry_linked_copy_details(UUID, &spec, &[(file, text)]).await; + if !named { + assert!(details.is_empty(), "{protocol} {text:?}: {details:?}"); + continue; + } + assert_eq!(details.len(), 1, "{protocol} {text:?}: {details:?}"); + assert!( + details[0].contains(&format!("lp2@{spec}::")) && details[0].contains("UNPATCHED"), + "{protocol} {text:?}: {}", + details[0] + ); + } + } +} diff --git a/crates/socket-patch-core/src/formats/yarn/copy.rs b/crates/socket-patch-core/src/formats/yarn/copy.rs index 3c4d5df3b..12daca80e 100644 --- a/crates/socket-patch-core/src/formats/yarn/copy.rs +++ b/crates/socket-patch-core/src/formats/yarn/copy.rs @@ -26,12 +26,21 @@ pub(crate) enum CopyRef { /// `node_modules`: the lock records the placeholder version /// `0.0.0-use.local`, so the package's name AND version are its /// `package.json`'s. - Linked(String), + Linked(String, LinkKind), /// A url tarball (`#` fragment dropped): the package is the registry /// package its `//-/-.tgz` path names. Url(String), } +/// Which berry protocol a [`CopyRef::Linked`] copy came from. yarn installs +/// a `link:` to any directory, a package or plain assets, but refuses a +/// `portal:` whose target has no `package.json` (#1579). +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum LinkKind { + Portal, + Link, +} + impl CopyRef { /// The root-relative file to read for [`CopyRef::identity`]: the tarball /// itself, or the directory's `package.json`. `None` for a url, which @@ -42,14 +51,14 @@ impl CopyRef { /// UNC path) a network share. Such a copy is never read, so its package /// stays unknown (#1516). pub(crate) fn read_path(&self) -> Option { - if let CopyRef::Tarball(rel) | CopyRef::Directory(rel) | CopyRef::Linked(rel) = self { + if let CopyRef::Tarball(rel) | CopyRef::Directory(rel) | CopyRef::Linked(rel, _) = self { if crate::utils::relpath::is_anchored(rel) { return None; } } match self { CopyRef::Tarball(rel) => Some(rel.clone()), - CopyRef::Directory(dir) | CopyRef::Linked(dir) => Some(if dir.is_empty() { + CopyRef::Directory(dir) | CopyRef::Linked(dir, _) => Some(if dir.is_empty() { "package.json".to_string() } else { format!("{dir}/package.json") @@ -70,7 +79,7 @@ impl CopyRef { let name = match self { CopyRef::Tarball(_) => tarball_manifest_name(bytes?)?, CopyRef::Directory(_) => manifest_name(bytes?)?, - CopyRef::Linked(_) => { + CopyRef::Linked(..) => { let manifest = parse_manifest(bytes?)?; let name = manifest.get("name")?.as_str()?.to_string(); let version = manifest.get("version")?.as_str()?.to_string(); @@ -81,6 +90,20 @@ impl CopyRef { Some((name, lock_version.to_string())) } + /// The package a [`CopyRef::Linked`] copy's manifest names when it + /// names no version, so [`CopyRef::identity`] can't say: the copy may + /// be any version of that package, and of no other (#1579). `None` for + /// any other copy, or a manifest that names no package (an empty or + /// invalid npm name names none, so the copy may be any package). + pub(crate) fn linked_name(&self, bytes: Option<&[u8]>) -> Option { + match self { + CopyRef::Linked(..) => { + manifest_name(bytes?).filter(|n| crate::vendor::is_safe_npm_name(n)) + } + _ => None, + } + } + /// Where the copy installs from, for a warning or diagnostic: "installs /// it from …" / "links it to …". A url is redacted, since the phrase /// may be published (a VEX document, a JSON envelope). @@ -89,7 +112,7 @@ impl CopyRef { CopyRef::Tarball(rel) | CopyRef::Directory(rel) => { format!("installs it from the user's file:{rel}") } - CopyRef::Linked(rel) => { + CopyRef::Linked(rel, _) => { let rel = if rel.is_empty() { "." } else { rel }; format!("links it to the user's directory {rel} (portal:/link:)") } @@ -142,12 +165,11 @@ pub(crate) fn berry_copy_ref(reference: &str) -> Option { )); } let (path, linked) = if let Some(path) = reference.strip_prefix("file:") { - (path, false) - } else if let Some(path) = reference - .strip_prefix("portal:") - .or_else(|| reference.strip_prefix("link:")) - { - (path, true) + (path, None) + } else if let Some(path) = reference.strip_prefix("portal:") { + (path, Some(LinkKind::Portal)) + } else if let Some(path) = reference.strip_prefix("link:") { + (path, Some(LinkKind::Link)) } else { return None; }; @@ -156,8 +178,8 @@ pub(crate) fn berry_copy_ref(reference: &str) -> Option { let path = path.split("::").next().unwrap_or_default(); let path = path.split('#').next().unwrap_or_default(); let rel = berry_copy_path(reference, path)?; - Some(if linked { - CopyRef::Linked(rel) + Some(if let Some(kind) = linked { + CopyRef::Linked(rel, kind) } else if is_tarball_path(path) { CopyRef::Tarball(rel) } else { @@ -212,13 +234,6 @@ fn copy_path(base: &str, path: &str) -> Option { )) } -/// Whether `bytes` parse as a JSON `package.json` (any content): a copy -/// whose manifest doesn't (a Git LFS pointer, an encrypted blob) can't be -/// read for its package (#1561). -pub(crate) fn is_json_manifest(bytes: &[u8]) -> bool { - parse_manifest(bytes).is_some() -} - fn parse_manifest(bytes: &[u8]) -> Option { let bytes = crate::formats::text::strip_bom_bytes(bytes); serde_json::from_slice::(bytes).ok() @@ -320,11 +335,11 @@ mod tests { ); assert_eq!( berry_copy_ref(&format!("portal:../lp{ws}")), - Some(CopyRef::Linked("packages/lp".into())) + Some(CopyRef::Linked("packages/lp".into(), LinkKind::Portal)) ); assert_eq!( berry_copy_ref("link:./vend/lp::locator=app%40workspace%3A."), - Some(CopyRef::Linked("vend/lp".into())) + Some(CopyRef::Linked("vend/lp".into(), LinkKind::Link)) ); assert_eq!(berry_copy_ref("npm:1.3.0"), None); assert_eq!(berry_copy_ref("workspace:packages/b"), None); @@ -335,27 +350,43 @@ mod tests { #[test] fn absolute_linked_refs_keep_the_path_and_are_unread() { let ws = "::locator=app%40workspace%3A."; - for (reference, path) in [ - (format!("portal:/home/me/lp{ws}"), "/home/me/lp"), - (format!("link:/home/me/lp{ws}"), "/home/me/lp"), - (format!("portal:C:/Users/me/lp{ws}"), "C:/Users/me/lp"), + for (reference, path, kind) in [ + ( + format!("portal:/home/me/lp{ws}"), + "/home/me/lp", + LinkKind::Portal, + ), + ( + format!("link:/home/me/lp{ws}"), + "/home/me/lp", + LinkKind::Link, + ), + ( + format!("portal:C:/Users/me/lp{ws}"), + "C:/Users/me/lp", + LinkKind::Portal, + ), ] { assert_eq!( berry_copy_ref(&reference), - Some(CopyRef::Linked(path.into())), + Some(CopyRef::Linked(path.into(), kind)), "{reference}" ); } - assert!(CopyRef::Linked("/home/me/lp".into()).read_path().is_none()); + assert!(CopyRef::Linked("/home/me/lp".into(), LinkKind::Portal) + .read_path() + .is_none()); assert_eq!( - CopyRef::Linked("vend/lp".into()).read_path().as_deref(), + CopyRef::Linked("vend/lp".into(), LinkKind::Link) + .read_path() + .as_deref(), Some("vend/lp/package.json") ); } #[test] fn linked_identity_reads_name_and_version_from_the_manifest() { - let linked = CopyRef::Linked("vend/lp".into()); + let linked = CopyRef::Linked("vend/lp".into(), LinkKind::Portal); assert_eq!(linked.read_path().as_deref(), Some("vend/lp/package.json")); let manifest = br#"{"name":"left-pad","version":"1.3.0"}"#; assert_eq!( @@ -373,4 +404,24 @@ mod tests { ); assert_eq!(dir.identity("1.3.0", None), None); } + + /// #1579: a linked manifest naming a package but no version names that + /// package; an empty or invalid name names none. + #[test] + fn linked_name_reads_a_valid_name_only() { + let linked = CopyRef::Linked("vend/lp".into(), LinkKind::Portal); + assert_eq!( + linked + .linked_name(Some(br#"{"name":"left-pad"}"#)) + .as_deref(), + Some("left-pad") + ); + for manifest in [&br#"{"name":""}"#[..], br#"{"name":"../x"}"#, br#"{}"#] { + assert_eq!(linked.linked_name(Some(manifest)), None); + } + assert_eq!( + CopyRef::Directory("lp".into()).linked_name(Some(br#"{"name":"left-pad"}"#)), + None + ); + } } diff --git a/crates/socket-patch-core/src/vendor/yarn_berry_lock.rs b/crates/socket-patch-core/src/vendor/yarn_berry_lock.rs index ceb6ebb66..e26e33064 100644 --- a/crates/socket-patch-core/src/vendor/yarn_berry_lock.rs +++ b/crates/socket-patch-core/src/vendor/yarn_berry_lock.rs @@ -275,11 +275,18 @@ impl NpmLockBackend for YarnBerryBackend { dependency name, from {origin}; the name-keyed resolutions entry \ vendoring writes never reaches it, so that copy stays UNPATCHED" ) + } else if copy.versionless { + format!( + "{YARN_LOCK} entry `{key}` installs {name} under another dependency \ + name, from {origin}, whose package.json names no version, so it may \ + be {name}@{version}; the name-keyed resolutions entry vendoring \ + writes never reaches it, so if it is, that copy stays UNPATCHED" + ) } else { // An unidentified `portal:` / `link:` copy has no // version to name (#1540, #1561). let what = - if matches!(copy.source, crate::formats::yarn::copy::CopyRef::Linked(_)) { + if matches!(copy.source, crate::formats::yarn::copy::CopyRef::Linked(..)) { "a package".to_string() } else { format!("version {version}") @@ -2569,12 +2576,13 @@ __metadata: /// absolute one (#1540): named as a possible unpatched copy, while the /// registry entry still vendors. It was dropped silently. Control: a /// `link:` to an existing plain directory with no `package.json` is no - /// package copy and stays unnamed. + /// package copy and stays unnamed; a `portal:` to one is named, since + /// yarn refuses a manifest-less portal (#1579). #[tokio::test] async fn issue_1561_missing_relative_portal_and_link_targets_are_named() { for protocol in ["portal", "link"] { for target in ["missing", "empty", "lfs-pointer", "plain-dir"] { - let plain_dir = target == "plain-dir"; + let plain_dir = target == "plain-dir" && protocol == "link"; let key = format!("lp2@{protocol}:./vend/lp::locator=vendor-spike%40workspace%3A."); let block = format!( "\n\"{key}\":\n version: 0.0.0-use.local\n resolution: \"{key}\"\n \ @@ -2626,6 +2634,67 @@ __metadata: } } + /// #1579: a `portal:` target with no `package.json` (a `.gitkeep` + /// placeholder; yarn refuses a manifest-less portal, so it is not what + /// gets installed) and a linked manifest with no `name` are unknown, and + /// one naming left-pad with no version may be left-pad: each is named as + /// a possible unpatched copy while the registry entry still vendors. + /// Controls: a `link:` to a placeholder directory (plain assets) and a + /// versionless manifest naming another package stay unnamed. + #[tokio::test] + async fn issue_1579_portal_placeholder_and_versionless_linked_manifests_are_named() { + for protocol in ["portal", "link"] { + for (target, named) in [ + ("gitkeep", protocol == "portal"), + ("no-name", true), + ("no-version-left-pad", true), + ("no-version-other", false), + ] { + let key = format!("lp2@{protocol}:./vend/lp::locator=vendor-spike%40workspace%3A."); + let block = format!( + "\n\"{key}\":\n version: 0.0.0-use.local\n resolution: \"{key}\"\n \ + languageName: node\n linkType: soft\n" + ); + let lock = format!("{B3_BEFORE_LOCK}{block}"); + let fx = fixture_with(B3_BEFORE_PKG, &lock).await; + let dir = fx.tmp.path().join("vend/lp"); + tokio::fs::create_dir_all(&dir).await.unwrap(); + let (file, text) = match target { + "gitkeep" => (".gitkeep", ""), + "no-name" => ("package.json", r#"{"version":"1.3.0"}"#), + "no-version-left-pad" => ("package.json", r#"{"name":"left-pad"}"#), + _ => ("package.json", r#"{"name":"other-pkg"}"#), + }; + tokio::fs::write(dir.join(file), text).await.unwrap(); + + let (result, entry, warnings) = expect_done(fx.vendor(false).await); + assert!(result.success, "{protocol}/{target}: {:?}", result.error); + assert!(entry.is_some(), "{protocol}/{target}: registry vendors"); + let copies: Vec<&VendorWarning> = warnings + .iter() + .filter(|w| w.code == "vendor_copy_unpatched") + .collect(); + if !named { + assert!(copies.is_empty(), "{protocol}/{target}: {warnings:?}"); + continue; + } + assert_eq!(copies.len(), 1, "{protocol}/{target}: {warnings:?}"); + let (what, why) = if target == "no-version-left-pad" { + ("installs left-pad under", "names no version") + } else { + ("installs a package under", "can't be read") + }; + assert!( + copies[0].detail.contains(&format!("`\"{key}\"`")) + && copies[0].detail.contains(what) + && copies[0].detail.contains(why), + "{protocol}/{target}: {}", + copies[0].detail + ); + } + } + } + /// #939 (review): when left-pad@1.3.0 is locked ONLY under another /// dependency name, vendoring refuses `vendor_lock_entry_not_rewritable` /// naming that copy, in the loop and in the download plan alike, not diff --git a/crates/socket-patch-core/src/vendor/yarn_copies.rs b/crates/socket-patch-core/src/vendor/yarn_copies.rs index 405757273..acb323bbf 100644 --- a/crates/socket-patch-core/src/vendor/yarn_copies.rs +++ b/crates/socket-patch-core/src/vendor/yarn_copies.rs @@ -13,7 +13,7 @@ use std::path::Path; use crate::formats::yarn::blocks::{berry_field, classic_field}; -use crate::formats::yarn::copy::{berry_copy_ref, classic_copy_ref, is_json_manifest, CopyRef}; +use crate::formats::yarn::copy::{berry_copy_ref, classic_copy_ref, CopyRef, LinkKind}; use crate::formats::yarn::is_berry_lock; use crate::formats::yarn::patterns::classic_key_real_name; use crate::formats::yarn::source::{classic_copy_source, CopySource}; @@ -36,6 +36,9 @@ pub(crate) struct OtherNameCopy { /// `name`, so it is named, but never taken as proof that `name` is /// installed (#1516, #1540). pub(crate) identified: bool, + /// An unidentified `portal:` / `link:` copy whose `package.json` names + /// `name` but no version (#1579), as opposed to one that can't be read. + pub(crate) versionless: bool, } /// Every live entry of `lock_text` (either grammar) that installs @@ -94,7 +97,7 @@ pub(crate) async fn other_name_copies( // Only a `portal:` / `link:` copy needs its manifest for the // version; any other copy of another version is no copy of this // one, and is never read (a `file:` tarball is unpacked to read). - if !matches!(source, CopyRef::Linked(_)) && lock_version != version { + if !matches!(source, CopyRef::Linked(..)) && lock_version != version { continue; } // A path into `.socket/` is socket-patch's own wiring, which @@ -109,52 +112,50 @@ pub(crate) async fn other_name_copies( Some(rel) => read_regular_to_bytes(&project_root.join(rel)).await.ok(), None => None, }; - let parsed = bytes.as_deref().is_some_and(is_json_manifest); + let linked_name = source.linked_name(bytes.as_deref()); let identified = match copy_identity(&source, &lock_version, bytes).await { Some((n, v)) if n == name && v == version => true, Some(_) => continue, - None if matches!(source, CopyRef::Linked(_)) - && !linked_copy_unknown(ProjectView::Disk(project_root), &source, parsed).await => + None if matches!(source, CopyRef::Linked(..)) + && !linked_copy_unknown(ProjectView::Disk(project_root), &source).await => { continue } + // A manifest naming another package but no version is no copy + // of this one. + None if linked_name.as_deref().is_some_and(|n| n != name) => continue, None => false, }; out.push(OtherNameCopy { key, source, identified, + versionless: !identified && linked_name.is_some(), }); } out } /// Whether a `portal:` / `link:` copy whose manifest named no versioned -/// package may be any version of any package. Its lock version is the +/// package may be a copy of some package. Its lock version is the /// placeholder `0.0.0-use.local`, so only its `package.json` says what it -/// is; `manifest_parsed` says that file was read and parsed as JSON. A copy -/// is unknown when it is never read (an absolute path, #1540) or its -/// target can't be read (#1561): a missing directory, an EMPTY one (the -/// placeholder git leaves for a submodule CI never checked out), or a -/// `package.json` that is no readable regular file or no JSON (a Git LFS -/// pointer, an encrypted blob). Only a non-empty directory with no -/// `package.json` at all (a `link:` to plain assets), or a manifest parsed -/// and found to name no versioned package, is no package copy. -pub(crate) async fn linked_copy_unknown( - view: ProjectView<'_>, - source: &CopyRef, - manifest_parsed: bool, -) -> bool { - let CopyRef::Linked(dir) = source else { +/// is. A copy is unknown when it is never read (an absolute path, #1540), +/// its target can't be read (#1561: a missing directory, an EMPTY one, a +/// `package.json` that is no readable regular file or no JSON), or its +/// manifest parses but names no package or no version (#1579): yarn links +/// it all the same, so it may hold any code. A `portal:` with no +/// `package.json` is unknown too: yarn refuses to install one, so the +/// directory on disk (a `.gitkeep` placeholder a fetch step fills) is not +/// what will be installed (#1579). Only a `link:` to a non-empty directory +/// with no `package.json` at all (plain assets) is no package copy. +pub(crate) async fn linked_copy_unknown(view: ProjectView<'_>, source: &CopyRef) -> bool { + let CopyRef::Linked(dir, kind) = source else { return false; }; - if manifest_parsed { - return false; - } let Some(manifest) = source.read_path() else { return true; }; - if !view.is_dir(dir) || view.exists_no_follow(&manifest).await { + if *kind == LinkKind::Portal || !view.is_dir(dir) || view.exists_no_follow(&manifest).await { return true; } view.list_dir(dir) @@ -186,7 +187,7 @@ pub(crate) fn copy_origin(source: &CopyRef) -> &'static str { match source { CopyRef::Tarball(_) => "a file: tarball", CopyRef::Directory(_) => "a file: directory, which yarn copies into node_modules", - CopyRef::Linked(_) => "a portal:/link: directory, which yarn links into node_modules", + CopyRef::Linked(..) => "a portal:/link: directory, which yarn links into node_modules", CopyRef::Url(_) => "a URL tarball", } } diff --git a/crates/socket-patch-core/src/vex/discover/mod.rs b/crates/socket-patch-core/src/vex/discover/mod.rs index 9bc92eeb9..0c02a4ee5 100644 --- a/crates/socket-patch-core/src/vex/discover/mod.rs +++ b/crates/socket-patch-core/src/vex/discover/mod.rs @@ -511,6 +511,10 @@ pub struct UnidentifiedCopy { /// berry `portal:` / `link:` copy, whose version is its unread /// manifest's), so it may be any version (#1540). pub version: Option, + /// The package the copy's manifest names when it names no version (a + /// berry `portal:` / `link:` copy, #1579): it may be any version of that + /// package only. `None` when it may be any package. + pub name: Option, /// The lock entry's key, as the lock spells it. pub key: String, /// How that entry installs, completing "lock entry `` …". @@ -977,18 +981,21 @@ impl Discovery { /// directory, a tarball with no readable `package.json`). It may be any /// package at that version, so [`Discovery::contest_within_locks`] /// takes it as an unpatched copy of every package `file` wires at - /// `version` (at any version when `version` is `None`, #1540): vouching + /// `version` (at any version when `version` is `None`, #1540; of + /// package `name` only when the copy names one, #1579): vouching /// for one would rest on a copy nobody read (#1516). Matched there, /// once every extractor has pushed its refs. pub(crate) fn unidentified_copy( &mut self, file: &str, + name: Option<&str>, version: Option<&str>, key: &str, how: &str, ) { self.unidentified_copies.push(UnidentifiedCopy { file: PathBuf::from(file), + name: name.map(str::to_string), version: version.map(str::to_string), key: key.to_string(), how: how.to_string(), @@ -1009,6 +1016,11 @@ impl Discovery { Some(v) => r.purl.rsplit_once('@').is_some_and(|(_, rv)| rv == v), None => true, }) + .filter(|r| { + u.name.as_ref().is_none_or(|n| { + crate::utils::purl::purl_parts(&r.purl).is_some_and(|(_, rn, _)| &rn == n) + }) + }) .map(|r| r.purl.as_str()) .collect(); for purl in purls { diff --git a/crates/socket-patch-core/src/vex/discover/yarn.rs b/crates/socket-patch-core/src/vex/discover/yarn.rs index 0bcbd5584..f6401483c 100644 --- a/crates/socket-patch-core/src/vex/discover/yarn.rs +++ b/crates/socket-patch-core/src/vex/discover/yarn.rs @@ -890,24 +890,23 @@ async fn record_copies(ctx: &DiscoverCtx<'_>, copies: Vec, out: &mut D Some(rel) => ctx.read_advisory_bytes(&rel).await, None => None, }; - let parsed = bytes - .as_deref() - .is_some_and(crate::formats::yarn::copy::is_json_manifest); + let linked_name = copy.source.linked_name(bytes.as_deref()); let Some((name, version)) = crate::vendor::yarn_copies::copy_identity(©.source, ©.version, bytes).await else { - let linked = matches!(copy.source, CopyRef::Linked(_)); + let linked = matches!(copy.source, CopyRef::Linked(..)); if linked - && !crate::vendor::yarn_copies::linked_copy_unknown(ctx.view, ©.source, parsed) - .await + && !crate::vendor::yarn_copies::linked_copy_unknown(ctx.view, ©.source).await { continue; } - out.unidentified_copy( - YARN_LOCK, - (!linked).then_some(copy.version.as_str()), - ©.key, - &format!( + let how = match &linked_name { + Some(name) => format!( + "{}, whose package.json names {name} but no version, so it may be any \ + version of {name} (yarn keys it by the dependency name)", + copy.source.how() + ), + None => format!( "{}, whose package can't be read, so it may be {} of any package \ (yarn keys it by the dependency name)", copy.source.how(), @@ -917,6 +916,13 @@ async fn record_copies(ctx: &DiscoverCtx<'_>, copies: Vec, out: &mut D "this version" } ), + }; + out.unidentified_copy( + YARN_LOCK, + linked_name.as_deref(), + (!linked).then_some(copy.version.as_str()), + ©.key, + &how, ); continue; }; @@ -2016,6 +2022,103 @@ mod tests { } } + /// #1579: two more `portal:` / `link:` shapes whose package is unknown. + /// A `portal:` target with no `package.json` (a `.gitkeep` placeholder a + /// fetch step fills before the install) can't be what yarn installs, + /// since yarn refuses a manifest-less portal; and a manifest with no + /// `name` may hold any package. Both contest every ref the lock wires. A + /// manifest that names a package but no version may be any version of + /// THAT package: it contests left-pad when it names left-pad, and nothing + /// when it names another package. Both were dropped as "no package + /// copy", so `vex` attested `not_affected` while yarn linked unpatched + /// left-pad@1.3.0 as `node_modules/lp2`. + #[tokio::test] + async fn issue_1579_portal_placeholder_and_versionless_linked_manifests_contest() { + let wired = berry_vendored_block("left-pad", "1.3.0", UUID_A); + for protocol in ["portal", "link"] { + for (target, contested) in [ + ("gitkeep", protocol == "portal"), + ("no-name", true), + ("no-version-left-pad", true), + ("no-version-other", false), + ] { + let p = Project::new(); + let (file, text) = match target { + "gitkeep" => ("vend/lp/.gitkeep", ""), + "no-name" => ("vend/lp/package.json", r#"{"version":"1.3.0"}"#), + "no-version-left-pad" => ("vend/lp/package.json", r#"{"name":"left-pad"}"#), + _ => ("vend/lp/package.json", r#"{"name":"other-pkg"}"#), + }; + p.write(file, text); + let descriptor = format!("lp2@{protocol}:./vend/lp::locator=app%40workspace%3A."); + p.write( + "yarn.lock", + berry(&[ + wired.clone(), + berry_block(&descriptor, "0.0.0-use.local", &descriptor, None), + ]), + ); + p.write( + "package.json", + package_json_with_resolutions(serde_json::json!({ + "left-pad": format!("file:./.socket/vendor/npm/{UUID_A}/left-pad-1.3.0.tgz"), + })), + ); + let out = run(&p).await; + if !contested { + assert_refs( + &out, + &[("pkg:npm/left-pad@1.3.0", UUID_A, WiringMode::Vendored)], + ); + continue; + } + assert!(out.refs.is_empty(), "{protocol} {target}: {:#?}", out.refs); + let why = if target == "no-version-left-pad" { + "names left-pad but no version" + } else { + "can't be read" + }; + assert!( + out.diagnostics + .iter() + .any(|d| d.code == DIAG_REF_UNATTRIBUTABLE + && d.detail.contains(&format!("lp2@{protocol}:")) + && d.detail.contains(why)), + "{protocol} {target}: {:#?}", + out.diagnostics + ); + } + } + } + + /// #1579: a copy whose manifest names a package but no version contests + /// every version of that package the lock wires, and no other package. + #[tokio::test] + async fn issue_1579_versionless_linked_manifest_contests_only_its_name() { + let descriptor = "lp2@portal:./vend/lp::locator=app%40workspace%3A."; + let p = Project::new(); + p.write("vend/lp/package.json", r#"{"name":"left-pad"}"#); + p.write( + "yarn.lock", + berry(&[ + berry_vendored_block("left-pad", "1.3.0", UUID_A), + berry_vendored_block("right-pad", "1.0.1", UUID_B), + berry_block(descriptor, "0.0.0-use.local", descriptor, None), + ]), + ); + p.write( + "package.json", + package_json_with_resolutions(serde_json::json!({ + "left-pad": format!("file:./.socket/vendor/npm/{UUID_A}/left-pad-1.3.0.tgz"), + "right-pad": format!("file:./.socket/vendor/npm/{UUID_B}/right-pad-1.0.1.tgz"), + })), + ); + assert_refs( + &run(&p).await, + &[("pkg:npm/right-pad@1.0.1", UUID_B, WiringMode::Vendored)], + ); + } + /// #1540 control: a RELATIVE `link:` to a plain directory (no /// `package.json`, which `link:` allows) is read and holds no package, /// so it contests nothing. diff --git a/docs/ecosystems.md b/docs/ecosystems.md index 30f463221..c35f6d056 100644 --- a/docs/ecosystems.md +++ b/docs/ecosystems.md @@ -302,9 +302,13 @@ Each Legacy format has an upgrade path and an undo path. Both work in v5: the lock is untrusted input; a `portal:` / `link:` copy whose `package.json` isn't read (an absolute path, or a relative target that is missing or empty, such as a git submodule CI didn't check out, or whose `package.json` is no JSON, such as a Git LFS - pointer) has no version either, so it may be any version of any package and `vex` - attests nothing that lock wires. A `link:` to a non-empty directory with no - `package.json` is not a package copy. Use a relative path to let the copy be read. Hosted mode still pins the + pointer, or whose `package.json` names no package) has no version either, so it may + be any version of any package and `vex` attests nothing that lock wires. One whose + `package.json` names a package but no version may be any version of that package. A + `portal:` to a directory with no `package.json` (a `.gitkeep` placeholder a fetch step + fills) is unknown too, since yarn refuses to install a portal without one; only a + `link:` to a non-empty directory with no `package.json` is not a package copy. Use a + relative path to let the copy be read. Hosted mode still pins the registry entry and names the copy (`redirect_copy_unpatched`, unless a yarn classic skip warning already names it); vendored mode still wires the registry entry and names it (`vendor_copy_unpatched` for berry; the yarn classic codes below for classic); and