From 13411d7dcdb0b34b12de10fc9b1c6860dd0b5352 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Fri, 9 Oct 2026 14:03:09 -0400 Subject: [PATCH 1/3] Weigh BOM and parent management before pinning The vendored Maven reactor pinned every wired local root, and a local dependencyManagement pin beats both imported BOMs and parents resolved outside the checkout. A project whose corporate BOM or Spring Boot parent managed commons-text at 1.11.0 was silently moved to 1.10.0-socket.*, vex attested it, and a later BOM bump never took effect because re-runs reported the pin in sync. Before pinning a root the planner now reads the external parent chain and every import-scope BOM (local caches or the registry, like other upstream metadata), interpolating properties in Maven's order. Management or an inherited literal at another version leaves the root unpinned with conflicting_managed_version; metadata that cannot be read leaves it unpinned with management_unresolved. A re-run after a BOM bump drops the pin. vendor --check reads the metadata from the local repository only and accepts either decision when some of it is missing. Fixes #488. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/socket-patch-cli/CLI_CONTRACT.md | 2 +- .../socket-patch-core/src/vendor/jvm/apply.rs | 53 +- .../src/vendor/jvm/maven_reactor.rs | 692 ++++++++++++++++++ .../socket-patch-core/src/vendor/jvm/mod.rs | 68 +- .../src/vendor/maven_repo.rs | 36 + docs/design/maven-vendoring.md | 14 + 6 files changed, 858 insertions(+), 7 deletions(-) diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index bb4592f4b..dfd44c504 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -2226,7 +2226,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` | +| `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_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/apply.rs b/crates/socket-patch-core/src/vendor/jvm/apply.rs index c5d66f615..d66788f7d 100644 --- a/crates/socket-patch-core/src/vendor/jvm/apply.rs +++ b/crates/socket-patch-core/src/vendor/jvm/apply.rs @@ -30,9 +30,9 @@ use super::super::state::{VendorEntry, WiringAction, WiringRecord}; use super::super::{RevertOpts, RevertOutcome, VendorWarning}; use super::{ coursier_tree, gradle, layout, maven_reactor, op_of, op_str, sbt, scala_cli, sha256_hex, - Coords, JvmPlan, JvmUnplan, Shape, CONFIG_LINE_KIND, COURSIER_INDEX_KIND, CREATED_DIR_KIND, - DERIVED_METADATA_KIND, KINDS, OWNED_FILE_KIND, POM_FRAGMENT_KIND, SBT_FRAGMENT_KIND, - SETTINGS_FRAGMENT_KIND, TREE_KIND, VERIFICATION_FRAGMENT_KIND, + Coords, JvmPlan, JvmUnplan, ReadFn, Shape, CONFIG_LINE_KIND, COURSIER_INDEX_KIND, + CREATED_DIR_KIND, DERIVED_METADATA_KIND, KINDS, OWNED_FILE_KIND, POM_FRAGMENT_KIND, + SBT_FRAGMENT_KIND, SETTINGS_FRAGMENT_KIND, TREE_KIND, VERIFICATION_FRAGMENT_KIND, }; /// Whether `entry` was written by this backend: it has wiring and every @@ -865,6 +865,33 @@ pub fn entry_wired_checked(root: &Path, entry: &VendorEntry) -> Result, + patch: &super::JvmPatch<'_>, + local_repo: Option<&Path>, +) -> maven_reactor::ExternalPoms { + let mut known = maven_reactor::ExternalPoms::new(); + loop { + let need = maven_reactor::external_poms_needed(read, patch, &known); + if need.is_empty() { + return known; + } + for (g, a, v) in need { + let bytes = local_repo.and_then(|repo| { + let path = repo + .join(g.replace('.', "/")) + .join(&a) + .join(&v) + .join(format!("{a}-{v}.pom")); + read_regular_to_bytes_sync(&path).ok() + }); + known.insert((g, a, v), bytes); + } + } +} + /// Verify every recorded file plus the effective wiring, without writes or network I/O. pub fn check_entry( root: &Path, @@ -944,8 +971,26 @@ pub fn check_entry( patched_members: &patched, }; let config = !entry.wiring.iter().any(|w| op_of(w) == "config_none"); - let plan = super::plan_with_config(shape_of(&entry.wiring), &read, &list, &patch, config) + // The management a Maven pin would override from outside the checkout + // (#488), read from the local repository only (no network). When some + // of it is not there, either decision `vendor` could have made is in + // sync: the pin it wrote when it could read it, or none. + let shape = shape_of(&entry.wiring); + let external = if matches!(shape, Shape::MavenReactor | Shape::Mixed) { + Some(local_external_poms(&read, &patch, local_repo)) + } else { + None + }; + let plan = super::plan_with_external(shape, &read, &list, &patch, config, external.as_ref()) .map_err(|e| e.detail)?; + let incomplete = external + .as_ref() + .is_some_and(|known| known.values().any(Option::is_none)); + let plan = if incomplete && !plan.writes.is_empty() { + super::plan_with_config(shape, &read, &list, &patch, config).map_err(|e| e.detail)? + } else { + plan + }; if let Some(w) = plan.writes.first() { return Err(format!("vendored wiring or metadata drifted: {}", w.rel)); } diff --git a/crates/socket-patch-core/src/vendor/jvm/maven_reactor.rs b/crates/socket-patch-core/src/vendor/jvm/maven_reactor.rs index e1e2a0e2d..2d07d07c1 100644 --- a/crates/socket-patch-core/src/vendor/jvm/maven_reactor.rs +++ b/crates/socket-patch-core/src/vendor/jvm/maven_reactor.rs @@ -104,6 +104,20 @@ pub fn plan_with_config( read: ReadFn<'_>, patch: &JvmPatch<'_>, config_enabled: bool, +) -> Result { + plan_with_external(read, patch, config_enabled, None) +} + +/// [`plan_with_config`] that also weighs the management a local root's pin +/// would override from outside the checkout: imported BOMs and parents +/// resolved from a repository (#488). `external` holds those poms as the +/// caller could fetch them ([`external_poms_needed`] says which); `None` +/// skips the check (callers that plan only to probe a shape). +pub fn plan_with_external( + read: ReadFn<'_>, + patch: &JvmPatch<'_>, + config_enabled: bool, + external: Option<&ExternalPoms>, ) -> Result { let (g, a, v) = (patch.group_id, patch.artifact_id, patch.version); if !safe_coordinates(g, a, v) { @@ -149,6 +163,51 @@ pub fn plan_with_config( ); } + if let Some(external) = external { + let lookup = |gav: &Gav| match external.get(gav) { + Some(Some(bytes)) => Lookup::Found(bytes.as_slice()), + _ => Lookup::Unavailable, + }; + for root in reactor.wired_roots() { + if unpinned.contains(&root) || managed_roots.contains(&root) { + continue; + } + match reactor.external_management(&root, patch, &lookup) { + Ok(None) => {} + Ok(Some(conflict)) => { + warnings.push(degraded( + "conflicting_managed_version", + format!( + "{}: {}:{} is {} at {}, not {}; {root} is not pinned, so the build \ + keeps resolving {}", + conflict.at, + patch.group_id, + patch.artifact_id, + conflict.how, + conflict.version, + patch.version, + conflict.version + ), + )); + unpinned.insert(root); + } + Err(Missing::Unresolved(why) | Missing::Need(_, why)) => { + warnings.push(degraded( + "management_unresolved", + format!( + "{why}, so whether a pin of {}:{}:{} in {root} would override a \ + different managed version cannot be told; {root} is not pinned \ + (resolve the project once, e.g. `mvn -q dependency:resolve`, \ + and vendor again)", + patch.group_id, patch.artifact_id, patch.version + ), + )); + unpinned.insert(root); + } + } + } + } + let banning = reactor .scope .iter() @@ -609,6 +668,411 @@ pub fn contains_module(read: ReadFn<'_>, rel: &str) -> bool { pub(crate) type Gav = (String, String, String); +/// Poms from outside the checkout (parents, imported BOMs) by GAV, as a +/// caller fetched them: `None` when looked up and unavailable. +pub type ExternalPoms = BTreeMap<(String, String, String), Option>>; + +/// Most external poms one plan reads (a BOM graph is shallow; Spring Boot's +/// imports a few dozen). +const MAX_EXTERNAL_POMS: usize = 256; +/// Deepest parent chain / import nesting followed outside the checkout. +const MAX_EXTERNAL_DEPTH: usize = 16; + +/// An external pom as a lookup sees it. +enum Lookup<'b> { + Found(&'b [u8]), + /// Looked up and unavailable (in no local cache, not fetchable). + Unavailable, + /// Not looked up yet: [`external_poms_needed`] collects these. + Unknown, +} + +/// Why external management could not be decided: an unresolvable value, +/// or a pom (with what it was needed for) that is unavailable or not yet +/// looked up. +enum Missing { + Unresolved(String), + Need(Gav, String), +} + +/// Management from outside the checkout that disagrees with the patch. +struct ManagedConflict { + /// The reactor pom whose effective model it is. + at: String, + /// How it is managed (`managed by the imported BOM g:a:v`, …). + how: String, + version: String, +} + +/// The external poms [`plan_with_external`] still needs for `patch`, given +/// `known` (fetched or found unavailable): call until it returns nothing, +/// fetching each, then plan. Empty when the reactor cannot be read (the +/// plan refuses it on its own). +pub fn external_poms_needed( + read: ReadFn<'_>, + patch: &JvmPatch<'_>, + known: &ExternalPoms, +) -> Vec { + let Ok(reactor) = Reactor::discover(read) else { + return Vec::new(); + }; + if known.len() >= MAX_EXTERNAL_POMS { + return Vec::new(); + } + let lookup = |gav: &Gav| match known.get(gav) { + Some(Some(bytes)) => Lookup::Found(bytes.as_slice()), + Some(None) => Lookup::Unavailable, + None => Lookup::Unknown, + }; + let mut need = Vec::new(); + for root in reactor.wired_roots() { + if let Err(Missing::Need(gav, _)) = reactor.external_management(&root, patch, &lookup) { + if !known.contains_key(&gav) && !need.contains(&gav) { + need.push(gav); + } + } + } + need +} + +/// One pom outside the checkout, parsed. +struct ExternalPom { + gav: Gav, + pom: Pom, +} + +/// `${name}` interpolation through `lookup`; `None` when a name is +/// undefined or the nesting is too deep. +fn interpolate_by( + value: &str, + lookup: &dyn Fn(&str) -> Option, + depth: usize, +) -> Option { + if depth > MAX_INTERPOLATION_DEPTH { + return None; + } + let mut out = String::new(); + let mut rest = value; + while let Some(at) = rest.find("${") { + out.push_str(&rest[..at]); + let after = &rest[at + 2..]; + let close = after.find('}')?; + let raw = lookup(&after[..close])?; + out.push_str(&interpolate_by(&raw, lookup, depth + 1)?); + rest = &after[close + 1..]; + } + out.push_str(rest); + Some(out) +} + +impl Reactor { + /// Management of `patch`'s g:a that a pin in local root `root` would + /// override and that does not manage it at the patch's base version: + /// for each pom resolving through `root` whose own chain declares no + /// version for g:a in the checkout, the version an external parent + /// chain declares or manages, else the first imported BOM (the + /// project's own imports, then the external parents') that manages it. + /// `Ok(None)` when nothing outside the checkout manages g:a, or only at + /// the base version. + fn external_management<'b>( + &self, + root: &str, + patch: &JvmPatch<'_>, + lookup: &dyn Fn(&Gav) -> Lookup<'b>, + ) -> Result, Missing> { + let (g, a) = (patch.group_id, patch.artifact_id); + let ext_chain = self.external_chain(root, lookup)?; + for rel in self.scope.iter().filter(|rel| self.local_root(rel) == root) { + let locally_versioned = self.chain(rel).any(|p| { + let doc = &self.poms[p].doc; + doc.keyed_declarations(g, a) + .iter() + .any(|(dep, _)| doc.child(*dep, "version").is_some()) + }); + if locally_versioned { + continue; + } + // Maven interpolates after inheritance: the reactor pom's own + // chain overrides an external parent's properties. + let props = |name: &str| -> Option { + self.lookup(rel, name).or_else(|| { + ext_chain + .iter() + .find_map(|e| e.pom.props.get(name).cloned()) + }) + }; + let resolve = |value: &str, what: &str| -> Result { + interpolate_by(value, &props, 0).ok_or_else(|| { + Missing::Unresolved(format!("{rel}: {what} {value} is not defined")) + }) + }; + // An external parent's own declaration or management of g:a. + for ext in &ext_chain { + let doc = &ext.pom.doc; + let (eg, ea, ev) = &ext.gav; + if let Some((dep, managed)) = jar_declaration(doc, g, a) { + let Some(version) = doc.child_text(dep, "version") else { + continue; + }; + let v = resolve( + &version, + &format!("{g}:{a} version in parent {eg}:{ea}:{ev}"), + )?; + // An inherited `` literal is never + // overridden by management, so a pin cannot reach it + // even at the base version. + if managed && is_base_like(&v, patch.version) { + return Ok(None); + } + let how = if managed { + "managed by" + } else { + "declared (a literal no pin overrides) by" + }; + return Ok(Some(ManagedConflict { + at: rel.clone(), + how: format!("{how} the parent {eg}:{ea}:{ev}"), + version: v, + })); + } + } + // Imported BOMs: the reactor chain's own (nearest first), then + // the external parents'. + let mut imports: Vec<(Gav, String)> = Vec::new(); + for p in self.chain(rel) { + for (ig, ia, iv) in imports_of(&self.poms[p].doc) { + let what = format!("the BOM import {ig}:{ia} in {p}"); + let gav = ( + resolve(&ig, &what)?, + resolve(&ia, &what)?, + resolve(&iv, &what)?, + ); + imports.push((gav, p.to_string())); + } + } + for ext in &ext_chain { + for (ig, ia, iv) in imports_of(&ext.pom.doc) { + let (eg, ea, ev) = &ext.gav; + let what = format!("the BOM import {ig}:{ia} in parent {eg}:{ea}:{ev}"); + let gav = ( + resolve(&ig, &what)?, + resolve(&ia, &what)?, + resolve(&iv, &what)?, + ); + imports.push((gav, format!("{eg}:{ea}:{ev}"))); + } + } + for (bom, _) in &imports { + let mut seen = BTreeSet::new(); + if let Some(v) = bom_manages(bom, g, a, lookup, &mut seen, 0)? { + if is_base_like(&v, patch.version) { + return Ok(None); + } + let (bg, ba, bv) = bom; + return Ok(Some(ManagedConflict { + at: rel.clone(), + how: format!("managed by the imported BOM {bg}:{ba}:{bv}"), + version: v, + })); + } + } + } + Ok(None) + } + + /// The parents of local root `root` outside the checkout, nearest + /// first. + fn external_chain<'b>( + &self, + root: &str, + lookup: &dyn Fn(&Gav) -> Lookup<'b>, + ) -> Result, Missing> { + let mut chain: Vec = Vec::new(); + let mut parent = self.poms[root].parent.as_ref().map(|p| { + let local = + |v: &Option| v.as_deref().and_then(|v| self.interpolate(root, v, 0)); + (local(&p.group), local(&p.artifact), local(&p.version)) + }); + while let Some((pg, pa, pv)) = parent { + let (Some(pg), Some(pa), Some(pv)) = (pg, pa, pv) else { + return Err(Missing::Unresolved(format!( + "a parent of {root} has no literal groupId/artifactId/version" + ))); + }; + if chain.len() >= MAX_EXTERNAL_DEPTH { + return Err(Missing::Unresolved(format!( + "the parents of {root} nest too deep" + ))); + } + let gav = (pg, pa, pv); + let pom = fetch_external(&gav, lookup, &format!("the parent of {root}"))?; + parent = pom + .parent + .as_ref() + .map(|p| (p.group.clone(), p.artifact.clone(), p.version.clone())); + chain.push(ExternalPom { gav, pom }); + } + Ok(chain) + } +} + +/// `gav` parsed, or why not. +fn fetch_external<'b>( + gav: &Gav, + lookup: &dyn Fn(&Gav) -> Lookup<'b>, + role: &str, +) -> Result { + let (g, a, v) = gav; + let named = format!("{role} {g}:{a}:{v}"); + if !safe_coordinates(g, a, v) { + return Err(Missing::Unresolved(format!( + "{named} has unsafe coordinates" + ))); + } + match lookup(gav) { + Lookup::Found(bytes) => Pom::parse(&format!("{g}:{a}:{v}"), bytes.to_vec()) + .map_err(|e| Missing::Unresolved(format!("{named} is unreadable ({})", e.detail))), + Lookup::Unavailable => Err(Missing::Need( + gav.clone(), + format!("{named} is in no local Maven repository"), + )), + Lookup::Unknown => Err(Missing::Need( + gav.clone(), + format!("{named} is not read yet"), + )), + } +} + +/// The top-level declaration of g:a's main jar in `doc`: its +/// `` entry (`true`) before a `` one. +fn jar_declaration(doc: &Doc, g: &str, a: &str) -> Option<(usize, bool)> { + let is_jar = |dep: usize| { + doc.child_text(dep, "groupId").as_deref() == Some(g) + && doc.child_text(dep, "artifactId").as_deref() == Some(a) + && doc + .child_text(dep, "classifier") + .is_none_or(|c| c.is_empty()) + && doc + .child_text(dep, "type") + .is_none_or(|t| t.is_empty() || t == "jar") + }; + let managed = doc + .child(doc.project, "dependencyManagement") + .and_then(|dm| doc.child(dm, "dependencies")) + .and_then(|deps| doc.children(deps, "dependency").find(|d| is_jar(*d))); + if let Some(dep) = managed { + return Some((dep, true)); + } + doc.child(doc.project, "dependencies") + .and_then(|deps| doc.children(deps, "dependency").find(|d| is_jar(*d))) + .map(|dep| (dep, false)) +} + +/// The raw `(groupId, artifactId, version)` of each top-level +/// `import` BOM in `doc`, in order. +fn imports_of(doc: &Doc) -> Vec<(String, String, String)> { + let Some(deps) = doc + .child(doc.project, "dependencyManagement") + .and_then(|dm| doc.child(dm, "dependencies")) + else { + return Vec::new(); + }; + doc.children(deps, "dependency") + .filter(|d| { + doc.child_text(*d, "scope").as_deref() == Some("import") + && doc.child_text(*d, "type").as_deref() == Some("pom") + }) + .map(|d| { + ( + doc.child_text(d, "groupId").unwrap_or_default(), + doc.child_text(d, "artifactId").unwrap_or_default(), + doc.child_text(d, "version").unwrap_or_default(), + ) + }) + .collect() +} + +/// The version BOM `bom` manages g:a at, in the BOM's own effective model +/// (its parents' management and properties, then its own imports); `None` +/// when it does not manage g:a. +fn bom_manages<'b>( + bom: &Gav, + g: &str, + a: &str, + lookup: &dyn Fn(&Gav) -> Lookup<'b>, + seen: &mut BTreeSet, + depth: usize, +) -> Result, Missing> { + if !seen.insert(bom.clone()) { + return Ok(None); + } + if depth > MAX_EXTERNAL_DEPTH || seen.len() > MAX_EXTERNAL_POMS { + return Err(Missing::Unresolved(format!( + "the BOM imports under {}:{}:{} nest too deep", + bom.0, bom.1, bom.2 + ))); + } + // The BOM and its parents, nearest first. + let mut chain: Vec = Vec::new(); + let mut next = Some(bom.clone()); + while let Some(gav) = next { + if chain.len() >= MAX_EXTERNAL_DEPTH { + return Err(Missing::Unresolved(format!( + "the parents of the BOM {}:{}:{} nest too deep", + bom.0, bom.1, bom.2 + ))); + } + let role = if chain.is_empty() { + "the imported BOM".to_string() + } else { + format!("a parent of the imported BOM {}:{}:{}", bom.0, bom.1, bom.2) + }; + let pom = fetch_external(&gav, lookup, &role)?; + next = pom + .parent + .as_ref() + .map(|p| (p.group.clone(), p.artifact.clone(), p.version.clone())) + .and_then(|(pg, pa, pv)| Some((pg?, pa?, pv?))); + chain.push(ExternalPom { gav, pom }); + } + let props = |name: &str| -> Option { + let own = &chain[0].pom; + match name { + "project.version" | "pom.version" | "version" => { + return own.effective_version().map(str::to_string) + } + "project.groupId" => return own.effective_group().map(str::to_string), + "project.parent.version" => return own.parent.as_ref()?.version.clone(), + _ => {} + } + chain.iter().find_map(|e| e.pom.props.get(name).cloned()) + }; + let resolve = |value: &str| -> Result { + interpolate_by(value, &props, 0).ok_or_else(|| { + Missing::Unresolved(format!( + "the BOM {}:{}:{} manages {g}:{a} at {value}, which it does not define", + bom.0, bom.1, bom.2 + )) + }) + }; + for ext in &chain { + if let Some((dep, true)) = jar_declaration(&ext.pom.doc, g, a) { + if let Some(version) = ext.pom.doc.child_text(dep, "version") { + return resolve(&version).map(Some); + } + } + } + for ext in &chain { + for (ig, ia, iv) in imports_of(&ext.pom.doc) { + let gav = (resolve(&ig)?, resolve(&ia)?, resolve(&iv)?); + if let Some(v) = bom_manages(&gav, g, a, lookup, seen, depth + 1)? { + return Ok(Some(v)); + } + } + } + Ok(None) +} + /// Metadata needed to verify upstream parents and imported BOMs in Gradle. pub(crate) struct MetadataModel { pub parent: Option, @@ -2520,6 +2984,234 @@ mod tests { "#; + // ── management from outside the checkout (#488) ── + + fn bom(artifact: &str, version: &str, managed: &str) -> Vec { + format!( + "4.0.0com.corp\ + {artifact}{version}\ + pom{managed}\ + " + ) + .into_bytes() + } + + fn corp(artifact: &str, version: &str) -> (String, String, String) { + ("com.corp".into(), artifact.into(), version.into()) + } + + /// A reactor whose root carries `root_extra` (a BOM import, a parent) + /// and whose module declares commons-text without a version. + fn external_reactor(root_head: &str, root_extra: &str) -> Fs { + let root = format!( + "\n \ + 4.0.0\n {root_head}\n com.example\n \ + root\n 1.0.0\n \ + pom\n a\n \ + {root_extra}\n\n" + ); + let a = "\n 4.0.0\n \n \ + com.example\n root\n \ + 1.0.0\n \n a\n \ + \n org.apache.commons\ + commons-text\n \n\ + \n"; + fs(&[("pom.xml", &root), ("a/pom.xml", a)]) + } + + const IMPORT_BOM: &str = "\ + com.corpcorp-bom${bom.version}\ + pomimport"; + + /// Fetch what the planner asks for from `repo` (absent: unavailable), + /// then plan. + fn run_external( + files: &Fs, + repo: &ExternalPoms, + ) -> (Result, ExternalPoms) { + let read = |p: &str| files.get(p).cloned(); + let mut known = ExternalPoms::new(); + loop { + let need = external_poms_needed(&read, &patch(), &known); + if need.is_empty() { + break; + } + for gav in need { + let bytes = repo.get(&gav).cloned().flatten(); + known.insert(gav, bytes); + } + } + ( + plan_with_external(&read, &patch(), true, Some(&known)), + known, + ) + } + + fn root_pinned(plan: &JvmPlan, files: &Fs) -> bool { + let after = applied(files, plan); + text(&after, "pom.xml").contains(PIN_TAG) + } + + #[test] + fn imported_bom_managing_another_version_leaves_the_root_unpinned() { + let files = external_reactor( + "2", + IMPORT_BOM, + ); + let repo = ExternalPoms::from([( + corp("corp-bom", "2"), + Some(bom("corp-bom", "2", &dep("1.11.0"))), + )]); + let (plan, known) = run_external(&files, &repo); + let plan = plan.unwrap(); + assert_eq!(known.keys().collect::>(), [&corp("corp-bom", "2")]); + assert!( + reasons(&plan).contains(&"conflicting_managed_version".to_string()), + "{:?}", + plan.warnings + ); + assert!(plan.warnings.iter().any(|w| w.detail.contains("1.11.0"))); + assert!(!root_pinned(&plan, &files), "the root must not be pinned"); + // Without the external weighing (the pre-#488 plan) it was pinned. + assert!(root_pinned(&run(&files).unwrap(), &files)); + } + + #[test] + fn imported_bom_at_the_base_version_is_pinned() { + let files = external_reactor( + "1", + IMPORT_BOM, + ); + let repo = ExternalPoms::from([( + corp("corp-bom", "1"), + Some(bom("corp-bom", "1", &dep("1.10.0"))), + )]); + let plan = run_external(&files, &repo).0.unwrap(); + assert!(reasons(&plan).is_empty(), "{:?}", plan.warnings); + assert!(root_pinned(&plan, &files)); + } + + #[test] + fn a_bom_bump_after_vendoring_removes_the_pin() { + let v1 = external_reactor( + "1", + IMPORT_BOM, + ); + let repo = ExternalPoms::from([ + ( + corp("corp-bom", "1"), + Some(bom("corp-bom", "1", &dep("1.10.0"))), + ), + ( + corp("corp-bom", "2"), + Some(bom("corp-bom", "2", &dep("1.11.0"))), + ), + ]); + let first = run_external(&v1, &repo).0.unwrap(); + let mut after = applied(&v1, &first); + assert!(text(&after, "pom.xml").contains(PIN_TAG)); + let bumped = text(&after, "pom.xml").replace( + "1", + "2", + ); + after.insert("pom.xml".into(), bumped.into_bytes()); + let again = run_external(&after, &repo).0.unwrap(); + assert!(reasons(&again).contains(&"conflicting_managed_version".to_string())); + assert!( + !root_pinned(&again, &after), + "the re-run must drop the pin, not report it in sync" + ); + } + + #[test] + fn a_nested_bom_import_is_followed() { + let files = external_reactor( + "2", + IMPORT_BOM, + ); + let outer = "com.corpinner-bom\ + 7pomimport"; + let repo = ExternalPoms::from([ + (corp("corp-bom", "2"), Some(bom("corp-bom", "2", outer))), + ( + corp("inner-bom", "7"), + Some(bom("inner-bom", "7", &dep("1.11.0"))), + ), + ]); + let plan = run_external(&files, &repo).0.unwrap(); + assert!(reasons(&plan).contains(&"conflicting_managed_version".to_string())); + assert!(!root_pinned(&plan, &files)); + } + + #[test] + fn an_external_parent_managing_another_version_leaves_the_root_unpinned() { + let parent = "com.corpcorp-parent\ + 2"; + let files = external_reactor(parent, ""); + let repo = ExternalPoms::from([( + corp("corp-parent", "2"), + Some(bom("corp-parent", "2", &dep("1.11.0"))), + )]); + let plan = run_external(&files, &repo).0.unwrap(); + assert!(reasons(&plan).contains(&"conflicting_managed_version".to_string())); + assert!(!root_pinned(&plan, &files)); + } + + #[test] + fn a_local_property_overriding_an_external_parents_version_is_honored() { + // The parent manages `${ct.version}` (1.10.0 by default); the local + // root overrides it to 1.11.0, which Maven interpolates after + // inheritance. + let parent = "com.corpcorp-parent\ + 3\n \ + 1.11.0"; + let files = external_reactor(parent, ""); + let managed = "org.apache.commons\ + commons-text${ct.version}"; + let mut parent_pom = String::from_utf8(bom("corp-parent", "3", managed)).unwrap(); + parent_pom = parent_pom.replace( + "pom", + "pom1.10.0", + ); + let repo = ExternalPoms::from([(corp("corp-parent", "3"), Some(parent_pom.into_bytes()))]); + let plan = run_external(&files, &repo).0.unwrap(); + assert!( + reasons(&plan).contains(&"conflicting_managed_version".to_string()), + "{:?}", + plan.warnings + ); + assert!(!root_pinned(&plan, &files)); + } + + #[test] + fn unavailable_external_management_leaves_the_root_unpinned() { + let files = external_reactor( + "2", + IMPORT_BOM, + ); + let plan = run_external(&files, &ExternalPoms::new()).0.unwrap(); + assert!( + reasons(&plan).contains(&"management_unresolved".to_string()), + "{:?}", + plan.warnings + ); + assert!(!root_pinned(&plan, &files)); + } + + #[test] + fn a_bom_that_does_not_manage_the_artifact_keeps_the_pin() { + let files = external_reactor( + "2", + IMPORT_BOM, + ); + let other = "junitjunit\ + 4.13.2"; + let repo = ExternalPoms::from([(corp("corp-bom", "2"), Some(bom("corp-bom", "2", other)))]); + let plan = run_external(&files, &repo).0.unwrap(); + assert!(reasons(&plan).is_empty(), "{:?}", plan.warnings); + assert!(root_pinned(&plan, &files)); + } + fn module(name: &str, deps: &str) -> String { format!( "\n 4.0.0\n \n \ diff --git a/crates/socket-patch-core/src/vendor/jvm/mod.rs b/crates/socket-patch-core/src/vendor/jvm/mod.rs index 7717b7c6b..f5f62ac22 100644 --- a/crates/socket-patch-core/src/vendor/jvm/mod.rs +++ b/crates/socket-patch-core/src/vendor/jvm/mod.rs @@ -371,13 +371,29 @@ pub fn plan_with_config( list: ListFn<'_>, patch: &JvmPatch<'_>, config_enabled: bool, +) -> Result { + plan_with_external(shape, read, list, patch, config_enabled, None) +} + +/// [`plan_with_config`] weighing the management a Maven pin would +/// override from outside the checkout (see +/// [`maven_reactor::plan_with_external`]). +pub fn plan_with_external( + shape: Shape, + read: ReadFn<'_>, + list: ListFn<'_>, + patch: &JvmPatch<'_>, + config_enabled: bool, + external: Option<&maven_reactor::ExternalPoms>, ) -> Result { match shape { - Shape::MavenReactor => maven_reactor::plan_with_config(read, patch, config_enabled), + Shape::MavenReactor => { + maven_reactor::plan_with_external(read, patch, config_enabled, external) + } Shape::Gradle => gradle::plan(read, list, patch), Shape::Mixed => { // Both halves or neither: a refusal of either writes nothing. - let maven = maven_reactor::plan_with_config(read, patch, config_enabled)?; + let maven = maven_reactor::plan_with_external(read, patch, config_enabled, external)?; let gradle = gradle::plan(read, list, patch)?; Ok(compose(maven, gradle)) } @@ -957,6 +973,54 @@ mod tests { assert_eq!(testing::dirs(root), pristine_dirs); } + /// #488: `vendor --check` weighs an imported BOM from the local + /// repository. A BOM it cannot read accepts the pin `vendor` wrote; a + /// BOM bumped to another version of the artifact reports the pin as + /// drift (the next `vendor` drops it). + #[tokio::test] + async fn check_weighs_an_imported_bom_from_the_local_repository() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path().join("project"); + let repo = dir.path().join("m2"); + let pom = "\n 4.0.0\n com.x\n \ + app\n 1\n \n \ + \n \n com.corp\n \ + corp-bom\n 2\n \ + pom\n import\n \n \ + \n \n \n \n \ + org.apache.commons\n commons-text\n \ + \n \n\n"; + testing::populate(&root, &[("pom.xml", pom)]); + let mut ledger = std::collections::BTreeMap::new(); + let p = mixed_patch(); + // Vendored where the BOM managed the base version: pinned. + testing::vendor(&root, Shape::MavenReactor, &p, &mut ledger) + .await + .unwrap(); + let entry = ledger.values().next().unwrap().clone(); + assert!(std::fs::read_to_string(root.join("pom.xml")) + .unwrap() + .contains(&p.suffixed_version())); + apply::check_entry(&root, &entry, None).expect("no local BOM: the pin is accepted"); + apply::check_entry(&root, &entry, Some(&repo)).expect("BOM absent from the repository"); + let bom_dir = repo.join("com/corp/corp-bom/2"); + std::fs::create_dir_all(&bom_dir).unwrap(); + let bom = |version: &str| { + format!( + "4.0.0com.corp\ + corp-bom2pom\ + org.apache.commons\ + commons-text{version}\ + " + ) + }; + std::fs::write(bom_dir.join("corp-bom-2.pom"), bom("1.10.0")).unwrap(); + apply::check_entry(&root, &entry, Some(&repo)).expect("the BOM manages the base"); + std::fs::write(bom_dir.join("corp-bom-2.pom"), bom("1.11.0")).unwrap(); + let drift = apply::check_entry(&root, &entry, Some(&repo)).unwrap_err(); + assert!(drift.contains("drifted"), "{drift}"); + } + /// #395: a refusal by either half writes nothing for the other. #[tokio::test] async fn mixed_root_refusal_on_either_side_writes_nothing() { diff --git a/crates/socket-patch-core/src/vendor/maven_repo.rs b/crates/socket-patch-core/src/vendor/maven_repo.rs index 31df8839e..f387a4ec3 100644 --- a/crates/socket-patch-core/src/vendor/maven_repo.rs +++ b/crates/socket-patch-core/src/vendor/maven_repo.rs @@ -937,6 +937,17 @@ async fn vendor_maven_jvm( Shape::Sbt => { super::jvm::sbt::plan_with_digest(&read, &patch, gate_pass.deps_digest.as_deref()) } + Shape::MavenReactor | Shape::Mixed => { + let external = external_maven_poms(&read, &patch, &local, service).await; + super::jvm::plan_with_external( + shape, + &read, + &list, + &patch, + config_enabled, + Some(&external), + ) + } _ => super::jvm::plan_with_config(shape, &read, &list, &patch, config_enabled), }; if let Some(rel) = reader.escaped() { @@ -1310,6 +1321,31 @@ async fn collect_gradle_metadata( Ok(model.properties) } +/// The poms outside the checkout (external parents, imported BOMs) the +/// reactor planner weighs a pin against (#488), from the local caches or +/// the registry like any upstream metadata; one that neither has is +/// recorded unavailable, and the planner then leaves that root unpinned. +async fn external_maven_poms( + read: super::jvm::ReadFn<'_>, + patch: &super::jvm::JvmPatch<'_>, + local: &LocalSources, + service: Option<&VendorServiceConfig>, +) -> super::jvm::maven_reactor::ExternalPoms { + let mut known = super::jvm::maven_reactor::ExternalPoms::new(); + loop { + let need = super::jvm::maven_reactor::external_poms_needed(read, patch, &known); + if need.is_empty() { + return known; + } + for (g, a, v) in need { + let bytes = acquire_upstream_metadata(local, &g, &a, &v, "pom", service) + .await + .ok(); + known.insert((g, a, v), bytes); + } + } +} + /// A parent's or BOM's metadata file: the local caches only (never the /// patched GAV's own directory), else the registry. async fn acquire_upstream_metadata( diff --git a/docs/design/maven-vendoring.md b/docs/design/maven-vendoring.md index 8e0df2b06..658eed563 100644 --- a/docs/design/maven-vendoring.md +++ b/docs/design/maven-vendoring.md @@ -80,6 +80,20 @@ produce specific warnings; the backend does not silently claim those unsupported declarations are patched. An enforcer repository ban omits the fallback repository and warns that the tail requires Maven 3.9.2 or newer. +A local root's pin beats every management from outside the checkout, so it is +weighed against that management first: the parents a local root resolves from +a repository (``, a corporate or Spring Boot parent) and every +`import` BOM of the reactor and of those parents, read from the +local caches or the registry like other upstream metadata. Properties follow +Maven's order (the reactor's own values override an external parent's). When +that management, or a literal an external parent declares, sets the artifact to +another version than the patch's base, the root is not pinned +(`conflicting_managed_version`); when a needed POM is in no cache and cannot be +fetched, the root is not pinned either (`management_unresolved`, resolve the +project once and vendor again). A re-run after a BOM bump drops the pin the +same way. `vendor --check` reads that metadata from the local repository only +and accepts either decision when some of it is missing. + Maven 3.9.2+ can read the repository tail without copying jars into `~/.m2` and without routing through mirrors. Older Maven versions use the fallback file repository, which copies the suffixed artifact into the local cache. A From d2f29a5e0e3dced0d4ea6c999034a35117ee2591 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 9 Oct 2026 20:34:21 +0000 Subject: [PATCH 2/3] Weigh every module before skipping the Maven root pin external_management returned Ok(None) as soon as one module's inherited parent or imported BOM resolved the base version, so a later module that imports its own BOM (or sets its own property) at another version was never checked. The root pin then overrode that module's management and reintroduced the #488 downgrade. Continue to the next module instead and only clear the root once every module agrees. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01KoEk4Y9wedBsjtt9kTUPVY --- .../src/vendor/jvm/maven_reactor.rs | 52 +++++++++++++++++-- 1 file changed, 49 insertions(+), 3 deletions(-) diff --git a/crates/socket-patch-core/src/vendor/jvm/maven_reactor.rs b/crates/socket-patch-core/src/vendor/jvm/maven_reactor.rs index 2d07d07c1..e34a46637 100644 --- a/crates/socket-patch-core/src/vendor/jvm/maven_reactor.rs +++ b/crates/socket-patch-core/src/vendor/jvm/maven_reactor.rs @@ -782,7 +782,11 @@ impl Reactor { ) -> Result, Missing> { let (g, a) = (patch.group_id, patch.artifact_id); let ext_chain = self.external_chain(root, lookup)?; - for rel in self.scope.iter().filter(|rel| self.local_root(rel) == root) { + // Every module under this root must agree: one module whose + // management already resolves the base version says nothing about + // the next, which may interpolate a different one the root pin would + // override. + 'modules: for rel in self.scope.iter().filter(|rel| self.local_root(rel) == root) { let locally_versioned = self.chain(rel).any(|p| { let doc = &self.poms[p].doc; doc.keyed_declarations(g, a) @@ -822,7 +826,7 @@ impl Reactor { // overridden by management, so a pin cannot reach it // even at the base version. if managed && is_base_like(&v, patch.version) { - return Ok(None); + continue 'modules; } let how = if managed { "managed by" @@ -866,7 +870,7 @@ impl Reactor { let mut seen = BTreeSet::new(); if let Some(v) = bom_manages(bom, g, a, lookup, &mut seen, 0)? { if is_base_like(&v, patch.version) { - return Ok(None); + continue 'modules; } let (bg, ba, bv) = bom; return Ok(Some(ManagedConflict { @@ -3123,6 +3127,48 @@ mod tests { ); } + #[test] + fn a_later_module_importing_another_version_leaves_the_root_unpinned() { + // Module `a` sees the root's BOM at the base version; module `b` + // imports its own BOM managing another one. A root pin would + // override `b`'s management, so one agreeing module is not enough. + let mut files = external_reactor( + "1", + IMPORT_BOM, + ); + let root = text(&files, "pom.xml") + .replace("a", "ab"); + files.insert("pom.xml".into(), root.into_bytes()); + let b = "\n 4.0.0\n \n \ + com.example\n root\n \ + 1.0.0\n \n b\n \ + \ + com.corpcorp-bom2\ + pomimport\ + \n \ + \n org.apache.commons\ + commons-text\n \n\ + \n"; + files.insert("b/pom.xml".into(), b.as_bytes().to_vec()); + let repo = ExternalPoms::from([ + ( + corp("corp-bom", "1"), + Some(bom("corp-bom", "1", &dep("1.10.0"))), + ), + ( + corp("corp-bom", "2"), + Some(bom("corp-bom", "2", &dep("1.11.0"))), + ), + ]); + let plan = run_external(&files, &repo).0.unwrap(); + assert!( + reasons(&plan).contains(&"conflicting_managed_version".to_string()), + "{:?}", + plan.warnings + ); + assert!(!root_pinned(&plan, &files), "the root must not be pinned"); + } + #[test] fn a_nested_bom_import_is_followed() { let files = external_reactor( From 9858181c5fabe6c17bbc387747d4437c9c86c8b6 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 9 Oct 2026 20:36:18 +0000 Subject: [PATCH 3/3] Weigh BOM imports declared in Maven profiles imports_of read only the project's top-level dependencyManagement, so a import BOM inside a profile (activeByDefault included) was never weighed and the root pin could override the version Maven actually uses. Read every profile's management too, after the project's, as Maven appends it. Activation is not evaluated, so a profile BOM that might apply keeps the root unpinned rather than risking a silent downgrade. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01KoEk4Y9wedBsjtt9kTUPVY --- .../src/vendor/jvm/maven_reactor.rs | 49 +++++++++++++++---- 1 file changed, 40 insertions(+), 9 deletions(-) diff --git a/crates/socket-patch-core/src/vendor/jvm/maven_reactor.rs b/crates/socket-patch-core/src/vendor/jvm/maven_reactor.rs index e34a46637..920959c46 100644 --- a/crates/socket-patch-core/src/vendor/jvm/maven_reactor.rs +++ b/crates/socket-patch-core/src/vendor/jvm/maven_reactor.rs @@ -972,16 +972,24 @@ fn jar_declaration(doc: &Doc, g: &str, a: &str) -> Option<(usize, bool)> { .map(|dep| (dep, false)) } -/// The raw `(groupId, artifactId, version)` of each top-level -/// `import` BOM in `doc`, in order. +/// The raw `(groupId, artifactId, version)` of each +/// `import` BOM in `doc`, in order: the project's own, then +/// each profile's (Maven appends an active profile's management after the +/// project's). Profile activation is not evaluated, so every profile counts: +/// a BOM that might apply is weighed rather than letting a root pin override +/// it. fn imports_of(doc: &Doc) -> Vec<(String, String, String)> { - let Some(deps) = doc - .child(doc.project, "dependencyManagement") - .and_then(|dm| doc.child(dm, "dependencies")) - else { - return Vec::new(); - }; - doc.children(deps, "dependency") + let profiles = doc + .child(doc.project, "profiles") + .into_iter() + .flat_map(|ps| doc.children(ps, "profile")); + std::iter::once(doc.project) + .chain(profiles) + .filter_map(|model| { + doc.child(model, "dependencyManagement") + .and_then(|dm| doc.child(dm, "dependencies")) + }) + .flat_map(|deps| doc.children(deps, "dependency")) .filter(|d| { doc.child_text(*d, "scope").as_deref() == Some("import") && doc.child_text(*d, "type").as_deref() == Some("pom") @@ -3169,6 +3177,29 @@ mod tests { assert!(!root_pinned(&plan, &files), "the root must not be pinned"); } + #[test] + fn a_profile_bom_import_is_weighed() { + let profile = format!( + "corptrue\ + {IMPORT_BOM}" + ); + let files = external_reactor( + "2", + &profile, + ); + let repo = ExternalPoms::from([( + corp("corp-bom", "2"), + Some(bom("corp-bom", "2", &dep("1.11.0"))), + )]); + let plan = run_external(&files, &repo).0.unwrap(); + assert!( + reasons(&plan).contains(&"conflicting_managed_version".to_string()), + "{:?}", + plan.warnings + ); + assert!(!root_pinned(&plan, &files), "the root must not be pinned"); + } + #[test] fn a_nested_bom_import_is_followed() { let files = external_reactor(