Skip to content

Require sbt resolution evidence before attesting vendored pins - #2008

Open
Mikola Lysenko (mikolalysenko) wants to merge 3 commits into
mainfrom
fix/vendored-sbt-evidence
Open

Mikola Lysenko (mikolalysenko) wants to merge 3 commits into
mainfrom
fix/vendored-sbt-evidence

Conversation

@mikolalysenko

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

Copy link
Copy Markdown
Collaborator

Problem

A vendored sbt pin was attested through its vendor ledger entry as long as socket-patch-vendor.sbt still held its row (vendor::jvm::sbt::wired_checked). Discovery's vendored_check (vex/discover/sbt.rs) already found sbt_resolution_unverified and vendored_tree_missing, but only as diagnostics. Nothing used them: JVM ledger entries skip vendored_claim, so vex issued not_affected for:

  • a pin with no sbt update since vendoring, or with stale or partial evidence
  • a pin with a tree file missing or modified
  • a pin whose build reassigns dependencyOverrides :=, which replaces the generated override, along with resolvers := and build.sbt.lock

Hosted sbt already treats these cases as unverified. Under the owner rule, hosted's stricter behavior is the ground truth.

Fix

vendored_check now also marks the pin's (purl, uuid) as Unattested, using two new UnattestedKinds: SbtResolutionUnverified and SbtVendoredTreeMissing. The verdict comes from the same verify_with + evidence_blocker code that decides whether a hosted sbt ref gets its lock pin, so hosted and vendored go through one evidence route.

  • The VEX plan omits the ledger entry and reports one of two new codes, vex_sbt_resolution_unverified or vex_sbt_vendored_tree_missing, as a run warning and as failed[].reason. A mark is checked both against the candidate's uuid and against the vendor entry that a Vendored basis actually attests.
  • Each cause of sbt_resolution_unverified names 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 repeats sbt update.
  • Liveness does not change, so vendor --check, scan --prune and the takeover classifier still see the intact wiring as live.
  • A memory view has no committed tree to hash, so it reports no missing tree. Its pins are still unverified, just as a hosted pin read from memory gets no lock pin.
  • The planners are unchanged.

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's verify_with asks that question, and it already runs the hosted check_new blockers through evidence_blocker.

Tests

Each new behavior test fails with the fix disabled; I checked by disabling only the new unattested push:

  • vex::discover::sbt::tests (core): gson is vendored for real through the JVM planner, with synthetic update_cache evidence.
    • vendored_pin_without_evidence_is_unattested
    • vendored_pin_with_stale_evidence_is_unattested
    • vendored_pin_under_an_overrides_reassignment_is_unattested
    • vendored_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_tree
    • vendored_pin_with_verifying_evidence_is_attestable: the control case.
  • e2e_vex_lockfile::sbt_vendored, hermetic, all OSes:
    • cell b, before sbt update: not attested, and vex_sbt_resolution_unverified is reported.
    • cell f, ThisBuild / dependencyOverrides := after sbt update: the warning detail names dependencyOverrides. The edit keeps its old mtime, so staleness can't stand in for the reassignment on a slow run.
    • cell g, the manifest restored under the vendored uuid or under another uuid: not attested, and vex_sbt_resolution_unverified is 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 warnings and cargo fmt --all -- --check: clean.

Docs: CLI_CONTRACT.md gets the two new omission codes with per-cause remedies, and the sbt_resolution_unverified / vendored_tree_missing rows now say what happens to a vendored pin. docs/ecosystems.md gets a note on vendored sbt VEX.

