From 62d6107fcb3f876fd74d8c00784b06b2b362258f Mon Sep 17 00:00:00 2001 From: "Contain.Security" Date: Mon, 29 Jun 2026 22:36:47 -0700 Subject: [PATCH] Fix project-scoped affects, duplicate bom-refs CycloneDX VEX/VDR exports set every affects[].ref to the project UUID and emitted one vulnerability entry per finding, all sharing the bare vulnerability UUID as their bom-ref. That violated the CycloneDX rule that every bom-ref is unique within a BOM, and on re-import the project-level affects broadcast one component's analysis to every component sharing the vulnerability. Findings are now grouped by (vulnerability, analysis fingerprint) per CycloneDX VEX Use-Case 13: one entry per group, affects[] carrying the sorted union of affected component bom-refs. When a vulnerability splits across analyses, each entry's bom-ref gets a deterministic suffix (/0, /1) so uniqueness holds. The Variant enum now carries capability flags in place of scattered switch/== checks. ModelConverter.analysisFingerprint is pinned to its positional, enum-name contract, with a round-trip regression proving an exported VEX re-imports its analysis onto the analysed component only. Fixes #6016 Fixes #6017 Signed-off-by: Contain.Security --- .../parser/cyclonedx/CycloneDXExporter.java | 69 ++++-- .../parser/cyclonedx/util/ModelConverter.java | 234 ++++++++++++++---- .../cyclonedx/CycloneDXVexImporterTest.java | 60 +++++ .../cyclonedx/CycloneDxBomAssertions.java | 112 +++++++++ .../cyclonedx/util/ModelConverterTest.java | 90 +++++++ .../resources/v1/BomResourceTest.java | 32 +-- .../resources/v1/VexResourceTest.java | 75 +++++- 7 files changed, 579 insertions(+), 93 deletions(-) create mode 100644 apiserver/src/test/java/org/dependencytrack/parser/cyclonedx/CycloneDxBomAssertions.java create mode 100644 apiserver/src/test/java/org/dependencytrack/parser/cyclonedx/util/ModelConverterTest.java diff --git a/apiserver/src/main/java/org/dependencytrack/parser/cyclonedx/CycloneDXExporter.java b/apiserver/src/main/java/org/dependencytrack/parser/cyclonedx/CycloneDXExporter.java index c663a2bb33..061e8b50ef 100644 --- a/apiserver/src/main/java/org/dependencytrack/parser/cyclonedx/CycloneDXExporter.java +++ b/apiserver/src/main/java/org/dependencytrack/parser/cyclonedx/CycloneDXExporter.java @@ -48,10 +48,53 @@ public enum Format { } public enum Variant { - INVENTORY, - INVENTORY_WITH_VULNERABILITIES, - VDR, - VEX + INVENTORY (false, false, false, true, true), + INVENTORY_WITH_VULNERABILITIES (false, false, true, true, true), + VDR (true, true, true, true, true), + VEX (true, true, true, false, false); + + private final boolean filtersToVulnerableComponents; + private final boolean includesAnalysis; + private final boolean emitsFindings; + private final boolean includesDependencyGraph; + private final boolean includesServices; + + Variant(final boolean filtersToVulnerableComponents, + final boolean includesAnalysis, + final boolean emitsFindings, + final boolean includesDependencyGraph, + final boolean includesServices) { + this.filtersToVulnerableComponents = filtersToVulnerableComponents; + this.includesAnalysis = includesAnalysis; + this.emitsFindings = emitsFindings; + this.includesDependencyGraph = includesDependencyGraph; + this.includesServices = includesServices; + } + + /** Whether {@code components[]} is restricted to components that have findings. */ + public boolean filtersToVulnerableComponents() { + return filtersToVulnerableComponents; + } + + /** Whether per-component VEX analysis is emitted on vulnerability entries. */ + public boolean includesAnalysis() { + return includesAnalysis; + } + + /** Whether findings are loaded and emitted as {@code vulnerabilities[]}. */ + public boolean emitsFindings() { + return emitsFindings; + } + + /** Whether the dependency graph is emitted. */ + public boolean includesDependencyGraph() { + return includesDependencyGraph; + } + + /** Whether services are emitted. */ + public boolean includesServices() { + return includesServices; + } } private final QueryManager qm; @@ -70,11 +113,9 @@ public Bom create(final Project project) { components = qm.getAllComponents(project); services = qm.getAllServiceComponents(project); } - final List findings = switch (variant) { - case INVENTORY_WITH_VULNERABILITIES, VDR, VEX -> withJdbiHandle(handle -> - handle.attach(FindingDao.class).getFindings(project.getId(), true)); - default -> null; - }; + final List findings = variant.emitsFindings() + ? withJdbiHandle(handle -> handle.attach(FindingDao.class).getFindings(project.getId(), true)) + : null; return create(components, services, findings, project); } @@ -84,8 +125,8 @@ public Bom create(final Component component) { return create(components, null, null, null); } - private Bom create(Listcomponents, final List services, final List findings, final Project project) { - if (Variant.VDR == variant) { + private Bom create(List components, final List services, final List findings, final Project project) { + if (variant.filtersToVulnerableComponents()) { final Set vulnerableComponentUuids = findings.stream() .map(finding -> (UUID) finding.getComponent().get("uuid")) .collect(Collectors.toSet()); @@ -93,8 +134,8 @@ private Bom create(Listcomponents, final List servi .filter(component -> vulnerableComponentUuids.contains(component.getUuid())) .toList(); } - final List cycloneComponents = (Variant.VEX != variant && components != null) ? components.stream().map(component -> ModelConverter.convert(component)).collect(Collectors.toList()) : null; - final List cycloneServices = (Variant.VEX != variant && services != null) ? services.stream().map(service -> ModelConverter.convert(qm, service)).collect(Collectors.toList()) : null; + final List cycloneComponents = components != null ? components.stream().map(component -> ModelConverter.convert(component)).collect(Collectors.toList()) : null; + final List cycloneServices = (variant.includesServices() && services != null) ? services.stream().map(service -> ModelConverter.convert(qm, service)).collect(Collectors.toList()) : null; final Bom bom = new Bom(); bom.setSerialNumber("urn:uuid:" + UUID.randomUUID()); bom.setVersion(1); @@ -102,7 +143,7 @@ private Bom create(Listcomponents, final List servi bom.setComponents(cycloneComponents); bom.setServices(cycloneServices); bom.setVulnerabilities(ModelConverter.generateVulnerabilities(qm, variant, findings)); - if (cycloneComponents != null) { + if (variant.includesDependencyGraph() && cycloneComponents != null) { bom.setDependencies(ModelConverter.generateDependencies(project, components)); } return bom; diff --git a/apiserver/src/main/java/org/dependencytrack/parser/cyclonedx/util/ModelConverter.java b/apiserver/src/main/java/org/dependencytrack/parser/cyclonedx/util/ModelConverter.java index 7a082ff829..9acc0bd951 100644 --- a/apiserver/src/main/java/org/dependencytrack/parser/cyclonedx/util/ModelConverter.java +++ b/apiserver/src/main/java/org/dependencytrack/parser/cyclonedx/util/ModelConverter.java @@ -76,11 +76,14 @@ import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import java.util.HashMap; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Objects; import java.util.Optional; import java.util.Set; +import java.util.TreeMap; import java.util.UUID; import java.util.function.BiConsumer; import java.util.function.Consumer; @@ -97,6 +100,14 @@ public class ModelConverter { private static final Logger LOGGER = LoggerFactory.getLogger(ModelConverter.class); + /** + * U+001F (Unit Separator) field delimiter for composite keys. A reserved control character + * with no legitimate occurrence in vulnerability UUIDs or analysis-detail prose, so it cannot + * collide with payload data when used to glue group-key fields together. Package-private so + * tests pin against the production constant rather than re-declaring the literal byte. + */ + static final char FIELD_SEPARATOR = '\u001F'; + /** * Private Constructor. */ @@ -927,14 +938,105 @@ public static org.cyclonedx.model.Service convert(final QueryManager qm, final S return cycloneService; } - public static org.cyclonedx.model.vulnerability.Vulnerability convert(final QueryManager qm, final CycloneDXExporter.Variant variant, - final Finding finding) { - final Component component = qm.getObjectByUuid(Component.class, finding.getComponent().get("uuid").toString()); - final Project project = component.getProject(); - final Vulnerability vulnerability = qm.getObjectByUuid(Vulnerability.class, finding.getVulnerability().get("uuid").toString()); + /** + * Stable serialisation of the analysis fields used to distinguish per-component analyses that + * share a vulnerability. Null fields serialise to the empty string; non-null enums use + * {@link Enum#name()} so the contract does not depend on {@link String#valueOf(Object)} + * returning the literal {@code "null"} for absent values. + */ + static String analysisFingerprint(final Analysis analysis) { + if (analysis == null) { + return ""; + } + return enumName(analysis.getAnalysisState()) + FIELD_SEPARATOR + + enumName(analysis.getAnalysisJustification()) + FIELD_SEPARATOR + + enumName(analysis.getAnalysisResponse()) + FIELD_SEPARATOR + + Objects.toString(trimToNull(analysis.getAnalysisDetails()), ""); + } + + private static String enumName(final Enum value) { + return value == null ? "" : value.name(); + } + + private static String componentUuid(final Finding finding) { + return Objects.requireNonNull(finding.getComponent().get("uuid"), "componentUuid").toString(); + } + + private static String vulnerabilityUuid(final Finding finding) { + return Objects.requireNonNull(finding.getVulnerability().get("uuid"), "vulnerabilityUuid").toString(); + } + + private static String analysisCacheKey(final Finding finding) { + return componentUuid(finding) + FIELD_SEPARATOR + vulnerabilityUuid(finding); + } + + private static Analysis resolveAnalysis(final QueryManager qm, final Finding finding) { + final Component component = qm.getObjectByUuid(Component.class, componentUuid(finding)); + final Vulnerability vulnerability = qm.getObjectByUuid(Vulnerability.class, vulnerabilityUuid(finding)); + return qm.getAnalysis(component, vulnerability); + } + + /** + * Build a stable grouping key for a finding. The key is the vulnerability identifier alone for + * variants that do not emit analysis (so every component affected by a vulnerability collapses + * into one group), widened with the analysis fingerprint when the variant emits analysis. + */ + private static String buildGroupKey(final CycloneDXExporter.Variant variant, final Finding finding, + final Map analysisCache) { + final String vulnUuid = vulnerabilityUuid(finding); + if (!variant.includesAnalysis()) { + return vulnUuid; + } + return vulnUuid + FIELD_SEPARATOR + analysisFingerprint(analysisCache.get(analysisCacheKey(finding))); + } + + /** + * Convert one group of findings (sharing a vulnerability and, when analysis is emitted, an + * analysis fingerprint) into a single CycloneDX vulnerability entry whose {@code affects[]} + * carries the sorted union of affected component bom-refs. + */ + private static org.cyclonedx.model.vulnerability.Vulnerability convertGroup( + final QueryManager qm, final CycloneDXExporter.Variant variant, + final List groupFindings, final Map analysisCache) { + final Finding first = groupFindings.get(0); + final Vulnerability vulnerability = qm.getObjectByUuid(Vulnerability.class, vulnerabilityUuid(first)); final org.cyclonedx.model.vulnerability.Vulnerability cdxVulnerability = new org.cyclonedx.model.vulnerability.Vulnerability(); - cdxVulnerability.setBomRef(vulnerability.getUuid().toString()); + populateScalarFields(cdxVulnerability, vulnerability); + cdxVulnerability.setAffects(buildAffects(groupFindings)); + + if (variant.includesAnalysis()) { + final Analysis analysis = analysisCache.get(analysisCacheKey(first)); + if (analysis != null) { + cdxVulnerability.setAnalysis(buildCdxAnalysis(analysis)); + } + } + return cdxVulnerability; + } + + /** + * The sorted, de-duplicated union of the affected component bom-refs in a group. Sorting keeps + * the output deterministic regardless of finding order. + */ + private static List buildAffects( + final List groupFindings) { + final List componentUuids = groupFindings.stream() + .map(ModelConverter::componentUuid) + .distinct() + .sorted() + .toList(); + final List affects = new ArrayList<>(componentUuids.size()); + for (final String uuid : componentUuids) { + final org.cyclonedx.model.vulnerability.Vulnerability.Affect affect = new org.cyclonedx.model.vulnerability.Vulnerability.Affect(); + affect.setRef(uuid); + affects.add(affect); + } + return affects; + } + + private static void populateScalarFields( + final org.cyclonedx.model.vulnerability.Vulnerability cdxVulnerability, + final Vulnerability vulnerability) { cdxVulnerability.setId(vulnerability.getVulnId()); // Add the vulnerability source org.cyclonedx.model.vulnerability.Vulnerability.Source cdxSource = new org.cyclonedx.model.vulnerability.Vulnerability.Source(); @@ -1021,48 +1123,26 @@ public static org.cyclonedx.model.vulnerability.Vulnerability convert(final Quer cdxVulnerability.setCreated(vulnerability.getCreated()); cdxVulnerability.setPublished(vulnerability.getPublished()); cdxVulnerability.setUpdated(vulnerability.getUpdated()); + } - if (CycloneDXExporter.Variant.INVENTORY_WITH_VULNERABILITIES == variant || CycloneDXExporter.Variant.VDR == variant) { - final List affects = new ArrayList<>(); - final org.cyclonedx.model.vulnerability.Vulnerability.Affect affect = new org.cyclonedx.model.vulnerability.Vulnerability.Affect(); - affect.setRef(component.getUuid().toString()); - affects.add(affect); - cdxVulnerability.setAffects(affects); - } else if (CycloneDXExporter.Variant.VEX == variant && project != null) { - final List affects = new ArrayList<>(); - final org.cyclonedx.model.vulnerability.Vulnerability.Affect affect = new org.cyclonedx.model.vulnerability.Vulnerability.Affect(); - affect.setRef(project.getUuid().toString()); - affects.add(affect); - cdxVulnerability.setAffects(affects); - } - - if (CycloneDXExporter.Variant.VEX == variant || CycloneDXExporter.Variant.VDR == variant) { - final Analysis analysis = qm.getAnalysis( - qm.getObjectByUuid(Component.class, component.getUuid()), - qm.getObjectByUuid(Vulnerability.class, vulnerability.getUuid()) - ); - if (analysis != null) { - final org.cyclonedx.model.vulnerability.Vulnerability.Analysis cdxAnalysis = new org.cyclonedx.model.vulnerability.Vulnerability.Analysis(); - if (analysis.getAnalysisResponse() != null) { - final org.cyclonedx.model.vulnerability.Vulnerability.Analysis.Response response = convertDtVulnAnalysisResponseToCdxAnalysisResponse(analysis.getAnalysisResponse()); - if (response != null) { - List responses = new ArrayList<>(); - responses.add(response); - cdxAnalysis.setResponses(responses); - } - } - if (analysis.getAnalysisState() != null) { - cdxAnalysis.setState(convertDtVulnAnalysisStateToCdxAnalysisState(analysis.getAnalysisState())); - } - if (analysis.getAnalysisJustification() != null) { - cdxAnalysis.setJustification(convertDtVulnAnalysisJustificationToCdxAnalysisJustification(analysis.getAnalysisJustification())); - } - cdxAnalysis.setDetail(StringUtils.trimToNull(analysis.getAnalysisDetails())); - cdxVulnerability.setAnalysis(cdxAnalysis); + private static org.cyclonedx.model.vulnerability.Vulnerability.Analysis buildCdxAnalysis(final Analysis analysis) { + final org.cyclonedx.model.vulnerability.Vulnerability.Analysis cdxAnalysis = new org.cyclonedx.model.vulnerability.Vulnerability.Analysis(); + if (analysis.getAnalysisResponse() != null) { + final org.cyclonedx.model.vulnerability.Vulnerability.Analysis.Response response = convertDtVulnAnalysisResponseToCdxAnalysisResponse(analysis.getAnalysisResponse()); + if (response != null) { + List responses = new ArrayList<>(); + responses.add(response); + cdxAnalysis.setResponses(responses); } } - - return cdxVulnerability; + if (analysis.getAnalysisState() != null) { + cdxAnalysis.setState(convertDtVulnAnalysisStateToCdxAnalysisState(analysis.getAnalysisState())); + } + if (analysis.getAnalysisJustification() != null) { + cdxAnalysis.setJustification(convertDtVulnAnalysisJustificationToCdxAnalysisJustification(analysis.getAnalysisJustification())); + } + cdxAnalysis.setDetail(StringUtils.trimToNull(analysis.getAnalysisDetails())); + return cdxAnalysis; } /** @@ -1280,6 +1360,25 @@ public static AnalysisJustification convertCdxVulnAnalysisJustificationToDtAnaly } } + /** + * Group findings by {@code (vulnerability, analysis fingerprint)} and emit one + * {@link org.cyclonedx.model.vulnerability.Vulnerability} entry per group with {@code affects[]} + * carrying the union of affected component bom-refs — the pattern documented as CycloneDX VEX + * Use-Case 13. + * + *

