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

feat: add "vertical" output format #889

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

G-Rath
Copy link
Collaborator

@G-Rath G-Rath commented Mar 26, 2024

This adds a new "vertical" output format that is designed for humans and based on the output of osv-detector, which effectively aims to group the output relating to each entity being scanned in vertical slices:

image

Unfortunately I think it suffers significantly due to the assumptions made by the rest of the codebase for outputting that made sense when the final output was a table i.e. we dump a lot of information as we go about scanning, config files, vulnerability filtering, and so on that really should be grouped but currently cannot because they're all outputted at different stages - I think a way to address that could be using some sort of event-emitter type pattern so that the reporters could be responsible for deciding what they actually do (e.g. r.Emit("filtered-vulnerability", ...) and then most reporters could choose to just print immediately, and ones like "vertical" could choose to add it to an internal struct), but I think that'll involve a lot more work; for now I'm just going to ignore the pre-results output.

Resolves #85

@G-Rath G-Rath force-pushed the add-vertical-reporter branch 3 times, most recently from 505d532 to bae0efe Compare March 28, 2024 02:59
@codecov-commenter
Copy link

codecov-commenter commented Mar 28, 2024

Codecov Report

Attention: Patch coverage is 93.56725% with 11 lines in your changes are missing coverage. Please review.

Project coverage is 64.10%. Comparing base (c2bd45e) to head (b8ac63e).

Files Patch % Lines
pkg/reporter/vertical_reporter.go 76.47% 8 Missing ⚠️
internal/output/vertical.go 97.77% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #889      +/-   ##
==========================================
+ Coverage   63.67%   64.10%   +0.42%     
==========================================
  Files         146      148       +2     
  Lines       11960    12131     +171     
==========================================
+ Hits         7616     7776     +160     
- Misses       3878     3888      +10     
- Partials      466      467       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@G-Rath
Copy link
Collaborator Author

G-Rath commented Apr 18, 2024

I realised that the tests I've added here actually serve for all the output formatters, so I'm going to split this into two PRs - one adding a general set of tests that get used across all outputters to showcase their output, and a second adding the new "vertical" output format

another-rex pushed a commit that referenced this pull request May 24, 2024
This introduces a set of crafted scanner results that each supported
`output` format is run through to showcase how they look across all the
different results possible from a scanner run - it originally started
life as the tests for #889 but I realised they could base used more
generally for testing and reviewing all the outputters, so here we are.

~It looks like this has also revealed the SARIF output is unstable in
its ordering, which I'll aim to address in a dedicated PR~
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.

Optimise human readable output for narrow terminals
2 participants