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 e36d0fbbf..f795b824d 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 @@ -1237,6 +1237,134 @@ async fn classic_file_directory_dependency_is_named_and_not_attested() { ); } +/// #1236: the same `file:` directory declared under ANOTHER dependency +/// name (`"lp2": "file:./lpdir"`, lpdir being left-pad@1.3.0) locks as +/// `"lp2@file:./lpdir"`, beside the registry left-pad. yarn 1 copies it +/// into `node_modules/lp2`, so it stays unpatched whatever the pin does. +/// `scan --mode hosted` must still pin the registry block, name the copy +/// (`redirect_yarn_classic_directory_skipped`), 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_file_directory_copy_under_another_name_is_named_and_not_attested() { + if !require_yarn_classic("e2e_redirect_yarn_classic_build (other-name file:)", |c| { + cache_env::isolate(c); + }) { + return; + } + let tmp = tempfile::tempdir().unwrap(); + let proj = tmp.path().join("proj"); + let copy = proj.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":"other-name-classic","version":"0.0.0","private":true,"dependencies":{{"{DEP}":"{DEP_VERSION}","lp2":"file:./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!( + "(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:./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_yarn_classic_directory_skipped"), + "the other-name copy must be named: {env}" + ); + assert!(env.to_string().contains("lp2@file:./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/source.rs b/crates/socket-patch-core/src/formats/yarn/source.rs index 859827ead..b9adf297a 100644 --- a/crates/socket-patch-core/src/formats/yarn/source.rs +++ b/crates/socket-patch-core/src/formats/yarn/source.rs @@ -74,6 +74,95 @@ 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_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, + } +} + +/// `name` of a `package.json`. +pub(crate) fn manifest_name(bytes: &[u8]) -> Option { + let bytes = crate::formats::text::strip_bom_bytes(bytes); + serde_json::from_slice::(bytes) + .ok()? + .get("name")? + .as_str() + .map(str::to_string) +} + +/// 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. +pub(crate) fn registry_tarball_name(url: &str, version: &str) -> Option { + let path = url.split_once("://").map_or(url, |(_, rest)| rest); + let path = path.split(['?']).next()?; + let (before, file) = path.rsplit_once("/-/")?; + let mut segs: Vec = before + .split('/') + .skip(1) // the host + .map(|seg| crate::utils::purl::percent_decode_purl_component(seg).into_owned()) + .collect(); + let leaf_name = segs.pop()?; + let (scope, leaf_name) = match leaf_name.split_once('/') { + Some((scope, leaf)) => (Some(scope.to_string()), leaf.to_string()), + None => (segs.pop().filter(|s| s.starts_with('@')), leaf_name), + }; + if file != format!("{leaf_name}-{version}.tgz") { + return None; + } + Some(match scope { + Some(scope) => format!("{scope}/{leaf_name}"), + None => leaf_name, + }) +} + /// A GitHub codeload tarball: what yarn 1 locks a hosted-git shorthand to. fn is_codeload_tarball(resolved: &str) -> bool { resolved diff --git a/crates/socket-patch-core/src/hosted/engine.rs b/crates/socket-patch-core/src/hosted/engine.rs index 3cdb08a39..53f5a7398 100644 --- a/crates/socket-patch-core/src/hosted/engine.rs +++ b/crates/socket-patch-core/src/hosted/engine.rs @@ -602,6 +602,35 @@ pub async fn read_candidate_files( out.read(view, unreadable, crate::patch::redirect::YARNRC_REL) .await; 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 + .files + .get("yarn.lock") + .map(|lock| { + crate::vendor::lock_inventory::yarn::classic_entries(lock) + .iter() + .filter_map(|e| { + crate::formats::yarn::source::classic_file_directory(&e.patterns) + }) + .collect() + }) + .unwrap_or_default(); + for dir in dirs { + let rel = if dir.is_empty() { + "package.json".to_string() + } else { + format!("{dir}/package.json") + }; + if out.files.contains_key(&rel) { + continue; + } + if let Some(text) = read_advisory(view, unreadable, &rel).await { + out.files.insert(rel, text); + } + } } // Cargo workspace members (and in-root path dependencies) declare diff --git a/crates/socket-patch-core/src/patch/redirect/mod.rs b/crates/socket-patch-core/src/patch/redirect/mod.rs index a9a882c25..874dd7e94 100644 --- a/crates/socket-patch-core/src/patch/redirect/mod.rs +++ b/crates/socket-patch-core/src/patch/redirect/mod.rs @@ -3707,6 +3707,22 @@ fn rewrite_yarn_classic_with( // re-pin never changes a block's key line. let heads: Vec<(Vec, Option)> = blocks.iter().map(|b| classic_block_head(&b.key)).collect(); + // The package each `file:` directory / url copy really installs, with + // its version, when its key names another (#1236): read once per block. + let renamed: Vec> = blocks + .iter() + .zip(&heads) + .map(|(b, (patterns, _))| { + let version = classic_field(&b.lines, "version")?; + let (name, source) = crate::formats::yarn::source::classic_copy_real_name( + patterns, + classic_field(&b.lines, "resolved"), + version, + |rel| files.get(rel).cloned(), + )?; + Some((name, version.to_string(), source)) + }) + .collect(); // A pinned block's new lines are kept in `blocks[i].lines` (so a later // dep reads the pinned fields) and spliced over the block's original // byte span once, at the end: re-scanning and re-copying the whole lock @@ -3737,6 +3753,43 @@ fn rewrite_yarn_classic_with( // matching on the alias name alone would hijack the fork. let (patterns, real_name) = &heads[i]; if real_name.as_deref() != Some(fname.as_str()) { + // yarn 1 keys a `file:` directory or url copy by the + // DEPENDENCY name (`"lp2@file:./lpdir"`), so a copy of this + // package under another name is read from the copy itself + // (#1236). No rewrite reaches it: named, and never assumed + // applied by the in-run VEX. + let Some((version, source)) = renamed[i] + .as_ref() + .filter(|(name, _, _)| *name == fname) + .map(|(_, version, source)| (version, *source)) + else { + continue; + }; + if *version != dep.version { + continue; + } + copy_skipped = true; + result.bundled_skipped_uuids.insert(dep.patch_uuid.clone()); + let (code, from) = match source { + CopySource::Directory => ( + "redirect_yarn_classic_directory_skipped", + "a file: directory, which yarn copies into node_modules rather than \ + fetching a tarball", + ), + _ => ( + "redirect_yarn_classic_non_registry_entry_skipped", + "a URL tarball", + ), + }; + result.warnings.push(RewriteWarning { + code: code.into(), + detail: format!( + "lock entry `{}` installs {fname}@{version} under another dependency \ + name, from {from}; the hosted redirect leaves it untouched, so this \ + copy stays unpatched", + blocks[i].key + ), + }); continue; } let block = &blocks[i]; @@ -11227,6 +11280,79 @@ mod tests { } } + /// #1236: yarn 1 locks a `file:` directory or url copy under the + /// DEPENDENCY name (`"lp2@file:./lpdir"`), so a copy of the patched + /// left-pad@1.3.0 declared as `lp2` is read from the copy itself (the + /// directory's `package.json`, the registry url's path). The registry + /// block is still pinned; the copy is named with the same codes as a + /// same-name copy and kept out of the in-run VEX. Controls: a copy of + /// another package, of another version, or whose manifest is not + /// readable is not named. + #[test] + fn issue_1236_yarn_classic_other_name_copy_is_named() { + 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 dir_block = "\"lp2@file:./lpdir\":\n version \"1.3.0\"\n"; + let url_block = "\"lp2@https://registry.npmjs.org/left-pad/-/left-pad-1.3.0.tgz\":\n \ + version \"1.3.0\"\n \ + resolved \"https://registry.npmjs.org/left-pad/-/left-pad-1.3.0.tgz#bbbb\"\n"; + let run = |copy: &str, manifest: Option<&str>| { + let mut files = BTreeMap::new(); + files.insert( + "yarn.lock".to_string(), + format!("# yarn lockfile v1\n\n\n{registry_block}\n{copy}"), + ); + if let Some(m) = manifest { + files.insert("lpdir/package.json".to_string(), m.to_string()); + } + 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\""), + "the registry block is still pinned: {:?}", + r.files + ); + assert!(r.files["yarn.lock"].contains(copy), "copy byte-identical"); + r + }; + let left_pad = r#"{"name":"left-pad","version":"1.3.0"}"#; + for (copy, code) in [ + (dir_block, "redirect_yarn_classic_directory_skipped"), + ( + url_block, + "redirect_yarn_classic_non_registry_entry_skipped", + ), + ] { + let r = run(copy, Some(left_pad)); + let named: Vec<&RewriteWarning> = + r.warnings.iter().filter(|w| w.code == code).collect(); + assert_eq!(named.len(), 1, "{copy}: {:?}", r.warnings); + assert!(named[0].detail.contains("lp2@"), "{:?}", named[0]); + assert!(r.bundled_skipped_uuids.contains(&ovr.patch_uuid), "{copy}"); + } + let other_version = dir_block.replace("1.3.0", "1.2.0"); + for (case, copy, manifest) in [ + ( + "another package", + dir_block, + Some(r#"{"name":"other-pkg","version":"1.3.0"}"#), + ), + ("another version", other_version.as_str(), Some(left_pad)), + ("no readable manifest", dir_block, None), + ] { + let r = run(copy, manifest); + assert!(r.warnings.is_empty(), "{case}: {:?}", r.warnings); + assert!(r.bundled_skipped_uuids.is_empty(), "{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, 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 fb1fb2f17..fe2d53c4e 100644 --- a/crates/socket-patch-core/src/vendor/yarn_classic_lock.rs +++ b/crates/socket-patch-core/src/vendor/yarn_classic_lock.rs @@ -120,9 +120,14 @@ impl NpmLockBackend for YarnClassicBackend { // ── 3. Find the rewritable blocks (pre-flight, BEFORE staging) ──── let blocks = scan_blocks_shared(&text); + let other_name = + other_name_copy_warnings(project_root, &blocks, &coords.name, &coords.version).await; let (candidate_keys, skipped) = - rewritable_candidates(&blocks, &coords.name, &coords.version)?; + rewritable_candidates(&blocks, &coords.name, &coords.version).map_err(|o| { + only_other_name_copies(o, &blocks, &coords.name, &coords.version, &other_name) + })?; warnings.extend(skipped); + warnings.extend(other_name); Ok(YarnClassicPlan { text, candidate_keys, @@ -337,6 +342,108 @@ fn rewritable_candidates( Ok((candidate_keys, skipped)) } +/// A named warning for each block that installs `name@version` under +/// ANOTHER dependency name (#1236): yarn 1 keys a `file:` directory or url +/// copy by the name the depender gave it (`"lp2@file:./lpdir"`), so which +/// package it is comes from the copy itself (the directory's +/// `package.json`, the registry url's path). No re-lock of `name` reaches +/// that copy, so it stays unpatched; the registry blocks are still wired. +async fn other_name_copy_warnings( + project_root: &Path, + blocks: &[LockBlock], + name: &str, + version: &str, +) -> Vec { + let mut out = Vec::new(); + for block in blocks { + let patterns = split_key_patterns(&block.key); + if classic_key_real_name(&patterns) == Some(name) + || classic_field(&block.lines, "version") != Some(version) + { + continue; + } + let manifest = crate::formats::yarn::source::classic_file_directory(&patterns).map(|dir| { + if dir.is_empty() { + "package.json".to_string() + } else { + format!("{dir}/package.json") + } + }); + let text = match manifest { + Some(rel) => read_regular_to_string(&project_root.join(&rel)).await.ok(), + None => None, + }; + let copy = crate::formats::yarn::source::classic_copy_real_name( + &patterns, + classic_field(&block.lines, "resolved"), + version, + |_| text.clone(), + ); + let Some((_, source)) = copy.filter(|(n, _)| n == name) else { + continue; + }; + let (code, from) = match source { + CopySource::Directory => ( + "vendor_link_entry_skipped", + "a file: directory, which yarn copies into node_modules", + ), + _ => ( + "vendor_yarn_classic_non_registry_entry_skipped", + "a URL tarball", + ), + }; + out.push(VendorWarning::new( + code, + format!( + "lock entry `{}` installs {name}@{version} under another dependency name, \ + from {from}; vendoring can't rewire it, so this copy stays UNPATCHED", + block.key + ), + )); + } + out +} + +/// A `vendor_lock_entry_not_found` refusal of a package whose only copies +/// are locked under ANOTHER dependency name ([`other_name_copy_warnings`]) +/// becomes `vendor_lock_entry_not_rewritable` naming them: the package IS +/// installed, and `yarn install` can't re-lock those copies to it (#1236). +/// A block of the package under its OWN name (one with no `resolved`, +/// which `yarn install` does re-lock) keeps the "not found" remedy. Any +/// other refusal passes through. +fn only_other_name_copies( + refusal: Box, + blocks: &[LockBlock], + name: &str, + version: &str, + other_name: &[VendorWarning], +) -> Box { + if other_name.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!( + "every {YARN_LOCK} block for {name}@{version} is a copy locked under another \ + dependency name, which vendoring can't rewire — those copies stay UNPATCHED and \ + `yarn install` will not help: {}", + details.join("; ") + ), + )) +} + +/// Whether any block locks `name@version` under the package's own name. +fn has_own_name_block(blocks: &[LockBlock], name: &str, version: &str) -> bool { + blocks.iter().any(|b| { + classic_key_real_name(&split_key_patterns(&b.key)) == Some(name) + && classic_field(&b.lines, "version") == Some(version) + }) +} + /// The lock as [`vendor_yarn_classic`]'s step 2 leaves it: read, re-sniffed /// and scanned into blocks. Read once for the vendor loop's download plan /// ([`preflight_packages`]); the loop itself runs the same steps inline, @@ -367,16 +474,34 @@ pub(crate) async fn preflight_packages( project_root: &Path, packages: &[(&str, &PatchRecord)], ) -> Vec> { - super::npm_common::gate_packages( - read_project(project_root).await, - packages, - |project, coords| { - let (name, version) = (coords.name.as_str(), coords.version.as_str()); - rewritable_candidates(&project.blocks, name, version) - .map(drop) - .map_err(|o| super::npm_common::refusal_code(&o)) - }, - ) + let project = read_project(project_root).await; + let blocks = project.as_ref().ok().map(|p| Arc::clone(&p.blocks)); + let mut gated = super::npm_common::gate_packages(project, packages, |project, coords| { + let (name, version) = (coords.name.as_str(), coords.version.as_str()); + rewritable_candidates(&project.blocks, name, version) + .map(drop) + .map_err(|o| super::npm_common::refusal_code(&o)) + }); + // The loop's step 3 turns "not found" into "not rewritable" when the + // package's only copies are locked under another name; so does the plan. + if let Some(blocks) = blocks { + for ((purl, record), gate) in packages.iter().zip(gated.iter_mut()) { + if *gate != Err("vendor_lock_entry_not_found") { + continue; + } + let Ok(coords) = super::npm_common::guard_coordinates(purl, record) else { + continue; + }; + if !has_own_name_block(&blocks, &coords.name, &coords.version) + && !other_name_copy_warnings(project_root, &blocks, &coords.name, &coords.version) + .await + .is_empty() + { + *gate = Err("vendor_lock_entry_not_rewritable"); + } + } + } + gated } /// Undo one yarn-classic vendored package: restore the recorded lock blocks @@ -1568,6 +1693,97 @@ left-pad@^1.3.0: assert!(text.contains("\"left-pad@file:./local-left-pad\":\n version \"1.3.0\"")); } + /// #1236: yarn 1 locks a `file:` directory or url copy under the + /// DEPENDENCY name, so a copy of left-pad@1.3.0 declared as `lp2` is + /// read from the copy itself and named; the registry block is still + /// wired and the copy stays byte-untouched. Control: a directory + /// holding another package is not named. + #[tokio::test] + async fn issue_1236_other_name_copies_are_named() { + let extra = r#" +"lp2@file:./lpdir": + version "1.3.0" + +"lp3@https://registry.npmjs.org/left-pad/-/left-pad-1.3.0.tgz": + version "1.3.0" + resolved "https://registry.npmjs.org/left-pad/-/left-pad-1.3.0.tgz#bbbb" +"#; + let lock = format!("{Y2_BEFORE}{extra}"); + for (fork, named) in [("left-pad", true), ("other-pkg", false)] { + let fx = fixture_with_lock(&lock).await; + tokio::fs::create_dir_all(fx.root().join("lpdir")) + .await + .unwrap(); + tokio::fs::write( + fx.root().join("lpdir/package.json"), + format!(r#"{{"name":"{fork}","version":"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 dir: Vec<&VendorWarning> = warnings + .iter() + .filter(|w| w.code == "vendor_link_entry_skipped" && w.detail.contains("lp2@")) + .collect(); + assert_eq!(dir.len(), usize::from(named), "{fork}: {warnings:?}"); + let url: Vec<&VendorWarning> = warnings + .iter() + .filter(|w| { + w.code == "vendor_yarn_classic_non_registry_entry_skipped" + && w.detail.contains("lp3@") + }) + .collect(); + assert_eq!(url.len(), 1, "{fork}: {warnings:?}"); + let text = fx.lock_text().await; + assert!(text.contains("\"lp2@file:./lpdir\":\n version \"1.3.0\"")); + } + } + + /// #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 + /// `vendor_lock_entry_not_found` with a `yarn install` remedy that + /// can't help. Control: a directory holding another package is still + /// "not found". + #[tokio::test] + async fn issue_1236_only_other_name_copies_are_refused_as_not_rewritable() { + let lock = "# yarn lockfile v1\n\n\n\"lp2@file:./lpdir\":\n version \"1.3.0\"\n"; + // Review: beside a same-name block with no `resolved`, which + // `yarn install` does re-lock, "not found" and its remedy stand. + let with_unresolved = format!("{lock}\nleft-pad@^1.3.0:\n version \"1.3.0\"\n"); + for (lock, fork, code) in [ + (lock, "left-pad", "vendor_lock_entry_not_rewritable"), + (lock, "other-pkg", "vendor_lock_entry_not_found"), + ( + with_unresolved.as_str(), + "left-pad", + "vendor_lock_entry_not_found", + ), + ] { + let fx = fixture_with_lock(lock).await; + tokio::fs::create_dir_all(fx.root().join("lpdir")) + .await + .unwrap(); + tokio::fs::write( + fx.root().join("lpdir/package.json"), + format!(r#"{{"name":"{fork}","version":"1.3.0"}}"#), + ) + .await + .unwrap(); + let detail = expect_refused(fx.vendor(false).await, code); + if code == "vendor_lock_entry_not_rewritable" { + assert!(detail.contains("lp2@file:./lpdir"), "{detail}"); + assert!(detail.contains("yarn install` will not help"), "{detail}"); + } + assert_eq!(fx.lock_text().await, lock); + let pre = + preflight_packages(fx.root(), &[("pkg:npm/left-pad@1.3.0", &fx.record)]).await; + assert_eq!(pre, vec![Err(code)], "{fork}"); + } + } + #[tokio::test] async fn no_matching_block_is_refused_before_any_write() { // The lock only knows a different version. diff --git a/crates/socket-patch-core/src/vex/discover/yarn.rs b/crates/socket-patch-core/src/vex/discover/yarn.rs index c871e772e..5efa3a844 100644 --- a/crates/socket-patch-core/src/vex/discover/yarn.rs +++ b/crates/socket-patch-core/src/vex/discover/yarn.rs @@ -92,10 +92,12 @@ use super::{ use crate::formats::yarn::blocks::{berry_field, classic_field}; use crate::formats::yarn::is_berry_lock; use crate::formats::yarn::patterns::{ - classic_key_real_name, pattern_real_name, resolution_selector_target, split_resolved_sha1, - BerryLocator, + classic_key_real_name, pattern_real_name, resolution_selector_target, split_pattern, + split_resolved_sha1, BerryLocator, +}; +use crate::formats::yarn::source::{ + classic_copy_source, manifest_name, registry_tarball_name, CopySource, }; -use crate::formats::yarn::source::{classic_copy_source, CopySource}; use crate::utils::digest::is_sri_pin; use crate::vendor::lock_inventory::yarn::{ berry_checksum_pin, berry_entries, classic_entries, BerryLock, YarnEntry, @@ -124,7 +126,7 @@ pub(crate) async fn extract(ctx: &DiscoverCtx<'_>, out: &mut Discovery) { if is_berry_lock(text) { extract_berry(ctx, berry_entries(text), out).await; } else { - extract_classic(ctx, classic_entries(text), out); + extract_classic(ctx, classic_entries(text), out).await; } } @@ -139,12 +141,45 @@ fn stray_top_level_line(text: &str) -> Option<&str> { // ── classic ────────────────────────────────────────────────────────────── -fn extract_classic(ctx: &DiscoverCtx<'_>, entries: Vec, out: &mut Discovery) { +async fn extract_classic(ctx: &DiscoverCtx<'_>, entries: Vec, out: &mut Discovery) { + let mut copies: Vec = Vec::new(); for entry in entries { if entry.live && !entry.patterns.is_empty() { - classic_block(ctx, &entry, out); + classic_block(ctx, &entry, &mut copies, out); } } + record_copies(ctx, copies, out).await; +} + +/// 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 { + let version = classic_field(&entry.block.lines, "version")?; + let file = entry.patterns.iter().find_map(|p| { + let (_, range) = split_pattern(p)?; + range.strip_prefix("file:") + }); + let source = match file { + Some(path) => { + let path = path.split('#').next().unwrap_or_default(); + CopyRef::File(crate::utils::cargo_workspace::normalize_rel("", path)?) + } + None => { + let url = resolved?.split('#').next().unwrap_or_default(); + if !url.starts_with("http") { + return None; + } + CopyRef::Url(url.to_string()) + } + }; + Some(LockCopy { + key: entry.block.key.clone(), + source, + version: version.to_string(), + }) } /// The purl a block stands for when every key pattern names one package. @@ -165,7 +200,12 @@ fn classic_block_purl(entry: &YarnEntry) -> Option { /// installs ONE copy per `name@version`, and which block it takes depends /// on which pattern it resolves first, so no wiring of the same version in /// this lock is attested beside it. -fn classic_block(ctx: &DiscoverCtx<'_>, entry: &YarnEntry, out: &mut Discovery) { +fn classic_block( + ctx: &DiscoverCtx<'_>, + entry: &YarnEntry, + copies: &mut Vec, + out: &mut Discovery, +) { let YarnEntry { block, patterns, .. } = entry; @@ -205,6 +245,9 @@ fn classic_block(ctx: &DiscoverCtx<'_>, entry: &YarnEntry, out: &mut Discovery) "installs from a file: directory, which yarn copies into node_modules \ rather than fetching a tarball", ); + // Locked under the dependency name: the directory's own + // `package.json` says which package this copy is (#1236). + copies.extend(classic_copy(entry, resolved)); return; } // `link:` ranges install from the working tree; `resolved` is inert. @@ -239,6 +282,13 @@ fn classic_block(ctx: &DiscoverCtx<'_>, entry: &YarnEntry, out: &mut Discovery) ), ); } + // A user's `file:` tarball or url copy is keyed by the dependency + // name, so the package it installs is read from the copy (#1236). + if classic_copy_source(patterns, Some(resolved)) == CopySource::RemoteTarball + && !root_anchored_spelling(resolved) + { + copies.extend(classic_copy(entry, Some(resolved))); + } return; }; let name = match names.as_slice() { @@ -299,7 +349,7 @@ struct BerryHostedKeyed { async fn extract_berry(ctx: &DiscoverCtx<'_>, lock: BerryLock, out: &mut Discovery) { let mut vendored: Vec = Vec::new(); let mut hosted_keyed: Vec = Vec::new(); - let mut copies: Vec = Vec::new(); + let mut copies: Vec = Vec::new(); for entry in lock.entries.iter().filter(|e| e.live) { berry_block( ctx, @@ -311,7 +361,7 @@ async fn extract_berry(ctx: &DiscoverCtx<'_>, lock: BerryLock, out: &mut Discove out, ); } - record_berry_copies(ctx, copies, out).await; + record_copies(ctx, copies, out).await; confirm_berry_hosted_keyed(ctx, hosted_keyed, out).await; confirm_berry_vendored(ctx, vendored, out).await; } @@ -389,7 +439,7 @@ fn berry_block( cache_key: Option<&str>, vendored: &mut Vec, hosted_keyed: &mut Vec, - copies: &mut Vec, + copies: &mut Vec, out: &mut Discovery, ) { let block = &entry.block; @@ -436,11 +486,20 @@ fn berry_block( } else if locator_version.is_none() && !root_anchored_spelling(spec) { // A user's `file:` / url copy: yarn keys it by the DEPENDENCY // name (`lp2@file:…`), so which package it installs is read - // from the copy itself ([`record_berry_copies`], #939). - if let Some(version) = berry_field(&block.lines, "version") { - copies.push(BerryCopy { + // from the copy itself ([`record_copies`], #939). + let source = match reference.strip_prefix("file:") { + Some(path) => { + let path = path.split(['#', ':']).next().unwrap_or_default(); + berry_file_copy_path(reference, path).map(CopyRef::File) + } + None => Some(CopyRef::Url( + reference.split('#').next().unwrap_or_default().to_string(), + )), + }; + if let (Some(version), Some(source)) = (berry_field(&block.lines, "version"), source) { + copies.push(LockCopy { key: block.key.clone(), - reference: reference.to_string(), + source, version: version.to_string(), }); } @@ -726,62 +785,63 @@ fn emit( } } -/// A berry lock entry that installs a user's `file:` tarball / directory or -/// url tarball (not a Socket wiring), awaiting [`record_berry_copies`]. -struct BerryCopy { +/// A yarn lock entry (berry or classic) that installs a user's `file:` +/// tarball / directory or url tarball (not a Socket wiring), awaiting +/// [`record_copies`]. +struct LockCopy { key: String, - /// The locator's reference (`file:[#…][::…]` or a url). - reference: String, + source: CopyRef, version: String, } -/// Record each [`BerryCopy`] as an unpatched copy of the package it really -/// installs (#939). yarn keys a `file:` / url dependency by 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 `resolutions` pin of -/// `left-pad` reaches it. The real name comes from the copy itself: the -/// registry tarball path of a url (`…//-/-.tgz`), the -/// `package.json` inside a `file:` tarball, or a `file:` directory's -/// `package.json`. A copy whose name cannot be read is left alone. -async fn record_berry_copies(ctx: &DiscoverCtx<'_>, copies: Vec, out: &mut Discovery) { +/// Where a [`LockCopy`]'s bytes come from. +enum CopyRef { + /// A root-relative `file:` tarball or directory. + File(String), + /// A url tarball, `#` fragment dropped. + Url(String), +} + +/// Record each [`LockCopy`] as an unpatched copy of the package it really +/// installs (#939 berry, #1236 classic). yarn keys a `file:` / url +/// dependency by 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 name comes from the copy itself: the registry tarball path +/// of a url (`…//-/-.tgz`), the `package.json` inside +/// a `file:` tarball, or a `file:` directory's `package.json`. A copy whose +/// name cannot be read is left alone. +async fn record_copies(ctx: &DiscoverCtx<'_>, copies: Vec, out: &mut Discovery) { for copy in copies { - let (name, how) = if let Some(path) = copy.reference.strip_prefix("file:") { - let path = path - .split(['#', ':']) - .next() - .unwrap_or_default() - .to_string(); - let Some(rel) = berry_file_copy_path(©.reference, &path) else { - continue; - }; - let name = if is_tarball_leaf(&rel) { - match ctx.read_advisory_bytes(&rel).await { - Some(bytes) => { - tokio::task::spawn_blocking(move || tarball_package_name(&bytes)) - .await - .ok() - .flatten() + let (name, how) = match ©.source { + CopyRef::File(rel) => { + let name = if is_tarball_leaf(rel) { + match ctx.read_advisory_bytes(rel).await { + Some(bytes) => { + tokio::task::spawn_blocking(move || tarball_package_name(&bytes)) + .await + .ok() + .flatten() + } + None => None, } - None => None, - } - } else { - let manifest = if rel.is_empty() { - PACKAGE_JSON.to_string() } else { - format!("{rel}/{PACKAGE_JSON}") + let manifest = if rel.is_empty() { + PACKAGE_JSON.to_string() + } else { + format!("{rel}/{PACKAGE_JSON}") + }; + match ctx.read_advisory_text(&manifest).await { + Some(text) => manifest_name(text.as_bytes()), + None => None, + } }; - match ctx.read_advisory_text(&manifest).await { - Some(text) => manifest_name(text.as_bytes()), - None => None, - } - }; - (name, format!("installs it from the user's file:{path}")) - } else { - let url = copy.reference.split(['#']).next().unwrap_or_default(); - ( + (name, format!("installs it from the user's file:{rel}")) + } + CopyRef::Url(url) => ( registry_tarball_name(url, ©.version), format!("installs it from {url:?}"), - ) + ), }; let Some(name) = name else { continue; @@ -824,48 +884,12 @@ fn is_tarball_leaf(path: &str) -> bool { path.ends_with(".tgz") || path.ends_with(".tar.gz") } -/// `name` of a `package.json`. -fn manifest_name(bytes: &[u8]) -> Option { - let bytes = bytes.strip_prefix(b"\xef\xbb\xbf").unwrap_or(bytes); - serde_json::from_slice::(bytes) - .ok()? - .get("name")? - .as_str() - .map(str::to_string) -} - /// `name` of the `package.json` inside an npm tarball. fn tarball_package_name(bytes: &[u8]) -> Option { let map = crate::patch::package::read_archive_bytes_to_map(bytes).ok()?; manifest_name(map.get(PACKAGE_JSON)?) } -/// 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. -fn registry_tarball_name(url: &str, version: &str) -> Option { - let path = url.split_once("://").map_or(url, |(_, rest)| rest); - let path = path.split(['?']).next()?; - let (before, file) = path.rsplit_once("/-/")?; - let mut segs: Vec = before - .split('/') - .skip(1) // the host - .map(|seg| crate::utils::purl::percent_decode_purl_component(seg).into_owned()) - .collect(); - let leaf_name = segs.pop()?; - let (scope, leaf_name) = match leaf_name.split_once('/') { - Some((scope, leaf)) => (Some(scope.to_string()), leaf.to_string()), - None => (segs.pop().filter(|s| s.starts_with('@')), leaf_name), - }; - if file != format!("{leaf_name}-{version}.tgz") { - return None; - } - Some(match scope { - Some(scope) => format!("{scope}/{leaf_name}"), - None => leaf_name, - }) -} - #[cfg(test)] mod tests { use super::super::testing::*; @@ -1264,6 +1288,95 @@ mod tests { } } + /// #1236: yarn 1 keys a `file:` directory / tarball or url dependency by + /// the DEPENDENCY name (`"lp2@file:./lpdir"`), so a copy of the wired + /// left-pad@1.3.0 under another name escapes the pin and installs + /// unpatched. Its package is read from the copy itself (the directory's + /// or tarball's `package.json`, the registry url's path) and it contests + /// the wiring in the same lock, hosted and vendored alike. Control: a + /// copy holding another package is not one. + #[tokio::test] + async fn issue_1236_classic_other_name_copy_contests_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 copies = [ + ( + "file: directory", + "\"lp2@file:./lpdir\":\n version \"1.3.0\"\n\n".to_string(), + ), + ( + "file: tarball", + classic_block( + "\"lp2@file:./forks/left-pad-1.3.0.tgz\"", + "1.3.0", + &format!("file:./forks/left-pad-1.3.0.tgz#{SHA1}"), + None, + ), + ), + ( + "registry tarball url", + classic_block( + "\"lp2@https://registry.npmjs.org/left-pad/-/left-pad-1.3.0.tgz\"", + "1.3.0", + &format!("https://registry.npmjs.org/left-pad/-/left-pad-1.3.0.tgz#{SHA1}"), + None, + ), + ), + ]; + let project = |blocks: &[String], fork: &str| { + let p = Project::new(); + p.write("yarn.lock", classic(blocks)); + p.write( + "lpdir/package.json", + format!(r#"{{"name":"{fork}","version":"1.3.0"}}"#), + ); + p.write("forks/left-pad-1.3.0.tgz", npm_tgz(fork, "1.3.0")); + p + }; + for (mode, wired) in &wirings { + assert_eq!( + run(&project(std::slice::from_ref(wired), "left-pad")) + .await + .refs + .len(), + 1, + "{mode}: control, the wiring is a ref" + ); + 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 `file:` copies hold another package: not a copy of left-pad. + for (case, copy) in &copies[..2] { + let out = run(&project(&[wired.clone(), copy.clone()], "other-pkg")).await; + assert_eq!(out.refs.len(), 1, "{mode} {case}: {:#?}", 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. diff --git a/docs/ecosystems.md b/docs/ecosystems.md index 3fb540c5d..769677896 100644 --- a/docs/ecosystems.md +++ b/docs/ecosystems.md @@ -207,7 +207,13 @@ The backticked slug in each row is the value `-e`/`--ecosystems` accepts (e.g. that copy. Hosted and vendored modes leave the entry untouched (`redirect_yarn_classic_directory_skipped` / `vendor_link_entry_skipped`, naming it) and that copy stays unpatched; `vex` never attests the package from that lock while the - copy is there. When every entry of the package is such a copy (git, `file:` directory, + copy is there. This holds whatever dependency name the project gives the copy: yarn 1 + locks `"lp2": "file:./lpdir"` as `lp2@file:./lpdir`, so which package it is comes from + the directory's `package.json` (and, for `vex`, a `file:` tarball's manifest or a + registry tarball URL's path; hosted and vendored scans name a URL copy with + `redirect_yarn_classic_non_registry_entry_skipped` / + `vendor_yarn_classic_non_registry_entry_skipped`). A git copy under another name + records no package name in the lock and is not detected. When every entry of the package is such a copy (git, `file:` directory, non-registry tarball or `link:`), vendoring refuses with `vendor_lock_entry_not_rewritable` naming them, since `yarn install` can't help. - **bun** — text `bun.lock` lockfileVersion 0, 1 or 2: 0 is the `--save-text-lockfile`