Skip to content

Commit 73c2383

Browse files
Fix npm linked-store alias copies left unpatched (#852) (#987)
* Start fix for #852 Assisted-by: Claude Code:claude-opus-5-5 * Patch npm linked-store alias copies With npm 9-11's install-strategy=linked, an alias install such as "lp": "npm:left-pad@1.3.0" lives in a store entry named after the alias (node_modules/.store/lp@1.3.0-<hash>/node_modules/lp). Agent mode only looked for node_modules/left-pad inside store entries, so: - beside a plain left-pad copy, apply patched only the plain copy and exited 0, and vex attested not_affected while require('lp') still loaded unpatched code; - with only the alias installed, apply reported the package "not found on disk" and vex refused with package_not_found. The resolver now searches npm linked-store entries for alias copies the same way it searches an importer tree. The store variant fan-out used by apply, rollback and vex also probes a same-version entry under another name at its own dir. In both cases the entry's package.json stays the authority on name and version. Fixes #852 Assisted-by: Claude Code:claude-opus-5-5 * Cover linked-store alias copies in vex Adds the npm linked-store alias layout from #852 to vex's every-copy regression: an unpatched alias-named store entry must keep the purl out of the VEX document, and all copies patched must attest it. Refs #852 Assisted-by: Claude Code:claude-opus-5-5 * Route Gradle digests through utils::digest main has failed socket-patch-core's lib tests since Gradle support (#646) and the digest helpers (#865) both landed. The guard test production_digests_go_through_the_helpers flags three files #646 added that still hash inline: crawlers/gradle_cache.rs, patch/jvm_jar.rs and patch/sidecars/maven.rs. That breaks test, test-release and coverage on every open PR. Each inline sha1/sha256 call now goes through sha1_hex_of or sha256_hex_of, which compute the same lowercase hex. Behaviour is unchanged. Assisted-by: Claude Code:claude-opus-5-5 (cherry picked from commit 659ac2c) --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 7642053 commit 73c2383

2 files changed

Lines changed: 141 additions & 8 deletions

File tree

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -993,7 +993,8 @@ fn verify_mode_requires_every_installed_copy_patched() {
993993

994994
/// Regressions #603 and #601: the every-copy rule covers store copies
995995
/// too. `apply` patches a package's other store copies (a Deno `_1` copy
996-
/// index, a pnpm peer variant) and a copy bundled inside another
996+
/// index, a pnpm peer variant, an npm linked-store entry named after an
997+
/// alias, #852) and a copy bundled inside another
997998
/// package's store entry, so `vex` must hash each of them: one pristine
998999
/// copy omits the purl, and all patched attests it. Each layout has the
9991000
/// primary copy linked from the importer, as `deno install`, pnpm and vlt
@@ -1029,6 +1030,11 @@ fn verify_mode_requires_every_store_copy_patched() {
10291030
".pnpm/store-pkg@1.0.0/node_modules/store-pkg",
10301031
".pnpm/bundler@1.0.0/node_modules/bundler/node_modules/store-pkg",
10311032
),
1033+
(
1034+
"npm linked-store alias entry (#852)",
1035+
".store/store-pkg@1.0.0-iv4j8hdajpqgDc7lVr5hdA/node_modules/store-pkg",
1036+
".store/sp@1.0.0-NCKE2NXgCY5tgWWRE6qdYA/node_modules/sp",
1037+
),
10321038
];
10331039

10341040
let run = |primary: &str, other: &str, other_bytes: &[u8]| {

‎crates/socket-patch-core/src/crawlers/npm_crawler.rs‎

Lines changed: 134 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1689,7 +1689,10 @@ impl NpmCrawler {
16891689
.then_some((index, pkg_path))
16901690
})
16911691
.collect();
1692-
if !store_entry {
1692+
// npm's linked store keeps an alias install in an entry named after
1693+
// the alias, its real dir `node_modules/<alias>` (#852), so those
1694+
// entries are searched for alias copies like an importer tree.
1695+
if !store_entry || is_npm_linked_store_entry(&nm_path) {
16931696
matched.extend(Self::alias_copies(&nm_path, &listing, pending));
16941697
}
16951698
let gvs_member = !store_entry && may_be_gvs_workspace_member(&listing);
@@ -3078,22 +3081,37 @@ async fn find_store_peer_variant_copies_reusing(
30783081
} in entries
30793082
{
30803083
// Fast advertisement filter; undecodable pnpm names stay
3081-
// probeable, undecodable vlt ids are never variants.
3082-
match (advertised, layout) {
3084+
// probeable, undecodable vlt ids are never variants. npm's
3085+
// linked store names an alias install's entry after the ALIAS
3086+
// (`.store/lp@1.3.0-<hash>/node_modules/lp` holds the real
3087+
// `left-pad@1.3.0`, #852), so there a same-version entry under
3088+
// another name is probed at its own dir.
3089+
let dir_key = match (advertised, layout) {
3090+
(Some((n, v)), StoreLayout::NpmLinked)
3091+
if n != full_name
3092+
&& v == version
3093+
&& n.split('/').all(is_safe_npm_component) =>
3094+
{
3095+
n
3096+
}
30833097
(Some((n, v)), _) if n != full_name || v != version => continue,
30843098
(None, StoreLayout::Vlt) => continue,
3085-
_ => {}
3086-
}
3087-
// `full_name` may be scoped (`@s/n`) — Path::join handles the
3099+
_ => full_name.clone(),
3100+
};
3101+
let alias_entry = dir_key != full_name;
3102+
// `dir_key` may be scoped (`@s/n`) — Path::join handles the
30883103
// two-segment relative form.
3089-
let mut candidate = entry_nm.join(&full_name);
3104+
let mut candidate = entry_nm.join(&dir_key);
30903105
// Real dirs only: a link here is another entry's physical
30913106
// copy, reached via that entry, unless it is the entry's own
30923107
// `package` dir (Yarn 4), the physical copy itself.
30933108
let Ok(meta) = tokio::fs::symlink_metadata(&candidate).await else {
30943109
continue;
30953110
};
30963111
if !meta.is_dir() {
3112+
if alias_entry {
3113+
continue;
3114+
}
30973115
let (nm, key) = (entry_nm.clone(), full_name.clone());
30983116
match run_walk(move || store_entry_own_package_sync(&nm, &key)).await {
30993117
Some(own) => candidate = own,
@@ -3204,6 +3222,30 @@ fn store_entry_package_dir_sync(entry_nm: &Path) -> Option<(PathBuf, PathBuf)> {
32043222
Some((own, canonical))
32053223
}
32063224

3225+
/// Whether `entry_nm` is the `node_modules` of an entry in npm's
3226+
/// `install-strategy=linked` store: `node_modules/.store/<entry>/node_modules`,
3227+
/// or `node_modules/.store/@scope/<entry>/node_modules` for a scoped package.
3228+
fn is_npm_linked_store_entry(entry_nm: &Path) -> bool {
3229+
let is_named =
3230+
|dir: Option<&Path>, name: &str| dir.and_then(Path::file_name) == Some(OsStr::new(name));
3231+
let Some(mut parent) = entry_nm.parent().and_then(Path::parent) else {
3232+
return false;
3233+
};
3234+
if parent
3235+
.file_name()
3236+
.and_then(OsStr::to_str)
3237+
.is_some_and(|name| name.starts_with('@'))
3238+
{
3239+
match parent.parent() {
3240+
Some(store) => parent = store,
3241+
None => return false,
3242+
}
3243+
}
3244+
is_named(Some(entry_nm), "node_modules")
3245+
&& is_named(Some(parent), NPM_LINKED_STORE_NAME)
3246+
&& is_named(parent.parent(), "node_modules")
3247+
}
3248+
32073249
/// Whether `pkg_path` is the physical dir one of `copies` resolves to.
32083250
fn resolves_to_any_sync(pkg_path: &Path, copies: &[CrawledPackage]) -> bool {
32093251
let Ok(canon) = std::fs::canonicalize(pkg_path) else {
@@ -4955,6 +4997,91 @@ mod tests {
49554997
);
49564998
}
49574999

5000+
/// #852: under `install-strategy=linked`, npm 9–11 store an alias
5001+
/// install (`"lp": "npm:left-pad@1.3.0"`) in an entry named after the
5002+
/// ALIAS: `.store/lp@1.3.0-<hash>/node_modules/lp` holds the real
5003+
/// `left-pad@1.3.0`, and the importer's `node_modules/lp` links to it.
5004+
/// Beside a plain copy, the plain copy is the resolver's primary and
5005+
/// the alias entry is the peer-variant fan-out's to find, or apply
5006+
/// leaves `require('lp')` unpatched while VEX attests `not_affected`.
5007+
#[tokio::test]
5008+
async fn test_npm_linked_store_alias_entry_beside_a_plain_copy_is_a_copy() {
5009+
let tmp = tempfile::tempdir().unwrap();
5010+
let root: PathBuf = tmp.path().components().collect();
5011+
let nm = root.join("node_modules");
5012+
let store = nm.join(".store");
5013+
5014+
let plain = store.join("left-pad@1.3.0-iv4j8hdajpqgDc7lVr5hdA/node_modules/left-pad");
5015+
write_pkg(&plain, "left-pad", "1.3.0");
5016+
let alias = store.join("lp@1.3.0-NCKE2NXgCY5tgWWRE6qdYA/node_modules/lp");
5017+
write_pkg(&alias, "left-pad", "1.3.0");
5018+
// A scoped alias name, and an unscoped alias of a scoped package.
5019+
let scoped_alias = store.join("@x/pad@1.3.0-AAAAAAAAAAAAAAAAAAAAAA/node_modules/@x/pad");
5020+
write_pkg(&scoped_alias, "left-pad", "1.3.0");
5021+
// An alias of another version, and an alias entry whose own dir is
5022+
// a link (a dependency edge), are not copies.
5023+
let other = store.join("lp2@1.2.0-BBBBBBBBBBBBBBBBBBBBBB/node_modules/lp2");
5024+
write_pkg(&other, "left-pad", "1.2.0");
5025+
let edge_entry = store.join("lp3@1.3.0-CCCCCCCCCCCCCCCCCCCCCC/node_modules");
5026+
std::fs::create_dir_all(&edge_entry).unwrap();
5027+
link_dir(&plain, &edge_entry.join("lp3"));
5028+
link_dir(&plain, &nm.join("left-pad"));
5029+
link_dir(&alias, &nm.join("lp"));
5030+
5031+
let purl = "pkg:npm/left-pad@1.3.0".to_string();
5032+
let found = NpmCrawler::new()
5033+
.find_by_purls(&nm, std::slice::from_ref(&purl))
5034+
.await
5035+
.unwrap();
5036+
assert_eq!(copy_paths(&found, &purl), vec![nm.join("left-pad")]);
5037+
5038+
let mut variants = find_store_peer_variant_copies(&nm.join("left-pad")).await;
5039+
variants.sort();
5040+
let mut want = vec![alias.clone(), scoped_alias.clone()];
5041+
want.sort();
5042+
assert_eq!(variants, want);
5043+
5044+
// VEX's every-installed-copy set sees the alias entries too.
5045+
let mut all = with_store_peer_variant_copies(vec![nm.join("left-pad")]).await;
5046+
all.sort();
5047+
let mut want = vec![nm.join("left-pad"), alias.clone(), scoped_alias.clone()];
5048+
want.sort();
5049+
assert_eq!(all, want);
5050+
}
5051+
5052+
/// #852: with only the alias installed, the alias-named linked store
5053+
/// entry is the purl's only copy. Apply reported it "not found on
5054+
/// disk" (exit 0) and VEX refused with `package_not_found`.
5055+
#[tokio::test]
5056+
async fn test_npm_linked_store_alias_only_install_is_resolved() {
5057+
let tmp = tempfile::tempdir().unwrap();
5058+
let root: PathBuf = tmp.path().components().collect();
5059+
let nm = root.join("node_modules");
5060+
let store = nm.join(".store");
5061+
5062+
let alias = store.join("lp@1.3.0-NCKE2NXgCY5tgWWRE6qdYA/node_modules/lp");
5063+
write_pkg(&alias, "left-pad", "1.3.0");
5064+
let scoped = store.join("sp@2.0.0-DDDDDDDDDDDDDDDDDDDDDD/node_modules/sp");
5065+
write_pkg(&scoped, "@s/pkg", "2.0.0");
5066+
link_dir(&alias, &nm.join("lp"));
5067+
link_dir(&scoped, &nm.join("sp"));
5068+
5069+
let pad = "pkg:npm/left-pad@1.3.0".to_string();
5070+
let scoped_purl = "pkg:npm/%40s/pkg@2.0.0".to_string();
5071+
let found = NpmCrawler::new()
5072+
.find_by_purls(&nm, &[pad.clone(), scoped_purl.clone()])
5073+
.await
5074+
.unwrap();
5075+
assert_eq!(copy_paths(&found, &pad), vec![alias.clone()]);
5076+
let copy = &found[&pad][0];
5077+
assert_eq!(
5078+
(copy.name.as_str(), copy.version.as_str()),
5079+
("left-pad", "1.3.0")
5080+
);
5081+
assert_eq!(copy_paths(&found, &scoped_purl), vec![scoped.clone()]);
5082+
assert_eq!(found[&scoped_purl][0].namespace.as_deref(), Some("@s"));
5083+
}
5084+
49585085
/// #362: pnpm's `virtualStoreDir` moves the virtual store, and
49595086
/// `node_modules/.modules.yaml` records where (relative to
49605087
/// `node_modules`: JSON on pnpm 10+, YAML before). A relocated store

0 commit comments

Comments
 (0)