Review findings (/code-review high)

  1. 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.sbt under U2, the retain gate does miss the mark. Verification then fails with vendor_uuid_mismatch: checked_tree_jar requires entry.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 the Vendored basis attests. Cell g asserts vex_sbt_resolution_unverified for both uuids, and fails without that check.

  2. 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 from vex_consumed::maven_copies: a copy of the suffixed version in .socket/sbt-hosted/maven2, ~/.m2 or a Gradle cache. The hosted copy in .socket/sbt-hosted is 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/maven2 tree, which socket-patch vendor writes 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 reads file: repositories in place, and Ivy serves them from their origin (docs/design/sbt-template-probe.md case 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 a dependencyOverrides := reassignment; that may be worth a separate look.

  3. Remedies: fixed. Every verify / evidence_blocker message now carries an accurate remedy:

    • project definitions that can't be read statically: only literal project / Project("<id>", file("<dir>")) builds can be verified
    • := reassignment: append with += instead
    • build.sbt.lock: can't be verified while the lock exists
    • another version: run sbt update, and if it persists, remove what forces that version
    • other bytes: remove or restore the copy, then run sbt update

    These messages are shared with the hosted diagnostic. CLI_CONTRACT.md and docs/ecosystems.md are corrected to match.

  4. 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, and unattest_unwired_copies builds marks only from refs. None of them assumes a ref exists.

  5. Cell f: fixed. It asserts the warning detail names `dependencyOverrides`, and build.sbt keeps its original mtime after the edit.

  6. Windows verbatim paths: fixed in the tests. The core test uses the raw tempdir path. Production is unaffected: location_path parses the URIs sbt writes, and the JVM never emits \\?\. The e2e helper already strips \\?\.

  7. Duplicate sbt update in the note: fixed. The note's remedy no longer names it.

  8. 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 reads unattested.

  9. Duplicated helpers: fixed. file_uri and set_mtime live in crawlers::sbt_evidence::testing (#[cfg(test)]), shared by both test modules.

  10. 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 high on 02e9bb5, cloud routine)

  1. The hosted-over-vendored takeover skipped the mark gate: confirmed and fixed. The takeover swaps in the redirect ledger's uuid after the early unattested gate 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.
  2. The overrides/resolvers remedy was wrong for ~= and --=: fixed. The detail now names the operators and the settings involved (resolvers, externalResolvers, fullResolvers) and says to keep the existing entries. docs/ecosystems.md and CLI_CONTRACT.md match.
  3. The CLI's fixed SbtResolutionUnverified suffix contradicted never-attested builds: fixed. It now defers to the detail's remedy.
  4. A memory view said "run 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).
  5. The tree-missing detail lost the generated file's name: fixed. It reads "socket-patch-vendor.sbt pins , which is missing". Test: vendored_pin_with_a_missing_tree_file_is_unattested.
  6. Test timing: fixed. Every build source is now dated before the evidence.

Declined: the duplicated set_mtime and warning_details test helpers. That's cleanup only.

Tests (cloud run): cargo test -p socket-patch-core --lib -- vex::discover:: crawlers::sbt crawlers::scala patch::redirect::sbt gives 486 passed, golden included. CLI --lib vex_sources and --test e2e_vex_lockfile sbt pass. clippy and fmt are clean.

Landing note: #2030 conflicts with this PR in vex/discover/sbt.rs and UnattestedKind. 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_affected VEX statements from the vendor ledger alone. Discovery now marks the pin’s (purl, uuid) as Unattested when local target/ evidence fails the same verify_with / evidence_blocker checks 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/maven2 tree. Diagnostics (sbt_resolution_unverified, vendored_tree_missing) still fire; they now drive omission via new kinds SbtResolutionUnverified and SbtVendoredTreeMissing.

The VEX plan emits vex_sbt_resolution_unverified / vex_sbt_vendored_tree_missing (warnings and failed[].reason), refactors unattested gating into shared helpers, and checks marks against the vendor entry uuid on a Vendored basis (not only the candidate uuid). Wiring liveness is unchanged (vendor --check, scan still treat pins as live). Blocker and verify messages now include actionable remedies; docs in CLI_CONTRACT.md and docs/ecosystems.md match. 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 shared sbt_evidence::testing helpers.

Reviewed by Cursor Bugbot for commit 02e9bb5. Configure here.


Generated by Claude Code

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>
@mikolalysenko
Mikola Lysenko (mikolalysenko) marked this pull request as ready for review November 5, 2026 16:27
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

bugbot run


Generated by Claude Code

@mikolalysenko Mikola Lysenko (mikolalysenko) added the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label Nov 5, 2026
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Labeled Ready for review (burn-down agent).

  • Head: cd2959b54675644b306a6c8c235ab56c94d14b1f
  • CI: 80/82 checks green; the only red is the e2e_vex_build pdm leg (+ its ci-ok rollup), which is main-wide: hishel 2.0.0 (released 2026-11-05) breaks pdm lock on every PR. Fix pdm e2e red on every PR: pin hishel<2 #2028 fixes it; merge main in after it lands.
  • Bugbot: reviewed this head, no issues found.
  • Mergeable, no CHANGELOG.md change.
  • Slack announcement not sent this run (no Slack send tool available); the next run will retry.

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>
@mikolalysenko Mikola Lysenko (mikolalysenko) removed the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label Nov 5, 2026
@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 02e9bb5. Configure here.

@mikolalysenko Mikola Lysenko (mikolalysenko) added the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label Nov 5, 2026
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Labeled Ready for review again (burn-down agent).

  • Head: 02e9bb59d7c008f2ac967e536122337276ed4cb3
  • CI: 47/49 checks green. The only red check is the ubuntu e2e_vex_build pdm leg (+ its ci-ok rollup), and it is red on every PR: hishel 2.0.0 breaks pdm lock (module 'hishel' has no attribute 'SyncSqliteStorage'). Fix pdm e2e red on every PR: pin hishel<2 #2028 fixes it.
  • Bugbot reviewed this head and found no issues.
  • Mergeable, no CHANGELOG.md change.
  • Slack announcement not sent: this run had no Slack send tool. The next run will retry.

Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Final review brief (for the human approver)

What it does: discovery now marks a vendored sbt pin (socket-patch-vendor.sbt) as unattested when its vendored tree is missing or modified, or when sbt resolution evidence doesn't show the patched version. Before, those cases only produced a diagnostic. The VEX plan turns the marks into omissions with two new codes (vex_sbt_resolution_unverified, vex_sbt_vendored_tree_missing), and every blocker message now says how to fix it.

Risk: low to medium. Every change makes attestation stricter, so a mistake loses a not_affected rather than adding a false one. The visible change: a vendored sbt pin is not attested until sbt update has run since vendoring (a fresh CI checkout with no target/, or vendor --vex right after vendoring, will omit it). The docs describe this.

Look here

  • sbt.rs:342: vendored_check, the new unattested marks and the shared withhold text
  • vex_sources.rs:588: new gate on the Vendored basis by entry uuid. It applies to every ecosystem's vendored basis, not just sbt (fail-closed)
  • vex_sources.rs:295: unattested_mark/gate_unattested refactor
  • sbt.rs:111: evidence_blocker remedy text (shared with the hosted diagnostic)
  • crates/socket-patch-cli/CLI_CONTRACT.md: rows for the new codes

Verified

  • Read the full diff (8 files, +513/-87). CHANGELOG.md untouched; no debug leftovers.
  • cargo test -p socket-patch-core --lib -- vex::discover::sbt crawlers::sbt_evidence: 22 passed. cargo test -p socket-patch-cli --test e2e_vex_lockfile -- sbt: 10 passed.
  • Mutation checks: dropping the unattested push fails 5 core tests and e2e cells b, f, g. Dropping the new Vendored gate fails cell g. So the tests do cover the change.
  • Only the VEX plan reads Discovery::unattested, so marks with no ref break no other consumer.
  • CI: clippy green. ci-ok is red only because of the pdm e2e_vex_build job (pdm lock fails since hishel 2.0 came out). Fix pdm e2e red on every PR: pin hishel<2 #2028 fixes that, and the same job is red on every open PR. Once Fix pdm e2e red on every PR: pin hishel<2 #2028 merges and this branch picks up main, CI should go green. No open Bugbot threads.

Changes I made: none.

Open questions (not blocking)

  • Is it intended that the Vendored gate at vex_sources.rs:588 now covers npm, Gradle and Maven vendor entries too? It is fail-closed, and no current mark kind hits it.
  • Nit: e2e cell c (sbt_vendored_vex_flags_a_tampered_tree) still passes with the mark disabled, because the pin is omitted for another reason. Asserting vex_sbt_vendored_tree_missing there would pin down the new code.
  • Follow-up idea: hosted sbt's maven_copies fallback can still attest under a dependencyOverrides := reassignment (finding 2 in the PR body). This is now the laxer path.

Auto-merge is not armed yet because ci-ok is red. The merge steward arms it once CI is green on this head, so approving then sends it straight to the merge queue.


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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants