Skip to content

Fix pnpm vendored refusal missing quoted scoped aliases (#957) - #986

Open
Mikola Lysenko (mikolalysenko) wants to merge 3 commits into
mainfrom
agent/fix-pnpm-quoted-alias-refusal
Open

Mikola Lysenko (mikolalysenko) wants to merge 3 commits into
mainfrom
agent/fix-pnpm-quoted-alias-refusal

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 #957

Summary

Vendored pnpm 9+ now refuses a package that a scoped npm: alias references (sl: npm:@scope/pkg@x), in a root importer or in a dependent's snapshot, exactly as it already refused an unscoped alias. Before this change, vendor / scan --mode vendored reported success, left a dangling quoted reference, broke every pnpm install --frozen-lockfile (ERR_PNPM_LOCKFILE_MISSING_DEPENDENCY) and let VEX attest not_affected.

Root cause

check_rewritable_refs in crates/socket-patch-core/src/vendor/pnpm_lock.rs refuses a package that an npm: alias references, because the pair surgery can't rewrite that reference. To do that it compares raw YAML values (importer version: and snapshot body dependency values) against the unquoted registry key name@version. pnpm 9+ quotes any value that starts with @, so a scoped alias ('@isaacs/string-locale-compare@1.1.0') never matches. This affects the indexed path (LockIndex::build stores raw values in first_snapshot_rest* / first_importer_ver*) and the non-indexed fallback loops alike.

Fix

  • Values are unquoted once, at all four comparison sites (indexed and scan, importer and snapshot) and where LockIndex::build keys first_snapshot_rest*, first_snapshot_dep_rest_paren, first_importer_ver* and first_importer_catalog. The scan and the index therefore still agree. A scoped alias, including a peer-suffixed one, is now refused like an unscoped alias. The refusal text shows the unquoted reference.
  • The index-vs-scan oracle (indexed_lock_probes_match_the_scans) now generates the quoted spelling pnpm writes for @-leading values, so any future drift between the two paths on that shape fails the test.
  • The legacy pnpm 7/8 backend already refuses this shape, as the issue's matrix shows, so it is unchanged. The npm, pypi and gem wrappers only dispatch the binary, so they need no parallel change.
  • The commit "Route Gradle digests through utils::digest" is ported from Route Gradle digests through utils::digest #878, which fixes the production_digests_go_through_the_helpers guard test that is red on main (coverage / test). It becomes a no-op once Route Gradle digests through utils::digest #878 lands.

Test evidence

Issue Test Without fix With fix
#957 importer shape (+ peer-suffixed, scan & indexed) vendor::pnpm_lock::tests::quoted_scoped_alias_references_refuse FAILED ("an aliased importer version must refuse (scan)") ok
#957 snapshot shape (bug-hunt follow-up comment) same unit test, snapshot cases FAILED ok
#957 importer, real pnpm 10 e2e_vendor_pnpm_build::pnpm_vendor_refuses_quoted_scoped_alias_references (importer leg) FAILED: status: success, applied: 1 ok: vendor_lock_entry_unsupported, lock/package.json byte-identical, no artifact, untouched lock frozen-installs
#957 snapshot via a file: tarball dep, real pnpm 10 same e2e (snapshot leg) FAILED: status: success, applied: 1 ok

Commands run locally on a406a10:

  • cargo clippy --workspace --all-features -- -D warnings: clean. rustfmt --check on every touched file: clean. CI runs no fmt check, and main itself is not cargo fmt-clean, so a whole-workspace cargo fmt is not part of this PR.
  • cargo test --workspace --all-features --no-fail-fast: 10,820 passed, 340 ignored, 12 failed. All 12 failures are failure-injection tests that make paths read-only or unremovable, and the sandbox runs as root, which bypasses those permissions. Re-run as an unprivileged user (setpriv --reuid=65534), all 12 pass.
  • cargo test -p socket-patch-core --lib vendor::pnpm: 211 passed, including the 600-seed index oracle.

🤖 Generated with Claude Code

https://claude.ai/code/session_018PuTamr8nmfojGXJ7zn9Xc


Generated by Claude Code

Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

[agent] coverage failed on the empty start commit 6ef1be2, which is plain main, so the failure isn't caused by this PR. The failing test is utils::digest::tests::production_digests_go_through_the_helpers: crawlers/gradle_cache.rs, patch/jvm_jar.rs and patch/sidecars/maven.rs still hash inline on main. The open fix is #878. Its commit is cherry-picked here as fa2202a, and it becomes a no-op once #878 lands. Locally, that test passes with the commit applied.


Generated by Claude Code

pnpm 9+ quotes a lock value that starts with `@`, so a scoped npm
alias (`sl: npm:@scope/pkg@1.1.0`) is written as
`'@scope/pkg@1.1.0'` in the importer or a dependent's snapshot. The
vendored "aliased reference" refusal compared that raw value with the
unquoted `name@version`, never matched, and vendoring reported success
over a lock that every frozen install rejects
(ERR_PNPM_LOCKFILE_MISSING_DEPENDENCY) while VEX attested the package.

Unquote importer versions and snapshot dependency values once, both in
the scan and where the lock index keys them, so scoped aliases (and
their peer-suffixed spellings) are refused like unscoped ones. The
index-vs-scan oracle now generates the quoted spelling too.

Fixes #957

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) force-pushed the agent/fix-pnpm-quoted-alias-refusal branch from fa2202a to a406a10 Compare October 7, 2026 06:11
@mikolalysenko
Mikola Lysenko (mikolalysenko) marked this pull request as ready for review October 7, 2026 06:31
@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 a406a10. Configure here.

@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

Ready for review — head a406a10e97109130ef97ba67caa236fa76ab502e.

  • CI: 464/464 check runs completed, 0 failed (skips are matrix legs not triggered by this diff). Merge state is only waiting on review.
  • Bugbot: reviewed a406a10, no findings.
  • Reviewer focus: the unquote step in vendor/pnpm_lock.rs LockIndex::build and the four comparison sites must stay in sync; the index-vs-scan oracle test now covers the quoted @-leading spelling. The Gradle-digest commit is ported from Route Gradle digests through utils::digest #878 to fix the guard test that's red on main, and becomes a no-op once Route Gradle digests through utils::digest #878 lands.

Generated by Claude Code

This branch has not been deployed

No deployments
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

2 participants