Fix hosted yarn classic rescan of v4 pins (#1462)

[Mikola Lysenko (mikolalysenko)]

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

Fixes #1462

Summary

Hosted yarn classic users upgrading from v4.0.0 (the current npm latest) can have a yarn.lock pin that v4 wrote for a patch whose package.json adds a dependency the lock never locked. yarn installs the patched package without that dependency. On main such a pin passes silently:

• Rescan: with a sha1 grant it reports applied and no warning. With a sha512-only grant it warns, but says the package "is not pinned", which is wrong because the earlier pin is still in the lock.
• VEX: vex and scan --vex attest not_affected after install.

Root cause

• select_classic_artifacts (hosted/engine.rs) skipped every artifact the lock already pins as "<url>#…" when the grant carries a sha1. The served package.json was never read, so the Yarn classic: a patch that adds a dependency to the package's own package.json leaves vendored yarn.lock with a dangling dependency (offline frozen install fails, lock churns), and hosted silently installs without it #591 dependency check in rewrite_yarn_classic_with (patch/redirect/mod.rs) never ran: it only runs when a manifest exists.
• VEX hash-verifies the installed patched files. Those files are patched, so nothing looked at whether the patched package's dependencies are installed.

Fix

1. The served tarball is read for pinned artifacts too. The cheap "pinned already" lookup still runs first. A fully pinned rescan still never reads the lock's blocks (Perf regression: yarn-classic/hosted wall +165% (5096c41a, #1328) #1423), and they are read at most once overall. The cost is that a rescan now fetches one tarball per pinned yarn classic artifact, one after another (the classic fetch loop in scan/hosted.rs is sequential; fetching concurrently would be a follow-up). That is the only way to catch a pin an earlier release wrote, because nothing in the lock records whether the check ran.
2. A stale pin is refused like a fresh one. It gets the redirect_yarn_classic_dep_manifest_unlocked warning, and the detail now says "an earlier run's pin of it is not confirmed … installs the patched package without them" when the lock already pins this artifact. Being refused means it is never confirmed (the refused_yarn_classic_uuids path), so it is listed as a redirect_unconfirmed skip and in-run scan --vex doesn't attest it. Following the contract for unconfirmed rows (Decide: one shape for the --json top-level error (scan and get emit both a string and a {code, message} object) #704), the exit code and status stay unchanged, and the lock is left as found.
3. VEX checks dependencies for patches that change package.json. This applies to hosted npm-family copies only. A non-optional dependencies entry that node can't resolve from the installed copy is omitted with a new dependency_not_installed tag. Resolution follows node's lookup: node_modules/<name> in the copy or in any non-node_modules ancestor, which covers bundled, hoisted, and pnpm/bun-sibling layouts. A copy outside a node_modules tree, an unreadable manifest, or a name that isn't a plain package name never counts as evidence. The check runs only when the patch record touches package.json, so other patches see no change.

CLI_CONTRACT.md documents the new omission tag and the extended warning. The npm/pypi/gem wrappers only dispatch to the binary, so they need no change.

Tests (red on main → green)

Issue item	Test	main	this PR
#1462 sha1 grant: rescan confirmed the v4 pin	scan hosted_yarn_classic_sha1::issue_1462_v4_pin_with_unlocked_dependency_is_not_confirmed_sha1_grant	FAIL (no warning)	ok
#1462 sha512-only grant: pin kept, wrong "not pinned" text	…_sha512_only_grant	FAIL (detail)	ok
#1462 vex attests after install	e2e_vex_lockfile yarn::hosted_patch_adding_an_uninstalled_dependency_is_omitted (classic and berry 2/3/4)	FAIL (attested)	ok
helper: node lookup layouts	core vex::verify::tests::unresolved_npm_dependency_follows_node_lookup	n/a	ok
#1423 read-count guard kept	core classic_artifact_targets_read_the_lock_blocks_at_most_once (now expects pinned artifacts selected, still 0 block reads)	n/a	ok

Commands: cargo test -p socket-patch-cli --all-features --test scan hosted_yarn_classic, cargo test -p socket-patch-cli --all-features --test e2e_vex_lockfile yarn -- --include-ignored, plus cargo fmt --all -- --check (clean), cargo clippy --workspace --all-features -- -D warnings (clean) and cargo test -p socket-patch-core --all-features --lib (6408 pass). Four unrelated core tests fail locally only because this sandbox runs as root, which ignores the read-only permissions they rely on (copy_tree symlinked root, vlt_heal unremovable lock, pypi_poetry / pypi_requirements write failure). The full CLI matrix runs in CI; it didn't fit on the sandbox disk.

/code-review high findings

Fixed in Resolve npm deps from the copy's real path:

• The dependency lookup now starts from the copy's real path, so pnpm, bun isolated and npm linked symlinks into a store find their siblings.
• The file probes run through run_blocking.
• It reuses npm_common::is_safe_npm_name.
• The omission wording no longer claims the lockfile is at fault.

Not changed, with reasons:

• A failed tarball fetch now drops an already-pinned dep. This is intended and fails closed. A rescan that cannot read the served manifest cannot tell a good v5 pin from a v4 one (Hosted yarn classic rescan and VEX accept a pin written by v4.0.0 whose patched package.json adds a dependency the lock doesn't have, so yarn installs the patched package without it #1462), so it no longer confirms the pin. The lock is left as it was.
• Every pinned artifact is fetched on every rescan (Perf regression: yarn-classic/hosted wall +165% (5096c41a, #1328) #1423 cost). Nothing in the lock records that the check ran (v5 keeps no hosted ledger), so a per-pin marker would need a new persisted file. The fetches run one after another, and the lock's blocks are still never read on a fully pinned rescan. Concurrent fetches or a ledger-based skip would be a follow-up. A ledger-based skip would be a follow-up.
• The check covers pre-existing deps, not only added ones. The record keeps only hashes, so the pristine manifest is not available offline. An installed copy missing a declared non-optional dependency is broken whatever the cause, so withholding the attestation is the safe answer.
• A hosted URL in a comment or a file: block selects a fetch. This needs a hand-edited lock. The cost is one fetch, and the rewriter still skips non-registry blocks.
• Earlier pin under a rotated URL token gets the "not pinned" wording. This affects the warning text only: the dep is refused and unconfirmed either way.
• Per-refused-dep block scan. This runs only on the refusal path, which is rare and needs a network fetch anyway.
• The missing dependency's name is not in the omission. The omission is a stable tag, and the name is easy to see in the patched package.json.

Not covered

Lock-only vex (nothing installed) still attests a stale v4 pin from its wiring. With no installed copy and no network fetch there is no local evidence of the served manifest. Whether hosted VEX should need installed evidence by default is the open #1099 decision.

🤖 Generated with Claude Code

https://claude.ai/code/session_01VDPgtA8XT3RDR96zCibCmo

---

Note

Medium Risk
Changes hosted scan confirmation and VEX attestation for npm hosted wiring; behavior is intentionally stricter (more warnings/omissions, extra tarball fetches on Yarn Classic rescan) but locks are not rewritten and exit codes stay the same for unconfirmed pins.

Overview
Fixes #1462: hosted Yarn Classic pins from older releases could attest or confirm patches whose served package.json adds dependencies yarn.lock never locked, while Yarn still installs the patched package without those deps.

Hosted rescan now fetches the served tarball for every Yarn Classic artifact the lock already pins (not only unpinned/registry-copy deps), so the existing manifest-vs-lock check runs on every run. Stale v4-style pins get redirect_yarn_classic_dep_manifest_unlocked with wording that the earlier pin is not confirmed; they stay unconfirmed (redirect_unconfirmed), the lock is untouched, and exit behavior is unchanged per #704.

VEX adds omission tag dependency_not_installed for hosted npm-family copies when the patch touches package.json and a non-optional declared dependency cannot be resolved via Node’s node_modules lookup (hoisted, bundled, pnpm/bun siblings via real path). CLI contract and user-facing omission text are updated; e2e and scan tests cover rescans (sha1 and sha512 grants) and VEX attest/omit behavior.

^{Reviewed by Cursor Bugbot for commit fdde961. Configure here.}

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[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 fdde961. Configure here.}

[Mikola Lysenko (mikolalysenko)]

Ready for review (burn-down agent)

• Head: fdde961b06784ba56fef4625555d5ab65cadd79a
• CI: 35/35 green, 19 path-skipped; no main-wide failures.
• Bugbot: reviewed this head, check passed. Its one earlier finding (unreadable manifest in vex/verify.rs) was fixed in fdde961 and the thread is resolved.
• Mergeable, CHANGELOG.md untouched.
• Reviewer: note the rescan now fetches one tarball per already-pinned yarn classic artifact (the cost of catching v4-era pins), and the new dependency_not_installed VEX omission tag in CLI_CONTRACT.md.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Final review brief

What it does: A hosted yarn classic rescan used to skip every artifact yarn.lock already pinned, so it never read the served package.json. A v4.0.0 pin for a patch that adds a dependency the lock never locked therefore passed as applied. The rescan now fetches the served tarball for pinned artifacts too, and a stale pin like that gets the redirect_yarn_classic_dep_manifest_unlocked warning, stays unconfirmed, and the lock is left as found. Separately, VEX now omits a hosted npm-family copy, with the new dependency_not_installed tag, when its patch touches package.json and node can't resolve one of its non-optional dependencies. Fixes #1462.

Risk: low to medium. Every change fails closed: rows are left unconfirmed or omitted, never wrongly attested. The medium part is operational. Each scan --mode hosted now downloads every pinned yarn classic tarball, and a failed fetch turns a pin that used to confirm into an npm_tarball_unavailable skip. The PR description says this is intended.

Look here:

• engine.rs:1294-1320: the new selection, pinned OR registry copy.
• scan/hosted.rs:1221-1241:`` the fetch loop, where a failure drops an already-pinned patch.
• redirect/mod.rs:4097-4125:``` how a stale pin is refused, with the "earlier run's pin" wording.
• vex/verify.rs:482-549: unresolved_npm_dependency / node_resolves. Lookup follows node's rules from the copy's real path, and names are checked with is_safe_npm_name before any join.
• CLI_CONTRACT.md:1445: the new omission tag.

Verified:

• Read the full diff.
• Ran cargo test -p socket-patch-core --all-features --lib -- vex::verify classic_artifact_targets (41 pass).
• The two issue_1462_* scan tests would fail on main.
• Path traversal (../../escape is tested), FIFO manifests, pnpm/bun symlinked store layouts and Windows \\?\ paths are all handled.
• No debug leftovers and no scope creep. CHANGELOG.md untouched.
• CI on fdde961: ci-ok and clippy green. Cursor Bugbot passed. No unresolved threads.

Changes I made: none.

Open questions (non-blocking):

1. The description says the pinned-tarball fetches run "concurrently, from Fix yarn classic hosted scan lock rescans (#1423) #1426". The classic fetch loop at scan/hosted.rs:1221 is actually sequential (for … await). A lock with many hosted classic pins pays N sequential downloads on every rescan. A follow-up could fetch them concurrently.
2. The CLI_CONTRACT row says the dependency is missing "because the lockfile never locked it". The check covers any hosted npm-family copy with any unresolvable non-optional dependency, so "for example" would be more accurate.

Auto-merge (squash) is armed, so approving sends it straight to the merge queue.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

[agent] Both open questions in the review brief are right.

1. The classic fetch loop is sequential, as the brief says. I've corrected the PR description; making it concurrent is left as a follow-up.
2. I've widened the CLI_CONTRACT dependency_not_installed row in 5fffa69 to say "for example because the lockfile never locked it". The change is docs only.

---

Generated by Claude Code