feat: Implement Yarn Berry Analyser

[segovia]

I have implemented an Analyzer to address the issue that DependencyCheck does not work with projects that use yarn berry (#4894). This is a first draft and I would be happy to implement tests and make any other modifications if there is interest to merge this to main.

[marcelstoer]

Thank you so much for this! This really simplifies our lives with multi-language projects driven by a Maven build (Java backend, JavaScript frontend). Btw, it might also fix #4215.

I think this can and should be improved to reduce future maintenance costs. Think about what happens when Yarn releases the next breaking change (like classic --> Berry)?

I would like to see a Yarn analyzer abstraction with some sort of auto-detection mechanism. IMO users should only have to set the yarnAuditAnalyzerEnabled without having to worry about the implementation. yarnAuditAnalyzerEnabled and yarnBerryAuditAnalyzerEnabled are actually mutually exclusive.

Can you extract a YarnAuditAnalyzer superclass with two subclasses for classic/v1 and Berry? Or, if they don't share enough, extract a YarnAuditAnalyzer interface with two implementations? They would need to be accompanied with a matching factory that introspects the lock file (or something else?) to instantiate the matching implementation.

/CC @chadlwilson @jeremylong

[jeremylong]

@marcelstoer completely agree - your suggestions are spot on.

[segovia]

With the updated PR, I have refactored YarnAuditAnalyzer into: AbstractYarnAuditAnalyzer, YarnClassicAuditAnalyzer and YarnBerryAuditAnalyzer, where the first contains shared analyzer code, the second is basically the same as the original YarnAuditAnalyzer and the third contains the Yarn Berry implementation.
As suggested, both analyzers share settings and are enabled or disabled with yarnAuditAnalyzerEnabled. They check the major version of yarn to determine if the Yarn Classic Analyzer should be used or the Berry, so they will run mutually exclusively.

[jeremylong]

LGTM - I left some suggestions around the yarn version number. I prefer to use something like semver4j as we already have it on the classpath.

[jeremylong]

It looks like the build failed:

 YarnBerryAuditAnalyzerIT.testAnalyzePackageYarn:46 More than 1 dependency should be identified

Do we need to update the workflow itself to include yarn berry?

[chadlwilson]

Do we need to update the workflow itself to include yarn berry?

FWIW the canonical way to run and install yarn 2+ is via corepack and source controlling yarn itself alongside a package.json which defines the version number.

https://yarnpkg.com/getting-started/install

Although you can use yarn 1 to switch dynamically to yarn 2+.projects if you have to.

Ideally the tests should test a project that has package.json etc preconfigured for yarn usage and 'corepack enable' run on the npm install. This has the advantage that specific yarn versions are source controlled alongside the tests in the .yarn folder (canonically).

[marcelstoer]

@segovia thanks for the updated version. This is much more maintainable now!

[segovia]

Thank you all for your feedback and suggestions. With my latest update, I incorporated semver4j and named the magic number 1 as YARN_CLASSIC_MAJOR_VERSION.

Concerning the integration test, I suggest putting the latest yarn berry .cjs file somewhere. In my commit, I put it in core/src/test/resources/yarn-berry-audit/.yarn/releases/yarn-4.6.0.cjs, but if you don't want the yarn .cjs to be part of the commit, it could be placed anywhere else. I am not familiar with the container running the tests, but if we add it somewhere specific there, I can reference it in the .yarnrc.yml file.

[jeremylong]

@segovia sorry about the back-and-forth on this one. Can you double-check the comparisons for the String -> 'int conversion?

[segovia]

I pushed an update in an attempt to fix the tests. Unfortunately, I am not able to run all the tests locally. In case it fails again, is there some documentation on how to run the tests locally? Or do you have any tips?

[jeremylong]

I've been trying to figure out why the test case is failing - locally I've just been using variations of this:

mvn clean verify -Dtest=org.owasp.dependencycheck.analyzer.YarnBerryAuditAnalyzerIT  -Dsurefire.failIfNoSpecifiedTests=false

In order for the tests to run on core, utils has to have been packaged in the execution.

[jeremylong]

Looking at why the tests were failing and the fix - I don't think the enable/disable function is going to work correctly in the wild. I know how to fix it - but it will take a bit of refactoring. Instead of having two separate analyzers a single analyzer should use the correct mechanism to fetchYarnAdvisories vs fetchYarnAuditJson. You'll notice in these we set the ProcessBuilder.directory so if we run --version from there we should get the correct version so we'll know which command to run.

[jeremylong]

@segovia do you want me to do the refactoring - or do you want to?

[segovia]

@jeremylong I've taken your feedback into consideration. Now we determine which method to use based on the dependency's directory. Hopefully, this is what you were thinking. In any case, if any further modifications are required, please feel free to adapt.

[jeremylong]

@segovia thank you for all your work on this. It might be another day or two before I can do a final review and get this merged.

[jeremylong]

LGTM

[jeremylong]

@segovia thanks for contributing!!!!

[segovia]

Thanks for the feedback and merging!