google / osv-scanner

Vulnerability scanner written in Go which uses the data provided by https://osv.dev
https://google.github.io/osv-scanner/
Apache License 2.0
6.01k stars 337 forks source link

Add `experimental-download-offline-databases` flag #1039

Closed cuixq closed 2 weeks ago

cuixq commented 2 weeks ago

Currently flags experimental-offline and experimental-local-db are confusing sometimes.

This PR renames experimental-local-db to experimental-download-database to make it more explicit whether to download the database or not.

For now, experimental-download-database only works when experimental-offline is set.

internal/local is also modified to reflect the change in the naming and meaning of this flag.

codecov-commenter commented 2 weeks ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 65.29%. Comparing base (72afdb8) to head (d9b037c).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1039 +/- ## ========================================== + Coverage 65.23% 65.29% +0.05% ========================================== Files 150 150 Lines 12497 12498 +1 ========================================== + Hits 8153 8160 +7 + Misses 3882 3878 -4 + Partials 462 460 -2 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

cuixq commented 2 weeks ago

For now, this flag is only valid when experimental-offline is set. The motivation is that experimental-offline and experiment-local-db is confusing, so we are trying to separate the offline part and the download part.

I think ideally, we can have some subcommand for example download to handle downloading the database, and I like the idea to specify the systems to download in the command!

G-Rath commented 2 weeks ago

Is the intention for this feature to be used independently of a scanning run, or as part of one? Would it be possible to include another flag that accepted a subset of ecosystems to download

@andrewpollock I proposed basically this exact thing in subcommand form at our weekly catchup 😄

My thinking is especially now that we have both subcommands and interactive-ness, we could provide a very nice interface for managing these databases and their location without overloading scan with a lot of flags

Part of me now wonders if it could be worth having a dedicated offline subcommand instead of a flag? but maybe that's just trading a (flag) swing for a (subcommand) roundabout?

another-rex commented 2 weeks ago

I personally think download-offline-databases makes more sense than offline-download-databases.

cuixq commented 2 weeks ago

@G-Rath do you mind taking another look? Thanks.