Skip to content

Commit 3d32b16

Browse files
Fix yarn classic copies locked under another name (#1236) (#1242)
* Start fix for #1236 Assisted-by: Claude Code:claude-opus-5-5 * Stop yarn classic VEX attesting a renamed copy yarn 1 locks a file: or url dependency under the name the project gave it, so "lp2": "file:./lpdir" locks as lp2@file:./lpdir even when lpdir is left-pad@1.3.0. VEX read the package name from that lock key only, so it never saw that copy and attested left-pad not_affected while node_modules/lp2 installed the unpatched bytes. Classic discovery now reads such a copy for the package it really holds (the directory's package.json, the tarball's manifest, or the registry url's path), the way yarn berry has since #939, and that copy contests the hosted or vendored pin in the same lock. Berry and classic share one reader. Refs #1236 Assisted-by: Claude Code:claude-opus-5-5 * Name renamed yarn classic copies in scans A hosted or vendored yarn classic scan only matched lock blocks by the name in their key, so a copy of the patched package declared under another dependency name ("lp2": "file:./lpdir", or a registry tarball URL) was never mentioned: the scan exited 0 with no warning while that copy kept installing unpatched. Both scans now read such a copy for the package it really holds (the directory's package.json, or the registry URL's path) and name it with the same warnings a same-name copy gets (redirect_yarn_classic_directory_skipped / redirect_yarn_classic_non_registry_entry_skipped hosted, vendor_link_entry_skipped / vendor_yarn_classic_non_registry_entry_ skipped vendored). The hosted run also keeps that patch out of the in-run VEX's assumed-applied set. The registry copy is still pinned. Refs #1236 Assisted-by: Claude Code:claude-opus-5-5 * Test and document renamed yarn classic copies Adds a real-yarn-1 end-to-end test: a project that declares "lp2": "file:./lpdir" (lpdir being left-pad@1.3.0) beside the registry left-pad. The hosted scan still pins the registry copy, names lp2, and neither the in-run VEX nor a lock-only vex attests left-pad. docs/ecosystems.md now says the file: directory rule holds whatever dependency name the copy carries, and that a git copy under another name records no package name in the lock and is not detected. Refs #1236 Assisted-by: Claude Code:claude-opus-5-5 * Strip package.json BOMs through the shared helper The package.json name reader that moved into formats/yarn/source.rs stripped a UTF-8 BOM inline, which the #905 architecture test forbids outside formats::text. It now calls strip_bom_bytes. Refs #1236 Assisted-by: Claude Code:claude-opus-5-5 * Refuse vendoring a package only locked renamed When every yarn.lock copy of a package was locked under another dependency name ("lp2@file:./lpdir" being left-pad@1.3.0), vendoring refused with vendor_lock_entry_not_found and told you to run `yarn install`, which can't help: the package is installed, just not under its own name. It now refuses with vendor_lock_entry_not_rewritable naming those copies, in the vendor loop and in its download plan alike, the same way a lock holding only git or file: directory copies is refused. Refs #1236 Assisted-by: Claude Code:claude-opus-5-5 * Keep yarn install advice for a stale own-name block The previous change turned "not found" into "not rewritable" whenever a renamed copy of the package was locked. If the package also has a stale block under its own name (no `resolved`), `yarn install` does re-lock it, so the refusal now keeps the "not found" code and its `yarn install` remedy in that case. That applies in the vendor loop and in its download plan. Refs #1236 Assisted-by: Claude Code:claude-opus-5-5 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 83969ec commit 3d32b16

7 files changed

Lines changed: 815 additions & 108 deletions

File tree

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

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1247,6 +1247,134 @@ async fn classic_file_directory_dependency_is_named_and_not_attested() {
12471247
);
12481248
}
12491249

