search

clj-holmes / clj-watson

clojure deps SCA
Eclipse Public License 2.0
77 stars 8 forks source link

Consider clearly conveying important options #79

Open lread opened 1 month ago

lread commented 1 month ago

Currently

Clj-watson conveys the properties we've overridden via clj-watson.properties:

Merging additional properties:
  # clj-watson.properties file
  nvd.api.key=31a0****-****-****-****-********5002

But

Since scanning for vulnerabilities is a serious thing, we should convey more.

As a user, I want to make sure clj-watson has understood what I've asked it to do and also convey any important defaults.

So

To me, also conveying:

Next

Happy to explore/discuss/do/help/whatever. Lemme know.

seancorfield commented 2 weeks ago
lread commented 2 weeks ago
  • I think the env vars aspect is covered (the property specified, but not the value)?

Yes, I think it is a great start.

  • I'm ambivalent about command-line options since those are very obvious at the point of invocation.

I did add some decent cmd line arg checking, so you might be right here. A typo shouldn't burn the user.

But one value is outright command line unfriendly. The --alias option accepts *, but * will be expanded by the bash shell to all files in the current directory. So the user must quote it: --alias '*'.

  • I think all the properties from clj-watson.properties are conveyed?

Yeah, I think so.

  • Are there any other "important" defaults that clj-watson should convey beyond the DB location?

Yeah, db location would be a good one.

Because properties can be sourced from:

When a user has opted to override a default, it would be nice to show not only where that default is coming from but also when a user-specified property is being overridden by another user-specified property.

Stating what the user has selected can nudge them to wonder what else they can do and to read the usage help and/or docs.

Here's what the kind of thing I am thinking of:

Running dependency-check scan against /home/lee/proj/oss/clj-commons/clj-yaml/deps.edn
- Scanning dependencies from default deps
- Remediations will NOT be suggested.
- Will exit with 0 on when vulnerabilities found.
Settings of note:
- nvd.api.key <secret>    
  From system property (overriding env var CLJ_WATSON_NVD_API_KEY)
- data.directory /home/lee/.m2/repository/org/owasp/dependency-check-utils/10.0.3/data/9.0 
  (default)

Or when aliases specified maybe (maybe I selected --aliases '*' here).

Running dependency-check scan against /home/lee/proj/oss/clj-commons/clj-yaml/deps.edn
- Scanning dependencies from aliases :1.8, :1.9, :1.10, :1.11, :1.12, :test, :native-test:, :build, :clj-condo, :eastwood
...

We could outright ignore overriding via the deprecated --dependency-check-properties. (We already warn the user this is deprecated).

Anyhow, that's the kind of thing I am thinking of. Now that I've fleshed it out a bit lemme know what you think.

seancorfield commented 2 weeks ago

That's... very chatty... I'll have to give this some thought.

lread commented 2 weeks ago

Yeah, maybe so. My thinking is that the extra clarity is good for a security scanning tool. But worth sleeping on this for a bit.

lread commented 2 weeks ago

For reference, here's our current printlns:

Read 39 dependency-check properties.
Merging additional properties:
  # clj-watson.properties file
  nvd.api.key=31a0****-****-****-****-********5002

Setting nvd.api.key from CLJ_WATSON_NVD_API_KEY.

So, we currently have 5 significant lines, which I am proposing to replace with 9 (admittedly more fleshy and admittedly TBD) lines.