Repository navigation
Stop attesting NuGet versions the writers refuse - #2007
Draft
Mikola Lysenko (mikolalysenko) wants to merge 2 commits into
Draft
Mikola Lysenko (mikolalysenko) wants to merge 2 commits into
Mikola Lysenko (mikolalysenko) wants to merge 2 commits into
Conversation
When a governed packages.lock.json locks a Socket-mapped NuGet id at a version other than the patched one (another target framework, or another project's lock), both writers refuse to wire it (redirect_nuget_lock_other_version, vendor_nuget_lock_other_version): the exact-id mapping routes every version to a source that serves only the patched one, so that framework cannot restore. Discovery instead emitted an attested ref per locked version, including the unpatched one. Discovery now applies the writers' rule. Each ref of such a source is still emitted (rollback, remove, list and the ledgers' liveness gates keep seeing the wiring) but marked with the new UnattestedKind::NugetLockOtherVersion, which the VEX plan omits with the vex_nuget_lock_other_version note. The patched version comes from where the files name it: the vendored feed's single nupkg; a hosted url names only the uuid, so any two locked versions qualify there. Lock versions are now keyed through normalize_nuget_version, as the writers' locked_at / other_versions match them, so 1.0 and 1.0.0 are one version with one pin check rather than two refs with independent pins. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Vendored: the feed's single nupkg names the patched version, so it is the only ref, even when no lock entry is at it (keeping the ledger's vendored_claim live); other locked versions are reported in the unattested detail, never as refs pointing at nupkgs that don't exist. - Each ref's other-version check excludes its own version: the detail and file name the lock holding the OTHER version, listed in NuGet precedence (13.0.9 before 13.0.10), and say "also lock" only when the ref's version is locked. - Trim `resolved` before normalizing; an empty one is diagnosed and counts as another version (the writers would refuse it). - The contentHash-conflict diagnostic names the locks and spellings. - Only Socket-mapped ids are indexed; one push_ref helper marks refs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Sometimes a governed
packages.lock.jsonlocks a Socket-mapped NuGet id at a version other than the patched one: in another target framework, or in another project's governed lock. Both writers refuse to wire that:rewrite_nuget→redirect_nuget_lock_other_versionnuget_prelude→vendor_nuget_lock_other_versionThe exact-id
<packageSourceMapping>sends every version of the id to a source that serves only the patched version, so that framework can't restore. Discovery (vex/discover/nuget.rs) still emitted one attested ref per locked version, the unpatched one included. It also keyed versions by rawresolved.trim(), while the writers match throughnormalize_nuget_version. As a result,13.0.3and13.0.3.0became two refs, each with its own pin check.Fix
normalize_nuget_version(resolved.trim()).PurlKey).resolvedgetspatched_ref_invalid. The writers would count it as another version, so it withholds the statement the same way.UnattestedKind::NugetLockOtherVersion. The VEX plan then omits it withvex_nuget_lock_other_version. The note names the other versions in NuGet precedence order, and the lock that holds the first of them.<idLower>.<versionNorm>.nupkgnames the patched version, and that is the only ref, pinned by the lock entries at it if there are any. It is still the ref when no lock entry is at that version, so the ledger'svendored_claimstays live. Other locked versions show up only in the detail, never as refs that point at nupkgs that don't exist. With no single feed nupkg (no feed, or several nupkgs), the lock's versions are the refs, checked the same way as hosted.record_mismatch(see finding 4).Why an unattested kind rather than a diagnostic plus dropping the ref
Dropping the refs (or moving them to
shadowed) makeshosted_claim/vendored_claimreturnSome(false)for the ledger record of the patched version. Thenscan,vendor --check,rollbackandremovewould treat Socket's own wiring, which is still present, as reverted. Re-runningscan/vendorcan't clear that either.Unattestedis meant for exactly this case.Review findings (
/code-review high)vendored_claimon the real artifact returnedSome(false). The feed's artifact is now the ref, unpinned and unattested. The test assertsvendored_claim == Some(true).set_aside_competing_patterns, Hosted/vendored NuGet mapping isn't exclusive when nuget.config already maps the exact package id to another source, so restore races the Socket feed against nuget.org (NU1403 with a lock, silently unpatched without one) #462). So a non-exclusive mapping is never writer output. With one in place, the patched version's lockcontentHashfails a restore from the competing source anyway. This module's non-goals already say so.local_record_by_uuid) or from the API, names another package, so it is gatedrecord_mismatch. Without a record it is gatedrecord_unavailable.hosted_claimfinds no ref wiring 13.0.3.Discoveryfield and a plan pass after record resolution, with no change in behavior. CLI_CONTRACT.md now documents this split." 13.0.3 "next to13.0.3used to read as two versions. Values are now trimmed before normalizing, and an empty value is diagnosed.packages.lock.json, src/App/packages.lock.json: Newtonsoft.Json 13.0.3 / 13.0.3.0 ….read_dirof a single uuid directory.push_refhelper now marks and pushes refs.Tests (each fails on the code it fixes, checked by splicing the older extractor under the new tests)
hosted_id_locked_at_two_versions_is_unattestedcovers:vendored_id_locked_at_another_version_is_unattestedcovers:vendored_claim == Some(true)13.0.3.0against the feed's13.0.3, which is not "other"non_normalized_version_spellings_are_one_versioncovers:13.0.3," 13.0.3 "and13.0.3.0→ one refpatched_ref_invalid, with the diagnostic naming both locks and spellingsRuns:
cargo test -p socket-patch-core --lib -- vex::discover:: formats::nuget vendor::nuget: 582 passedcommitted_fixture_corpus_matches_goldenpasses, with no golden changescargo clippy --locked --workspace --all-features -- -D warnings: cleancargo fmt --all -- --check: cleanNot done
🤖 Generated with Claude Code