clj-holmes / clj-watson

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

Streamline dependency(-)check.properties overrides #103

Closed seancorfield closed 4 weeks ago

seancorfield commented 1 month ago

Split off from #66 cc @lread

  1. Confirmed: Figure out whether clj-watson's dependency-check.properties file overrides DependencyCheck core's dependencycheck.properties file (i.e., does DC core always read dc.p and merge in the settings it is passed from clj-watson's dependency-check.properties file + clj-watson.properties file?). a. Unneeded: If it doesn't auto-merge like that, we need to have clj-watson read dc.p and do the merge with d-c.p itself.
  2. Eliminate everything from clj-watson's d-c.p that is the same in the underlying dc.p, leaving only things that need to be different (the ${pom.*} substitutions, etc); this will remove the data.directory= setting so the database because more CI cache-friendly by default.

Existing behavior, after CLI parsing improvements: If -d is specified, it must exist.

lread commented 1 month ago

I think we should also deprecate the -d option? These are clj-watson's overrides for dependency check defaults.

seancorfield commented 1 month ago

Yes, this 6.0 release is probably a good time to deprecate it (in favor of clj-watson.properties and JVM properties/env vars).

seancorfield commented 4 weeks ago

Confirmed that the core dependencycheck.properties are used/overridden by clj-watson's own dependency-check.properties file (as expected - yay!), and that without data.directory in the latter, we get the DB files here:

> ls ~/.m2/repository/org/owasp/dependency-check-utils/10.0.3/data/9.0/
cache  odc.mv.db  odc.update.lock

Now I'm going to see what's actually getting overridden in clj-watson, so we can streamline dependency-check.properties...

seancorfield commented 4 weeks ago

The differences are mostly turning off a bunch of analyzers (presumably because it speeds things up and they are not applicable in a JVM world) but there are some other kind of weird differences (a few of these may have been made to speed up clj-watson when the API was congested: start year, # threads, delay):

1c1
< analyzer.archive.enabled=true
---
> analyzer.archive.enabled=false
3,6c3,5
< analyzer.assembly.enabled=true
< analyzer.autoconf.enabled=true
< analyzer.bundle.audit.enabled=true
< analyzer.carthage.enabled=true
---
> analyzer.assembly.enabled=false
> analyzer.autoconf.enabled=false
> analyzer.bundle.audit.enabled=false
8c7
< analyzer.central.parallel.analysis=false
---
> analyzer.central.parallel.analysis=true
13c12
< analyzer.cmake.enabled=true
---
> analyzer.cmake.enabled=false
15,16c14
< analyzer.composer.lock.enabled=true
< analyzer.cpanfile.enabled=true
---
> analyzer.composer.lock.enabled=false
19d16
< analyzer.dart.enabled=true
25,26c22,23
< analyzer.golang.dep.enabled=true
< analyzer.golang.mod.enabled=true
---
> analyzer.golang.dep.enabled=false
> analyzer.golang.mod.enabled=false
29,31d25
< analyzer.knownexploited.enabled=true
< analyzer.libman.enabled=true
< analyzer.maveninstall.enabled=true
33c27
< analyzer.msbuildproject.enabled=true
---
> analyzer.msbuildproject.enabled=false
35c29
< analyzer.nexus.proxy=true
---
> analyzer.nexus.proxy=false
37c31
< analyzer.node.audit.enabled=true
---
> analyzer.node.audit.enabled=false
39,40c33,34
< analyzer.node.audit.use.cache=true
< analyzer.node.package.enabled=true
---
> analyzer.node.audit.use.cache=false
> analyzer.node.package.enabled=false
42,43c36,37
< analyzer.nugetconf.enabled=true
< analyzer.nuspec.enabled=true
---
> analyzer.nugetconf.enabled=false
> analyzer.nuspec.enabled=false
50,55c44,47
< analyzer.pip.enabled=true
< analyzer.pipfile.enabled=true
< analyzer.pnpm.audit.enabled=true
< analyzer.poetry.enabled=true
< analyzer.python.distribution.enabled=true
< analyzer.python.package.enabled=true
---
> analyzer.pip.enabled=false
> analyzer.pipfile.enabled=false
> analyzer.python.distribution.enabled=false
> analyzer.python.package.enabled=false
57c49
< analyzer.retirejs.enabled=true
---
> analyzer.retirejs.enabled=false
61,63c53,54
< analyzer.ruby.gemspec.enabled=true
< analyzer.swift.package.manager.enabled=true
< analyzer.swift.package.resolved.enabled=true
---
> analyzer.ruby.gemspec.enabled=false
> analyzer.swift.package.manager.enabled=false
67c58
< archive.scan.depth=3
---
> archive.scan.depth=10
70a62
> cve.check.validforhours=12
71a64,68
> cve.startyear=2002
> cve.url.base=https://nvd.nist.gov/feeds/json/cve/1.1/nvdcve-1.1-%d.json.gz
> cve.url.modified.validfordays=7
> cve.url.modified=https://nvd.nist.gov/feeds/json/cve/1.1/nvdcve-1.1-modified.json.gz
> cve.url.original=https://nvd.nist.gov/feeds/json/cve/1.1/nvdcve-1.1-modified.json.gz
73c70
< data.directory=[JAR]/data/9.0
---
> data.directory=/tmp/db/
75,78c72
< data.file_name=odc.mv.db
< data.password=DC-Pass1337!
< data.user=dcuser
< data.version=5.5
---
> data.version=5.2
81c75
< database.batchinsert.maxsize=1000
---
> database.batchinsert.maxsize=3000
84d77
< ecosystem.skip.cpeanalyzer=npm
86,88d78
< hosted.suppressions.enabled=true
< hosted.suppressions.url=https://jeremylong.github.io/DependencyCheck/suppressions/publishedSuppressions.xml
< hosted.suppressions.validforhours=2
90,93c80,82
< kev.check.validforhours=24
< kev.url=https://www.cisa.gov/sites/default/files/feeds/known_exploited_vulnerabilities.json
< max.download.threads=1
< nvd.api.check.validforhours=4
---
> max.download.threads=8
> nvd.api.check.validforhours=12
> nvd.api.datafeed.startyear=2002
95c84
< nvd.api.delay=0
---
> nvd.api.delay=2000
97,99c86,89
< odc.analysis.timeout=180
< odc.application.name=${pom.name}
< odc.application.version=${pom.version}
---
> nvd.newyear.grace.period=10
> odc.analysis.timeout=30
> odc.application.name=clj-watson
> odc.application.version=5.0.0
104c94
< odc.settings.mask=.*password.*,.*token.*,.*api.key.*
---
> odc.settings.mask=.*password.*,.*token.*
seancorfield commented 4 weeks ago

I'm inclined to keep all the analyzer.*=false disabling lines -- and add the several that are missing -- but remove/revert nearly all of the other differences.

seancorfield commented 4 weeks ago

OK, PR #105 has my (unsigned!) commits for streamlining the properties file and updating the README, but not the deprecation of -d -- see PR for comments about that.

I've since opened PR #106 which has both of those changes as a single signed commit, if the changes look good (otherwise I'll continue on this new PR with whatever you think needs updating), plus I'll add whatever changes we decide we need for deprecating -d but final review and merging.

seancorfield commented 4 weeks ago

Closed via #106