From 319e1ed339e99fc55be0c267e663e474d65c9f15 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 5 Oct 2026 10:29:40 +0000 Subject: [PATCH] Resolve Maven properties and dependencyManagement from in-repo parent poms A child pom whose lives elsewhere in the scanned tree (not at ../pom.xml) left ${...} versions from the parent's unresolved, so the dependency had no version or purl. Index every pom.xml in the scan by coordinates and walk the parent chain (relativePath first, then exact groupId:artifactId:version), inheriting properties and managed versions with nearer poms taking precedence. Co-authored-by: Ibrahim Rahhal --- src/deps/ecosystems/maven.rs | 286 ++++++++++++++++-- src/deps/tests/maven_tests.rs | 95 ++++++ tests/fixtures/README.md | 2 + .../java-maven-parent-cycle/a/pom.xml | 22 ++ .../java-maven-parent-cycle/b/pom.xml | 22 ++ .../platform/aggregate/pom.xml | 32 ++ tests/fixtures/java-maven-repo-parent/pom.xml | 17 ++ .../services/auth/pom.xml | 47 +++ .../services/billing/pom.xml | 21 ++ .../services/legacy/pom.xml | 20 ++ 10 files changed, 536 insertions(+), 28 deletions(-) create mode 100644 tests/fixtures/java-maven-parent-cycle/a/pom.xml create mode 100644 tests/fixtures/java-maven-parent-cycle/b/pom.xml create mode 100644 tests/fixtures/java-maven-repo-parent/platform/aggregate/pom.xml create mode 100644 tests/fixtures/java-maven-repo-parent/pom.xml create mode 100644 tests/fixtures/java-maven-repo-parent/services/auth/pom.xml create mode 100644 tests/fixtures/java-maven-repo-parent/services/billing/pom.xml create mode 100644 tests/fixtures/java-maven-repo-parent/services/legacy/pom.xml diff --git a/src/deps/ecosystems/maven.rs b/src/deps/ecosystems/maven.rs index 70e9b1c..c2a6be1 100644 --- a/src/deps/ecosystems/maven.rs +++ b/src/deps/ecosystems/maven.rs @@ -1,6 +1,7 @@ -use std::path::Path; +use std::collections::{HashMap, HashSet}; +use std::path::{Component, Path, PathBuf}; -use crate::deps::detect::DepFileKind; +use crate::deps::detect::{DepFileKind, DetectedFile}; use crate::deps::ecosystems::classify_constraint; use crate::deps::ecosystems::evaluate::{ constraint_to_findings, dep001, file_in_dir, parent_dir, ScanContext, @@ -9,11 +10,12 @@ use crate::deps::model::{DependencyEdge, DependencyNode, Ecosystem, PackageId, S use crate::deps::DepsError; pub fn scan_maven_projects(ctx: &mut ScanContext<'_>) -> Result<(), DepsError> { + let poms = PomIndex::build(ctx.detected); for f in ctx.detected { match f.kind { DepFileKind::MavenPom => { let dir = parent_dir(&f.path); - scan_maven_pom(ctx, &dir, &f.path)?; + scan_maven_pom(ctx, &poms, &dir, &f.path)?; } DepFileKind::GradleBuild => { let dir = parent_dir(&f.path); @@ -33,7 +35,191 @@ struct MavenDep { scope: Scope, } -fn scan_maven_pom(ctx: &mut ScanContext<'_>, dir: &Path, pom_path: &Path) -> Result<(), DepsError> { +/// A `` reference: the coordinates a child inherits from. +#[derive(Clone, Debug, PartialEq, Eq)] +struct ParentRef { + group: String, + artifact: String, + version: String, + /// `None` when absent (Maven defaults to `../pom.xml`); `Some("")` for + /// an explicit empty ``, which disables the path lookup. + relative_path: Option, +} + +/// One pom.xml in the scanned tree, reduced to what parent resolution needs. +struct PomInfo { + /// Pom content with `NON_DEPENDENCY_SECTIONS` stripped. + content: String, + group: String, + artifact: String, + /// The project version, resolved against the pom's own properties. + version: String, + parent: Option, +} + +/// Every pom.xml found in the scan, so a child can inherit properties and +/// `` from a parent that lives elsewhere in the repo +/// rather than only at `../pom.xml`. +struct PomIndex { + by_path: HashMap, + by_coords: HashMap<(String, String, String), PathBuf>, +} + +/// Parent chains longer than this are treated as broken. +const MAX_PARENT_DEPTH: usize = 64; + +impl PomIndex { + fn build(detected: &[DetectedFile]) -> Self { + let mut by_path = HashMap::new(); + let mut by_coords = HashMap::new(); + for f in detected.iter().filter(|f| f.kind == DepFileKind::MavenPom) { + // Unreadable or malformed poms are reported when scanned on + // their own; here they just can't serve as parents. + let Ok(raw) = std::fs::read_to_string(&f.path) else { + continue; + }; + if !raw.trim_start().starts_with('<') { + continue; + } + let content = strip_sections(&raw, NON_DEPENDENCY_SECTIONS); + let info = pom_info(content); + let key = normalize_path(&f.path); + if !info.group.is_empty() && !info.artifact.is_empty() && !info.version.is_empty() { + by_coords + .entry(( + info.group.clone(), + info.artifact.clone(), + info.version.clone(), + )) + .or_insert_with(|| key.clone()); + } + by_path.insert(key, info); + } + Self { by_path, by_coords } + } + + /// Stripped contents of the pom's in-repo ancestors, nearest parent + /// first. The chain stops at the first parent not present in the scan + /// (typically one published to a registry). + fn ancestors(&self, pom_path: &Path) -> Vec<&str> { + let mut out = Vec::new(); + let mut current_path = normalize_path(pom_path); + let mut visited = HashSet::from([current_path.clone()]); + while out.len() < MAX_PARENT_DEPTH { + let Some(parent) = self + .by_path + .get(¤t_path) + .and_then(|info| info.parent.as_ref()) + else { + break; + }; + let Some(parent_path) = self.find_parent(¤t_path, parent) else { + break; + }; + if !visited.insert(parent_path.clone()) { + break; + } + out.push(self.by_path[&parent_path].content.as_str()); + current_path = parent_path; + } + out + } + + /// Maven's lookup order: `` (default `../pom.xml`) when + /// the pom there has the referenced coordinates, then any pom in the + /// scan with exactly those coordinates. + fn find_parent(&self, child_path: &Path, parent: &ParentRef) -> Option { + let relative = parent.relative_path.as_deref().unwrap_or("../pom.xml"); + if !relative.is_empty() { + let candidate = normalize_path(&parent_dir(child_path).join(relative)); + for path in [candidate.join("pom.xml"), candidate] { + if self + .by_path + .get(&path) + .is_some_and(|info| parent_matches(info, parent)) + { + return Some(path); + } + } + } + if parent.version.contains("${") { + return None; + } + self.by_coords + .get(&( + parent.group.clone(), + parent.artifact.clone(), + parent.version.clone(), + )) + .cloned() + } +} + +/// A placeholder parent version (CI-friendly `${revision}`) can only be +/// checked on groupId/artifactId; the relativePath already pins the file. +fn parent_matches(info: &PomInfo, parent: &ParentRef) -> bool { + info.group == parent.group + && info.artifact == parent.artifact + && (info.version == parent.version || parent.version.contains("${")) +} + +/// Lexically collapse `.` and `..` so `a/b/../pom.xml` and `./a/pom.xml` +/// key the same entry as `a/pom.xml`. +fn normalize_path(path: &Path) -> PathBuf { + let mut out = PathBuf::new(); + for component in path.components() { + match component { + Component::CurDir => {} + Component::ParentDir => { + if matches!(out.components().next_back(), Some(Component::Normal(_))) { + out.pop(); + } else { + out.push(".."); + } + } + other => out.push(other.as_os_str()), + } + } + out +} + +fn pom_info(content: String) -> PomInfo { + let head = project_head(&content); + let (own, parent) = match split_section(head, "parent") { + Some((own, parent_block)) => ( + own, + Some(ParentRef { + group: extract_xml_tag(parent_block, "groupId"), + artifact: extract_xml_tag(parent_block, "artifactId"), + version: extract_xml_tag(parent_block, "version"), + relative_path: extract_optional_xml_tag(parent_block, "relativePath"), + }), + ), + None => (head.to_string(), None), + }; + let mut group = extract_xml_tag(&own, "groupId"); + if group.is_empty() { + group = parent.as_ref().map(|p| p.group.clone()).unwrap_or_default(); + } + let artifact = extract_xml_tag(&own, "artifactId"); + let version = resolve_properties(raw_properties(&content), &content) + .remove("project.version") + .unwrap_or_else(|| pom_project_version(&content)); + PomInfo { + content, + group, + artifact, + version, + parent, + } +} + +fn scan_maven_pom( + ctx: &mut ScanContext<'_>, + poms: &PomIndex, + dir: &Path, + pom_path: &Path, +) -> Result<(), DepsError> { let rel = pom_path .strip_prefix(ctx.root) .unwrap_or(pom_path) @@ -51,7 +237,7 @@ fn scan_maven_pom(ctx: &mut ScanContext<'_>, dir: &Path, pom_path: &Path) -> Res dep001(ctx.findings, ctx.policy, &rel, "Maven"); - let deps = parse_pom_dependencies(&content)?; + let deps = parse_pom_dependencies(&content, &poms.ancestors(pom_path))?; for dep in deps { let name = dep.artifact.clone(); let declared = dep.version.clone(); @@ -101,15 +287,24 @@ fn scan_maven_pom(ctx: &mut ScanContext<'_>, dir: &Path, pom_path: &Path) -> Res /// dependencies; stripped before any dependency or property extraction. const NON_DEPENDENCY_SECTIONS: &[&str] = &["profiles", "build", "reporting"]; -fn parse_pom_dependencies(content: &str) -> Result, DepsError> { +/// `ancestors` are the stripped contents of the pom's in-repo parents, +/// nearest first. Their `` and `` are +/// inherited, with nearer poms (and the pom itself) taking precedence. +fn parse_pom_dependencies(content: &str, ancestors: &[&str]) -> Result, DepsError> { let stripped = strip_sections(content, NON_DEPENDENCY_SECTIONS); - let props = parse_pom_properties(&stripped); - let (rest, management) = split_dependency_management(&stripped); - let managed: std::collections::HashMap<(String, String), String> = parse_pom_regex(management) - .into_iter() - .filter(|d| !d.version.is_empty()) - .map(|d| ((d.group, d.artifact), d.version)) - .collect(); + let mut raw_props = HashMap::new(); + let mut managed = HashMap::new(); + for pom in ancestors + .iter() + .rev() + .copied() + .chain(std::iter::once(stripped.as_str())) + { + raw_props.extend(raw_properties(pom)); + managed.extend(managed_versions(pom)); + } + let props = resolve_properties(raw_props, &stripped); + let (rest, _) = split_dependency_management(&stripped); let mut deps = parse_pom_regex(&rest); for dep in &mut deps { if dep.version.is_empty() { @@ -159,9 +354,20 @@ fn split_section<'a>(content: &'a str, tag: &str) -> Option<(String, &'a str)> { Some((rest, inner)) } -/// Collect `` entries plus the built-in `project.version`. -fn parse_pom_properties(content: &str) -> std::collections::HashMap { - let mut props = std::collections::HashMap::new(); +/// Versions pinned by the pom's own ``, keyed by +/// (groupId, artifactId), placeholders left unresolved. +fn managed_versions(content: &str) -> HashMap<(String, String), String> { + let (_, management) = split_dependency_management(content); + parse_pom_regex(management) + .into_iter() + .filter(|d| !d.version.is_empty()) + .map(|d| ((d.group, d.artifact), d.version)) + .collect() +} + +/// The pom's own `` entries, placeholders left unresolved. +fn raw_properties(content: &str) -> HashMap { + let mut props = HashMap::new(); if let Some(start) = content.find("") { let rest = &content[start + "".len()..]; if let Some(end) = rest.find("") { @@ -188,6 +394,16 @@ fn parse_pom_properties(content: &str) -> std::collections::HashMap, + content: &str, +) -> HashMap { // Property values may reference other properties; resolve the map to a // fixed point, bounded to guard against definition cycles. resolve_props_fixed_point(&mut props); @@ -217,25 +433,29 @@ fn resolve_props_fixed_point(props: &mut std::collections::HashMap`: first `` before ``, -/// excluding the `` block. A child that inherits its version has -/// none of its own, so fall back to the parent's (Maven's inheritance rule). -fn pom_project_version(content: &str) -> String { +/// The part of the pom holding its own coordinates and ``: before +/// `` and any nested section that may carry unrelated +/// ``/`` tags of its own (plugin versions in ``, +/// managed versions in ``, etc). +fn project_head(content: &str) -> &str { let head = content.split("").next().unwrap_or(content); - // The project's own lives among its coordinates, before any - // nested section that may carry unrelated tags of its own - // (plugin versions in , managed versions in - // , etc). let nested_start = NON_DEPENDENCY_SECTIONS .iter() .copied() .chain(["dependencyManagement"]) .filter_map(|tag| head.find(&format!("<{tag}>"))) .min(); - let head = match nested_start { + match nested_start { Some(pos) => &head[..pos], None => head, - }; + } +} + +/// The pom's own ``, excluding the `` block. A child that +/// inherits its version has none of its own, so fall back to the parent's +/// (Maven's inheritance rule). +fn pom_project_version(content: &str) -> String { + let head = project_head(content); if let Some((cleaned, parent)) = split_section(head, "parent") { let own = extract_xml_tag(&cleaned, "version"); if !own.is_empty() { @@ -297,15 +517,25 @@ fn parse_pom_regex(content: &str) -> Vec { } fn extract_xml_tag(block: &str, tag: &str) -> String { + extract_optional_xml_tag(block, tag).unwrap_or_default() +} + +/// Like `extract_xml_tag`, but tells an absent tag (`None`) apart from an +/// empty one (`` or ``, `Some("")`). +fn extract_optional_xml_tag(block: &str, tag: &str) -> Option { let open = format!("<{tag}>"); let close = format!(""); if let Some(start) = block.find(&open) { let rest = &block[start + open.len()..]; if let Some(end) = rest.find(&close) { - return rest[..end].trim().to_string(); + return Some(rest[..end].trim().to_string()); } } - String::new() + let self_closing = [format!("<{tag}/>"), format!("<{tag} />")]; + if self_closing.iter().any(|t| block.contains(t.as_str())) { + return Some(String::new()); + } + None } fn scan_gradle(ctx: &mut ScanContext<'_>, dir: &Path, gradle_path: &Path) -> Result<(), DepsError> { diff --git a/src/deps/tests/maven_tests.rs b/src/deps/tests/maven_tests.rs index c1e3e79..c29c84e 100644 --- a/src/deps/tests/maven_tests.rs +++ b/src/deps/tests/maven_tests.rs @@ -232,6 +232,101 @@ fn maven_property_aliasing_project_version_resolves() { assert_eq!(*n.id(), PackageId("pkg:maven/com.acme/shared@1.2.3".into())); } +/// A parent pom elsewhere in the repo (not at `../pom.xml`) is found by its +/// coordinates, and its `` resolve the child's placeholders. +#[test] +fn maven_parent_properties_resolve_from_pom_elsewhere_in_repo() { + let inv = scan_fixture("java-maven-repo-parent"); + let n = inv + .node("nimbus-jose-jwt") + .expect("nimbus-jose-jwt missing"); + assert_eq!(n.version(), Some("10.4")); + assert_eq!( + *n.id(), + PackageId("pkg:maven/com.nimbusds/nimbus-jose-jwt@10.4".into()) + ); + assert_eq!( + inv.node("commons-httpclient").unwrap().version(), + Some("3.1") + ); +} + +/// Properties inherit through every in-repo ancestor, nearer poms winning. +#[test] +fn maven_grandparent_properties_resolve_and_child_overrides_win() { + let inv = scan_fixture("java-maven-repo-parent"); + assert_eq!(inv.node("slf4j-api").unwrap().version(), Some("2.0.9")); + assert_eq!( + inv.node("guava").unwrap().version(), + Some("32.1.3-jre"), + "the child's own guava.version overrides the grandparent's" + ); +} + +/// A versionless dependency takes its version from the parent's +/// ``, resolved with the parent's properties. +#[test] +fn maven_parent_dependency_management_pins_child_versions() { + let inv = scan_fixture("java-maven-repo-parent"); + let n = inv + .node("jackson-databind") + .expect("jackson-databind missing"); + assert_eq!(n.version(), Some("2.15.2")); +} + +/// `${project.version}` in a child without its own `` resolves to +/// the inherited parent version. +#[test] +fn maven_project_version_inherits_from_in_repo_parent() { + let inv = scan_fixture("java-maven-repo-parent"); + assert_eq!( + inv.node("gccs-common").unwrap().version(), + Some("8.384-SNAPSHOT") + ); +} + +/// `` (pointing at a directory) locates the parent even when +/// its version is a placeholder that can't be matched by coordinates. +#[test] +fn maven_relative_path_locates_parent() { + let inv = scan_fixture("java-maven-repo-parent"); + assert_eq!(inv.node("httpclient").unwrap().version(), Some("3.1")); +} + +/// A parent version that is not in the repo must not borrow properties +/// from a different version of the same parent that is. +#[test] +fn maven_parent_version_mismatch_does_not_resolve() { + let inv = scan_fixture("java-maven-repo-parent"); + assert_eq!( + inv.node("json-smart").unwrap().version(), + Some("${nimbus-jose-jwt.version}") + ); +} + +/// The parent poms' own `` entries are not +/// dependencies and must not surface as nodes. +#[test] +fn maven_in_repo_parent_management_does_not_emit_nodes() { + let inv = scan_fixture("java-maven-repo-parent"); + assert_eq!( + inv.graph + .nodes + .iter() + .filter(|n| n.name() == "jackson-databind") + .count(), + 1 + ); +} + +/// Poms naming each other as parents must not loop forever. +#[test] +fn maven_parent_cycle_terminates() { + let inv = scan_fixture("java-maven-parent-cycle"); + assert!(inv.node("from-a").is_some()); + assert!(inv.node("from-b").is_some()); +} + #[test] fn maven_scan_yields_root_edges() { let inv = scan_fixture("java-maven"); diff --git a/tests/fixtures/README.md b/tests/fixtures/README.md index 8a43b37..db102ad 100644 --- a/tests/fixtures/README.md +++ b/tests/fixtures/README.md @@ -19,6 +19,8 @@ Offline fixture projects for `corgea deps` unit and CLI tests per `docs/PRD_DEPS | `python-pip-nolock` | DEP001 + requirements.txt | | `python-uv-requirements` | `uv.lock` does not suppress requirements scanning | | `java-maven` / `java-gradle` | Maven/Gradle parsers | +| `java-maven-repo-parent` | parent/grandparent poms elsewhere in the repo: inherited properties and `` | +| `java-maven-parent-cycle` | poms naming each other as parents terminate | | `go-mod-smoke` | detection only | | `malformed/` | graceful parse errors | | `vuln-db.json` | mock DEP010 advisories | diff --git a/tests/fixtures/java-maven-parent-cycle/a/pom.xml b/tests/fixtures/java-maven-parent-cycle/a/pom.xml new file mode 100644 index 0000000..54589eb --- /dev/null +++ b/tests/fixtures/java-maven-parent-cycle/a/pom.xml @@ -0,0 +1,22 @@ + + + 4.0.0 + + com.acme + b + 1.0.0 + + a + + + 1.1.1 + + + + + com.acme + from-b + ${b.version} + + + diff --git a/tests/fixtures/java-maven-parent-cycle/b/pom.xml b/tests/fixtures/java-maven-parent-cycle/b/pom.xml new file mode 100644 index 0000000..0a7a959 --- /dev/null +++ b/tests/fixtures/java-maven-parent-cycle/b/pom.xml @@ -0,0 +1,22 @@ + + + 4.0.0 + + com.acme + a + 1.0.0 + + b + + + 2.2.2 + + + + + com.acme + from-a + ${a.version} + + + diff --git a/tests/fixtures/java-maven-repo-parent/platform/aggregate/pom.xml b/tests/fixtures/java-maven-repo-parent/platform/aggregate/pom.xml new file mode 100644 index 0000000..fed134f --- /dev/null +++ b/tests/fixtures/java-maven-repo-parent/platform/aggregate/pom.xml @@ -0,0 +1,32 @@ + + + 4.0.0 + + + com.acme + acme-root + 1.0.0 + + com.acme.gccs + gccs-aggregate-all + 8.384-SNAPSHOT + pom + US-TUCI-Atlas-GCCS + + + 3.1 + 10.4 + 2.15.2 + false + + + + + + com.fasterxml.jackson.core + jackson-databind + ${jackson.version} + + + + diff --git a/tests/fixtures/java-maven-repo-parent/pom.xml b/tests/fixtures/java-maven-repo-parent/pom.xml new file mode 100644 index 0000000..145f5ed --- /dev/null +++ b/tests/fixtures/java-maven-repo-parent/pom.xml @@ -0,0 +1,17 @@ + + + 4.0.0 + com.acme + acme-root + 1.0.0 + pom + + + 2.0.9 + 31.0-jre + + + + platform/aggregate + + diff --git a/tests/fixtures/java-maven-repo-parent/services/auth/pom.xml b/tests/fixtures/java-maven-repo-parent/services/auth/pom.xml new file mode 100644 index 0000000..a09b09e --- /dev/null +++ b/tests/fixtures/java-maven-repo-parent/services/auth/pom.xml @@ -0,0 +1,47 @@ + + + 4.0.0 + + + com.acme.gccs + gccs-aggregate-all + 8.384-SNAPSHOT + + auth-service + + + 32.1.3-jre + + + + + com.nimbusds + nimbus-jose-jwt + ${nimbus-jose-jwt.version} + + + commons-httpclient + commons-httpclient + ${commons-httpclient.version} + + + com.fasterxml.jackson.core + jackson-databind + + + org.slf4j + slf4j-api + ${slf4j.version} + + + com.google.guava + guava + ${guava.version} + + + com.acme.gccs + gccs-common + ${project.version} + + + diff --git a/tests/fixtures/java-maven-repo-parent/services/billing/pom.xml b/tests/fixtures/java-maven-repo-parent/services/billing/pom.xml new file mode 100644 index 0000000..89fbbe9 --- /dev/null +++ b/tests/fixtures/java-maven-repo-parent/services/billing/pom.xml @@ -0,0 +1,21 @@ + + + 4.0.0 + + + com.acme.gccs + gccs-aggregate-all + ${revision} + ../../platform/aggregate + + billing-service + + + + org.apache.httpcomponents + httpclient + ${commons-httpclient.version} + + + diff --git a/tests/fixtures/java-maven-repo-parent/services/legacy/pom.xml b/tests/fixtures/java-maven-repo-parent/services/legacy/pom.xml new file mode 100644 index 0000000..6b50040 --- /dev/null +++ b/tests/fixtures/java-maven-repo-parent/services/legacy/pom.xml @@ -0,0 +1,20 @@ + + + 4.0.0 + + + com.acme.gccs + gccs-aggregate-all + 8.383-SNAPSHOT + + legacy-service + + + + net.minidev + json-smart + ${nimbus-jose-jwt.version} + + +