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

Hide suppressed vulnerabilities when --show-suppressed is not given #1322

Merged
merged 1 commit into from
May 30, 2023
Merged

Hide suppressed vulnerabilities when --show-suppressed is not given #1322

merged 1 commit into from
May 30, 2023

Conversation

jamestran201
Copy link

Closes #1053

This PR fixes a bug where suppressed vulnerabilities are displayed even though the --show-suppressed flag is not provided. The fix is made by checking the value of the showSuppressed option before including the suppressed vulns in the result. This PR also adds tests to assert the behaviour with/without --show-suppressed.

Signed-off-by: James Tran <jamestran201@github.com>
Comment on lines +35 to +37
packages := generatePackages(t)
matches := generateMatches(t, packages[0], packages[0])
ignoredMatches := generateIgnoredMatches(t, packages[1])
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

generatePackages() returns 2 packages. I wanted to use one package for generating ignored matches, so I passed the first package twice to generateMatches(). This feels a bit strange, but I'm not sure how to change it.

@spiffcs
Copy link
Contributor

spiffcs commented May 30, 2023

Thanks for the awesome fix @jamestran201! It looks like there has been some drift in the source of truth for grype's config structs that this PR highlights pretty welll (not that we have to address it here).

When I was doing a quick review I noticed that showSuppressed was being passed to the presenter constructor separately from models.PresenterConfig. I thought it might be a quick change to just add showSuppressed onto models.PresenterConfig, but that also seemed a little odd given there are a couple Config floating around:

  • presenter.Config
  • models.PresenterConfig
  • Global config.Application

cc @wagoodman @kzantow since they've been doing some work on config refactors across our tooling, but I 🟢 this PR as a good fix with this comment serving as a promise to address the code confusion and possibly eventually stick showSuppressed onto it's own forever home in the code

@spiffcs spiffcs merged commit c1f6772 into anchore:main May 30, 2023
@wagoodman
Copy link
Contributor

@spiffcs there are some oddities here, but I'll take off the 20-20 glasses for a second and answer:

  • the presenter package is where all presenter implementations are wired/selected. The presenter.Config is the input needed for generating the single presenter. Think of this as "how to do the presentation" options (within the lib context)
  • config.Application is the cmd-level concern for representing all application objects (outside of the lib context).
  • models.PresenterConfig is a config that represents all data needed to be shown. Think of this as the "what should be presented" (within the lib context)

Overall, there is a need to cleanup these objects (and a reevaluation of some of them too as you pointed out).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants