Repository navigation
Stop vendored Gradle liveness attesting a version the lockfile overrides - #2004
Mikola Lysenko (mikolalysenko) wants to merge 5 commits into
Conversation
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>
|
bugbot run Generated by Claude Code |
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>
|
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 6932ced. Configure here.
|
Ready for review at 6932ced.
Generated by Claude Code |
Final review brief (for the human approver)What it does: the vendored Gradle planner now reads every lock in the build: standard locks, legacy 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: Look here
Verified
Changes I made: none. Open questions (non-blocking)
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>
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. Ifgradle.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.rsnow runscheck_locksaftercheck_graph. It reads the build's lock inventory, which is now shared with hosted discovery:ScriptGraph::read_locks, coveringgradle.lockfile,buildscript-gradle.lockfile,settings-gradle.lockfileand the legacygradle/dependency-locks/*.lockfileof every project of every build. It also reads every customlockFilethe graph can follow (ScriptGraph::custom_lock_files).Each problem below adds a
vendor_jvm_degradedwarning.wiredrequires a re-plan with no degraded warning, so each one also withholds VEX attestation. The patch is still applied.gradle_lock_overrides_vendoredgradle_settings_lock_unroutedsettings-gradle.lockfileentry exists, but that build's settings script does not hold the in-blockexclusiveContententry for the GAV in every block that needs one (buildscript, andpluginManagementwhen there are settingsplugins{}).gradle_lock_unreadablegradle_custom_lock_filelockFilelocation can't be followed statically. Only a literalfile('…')or"$projectDir/…"in a project's own build script, outsideallprojects/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:
Committed::OrPlanned).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 indocs/design/maven-vendoring.md.Liveness judges committed wiring, never a re-plan
wiredre-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 reportsgradle_wiring_uncommittedwhen it would still write any wiring file. This reason is liveness-only;vendornever reports it. It is documented inCLI_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.buildscript, andpluginManagementfor settingsplugins{}, including aplugins{}block added after vendoring. This is the gap from the last round.buildSrcand included builds:referenceschecks only the root's apply line.referencesmatches only GAV and uuid, so a row whose hash no longer matches the tree passed.maven-metadata.xml.gradle/verification-metadata.xml: patched hashes missing.is_ours; it is counted too.Not counted, on purpose:
.gitattributes,.gitignore): Gradle never reads them, so they don't change what this checkout resolves.wired_checkedadds only parse guards on top ofwired. The Maven half'sattestable_checkedalready requiresedits.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_entrynow also asserts that the unlocked, stripped case is not live.Review findings (
/code-review high)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 inlock_at_another_version_withholds_liveness).two_vendored_versions_of_a_ga_do_not_override_each_other.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 enablesdependencyLocking), so every lock of the build counts. The message now names the locking configurations.readcan't return (I/O error, refused symlink outside the checkout, non-UTF-8) now fails closed. Test:unreadable_lock_files_fail_closed.gradle_lock_unreadablewith an accurate message, documented inCLI_CONTRACT.md.lockFiledegraded 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 isgradle_custom_lock_file. Tests:custom_lock_file_is_followed_when_static, plus the path-resolution cases ingradle::graph::tests::custom_lock_file_and_settings_classpath. Hosted keeps its blanket rejection oflockFile, because the hosted planner can't rewrite such a lock.plan: refuted as a blocker, noted for follow-up. The lock pass reuses theScriptGraphthatplanalready 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 everyplancall (one per patch, and one per entry invexliveness) 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 throughplan,plan_with_external,wired,entry_wired_checkedand the CLIvexloop. That is better done as its own change than bolted onto locks alone.ScriptGraph::read_locksis now the single inventory. Hosted discovery and the vendored check both use it,ScriptGraph::custom_lock_fileis built oncustom_lock_files, andis_settings_lockmoved togradle::locks(re-exported frompatch::redirect::gradle). The hosted planner's lock rewrite still lists files through the samelockfile_pathsbecause it needs the raw text to edit.Tests
vendor::jvm::gradle::testsfail against the previous commit'scheck_locks, verified by splicing the old function back in. They pass with this change:lock_at_another_version_withholds_livenesssettings_lock_needs_the_committed_in_block_entrytwo_vendored_versions_of_a_ga_do_not_override_each_otherunreadable_lock_files_fail_closedcustom_lock_file_is_followed_when_staticlock_at_another_version_withholds_livenessalso fails on origin/main.liveness_judges_committed_wiring_not_a_replanfails 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-endvexon vendored JVM entries is unchanged.cargo clippy --locked --workspace --all-features -- -D warningsandcargo fmt --all -- --checkare clean.Not done
gradle_range_excludes_vendored.Review findings, round 2 (
/code-review highon4cff8ec..6932ced, cloud routine)configure(...)and the per-project hooks weren't foreign blocks. Confirmed and fixed.all_blocksonly matchesname {, but Gradle's form isconfigure(<projects>) { … }. Thegradle.beforeProjectandafterProjecthooks weren't listed at all. AlockFileset in any of these was followed against the root's own directory, so the configured projects' locks (for exampleapp/…at another version) were never read. Every foreign block now skips its argument list and has to open right there or after one.method. AlockFileinside such a block is left unfollowed, which degrades togradle_custom_lock_file.implementation(project(":lib"))followed by the script's owndependencyLocking { … }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 on6932ced.--write-verification-metadata(the remedyverification_parent_chain_unhandlednames) 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 on6932ced.custom_lock_file_is_followed_when_static(extended).Not changed:
settings-gradle.lockfilein a subproject directory is classified as a settings lock. The lock listing classifies by basename, before this PR's code runs. This only over-reportsgradle_settings_lock_unrouted, which is the fail-safe direction. Left for a follow-up.breakper 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_cliand--lib vex_sourcespass. 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
lockFilethat it can follow, compares them to the vendored index, and emits newvendor_jvm_degradedreasons whenever a lock pins another version, asettings-gradle.lockfilelacks the committed in-block routing, a file cannot be read, or a custom path cannot be resolved. Liveness (wired) re-plans withCommitted::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
ScriptGraphexposescustom_lock_files,read_locks, and helpers for literalfile(…)resolution; hosted discovery reuses that inventory.is_settings_lockmoved togradle::locks, docs/CLI_CONTRACT.mddescribe 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