Add the total types of vulnerabilities in Grype output #946

zhiburt · 2022-10-04T22:12:56Z

Hi there,

Thank you for keeping "Good first issue".

Let me know if I did it correct.
Notice I haven't include any tests.

close #877

Take care

Signed-off-by: Maxim Zhiburt <zhiburt@gmail.com>

willyw0nka · 2022-10-05T09:04:41Z

Hi 👋, I think that showing vulnerabilities from Critical to Unknown would be more valuable that the correct shown order (Unknown to Critical). Similar to the order proposed in #709. Maybe some contributor can corroborate this feedback

Signed-off-by: Maxim Zhiburt <zhiburt@gmail.com>

zhiburt · 2022-10-05T11:52:05Z

freedom-isnotanarchy · 2022-10-05T16:27:43Z

Would it be possible/desirable, to tack on "Fixed: ##" , at the end.
I believe "Fixed:" , is also in the spirit of this PR.
Thank you for your consideration.

Signed-off-by: Maxim Zhiburt <zhiburt@gmail.com>

zhiburt · 2022-10-05T17:59:51Z

@freedom-isnotanarchy

Done (thanks for comment about the format).
I've used ; instead of , as I take these to be a bit different.

freedom-isnotanarchy · 2022-10-05T20:16:43Z

TODAY:

 Vulnerability DB        [updated]
 ✔ Indexed .
 ✔ Cataloged packages      [1229 packages]
 
geronimo-javamail_1.4_spec      1.7.1                     java-archive  CVE-2008-0732        Low
NAME                            INSTALLED       FIXED-IN  TYPE          VULNERABILITY        SEVERITY
gson                            2.8.6           2.8.9     java-archive  GHSA-4jrv-ppp4-jm57  High
spring-core                     5.2.12.RELEASE            java-archive  CVE-2021-22118       High
spring-core                     5.3.22                    java-archive  CVE-2016-1000027     Critical
xmlsec                          2.1.4           2.1.7     java-archive  GHSA-j8wc-gxx9-82hx  High

YOUR PR: [https://github.com//pull/946]

 Vulnerability DB        [updated]
 ✔ Indexed .
 ✔ Cataloged packages      [1229 packages]
 
NAME                            INSTALLED       FIXED-IN  TYPE          VULNERABILITY        SEVERITY
geronimo-javamail_1.4_spec      1.7.1                     java-archive  CVE-2008-0732        Low
gson                            2.8.6           2.8.9     java-archive  GHSA-4jrv-ppp4-jm57  High
spring-core                     5.2.12.RELEASE            java-archive  CVE-2021-22118       High
spring-core                     5.3.22                    java-archive  CVE-2016-1000027     Critical
xmlsec                          2.1.4           2.1.7     java-archive  GHSA-j8wc-gxx9-82hx  High
[Critcal: 1, High: 3, Medium: 0, Low: 0, Unkonwn: 0; Fixed: 2]

[Critcal: 1, High: 3, Medium: 0, Low: 0, Unkonwn: 0; Fixed: 2] <-- Choice 1) The beautiful work, you have done.
[Critcal: 1, High: 3, Medium: 0, Low: 0, Unkonwn: 0, Fixed: 2] <-- Choice 2) Only use all commas. For symmetry.
Summary: [Critcal: 1, High: 3, Medium: 0, Low: 0, Unkonwn: 0, Fixed: 2] <-- Choice 3) Add "Summary:"

zhiburt · 2022-10-05T21:18:49Z

So shall I make it Summary: [Critcal: 1, High: 3, Medium: 0, Low: 0, Unkonwn: 0, Fixed: 2] ?

freedom-isnotanarchy · 2022-10-05T23:22:21Z

(1) Yes, personally, I prefer this format:

NAME                            INSTALLED       FIXED-IN  TYPE          VULNERABILITY        SEVERITY
geronimo-javamail_1.4_spec      1.7.1                     java-archive  CVE-2008-0732        Low
gson                            2.8.6           2.8.9     java-archive  GHSA-4jrv-ppp4-jm57  High
spring-core                     5.2.12.RELEASE            java-archive  CVE-2021-22118       High
spring-core                     5.3.22                    java-archive  CVE-2016-1000027     Critical
xmlsec                          2.1.4           2.1.7     java-archive  GHSA-j8wc-gxx9-82hx  High
Summary: [Critical: 1, High: 3, Medium: 0, Low: 0, Unknown: 0, Fixed: 2]

