Repository navigation
Require sbt resolution evidence before attesting vendored pins - #2008
Mikola Lysenko (mikolalysenko) wants to merge 3 commits into
Conversation
A vendored sbt pin attested through its vendor ledger entry whenever the generated socket-patch-vendor.sbt still held its row: discovery's vendored_check diagnosed sbt_resolution_unverified / vendored_tree_missing, but nothing consumed them, so vex attested a pin with no or stale sbt evidence, a missing tree file, or a dependencyOverrides := reassignment that replaces the generated override. vendored_check now also marks the pin's (purl, uuid) Unattested (new kinds SbtResolutionUnverified / SbtVendoredTreeMissing), using the exact check and build-wide blockers the hosted ref's lock pin uses. The VEX plan omits the ledger entry as vex_sbt_resolution_unverified / vex_sbt_vendored_tree_missing. Liveness is untouched, so vendor --check and scan --prune keep treating the intact wiring as live. Planners are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
bugbot run Generated by Claude Code |
|
Labeled Ready for review (burn-down agent).
Generated by Claude Code |
- The VEX plan also checks an Unattested mark against the vendor entry a Vendored basis actually attests (its uuid can differ from the candidate's when the manifest files the key under another uuid). - Every sbt_resolution_unverified cause names its own remedy (or that it has none); the vex note's remedy no longer repeats `sbt update`. - A memory view has no committed tree: no vendored_tree_missing there. - One text per finding feeds both the diagnostic and the mark. - Shared sbt evidence test helpers; tests use the raw tempdir path (a canonical Windows path is verbatim, which sbt never records). - e2e cell f asserts the dependencyOverrides detail with a stable mtime; new cell g covers a manifest present under the same or another uuid. - Doc comments and CLI_CONTRACT / ecosystems.md claims corrected. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
bugbot run Generated by Claude Code |
There was a problem hiding this comment.
✅ 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 02e9bb5. Configure here.
|
Labeled Ready for review again (burn-down agent).
Generated by Claude Code |
Final review brief (for the human approver)What it does: discovery now marks a vendored sbt pin ( Risk: low to medium. Every change makes attestation stricter, so a mistake loses a Look here
Verified
Changes I made: none. Open questions (not blocking)
Auto-merge is not armed yet because Generated by Claude Code |
Second review round on vendored sbt evidence. - The hosted-over-vendored takeover swaps in the redirect ledger's uuid after the early `unattested` gate ran, and only the vendored basis was re-checked afterwards, so a mark on the taken-over uuid was attested. The post-basis check now covers the takeover too. - The overrides / resolvers remedy told every match to "append with `+=` instead", though the scan also flags `~=` and `--=`, and on `externalResolvers` / `fullResolvers`. The detail now names the operators and the settings and says to keep the existing entries. - A memory view's vendored pin said "run `sbt update`", which cannot help there; the detail now says the view holds no evidence. - The tree-missing detail names `socket-patch-vendor.sbt` again. - The CLI's fixed `SbtResolutionUnverified` suffix no longer promises that resolution records fix every cause (some builds are never attested); it defers to the detail's remedy. - The overrides test dates every build source before the evidence, not only build.sbt. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Problem
A vendored sbt pin was attested through its vendor ledger entry as long as
socket-patch-vendor.sbtstill held its row (vendor::jvm::sbt::wired_checked). Discovery'svendored_check(vex/discover/sbt.rs) already foundsbt_resolution_unverifiedandvendored_tree_missing, but only as diagnostics. Nothing used them: JVM ledger entries skipvendored_claim, sovexissuednot_affectedfor:sbt updatesince vendoring, or with stale or partial evidencedependencyOverrides :=, which replaces the generated override, along withresolvers :=andbuild.sbt.lockHosted sbt already treats these cases as unverified. Under the owner rule, hosted's stricter behavior is the ground truth.
Fix
vendored_checknow also marks the pin's(purl, uuid)asUnattested, using two newUnattestedKinds:SbtResolutionUnverifiedandSbtVendoredTreeMissing. The verdict comes from the sameverify_with+evidence_blockercode that decides whether a hosted sbt ref gets its lock pin, so hosted and vendored go through one evidence route.vex_sbt_resolution_unverifiedorvex_sbt_vendored_tree_missing, as a run warning and asfailed[].reason. A mark is checked both against the candidate's uuid and against the vendor entry that aVendoredbasis actually attests.sbt_resolution_unverifiednames its own remedy, or says it has none. For example, a build whose projects can't be read statically is never verifiable, and a version conflict needs whatever forces the other version removed. The note's own remedy no longer repeatssbt update.vendor --check,scan --pruneand the takeover classifier still see the intact wiring as live.I didn't route this through
ResolutionDoc+gate::check_new. That gate asks whether a new pin may be written against the base version, and for an existing pin it only gives advisories. It doesn't ask whether the evidence shows the pinned bytes. Discovery'sverify_withasks that question, and it already runs the hostedcheck_newblockers throughevidence_blocker.Tests
Each new behavior test fails with the fix disabled; I checked by disabling only the new
unattestedpush:vex::discover::sbt::tests(core): gson is vendored for real through the JVM planner, with syntheticupdate_cacheevidence.vendored_pin_without_evidence_is_unattestedvendored_pin_with_stale_evidence_is_unattestedvendored_pin_under_an_overrides_reassignment_is_unattestedvendored_pin_with_a_missing_tree_file_is_unattested: also checks that the diagnostic and the mark carry the same text.vendored_pin_over_a_memory_view_reports_no_missing_treevendored_pin_with_verifying_evidence_is_attestable: the control case.e2e_vex_lockfile::sbt_vendored, hermetic, all OSes:sbt update: not attested, andvex_sbt_resolution_unverifiedis reported.ThisBuild / dependencyOverrides :=aftersbt update: the warning detail namesdependencyOverrides. The edit keeps its old mtime, so staleness can't stand in for the reassignment on a slow run.vex_sbt_resolution_unverifiedis reported.Commands run:
cargo test -p socket-patch-core --lib -- vendor::jvm:: vex::discover:: formats::sbt crawlers::sbt_evidence: 767 passed.vex::discover::testing::golden::tests::committed_fixture_corpus_matches_golden: passes with no golden changes. The corpus has no sbt fixture.cargo test -p socket-patch-cli --test e2e_vex_lockfile -- sbt: 10 passed.cargo test -p socket-patch-cli --lib -- vex_sources: 14 passed.cargo clippy --locked --workspace --all-features -- -D warningsandcargo fmt --all -- --check: clean.Docs:
CLI_CONTRACT.mdgets the two new omission codes with per-cause remedies, and thesbt_resolution_unverified/vendored_tree_missingrows now say what happens to a vendored pin.docs/ecosystems.mdgets a note on vendored sbt VEX.Review findings (
/code-review high)False attestation through a manifest-owned candidate: refuted as a live bug, hardened anyway. With the manifest recording gson under U1 and the ledger plus
socket-patch-vendor.sbtunder U2, the retain gate does miss the mark. Verification then fails withvendor_uuid_mismatch:checked_tree_jarrequiresentry.uuid == record.uuid, so nothing is attested. I confirmed this by running new e2e cell g with the new gate disabled. With the manifest under the same uuid, the existing gate already catches the mark. Even so, the plan now also checks a mark against the entry that theVendoredbasis attests. Cell g assertsvex_sbt_resolution_unverifiedfor both uuids, and fails without that check.Parity with hosted's installed-copy fallback: kept stricter, deliberately. When a hosted sbt pin fails
verify_with, it loses the lockfile basis but can still attest fromvex_consumed::maven_copies: a copy of the suffixed version in.socket/sbt-hosted/maven2,~/.m2or a Gradle cache. The hosted copy in.socket/sbt-hostedis written by sbt itself: the generated file downloads it at load time, after checking its sha256. That copy is therefore evidence that sbt loaded the pin. For a vendored pin, the equivalent of that copy is the committed.socket/vendor/maven2tree, whichsocket-patch vendorwrites before sbt has ever run. Hashing the tree is exactly the check (Basis::Vendored) that produced the false attestation. In the hosted model the tree plays the role of the lock pin, not of an installed copy. sbt keeps no copy of its own outside the tree: Coursier readsfile:repositories in place, and Ivy serves them from their origin (docs/design/sbt-template-probe.mdcase i). That leaves no copy that sbt consumed for vendored to fall back on. I didn't touch hosted, though its fallback also attests a downloaded copy under adependencyOverrides :=reassignment; that may be worth a separate look.Remedies: fixed. Every
verify/evidence_blockermessage now carries an accurate remedy:project/Project("<id>", file("<dir>"))builds can be verified:=reassignment: append with+=insteadbuild.sbt.lock: can't be verified while the lock existssbt update, and if it persists, remove what forces that versionsbt updateThese messages are shared with the hosted diagnostic.
CLI_CONTRACT.mdanddocs/ecosystems.mdare corrected to match.Doc comments: fixed.
Discovery::unattested(the field and the function) now covers marks with no ref. Consumers audited: the VEX plan matches marks by(purl, uuid), the golden render lists them, andunattest_unwired_copiesbuilds marks only from refs. None of them assumes a ref exists.Cell f: fixed. It asserts the warning detail names
`dependencyOverrides`, andbuild.sbtkeeps its original mtime after the edit.Windows verbatim paths: fixed in the tests. The core test uses the raw tempdir path. Production is unaffected:
location_pathparses the URIs sbt writes, and the JVM never emits\\?\. The e2e helper already strips\\?\.Duplicate
sbt updatein the note: fixed. The note's remedy no longer names it.Memory view: fixed. With no disk root, the tree check is skipped (
vendored_pin_over_a_memory_view_reports_no_missing_tree). The resolution check still runs and leaves the pin unverified, for the reason given above. No memory-view consumer readsunattested.Duplicated helpers: fixed.
file_uriandset_mtimelive incrawlers::sbt_evidence::testing(#[cfg(test)]), shared by both test modules.One text for the diagnostic and the mark: fixed. The diagnostic for a tree file is now
socket-patch-vendor.sbt: patch <uuid>: <rel> is missing|modified.Review findings, round 2 (
/code-review highon02e9bb5, cloud routine)unattestedgate has run, and the post-basis re-check covered only the vendored basis. A mark on the taken-over uuid was therefore attested. The post-basis check now covers the takeover as well. Test:dead_vendor_claim_falls_through_to_a_live_hosted_claim(extended). It fails with the new check disabled.~=and--=: fixed. The detail now names the operators and the settings involved (resolvers,externalResolvers,fullResolvers) and says to keep the existing entries.docs/ecosystems.mdandCLI_CONTRACT.mdmatch.SbtResolutionUnverifiedsuffix contradicted never-attested builds: fixed. It now defers to the detail's remedy.sbt update": fixed. The detail now says the view holds no resolution evidence. Test:vendored_pin_over_a_memory_view_reports_no_missing_tree(extended).socket-patch-vendor.sbtpins , which is missing". Test:vendored_pin_with_a_missing_tree_file_is_unattested.Declined: the duplicated
set_mtimeandwarning_detailstest helpers. That's cleanup only.Tests (cloud run):
cargo test -p socket-patch-core --lib -- vex::discover:: crawlers::sbt crawlers::scala patch::redirect::sbtgives 486 passed, golden included. CLI--lib vex_sourcesand--test e2e_vex_lockfile sbtpass. clippy and fmt are clean.Landing note: #2030 conflicts with this PR in
vex/discover/sbt.rsandUnattestedKind. It will be brought current once this lands.Note
Medium Risk
Changes when manifest-less VEX attests vendored sbt (compliance output), but only tightens false positives; behavior is heavily tested and does not alter patch wiring liveness.
Overview
Vendored sbt pins no longer get
not_affectedVEX statements from the vendor ledger alone. Discovery now marks the pin’s(purl, uuid)asUnattestedwhen localtarget/evidence fails the sameverify_with/evidence_blockerchecks hosted sbt already uses—missing or stale evidence, build-wide blockers (dependencyOverrides/resolvers:=,build.sbt.lock, non-static projects), wrong version or bytes, or a missing/modified.socket/vendor/maven2tree. Diagnostics (sbt_resolution_unverified,vendored_tree_missing) still fire; they now drive omission via new kindsSbtResolutionUnverifiedandSbtVendoredTreeMissing.The VEX plan emits
vex_sbt_resolution_unverified/vex_sbt_vendored_tree_missing(warnings andfailed[].reason), refactors unattested gating into shared helpers, and checks marks against the vendor entry uuid on aVendoredbasis (not only the candidate uuid). Wiring liveness is unchanged (vendor --check,scanstill treat pins as live). Blocker and verify messages now include actionable remedies; docs inCLI_CONTRACT.mdanddocs/ecosystems.mdmatch. Memory views skip tree hashing but still withhold on unverified resolution.Tests cover core discover scenarios, expanded e2e cells (pre-
sbt update, overrides, manifest uuid mismatch), and sharedsbt_evidence::testinghelpers.Reviewed by Cursor Bugbot for commit 02e9bb5. Configure here.
Generated by Claude Code