For variants that do not emit analysis (INVENTORY_WITH_VULNERABILITIES) every finding for a + * given vulnerability collapses into one group. For variants that do (VDR, VEX) findings sharing + * the same per-component analysis for a given vulnerability are grouped together. + * + *

Each emitted entry carries a {@code bom-ref}. Where one vulnerability produces a single + * group (the common case) the {@code bom-ref} is the bare Dependency-Track vulnerability UUID — + * matching the Hyades reference exports. Where one vulnerability produces multiple groups (the + * Use-Case 13 analysis split) entries carry a numeric suffix ({@code /0}, {@code /1}, + * …) so the spec rule that every {@code bom-ref} is unique within the BOM still holds. + * + *

Per-finding {@link Analysis} lookups are memoised so the group-key pass and the + * group-conversion pass share a single query per {@code (component, vulnerability)} pair. + */ public static List generateVulnerabilities( final QueryManager qm, final CycloneDXExporter.Variant variant, @@ -1288,11 +1387,46 @@ public static List generateVuln if (findings == null) { return Collections.emptyList(); } - final var vulnerabilitiesSeen = new HashSet(); - return findings.stream() - .map(finding -> convert(qm, variant, finding)) - .filter(vulnerabilitiesSeen::add) - .toList(); + // Only populated for variants that emit analysis; otherwise it stays empty and the + // includesAnalysis() guards short-circuit the reads. + final Map analysisCache = new HashMap<>( + variant.includesAnalysis() ? findings.size() : 0); + // TreeMap keeps the emitted entries (and their bom-ref suffixes) in deterministic key order. + final Map> groups = new TreeMap<>(); + for (final Finding finding : findings) { + if (variant.includesAnalysis()) { + analysisCache.computeIfAbsent(analysisCacheKey(finding), k -> resolveAnalysis(qm, finding)); + } + groups.computeIfAbsent(buildGroupKey(variant, finding, analysisCache), k -> new ArrayList<>()) + .add(finding); + } + // Pre-pass: a vulnerability that produced more than one group (the Use-Case 13 analysis + // split) needs a disambiguating suffix on each bom-ref; singletons keep the bare UUID. + final Map vulnUuidGroupCount = new HashMap<>(); + for (final List group : groups.values()) { + vulnUuidGroupCount.merge(vulnerabilityUuid(group.get(0)), 1, Integer::sum); + } + final Map vulnUuidNextIndex = new HashMap<>(); + final List result = new ArrayList<>(groups.size()); + for (final List group : groups.values()) { + final org.cyclonedx.model.vulnerability.Vulnerability cdxVuln = convertGroup(qm, variant, group, analysisCache); + cdxVuln.setBomRef(buildVulnBomRef(group, vulnUuidGroupCount, vulnUuidNextIndex)); + result.add(cdxVuln); + } + return result; + } + + private static String buildVulnBomRef(final List group, + final Map vulnUuidGroupCount, + final Map vulnUuidNextIndex) { + final String vulnUuid = vulnerabilityUuid(group.get(0)); + if (vulnUuidGroupCount.getOrDefault(vulnUuid, 0) <= 1) { + return vulnUuid; + } + // The slash isolates the suffix from the UUID alphabet, so a singleton's bare UUID can never + // be a prefix collision with a multi-group entry. Indices follow TreeMap key order. + final int idx = vulnUuidNextIndex.merge(vulnUuid, 0, (old, ignored) -> old + 1); + return vulnUuid + "/" + idx; } public static org.cyclonedx.model.Component.Scope mapCdxScope(Scope scope) { diff --git a/apiserver/src/test/java/org/dependencytrack/parser/cyclonedx/CycloneDXVexImporterTest.java b/apiserver/src/test/java/org/dependencytrack/parser/cyclonedx/CycloneDXVexImporterTest.java index 17f22cc3ce..8f3b0e8f07 100644 --- a/apiserver/src/test/java/org/dependencytrack/parser/cyclonedx/CycloneDXVexImporterTest.java +++ b/apiserver/src/test/java/org/dependencytrack/parser/cyclonedx/CycloneDXVexImporterTest.java @@ -20,7 +20,9 @@ import org.apache.commons.io.IOUtils; import org.assertj.core.api.Assertions; +import org.cyclonedx.Version; import org.cyclonedx.exception.ParseException; +import org.cyclonedx.model.Bom; import org.cyclonedx.parsers.BomParserFactory; import org.dependencytrack.PersistenceCapableTest; import org.dependencytrack.model.Analysis; @@ -32,6 +34,7 @@ import org.dependencytrack.model.Project; import org.dependencytrack.model.Severity; import org.dependencytrack.model.Vulnerability; +import org.dependencytrack.persistence.command.MakeAnalysisCommand; import org.junit.jupiter.api.Test; import javax.jdo.Query; @@ -204,6 +207,63 @@ public void shouldPersistLastResponseAndCommentEach() throws ParseException { .containsExactly("Vendor Response: NOT_SET → UPDATE"); } + /** + * Round-trip regression for the VEX bom-ref fix: when several components share a vulnerability + * but only one is analysed, an exported-then-reimported VEX must apply that analysis to the + * analysed component only. Before the fix every {@code affects[].ref} pointed at the project + * root, which the importer broadcast to every vulnerable component. + */ + @Test + public void shouldApplyExportedVexAnalysisOnlyToAnalyzedComponent() throws Exception { + final var project = new Project(); + project.setName("acme-app"); + project.setVersion("1.0.0"); + qm.persist(project); + + final var componentA = new Component(); + componentA.setProject(project); + componentA.setName("acme-lib-a"); + componentA.setVersion("1.0.0"); + qm.persist(componentA); + + final var componentB = new Component(); + componentB.setProject(project); + componentB.setName("acme-lib-b"); + componentB.setVersion("1.0.0"); + qm.persist(componentB); + + final var vuln = new Vulnerability(); + vuln.setVulnId("INT-001"); + vuln.setSource(Vulnerability.Source.INTERNAL); + vuln.setSeverity(Severity.HIGH); + vuln.setComponents(List.of(componentA, componentB)); + qm.persist(vuln); + qm.addVulnerability(vuln, componentA, "none"); + qm.addVulnerability(vuln, componentB, "none"); + + // Analyse componentA only. + qm.makeAnalysis(new MakeAnalysisCommand(componentA, vuln) + .withState(AnalysisState.RESOLVED) + .withResponse(AnalysisResponse.UPDATE) + .withSuppress(true)); + + // Export a VEX for the project, then re-import it into the same project. + final var exporter = new CycloneDXExporter(CycloneDXExporter.Variant.VEX, qm); + final byte[] vexBytes = exporter + .export(exporter.create(project), CycloneDXExporter.Format.JSON, Version.VERSION_15) + .getBytes(StandardCharsets.UTF_8); + final Bom vex = BomParserFactory.createParser(vexBytes).parse(vexBytes); + vexImporter.applyVex(qm, vex, project); + + // componentA keeps its analysis; componentB must never receive one. + final Analysis analysisA = qm.getAnalysis(componentA, vuln); + Assertions.assertThat(analysisA).isNotNull(); + Assertions.assertThat(analysisA.getAnalysisState()).isEqualTo(AnalysisState.RESOLVED); + + final Analysis analysisB = qm.getAnalysis(componentB, vuln); + Assertions.assertThat(analysisB).isNull(); + } + private static final String OWASP_VECTOR = "OWASP/SL:1/M:1/O:0/S:2/ED:1/EE:1/A:1/ID:1/LC:2/LI:1/LAV:1/LAC:1/FD:1/RD:1/NC:2/PV:3"; diff --git a/apiserver/src/test/java/org/dependencytrack/parser/cyclonedx/CycloneDxBomAssertions.java b/apiserver/src/test/java/org/dependencytrack/parser/cyclonedx/CycloneDxBomAssertions.java new file mode 100644 index 0000000000..b5ea39e120 --- /dev/null +++ b/apiserver/src/test/java/org/dependencytrack/parser/cyclonedx/CycloneDxBomAssertions.java @@ -0,0 +1,112 @@ +/* + * This file is part of Dependency-Track. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + * Copyright (c) OWASP Foundation. All Rights Reserved. + */ +package org.dependencytrack.parser.cyclonedx; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import org.assertj.core.api.Assertions; + +import java.util.ArrayList; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; + +/** + * Test-only assertions that enforce CycloneDX specification rules JSON Schema cannot express. + * + *

The CycloneDX 1.5+ schema description on every {@code bom-ref}-bearing object states that + * "Every bom-ref MUST be unique within the BOM" — across {@code metadata.component}, + * {@code components[]}, {@code services[]}, and {@code vulnerabilities[]}. JSON Schema draft-07 + * cannot express cross-property uniqueness, so producers must be guarded by separate test-side + * assertions like the one below. + */ +public final class CycloneDxBomAssertions { + + private static final ObjectMapper MAPPER = new ObjectMapper(); + + private CycloneDxBomAssertions() { + } + + /** + * Assert that every {@code bom-ref} value appearing anywhere in the BOM is unique across the + * whole document. + * + * @param json the serialised CycloneDX BOM + * @throws AssertionError if any {@code bom-ref} value occurs more than once, or the BOM cannot + * be parsed + */ + public static void assertBomRefsUnique(final String json) { + final JsonNode root; + try { + root = MAPPER.readTree(json); + } catch (Exception e) { + throw new AssertionError("Failed to parse BOM JSON for bom-ref uniqueness check", e); + } + final Map> refToPaths = new LinkedHashMap<>(); + collectBomRefs(root, "$", refToPaths); + + final Map> duplicates = new LinkedHashMap<>(); + refToPaths.forEach((ref, paths) -> { + if (paths.size() > 1) { + duplicates.put(ref, paths); + } + }); + Assertions.assertThat(duplicates) + .as("CycloneDX bom-refs that appear more than once across the document; " + + "value -> JSON paths where the duplicate occurred") + .isEmpty(); + } + + private static void collectBomRefs(final JsonNode node, final String path, + final Map> sink) { + if (node == null) { + return; + } + if (node.isObject()) { + final JsonNode ref = node.get("bom-ref"); + if (ref != null && ref.isTextual()) { + sink.computeIfAbsent(ref.asText(), k -> new ArrayList<>()).add(path); + } + node.fields().forEachRemaining(entry -> + collectBomRefs(entry.getValue(), path + appendKey(entry.getKey()), sink)); + } else if (node.isArray()) { + int i = 0; + for (final JsonNode child : node) { + collectBomRefs(child, path + "[" + i + "]", sink); + i++; + } + } + } + + /** + * Render a JSON object key as a path segment. Plain identifiers use dot-notation + * ({@code .foo}); keys with characters that would make the dot form ambiguous — + * {@code .}, {@code [}, {@code ]}, {@code "}, or whitespace — fall back to bracket + * notation with the key escaped ({@code ["foo.bar"]}). + */ + private static String appendKey(final String key) { + for (int i = 0; i < key.length(); i++) { + final char c = key.charAt(i); + if (c == '.' || c == '[' || c == ']' || c == '"' || Character.isWhitespace(c)) { + return "[\"" + key.replace("\\", "\\\\").replace("\"", "\\\"") + "\"]"; + } + } + return "." + key; + } +} diff --git a/apiserver/src/test/java/org/dependencytrack/parser/cyclonedx/util/ModelConverterTest.java b/apiserver/src/test/java/org/dependencytrack/parser/cyclonedx/util/ModelConverterTest.java new file mode 100644 index 0000000000..066f712b65 --- /dev/null +++ b/apiserver/src/test/java/org/dependencytrack/parser/cyclonedx/util/ModelConverterTest.java @@ -0,0 +1,90 @@ +/* + * This file is part of Dependency-Track. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + * Copyright (c) OWASP Foundation. All Rights Reserved. + */ +package org.dependencytrack.parser.cyclonedx.util; + +import org.dependencytrack.model.Analysis; +import org.dependencytrack.model.AnalysisJustification; +import org.dependencytrack.model.AnalysisResponse; +import org.dependencytrack.model.AnalysisState; +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +class ModelConverterTest { + + /** + * Reference the production constant directly so a future change to the byte used as the + * group-key separator forces this test to be re-checked rather than silently passing + * against a stale literal. + */ + private static final String SEP = String.valueOf(ModelConverter.FIELD_SEPARATOR); + + @Test + void analysisFingerprintReturnsEmptyForNullAnalysis() { + assertThat(ModelConverter.analysisFingerprint(null)).isEmpty(); + } + + @Test + void analysisFingerprintEmitsEnumNameNotToString() { + final Analysis analysis = new Analysis(); + analysis.setAnalysisState(AnalysisState.EXPLOITABLE); + analysis.setAnalysisJustification(AnalysisJustification.CODE_NOT_REACHABLE); + analysis.setAnalysisResponse(AnalysisResponse.UPDATE); + analysis.setAnalysisDetails("hand-triaged"); + + assertThat(ModelConverter.analysisFingerprint(analysis)) + .isEqualTo("EXPLOITABLE" + SEP + "CODE_NOT_REACHABLE" + SEP + "UPDATE" + SEP + "hand-triaged"); + } + + @Test + void analysisFingerprintFormatIsStableAcrossNullFields() { + final Analysis stateOnly = new Analysis(); + stateOnly.setAnalysisState(AnalysisState.EXPLOITABLE); + + // The fingerprint contract is positional — null fields collapse to the empty string so a + // future "EXPLOITABLE/null/null/null" analysis cannot collide with an analysis whose state + // happens to render as "EXPLOITABLEnull" via String.valueOf(Object). + assertThat(ModelConverter.analysisFingerprint(stateOnly)) + .isEqualTo("EXPLOITABLE" + SEP + "" + SEP + "" + SEP + ""); + } + + @Test + void analysisFingerprintTrimsBlankDetailsToEmpty() { + final Analysis blankDetails = new Analysis(); + blankDetails.setAnalysisState(AnalysisState.RESOLVED); + blankDetails.setAnalysisDetails(" "); + + assertThat(ModelConverter.analysisFingerprint(blankDetails)) + .isEqualTo("RESOLVED" + SEP + "" + SEP + "" + SEP + ""); + } + + @Test + void analysisFingerprintDistinguishesByEachField() { + final Analysis a = new Analysis(); + a.setAnalysisState(AnalysisState.EXPLOITABLE); + a.setAnalysisResponse(AnalysisResponse.UPDATE); + + final Analysis b = new Analysis(); + b.setAnalysisState(AnalysisState.EXPLOITABLE); + b.setAnalysisResponse(AnalysisResponse.WORKAROUND_AVAILABLE); + + assertThat(ModelConverter.analysisFingerprint(a)) + .isNotEqualTo(ModelConverter.analysisFingerprint(b)); + } +} diff --git a/apiserver/src/test/java/org/dependencytrack/resources/v1/BomResourceTest.java b/apiserver/src/test/java/org/dependencytrack/resources/v1/BomResourceTest.java index d730d4f5fc..894999891f 100644 --- a/apiserver/src/test/java/org/dependencytrack/resources/v1/BomResourceTest.java +++ b/apiserver/src/test/java/org/dependencytrack/resources/v1/BomResourceTest.java @@ -62,6 +62,7 @@ import org.dependencytrack.model.Vulnerability; import org.dependencytrack.notification.NotificationScope; import org.dependencytrack.notification.proto.v1.BomValidationFailedSubject; +import org.dependencytrack.parser.cyclonedx.CycloneDxBomAssertions; import org.dependencytrack.parser.cyclonedx.CycloneDxValidator; import org.dependencytrack.persistence.command.MakeAnalysisCommand; import org.dependencytrack.resources.v1.vo.BomSubmitRequest; @@ -758,25 +759,7 @@ void exportProjectAsCycloneDxInventoryWithVulnerabilitiesTest() { "affects": [ { "ref": "${json-unit.matches:componentWithVulnUuid}" - } - ] - }, - { - "bom-ref": "${json-unit.matches:vulnUuid}", - "id": "INT-001", - "source": { - "name": "INTERNAL" - }, - "ratings": [ - { - "source": { - "name": "INTERNAL" - }, - "severity": "high", - "method": "other" - } - ], - "affects": [ + }, { "ref": "${json-unit.matches:componentWithVulnAndAnalysisUuid}" } @@ -785,6 +768,7 @@ void exportProjectAsCycloneDxInventoryWithVulnerabilitiesTest() { ] } """)); + CycloneDxBomAssertions.assertBomRefsUnique(jsonResponse); // Ensure the dependency graph did not get deleted during export. // https://github.com/DependencyTrack/dependency-track/issues/2494 @@ -892,6 +876,11 @@ void exportProjectAsCycloneDxVdrTest() { .withMatcher("componentWithoutVulnUuid", equalTo(componentWithoutVuln.getUuid().toString())) .withMatcher("componentWithVulnUuid", equalTo(componentWithVuln.getUuid().toString())) .withMatcher("componentWithVulnAndAnalysisUuid", equalTo(componentWithVulnAndAnalysis.getUuid().toString())) + // One CVE affects two components with distinct analysis fingerprints (no analysis vs + // RESOLVED), so VDR emits two entries with deterministic suffixed bom-refs. The empty + // fingerprint sorts before "RESOLVED", so the un-analysed component is /0. + .withMatcher("vulnUuidSuffix0", equalTo(vulnerability.getUuid().toString() + "/0")) + .withMatcher("vulnUuidSuffix1", equalTo(vulnerability.getUuid().toString() + "/1")) .isEqualTo(json(""" { "bomFormat": "CycloneDX", @@ -946,7 +935,7 @@ void exportProjectAsCycloneDxVdrTest() { ], "vulnerabilities": [ { - "bom-ref": "${json-unit.matches:vulnUuid}", + "bom-ref": "${json-unit.matches:vulnUuidSuffix0}", "id": "INT-001", "source": { "name": "INTERNAL" @@ -967,7 +956,7 @@ void exportProjectAsCycloneDxVdrTest() { ] }, { - "bom-ref": "${json-unit.matches:vulnUuid}", + "bom-ref": "${json-unit.matches:vulnUuidSuffix1}", "id": "INT-001", "source": { "name": "INTERNAL" @@ -996,6 +985,7 @@ void exportProjectAsCycloneDxVdrTest() { ] } """)); + CycloneDxBomAssertions.assertBomRefsUnique(jsonResponse); // Ensure the dependency graph did not get deleted during export. // https://github.com/DependencyTrack/dependency-track/issues/2494 diff --git a/apiserver/src/test/java/org/dependencytrack/resources/v1/VexResourceTest.java b/apiserver/src/test/java/org/dependencytrack/resources/v1/VexResourceTest.java index b036722d8d..081ecf2777 100644 --- a/apiserver/src/test/java/org/dependencytrack/resources/v1/VexResourceTest.java +++ b/apiserver/src/test/java/org/dependencytrack/resources/v1/VexResourceTest.java @@ -41,6 +41,7 @@ import org.dependencytrack.model.ProjectCollectionLogic; import org.dependencytrack.model.Severity; import org.dependencytrack.model.Vulnerability; +import org.dependencytrack.parser.cyclonedx.CycloneDxBomAssertions; import org.dependencytrack.parser.cyclonedx.CycloneDxValidator; import org.dependencytrack.persistence.command.MakeAnalysisCommand; import org.glassfish.jersey.inject.hk2.AbstractBinder; @@ -185,6 +186,8 @@ public void exportProjectAsCycloneDxTest() { .withMatcher("vulnAUuid", equalTo(vulnA.getUuid().toString())) .withMatcher("vulnBUuid", equalTo(vulnB.getUuid().toString())) .withMatcher("projectUuid", equalTo(project.getUuid().toString())) + .withMatcher("componentWithVulnUuid", equalTo(componentWithVuln.getUuid().toString())) + .withMatcher("componentWithVulnAndAnalysisUuid", equalTo(componentWithVulnAndAnalysis.getUuid().toString())) .isEqualTo(""" { "bomFormat": "CycloneDX", @@ -207,6 +210,20 @@ public void exportProjectAsCycloneDxTest() { } ] }, + "components": [ + { + "type": "library", + "bom-ref": "${json-unit.matches:componentWithVulnUuid}", + "name": "acme-lib-b", + "version": "1.0.0" + }, + { + "type": "library", + "bom-ref": "${json-unit.matches:componentWithVulnAndAnalysisUuid}", + "name": "acme-lib-c", + "version": "1.0.0" + } + ], "vulnerabilities": [ { "bom-ref": "${json-unit.matches:vulnAUuid}", @@ -225,7 +242,7 @@ public void exportProjectAsCycloneDxTest() { ], "affects": [ { - "ref": "${json-unit.matches:projectUuid}" + "ref": "${json-unit.matches:componentWithVulnUuid}" } ] }, @@ -252,13 +269,14 @@ public void exportProjectAsCycloneDxTest() { }, "affects": [ { - "ref": "${json-unit.matches:projectUuid}" + "ref": "${json-unit.matches:componentWithVulnAndAnalysisUuid}" } ] } ] } """); + CycloneDxBomAssertions.assertBomRefsUnique(jsonResponse); } @Test @@ -569,6 +587,8 @@ public void exportVexWithSameVulnAnalysisValidJsonTest() { .withOptions(Option.IGNORING_ARRAY_ORDER) .withMatcher("vulnUuid", equalTo(vuln.getUuid().toString())) .withMatcher("projectUuid", equalTo(project.getUuid().toString())) + .withMatcher("componentAUuid", equalTo(componentAWithVuln.getUuid().toString())) + .withMatcher("componentBUuid", equalTo(componentBWithVuln.getUuid().toString())) .isEqualTo(""" { "bomFormat": "CycloneDX", @@ -591,6 +611,20 @@ public void exportVexWithSameVulnAnalysisValidJsonTest() { } ] }, + "components": [ + { + "type": "library", + "bom-ref": "${json-unit.matches:componentAUuid}", + "name": "acme-lib-a", + "version": "1.0.0" + }, + { + "type": "library", + "bom-ref": "${json-unit.matches:componentBUuid}", + "name": "acme-lib-b", + "version": "1.0.0" + } + ], "vulnerabilities": [ { "bom-ref": "${json-unit.matches:vulnUuid}", @@ -615,13 +649,17 @@ public void exportVexWithSameVulnAnalysisValidJsonTest() { }, "affects": [ { - "ref": "${json-unit.matches:projectUuid}" + "ref": "${json-unit.matches:componentAUuid}" + }, + { + "ref": "${json-unit.matches:componentBUuid}" } ] } ] } """); + CycloneDxBomAssertions.assertBomRefsUnique(jsonResponse); } @Test @@ -676,8 +714,14 @@ public void exportVexWithDifferentVulnAnalysisValidJsonTest() { assertThatNoException().isThrownBy(() -> CycloneDxValidator.getInstance().validate(jsonResponse.getBytes())); assertThatJson(jsonResponse) .withOptions(Option.IGNORING_ARRAY_ORDER) - .withMatcher("vulnUuid", equalTo(vuln.getUuid().toString())) .withMatcher("projectUuid", equalTo(project.getUuid().toString())) + .withMatcher("componentAUuid", equalTo(componentAWithVuln.getUuid().toString())) + .withMatcher("componentBUuid", equalTo(componentBWithVuln.getUuid().toString())) + // One CVE split across two distinct analysis fingerprints produces two entries; their + // bom-refs are disambiguated with a deterministic suffix in fingerprint-sort order + // ("EXPLOITABLE" < "IN_TRIAGE"), so the exploitable group is /0 and in_triage is /1. + .withMatcher("vulnUuidSuffix0", equalTo(vuln.getUuid().toString() + "/0")) + .withMatcher("vulnUuidSuffix1", equalTo(vuln.getUuid().toString() + "/1")) .isEqualTo(""" { "bomFormat": "CycloneDX", @@ -700,9 +744,23 @@ public void exportVexWithDifferentVulnAnalysisValidJsonTest() { } ] }, + "components": [ + { + "type": "library", + "bom-ref": "${json-unit.matches:componentAUuid}", + "name": "acme-lib-a", + "version": "1.0.0" + }, + { + "type": "library", + "bom-ref": "${json-unit.matches:componentBUuid}", + "name": "acme-lib-b", + "version": "1.0.0" + } + ], "vulnerabilities": [ { - "bom-ref": "${json-unit.matches:vulnUuid}", + "bom-ref": "${json-unit.matches:vulnUuidSuffix1}", "id": "INT-001", "source": { "name": "INTERNAL" @@ -724,12 +782,12 @@ public void exportVexWithDifferentVulnAnalysisValidJsonTest() { }, "affects": [ { - "ref": "${json-unit.matches:projectUuid}" + "ref": "${json-unit.matches:componentAUuid}" } ] }, { - "bom-ref": "${json-unit.matches:vulnUuid}", + "bom-ref": "${json-unit.matches:vulnUuidSuffix0}", "id": "INT-001", "source": { "name": "INTERNAL" @@ -751,13 +809,14 @@ public void exportVexWithDifferentVulnAnalysisValidJsonTest() { }, "affects": [ { - "ref": "${json-unit.matches:projectUuid}" + "ref": "${json-unit.matches:componentBUuid}" } ] } ] } """); + CycloneDxBomAssertions.assertBomRefsUnique(jsonResponse); } @Test