Skip to content

Commit ffa6ff1

Browse files
chadlwilsonchadlwilson
committed
fix: improve consistency of results from YarnAuditAnalyzer
Signed-off-by: Chad Wilson <29788154+chadlwilson@users.noreply.github.com>
1 parent 02bb473 commit ffa6ff1

19 files changed

Lines changed: 397 additions & 16542 deletions

File tree

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

Lines changed: 34 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import org.apache.commons.lang3.StringUtils;
2222
import org.json.JSONException;
2323
import org.json.JSONObject;
24+
import org.jspecify.annotations.NonNull;
2425
import org.owasp.dependencycheck.Engine;
2526
import org.owasp.dependencycheck.analyzer.exception.AnalysisException;
2627
import org.owasp.dependencycheck.analyzer.exception.UnexpectedAnalysisException;
@@ -43,8 +44,11 @@
4344
import java.nio.file.Files;
4445
import java.util.ArrayList;
4546
import java.util.List;
46-
import java.util.stream.Stream;
47+
import java.util.Map;
48+
import java.util.stream.Collectors;
4749

50+
import static org.apache.commons.lang3.StringUtils.defaultIfBlank;
51+
import static org.apache.commons.lang3.SystemUtils.getEnvironmentVariable;
4852
import static org.owasp.dependencycheck.utils.FileUtils.existsWithContent;
4953

