Fix hosted cargo contested lock in vex and restore (#679, #863)

[Mikola Lysenko (mikolalysenko)]

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

Fixes #679
Fixes #863

Summary

After scan --mode hosted pins a crate such as cfg-if 1.0.4, a dependency added later (cargo add, a merge or a path member) can lock its own crates.io copy of the same name@version. Cargo never unifies packages from different sources, so Cargo.lock holds two cfg-if 1.0.4 blocks and the build compiles the unpatched one for that dependent.

• Hosted cargo vex attests not_affected while Cargo.lock also builds an unpatched crates.io copy of the same crate@version #679 (vex): vex/discover/cargo.rs now records each non-Socket block (crates.io, git) of a hosted crate's name@version as a same-lock unpatched_copy. The shared contest_within_locks rule then withholds the hosted ref from attestation, with a patched_ref_unattributable diagnostic naming the crates.io entry. Before, it was attested not_affected. The ref is kept as shadowed, so rollback / remove / list / the takeover still unwind it (npm hosted pin next to a bundled copy can't be unwound: rollback/remove refuse it, and the vendored takeover skips the restore, so vendor --revert lands back on hosted and allow-remote=all stays #828 semantics). It is not rewirable, because a re-scan can't rewire a transitive crates.io dependent (redirect_cargo_transitive_dependents).
• Hosted cargo remove writes a duplicate cfg-if block into Cargo.lock when crates.io also locks the same crate@version, so cargo can no longer parse the lock while remove reports success #863 (remove / rollback): patch/redirect/upstream/cargo.rs merges the Socket block into an existing crates.io block for the same name@version. Before, it spliced crates.io's source into the Socket block and left two identical blocks, which cargo refuses to parse. Now it drops the Socket block (and a v1 lock's [metadata] line) and respells dependents' references the way cargo writes them: the bare name when the crate is unique, "name version" when only the version is, and the full id in v1. The twin already carries the checksum, so this path needs no index lookup and works offline.

Root cause

Neither the cargo VEX extractor nor the cargo upstream restore handled a lock in which the hosted pin and a crates.io copy share one name@version.

Tests (red → green)

Issue	Test	Red before fix
#679	socket-patch-core vex::discover::cargo::tests::crates_io_twin_of_the_hosted_crate_withholds_the_ref (crates.io and git twins; another version doesn't count)	FAILED with the new call disabled
#863	patch::redirect::upstream::cargo::tests::restore_merges_into_an_existing_crates_io_twin, restore_merge_keeps_versioned_refs_while_the_name_is_ambiguous, restore_merge_in_a_v1_lock	all 3 FAILED (restore refused / duplicate)
both, real cargo	e2e_redirect_cargo_shapes::cargo_hosted_contested_by_a_later_crates_io_copy: hosted scan → fresh checkout adds crc32fast = "=1.5.0" → contested lock (2 blocks); vex doesn't attest cfg-if; remove exits 0 leaving one crates.io block; cargo build --locked --offline succeeds	new

CLI_CONTRACT.md: the cargo discovery row and the cargo bullet in "Hosted unwind coverage" are updated.

Commands run

• cargo clippy --workspace --all-features -- -D warnings: clean
• cargo test -p socket-patch-core --lib: 6113 passed
• cargo test -p socket-patch-cli --test e2e_redirect_cargo_shapes: 28 passed (cargo 1.93.1)
• cargo test -p socket-patch-cli --test mode_migration_cargo: 28 passed
• e2e_vendor_cargo_build: the 2 old-toolchain legs (*_old_toolchains, *_below_1_45) fail locally. They exercise vendored wiring under old rustup toolchains, which this diff doesn't touch, so I'm leaving them to CI.
• rustfmt on the changed files

🤖 Generated with Claude Code

---

Note

Medium Risk
Changes hosted Cargo lockfile restore and VEX attestation for duplicate name@version blocks; incorrect merge or reference respelling could break cargo build --locked or mis-attest patches.

Overview
Fixes hosted Cargo when Cargo.lock holds both a Socket sparse-index block and a separate crates.io (or git) block for the same name@version—typically after a dependency is added post-scan (#679, #863).

VEX / discovery adds unpatched_twins: any non-Socket twin of a hosted crate is recorded via unpatched_copy, so attestation is withheld (patched_ref_unattributable, “UNPATCHED”) while the pin stays shadowed so rollback / remove / list still unwind it. Upstream restore no longer rewrites the Socket block to crates.io (which duplicated blocks and broke cargo parsing); it drops the Socket [[package]] (and v1 [metadata] checksum lines), skips index lookup when the twin already has the checksum, and respells dependent references with merged_references to match what Cargo would write.

CLI_CONTRACT.md documents the contested-lock behavior for discovery and hosted unwind. Tests cover unit restore/merge paths, discovery withholding, and a new e2e shape cargo_hosted_contested_by_a_later_crates_io_copy.

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

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

BugBot review

[cursor]

Stale Bugbot comment from a previous run.

[Mikola Lysenko (mikolalysenko)]

bugbot run

---

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 659fcbc. Configure here.}

[cursor]

🔒 Agentic Security Review

Severity: MEDIUM

Description: Hosted Cargo restore treats a contested crates.io twin as already bound and skips the sparse-index checksum rebind. twin_of matches only name, version, and source registry+https://github.com/rust-lang/crates.io-index, then those hits are filtered out of UpstreamClient::cargo_cksum (the lookup that refuses offline and returns the public index cksum). Restore deletes the Socket [[package]] block and its v1 [metadata] checksum line, respells dependents onto the surviving twin, and marks the uuid handled, so the Cargo.toml registry pin and unused [registries.socket-patch-<uuid>] block are removed. The twin checksum bytes are never read or compared. A later cargo fetch from the canonical index still fails closed when that checksum differs from the index cksum, before crate code runs. A [source.crates-io] replace-with registry or vendor directory does not consult that index: cargo accepts the .crate when its sha256 equals the lock checksum. This merge moves former Socket-sourced dependents, which were outside crates.io replacement, onto that checksum.

Impact: Rollback, remove, and vendored takeover all run this restore. A lockfile author can leave a non-index sha256 on the crates.io twin; restore now succeeds offline and publishes that sha256 as the only pin. Builds that use the real crates.io index error on the mismatch. Builds whose crates.io source is replaced or vendored, including a replacement committed next to the lockfile, run the bytes that hash to the preserved checksum (crate code and build scripts). Re-resolving cargo_cksum would have written the public-index checksum and made those divergent bytes fail closed. An honest twin produced by cargo against the real index is unchanged, because that checksum is already the index cksum.

Remediation: Still call cargo_cksum for a contested twin and refuse that pin when the lookup fails or the twin's inline or v1 metadata checksum is not exactly the sparse-index cksum, including under --offline. Only after that match, delete the Socket block, keep the twin, and mark the uuid handled.