From 7c9ece1c7f36180a9cdb36a910f77e1f23b3c0ff Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 10:24:59 +0000 Subject: [PATCH 1/4] Start fix for #1951 Assisted-by: Claude Code:claude-opus-5-5 From a941702950c2dcc8535751f1172773551d49b5fa Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 10:28:49 +0000 Subject: [PATCH 2/4] Test hosted names yarn copies as vendored does Hosted scans name a yarn classic file: directory copy through their own reader, which drops a copy outside the project root (file:../x) that vendored and VEX read and name. Add regression tests for the rewriter and the hosted manifest read, plus the shared reader they will use. Refs #1951 Assisted-by: Claude Code:claude-opus-5-5 --- .../src/formats/yarn/copy.rs | 32 +++++++- crates/socket-patch-core/src/hosted/engine.rs | 49 ++++++++++++ .../src/patch/redirect/mod.rs | 79 +++++++++++++++++++ 3 files changed, 159 insertions(+), 1 deletion(-) diff --git a/crates/socket-patch-core/src/formats/yarn/copy.rs b/crates/socket-patch-core/src/formats/yarn/copy.rs index fa502cd97..5da7bac80 100644 --- a/crates/socket-patch-core/src/formats/yarn/copy.rs +++ b/crates/socket-patch-core/src/formats/yarn/copy.rs @@ -11,7 +11,7 @@ //! [`CopyRef`], and so cannot disagree on which package it is. use super::patterns::split_classic_pattern; -use super::source::{is_tarball_path, registry_tarball_name}; +use super::source::{classic_copy_source, is_tarball_path, registry_tarball_name, CopySource}; /// Where a yarn lock entry's copy comes from, read for the package it holds. #[derive(Debug, Clone, PartialEq, Eq)] @@ -165,6 +165,36 @@ pub(crate) fn classic_copy_ref(patterns: &[String], resolved: Option<&str>) -> O } } +/// The package a classic `file:` directory or url copy really installs, +/// whatever dependency name its key carries (#1236), with where yarn 1 +/// installs it from: read through [`classic_copy_ref`] and +/// [`CopyRef::identity`], the reader vendored and VEX use, so the hosted +/// rewriter makes the same decision for every copy (#1951). `read_text` +/// gets [`CopyRef::read_path`] (root-relative, possibly climbing above the +/// root; never absolute). `None` for any other block, a `file:` tarball +/// (whose bytes a text reader can't open), a url naming no registry +/// package, or an unreadable or nameless manifest. +pub(crate) fn classic_copy_name( + patterns: &[String], + resolved: Option<&str>, + version: &str, + read_text: impl Fn(&str) -> Option, +) -> Option<(String, CopySource)> { + let source = classic_copy_source(patterns, resolved); + let copy = classic_copy_ref(patterns, resolved)?; + match (source, ©) { + (CopySource::Directory, CopyRef::Directory(_)) + | (CopySource::RemoteTarball, CopyRef::Url(_)) => {} + _ => return None, + } + let text = match copy.read_path() { + Some(rel) => Some(read_text(&rel)?), + None => None, + }; + let (name, _) = copy.identity(version, text.as_deref().map(str::as_bytes))?; + Some((name, source)) +} + /// The copy a berry locator `reference` installs from: a `file:` tarball or /// directory, a `portal:` / `link:` directory, an `exec:` script (each /// relative to the workspace its `locator=` binding names), an http(s) diff --git a/crates/socket-patch-core/src/hosted/engine.rs b/crates/socket-patch-core/src/hosted/engine.rs index f214e686d..33cc65660 100644 --- a/crates/socket-patch-core/src/hosted/engine.rs +++ b/crates/socket-patch-core/src/hosted/engine.rs @@ -4329,6 +4329,55 @@ mod tests { (read, done) } + /// #1951: the hosted run reads a yarn classic `file:` directory copy's + /// `package.json` where vendored and VEX read it + /// ([`crate::formats::yarn::copy::CopyRef::read_path`]): from outside + /// the root too, since yarn 1 installs `file:../ext/lpdir` all the same, + /// but never at an absolute path a lock names (#1516). + #[tokio::test] + async fn issue_1951_yarn_classic_copy_manifests_read_as_vendored_reads_them() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join("app"); + let manifest = r#"{"name":"left-pad","version":"1.3.0"}"#; + for dir in ["app/lpdir", "ext/lpdir", "abs/lpdir"] { + let dir = tmp.path().join(dir); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::write(dir.join("package.json"), manifest).unwrap(); + } + let abs = tmp.path().join("abs/lpdir"); + let abs = abs.to_str().unwrap().replace('\\', "/"); + std::fs::write( + root.join("yarn.lock"), + format!( + "# yarn lockfile v1\n\n\n\ + left-pad@1.3.0:\n version \"1.3.0\"\n \ + resolved \"https://registry.yarnpkg.com/left-pad/-/left-pad-1.3.0.tgz#bbbb\"\n\n\ + \"lp2@file:./lpdir\":\n version \"1.3.0\"\n\n\ + \"lp3@file:../ext/lpdir\":\n version \"1.3.0\"\n\n\ + \"lp4@file:{abs}\":\n version \"1.3.0\"\n" + ), + ) + .unwrap(); + let candidates = vec![left_pad_candidate()]; + let read = + read_candidate_files(&ProjectView::Disk(&root), &BTreeSet::new(), &candidates).await; + assert_eq!( + read.files.get("lpdir/package.json").map(String::as_str), + Some(manifest) + ); + assert_eq!( + read.files.get("../ext/lpdir/package.json").map(String::as_str), + Some(manifest), + "{:?}", + read.files.keys() + ); + assert!( + !read.files.keys().any(|k| k.contains("abs/lpdir")), + "{:?}", + read.files.keys() + ); + } + #[tokio::test] async fn issue_490_the_root_manifest_overrides_reach_the_npm_rewriter() { let redirected = |done: &Rewritten| { diff --git a/crates/socket-patch-core/src/patch/redirect/mod.rs b/crates/socket-patch-core/src/patch/redirect/mod.rs index ccb8bb226..8a64cb715 100644 --- a/crates/socket-patch-core/src/patch/redirect/mod.rs +++ b/crates/socket-patch-core/src/patch/redirect/mod.rs @@ -13751,6 +13751,85 @@ mod tests { } } + /// #1951: the hosted rewriter names a yarn classic `file:` / url copy + /// through the same reader vendored and VEX use + /// ([`crate::formats::yarn::copy::classic_copy_ref`]), so the modes + /// make the same decision for every copy. yarn 1 installs + /// `file:../ext/lpdir` from outside the root as readily as from inside + /// it, so that copy is read and named; an absolute `file:` path is never + /// read (a lock is untrusted input, #1516); a codeload tarball names no + /// registry package. + #[test] + fn issue_1951_yarn_classic_copy_named_as_vendored_reads_it() { + use crate::formats::yarn::copy::classic_copy_ref; + let ovr = npm_override( + "left-pad", + "1.3.0", + "http://p.test/lp.tgz", + "sha512-PATCHED==", + ); + let registry_block = "left-pad@1.3.0:\n version \"1.3.0\"\n \ + resolved \"https://registry.yarnpkg.com/left-pad/-/left-pad-1.3.0.tgz#bbbb\"\n \ + integrity sha512-UPSTREAMupstream==\n"; + let left_pad = r#"{"name":"left-pad","version":"1.3.0"}"#; + let manifests = BTreeMap::from([ + ("../ext/lpdir/package.json".to_string(), left_pad.to_string()), + ("/abs/lpdir/package.json".to_string(), left_pad.to_string()), + ("lpdir/package.json".to_string(), left_pad.to_string()), + ]); + let codeload = "https://codeload.github.com/stevemao/left-pad/tar.gz/abc"; + for (case, patterns, resolved, named) in [ + ("in-root directory", "lp2@file:./lpdir", None, true), + ("out-of-root directory", "lp2@file:../ext/lpdir", None, true), + ("absolute directory", "lp2@file:/abs/lpdir", None, false), + ( + "codeload tarball", + "lp2@stevemao/left-pad#v1.3.0", + Some(codeload), + false, + ), + ] { + // What vendored and VEX decide for this copy. + let pats = vec![patterns.to_string()]; + let vendored = classic_copy_ref(&pats, resolved).and_then(|copy| { + let bytes = copy + .read_path() + .and_then(|rel| manifests.get(&rel)) + .map(|t| t.as_bytes()); + copy.identity("1.3.0", bytes) + }); + assert_eq!( + vendored.is_some(), + named, + "{case}: vendored reads {vendored:?}" + ); + + let mut block = format!("\"{patterns}\":\n version \"1.3.0\"\n"); + if let Some(url) = resolved { + block.push_str(&format!(" resolved \"{url}\"\n")); + } + let mut files = manifests.clone(); + files.insert( + "yarn.lock".to_string(), + format!("# yarn lockfile v1\n\n\n{registry_block}\n{block}"), + ); + let mut r = RewriteResult::default(); + rewrite_yarn_classic(&files, std::slice::from_ref(&ovr), &mut r); + assert!( + r.files["yarn.lock"].contains("resolved \"http://p.test/lp.tgz#5ha1\""), + "{case}: the registry block is still pinned: {:?}", + r.warnings + ); + let hosted = r.warnings.iter().any(|w| w.detail.contains("lp2@")); + assert_eq!(hosted, named, "{case}: {:?}", r.warnings); + assert_eq!( + r.bundled_skipped_uuids.contains(&ovr.patch_uuid), + named, + "{case}" + ); + } + } + /// #363: yarn 1 fetches a git-pattern block with its git fetcher from /// the block's `resolved`, so a hosted tarball there makes every later /// install fail. The block stays byte-identical with a named warning, From 682488b16c16b27af2537978a3564f04c28d6229 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 10:32:32 +0000 Subject: [PATCH 3/4] Read yarn classic copies through one reader A hosted scan named a yarn classic file: directory or url copy through its own reader in formats/yarn/source.rs, which dropped a directory outside the project root (file:../ext/x). Vendored and VEX read that copy and named it, so hosted reported a clean pin while yarn kept installing the unpatched out-of-root copy, without the directory-skipped warning vendored gives. Hosted now reads the copy's manifest and identity through formats::yarn::copy (classic_copy_ref, CopyRef::read_path and identity), the same reader vendored and VEX use: out-of-root copies are read, absolute paths never are. The second reader and its third dir/package.json join are deleted. Fixes #1951 Assisted-by: Claude Code:claude-opus-5-5 --- .../src/formats/yarn/source.rs | 54 ------------------- crates/socket-patch-core/src/hosted/engine.rs | 25 ++++----- .../src/patch/redirect/mod.rs | 7 ++- 3 files changed, 18 insertions(+), 68 deletions(-) diff --git a/crates/socket-patch-core/src/formats/yarn/source.rs b/crates/socket-patch-core/src/formats/yarn/source.rs index 126adef03..0d6732fac 100644 --- a/crates/socket-patch-core/src/formats/yarn/source.rs +++ b/crates/socket-patch-core/src/formats/yarn/source.rs @@ -1,7 +1,6 @@ //! Where yarn 1 installs a classic lock block's copy from, decided once for //! every mode that reads or rewrites the block. -use super::copy::manifest_name; use super::patterns::split_classic_pattern; use crate::vendor::npm_origin::npm_spec_is_registry; @@ -75,59 +74,6 @@ pub(crate) fn classic_copy_source(patterns: &[String], resolved: Option<&str>) - } } -/// The root-relative `file:` directory a classic block's key names, if -/// any: yarn 1 COPIES it into node_modules under the DEPENDENCY name -/// (`"lp2@file:./lpdir"`), so which package that copy is comes from the -/// directory's own `package.json` (#1236). `None` for a `file:` tarball or -/// a path that leaves the root. -pub(crate) fn classic_file_directory(patterns: &[String]) -> Option { - patterns.iter().find_map(|p| { - let path = split_classic_pattern(p)?.1.strip_prefix("file:")?; - let path = path.split('#').next().unwrap_or_default(); - if is_tarball_path(path) { - return None; - } - crate::utils::cargo_workspace::normalize_rel("", path) - }) -} - -/// The package a classic `file:` directory or url copy really installs, -/// whatever dependency name its key carries (#1236): the directory's -/// `package.json` `name` (read through `read_text`, given the -/// root-relative manifest path), or the registry package a url tarball's -/// path names. `None` when neither says (a `file:` tarball, a -/// non-registry url, an unreadable manifest). -pub(crate) fn classic_copy_real_name( - patterns: &[String], - resolved: Option<&str>, - version: &str, - read_text: impl Fn(&str) -> Option, -) -> Option<(String, CopySource)> { - match classic_copy_source(patterns, resolved) { - CopySource::Directory => { - let dir = classic_file_directory(patterns)?; - let manifest = if dir.is_empty() { - "package.json".to_string() - } else { - format!("{dir}/package.json") - }; - let name = manifest_name(read_text(&manifest)?.as_bytes())?; - Some((name, CopySource::Directory)) - } - CopySource::RemoteTarball => { - let url = resolved?.split('#').next().unwrap_or_default(); - if !url.starts_with("http") { - return None; - } - Some(( - registry_tarball_name(url, version)?, - CopySource::RemoteTarball, - )) - } - _ => None, - } -} - /// The package an npm registry tarball url serves, from its /// `//-/-.tgz` path (`` may be `@scope/leaf`, /// its `@` / `/` possibly percent-encoded); `None` for any other shape. diff --git a/crates/socket-patch-core/src/hosted/engine.rs b/crates/socket-patch-core/src/hosted/engine.rs index 33cc65660..501e0387f 100644 --- a/crates/socket-patch-core/src/hosted/engine.rs +++ b/crates/socket-patch-core/src/hosted/engine.rs @@ -693,26 +693,25 @@ pub async fn read_candidate_files( out.read(view, unreadable, NPMRC_REL).await; // A `file:` directory copy is locked under the DEPENDENCY name, so // the classic rewriter reads which package it is from the - // directory's `package.json` (#1236). Advisory: never rewritten, - // and an unreadable one only leaves that copy unnamed. - let dirs: BTreeSet = out + // directory's `package.json` (#1236), at the path vendored and VEX + // read it from (#1951): outside the root too, never at an absolute + // path. Advisory: never rewritten, and an unreadable one only + // leaves that copy unnamed. + let manifests: BTreeSet = out .files .get("yarn.lock") .map(|lock| { + use crate::formats::yarn::copy::{classic_copy_ref, CopyRef}; crate::vendor::lock_inventory::yarn::classic_entries(lock) .iter() - .filter_map(|e| { - crate::formats::yarn::source::classic_file_directory(&e.patterns) + .filter_map(|e| match classic_copy_ref(&e.patterns, None)? { + copy @ CopyRef::Directory(_) => copy.read_path(), + _ => None, }) .collect() }) .unwrap_or_default(); - for dir in dirs { - let rel = if dir.is_empty() { - "package.json".to_string() - } else { - format!("{dir}/package.json") - }; + for rel in manifests { if out.files.contains_key(&rel) { continue; } @@ -4366,7 +4365,9 @@ mod tests { Some(manifest) ); assert_eq!( - read.files.get("../ext/lpdir/package.json").map(String::as_str), + read.files + .get("../ext/lpdir/package.json") + .map(String::as_str), Some(manifest), "{:?}", read.files.keys() diff --git a/crates/socket-patch-core/src/patch/redirect/mod.rs b/crates/socket-patch-core/src/patch/redirect/mod.rs index 8a64cb715..20c573ffa 100644 --- a/crates/socket-patch-core/src/patch/redirect/mod.rs +++ b/crates/socket-patch-core/src/patch/redirect/mod.rs @@ -4590,7 +4590,7 @@ fn rewrite_yarn_classic_with( .zip(&heads) .map(|(b, (patterns, _))| { let version = classic_field(&b.lines, "version")?; - let (name, source) = crate::formats::yarn::source::classic_copy_real_name( + let (name, source) = crate::formats::yarn::copy::classic_copy_name( patterns, classic_field(&b.lines, "resolved"), version, @@ -13773,7 +13773,10 @@ mod tests { integrity sha512-UPSTREAMupstream==\n"; let left_pad = r#"{"name":"left-pad","version":"1.3.0"}"#; let manifests = BTreeMap::from([ - ("../ext/lpdir/package.json".to_string(), left_pad.to_string()), + ( + "../ext/lpdir/package.json".to_string(), + left_pad.to_string(), + ), ("/abs/lpdir/package.json".to_string(), left_pad.to_string()), ("lpdir/package.json".to_string(), left_pad.to_string()), ]); From 0c263aedcbccdd28b505b97cf6378a072f5943c6 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 11:00:00 +0000 Subject: [PATCH 4/4] Expect one yarn reader's warning outside the root A yarn classic file: directory copy outside the project root is now named by the yarn rewriter, with the same redirect_yarn_classic_directory_skipped warning an in-root copy gets, instead of the engine's generic redirect_copy_unpatched fallback. The e2e now asserts that code, and that the copy is named exactly once. The rewriter test's doc says what it compares: which package the copy holds. Refs #1951 Assisted-by: Claude Code:claude-opus-5-5 --- .../tests/e2e_redirect_yarn_classic_build.rs | 26 ++++++++++++++----- .../src/patch/redirect/mod.rs | 12 +++++---- 2 files changed, 26 insertions(+), 12 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 fc7b1b5eb..e38536c43 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 @@ -1542,9 +1542,10 @@ 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. +/// it stays unpatched. `scan --mode hosted` must name the copy exactly as +/// it names an in-root one (`redirect_yarn_classic_directory_skipped`, +/// read through the reader vendored and VEX use, #1951), once, 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() { @@ -1635,11 +1636,22 @@ async fn classic_outside_root_copy_under_another_name_is_named_and_not_attested( 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}" + let naming: Vec<&str> = env["warnings"] + .as_array() + .into_iter() + .flatten() + .filter(|w| { + w["detail"] + .as_str() + .is_some_and(|d| d.contains("lp2@file:../ext/lpdir")) + }) + .filter_map(|w| w["code"].as_str()) + .collect(); + assert_eq!( + naming, + ["redirect_yarn_classic_directory_skipped"], + "the outside-root copy must be named once, as an in-root one is: {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"), diff --git a/crates/socket-patch-core/src/patch/redirect/mod.rs b/crates/socket-patch-core/src/patch/redirect/mod.rs index 20c573ffa..d4d30c7de 100644 --- a/crates/socket-patch-core/src/patch/redirect/mod.rs +++ b/crates/socket-patch-core/src/patch/redirect/mod.rs @@ -13751,10 +13751,12 @@ mod tests { } } - /// #1951: the hosted rewriter names a yarn classic `file:` / url copy - /// through the same reader vendored and VEX use - /// ([`crate::formats::yarn::copy::classic_copy_ref`]), so the modes - /// make the same decision for every copy. yarn 1 installs + /// #1951: the hosted rewriter reads which package a yarn classic + /// `file:` / url copy holds through the same reader vendored and VEX use + /// ([`crate::formats::yarn::copy::classic_copy_ref`] and + /// [`crate::formats::yarn::copy::CopyRef::identity`]), so the modes + /// identify the same copies. (A copy no reader identifies is a separate + /// rule: vendored keeps it as a possible copy, #1516.) yarn 1 installs /// `file:../ext/lpdir` from outside the root as readily as from inside /// it, so that copy is read and named; an absolute `file:` path is never /// read (a lock is untrusted input, #1516); a codeload tarball names no @@ -13792,7 +13794,7 @@ mod tests { false, ), ] { - // What vendored and VEX decide for this copy. + // The package vendored and VEX identify this copy as. let pats = vec![patterns.to_string()]; let vendored = classic_copy_ref(&pats, resolved).and_then(|copy| { let bytes = copy