Allow offline vulnerability database and transitive resolution to be enabled/disabled separately.

[michaelkedar]

Currently, enabling offline mode (--experimental-offline) in scanning makes the scanner use a local vulnerability database and disables transitive dependency resolution for Maven pom.xml files.

But there are use cases for having one and not the other, i.e.

• Disable transitive resolution, but still use the OSV.dev vulnerability API, as in Support a --no-resolve flag to avoid resolving transitive dependencies during scanning #1121
• Use a local vuln db, but still do transitive scanning in environments with access to the maven registry but not osv.dev.

With #1286 adding the --experimental-resolution-data-source flag, we could allow setting this to none or disable to disable transitive resolution, and make the --experimental-offline flag affect only the vulnerability database. (But that would mean 'offline mode' may still attempt to make network requests...)

[cuixq]

I think we still should follow the assumption not to make network requests with --experimental-offline flag.

Maybe make this flag to take a string:

• all or empty: default, no network requests at all
• vulnerability: use offline database for vulnerability, but performs dependency resolution
• dependency: does not perform transitive dependency scanning but making requests to OSV

[oliverchang]

I think we still should follow the assumption not to make network requests with --experimental-offline flag.

Maybe make this flag to take a string:

• all or empty: default, no network requests at all
• vulnerability: use offline database for vulnerability, but performs dependency resolution
• dependency: does not perform transitive dependency scanning but making requests to OSV

+1! But I'd rename dependency to pkg-registry or something similar to be clearer.

[michaelkedar]

• all or empty: default, no network requests at all

Hm, the desired behaviour seems impossible with urfave/cli:

# using cli.StringFlag
# original behaviour is broken:
> osv-scanner scan --experimental-offline
flag needs an argument: -experimental-offline

> osv-scanner scan --experimental-offline --experimental-download-offline-databases
experimental-offline = "--experimental-download-offline-databases"
experimental-download-offline-databases = false

# now the string is always required:
> osv-scanner scan --experimental-offline all
experimental-offline = "all"
> osv-scanner scan --experimental-offline=vulnerability
experimental-offline = "vulnerability"
> osv-scanner scan --experimental-offline=
experimental-offline = ""

Personally, I'd prefer to keep the --experimental-offline syntax the same as a boolean flag.

How about adding new flags e.g. --experimental-offline-vulnerabilities and --experimental-no-resolve (or --experimental-resolution-data-source=none), and have --experimental-offline set both?

[oliverchang]

How about adding new flags e.g. --experimental-offline-vulnerabilities and --experimental-no-resolve (or --experimental-resolution-data-source=none), and have --experimental-offline set both?

SGTM, this seems pretty reasonable.

Now that I think about it, I think we also discovered we couldn't have an empty string work as "all" by default for --call-analysis / --no-call-analysis in #513. @hogo6002 is that right?

[cuixq]

SGTM as well - I am leaning to have --experimental-no-resolve for turning off resolution.

[hogo6002]

Now that I think about it, I think we also discovered we couldn't have an empty string work as "all" by default for --call-analysis / --no-call-analysis in #513. @hogo6002 is that right?

Yeah, I think we can't directly pass an empty string value to a cli.StringFlag, but Gareth mentioned we can use cli.GenericFlag to implement it. #513 (comment)

[oliverchang]

Now that I think about it, I think we also discovered we couldn't have an empty string work as "all" by default for --call-analysis / --no-call-analysis in #513. @hogo6002 is that right?

Yeah, I think we can't directly pass an empty string value to a cli.StringFlag, but Gareth mentioned we can use cli.GenericFlag to implement it. #513 (comment)

Interesting, but since we didn't go with this for --call-analysis, we should be consistent and just go with an approach that doesn't rely on this. i.e. @michaelkedar 's suggestion in #1339 (comment)