fix: improve consistency of results from YarnAuditAnalyzer

Signed-off-by: Chad Wilson <29788154+chadlwilson@users.noreply.github.com>

Author: chadlwilson

core/src/main/java/org/owasp/dependencycheck/analyzer/YarnAuditAnalyzer.java

@@ -21,6 +21,7 @@
 import org.apache.commons.lang3.StringUtils;
 import org.json.JSONException;
 import org.json.JSONObject;
+import org.jspecify.annotations.NonNull;
 import org.owasp.dependencycheck.Engine;
 import org.owasp.dependencycheck.analyzer.exception.AnalysisException;
 import org.owasp.dependencycheck.analyzer.exception.UnexpectedAnalysisException;
@@ -42,9 +43,14 @@
 import java.io.IOException;
 import java.nio.file.Files;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
+import static org.apache.commons.lang3.StringUtils.defaultIfBlank;
+import static org.apache.commons.lang3.SystemUtils.getEnvironmentVariable;
 import static org.owasp.dependencycheck.utils.FileUtils.existsWithContent;
 
 @ThreadSafe
@@ -55,12 +61,17 @@ public class YarnAuditAnalyzer extends AbstractNpmAnalyzer {
      */
     private static final Logger LOGGER = LoggerFactory.getLogger(YarnAuditAnalyzer.class);
 
-    private static final int YARN_BERRY_MAJOR_VERSION_MIN = 2;
+    /***
+     * Minimum Yarn major version supported. Only in Yarn v4 was support for the npm audit bulk API added.
+     */
+    private static final int YARN_MAJOR_VERSION_MIN = 4;
 
     /**
      * The file name to scan.
      */
     public static final String YARN_PACKAGE_LOCK = "yarn.lock";
+    static final String YARN_ENV_IGNORE_PATH = "YARN_IGNORE_PATH";
+    static final String YARN_ENV_ENABLE_TELEMETRY = "YARN_ENABLE_TELEMETRY";
 
     /**
      * Filter that detects files named "yarn.lock"
@@ -101,8 +112,7 @@ public AnalysisPhase getAnalysisPhase() {
      */
     private Semver getYarnVersion(File dependencyDirectory) {
         List<String> args = List.of(yarnPath, "--version");
-        final ProcessBuilder builder = new ProcessBuilder(args);
-        builder.directory(dependencyDirectory);
+        final ProcessBuilder builder = createYarnBuilder(dependencyDirectory, args);
         try {
             final Process process = builder.start();
             try (ProcessReader processReader = new ProcessReader(process)) {
@@ -217,8 +227,8 @@ protected void analyzeDependency(Dependency dependency, Engine engine) throws An
         }
         File dependencyDirectory = getDependencyDirectory(packageLock);
         final var yarnVersion = getYarnVersion(dependencyDirectory);
-        if (yarnVersion.getMajor() < YARN_BERRY_MAJOR_VERSION_MIN) {
-            LOGGER.warn("Yarn dependency skipped: {} - Yarn Classic (v{}) is not supported.", dependency.getActualFile(), yarnVersion);
+        if (yarnVersion.getMajor() < YARN_MAJOR_VERSION_MIN) {
+            LOGGER.warn("Yarn dependency skipped: {} - Yarn v{} (prior to v{}) is not supported.", dependency.getActualFile(), yarnVersion, YARN_MAJOR_VERSION_MIN);
             return;
         }
 
@@ -229,7 +239,7 @@ protected void analyzeDependency(Dependency dependency, Engine engine) throws An
             List<Advisory> advisories = parseAdvisoryJsons(advisoryJsons);
             processResults(advisories, engine, dependency, new HashSetValuedHashMap<>());
         } catch (JSONException e) {
-            throw new AnalysisException("Failed to parse the response from NPM Audit API (YarnAuditAnalyzer).", e);
+            throw new AnalysisException("Failed to parse the advisories from `yarn npm audit` (YarnAuditAnalyzer).", e);
         } catch (CpeValidationException e) {
             throw new UnexpectedAnalysisException(e);
         }
@@ -257,25 +267,26 @@ private List<JSONObject> fetchYarnAdvisories(Dependency dependency, boolean skip
         args.add("--recursive");
         args.add("--no-deprecations");
         args.add("--json");
-        final ProcessBuilder builder = new ProcessBuilder(args);
-        builder.directory(getDependencyDirectory(dependency.getActualFile()));
-
-        final String advisoriesJsons = startAndReadStdoutToString(builder);
+        final String advisoriesJsons = startAndReadStdoutToString(createYarnBuilder(getDependencyDirectory(dependency.getActualFile()), args));
 
         LOGGER.debug("Advisories JSON: {}", advisoriesJsons);
-        final String[] advisoriesJsonArray = Stream.of(advisoriesJsons.split("\n"))
+        return Arrays.stream(Stream.of(advisoriesJsons.split("\n"))
                 .filter(s -> !s.isBlank())
-                .toArray(String[]::new);
-        try {
-            final List<JSONObject> advisories = new ArrayList<>();
-            for (String advisoriesJson : advisoriesJsonArray) {
-                advisories.add(new JSONObject(advisoriesJson));
-            }
+                .toArray(String[]::new))
+                .map(JSONObject::new)
+                .collect(Collectors.toList());
+    }
 
-            return advisories;
-        } catch (JSONException e) {
-            throw new AnalysisException("Failed to parse the response from NPM Audit API (YarnAuditAnalyzer).", e);
-        }
+    private static @NonNull ProcessBuilder createYarnBuilder(File dependencyDirectory, List<String> args) {
+        final ProcessBuilder builder = new ProcessBuilder(args).directory(dependencyDirectory);
+
+        builder.environment().putAll(Map.of(
+                // Default to disable use of yarnPath
+                YARN_ENV_IGNORE_PATH, defaultIfBlank(getEnvironmentVariable(YARN_ENV_IGNORE_PATH, null), "true"),
+                // Force disable telemetry
+                YARN_ENV_ENABLE_TELEMETRY, "false"
+        ));
+        return builder;
     }
 
     private static List<Advisory> parseAdvisoryJsons(List<JSONObject> advisoryJsons) throws JSONException {

core/src/test/java/org/owasp/dependencycheck/analyzer/YarnAuditAnalyzerIT.java

@@ -17,11 +17,16 @@
  */
 package org.owasp.dependencycheck.analyzer;
 
+import org.apache.commons.lang3.SystemUtils;
 import org.jspecify.annotations.NonNull;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Nested;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.NullSource;
+import org.junit.jupiter.params.provider.ValueSource;
+import org.mockito.MockedStatic;
 import org.owasp.dependencycheck.BaseTest;
 import org.owasp.dependencycheck.Engine;
 import org.owasp.dependencycheck.dependency.Dependency;
@@ -35,6 +40,7 @@
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.Mockito.mockStatic;
 
 class YarnAuditAnalyzerIT extends BaseTest {
 
@@ -55,13 +61,20 @@ void cleanup() {
     }
 
     @Nested
-    class Classic {
+    class UnsupportedYarnVersion {
         @Test
         void testAnalyzePackageYarnClassic() throws Exception {
             final Dependency toScan = new Dependency(BaseTest.getResourceAsFile(YarnAuditAnalyzerIT.this, "yarn/yarn-classic-audit/yarn.lock"));
             analyzer.analyze(toScan, engine);
             assertEquals(0, engine.getDependencies().length, "No dependencies should be identified");
         }
+
+        @Test
+        void testAnalyzePackageYarnBerry() throws Exception {
+            final Dependency toScan = new Dependency(BaseTest.getResourceAsFile(YarnAuditAnalyzerIT.this, "yarn/yarn-berry-audit-unsupported/yarn.lock"));
+            analyzer.analyze(toScan, engine);
+            assertEquals(0, engine.getDependencies().length, "No dependencies should be identified");
+        }
     }
 
     @Nested
@@ -77,10 +90,39 @@ void testAnalyzeWithBadYarnConfiguration() {
             assertThat(exception.getMessage(), containsString("Unable to determine yarn version"));
             assertThat(exception.getCause().getMessage(), allOf(
                     containsString("exit value 1"),
-                    containsString("bad-path-to-yarn.js")
+                    containsString("Couldn't parse \"bad-value\" as a boolean")
             ));
         }
 
+        @ParameterizedTest
+        @NullSource
+        @ValueSource(strings = {" ", "1", "true"})
+        void testAnalyzeIgnoresBadYarnPath(String envValue) throws Exception {
+            try (MockedStatic<SystemUtils> systemMock = mockStatic(SystemUtils.class)) {
+                systemMock.when(() -> SystemUtils.getEnvironmentVariable("YARN_IGNORE_PATH", null)).thenReturn(envValue);
+
+                final Dependency toScan = new Dependency(BaseTest.getResourceAsFile(YarnAuditAnalyzerIT.this, "yarn/yarn-berry-audit-bad-path/yarn.lock"));
+                analyzer.analyze(toScan, engine);
+                assertEquals(0, engine.getDependencies().length, "No dependency should be identified");
+            }
+        }
+
+        @ParameterizedTest
+        @ValueSource(strings = {"0", "false"})
+        void testAnalyzeAllowsYarnPath(String envValue) {
+            try (MockedStatic<SystemUtils> systemMock = mockStatic(SystemUtils.class)) {
+                systemMock.when(() -> SystemUtils.getEnvironmentVariable("YARN_IGNORE_PATH", null)).thenReturn(envValue);
+
+                final Dependency toScan = new Dependency(BaseTest.getResourceAsFile(YarnAuditAnalyzerIT.this, "yarn/yarn-berry-audit-bad-path/yarn.lock"));
+                IllegalStateException exception = assertThrows(IllegalStateException.class, () -> analyzer.analyze(toScan, engine));
+                assertThat(exception.getMessage(), containsString("Unable to determine yarn version"));
+                assertThat(exception.getCause().getMessage(), allOf(
+                        containsString("no such file or directory"), // yarnrc yarnPath points to non-existent path so we can detect usage
+                        containsString("does-not-exist/yarn.js")
+                ));
+            }
+        }
+
         @Test
         void testAnalyzeWithBadPackageManagerConfiguration() {
             IllegalStateException exception = assertThrows(IllegalStateException.class, () -> testAnalyzeForUglifyJs("yarn/yarn-berry-audit-bad-package-manager/yarn.lock"));

core/src/test/resources/yarn/yarn-berry-audit-bad-package-manager/.yarnrc.yml

@@ -1,5 +1,3 @@
 enableGlobalCache: true
 
-enableTelemetry: false
-
 nodeLinker: node-modules

core/src/test/resources/yarn/yarn-berry-audit-bad-path/.yarnrc.yml

@@ -0,0 +1,5 @@
+enableGlobalCache: true
+
+nodeLinker: node-modules
+
+yarnPath: does-not-exist/yarn.js

core/src/test/resources/yarn/yarn-berry-audit-bad-path/package.json

@@ -0,0 +1,19 @@
+{
+  "name": "owasp-nodejs-goat",
+  "private": true,
+  "version": "1.3.0",
+  "description": "A tool to learn OWASP Top 10 for node.js developers",
+  "main": "server.js",
+  "comments": {
+    "//": "a9 insecure components"
+  },
+  "scripts": {
+    "start": "node server.js",
+    "test": "node node_modules/grunt-cli/bin/grunt test",
+    "db:seed": "grunt db-reset",
+    "precommit": "grunt precommit"
+  },
+  "repository": "https://github.com/OWASP/NodejsGoat",
+  "license": "Apache 2.0",
+  "packageManager": "yarn@4.13.0"
+}

core/src/test/resources/yarn/yarn-berry-audit-bad-path/yarn.lock

@@ -0,0 +1,12 @@
+# This file is generated by running "yarn install" inside your project.
+# Manual changes might be lost - proceed with caution!
+
+__metadata:
+  version: 8
+  cacheKey: 10c0
+
+"owasp-nodejs-goat@workspace:.":
+  version: 0.0.0-use.local
+  resolution: "owasp-nodejs-goat@workspace:."
+  languageName: unknown
+  linkType: soft

core/src/test/resources/yarn/yarn-berry-audit-bad-yarnrc/.yarnrc.yml

@@ -1,7 +1,3 @@
-enableGlobalCache: true
-
-enableTelemetry: false
+enableGlobalCache: "bad-value"
 
 nodeLinker: node-modules
-
-yarnPath: bad-path-to-yarn.js

core/src/test/resources/yarn/yarn-berry-audit-no-deprecations/.yarnrc.yml

@@ -1,5 +1,3 @@
 enableGlobalCache: true
 
-enableTelemetry: false
-
 nodeLinker: node-modules

core/src/test/resources/yarn/yarn-berry-audit-no-vulnerability/.yarnrc.yml

@@ -1,5 +1,3 @@
 enableGlobalCache: true
 
-enableTelemetry: false
-
 nodeLinker: node-modules

core/src/test/resources/yarn/yarn-berry-audit-unsupported/.yarnrc.yml

@@ -0,0 +1,3 @@
+enableGlobalCache: true
+
+nodeLinker: node-modules