PR Decorator Fix - Same CVEs should be aggregated in the same record#1067
PR Decorator Fix - Same CVEs should be aggregated in the same record#1067orto17 wants to merge 1 commit intojfrog:v3_erfrom
Conversation
orto17
commented
Feb 22, 2026
orto17
commented
- All tests passed. If this feature is not already covered by the tests, I added new tests.
- This pull request is on the dev branch.
- I used gofmt for formatting the code before submitting the pull request.
- Update documentation about new features / new supported technologies
orto17
added
the
safe to test
github-actions
bot
removed
the
safe to test
orto17
changed the title
| if existing, ok := byKey[key]; ok { | ||
| existing.ImpactPaths = append(existing.ImpactPaths, v.ImpactPaths...) | ||
| if len(v.FixedVersions) > 0 { | ||
| existing.FixedVersions = append(existing.FixedVersions, v.FixedVersions...) |
There was a problem hiding this comment.
can't we have duplicates here?
| return v.IssueId + "|" + strings.Join(ids, ",") | ||
| } | ||
|
|
||
| func aggregateVulnerabilitiesByCve(vulnerabilities []formats.VulnerabilityOrViolationRow) []formats.VulnerabilityOrViolationRow { |
There was a problem hiding this comment.
shouldnt we apply this on violations too?
| existing.FixedVersions = append(existing.FixedVersions, v.FixedVersions...) | ||
| } | ||
| } else { | ||
| agg := *v |
There was a problem hiding this comment.
The merge step only appends ImpactPaths and FixedVersions. All other fields (Severity, Applicable, ImpactedDependencyVersion, Components, etc.) are kept from the first occurrence and silently discarded from subsequent ones. This works fine for the summary table (which only renders paths), but getComponentIssueIdentifier in the details section uses ImpactedDependencyName/Version directly, so the details heading will only show the first row's version. we need to either document it or fix it if it will be used in other contexts
| ids = append(ids, cve.Id) | ||
| } | ||
| sort.Strings(ids) | ||
| return v.IssueId + "|" + strings.Join(ids, ",") |
There was a problem hiding this comment.
The key is IssueId + "|" + sorted CVE IDs. If Xray reports different IssueId values for the same CVE across different dependency paths (e.g. XRAY-12345 vs XRAY-12346 for the same CVE-2017-1000487), they won't be aggregated and the bug persists. Worth verifying that IssueId is always stable per CVE, or whether the key should be just the sorted CVE IDs.
| } | ||
| } | ||
|
|
||
| func TestVulnerabilitiesContent_aggregatesSameCveIntoOneRow(t *testing.T) { |
There was a problem hiding this comment.
please add unit test for aggregateVulnerabilitiesByCve
eranturgeman
left a comment
eranturgeman
left a comment
There was a problem hiding this comment.
approved! see my comments
2 participants
