Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add Yarn Audit analyzer #3063

Merged
merged 8 commits into from
Jan 18, 2021
Merged

feat: add Yarn Audit analyzer #3063

merged 8 commits into from
Jan 18, 2021

Conversation

ssthom
Copy link
Contributor

@ssthom ssthom commented Jan 8, 2021

Fixes Issue

Fixes: #2842
Fixes: #2814
Fixes: #2641

Description of Change

Updated the NodeAuditAnalyzer to be able to process the yarn.lock file. It uses yarn audit --verbose to get the request it sends to the Node Audit API and process that as if it was from the NPM lock file. It also uses the yarn audit groups param to be able to handle DependencyCheck's --nodeAuditSkipDevDependencies param

Have test cases been added to cover the new functionality?

yes and also tested against my local yarn project

@boring-cyborg boring-cyborg bot added core changes to core tests test cases labels Jan 8, 2021
@ssthom
Copy link
Contributor Author

ssthom commented Jan 8, 2021

@jeremylong First time developing in this repo. Let me know if this is the right approach for this?

@jeremylong
Copy link
Owner

Thanks for the PR - and there are things in here that will be used but I would like to see this re-structured a bit. When we add a reliance on an external tool we should put this into an entirely separate analyzer; so in this case like YarnAuditAnalyzer and possibly extracting out some of the duplicate code into a parent class for both yarn and node audit analyzers.

One of the main reasons is so we can test to see if the tool is installed and working in the prepareFileTypeAnalyzer() method and output an appropriate warning if it is not, such as with

throw new InitializationException(String.format("Go executable not found. Disabling %s: %s", ANALYZER_NAME, exitValue));

In this case, if no yarn.lock is in the scan then the prepareFileTypeAnalyzer() would not be called. If a yarn.lock existed and yarn was not installed we can throw the error during setup with sufficient instructions for an end user to know they need to install yarn to be able to complete the scan.

@boring-cyborg boring-cyborg bot added ant changes to ant cli changes to the cli utils changes to utils labels Jan 11, 2021
@ssthom ssthom changed the title feat: add yarn.lock Node Audit analyzer feat: add Yarn Audit analyzer Jan 11, 2021
@ssthom
Copy link
Contributor Author

ssthom commented Jan 11, 2021

@jeremylong How about this? I separated it into it's own Analyzer and moved some of the common code into AbstractNpmAnalyzer

It now prints the following to the user if Yarn isn't installed

[WARN] The Yarn Audit Analyzer has been disabled. Yarn executable was not found.

@jeremylong
Copy link
Owner

@ssthom initial review - this looks great. really appreciate such a complete PR. I will do some testing over the weekend, merge the PR, and then publish a release. You don't have to worry about this as I'll handle it (just writing it down so I have notes); update to include:

@josefschabasser
Copy link

Hi!
Sorry to interrupt you, but will this work with the docker image (owasp/dependency-check)?
Best regards!

@jeremylong
Copy link
Owner

Current branch can be found here: https://github.com/jeremylong/DependencyCheck/tree/ssthom-yarnAudit

It looks like we also need to add yarnPath - as such, it might take a little bit longer. Also, we likely need to install yarn in the docker image as well.

@jeremylong
Copy link
Owner

@ssthom testing is finding more issues... As I'm using a newer version of node then you were in testing I'm getting the following from yarn audit --offline:

error [email protected]: The engine "node" is incompatible with this module. Expected version "12.x.x". Got "15.6.0"
error Found incompatible module.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ant changes to ant cli changes to the cli core changes to core tests test cases utils changes to utils
Projects
None yet
3 participants