5054
@ThreadSafe
@@ -55,12 +59,19 @@ public class YarnAuditAnalyzer extends AbstractNpmAnalyzer {
5559
*/
5660
private static final Logger LOGGER = LoggerFactory.getLogger(YarnAuditAnalyzer.class);
5761

58-
private static final int YARN_BERRY_MAJOR_VERSION_MIN = 2;
62+
/***
63+
* Minimum Yarn version supported. Only in Yarn v4 was support for the newer npm audit bulk API added, however 2.4.0
64+
* added support for `yarn npm audit` via the legacy API (deprecated, likely decommissioned in 2026). package.jsons
65+
* or environments implying a version lower than this will be ignored.
66+
*/
67+
private static final String YARN_VERSION_MIN = "2.4.0";
5968

6069
/**
6170
* The file name to scan.
6271
*/
6372
public static final String YARN_PACKAGE_LOCK = "yarn.lock";
73+
static final String YARN_ENV_IGNORE_PATH = "YARN_IGNORE_PATH";
74+
static final String YARN_ENV_ENABLE_TELEMETRY = "YARN_ENABLE_TELEMETRY";
6475

6576
/**
6677
* Filter that detects files named "yarn.lock"
@@ -101,8 +112,7 @@ public AnalysisPhase getAnalysisPhase() {
101112
*/
102113
private Semver getYarnVersion(File dependencyDirectory) {
103114
List<String> args = List.of(yarnPath, "--version");
104-
final ProcessBuilder builder = new ProcessBuilder(args);
105-
builder.directory(dependencyDirectory);
115+
final ProcessBuilder builder = createYarnBuilder(dependencyDirectory, args);
106116
try {
107117
final Process process = builder.start();
108118
try (ProcessReader processReader = new ProcessReader(process)) {
@@ -199,8 +209,8 @@ private String startAndReadStdoutToString(ProcessBuilder builder) throws Analysi
199209
}
200210

201211
/**
202-
* Analyzes the yarn lock file to determine vulnerable dependencies. Uses
203-
* yarn audit --offline to generate the payload to be sent to the NPM API.
212+
* Analyzes the yarn lock file to determine vulnerable dependencies using the Yarn CLI to talk to the npm audit
213+
* bulk API and parsed advisories in simple return format.
204214
*
205215
* @param dependency the yarn lock file
206216
* @param engine the analysis engine
@@ -217,8 +227,8 @@ protected void analyzeDependency(Dependency dependency, Engine engine) throws An
217227
}
218228
File dependencyDirectory = getDependencyDirectory(packageLock);
219229
final var yarnVersion = getYarnVersion(dependencyDirectory);
220-
if (yarnVersion.getMajor() < YARN_BERRY_MAJOR_VERSION_MIN) {
221-
LOGGER.warn("Yarn dependency skipped: {} - Yarn Classic (v{}) is not supported.", dependency.getActualFile(), yarnVersion);
230+
if (yarnVersion.isLowerThan(YARN_VERSION_MIN)) {
231+
LOGGER.warn("Yarn dependency skipped: {} - Yarn v{} (prior to v{}) is not supported.", dependency.getActualFile(), yarnVersion, YARN_VERSION_MIN);
222232
return;
223233
}
224234

@@ -229,7 +239,7 @@ protected void analyzeDependency(Dependency dependency, Engine engine) throws An
229239
List<Advisory> advisories = parseAdvisoryJsons(advisoryJsons);
230240
processResults(advisories, engine, dependency, new HashSetValuedHashMap<>());
231241
} catch (JSONException e) {
232-
throw new AnalysisException("Failed to parse the response from NPM Audit API (YarnAuditAnalyzer).", e);
242+
throw new AnalysisException("Failed to parse the advisories from `yarn npm audit` (YarnAuditAnalyzer).", e);
233243
} catch (CpeValidationException e) {
234244
throw new UnexpectedAnalysisException(e);
235245
}
@@ -257,25 +267,25 @@ private List<JSONObject> fetchYarnAdvisories(Dependency dependency, boolean skip
257267
args.add("--recursive");
258268
args.add("--no-deprecations");
259269
args.add("--json");
260-
final ProcessBuilder builder = new ProcessBuilder(args);
261-
builder.directory(getDependencyDirectory(dependency.getActualFile()));
262-
263-
final String advisoriesJsons = startAndReadStdoutToString(builder);
270+
final String advisoriesJsons = startAndReadStdoutToString(createYarnBuilder(getDependencyDirectory(dependency.getActualFile()), args));
264271

265272
LOGGER.debug("Advisories JSON: {}", advisoriesJsons);
266-
final String[] advisoriesJsonArray = Stream.of(advisoriesJsons.split("\n"))
273+
return advisoriesJsons.lines()
267274
.filter(s -> !s.isBlank())
268-
.toArray(String[]::new);
269-
try {
270-
final List<JSONObject> advisories = new ArrayList<>();
271-
for (String advisoriesJson : advisoriesJsonArray) {
272-
advisories.add(new JSONObject(advisoriesJson));
273-
}
275+
.map(JSONObject::new)
276+
.collect(Collectors.toList());
277+
}
274278

275-
return advisories;
276-
} catch (JSONException e) {
277-
throw new AnalysisException("Failed to parse the response from NPM Audit API (YarnAuditAnalyzer).", e);
278-
}
279+
private static @NonNull ProcessBuilder createYarnBuilder(File dependencyDirectory, List<String> args) {
280+
final ProcessBuilder builder = new ProcessBuilder(args).directory(dependencyDirectory);
281+
282+
builder.environment().putAll(Map.of(
283+
// Default to disable use of yarnPath
284+
YARN_ENV_IGNORE_PATH, defaultIfBlank(getEnvironmentVariable(YARN_ENV_IGNORE_PATH, null), "true"),
285+
// Force disable telemetry
286+
YARN_ENV_ENABLE_TELEMETRY, "false"
287+
));
288+
return builder;
279289
}
280290

281291
private static List<Advisory> parseAdvisoryJsons(List<JSONObject> advisoryJsons) throws JSONException {

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

Lines changed: 53 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,16 @@
1717
*/
1818
package org.owasp.dependencycheck.analyzer;
1919

20+
import org.apache.commons.lang3.SystemUtils;
2021
import org.jspecify.annotations.NonNull;
2122
import org.junit.jupiter.api.AfterEach;
2223
import org.junit.jupiter.api.BeforeEach;
2324
import org.junit.jupiter.api.Nested;
2425
import org.junit.jupiter.api.Test;
26+
import org.junit.jupiter.params.ParameterizedTest;
27+
import org.junit.jupiter.params.provider.NullSource;
28+
import org.junit.jupiter.params.provider.ValueSource;
29+
import org.mockito.MockedStatic;
2530
import org.owasp.dependencycheck.BaseTest;
2631
import org.owasp.dependencycheck.Engine;
2732
import org.owasp.dependencycheck.dependency.Dependency;
@@ -35,6 +40,7 @@
3540
import static org.junit.jupiter.api.Assertions.assertEquals;
3641
import static org.junit.jupiter.api.Assertions.assertThrows;
3742
import static org.junit.jupiter.api.Assertions.assertTrue;
43+
import static org.mockito.Mockito.mockStatic;
3844

3945
class YarnAuditAnalyzerIT extends BaseTest {
4046

@@ -55,32 +61,63 @@ void cleanup() {
5561
}
5662

5763
@Nested
58-
class Classic {
64+
class YarnUnsupported {
5965
@Test
60-
void testAnalyzePackageYarnClassic() throws Exception {
66+
void testYarnClassicUnsupported() throws Exception {
6167
final Dependency toScan = new Dependency(BaseTest.getResourceAsFile(YarnAuditAnalyzerIT.this, "yarn/yarn-classic-audit/yarn.lock"));
6268
analyzer.analyze(toScan, engine);
6369
assertEquals(0, engine.getDependencies().length, "No dependencies should be identified");
6470
}
65-
}
6671

67-
@Nested
68-
class Berry {
6972
@Test
70-
void testAnalyzePackage() throws Exception {
71-
testAnalyzeForUglifyJs("yarn/yarn-berry-audit/yarn.lock");
73+
void testYarnBerryUnsupported() throws Exception {
74+
final Dependency toScan = new Dependency(BaseTest.getResourceAsFile(YarnAuditAnalyzerIT.this, "yarn/yarn-berry-audit-unsupported/yarn.lock"));
75+
analyzer.analyze(toScan, engine);
76+
assertEquals(0, engine.getDependencies().length, "No dependencies should be identified");
7277
}
78+
}
7379

80+
@Nested
81+
class YarnConfiguration {
7482
@Test
7583
void testAnalyzeWithBadYarnConfiguration() {
7684
IllegalStateException exception = assertThrows(IllegalStateException.class, () -> testAnalyzeForUglifyJs("yarn/yarn-berry-audit-bad-yarnrc/yarn.lock"));
7785
assertThat(exception.getMessage(), containsString("Unable to determine yarn version"));
7886
assertThat(exception.getCause().getMessage(), allOf(
7987
containsString("exit value 1"),
80-
containsString("bad-path-to-yarn.js")
88+
containsString("Couldn't parse \"bad-value\" as a boolean")
8189
));
8290
}
8391

92+
@ParameterizedTest
93+
@NullSource
94+
@ValueSource(strings = {" ", "1", "true"})
95+
void testAnalyzeIgnoresBadYarnPath(String envValue) throws Exception {
96+
try (MockedStatic<SystemUtils> systemMock = mockStatic(SystemUtils.class)) {
97+
systemMock.when(() -> SystemUtils.getEnvironmentVariable("YARN_IGNORE_PATH", null)).thenReturn(envValue);
98+
99+
final Dependency toScan = new Dependency(BaseTest.getResourceAsFile(YarnAuditAnalyzerIT.this, "yarn/yarn-berry-audit-bad-path/yarn.lock"));
100+
analyzer.analyze(toScan, engine);
101+
assertEquals(0, engine.getDependencies().length, "No dependency should be identified");
102+
}
103+
}
104+
105+
@ParameterizedTest
106+
@ValueSource(strings = {"0", "false"})
107+
void testAnalyzeAllowsYarnPath(String envValue) {
108+
try (MockedStatic<SystemUtils> systemMock = mockStatic(SystemUtils.class)) {
109+
systemMock.when(() -> SystemUtils.getEnvironmentVariable("YARN_IGNORE_PATH", null)).thenReturn(envValue);
110+
111+
final Dependency toScan = new Dependency(BaseTest.getResourceAsFile(YarnAuditAnalyzerIT.this, "yarn/yarn-berry-audit-bad-path/yarn.lock"));
112+
IllegalStateException exception = assertThrows(IllegalStateException.class, () -> analyzer.analyze(toScan, engine));
113+
assertThat(exception.getMessage(), containsString("Unable to determine yarn version"));
114+
assertThat(exception.getCause().getMessage(), allOf(
115+
containsString("no such file or directory"), // yarnrc yarnPath points to non-existent path so we can detect usage
116+
containsString("does-not-exist/yarn.js")
117+
));
118+
}
119+
}
120+
84121
@Test
85122
void testAnalyzeWithBadPackageManagerConfiguration() {
86123
IllegalStateException exception = assertThrows(IllegalStateException.class, () -> testAnalyzeForUglifyJs("yarn/yarn-berry-audit-bad-package-manager/yarn.lock"));
@@ -90,6 +127,14 @@ void testAnalyzeWithBadPackageManagerConfiguration() {
90127
containsString("4.999.0-bad-version")
91128
));
92129
}
130+
}
131+
132+
@Nested
133+
class SuccessfulAnalysis {
134+
@Test
135+
void testAnalyzePackage() throws Exception {
136+
testAnalyzeForUglifyJs("yarn/yarn-berry-audit/yarn.lock");
137+
}
93138

94139
@Test
95140
void testAnalyzePackageNoVulnerability() throws Exception {

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
11
enableGlobalCache: true
22

3-
enableTelemetry: false
4-
53
nodeLinker: node-modules

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
enableGlobalCache: true
2+
3+
nodeLinker: node-modules
4+
5+
yarnPath: does-not-exist/yarn.js

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
{
2+
"name": "owasp-nodejs-goat",
3+
"private": true,
4+
"version": "1.3.0",
5+
"description": "A tool to learn OWASP Top 10 for node.js developers",
6+
"main": "server.js",
7+
"comments": {
8+
"//": "a9 insecure components"
9+
},
10+
"scripts": {
11+
"start": "node server.js",
12+
"test": "node node_modules/grunt-cli/bin/grunt test",
13+
"db:seed": "grunt db-reset",
14+
"precommit": "grunt precommit"
15+
},
16+
"repository": "https://github.com/OWASP/NodejsGoat",
17+
"license": "Apache 2.0",
18+
"packageManager": "yarn@4.13.0"
19+
}

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
# This file is generated by running "yarn install" inside your project.
2+
# Manual changes might be lost - proceed with caution!
3+
4+
__metadata:
5+
version: 8
6+
cacheKey: 10c0
7+
8+
"owasp-nodejs-goat@workspace:.":
9+
version: 0.0.0-use.local
10+
resolution: "owasp-nodejs-goat@workspace:."
11+
languageName: unknown
12+
linkType: soft

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

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,3 @@
1-
enableGlobalCache: true
2-
3-
enableTelemetry: false
1+
enableGlobalCache: "bad-value"
42

53
nodeLinker: node-modules
6-
7-
yarnPath: bad-path-to-yarn.js

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
11
enableGlobalCache: true
22

3-
enableTelemetry: false
4-
53
nodeLinker: node-modules

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
11
enableGlobalCache: true
22

3-
enableTelemetry: false
4-
53
nodeLinker: node-modules

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
enableGlobalCache: true
2+
3+
nodeLinker: node-modules

0 commit comments

Comments
 (0)