feat: package and utilize generated suppression file

[jeremylong]

Description of Change

First of three PRs around cleaning up the differences between the generated and base suppression file. This PR enhances the build to download the current generated suppression file, packages it into the core lib, and loads both the base and generated suppression files during execution.

[Copilot]

Pull Request Overview

This PR introduces a mechanism to package a generated suppressions file with the application during build time by downloading it from a hosted URL. The downloaded file is then loaded alongside the existing base suppression file.

• Adds Maven exec plugin to download suppressions from a hosted URL during the build process
• Extends the suppression loading logic to include the downloaded generated-suppressions.xml file
• Refactors the packaged suppression loading method to accept a filename parameter for reusability

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File	Description
pom.xml	Adds version management for exec-maven-plugin
core/pom.xml	Configures exec-maven-plugin to download suppressions file using wget during build
core/src/main/java/org/owasp/dependencycheck/analyzer/AbstractSuppressionAnalyzer.java	Adds constant for generated suppressions filename and refactors loading logic to support both base and generated suppression files

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[chadlwilson]

With this approach, ODC needs to load the generatedSuppressions twice, once off packaged, and again off "hosted" suppressions within the HostedSuppressionsDataSource - which will largely overlap. Does it de-duplicate these in memory at least? Is this a significant source of slowness?

It's slightly more complex, but I wonder if it would be cleaner to make this self contained somewhere within the below to just pre-populate the user's cache in the data directory on disk; to keep all logic and workflow related to these suppressions self-contained, and with consistent language?

With this approach it would happen

• on first use, prior to any attempt to update
• after hosted suppressions are purged
• regardless of whether hosted suppressions updates are enabled.

DependencyCheck/core/src/main/java/org/owasp/dependencycheck/analyzer/AbstractSuppressionAnalyzer.java

Lines 260 to 264 in a9f313a

	if (!repoFile.isFile() || repoFile.length() <= 1L) {
	repoEmpty = true;
	LOGGER.warn("Hosted Suppressions file is empty or missing - attempting to force the update");
	getSettings().setBoolean(Settings.KEYS.HOSTED_SUPPRESSIONS_FORCEUPDATE, true);
	}

[nhumblot]

With this approach, ODC needs to load the generatedSuppressions twice, once off packaged, and again off "hosted" suppressions within the HostedSuppressionsDataSource - which will largely overlap. Does it de-duplicate these in memory at least? Is this a significant source of slowness?

It's slightly more complex, but I wonder if it would be cleaner to make this self contained somewhere within the below to just pre-populate the user's cache in the data directory on disk; to keep all logic and workflow related to these suppressions self-contained, and with consistent language?

With this approach it would happen

* on first use, prior to any attempt to update

* after hosted suppressions are purged

* regardless of whether hosted suppressions updates are enabled.

DependencyCheck/core/src/main/java/org/owasp/dependencycheck/analyzer/AbstractSuppressionAnalyzer.java

Lines 260 to 264 in a9f313a

	if (!repoFile.isFile() || repoFile.length() <= 1L) {
	repoEmpty = true;
	LOGGER.warn("Hosted Suppressions file is empty or missing - attempting to force the update");
	getSettings().setBoolean(Settings.KEYS.HOSTED_SUPPRESSIONS_FORCEUPDATE, true);
	}

On my machine, a forced update took 148ms to be loaded on 4G connection with a phone.

We seem to load all elements in a List<SuppressionRule> and we add them through a void addAll(List<SuppressionRule>) call. I don't see any de-duplication so my guess is we are duplicating suppression rules between base and hosted suppressions as of now. It does not change much from now, so I may say it can be a future improvement opportunity (impact would have to be more thoroughly investigated) that could be tracked, and not a necessary change in this PR 🤔

[chadlwilson]

My concern is more about evaluation/analysis rather than loading the file.

I haven't looked at how smart the algorithm is or how parallelized, but my concern would be that there is an O(n^2) issue here, e.g if every suppression is evaluated for every single possible dependency. We also don't really have any automation to ensure the regexes aren't insanely expensive which might have a multiplicative effect.

