fix(vendor): gem wiring reconstruction, dir-artifact file inventory, per-section lock remotes - #179
fix(vendor): gem wiring reconstruction, dir-artifact file inventory, per-section lock remotes#179Mikola Lysenko (mikolalysenko) wants to merge 6 commits into
Conversation
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
Bugbot Autofix is ON. A cloud agent has been kicked off to fix the reported issue.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 0f24244. Configure here.
|
Review polish pushed as e5d975e — all 5 findings addressed, none skipped. Major — service-provenance repair loop (repair_vendor.rs): confirmed and fixed at the post-verify seam. A member-verified dir-shaped rebuild of a NON-reconstructed entry that fails only the recorded-inventory comparison now recomputes the inventory from the verified rebuild, persists it, and names the source switch ( Minor — vendor_inventory_missing noise: gated to gem, the one backend that records inventories. Pinned inside Minor — existing empty-wiring entries never healed: pass 1 now backfills full revert-capable wiring via Minor — reconstruction doc wrinkles: both documented on Minor — gem_remotes scheme filter: confirmed empirically against real bundler 4.0.15 (a Gemfile mixing an https source with a Verification: core 2474 green; cli lib 350; 🤖 Generated with Claude Code |
|
Bugbot finding (no-ledger restore TOFU hole) — CONFIRMED and fixed in bd4fae8. Verified against the code first: pass 2's Healthy branch ran Fix (mirrors the PR's own
Tests (both verified RED on the pre-fix code): Runs: repair_vendor_e2e 24/24 (incl. G1's byte-identical + revert oracle), repair_vendor_flavors_e2e 28/28, core 🤖 Generated with Claude Code |
d063cb1 to
99f9cd0
Compare
bd4fae8 to
914004c
Compare
|
Rebased Conflicts resolved (1):
Verification (all green):
🤖 Generated with Claude Code |
Pull request was closed
…truction, dir file inventory, per-section lock remotes
Three verified holes in the vendored-gem integrity story, each fixed at
its seam and pinned by tests that fail without the fix.
1. repair's no-ledger reconstruction synthesized entries with EMPTY
wiring; a later `vendor --revert` of such a gem entry deleted the
artifact dir, reported success with zero warnings, and left the
Gemfile `path:` + lock PATH section pointing at the deleted dir
(next `bundle install` hard-fails). Now:
- gem::reconstruct_gem_wiring rebuilds full revert-capable wiring
from the backend's own recognizable pair edit (exact-pin line /
managed fence, PATH section, `!` dependency pin), grammar-strict
and fail-closed; for an exact-pin fixture the reconstructed ledger
is byte-identical to the original. Two documented degradations: a
pre-vendor RANGE constraint restores as the canonical exact pin,
and a CHECKSUMS sha256= token is not offline-recoverable — the
bare entry stays for a plain `bundle install` to refill (bundler
4.0.15 verified: plain install heals it byte-identically, frozen
fails with a self-explanatory message until then), surfaced as
vendor_checksum_unrecoverable.
- repair routes reconstruction per ecosystem (repair_vendor.rs
seam); non-gem ecosystems keep empty wiring — their pre-vendor
originals are registry integrity material no offline source can
reproduce — and the gap now surfaces loudly as
vendor_wiring_unknown instead of silently.
- revert_gem refuses an empty-wiring entry outright
(vendor_wiring_unknown + manual cleanup steps) instead of
stranding the pair edit. The npm-family revert has the same hole
and still 'succeeds' silently — the shared guard belongs in
dispatch_revert_one (outside this PR's scope, see PR body).
2. check_vendored_artifact's whole-file drift cross-check only covered
file-shaped artifacts; a vendored gem DIRECTORY was verified purely
by record.files afterHashes, so tampered/drifted UNPATCHED files
(or the stub gemspec) were blessed Healthy by repair and attested
by VEX. VendorArtifact gains `fileInventory` (relative path →
plain sha256, sorted, camelCase) recorded by vendor_gem at vendor
time and verified whole-tree inside verify_vendored_patch_record
(missing/extra/modified files and planted symlinks all fail;
vendor_inventory_mismatch → Corrupt → repair rebuilds; VEX refuses
while tampered). Backward-tolerant: entries without the field keep
member-only verification and repair names the gap
(vendor_inventory_missing).
3. inventory_gemfile_lock captured the FIRST `remote:` across ALL GEM
sections and resolved every spec against it, so specs under a
second GEM source (private gem server) got a wrong (e.g.
rubygems.org) URL — a 404 at best, a private-gem name leak at
worst. Specs now resolve against their OWN section's remote
(bundler ≥ 2 emits one section per source and hard-errors on
multiple global sources — verified on 4.0.15); a legacy bundler-1.x
section with several remotes is genuinely ambiguous and stays
discovery-only (fail-closed, no guessed URL). gem_remote_base had
the same first-remote assumption in ledger recovery: recovery now
requires the lock's GEM sources to agree on a single remote and
refuses ambiguity.
Verified: core 2474/0, cli lib 350/0, repair_vendor_e2e 20/20 (5 new
gem rows), adjacent vendor/vex/scan suites green, e2e_vendor_gem_build
capstone green under real bundler 4.0.15, clippy+fmt clean.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…backfill empty gem wiring, scheme-agnostic remote ambiguity Review follow-ups on the gem vendored-integrity batch: - repair's post-verify no longer deterministic-fails a service-vendored gem entry: a member-verified local rebuild whose tree differs from the recorded fileInventory (the service's converter stub vs the local one) refreshes the inventory from the verified rebuild and persists it, with the provenance named (vendor_inventory_refreshed) — instead of deleting the rebuild, stranding the wired pair on a dead dir, and re-failing every later repair. - pass 1 backfills full revert-capable wiring for EXISTING empty-wiring gem ledger entries (the pre-reconstruction repair population) from the live pair while healthy, so `vendor --revert` stops refusing. - the vendor_inventory_missing nag is gated to gem — the one backend that records inventories; cargo/golang/composer re-vendors record nothing, so the advice was permanent per-run noise there. - gem_remotes collects lock remotes scheme-agnostically before the ambiguity refusal: a mixed http+file:// lock (real bundler 4.0.15 shape, one GEM section per `source "file://…" do` block) no longer collapses to one "agreed" remote and leaks the gem name to the http registry; a lone non-http remote refuses instead of defaulting to rubygems.org. - reconstruct_gem_wiring docs: the trailing-comment loss degradation and the deliberately-conservative bare-CHECKSUMS warning are documented. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ger entry The no-ledger restore (pass 2) ran check_vendored_artifact on a synthesized entry with no fileInventory/sha256 — verifying only the patched members — and then filled the fingerprint from the LIVE artifact and persisted it. For anything without an npm-family wired lockfile integrity (gem dirs foremost), a tampered UNPATCHED file was thereby canonized: later repairs enforce the tampered tree and VEX attests it (trust-on-first-use). Healthy-but-unanchored reconstructions are now queued as SOFT rebuild candidates: the ledger entry is restored fingerprint-less first (so the restore survives any rebuild failure and wiring originals carry forward), the untrusted live tree is cleared only once the patch sources and a pristine source are both secured, and the canonical fingerprint derives from the member-verified local rebuild — exactly the dispatch every other rebuild here uses. When no trustworthy pristine source exists (not installed, no recoverable registry fragment, offline, fetch failure) the entry stays in the legacy member-only state with a vendor_inventory_unverified warning — later repairs keep naming the gap (vendor_inventory_missing) instead of enforcing a tampered tree. Anchored file-shaped artifacts (rewired npm-family lock integrity, verified intact) restore as before. Pinned by two gem e2e tests (both verified RED on the pre-fix code): tampered-unpatched-file + lost ledger is healed by the rebuild with the restored ledger byte-identical to the pre-tamper original, and the no-pristine-source variant restores without a fingerprint, warns, and stays in the legacy-warn state. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
914004c to
bf55fa1
Compare
|
Rebased onto main — new tip bf55fa1 (was 914004c). What happened: the CONFLICTING state was the stale stacked base — this PR sat on Verification on the final base:
🤖 Generated with Claude Code |
|
Heads-up for reviewers: the red Otherwise green and MERGEABLE after the rebase onto current main. Awaiting an approving review to merge. 🤖 Generated with Claude Code |
Conflict: crates/socket-patch-core/src/vendor/mod.rs — both sides widened adjacent re-export lines. Resolved as a union: * main (#174) exports state::carry_forward_wiring, the extracted re-vendor ledger reconciliation persist_vendor_entry now calls; * this branch exports verify::artifact_is_file_shaped and verify::compute_dir_inventory, which the CLI needs because verify is pub(crate). Both hunks are kept verbatim; neither side's symbol set changes. The gem sibling (#177) was already in this branch's base, so main brings only pnpm/redirect work. carry_forward_wiring reconciles wiring, pnpm meta and the go-takeover flag and never touches entry.artifact, so the new fileInventory on a re-vendor keeps the freshly computed inventory — no interaction. Main added no VendorArtifact literal, so the mechanical file_inventory field addition stays complete. Verified: core lib 2215/0, cli lib 376/0, repair_vendor_e2e 24/0, in_process_vendor 30/0, e2e_vex_vendor 8/0, setup_contract_gaps 10/0, e2e_gem 8/0, in_process_gem_apply 7/0, e2e_vendor_gem_build 5/0, e2e_vendor_pnpm_build 6/0. vendor/mod.rs is rustfmt-clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ix/gem-vendor-integrity
|
Windows CI update: the 🤖 Generated with Claude Code |
|
The hosted-e2e fix is up: #199 (production publishes zero free gem patches — pin retired, gem canary-ized; live suite 15/0 on that branch). Once #199 merges, a rerun here goes fully green. 🤖 Generated with Claude Code |
STACKED on PR #175 (
fix/gem-bundler-audit) — base branch isfix/gem-bundler-audit, notmain.Closes three verified vendored-integrity holes (all P2), each fixed at its seam and pinned by a test that fails without it. Every bundler-behavior assumption below was validated against a real bundler 4.0.15 in a scratch project before being baked in.
1. repair reconstruction synthesized empty wiring → revert stranded the pair edit
repair's no-ledger reconstruction (repair_vendor.rs) created ledger entries withwiring: []. A latervendor --revertof such a gem entry deleted the artifact dir, reported success with zero warnings, and left the Gemfilepath:+ lock PATH section pointing at the deleted dir — the nextbundle installhard-fails.vendor::gem::reconstruct_gem_wiring(new, core): rebuilds full revert-capable wiring records from the backend's own recognizable pair edit — the exact-pinpath:line / managed fence in the Gemfile, the grammar-strict PATH section, and thename (= version)!DEPENDENCIES pin (the same recognizers fix(gem): bundler vendored+hosted audit — 13 confirmed bugs fixed, tests hardened #175's re-vendor-new-uuid path trusts). Grammar-strict and fail-closed: any shape vendor does not write yieldsErr, never a guessed record. For an exact-pin project the reconstructed ledger entry is byte-identical to the original (pinned e2e assertion onstate.json).~> 3.1) is unrecoverable; the reconstructed original is the canonical exact pin of the locked version — a consistent, installable pair resolving to the same version (pinned).sha256=token is unrecoverable offline; no checksum record is emitted, revert leaves bundler's bare path-form entry, and repair surfacesvendor_checksum_unrecoverable. Empirically verified on bundler 4.0.15: a plainbundle installrefills the line byte-identically; frozen installs fail with a self-explanatoryempty/missing CHECKSUMS entrymessage until then (both bare and deleted behave identically, so nothing is deleted).repair_vendor.rsreconstruction seam (reconstruct_entry_wiring): routes per ecosystem. gem gets real wiring; every other ecosystem keeps empty wiring — npm reconstruction populates nothing either (checked, same hole), because its pre-vendor originals are registry integrity strings (sha512-…SRI etc.) no offline source can reproduce — and the gap now surfaces loudly as avendor_wiring_unknownwarning instead of silently.revert_gemempty-wiring guard: reverting a gem entry with no wiring records now fails loudly (vendor_wiring_unknown+ the manual cleanup steps, files and artifact untouched) instead of silently deleting the artifact.Out of owned scope (recorded, not changed): the npm-family (and pypi/cargo/…) revert of an empty-wiring reconstructed entry still "succeeds" silently, leaving the lock's
file:.socket/vendor/…resolution pointing at the deleted artifact (pinned as current behavior by the pre-existing e2e test 7). The shared guard belongs indispatch_revert_one(crates/socket-patch-cli/src/commands/vendor.rs) — outside this PR's owned files. Follow-up recommended; the reconstruction-timevendor_wiring_unknownwarning added here at least fronts the hazard for every ecosystem.2. Dir-shaped artifacts verified only their patched members
check_vendored_artifact's whole-file drift cross-check only covered file-shaped artifacts (.tgz/.whl/…). A vendored gem directory was verified purely byrecord.filesafterHashes, so tampered/drifted unpatched files (or the stub gemspec) were blessed Healthy byrepairand attested by VEX.state.rs):VendorArtifact.fileInventory— relative forward-slashed path → plain sha256 hex,BTreeMap(sorted on the wire), camelCase,skip_serializing_ifnone. Documented where the artifact schema is documented. Backward-tolerant: entries without it keep member-only verification.vendor_gem(stub gemspec included);repair's ledger restore / rebuild paths fill it viafill_artifact_fingerprint.verify_vendored_patch_record's dir arm (so both repair and VEX get it): missing, extra, and modified files all fail with the newvendor_inventory_mismatchtag →Corrupt→ repair rebuilds; VEX refuses to attest while tampered. Planted symlinks/FIFOs fail the walk (vendor_artifact_unreadable) — never hashed through.vendor_inventory_missing).vendor_inventory_missingwarning duringrepair. Follow-up: record inventories in those backends too.3. Multi-source Gemfile.locks resolved every spec against the FIRST remote
inventory_gemfile_lockcaptured the firstremote:across all GEM sections and built every spec's download URL from it, so specs under a second GEM source (private gem server) got a wrong URL — a 404 at best, a private gem name leaked to the public registry at worst.source … doblock produces one GEM section per source, oneremote:each; multiple global sources hard-error (This Gemfile contains multiple global sources…), so a single section with severalremote:lines can only be a legacy bundler 1.x multisource lock.resolved: None; the fetch layer then refuses with "no download URL") — fail-closed, never guessed.gem_remote_base(ledger recovery) had the same first-remote assumption, and worse: a vendored gem's spec block has moved into the PATH section, so its origin section is unrecoverable. Recovery (gem_remotes) now proceeds only when the lock's GEM sources agree on a single distinct remote; multiple distinct remotes refuse with "multiple GEM sources … ambiguous". No lock / no remote keeps the pre-existing rubygems.org default.Which test pins which change (all RED on the base commit, verified by run)
repair_reconstructs_gem_wiring_and_revert_byte_restores— reconstructedstate.jsonbyte-equal to the original; revert byte-restores Gemfile+lock, artifact gone (base:"wiring": [], revert exit 0 with stranded pair)revert_of_empty_wiring_gem_entry_fails_loudly— exit 1,vendor_wiring_unknownnamed, artifact + files untouched (base: exit 0, artifact deleted)repair_reconstruction_flags_unrecoverable_gem_checksum—vendor_checksum_unrecoverablesurfaced; revert leaves exactly the bare CHECKSUMS line (base: no event)repair_gem_dir_tamper_matrix_and_vex_refusal— tampered/deleted/extra file each → VEX refusal (vendor_inventory_mismatch) + repair rebuild; heals byte-exact (base:fileInventorynull, everything attested)repair_warns_on_legacy_gem_entry_without_inventory— no rebuild +vendor_inventory_missing(base: fails at fixture, no inventory exists)gem::tests::reconstruction_reproduces_vendor_wiring_for_pinned_declaration/_for_managed_block— reconstruction == vendor's own records, revert byte-restoresgem::tests::reconstruction_degrades_range_constraint_to_exact_pin,_preserves_trailing_options,_flags_unrecoverable_checksum,_refuses_foreign_or_mismatched_wiringgem::tests::revert_refuses_empty_wiring_entry(RED on base: revert succeeded)gem::tests::vendor_records_dir_file_inventory— hand-pinned sha256sverify::tests::dir_inventory_detects_unpatched_tamper_missing_and_extra_files,dir_inventory_refuses_planted_symlinkstate::tests::file_inventory_round_trips_sorted_camel_caselock_inventory::tests::gemfile_lock_multi_source_resolves_each_spec_against_its_own_remote(RED on base: second spec inherited the first remote)lock_inventory::tests::gemfile_lock_legacy_multi_remote_section_is_discovery_only(RED on base: guessed first remote)lock_inventory::recover_tests::gem_recovery_refuses_ambiguous_multi_source_lock(RED on base: guessed)Verification
repair_vendor_e2e: 20/20 (15 pre-existing npm rows untouched + 5 new gem rows)e2e_vex_vendor8,in_process_vendor27,repair_invariants20,repair_vendor_flavors_e2e28,scan_vendor_e2e18,setup_contract_gaps10,e2e_gem8,in_process_gem_apply7,in_process_gem_multi_platform4,in_process_scan24,ecosystem_dispatch_e2e18,in_process_rollback_all_ecosystems8docker_e2e_gem2/2,docker_e2e_vendor_gem1/1 (fresh image)e2e_vendor_gem_buildcapstone green under the real host bundler 4.0.15 (vendor → VEX → fresh-checkout frozen install → revert byte-restore)cargo fmt --checkclean;cargo clippy --all-targetswarning-clean on both cratesReviewer notes
VendorArtifactschema addition forced a mechanicalfile_inventory: None,line in every literal constructor (~34 sites across backends/tests, outside the owned file list but compiler-forced); each is a single line, nothing functional.vendor/mod.rs(artifact_is_file_shaped,compute_dir_inventory) —verifyispub(crate), the CLI needs the symbols.revert_gem's guard andvendor_gem's inventory recording go slightly beyond "export recognizers only" forgem.rs; both are additive, directly serve the mission items, and are covered by the equivalence/guard tests. The alternative placements (dispatch_revert_one / persist_vendor_entry) are outside the owned files.🤖 Generated with Claude Code
Note
Medium Risk
Touches vendor ledger schema, repair/revert paths, and lockfile URL resolution—high test coverage but behavior changes affect gem repair and multi-source locks.
Overview
Closes three vendored-integrity gaps: ledger repair that could not restore gem wiring (revert stranded
path:edits), dir-shaped artifacts that only checked patched files, and Gemfile.lock inventory that used the first GEMremote:for every spec.Repair + gem revert.
repairnow routes per-ecosystem wiring reconstruction: gems callreconstruct_gem_wiringto rebuild revert-capableGemfile/Gemfile.lockrecords from vendor-shaped live files (fail-closed on foreign shapes). Other ecosystems still get empty wiring with an explicitvendor_wiring_unknownwarning.revert_gemrefuses entries with empty wiring instead of deleting the artifact and leaving brokenpath:wiring.Dir artifact integrity. Ledger entries add optional
fileInventory(whole-tree path → sha256).vendor_gemrecords it; verify/repair/VEX compare the full directory (tamper on unpatched files →vendor_inventory_mismatch). Legacy entries without inventory keep member-only checks plusvendor_inventory_missingwarnings.Gem lock remotes.
inventory_gemfile_lockresolves each spec against its own GEM section’sremote:; ambiguous multi-remote sections stay discovery-only. Ledger recovery refuses guessed downloads when multiple distinct GEM sources exist.Reviewed by Cursor Bugbot for commit 0f24244. Configure here.