Skip to content

Fix npm linked-store alias copies left unpatched (#852) - #987

Merged
Mikola Lysenko (mikolalysenko) merged 4 commits into
mainfrom
agent/fix-npm-linked-store-alias
Oct 7, 2026
Merged

Mikola Lysenko (mikolalysenko) merged 4 commits into
mainfrom
agent/fix-npm-linked-store-alias

Conversation

@mikolalysenko

@mikolalysenko Mikola Lysenko (mikolalysenko) commented Oct 7, 2026 •

Copy link
Copy Markdown
Collaborator

LLM Description written by Claude Code:claude-opus-5-5

Fixes #852

Summary

With npm 9–11's install-strategy=linked, agent mode now patches, verifies and rolls back an npm alias copy that lives in an alias-named store entry. Before this change:

  • Alias plus a plain copy: apply patched only left-pad and exited 0. vex attested not_affected while require('lp') loaded unpatched code.
  • Alias only: apply reported "1 not found on disk" (exit 0), and vex omitted the purl as not installed.

Root cause

npm names the store entry for an alias install after the alias: node_modules/.store/lp@1.3.0-<hash>/node_modules/lp holds the real left-pad@1.3.0. Agent mode missed that copy in two places:

  • Resolver (NpmCrawler::find_by_purls → visit_resolver_dir): inside a store entry it only probed node_modules/<real name>. The Fix agent mode skipping npm-aliased copies (#356) #738 alias pass (alias_copies) ran on importer trees only.
  • Store variant fan-out (find_store_peer_variant_copies, used by apply, rollback and VEX through with_store_peer_variant_copies): it skipped every entry whose dir name advertised another name, then probed node_modules/<real name>.

Fix

  • visit_resolver_dir also runs alias_copies on npm linked-store entries (new is_npm_linked_store_entry, which handles .store/<entry> and .store/@scope/<entry>). It counts real dirs only, and the dir's package.json stays the authority on name@version.
  • On the NpmLinked layout, find_store_peer_variant_copies probes a same-version entry under another name at node_modules/<advertised name>. That dir must be a real directory, since a link is a dependency edge into another entry. Its package.json must name the target, and the name's components pass is_safe_npm_component.
  • No other layout changes. pnpm, Bun, Deno and vlt name store entries after the real package, and Yarn 4's .store entries don't decode as npm entries.
  • The npm/PyPI/gem wrappers need no change, because the logic lives in the Rust core.

Ported CI fix: this PR carries 87bbd60, a cherry-pick of #878's "Route Gradle digests through utils::digest". main's utils::digest::tests::production_digests_go_through_the_helpers guard is red: Gradle/JVM/Maven code hashes inline. Without the port, coverage fails for reasons unrelated to this PR. It becomes a no-op once #878 lands.

Test evidence

Issue Variant Test Before → after
#852 alias + plain copy (also scoped alias name, other-version alias ignored, link-only entry ignored) npm_crawler::tests::test_npm_linked_store_alias_entry_beside_a_plain_copy_is_a_copy FAILED → ok
#852 alias only (also unscoped alias of a scoped package) npm_crawler::tests::test_npm_linked_store_alias_only_install_is_resolved FAILED → ok
#852 vex every-copy rule, new npm linked-store alias entry (#852) layout e2e_vex::verify_mode_requires_every_store_copy_patched FAILED on main's crawler ("an unpatched store copy must keep the purl out of the VEX doc") → ok

The issue's repro with a real npm 10.9.4 / Node 22 install-strategy=linked install and the hand-staged left-pad@1.3.0 patch, run with a binary built from main and one built from this branch:

case main this PR
alias + plain apply "1 of 1 applied"; vex not_affected; lp unpatched apply 1 of 1; vex not_affected; left-pad and lp both patched
alias only apply "1 not found on disk"; vex exit 1, omitted as not installed; lp unpatched apply 1 of 1; vex not_affected; lp patched

rollback --offline --yes on the alias + plain case restores both copies.

Commands run locally:

  • cargo clippy --workspace --all-features -- -D warnings: clean.
  • cargo fmt --all -- --check: no diffs in files this PR touches. The local rustfmt reports existing diffs in files this PR doesn't touch.
  • cargo test -p socket-patch-core --all-features --lib: 5247 passed, 5 failed.
    • 4 of the failures can't pass as root, because the sandbox runs as root and chmod can't make a dir read-only: copy_tree::relax_loop_must_not_traverse_symlinked_root, vlt_heal::an_unremovable_hidden_lock_keeps_every_store_entry, pypi_poetry::wire_write_failure_maps_error_and_leaves_lock_untouched, pypi_requirements::wire_failure_rolls_back_already_written_files.
    • The 5th, the main digest guard, passes after the port.
  • cargo test -p socket-patch-core --all-features --test crawler_npm_e2e --test covgap_crawlers_npm_crawler: 93 passed.
  • cargo test -p socket-patch-cli --all-features --test e2e_vex: 19 passed.
  • A full cargo test --workspace --all-features couldn't finish here: it hit the sandbox's disk allowance while linking test binaries, with no test failures. CI runs the full matrix.

🤖 Generated with Claude Code

https://claude.ai/code/session_01RL7etZZRZ4tKZVNsPFD3KL


Generated by Claude Code

Assisted-by: Claude Code:claude-opus-5-5
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
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
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)
@mikolalysenko
Mikola Lysenko (mikolalysenko) marked this pull request as ready for review October 7, 2026 06:51
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 87bbd60. Configure here.

@mikolalysenko

Mikola Lysenko (mikolalysenko) commented Oct 7, 2026 •

Copy link
Copy Markdown
Collaborator Author

[agent] Resolved: both one-time re-runs passed on 87bbd60, and CI is now fully green.

Original note: two CI jobs failed on 87bbd60 for reasons I don't think this PR caused.

  1. e2e (ubuntu-latest, e2e_vendor_maven_build, maven, 4.0.0-rc-6): the test's fixture warm-up got a 404 from Maven Central for org.apache.maven.plugins:maven-dependency-plugin:3.6.1 on all 3 attempts, before any socket-patch code ran. The re-run passed.
  2. native (macos-latest, 1.3.10) (Bun patch compatibility): 52 of 53 cells passed. The preexisting-manifest vendored cell failed its refusalCodesExact check. It passed on every other runner with the same binary, and passed on the re-run.

Generated by Claude Code

@mikolalysenko Mikola Lysenko (mikolalysenko) added the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label Oct 7, 2026
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

[agent] Ready for review at 87bbd60.


Generated by Claude Code

@mikolalysenko
Mikola Lysenko (mikolalysenko) merged commit 73c2383 into main Oct 7, 2026
719 of 721 checks passed
@mikolalysenko
Mikola Lysenko (mikolalysenko) deleted the agent/fix-npm-linked-store-alias branch October 7, 2026 12:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review

Projects

None yet

3 participants