Closed fmscorreia closed 5 years ago
Thanks for your contribution. Overall it looks good, but I would suggest considering to use a dynamic task for OWASP dependency check. The main reason is to remove the need to add support for every language that is supported by dependency check. In this case, I think the coupling is too high - this was the motivation for introducing the dynamic task. Also, after putting all this effort - can you add some tests? Even a basic one just to make sure it's functional. Check out Snyk/Zap test as a reference.
Hello, thanks for the feedback! I'll look into the Snyk/ZAP tests and add at least a basic one for dependency check as you suggested. I completely agree with the motivation for dynamic tasks, and will get into that as well after the tests
I've added some basic tests, I'm not very familiar with Ruby and rspec, so forgive the simplicity. The tests only check the dependency-check CLI tool, not the sbt, maven nor gradle plugins. Since a transition to a dynamic task is on the roadmap I suppose it's not much of an issue, but if necessary I'll improve the tests with more checks.
Merged - thank you!
Thanks :)
The Dependency Check task only offers support for the CLI tool and the
sbt dependencyCheck
plugin. Report analysis is not functional, as Dependency Check outputs CVSS scores that are on a 0-10 scale, with decimals, while Glue expects a 1-3 discrete scale.The contributions on this PR are as follows:
TOOLS.md
amended, the name of the task was incorrect.build.gradle
/pom.xml
, configured to output a report in XML format. Glue expects the report path to be the default of each plugin.runsystem
function inlib/glue/util.rb
. For sufficiently large outputs (e.g Maven plugin) the stdout buffer would fill up, and a deadlock ensued (see this post).docker/glue/Dockerfile
, the JDK installation was uncommented. This adds to the Docker image size, but is necessary for the Maven plugin. GPG imports were also added for Ruby, also necessary to successfully build the Docker image.Changes not directly related to this PR's objective:
docker/glue/Dockerfile
, retirejs was locked to v1.6.2. The most recent 2.x.x versions break the retirejs task, as already discussed in a previous PR -> https://github.com/OWASP/glue/pull/143