Skip to content

Stop attesting a Cargo pin a workspace member resolves from crates.io - #2019

Draft
Mikola Lysenko (mikolalysenko) wants to merge 4 commits into
fix/cargo-members-liveness-originfrom
fix/cargo-mixed-member-pins
Draft

Mikola Lysenko (mikolalysenko) wants to merge 4 commits into
fix/cargo-members-liveness-originfrom
fix/cargo-mixed-member-pins

Conversation

@mikolalysenko

@mikolalysenko Mikola Lysenko (mikolalysenko) commented Nov 4, 2026 •

Copy link
Copy Markdown
Collaborator

Stacked on #2012 (base fix/cargo-members-liveness-origin).

Problem

#2012 made Cargo VEX discovery read workspace member manifests, but left one case open. When one declaration pins a crate to socket-patch-<uuid> and another declaration of the same crate is not pinned (crates.io, another registry, a path or git) with a requirement that may select the locked version, discovery still emitted an attested Cargo.lock ref. "Some declaration pins the patch" was enough. The other declaration can be the root's or a member's.

Cargo resolves each source on its own. The unpinned declaration builds a separate, unpatched copy, and that copy can be the very version the pin covers. The hosted rewriter pins every declaration in the root and every member (patch/redirect/mod.rs), so this mixed state comes from a partial revert or a hand edit.

Fix

Each declaration of a Socket-locked crate is classified once (route). It is one of three things:

  • routed to a Socket patch: by its own socket-patch-<uuid> registry, or by a Socket [patch.<source>] registry entry in the root manifest or project config that redirects it;
  • unpinned: cargo builds its own copy for it;
  • saying nothing about the locked version. That covers a requirement that excludes it, a [patch] entry itself, a [workspace.dependencies] template no manifest inherits, and a malformed socket-patch-* name (diagnosed elsewhere).

