search

jeremylong / DependencyCheck

OWASP dependency-check is a software composition analysis utility that detects publicly disclosed vulnerabilities in application dependencies.
https://owasp.org/www-project-dependency-check/
Apache License 2.0
6.35k stars 1.27k forks source link

Configurable timeout and retry for the writelock and improve logging #4910

Open aikebah opened 2 years ago

aikebah commented 2 years ago

Is your feature request related to a problem? Yes.

While running the tests of the dependency-check-gradle plugin I was waiting without any progress feedback due to a failing writelock (likely due to a previously early-kill of a test that appeared to not progress at all. When I finally kept it running for 'forever' it gave up (failed on an exception) after 40 minutes as it could not obtain the WriteLock.

Describe the solution you'd like As the WriteLock is used for cases than only the NVD CVE database update we should make it at least development-time configurable, so that locking for RetireJS and the hosted suppressions will time-out much sooner.

Maybe even allow end-user overrides for the 'patience' of (some of the) individual locking attempts (so that users can reconfigure to a shorter timeout on their builds when a dedicated job to e.g. daily update the NVD CVE datastream ensures that only CVE modified needs to be processed so that the updates should finish much sooner).

Currently only when the user has enabled debug logging there is some information logged that an update is waiting for a lock. It would be good if there is at least once, and preferably every x minutes, an INFO-level log that the system is (still) waiting to obtain a lock so that users are aware that a lock is in the way of progressing the build (when it is the only build running on the system they know that there is a stale lock locking them out).

Describe alternatives you've considered Keep the current code so that any locking attempt with a stale lock will require the user to wait 40 minutes to see their build fail on the lock timeout exception.

aikebah commented 2 years ago

@jeremylong Would like to hear your thoughts on this, especially on the development-time-only versus user-configurable part of it.

jeremylong commented 2 years ago

The improved logging is very much needed - as I have run into this same issue when testing. As for the timeouts - there are two: 1) for the 40 minute wait and 2) for the time that ODC will ignore the write lock and just continue: https://github.com/jeremylong/DependencyCheck/blob/21cd89bdc2531632e18d0b0e085c6d85112853d0/core/src/main/java/org/owasp/dependencycheck/utils/WriteLock.java#L256-L258

Not sure why I implemented a 40 minute wait - yet if the lock is 30 minutes old we just delete it and move on... Maybe we could add a --removeLock (or maybe clearLock?) instead of making the timeouts adjustable? Thoughts?