Skip to content

Commit ed9c718

Browse files
authored
fix: exclude deprecations from Yarn Berry audit results (#8380)
Signed-off-by: Chad Wilson <[email protected]>
1 parent 20a7f43 commit ed9c718

File tree

29 files changed

+334
-2026
lines changed

29 files changed

+334
-2026
lines changed

.github/workflows/build-pull-requests.yml

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,10 @@ jobs:
4343
check-latest: true
4444
cache: 'maven'
4545
cache-dependency-path: '**/pom.xml'
46-
- uses: pnpm/action-setup@fc06bc1257f339d1d5d8b3a19a8cae5388b55320 # v5.0.0
46+
- uses: actions/setup-node@53b83947a5a98c8d113130e565377fae1a50d02f # v6.3.0
4747
with:
48-
version: 6.0.2
48+
node-version: 24
49+
- run: npm install -g corepack && corepack enable
4950
- name: Build/Test with Maven
5051
id: build
5152
run: >
@@ -132,9 +133,10 @@ jobs:
132133
check-latest: true
133134
cache: 'maven'
134135
cache-dependency-path: '**/pom.xml'
135-
- uses: pnpm/action-setup@fc06bc1257f339d1d5d8b3a19a8cae5388b55320 # v5.0.0
136+
- uses: actions/setup-node@53b83947a5a98c8d113130e565377fae1a50d02f # v6.3.0
136137
with:
137-
version: 6.0.2
138+
node-version: 24
139+
- run: npm install -g corepack && corepack enable
138140
- name: Regression Test Maven Plugin
139141
id: build
140142
env:

.github/workflows/build-release.yml

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,9 +54,10 @@ jobs:
5454
server-id: central
5555
server-username: ${{ secrets.CENTRAL_USER }}
5656
server-password: ${{ secrets.CENTRAL_PASSWORD }}
57-
- uses: pnpm/action-setup@fc06bc1257f339d1d5d8b3a19a8cae5388b55320 # v5.0.0
57+
- uses: actions/setup-node@53b83947a5a98c8d113130e565377fae1a50d02f # v6.3.0
5858
with:
59-
version: 6.0.2
59+
node-version: 24
60+
- run: npm install -g corepack && corepack enable
6061
- name: Configure Git user
6162
run: |
6263
git config user.email "[email protected]"

.github/workflows/build.yml

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,9 +59,10 @@ jobs:
5959
server-id: central
6060
server-username: ${{ secrets.CENTRAL_USER }}
6161
server-password: ${{ secrets.CENTRAL_PASSWORD }}
62-
- uses: pnpm/action-setup@fc06bc1257f339d1d5d8b3a19a8cae5388b55320 # v5.0.0
62+
- uses: actions/setup-node@53b83947a5a98c8d113130e565377fae1a50d02f # v6.3.0
6363
with:
64-
version: 6.0.2
64+
node-version: 24
65+
- run: npm install -g corepack && corepack enable
6566
- name: Build/Test Snapshot with Maven${{ steps.install-gpg-key.outcome == 'success' && ' (then Deploy)' || '' }}
6667
id: build-snapshot
6768
env:

.github/workflows/false-positive-approvals.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ jobs:
2525
- uses: actions/checkout@v6
2626
with:
2727
ref: generatedSuppressions
28-
- uses: actions/[email protected]
28+
- uses: actions/setup-node@53b83947a5a98c8d113130e565377fae1a50d02f # v6.3.0
2929
- run: |
3030
npm install [email protected]
3131
npm install fs

.github/workflows/false-positive-ops.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ jobs:
4141
with:
4242
issue-body: ${{ github.event.issue.body }}
4343
template-path: odc/.github/ISSUE_TEMPLATE/false-positive-report.yml
44-
- uses: actions/[email protected]
44+
- uses: actions/setup-node@53b83947a5a98c8d113130e565377fae1a50d02f # v6.3.0
4545
with:
4646
node-version: 14
4747
- name: Initialize npm

.github/workflows/publish-suppressions.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ jobs:
1515
- uses: actions/checkout@v6
1616
with:
1717
ref: generatedSuppressions
18-
- uses: actions/[email protected]
18+
- uses: actions/setup-node@53b83947a5a98c8d113130e565377fae1a50d02f # v6.3.0
1919
- run: |
2020
npm install fs
2121
- name: Create Generated Suppressions XML

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

Lines changed: 52 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -114,42 +114,31 @@ public AnalysisPhase getAnalysisPhase() {
114114
}
115115

116116
/**
117-
* Extracts the major version from a version string.
117+
* Determines the Yarn major version implied by the metadata in the passed directory.
118118
*
119-
* @param dependency the dependency to extract the yarn version from
120-
* @return the major version (e.g., `4` from "4.2.1")
119+
* @param dependencyDirectory The directory containing the lockfile and/or package.json
120+
* @return the yarn version detected
121121
*/
122-
private int getYarnMajorVersion(Dependency dependency) {
123-
final var yarnVersion = getYarnVersion(dependency);
124-
try {
125-
final var semver = Semver.coerce(yarnVersion);
126-
return semver.getMajor();
127-
} catch (SemverException e) {
128-
throw new IllegalStateException("Invalid version string format", e);
129-
}
130-
}
131-
132-
private String getYarnVersion(Dependency dependency) {
133-
final List<String> args = new ArrayList<>();
134-
args.add(getYarn());
135-
args.add("--version");
122+
private Semver getYarnVersion(File dependencyDirectory) {
123+
List<String> args = List.of(yarnPath, "--version");
136124
final ProcessBuilder builder = new ProcessBuilder(args);
137-
builder.directory(getDependencyDirectory(dependency));
138-
LOGGER.debug("Launching: {}", args);
125+
builder.directory(dependencyDirectory);
139126
try {
140127
final Process process = builder.start();
141128
try (ProcessReader processReader = new ProcessReader(process)) {
142129
processReader.readAll();
143130
final int exitValue = process.waitFor();
131+
final var yarnVersion = StringUtils.trimToEmpty(processReader.getOutput());
144132
if (exitValue != 0) {
145-
throw new IllegalStateException("Unable to determine yarn version, unexpected response.");
133+
throw new IllegalStateException(String.format("Unable to determine yarn version, unexpected response (exit value %s, output: %s, error: %s)", exitValue, yarnVersion, processReader.getError()));
146134
}
147-
final var yarnVersion = processReader.getOutput();
148135
if (StringUtils.isBlank(yarnVersion)) {
149136
throw new IllegalStateException("Unable to determine yarn version, blank output.");
150137
}
151-
return yarnVersion;
138+
return Semver.coerce(yarnVersion);
152139
}
140+
} catch (SemverException e) {
141+
throw new IllegalStateException("Invalid version string format", e);
153142
} catch (Exception ex) {
154143
throw new IllegalStateException("Unable to determine yarn version.", ex);
155144
}
@@ -169,62 +158,35 @@ protected void prepareFileTypeAnalyzer(Engine engine) throws InitializationExcep
169158
LOGGER.debug("{} Analyzer is disabled skipping yarn executable check", getName());
170159
return;
171160
}
172-
final List<String> args = new ArrayList<>();
173-
args.add(getYarn());
174-
args.add("--help");
175-
final ProcessBuilder builder = new ProcessBuilder(args);
176-
LOGGER.debug("Launching: {}", args);
177161
try {
178-
final Process process = builder.start();
179-
try (ProcessReader processReader = new ProcessReader(process)) {
180-
processReader.readAll();
181-
final int exitValue = process.waitFor();
182-
final int expectedExitValue = 0;
183-
final int yarnExecutableNotFoundExitValue = 127;
184-
switch (exitValue) {
185-
case expectedExitValue:
186-
LOGGER.debug("{} is enabled.", getName());
187-
break;
188-
case yarnExecutableNotFoundExitValue:
189-
default:
190-
this.setEnabled(false);
191-
LOGGER.warn("The {} has been disabled after receiving exit value {}. Yarn executable was not " +
192-
"found or received a non-zero exit value.", getName(), exitValue);
193-
}
194-
}
195-
} catch (Exception ex) {
162+
cacheYarnCommandPath();
163+
getYarnVersion(new File("."));
164+
} catch (Exception ex){
196165
this.setEnabled(false);
197-
LOGGER.warn("The {} has been disabled after receiving an exception. This can occur when Yarn executable " +
198-
"is not found.", getName());
199-
throw new InitializationException("Unable to read yarn audit output.", ex);
166+
LOGGER.warn("The {} has been disabled after failing to find yarn. Yarn executable was not " +
167+
"found or received a non-zero exit value: {}", getName(), ex.getMessage());
168+
throw new InitializationException("Unable to determine yarn executable to use.", ex);
200169
}
201170
}
202171

203172
/**
204-
* Attempts to determine the path to `yarn`.
205-
*
206-
* @return the path to `yarn`
173+
* Attempts to determine and cache the path to `yarn`.
207174
*/
208-
private String getYarn() {
209-
final String value;
210-
synchronized (this) {
211-
if (yarnPath == null) {
212-
final String path = getSettings().getString(Settings.KEYS.ANALYZER_YARN_PATH);
213-
if (path == null) {
214-
yarnPath = "yarn";
215-
} else {
216-
final File yarnFile = new File(path);
217-
if (yarnFile.isFile()) {
218-
yarnPath = yarnFile.getAbsolutePath();
219-
} else {
220-
LOGGER.warn("Provided path to `yarn` executable is invalid.");
221-
yarnPath = "yarn";
222-
}
223-
}
175+
private void cacheYarnCommandPath() {
176+
String value = getSettings().getString(Settings.KEYS.ANALYZER_YARN_PATH);
177+
if (value == null || value.isBlank()) {
178+
value = "yarn";
179+
} else {
180+
File fileValue = new File(value);
181+
if (fileValue.isFile()) {
182+
value = fileValue.getAbsolutePath();
183+
} else {
184+
LOGGER.warn("Provided path to `yarn` executable is invalid; defaulting to `yarn`.");
185+
value = "yarn";
224186
}
225-
value = yarnPath;
226187
}
227-
return value;
188+
189+
yarnPath = value;
228190
}
229191

230192
/**
@@ -247,7 +209,7 @@ private String startAndReadStdoutToString(ProcessBuilder builder) throws Analysi
247209
LOGGER.debug("Process Error Out: {}", errOutput);
248210
LOGGER.debug("Process Out: {}", processReader.getOutput());
249211
}
250-
return new String(Files.readAllBytes(tmpFile.toPath()), StandardCharsets.UTF_8);
212+
return Files.readString(tmpFile.toPath());
251213
} catch (InterruptedException ex) {
252214
Thread.currentThread().interrupt();
253215
throw new AnalysisException("Yarn audit process was interrupted.", ex);
@@ -274,16 +236,16 @@ protected void analyzeDependency(Dependency dependency, Engine engine) throws An
274236
if (!packageLock.isFile() || packageLock.length() == 0 || !shouldProcess(packageLock)) {
275237
return;
276238
}
277-
final File packageJson = new File(packageLock.getParentFile(), "package.json");
239+
File dependencyDirectory = getDependencyDirectory(packageLock);
240+
final var yarnVersion = getYarnVersion(dependencyDirectory);
278241
final List<Advisory> advisories;
279242
final MultiValuedMap<String, String> dependencyMap = new HashSetValuedHashMap<>();
280-
final var yarnMajorVersion = getYarnMajorVersion(dependency);
281-
if (YARN_CLASSIC_MAJOR_VERSION < yarnMajorVersion) {
282-
LOGGER.info("Analyzing using Yarn Berry audit");
243+
if (YARN_CLASSIC_MAJOR_VERSION < yarnVersion.getMajor()) {
244+
LOGGER.info("Analyzing using Yarn Berry ({}) audit for {}", yarnVersion, dependency.getActualFilePath());
283245
advisories = analyzePackageWithYarnBerry(dependency);
284246
} else {
285-
LOGGER.info("Analyzing using Yarn Classic audit");
286-
advisories = analyzePackageWithYarnClassic(packageLock, packageJson, dependency, dependencyMap);
247+
LOGGER.info("Analyzing using Yarn Classic ({}) audit for {}", yarnVersion, dependency.getActualFilePath());
248+
advisories = analyzePackageWithYarnClassic(packageLock, dependency, dependencyMap);
287249
}
288250
try {
289251
processResults(advisories, engine, dependency, dependencyMap);
@@ -292,9 +254,9 @@ protected void analyzeDependency(Dependency dependency, Engine engine) throws An
292254
}
293255
}
294256

295-
private JsonObject fetchYarnAuditJson(Dependency dependency, boolean skipDevDependencies) throws AnalysisException {
257+
private JsonObject fetchYarnAuditJson(File dependencyDirectory, boolean skipDevDependencies) throws AnalysisException {
296258
final List<String> args = new ArrayList<>();
297-
args.add(getYarn());
259+
args.add(yarnPath);
298260
args.add("audit");
299261
//offline audit is not supported - but the audit request is generated in the verbose output
300262
args.add("--offline");
@@ -305,13 +267,14 @@ private JsonObject fetchYarnAuditJson(Dependency dependency, boolean skipDevDepe
305267
args.add("--json");
306268
args.add("--verbose");
307269
final ProcessBuilder builder = new ProcessBuilder(args);
308-
builder.directory(getDependencyDirectory(dependency));
270+
builder.directory(dependencyDirectory);
309271
LOGGER.debug("Launching: {}", args);
310272

311273
final String verboseJson = startAndReadStdoutToString(builder);
312274
final String auditRequestJson = Arrays.stream(verboseJson.split("\n"))
313275
.filter(line -> line.contains("Audit Request"))
314-
.findFirst().get();
276+
.findFirst()
277+
.orElseThrow(() -> new AnalysisException("No results from Yarn Classic (offline step) - possibly trying to use classic analyzer on Yarn Berry lockfile"));
315278
String auditRequest;
316279
try (JsonReader reader = Json.createReader(IOUtils.toInputStream(auditRequestJson, StandardCharsets.UTF_8))) {
317280
final JsonObject jsonObject = reader.readObject();
@@ -323,8 +286,8 @@ private JsonObject fetchYarnAuditJson(Dependency dependency, boolean skipDevDepe
323286
return Json.createReader(IOUtils.toInputStream(auditRequest, StandardCharsets.UTF_8)).readObject();
324287
}
325288

326-
private static File getDependencyDirectory(Dependency dependency) {
327-
final File folder = dependency.getActualFile().getParentFile();
289+
private static File getDependencyDirectory(File lockFile) {
290+
final File folder = lockFile.getParentFile();
328291
if (!folder.isDirectory()) {
329292
throw new IllegalArgumentException(String.format("%s should have been a directory.", folder.getAbsolutePath()));
330293
}
@@ -337,7 +300,6 @@ private static File getDependencyDirectory(Dependency dependency) {
337300
* submitting the payload, and returning the identified advisories.
338301
*
339302
* @param lockFile a reference to the package-lock.json
340-
* @param packageFile a reference to the package.json
341303
* @param dependency a reference to the dependency-object for the yarn.lock
342304
* @param dependencyMap a collection of module/version pairs; during
343305
* creation of the payload the dependency map is populated with the
@@ -346,16 +308,16 @@ private static File getDependencyDirectory(Dependency dependency) {
346308
* @throws AnalysisException thrown when there is an error creating or
347309
* submitting the npm audit API payload
348310
*/
349-
private List<Advisory> analyzePackageWithYarnClassic(final File lockFile, final File packageFile,
350-
Dependency dependency, MultiValuedMap<String, String> dependencyMap)
311+
private List<Advisory> analyzePackageWithYarnClassic(final File lockFile, Dependency dependency,
312+
MultiValuedMap<String, String> dependencyMap)
351313
throws AnalysisException {
352314
try {
353315
final boolean skipDevDependencies = getSettings().getBoolean(Settings.KEYS.ANALYZER_NODE_AUDIT_SKIPDEV, false);
354316
// Retrieves the contents of package-lock.json from the Dependency
355-
final JsonObject lockJson = fetchYarnAuditJson(dependency, skipDevDependencies);
317+
final JsonObject lockJson = fetchYarnAuditJson(getDependencyDirectory(lockFile), skipDevDependencies);
356318
// Retrieves the contents of package-lock.json from the Dependency
357319
final JsonObject packageJson;
358-
try (JsonReader packageReader = Json.createReader(Files.newInputStream(packageFile.toPath()))) {
320+
try (JsonReader packageReader = Json.createReader(Files.newInputStream(lockFile.getParentFile().toPath().resolve("package.json")))) {
359321
packageJson = packageReader.readObject();
360322
}
361323
// Modify the payload to meet the NPM Audit API requirements
@@ -385,7 +347,7 @@ private List<Advisory> analyzePackageWithYarnClassic(final File lockFile, final
385347
private List<JSONObject> fetchYarnAdvisories(Dependency dependency, boolean skipDevDependencies) throws AnalysisException {
386348
final List<String> args = new ArrayList<>();
387349

388-
args.add(getYarn());
350+
args.add(yarnPath);
389351
args.add("npm");
390352
args.add("audit");
391353
if (skipDevDependencies) {
@@ -394,9 +356,10 @@ private List<JSONObject> fetchYarnAdvisories(Dependency dependency, boolean skip
394356
}
395357
args.add("--all");
396358
args.add("--recursive");
359+
args.add("--no-deprecations");
397360
args.add("--json");
398361
final ProcessBuilder builder = new ProcessBuilder(args);
399-
builder.directory(getDependencyDirectory(dependency));
362+
builder.directory(getDependencyDirectory(dependency.getActualFile()));
400363

401364
final String advisoriesJsons = startAndReadStdoutToString(builder);
402365

core/src/test/java/org/owasp/dependencycheck/EngineIT.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ void testEngine() throws DatabaseException, ReportException {
9595
getSettings().setBoolean(Settings.KEYS.ANALYZER_NODE_PACKAGE_ENABLED, false);
9696
getSettings().setBoolean(Settings.KEYS.ANALYZER_NODE_AUDIT_ENABLED, false);
9797
getSettings().setBoolean(Settings.KEYS.ANALYZER_PNPM_AUDIT_ENABLED, false);
98+
getSettings().setBoolean(Settings.KEYS.ANALYZER_YARN_AUDIT_ENABLED, false);
9899
getSettings().setBoolean(Settings.KEYS.ANALYZER_EXPERIMENTAL_ENABLED, true);
99100
getSettings().setBoolean(Settings.KEYS.ANALYZER_BUNDLE_AUDIT_ENABLED, false);
100101
getSettings().setBoolean(Settings.KEYS.ANALYZER_MIX_AUDIT_ENABLED, false);

0 commit comments

Comments
 (0)