Consider OS distro during vulnerability matching#5782
Consider OS distro during vulnerability matching#5782nscuro wants to merge 1 commit intoDependencyTrack:masterfrom
Conversation
* Where possible, enriches an affected package's PURL with `distro` qualifier inferred from the package's `ecosystem`. e.g. `ecosystem=Debian:7` becomes `distro=debian-11`, `ecosystem=Ubuntu:20.04:LTS` becomes `distro=ubuntu-20.04` etc. * During vulnerability analysis, if both component and matching criteria have a PURL `distro` qualifier, ensures they match. Matching can handle codename <-> version comparisons, e.g. for Ubuntu `focal` would match `20.04` and vice versa. * Generally improves performance of OSV mirroring by using fewer transactions and disabling ORM features that caused expensive unnecessary queries. Currently Alpine, Debian, and Ubuntu distribution matching is implemented. These seem to work for SBOMs generated with Trivy and Syft. The codename <-> version mapping is currently hardcoded for Debian and Ubuntu. There is a fallback mechanism that will handle exact matches, such that when Debian publishes a hypothetical "foo" release, we can still match components with vulnerabilities if both `distro` qualifiers are exactly "foo". Debian and Ubuntu provide CSV which we could regularly fetch at runtime, but this involves more work to coordinate. Fixes DependencyTrack#1374 Fixes DependencyTrack#5776 Fixes DependencyTrack#4445 Fixes DependencyTrack#4725 Signed-off-by: nscuro <nscuro@protonmail.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.
Pull request overview
This pull request implements OS distribution-aware vulnerability matching to address false positives in vulnerability analysis for Debian, Ubuntu, and Alpine packages. The changes enable Dependency-Track to correctly match vulnerabilities by considering the specific OS distribution version when analyzing components.
Changes:
- Introduces new
OsDistributionmodel with implementations for Alpine, Debian, and Ubuntu distributions, including codename-to-version mapping - Enhances OSV mirroring to enrich PURLs with distro qualifiers inferred from package ecosystems (e.g.,
Debian:11→distro=debian-11) - Modifies vulnerability analysis to enforce distro matching when both component and vulnerability have distro qualifiers
- Updates persistence layer to include PURL qualifiers in VulnerableSoftware lookups
- Improves OSV mirroring performance through transaction optimizations and disabled ORM features
- Changes default OSV enabled ecosystems from null to "Alpine;Debian;Ubuntu"
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/org/dependencytrack/model/OsDistribution.java | New sealed interface defining OS distribution matching logic for Alpine, Debian, and Ubuntu with codename/version mapping |
| src/test/java/org/dependencytrack/model/OsDistributionTest.java | Comprehensive unit tests for OsDistribution parsing and matching logic |
| src/main/java/org/dependencytrack/util/PurlUtil.java | Adds utility methods to extract distro qualifiers from PURLs |
| src/main/java/org/dependencytrack/tasks/scanners/AbstractVulnerableSoftwareAnalysisTask.java | Implements distro matching check before version comparison in vulnerability analysis |
| src/main/java/org/dependencytrack/tasks/OsvDownloadTask.java | Enriches OSV PURLs with distro qualifiers and refactors transaction handling for performance |
| src/main/java/org/dependencytrack/persistence/VulnerableSoftwareQueryManager.java | Adds purlQualifiers parameter to vulnerability software lookups |
| src/main/java/org/dependencytrack/persistence/QueryManager.java | Delegates to VulnerableSoftwareQueryManager with new purlQualifiers parameter |
| src/main/java/org/dependencytrack/model/ConfigPropertyConstants.java | Changes default OSV enabled ecosystems from null to "Alpine;Debian;Ubuntu" |
| dev/docker-compose.yml | Development-only changes to image tag and Java memory settings |
| dev/docker-compose.postgres.yml | Adds PostgreSQL monitoring with pg_stat_statements and pghero service |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| private static boolean comparePurlVersions(PackageURL componentPurl, VulnerableSoftware vs, Version targetVersion) { | ||
| final String componentDistroQualifier = PurlUtil.getDistroQualifier(componentPurl); | ||
| final String vsDistroQualifier = PurlUtil.getDistroQualifier(vs.getPurl()); | ||
|
|
||
| // When both the component and the vulnerable software record have a distro | ||
| // qualifier, they must match *before* we perform the actual version comparison. | ||
| if (componentDistroQualifier != null && vsDistroQualifier != null) { | ||
| // Simplest case: the qualifiers just match without special interpretation. | ||
| if (!componentDistroQualifier.equals(vsDistroQualifier)) { | ||
| // Could still match, but depends on distro semantics. | ||
| // e.g. "debian-13" should match "trixie". | ||
| final var componentDistro = OsDistribution.of(componentPurl); | ||
| final var vsDistro = OsDistribution.of(PurlUtil.silentPurl(vs.getPurl())); | ||
|
|
||
| if (componentDistro == null || vsDistro == null) { | ||
| // At least one of the distros could not be identified. | ||
| // Have to assume they don't match. | ||
| return false; | ||
| } | ||
|
|
||
| if (!componentDistro.matches(vsDistro)) { | ||
| // Actual mismatch, e.g. "debian-13" != "sid". | ||
| return false; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The new distro matching logic in comparePurlVersions is not covered by tests in the InternalAnalysisTaskPurlMatchingTest or similar test files. This is critical functionality that determines whether components match vulnerabilities. Consider adding comprehensive tests that verify distro matching works correctly for various scenarios including: matching distros, mismatched distros, one side with distro and other without, and edge cases with unknown distros.
| // When both the component and the vulnerable software record have a distro | ||
| // qualifier, they must match *before* we perform the actual version comparison. | ||
| if (componentDistroQualifier != null && vsDistroQualifier != null) { | ||
| // Simplest case: the qualifiers just match without special interpretation. | ||
| if (!componentDistroQualifier.equals(vsDistroQualifier)) { | ||
| // Could still match, but depends on distro semantics. | ||
| // e.g. "debian-13" should match "trixie". | ||
| final var componentDistro = OsDistribution.of(componentPurl); | ||
| final var vsDistro = OsDistribution.of(PurlUtil.silentPurl(vs.getPurl())); | ||
|
|
||
| if (componentDistro == null || vsDistro == null) { | ||
| // At least one of the distros could not be identified. | ||
| // Have to assume they don't match. | ||
| return false; | ||
| } | ||
|
|
||
| if (!componentDistro.matches(vsDistro)) { | ||
| // Actual mismatch, e.g. "debian-13" != "sid". | ||
| return false; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The distro matching logic only enforces matching when BOTH the component and vulnerability have distro qualifiers. When only one side has a distro qualifier, matching proceeds without distro checks. While this may be intentional to avoid over-filtering, consider adding a comment explaining this design decision and the tradeoffs. For example, a Debian 11 component might match a vulnerability that doesn't specify a distro but is only applicable to Debian 12.
| final Vers vulnerableVersionRange = vs.getVers(); | ||
|
|
||
| if (vulnerableVersionRange == null) { |
There was a problem hiding this comment.
The method calls vs.getVers() twice. The variable vulnerableVersionRange is assigned on line 190 but then line 196 calls vs.getVers() again instead of using the local variable. This is a minor redundancy - consider using vulnerableVersionRange.contains(targetVersion.toString()) instead.
| services: | ||
| apiserver: | ||
| image: dependencytrack/apiserver:snapshot-alpine | ||
| image: dependencytrack/apiserver:local-alpine |
There was a problem hiding this comment.
The code changes the image tag from snapshot-alpine to local-alpine which appears to be developer-specific. This change should likely be reverted or the file should not be included in this PR, as it's intended for local development environment configuration and this change might not work for other developers.
| image: dependencytrack/apiserver:local-alpine | |
| image: dependencytrack/apiserver:snapshot-alpine |
| VULNERABILITY_SOURCE_GITHUB_ADVISORIES_LAST_MODIFIED_EPOCH_SECONDS("vuln-source", "github.advisories.last.modified.epoch.seconds", null, PropertyType.INTEGER, "Epoch timestamp in seconds of the latest observed GHSA modification time"), | ||
| VULNERABILITY_SOURCE_GOOGLE_OSV_BASE_URL("vuln-source", "google.osv.base.url", "https://osv-vulnerabilities.storage.googleapis.com/", PropertyType.URL, "A base URL pointing to the hostname and path for OSV mirroring"), | ||
| VULNERABILITY_SOURCE_GOOGLE_OSV_ENABLED("vuln-source", "google.osv.enabled", null, PropertyType.STRING, "List of enabled ecosystems to mirror OSV"), | ||
| VULNERABILITY_SOURCE_GOOGLE_OSV_ENABLED("vuln-source", "google.osv.enabled", "Alpine;Debian;Ubuntu", PropertyType.STRING, "List of enabled ecosystems to mirror OSV"), |
There was a problem hiding this comment.
The default value for OSV enabled ecosystems changed from null to "Alpine;Debian;Ubuntu". This is a breaking change that will automatically enable OSV mirroring for these ecosystems for existing installations. This could cause unexpected network traffic, disk usage, and processing load for users upgrading to this version. Consider documenting this in the PR description and release notes, or providing a migration strategy.
|
|
||
| @Override | ||
| public String purlQualifierValue() { | ||
| return "ubuntu-" + (version != null ? version : series); |
There was a problem hiding this comment.
The null check for version in the ternary expression is redundant because the compact constructor on line 315 requires version to be non-null. This check can be simplified to just return "ubuntu-" + version. The same issue exists in DebianDistribution.purlQualifierValue() where version CAN be null, so the pattern is inconsistent between the two distributions.
| return "ubuntu-" + (version != null ? version : series); | |
| return "ubuntu-" + version; |
| vs.setPurlQualifiers((purl.getQualifiers() != null && !purl.getQualifiers().isEmpty()) | ||
| ? Json.createObjectBuilder(purl.getQualifiers()).build().toString() | ||
| : null); |
There was a problem hiding this comment.
The purlQualifiers are being serialized to JSON using Json.createObjectBuilder(purl.getQualifiers()).build().toString(). However, JSON object key ordering is not guaranteed to be consistent, which could cause VulnerableSoftware records with identical qualifiers but different key ordering to be treated as different records. This could lead to duplicate entries or lookup failures. Consider using a TreeMap to ensure consistent key ordering before serialization, or use a canonical JSON representation.
| image: dependencytrack/apiserver:snapshot-alpine | ||
| image: dependencytrack/apiserver:local-alpine | ||
| environment: | ||
| EXTRA_JAVA_OPTIONS: "-Xmx2g" |
There was a problem hiding this comment.
The EXTRA_JAVA_OPTIONS environment variable for memory configuration appears to be developer-specific. Consider whether this should be included in this PR or if the entire docker-compose.yml changes should be excluded from version control or documented separately as optional development configuration.
| EXTRA_JAVA_OPTIONS: "-Xmx2g" | |
| # Optional: Configure JVM options (e.g., heap size) via EXTRA_JAVA_OPTIONS in your local environment. |
| purl = PackageURLBuilder.aPackageURL() | ||
| .withType(purl.getType()) | ||
| .withNamespace(purl.getNamespace()) | ||
| .withName(purl.getName()) | ||
| .withVersion(purl.getVersion()) | ||
| .withQualifier("distro", distro.purlQualifierValue()) | ||
| .withSubpath(purl.getSubpath()) | ||
| .build(); |
There was a problem hiding this comment.
When adding the distro qualifier to a PURL, existing qualifiers from the original PURL (other than distro) are not preserved. The PackageURLBuilder only adds the distro qualifier but doesn't copy other existing qualifiers like arch, epoch, etc. This could cause data loss if the original PURL had other qualifiers. Consider iterating through existing qualifiers and preserving them when building the new PURL.
| purl = PackageURLBuilder.aPackageURL() | |
| .withType(purl.getType()) | |
| .withNamespace(purl.getNamespace()) | |
| .withName(purl.getName()) | |
| .withVersion(purl.getVersion()) | |
| .withQualifier("distro", distro.purlQualifierValue()) | |
| .withSubpath(purl.getSubpath()) | |
| .build(); | |
| final PackageURLBuilder builder = PackageURLBuilder.aPackageURL() | |
| .withType(purl.getType()) | |
| .withNamespace(purl.getNamespace()) | |
| .withName(purl.getName()) | |
| .withVersion(purl.getVersion()) | |
| .withSubpath(purl.getSubpath()); | |
| if (purl.getQualifiers() != null && !purl.getQualifiers().isEmpty()) { | |
| purl.getQualifiers().forEach((key, value) -> { | |
| if (!"distro".equals(key)) { | |
| builder.withQualifier(key, value); | |
| } | |
| }); | |
| } | |
| builder.withQualifier("distro", distro.purlQualifierValue()); | |
| purl = builder.build(); |
| @@ -43,5 +48,16 @@ services: | |||
| - "postgres-data:/var/lib/postgresql/data" | |||
| restart: unless-stopped | |||
|
|
|||
| pghero: | |||
| image: ankane/pghero | |||
| depends_on: | |||
| postgres: | |||
| condition: service_healthy | |||
| environment: | |||
| DATABASE_URL: "postgres://dtrack:dtrack@postgres:5432/dtrack" | |||
| ports: | |||
| - "127.0.0.1:8432:8080" | |||
| restart: unless-stopped | |||
There was a problem hiding this comment.
The addition of pghero service and PostgreSQL configuration for pg_stat_statements monitoring is a development-only change that doesn't relate to the PR's stated purpose of OS distro vulnerability matching. Consider excluding these development environment changes from this PR or documenting them separately if they're needed for testing/debugging the new functionality.
Description
Considers OS distro during vulnerability matching.
distroqualifier inferred from the package'secosystem. e.g.ecosystem=Debian:7becomesdistro=debian-11,ecosystem=Ubuntu:20.04:LTSbecomesdistro=ubuntu-20.04etc.distroqualifier, ensures they match. Matching can handle codename <-> version comparisons, e.g. for Ubuntufocalwould match20.04and vice versa.Currently Alpine, Debian, and Ubuntu distribution matching is implemented. These seem to work for SBOMs generated with Trivy and Syft.
The codename <-> version mapping is currently hardcoded for Debian and Ubuntu. There is a fallback mechanism that will handle exact matches, such that when Debian publishes a hypothetical "foo" release, we can still match components with vulnerabilities if both
distroqualifiers are exactly "foo".Debian and Ubuntu provide CSV which we could regularly fetch at runtime, but this involves more work to coordinate.
Addressed Issue
Fixes #1374
Fixes #5776
Fixes #4445
Fixes #4725
Additional Details
Checklist
This PR implements an enhancement, and I have provided tests to verify that it works as intendedThis PR introduces changes to the database model, and I have added corresponding update logicThis PR introduces new or alters existing behavior, and I have updated the documentation accordingly