(2) But on the higher level, any "Summary" is good, to me.
I am just one person.
And thank you, for your effort.

spiffcs · 2022-10-06T03:13:04Z

Thanks @zhiburt for the PR! Also huge shoutout to all the feedback already thanks everyone.

I kicked off the CI jobs so we can get those running. I have a backlog of things I'm getting through but I'll take a look here when time permits to give a good review.

Signed-off-by: Maxim Zhiburt <zhiburt@gmail.com>

* main: Update grype bootstrap tools to latest versions. (anchore#947) chore: add more quality gate images (anchore#950) Add in-depth quality gate checks (anchore#949) Update Syft to v0.58.0 (anchore#941)

spiffcs · 2022-10-06T16:00:12Z

Thanks for keeping this in the status lines. Since it doesn't change the output of the table I think this is a great addition. Let me get the tests passing and lint fixed and I'll tag a second reviewer from the team.

^--^ ^------------^ | | | +-> Summary in present tense. | +-------> Type: chore, docs, feat, fix, refactor, style, or test. [optional body] [optional footer(s)] Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>

spiffcs · 2022-10-07T04:01:55Z

Lint's are fixed - I'll have more time to look at the panic later - if anyone else want's to give it a shot feel free to add a commit here for those tests

Signed-off-by: Albert Simon <simon.albert75@gmail.com>

Fixed integration tests

zhiburt · 2022-10-17T13:46:21Z

@willyw0nka

CI need an approval

willyw0nka · 2022-10-17T13:53:42Z

I'm not a mantainer, so @spiffcs has to approve it and kick the CI

zhiburt · 2022-10-17T14:10:57Z

btw: thanks for your commit with the fix.

kzantow · 2022-10-17T14:15:42Z

ui/event_handlers.go

+		_, _ = io.WriteString(scanningLine, fmt.Sprintf(statusTitleTemplate+"%s", spin, title, auxInfo))
+
+		auxInfo2 := auxInfoFormat.Sprintf(
+			"[Critical: %d, High: %d, Medium: %d, Low: %d, Unknown: %d, Fixed: %d]",


A quick UX note: maybe we don't show the types that have 0 reported vulnerabilities so this line may be shorter?

In my personal thought: Always show all columns. Show all, even if they have "0 count".
Breaking symmetry, can be troublesome, in my opinion.

I didn't have a strong opinion about this, just a suggestion and fine as-is 👍

willyw0nka · 2022-10-21T11:18:06Z

It seems that Mac Acceptance tests failed due to a github out of service error (HTTP 503), can anyone trigger the action again?

wagoodman · 2022-10-24T13:38:08Z

Hey folks -- I love the idea on outputting summarizing vulnerability result information directly in the TUI! I had a few comments.

From a visual perspective, a line that starts with a status icon (the checkbox / spinner) and a bolded title is meant to convey that there is a task that is occurring. The icon helps understand the status in a glanceable way and the bold title stands out as the tense-approriate verb (e.g. "parsing" vs "parsed"). The current "summary" title/status doesn't represent a task (some work done).
There isn't an indication that the summary line is only for the preceding task.
There is a lot of information trying to come across in a single line, which could get rather long (maybe not prohibitively so though).

With these comments in mind, I wanted to put forth an alternative (opinionated) output that addresses these points:

This trades horizontal real estate with vertical real estate to show more information. The tree-like output is consistent with how we show related information in the log output and helps to indicate "this additional information belongs to the previous row".

What do folks think about this proposal? I tweaked the current branch to output this but didn't push anything yet until I got more feedback.

kzantow · 2022-10-24T17:42:02Z

I, personally, like this much better @wagoodman -- attaching the information the previous scan item, and helps avoid as wide a line with this info 👍

Going this route, I might move the unknown to its own line, too?

freedom-isnotanarchy · 2022-10-24T18:09:45Z

Placeholder: Let me gather some thoughts, and come back to make a Comment, soon
Picture: A comparison of both proposals, placed on the same picture. For your consideration ...

gh-greg · 2022-10-26T05:06:15Z

Is this Feature/PR, related to this ?
Add the total types of vulnerabilities in Grype output
#877

kzantow · 2022-10-26T13:40:28Z

@gh-greg yes (you can also see this in the description and the Development section on the right -- we try our best to link PRs to the issue they relate to)

willyw0nka · 2022-10-27T08:27:43Z

Hi, @wagoodman reasoning seems ok to me. How would the --only-fixed flag affect the summary?

gh-greg · 2022-10-28T04:53:48Z

Regarding Printing Summary:
You ask, what is Twistlock / PrismaCloud 's' default CLI behaviour ?
To print this:

Vulnerabilities found for image MY_IMAGE: total - 114, critical - 1, high - 6, medium - 57, low - 50
[PRISMACLOUD] Found 1 relevant files
[PRISMACLOUD] Found 114 vulnerabilities in 1 images

Signed-off-by: Alex Goodman <alex.goodman@anchore.com>

spiffcs · 2023-02-10T18:45:02Z

Updating conflicts and getting this tested now

* main: (99 commits) chore(deps): bump actions/cache from 3.2.4 to 3.2.5 (anchore#1129) chore(deps): bump github.com/docker/docker (anchore#1128) Update Syft to v0.71.0 (anchore#1126) chore(deps): bump github/codeql-action from 2.2.1 to 2.2.3 (anchore#1125) Update grype bootstrap tools to latest versions. (anchore#1124) chore(deps): bump golang.org/x/term from 0.4.0 to 0.5.0 (anchore#1123) Update grype bootstrap tools to latest versions. (anchore#1122) Update grype bootstrap tools to latest versions. (anchore#1116) Update Syft to v0.70.0 (anchore#1117) chore(deps): bump github.com/docker/docker (anchore#1114) Update grype bootstrap tools to latest versions. (anchore#1112) Update Syft to v0.69.1 (anchore#1111) chore: prune cosign dependency for grype builds (anchore#1100) Update grype bootstrap tools to latest versions. (anchore#1108) Update Syft to v0.69.0 (anchore#1109) chore(deps): bump actions/cache from 3.2.3 to 3.2.4 (anchore#1107) chore: add new images to quality gate (anchore#1106) chore: bump yardstick for better quality gate filtering (anchore#1101) chore(deps): bump actions/cache from 3.0.11 to 3.2.3 (anchore#1096) chore(deps): bump github/codeql-action from 2.1.39 to 2.2.1 (anchore#1097) ... Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>

Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>

spiffcs · 2023-02-10T21:07:46Z

Integration tests are failing with a panic - fixing this - then will post a screenshot of the final state - If we're happy with the new visualization we can merge 🥳

Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>

spiffcs · 2023-02-10T21:36:19Z

New output format (thanks @wagoodman ):

Vs old output:

spiffcs · 2023-02-10T21:41:29Z

There is one todo comment for debug logging - adding that now

Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>

wagoodman

The code I pushed was meant to be illustrative and was PoC-quality (and needed refactoring), my comments really are touching on that aspect.

vulnerabilitiesList and VulnerabilitiesCategories are enumerating the severities manually, however, it feels like it should have more of a collection on it. Something like BySeverity map[vulnerability.Severity]*progress.Manual (with one extra detail, the public facing VulnerabilitiesCategories should expose progress.Monitorable and not *progress.Manual). This way there shouldn't need to be refactors in this area if we add a severity (or remove one).
We should lean more heavily into using viulnerability.Severity and surrounding helpers (such as vulnerability.ParseSeveriry)
Are we leaving off negligable severity intentionally? if so, that should be a caller concern (in the log summary handler and tui handler) but the monitor code should still include those severity counts.
nit: naming for VulnerabilitiesCategories is awkward since both words are plural. Maybe something more like VulnerabilityCategories.

I've got more comments, but I think these could have enough impact to nullify them, I'll wait to hear back about these first.

spiffcs · 2023-02-13T15:03:03Z

The code I pushed was meant to be illustrative and was PoC-quality (and needed refactoring), my comments really are touching on that aspect.

vulnerabilitiesList and VulnerabilitiesCategories are enumerating the severities manually, however, it feels like it should have more of a collection on it. Something like BySeverity map[vulnerability.Severity]*progress.Manual (with one extra detail, the public facing VulnerabilitiesCategories should expose progress.Monitorable and not *progress.Manual). This way there shouldn't need to be refactors in this area if we add a severity (or remove one).

I'll get the above cleaned up where we have something like BySeverity map. I'll also expose progress.Monitorable

We should lean more heavily into using viulnerability.Severity and surrounding helpers (such as vulnerability.ParseSeveriry)

I'll see if there are are any other helpers we need and expose this further up so we can do something like VulnerabilitiesBySeverity which uses the by severity map we talked about above.

Are we leaving off negligable severity intentionally? if so, that should be a caller concern (in the log summary handler and tui handler) but the monitor code should still include those severity counts.

No - I'll add that in with the above comments

nit: naming for VulnerabilitiesCategories is awkward since both words are plural. Maybe something more like VulnerabilityCategories.

👍

I've got more comments, but I think these could have enough impact to nullify them, I'll wait to hear back about these first.

Cool! I'll address the above and we can do the small clean up after

…vulnerabilities Signed-off-by: Alex Goodman <alex.goodman@anchore.com>

Signed-off-by: Alex Goodman <alex.goodman@anchore.com>

Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>

* main: Add the total types of vulnerabilities in Grype output (#946) chore(deps): bump gorm.io/gorm from 1.23.5 to 1.23.10 (#1157)

Add the total types of vulnerabilities in Grype output

1c6448e

Signed-off-by: Maxim Zhiburt <zhiburt@gmail.com>

Reverse the order of categories

5e87067

Signed-off-by: Maxim Zhiburt <zhiburt@gmail.com>

Add Fixed stat

1c1d54d

Signed-off-by: Maxim Zhiburt <zhiburt@gmail.com>

zhiburt and others added 2 commits October 6, 2022 08:35

Fix lints

3f348f8

Signed-off-by: Maxim Zhiburt <zhiburt@gmail.com>

Merge branch 'main' into patch-total-types-of-vulnerabilities

968868c

* main: Update grype bootstrap tools to latest versions. (anchore#947) chore: add more quality gate images (anchore#950) Add in-depth quality gate checks (anchore#949) Update Syft to v0.58.0 (anchore#941)

fix: update lint issues for CI checks

620c551

^--^ ^------------^ | | | +-> Summary in present tense. | +-------> Type: chore, docs, feat, fix, refactor, style, or test. [optional body] [optional footer(s)] Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>

willyw0nka and others added 2 commits October 13, 2022 16:21

Fixed integration tests

484febd

Signed-off-by: Albert Simon <simon.albert75@gmail.com>

Merge pull request #1 from willyw0nka/fix-integration-tests

766cc64

Fixed integration tests

kzantow reviewed Oct 17, 2022

View reviewed changes

add todo for logging summary

7c2c7b8

Signed-off-by: Alex Goodman <alex.goodman@anchore.com>

spiffcs added 2 commits February 10, 2023 13:53

fix: static-analysis

138e467

Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>

test: add metadata provider back into testing

46e5cc5

Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>

chore: debug summary

78dc2d9

Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>

wagoodman reviewed Feb 13, 2023

View reviewed changes

wagoodman and others added 3 commits March 3, 2023 13:50

Merge remote-tracking branch 'origin/main' into patch-total-types-of-…

eac8e0a

…vulnerabilities Signed-off-by: Alex Goodman <alex.goodman@anchore.com>

refactor matching bus event types

9ca2493

Signed-off-by: Alex Goodman <alex.goodman@anchore.com>

refactor: update formatfn to be reusable

4fe76a8

Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>

wagoodman approved these changes Mar 3, 2023

View reviewed changes

fix: update fn call in root

b43fff2

Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>

spiffcs enabled auto-merge (squash) March 3, 2023 20:21

fix: import fix

68471f2

Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>

spiffcs merged commit b88e961 into anchore:main Mar 3, 2023

spiffcs added a commit that referenced this pull request Mar 3, 2023

Merge branch 'main' into pipeline-parity

569fb8f

* main: Add the total types of vulnerabilities in Grype output (#946) chore(deps): bump gorm.io/gorm from 1.23.5 to 1.23.10 (#1157)

spiffcs added the enhancement New feature or request label Mar 3, 2023

This was referenced Mar 28, 2023

Bump github.com/anchore/grype from 0.32.0 to 0.60.0 Scrumplex/kubei#56

Closed

Bump github.com/anchore/grype from 0.32.0 to 0.60.0 doytsujin/kubei#59

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add the total types of vulnerabilities in Grype output #946

Add the total types of vulnerabilities in Grype output #946

zhiburt commented Oct 4, 2022 •

edited

Loading

willyw0nka commented Oct 5, 2022

zhiburt commented Oct 5, 2022

freedom-isnotanarchy commented Oct 5, 2022

zhiburt commented Oct 5, 2022 •

edited

Loading

freedom-isnotanarchy commented Oct 5, 2022 •

edited

Loading

zhiburt commented Oct 5, 2022

freedom-isnotanarchy commented Oct 5, 2022 •

edited

Loading

spiffcs commented Oct 6, 2022

spiffcs commented Oct 6, 2022

spiffcs commented Oct 7, 2022

zhiburt commented Oct 17, 2022 •

edited

Loading

willyw0nka commented Oct 17, 2022

zhiburt commented Oct 17, 2022

kzantow Oct 17, 2022

freedom-isnotanarchy Oct 17, 2022

kzantow Oct 17, 2022

willyw0nka commented Oct 21, 2022

wagoodman commented Oct 24, 2022

kzantow commented Oct 24, 2022 •

edited

Loading

freedom-isnotanarchy commented Oct 24, 2022

gh-greg commented Oct 26, 2022 •

edited

Loading

kzantow commented Oct 26, 2022 •

edited

Loading

willyw0nka commented Oct 27, 2022

gh-greg commented Oct 28, 2022 •

edited

Loading

spiffcs commented Feb 10, 2023

spiffcs commented Feb 10, 2023

spiffcs commented Feb 10, 2023 •

edited

Loading

spiffcs commented Feb 10, 2023

wagoodman left a comment

spiffcs commented Feb 13, 2023 •

edited

Loading

Add the total types of vulnerabilities in Grype output #946

Add the total types of vulnerabilities in Grype output #946

Conversation

zhiburt commented Oct 4, 2022 • edited Loading

willyw0nka commented Oct 5, 2022

zhiburt commented Oct 5, 2022

freedom-isnotanarchy commented Oct 5, 2022

zhiburt commented Oct 5, 2022 • edited Loading

freedom-isnotanarchy commented Oct 5, 2022 • edited Loading

TODAY:

YOUR PR: [https://github.com//pull/946]

zhiburt commented Oct 5, 2022

freedom-isnotanarchy commented Oct 5, 2022 • edited Loading

spiffcs commented Oct 6, 2022

spiffcs commented Oct 6, 2022

spiffcs commented Oct 7, 2022

zhiburt commented Oct 17, 2022 • edited Loading

willyw0nka commented Oct 17, 2022

zhiburt commented Oct 17, 2022

kzantow Oct 17, 2022

Choose a reason for hiding this comment

freedom-isnotanarchy Oct 17, 2022

Choose a reason for hiding this comment

kzantow Oct 17, 2022

Choose a reason for hiding this comment

willyw0nka commented Oct 21, 2022

wagoodman commented Oct 24, 2022

kzantow commented Oct 24, 2022 • edited Loading

freedom-isnotanarchy commented Oct 24, 2022

gh-greg commented Oct 26, 2022 • edited Loading

kzantow commented Oct 26, 2022 • edited Loading

willyw0nka commented Oct 27, 2022

gh-greg commented Oct 28, 2022 • edited Loading

spiffcs commented Feb 10, 2023

spiffcs commented Feb 10, 2023

spiffcs commented Feb 10, 2023 • edited Loading

spiffcs commented Feb 10, 2023

wagoodman left a comment

Choose a reason for hiding this comment

spiffcs commented Feb 13, 2023 • edited Loading

zhiburt commented Oct 4, 2022 •

edited

Loading

zhiburt commented Oct 5, 2022 •

edited

Loading

freedom-isnotanarchy commented Oct 5, 2022 •

edited

Loading

freedom-isnotanarchy commented Oct 5, 2022 •

edited

Loading

zhiburt commented Oct 17, 2022 •

edited

Loading

kzantow commented Oct 24, 2022 •

edited

Loading

gh-greg commented Oct 26, 2022 •

edited

Loading

kzantow commented Oct 26, 2022 •

edited

Loading

gh-greg commented Oct 28, 2022 •

edited

Loading

spiffcs commented Feb 10, 2023 •

edited

Loading

spiffcs commented Feb 13, 2023 •

edited

Loading