What the classification decides when some declaration names the lock's patch A:

  • Unpinned declaration: the ref stays (rollback, remove and the ledger liveness gates keep working) but is marked unattested with the new UnattestedKind::CargoUnpinnedDeclaration.
    • The mark names the declaration's manifest, requirement and source.
    • It also says whether re-running scan can pin the declaration. Only a crates.io declaration whose requirement matches no other locked version can be re-pinned; for anything else the rewriter refuses, so the remedy is to change the declaration or roll the patch back.
  • What counts as a pin: registry = "socket-patch-<uuid>" beside path, git or registry-index is not a pin, because cargo builds that other source. This matches the hosted rewriter's own pinned check.
  • Another patch B: a declaration routed to patch B that may select the locked version, where the lock does not resolve the crate from B, adds B's Cargo.toml ref. The CLI then gates the package as wiring_conflict.
  • Same-version twin: when the lock already holds a same-version copy from another source, no mark is added. The existing twin rule (Hosted cargo vex attests not_affected while Cargo.lock also builds an unpatched crates.io copy of the same crate@version #679) withholds the ref and names the lock entry.

Version requirements go through patch::redirect::cargo_req_selects, with the lock's other versions of the crate, so "may select" and "ambiguous" agree with the rewriter.

CLI: a new note arm vex_cargo_unpinned_declaration in vex_sources.rs. Its remedy is generic ("not attested while that declaration stands"); the per-case remedy is in the detail. Both places in CLI_CONTRACT.md are documented.

Vendored mode

Vendored mode does not have this bug. Its wiring is a root [patch.crates-io] entry, which cargo applies workspace-wide to every crates.io declaration the copy satisfies. The related gap (a member declaring the crate from another registry or git) is handled in #2024, which is stacked on this PR.

Review findings (/code-review high)

All nine were confirmed and fixed. For the behaviour bugs I wrote the test first and saw it fail; 7 tests failed before the fix.

  1. Fixed. A crates.io declaration that a Socket [patch] registry entry redirects was marked unpinned. Redirected declarations now count as pins. Added out.unattested.is_empty() to patch_table_registry_pin_counts_as_the_declaration and config_patch_registry_pin_counts_for_a_declared_crate, plus a case where a declaration from another registry, not redirected by [patch.crates-io], is still marked.
  2. Fixed. A socket registry beside path / git / registry-index counted as pinned. Test: a_socket_registry_beside_another_source_is_not_a_pin. Next to a real pin, it marks the ref. As the only declaration, it keeps the ref because Socket's wiring is present, but marks it.
  3. Fixed. The remedy now depends on the case (the_remedy_says_whether_a_rescan_can_pin_it). A requirement that also matches another locked version names that version and says scan cannot pin it.
  4. Fixed. In the same-version twin case the mark is skipped, and the Hosted cargo vex attests not_affected while Cargo.lock also builds an unpatched crates.io copy of the same crate@version #679 diagnostic stands (a_locked_same_version_twin_is_left_to_the_twin_rule).
  5. Fixed. A [workspace.dependencies] template counts only when inherited (an_unpinned_workspace_template_counts_only_when_inherited).
  6. Fixed. A registry-index declaration is reported as "the registry index ", with the roll-back remedy.
  7. Fixed. Root pins A and a member pins B: B's ref is emitted beside A's (a_member_pinned_to_another_patch_is_a_conflict_pair). No conflict is emitted when B's requirement excludes the locked version, or when the lock resolves the crate from B at another version (a separate, patched package with its own ref). This overlaps with the split-patch part of Withhold a vendored Cargo copy a member's other source bypasses #2024; see the note there.
  8. Fixed. The code uses cargo_req_selects with the lock's other versions; Decl::accepts is gone.
  9. Fixed. Decl carries a DeclSource enum and a live flag. The origin text is formatted only for messages, and [patch] pins use Decl::patch, which has no dead version parameter.

Tests

  • cargo test -p socket-patch-core --lib -- vex::discover:: vendor::cargo patch::redirect:: utils::cargo_workspace: 1555 passed. The golden corpus test passes with no golden changes.
  • cargo test -p socket-patch-cli --lib vex_sources: 15 passed.
  • cargo test -p socket-patch-cli --test in_process_redirect cargo: 6 passed.
  • cargo clippy --locked --workspace --all-features -- -D warnings and cargo fmt --all -- --check are clean.

Not done

  • The lockless path (unresolved_manifest_pins, from Read Cargo members in discovery; match liveness uuids by origin #2012) still treats a socket-patch-<uuid> registry beside path / git as pinning, and an uninherited template as a declaration. It is unchanged here.
  • In the old "no declaration pins the lock's patch" branch, the conflict pair is still emitted without a requirement check.
  • I did not read the lock's per-package dependencies lists to tell which copy a declaration actually resolved.

Brought current with #2012, plus round-2 review findings (cloud routine)

Merged the updated #2012 in. It merged cleanly, and the targeted core tests pass (688).

Review findings, round 2 (/code-review high on 7837fb4)

  1. The remedy treated registry = "crates-io" as another registry: fixed. The hosted rewriter re-pins that declaration (Hosted cargo redirect refuses a crates.io dependency declared with an explicit registry = "crates-io", treating crates.io as "another registry" #386), so the note now says a re-scan pins it. Test: the_remedy_says_whether_a_rescan_can_pin_it (extended).
  2. FALSE ATTESTATION: a shadowed [patch] Socket pin counted as pinning declarations. Confirmed. Cargo ignores a [patch.crates-io] entry when a crates.io URL table replaces it, or when the config redefines the key. patch_pins still counted such an entry as honored, so a member's crates.io declaration was taken as routed to the patch. Fixed in Withhold a vendored Cargo copy a member's other source bypasses #2024, where the Shadow classifier lives (the next PR in this chain). Test there: a_shadowed_patch_pin_pins_no_declaration.

Not changed:

Tests (cloud run): cargo test -p socket-patch-core --lib -- vex::discover:: gives 465 passed, golden included. clippy and fmt are clean.


Note

Medium Risk
Changes attestation semantics for hosted Cargo pins in multi-manifest workspaces; incorrect classification could omit valid VEX statements or miss risky mixed wiring, though behavior is heavily tested.

Overview
Cargo VEX discovery no longer attests a Cargo.lock hosted pin when another root or workspace-member declaration of the same crate is not wired to Socket (crates.io, another registry, registry-index, path, or git) but its version requirement may still resolve the patched lock version. Those mixed states are flagged as unattested via vex_cargo_unpinned_declaration / UnattestedKind::CargoUnpinnedDeclaration: the pin stays discoverable for list, rollback, and ledger liveness, but vex skips the statement and explains whether scan can re-pin the stray declaration or the user must change it or roll back.

Discovery now classifies every declaration (route): Socket registry or [patch] redirect, unpinned copy Cargo builds separately, or irrelevant to the locked version. Decl tracks source kind and whether [workspace.dependencies] is actually inherited; socket-patch-<uuid> beside path/git/registry-index is not treated as a pin; conflicting declarations routed to another patch still surface as wiring_conflict, and existing same-version twin handling is unchanged.

CLI_CONTRACT.md documents the new unattested case and warning code; vex_sources.rs maps it to plan notes and tests the gating path.

Reviewed by Cursor Bugbot for commit ebe84e1. Configure here.


Generated by Claude Code

Cargo resolves each source on its own. When one declaration of a crate
pins it to `socket-patch-<uuid>` but another (the root's or a workspace
member's) names no Socket registry (crates.io, another registry, a path
or git) with a requirement the locked patched version satisfies, that
declaration builds a separate, unpatched copy, possibly of the very
version the pin covers. Discovery still attested the Cargo.lock ref
because "some declaration pins the patch" was enough.

The hosted rewriter pins every declaration, so this mixed state is a
partial revert or a hand edit. The ref now stays (rollback, remove and
the ledgers' liveness still see the pin) but is marked unattested with
the new `UnattestedKind::CargoUnpinnedDeclaration`, naming the unpinned
declaration's manifest. When Cargo.lock already locks a same-version
copy from another source beside the Socket entry, the detail names that
lock entry as direct evidence. A declaration whose requirement excludes
the patched version, and a `[patch]` registry entry (which redirects
declarations rather than adding one), do not withhold the statement.

The CLI omits the ref as `vex_cargo_unpinned_declaration`; documented in
CLI_CONTRACT.md.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review fixes for the unpinned-declaration mark. Each declaration of a
Socket-locked crate is now classified once (`route`): routed to a Socket
patch (its own `socket-patch-<uuid>` registry, or a Socket `[patch.<source>]`
registry entry that redirects it), unpinned (cargo builds its own copy),
or saying nothing about the locked version.

- A crates.io (or registry) declaration that a Socket `[patch.<source>]`
  registry entry in the root manifest or project config redirects is no
  longer counted as unpinned.
- A declaration naming `registry = "socket-patch-<uuid>"` beside `path`,
  `git` or `registry-index` is not a pin (the hosted rewriter's own pinned
  check): cargo builds that other source, so it now marks the ref.
- Requirements go through `patch::redirect::cargo_req_selects` with the
  lock's other versions of the crate, and the detail says whether a
  re-scan can pin the declaration: only a crates.io declaration whose
  requirement matches no other locked version; otherwise the rewriter
  refuses (`redirect_cargo_toml_dep_unrewritable`), so the remedy is to
  change the declaration or roll back. The CLI note's remedy is now
  generic and the per-case remedy lives in the detail.
- `registry-index` declarations are reported as such, not as crates.io.
- A `[workspace.dependencies]` template counts only when some manifest
  inherits it (`workspace = true`).
- When the lock already holds a same-version copy from another source,
  no mark is added: the twin rule (#679) withholds the ref and names it.
- Root pinned to patch A with a member pinned to patch B (for the locked
  version, and B not resolved by the lock) now emits B's Cargo.toml ref
  beside A's, so the CLI's wiring-conflict gate runs.
- `Decl` carries a `DeclSource` enum and a `live` flag; the origin text is
  formatted only for messages, and `[patch]` pins are built with
  `Decl::patch` (no dead version parameter).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Mikola Lysenko (mikolalysenko) added a commit that referenced this pull request Nov 6, 2026
`vendor` wires a crate through the root's `[patch.crates-io]`, which
cargo applies only to declarations from crates.io. A workspace member
(or the root) that declares the crate from another registry, a
`registry-index` or git, at a requirement that may select the patched
version, resolves its own unpatched copy. The vendored ref stayed
attested beside it because the lock copy claim only reads sourceless
entries.

The ref now stays (rollback, remove and the ledgers still see the
wiring) but is marked unattested with the new
`UnattestedKind::CargoPatchBypassed`, naming the declaration. A
same-version Cargo.lock entry from any registry or git source beside the
copy is marked the same way, naming the lock entry. The check reads the
declarations through #2019's classification: live declarations only, a
registry dependency counts as covered when a `[patch.<registry>]` table
pointing at the same copy redirects it (`Decl::patched_by`), a path
declaration (the user's own fork) keeps the existing "ignored" rule, and
a `socket-patch-*` registry dependency is hosted wiring. The CLI omits
the ref as `vex_cargo_patch_bypassed`; documented in CLI_CONTRACT.md.

The workspace member walk ran only when the lock held a Socket-sourced
package (or, with no lock, a manifest named a `socket-patch-` registry).
A vendored copy's lock entry has no source and it has no such pin, so
the walk now also runs whenever the root manifest or the project config
wires a Socket copy through `[patch]`, whatever the lock's state.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Second review round. The unpinned-declaration remedy sent every
declaration with an explicit `registry` key to "it names another
registry ... roll the patch back", including `registry = "crates-io"`,
which the hosted rewriter re-pins (#386); it now says a re-scan pins it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Mikola Lysenko (mikolalysenko) pushed a commit that referenced this pull request Nov 6, 2026
FALSE ATTESTATION (found reviewing #2019; fixed here, where `Shadow`
lives): a root `[patch.crates-io]` Socket registry pin cargo ignores
(a `[patch."<crates.io URL>"]` table replaces it, or the config
redefines the key) still counted as pinning every crates.io
declaration of the crate, so a member's plain declaration was taken as
routed to the patch and the hosted ref attested while cargo builds
crates.io. Such an entry stays a declaration (Socket's wiring is
present, the ref stays) but is not live, and `route` only weighs live
`[patch]` pins, as the vendored bypass check already weighs only
honored entries.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

bugbot run


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 ebe84e1. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants