-
Notifications
You must be signed in to change notification settings - Fork 551
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
Optionally orient results by CVE #1020
Conversation
97dcf9b
to
27bd193
Compare
Signed-off-by: Alex Goodman <alex.goodman@anchore.com>
27bd193
to
c428d99
Compare
Signed-off-by: Alex Goodman <alex.goodman@anchore.com>
3699395
to
6ca32ae
Compare
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.
LGTM -- only real question is naming of --by-cve
and subsequently referring to this as normalize
e.g. the NormalizeByCVE
flag.
@@ -167,6 +169,11 @@ func setRootFlags(flags *pflag.FlagSet) { | |||
"ignore matches for vulnerabilities that are fixed", | |||
) | |||
|
|||
flags.BoolP( | |||
"by-cve", "", false, |
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.
nit: by-cve
to me, at least, is not the most clear... maybe something like --cve-first
, --orient-by-cve
, or --group-by-cve
?
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.
Yea I can see how a user could conflate it. I also agree none of the current options standout as a clear winner. I like that --by-cve does not show GHSA. That feels right. The inverse --by-ghsa would not show CVE.
I also ran it locally and it feels right for now, but might be biased since it was the first one I read.
I say ship, and if er end up hating it we can change up the ergonomics if it still feels bad.
Keith also had a good point of eventually making this the default.
Waiting for checks then will merge |
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
* main: chore: add GitLab Community Edition image to quality gate (#1035) Update Syft to v0.63.0 (#1037) fix: Exclude binary packages that have overlap by file ownership relationship (#1024) docs: update quality gate docs (#1032) Optionally orient results by CVE (#1020) Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
@wagoodman thanks for this feature, I tried to run this on an elasticsearch image and don't see CVE IDs where CVE IDs look available. See output below and example:
|
@anandgia is there a specific image to use to reproduce this? |
Yes @kzantow i put the image in the above description docker.elastic.co/elasticsearch/elasticsearch:6.8.23 |
I confirm the observation by @anandgia (and additionally notice
Is there anything I can do to help debug this? Perhaps opening a dedicated issue for issues around |
@JipSogeti I created a new issue for the performance concern: #1185 |
This PR introduces a new
--by-cve
option which will orient all results by CVE. For instance, if there is a GHSA that was directly matched on, then the results will instead show the equivalent CVE in the results in its place. If that CVE result was already being shown in the results, then the match details from the GHSA result will be appended to the existing result -- in this way there is always a record of how the CVE match was made (by GHSA).Additionally the JSON results are also re-oriented around CVEs. For example, before this PR a GHSA result would be shown similar to such:
After this change:
The specific changes made:
--by-cve
optionvulnerabilityID
attribute under thematch.Detail.Found
map, since the vulnerability record shown may no longer match what was actually matched on during the search.grype.VulnerabiliyMatcher
object. This encapsulates much of the logic incmd
into something that is accessible in the API, allowing users to correctly apply ignore rules and evaluate severity thresholds without having to implement this themselves.Closes #204