1250+
/// #1236: the same `file:` directory declared under ANOTHER dependency
1251+
/// name (`"lp2": "file:./lpdir"`, lpdir being left-pad@1.3.0) locks as
1252+
/// `"lp2@file:./lpdir"`, beside the registry left-pad. yarn 1 copies it
1253+
/// into `node_modules/lp2`, so it stays unpatched whatever the pin does.
1254+
/// `scan --mode hosted` must still pin the registry block, name the copy
1255+
/// (`redirect_yarn_classic_directory_skipped`), and neither its in-run VEX
1256+
/// nor a lock-only `vex` may attest left-pad not_affected.
1257+
#[tokio::test(flavor = "multi_thread")]
1258+
#[serial_test::serial]
1259+
async fn classic_file_directory_copy_under_another_name_is_named_and_not_attested() {
1260+
if !require_yarn_classic("e2e_redirect_yarn_classic_build (other-name file:)", |c| {
1261+
cache_env::isolate(c);
1262+
}) {
1263+
return;
1264+
}
1265+
let tmp = tempfile::tempdir().unwrap();
1266+
let proj = tmp.path().join("proj");
1267+
let copy = proj.join("lpdir");
1268+
std::fs::create_dir_all(&copy).unwrap();
1269+
std::fs::write(
1270+
copy.join("package.json"),
1271+
format!(r#"{{"name":"{DEP}","version":"{DEP_VERSION}","main":"index.js"}}"#),
1272+
)
1273+
.unwrap();
1274+
let orig: &[u8] = b"module.exports = function leftPad(s) { return s; };\n";
1275+
std::fs::write(copy.join("index.js"), orig).unwrap();
1276+
std::fs::write(
1277+
proj.join("package.json"),
1278+
format!(
1279+
r#"{{"name":"other-name-classic","version":"0.0.0","private":true,"dependencies":{{"{DEP}":"{DEP_VERSION}","lp2":"file:./lpdir"}}}}"#
1280+
),
1281+
)
1282+
.unwrap();
1283+
let cache = tmp.path().join("yarn-cache");
1284+
let install = corepack(
1285+
&proj,
1286+
&yarn_classic(),
1287+
&["install", "--no-progress"],
1288+
&[("YARN_CACHE_FOLDER", cache.to_str().unwrap())],
1289+
);
1290+
if !install.status.success() {
1291+
skip!(
1292+
"(other-name file:): fixture `yarn install` failed:\n{}",
1293+
String::from_utf8_lossy(&install.stderr)
1294+
);
1295+
return;
1296+
}
1297+
let lock_pristine = std::fs::read_to_string(proj.join("yarn.lock")).unwrap();
1298+
assert!(
1299+
lock_pristine.contains("lp2@file:./lpdir"),
1300+
"fixture must lock the copy under its dependency name:\n{lock_pristine}"
1301+
);
1302+
1303+
let installed_dir = proj.join("node_modules").join(DEP);
1304+
let installed_orig = std::fs::read(installed_dir.join("index.js")).unwrap();
1305+
let patched: Vec<u8> = [MARKER.as_bytes(), &installed_orig].concat();
1306+
let tgz_path = tmp.path().join("patched.tgz");
1307+
build_patched_tgz(&installed_dir, &patched, &tgz_path);
1308+
let tgz = std::fs::read(&tgz_path).unwrap();
1309+
let server = mock_hosted_grant(&tgz, &installed_orig, &patched, "other-name fixture").await;
1310+
1311+
let api_url = server.uri();
1312+
let api = [
1313+
"--api-url",
1314+
api_url.as_str(),
1315+
"--org",
1316+
ORG,
1317+
"--api-token",
1318+
"fake",
1319+
];
1320+
let mut args = vec![
1321+
"scan",
1322+
"--mode",
1323+
"hosted",
1324+
"--json",
1325+
"--yes",
1326+
"--cwd",
1327+
proj.to_str().unwrap(),
1328+
"--vex",
1329+
"out.vex.json",
1330+
"--vex-product",
1331+
PRODUCT,
1332+
];
1333+
args.extend(api);
1334+
let (code, stdout, stderr) = run_socket(&proj, &args);
1335+
println!("scan exit {code}\nstdout:\n{stdout}\nstderr:\n{stderr}");
1336+
let env: serde_json::Value = serde_json::from_str(&stdout)
1337+
.unwrap_or_else(|e| panic!("scan --json output is not JSON: {e}\n{stdout}\n{stderr}"));
1338+
let lock = std::fs::read_to_string(proj.join("yarn.lock")).unwrap();
1339+
assert!(
1340+
lock.contains(&format!("{UUID}/{DEP}-{DEP_VERSION}.tgz")),
1341+
"the registry left-pad block must still be pinned:\n{lock}"
1342+
);
1343+
assert!(
1344+
env.to_string()
1345+
.contains("redirect_yarn_classic_directory_skipped"),
1346+
"the other-name copy must be named: {env}"
1347+
);
1348+
assert!(env.to_string().contains("lp2@file:./lpdir"), "{env}");
1349+
let vex = std::fs::read_to_string(proj.join("out.vex.json")).unwrap_or_default();
1350+
assert!(
1351+
!vex.contains("not_affected"),
1352+
"the in-run VEX must not attest left-pad:\n{vex}\n{env}"
1353+
);
1354+
1355+
// Lock-only: with node_modules gone, `vex` reads only the lock.
1356+
std::fs::remove_dir_all(proj.join("node_modules")).unwrap();
1357+
let mut args = vec![
1358+
"vex",
1359+
"--cwd",
1360+
proj.to_str().unwrap(),
1361+
"--output",
1362+
"lock-only.vex.json",
1363+
"--product",
1364+
PRODUCT,
1365+
"--patch-server-url",
1366+
api_url.as_str(),
1367+
];
1368+
args.extend(api);
1369+
let (code, stdout, stderr) = run_socket(&proj, &args);
1370+
println!("vex exit {code}\nstdout:\n{stdout}\nstderr:\n{stderr}");
1371+
let vex = std::fs::read_to_string(proj.join("lock-only.vex.json")).unwrap_or_default();
1372+
assert!(
1373+
!vex.contains("not_affected"),
1374+
"lock-only vex must not attest left-pad:\n{vex}\n{stdout}\n{stderr}"
1375+
);
1376+
}
1377+
12501378
/// A mock patch API granting one hosted patch of `DEP@DEP_VERSION` whose
12511379
/// tarball is `tgz` (`index.js` from `orig` to `patched`).
12521380
async fn mock_hosted_grant(tgz: &[u8], orig: &[u8], patched: &[u8], title: &str) -> MockServer {

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

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,95 @@ pub(crate) fn classic_copy_source(patterns: &[String], resolved: Option<&str>) -
7474
}
7575
}
7676

