From fb378630daefcee9421ef659e6bf36774414d214 Mon Sep 17 00:00:00 2001 From: Test Date: Thu, 30 Jul 2026 13:04:05 +0200 Subject: [PATCH 1/6] COR-1258: generate CycloneDX SBOMs alongside scan reports Add `corgea scan --sbom [FILE]` (default bom.json): after a blast scan completes, generate a CycloneDX SBOM locally from the project tree via the offline deps engine, alongside any --out-format report. - Upgrade the CycloneDX emitter from bare 1.4 to spec 1.7: serialNumber, metadata (timestamp, tool, root component), bom-refs on components, dependsOn grouped per ref. Output validates against the official bom-1.7 JSON schema. - Maven: resolve ${property} / ${project.version} version placeholders and emit root->dependency graph edges (dependencies array was always empty for pure-Maven projects). - Warn on stderr when detected ecosystems (Go, Cargo) are not included in the SBOM, instead of silently emitting an empty-looking document. - Error cleanly (exit 2) on nonexistent paths in deps subcommands. - Tests: e2e scan --sbom against the HTTP stub, Maven property/edge coverage, warning + path-error CLI tests, emitter unit tests. Scrub GIT_* env in the repo-info test so it passes inside git hooks. --- skills/corgea/SKILL.md | 3 + src/deps/ecosystems/maven.rs | 91 ++++++++++- src/deps/mod.rs | 7 + src/deps/report.rs | 93 +++++++++++- src/deps/run.rs | 9 +- src/deps/tests/maven_tests.rs | 78 ++++++++++ src/deps/tests/report_tests.rs | 37 ++++- src/main.rs | 16 ++ src/scanners/blast.rs | 15 ++ src/utils/generic.rs | 51 +++---- tests/cli_deps.rs | 54 +++++++ tests/cli_scan_sbom.rs | 191 ++++++++++++++++++++++++ tests/fixtures/java-maven-props/pom.xml | 27 ++++ 13 files changed, 625 insertions(+), 47 deletions(-) create mode 100644 tests/cli_scan_sbom.rs create mode 100644 tests/fixtures/java-maven-props/pom.xml diff --git a/skills/corgea/SKILL.md b/skills/corgea/SKILL.md index ddff6c3..df32d48 100644 --- a/skills/corgea/SKILL.md +++ b/skills/corgea/SKILL.md @@ -37,6 +37,8 @@ corgea scan --fail-on malicious # Exit 1 if any dependency is cla corgea scan --fail-on HI,malicious # Comma-separated conditions combine corgea scan --fail # Exit 1 based on project blocking rules corgea scan --out-format json --out-file r.json # Export (json, html, sarif, markdown) +corgea scan --sbom # Also write a CycloneDX SBOM to bom.json +corgea scan --sbom sbom.cdx.json # SBOM to a custom file corgea scan --project-name my-service # Override project name ``` @@ -367,6 +369,7 @@ corgea upload report.json --project-name my-app ```bash corgea scan --out-format html --out-file report.html corgea scan --out-format sarif --out-file report.sarif +corgea scan --out-format sarif --out-file report.sarif --sbom bom.json # SARIF + CycloneDX SBOM ``` ## Severity Levels diff --git a/src/deps/ecosystems/maven.rs b/src/deps/ecosystems/maven.rs index a5d2469..0935d65 100644 --- a/src/deps/ecosystems/maven.rs +++ b/src/deps/ecosystems/maven.rs @@ -5,7 +5,7 @@ use crate::deps::ecosystems::classify_constraint; use crate::deps::ecosystems::evaluate::{ constraint_to_findings, dep001, file_in_dir, parent_dir, ScanContext, }; -use crate::deps::model::{DependencyNode, Ecosystem, PackageId, Scope, SourceType}; +use crate::deps::model::{DependencyEdge, DependencyNode, Ecosystem, PackageId, Scope, SourceType}; use crate::deps::DepsError; pub fn scan_maven_projects(ctx: &mut ScanContext<'_>) -> Result<(), DepsError> { @@ -68,6 +68,14 @@ fn scan_maven_pom(ctx: &mut ScanContext<'_>, dir: &Path, pom_path: &Path) -> Res Some(package_id.clone()), false, )); + ctx.graph.edges.push(DependencyEdge { + from: PackageId::root(), + to: package_id.clone(), + declared_constraint: declared.clone(), + resolved_version: Some(dep.version.clone()), + scope: dep.scope, + source_file: rel.clone(), + }); ctx.graph.nodes.push(DependencyNode { id: package_id, name, @@ -90,7 +98,86 @@ fn scan_maven_pom(ctx: &mut ScanContext<'_>, dir: &Path, pom_path: &Path) -> Res } fn parse_pom_dependencies(content: &str) -> Result, DepsError> { - Ok(parse_pom_regex(content)) + let props = parse_pom_properties(content); + let mut deps = parse_pom_regex(content); + for dep in &mut deps { + dep.version = resolve_placeholders(&dep.version, &props); + } + Ok(deps) +} + +/// Collect `` entries plus the built-in `project.version`. +fn parse_pom_properties(content: &str) -> std::collections::HashMap { + let mut props = std::collections::HashMap::new(); + if let Some(start) = content.find("") { + let rest = &content[start + "".len()..]; + if let Some(end) = rest.find("") { + let mut block = &rest[..end]; + while let Some(open_start) = block.find('<') { + let after = &block[open_start + 1..]; + let Some(open_end) = after.find('>') else { + break; + }; + let tag = after[..open_end].trim(); + let body = &after[open_end + 1..]; + if tag.starts_with('/') || tag.starts_with('!') || tag.ends_with('/') { + block = body; + continue; + } + let close = format!(""); + match body.find(&close) { + Some(close_pos) => { + props.insert(tag.to_string(), body[..close_pos].trim().to_string()); + block = &body[close_pos + close.len()..]; + } + None => block = body, + } + } + } + } + let project_version = pom_project_version(content); + if !project_version.is_empty() && !project_version.contains("${") { + props.insert("project.version".to_string(), project_version); + } + props +} + +/// The pom's own ``: first `` before ``, +/// ignoring any `` block. +fn pom_project_version(content: &str) -> String { + let head = content.split("").next().unwrap_or(content); + if let (Some(ps), Some(pe)) = (head.find(""), head.find("")) { + if ps < pe { + let cleaned = format!("{}{}", &head[..ps], &head[pe + "".len()..]); + return extract_xml_tag(&cleaned, "version"); + } + } + extract_xml_tag(head, "version") +} + +/// Substitute `${name}` placeholders; unresolved ones pass through unchanged. +fn resolve_placeholders(raw: &str, props: &std::collections::HashMap) -> String { + if !raw.contains("${") { + return raw.to_string(); + } + let mut out = String::new(); + let mut rest = raw; + while let Some(start) = rest.find("${") { + out.push_str(&rest[..start]); + let after = &rest[start + 2..]; + let Some(end) = after.find('}') else { + out.push_str(&rest[start..]); + return out; + }; + let key = &after[..end]; + match props.get(key) { + Some(v) => out.push_str(v), + None => out.push_str(&rest[start..start + 2 + end + 1]), + } + rest = &after[end + 1..]; + } + out.push_str(rest); + out } fn parse_pom_regex(content: &str) -> Vec { diff --git a/src/deps/mod.rs b/src/deps/mod.rs index 2c5b57e..1b872af 100644 --- a/src/deps/mod.rs +++ b/src/deps/mod.rs @@ -62,6 +62,13 @@ impl Inventory { /// Scan a directory tree: detect files, build the graph, evaluate policy. pub fn scan(root: &Path, policy: &Policy) -> Result { + if !root.exists() { + return Err(DepsError(format!( + "path does not exist: {}", + root.display() + ))); + } + let detected = detect::detect_dependency_files(root); let mut graph = DependencyGraph::default(); let mut findings = Vec::new(); diff --git a/src/deps/report.rs b/src/deps/report.rs index 810f466..549632f 100644 --- a/src/deps/report.rs +++ b/src/deps/report.rs @@ -1,9 +1,59 @@ use serde_json::{json, Value}; use std::fmt::Write as _; -use crate::deps::model::DependencyGraph; +use crate::deps::detect::DetectedFile; +use crate::deps::model::{DependencyGraph, Ecosystem}; use crate::deps::Inventory; +/// Load the policy at `root`, scan the tree, and emit a CycloneDX SBOM. +pub fn sbom(root: &std::path::Path) -> Result { + let policy = crate::deps::run::load_policy(root)?; + let inv = crate::deps::scan(root, &policy)?; + warn_unsupported_ecosystems(&inv.detected_files); + Ok(to_cyclonedx(&inv)) +} + +/// Print one deduplicated stderr warning per detected ecosystem that +/// `to_cyclonedx` does not emit components for (currently Go and Cargo), +/// so `deps sbom` / `scan --sbom` don't silently produce an empty-looking +/// SBOM for those trees. +pub fn warn_unsupported_ecosystems(detected: &[DetectedFile]) { + let mut by_ecosystem: std::collections::BTreeMap< + &'static str, + std::collections::BTreeSet, + > = std::collections::BTreeMap::new(); + + for f in detected { + let Some(label) = unsupported_ecosystem_label(f.ecosystem) else { + continue; + }; + let file_name = f + .path + .file_name() + .map(|n| n.to_string_lossy().into_owned()) + .unwrap_or_default(); + by_ecosystem.entry(label).or_default().insert(file_name); + } + + for (label, files) in by_ecosystem { + let files: Vec = files.into_iter().collect(); + eprintln!( + "Warning: detected {} but {} is not yet included in SBOMs", + files.join(", "), + label + ); + } +} + +fn unsupported_ecosystem_label(ecosystem: Ecosystem) -> Option<&'static str> { + match ecosystem { + Ecosystem::Npm | Ecosystem::PyPI | Ecosystem::Maven => None, + Ecosystem::Go => Some("Go"), + Ecosystem::Cargo => Some("Cargo"), + Ecosystem::Unknown => Some("this ecosystem"), + } +} + pub fn to_json(inv: &Inventory) -> Value { inventory_to_json(inv) } @@ -56,7 +106,8 @@ fn severity_to_sarif(sev: crate::deps::model::Severity) -> &'static str { } } -pub fn to_cyclonedx(graph: &DependencyGraph) -> Value { +pub fn to_cyclonedx(inv: &Inventory) -> Value { + let graph = &inv.graph; let components: Vec = graph .nodes .iter() @@ -64,6 +115,7 @@ pub fn to_cyclonedx(graph: &DependencyGraph) -> Value { .map(|n| { json!({ "type": "library", + "bom-ref": n.id().0, "name": n.name(), "version": n.version(), "purl": n.id().0, @@ -71,21 +123,46 @@ pub fn to_cyclonedx(graph: &DependencyGraph) -> Value { }) .collect(); - let deps: Vec = graph - .edges + let mut depends_on: std::collections::BTreeMap<&str, Vec<&str>> = + std::collections::BTreeMap::new(); + for e in &graph.edges { + depends_on.entry(&e.from.0).or_default().push(&e.to.0); + } + let deps: Vec = depends_on .iter() - .map(|e| { + .map(|(from, tos)| { json!({ - "ref": e.from.0, - "dependsOn": [e.to.0], + "ref": from, + "dependsOn": tos, }) }) .collect(); + let root_name = std::fs::canonicalize(&inv.root) + .ok() + .and_then(|p| p.file_name().map(|n| n.to_string_lossy().into_owned())) + .unwrap_or_else(|| "project".to_string()); + json!({ "bomFormat": "CycloneDX", - "specVersion": "1.4", + "specVersion": "1.7", + "serialNumber": format!("urn:uuid:{}", uuid::Uuid::new_v4()), "version": 1, + "metadata": { + "timestamp": chrono::Utc::now().to_rfc3339_opts(chrono::SecondsFormat::Secs, true), + "tools": { + "components": [{ + "type": "application", + "name": "corgea", + "version": env!("CARGO_PKG_VERSION"), + }] + }, + "component": { + "type": "application", + "bom-ref": "root", + "name": root_name, + }, + }, "components": components, "dependencies": deps, }) diff --git a/src/deps/run.rs b/src/deps/run.rs index 3084214..40f7fff 100644 --- a/src/deps/run.rs +++ b/src/deps/run.rs @@ -9,7 +9,7 @@ use serde_json::{json, Value}; use crate::deps::findings::Finding; use crate::deps::model::{DependencyNode, Severity}; use crate::deps::policy::Policy; -use crate::deps::report::{graph_nodes_json, table_output, to_cyclonedx, to_json, to_sarif}; +use crate::deps::report::{graph_nodes_json, table_output, to_json, to_sarif}; use crate::deps::{scan, DepsError}; #[derive(Subcommand, Debug, Clone)] @@ -236,13 +236,10 @@ fn run_inner(sub: DepsSubcommand) -> Result { Ok(0) } DepsSubcommand::Sbom { format, path, out } => { - let root = Path::new(&path); - let policy = load_policy(root)?; - let inv = scan(root, &policy)?; if format != "cyclonedx" { return Err(DepsError(format!("unsupported SBOM format: {format}"))); } - let sbom = to_cyclonedx(&inv.graph).to_string(); + let sbom = crate::deps::report::sbom(Path::new(&path))?.to_string(); if let Some(out_path) = out { std::fs::write(&out_path, sbom) .map_err(|e| DepsError(format!("write sbom: {e}")))?; @@ -728,7 +725,7 @@ fn shell_word(value: &str) -> String { format!("'{}'", value.replace('\'', "'\\''")) } -fn load_policy(root: &Path) -> Result { +pub(crate) fn load_policy(root: &Path) -> Result { let policy_path = root.join(".corgea").join("deps.yml"); if !policy_path.exists() { return Ok(Policy::default()); diff --git a/src/deps/tests/maven_tests.rs b/src/deps/tests/maven_tests.rs index 6d6390d..16859af 100644 --- a/src/deps/tests/maven_tests.rs +++ b/src/deps/tests/maven_tests.rs @@ -113,6 +113,84 @@ fn maven_latest_keyword_is_dep004() { assert_eq!(f.severity, Severity::High); } +#[test] +fn maven_property_version_resolves_in_graph_and_purl() { + let inv = scan_fixture("java-maven-props"); + let n = inv.node("guava").expect("guava node missing"); + assert_eq!(n.version(), Some("32.1.3-jre")); + assert_eq!( + *n.id(), + PackageId("pkg:maven/com.google.guava/guava@32.1.3-jre".into()) + ); +} + +#[test] +fn maven_project_version_placeholder_resolves() { + let inv = scan_fixture("java-maven-props"); + let n = inv.node("acme-core").expect("acme-core node missing"); + assert_eq!(n.version(), Some("1.2.3")); + assert_eq!( + *n.id(), + PackageId("pkg:maven/com.acme/acme-core@1.2.3".into()) + ); +} + +#[test] +fn maven_unresolvable_placeholder_passes_through() { + let inv = scan_fixture("java-maven-props"); + let n = inv.node("mystery-lib").expect("mystery-lib node missing"); + assert_eq!(n.version(), Some("${undefined.version}")); +} + +#[test] +fn maven_property_version_resolves_in_cyclonedx_purl() { + let inv = scan_fixture("java-maven-props"); + let v = crate::deps::report::to_cyclonedx(&inv); + let components = v["components"].as_array().expect("components array"); + assert!(components + .iter() + .any(|c| c["purl"] == "pkg:maven/com.google.guava/guava@32.1.3-jre")); + assert!(components + .iter() + .all(|c| !c["purl"].as_str().unwrap().contains("${guava.version}"))); +} + +#[test] +fn maven_scan_yields_root_edges() { + let inv = scan_fixture("java-maven"); + assert!(!inv.graph.edges.is_empty(), "maven graph must carry edges"); + for name in ["guava", "commons-lang3", "slf4j-api", "internal-bom"] { + let id = inv.node(name).unwrap().id().clone(); + assert!( + inv.graph + .edges + .iter() + .any(|e| e.from == PackageId::root() && e.to == id), + "root -> {name} edge missing" + ); + } +} + +#[test] +fn maven_cyclonedx_dependencies_refs_resolve_to_components() { + let inv = scan_fixture("java-maven"); + let v = crate::deps::report::to_cyclonedx(&inv); + let deps = v["dependencies"].as_array().expect("dependencies array"); + assert!(!deps.is_empty(), "dependencies array must be non-empty"); + let components = v["components"].as_array().expect("components array"); + let bom_refs: Vec<&str> = components + .iter() + .filter_map(|c| c["bom-ref"].as_str()) + .collect(); + for d in deps { + let from = d["ref"].as_str().expect("ref"); + assert!(from == "root" || bom_refs.contains(&from)); + for to in d["dependsOn"].as_array().expect("dependsOn") { + assert!(bom_refs.contains(&to.as_str().unwrap())); + } + } +} + #[test] fn maven_snapshot_is_dep021_high() { let inv = scan_fixture("java-maven"); diff --git a/src/deps/tests/report_tests.rs b/src/deps/tests/report_tests.rs index 9668036..d767bba 100644 --- a/src/deps/tests/report_tests.rs +++ b/src/deps/tests/report_tests.rs @@ -71,11 +71,46 @@ fn dep004_report_values_remain_catalog_hydrated_and_dynamic() { #[test] fn report_cyclonedx_has_components_and_deps() { let inv = scan_fixture("node-app"); - let v = to_cyclonedx(&inv.graph); + let v = to_cyclonedx(&inv); assert_eq!(v["bomFormat"], "CycloneDX"); + assert_eq!(v["specVersion"], "1.7"); let components = v["components"].as_array().expect("components array"); assert!(components .iter() .any(|c| c["purl"] == "pkg:npm/express@4.18.2")); + assert!(components + .iter() + .all(|c| c["bom-ref"] == c["purl"] && c["bom-ref"] != "root")); assert!(v.get("dependencies").is_some()); } + +#[test] +fn report_cyclonedx_has_metadata_and_serial_number() { + let inv = scan_fixture("node-app"); + let v = to_cyclonedx(&inv); + let serial = v["serialNumber"].as_str().expect("serialNumber"); + assert!(serial.starts_with("urn:uuid:")); + assert!(v["metadata"]["timestamp"].as_str().is_some()); + assert_eq!(v["metadata"]["tools"]["components"][0]["name"], "corgea"); + assert_eq!( + v["metadata"]["tools"]["components"][0]["version"], + env!("CARGO_PKG_VERSION") + ); + assert_eq!(v["metadata"]["component"]["type"], "application"); + assert_eq!(v["metadata"]["component"]["name"], "node-app"); +} + +#[test] +fn report_cyclonedx_groups_depends_on_per_ref() { + let inv = scan_fixture("node-app"); + let v = to_cyclonedx(&inv); + let deps = v["dependencies"].as_array().expect("dependencies array"); + let mut refs: Vec<&str> = deps.iter().filter_map(|d| d["ref"].as_str()).collect(); + let total = refs.len(); + refs.sort(); + refs.dedup(); + assert_eq!(refs.len(), total, "each ref appears exactly once"); + assert!(deps + .iter() + .all(|d| d["dependsOn"].as_array().is_some_and(|a| !a.is_empty()))); +} diff --git a/src/main.rs b/src/main.rs index 30057f0..d18ab82 100644 --- a/src/main.rs +++ b/src/main.rs @@ -145,6 +145,15 @@ enum Commands { help = "The name of the Corgea project. Defaults to git repository name if found, otherwise to the current directory name." )] project_name: Option, + + #[arg( + long, + value_name = "FILE", + num_args = 0..=1, + default_missing_value = "bom.json", + help = "Generate a CycloneDX SBOM of the project after the scan completes, alongside any report. Optionally specify the output file. Defaults to bom.json." + )] + sbom: Option, }, /// Wait for the latest in progress scan Wait { scan_id: Option }, @@ -570,6 +579,7 @@ fn main() { target, exclude, project_name, + sbom, }) => { verify_token_and_exit_when_fail(&corgea_config); if let Some(level) = fail_on { @@ -670,6 +680,11 @@ fn main() { std::process::exit(1); } + if sbom.is_some() && *scanner != Scanner::Blast { + ::log::error!("sbom is only supported with blast scanner."); + std::process::exit(1); + } + match scanner { Scanner::Snyk => scan::run_snyk(&corgea_config, project_name.clone()), Scanner::Semgrep => scan::run_semgrep(&corgea_config, project_name.clone()), @@ -686,6 +701,7 @@ fn main() { target.clone(), exclude.clone(), project_name.clone(), + sbom.clone(), ), } } diff --git a/src/scanners/blast.rs b/src/scanners/blast.rs index a4c1384..2cb804b 100644 --- a/src/scanners/blast.rs +++ b/src/scanners/blast.rs @@ -24,6 +24,7 @@ pub fn run( target: Option, exclude: Option, project_name: Option, + sbom: Option, ) { // Validate that only_uncommitted and target are not used together if *only_uncommitted && target.is_some() { @@ -417,6 +418,20 @@ pub fn run( } } + if let Some(sbom_file) = sbom { + match corgea::deps::report::sbom(std::path::Path::new(".")) { + Ok(doc) => { + let json = serde_json::to_string_pretty(&doc).expect("serialize SBOM"); + fs::write(&sbom_file, json).expect("Failed to write SBOM file, check if the file path is valid and you have the necessary permissions to write to it."); + println!("CycloneDX SBOM written to: {}\n", sbom_file); + } + Err(e) => { + log::error!("\n\nFailed to generate SBOM: {}\n\n", e); + std::process::exit(1); + } + } + } + print!("\n\nThank you for using Corgea! 🐕\n\n"); if let Some(fail_on) = fail_on { diff --git a/src/utils/generic.rs b/src/utils/generic.rs index c316d17..d2e8ab4 100644 --- a/src/utils/generic.rs +++ b/src/utils/generic.rs @@ -354,41 +354,32 @@ mod tests { use std::fs; use std::process::Command; + // Git exports GIT_DIR/GIT_INDEX_FILE/etc. to hooks; scrub them so the + // test's git subprocesses operate on the temp repo even when the test + // suite itself runs inside a pre-commit hook. + fn git(root: &std::path::Path, args: &[&str]) { + let mut cmd = Command::new("git"); + for (name, _) in std::env::vars() { + if name.starts_with("GIT_") { + cmd.env_remove(name); + } + } + assert!( + cmd.args(args).current_dir(root).status().unwrap().success(), + "git {args:?} failed" + ); + } + #[test] fn get_repo_info_at_root_only_not_nested_cwd() { let dir = tempfile::tempdir().unwrap(); let root = dir.path(); - assert!(Command::new("git") - .args(["init"]) - .current_dir(root) - .status() - .unwrap() - .success()); - assert!(Command::new("git") - .args(["config", "user.email", "test@example.com"]) - .current_dir(root) - .status() - .unwrap() - .success()); - assert!(Command::new("git") - .args(["config", "user.name", "Test"]) - .current_dir(root) - .status() - .unwrap() - .success()); + git(root, &["init"]); + git(root, &["config", "user.email", "test@example.com"]); + git(root, &["config", "user.name", "Test"]); fs::write(root.join("README"), "hi").unwrap(); - assert!(Command::new("git") - .args(["add", "README"]) - .current_dir(root) - .status() - .unwrap() - .success()); - assert!(Command::new("git") - .args(["commit", "-m", "init"]) - .current_dir(root) - .status() - .unwrap() - .success()); + git(root, &["add", "README"]); + git(root, &["commit", "-m", "init"]); let root_s = root.to_str().unwrap(); let nested = root.join("pkg").join("inner"); diff --git a/tests/cli_deps.rs b/tests/cli_deps.rs index 5c19c97..e3fc2ee 100644 --- a/tests/cli_deps.rs +++ b/tests/cli_deps.rs @@ -972,3 +972,57 @@ fn run_git(repo: &std::path::Path, args: &[&str]) { String::from_utf8_lossy(&output.stderr) ); } + +#[test] +fn cli_sbom_warns_on_unsupported_ecosystem_and_still_succeeds() { + let (mut cmd, _home) = corgea_isolated(); + let project = TempDir::new().expect("project dir"); + std::fs::write( + project.path().join("go.mod"), + "module example.com/foo\n\ngo 1.22\n", + ) + .expect("write go.mod"); + + let out = cmd + .args(["deps", "sbom", project.path().to_str().unwrap()]) + .output() + .expect("failed to run corgea"); + + assert!( + out.status.success(), + "stderr: {}", + String::from_utf8_lossy(&out.stderr) + ); + + let stderr = String::from_utf8_lossy(&out.stderr); + assert!( + stderr.contains("go.mod") && stderr.contains("Go") && stderr.contains("not yet included"), + "expected an unsupported-ecosystem warning on stderr, got: {stderr}" + ); + + let sbom: serde_json::Value = + serde_json::from_slice(&out.stdout).expect("stdout must be valid CycloneDX JSON"); + assert_eq!(sbom["bomFormat"], "CycloneDX"); + assert_eq!( + sbom["components"].as_array().map(|a| a.len()), + Some(0), + "Go-only tree should produce zero SBOM components" + ); +} + +#[test] +fn cli_sbom_nonexistent_path_fails_cleanly() { + let (mut cmd, _home) = corgea_isolated(); + let out = cmd + .args(["deps", "sbom", "/does/not/exist/corgea-cli-test"]) + .output() + .expect("failed to run corgea"); + + assert!(!out.status.success()); + assert_eq!(out.status.code(), Some(2)); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!( + stderr.contains("path does not exist"), + "expected a clean path-does-not-exist error, got: {stderr}" + ); +} diff --git a/tests/cli_scan_sbom.rs b/tests/cli_scan_sbom.rs new file mode 100644 index 0000000..9edf9b2 --- /dev/null +++ b/tests/cli_scan_sbom.rs @@ -0,0 +1,191 @@ +//! End-to-end coverage for `corgea scan --sbom`: drives the real binary +//! through the blast scan flow (token verify -> upload -> poll -> issues) +//! against a stubbed HTTP server, then asserts the CycloneDX SBOM the +//! scanner generates locally afterwards (src/scanners/blast.rs, near the +//! end of `run`). + +mod common; + +use common::{corgea_isolated, spawn_http_stub}; +use std::fs; +use tempfile::TempDir; + +/// Route table for a minimal successful blast scan: +/// - `GET /api/v1/verify` -> token ok +/// - `POST /api/v1/start-scan` -> hands back a transfer id +/// - `PATCH /api/v1/start-scan//` -> single chunk completes upload, returns scan_id +/// - `GET /api/v1/scan/` -> status complete +/// - `GET /api/v1/scan//issues*` -> empty issues page (report_scan_status) +fn spawn_scan_stub(scan_id: &'static str) -> String { + spawn_http_stub(move |path| { + let p = path.split('?').next().unwrap_or(path); + if p == "/api/v1/verify" { + ("200 OK", r#"{"status":"ok"}"#.to_string()) + } else if p == "/api/v1/start-scan" { + ("200 OK", r#"{"transfer_id":"transfer-1"}"#.to_string()) + } else if p == "/api/v1/start-scan/transfer-1/" { + ( + "200 OK", + format!(r#"{{"scan_id":"{}","project_id":"1"}}"#, scan_id), + ) + } else if p == format!("/api/v1/scan/{}", scan_id) { + ( + "200 OK", + format!( + r#"{{"id":"{}","project":"proj","repo":null,"branch":null,"status":"complete","engine":"blast","created_at":"2026-01-01T00:00:00Z"}}"#, + scan_id + ), + ) + } else if p == format!("/api/v1/scan/{}/issues", scan_id) { + ( + "200 OK", + r#"{"status":"ok","issues":[],"page":1,"total_pages":1,"total_issues":0}"# + .to_string(), + ) + } else { + ("404 Not Found", r#"{"message":"not found"}"#.to_string()) + } + }) +} + +/// Project dir with a small node package + lockfile, matching the shapes in +/// `tests/fixtures/node-app`, so the SBOM has real component content. +fn write_node_project(dir: &std::path::Path) { + fs::write( + dir.join("package.json"), + r#"{ + "name": "node-app", + "version": "1.0.0", + "dependencies": { + "express": "^4.18.2" + } +} +"#, + ) + .expect("write package.json"); + + fs::write( + dir.join("package-lock.json"), + r#"{ + "name": "node-app", + "version": "1.0.0", + "lockfileVersion": 3, + "requires": true, + "packages": { + "": { + "name": "node-app", + "version": "1.0.0", + "dependencies": { "express": "^4.18.2" } + }, + "node_modules/express": { + "version": "4.18.2", + "resolved": "https://registry.npmjs.org/express/-/express-4.18.2.tgz" + } + } +} +"#, + ) + .expect("write package-lock.json"); +} + +/// `--sbom` with no value writes the default `bom.json`, with real +/// CycloneDX content sourced from the project's npm lockfile. +#[test] +fn scan_sbom_default_filename_writes_cyclonedx_bom() { + let base_url = spawn_scan_stub("scan-default"); + let (mut cmd, _home) = corgea_isolated(); + let project = TempDir::new().expect("project dir"); + write_node_project(project.path()); + + cmd.current_dir(project.path()) + .env("CORGEA_URL", &base_url) + .env("CORGEA_TOKEN", "test-token") + .args(["scan", "--sbom"]); + + let output = cmd.output().expect("run corgea scan --sbom"); + assert!( + output.status.success(), + "stdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + + let bom_path = project.path().join("bom.json"); + assert!(bom_path.exists(), "expected default bom.json to be written"); + + let bom: serde_json::Value = + serde_json::from_str(&fs::read_to_string(&bom_path).expect("read bom.json")) + .expect("bom.json must be valid JSON"); + + assert_eq!(bom["bomFormat"], "CycloneDX"); + assert_eq!(bom["specVersion"], "1.7"); + let components = bom["components"].as_array().expect("components array"); + assert!( + components.iter().any(|c| c["purl"] + .as_str() + .is_some_and(|p| p.starts_with("pkg:npm/"))), + "expected at least one npm component, got: {}", + bom["components"] + ); +} + +/// `--sbom ` honors a custom output path. +#[test] +fn scan_sbom_custom_filename() { + let base_url = spawn_scan_stub("scan-custom"); + let (mut cmd, _home) = corgea_isolated(); + let project = TempDir::new().expect("project dir"); + write_node_project(project.path()); + + cmd.current_dir(project.path()) + .env("CORGEA_URL", &base_url) + .env("CORGEA_TOKEN", "test-token") + .args(["scan", "--sbom", "custom-sbom.json"]); + + let output = cmd + .output() + .expect("run corgea scan --sbom custom-sbom.json"); + assert!( + output.status.success(), + "stdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + + let bom_path = project.path().join("custom-sbom.json"); + assert!(bom_path.exists(), "expected custom-sbom.json to be written"); + assert!( + !project.path().join("bom.json").exists(), + "default bom.json should not be written when a custom name is given" + ); + + let bom: serde_json::Value = + serde_json::from_str(&fs::read_to_string(&bom_path).expect("read custom-sbom.json")) + .expect("custom-sbom.json must be valid JSON"); + assert_eq!(bom["bomFormat"], "CycloneDX"); + assert_eq!(bom["specVersion"], "1.7"); +} + +/// Without `--sbom`, no bom file is produced anywhere in the project. +#[test] +fn scan_without_sbom_flag_writes_no_bom_file() { + let base_url = spawn_scan_stub("scan-none"); + let (mut cmd, _home) = corgea_isolated(); + let project = TempDir::new().expect("project dir"); + write_node_project(project.path()); + + cmd.current_dir(project.path()) + .env("CORGEA_URL", &base_url) + .env("CORGEA_TOKEN", "test-token") + .args(["scan"]); + + let output = cmd.output().expect("run corgea scan"); + assert!( + output.status.success(), + "stdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + + assert!(!project.path().join("bom.json").exists()); +} diff --git a/tests/fixtures/java-maven-props/pom.xml b/tests/fixtures/java-maven-props/pom.xml new file mode 100644 index 0000000..cb3c1d0 --- /dev/null +++ b/tests/fixtures/java-maven-props/pom.xml @@ -0,0 +1,27 @@ + + + 4.0.0 + com.acme + java-maven-props-app + 1.2.3 + + 32.1.3-jre + + + + com.google.guava + guava + ${guava.version} + + + com.acme + acme-core + ${project.version} + + + org.example + mystery-lib + ${undefined.version} + + + From 7b7badef6f0063dad80bf6a231e4b66c8fb15a9b Mon Sep 17 00:00:00 2001 From: Test Date: Thu, 30 Jul 2026 13:41:21 +0200 Subject: [PATCH 2/6] Address PR review: schema-valid SBOMs for unresolved, duplicate, and inherited-version deps - Omit component version/purl when the version is unresolved (? placeholder or ${...} Maven property) instead of emitting null versions and invalid @? purls; bom-ref keeps the internal id. - Dedup components by bom-ref and dependsOn per ref (schema uniqueItems). - Maven: project.version falls back to the parent's version for child POMs and resolves through properties (${revision} CI versioning). - scan --sbom write failures exit 1 with a clean error instead of panicking. All emitter outputs re-validated against the official bom-1.7 schema. --- src/deps/ecosystems/maven.rs | 11 +++- src/deps/report.rs | 42 ++++++++------ src/deps/tests/maven_tests.rs | 30 +++++++++- src/deps/tests/report_tests.rs | 67 ++++++++++++++++++++++ src/scanners/blast.rs | 5 +- tests/cli_scan_sbom.rs | 28 +++++++++ tests/fixtures/java-maven-parent/pom.xml | 19 ++++++ tests/fixtures/java-maven-revision/pom.xml | 19 ++++++ 8 files changed, 200 insertions(+), 21 deletions(-) create mode 100644 tests/fixtures/java-maven-parent/pom.xml create mode 100644 tests/fixtures/java-maven-revision/pom.xml diff --git a/src/deps/ecosystems/maven.rs b/src/deps/ecosystems/maven.rs index 0935d65..fbeba50 100644 --- a/src/deps/ecosystems/maven.rs +++ b/src/deps/ecosystems/maven.rs @@ -135,7 +135,7 @@ fn parse_pom_properties(content: &str) -> std::collections::HashMap std::collections::HashMap`: first `` before ``, -/// ignoring any `` block. +/// 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 = content.split("").next().unwrap_or(content); if let (Some(ps), Some(pe)) = (head.find(""), head.find("")) { if ps < pe { let cleaned = format!("{}{}", &head[..ps], &head[pe + "".len()..]); - return extract_xml_tag(&cleaned, "version"); + let own = extract_xml_tag(&cleaned, "version"); + if !own.is_empty() { + return own; + } + return extract_xml_tag(&head[ps..pe], "version"); } } extract_xml_tag(head, "version") diff --git a/src/deps/report.rs b/src/deps/report.rs index 549632f..4dec3ac 100644 --- a/src/deps/report.rs +++ b/src/deps/report.rs @@ -106,27 +106,37 @@ fn severity_to_sarif(sev: crate::deps::model::Severity) -> &'static str { } } +/// A version is resolved when it names a concrete release — not the `?` +/// placeholder for lockfile-less manifests or an unsubstituted `${...}` +/// Maven property. Unresolved versions (and the purls fabricated from +/// them) must be omitted, or the document fails CycloneDX 1.7 validation. +fn is_resolved_version(version: &str) -> bool { + version != "?" && !version.contains("${") +} + pub fn to_cyclonedx(inv: &Inventory) -> Value { let graph = &inv.graph; - let components: Vec = graph - .nodes - .iter() - .filter(|n| n.name() != "root") - .map(|n| { - json!({ - "type": "library", - "bom-ref": n.id().0, - "name": n.name(), - "version": n.version(), - "purl": n.id().0, - }) - }) - .collect(); + // Deduplicate by bom-ref: multi-module trees list the same package once + // per manifest, but the schema sets uniqueItems on components. + let mut components: std::collections::BTreeMap<&str, Value> = std::collections::BTreeMap::new(); + for n in graph.nodes.iter().filter(|n| n.name() != "root") { + let mut c = json!({ + "type": "library", + "bom-ref": n.id().0, + "name": n.name(), + }); + if let Some(v) = n.version().filter(|v| is_resolved_version(v)) { + c["version"] = json!(v); + c["purl"] = json!(n.id().0); + } + components.entry(&n.id().0).or_insert(c); + } + let components: Vec = components.into_values().collect(); - let mut depends_on: std::collections::BTreeMap<&str, Vec<&str>> = + let mut depends_on: std::collections::BTreeMap<&str, std::collections::BTreeSet<&str>> = std::collections::BTreeMap::new(); for e in &graph.edges { - depends_on.entry(&e.from.0).or_default().push(&e.to.0); + depends_on.entry(&e.from.0).or_default().insert(&e.to.0); } let deps: Vec = depends_on .iter() diff --git a/src/deps/tests/maven_tests.rs b/src/deps/tests/maven_tests.rs index 16859af..31dcb55 100644 --- a/src/deps/tests/maven_tests.rs +++ b/src/deps/tests/maven_tests.rs @@ -152,7 +152,35 @@ fn maven_property_version_resolves_in_cyclonedx_purl() { .any(|c| c["purl"] == "pkg:maven/com.google.guava/guava@32.1.3-jre")); assert!(components .iter() - .all(|c| !c["purl"].as_str().unwrap().contains("${guava.version}"))); + .filter_map(|c| c["purl"].as_str()) + .all(|p| !p.contains("${"))); + // mystery-lib's ${undefined.version} never resolves: no version, no purl. + let mystery = components + .iter() + .find(|c| c["name"] == "mystery-lib") + .expect("mystery-lib component present"); + assert!(mystery.get("version").is_none()); + assert!(mystery.get("purl").is_none()); +} + +/// A child POM with no `` of its own inherits the parent's, so +/// `${project.version}` must resolve to the parent version. +#[test] +fn maven_project_version_inherits_from_parent() { + let inv = scan_fixture("java-maven-parent"); + let n = inv.node("shared").expect("shared node missing"); + assert_eq!(n.version(), Some("1.2.3")); + assert_eq!(*n.id(), PackageId("pkg:maven/com.acme/shared@1.2.3".into())); +} + +/// CI-friendly versioning: `${revision}` resolves through +/// `` before feeding `${project.version}`. +#[test] +fn maven_project_version_resolves_revision_property() { + let inv = scan_fixture("java-maven-revision"); + let n = inv.node("core").expect("core node missing"); + assert_eq!(n.version(), Some("2.0.0")); + assert_eq!(*n.id(), PackageId("pkg:maven/com.acme/core@2.0.0".into())); } #[test] diff --git a/src/deps/tests/report_tests.rs b/src/deps/tests/report_tests.rs index d767bba..00a9ca6 100644 --- a/src/deps/tests/report_tests.rs +++ b/src/deps/tests/report_tests.rs @@ -1,6 +1,30 @@ use super::common::scan_fixture; use crate::deps::catalog::emitted_definition; +use crate::deps::model::{DependencyEdge, DependencyGraph, DependencyNode, PackageId, Scope}; use crate::deps::report::{table_output, to_cyclonedx, to_json, to_sarif}; +use crate::deps::Inventory; + +/// Inventory built straight from nodes/edges, for emitter cases the +/// on-disk fixtures don't produce (unresolved versions, duplicates). +fn inventory_with(nodes: Vec, edges: Vec) -> Inventory { + Inventory { + root: std::path::PathBuf::from("."), + detected_files: vec![], + graph: DependencyGraph { nodes, edges }, + findings: vec![], + } +} + +fn root_edge(to: &PackageId) -> DependencyEdge { + DependencyEdge { + from: PackageId::root(), + to: to.clone(), + declared_constraint: String::new(), + resolved_version: None, + scope: Scope::Production, + source_file: String::new(), + } +} #[test] fn report_json_has_findings_and_graph() { @@ -100,6 +124,49 @@ fn report_cyclonedx_has_metadata_and_serial_number() { assert_eq!(v["metadata"]["component"]["name"], "node-app"); } +/// Unresolved versions (`?` placeholder, `${...}` Maven properties) must not +/// surface as a `version` or a fabricated purl — both fail 1.7 validation. +#[test] +fn report_cyclonedx_omits_unresolved_versions_and_purls() { + let unresolved = DependencyNode::new_npm("left-pad", "?"); + let inv = inventory_with(vec![unresolved], vec![]); + let v = to_cyclonedx(&inv); + let c = &v["components"][0]; + assert_eq!(c["name"], "left-pad"); + assert!( + c.get("version").is_none(), + "unresolved version must be omitted" + ); + assert!( + c.get("purl").is_none(), + "fabricated @? purl must be omitted" + ); + assert_eq!( + c["bom-ref"], "pkg:npm/left-pad@?", + "bom-ref still identifies the node" + ); +} + +/// Multi-module trees list the same package once per manifest; the schema +/// sets uniqueItems on components and dependsOn, so both must deduplicate. +#[test] +fn report_cyclonedx_dedups_components_and_depends_on() { + let a = DependencyNode::new_npm("express", "4.18.2"); + let b = DependencyNode::new_npm("express", "4.18.2"); + let id = a.id().clone(); + let inv = inventory_with(vec![a, b], vec![root_edge(&id), root_edge(&id)]); + let v = to_cyclonedx(&inv); + let components = v["components"].as_array().unwrap(); + assert_eq!(components.len(), 1, "duplicate components must collapse"); + let deps = v["dependencies"].as_array().unwrap(); + assert_eq!(deps.len(), 1); + assert_eq!( + deps[0]["dependsOn"].as_array().unwrap().len(), + 1, + "duplicate dependsOn entries must collapse" + ); +} + #[test] fn report_cyclonedx_groups_depends_on_per_ref() { let inv = scan_fixture("node-app"); diff --git a/src/scanners/blast.rs b/src/scanners/blast.rs index 2cb804b..193d408 100644 --- a/src/scanners/blast.rs +++ b/src/scanners/blast.rs @@ -422,7 +422,10 @@ pub fn run( match corgea::deps::report::sbom(std::path::Path::new(".")) { Ok(doc) => { let json = serde_json::to_string_pretty(&doc).expect("serialize SBOM"); - fs::write(&sbom_file, json).expect("Failed to write SBOM file, check if the file path is valid and you have the necessary permissions to write to it."); + if let Err(e) = fs::write(&sbom_file, json) { + log::error!("\n\nFailed to write SBOM to '{}': {}\n\n", sbom_file, e); + std::process::exit(1); + } println!("CycloneDX SBOM written to: {}\n", sbom_file); } Err(e) => { diff --git a/tests/cli_scan_sbom.rs b/tests/cli_scan_sbom.rs index 9edf9b2..d1a43ab 100644 --- a/tests/cli_scan_sbom.rs +++ b/tests/cli_scan_sbom.rs @@ -166,6 +166,34 @@ fn scan_sbom_custom_filename() { assert_eq!(bom["specVersion"], "1.7"); } +/// An unwritable `--sbom` path fails with a clean error and exit 1, not a panic. +#[test] +fn scan_sbom_unwritable_path_errors_cleanly() { + let base_url = spawn_scan_stub("scan-badpath"); + let (mut cmd, _home) = corgea_isolated(); + let project = TempDir::new().expect("project dir"); + write_node_project(project.path()); + + cmd.current_dir(project.path()) + .env("CORGEA_URL", &base_url) + .env("CORGEA_TOKEN", "test-token") + .args(["scan", "--sbom", "missing-dir/bom.json"]); + + let output = cmd + .output() + .expect("run corgea scan --sbom missing-dir/bom.json"); + assert_eq!( + output.status.code(), + Some(1), + "clean exit 1, not a panic (101)" + ); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("Failed to write SBOM"), + "stderr should name the write failure, got:\n{stderr}" + ); +} + /// Without `--sbom`, no bom file is produced anywhere in the project. #[test] fn scan_without_sbom_flag_writes_no_bom_file() { diff --git a/tests/fixtures/java-maven-parent/pom.xml b/tests/fixtures/java-maven-parent/pom.xml new file mode 100644 index 0000000..eb13152 --- /dev/null +++ b/tests/fixtures/java-maven-parent/pom.xml @@ -0,0 +1,19 @@ + + + 4.0.0 + + com.acme + acme-parent + 1.2.3 + + com.acme + child-module + + + + com.acme + shared + ${project.version} + + + diff --git a/tests/fixtures/java-maven-revision/pom.xml b/tests/fixtures/java-maven-revision/pom.xml new file mode 100644 index 0000000..9519fe7 --- /dev/null +++ b/tests/fixtures/java-maven-revision/pom.xml @@ -0,0 +1,19 @@ + + + 4.0.0 + com.acme + app + ${revision} + + + 2.0.0 + + + + + com.acme + core + ${project.version} + + + From e89679f4061d95354bc79b66cfa821cefe831578 Mon Sep 17 00:00:00 2001 From: Test Date: Thu, 30 Jul 2026 13:51:50 +0200 Subject: [PATCH 3/6] Move graphed-ecosystem policy next to scan_all unsupported_ecosystem_label re-encoded which ecosystems produce graph nodes; derive it from ecosystems::is_graphed, defined beside the scanner dispatch so a future Go/Cargo scanner updates both in one place. --- src/deps/ecosystems/mod.rs | 10 ++++++++++ src/deps/report.rs | 12 +++++++----- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/src/deps/ecosystems/mod.rs b/src/deps/ecosystems/mod.rs index 02de0ce..4c0b66d 100644 --- a/src/deps/ecosystems/mod.rs +++ b/src/deps/ecosystems/mod.rs @@ -10,6 +10,16 @@ pub fn scan_all(ctx: &mut ScanContext<'_>) -> Result<(), DepsError> { evaluate::scan_all(ctx) } +/// Whether `scan_all` builds graph nodes for this ecosystem. Keep in sync +/// with the scanners wired in `evaluate::scan_all` — detect-only ecosystems +/// (Go, Cargo) are found on disk but contribute nothing to the graph or SBOM. +pub fn is_graphed(ecosystem: Ecosystem) -> bool { + matches!( + ecosystem, + Ecosystem::Npm | Ecosystem::PyPI | Ecosystem::Maven + ) +} + use crate::deps::model::{ConstraintKind, Ecosystem}; /// Classify a raw declared constraint string. diff --git a/src/deps/report.rs b/src/deps/report.rs index 4dec3ac..74f6596 100644 --- a/src/deps/report.rs +++ b/src/deps/report.rs @@ -46,12 +46,14 @@ pub fn warn_unsupported_ecosystems(detected: &[DetectedFile]) { } fn unsupported_ecosystem_label(ecosystem: Ecosystem) -> Option<&'static str> { - match ecosystem { - Ecosystem::Npm | Ecosystem::PyPI | Ecosystem::Maven => None, - Ecosystem::Go => Some("Go"), - Ecosystem::Cargo => Some("Cargo"), - Ecosystem::Unknown => Some("this ecosystem"), + if crate::deps::ecosystems::is_graphed(ecosystem) { + return None; } + Some(match ecosystem { + Ecosystem::Go => "Go", + Ecosystem::Cargo => "Cargo", + _ => "this ecosystem", + }) } pub fn to_json(inv: &Inventory) -> Value { From a3639070540bba4053b8484cda2c3817740015a2 Mon Sep 17 00:00:00 2001 From: Test Date: Thu, 30 Jul 2026 15:27:26 +0200 Subject: [PATCH 4/6] Maven: honor dependencyManagement and chained property resolution - dependencyManagement entries no longer emit components/edges; their versions fill versionless direct declarations in the same POM. - Property values resolve to a fixed point (bounded, cycle-safe) so chained ${...} references fully substitute. - Empty versions count as unresolved in the SBOM emitter (no bare @ purls). Parent/imported BOM resolution stays deferred to COR-1733. --- src/deps/ecosystems/maven.rs | 41 +++++++++++++++++++++- src/deps/report.rs | 10 +++--- src/deps/tests/maven_tests.rs | 39 +++++++++++++++++++++ src/deps/tests/report_tests.rs | 42 +++++++++++++---------- tests/fixtures/java-maven-chained/pom.xml | 20 +++++++++++ tests/fixtures/java-maven-managed/pom.xml | 29 ++++++++++++++++ 6 files changed, 156 insertions(+), 25 deletions(-) create mode 100644 tests/fixtures/java-maven-chained/pom.xml create mode 100644 tests/fixtures/java-maven-managed/pom.xml diff --git a/src/deps/ecosystems/maven.rs b/src/deps/ecosystems/maven.rs index fbeba50..fb0281e 100644 --- a/src/deps/ecosystems/maven.rs +++ b/src/deps/ecosystems/maven.rs @@ -99,13 +99,40 @@ fn scan_maven_pom(ctx: &mut ScanContext<'_>, dir: &Path, pom_path: &Path) -> Res fn parse_pom_dependencies(content: &str) -> Result, DepsError> { let props = parse_pom_properties(content); - let mut deps = parse_pom_regex(content); + let (rest, management) = split_dependency_management(content); + 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 deps = parse_pom_regex(&rest); for dep in &mut deps { + if dep.version.is_empty() { + if let Some(v) = managed.get(&(dep.group.clone(), dep.artifact.clone())) { + dep.version = v.clone(); + } + } dep.version = resolve_placeholders(&dep.version, &props); } Ok(deps) } +/// Split out the `` section: its entries pin versions +/// for the project's dependencies but are not dependencies themselves. +/// Returns (pom without the section, the section's inner content). +fn split_dependency_management(content: &str) -> (String, &str) { + const OPEN: &str = ""; + const CLOSE: &str = ""; + if let (Some(s), Some(e)) = (content.find(OPEN), content.find(CLOSE)) { + if s < e { + let management = &content[s + OPEN.len()..e]; + let rest = format!("{}{}", &content[..s], &content[e + CLOSE.len()..]); + return (rest, management); + } + } + (content.to_string(), "") +} + /// Collect `` entries plus the built-in `project.version`. fn parse_pom_properties(content: &str) -> std::collections::HashMap { let mut props = std::collections::HashMap::new(); @@ -135,6 +162,18 @@ fn parse_pom_properties(content: &str) -> std::collections::HashMap = props + .iter() + .map(|(k, v)| (k.clone(), resolve_placeholders(v, &props))) + .collect(); + if resolved == props { + break; + } + props = resolved; + } let project_version = resolve_placeholders(&pom_project_version(content), &props); if !project_version.is_empty() && !project_version.contains("${") { props.insert("project.version".to_string(), project_version); diff --git a/src/deps/report.rs b/src/deps/report.rs index 74f6596..245c334 100644 --- a/src/deps/report.rs +++ b/src/deps/report.rs @@ -108,12 +108,12 @@ fn severity_to_sarif(sev: crate::deps::model::Severity) -> &'static str { } } -/// A version is resolved when it names a concrete release — not the `?` -/// placeholder for lockfile-less manifests or an unsubstituted `${...}` -/// Maven property. Unresolved versions (and the purls fabricated from -/// them) must be omitted, or the document fails CycloneDX 1.7 validation. +/// A version is resolved when it names a concrete release — not empty, not +/// the `?` placeholder for lockfile-less manifests, not an unsubstituted +/// `${...}` Maven property. Unresolved versions (and the purls fabricated +/// from them) must be omitted, or the document fails CycloneDX 1.7 validation. fn is_resolved_version(version: &str) -> bool { - version != "?" && !version.contains("${") + !version.is_empty() && version != "?" && !version.contains("${") } pub fn to_cyclonedx(inv: &Inventory) -> Value { diff --git a/src/deps/tests/maven_tests.rs b/src/deps/tests/maven_tests.rs index 31dcb55..ebe597e 100644 --- a/src/deps/tests/maven_tests.rs +++ b/src/deps/tests/maven_tests.rs @@ -173,6 +173,45 @@ fn maven_project_version_inherits_from_parent() { assert_eq!(*n.id(), PackageId("pkg:maven/com.acme/shared@1.2.3".into())); } +/// `` entries pin versions but are not dependencies: +/// they must not surface as nodes, and a versionless direct declaration +/// takes its version from the managed entry. +#[test] +fn maven_dependency_management_pins_versions_without_emitting_nodes() { + let inv = scan_fixture("java-maven-managed"); + let n = inv.node("guava").expect("guava node missing"); + assert_eq!(n.version(), Some("32.1.3-jre")); + assert_eq!( + *n.id(), + PackageId("pkg:maven/com.google.guava/guava@32.1.3-jre".into()) + ); + assert!( + inv.node("junit-bom").is_none(), + "management-only entries must not become nodes" + ); + assert_eq!( + inv.graph + .nodes + .iter() + .filter(|n| n.name() == "guava") + .count(), + 1, + "managed + declared must collapse to one node" + ); +} + +/// Properties referencing other properties resolve to a fixed point. +#[test] +fn maven_chained_properties_resolve_to_fixed_point() { + let inv = scan_fixture("java-maven-chained"); + let n = inv.node("guava").expect("guava node missing"); + assert_eq!(n.version(), Some("2.5.0")); + assert_eq!( + *n.id(), + PackageId("pkg:maven/com.google.guava/guava@2.5.0".into()) + ); +} + /// CI-friendly versioning: `${revision}` resolves through /// `` before feeding `${project.version}`. #[test] diff --git a/src/deps/tests/report_tests.rs b/src/deps/tests/report_tests.rs index 00a9ca6..18edd60 100644 --- a/src/deps/tests/report_tests.rs +++ b/src/deps/tests/report_tests.rs @@ -124,27 +124,31 @@ fn report_cyclonedx_has_metadata_and_serial_number() { assert_eq!(v["metadata"]["component"]["name"], "node-app"); } -/// Unresolved versions (`?` placeholder, `${...}` Maven properties) must not -/// surface as a `version` or a fabricated purl — both fail 1.7 validation. +/// Unresolved versions (empty, `?` placeholder, `${...}` Maven properties) +/// must not surface as a `version` or a fabricated purl — both fail 1.7 +/// validation. #[test] fn report_cyclonedx_omits_unresolved_versions_and_purls() { - let unresolved = DependencyNode::new_npm("left-pad", "?"); - let inv = inventory_with(vec![unresolved], vec![]); - let v = to_cyclonedx(&inv); - let c = &v["components"][0]; - assert_eq!(c["name"], "left-pad"); - assert!( - c.get("version").is_none(), - "unresolved version must be omitted" - ); - assert!( - c.get("purl").is_none(), - "fabricated @? purl must be omitted" - ); - assert_eq!( - c["bom-ref"], "pkg:npm/left-pad@?", - "bom-ref still identifies the node" - ); + for bad_version in ["?", ""] { + let unresolved = DependencyNode::new_npm("left-pad", bad_version); + let inv = inventory_with(vec![unresolved], vec![]); + let v = to_cyclonedx(&inv); + let c = &v["components"][0]; + assert_eq!(c["name"], "left-pad"); + assert!( + c.get("version").is_none(), + "version {bad_version:?} must be omitted" + ); + assert!( + c.get("purl").is_none(), + "purl fabricated from {bad_version:?} must be omitted" + ); + assert_eq!( + c["bom-ref"].as_str().unwrap(), + format!("pkg:npm/left-pad@{bad_version}"), + "bom-ref still identifies the node" + ); + } } /// Multi-module trees list the same package once per manifest; the schema diff --git a/tests/fixtures/java-maven-chained/pom.xml b/tests/fixtures/java-maven-chained/pom.xml new file mode 100644 index 0000000..71ec002 --- /dev/null +++ b/tests/fixtures/java-maven-chained/pom.xml @@ -0,0 +1,20 @@ + + + 4.0.0 + com.acme + app + 1.0.0 + + + 2.5.0 + ${base.version} + + + + + com.google.guava + guava + ${guava.version} + + + diff --git a/tests/fixtures/java-maven-managed/pom.xml b/tests/fixtures/java-maven-managed/pom.xml new file mode 100644 index 0000000..d7d9137 --- /dev/null +++ b/tests/fixtures/java-maven-managed/pom.xml @@ -0,0 +1,29 @@ + + + 4.0.0 + com.acme + app + 1.0.0 + + + + + com.google.guava + guava + 32.1.3-jre + + + org.junit + junit-bom + 5.10.0 + + + + + + + com.google.guava + guava + + + From 6d10200ba8f6c82e93243ac58dc7b3ca01327d40 Mon Sep 17 00:00:00 2001 From: Test Date: Thu, 30 Jul 2026 16:23:34 +0200 Subject: [PATCH 5/6] Fix root-name collision in SBOM filter and Maven project-version resolution - to_cyclonedx filters the synthetic root by id, not display name, so a real package named 'root' keeps its component (no dangling dependsOn). - pom_project_version stops at the first nested section (dependencyManagement/build/reporting/profiles) so plugin versions can't hijack ${project.version}; parent fallback unchanged. - Property fixed-point resolution re-runs after project.version is inserted so properties aliasing ${project.version} resolve. --- src/deps/ecosystems/maven.rs | 42 +++++++++++++++++++----- src/deps/report.rs | 2 +- src/deps/tests/maven_tests.rs | 10 ++++++ src/deps/tests/report_tests.rs | 31 +++++++++++++++++ tests/fixtures/java-maven-alias/pom.xml | 19 +++++++++++ tests/fixtures/java-maven-parent/pom.xml | 10 ++++++ 6 files changed, 105 insertions(+), 9 deletions(-) create mode 100644 tests/fixtures/java-maven-alias/pom.xml diff --git a/src/deps/ecosystems/maven.rs b/src/deps/ecosystems/maven.rs index fb0281e..a1476a6 100644 --- a/src/deps/ecosystems/maven.rs +++ b/src/deps/ecosystems/maven.rs @@ -164,21 +164,30 @@ fn parse_pom_properties(content: &str) -> std::collections::HashMap`) only + // resolve now that project.version itself is in the map. + resolve_props_fixed_point(&mut props); + props +} + +/// Resolve `${...}` references among property values to a fixed point, +/// bounded to guard against definition cycles. +fn resolve_props_fixed_point(props: &mut std::collections::HashMap) { for _ in 0..5 { let resolved: std::collections::HashMap = props .iter() - .map(|(k, v)| (k.clone(), resolve_placeholders(v, &props))) + .map(|(k, v)| (k.clone(), resolve_placeholders(v, props))) .collect(); - if resolved == props { + if resolved == *props { break; } - props = resolved; - } - let project_version = resolve_placeholders(&pom_project_version(content), &props); - if !project_version.is_empty() && !project_version.contains("${") { - props.insert("project.version".to_string(), project_version); + *props = resolved; } - props } /// The pom's own ``: first `` before ``, @@ -186,6 +195,23 @@ fn parse_pom_properties(content: &str) -> std::collections::HashMap String { 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 = [ + "", + "", + "", + "", + ] + .iter() + .filter_map(|tag| head.find(tag)) + .min(); + let head = match nested_start { + Some(pos) => &head[..pos], + None => head, + }; if let (Some(ps), Some(pe)) = (head.find(""), head.find("")) { if ps < pe { let cleaned = format!("{}{}", &head[..ps], &head[pe + "".len()..]); diff --git a/src/deps/report.rs b/src/deps/report.rs index 245c334..6d5a1d2 100644 --- a/src/deps/report.rs +++ b/src/deps/report.rs @@ -121,7 +121,7 @@ pub fn to_cyclonedx(inv: &Inventory) -> Value { // Deduplicate by bom-ref: multi-module trees list the same package once // per manifest, but the schema sets uniqueItems on components. let mut components: std::collections::BTreeMap<&str, Value> = std::collections::BTreeMap::new(); - for n in graph.nodes.iter().filter(|n| n.name() != "root") { + for n in graph.nodes.iter().filter(|n| n.id().0 != "root") { let mut c = json!({ "type": "library", "bom-ref": n.id().0, diff --git a/src/deps/tests/maven_tests.rs b/src/deps/tests/maven_tests.rs index ebe597e..9a90fe5 100644 --- a/src/deps/tests/maven_tests.rs +++ b/src/deps/tests/maven_tests.rs @@ -222,6 +222,16 @@ fn maven_project_version_resolves_revision_property() { assert_eq!(*n.id(), PackageId("pkg:maven/com.acme/core@2.0.0".into())); } +/// A property aliasing `${project.version}` (e.g. ``) must +/// resolve once `project.version` is derived, not just plain properties. +#[test] +fn maven_property_aliasing_project_version_resolves() { + let inv = scan_fixture("java-maven-alias"); + let n = inv.node("shared").expect("shared node missing"); + assert_eq!(n.version(), Some("1.2.3")); + assert_eq!(*n.id(), PackageId("pkg:maven/com.acme/shared@1.2.3".into())); +} + #[test] fn maven_scan_yields_root_edges() { let inv = scan_fixture("java-maven"); diff --git a/src/deps/tests/report_tests.rs b/src/deps/tests/report_tests.rs index 18edd60..3a1e857 100644 --- a/src/deps/tests/report_tests.rs +++ b/src/deps/tests/report_tests.rs @@ -171,6 +171,37 @@ fn report_cyclonedx_dedups_components_and_depends_on() { ); } +/// The synthetic root is `PackageId("root")` (an id), not a package named +/// "root". A real package named "root" must keep its component and not be +/// dropped by the root filter, or its `dependsOn` ref dangles. +#[test] +fn report_cyclonedx_keeps_component_named_root() { + let n = DependencyNode::new_npm("root", "1.0.0"); + let id = n.id().clone(); + let inv = inventory_with(vec![n], vec![root_edge(&id)]); + let v = to_cyclonedx(&inv); + let components = v["components"].as_array().expect("components array"); + assert!( + components.iter().any(|c| c["purl"] == "pkg:npm/root@1.0.0"), + "package literally named root must keep its component" + ); + + let bom_refs: Vec<&str> = components + .iter() + .filter_map(|c| c["bom-ref"].as_str()) + .collect(); + let deps = v["dependencies"].as_array().expect("dependencies array"); + for d in deps { + for to in d["dependsOn"].as_array().expect("dependsOn") { + let to = to.as_str().unwrap(); + assert!( + to == "root" || bom_refs.contains(&to), + "dependsOn ref {to} must resolve to a component or the literal root" + ); + } + } +} + #[test] fn report_cyclonedx_groups_depends_on_per_ref() { let inv = scan_fixture("node-app"); diff --git a/tests/fixtures/java-maven-alias/pom.xml b/tests/fixtures/java-maven-alias/pom.xml new file mode 100644 index 0000000..9d9cf53 --- /dev/null +++ b/tests/fixtures/java-maven-alias/pom.xml @@ -0,0 +1,19 @@ + + + 4.0.0 + com.acme + app + 1.2.3 + + + ${project.version} + + + + + com.acme + shared + ${shared.version} + + + diff --git a/tests/fixtures/java-maven-parent/pom.xml b/tests/fixtures/java-maven-parent/pom.xml index eb13152..7525b83 100644 --- a/tests/fixtures/java-maven-parent/pom.xml +++ b/tests/fixtures/java-maven-parent/pom.xml @@ -9,6 +9,16 @@ com.acme child-module + + + + org.apache.maven.plugins + maven-compiler-plugin + 3.11.0 + + + + com.acme From 37362688d95707fc362518ee5db11a574cbe4852 Mon Sep 17 00:00:00 2001 From: Test Date: Thu, 30 Jul 2026 16:32:34 +0200 Subject: [PATCH 6/6] Simplify: shared split_section helper, PackageId::root() comparison split_dependency_management and pom_project_version's parent handling used the same find-open/find-close/remove-section scanning; extract it into split_section. Compare the synthetic root via PackageId::root() instead of a raw string literal, matching the rest of the codebase. --- src/deps/ecosystems/maven.rs | 38 ++++++++++++++++++++---------------- src/deps/report.rs | 4 ++-- 2 files changed, 23 insertions(+), 19 deletions(-) diff --git a/src/deps/ecosystems/maven.rs b/src/deps/ecosystems/maven.rs index a1476a6..77aba93 100644 --- a/src/deps/ecosystems/maven.rs +++ b/src/deps/ecosystems/maven.rs @@ -121,16 +121,23 @@ fn parse_pom_dependencies(content: &str) -> Result, DepsError> { /// for the project's dependencies but are not dependencies themselves. /// Returns (pom without the section, the section's inner content). fn split_dependency_management(content: &str) -> (String, &str) { - const OPEN: &str = ""; - const CLOSE: &str = ""; - if let (Some(s), Some(e)) = (content.find(OPEN), content.find(CLOSE)) { - if s < e { - let management = &content[s + OPEN.len()..e]; - let rest = format!("{}{}", &content[..s], &content[e + CLOSE.len()..]); - return (rest, management); - } + split_section(content, "dependencyManagement").unwrap_or_else(|| (content.to_string(), "")) +} + +/// Locate a `...` section and split content into (content with +/// the section removed, the section's inner content). `None` if the tag +/// isn't present or is malformed (close before open). +fn split_section<'a>(content: &'a str, tag: &str) -> Option<(String, &'a str)> { + let open = format!("<{tag}>"); + let close = format!(""); + let s = content.find(&open)?; + let e = content.find(&close)?; + if s >= e { + return None; } - (content.to_string(), "") + let inner = &content[s + open.len()..e]; + let rest = format!("{}{}", &content[..s], &content[e + close.len()..]); + Some((rest, inner)) } /// Collect `` entries plus the built-in `project.version`. @@ -212,15 +219,12 @@ fn pom_project_version(content: &str) -> String { Some(pos) => &head[..pos], None => head, }; - if let (Some(ps), Some(pe)) = (head.find(""), head.find("")) { - if ps < pe { - let cleaned = format!("{}{}", &head[..ps], &head[pe + "".len()..]); - let own = extract_xml_tag(&cleaned, "version"); - if !own.is_empty() { - return own; - } - return extract_xml_tag(&head[ps..pe], "version"); + if let Some((cleaned, parent)) = split_section(head, "parent") { + let own = extract_xml_tag(&cleaned, "version"); + if !own.is_empty() { + return own; } + return extract_xml_tag(parent, "version"); } extract_xml_tag(head, "version") } diff --git a/src/deps/report.rs b/src/deps/report.rs index 6d5a1d2..2ff012f 100644 --- a/src/deps/report.rs +++ b/src/deps/report.rs @@ -2,7 +2,7 @@ use serde_json::{json, Value}; use std::fmt::Write as _; use crate::deps::detect::DetectedFile; -use crate::deps::model::{DependencyGraph, Ecosystem}; +use crate::deps::model::{DependencyGraph, Ecosystem, PackageId}; use crate::deps::Inventory; /// Load the policy at `root`, scan the tree, and emit a CycloneDX SBOM. @@ -121,7 +121,7 @@ pub fn to_cyclonedx(inv: &Inventory) -> Value { // Deduplicate by bom-ref: multi-module trees list the same package once // per manifest, but the schema sets uniqueItems on components. let mut components: std::collections::BTreeMap<&str, Value> = std::collections::BTreeMap::new(); - for n in graph.nodes.iter().filter(|n| n.id().0 != "root") { + for n in graph.nodes.iter().filter(|n| *n.id() != PackageId::root()) { let mut c = json!({ "type": "library", "bom-ref": n.id().0,