Skip to content

Stop vendored Gradle liveness attesting a version the lockfile overrides - #2004

Open
Mikola Lysenko (mikolalysenko) wants to merge 5 commits into
mainfrom
fix/vendored-gradle-lock-liveness
Open

Mikola Lysenko (mikolalysenko) wants to merge 5 commits into
mainfrom
fix/vendored-gradle-lock-liveness

Conversation

@mikolalysenko

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

Copy link
Copy Markdown
Collaborator

Problem

Vendored Gradle keeps the original GAV, routes it to the committed repository with exclusiveContent, and leaves lock files unchanged. Neither the vendored planner nor its VEX liveness (vendor::jvm::gradle::wired / wired_checked, which re-plan and require no degraded warning) read any lock file. If gradle.lockfile (or another lock of the build) pins the patched GA at a different version, Gradle resolves that locked version (or fails lock validation), yet vendored liveness still attested the patch. Hosted discovery already checks every lock (vex/discover/gradle.rs).

Fix

The planner in vendor/jvm/gradle.rs now runs check_locks after check_graph. It reads the build's lock inventory, which is now shared with hosted discovery: ScriptGraph::read_locks, covering gradle.lockfile, buildscript-gradle.lockfile, settings-gradle.lockfile and the legacy gradle/dependency-locks/*.lockfile of every project of every build. It also reads every custom lockFile the graph can follow (ScriptGraph::custom_lock_files).

Each problem below adds a vendor_jvm_degraded warning. wired requires a re-plan with no degraded warning, so each one also withholds VEX attestation. The patch is still applied.

