-
-
Notifications
You must be signed in to change notification settings - Fork 39
Support for CSAF-based Vulnerability Analyser #1894
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Christian Banse <oxisto@aybaze.com>
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Adds a CSAF-based vulnerability analyzer, including a new scanner, config, persistence for advisories, and protobuf updates to carry CSAF scan results and confidence scores.
- Introduces CsafScannerProcessor and supplier, wired into the scanning pipeline
- Adds Advisory entity/table and repositories for CSAF document storage and access
- Extends protobuf API with SCANNER_CSAF and per-vulnerability matching confidence; adds dependencies for CSAF matching
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| vulnerability-analyzer/src/main/resources/application.properties | Enables CSAF scanner, configures partitions/threshold, disables OSS Index, and disables config caching |
| vulnerability-analyzer/src/main/java/org/dependencytrack/vulnanalyzer/processor/scanner/csaf/CsafScannerProcessorSupplier.java | Supplies the new CSAF scanner processor and identifies scanner |
| vulnerability-analyzer/src/main/java/org/dependencytrack/vulnanalyzer/processor/scanner/csaf/CsafScannerProcessor.java | Implements CSAF matching against stored advisories and emits CycloneDX BOM vulnerabilities with confidence |
| vulnerability-analyzer/src/main/java/org/dependencytrack/vulnanalyzer/config/CsafScannerConfig.java | Provides runtime-dynamic config for CSAF scanner enablement and threshold |
| vulnerability-analyzer/pom.xml | Adds CSAF matching and pbandk runtime dependencies |
| proto/src/main/proto/org/dependencytrack/vulnanalysis/v1/vuln_analysis.proto | Adds SCANNER_CSAF and matching_confidence map to ScannerResult |
| proto/src/main/proto/org/dependencytrack/mirror/v1/csaf_data.proto | Defines mirror API message for CSAF documents |
| pom.xml | Manages csaf-matching-jvm version in dependencyManagement |
| docker-compose.yml | Adds pgAdmin service for local Postgres administration (dev) |
| commons-persistence/src/main/resources/schema.sql | Creates ADVISORY table for persisted CSAF documents |
| commons-persistence/src/main/java/org/dependencytrack/persistence/repository/ComponentRepository.java | Panache repository for Component |
| commons-persistence/src/main/java/org/dependencytrack/persistence/repository/AdvisoryRepository.java | Panache repository for Advisory |
| commons-persistence/src/main/java/org/dependencytrack/persistence/model/Vulnerability.java | Adds CSAF as a vulnerability Source |
| commons-persistence/src/main/java/org/dependencytrack/persistence/model/Advisory.java | JPA entity mapping for the ADVISORY table |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| @ConfigProperty(name = "dtrack.scanner.csaf.enabled") final Provider<Optional<Boolean>> enabledProvider, | ||
| @ConfigProperty(name = "dtrack.scanner.csaf.threshold") final Provider<Optional<Integer>> thresholdProvider |
Copilot
AI
Oct 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Configuration key mismatch: the application.properties uses scanner.csaf.enabled and scanner.csaf.threshold, but this class reads dtrack.scanner.csaf.enabled and dtrack.scanner.csaf.threshold, causing the scanner to appear disabled. Align the keys, e.g. change to scanner.csaf.enabled and scanner.csaf.threshold to match the properties.
| @ConfigProperty(name = "dtrack.scanner.csaf.enabled") final Provider<Optional<Boolean>> enabledProvider, | |
| @ConfigProperty(name = "dtrack.scanner.csaf.threshold") final Provider<Optional<Integer>> thresholdProvider | |
| @ConfigProperty(name = "scanner.csaf.enabled") final Provider<Optional<Boolean>> enabledProvider, | |
| @ConfigProperty(name = "scanner.csaf.threshold") final Provider<Optional<Integer>> thresholdProvider |
| scanner.csaf.enabled=true | ||
| scanner.csaf.topic-partitions=2 | ||
| scanner.csaf.threshold=80 |
Copilot
AI
Oct 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These keys are not read by CsafScannerConfig, which expects dtrack.scanner.csaf.enabled/threshold. Either rewrite these to dtrack.scanner.csaf.enabled and dtrack.scanner.csaf.threshold or update CsafScannerConfig to use scanner.csaf.* so the feature can be enabled as intended.
| scanner.csaf.enabled=true | |
| scanner.csaf.topic-partitions=2 | |
| scanner.csaf.threshold=80 | |
| dtrack.scanner.csaf.enabled=true | |
| scanner.csaf.topic-partitions=2 | |
| dtrack.scanner.csaf.threshold=80 |
| if (!config.enabled().orElse(false)) { | ||
| return; | ||
| } |
Copilot
AI
Oct 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dropping the record when the scanner is disabled causes the pipeline to lose messages (no ScannerResult is forwarded). Forward an explicit result (e.g., with an empty BOM and a 'skipped/disabled' status) to ensure downstream processors/consumers receive a consistent outcome.
| } | ||
|
|
||
| // Otherwise, we will use the index of the vulnerability | ||
| return prefix + "-VULNERABILITY" + vulnIndex; |
Copilot
AI
Oct 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing hyphen before the index yields IDs like '...-VULNERABILITY12'. Use '-VULNERABILITY-' to ensure consistent formatting: return prefix + "-VULNERABILITY-" + vulnIndex;
| return prefix + "-VULNERABILITY" + vulnIndex; | |
| return prefix + "-VULNERABILITY-" + vulnIndex; |
| return "CSAF-" + Hex.encodeHexString( | ||
| digest.digest( | ||
| doc.getPublisher().getNamespace().toString().getBytes() | ||
| )).substring(0, 8) + "-" + doc.getTracking().getId(); |
Copilot
AI
Oct 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use a deterministic charset when converting strings to bytes to avoid platform-dependent behavior. Replace getBytes() with getBytes(StandardCharsets.UTF_8) and import java.nio.charset.StandardCharsets.
| @Transactional | ||
| private ScannerResult analyzeComponent(ScanTask value) { | ||
| var component = value.getComponent(); | ||
|
|
||
| // Retrieve some more information about the component from the database | ||
| var componentEntity = QuarkusTransaction.joiningExisting().call(() -> componentRepository.find("uuid = :uuid", Map.of("uuid", UUID.fromString(component.getUuid()))) | ||
| .firstResultOptional() | ||
| .orElseGet(() ->{ | ||
| // If the component is not found for some reason, we can at least fill CPE and PURL | ||
| var entity = new org.dependencytrack.persistence.model.Component(); | ||
| entity.setPurl(component.getPurl()); | ||
| entity.setCpe(component.getCpe()); | ||
| return entity; | ||
| })); | ||
|
|
||
| return internalAnalyzeComponent(component, createProtobomNode(componentEntity)); | ||
| } |
Copilot
AI
Oct 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mixing @transactional with QuarkusTransaction.joiningExisting() is redundant; inside a transactional method you can query via componentRepository directly. Simplify by removing the QuarkusTransaction wrapper here (or drop @transactional and manage the transaction explicitly, but avoid both).
|
|
||
| return new Node( | ||
| Long.toString(component.getId()), | ||
| Node.NodeType.Companion.fromValue(0), |
Copilot
AI
Oct 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid magic numbers for enum values. Use a named constant (e.g., Node.NodeType.) or a helper that conveys the intended default type instead of fromValue(0).
| Node.NodeType.Companion.fromValue(0), | |
| Node.NodeType.UNSPECIFIED, |
| if (!config.enabled().orElse(false)) { | ||
| parsedDocs = List.of(); | ||
| } else { | ||
| LOGGER.info("Creating csaf scanner processor once?"); |
Copilot
AI
Oct 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The log message reads like a leftover debug note. Consider rephrasing to a clear, actionable message (or drop to DEBUG) without the question mark.
| LOGGER.info("Creating csaf scanner processor once?"); | |
| LOGGER.info("Initializing CSAF scanner processor."); |
| quarkus.container-image.registry=ghcr.io | ||
| quarkus.container-image.group=dependencytrack | ||
|
|
||
| # Caching of config properties is contra-productive since we want an immediate |
Copilot
AI
Oct 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Corrected spelling of 'contra-productive' to 'counterproductive'.
| # Caching of config properties is contra-productive since we want an immediate | |
| # Caching of config properties is counterproductive since we want an immediate |
| * This function tries to compute a unique ID of a {@link Csaf.Document} so we can set | ||
| * it as a primary key for an {@link Advisory}. | ||
| * We use the prefix "CSAF" plus a truncated hash of the publisher namespace and the tracking ID. | ||
| * |
Copilot
AI
Oct 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Javadoc says the hash includes both the publisher namespace and the tracking ID, but the implementation hashes only the namespace and appends the tracking ID as plain text. Update the comment to match the implementation or adjust the code to include both in the hash.
Signed-off-by: Christian Banse <christian.banse@aisec.fraunhofer.de>
Signed-off-by: Christian Banse <oxisto@aybaze.com>
|
@oxisto Is there any progress on this? :-) |
I am currently on vacation, I will progress this in January |
Description
This adds a new vulnerability analyser based on the information found in a CSAF document.
Addressed Issue
Addresses DependencyTrack/dependency-track#2353
Additional Details
Checklist
- [ ] This PR fixes a defect, and I have provided tests to verify that the fix is effective- [ ] This PR introduces new or alters existing behavior, and I have updated the documentation accordingly