From cc4b0bed62ed50d18deaf3d1a83d150e849a78a1 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 28 Oct 2026 15:34:19 +0000 Subject: [PATCH 1/5] Start fix for #513, #459 Assisted-by: Claude Code:claude-opus-5-5 From 253ab73747abc04ac9fb322cbfa9c92b94a18310 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 28 Oct 2026 15:45:00 +0000 Subject: [PATCH 2/5] Test Maven reactor pins on uncertain properties A reactor module whose version is an undefined ${prop} or one a profile raises still got a root pin and a rewrite, so the build kept an unpatched jar while vex attested not_affected. These tests capture #513 (external-parent property, Maven 4 inference) and #459 (profile overrides) and fail on main. Assisted-by: Claude Code:claude-opus-5-5 --- .../socket-patch-cli/tests/vendor_jvm_cli.rs | 47 ++++ .../src/vendor/jvm/maven_reactor.rs | 203 +++++++++++++++++- 2 files changed, 246 insertions(+), 4 deletions(-) diff --git a/crates/socket-patch-cli/tests/vendor_jvm_cli.rs b/crates/socket-patch-cli/tests/vendor_jvm_cli.rs index bd75dd126..45ea33920 100644 --- a/crates/socket-patch-cli/tests/vendor_jvm_cli.rs +++ b/crates/socket-patch-cli/tests/vendor_jvm_cli.rs @@ -1153,6 +1153,53 @@ fn gradle_vendor_533_unpatched_classifier_copy_withholds_vex() { assert!(!vex.exists(), "{env}"); } +/// #513 / #459: a reactor `${prop}` version that can't be pinned with +/// certainty (undefined in the checkout, or raised by a profile) is left +/// as is with a warning, and `vex` does not attest the package the build +/// still takes unpatched. +#[test] +fn maven_reactor_uncertain_property_version_withholds_vex() { + for (extra, reason) in [ + ("", "property_unresolved"), + ( + "1.0\ + new1.1\ + ", + "conflicting_literal_version", + ), + ] { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + fixture(root, Shape::Reactor, &[("foo", FOO_UUID)]); + let a = std::fs::read_to_string(root.join("proj/a/pom.xml")).unwrap(); + let a = a + .replace( + "1.0", + "${foo.version}", + ) + .replace( + "a", + &format!("a{extra}"), + ); + write(root, "proj/a/pom.xml", a.as_bytes()); + let env = ok(root, &["vendor"]); + assert!(env.to_string().contains(&format!("reason: {reason}")), "{env}"); + assert_eq!(std::fs::read_to_string(root.join("proj/a/pom.xml")).unwrap(), a); + assert!(!wiring_text(root).contains("-socket."), "{reason}: pinned"); + let vex = root.join("vex.json"); + let (_, env) = socket( + root, + &["vex", "-O", vex.to_str().unwrap(), "--product", "pkg:generic/x@1"], + ); + assert_eq!( + events(&env), + [(purl("foo"), "skipped".to_string(), "vendor_unwired".to_string())], + "{reason}: {env}" + ); + assert!(!vex.exists(), "{env}"); + } +} + /// #429 repair: a deleted owned `.gitattributes` (and nothing else) is /// rewritten by `repair` (no download), so the next autocrlf clone keeps the /// script and index LF. 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 c8c54f681..783bfe19a 100644 --- a/crates/socket-patch-core/src/vendor/jvm/maven_reactor.rs +++ b/crates/socket-patch-core/src/vendor/jvm/maven_reactor.rs @@ -4043,10 +4043,8 @@ mod tests { plan.warnings[0].detail ); assert_eq!(text(&after, "a/pom.xml"), a); - assert!( - text(&after, "pom.xml").contains(""), - "still pinned" - ); + // a's explicit `${remote.prop}` beats any pin (#513). + assert!(!text(&after, "pom.xml").contains("")); } #[test] @@ -5141,6 +5139,203 @@ mod tests { assert!(text(&after, "pom.xml").contains(&format!("{SV}"))); } + // ── uncertain property versions (#513, #459) ── + + /// No pom of the reactor names the suffixed version: `vex` sees the + /// patch as unwired. + fn unwired(files: &Fs) -> bool { + files + .iter() + .filter(|(rel, _)| rel.ends_with(".xml")) + .all(|(_, bytes)| !String::from_utf8_lossy(bytes).contains(SV)) + } + + /// `profile` as a profile of `pom`, placed before its `` + /// (or ``). + fn with_profile(pom: &str, id: &str, props: &str) -> String { + let profile = format!( + "{id}true\ + {props}\ + \n " + ); + let at = pom + .find("") + .or_else(|| pom.find("")) + .unwrap(); + format!("{}{profile}{}", &pom[..at], &pom[at..]) + } + + /// #513 case 1: a property only an external parent defines keeps the + /// module's explicit version over any pin, so the root is not pinned. + #[test] + fn a_property_from_an_external_parent_leaves_the_root_unpinned() { + let parent = "com.corpcorp-parent\ + 3"; + let mut files = external_reactor(parent, ""); + let a = text(&files, "a/pom.xml").replace( + "", + "${ct.version}", + ); + files.insert("a/pom.xml".into(), a.clone().into_bytes()); + let plan = run(&files).unwrap(); + assert_eq!(reasons(&plan), ["property_unresolved"]); + let after = applied(&files, &plan); + assert_eq!(text(&after, "a/pom.xml"), a); + assert!(!root_pinned(&plan, &files)); + assert!(unwired(&after)); + } + + /// #513 case 2: Maven 4.1.0 infers an empty `` from + /// `../pom.xml`, so the root's property resolves and is rewritten. + #[test] + fn a_maven_4_inferred_parent_is_local() { + let root = "\n \ + 4.1.0\n com.example\n \ + root\n 1.0.0\n \ + pom\n a\n \ + 1.10.0\n\n"; + let a = format!( + "\n \ + 4.1.0\n \n a\n \ + \n {}\n \n\n", + dep("${ct.version}") + ); + let files = fs(&[("pom.xml", root), ("a/pom.xml", &a)]); + let (plan, after) = vendor(&files); + assert!(plan.warnings.is_empty(), "{:?}", plan.warnings); + assert_eq!(text(&after, "a/pom.xml"), a.replace("${ct.version}", SV)); + assert!(root_pinned(&plan, &files)); + } + + /// #459: a profile of the declaring module raising the property keeps + /// the declaration and leaves the root unpinned. + #[test] + fn a_profile_overriding_the_version_property_is_left_alone() { + let root = ROOT.replace( + " ", + " 1.10.0\n ", + ); + let a = with_profile( + &module("a", &dep("${ct.version}")), + "modern-jdk", + "1.11.0", + ); + let files = fs(&[ + ("pom.xml", &root), + ("a/pom.xml", &a), + ("b/pom.xml", &module("b", "")), + ]); + let plan = run(&files).unwrap(); + assert_eq!(reasons(&plan), ["conflicting_literal_version"]); + assert!( + plan.warnings[0].detail.contains("profile modern-jdk"), + "{:?}", + plan.warnings + ); + let after = applied(&files, &plan); + assert_eq!(text(&after, "a/pom.xml"), a); + assert!(!root_pinned(&plan, &files)); + assert!(unwired(&after)); + } + + /// #459: the same through the parent's profile, and through a profile + /// overriding a property the version is built from. + #[test] + fn a_profile_up_the_chain_or_on_a_nested_property_is_weighed() { + let a = module("a", &dep("${ct.version}")); + for (props, profile) in [ + ( + "1.10.0", + "1.11.0", + ), + ( + "101.${ct.minor}.0", + "11", + ), + ] { + let root = with_profile( + &ROOT.replace( + " ", + &format!(" {props}\n "), + ), + "corp", + profile, + ); + let files = fs(&[ + ("pom.xml", &root), + ("a/pom.xml", &a), + ("b/pom.xml", &module("b", "")), + ]); + let plan = run(&files).unwrap(); + assert_eq!(reasons(&plan), ["conflicting_literal_version"], "{props}"); + let after = applied(&files, &plan); + assert_eq!(text(&after, "a/pom.xml"), a); + assert!(unwired(&after), "{props}"); + } + } + + /// #459: a module's profile overriding the property an inherited + /// management declaration reads keeps that declaration. + #[test] + fn an_inheriting_modules_profile_keeps_the_managed_property() { + let root = ROOT.replace( + " ", + " 1.10.0\n \ + \n \n \ + org.apache.commonscommons-text\ + ${ct.version}\n \n \ + \n ", + ); + let bare = "org.apache.commons\ + commons-text"; + let a = with_profile( + &module("a", bare), + "new", + "1.12.0", + ); + let files = fs(&[ + ("pom.xml", &root), + ("a/pom.xml", &a), + ("b/pom.xml", &module("b", bare)), + ]); + let plan = run(&files).unwrap(); + assert_eq!(reasons(&plan), ["conflicting_literal_version"]); + assert!(unwired(&applied(&files, &plan))); + } + + /// A profile that sets the base version (or a user property that wins + /// over every profile) changes nothing. + #[test] + fn a_profile_agreeing_with_the_base_or_beaten_by_maven_config_is_rewritten() { + let root = ROOT.replace( + " ", + " 1.10.0\n ", + ); + let agreeing = with_profile( + &module("a", &dep("${ct.version}")), + "same", + "1.10.0", + ); + let raising = with_profile( + &module("a", &dep("${ct.version}")), + "new", + "1.11.0", + ); + for (a, config) in [(&agreeing, None), (&raising, Some("-Dct.version=1.10.0\n"))] { + let mut files = fs(&[ + ("pom.xml", &root), + ("a/pom.xml", a), + ("b/pom.xml", &module("b", "")), + ]); + if let Some(config) = config { + files.insert(".mvn/maven.config".into(), config.as_bytes().to_vec()); + } + let (plan, after) = vendor(&files); + assert!(plan.warnings.is_empty(), "{:?}", plan.warnings); + assert_eq!(text(&after, "a/pom.xml"), a.replace("${ct.version}", SV)); + } + } + #[test] fn plugin_configuration_modules_do_not_make_a_reactor() { let ear = "4.0.0t\ From cd2f751a0082e42847a508b1f28098cb7721c9b1 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 28 Oct 2026 15:47:16 +0000 Subject: [PATCH 3/5] Leave Maven roots unpinned on uncertain properties The vendored Maven reactor planner pinned and rewrote a ${prop} version it could not resolve with certainty, so the build kept Central's unpatched jar while vendor reported applied and vex attested not_affected. - An undefined property now leaves the local root unpinned, like a conflicting literal: the module's explicit version beats any pin. - Profile are read. A version a profile on the chain of the declaring pom (or of a pom inheriting it) would move off the base is left as is with conflicting_literal_version, and the root is not pinned. maven.config user properties still win. - A Maven 4 without coordinates resolves to ../pom.xml, so a property defined in the checkout's root is found and patched. Fixes #513, #459. Assisted-by: Claude Code:claude-opus-5-5 --- .../socket-patch-cli/tests/vendor_jvm_cli.rs | 24 +++- .../src/vendor/jvm/maven_reactor.rs | 120 +++++++++++++++--- 2 files changed, 124 insertions(+), 20 deletions(-) diff --git a/crates/socket-patch-cli/tests/vendor_jvm_cli.rs b/crates/socket-patch-cli/tests/vendor_jvm_cli.rs index 45ea33920..17b8ec7af 100644 --- a/crates/socket-patch-cli/tests/vendor_jvm_cli.rs +++ b/crates/socket-patch-cli/tests/vendor_jvm_cli.rs @@ -1183,17 +1183,33 @@ fn maven_reactor_uncertain_property_version_withholds_vex() { ); write(root, "proj/a/pom.xml", a.as_bytes()); let env = ok(root, &["vendor"]); - assert!(env.to_string().contains(&format!("reason: {reason}")), "{env}"); - assert_eq!(std::fs::read_to_string(root.join("proj/a/pom.xml")).unwrap(), a); + assert!( + env.to_string().contains(&format!("reason: {reason}")), + "{env}" + ); + assert_eq!( + std::fs::read_to_string(root.join("proj/a/pom.xml")).unwrap(), + a + ); assert!(!wiring_text(root).contains("-socket."), "{reason}: pinned"); let vex = root.join("vex.json"); let (_, env) = socket( root, - &["vex", "-O", vex.to_str().unwrap(), "--product", "pkg:generic/x@1"], + &[ + "vex", + "-O", + vex.to_str().unwrap(), + "--product", + "pkg:generic/x@1", + ], ); assert_eq!( events(&env), - [(purl("foo"), "skipped".to_string(), "vendor_unwired".to_string())], + [( + purl("foo"), + "skipped".to_string(), + "vendor_unwired".to_string() + )], "{reason}: {env}" ); assert!(!vex.exists(), "{env}"); 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 783bfe19a..115e10695 100644 --- a/crates/socket-patch-core/src/vendor/jvm/maven_reactor.rs +++ b/crates/socket-patch-core/src/vendor/jvm/maven_reactor.rs @@ -1311,8 +1311,13 @@ struct Pom { parent: Option, /// Top-level `` (profiles excluded). props: BTreeMap, + /// Each profile's id and ``, in document order. + profile_props: Vec<(String, BTreeMap)>, } +/// A pom and one of its profiles' ``. +type Overlay<'a> = (&'a str, &'a BTreeMap); + struct ParentRef { group: Option, artifact: Option, @@ -1335,12 +1340,26 @@ impl Pom { version: doc.child_text(p, "version"), relative_path: doc.child(p, "relativePath").map(|r| doc.text_of(r)), }); - let mut props = BTreeMap::new(); - if let Some(p) = doc.child(project, "properties") { - for &c in &doc.nodes[p].children { - props.insert(doc.nodes[c].name.clone(), doc.text_of(c)); + let properties_of = |model: usize| { + let mut props = BTreeMap::new(); + if let Some(p) = doc.child(model, "properties") { + for &c in &doc.nodes[p].children { + props.insert(doc.nodes[c].name.clone(), doc.text_of(c)); + } } - } + props + }; + let props = properties_of(project); + let profile_props = doc + .child(project, "profiles") + .into_iter() + .flat_map(|ps| doc.children(ps, "profile")) + .map(|p| { + let id = doc.child_text(p, "id").unwrap_or_else(|| "default".into()); + (id, properties_of(p)) + }) + .filter(|(_, props)| !props.is_empty()) + .collect(); Ok(Pom { dir: rel.rsplit_once('/').map_or("", |(d, _)| d).to_string(), group: doc.child_text(project, "groupId"), @@ -1351,6 +1370,7 @@ impl Pom { .unwrap_or_else(|| "jar".to_string()), parent, props, + profile_props, doc, }) } @@ -1516,6 +1536,18 @@ impl Reactor { /// Interpolate every `${name}` in `value` in the context of `rel`; /// `None` when any name is undefined in the checkout. fn interpolate(&self, rel: &str, value: &str, depth: usize) -> Option { + self.interpolate_with(rel, value, depth, None) + } + + /// [`Self::interpolate`] with `overlay`'s properties (a profile's) + /// layered over its pom's own, as when that profile is active. + fn interpolate_with( + &self, + rel: &str, + value: &str, + depth: usize, + overlay: Option>, + ) -> Option { if depth > MAX_INTERPOLATION_DEPTH { return None; } @@ -1525,8 +1557,8 @@ impl Reactor { out.push_str(&rest[..at]); let after = &rest[at + 2..]; let close = after.find('}')?; - let raw = self.lookup(rel, &after[..close])?; - out.push_str(&self.interpolate(rel, &raw, depth + 1)?); + let raw = self.lookup_with(rel, &after[..close], overlay)?; + out.push_str(&self.interpolate_with(rel, &raw, depth + 1, overlay)?); rest = &after[close + 1..]; } out.push_str(rest); @@ -1537,6 +1569,10 @@ impl Reactor { /// model's own version expressions, then `` up the local /// chain. fn lookup(&self, rel: &str, name: &str) -> Option { + self.lookup_with(rel, name, None) + } + + fn lookup_with(&self, rel: &str, name: &str, overlay: Option>) -> Option { let pom = &self.poms[rel]; match name { "project.version" | "pom.version" => { @@ -1553,7 +1589,12 @@ impl Reactor { if let Some(v) = self.cli_props.get(name) { return Some(v.clone()); } - if let Some(v) = self.chain(rel).find_map(|p| self.poms[p].props.get(name)) { + if let Some(v) = self.chain(rel).find_map(|p| { + overlay + .filter(|(at, _)| *at == p) + .and_then(|(_, props)| props.get(name)) + .or_else(|| self.poms[p].props.get(name)) + }) { return Some(v.clone()); } (name == "version") @@ -1561,6 +1602,31 @@ impl Reactor { .flatten() } + /// A version `declared` (with `${...}`) reaches in some pom that reads + /// it, `rel` or an inheritor, when a profile on that pom's chain is + /// active, and that is neither the base nor `sv`: the pom and the + /// profile's id, and the version. Profile activation is not evaluated, + /// so every profile counts. + fn profile_override( + &self, + rel: &str, + declared: &str, + base: &str, + sv: &str, + ) -> Option<(String, String, String)> { + std::iter::once(rel) + .chain(self.inheritors(rel)) + .find_map(|reader| { + self.chain(reader).find_map(|at| { + self.poms[at].profile_props.iter().find_map(|(id, props)| { + let v = self.interpolate_with(reader, declared, 0, Some((at, props)))?; + (!is_base_like(&v, base) && v != sv) + .then(|| (format!("profile {id} of {at}"), v, reader.to_string())) + }) + }) + }) + } + /// Every pom in scope that inherits from `rel` (has it on its chain). fn inheritors<'s>(&'s self, rel: &'s str) -> impl Iterator + 's { self.scope @@ -1643,9 +1709,11 @@ impl Reactor { "property_unresolved", format!( "{at}: {ga} version {declared} is not defined in the checkout; \ - left as is, probably unpatched" + left as is, probably unpatched, and {root} is not pinned" ), )); + // The explicit version beats any pin of the root. + unpinned.insert(root); continue; } } @@ -1665,6 +1733,24 @@ impl Reactor { (!is_base_like(&v, patch.version) && v != sv).then(|| (p.to_string(), v)) }) }); + let profiled = || { + declared + .contains("${") + .then(|| self.profile_override(rel, &declared, patch.version, sv)) + .flatten() + }; + if let Some((profile, v, p)) = profiled() { + warnings.push(degraded( + "conflicting_literal_version", + format!( + "{at}: {ga} version {declared} resolves to {v} in {p} under \ + {profile}, not {}; left as is and {root} is not pinned", + patch.version + ), + )); + unpinned.insert(root); + continue; + } if let Some((p, v)) = overridden.flatten() { warnings.push(degraded( "conflicting_literal_version", @@ -1910,8 +1996,14 @@ fn local_parent_of( (Some(a), Some(b)) => a == b, _ => true, }; - let matches = want_group.as_deref() == candidate.effective_group() - && want_artifact == candidate.artifact + // Maven 4 (model 4.1.0) infers omitted parent coordinates from the + // pom at the relative path. + let matches = want_group + .as_deref() + .is_none_or(|g| Some(g) == candidate.effective_group()) + && want_artifact + .as_deref() + .is_none_or(|a| Some(a) == candidate.artifact.as_deref()) && versions_agree; Ok(matches.then_some(parent_rel)) } @@ -5288,11 +5380,7 @@ mod tests { ); let bare = "org.apache.commons\ commons-text"; - let a = with_profile( - &module("a", bare), - "new", - "1.12.0", - ); + let a = with_profile(&module("a", bare), "new", "1.12.0"); let files = fs(&[ ("pom.xml", &root), ("a/pom.xml", &a), From 1d77282a63928dc3ad290b278f18b612e9b9265b Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 28 Oct 2026 16:05:31 +0000 Subject: [PATCH 4/5] Withhold Maven vex on modules left at base Unpinning the root was not enough: vex counted the patch as wired whenever any pom named the suffixed version, so a rewritten sibling let it attest while a module left at an uncertain ${prop} built the unpatched jar. - vex liveness for a reactor now re-plans the declarations and withholds when one is left at an undefined, inheritor-overridden or profile-overridden version. The GC and --check reference test is unchanged, so a referenced tree is never pruned. - A possibly-matching declaration with an undefined ${...} groupId or artifactId and its own version also leaves the root unpinned. - Parent inference is limited to model 4.1.0, and a Maven 4 child's ${project.version}/${project.groupId} come from the inferred parent. - The inheritor and profile override searches are one walk. Assisted-by: Claude Code:claude-opus-5-5 --- .../socket-patch-core/src/vendor/jvm/apply.rs | 9 +- .../src/vendor/jvm/maven_reactor.rs | 283 ++++++++++++++---- 2 files changed, 229 insertions(+), 63 deletions(-) diff --git a/crates/socket-patch-core/src/vendor/jvm/apply.rs b/crates/socket-patch-core/src/vendor/jvm/apply.rs index e9d17f762..212a39a50 100644 --- a/crates/socket-patch-core/src/vendor/jvm/apply.rs +++ b/crates/socket-patch-core/src/vendor/jvm/apply.rs @@ -830,10 +830,11 @@ pub fn entry_references(root: &Path, entry: &VendorEntry) -> bool { referenced || reader.read_error.borrow().is_some() } -/// The liveness proof `vex` needs: every half of the entry is wired, and -/// a Gradle half's script is intact and re-plans with no refusal and no +/// The liveness proof `vex` needs: every half of the entry is wired, a +/// Gradle half's script is intact and re-plans with no refusal and no /// degraded warning (a conflicting rule or unchecked build logic added -/// since vendoring withholds attestation). +/// since vendoring withholds attestation), and no Maven declaration is +/// left at a version that may still read the base. pub fn entry_wired_checked(root: &Path, entry: &VendorEntry) -> Result { let (g, a, v) = entry_gav(entry)?; let c = Coords { @@ -856,7 +857,7 @@ pub fn entry_wired_checked(root: &Path, entry: &VendorEntry) -> Result, c: &Coords<'_>) -> Result, c: &Coords<'_>) -> Result { + let sv = c.suffixed_version(); + let reactor = Reactor::discover(read)?; + if !reactor + .scope + .iter() + .any(|rel| reactor.poms[rel].doc.masked.contains(&sv)) + { + return Ok(false); + } + let patch = JvmPatch { + group_id: c.group_id, + artifact_id: c.artifact_id, + version: c.version, + uuid: c.uuid, + jar: &[], + upstream_pom: &[], + upstream_module: None, + extra_artifacts: &[], + patched_members: &[], + }; + let mut uncertain = BTreeSet::new(); + for rel in &reactor.scope { + reactor.rewrite_declarations( + rel, + &patch, + &sv, + &mut BTreeMap::new(), + &mut Vec::new(), + &mut BTreeSet::new(), + &mut BTreeSet::new(), + &mut uncertain, + ); + } + Ok(uncertain.is_empty()) +} + /// `