Reason When
gradle_lock_overrides_vendored An entry for the GA is at a version no vendored row serves (neither this patch's version nor another vendored version of the GA).
gradle_settings_lock_unrouted A settings-gradle.lockfile entry exists, but that build's settings script does not hold the in-block exclusiveContent entry for the GAV in every block that needs one (buildscript, and pluginManagement when there are settings plugins{}).
gradle_lock_unreadable A listed lock file, or a followed custom one, can't be read as UTF-8 text: I/O error, refused or outside the checkout, missing, or not UTF-8.
gradle_custom_lock_file A lockFile location can't be followed statically. Only a literal file('…') or "$projectDir/…" in a project's own build script, outside allprojects / subprojects / configure / project(…) blocks and inside the checkout, can be followed.

How hosted's rules map to vendored (also in the doc comment). Hosted pins a suffixed version: a lock below the base is invalid, and a lock above it is LockAboveBase (unattested). Vendored keeps the original GAV, so a lock entry is served only at a vendored version. Any other locked version, above or below, means the build doesn't consume the patch.

Hosted rejects every settings-classpath lock, because its script runs after that classpath resolves. Vendored instead routes that classpath with in-block entries, so it accepts a settings lock only when the settings script actually holds them:

  • Vendoring: the entry may already be committed, or written by this plan (Committed::OrPlanned).
  • Liveness: the entry must already be committed (Committed::Only); one a re-plan would write doesn't count.

Anything else is rejected, as hosted rejects it.

Docs: the reason codes are added to the vendored table in CLI_CONTRACT.md, with a paragraph in docs/design/maven-vendoring.md.

Liveness judges committed wiring, never a re-plan

wired re-plans over the committed tree but used to ignore the re-plan's writes. So wiring a re-plan would add again stood in for wiring missing from the checkout. For example, a settings classpath on the module whose in-block entry had been removed still attested, with or without a settings lock.

The liveness re-plan (Committed::Only) now reports gradle_wiring_uncommitted when it would still write any wiring file. This reason is liveness-only; vendor never reports it. It is documented in CLI_CONTRACT.md. Vendoring's own first run still counts the entries it is about to write.

Audit of wired / wired_checked: every place where a pending re-plan write masked missing committed wiring is now caught.

  • Settings in-block entries: buildscript, and pluginManagement for settings plugins{}, including a plugins{} block added after vendoring. This is the gap from the last round.
  • Apply lines of buildSrc and included builds: references checks only the root's apply line.
  • Index rows: references matches only GAV and uuid, so a row whose hash no longer matches the tree passed.
  • The derived maven-metadata.xml.
  • gradle/verification-metadata.xml: patched hashes missing.
  • The script: already covered by is_ours; it is counted too.

Not counted, on purpose:

  • Tree files: rebuilt from the committed bytes. The script checks their hashes against the index, and index writes do count.
  • Git hygiene files (.gitattributes, .gitignore): Gradle never reads them, so they don't change what this checkout resolves.

wired_checked adds only parse guards on top of wired. The Maven half's attestable_checked already requires edits.is_empty() from its declaration rewrite. A missing Maven repository tail makes the suffixed version unresolvable, so the build fails rather than using another copy. sbt and scala-cli liveness were not audited.

Test: liveness_judges_committed_wiring_not_a_replan. All 7 cases above were attested before this change (verified: all 7 were listed as masked), and none are now. A missing hygiene file still attests, and vendoring's first run reports no degraded warning. settings_lock_needs_the_committed_in_block_entry now also asserts that the unlocked, stripped case is not live.

Review findings (/code-review high)

  1. False attestation of a settings lock without the in-block entry: confirmed, fixed. See the table and mapping above. Test: settings_lock_needs_the_committed_in_block_entry. A settings lock is accepted with the committed entry. If the entry is stripped, liveness fails even though a re-plan would write it again. A settings script with no in-block entry is rejected (also covered in lock_at_another_version_withholds_liveness).
  2. Two vendored versions of one GA degraded each other: confirmed, fixed. A lock at a version the vendored index serves is no longer an override. Test: two_vendored_versions_of_a_ga_do_not_override_each_other.
  3. Degradation is build-wide: kept conservative, by design. A Gradle lock file belongs to one project and lists exactly the configurations that resolve each module, so only entries for the GA count; other locks are ignored. A project that locks the GA at another version (an unrelated tools/ test classpath included) does resolve the GA, just at that version, so that part of the build doesn't consume the patch. Hosted, which is ground truth, also withholds the whole build for one above-base lock. What can't be decided statically is whether Gradle reads a lock at all (whether that project enables dependencyLocking), so every lock of the build counts. The message now names the locking configurations.
  4. Unreadable locks skipped (fail open): confirmed, fixed. Paths come from the directory listing, so a listed lock that read can't return (I/O error, refused symlink outside the checkout, non-UTF-8) now fails closed. Test: unreadable_lock_files_fail_closed.
  5. Unreadable locks reported as an override: confirmed, fixed. They now get gradle_lock_unreadable with an accurate message, documented in CLI_CONTRACT.md.
  6. Every lockFile degraded every patch: confirmed, fixed. A followable custom lock is read and parsed like any lock. A missing one fails closed, since it was set explicitly. Only an unfollowable one is gradle_custom_lock_file. Tests: custom_lock_file_is_followed_when_static, plus the path-resolution cases in gradle::graph::tests::custom_lock_file_and_settings_classpath. Hosted keeps its blanket rejection of lockFile, because the hosted planner can't rewrite such a lock.
  7. Perf, locks re-read on every plan: refuted as a blocker, noted for follow-up. The lock pass reuses the ScriptGraph that plan already builds. Per call it lists two directories per project and reads each lock once. That is the same order as the existing per-call cost, since every plan call (one per patch, and one per entry in vex liveness) already re-collects and re-tokenizes the whole script graph. Lock files are small line-oriented text. A real per-root cache has to cover the graph and the locks together, and it needs a planning context threaded through plan, plan_with_external, wired, entry_wired_checked and the CLI vex loop. That is better done as its own change than bolted onto locks alone.
  8. Duplicated lock gathering: confirmed, fixed. ScriptGraph::read_locks is now the single inventory. Hosted discovery and the vendored check both use it, ScriptGraph::custom_lock_file is built on custom_lock_files, and is_settings_lock moved to gradle::locks (re-exported from patch::redirect::gradle). The hosted planner's lock rewrite still lists files through the same lockfile_paths because it needs the raw text to edit.

Tests

  • All 5 lock tests in vendor::jvm::gradle::tests fail against the previous commit's check_locks, verified by splicing the old function back in. They pass with this change:
    • lock_at_another_version_withholds_liveness
    • settings_lock_needs_the_committed_in_block_entry
    • two_vendored_versions_of_a_ga_do_not_override_each_other
    • unreadable_lock_files_fail_closed
    • custom_lock_file_is_followed_when_static
  • lock_at_another_version_withholds_liveness also fails on origin/main.
  • liveness_judges_committed_wiring_not_a_replan fails before the committed-wiring change (all 7 cases masked) and passes after it.
  • cargo test -p socket-patch-core --lib -- vendor::jvm:: vex::discover:: gradle::: 776 passed.
  • cargo test -p socket-patch-core --lib -- vex::discover::testing::golden::tests::committed_fixture_corpus_matches_golden vendor:: patch::redirect::gradle: 2893 passed. No golden changes.
  • cargo test -p socket-patch-cli --test contract_gradle_codes --test vendor_jvm_cli --test gradle_agent_cli: 1 + 54 + 54 passed. Every new reason code is documented, and end-to-end vex on vendored JVM entries is unchanged.
  • cargo clippy --locked --workspace --all-features -- -D warnings and cargo fmt --all -- --check are clean.

Not done

  • No planner refusal. Degrading is the smaller, clearly-correct change. A refusal when every governing lock pins another version could follow later, alongside gradle_range_excludes_vendored.
  • No lock rewriting. Vendored keeps the original GAV, so a lock at the vendored version needs no edit.
  • Per-root planning cache. See finding 7.
  • sbt / scala-cli liveness were not audited for the committed-wiring rule.

Review findings, round 2 (/code-review high on 4cff8ec..6932ced, cloud routine)

  1. FALSE ATTESTATION: configure(...) and the per-project hooks weren't foreign blocks. Confirmed and fixed. all_blocks only matches name {, but Gradle's form is configure(<projects>) { … }. The gradle.beforeProject and afterProject hooks weren't listed at all. A lockFile set in any of these was followed against the root's own directory, so the configured projects' locks (for example app/… at another version) were never read. Every foreign block now skips its argument list and has to open right there or after one .method. A lockFile inside such a block is left unfollowed, which degrades to gradle_custom_lock_file.
  2. A project dependency marked the script's own block foreign: confirmed and fixed. implementation(project(":lib")) followed by the script's own dependencyLocking { … } no longer counts, because of the stricter block-opening rule in finding 1. Test for 1 and 2: custom_lock_file_and_settings_classpath (extended). It failed on 6932ced.
  3. A regenerated verification entry made a working vendoring dead: confirmed and fixed. Gradle's --write-verification-metadata (the remedy verification_parent_chain_unhandled names) rewrites the jar entry in Gradle's own form. The byte difference then counted as uncommitted wiring. A pending verification-file write no longer counts when the committed file already pins the patched jar's sha256. A file that lacks that hash still counts. Test: a_regenerated_verification_entry_keeps_the_wiring_live. It failed on 6932ced.
  4. A custom lock equal to a listed default was checked twice: fixed. Test: custom_lock_file_is_followed_when_static (extended).

Not changed:

  • A settings-gradle.lockfile in a subproject directory is classified as a settings lock. The lock listing classifies by basename, before this PR's code runs. This only over-reports gradle_settings_lock_unrouted, which is the fail-safe direction. Left for a follow-up.
  • The first-finding break per lock file, the duplicated index-version extraction and "(and N more)" formatting, the lazy foreign-range computation, and discovery cloning lock texts. These affect report completeness, cleanliness and performance only.

Tests (cloud run): cargo test -p socket-patch-core --lib -- vendor::jvm gradle:: vex::discover:: gives 777 passed, golden included. CLI --test vendor_jvm_cli and --lib vex_sources pass. clippy and fmt are clean.


Note

Medium Risk
Touches Gradle vendoring attestation logic and introduces new degraded cases that can block VEX; thorough tests mitigate regressions but supply-chain signals now depend on more static analysis assumptions.

Overview
Summary
The vendored Gradle planner now records every Gradle lock and custom lockFile that it can follow, compares them to the vendored index, and emits new vendor_jvm_degraded reasons whenever a lock pins another version, a settings-gradle.lockfile lacks the committed in-block routing, a file cannot be read, or a custom path cannot be resolved. Liveness (wired) re-plans with Committed::Only, counting only committed wiring and failing when pending writes remain plus the prior lock issues, so VEX no longer attests while those problems exist.

Supporting changes
ScriptGraph exposes custom_lock_files, read_locks, and helpers for literal file(…) resolution; hosted discovery reuses that inventory. is_settings_lock moved to gradle::locks, docs/CLI_CONTRACT.md describe the new reasons, and extensive tests cover lock conflicts, settings locks, duplicate vendored versions, unreadable files, custom paths, and committed-wiring gaps.

Reviewed by Cursor Bugbot for commit 6932ced. Configure here.


Generated by Claude Code

Vendored Gradle keeps the original GAV and leaves lock files unchanged,
but neither its planner nor its VEX liveness (`wired`, which re-plans)
read any lock file. A `gradle.lockfile` pinning the patched GA at another
version makes Gradle resolve that version (or fail lock validation), yet
vendored liveness still attested the patch.

The planner now reads every lock file of every project of the build with
the shared `gradle::locks` reader (per-project, buildscript, settings and
legacy `gradle/dependency-locks/` files, via `ScriptGraph::lockfile_paths`)
and degrades with `gradle_lock_overrides_vendored` when any of them locks
the patched GA at a version other than the vendored one, or is not UTF-8.
A custom `lockFile` location is degraded with `gradle_custom_lock_file`.
Since `wired` requires a re-plan with no degraded warning, both withhold
VEX attestation; vendoring itself still applies.

Mapping from hosted: hosted pins a suffixed version (below-base lock is
invalid, above-base unattested); vendored serves only its own version, so
any other locked version is unattested. Hosted rejects every settings
classpath lock because its script runs too late; vendored wires that
classpath with an in-block exclusiveContent entry, so a settings lock at
the vendored version is served.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review findings on the vendored Gradle lock check:

- A settings-classpath lock is accepted only when that build's settings
  script holds the in-block exclusiveContent entry for the GAV in every
  block that needs one. Liveness (`wired`) now re-plans with
  `Committed::Only`, so it counts only the committed entry, never one a
  re-plan would write; vendoring counts the entry it writes. Otherwise
  `gradle_settings_lock_unrouted`, as hosted rejects it.
- A lock at another version of the GA that the vendored index also
  serves is not an override, so two vendored versions of one GA no
  longer degrade each other.
- A listed lock file that cannot be read as UTF-8 text (I/O error,
  refused symlink, non-UTF-8) fails closed with its own reason,
  `gradle_lock_unreadable`, instead of being skipped.
- A custom `lockFile` set as a literal `file(...)` (or
  `"$projectDir/..."`) in a project's own build script, inside the
  checkout, is read and checked like any lock (missing fails closed);
  only an unfollowable one is `gradle_custom_lock_file`.
- One lock inventory: `ScriptGraph::read_locks` (and
  `custom_lock_files`), used by hosted discovery and the vendored check;
  `is_settings_lock` moves to `gradle::locks`.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`wired` re-plans over the committed tree but ignored the re-plan's
writes, so wiring a re-plan would add again stood in for wiring missing
from the checkout. With no settings lock, a settings classpath on the
patched module whose in-block exclusiveContent entry was removed still
attested, and so did every other pending write.

The liveness re-plan (`Committed::Only`) now degrades with
`gradle_wiring_uncommitted` when it would still write any wiring file:
- settings files: in-block `buildscript` / `pluginManagement` entries
  (a settings `plugins{}` block added since vendoring included) and the
  apply line of every wired build (`buildSrc`, included builds; only the
  root's was checked by `references`);
- the index (a row whose hash no longer matches the tree);
- the derived `maven-metadata.xml`;
- `gradle/verification-metadata.xml` (missing patched hashes);
- the script (already checked by `is_ours`).
Tree files (rebuilt from the committed bytes, hashes checked against
the index by the script) and the git hygiene files (`.gitattributes`,
`.gitignore`, which Gradle never reads) do not count. Vendoring's own
first run is unchanged.

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.

Stale Bugbot comment from a previous run.

The project(...) exclusion only looked up to 7 tokens ahead for the
block's '{', so a named-argument call such as
project(path: ':a', configuration: 'x') { lockFile = file(...) } was
read as this project's own followable lock and resolved against the
wrong directory. Skip the whole argument list, then look for the
block. Addresses the Bugbot finding on gradle/graph.rs.

Co-Authored-By: Claude <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 6932ced. Configure here.

@mikolalysenko
Mikola Lysenko (mikolalysenko) marked this pull request as ready for review November 6, 2026 12:12
@mikolalysenko Mikola Lysenko (mikolalysenko) added the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label Nov 6, 2026
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Ready for review at 6932ced.

  • CI: 30/32 non-skipped checks green. The only failures are the main-wide e2e … pdm:: --ignored leg (module 'hishel' has no attribute 'SyncSqliteStorage', also red on main; Fix pdm e2e red on every PR: pin hishel<2 #2028 fixes it) and the ci-ok aggregate that follows from it.
  • Bugbot: reviewed 6932ced; no unresolved findings.
  • Promoted from draft by the burn-down agent this run. No Slack announcement was sent this run (the Slack send action wasn't available), so there's no slack-announced marker yet.

Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Final review brief (for the human approver)

What it does: the vendored Gradle planner now reads every lock in the build: standard locks, legacy dependency-locks/ files and statically followable custom lockFiles. It uses the same inventory as hosted discovery (ScriptGraph::read_locks). A lock that pins the patched GA at an unvendored version, an unrouted settings lock, or an unreadable or unfollowable lock now degrades VEX. Liveness (wired) re-plans in Committed::Only mode and reports gradle_wiring_uncommitted when wiring is missing from the checkout.

Risk: low-medium. Every new path only adds degraded warnings, so the worst case is VEX withheld, never a false attestation. What vendoring writes is unchanged: plan still runs OrPlanned, and the tests assert that a re-plan writes nothing. The hosted discovery refactor is behaviour-equivalent.

Look here

Verified

  • I read the full diff and the head versions of wired, finish_writes and the verification edit.
  • I checked that the new tests (override above and below, per-kind locks, settings locks, unreadable and refused reads, custom locks, 7 masked-wiring cases) would fail without the change.
  • Path safety and CRLF behaviour are fine. CHANGELOG.md is untouched and there is no debug code.
  • CI: 30/32 non-skipped checks green. The only failures are the main-wide e2e_vex_build pdm:: leg (hishel) and the ci-ok aggregate; Fix pdm e2e red on every PR: pin hishel<2 #2028 fixes both.
  • Bugbot: clean on this head.

Changes I made: none.

Open questions (non-blocking)

  • The jar's verification-metadata.xml entry is compared to the canonical element, including origin="socket-patch". If Gradle's --write-verification-metadata rewrites that element, liveness now reports gradle_wiring_uncommitted where it used to attest. That is fail-closed, but worth confirming.
  • custom_lock_files tokenizes every script before checking for a lockFile token, so an early return would save work on every plan and liveness re-plan.

Auto-merge is not armed yet, because CI is red on the main-wide pdm failure. Once #2028 lands and this goes green, the next final-reviewer run arms it, and approving then sends it straight to the merge queue.


Generated by Claude Code

Second review round on vendored Gradle lock liveness.

- FALSE ATTESTATION: "configure" in the foreign-block list never
  matched (`all_blocks` wants `name {`, Gradle's form is
  `configure(<projects>) { … }`), and per-project hooks
  (`gradle.beforeProject`, `afterProject`) were not listed, so a
  `lockFile` set there was followed against the root's own directory
  and the configured projects' locks were never read. Every foreign
  block now skips its argument list and must open right there (or after
  one `.method`), which also fixes the converse: a project DEPENDENCY
  (`implementation(project(':lib'))`) followed by the script's own
  `dependencyLocking` block no longer marks it foreign.
- Liveness counted any byte difference in verification-metadata.xml as
  uncommitted wiring, so after `./gradlew --write-verification-metadata`
  (the remedy the planner itself names) rewrote the jar entry in
  Gradle's form, a working vendoring was dead. The write no longer
  counts when the committed file already pins the patched jar's sha256.
- A followed custom `lockFile` equal to an already-listed lock is
  checked once.

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