Skip to content
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

fix: result merging for multi-plat images needs to take PackageName and PackageVersion into account #37

Merged
merged 3 commits into from
Mar 26, 2024

Conversation

jamestelfer
Copy link
Member

@jamestelfer jamestelfer commented Mar 26, 2024

Purpose

Multi-platform images require multiple results to be merged. This was initially done using the CVE as the key for a finding detail. However, looking at real results from ECR it's apparent that the key is actually a triple: Name (CVE), PackageName and PackageVersion.

This mismatch was causing results for multi-platform images that were inconsistent with an equivalent simple image reference.

Fixing this involved changing the comparer for the Details struct to look up by the three values, and ensuring that sorting in the report made sense.

Context

Fixes issue introduced in #30

Makes running locally more approachable.
A detail is not unique by vulnerability, as I had presumed. PackageName and PackageVersion are part of the uniqueness: a single CVE can appear multiple times in a report against multiple packages.
Also, CVEs need to sort descending when repeated.
Copy link

Package Line Rate Health
github.com/cultureamp/ecrscanresults/finding 95%
github.com/cultureamp/ecrscanresults/findingconfig 70%
github.com/cultureamp/ecrscanresults/registry 11%
github.com/cultureamp/ecrscanresults/report 86%
github.com/cultureamp/ecrscanresults/runtimeerrors 83%
Summary 66% (444 / 674)

@jamestelfer jamestelfer changed the title Fix-merge fix: result merging for multi-plat images needs to take PackageName and PackageVersion into account Mar 26, 2024
@jamestelfer jamestelfer marked this pull request as ready for review March 26, 2024 12:51
@jamestelfer jamestelfer requested a review from n-tucker March 26, 2024 12:51
@jamestelfer jamestelfer merged commit a48da75 into main Mar 26, 2024
10 checks passed
@jamestelfer jamestelfer deleted the fix-merge branch March 26, 2024 22:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants