Fix silent hosted pnpm pin beside a file: copy

[Mikola Lysenko (mikolalysenko)]

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

Refs #935

This fixes the hosted half of #935. Vendored pnpm still writes its
override silently beside a file: copy. That gap is in the pnpm vendor
backend, which has no post-write discovery check to hook into, so it stays
open on #935 as a follow-up (see below).

What users saw

Take a pnpm workspace where one member depends on registry left-pad@1.3.0
and another on a file: directory or tarball copy of the same release.
scan --mode hosted pinned the registry entry and reported success
with no warning about the file: copy, which pnpm keeps installing
unpatched. The same run's --vex then attested the package not_affected.
A later standalone vex correctly refused (patched_ref_unattributable).
Bisected by the pnpm bug-hunt to f3c6313 (#1008).

Root cause

The hosted engine re-runs lockfile discovery over the rewrite before
writing (unattributed_pins in hosted/engine.rs). Before #1008, a pin
whose own lock also installs an unpatched copy of the same name@version
was dropped from discovery, so the engine skipped the patch as
redirect_unattributable. #1008 (for #828) moved such refs into
Discovery::shadowed so rollback and remove can still undo them, and
counted them as pins, so the engine let the pin through silently.

The pnpm rewriter can't see these copies itself, because a directory
copy's version lives in its own package.json, not in the lock. So nothing
recorded the uuid in bundled_skipped_uuids, and the in-run VEX assumed it
applied.

Fix

The registry entry is still pinned, which matches npm
(redirect_npm_non_registry_entry_skipped) and yarn classic
(redirect_yarn_classic_directory_skipped). The registry copy is what most
of the app loads, so skipping the whole patch would leave it vulnerable
too. For each shadowed hosted pin whose own pnpm lock carries a file:
copy (from discovery's unpatched_copies), the engine now:

• adds a redirect_copy_unpatched warning naming the lock and the entry,
  once per (lock, entry);
• records the uuid in bundled_skipped_uuids, so the in-run --vex does
  not attest it (the same mechanism the yarn classic rewriter uses).

CLI_CONTRACT.md documents the new warning code and updates the
attribution-gate paragraph.

Review notes

/code-review high ran twice.

• The first pass rejected the original approach of vetoing the whole patch,
  because it contradicts the npm and yarn classic policy of patching the
  registry copy and warning about the other one. It also changed npm
  behaviour without a test. Reworked in d14593d.
• The second pass found that the in-run VEX still attested the package,
  that dedup by matching warning text was fragile, and that the warning
  wasn't scoped to pnpm. All three are fixed in this push: the uuid joins
  bundled_skipped_uuids, the warning is limited to pnpm locks and keyed
  by (file, entry), and there's a new --vex test.
• Not changed: a reviewer suggested moving detection into the pnpm
  rewriter. The rewriter only sees lock text, and a directory copy's
  version needs its package.json. Discovery already reads that
  (record_pnpm_file_copies), so the engine reuses it rather than
  duplicating the read.

Tests

New in crates/socket-patch-cli/tests/in_process_redirect_pnpm.rs:

Case	Test	Without fix	With fix
#935 directory copy, scan warns	hosted_pnpm_file_directory_copy_is_named	fails (no warning)	passes
#935 tarball copy, scan warns	hosted_pnpm_file_tarball_copy_is_named	fails (no warning)	passes
#935 in-run --vex doesn't attest	hosted_pnpm_file_copy_is_not_attested_by_the_in_run_vex	fails (not_affected statement)	passes

• in_process_redirect_pnpm: 35/35 pass.
• cargo fmt --check and cargo clippy --workspace --all-features -D warnings: clean.
• cargo test --workspace --all-features: every target passes except 13 tests that fail because of this container (not checked against main here). 12 are permission-based write-failure tests, which can't fail a write when the container runs as root. One is an e2e_bun_lockb test whose rollback fails reaching patches-api.socket.dev (tunnel error). The same 13 failed before and after each change in this PR, and none of them is in the hosted pnpm or engine-gate path. CI is the check for them.

Follow-up (stays on #935)

Vendored pnpm: scan --mode vendored still writes
overrides: {left-pad@1.3.0: file:.socket/vendor/…} beside a file: copy
without a warning. npm and Bun vendored emit
vendor_non_registry_entry_skipped here. The pnpm backend
(vendor/pnpm_lock.rs preflight) needs the same, reading the copy's
version the way record_pnpm_file_copies does.

🤖 Generated with Claude Code

https://claude.ai/code/session_01H7rWeixkY6z9hT2QNYmWLC

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[cursor]

✅ 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 33b60e7. Configure here.}

[Mikola Lysenko (mikolalysenko)]

Labeled Ready for review by the burn-down agent.

• Head: 33b60e7f00 (mergeable against main, no conflicts; CHANGELOG.md untouched)
• CI: 36/36 green + 50 path-skipped; no main-wide failures
• Bugbot: reviewed 33b60e7, no new issues; no open review threads
• Reviewer focus: the hosted pnpm path in hosted/engine.rs that now refuses/warns instead of silently pinning beside a same-lock file: copy, and in_process_redirect_pnpm.rs. The vendored half of Warn during pnpm scans when non-registry copies cannot be patched #935 is a follow-up.
• Slack announcement not sent: the Slack send tool isn't available to this run.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Final review brief

What it does. In hosted scan/get, the engine now detects when a pin it just confirmed is shadowed by a pnpm file: directory or tarball copy of the same name@version in the same pnpm-lock.yaml/shrinkwrap.yaml. For each such pin it emits a redirect_copy_unpatched warning and adds the uuid to bundled_skipped_uuids, so the in-run --vex no longer attests the package not_affected. The registry pin is still written, as for npm and yarn classic.

Risk: low. It only adds a warning and makes the in-run VEX more conservative. Which pins are written, the veto logic and the exit codes don't change. The extra work is a filter over a discovery the engine already runs, with no new I/O.

Look here

• engine.rs:1892-1927: the detection loop. It uses the same purl + file predicate as Discovery::contest_within_locks and is scoped to pnpm lock filenames.
• engine.rs:1734-1735: the results are merged only on the final, non-vetoed pass.
• scan/hosted.rs:1757-1762:`` the existing consumer that removes skipped uuids from assume_applied.
• in_process_redirect_pnpm.rs:2650-2825:``` the 3 new tests.

Verified

• Read the full diff and the surrounding engine and discovery code.
• cargo test -p socket-patch-cli --test in_process_redirect_pnpm: 35/35 pass. cargo test -p socket-patch-core --lib hosted::engine: 60/60 pass.
• With the merge lines at engine.rs:1734-1735 reverted, all 3 new tests fail, so they exercise the fix.
• CI is 36/36 green on this head, including ci-ok and clippy. Bugbot found no issues. CHANGELOG.md is untouched and there are no open threads.

Changes I made: none.

Open questions (non-blocking nits)

• hosted_pnpm_file_copy_is_not_attested_by_the_in_run_vex turns a missing VEX file into an empty list. It does fail without the fix today, but asserting that the file exists would harden it.
• The VEX skip works per uuid, so the same patch pinned in a second, clean lock also falls back to installed-tree verification. That is conservative, and the CLI_CONTRACT.md row could say so.

Auto-merge is armed: approving sends this straight to the merge queue.

---

Generated by Claude Code