Add rating source tracking for context-aware OWASP scores#1928
Add rating source tracking for context-aware OWASP scores#1928fahedouch wants to merge 2 commits intoDependencyTrack:mainfrom
Conversation
faf1f2a to
1b90695
Compare
There was a problem hiding this comment.
Pull request overview
This PR introduces tracking of the source of component-level OWASP Risk Rating overrides so that context-aware scores (e.g., from VEX) can be applied with a defined precedence model, aiming to prevent lower-precedence sources from overwriting higher-precedence ratings.
Changes:
- Adds a new
OWASPSOURCEcolumn toANALYSISplus an index, and wires it into theAnalysismodel. - Introduces a
RatingSourceenum with precedence + overwrite logic, and adds unit tests for it. - Extends analysis update flows to carry OWASP vector/score/source via
MakeAnalysisCommand, and imports OWASP ratings from CycloneDX VEX asRatingSource.VEX.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| migration/src/main/resources/migration/changelog-v5.8.0.xml | Adds OWASPSOURCE column and index in Liquibase. |
| migration/src/main/resources/migration/changelog-main.xml | Includes the v5.8.0 changelog in the master migration chain. |
| apiserver/src/test/java/org/dependencytrack/model/RatingSourceTest.java | Adds tests validating precedence and overwrite behavior. |
| apiserver/src/main/java/org/dependencytrack/persistence/command/MakeAnalysisCommand.java | Adds OWASP vector/score/source fields and a convenience withOwasp(...) method. |
| apiserver/src/main/java/org/dependencytrack/persistence/AnalysisQueryManager.java | Applies precedence when updating OWASP vector/score and persists owaspSource. |
| apiserver/src/main/java/org/dependencytrack/parser/cyclonedx/CycloneDXVexImporter.java | Imports OWASP ratings from CycloneDX VEX and sets source to VEX, with basic validation. |
| apiserver/src/main/java/org/dependencytrack/model/RatingSource.java | Defines precedence order and overwrite logic. |
| apiserver/src/main/java/org/dependencytrack/model/Analysis.java | Adds the persisted owaspSource field mapped to OWASPSOURCE. |
| RATING_SOURCE_TRACKING.md | Documents the precedence model and examples. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| import javax.jdo.PersistenceManager; | ||
| import javax.jdo.Query; | ||
| import java.math.BigDecimal; |
There was a problem hiding this comment.
java.math.BigDecimal is imported but never used in this class. This will fail compilation due to an unused import; please remove it (or use it if intended).
| import java.math.BigDecimal; |
| * <li>MANUAL - User-provided ratings</li> | ||
| * <li>VEX - Ratings from VEX documents</li> | ||
| * <li>POLICY - Ratings from organizational policies</li> |
There was a problem hiding this comment.
The precedence order documented here (MANUAL > VEX > POLICY > NVD) contradicts the precedence defined in RatingSource (POLICY > VEX > MANUAL > NVD) and the PR description. Please correct the Javadoc to reflect the actual precedence to avoid future logic being implemented based on the wrong ordering.
| * <li>MANUAL - User-provided ratings</li> | |
| * <li>VEX - Ratings from VEX documents</li> | |
| * <li>POLICY - Ratings from organizational policies</li> | |
| * <li>POLICY - Ratings from organizational policies</li> | |
| * <li>VEX - Ratings from VEX documents</li> | |
| * <li>MANUAL - User-provided ratings</li> |
| command = command.withOwasp( | ||
| rating.getVector(), | ||
| rating.getScore(), | ||
| RatingSource.VEX); | ||
| break; // Only use the first OWASP rating |
There was a problem hiding this comment.
For OWASP ratings, this logic allows a rating with severity != null but vector == null (and potentially score == null) to be treated as valid, then it calls withOwasp(null, null, VEX) and immediately breaks. That means a later OWASP rating with a usable vector+score in the same VEX document would be ignored, and no OWASP data will be applied. Consider requiring vector+score for OWASP imports (since those are the only fields persisted), or only break once at least one of vector/score is actually applied.
| command = command.withOwasp( | |
| rating.getVector(), | |
| rating.getScore(), | |
| RatingSource.VEX); | |
| break; // Only use the first OWASP rating | |
| if (rating.getVector() == null || rating.getScore() == null) { | |
| LOGGER.warn("VEX OWASP rating is missing vector and/or score - skipping"); | |
| continue; | |
| } | |
| command = command.withOwasp( | |
| rating.getVector(), | |
| rating.getScore(), | |
| RatingSource.VEX); | |
| break; // Only use the first OWASP rating with usable vector and score |
| @Persistent(defaultFetchGroup = "true") | ||
| @Column(name = "OWASPSOURCE", jdbcType = "VARCHAR") | ||
| @JsonProperty(value = "owaspSource") | ||
| private RatingSource owaspSource; |
There was a problem hiding this comment.
Adding owaspSource to the JDO model is not sufficient to enforce precedence if OWASP scores/vectors are also written via the JDBI-based policy/analysis upserts: those SQL paths currently don't read/write OWASPSOURCE, so the column will remain NULL or stale while OWASPVECTOR/OWASPSCORE change. This breaks the intended precedence rules (e.g., policy updates won't mark the source as POLICY, and VEX may overwrite rows with NULL source). Please update the relevant JDBI mappers/queries/upserts to include OWASPSOURCE wherever OWASP ratings are set.
| <addColumn tableName="ANALYSIS"> | ||
| <column name="OWASPSOURCE" type="VARCHAR(50)"> | ||
| <constraints nullable="true"/> | ||
| </column> | ||
| </addColumn> |
There was a problem hiding this comment.
The new OWASPSOURCE column is nullable and there's no data migration/backfill. As a result, any existing analyses that already have OWASPVECTOR/OWASPSCORE populated will have a NULL source and will be treated as overwritable by any new source (since canOverwrite(null) returns true). If the goal is to prevent VEX imports from overwriting existing policy/manual ratings, consider backfilling OWASPSOURCE for existing rows (or introducing an explicit default/UNKNOWN source with appropriate precedence).
|
|
||
| import org.junit.jupiter.api.Test; | ||
|
|
||
| import static org.junit.jupiter.api.Assertions.*; |
There was a problem hiding this comment.
Static wildcard import is disallowed by this repo's Checkstyle rules (AvoidStarImport in support/checkstyle/config.xml). Please replace import static org.junit.jupiter.api.Assertions.*; with explicit static imports for the assertions used in this test.
| import static org.junit.jupiter.api.Assertions.*; | |
| import static org.junit.jupiter.api.Assertions.assertFalse; | |
| import static org.junit.jupiter.api.Assertions.assertTrue; |
apiserver/src/main/java/org/dependencytrack/model/Analysis.java
Outdated
Show resolved
Hide resolved
aabffe2 to
e62e0c7
Compare
|
Frontend PR
|
Signed-off-by: Fahed Dorgaa <fahed.dorgaa@gmail.com>
e5b23d3 to
e791dc0
Compare
Signed-off-by: Fahed Dorgaa <fahed.dorgaa@gmail.com>
e791dc0 to
ad2c4a5
Compare
Not up to standards ⛔🟢 Issues
|
| Category | Results |
|---|---|
| CodeStyle | 10 minor |
🟢 Metrics 38 complexity
Metric Results Complexity 38
🔴 Coverage 57.26% diff coverage · -0.26% coverage variation
Metric Results Coverage variation ✅ -0.26% coverage variation (-1.00%) Diff coverage ❌ 57.26% diff coverage (70.00%) Coverage variation details
Coverable lines Covered lines Coverage Common ancestor commit (7b650af) 26069 22357 85.76% Head commit (ad2c4a5) 27649 (+1580) 23640 (+1283) 85.50% (-0.26%) Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch:
<coverage of head commit> - <coverage of common ancestor commit>Diff coverage details
Coverable lines Covered lines Diff coverage Pull request (#1928) 124 71 57.26% Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified:
<covered lines added or modified>/<coverable lines added or modified> * 100%
TIP This summary will be updated as you push new changes. Give us feedback
| @Persistent(defaultFetchGroup = "true") | ||
| @Column(name = "OWASPSEVERITY") | ||
| @Extensions(value = { | ||
| @Extension(vendorName = "datanucleus", key = "insert-function", value = "CAST(? AS severity)"), | ||
| @Extension(vendorName = "datanucleus", key = "update-function", value = "CAST(? AS severity)") | ||
| }) | ||
| @JsonProperty(value = "owaspSeverity") | ||
| private Severity owaspSeverity; |
There was a problem hiding this comment.
What is the use case for tracking the severity for OWASP ratings separately? Adding more severity fields adds complexity to all queries that operate on severities, including metrics calculation.
Analogue to a previous comment, if we do this for OWASP ratings, why not also for CVSS?

Description
fix DependencyTrack/dependency-track#5796
This PR enables context-aware OWASP Risk Rating scores at the component level, so the same CVE can have different scores depending on the deployment context (fixes #5796).
What changed
I added a precedence system to track where OWASP ratings come from:
Higher precedence sources can overwrite lower ones, but not vice versa. This prevents VEX imports from accidentally overwriting your policy ratings or analyst assessments.
I applied Considerations about when to provide severity, vector and/or score fields which are recently proposed in openvex rating field integration qui me semble pertinente :
Use case
Import contextual OWASP scores from VEX documents (e.g., generated by VENS) that reflect your actual deployment - internet-facing vs internal, production vs dev, sensitive data handling, etc.
Addressed Issue
Additional Details
Checklist