77+
/// The root-relative `file:` directory a classic block's key names, if
78+
/// any: yarn 1 COPIES it into node_modules under the DEPENDENCY name
79+
/// (`"lp2@file:./lpdir"`), so which package that copy is comes from the
80+
/// directory's own `package.json` (#1236). `None` for a `file:` tarball or
81+
/// a path that leaves the root.
82+
pub(crate) fn classic_file_directory(patterns: &[String]) -> Option<String> {
83+
patterns.iter().find_map(|p| {
84+
let path = split_pattern(p)?.1.strip_prefix("file:")?;
85+
let path = path.split('#').next().unwrap_or_default();
86+
if is_tarball_path(path) {
87+
return None;
88+
}
89+
crate::utils::cargo_workspace::normalize_rel("", path)
90+
})
91+
}
92+
93+
/// The package a classic `file:` directory or url copy really installs,
94+
/// whatever dependency name its key carries (#1236): the directory's
95+
/// `package.json` `name` (read through `read_text`, given the
96+
/// root-relative manifest path), or the registry package a url tarball's
97+
/// path names. `None` when neither says (a `file:` tarball, a
98+
/// non-registry url, an unreadable manifest).
99+
pub(crate) fn classic_copy_real_name(
100+
patterns: &[String],
101+
resolved: Option<&str>,
102+
version: &str,
103+
read_text: impl Fn(&str) -> Option<String>,
104+
) -> Option<(String, CopySource)> {
105+
match classic_copy_source(patterns, resolved) {
106+
CopySource::Directory => {
107+
let dir = classic_file_directory(patterns)?;
108+
let manifest = if dir.is_empty() {
109+
"package.json".to_string()
110+
} else {
111+
format!("{dir}/package.json")
112+
};
113+
let name = manifest_name(read_text(&manifest)?.as_bytes())?;
114+
Some((name, CopySource::Directory))
115+
}
116+
CopySource::RemoteTarball => {
117+
let url = resolved?.split('#').next().unwrap_or_default();
118+
if !url.starts_with("http") {
119+
return None;
120+
}
121+
Some((
122+
registry_tarball_name(url, version)?,
123+
CopySource::RemoteTarball,
124+
))
125+
}
126+
_ => None,
127+
}
128+
}
129+
130+
/// `name` of a `package.json`.
131+
pub(crate) fn manifest_name(bytes: &[u8]) -> Option<String> {
132+
let bytes = crate::formats::text::strip_bom_bytes(bytes);
133+
serde_json::from_slice::<serde_json::Value>(bytes)
134+
.ok()?
135+
.get("name")?
136+
.as_str()
137+
.map(str::to_string)
138+
}
139+
140+
/// The package an npm registry tarball url serves, from its
141+
/// `/<name>/-/<leaf>-<version>.tgz` path (`<name>` may be `@scope/leaf`,
142+
/// its `@` / `/` possibly percent-encoded); `None` for any other shape.
143+
pub(crate) fn registry_tarball_name(url: &str, version: &str) -> Option<String> {
144+
let path = url.split_once("://").map_or(url, |(_, rest)| rest);
145+
let path = path.split(['?']).next()?;
146+
let (before, file) = path.rsplit_once("/-/")?;
147+
let mut segs: Vec<String> = before
148+
.split('/')
149+
.skip(1) // the host
150+
.map(|seg| crate::utils::purl::percent_decode_purl_component(seg).into_owned())
151+
.collect();
152+
let leaf_name = segs.pop()?;
153+
let (scope, leaf_name) = match leaf_name.split_once('/') {
154+
Some((scope, leaf)) => (Some(scope.to_string()), leaf.to_string()),
155+
None => (segs.pop().filter(|s| s.starts_with('@')), leaf_name),
156+
};
157+
if file != format!("{leaf_name}-{version}.tgz") {
158+
return None;
159+
}
160+
Some(match scope {
161+
Some(scope) => format!("{scope}/{leaf_name}"),
162+
None => leaf_name,
163+
})
164+
}
165+
77166
/// A GitHub codeload tarball: what yarn 1 locks a hosted-git shorthand to.
78167
fn is_codeload_tarball(resolved: &str) -> bool {
79168
resolved

‎crates/socket-patch-core/src/hosted/engine.rs‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -590,6 +590,35 @@ pub async fn read_candidate_files(
590590
out.read(view, unreadable, crate::patch::redirect::YARNRC_REL)
591591
.await;
592592
out.read(view, unreadable, NPMRC_REL).await;
593+
// A `file:` directory copy is locked under the DEPENDENCY name, so
594+
// the classic rewriter reads which package it is from the
595+
// directory's `package.json` (#1236). Advisory: never rewritten,
596+
// and an unreadable one only leaves that copy unnamed.
597+
let dirs: BTreeSet<String> = out
598+
.files
599+
.get("yarn.lock")
600+
.map(|lock| {
601+
crate::vendor::lock_inventory::yarn::classic_entries(lock)
602+
.iter()
603+
.filter_map(|e| {
604+
crate::formats::yarn::source::classic_file_directory(&e.patterns)
605+
})
606+
.collect()
607+
})
608+
.unwrap_or_default();
609+
for dir in dirs {
610+
let rel = if dir.is_empty() {
611+
"package.json".to_string()
612+
} else {
613+
format!("{dir}/package.json")
614+
};
615+
if out.files.contains_key(&rel) {
616+
continue;
617+
}
618+
if let Some(text) = read_advisory(view, unreadable, &rel).await {
619+
out.files.insert(rel, text);
620+
}
621+
}
593622
}
594623

595624
// Cargo workspace members (and in-root path dependencies) declare

0 commit comments

Comments
 (0)