From bf925d92036d528ca8770f8580648fc6278e7f8a Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 02:22:37 +0000 Subject: [PATCH 1/7] Start fix for #1516 Assisted-by: Claude Code:claude-opus-5-5 From 97f5f3108d8db1e98ccaeb78cf8e7d0efc1dd863 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 02:29:16 +0000 Subject: [PATCH 2/7] Read yarn copies outside root or package/ dir A renamed yarn copy from a file: path outside the project, a tarball whose top directory isn't package/, or a non-registry URL was taken for "not this package". The scan then succeeded and VEX attested not_affected while that copy installed unpatched (#1516). Read outside-root copies where yarn installs them from, strip a tarball's first path segment as yarn does, and fail closed: an unreadable copy of the same version is named as a possible copy and blocks attestation of that version from the lock. WIP: tests written, not yet run. Assisted-by: Claude Code:claude-opus-5-5 --- .../tests/e2e_redirect_yarn_classic_build.rs | 126 ++++++++++++++++ .../src/formats/yarn/copy.rs | 81 +++++++++-- crates/socket-patch-core/src/patch/package.rs | 38 +++++ .../src/vendor/yarn_berry_lock.rs | 39 +++-- .../src/vendor/yarn_classic_lock.rs | 133 ++++++++++++++--- .../src/vendor/yarn_copies.rs | 45 ++++-- .../socket-patch-core/src/vex/discover/mod.rs | 26 +++- .../src/vex/discover/yarn.rs | 136 +++++++++++++++++- 8 files changed, 567 insertions(+), 57 deletions(-) diff --git a/crates/socket-patch-cli/tests/e2e_redirect_yarn_classic_build.rs b/crates/socket-patch-cli/tests/e2e_redirect_yarn_classic_build.rs index ae4599e04..d193403bb 100644 --- a/crates/socket-patch-cli/tests/e2e_redirect_yarn_classic_build.rs +++ b/crates/socket-patch-cli/tests/e2e_redirect_yarn_classic_build.rs @@ -1481,6 +1481,132 @@ async fn classic_file_directory_copy_under_another_name_is_named_and_not_atteste ); } +/// #1516: the #1236 copy, but from a directory OUTSIDE the project root +/// (`"lp2": "file:../ext/lpdir"`, shared code in a non-workspace +/// monorepo). yarn 1 installs it into `node_modules/lp2` all the same, so +/// it stays unpatched. `scan --mode hosted` must name the copy +/// (`redirect_copy_unpatched`), and neither its in-run VEX nor a lock-only +/// `vex` may attest left-pad not_affected. +#[tokio::test(flavor = "multi_thread")] +#[serial_test::serial] +async fn classic_outside_root_copy_under_another_name_is_named_and_not_attested() { + if !require_yarn_classic("e2e_redirect_yarn_classic_build (outside-root other-name file:)", |c| { + cache_env::isolate(c); + }) { + return; + } + let tmp = tempfile::tempdir().unwrap(); + let proj = tmp.path().join("proj"); + let copy = tmp.path().join("ext").join("lpdir"); + std::fs::create_dir_all(©).unwrap(); + std::fs::write( + copy.join("package.json"), + format!(r#"{{"name":"{DEP}","version":"{DEP_VERSION}","main":"index.js"}}"#), + ) + .unwrap(); + let orig: &[u8] = b"module.exports = function leftPad(s) { return s; };\n"; + std::fs::write(copy.join("index.js"), orig).unwrap(); + std::fs::write( + proj.join("package.json"), + format!( + r#"{{"name":"outside-root-classic","version":"0.0.0","private":true,"dependencies":{{"{DEP}":"{DEP_VERSION}","lp2":"file:../ext/lpdir"}}}}"# + ), + ) + .unwrap(); + let cache = tmp.path().join("yarn-cache"); + let install = corepack( + &proj, + &yarn_classic(), + &["install", "--no-progress"], + &[("YARN_CACHE_FOLDER", cache.to_str().unwrap())], + ); + if !install.status.success() { + skip!( + "(outside-root other-name file:): fixture `yarn install` failed:\n{}", + String::from_utf8_lossy(&install.stderr) + ); + return; + } + let lock_pristine = std::fs::read_to_string(proj.join("yarn.lock")).unwrap(); + assert!( + lock_pristine.contains("lp2@file:../ext/lpdir"), + "fixture must lock the copy under its dependency name:\n{lock_pristine}" + ); + + let installed_dir = proj.join("node_modules").join(DEP); + let installed_orig = std::fs::read(installed_dir.join("index.js")).unwrap(); + let patched: Vec = [MARKER.as_bytes(), &installed_orig].concat(); + let tgz_path = tmp.path().join("patched.tgz"); + build_patched_tgz(&installed_dir, &patched, &tgz_path); + let tgz = std::fs::read(&tgz_path).unwrap(); + let server = mock_hosted_grant(&tgz, &installed_orig, &patched, "other-name fixture").await; + + let api_url = server.uri(); + let api = [ + "--api-url", + api_url.as_str(), + "--org", + ORG, + "--api-token", + "fake", + ]; + let mut args = vec![ + "scan", + "--mode", + "hosted", + "--json", + "--yes", + "--cwd", + proj.to_str().unwrap(), + "--vex", + "out.vex.json", + "--vex-product", + PRODUCT, + ]; + args.extend(api); + let (code, stdout, stderr) = run_socket(&proj, &args); + println!("scan exit {code}\nstdout:\n{stdout}\nstderr:\n{stderr}"); + let env: serde_json::Value = serde_json::from_str(&stdout) + .unwrap_or_else(|e| panic!("scan --json output is not JSON: {e}\n{stdout}\n{stderr}")); + let lock = std::fs::read_to_string(proj.join("yarn.lock")).unwrap(); + assert!( + lock.contains(&format!("{UUID}/{DEP}-{DEP_VERSION}.tgz")), + "the registry left-pad block must still be pinned:\n{lock}" + ); + assert!( + env.to_string().contains("redirect_copy_unpatched"), + "the outside-root copy must be named: {env}" + ); + assert!(env.to_string().contains("lp2@file:../ext/lpdir"), "{env}"); + let vex = std::fs::read_to_string(proj.join("out.vex.json")).unwrap_or_default(); + assert!( + !vex.contains("not_affected"), + "the in-run VEX must not attest left-pad:\n{vex}\n{env}" + ); + + // Lock-only: with node_modules gone, `vex` reads only the lock. + std::fs::remove_dir_all(proj.join("node_modules")).unwrap(); + let mut args = vec![ + "vex", + "--cwd", + proj.to_str().unwrap(), + "--output", + "lock-only.vex.json", + "--product", + PRODUCT, + "--patch-server-url", + api_url.as_str(), + ]; + args.extend(api); + let (code, stdout, stderr) = run_socket(&proj, &args); + println!("vex exit {code}\nstdout:\n{stdout}\nstderr:\n{stderr}"); + let vex = std::fs::read_to_string(proj.join("lock-only.vex.json")).unwrap_or_default(); + assert!( + !vex.contains("not_affected"), + "lock-only vex must not attest left-pad:\n{vex}\n{stdout}\n{stderr}" + ); +} + /// A mock patch API granting one hosted patch of `DEP@DEP_VERSION` whose /// tarball is `tgz` (`index.js` from `orig` to `patched`). async fn mock_hosted_grant(tgz: &[u8], orig: &[u8], patched: &[u8], title: &str) -> MockServer { diff --git a/crates/socket-patch-core/src/formats/yarn/copy.rs b/crates/socket-patch-core/src/formats/yarn/copy.rs index ec1ea9cd1..1cc871e9b 100644 --- a/crates/socket-patch-core/src/formats/yarn/copy.rs +++ b/crates/socket-patch-core/src/formats/yarn/copy.rs @@ -93,9 +93,10 @@ impl CopyRef { /// The copy a classic block installs from its key patterns and `resolved`: /// a `file:` path in a key pattern (relative to the root, where yarn 1 -/// writes it), else an http(s) `resolved`. `None` for anything else, or a -/// `file:` path that leaves the root. The caller decides which blocks are -/// copies at all ([`super::source::classic_copy_source`]). +/// writes it, and read there even when it leaves the root: yarn installs +/// `file:../shared/left-pad` all the same, #1516), else an http(s) +/// `resolved`. `None` for anything else. The caller decides which blocks +/// are copies at all ([`super::source::classic_copy_source`]). pub(crate) fn classic_copy_ref(patterns: &[String], resolved: Option<&str>) -> Option { let file = patterns.iter().find_map(|p| { let (_, range) = split_classic_pattern(p)?; @@ -104,7 +105,7 @@ pub(crate) fn classic_copy_ref(patterns: &[String], resolved: Option<&str>) -> O match file { Some(path) => { let path = path.split('#').next().unwrap_or_default(); - let rel = crate::utils::cargo_workspace::normalize_rel("", path)?; + let rel = copy_path("", path)?; Some(if is_tarball_path(path) { CopyRef::Tarball(rel) } else { @@ -122,8 +123,8 @@ pub(crate) fn classic_copy_ref(patterns: &[String], resolved: Option<&str>) -> O /// The copy a berry locator `reference` installs from: a `file:` tarball or /// directory, a `portal:` / `link:` directory (each relative to the /// workspace its `locator=` binding names), or an http(s) url. `None` for -/// any other protocol (`npm:`, `workspace:`, `patch:`, `exec:`, git), or a -/// path that leaves the root. +/// any other protocol (`npm:`, `workspace:`, `patch:`, `exec:`, git). A +/// path that leaves the root is still read there ([`copy_path`]). pub(crate) fn berry_copy_ref(reference: &str) -> Option { if reference.starts_with("http") { return Some(CopyRef::Url( @@ -153,8 +154,8 @@ pub(crate) fn berry_copy_ref(reference: &str) -> Option { /// The root-relative path a berry `file:` / `portal:` / `link:` reference /// names: relative to the workspace in its `locator=` binding -/// (`b@workspace:packages/b`), or to the root when it has none. `None` when -/// it leaves the root. +/// (`b@workspace:packages/b`), or to the root when it has none +/// ([`copy_path`]). fn berry_copy_path(reference: &str, path: &str) -> Option { let workspace = reference .split_once("::") @@ -171,7 +172,29 @@ fn berry_copy_path(reference: &str, path: &str) -> Option { } else { workspace }; - crate::utils::cargo_workspace::normalize_rel(&workspace, path) + copy_path(&workspace, path) +} + +/// Where a copy's `path`, relative to the root-relative directory `base`, +/// is read from: normalized and root-relative, keeping one leading `../` +/// per level it climbs above the root (`../shared/left-pad`), or as +/// spelled when it is absolute. yarn installs a copy from outside the +/// project as readily as one inside it, so dropping such a path would +/// leave the copy unread and its package unattributed (#1516). `None` +/// only for an empty path. +fn copy_path(base: &str, path: &str) -> Option { + if path.is_empty() { + return None; + } + if crate::utils::relpath::is_anchored(path) { + return Some(path.to_string()); + } + let joined = if base.is_empty() { + path.to_string() + } else { + format!("{base}/{path}") + }; + Some(crate::utils::relpath::normalize_rel_keeping_escapes(&joined)) } fn parse_manifest(bytes: &[u8]) -> Option { @@ -181,7 +204,7 @@ fn parse_manifest(bytes: &[u8]) -> Option { /// `name` of the `package.json` inside an npm tarball. fn tarball_manifest_name(bytes: &[u8]) -> Option { - let map = crate::patch::package::read_archive_bytes_to_map(bytes).ok()?; + let map = crate::patch::package::read_archive_bytes_to_map_any_root(bytes).ok()?; manifest_name(map.get("package.json")?) } @@ -215,7 +238,39 @@ mod tests { "https://registry.npmjs.org/left-pad/-/left-pad-1.3.0.tgz".into() )) ); - assert_eq!(classic_copy_ref(&pats(&["lp2@file:../out"]), None), None); + // #1516: yarn installs a copy from outside the root all the same, + // so it is read there, not dropped. + assert_eq!( + classic_copy_ref(&pats(&["lp2@file:../ext/lpdir"]), None), + Some(CopyRef::Directory("../ext/lpdir".into())) + ); + assert_eq!( + classic_copy_ref( + &pats(&["lp2@file:../shared/lp.tgz"]), + Some("file:../shared/lp.tgz#abc") + ), + Some(CopyRef::Tarball("../shared/lp.tgz".into())) + ); + assert_eq!( + classic_copy_ref(&pats(&["lp2@file:/abs/lpdir"]), None), + Some(CopyRef::Directory("/abs/lpdir".into())) + ); + } + + /// #1516: yarn strips a `file:` tarball's first path segment whatever + /// its name, so a `git archive --prefix=left-pad-1.3.0/` tarball names + /// its package like a `package/` one. + #[test] + fn tarball_identity_reads_any_root_directory() { + let tarball = CopyRef::Tarball("lp.tgz".into()); + for root in ["package", "left-pad-1.3.0"] { + let bytes = crate::vex::discover::testing::npm_tgz_rooted(root, "left-pad", "1.3.0"); + assert_eq!( + tarball.identity("1.3.0", Some(&bytes)), + Some(("left-pad".into(), "1.3.0".into())), + "{root}/" + ); + } } #[test] @@ -227,8 +282,8 @@ mod tests { ); assert_eq!( berry_copy_ref(&format!("file:../../../lp.tgz#../../../lp.tgz{ws}")), - None, - "three levels up from packages/b leaves the root" + Some(CopyRef::Tarball("../lp.tgz".into())), + "three levels up from packages/b leaves the root, and is read there (#1516)" ); assert_eq!( berry_copy_ref(&format!("file:../forks/lp.tgz#../forks/lp.tgz{ws}")), diff --git a/crates/socket-patch-core/src/patch/package.rs b/crates/socket-patch-core/src/patch/package.rs index 39f1183d9..d2f2b9acf 100644 --- a/crates/socket-patch-core/src/patch/package.rs +++ b/crates/socket-patch-core/src/patch/package.rs @@ -143,6 +143,44 @@ pub fn read_archive_bytes_to_map_strict( read_archive_from_reader(bytes, true, true) } +/// [`read_archive_bytes_to_map`] keyed the way npm-family installers +/// extract a tarball: the first path segment stripped whatever its name +/// (`package/`, `left-pad-1.3.0/` from `git archive --prefix`, a GitHub +/// release's `-/`), where the lenient decoder strips only a +/// literal `package/` (#1516). A member that two different roots both +/// spell is dropped: which one the installer keeps depends on archive +/// order, which the map does not record. Members directly at the archive +/// root (no first segment to strip) are dropped too. +pub(crate) fn read_archive_bytes_to_map_any_root( + bytes: &[u8], +) -> Result>, ArchiveError> { + let raw = read_archive_from_reader(bytes, false, false)?; + let mut out: HashMap> = HashMap::new(); + let mut ambiguous: std::collections::HashSet = std::collections::HashSet::new(); + let mut roots: HashMap = HashMap::new(); + for (path, data) in raw { + let path = path.trim_start_matches("./"); + let Some((root, rest)) = path.split_once('/') else { + continue; + }; + let rest = rest.trim_start_matches('/'); + if rest.is_empty() || ambiguous.contains(rest) { + continue; + } + match roots.get(rest) { + Some(seen) if seen != root => { + out.remove(rest); + ambiguous.insert(rest.to_string()); + } + _ => { + roots.insert(rest.to_string(), root.to_string()); + out.insert(rest.to_string(), data); + } + } + } + Ok(out) +} + /// Read a source distribution without npm's `package/` prefix convention. /// Uses the same strict member checks and decompression limits. pub(crate) fn read_sdist_tar_bytes_to_map_strict( 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 167698af4..84571be78 100644 --- a/crates/socket-patch-core/src/vendor/yarn_berry_lock.rs +++ b/crates/socket-patch-core/src/vendor/yarn_berry_lock.rs @@ -244,8 +244,14 @@ impl NpmLockBackend for YarnBerryBackend { super::yarn_copies::other_name_copies(project_root, &lock_text, name, version).await; // The package IS installed, only under other names, and `yarn // install` can't re-lock those copies: not "not found" (#939). - if scan.target.is_none() && scan.alias_keys.is_empty() && !other_name.is_empty() { - let keys: Vec<&str> = other_name.iter().map(|(key, _)| key.as_str()).collect(); + // Only a copy that says it is `name@version` proves that; an + // unreadable one may be another package (#1516). + let keys: Vec<&str> = other_name + .iter() + .filter(|c| c.identified) + .map(|c| c.key.as_str()) + .collect(); + if scan.target.is_none() && scan.alias_keys.is_empty() && !keys.is_empty() { return Err(Box::new(refused( "vendor_lock_entry_not_rewritable", format!( @@ -257,15 +263,25 @@ impl NpmLockBackend for YarnBerryBackend { ), ))); } - for (key, source) in other_name { + for copy in other_name { + let key = ©.key; + let origin = super::yarn_copies::copy_origin(©.source); warnings.push(VendorWarning::new( "vendor_copy_unpatched", - format!( - "{YARN_LOCK} entry `{key}` installs {name}@{version} under another \ - dependency name, from {}; the name-keyed resolutions entry vendoring \ - writes never reaches it, so that copy stays UNPATCHED", - super::yarn_copies::copy_origin(&source) - ), + if copy.identified { + format!( + "{YARN_LOCK} entry `{key}` installs {name}@{version} under another \ + dependency name, from {origin}; the name-keyed resolutions entry \ + vendoring writes never reaches it, so that copy stays UNPATCHED" + ) + } else { + format!( + "{YARN_LOCK} entry `{key}` installs version {version} under another \ + dependency name, from {origin}, whose package can't be read, so it \ + may be {name}@{version}; the name-keyed resolutions entry vendoring \ + writes never reaches it, so if it is, that copy stays UNPATCHED" + ) + }, )); } let (target_idx, target_is_ours) = target_gate(&scan, name, version)?; @@ -730,14 +746,15 @@ pub(crate) async fn preflight_packages( let aliased = scan_berry_target(&blocks, &coords.name, &coords.version) .is_ok_and(|scan| !scan.alias_keys.is_empty()); if !aliased - && !super::yarn_copies::other_name_copies( + && super::yarn_copies::other_name_copies( project_root, &lock_text, &coords.name, &coords.version, ) .await - .is_empty() + .iter() + .any(|c| c.identified) { *gate = Err("vendor_lock_entry_not_rewritable"); } diff --git a/crates/socket-patch-core/src/vendor/yarn_classic_lock.rs b/crates/socket-patch-core/src/vendor/yarn_classic_lock.rs index 523e68978..9e6dd4ede 100644 --- a/crates/socket-patch-core/src/vendor/yarn_classic_lock.rs +++ b/crates/socket-patch-core/src/vendor/yarn_classic_lock.rs @@ -127,7 +127,7 @@ impl NpmLockBackend for YarnClassicBackend { only_other_name_copies(o, &blocks, &coords.name, &coords.version, &other_name) })?; warnings.extend(skipped); - warnings.extend(other_name); + warnings.extend(other_name.into_iter().map(|(w, _)| w)); Ok(YarnClassicPlan { text, candidate_keys, @@ -384,29 +384,39 @@ fn rewritable_candidates( /// (`"lp2@file:./lpdir"`), so which package it is comes from the copy /// itself ([`super::yarn_copies::other_name_copies`]). No re-lock of `name` /// reaches that copy, so it stays unpatched; the registry blocks are still -/// wired. +/// wired. Each warning comes with whether the copy was identified as +/// `name@version`; one whose package cannot be read is named as a copy it +/// may be (#1516). async fn other_name_copy_warnings( project_root: &Path, lock_text: &str, name: &str, version: &str, -) -> Vec { +) -> Vec<(VendorWarning, bool)> { super::yarn_copies::other_name_copies(project_root, lock_text, name, version) .await .into_iter() - .map(|(key, source)| { - let code = match source { + .map(|copy| { + let code = match copy.source { crate::formats::yarn::copy::CopyRef::Directory(_) => "vendor_link_entry_skipped", _ => "vendor_yarn_classic_non_registry_entry_skipped", }; - VendorWarning::new( - code, + let key = ©.key; + let origin = super::yarn_copies::copy_origin(©.source); + let detail = if copy.identified { format!( "lock entry `{key}` installs {name}@{version} under another dependency \ - name, from {}; vendoring can't rewire it, so this copy stays UNPATCHED", - super::yarn_copies::copy_origin(&source) - ), - ) + name, from {origin}; vendoring can't rewire it, so this copy stays UNPATCHED" + ) + } else { + format!( + "lock entry `{key}` installs version {version} under another dependency \ + name, from {origin}, whose package can't be read, so it may be \ + {name}@{version}; vendoring can't rewire it, so if it is, this copy stays \ + UNPATCHED" + ) + }; + (VendorWarning::new(code, detail), copy.identified) }) .collect() } @@ -423,15 +433,21 @@ fn only_other_name_copies( blocks: &[LockBlock], name: &str, version: &str, - other_name: &[VendorWarning], + other_name: &[(VendorWarning, bool)], ) -> Box { - if other_name.is_empty() + // Only a copy that says it is `name@version` proves the package is + // installed; an unreadable one keeps the "not found" remedy. + let details: Vec<&str> = other_name + .iter() + .filter(|(_, identified)| *identified) + .map(|(w, _)| w.detail.as_str()) + .collect(); + if details.is_empty() || has_own_name_block(blocks, name, version) || super::npm_common::refusal_code(&refusal) != "vendor_lock_entry_not_found" { return refusal; } - let details: Vec<&str> = other_name.iter().map(|w| w.detail.as_str()).collect(); Box::new(refused( "vendor_lock_entry_not_rewritable", format!( @@ -505,9 +521,10 @@ pub(crate) async fn preflight_packages( continue; }; if !has_own_name_block(&blocks, &coords.name, &coords.version) - && !other_name_copy_warnings(project_root, &text, &coords.name, &coords.version) + && other_name_copy_warnings(project_root, &text, &coords.name, &coords.version) .await - .is_empty() + .iter() + .any(|(_, identified)| *identified) { *gate = Err("vendor_lock_entry_not_rewritable"); } @@ -1946,6 +1963,90 @@ wow@^1.0.0: } } + /// #1516: renamed copies #1236's reader could not read were taken for + /// "not left-pad" and vendoring reported success without naming them, + /// while `node_modules/lp2` installed left-pad@1.3.0 unpatched: a + /// `file:` directory outside the root, a tarball whose root directory + /// is not `package/`, and a non-registry url. Each is named now (the + /// url one as a copy it may be). Control: the outside-root directory + /// holding another package is not named. + #[tokio::test] + async fn issue_1516_unread_other_name_copies_are_named() { + // A sibling directory of the project, spelled `..//lpdir` + // the way yarn records it. + let ext = tempfile::tempdir().unwrap(); + let ext_name = ext.path().file_name().unwrap().to_str().unwrap().to_string(); + let extra = format!( + "\n\"lp2@file:../{ext_name}/lpdir\":\n version \"1.3.0\"\n\n\ + \"lp3@file:./lp.tgz\":\n version \"1.3.0\"\n resolved \"file:./lp.tgz#cccc\"\n\n\ + \"lp4@http://127.0.0.1:8799/lp.tgz\":\n version \"1.3.0\"\n \ + resolved \"http://127.0.0.1:8799/lp.tgz#dddd\"\n" + ); + let lock = format!("{Y2_BEFORE}{extra}"); + for (fork, named) in [("left-pad", true), ("other-pkg", false)] { + let fx = fixture_with_lock(&lock).await; + // Same-parent tempdirs: `../` from the root is `ext`. + assert_eq!(fx.root().parent(), ext.path().parent()); + tokio::fs::create_dir_all(ext.path().join("lpdir")) + .await + .unwrap(); + tokio::fs::write( + ext.path().join("lpdir/package.json"), + format!(r#"{{"name":"{fork}","version":"1.3.0"}}"#), + ) + .await + .unwrap(); + tokio::fs::write( + fx.root().join("lp.tgz"), + crate::vex::discover::testing::npm_tgz_rooted("left-pad-1.3.0", fork, "1.3.0"), + ) + .await + .unwrap(); + let (result, entry, warnings) = expect_done(fx.vendor(false).await); + assert!(result.success, "{:?}", result.error); + assert_eq!(entry.unwrap().wiring.len(), 1, "the registry block"); + let count = |code: &str, key: &str| { + warnings + .iter() + .filter(|w| w.code == code && w.detail.contains(key)) + .count() + }; + assert_eq!( + count("vendor_link_entry_skipped", "lp2@"), + usize::from(named), + "{fork} outside-root dir: {warnings:?}" + ); + assert_eq!( + count("vendor_yarn_classic_non_registry_entry_skipped", "lp3@"), + usize::from(named), + "{fork} left-pad-1.3.0/ tarball: {warnings:?}" + ); + // The url names no package: named as a copy it may be, either way. + assert!( + warnings.iter().any(|w| { + w.code == "vendor_yarn_classic_non_registry_entry_skipped" + && w.detail.contains("lp4@") + && w.detail.contains("may be left-pad@1.3.0") + }), + "{fork} url: {warnings:?}" + ); + assert!(fx.lock_text().await.contains(extra.trim_start())); + } + } + + /// #1516: a copy whose package can't be read is never proof that the + /// package is installed, so a lock whose only other-name entry is an + /// unreadable url copy keeps the "not found" refusal and its remedy. + #[tokio::test] + async fn issue_1516_unread_copy_alone_is_not_found() { + let lock = "# yarn lockfile v1\n\n\n\"lp4@http://127.0.0.1:8799/lp.tgz\":\n \ + version \"1.3.0\"\n resolved \"http://127.0.0.1:8799/lp.tgz#dddd\"\n"; + let fx = fixture_with_lock(lock).await; + expect_refused(fx.vendor(false).await, "vendor_lock_entry_not_found"); + let pre = preflight_packages(fx.root(), &[("pkg:npm/left-pad@1.3.0", &fx.record)]).await; + assert_eq!(pre, vec![Err("vendor_lock_entry_not_found")]); + } + /// #1236 (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 383c5985c..038d45969 100644 --- a/crates/socket-patch-core/src/vendor/yarn_copies.rs +++ b/crates/socket-patch-core/src/vendor/yarn_copies.rs @@ -21,16 +21,34 @@ use crate::utils::fs::read_regular_to_bytes; use super::lock_inventory::yarn::{berry_entries, classic_entries}; -/// `(lock key, copy)` of every live entry of `lock_text` (either grammar) -/// that installs `name@version` under another dependency name, in lock -/// order. A copy whose package cannot be read (a missing directory, an -/// unreadable tarball, a non-registry url) is left out. +/// A lock entry under another dependency name that installs, or may +/// install, `name@version` ([`other_name_copies`]). +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct OtherNameCopy { + /// The lock entry's key, as the lock spells it. + pub(crate) key: String, + pub(crate) source: CopyRef, + /// The copy itself says it is `name@version`. `false` for a copy of + /// that version whose package cannot be read (a missing directory, a + /// tarball with no readable `package.json`, a non-registry url): it may + /// be `name`, so it is named, but never taken as proof that `name` is + /// installed (#1516). + pub(crate) identified: bool, +} + +/// Every live entry of `lock_text` (either grammar) that installs +/// `name@version` under another dependency name, in lock order. A copy of +/// that version whose package cannot be read is kept, unidentified: it +/// may hold `name`, and leaving it out would let the scan and `vex` vouch +/// for a lock that installs it unpatched (#1516). A `portal:` / `link:` +/// copy whose manifest cannot be read is left out: its version is that +/// manifest's, so nothing ties it to `version`. pub(crate) async fn other_name_copies( project_root: &Path, lock_text: &str, name: &str, version: &str, -) -> Vec<(String, CopyRef)> { +) -> Vec { let candidates: Vec<(String, CopyRef, String)> = if is_berry_lock(lock_text) { berry_entries(lock_text) .entries @@ -87,12 +105,17 @@ pub(crate) async fn other_name_copies( Some(rel) => read_regular_to_bytes(&project_root.join(rel)).await.ok(), None => None, }; - if copy_identity(&source, &lock_version, bytes) - .await - .is_some_and(|(n, v)| n == name && v == version) - { - out.push((key, source)); - } + 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(_)) => continue, + None => false, + }; + out.push(OtherNameCopy { + key, + source, + identified, + }); } out } diff --git a/crates/socket-patch-core/src/vex/discover/mod.rs b/crates/socket-patch-core/src/vex/discover/mod.rs index 63cb35d7a..ab16ed320 100644 --- a/crates/socket-patch-core/src/vex/discover/mod.rs +++ b/crates/socket-patch-core/src/vex/discover/mod.rs @@ -2976,6 +2976,12 @@ pub(crate) mod testing { /// A minimal npm tarball (`package/package.json` naming /// `name@version`) — a user's `file:` tarball copy. pub(crate) fn npm_tgz(name: &str, version: &str) -> Vec { + npm_tgz_rooted("package", name, version) + } + + /// [`npm_tgz`] with the manifest under `root/` instead of `package/` + /// (a `git archive --prefix` or GitHub release tarball, #1516). + pub(crate) fn npm_tgz_rooted(root: &str, name: &str, version: &str) -> Vec { let manifest = format!(r#"{{"name":"{name}","version":"{version}"}}"#); let mut tar = tar::Builder::new(flate2::write::GzEncoder::new( Vec::new(), @@ -2985,7 +2991,7 @@ pub(crate) mod testing { header.set_size(manifest.len() as u64); header.set_mode(0o644); header.set_cksum(); - tar.append_data(&mut header, "package/package.json", manifest.as_bytes()) + tar.append_data(&mut header, format!("{root}/package.json"), manifest.as_bytes()) .unwrap(); tar.into_inner().unwrap().finish().unwrap() } @@ -3018,14 +3024,26 @@ pub(crate) mod testing { /// A throwaway project root. pub(crate) struct Project { - dir: tempfile::TempDir, + _dir: tempfile::TempDir, + root: PathBuf, origins: Vec, } impl Project { pub(crate) fn new() -> Self { + Self::nested("") + } + + /// A project rooted at `sub/` inside the throwaway directory, so a + /// fixture can write files beside it (`../ext/…`) without leaving + /// the tempdir (#1516). + pub(crate) fn nested(sub: &str) -> Self { + let dir = tempfile::tempdir().expect("create tempdir"); + let root = dir.path().join(sub); + std::fs::create_dir_all(&root).expect("create project root"); Project { - dir: tempfile::tempdir().expect("create tempdir"), + _dir: dir, + root, origins: Vec::new(), } } @@ -3038,7 +3056,7 @@ pub(crate) mod testing { } pub(crate) fn root(&self) -> &Path { - self.dir.path() + &self.root } /// Write `content` at root-relative `rel`, creating parents. diff --git a/crates/socket-patch-core/src/vex/discover/yarn.rs b/crates/socket-patch-core/src/vex/discover/yarn.rs index 649aeb0eb..f039aa612 100644 --- a/crates/socket-patch-core/src/vex/discover/yarn.rs +++ b/crates/socket-patch-core/src/vex/discover/yarn.rs @@ -82,6 +82,9 @@ //! (a sibling checkout's artifacts) and is skipped silently, as in the npm //! extractor. +use std::collections::BTreeSet; +use std::path::Path; + use serde_json::Value; use super::{ @@ -857,8 +860,13 @@ struct LockCopy { /// the name the depender gave it, so `"lp2": "file:left-pad-1.3.0.tgz"` /// locks as `lp2@…` while `node_modules/lp2` IS left-pad@1.3.0, and no pin /// of `left-pad` reaches it. The real package comes from the copy itself -/// ([`CopyRef::identity`]); a copy whose package cannot be read is left -/// alone. +/// ([`CopyRef::identity`]). A copy whose package cannot be read (a +/// missing directory, a tarball with no readable `package.json`, a +/// non-registry url) may be any package at its lock version, so it is +/// recorded as a copy of every package this lock wires at that version: +/// vouching for those would rest on a copy nobody read (#1516). A +/// `portal:` / `link:` copy with no readable manifest has no version, and +/// is left alone. async fn record_copies(ctx: &DiscoverCtx<'_>, copies: Vec, out: &mut Discovery) { for copy in copies { let bytes = match copy.source.read_path() { @@ -868,6 +876,9 @@ async fn record_copies(ctx: &DiscoverCtx<'_>, copies: Vec, out: &mut D let Some((name, version)) = crate::vendor::yarn_copies::copy_identity(©.source, ©.version, bytes).await else { + if !matches!(copy.source, CopyRef::Linked(_)) { + record_unidentified_copy(©, out); + } continue; }; out.unpatched_copy( @@ -883,6 +894,31 @@ async fn record_copies(ctx: &DiscoverCtx<'_>, copies: Vec, out: &mut D } } +/// [`record_copies`] for a copy whose package cannot be read: an unpatched +/// copy of each package [`YARN_LOCK`] wires at the copy's lock version +/// (#1516). +fn record_unidentified_copy(copy: &LockCopy, out: &mut Discovery) { + let suffix = format!("@{}", copy.version); + let wired: BTreeSet = out + .refs + .iter() + .filter(|r| r.source_file == Path::new(YARN_LOCK) && r.purl.ends_with(&suffix)) + .map(|r| r.purl.clone()) + .collect(); + for purl in wired { + out.unpatched_copy( + YARN_LOCK, + Some(purl), + ©.key, + &format!( + "{}, whose package can't be read, so it may be this version of any package \ + (yarn keys it by the dependency name)", + copy.source.how() + ), + ); + } +} + #[cfg(test)] mod tests { use super::super::testing::*; @@ -1396,6 +1432,102 @@ mod tests { } } + /// #1516: three renamed classic copies #1236's reader could not read, + /// so each was taken for "not left-pad" and the wiring was attested + /// while `node_modules/lp2` installed left-pad@1.3.0 unpatched: a + /// `file:` directory or tarball outside the root (yarn installs it all + /// the same), a tarball whose root directory is not `package/` (yarn + /// strips the first segment whatever it is), and a non-registry url. + /// Each now contests the ref. Controls: an outside-root copy holding + /// another package, and an unreadable url copy of another version, do + /// not. + #[tokio::test] + async fn issue_1516_unread_classic_copies_contest_the_ref() { + let hosted = hosted_url("npm", "left-pad", "1.3.0", UUID_A, "left-pad-1.3.0.tgz"); + let wirings = [ + ( + "hosted", + classic_block("left-pad@1.3.0", "1.3.0", &hosted, Some(SRI)), + ), + ( + "vendored", + classic_block( + "left-pad@1.3.0", + "1.3.0", + &format!("file:./.socket/vendor/npm/{UUID_A}/left-pad-1.3.0.tgz#{SHA1}"), + Some(SRI), + ), + ), + ]; + let url_copy = |version: &str| { + classic_block( + "\"lp2@http://127.0.0.1:8799/lp.tgz\"", + version, + &format!("http://127.0.0.1:8799/lp.tgz#{SHA1}"), + None, + ) + }; + let copies = [ + ( + "outside-root file: directory", + "\"lp2@file:../ext/lpdir\":\n version \"1.3.0\"\n\n".to_string(), + ), + ( + "outside-root file: tarball", + classic_block( + "\"lp2@file:../ext/left-pad-1.3.0.tgz\"", + "1.3.0", + &format!("file:../ext/left-pad-1.3.0.tgz#{SHA1}"), + None, + ), + ), + ( + "tarball rooted at left-pad-1.3.0/", + classic_block( + "\"lp2@file:./lp.tgz\"", + "1.3.0", + &format!("file:./lp.tgz#{SHA1}"), + None, + ), + ), + ("non-registry url", url_copy("1.3.0")), + ]; + let project = |blocks: &[String], fork: &str| { + let p = Project::nested("p"); + p.write("yarn.lock", classic(blocks)); + p.write( + "../ext/lpdir/package.json", + format!(r#"{{"name":"{fork}","version":"1.3.0"}}"#), + ); + p.write("../ext/left-pad-1.3.0.tgz", npm_tgz(fork, "1.3.0")); + p.write("lp.tgz", npm_tgz_rooted("left-pad-1.3.0", fork, "1.3.0")); + p + }; + for (mode, wired) in &wirings { + for (case, copy) in &copies { + let out = run(&project(&[wired.clone(), copy.clone()], "left-pad")).await; + assert!(out.refs.is_empty(), "{mode} {case}: {:#?}", out.refs); + assert!( + out.diagnostics + .iter() + .any(|d| d.code == DIAG_REF_UNATTRIBUTABLE + && d.detail.contains("lp2@") + && d.detail.contains("UNPATCHED")), + "{mode} {case}: {:#?}", + out.diagnostics + ); + } + // The readable copies hold another package: not left-pad. + for (case, copy) in &copies[..3] { + let out = run(&project(&[wired.clone(), copy.clone()], "other-pkg")).await; + assert_eq!(out.refs.len(), 1, "{mode} {case}: {:#?}", out.diagnostics); + } + // An unreadable copy of another version can't be left-pad@1.3.0. + let out = run(&project(&[wired.clone(), url_copy("2.0.0")], "left-pad")).await; + assert_eq!(out.refs.len(), 1, "{mode} url 2.0.0: {:#?}", out.diagnostics); + } + } + /// Socket-shaped blocks that fail validation are DIAGNOSED, never refs: /// no version, mixed-package key, unsafe name, a leaf naming another /// package, a traversal leaf, a non-canonical vendored uuid. From be51d945cdab3cd9a1270957ffe63a5c9977d191 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 02:35:50 +0000 Subject: [PATCH 3/7] Format and document unread yarn copy handling Describe in docs/ecosystems.md how renamed yarn copies outside the project, tarballs with any top-level directory, and copies whose package can't be read are treated (#1516). Assisted-by: Claude Code:claude-opus-5-5 --- .../tests/e2e_redirect_yarn_classic_build.rs | 9 ++++++--- crates/socket-patch-core/src/formats/yarn/copy.rs | 4 +++- crates/socket-patch-core/src/vendor/yarn_classic_lock.rs | 8 +++++++- crates/socket-patch-core/src/vex/discover/mod.rs | 8 ++++++-- crates/socket-patch-core/src/vex/discover/yarn.rs | 7 ++++++- docs/ecosystems.md | 8 +++++++- 6 files changed, 35 insertions(+), 9 deletions(-) diff --git a/crates/socket-patch-cli/tests/e2e_redirect_yarn_classic_build.rs b/crates/socket-patch-cli/tests/e2e_redirect_yarn_classic_build.rs index d193403bb..b342cb232 100644 --- a/crates/socket-patch-cli/tests/e2e_redirect_yarn_classic_build.rs +++ b/crates/socket-patch-cli/tests/e2e_redirect_yarn_classic_build.rs @@ -1490,9 +1490,12 @@ async fn classic_file_directory_copy_under_another_name_is_named_and_not_atteste #[tokio::test(flavor = "multi_thread")] #[serial_test::serial] async fn classic_outside_root_copy_under_another_name_is_named_and_not_attested() { - if !require_yarn_classic("e2e_redirect_yarn_classic_build (outside-root other-name file:)", |c| { - cache_env::isolate(c); - }) { + if !require_yarn_classic( + "e2e_redirect_yarn_classic_build (outside-root other-name file:)", + |c| { + cache_env::isolate(c); + }, + ) { return; } let tmp = tempfile::tempdir().unwrap(); diff --git a/crates/socket-patch-core/src/formats/yarn/copy.rs b/crates/socket-patch-core/src/formats/yarn/copy.rs index 1cc871e9b..fe632e83c 100644 --- a/crates/socket-patch-core/src/formats/yarn/copy.rs +++ b/crates/socket-patch-core/src/formats/yarn/copy.rs @@ -194,7 +194,9 @@ fn copy_path(base: &str, path: &str) -> Option { } else { format!("{base}/{path}") }; - Some(crate::utils::relpath::normalize_rel_keeping_escapes(&joined)) + Some(crate::utils::relpath::normalize_rel_keeping_escapes( + &joined, + )) } fn parse_manifest(bytes: &[u8]) -> Option { diff --git a/crates/socket-patch-core/src/vendor/yarn_classic_lock.rs b/crates/socket-patch-core/src/vendor/yarn_classic_lock.rs index 9e6dd4ede..276bf4a39 100644 --- a/crates/socket-patch-core/src/vendor/yarn_classic_lock.rs +++ b/crates/socket-patch-core/src/vendor/yarn_classic_lock.rs @@ -1975,7 +1975,13 @@ wow@^1.0.0: // A sibling directory of the project, spelled `..//lpdir` // the way yarn records it. let ext = tempfile::tempdir().unwrap(); - let ext_name = ext.path().file_name().unwrap().to_str().unwrap().to_string(); + let ext_name = ext + .path() + .file_name() + .unwrap() + .to_str() + .unwrap() + .to_string(); let extra = format!( "\n\"lp2@file:../{ext_name}/lpdir\":\n version \"1.3.0\"\n\n\ \"lp3@file:./lp.tgz\":\n version \"1.3.0\"\n resolved \"file:./lp.tgz#cccc\"\n\n\ diff --git a/crates/socket-patch-core/src/vex/discover/mod.rs b/crates/socket-patch-core/src/vex/discover/mod.rs index ab16ed320..45c409685 100644 --- a/crates/socket-patch-core/src/vex/discover/mod.rs +++ b/crates/socket-patch-core/src/vex/discover/mod.rs @@ -2991,8 +2991,12 @@ pub(crate) mod testing { header.set_size(manifest.len() as u64); header.set_mode(0o644); header.set_cksum(); - tar.append_data(&mut header, format!("{root}/package.json"), manifest.as_bytes()) - .unwrap(); + tar.append_data( + &mut header, + format!("{root}/package.json"), + manifest.as_bytes(), + ) + .unwrap(); tar.into_inner().unwrap().finish().unwrap() } diff --git a/crates/socket-patch-core/src/vex/discover/yarn.rs b/crates/socket-patch-core/src/vex/discover/yarn.rs index f039aa612..352cf2033 100644 --- a/crates/socket-patch-core/src/vex/discover/yarn.rs +++ b/crates/socket-patch-core/src/vex/discover/yarn.rs @@ -1524,7 +1524,12 @@ mod tests { } // An unreadable copy of another version can't be left-pad@1.3.0. let out = run(&project(&[wired.clone(), url_copy("2.0.0")], "left-pad")).await; - assert_eq!(out.refs.len(), 1, "{mode} url 2.0.0: {:#?}", out.diagnostics); + assert_eq!( + out.refs.len(), + 1, + "{mode} url 2.0.0: {:#?}", + out.diagnostics + ); } } diff --git a/docs/ecosystems.md b/docs/ecosystems.md index adfc35a62..5f9a6a86e 100644 --- a/docs/ecosystems.md +++ b/docs/ecosystems.md @@ -285,7 +285,13 @@ Each Legacy format has an upgrade path and an undo path. Both work in v5: reaches that copy, so it stays unpatched. Which package such a copy is comes from the copy itself (the tarball's or directory's `package.json`, or the registry package a URL's path names; a berry `portal:` / `link:` target's `package.json` also gives the - version, since the lock records `0.0.0-use.local`). Hosted mode still pins the + version, since the lock records `0.0.0-use.local`). The copy is read wherever yarn + installs it from, including a `file:` path outside the project (`file:../shared/lp`), + and a tarball's `package.json` is found under whatever top-level directory it has, as + yarn strips the first path segment. A copy whose package can't be read (a non-registry + URL, a missing directory) is treated as possibly any package at its lock version: it + is named as a copy it may be, and `vex` attests nothing that lock wires at that + version. 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 From 55610e57c8d33096c6f0e14c9a21f02313a9c4a2 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 02:43:39 +0000 Subject: [PATCH 4/7] Create project dir in outside-root yarn e2e Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-cli/tests/e2e_redirect_yarn_classic_build.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/socket-patch-cli/tests/e2e_redirect_yarn_classic_build.rs b/crates/socket-patch-cli/tests/e2e_redirect_yarn_classic_build.rs index b342cb232..4dc56fffa 100644 --- a/crates/socket-patch-cli/tests/e2e_redirect_yarn_classic_build.rs +++ b/crates/socket-patch-cli/tests/e2e_redirect_yarn_classic_build.rs @@ -1502,6 +1502,7 @@ async fn classic_outside_root_copy_under_another_name_is_named_and_not_attested( let proj = tmp.path().join("proj"); let copy = tmp.path().join("ext").join("lpdir"); std::fs::create_dir_all(©).unwrap(); + std::fs::create_dir_all(&proj).unwrap(); std::fs::write( copy.join("package.json"), format!(r#"{{"name":"{DEP}","version":"{DEP_VERSION}","main":"index.js"}}"#), From cb2ce77e102b62abff4be62d14106107d77e7a17 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 03:08:50 +0000 Subject: [PATCH 5/7] Tighten unread yarn copy handling after review Match a copy whose package can't be read against the lock's refs once every extractor is done, so berry vendored and resolutions refs pushed after the copies are read are contested too, in one pass. Take a classic file: path that leaves the root as root-relative only when the root package.json declares it: yarn 1 writes a workspace member's "file:../b" relative to that member, so reading it from the root would name the wrong package or block attestation for nothing. Never read an absolute copy path from a lock, which could name any file or a Windows network share; such a copy stays unidentified. Assisted-by: Claude Code:claude-opus-5-5 --- .../src/formats/yarn/copy.rs | 90 +++++++++- .../src/vendor/yarn_classic_lock.rs | 15 ++ .../src/vendor/yarn_copies.rs | 17 +- .../socket-patch-core/src/vex/discover/mod.rs | 66 +++++++ .../src/vex/discover/testing/golden.rs | 2 + .../src/vex/discover/yarn.rs | 169 ++++++++++++------ 6 files changed, 303 insertions(+), 56 deletions(-) diff --git a/crates/socket-patch-core/src/formats/yarn/copy.rs b/crates/socket-patch-core/src/formats/yarn/copy.rs index fe632e83c..851dc7966 100644 --- a/crates/socket-patch-core/src/formats/yarn/copy.rs +++ b/crates/socket-patch-core/src/formats/yarn/copy.rs @@ -36,7 +36,17 @@ 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 /// names its package in its path. + /// + /// `None` too for an absolute path: a lock is untrusted input, and an + /// absolute spelling can name any file on the machine, or (a Windows + /// 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 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() { @@ -71,6 +81,17 @@ impl CopyRef { Some((name, lock_version.to_string())) } + /// The copy's path leaves the project root (`../shared/left-pad`) or + /// is absolute. + fn leaves_root(&self) -> bool { + match self { + CopyRef::Tarball(rel) | CopyRef::Directory(rel) | CopyRef::Linked(rel) => { + rel == ".." || rel.starts_with("../") || crate::utils::relpath::is_anchored(rel) + } + CopyRef::Url(_) => false, + } + } + /// 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). @@ -120,6 +141,44 @@ pub(crate) fn classic_copy_ref(patterns: &[String], resolved: Option<&str>) -> O } } +/// [`classic_copy_ref`] for a block of the lock at the root whose +/// `package.json` is `root_manifest`. yarn 1 writes a `file:` path as the +/// declaring `package.json` spelled it, relative to that package: the +/// root's for the root's own dependencies, a member's for a workspace +/// member's (`"b": "file:../b"` in `packages/a` is `packages/b`). A path +/// that leaves the root is therefore only taken as root-relative when the +/// root `package.json` declares exactly that pattern; any other such path +/// was resolved against a member and is left alone, as it was before +/// #1516. +pub(crate) fn classic_copy_ref_at_root( + patterns: &[String], + resolved: Option<&str>, + root_manifest: Option<&[u8]>, +) -> Option { + let copy = classic_copy_ref(patterns, resolved)?; + if copy.leaves_root() && !root_declares(root_manifest, patterns) { + return None; + } + Some(copy) +} + +/// Whether the `package.json` bytes `root_manifest` declare one of the +/// classic key `patterns` (`lp2@file:../ext/lpdir`) as an installed +/// dependency of their own. +fn root_declares(root_manifest: Option<&[u8]>, patterns: &[String]) -> bool { + let Some(manifest) = root_manifest.and_then(parse_manifest) else { + return false; + }; + ["dependencies", "devDependencies", "optionalDependencies"] + .iter() + .filter_map(|section| manifest.get(section)?.as_object()) + .flatten() + .any(|(name, spec)| { + spec.as_str() + .is_some_and(|spec| patterns.iter().any(|p| *p == format!("{name}@{spec}"))) + }) +} + /// The copy a berry locator `reference` installs from: a `file:` tarball or /// directory, a `portal:` / `link:` directory (each relative to the /// workspace its `locator=` binding names), or an http(s) url. `None` for @@ -253,9 +312,36 @@ mod tests { ), Some(CopyRef::Tarball("../shared/lp.tgz".into())) ); + let abs = classic_copy_ref(&pats(&["lp2@file:/abs/lpdir"]), None); + assert_eq!(abs, Some(CopyRef::Directory("/abs/lpdir".into()))); + assert_eq!( + abs.unwrap().read_path(), + None, + "an absolute path is never read" + ); + } + + /// #1516: a classic `file:` path that leaves the root is the root's own + /// only when the root `package.json` declares exactly that pattern; + /// otherwise yarn resolved it against a workspace member. + #[test] + fn classic_paths_leaving_the_root_need_a_root_declaration() { + let manifest = br#"{"dependencies":{"lp2":"file:../ext/lpdir"},"devDependencies":{"lp3":"file:./in"}}"#; + let at_root = |p: &str| classic_copy_ref_at_root(&pats(&[p]), None, Some(manifest)); + assert_eq!( + at_root("lp2@file:../ext/lpdir"), + Some(CopyRef::Directory("../ext/lpdir".into())) + ); + assert_eq!(at_root("b@file:../b"), None, "a member's path"); + assert_eq!( + classic_copy_ref_at_root(&pats(&["lp2@file:../ext/lpdir"]), None, None), + None, + "no root manifest" + ); assert_eq!( - classic_copy_ref(&pats(&["lp2@file:/abs/lpdir"]), None), - Some(CopyRef::Directory("/abs/lpdir".into())) + at_root("lp4@file:./other"), + Some(CopyRef::Directory("other".into())), + "an in-root path needs no declaration" ); } diff --git a/crates/socket-patch-core/src/vendor/yarn_classic_lock.rs b/crates/socket-patch-core/src/vendor/yarn_classic_lock.rs index 276bf4a39..0d1958e8f 100644 --- a/crates/socket-patch-core/src/vendor/yarn_classic_lock.rs +++ b/crates/socket-patch-core/src/vendor/yarn_classic_lock.rs @@ -2008,6 +2008,21 @@ wow@^1.0.0: ) .await .unwrap(); + // The root declares the outside-root path, so it is root-relative. + tokio::fs::write( + fx.root().join("package.json"), + serde_json::json!({ + "name": "app", + "version": "1.0.0", + "dependencies": { + "left-pad": "1.3.0", + "lp2": format!("file:../{ext_name}/lpdir"), + }, + }) + .to_string(), + ) + .await + .unwrap(); let (result, entry, warnings) = expect_done(fx.vendor(false).await); assert!(result.success, "{:?}", result.error); assert_eq!(entry.unwrap().wiring.len(), 1, "the registry block"); diff --git a/crates/socket-patch-core/src/vendor/yarn_copies.rs b/crates/socket-patch-core/src/vendor/yarn_copies.rs index 038d45969..9ae9329a5 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, CopyRef}; +use crate::formats::yarn::copy::{berry_copy_ref, classic_copy_ref_at_root, CopyRef}; 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}; @@ -49,7 +49,17 @@ pub(crate) async fn other_name_copies( name: &str, version: &str, ) -> Vec { - let candidates: Vec<(String, CopyRef, String)> = if is_berry_lock(lock_text) { + let berry = is_berry_lock(lock_text); + // Says which classic `file:` paths that leave the root are the root's + // own ([`classic_copy_ref_at_root`]). + let root_manifest = if berry { + None + } else { + read_regular_to_bytes(&project_root.join("package.json")) + .await + .ok() + }; + let candidates: Vec<(String, CopyRef, String)> = if berry { berry_entries(lock_text) .entries .into_iter() @@ -79,7 +89,8 @@ pub(crate) async fn other_name_copies( ) { return None; } - let source = classic_copy_ref(&e.patterns, resolved)?; + let source = + classic_copy_ref_at_root(&e.patterns, resolved, root_manifest.as_deref())?; let lock_version = classic_field(&e.block.lines, "version")?.to_string(); Some((e.block.key.clone(), source, lock_version)) }) diff --git a/crates/socket-patch-core/src/vex/discover/mod.rs b/crates/socket-patch-core/src/vex/discover/mod.rs index 45c409685..4b60d56eb 100644 --- a/crates/socket-patch-core/src/vex/discover/mod.rs +++ b/crates/socket-patch-core/src/vex/discover/mod.rs @@ -435,6 +435,17 @@ pub struct UnpatchedCopy { pub how: String, } +/// A lock entry that installs SOME package at `version` under another +/// dependency name, from a copy whose package can't be read +/// ([`Discovery::unidentified_copy`], #1516). +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct UnidentifiedCopy { + file: PathBuf, + version: String, + key: String, + how: String, +} + /// A ref another lock contests ([`Discovery::contest_across_locks`]): it /// was dropped from `refs` and diagnosed [`DIAG_REF_UNATTRIBUTABLE`]. Kept /// so a ledger reader can name the contesting lock instead of reporting the @@ -563,6 +574,10 @@ pub struct Discovery { /// Same-lock copies that contest that lock's own refs /// ([`Discovery::unpatched_copy`]). pub unpatched_copies: Vec, + /// Same-lock copies whose package can't be read, pending + /// [`Discovery::contest_within_locks`] ([`Discovery::unidentified_copy`]); + /// always empty once discovery returns (folded into `unpatched_copies`). + pub(crate) unidentified_copies: Vec, /// Refs in `refs` whose wiring a build bypasses ([`Unattested`]). pub unattested: Vec, /// Copies no wiring reaches, pending [`Discovery::unattest_unwired_copies`]; @@ -868,12 +883,63 @@ impl Discovery { }); } + /// A lock entry of `file` that installs a copy of some package at + /// `version` whose package can't be read (a non-registry url, a missing + /// 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`: 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, version: &str, key: &str, how: &str) { + self.unidentified_copies.push(UnidentifiedCopy { + file: PathBuf::from(file), + version: version.to_string(), + key: key.to_string(), + how: how.to_string(), + }); + } + + /// Fold [`Discovery::unidentified_copies`] into `unpatched_copies`: one + /// copy per ref of the same file at the same version. + fn expand_unidentified_copies(&mut self) { + if self.unidentified_copies.is_empty() { + return; + } + let mut wired: BTreeMap<(&Path, &str), BTreeSet<&str>> = BTreeMap::new(); + for r in &self.refs { + if let Some((_, version)) = r.purl.rsplit_once('@') { + wired + .entry((r.source_file.as_path(), version)) + .or_default() + .insert(r.purl.as_str()); + } + } + let mut expanded = Vec::new(); + for u in &self.unidentified_copies { + for purl in wired + .get(&(u.file.as_path(), u.version.as_str())) + .into_iter() + .flatten() + { + expanded.push(UnpatchedCopy { + purl: purl.to_string(), + file: u.file.clone(), + key: u.key.clone(), + how: u.how.clone(), + }); + } + } + self.unidentified_copies.clear(); + self.unpatched_copies.extend(expanded); + } + /// Drop every ref whose OWN lock also installs an unpatched copy of the /// same `name@version` ([`Discovery::unpatched_copy`]): the build ships /// that copy whatever the wiring does, so the ref is diagnosed /// ([`DIAG_REF_UNATTRIBUTABLE`], naming the entry) and not emitted, but /// [`Discovery::shadowed`]. Its uuid stays recognized (rule 11). fn contest_within_locks(&mut self) { + self.expand_unidentified_copies(); if self.unpatched_copies.is_empty() { return; } diff --git a/crates/socket-patch-core/src/vex/discover/testing/golden.rs b/crates/socket-patch-core/src/vex/discover/testing/golden.rs index 4e62d89ca..d08299347 100644 --- a/crates/socket-patch-core/src/vex/discover/testing/golden.rs +++ b/crates/socket-patch-core/src/vex/discover/testing/golden.rs @@ -135,6 +135,8 @@ fn render(out: &Discovery, root: &Path) -> Value { stale_selectors, // Already folded into `unattested` by the time a run returns. unwired_copies: _, + // Already folded into `unpatched_copies` by the time a run returns. + unidentified_copies: _, } = out; let render_ref = |r: &PatchedRef| { let PatchedRef { diff --git a/crates/socket-patch-core/src/vex/discover/yarn.rs b/crates/socket-patch-core/src/vex/discover/yarn.rs index 352cf2033..73a6ea6d0 100644 --- a/crates/socket-patch-core/src/vex/discover/yarn.rs +++ b/crates/socket-patch-core/src/vex/discover/yarn.rs @@ -82,9 +82,6 @@ //! (a sibling checkout's artifacts) and is skipped silently, as in the npm //! extractor. -use std::collections::BTreeSet; -use std::path::Path; - use serde_json::Value; use super::{ @@ -93,7 +90,7 @@ use super::{ DIAG_REF_INVALID, DIAG_REF_UNATTRIBUTABLE, }; use crate::formats::yarn::blocks::{berry_field, classic_field}; -use crate::formats::yarn::copy::{berry_copy_ref, classic_copy_ref, CopyRef}; +use crate::formats::yarn::copy::{berry_copy_ref, classic_copy_ref_at_root, CopyRef}; use crate::formats::yarn::is_berry_lock; use crate::formats::yarn::patterns::{ classic_key_real_name, classic_pattern_real_name, resolution_selector_target, @@ -145,9 +142,12 @@ fn stray_top_level_line(text: &str) -> Option<&str> { async fn extract_classic(ctx: &DiscoverCtx<'_>, entries: Vec, out: &mut Discovery) { let mut copies: Vec = Vec::new(); + // Says which `file:` paths that leave the root are the root's own + // ([`classic_copy_ref_at_root`]). + let root_manifest = ctx.read_advisory_bytes(PACKAGE_JSON).await; for entry in entries { if entry.live && !entry.patterns.is_empty() { - classic_block(ctx, &entry, &mut copies, out); + classic_block(ctx, &entry, root_manifest.as_deref(), &mut copies, out); } } record_copies(ctx, copies, out).await; @@ -156,12 +156,18 @@ async fn extract_classic(ctx: &DiscoverCtx<'_>, entries: Vec, out: &m /// The copy a classic `file:` / url block installs, to be read for the /// package it really holds ([`record_copies`], #1236): yarn 1 keys it by the /// DEPENDENCY name (`"lp2@file:./lpdir"`), like berry. The `file:` path is -/// the key pattern's, relative to the root (a path yarn resolved against a -/// workspace member reads nothing there, and is left alone). -fn classic_copy(entry: &YarnEntry, resolved: Option<&str>) -> Option { +/// the key pattern's, relative to the root, even when it leaves the root +/// (#1516); one that leaves the root and isn't the root `package.json`'s +/// own was resolved against a workspace member, and is left alone +/// ([`classic_copy_ref_at_root`]). +fn classic_copy( + entry: &YarnEntry, + resolved: Option<&str>, + root_manifest: Option<&[u8]>, +) -> Option { Some(LockCopy { key: entry.block.key.clone(), - source: classic_copy_ref(&entry.patterns, resolved)?, + source: classic_copy_ref_at_root(&entry.patterns, resolved, root_manifest)?, version: classic_field(&entry.block.lines, "version")?.to_string(), }) } @@ -187,6 +193,7 @@ fn classic_block_purl(entry: &YarnEntry) -> Option { fn classic_block( ctx: &DiscoverCtx<'_>, entry: &YarnEntry, + root_manifest: Option<&[u8]>, copies: &mut Vec, out: &mut Discovery, ) { @@ -231,7 +238,7 @@ fn classic_block( ); // Locked under the dependency name: the directory's own // `package.json` says which package this copy is (#1236). - copies.extend(classic_copy(entry, resolved)); + copies.extend(classic_copy(entry, resolved, root_manifest)); return; } // `link:` ranges install from the working tree; `resolved` is inert. @@ -273,7 +280,7 @@ fn classic_block( if classic_copy_source(patterns, Some(resolved)) == CopySource::RemoteTarball && !root_anchored_spelling(resolved) { - copies.extend(classic_copy(entry, Some(resolved))); + copies.extend(classic_copy(entry, Some(resolved), root_manifest)); } return; }; @@ -877,7 +884,16 @@ async fn record_copies(ctx: &DiscoverCtx<'_>, copies: Vec, out: &mut D crate::vendor::yarn_copies::copy_identity(©.source, ©.version, bytes).await else { if !matches!(copy.source, CopyRef::Linked(_)) { - record_unidentified_copy(©, out); + out.unidentified_copy( + YARN_LOCK, + ©.version, + ©.key, + &format!( + "{}, whose package can't be read, so it may be this version of any \ + package (yarn keys it by the dependency name)", + copy.source.how() + ), + ); } continue; }; @@ -894,31 +910,6 @@ async fn record_copies(ctx: &DiscoverCtx<'_>, copies: Vec, out: &mut D } } -/// [`record_copies`] for a copy whose package cannot be read: an unpatched -/// copy of each package [`YARN_LOCK`] wires at the copy's lock version -/// (#1516). -fn record_unidentified_copy(copy: &LockCopy, out: &mut Discovery) { - let suffix = format!("@{}", copy.version); - let wired: BTreeSet = out - .refs - .iter() - .filter(|r| r.source_file == Path::new(YARN_LOCK) && r.purl.ends_with(&suffix)) - .map(|r| r.purl.clone()) - .collect(); - for purl in wired { - out.unpatched_copy( - YARN_LOCK, - Some(purl), - ©.key, - &format!( - "{}, whose package can't be read, so it may be this version of any package \ - (yarn keys it by the dependency name)", - copy.source.how() - ), - ); - } -} - #[cfg(test)] mod tests { use super::super::testing::*; @@ -1432,15 +1423,17 @@ mod tests { } } - /// #1516: three renamed classic copies #1236's reader could not read, - /// so each was taken for "not left-pad" and the wiring was attested - /// while `node_modules/lp2` installed left-pad@1.3.0 unpatched: a + /// #1516: renamed classic copies #1236's reader could not read, so + /// each was taken for "not left-pad" and the wiring was attested while + /// `node_modules/lpN` installed left-pad@1.3.0 unpatched: a root /// `file:` directory or tarball outside the root (yarn installs it all /// the same), a tarball whose root directory is not `package/` (yarn - /// strips the first segment whatever it is), and a non-registry url. - /// Each now contests the ref. Controls: an outside-root copy holding - /// another package, and an unreadable url copy of another version, do - /// not. + /// strips the first segment whatever it is), a non-registry url, and an + /// absolute path (never read: an untrusted lock could name any file or + /// share). Each now contests the ref. Controls: an outside-root copy + /// holding another package; an unreadable url copy of another version; + /// and an outside-root path the root `package.json` doesn't declare, + /// which yarn resolved against a workspace member, not the root. #[tokio::test] async fn issue_1516_unread_classic_copies_contest_the_ref() { let hosted = hosted_url("npm", "left-pad", "1.3.0", UUID_A, "left-pad-1.3.0.tgz"); @@ -1461,7 +1454,7 @@ mod tests { ]; let url_copy = |version: &str| { classic_block( - "\"lp2@http://127.0.0.1:8799/lp.tgz\"", + "\"lp4@http://127.0.0.1:8799/lp.tgz\"", version, &format!("http://127.0.0.1:8799/lp.tgz#{SHA1}"), None, @@ -1470,10 +1463,12 @@ mod tests { let copies = [ ( "outside-root file: directory", - "\"lp2@file:../ext/lpdir\":\n version \"1.3.0\"\n\n".to_string(), + "lp1@", + "\"lp1@file:../ext/lpdir\":\n version \"1.3.0\"\n\n".to_string(), ), ( "outside-root file: tarball", + "lp2@", classic_block( "\"lp2@file:../ext/left-pad-1.3.0.tgz\"", "1.3.0", @@ -1483,18 +1478,40 @@ mod tests { ), ( "tarball rooted at left-pad-1.3.0/", + "lp3@", classic_block( - "\"lp2@file:./lp.tgz\"", + "\"lp3@file:./lp.tgz\"", "1.3.0", &format!("file:./lp.tgz#{SHA1}"), None, ), ), - ("non-registry url", url_copy("1.3.0")), + ("non-registry url", "lp4@", url_copy("1.3.0")), + ( + "absolute file: directory", + "lp5@", + "\"lp5@file:/srv/ext/lpdir\":\n version \"1.3.0\"\n\n".to_string(), + ), ]; let project = |blocks: &[String], fork: &str| { let p = Project::nested("p"); p.write("yarn.lock", classic(blocks)); + p.write( + "package.json", + serde_json::json!({ + "name": "p", + "version": "1.0.0", + "dependencies": { + "left-pad": "1.3.0", + "lp1": "file:../ext/lpdir", + "lp3": "file:./lp.tgz", + "lp4": "http://127.0.0.1:8799/lp.tgz", + "lp5": "file:/srv/ext/lpdir", + }, + "devDependencies": { "lp2": "file:../ext/left-pad-1.3.0.tgz" }, + }) + .to_string(), + ); p.write( "../ext/lpdir/package.json", format!(r#"{{"name":"{fork}","version":"1.3.0"}}"#), @@ -1504,21 +1521,21 @@ mod tests { p }; for (mode, wired) in &wirings { - for (case, copy) in &copies { + for (case, dep, copy) in &copies { let out = run(&project(&[wired.clone(), copy.clone()], "left-pad")).await; assert!(out.refs.is_empty(), "{mode} {case}: {:#?}", out.refs); assert!( out.diagnostics .iter() .any(|d| d.code == DIAG_REF_UNATTRIBUTABLE - && d.detail.contains("lp2@") + && d.detail.contains(*dep) && d.detail.contains("UNPATCHED")), "{mode} {case}: {:#?}", out.diagnostics ); } // The readable copies hold another package: not left-pad. - for (case, copy) in &copies[..3] { + for (case, _, copy) in &copies[..3] { let out = run(&project(&[wired.clone(), copy.clone()], "other-pkg")).await; assert_eq!(out.refs.len(), 1, "{mode} {case}: {:#?}", out.diagnostics); } @@ -1530,9 +1547,59 @@ mod tests { "{mode} url 2.0.0: {:#?}", out.diagnostics ); + // A workspace member's `"b": "file:../b"` locks as `b@file:../b`, + // relative to the member: not a root-relative path, left alone. + let member = "\"b@file:../b\":\n version \"1.3.0\"\n\n".to_string(); + let out = run(&project(&[wired.clone(), member], "left-pad")).await; + assert_eq!( + out.refs.len(), + 1, + "{mode} member path: {:#?}", + out.diagnostics + ); } } + /// #1516: an unidentified berry copy contests a ref the berry extractor + /// confirms only after reading the copies (a vendored entry, confirmed + /// by `resolutions`): the contest is matched once every ref is in. + #[tokio::test] + async fn issue_1516_unread_berry_copy_contests_a_vendored_ref() { + let wired = berry_vendored_block("left-pad", "1.3.0", UUID_A); + let url = |version: &str| { + berry_block( + "lp2@http://127.0.0.1:8799/lp.tgz", + version, + "lp2@http://127.0.0.1:8799/lp.tgz", + None, + ) + }; + let project = |blocks: &[String]| { + let p = Project::new(); + p.write("yarn.lock", berry(blocks)); + 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"), + })), + ); + p + }; + let out = run(&project(&[wired.clone(), url("1.3.0")])).await; + assert!(out.refs.is_empty(), "{:#?}", out.refs); + assert!( + out.diagnostics + .iter() + .any(|d| d.code == DIAG_REF_UNATTRIBUTABLE && d.detail.contains("lp2@")), + "{:#?}", + out.diagnostics + ); + assert_refs( + &run(&project(&[wired, url("2.0.0")])).await, + &[("pkg:npm/left-pad@1.3.0", UUID_A, WiringMode::Vendored)], + ); + } + /// Socket-shaped blocks that fail validation are DIAGNOSED, never refs: /// no version, mixed-package key, unsafe name, a leaf naming another /// package, a traversal leaf, a non-canonical vendored uuid. From 628950595fd3e3d40fcb5a3bdb35928c03df37c5 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 03:20:39 +0000 Subject: [PATCH 6/7] Make unidentified yarn copies a public field The CLI builds Discovery with struct update syntax, so a private field broke its build. Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-core/src/vex/discover/mod.rs | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/crates/socket-patch-core/src/vex/discover/mod.rs b/crates/socket-patch-core/src/vex/discover/mod.rs index 4b60d56eb..fafc90b54 100644 --- a/crates/socket-patch-core/src/vex/discover/mod.rs +++ b/crates/socket-patch-core/src/vex/discover/mod.rs @@ -439,11 +439,15 @@ pub struct UnpatchedCopy { /// dependency name, from a copy whose package can't be read /// ([`Discovery::unidentified_copy`], #1516). #[derive(Debug, Clone, PartialEq, Eq)] -pub(crate) struct UnidentifiedCopy { - file: PathBuf, - version: String, - key: String, - how: String, +pub struct UnidentifiedCopy { + /// Root-relative lock file. + pub file: PathBuf, + /// The lock entry's version. + pub version: String, + /// The lock entry's key, as the lock spells it. + pub key: String, + /// How that entry installs, completing "lock entry `` …". + pub how: String, } /// A ref another lock contests ([`Discovery::contest_across_locks`]): it @@ -577,7 +581,7 @@ pub struct Discovery { /// Same-lock copies whose package can't be read, pending /// [`Discovery::contest_within_locks`] ([`Discovery::unidentified_copy`]); /// always empty once discovery returns (folded into `unpatched_copies`). - pub(crate) unidentified_copies: Vec, + pub unidentified_copies: Vec, /// Refs in `refs` whose wiring a build bypasses ([`Unattested`]). pub unattested: Vec, /// Copies no wiring reaches, pending [`Discovery::unattest_unwired_copies`]; From 75d5610a8c25cf826ae5efb17324140ff7542733 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 03:50:32 +0000 Subject: [PATCH 7/7] Read every classic yarn file: key as root-relative yarn 1 writes a file: lock key relative to the project root and normalized, whichever package.json declared it: a workspace member's "file:../../../ext/lpdir" locks as "lp2@file:../ext/lpdir", and the root's "file:../ext/lpdir/" loses its trailing slash. The gate that took an outside-root key as the root's own only when the root package.json spelled it exactly dropped both, so the unpatched copy went unwarned and vex attested it again. Drop the gate. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01WyDjcHE4ZNtbUocsBEJvFH --- .../src/formats/yarn/copy.rs | 73 ------------------- .../src/vendor/yarn_classic_lock.rs | 15 ---- .../src/vendor/yarn_copies.rs | 17 +---- .../src/vex/discover/yarn.rs | 53 ++++++-------- 4 files changed, 26 insertions(+), 132 deletions(-) diff --git a/crates/socket-patch-core/src/formats/yarn/copy.rs b/crates/socket-patch-core/src/formats/yarn/copy.rs index 851dc7966..b00e09378 100644 --- a/crates/socket-patch-core/src/formats/yarn/copy.rs +++ b/crates/socket-patch-core/src/formats/yarn/copy.rs @@ -81,17 +81,6 @@ impl CopyRef { Some((name, lock_version.to_string())) } - /// The copy's path leaves the project root (`../shared/left-pad`) or - /// is absolute. - fn leaves_root(&self) -> bool { - match self { - CopyRef::Tarball(rel) | CopyRef::Directory(rel) | CopyRef::Linked(rel) => { - rel == ".." || rel.starts_with("../") || crate::utils::relpath::is_anchored(rel) - } - CopyRef::Url(_) => false, - } - } - /// 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). @@ -141,44 +130,6 @@ pub(crate) fn classic_copy_ref(patterns: &[String], resolved: Option<&str>) -> O } } -/// [`classic_copy_ref`] for a block of the lock at the root whose -/// `package.json` is `root_manifest`. yarn 1 writes a `file:` path as the -/// declaring `package.json` spelled it, relative to that package: the -/// root's for the root's own dependencies, a member's for a workspace -/// member's (`"b": "file:../b"` in `packages/a` is `packages/b`). A path -/// that leaves the root is therefore only taken as root-relative when the -/// root `package.json` declares exactly that pattern; any other such path -/// was resolved against a member and is left alone, as it was before -/// #1516. -pub(crate) fn classic_copy_ref_at_root( - patterns: &[String], - resolved: Option<&str>, - root_manifest: Option<&[u8]>, -) -> Option { - let copy = classic_copy_ref(patterns, resolved)?; - if copy.leaves_root() && !root_declares(root_manifest, patterns) { - return None; - } - Some(copy) -} - -/// Whether the `package.json` bytes `root_manifest` declare one of the -/// classic key `patterns` (`lp2@file:../ext/lpdir`) as an installed -/// dependency of their own. -fn root_declares(root_manifest: Option<&[u8]>, patterns: &[String]) -> bool { - let Some(manifest) = root_manifest.and_then(parse_manifest) else { - return false; - }; - ["dependencies", "devDependencies", "optionalDependencies"] - .iter() - .filter_map(|section| manifest.get(section)?.as_object()) - .flatten() - .any(|(name, spec)| { - spec.as_str() - .is_some_and(|spec| patterns.iter().any(|p| *p == format!("{name}@{spec}"))) - }) -} - /// The copy a berry locator `reference` installs from: a `file:` tarball or /// directory, a `portal:` / `link:` directory (each relative to the /// workspace its `locator=` binding names), or an http(s) url. `None` for @@ -321,30 +272,6 @@ mod tests { ); } - /// #1516: a classic `file:` path that leaves the root is the root's own - /// only when the root `package.json` declares exactly that pattern; - /// otherwise yarn resolved it against a workspace member. - #[test] - fn classic_paths_leaving_the_root_need_a_root_declaration() { - let manifest = br#"{"dependencies":{"lp2":"file:../ext/lpdir"},"devDependencies":{"lp3":"file:./in"}}"#; - let at_root = |p: &str| classic_copy_ref_at_root(&pats(&[p]), None, Some(manifest)); - assert_eq!( - at_root("lp2@file:../ext/lpdir"), - Some(CopyRef::Directory("../ext/lpdir".into())) - ); - assert_eq!(at_root("b@file:../b"), None, "a member's path"); - assert_eq!( - classic_copy_ref_at_root(&pats(&["lp2@file:../ext/lpdir"]), None, None), - None, - "no root manifest" - ); - assert_eq!( - at_root("lp4@file:./other"), - Some(CopyRef::Directory("other".into())), - "an in-root path needs no declaration" - ); - } - /// #1516: yarn strips a `file:` tarball's first path segment whatever /// its name, so a `git archive --prefix=left-pad-1.3.0/` tarball names /// its package like a `package/` one. diff --git a/crates/socket-patch-core/src/vendor/yarn_classic_lock.rs b/crates/socket-patch-core/src/vendor/yarn_classic_lock.rs index 0d1958e8f..276bf4a39 100644 --- a/crates/socket-patch-core/src/vendor/yarn_classic_lock.rs +++ b/crates/socket-patch-core/src/vendor/yarn_classic_lock.rs @@ -2008,21 +2008,6 @@ wow@^1.0.0: ) .await .unwrap(); - // The root declares the outside-root path, so it is root-relative. - tokio::fs::write( - fx.root().join("package.json"), - serde_json::json!({ - "name": "app", - "version": "1.0.0", - "dependencies": { - "left-pad": "1.3.0", - "lp2": format!("file:../{ext_name}/lpdir"), - }, - }) - .to_string(), - ) - .await - .unwrap(); let (result, entry, warnings) = expect_done(fx.vendor(false).await); assert!(result.success, "{:?}", result.error); assert_eq!(entry.unwrap().wiring.len(), 1, "the registry block"); diff --git a/crates/socket-patch-core/src/vendor/yarn_copies.rs b/crates/socket-patch-core/src/vendor/yarn_copies.rs index 9ae9329a5..038d45969 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_at_root, CopyRef}; +use crate::formats::yarn::copy::{berry_copy_ref, classic_copy_ref, CopyRef}; 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}; @@ -49,17 +49,7 @@ pub(crate) async fn other_name_copies( name: &str, version: &str, ) -> Vec { - let berry = is_berry_lock(lock_text); - // Says which classic `file:` paths that leave the root are the root's - // own ([`classic_copy_ref_at_root`]). - let root_manifest = if berry { - None - } else { - read_regular_to_bytes(&project_root.join("package.json")) - .await - .ok() - }; - let candidates: Vec<(String, CopyRef, String)> = if berry { + let candidates: Vec<(String, CopyRef, String)> = if is_berry_lock(lock_text) { berry_entries(lock_text) .entries .into_iter() @@ -89,8 +79,7 @@ pub(crate) async fn other_name_copies( ) { return None; } - let source = - classic_copy_ref_at_root(&e.patterns, resolved, root_manifest.as_deref())?; + let source = classic_copy_ref(&e.patterns, resolved)?; let lock_version = classic_field(&e.block.lines, "version")?.to_string(); Some((e.block.key.clone(), source, lock_version)) }) diff --git a/crates/socket-patch-core/src/vex/discover/yarn.rs b/crates/socket-patch-core/src/vex/discover/yarn.rs index 73a6ea6d0..7fedd9ae9 100644 --- a/crates/socket-patch-core/src/vex/discover/yarn.rs +++ b/crates/socket-patch-core/src/vex/discover/yarn.rs @@ -90,7 +90,7 @@ use super::{ DIAG_REF_INVALID, DIAG_REF_UNATTRIBUTABLE, }; use crate::formats::yarn::blocks::{berry_field, classic_field}; -use crate::formats::yarn::copy::{berry_copy_ref, classic_copy_ref_at_root, CopyRef}; +use crate::formats::yarn::copy::{berry_copy_ref, classic_copy_ref, CopyRef}; use crate::formats::yarn::is_berry_lock; use crate::formats::yarn::patterns::{ classic_key_real_name, classic_pattern_real_name, resolution_selector_target, @@ -142,12 +142,9 @@ fn stray_top_level_line(text: &str) -> Option<&str> { async fn extract_classic(ctx: &DiscoverCtx<'_>, entries: Vec, out: &mut Discovery) { let mut copies: Vec = Vec::new(); - // Says which `file:` paths that leave the root are the root's own - // ([`classic_copy_ref_at_root`]). - let root_manifest = ctx.read_advisory_bytes(PACKAGE_JSON).await; for entry in entries { if entry.live && !entry.patterns.is_empty() { - classic_block(ctx, &entry, root_manifest.as_deref(), &mut copies, out); + classic_block(ctx, &entry, &mut copies, out); } } record_copies(ctx, copies, out).await; @@ -156,18 +153,14 @@ async fn extract_classic(ctx: &DiscoverCtx<'_>, entries: Vec, out: &m /// The copy a classic `file:` / url block installs, to be read for the /// package it really holds ([`record_copies`], #1236): yarn 1 keys it by the /// DEPENDENCY name (`"lp2@file:./lpdir"`), like berry. The `file:` path is -/// the key pattern's, relative to the root, even when it leaves the root -/// (#1516); one that leaves the root and isn't the root `package.json`'s -/// own was resolved against a workspace member, and is left alone -/// ([`classic_copy_ref_at_root`]). -fn classic_copy( - entry: &YarnEntry, - resolved: Option<&str>, - root_manifest: Option<&[u8]>, -) -> Option { +/// the key pattern's: yarn 1 writes it relative to the root and normalized, +/// whichever `package.json` declared it (a member's `file:./lpdir` locks as +/// `file:./packages/a/lpdir`), so one that leaves the root is read there too +/// (#1516). +fn classic_copy(entry: &YarnEntry, resolved: Option<&str>) -> Option { Some(LockCopy { key: entry.block.key.clone(), - source: classic_copy_ref_at_root(&entry.patterns, resolved, root_manifest)?, + source: classic_copy_ref(&entry.patterns, resolved)?, version: classic_field(&entry.block.lines, "version")?.to_string(), }) } @@ -193,7 +186,6 @@ fn classic_block_purl(entry: &YarnEntry) -> Option { fn classic_block( ctx: &DiscoverCtx<'_>, entry: &YarnEntry, - root_manifest: Option<&[u8]>, copies: &mut Vec, out: &mut Discovery, ) { @@ -238,7 +230,7 @@ fn classic_block( ); // Locked under the dependency name: the directory's own // `package.json` says which package this copy is (#1236). - copies.extend(classic_copy(entry, resolved, root_manifest)); + copies.extend(classic_copy(entry, resolved)); return; } // `link:` ranges install from the working tree; `resolved` is inert. @@ -280,7 +272,7 @@ fn classic_block( if classic_copy_source(patterns, Some(resolved)) == CopySource::RemoteTarball && !root_anchored_spelling(resolved) { - copies.extend(classic_copy(entry, Some(resolved), root_manifest)); + copies.extend(classic_copy(entry, Some(resolved))); } return; }; @@ -1431,9 +1423,9 @@ mod tests { /// strips the first segment whatever it is), a non-registry url, and an /// absolute path (never read: an untrusted lock could name any file or /// share). Each now contests the ref. Controls: an outside-root copy - /// holding another package; an unreadable url copy of another version; - /// and an outside-root path the root `package.json` doesn't declare, - /// which yarn resolved against a workspace member, not the root. + /// holding another package; an unreadable url copy of another version. + /// A key the root `package.json` doesn't declare (a workspace member's, + /// which yarn 1 also locks root-relative) contests the same way. #[tokio::test] async fn issue_1516_unread_classic_copies_contest_the_ref() { let hosted = hosted_url("npm", "left-pad", "1.3.0", UUID_A, "left-pad-1.3.0.tgz"); @@ -1547,16 +1539,17 @@ mod tests { "{mode} url 2.0.0: {:#?}", out.diagnostics ); - // A workspace member's `"b": "file:../b"` locks as `b@file:../b`, - // relative to the member: not a root-relative path, left alone. - let member = "\"b@file:../b\":\n version \"1.3.0\"\n\n".to_string(); - let out = run(&project(&[wired.clone(), member], "left-pad")).await; - assert_eq!( - out.refs.len(), - 1, - "{mode} member path: {:#?}", - out.diagnostics + // yarn 1 locks a `file:` key relative to the root and normalized, + // whoever declared it (a member's `file:../../../ext/lpdir`, the + // root's `file:../ext/lpdir/`): a key the root `package.json` + // doesn't spell is read all the same. + let p = project(&[wired.clone(), copies[0].2.clone()], "left-pad"); + p.write( + "package.json", + r#"{"name":"p","version":"1.0.0","workspaces":["packages/*"],"dependencies":{"left-pad":"1.3.0"}}"#, ); + let out = run(&p).await; + assert!(out.refs.is_empty(), "{mode} undeclared: {:#?}", out.refs); } }