On a bigger project with, say, 1,000 dependencies (JavaScript!!) and lots of FPs due to use of CLI and generic dependency names, that might be an issue - either slowness or excessive junk/GC production.

Aside from perf, I also think it's a bit confusing to have 3 data sources in the code to reason about, which is why I suggest using this file essentially as an initial cache value for HostedSuppressions instead (when working offline, or if stale) since it is not valid/useful to have both the packaged version and a version received remotely. I'm happy to test and suggest a PR to this one if @jeremylong wants to see what it might look like.

That'd avoid having to do any de-duplication of individual suppressions since we can generally assume there are very limited duplications between base and hosted/generated/published (especially after Jeremy's adjacent cleanup PRs).

I think that helps us as it's already quite tough figuring out why we might not be able to replicate a given FP given combinations of (ODC version, ODC variant, enabled analyzers, enabled data sources, available data sources, hosted suppressions currentness, offline status, available local tools)

[jeremylong]

I like the idea of the embedded being the initial cache - especially if you don't mind putting together the PR ;).

I was also going to make sure there was a mechanism to prevent loading the same rule twice...

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The error message "Unable to load the base suppression data file" is misleading when used in a generic method that can load any packaged file. Since this method now handles both BASE_SUPPRESSION_FILE and HOSTED_SUPPRESSION_SNAPSHOT_FILE, the error message should be more generic, such as "Unable to load the packaged file: " + packagedFileName.

Suggested change

	throw new SuppressionParseException("Unable to load the base suppression data file", e);
	throw new SuppressionParseException("Unable to load the packaged file: " + packagedFileName, e);

[Copilot]

Variable name baseSuppresssionURL has a typo: "suppresssion" should be "suppression" (three s's instead of two).

Suggested change

	URL baseSuppresssionURL = getPackagedFile(BASE_SUPPRESSION_FILE);
	try (InputStream in = baseSuppresssionURL.openStream()) {
	URL baseSuppressionURL = getPackagedFile(BASE_SUPPRESSION_FILE);
	try (InputStream in = baseSuppressionURL.openStream()) {

[Copilot]

Variable name baseSuppresssionURL has a typo: "suppresssion" should be "suppression" (three s's instead of two).

[Copilot]

Missing JavaDoc documentation for the getPackagedFile method. The method should have documentation describing its purpose, parameters, return value, and exceptions thrown, especially since it's a utility method that can be reused for loading different packaged files.

Suggested change

	/**
	* Returns a {@link URL} to a file packaged with the application, such as a base suppression file.
	*
	* <p>This method constructs the appropriate URL depending on whether the application is running from a JAR,
	* a nested JAR, or from the file system during development.</p>
	*
	* @param packagedFileName the name of the packaged file to load
	* @return a {@link URL} pointing to the packaged file
	* @throws SuppressionParseException if the URL cannot be constructed or is malformed
	*/

[Copilot]

[nitpick] Unnecessary initialization to null. The variable baseSuppresssionURL is always assigned in the try block before being returned. Initialize it directly in the try block or declare it as final and assign it once: final URL baseSuppresssionURL = new URL(suppressionFileLocation);

Suggested change

	URL baseSuppresssionURL = null;
	try {
	baseSuppresssionURL = new URL(suppressionFileLocation);
	} catch (MalformedURLException e) {
	throw new SuppressionParseException("Unable to load the base suppression data file", e);
	}
	return baseSuppresssionURL;
	try {
	final URL baseSuppresssionURL = new URL(suppressionFileLocation);
	return baseSuppresssionURL;
	} catch (MalformedURLException e) {
	throw new SuppressionParseException("Unable to load the base suppression data file", e);
	}

[Copilot]

Variable name baseSuppresssionURL has a typo: "suppresssion" should be "suppression" (three s's instead of two). This variable is used in a generic method that handles any packaged file, so consider renaming it to something more generic like packagedFileURL to match the method's purpose.