Refuse yarn classic pins with unlocked deps (#591)

[Mikola Lysenko (mikolalysenko)]

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

Fixes #591

Stacked on #1328 (#558): the hosted half reuses that PR's served-tarball fetch. The base is agent/v5-yarn-classic-sha1, and GitHub retargets this PR to main once #1328 merges. This replaces #1329, which GitHub merged into the unprotected stack base as soon as auto-merge was requested. That branch has been reset, and auto-merge here waits until this PR targets main.

Summary

A patch that rewrites the patched package's own package.json to add a dependency, or to move one to a new range, no longer leaves a yarn classic lock that yarn can't install reproducibly.

• Vendored: the patch is refused before any wiring is written, as vendor_dep_manifest_unlocked, when a dependency descriptor (name@range) has no yarn.lock block of its own. The staged uuid dir is removed. When every descriptor is already locked, the sub-maps are recomputed as before.
• Hosted: for any classic entry it hasn't pinned yet, the scan reads the served tarball's package.json (one download per artifact, checked against the grant's sha512, shared with Hosted yarn classic rewrite drops the #sha1 fragment when the grant has no sha1, so yarn's cache serves stale bytes: yarn ≤1.17 silently installs the unpatched package, and yarn ≥1.19 fails every warm-cache install #558's sha1 derivation).
  • If a descriptor isn't locked, the pin is refused (redirect_yarn_classic_dep_manifest_unlocked). The uuid is never confirmed or attested, and the lock is left alone.
  • If every descriptor is already locked, the block's dependencies: / optionalDependencies: sub-maps are rewritten to match (redirect_yarn_classic_dep_manifest_rewritten).
• Both refusals name the descriptors and give a remedy: lock them first (for example yarn add is-odd@^3.0.0), then re-run.

Root cause

• Vendored rewrite_classic_block (vendor/yarn_classic_lock.rs) recomputed the sub-map from the patched manifest but never checked that each descriptor resolves to a block. The result was a dangling dependency: online frozen installs fetched it unpinned, --offline installs failed, and every plain yarn install re-saved the lock.
• Hosted rewrite_yarn_classic never saw the patched manifest, so yarn kept the old graph. An added dependency was never installed, and a changed range stayed at the old version, while scan and vex reported success.

Changes

• formats/yarn/classic_deps.rs is new. It holds the sub-map rebuild (moved from the vendored backend), unlocked_descriptors and dep_maps_match, shared by both writers.
• NpmLockBackend::manifest_refusal is a new hook, checked after staging and before wire. The yarn classic backend implements it.
• Hosted, disk and in-memory flows: yarn_classic_artifact_targets / fetch_hosted_classic_artifact / fetch_classic_artifacts return the sha1 plus the manifest. The manifest is passed to the classic rewriter through the existing per-URL metadata map, the one the berry bin: renderer uses.
  • A fetch is made only for a registry block of the package that isn't pinned yet, or when the grant lacks a sha1.
  • No fetch is made when an offline mirror refuses the lock outright.
• Docs: CLI_CONTRACT.md (yarn classic row of the vendor table, plus three code rows) and docs/ecosystems.md (yarn classic hosted notes).

Tests (red before, green after)

• vendor::yarn_classic_lock::tests::issue_591_added_dependency_without_a_lock_block_is_refused
• vendor::yarn_classic_lock::tests::issue_591_changed_range_without_a_lock_block_is_refused, which also checks that the patch vendors once the new range is locked.
• Both vendored tests ran red with the refusal hook disabled: vendoring returned Done.
• patch::redirect::tests::issue_591_hosted_classic_checks_the_served_manifest_against_the_lock covers an added dep, a changed range, an unchanged manifest, and a locked new range whose sub-map is rewritten.
• scan::hosted_yarn_classic_sha1::issue_591_served_dependency_the_lock_does_not_lock_is_refused (built binary + wiremock) ran red on the Hosted yarn classic rewrite drops the #sha1 fragment when the grant has no sha1, so yarn's cache serves stale bytes: yarn ≤1.17 silently installs the unpatched package, and yarn ≥1.19 fails every warm-cache install #558 base: it pinned and reported redirected: 1.
• formats::yarn::classic_deps::tests::*

Fixture updates

The hosted classic pin now reads the served tarball, so these mocks serve a real tarball whose grant hashes match it:

• covgap_commands_scan_hosted (2 classic tests), in_process_redirect (CRLF classic test), hosted_memory_parity (parity_yarn_classic, via a serve_npm_tarballs case option), and e2e_redirect_yarn_classic_build::mock_hosted_grant.
• In classic_redirect_tampered_hosted_tarball_fails_integrity, the scan's own read now gets the real bytes and yarn's install gets the tampered ones. Without that, the scan would refuse the tampered bytes before yarn's integrity check ever ran.
• Existing vendored sub-map tests now lock the descriptors their patches add.

Commands run

• cargo test --workspace --all-features --no-fail-fast: green except the e2e_vendor_cargo_build old-toolchain legs (x86_64 rustup 1.41 can't exec on this arm64 host). After the rebase onto the Fix fragmentless yarn classic hosted pins (#558) #1328 review fix, I re-ran socket-patch-core --lib and the 9 affected CLI binaries, including the real-yarn e2e_redirect_yarn_classic_build and e2e_vendor_yarn_classic_*, all green.
• cargo clippy --workspace --all-features -- -D warnings: clean.
• cargo fmt --all -- --check: only a pre-existing diff in an untouched file.

Not covered

• vendor --check / repair still pass a lock that an older release already vendored with a dangling descriptor (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 follow-up comment). This PR only stops new runs from writing one.
• A vendored --dry-run stages nothing, so it doesn't foresee vendor_dep_manifest_unlocked.

🤖 Generated with Claude Code

---

Note

Medium Risk
Changes yarn classic vendored and hosted lock rewriting and hosted tarball fetching; incorrect descriptor checks could block valid patches or allow broken locks, but behavior is guarded by new refusals and extensive tests.

Overview
Fixes #591 by aligning yarn classic vendored and hosted flows with how Yarn 1 resolves installs from yarn.lock when a patch rewrites the patched package's own package.json.

Vendored: After staging, a new NpmLockBackend::manifest_refusal hook refuses vendor_dep_manifest_unlocked when any name@range from the patched manifest has no lock block; the staged uuid dir is unstaged and nothing is written. When every descriptor is already locked, lock blocks still get recomputed dependencies / optionalDependencies sub-maps (logic moved into shared formats/yarn/classic_deps.rs).

Hosted: Classic pins that need a served tarball now use fetch_hosted_classic_artifact (sha1 + manifest, not sha1-only). The rewriter refuses pins with redirect_yarn_classic_dep_manifest_unlocked when the served manifest introduces unlocked descriptors, or rewrites sub-maps with redirect_yarn_classic_dep_manifest_rewritten when all descriptors are locked. Disk, memory, and scan paths share yarn_classic_artifact_targets / record_classic_artifact.

Docs & tests: CLI_CONTRACT.md and docs/ecosystems.md document the new codes; tests and mocks serve real tarballs where hosted classic now reads package.json.

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

[Mikola Lysenko (mikolalysenko)]

BugBot review

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

[cursor]

🔒 Agentic Security Review

Severity: MEDIUM

Description: Hosted yarn classic re-pins skip the #591 package.json dependency-closure check when the grant already has sha1 and yarn.lock already contains that artifact URL. The scan never fetches the tarball, so no served manifest is recorded, unlocked-descriptor refusal and sub-map rewrite do not run, and the uuid is still confirmed because the URL is already in the lock.

Impact: Yarn 1 installs only the dependencies named by the lock block's dependencies and optionalDependencies sub-maps. A patch that adds a dependency or changes a range is applied and attested while those sub-maps stay at the pre-check closure, so yarn install --frozen-lockfile never installs the added or retargeted descriptors. This is the re-scan path for sha1 grants, including every pin written before this PR (those sub-maps were never synced) and any later grant that returns the same artifact URL. Vendored mode still runs the staged-manifest check on every wet run.

Remediation: Fetch and record the served manifest for every classic pin that will be kept, including a lock that already contains this artifact URL. Refuse or rewrite sub-maps before confirming the uuid. Do not treat exact-URL presence plus a grant sha1 as proof the closure was checked; pre-#591 pins and same-URL re-grants never stored a manifest.