From 4b195d33642ba574a0938386c1ba6db5198ae840 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Wed, 4 Nov 2026 10:43:40 -0600 Subject: [PATCH 1/5] Stop vendored Gradle liveness attesting a version the lockfile overrides 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 --- crates/socket-patch-cli/CLI_CONTRACT.md | 2 +- .../src/vendor/jvm/gradle.rs | 176 ++++++++++++++++++ docs/design/maven-vendoring.md | 11 ++ 3 files changed, 188 insertions(+), 1 deletion(-) diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index a3ed909d6..45024c17b 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -2631,7 +2631,7 @@ with `reason: : `: |---|---|---| | `vendor_jvm_shape_unsupported` | refusal, nothing written | `gradle_below_6_8`, `android_or_kmp` (also `available-at` module redirects), `gradle_exclusive_content_conflict` (a user rule claiming the module, in any script), `gradle_range_excludes_vendored` (no declared selector admits the vendored version), `gradle_verification_unparseable`, `gradle_index_unreadable`, `build_file_unreadable` (including a non-UTF-8 settings file), `build_file_outside_root`, `not_build_root` (run from a directory an ancestor settings file includes or may include, from a project with no settings file of its own below one, or below an ancestor settings file that is not UTF-8; `repair` refuses there too), `no_build_file` (no `pom.xml`, Gradle, sbt or scala-cli build at the root), `legacy_maven_root` (the root's ledger still holds a pre-v5 single-POM `maven_pom_repository` entry; the whole root is refused until `vendor --revert`, then vendor again) | | `vendor_jvm_upstream_unavailable` | refusal | `classifier_unavailable` (a declared classifier jar no cache or registry has), `module_unavailable` (the pom declares Gradle module metadata that cannot be sourced), `pom_unavailable`, `verification_metadata_unavailable` | -| `vendor_jvm_degraded` | applied, VEX withheld | `gradle_unscanned_build_logic`, `unwired_build_logic` (a nonliteral included build), `settings_plugins_unwired`, `classifier_unpatched_copy` (a classifier jar carries an unpatched copy of a patched member), `verification_parent_chain_unhandled`, `conflicting_managed_version` / `management_unresolved` (Maven reactor: an imported BOM or an out-of-checkout parent manages the artifact at another version, or its POM is unavailable; that local root is not pinned) | +| `vendor_jvm_degraded` | applied, VEX withheld | `gradle_unscanned_build_logic`, `unwired_build_logic` (a nonliteral included build), `settings_plugins_unwired`, `classifier_unpatched_copy` (a classifier jar carries an unpatched copy of a patched member), `verification_parent_chain_unhandled`, `gradle_lock_overrides_vendored` (a dependency lock of the build, settings and buildscript classpath locks included, pins the module at another version than the vendored one, or is not UTF-8), `gradle_custom_lock_file` (a script sets `lockFile`, so a lock is unchecked), `conflicting_managed_version` / `management_unresolved` (Maven reactor: an imported BOM or an out-of-checkout parent manages the artifact at another version, or its POM is unavailable; that local root is not pinned) | | `vendor_jvm_note` | applied, informational | `range_declared` (a range, prefix or rich selector lists versions from the derived `maven-metadata.xml`), `ide_sources_unavailable` | | `vendor_jvm_upstream_unverified` | warning | Upstream metadata was taken offline and not authenticated against registry checksums. | diff --git a/crates/socket-patch-core/src/vendor/jvm/gradle.rs b/crates/socket-patch-core/src/vendor/jvm/gradle.rs index d95d23d89..31da4c1ef 100644 --- a/crates/socket-patch-core/src/vendor/jvm/gradle.rs +++ b/crates/socket-patch-core/src/vendor/jvm/gradle.rs @@ -23,6 +23,7 @@ use crate::gradle::dsl::{ self, all_blocks, is_ident, is_punct, matching_close, strings, Dsl, Tok, Token, }; use crate::gradle::graph::{filter_claims_group, DeclKind, ScriptGraph, Site}; +use crate::gradle::locks; use crate::gradle::selector::{admits, gradle_version_cmp, parse_selector, Selector}; use super::super::state::{WiringAction, WiringRecord}; @@ -337,6 +338,7 @@ pub fn plan( }; let graph = ScriptGraph::collect(&text, list, "", &[]); check_graph(&graph, &undecodable.borrow(), patch, &mut warnings)?; + check_locks(read, list, &graph, patch, &mut warnings); let mut targets = vec![root]; if let Some(bs) = read_buildsrc(read)? { @@ -774,6 +776,82 @@ fn check_graph( Ok(()) } +/// Gradle dependency locks (`gradle.lockfile`, `buildscript-gradle.lockfile`, +/// `settings-gradle.lockfile` and the legacy `gradle/dependency-locks/` +/// files of every project of every build, read with the same [`locks`] +/// reader the hosted backend uses). Vendoring leaves locks unchanged, so a +/// lock that pins the patched GA at another version wins: Gradle resolves +/// that version (from upstream, unless another vendored entry serves it) +/// or fails lock validation, and never consumes this patch there. That is +/// degraded (`gradle_lock_overrides_vendored`), which also withholds VEX +/// attestation: [`wired`] re-plans and requires no degraded warning. A +/// custom lock-file location (`lockFile`) hides a lock from this check and +/// is degraded too (`gradle_custom_lock_file`), as hosted discovery +/// invalidates it. +/// +/// The mapping from the hosted rules (`vex::discover::gradle`): hosted +/// pins a suffixed version, so a lock must name the suffix (a lock below +/// the base is invalid, one above it an unattested upstream release). +/// Vendored keeps the ORIGINAL GAV, so the one lock entry it serves is the +/// vendored version itself; any other version, above or below, is a build +/// that does not consume the patch. Hosted also rejects every +/// settings-classpath lock of the GA, because its script runs after that +/// classpath resolves; vendored wires the settings `buildscript` and +/// `pluginManagement` repositories with an in-block `exclusiveContent` +/// entry for the GAV, so a settings lock at the vendored version is served +/// and one at another version is degraded like any other lock. +fn check_locks( + read: ReadFn<'_>, + list: ListFn<'_>, + graph: &ScriptGraph, + patch: &JvmPatch<'_>, + warnings: &mut Vec, +) { + let (g, a, v) = (patch.group_id, patch.artifact_id, patch.version); + if let Some(rel) = graph.custom_lock_file() { + warnings.push(degraded( + "gradle_custom_lock_file", + format!( + "{rel} sets a custom dependency-lock file (`lockFile`), which may lock {g}:{a} \ + at a version other than the vendored {v}; that lock is unchecked" + ), + )); + } + let mut overrides = Vec::new(); + for rel in graph.lockfile_paths(list) { + let Some(bytes) = read(&rel) else { continue }; + let Some(text) = dsl::decode(&bytes) else { + overrides.push(format!( + "{rel} is not UTF-8, so a lock of {g}:{a} there is unchecked" + )); + continue; + }; + if let Some(e) = locks::parse(&text) + .entries_of(g, a) + .find(|e| e.version != v) + { + overrides.push(format!( + "{rel}:{} locks {g}:{a} at {}, not the vendored {v}", + e.line, e.version + )); + } + } + if let Some(first) = overrides.first() { + warnings.push(degraded( + "gradle_lock_overrides_vendored", + format!( + "{first}{}; Gradle resolves the locked version (or fails lock validation), so \ + that build does not consume the patched {g}:{a}:{v}; lock {v} \ + (`./gradlew dependencies --write-locks`) or vendor the locked version", + match overrides.len() { + 1 => String::new(), + n => format!(" (and {} more)", n - 1), + } + ), + )); + } +} + /// `../` once per segment of `dir`: the script path from that build. fn prefix_of(dir: &str) -> String { "../".repeat(if dir.is_empty() { @@ -4658,4 +4736,102 @@ mod tests { assert!(!live(&edited)); assert!(references(&|r: &str| edited.get(r).cloned(), &p.coords())); } + + /// A dependency lock of the build that pins the patched GA at another + /// version than the vendored one wins over the vendored GAV: Gradle + /// resolves the locked version, so liveness (and the planner) withhold + /// attestation. Every lock kind of every project of the build counts, + /// the settings classpath included; a lock at the vendored version, + /// another GA's lock and a nested build the root does not include do + /// not. A custom `lockFile` hides a lock and withholds it too. + #[test] + fn lock_at_another_version_withholds_liveness() { + let files = fs(&[ + ("settings.gradle", "include 'app'\n"), + ( + "app/build.gradle", + "dependencies { implementation 'com.google.code.gson:gson:2.10.1' }\n", + ), + ]); + let p = patch(); + let plan = run(&files, &p).unwrap(); + let after = applied(&files, &plan); + let live = |files: &BTreeMap>| { + wired( + &|r: &str| files.get(r).cloned(), + &|d: &str| list_of(files, d), + &p.coords(), + ) + }; + let with = |extra: &[(&str, &str)]| { + let mut out = after.clone(); + for (rel, text) in extra { + out.insert(rel.to_string(), text.as_bytes().to_vec()); + } + out + }; + assert!(live(&after)); + const AT: &str = "com.google.code.gson:gson:2.10.1=compileClasspath,runtimeClasspath\n"; + const ABOVE: &str = "com.google.code.gson:gson:2.11.0=compileClasspath,runtimeClasspath\n"; + const BELOW: &str = "com.google.code.gson:gson:2.9.0=runtimeClasspath\n"; + let other = "org.x:y:1=runtimeClasspath\nempty=\n"; + for (rel, text, consumed) in [ + ("app/gradle.lockfile", AT, true), + ("app/gradle.lockfile", other, true), + ("app/gradle.lockfile", ABOVE, false), + ("app/gradle.lockfile", BELOW, false), + ("gradle.lockfile", ABOVE, false), + ("app/buildscript-gradle.lockfile", ABOVE, false), + ("settings-gradle.lockfile", AT, true), + ("settings-gradle.lockfile", ABOVE, false), + ( + "app/gradle/dependency-locks/runtimeClasspath.lockfile", + "com.google.code.gson:gson:2.11.0\n", + false, + ), + // A nested build the root does not include governs nothing. + ("samples/demo/gradle.lockfile", ABOVE, true), + ] { + let locked = with(&[(rel, text)]); + assert_eq!(live(&locked), consumed, "{rel}: {text}"); + assert_eq!( + wired_checked( + &|r: &str| locked.get(r).cloned(), + &|d: &str| list_of(&locked, d), + &p.coords() + ), + Ok(consumed), + "{rel}: {text}" + ); + let replan = run(&locked, &p).unwrap(); + let degraded = replan.warnings.iter().any(|w| { + w.code == DEGRADED + && w.detail + .starts_with("reason: gradle_lock_overrides_vendored: ") + && w.detail.contains(rel) + }); + assert_eq!(degraded, !consumed, "{rel}: {text}"); + // A lock never stops the vendoring itself. + assert!(replan.writes.is_empty(), "{rel}"); + } + // Two locks at the vendored version and one above: still not live. + assert!(!live(&with(&[ + ("gradle.lockfile", AT), + ("app/gradle.lockfile", ABOVE), + ]))); + // A lock file that is not UTF-8 proves nothing. + let mut bad = after.clone(); + bad.insert("app/gradle.lockfile".into(), b"\xff\xfe\n".to_vec()); + assert!(!live(&bad)); + // A custom lock-file location hides the lock from the check. + let custom = with(&[( + "app/build.gradle", + "dependencyLocking { lockFile = file('locks/app.lockfile') }\n\ + dependencies { implementation 'com.google.code.gson:gson:2.10.1' }\n", + )]); + assert!(!live(&custom)); + assert!(run(&custom, &p).unwrap().warnings.iter().any(|w| { + w.code == DEGRADED && w.detail.starts_with("reason: gradle_custom_lock_file: ") + })); + } } diff --git a/docs/design/maven-vendoring.md b/docs/design/maven-vendoring.md index 748d32231..240ab9d8e 100644 --- a/docs/design/maven-vendoring.md +++ b/docs/design/maven-vendoring.md @@ -165,6 +165,17 @@ files receive an apply line for `.socket/gradle/socket-patch.settings.gradle`. Settings plugin and buildscript classpaths receive an in-block exclusive repository entry because those classpaths resolve before the apply line runs. +Because lockfiles stay unchanged, a lock that pins the patched module at +another version wins over the vendored GAV: Gradle resolves the locked version +or fails lock validation. Every lock file of every project of the build is +read (`gradle.lockfile`, `buildscript-gradle.lockfile`, +`settings-gradle.lockfile`, and the legacy `gradle/dependency-locks/`), and an +entry for the module at any version other than the vendored one is degraded +(`gradle_lock_overrides_vendored`), as is a custom `lockFile` location the +check cannot follow (`gradle_custom_lock_file`). The patch is still applied, +and `vex` does not attest it. A settings classpath lock at the vendored version +is served by the in-block entry, so it is not degraded. + The script, the index, the derived metadata and the `.gitattributes` files are owned text and are compared line-ending blind, so a `core.autocrlf` checkout passes `--check` and reverts clean. `.socket/gradle/.gitattributes` (`* -text`, From 4cff8ec7a2084df189bc3f1416b272e27915ad93 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Wed, 4 Nov 2026 14:56:54 -0600 Subject: [PATCH 2/5] Address review: route-checked settings locks, shared lock inventory 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 --- crates/socket-patch-cli/CLI_CONTRACT.md | 2 +- crates/socket-patch-core/src/gradle/graph.rs | 194 +++++- crates/socket-patch-core/src/gradle/locks.rs | 7 + .../src/patch/redirect/gradle.rs | 10 +- .../src/vendor/jvm/gradle.rs | 599 ++++++++++++++---- .../src/vex/discover/gradle.rs | 15 +- docs/design/maven-vendoring.md | 15 +- 7 files changed, 689 insertions(+), 153 deletions(-) diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index 45024c17b..696f603b1 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -2631,7 +2631,7 @@ with `reason: : `: |---|---|---| | `vendor_jvm_shape_unsupported` | refusal, nothing written | `gradle_below_6_8`, `android_or_kmp` (also `available-at` module redirects), `gradle_exclusive_content_conflict` (a user rule claiming the module, in any script), `gradle_range_excludes_vendored` (no declared selector admits the vendored version), `gradle_verification_unparseable`, `gradle_index_unreadable`, `build_file_unreadable` (including a non-UTF-8 settings file), `build_file_outside_root`, `not_build_root` (run from a directory an ancestor settings file includes or may include, from a project with no settings file of its own below one, or below an ancestor settings file that is not UTF-8; `repair` refuses there too), `no_build_file` (no `pom.xml`, Gradle, sbt or scala-cli build at the root), `legacy_maven_root` (the root's ledger still holds a pre-v5 single-POM `maven_pom_repository` entry; the whole root is refused until `vendor --revert`, then vendor again) | | `vendor_jvm_upstream_unavailable` | refusal | `classifier_unavailable` (a declared classifier jar no cache or registry has), `module_unavailable` (the pom declares Gradle module metadata that cannot be sourced), `pom_unavailable`, `verification_metadata_unavailable` | -| `vendor_jvm_degraded` | applied, VEX withheld | `gradle_unscanned_build_logic`, `unwired_build_logic` (a nonliteral included build), `settings_plugins_unwired`, `classifier_unpatched_copy` (a classifier jar carries an unpatched copy of a patched member), `verification_parent_chain_unhandled`, `gradle_lock_overrides_vendored` (a dependency lock of the build, settings and buildscript classpath locks included, pins the module at another version than the vendored one, or is not UTF-8), `gradle_custom_lock_file` (a script sets `lockFile`, so a lock is unchecked), `conflicting_managed_version` / `management_unresolved` (Maven reactor: an imported BOM or an out-of-checkout parent manages the artifact at another version, or its POM is unavailable; that local root is not pinned) | +| `vendor_jvm_degraded` | applied, VEX withheld | `gradle_unscanned_build_logic`, `unwired_build_logic` (a nonliteral included build), `settings_plugins_unwired`, `classifier_unpatched_copy` (a classifier jar carries an unpatched copy of a patched member), `verification_parent_chain_unhandled`, `gradle_lock_overrides_vendored` (a dependency lock of the build, buildscript and settings classpath locks included, pins the module at a version no vendored row serves), `gradle_settings_lock_unrouted` (a `settings-gradle.lockfile` locks the module but that build's settings script does not hold the in-block repository entry for it), `gradle_lock_unreadable` (a lock file, or a followed custom `lockFile`, is missing, unreadable, outside the checkout or not UTF-8), `gradle_custom_lock_file` (a `lockFile` location that cannot be followed statically: anything but a literal `file(…)` in a project's own build script), `conflicting_managed_version` / `management_unresolved` (Maven reactor: an imported BOM or an out-of-checkout parent manages the artifact at another version, or its POM is unavailable; that local root is not pinned) | | `vendor_jvm_note` | applied, informational | `range_declared` (a range, prefix or rich selector lists versions from the derived `maven-metadata.xml`), `ide_sources_unavailable` | | `vendor_jvm_upstream_unverified` | warning | Upstream metadata was taken offline and not authenticated against registry checksums. | diff --git a/crates/socket-patch-core/src/gradle/graph.rs b/crates/socket-patch-core/src/gradle/graph.rs index f44d6022a..70a93c622 100644 --- a/crates/socket-patch-core/src/gradle/graph.rs +++ b/crates/socket-patch-core/src/gradle/graph.rs @@ -1354,19 +1354,135 @@ impl ScriptGraph { /// The first script setting a custom dependency-lock file location /// (`lockFile = …` / `lockFile.set(…)`). pub fn custom_lock_file(&self) -> Option { - self.scripts.iter().find_map(|s| { + self.custom_lock_files() + .into_iter() + .next() + .map(|c| c.script) + } + + /// Every custom dependency-lock file location (`lockFile = …` / + /// `lockFile.set(…)`), in script order. See [`CustomLockFile::path`] + /// for when the location can be followed. + pub fn custom_lock_files(&self) -> Vec { + let mut out = Vec::new(); + for s in &self.scripts { let toks = dsl::tokens(&s.text, s.dsl); - (0..toks.len()) - .any(|i| { - is_ident(toks.get(i), "lockFile") - && ((is_punct(toks.get(i + 1), b'=') && !is_punct(toks.get(i + 2), b'=')) - || (is_punct(toks.get(i + 1), b'.') - && is_ident(toks.get(i + 2), "set") - && is_punct(toks.get(i + 3), b'('))) + // A project's own build script: `file(…)` resolves against it. + let project_dir = (s.kind == ScriptKind::Build) + .then(|| { + self.builds + .iter() + .flat_map(|b| &b.projects) + .find(|p| p.build_script.as_deref() == Some(s.rel.as_str())) + .map(|p| p.dir.clone()) }) - .then(|| s.rel.clone()) - }) + .flatten(); + // Blocks that configure other projects resolve `file(…)` + // against those. + let foreign: Vec<(usize, usize)> = ["allprojects", "subprojects", "configure"] + .iter() + .flat_map(|name| all_blocks(&toks, name)) + .chain( + (0..toks.len()) + .filter(|&i| is_ident(toks.get(i), "project")) + .filter_map(|i| { + let open = (i + 1..toks.len().min(i + 8)) + .find(|&j| is_punct(toks.get(j), b'{'))?; + matching_close(&toks, open).map(|c| (open, c)) + }), + ) + .collect(); + for i in 0..toks.len() { + if !is_ident(toks.get(i), "lockFile") { + continue; + } + let value = if is_punct(toks.get(i + 1), b'=') && !is_punct(toks.get(i + 2), b'=') { + i + 2 + } else if is_punct(toks.get(i + 1), b'.') + && is_ident(toks.get(i + 2), "set") + && is_punct(toks.get(i + 3), b'(') + { + i + 4 + } else { + continue; + }; + let path = project_dir + .as_deref() + .filter(|_| !foreign.iter().any(|(o, c)| i > *o && i < *c)) + .and_then(|dir| { + let arg = file_call_arg(&toks, value)?; + resolve_rel("", dir, &arg) + }); + out.push(CustomLockFile { + script: s.rel.clone(), + line: line_of(&s.text, toks[i].start), + path, + }); + } + } + out + } + + /// Every lock file of every build ([`Self::lockfile_paths`]) with its + /// parsed content: `None` for a file the listing shows that `read` + /// cannot give as text (unreadable, not UTF-8, refused). The one lock + /// inventory the hosted and vendored backends both check. + pub fn read_locks( + &self, + list: ListFn<'_>, + read: TextReadFn<'_>, + ) -> Vec<(String, Option)> { + self.lockfile_paths(list) + .into_iter() + .map(|rel| { + let state = read(&rel).map(|text| locks::parse(&text)); + (rel, state) + }) + .collect() + } +} + +/// A custom dependency-lock file location ([`ScriptGraph::custom_lock_files`]). +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct CustomLockFile { + /// The script that sets it. + pub script: String, + /// 1-based line of `lockFile`. + pub line: usize, + /// The lock file, relative to the graph's read root, when the value is + /// `file('')` with a literal path (or `"$projectDir/"`) in + /// a project's own build script, outside any `allprojects`, + /// `subprojects`, `configure` or `project(…)` block, and the path stays + /// inside the read root. `None`: it cannot be followed statically. + pub path: Option, +} + +/// The path argument of `file()` at `toks[at..]`, the call +/// closing the value: a literal, or a `"$projectDir/…"` / +/// `"${projectDir}/…"` string with no other interpolation (made +/// project-relative). +fn file_call_arg(toks: &[Token], at: usize) -> Option { + if !is_ident(toks.get(at), "file") + || !is_punct(toks.get(at + 1), b'(') + || !is_punct(toks.get(at + 3), b')') + || is_punct(toks.get(at + 4), b'.') + { + return None; + } + if let Some(lit) = literal_of(toks.get(at + 2)) { + return Some(lit.to_string()); } + let Some(Token { + tok: Tok::Str { value, .. }, + .. + }) = toks.get(at + 2) + else { + return None; + }; + let rest = value + .strip_prefix("$projectDir/") + .or_else(|| value.strip_prefix("${projectDir}/"))?; + (!rest.contains('$') && !rest.contains('\\')).then(|| rest.to_string()) } /// The subproject relation of a directory to an ancestor build: whether @@ -2656,6 +2772,64 @@ other = { group = \"x\", name = \"y\", version = \"1\" } )]); assert_eq!(g.custom_lock_file(), None); + // The location is followed only from a project's own build script, + // as a literal (or `$projectDir/`) `file(…)` inside the root. + let path_of = |files: &[(&str, &str)]| -> Vec> { + graph(files) + .custom_lock_files() + .into_iter() + .map(|c| c.path) + .collect() + }; + let lock = |v: &str| format!("dependencyLocking {{ lockFile = {v} }}\n"); + for (value, want) in [ + ("file('locks/app.lockfile')", Some("app/locks/app.lockfile")), + ( + "file(\"$projectDir/locks/x.lockfile\")", + Some("app/locks/x.lockfile"), + ), + ("file(\"${projectDir}/x.lockfile\")", Some("app/x.lockfile")), + ("file('../shared.lockfile')", Some("shared.lockfile")), + ("file('../../outside.lockfile')", None), + ("file('/abs/x.lockfile')", None), + ("file(\"$rootDir/x.lockfile\")", None), + ("file(name)", None), + ("file('x.lockfile').absoluteFile", None), + ("layout.projectDirectory.file('x.lockfile')", None), + ] { + let text = lock(value); + let files = [ + ("settings.gradle", "include 'app'\n"), + ("app/build.gradle", text.as_str()), + ]; + assert_eq!(path_of(&files), vec![want.map(str::to_string)], "{value}"); + } + let kts = [( + "build.gradle.kts", + "dependencyLocking { lockFile.set(file(\"gradle/x.lockfile\")) }\n", + )]; + assert_eq!(path_of(&kts), vec![Some("gradle/x.lockfile".to_string())]); + // Configuring other projects, or from an applied script: unfollowed. + for text in [ + "subprojects { dependencyLocking { lockFile = file('x.lockfile') } }\n", + "allprojects { dependencyLocking { lockFile = file('x.lockfile') } }\n", + "project(':app') { dependencyLocking { lockFile = file('x.lockfile') } }\n", + ] { + let files = [ + ("settings.gradle", "include 'app'\n"), + ("build.gradle", text), + ]; + assert_eq!(path_of(&files), vec![None], "{text}"); + } + let applied = [ + ("build.gradle", "apply from: 'locks.gradle'\n"), + ( + "locks.gradle", + "dependencyLocking { lockFile = file('x.lockfile') }\n", + ), + ]; + assert_eq!(path_of(&applied), vec![None]); + let g = graph(&[ ( "settings.gradle", diff --git a/crates/socket-patch-core/src/gradle/locks.rs b/crates/socket-patch-core/src/gradle/locks.rs index 5b9eebe34..d97c275d1 100644 --- a/crates/socket-patch-core/src/gradle/locks.rs +++ b/crates/socket-patch-core/src/gradle/locks.rs @@ -20,6 +20,13 @@ pub const LOCKFILE_NAMES: &[&str] = &[ "settings-gradle.lockfile", ]; +/// Whether `rel` is a settings-classpath lock (`settings-gradle.lockfile` +/// of any build): it locks what the settings script's own classpath +/// resolves, before the script runs. +pub fn is_settings_lock(rel: &str) -> bool { + rel.rsplit('/').next() == Some("settings-gradle.lockfile") +} + /// The legacy per-configuration lock directory, relative to a project. pub const LEGACY_LOCK_DIR: &str = "gradle/dependency-locks"; diff --git a/crates/socket-patch-core/src/patch/redirect/gradle.rs b/crates/socket-patch-core/src/patch/redirect/gradle.rs index 24b38c6b8..73005ffd2 100644 --- a/crates/socket-patch-core/src/patch/redirect/gradle.rs +++ b/crates/socket-patch-core/src/patch/redirect/gradle.rs @@ -94,12 +94,10 @@ const VENDORED_INDEX_REL: &str = vendored::INDEX_REL; const VERIFICATION_REL: &str = vendored::VERIFICATION_REL; const WRAPPER_PROPERTIES_REL: &str = "gradle/wrapper/gradle-wrapper.properties"; -/// Whether `rel` is a settings-classpath lock (`settings-gradle.lockfile` -/// of any build). Gradle resolves that classpath before any settings -/// script runs, so the hosted script can never pin what it locks. -pub fn is_settings_lock(rel: &str) -> bool { - rel.rsplit('/').next() == Some("settings-gradle.lockfile") -} +/// Whether `rel` is a settings-classpath lock. Gradle resolves that +/// classpath before any settings script runs, so the hosted script can +/// never pin what it locks. +pub use crate::gradle::locks::is_settings_lock; /// Whether `files` (root-relative) hold a root Gradle settings or build /// script. diff --git a/crates/socket-patch-core/src/vendor/jvm/gradle.rs b/crates/socket-patch-core/src/vendor/jvm/gradle.rs index 31da4c1ef..5edab54f0 100644 --- a/crates/socket-patch-core/src/vendor/jvm/gradle.rs +++ b/crates/socket-patch-core/src/vendor/jvm/gradle.rs @@ -23,8 +23,8 @@ use crate::gradle::dsl::{ self, all_blocks, is_ident, is_punct, matching_close, strings, Dsl, Tok, Token, }; use crate::gradle::graph::{filter_claims_group, DeclKind, ScriptGraph, Site}; -use crate::gradle::locks; use crate::gradle::selector::{admits, gradle_version_cmp, parse_selector, Selector}; +use crate::gradle::{locks, parent_rel}; use super::super::state::{WiringAction, WiringRecord}; use super::{ @@ -258,6 +258,26 @@ pub fn plan( read: ReadFn<'_>, list: ListFn<'_>, patch: &JvmPatch<'_>, +) -> Result { + plan_as(read, list, patch, Committed::OrPlanned) +} + +/// Which settings wiring a check may count as routing the GAV. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum Committed { + /// What the checkout holds, or what this plan writes: vendoring, where + /// the plan's own in-block entries will be committed. + OrPlanned, + /// Only what the checkout holds: liveness ([`wired`]) re-plans, and a + /// re-plan that would still add an entry proves nothing routed. + Only, +} + +fn plan_as( + read: ReadFn<'_>, + list: ListFn<'_>, + patch: &JvmPatch<'_>, + committed: Committed, ) -> Result { if super::wrapper_version(read, "gradle").is_some_and(|v| v < (6, 8, 0)) { return Err(shape_refusal( @@ -338,7 +358,7 @@ pub fn plan( }; let graph = ScriptGraph::collect(&text, list, "", &[]); check_graph(&graph, &undecodable.borrow(), patch, &mut warnings)?; - check_locks(read, list, &graph, patch, &mut warnings); + check_locks(read, list, &graph, patch, committed, &mut warnings); let mut targets = vec![root]; if let Some(bs) = read_buildsrc(read)? { @@ -776,82 +796,204 @@ fn check_graph( Ok(()) } -/// Gradle dependency locks (`gradle.lockfile`, `buildscript-gradle.lockfile`, -/// `settings-gradle.lockfile` and the legacy `gradle/dependency-locks/` -/// files of every project of every build, read with the same [`locks`] -/// reader the hosted backend uses). Vendoring leaves locks unchanged, so a -/// lock that pins the patched GA at another version wins: Gradle resolves -/// that version (from upstream, unless another vendored entry serves it) -/// or fails lock validation, and never consumes this patch there. That is -/// degraded (`gradle_lock_overrides_vendored`), which also withholds VEX -/// attestation: [`wired`] re-plans and requires no degraded warning. A -/// custom lock-file location (`lockFile`) hides a lock from this check and -/// is degraded too (`gradle_custom_lock_file`), as hosted discovery -/// invalidates it. +/// Gradle dependency locks: every lock file of every project of every +/// build (`gradle.lockfile`, `buildscript-gradle.lockfile`, +/// `settings-gradle.lockfile`, the legacy `gradle/dependency-locks/` +/// files), from the lock inventory hosted discovery checks too +/// ([`ScriptGraph::read_locks`]), plus every custom `lockFile` location +/// the graph can follow. Vendoring leaves locks unchanged, so a lock entry +/// of the patched GA wins over the vendored GAV: Gradle resolves the +/// locked version (or fails lock validation). Each finding below is +/// degraded, which also withholds VEX attestation ([`wired`] re-plans and +/// requires no degraded warning): +/// +/// - `gradle_lock_overrides_vendored`: an entry at a version no vendored +/// row serves (neither this patch's nor another vendored version of the +/// GA in the index, which the same script routes); +/// - `gradle_settings_lock_unrouted`: a settings-classpath entry whose +/// build's settings script does not hold the in-block `exclusiveContent` +/// entry for that GAV in every block that needs one; +/// - `gradle_lock_unreadable`: a listed lock file (or a followed custom +/// one) that cannot be read as UTF-8 text: unreadable, refused (outside +/// the checkout), missing, or not UTF-8; its entries are unknown; +/// - `gradle_custom_lock_file`: a `lockFile` location the graph cannot +/// follow ([`crate::gradle::graph::CustomLockFile::path`]). /// /// The mapping from the hosted rules (`vex::discover::gradle`): hosted -/// pins a suffixed version, so a lock must name the suffix (a lock below -/// the base is invalid, one above it an unattested upstream release). -/// Vendored keeps the ORIGINAL GAV, so the one lock entry it serves is the -/// vendored version itself; any other version, above or below, is a build -/// that does not consume the patch. Hosted also rejects every -/// settings-classpath lock of the GA, because its script runs after that -/// classpath resolves; vendored wires the settings `buildscript` and -/// `pluginManagement` repositories with an in-block `exclusiveContent` -/// entry for the GAV, so a settings lock at the vendored version is served -/// and one at another version is degraded like any other lock. +/// pins a suffixed version, so a lock must name the suffix (below the base +/// is invalid, above it an unattested upstream release). Vendored keeps the +/// ORIGINAL GAV, so a lock entry is served only at a vendored version; any +/// other version, above or below, is a build that does not consume the +/// patch. Hosted rejects every settings-classpath lock of the GA because +/// its script runs after that classpath resolves; vendored instead routes +/// that classpath with in-block entries, so a settings lock is accepted +/// only when the settings script holds them: when vendoring, as committed +/// or as this plan writes them ([`Committed::OrPlanned`]); for liveness, +/// as committed ([`Committed::Only`]). Anything else is rejected, as +/// hosted would. +/// +/// Scope: a lock file names the configurations of its own project that +/// resolve the GA, so only entries of the GA count; a project that locks +/// the GA at another version does resolve it there, and hosted withholds +/// the whole build for one such lock too. A lock file Gradle ignores (its +/// project never enables locking) cannot be told apart statically, so it +/// counts. fn check_locks( read: ReadFn<'_>, list: ListFn<'_>, graph: &ScriptGraph, patch: &JvmPatch<'_>, + committed: Committed, warnings: &mut Vec, ) { let (g, a, v) = (patch.group_id, patch.artifact_id, patch.version); - if let Some(rel) = graph.custom_lock_file() { - warnings.push(degraded( - "gradle_custom_lock_file", - format!( - "{rel} sets a custom dependency-lock file (`lockFile`), which may lock {g}:{a} \ - at a version other than the vendored {v}; that lock is unchecked" - ), - )); + let text = |rel: &str| read(rel).and_then(|bytes| dsl::decode(&bytes)); + let mut lock_files = graph.read_locks(list, &text); + let mut unfollowed = Vec::new(); + for c in graph.custom_lock_files() { + match c.path { + // Set explicitly, so expected to exist: a missing one is + // unreadable too (fail closed). + Some(path) => { + let state = text(&path).map(|t| locks::parse(&t)); + lock_files.push((path, state)); + } + None => unfollowed.push(format!("{}:{}", c.script, c.line)), + } } - let mut overrides = Vec::new(); - for rel in graph.lockfile_paths(list) { - let Some(bytes) = read(&rel) else { continue }; - let Some(text) = dsl::decode(&bytes) else { - overrides.push(format!( - "{rel} is not UTF-8, so a lock of {g}:{a} there is unchecked" - )); + let prefix = format!("{g}:{a}:"); + let vendored: BTreeSet = read(INDEX_REL) + .and_then(|bytes| String::from_utf8(bytes).ok()) + .and_then(|index| index_rows(&index)) + .unwrap_or_default() + .iter() + .filter_map(|row| { + let gav = row.split('\t').next()?; + gav.strip_prefix(&prefix).map(str::to_string) + }) + .collect(); + let served = |w: &str| w == v || vendored.contains(w); + + let (mut overrides, mut unrouted, mut unreadable) = (Vec::new(), Vec::new(), Vec::new()); + for (rel, state) in &lock_files { + let Some(state) = state else { + unreadable.push(rel.clone()); continue; }; - if let Some(e) = locks::parse(&text) - .entries_of(g, a) - .find(|e| e.version != v) - { - overrides.push(format!( - "{rel}:{} locks {g}:{a} at {}, not the vendored {v}", - e.line, e.version - )); + let settings = locks::is_settings_lock(rel); + for e in state.entries_of(g, a) { + if !served(&e.version) { + overrides.push(format!( + "{rel}:{} locks {g}:{a} at {} for {}, not the vendored {v}", + e.line, + e.version, + if e.confs.is_empty() { + "its configuration".to_string() + } else { + e.confs.join(",") + } + )); + break; + } + let planned = committed == Committed::OrPlanned && e.version == v; + if settings && !settings_routes(graph, parent_rel(rel), g, a, &e.version, planned) { + unrouted.push(format!( + "{rel}:{} locks {g}:{a}:{} on the settings classpath", + e.line, e.version + )); + break; + } } } - if let Some(first) = overrides.first() { + let first_of = |found: &[String]| { + found.first().map(|first| match found.len() { + 1 => first.clone(), + n => format!("{first} (and {} more)", n - 1), + }) + }; + if let Some(found) = first_of(&overrides) { warnings.push(degraded( "gradle_lock_overrides_vendored", format!( - "{first}{}; Gradle resolves the locked version (or fails lock validation), so \ - that build does not consume the patched {g}:{a}:{v}; lock {v} \ - (`./gradlew dependencies --write-locks`) or vendor the locked version", - match overrides.len() { - 1 => String::new(), - n => format!(" (and {} more)", n - 1), - } + "{found}; Gradle resolves the locked version (or fails lock validation), so \ + that build does not consume the patched {g}:{a}:{v}; lock {v} or vendor the \ + locked version" + ), + )); + } + if let Some(found) = first_of(&unrouted) { + warnings.push(degraded( + "gradle_settings_lock_unrouted", + format!( + "{found}, and that build's settings script does not route it to the vendored \ + repository (no in-block entry in its `buildscript` or `pluginManagement` \ + repositories); that classpath resolves before the apply line runs" + ), + )); + } + if let Some(found) = first_of(&unreadable) { + warnings.push(degraded( + "gradle_lock_unreadable", + format!( + "{found} cannot be read as UTF-8 text (missing, unreadable, outside the \ + checkout, or not UTF-8), so whether it locks {g}:{a} at another version is \ + unknown" + ), + )); + } + if let Some(found) = first_of(&unfollowed) { + warnings.push(degraded( + "gradle_custom_lock_file", + format!( + "{found} sets a custom dependency-lock file (`lockFile`) that cannot be \ + followed statically, so whether it locks {g}:{a} at another version than the \ + vendored {v} is unknown" ), )); } } +/// Whether the settings script of the build at `dir` routes `g:a:w` on its +/// own classpath: every block that needs an in-block entry for the GAV +/// (`buildscript`, and `pluginManagement` when it has settings `plugins{}`) +/// holds it, and at least one does. `planned`: an entry this plan adds +/// counts (see [`Committed`]). +fn settings_routes( + graph: &ScriptGraph, + dir: &str, + g: &str, + a: &str, + w: &str, + planned: bool, +) -> bool { + let Some(script) = graph + .builds + .iter() + .find(|b| b.dir == dir) + .and_then(|b| b.settings.as_deref()) + .and_then(|rel| graph.scripts.iter().find(|s| s.rel == rel)) + else { + return false; + }; + let coords = Coords { + group_id: g, + artifact_id: a, + version: w, + uuid: "", + }; + let kotlin = script.dsl == Dsl::Kotlin; + let mut routed = false; + for scope in ["pluginManagement", "buildscript"] { + match scoped_in_block_entry(&script.text, kotlin, &prefix_of(dir), &coords, scope) { + InBlock::Unneeded => {} + InBlock::Present => routed = true, + InBlock::Edited { .. } if planned => routed = true, + InBlock::Edited { .. } | InBlock::Unwired(_) => return false, + } + } + routed +} + /// `../` once per segment of `dir`: the script path from that build. fn prefix_of(dir: &str) -> String { "../".repeat(if dir.is_empty() { @@ -1327,7 +1469,7 @@ pub fn wired(read: ReadFn<'_>, list: ListFn<'_>, c: &Coords<'_>) -> bool { extra_artifacts: &extras, patched_members: &patched, }; - plan(read, list, &patch).is_ok_and(|p| { + plan_as(read, list, &patch, Committed::Only).is_ok_and(|p| { p.warnings.iter().all(|w| { w.code != DEGRADED // Added by the caller from the upstream chain; `--check` @@ -4737,101 +4879,310 @@ mod tests { assert!(references(&|r: &str| edited.get(r).cloned(), &p.coords())); } - /// A dependency lock of the build that pins the patched GA at another - /// version than the vendored one wins over the vendored GAV: Gradle - /// resolves the locked version, so liveness (and the planner) withhold - /// attestation. Every lock kind of every project of the build counts, - /// the settings classpath included; a lock at the vendored version, - /// another GA's lock and a nested build the root does not include do - /// not. A custom `lockFile` hides a lock and withholds it too. - #[test] - fn lock_at_another_version_withholds_liveness() { - let files = fs(&[ + const LOCK_AT: &str = "com.google.code.gson:gson:2.10.1=compileClasspath,runtimeClasspath\n"; + const LOCK_ABOVE: &str = "com.google.code.gson:gson:2.11.0=compileClasspath,runtimeClasspath\n"; + const LOCK_BELOW: &str = "com.google.code.gson:gson:2.9.0=runtimeClasspath\n"; + + fn live_in(files: &BTreeMap>, p: &JvmPatch<'_>) -> bool { + wired( + &|r: &str| files.get(r).cloned(), + &|d: &str| list_of(files, d), + &p.coords(), + ) + } + + fn with_files( + base: &BTreeMap>, + extra: &[(&str, &str)], + ) -> BTreeMap> { + let mut out = base.clone(); + for (rel, text) in extra { + out.insert(rel.to_string(), text.as_bytes().to_vec()); + } + out + } + + /// The degraded reasons of `plan`, in order. + fn degraded_reasons(plan: &JvmPlan) -> Vec { + plan.warnings + .iter() + .filter(|w| w.code == DEGRADED) + .filter_map(|w| { + Some( + w.detail + .strip_prefix("reason: ")? + .split(':') + .next()? + .to_string(), + ) + }) + .collect() + } + + /// `files` vendored with `p`. + fn vendored_with( + files: &BTreeMap>, + p: &JvmPatch<'_>, + ) -> BTreeMap> { + let plan = run(files, p).unwrap(); + applied(files, &plan) + } + + fn app_build() -> BTreeMap> { + fs(&[ ("settings.gradle", "include 'app'\n"), ( "app/build.gradle", "dependencies { implementation 'com.google.code.gson:gson:2.10.1' }\n", ), - ]); + ]) + } + + /// A dependency lock of the build that pins the patched GA at another + /// version than the vendored one wins over the vendored GAV: Gradle + /// resolves the locked version, so liveness (and the planner) withhold + /// attestation. Every lock kind of every project of the build counts; + /// a lock at the vendored version, another GA's lock and a nested build + /// the root does not include do not. A settings-classpath lock is + /// accepted only where the settings script routes it (see + /// `settings_lock_needs_the_committed_in_block_entry`); here it does + /// not, so it is rejected as hosted rejects it. + #[test] + fn lock_at_another_version_withholds_liveness() { let p = patch(); - let plan = run(&files, &p).unwrap(); - let after = applied(&files, &plan); - let live = |files: &BTreeMap>| { - wired( - &|r: &str| files.get(r).cloned(), - &|d: &str| list_of(files, d), - &p.coords(), - ) - }; - let with = |extra: &[(&str, &str)]| { - let mut out = after.clone(); - for (rel, text) in extra { - out.insert(rel.to_string(), text.as_bytes().to_vec()); - } - out - }; - assert!(live(&after)); - const AT: &str = "com.google.code.gson:gson:2.10.1=compileClasspath,runtimeClasspath\n"; - const ABOVE: &str = "com.google.code.gson:gson:2.11.0=compileClasspath,runtimeClasspath\n"; - const BELOW: &str = "com.google.code.gson:gson:2.9.0=runtimeClasspath\n"; + let after = vendored_with(&app_build(), &p); + assert!(live_in(&after, &p)); let other = "org.x:y:1=runtimeClasspath\nempty=\n"; - for (rel, text, consumed) in [ - ("app/gradle.lockfile", AT, true), - ("app/gradle.lockfile", other, true), - ("app/gradle.lockfile", ABOVE, false), - ("app/gradle.lockfile", BELOW, false), - ("gradle.lockfile", ABOVE, false), - ("app/buildscript-gradle.lockfile", ABOVE, false), - ("settings-gradle.lockfile", AT, true), - ("settings-gradle.lockfile", ABOVE, false), + const OVERRIDES: Option<&str> = Some("gradle_lock_overrides_vendored"); + for (rel, text, reason) in [ + ("app/gradle.lockfile", LOCK_AT, None), + ("app/gradle.lockfile", other, None), + ("app/gradle.lockfile", LOCK_ABOVE, OVERRIDES), + ("app/gradle.lockfile", LOCK_BELOW, OVERRIDES), + ("gradle.lockfile", LOCK_ABOVE, OVERRIDES), + ("app/buildscript-gradle.lockfile", LOCK_ABOVE, OVERRIDES), + ( + "settings-gradle.lockfile", + LOCK_AT, + Some("gradle_settings_lock_unrouted"), + ), + ("settings-gradle.lockfile", LOCK_ABOVE, OVERRIDES), ( "app/gradle/dependency-locks/runtimeClasspath.lockfile", "com.google.code.gson:gson:2.11.0\n", - false, + OVERRIDES, ), // A nested build the root does not include governs nothing. - ("samples/demo/gradle.lockfile", ABOVE, true), + ("samples/demo/gradle.lockfile", LOCK_ABOVE, None), ] { - let locked = with(&[(rel, text)]); - assert_eq!(live(&locked), consumed, "{rel}: {text}"); + let locked = with_files(&after, &[(rel, text)]); + assert_eq!(live_in(&locked, &p), reason.is_none(), "{rel}: {text}"); assert_eq!( wired_checked( &|r: &str| locked.get(r).cloned(), &|d: &str| list_of(&locked, d), &p.coords() ), - Ok(consumed), + Ok(reason.is_none()), "{rel}: {text}" ); let replan = run(&locked, &p).unwrap(); - let degraded = replan.warnings.iter().any(|w| { - w.code == DEGRADED - && w.detail - .starts_with("reason: gradle_lock_overrides_vendored: ") - && w.detail.contains(rel) - }); - assert_eq!(degraded, !consumed, "{rel}: {text}"); + assert_eq!( + degraded_reasons(&replan), + reason.into_iter().collect::>(), + "{rel}: {text}" + ); + assert!( + replan + .warnings + .iter() + .all(|w| w.code != DEGRADED || w.detail.contains(rel)), + "{rel}" + ); // A lock never stops the vendoring itself. assert!(replan.writes.is_empty(), "{rel}"); } // Two locks at the vendored version and one above: still not live. - assert!(!live(&with(&[ - ("gradle.lockfile", AT), - ("app/gradle.lockfile", ABOVE), - ]))); - // A lock file that is not UTF-8 proves nothing. + assert!(!live_in( + &with_files( + &after, + &[ + ("gradle.lockfile", LOCK_AT), + ("app/gradle.lockfile", LOCK_ABOVE) + ] + ), + &p + )); + } + + /// A settings-classpath lock at the vendored version is accepted only + /// when the settings script holds the in-block `exclusiveContent` entry + /// for the GAV. Vendoring counts the entry it writes; liveness counts + /// only the committed file, never a re-plan that would add it. + #[test] + fn settings_lock_needs_the_committed_in_block_entry() { + let p = patch(); + let files = with_files( + &app_build(), + &[ + ( + "settings.gradle", + "buildscript { dependencies { classpath 'com.google.code.gson:gson:2.10.1' } }\ninclude 'app'\n", + ), + ("settings-gradle.lockfile", LOCK_AT), + ], + ); + let plan = run(&files, &p).unwrap(); + assert_eq!(degraded_reasons(&plan), Vec::::new()); + let after = applied(&files, &plan); + let entry = in_block_line(false, "", &p.coords()); + let settings = String::from_utf8(after["settings.gradle"].clone()).unwrap(); + assert_eq!(settings.matches(&entry).count(), 1, "{settings}"); + assert!(live_in(&after, &p)); + // Locked at another version: rejected like any lock. + let above = with_files(&after, &[("settings-gradle.lockfile", LOCK_ABOVE)]); + assert!(!live_in(&above, &p)); + // The committed entry is gone: a re-plan would write it again, but + // nothing routes that classpath now. + let stripped = with_files( + &after, + &[("settings.gradle", &settings.replace(&entry, ""))], + ); + assert!(!run(&stripped, &p).unwrap().writes.is_empty()); + assert!(!live_in(&stripped, &p)); + // Without the lock, liveness is unchanged by this check. + let mut unlocked = stripped.clone(); + unlocked.remove("settings-gradle.lockfile"); + assert_eq!( + degraded_reasons( + &plan_as( + &|r: &str| unlocked.get(r).cloned(), + &|d: &str| list_of(&unlocked, d), + &p, + Committed::Only + ) + .unwrap() + ), + Vec::::new() + ); + } + + /// Two vendored versions of one GA (the index is keyed by GAV, and the + /// script routes each): a lock at the other vendored version is served, + /// not an override; a lock at a version neither serves overrides both. + #[test] + fn two_vendored_versions_of_a_ga_do_not_override_each_other() { + let v1 = patch(); + let v2 = JvmPatch { + version: "2.11.0", + uuid: "6f708192-a3b4-4c5d-8e6f-708192a3b4c5", + jar: b"PATCHED-JAR-2", + ..patch() + }; + let files = fs(&[ + ("settings.gradle", "include 'app', 'tools'\n"), + ( + "app/build.gradle", + "dependencies { implementation 'com.google.code.gson:gson:2.10.1' }\n", + ), + ( + "tools/build.gradle", + "dependencies { implementation 'com.google.code.gson:gson:2.11.0' }\n", + ), + ]); + let after = vendored_with(&vendored_with(&files, &v1), &v2); + let locked = with_files( + &after, + &[ + ("app/gradle.lockfile", LOCK_AT), + ("tools/gradle.lockfile", LOCK_ABOVE), + ], + ); + assert!(live_in(&locked, &v1)); + assert!(live_in(&locked, &v2)); + let unserved = with_files( + &locked, + &[( + "tools/gradle.lockfile", + "com.google.code.gson:gson:2.12.0=runtimeClasspath\n", + )], + ); + assert!(!live_in(&unserved, &v1)); + assert!(!live_in(&unserved, &v2)); + } + + /// A lock file the listing shows but that cannot be read as UTF-8 text + /// (an I/O error, a refused symlink out of the checkout, non-UTF-8 + /// bytes) fails closed, with its own reason. + #[test] + fn unreadable_lock_files_fail_closed() { + let p = patch(); + let after = vendored_with(&app_build(), &p); let mut bad = after.clone(); bad.insert("app/gradle.lockfile".into(), b"\xff\xfe\n".to_vec()); - assert!(!live(&bad)); - // A custom lock-file location hides the lock from the check. - let custom = with(&[( - "app/build.gradle", - "dependencyLocking { lockFile = file('locks/app.lockfile') }\n\ - dependencies { implementation 'com.google.code.gson:gson:2.10.1' }\n", - )]); - assert!(!live(&custom)); - assert!(run(&custom, &p).unwrap().warnings.iter().any(|w| { - w.code == DEGRADED && w.detail.starts_with("reason: gradle_custom_lock_file: ") - })); + assert!(!live_in(&bad, &p)); + assert_eq!( + degraded_reasons(&run(&bad, &p).unwrap()), + ["gradle_lock_unreadable"] + ); + // Listed, but the reader refuses it. + let listed = with_files(&after, &[("app/gradle.lockfile", LOCK_AT)]); + let read = |r: &str| { + (r != "app/gradle.lockfile") + .then(|| listed.get(r).cloned()) + .flatten() + }; + let list = |d: &str| list_of(&listed, d); + assert!(!wired(&read, &list, &p.coords())); + let plan = plan(&read, &list, &p).unwrap(); + assert_eq!(degraded_reasons(&plan), ["gradle_lock_unreadable"]); + assert!(plan.warnings[0].detail.contains("app/gradle.lockfile")); + } + + /// A custom `lockFile` location the graph can follow (a literal + /// `file(…)` in the project's own build script) is read and checked + /// like any lock, and fails closed when missing; one it cannot follow + /// withholds attestation. + #[test] + fn custom_lock_file_is_followed_when_static() { + let p = patch(); + let after = vendored_with(&app_build(), &p); + let custom = with_files( + &after, + &[( + "app/build.gradle", + "dependencyLocking { lockFile = file('locks/app.lockfile') }\n\ + dependencies { implementation 'com.google.code.gson:gson:2.10.1' }\n", + )], + ); + let at = with_files(&custom, &[("app/locks/app.lockfile", LOCK_AT)]); + assert!(live_in(&at, &p)); + assert_eq!( + degraded_reasons(&run(&at, &p).unwrap()), + Vec::::new() + ); + let above = with_files(&custom, &[("app/locks/app.lockfile", LOCK_ABOVE)]); + assert!(!live_in(&above, &p)); + assert_eq!( + degraded_reasons(&run(&above, &p).unwrap()), + ["gradle_lock_overrides_vendored"] + ); + assert!(!live_in(&custom, &p)); + assert_eq!( + degraded_reasons(&run(&custom, &p).unwrap()), + ["gradle_lock_unreadable"] + ); + let unfollowed = with_files( + &after, + &[( + "build.gradle", + "subprojects { dependencyLocking { lockFile = file('x.lockfile') } }\n", + )], + ); + assert!(!live_in(&unfollowed, &p)); + assert_eq!( + degraded_reasons(&run(&unfollowed, &p).unwrap()), + ["gradle_custom_lock_file"] + ); } } diff --git a/crates/socket-patch-core/src/vex/discover/gradle.rs b/crates/socket-patch-core/src/vex/discover/gradle.rs index 0ec3c6c7e..538691030 100644 --- a/crates/socket-patch-core/src/vex/discover/gradle.rs +++ b/crates/socket-patch-core/src/vex/discover/gradle.rs @@ -55,7 +55,7 @@ use super::{ }; use crate::gradle::locks; use crate::patch::redirect::gradle::{ - apply_line_digest, graph_of, index_digest, is_settings_lock, lockfile_paths, parse_index, + apply_line_digest, graph_of, index_digest, is_settings_lock, key_list, parse_index, settings_targets, GradleFiles, PinnedRowChecks, HOSTED_INDEX_REL, HOSTED_SCRIPT, HOSTED_SCRIPT_REL, MAX_ROUNDS, }; @@ -105,12 +105,15 @@ pub(crate) async fn extract(ctx: &DiscoverCtx<'_>, out: &mut Discovery) { .custom_lock_file() .map(|rel| format!("{rel} sets a custom dependency-lock file (`lockFile`)")) }); - let locks: Vec<(String, locks::LockState)> = lockfile_paths(&graph, &files) + // The shared lock inventory (`ScriptGraph::read_locks`, also the + // vendored backend's). The listing is the keys of `files`, so every + // listed lock reads; `read_build` leaves an unreadable one out. + let list = |dir: &str| key_list(&files, dir); + let read = |rel: &str| files.get(rel).cloned(); + let locks: Vec<(String, locks::LockState)> = graph + .read_locks(&list, &read) .into_iter() - .filter_map(|rel| { - let state = locks::parse(files.get(&rel)?); - Some((rel, state)) - }) + .filter_map(|(rel, state)| Some((rel, state?))) .collect(); let pinned_checks = PinnedRowChecks::new(&files, &graph); for row in rows { diff --git a/docs/design/maven-vendoring.md b/docs/design/maven-vendoring.md index 240ab9d8e..f656ab5fb 100644 --- a/docs/design/maven-vendoring.md +++ b/docs/design/maven-vendoring.md @@ -169,12 +169,15 @@ Because lockfiles stay unchanged, a lock that pins the patched module at another version wins over the vendored GAV: Gradle resolves the locked version or fails lock validation. Every lock file of every project of the build is read (`gradle.lockfile`, `buildscript-gradle.lockfile`, -`settings-gradle.lockfile`, and the legacy `gradle/dependency-locks/`), and an -entry for the module at any version other than the vendored one is degraded -(`gradle_lock_overrides_vendored`), as is a custom `lockFile` location the -check cannot follow (`gradle_custom_lock_file`). The patch is still applied, -and `vex` does not attest it. A settings classpath lock at the vendored version -is served by the in-block entry, so it is not degraded. +`settings-gradle.lockfile`, the legacy `gradle/dependency-locks/`, and a custom +`lockFile` set as a literal `file(…)` in a project's own build script). The +following are degraded: an entry at a version no vendored row serves +(`gradle_lock_overrides_vendored`; another vendored version of the same module +is served), a settings classpath entry whose settings script lacks the in-block +entry for it (`gradle_settings_lock_unrouted`; liveness requires the committed +entry), a lock file that cannot be read (`gradle_lock_unreadable`), and a +custom `lockFile` that cannot be followed (`gradle_custom_lock_file`). The +patch is still applied, and `vex` does not attest it. The script, the index, the derived metadata and the `.gitattributes` files are owned text and are compared line-ending blind, so a `core.autocrlf` checkout From d093bde75dc5554cec59744a97e937deb74a0387 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Thu, 5 Nov 2026 13:13:06 -0600 Subject: [PATCH 3/5] Judge vendored Gradle liveness on committed wiring only `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 --- crates/socket-patch-cli/CLI_CONTRACT.md | 2 +- .../src/vendor/jvm/gradle.rs | 178 ++++++++++++++++-- docs/design/maven-vendoring.md | 7 + 3 files changed, 173 insertions(+), 14 deletions(-) diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index 696f603b1..defd1469d 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -2631,7 +2631,7 @@ with `reason: : `: |---|---|---| | `vendor_jvm_shape_unsupported` | refusal, nothing written | `gradle_below_6_8`, `android_or_kmp` (also `available-at` module redirects), `gradle_exclusive_content_conflict` (a user rule claiming the module, in any script), `gradle_range_excludes_vendored` (no declared selector admits the vendored version), `gradle_verification_unparseable`, `gradle_index_unreadable`, `build_file_unreadable` (including a non-UTF-8 settings file), `build_file_outside_root`, `not_build_root` (run from a directory an ancestor settings file includes or may include, from a project with no settings file of its own below one, or below an ancestor settings file that is not UTF-8; `repair` refuses there too), `no_build_file` (no `pom.xml`, Gradle, sbt or scala-cli build at the root), `legacy_maven_root` (the root's ledger still holds a pre-v5 single-POM `maven_pom_repository` entry; the whole root is refused until `vendor --revert`, then vendor again) | | `vendor_jvm_upstream_unavailable` | refusal | `classifier_unavailable` (a declared classifier jar no cache or registry has), `module_unavailable` (the pom declares Gradle module metadata that cannot be sourced), `pom_unavailable`, `verification_metadata_unavailable` | -| `vendor_jvm_degraded` | applied, VEX withheld | `gradle_unscanned_build_logic`, `unwired_build_logic` (a nonliteral included build), `settings_plugins_unwired`, `classifier_unpatched_copy` (a classifier jar carries an unpatched copy of a patched member), `verification_parent_chain_unhandled`, `gradle_lock_overrides_vendored` (a dependency lock of the build, buildscript and settings classpath locks included, pins the module at a version no vendored row serves), `gradle_settings_lock_unrouted` (a `settings-gradle.lockfile` locks the module but that build's settings script does not hold the in-block repository entry for it), `gradle_lock_unreadable` (a lock file, or a followed custom `lockFile`, is missing, unreadable, outside the checkout or not UTF-8), `gradle_custom_lock_file` (a `lockFile` location that cannot be followed statically: anything but a literal `file(…)` in a project's own build script), `conflicting_managed_version` / `management_unresolved` (Maven reactor: an imported BOM or an out-of-checkout parent manages the artifact at another version, or its POM is unavailable; that local root is not pinned) | +| `vendor_jvm_degraded` | applied, VEX withheld | `gradle_unscanned_build_logic`, `unwired_build_logic` (a nonliteral included build), `settings_plugins_unwired`, `classifier_unpatched_copy` (a classifier jar carries an unpatched copy of a patched member), `verification_parent_chain_unhandled`, `gradle_lock_overrides_vendored` (a dependency lock of the build, buildscript and settings classpath locks included, pins the module at a version no vendored row serves), `gradle_settings_lock_unrouted` (a `settings-gradle.lockfile` locks the module but that build's settings script does not hold the in-block repository entry for it), `gradle_lock_unreadable` (a lock file, or a followed custom `lockFile`, is missing, unreadable, outside the checkout or not UTF-8), `gradle_custom_lock_file` (a `lockFile` location that cannot be followed statically: anything but a literal `file(…)` in a project's own build script), `gradle_wiring_uncommitted` (VEX liveness only, never reported by `vendor`: the committed settings files, index, derived `maven-metadata.xml`, script or verification file lack wiring a re-plan would write, such as a removed apply line or in-block entry; the git hygiene files do not count), `conflicting_managed_version` / `management_unresolved` (Maven reactor: an imported BOM or an out-of-checkout parent manages the artifact at another version, or its POM is unavailable; that local root is not pinned) | | `vendor_jvm_note` | applied, informational | `range_declared` (a range, prefix or rich selector lists versions from the derived `maven-metadata.xml`), `ide_sources_unavailable` | | `vendor_jvm_upstream_unverified` | warning | Upstream metadata was taken offline and not authenticated against registry checksums. | diff --git a/crates/socket-patch-core/src/vendor/jvm/gradle.rs b/crates/socket-patch-core/src/vendor/jvm/gradle.rs index 5edab54f0..eaea4f9c3 100644 --- a/crates/socket-patch-core/src/vendor/jvm/gradle.rs +++ b/crates/socket-patch-core/src/vendor/jvm/gradle.rs @@ -649,9 +649,14 @@ fn plan_as( writes.push(text_write(VERIFICATION_REL, new.into_bytes())); } + let tree_files = super::tree_files(&writes); + let writes = finish_writes(read, writes); + if committed == Committed::Only { + uncommitted_wiring(&writes, g, a, v, &mut warnings); + } Ok(JvmPlan { - tree_files: super::tree_files(&writes), - writes: finish_writes(read, writes), + tree_files, + writes, records, warnings, jar_rel: format!("{tree_dir}/{jar_name}"), @@ -659,6 +664,51 @@ fn plan_as( }) } +/// Owned files that only keep the wiring intact through git (EOL +/// conversion, an ignored `*.jar`); Gradle never reads them, so a missing +/// one changes nothing this checkout resolves. +const GIT_HYGIENE: &[&str] = &[ + GITATTRIBUTES_REL, + GITIGNORE_REL, + SCRIPT_GITATTRIBUTES_REL, + VENDOR_GITATTRIBUTES_REL, +]; + +/// Liveness ([`Committed::Only`]): a re-plan over the committed tree that +/// still writes wiring proves that wiring is not committed, and the build +/// may resolve another copy. Every pending write counts except the tree +/// itself (rebuilt from the committed bytes; the script checks its hashes +/// against the index, whose writes do count) and [`GIT_HYGIENE`]: the +/// settings files (apply lines of every wired build, in-block +/// `buildscript` / `pluginManagement` entries), the index, the derived +/// `maven-metadata.xml`, the script and `gradle/verification-metadata.xml`. +fn uncommitted_wiring( + writes: &[FileWrite], + g: &str, + a: &str, + v: &str, + warnings: &mut Vec, +) { + let pending: Vec<&str> = writes + .iter() + .filter(|w| !w.tree && !GIT_HYGIENE.contains(&w.rel.as_str())) + .map(|w| w.rel.as_str()) + .collect(); + if let Some(first) = pending.first() { + warnings.push(degraded( + "gradle_wiring_uncommitted", + format!( + "{first}{} lacks wiring a re-plan would write for {g}:{a}:{v}, so the build may \ + resolve a copy other than the vendored one; re-run `socket-patch vendor`", + match pending.len() { + 1 => String::new(), + n => format!(" (and {} more)", n - 1), + } + ), + )); + } +} + /// The whole-graph checks: Android / KMP and a user `exclusiveContent` /// claiming the GA anywhere refuse, naming the file (#461); a declaration /// shape the tree cannot serve refuses or warns (#511, #533); build logic @@ -1443,7 +1493,11 @@ pub fn references_checked(read: ReadFn<'_>, c: &Coords<'_>) -> Option { /// The liveness proof `vex` needs for this layout: `c` is /// [`references`]d, the script is ours (line endings aside), and a re-plan -/// over the committed tree is refused nowhere and degraded nowhere. +/// over the committed tree is refused nowhere and degraded nowhere. The +/// re-plan runs as [`Committed::Only`]: it counts only committed in-block +/// entries, and any wiring it would still write degrades it +/// ([`uncommitted_wiring`]), so a pending write never stands in for a +/// missing committed one. pub fn wired(read: ReadFn<'_>, list: ListFn<'_>, c: &Coords<'_>) -> bool { if !references(read, c) || !is_ours(read(SCRIPT_REL).as_deref(), SCRIPT) { return false; @@ -5050,21 +5104,27 @@ mod tests { ); assert!(!run(&stripped, &p).unwrap().writes.is_empty()); assert!(!live_in(&stripped, &p)); - // Without the lock, liveness is unchanged by this check. - let mut unlocked = stripped.clone(); - unlocked.remove("settings-gradle.lockfile"); - assert_eq!( + let liveness_reasons = |files: &BTreeMap>| { degraded_reasons( &plan_as( - &|r: &str| unlocked.get(r).cloned(), - &|d: &str| list_of(&unlocked, d), + &|r: &str| files.get(r).cloned(), + &|d: &str| list_of(files, d), &p, - Committed::Only + Committed::Only, ) - .unwrap() - ), - Vec::::new() + .unwrap(), + ) + }; + assert_eq!( + liveness_reasons(&stripped), + ["gradle_settings_lock_unrouted", "gradle_wiring_uncommitted"] ); + // Without the lock the classpath is still unrouted: the pending + // in-block write alone withholds it. + let mut unlocked = stripped.clone(); + unlocked.remove("settings-gradle.lockfile"); + assert!(!live_in(&unlocked, &p)); + assert_eq!(liveness_reasons(&unlocked), ["gradle_wiring_uncommitted"]); } /// Two vendored versions of one GA (the index is keyed by GAV, and the @@ -5185,4 +5245,96 @@ mod tests { ["gradle_custom_lock_file"] ); } + + /// Liveness judges the committed wiring, never what a re-plan would + /// write: with no lock at all, a settings classpath on the GA whose + /// in-block entry is gone, a settings `plugins{}` block added since + /// vendoring, a `buildSrc` or included build that lost its apply line, + /// a deleted derived `maven-metadata.xml`, a verification file without + /// the patched hashes and an index row whose hash no longer matches + /// the tree each withhold attestation. The git hygiene files are not + /// wiring Gradle reads, so a missing one does not. Vendoring's own + /// first run still counts the entries it writes. + #[test] + fn liveness_judges_committed_wiring_not_a_replan() { + let p = patch(); + let settings = + "buildscript { dependencies { classpath 'com.google.code.gson:gson:2.10.1' } }\n\ + includeBuild('tools')\ninclude 'app'\n"; + let verification = format!("{VM_HEAD}{VM_TAIL}"); + let files = with_files( + &app_build(), + &[ + ("settings.gradle", settings), + ("tools/build.gradle", ""), + ("buildSrc/build.gradle", ""), + (VERIFICATION_REL, &verification), + ], + ); + let plan = run(&files, &p).unwrap(); + assert_eq!(degraded_reasons(&plan), Vec::::new()); + let after = applied(&files, &plan); + assert!(live_in(&after, &p)); + let text = |rel: &str| String::from_utf8(after[rel].clone()).unwrap(); + let entry = in_block_line(false, "", &p.coords()); + let committed = text("settings.gradle"); + assert_eq!(committed.matches(&entry).count(), 1, "{committed}"); + let metadata = derived_metadata_rel("com.google.code.gson", "gson"); + let index = text(INDEX_REL); + let jar_sha = sha256_hex(p.jar); + assert!(index.contains(&jar_sha)); + let tampered_index = index.replace(&jar_sha, &"0".repeat(64)); + let plugins = format!("plugins {{ id 'com.example.p' version '1' }}\n{committed}"); + let mut cases: Vec<(&str, BTreeMap>)> = vec![ + ( + "settings in-block entry", + with_files( + &after, + &[("settings.gradle", &committed.replace(&entry, ""))], + ), + ), + ( + "settings plugins added", + with_files(&after, &[("settings.gradle", &plugins)]), + ), + ( + "included build apply line", + with_files(&after, &[("tools/settings.gradle", "")]), + ), + ( + "buildSrc apply line", + with_files(&after, &[("buildSrc/settings.gradle", "")]), + ), + ( + "verification hashes", + with_files(&after, &[(VERIFICATION_REL, &verification)]), + ), + ( + "index hash", + with_files(&after, &[(INDEX_REL, &tampered_index)]), + ), + ]; + let mut no_metadata = after.clone(); + no_metadata.remove(&metadata); + cases.push(("derived metadata", no_metadata)); + for (what, broken) in &cases { + assert!( + references(&|r: &str| broken.get(r).cloned(), &p.coords()), + "{what}" + ); + // Vendoring there repairs it. + assert!(!run(broken, &p).unwrap().writes.is_empty(), "{what}"); + } + let masked: Vec<&str> = cases + .iter() + .filter(|(_, broken)| live_in(broken, &p)) + .map(|(what, _)| *what) + .collect(); + assert_eq!(masked, Vec::<&str>::new()); + for hygiene in [GITIGNORE_REL, GITATTRIBUTES_REL, VENDOR_GITATTRIBUTES_REL] { + let mut missing = after.clone(); + missing.remove(hygiene); + assert!(live_in(&missing, &p), "{hygiene}"); + } + } } diff --git a/docs/design/maven-vendoring.md b/docs/design/maven-vendoring.md index f656ab5fb..008342287 100644 --- a/docs/design/maven-vendoring.md +++ b/docs/design/maven-vendoring.md @@ -179,6 +179,13 @@ entry), a lock file that cannot be read (`gradle_lock_unreadable`), and a custom `lockFile` that cannot be followed (`gradle_custom_lock_file`). The patch is still applied, and `vex` does not attest it. +`vex` liveness re-plans over the committed tree but judges only the committed +files: a re-plan that would still write a settings file (an apply line of any +wired build, an in-block entry), the index, the derived `maven-metadata.xml`, +the script or the verification file means that wiring is missing, and the +entry is not attested (`gradle_wiring_uncommitted`). The `.gitattributes` and +`.gitignore` files are not read by Gradle and do not count. + The script, the index, the derived metadata and the `.gitattributes` files are owned text and are compared line-ending blind, so a `core.autocrlf` checkout passes `--check` and reverts clean. `.socket/gradle/.gitattributes` (`* -text`, From 6932ced514678cfc4539be977d48e71a88ee2603 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 10:24:55 +0000 Subject: [PATCH 4/5] Treat project(...) blocks with long argument lists as foreign 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 --- crates/socket-patch-core/src/gradle/graph.rs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/crates/socket-patch-core/src/gradle/graph.rs b/crates/socket-patch-core/src/gradle/graph.rs index 70a93c622..0e96975e6 100644 --- a/crates/socket-patch-core/src/gradle/graph.rs +++ b/crates/socket-patch-core/src/gradle/graph.rs @@ -1386,7 +1386,16 @@ impl ScriptGraph { (0..toks.len()) .filter(|&i| is_ident(toks.get(i), "project")) .filter_map(|i| { - let open = (i + 1..toks.len().min(i + 8)) + // Skip the whole `project(…)` argument list, + // however long, so a named-argument call + // (`project(path: ':a', configuration: 'x') {`) + // still counts as configuring another project. + let after = if is_punct(toks.get(i + 1), b'(') { + matching_close(&toks, i + 1)? + 1 + } else { + i + 1 + }; + let open = (after..toks.len().min(after + 4)) .find(|&j| is_punct(toks.get(j), b'{'))?; matching_close(&toks, open).map(|c| (open, c)) }), @@ -2814,6 +2823,8 @@ other = { group = \"x\", name = \"y\", version = \"1\" } "subprojects { dependencyLocking { lockFile = file('x.lockfile') } }\n", "allprojects { dependencyLocking { lockFile = file('x.lockfile') } }\n", "project(':app') { dependencyLocking { lockFile = file('x.lockfile') } }\n", + "project(path: ':app', configuration: 'default') { dependencyLocking { lockFile = file('x.lockfile') } }\n", + "project(':app').afterEvaluate { dependencyLocking { lockFile = file('x.lockfile') } }\n", ] { let files = [ ("settings.gradle", "include 'app'\n"), From a0f921949e44bdf99ec0763c575426642296b29f Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 18:58:36 +0000 Subject: [PATCH 5/5] Follow configure() and hook blocks; keep regenerated verification live MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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() { … }`), 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 --- crates/socket-patch-core/src/gradle/graph.rs | 79 +++++++++++++------ .../src/vendor/jvm/gradle.rs | 73 ++++++++++++++++- 2 files changed, 128 insertions(+), 24 deletions(-) diff --git a/crates/socket-patch-core/src/gradle/graph.rs b/crates/socket-patch-core/src/gradle/graph.rs index 0e96975e6..c52d96e20 100644 --- a/crates/socket-patch-core/src/gradle/graph.rs +++ b/crates/socket-patch-core/src/gradle/graph.rs @@ -1378,28 +1378,43 @@ impl ScriptGraph { }) .flatten(); // Blocks that configure other projects resolve `file(…)` - // against those. - let foreign: Vec<(usize, usize)> = ["allprojects", "subprojects", "configure"] - .iter() - .flat_map(|name| all_blocks(&toks, name)) - .chain( - (0..toks.len()) - .filter(|&i| is_ident(toks.get(i), "project")) - .filter_map(|i| { - // Skip the whole `project(…)` argument list, - // however long, so a named-argument call - // (`project(path: ':a', configuration: 'x') {`) - // still counts as configuring another project. - let after = if is_punct(toks.get(i + 1), b'(') { - matching_close(&toks, i + 1)? + 1 - } else { - i + 1 - }; - let open = (after..toks.len().min(after + 4)) - .find(|&j| is_punct(toks.get(j), b'{'))?; - matching_close(&toks, open).map(|c| (open, c)) - }), - ) + // against those: `allprojects { … }`, `subprojects { … }`, + // `configure() { … }`, `project(':x') { … }` (also + // `project(':x').afterEvaluate { … }`) and the per-project + // hooks (`gradle.beforeProject { … }`, `afterProject`). The + // block must open right after the name and its argument list + // (or one `.method`), so `implementation(project(':lib'))` + // followed by the script's next block is not one. + const FOREIGN: [&str; 6] = [ + "allprojects", + "subprojects", + "configure", + "project", + "beforeProject", + "afterProject", + ]; + let foreign: Vec<(usize, usize)> = (0..toks.len()) + .filter(|&i| FOREIGN.iter().any(|name| is_ident(toks.get(i), name))) + .filter_map(|i| { + // Skip the whole argument list, however long, so a + // named-argument call (`project(path: ':a', …) {`) + // still counts. + let mut after = if is_punct(toks.get(i + 1), b'(') { + matching_close(&toks, i + 1)? + 1 + } else { + i + 1 + }; + if is_punct(toks.get(after), b'.') + && matches!( + toks.get(after + 1).map(|t| &t.tok), + Some(dsl::Tok::Ident(_)) + ) + { + after += 2; + } + let open = Some(after).filter(|&j| is_punct(toks.get(j), b'{'))?; + matching_close(&toks, open).map(|c| (open, c)) + }) .collect(); for i in 0..toks.len() { if !is_ident(toks.get(i), "lockFile") { @@ -2825,6 +2840,10 @@ other = { group = \"x\", name = \"y\", version = \"1\" } "project(':app') { dependencyLocking { lockFile = file('x.lockfile') } }\n", "project(path: ':app', configuration: 'default') { dependencyLocking { lockFile = file('x.lockfile') } }\n", "project(':app').afterEvaluate { dependencyLocking { lockFile = file('x.lockfile') } }\n", + "configure(allprojects) { dependencyLocking { lockFile = file('x.lockfile') } }\n", + "configure(subprojects.findAll { it.name != 'x' }) { dependencyLocking { lockFile = file('x.lockfile') } }\n", + "gradle.beforeProject { dependencyLocking { lockFile = file('x.lockfile') } }\n", + "gradle.afterProject { p -> dependencyLocking { lockFile = file('x.lockfile') } }\n", ] { let files = [ ("settings.gradle", "include 'app'\n"), @@ -2832,6 +2851,22 @@ other = { group = \"x\", name = \"y\", version = \"1\" } ]; assert_eq!(path_of(&files), vec![None], "{text}"); } + // A project DEPENDENCY before the script's own block is not a block + // configuring that project. + for text in [ + "dependencies { implementation(project(\":lib\")) }\ndependencyLocking { lockFile.set(file(\"locks/app.lockfile\")) }\n", + "dependencies {\n implementation project(':lib')\n}\ndependencyLocking { lockFile = file('locks/app.lockfile') }\n", + ] { + let files = [ + ("settings.gradle", "include 'app', 'lib'\n"), + ("app/build.gradle", text), + ]; + assert_eq!( + path_of(&files), + vec![Some("app/locks/app.lockfile".to_string())], + "{text}" + ); + } let applied = [ ("build.gradle", "apply from: 'locks.gradle'\n"), ( diff --git a/crates/socket-patch-core/src/vendor/jvm/gradle.rs b/crates/socket-patch-core/src/vendor/jvm/gradle.rs index eaea4f9c3..9571e86e9 100644 --- a/crates/socket-patch-core/src/vendor/jvm/gradle.rs +++ b/crates/socket-patch-core/src/vendor/jvm/gradle.rs @@ -652,7 +652,19 @@ fn plan_as( let tree_files = super::tree_files(&writes); let writes = finish_writes(read, writes); if committed == Committed::Only { - uncommitted_wiring(&writes, g, a, v, &mut warnings); + // The committed verification file already pins the patched jar + // (in Gradle's own form, say): a re-plan rewriting it in ours adds + // no wiring. + let jar_sha = sha256_hex(patch.jar); + let verified = read(VERIFICATION_REL) + .and_then(|bytes| String::from_utf8(bytes).ok()) + .is_some_and(|text| text.contains(&format!("value=\"{jar_sha}\""))); + let counted: Vec = writes + .iter() + .filter(|w| !(verified && w.rel == VERIFICATION_REL)) + .cloned() + .collect(); + uncommitted_wiring(&counted, g, a, v, &mut warnings); } Ok(JvmPlan { tree_files, @@ -681,7 +693,10 @@ const GIT_HYGIENE: &[&str] = &[ /// against the index, whose writes do count) and [`GIT_HYGIENE`]: the /// settings files (apply lines of every wired build, in-block /// `buildscript` / `pluginManagement` entries), the index, the derived -/// `maven-metadata.xml`, the script and `gradle/verification-metadata.xml`. +/// `maven-metadata.xml`, the script and `gradle/verification-metadata.xml` +/// (unless the committed file already pins the patched jar's sha256 in +/// some form: Gradle's `--write-verification-metadata` rewrites the entry +/// its own way, and the caller drops that write before this check). fn uncommitted_wiring( writes: &[FileWrite], g: &str, @@ -904,6 +919,9 @@ fn check_locks( match c.path { // Set explicitly, so expected to exist: a missing one is // unreadable too (fail closed). + // A path already listed (the default name, or reached from two + // scripts) is checked once. + Some(path) if lock_files.iter().any(|(rel, _)| *rel == path) => {} Some(path) => { let state = text(&path).map(|t| locks::parse(&t)); lock_files.push((path, state)); @@ -3381,6 +3399,38 @@ mod tests { assert_idempotent(&files, &p); } + /// Review on #2004 (round 2): Gradle's own `--write-verification-metadata` + /// (the remedy `verification_parent_chain_unhandled` names) rewrites + /// the jar entry in its own form with the same patched hash. A pending + /// verification-file write never makes the build resolve another copy + /// (a missing or wrong hash fails verification instead), so it does + /// not withhold liveness. + #[test] + fn a_regenerated_verification_entry_keeps_the_wiring_live() { + let before = format!( + "{VM_HEAD}{}{VM_TAIL}", + vm_component( + "com.google.code.gson", + "gson", + "2.10.1", + &[("gson-2.10.1.jar", "old"), ("gson-2.10.1.pom", "pomsha")] + ), + ); + let files = fs(&[("settings.gradle", ""), (VERIFICATION_REL, &before)]); + let p = JvmPatch { + upstream_pom: GSON_POM, + ..patch() + }; + let plan = run(&files, &p).unwrap(); + let after = applied(&files, &plan); + assert!(live_in(&after, &p)); + let vm = String::from_utf8(after[VERIFICATION_REL].clone()).unwrap(); + let regenerated = vm.replace("origin=\"socket-patch\"", "origin=\"Generated by Gradle\""); + assert_ne!(regenerated, vm); + let after = with_files(&after, &[(VERIFICATION_REL, ®enerated)]); + assert!(live_in(&after, &p)); + } + /// #646 review: a `core.autocrlf` checkout converts the verification /// file vendored with LF; the revert still takes the patched jar hash /// (and any other recorded entry) out, in the file's own newline, @@ -5227,6 +5277,25 @@ mod tests { degraded_reasons(&run(&above, &p).unwrap()), ["gradle_lock_overrides_vendored"] ); + // A custom path that is the default lock is checked once. + let default = with_files( + &after, + &[ + ( + "app/build.gradle", + "dependencyLocking { lockFile = file('gradle.lockfile') }\n\ + dependencies { implementation 'com.google.code.gson:gson:2.10.1' }\n", + ), + ("app/gradle.lockfile", LOCK_ABOVE), + ], + ); + let plan = run(&default, &p).unwrap(); + assert_eq!(degraded_reasons(&plan), ["gradle_lock_overrides_vendored"]); + assert!( + !plan.warnings[0].detail.contains("more)"), + "{}", + plan.warnings[0].detail + ); assert!(!live_in(&custom, &p)); assert_eq!( degraded_reasons(&run(&custom, &p).unwrap()),