From de04adeef1d2970075a34b0ba65bff34d4891cde Mon Sep 17 00:00:00 2001 From: Chad Wilson <29788154+chadlwilson@users.noreply.github.com> Date: Wed, 6 May 2026 12:22:48 +0800 Subject: [PATCH 1/5] fix: restore functionality of hostedSuppressionsForceUpdate; falling back to snapshot suppressions even if failing Signed-off-by: Chad Wilson <29788154+chadlwilson@users.noreply.github.com> --- .../dependencycheck/taskdefs/Update.java | 4 +- ant/src/site/markdown/config-update.md | 2 +- ant/src/site/markdown/configuration.md | 2 +- .../org/owasp/dependencycheck/CliParser.java | 2 +- cli/src/site/markdown/arguments.md | 2 +- .../analyzer/AbstractSuppressionAnalyzer.java | 212 +++++++------- .../update/HostedSuppressionsDataSource.java | 123 +++++--- .../owasp/dependencycheck/BaseDBTestCase.java | 2 +- .../org/owasp/dependencycheck/BaseTest.java | 58 +++- .../AbstractSuppressionAnalyzerTest.java | 273 +++++++++++------- .../HostedSuppressionsDataSourceTest.java | 225 +++++++++++++++ .../test/resources/suppressions-invalid.xml | 7 + .../maven/BaseDependencyCheckMojo.java | 2 +- maven/src/site/markdown/configuration.md | 2 +- .../configuration-aggregate.md | 2 +- .../configuration-update.md | 2 +- .../dependency-check-gradle/configuration.md | 2 +- 17 files changed, 640 insertions(+), 282 deletions(-) create mode 100644 core/src/test/java/org/owasp/dependencycheck/data/update/HostedSuppressionsDataSourceTest.java create mode 100644 core/src/test/resources/suppressions-invalid.xml diff --git a/ant/src/main/java/org/owasp/dependencycheck/taskdefs/Update.java b/ant/src/main/java/org/owasp/dependencycheck/taskdefs/Update.java index 9736dfc277e..6019a193964 100644 --- a/ant/src/main/java/org/owasp/dependencycheck/taskdefs/Update.java +++ b/ant/src/main/java/org/owasp/dependencycheck/taskdefs/Update.java @@ -20,12 +20,12 @@ import org.apache.tools.ant.BuildException; import org.apache.tools.ant.Project; import org.owasp.dependencycheck.Engine; +import org.owasp.dependencycheck.ant.logging.AntTaskHolder; import org.owasp.dependencycheck.data.nvdcve.DatabaseException; import org.owasp.dependencycheck.data.update.exception.UpdateException; import org.owasp.dependencycheck.utils.Downloader; import org.owasp.dependencycheck.utils.InvalidSettingException; import org.owasp.dependencycheck.utils.Settings; -import org.owasp.dependencycheck.ant.logging.AntTaskHolder; /** * An Ant task definition to execute dependency-check update. This will download @@ -203,7 +203,7 @@ public class Update extends Purge { */ private Boolean hostedSuppressionsForceUpdate; /** - * Whether the hosted suppressions file will be used. Defaults to true. + * Whether the hosted suppressions will be updated from the configured URL. Defaults to true. */ private Boolean hostedSuppressionsEnabled; /** diff --git a/ant/src/site/markdown/config-update.md b/ant/src/site/markdown/config-update.md index b310bc48788..aff1d4d2519 100644 --- a/ant/src/site/markdown/config-update.md +++ b/ant/src/site/markdown/config-update.md @@ -50,7 +50,7 @@ The following properties can be configured in the plugin. However, they are less | connectionString | The connection string used to connect to the database. See using a [database server](../data/database.html). |   | | databaseUser | The username used when connecting to the database. |   | | databasePassword | The password used when connecting to the database. |   | -| hostedSuppressionsEnabled | Whether the hosted suppression file will be used. | true | +| hostedSuppressionsEnabled | Whether the hosted suppressions will be updated from the configured URL. | true | | hostedSuppressionsUrl | The URL to a mirrored copy of the hosted suppressions file for internet-constrained environments | https://dependency-check.github.io/DependencyCheck/suppressions/publishedSuppressions.xml | | hostedSuppressionsUser | The user for a Basic-auth-protected mirrored copy of the hosted suppressions file for internet-constrained environments |   | | hostedSuppressionsPassword | The password/token for a Basic-auth-protected mirrored copy of the hosted suppressions file for internet-constrained environments |   | diff --git a/ant/src/site/markdown/configuration.md b/ant/src/site/markdown/configuration.md index 8d86595c0c6..54f0f3fa0dd 100644 --- a/ant/src/site/markdown/configuration.md +++ b/ant/src/site/markdown/configuration.md @@ -166,7 +166,7 @@ The following properties can be configured in the plugin. However, they are less | connectionString | The connection string used to connect to the database. See using a [database server](../data/database.html). |   | | databaseUser | The username used when connecting to the database. |   | | databasePassword | The password used when connecting to the database. |   | -| hostedSuppressionsEnabled | Whether the hosted suppression file will be used. | true | +| hostedSuppressionsEnabled | Whether the hosted suppressions will be updated from the configured URL. | true | | hostedSuppressionsUrl | The URL to a mirrored copy of the hosted suppressions file for internet-constrained environments | https://dependency-check.github.io/DependencyCheck/suppressions/publishedSuppressions.xml | | hostedSuppressionsUser | The user for a Basic-auth-protected mirrored copy of the hosted suppressions file for internet-constrained environments |   | | hostedSuppressionsPassword | The password/token for a Basic-auth-protected mirrored copy of the hosted suppressions file for internet-constrained environments |   | diff --git a/cli/src/main/java/org/owasp/dependencycheck/CliParser.java b/cli/src/main/java/org/owasp/dependencycheck/CliParser.java index 1fd2b0cf9fd..f7996daedde 100644 --- a/cli/src/main/java/org/owasp/dependencycheck/CliParser.java +++ b/cli/src/main/java/org/owasp/dependencycheck/CliParser.java @@ -547,7 +547,7 @@ private void addAdvancedOptions(final Options options) { .addOption(newOption(ARGUMENT.ENABLE_NEXUS, "Enable the Nexus Analyzer.")) .addOption(newOption(ARGUMENT.ARTIFACTORY_ENABLED, "Whether the Artifactory Analyzer should be enabled.")) .addOption(newOption(ARGUMENT.PURGE_NVD, "Purges the local NVD data cache")) - .addOption(newOption(ARGUMENT.DISABLE_HOSTED_SUPPRESSIONS, "Disable the usage of the hosted suppressions file")) + .addOption(newOption(ARGUMENT.DISABLE_HOSTED_SUPPRESSIONS, "Disable retrieval of the hosted suppressions from the configured URL.")) .addOption(newOption(ARGUMENT.HOSTED_SUPPRESSIONS_FORCEUPDATE, "Force the hosted suppressions file to update even" + " if autoupdate is disabled")) .addOption(newOptionWithArg(ARGUMENT.HOSTED_SUPPRESSIONS_VALID_FOR_HOURS, "hours", diff --git a/cli/src/site/markdown/arguments.md b/cli/src/site/markdown/arguments.md index 143b2205046..0e8d88d7207 100644 --- a/cli/src/site/markdown/arguments.md +++ b/cli/src/site/markdown/arguments.md @@ -134,7 +134,7 @@ Advanced Options | | \-\-dbUser | \ | The username used to connect to the database. |   | | \-d | \-\-data | \ | The location of the data directory used to store persistent data. | /usr/local/var/dependencycheck if installed through brew (→ [formula](https://github.com/Homebrew/homebrew-core/blob/master/Formula/d/dependency-check.rb#L29)). Otherwise, the data directory is created inside the install directory i.e. as a sibling to the `/bin`, `/lib` directories. | | | \-\-purge | | Delete the local copy of the NVD. This is used to force a refresh of the data. |   | -| | \-\-disableHostedSuppressions | | Whether the usage of the hosted suppressions file will be disabled. | false | +| | \-\-disableHostedSuppressions | | Disable retrieval of the hosted suppressions from the configured URL. | false | | | \-\-hostedSuppressionsForceUpdate | | Whether the hosted suppressions file will update regardless of the `noupdate` argument. | false | | | \-\-hostedSuppressionsValidForHours | \ | The number of hours to wait before checking for new updates of the hosted suppressions file | 2 | | | \-\-hostedSuppressionsUrl | \ | The URL to a mirrored copy of the hosted suppressions file for internet-constrained environments | https://dependency-check.github.io/DependencyCheck/suppressions/publishedSuppressions.xml | diff --git a/core/src/main/java/org/owasp/dependencycheck/analyzer/AbstractSuppressionAnalyzer.java b/core/src/main/java/org/owasp/dependencycheck/analyzer/AbstractSuppressionAnalyzer.java index a8635751162..d4210ef9532 100644 --- a/core/src/main/java/org/owasp/dependencycheck/analyzer/AbstractSuppressionAnalyzer.java +++ b/core/src/main/java/org/owasp/dependencycheck/analyzer/AbstractSuppressionAnalyzer.java @@ -17,41 +17,44 @@ */ package org.owasp.dependencycheck.analyzer; -import java.io.File; -import java.io.IOException; -import java.io.InputStream; -import java.net.MalformedURLException; -import java.net.URL; -import java.nio.file.Files; -import java.nio.file.Path; -import java.nio.file.StandardCopyOption; -import java.util.ArrayList; -import java.util.List; -import java.util.Set; -import java.util.regex.Pattern; -import javax.annotation.concurrent.ThreadSafe; - +import com.google.common.annotations.VisibleForTesting; import org.jspecify.annotations.NonNull; import org.owasp.dependencycheck.Engine; import org.owasp.dependencycheck.analyzer.exception.AnalysisException; import org.owasp.dependencycheck.data.update.HostedSuppressionsDataSource; +import org.owasp.dependencycheck.data.update.exception.UpdateException; import org.owasp.dependencycheck.dependency.Dependency; import org.owasp.dependencycheck.exception.InitializationException; import org.owasp.dependencycheck.exception.WriteLockException; -import org.owasp.dependencycheck.utils.WriteLock; -import org.owasp.dependencycheck.xml.suppression.SuppressionParseException; -import org.owasp.dependencycheck.xml.suppression.SuppressionParser; -import org.owasp.dependencycheck.xml.suppression.SuppressionRule; import org.owasp.dependencycheck.utils.DownloadFailedException; import org.owasp.dependencycheck.utils.Downloader; import org.owasp.dependencycheck.utils.FileUtils; import org.owasp.dependencycheck.utils.ResourceNotFoundException; import org.owasp.dependencycheck.utils.Settings; import org.owasp.dependencycheck.utils.TooManyRequestsException; +import org.owasp.dependencycheck.utils.WriteLock; +import org.owasp.dependencycheck.xml.suppression.SuppressionParseException; +import org.owasp.dependencycheck.xml.suppression.SuppressionParser; +import org.owasp.dependencycheck.xml.suppression.SuppressionRule; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.xml.sax.SAXException; +import javax.annotation.concurrent.ThreadSafe; +import java.io.File; +import java.io.IOException; +import java.io.InputStream; +import java.net.MalformedURLException; +import java.net.URL; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardCopyOption; +import java.util.ArrayList; +import java.util.List; +import java.util.regex.Pattern; + +import static org.owasp.dependencycheck.data.update.HostedSuppressionsDataSource.falsePositivesDueTo; + /** * Abstract base suppression analyzer that contains methods for parsing the * suppression XML file. @@ -78,16 +81,6 @@ public abstract class AbstractSuppressionAnalyzer extends AbstractAnalyzer { */ public static final String SUPPRESSION_OBJECT_KEY = "suppression.rules"; - /** - * Returns a list of file EXTENSIONS supported by this analyzer. - * - * @return a list of file EXTENSIONS supported by this analyzer. - */ - @SuppressWarnings("SameReturnValue") - public Set getSupportedExtensions() { - return null; - } - /** * The prepare method loads the suppression XML file. * @@ -102,13 +95,13 @@ public synchronized void prepareAnalyzer(Engine engine) throws InitializationExc try { loadSuppressionBaseData(engine); } catch (SuppressionParseException ex) { - throw new InitializationException("Error initializing the suppression analyzer: " + ex.getLocalizedMessage(), ex, true); + throw new InitializationException("Error initializing the suppression analyzer base data: " + ex, ex, true); } try { - loadSuppressionData(engine); + loadSuppressionUserData(engine); } catch (SuppressionParseException ex) { - throw new InitializationException("Warn initializing the suppression analyzer: " + ex.getLocalizedMessage(), ex, false); + throw new InitializationException("Warn initializing the suppression analyzer user data: " + ex, ex, false); } } @@ -145,12 +138,12 @@ protected void analyzeDependency(Dependency dependency, Engine engine) throws An * @param engine a reference to the ODC engine. * @throws SuppressionParseException thrown if the XML cannot be parsed. */ - private void loadSuppressionData(Engine engine) throws SuppressionParseException { - final List ruleList = new ArrayList<>(); + private void loadSuppressionUserData(Engine engine) throws SuppressionParseException { final SuppressionParser parser = new SuppressionParser(); final String[] suppressionFilePaths = getSettings().getArray(Settings.KEYS.SUPPRESSION_FILE); final List failedLoadingFiles = new ArrayList<>(); if (suppressionFilePaths != null && suppressionFilePaths.length > 0) { + final List ruleList = new ArrayList<>(); // Load all the suppression file paths for (final String suppressionFilePath : suppressionFilePaths) { try { @@ -160,20 +153,12 @@ private void loadSuppressionData(Engine engine) throws SuppressionParseException failedLoadingFiles.add(msg); } } + LOGGER.debug("{} user suppression rules were loaded from {} sources.", ruleList.size(), suppressionFilePaths.length - failedLoadingFiles.size()); + appendRules(engine, ruleList); } - LOGGER.debug("{} suppression rules were loaded.", ruleList.size()); - if (!ruleList.isEmpty()) { - if (engine.hasObject(SUPPRESSION_OBJECT_KEY)) { - @SuppressWarnings("unchecked") - final List rules = (List) engine.getObject(SUPPRESSION_OBJECT_KEY); - rules.addAll(ruleList); - } else { - engine.putObject(SUPPRESSION_OBJECT_KEY, ruleList); - } - } if (!failedLoadingFiles.isEmpty()) { - LOGGER.debug("{} suppression files failed to load.", failedLoadingFiles.size()); + LOGGER.debug("{} user suppression files failed to load.", failedLoadingFiles.size()); final StringBuilder sb = new StringBuilder(); failedLoadingFiles.forEach(sb::append); throw new SuppressionParseException(sb.toString()); @@ -187,35 +172,27 @@ private void loadSuppressionData(Engine engine) throws SuppressionParseException * @throws SuppressionParseException thrown if the XML cannot be parsed. */ private void loadSuppressionBaseData(final Engine engine) throws SuppressionParseException { - final SuppressionParser parser = new SuppressionParser(); - loadPackagedSuppressionBaseData(parser, engine); - loadHostedSuppressionBaseData(parser, engine); + loadPackagedBaseSuppressionData(engine); + loadHostedSuppressionBaseData(engine); } /** * Loads the suppression rules packaged with the application. * - * @param parser The suppression parser to use * @param engine a reference the dependency-check engine * @throws SuppressionParseException thrown if the XML cannot be parsed. */ - private void loadPackagedSuppressionBaseData(final SuppressionParser parser, final Engine engine) throws SuppressionParseException { - List ruleList = null; + @VisibleForTesting + void loadPackagedBaseSuppressionData(final Engine engine) throws SuppressionParseException { + List ruleList; URL baseSuppressionURL = getPackagedFile(BASE_SUPPRESSION_FILE); try (InputStream in = baseSuppressionURL.openStream()) { - ruleList = parser.parseSuppressionRules(in); + ruleList = new SuppressionParser().parseSuppressionRules(in); + LOGGER.debug("{} base suppression rules were loaded.", ruleList.size()); + appendRules(engine, ruleList); } catch (SAXException | IOException ex) { throw new SuppressionParseException("Unable to parse the base suppression data file", ex); } - if (ruleList != null && !ruleList.isEmpty()) { - if (engine.hasObject(SUPPRESSION_OBJECT_KEY)) { - @SuppressWarnings("unchecked") - final List rules = (List) engine.getObject(SUPPRESSION_OBJECT_KEY); - rules.addAll(ruleList); - } else { - engine.putObject(SUPPRESSION_OBJECT_KEY, ruleList); - } - } } private static @NonNull URL getPackagedFile(String packagedFileName) throws SuppressionParseException { @@ -230,13 +207,11 @@ private void loadPackagedSuppressionBaseData(final SuppressionParser parser, fin } else { suppressionFileLocation = "file:" + suppressionFileLocation + packagedFileName; } - URL baseSuppressionURL = null; try { - baseSuppressionURL = new URL(suppressionFileLocation); + return new URL(suppressionFileLocation); } catch (MalformedURLException e) { throw new SuppressionParseException("Unable to load the packaged file: " + packagedFileName, e); } - return baseSuppressionURL; } /** @@ -249,62 +224,66 @@ private void loadPackagedSuppressionBaseData(final SuppressionParser parser, fin * already been resolved by the dependency-check project. * * @param engine a reference the dependency-check engine - * @param parser The suppression parser to use */ - private void loadHostedSuppressionBaseData(final SuppressionParser parser, final Engine engine) { - final boolean enabled = getSettings().getBoolean(Settings.KEYS.HOSTED_SUPPRESSIONS_ENABLED, true); - if (!enabled) { - return; - } - + @VisibleForTesting + void loadHostedSuppressionBaseData(final Engine engine) { try { - final String configuredUrl = getSettings().getString(Settings.KEYS.HOSTED_SUPPRESSIONS_URL, - HostedSuppressionsDataSource.DEFAULT_SUPPRESSIONS_URL); - final URL url = new URL(configuredUrl); - final String fileName = new File(url.getPath()).getName(); - if (fileName.isBlank()) { - throw new IOException("Hosted Suppression URL must imply a filename"); - } - final File repoFile = new File(getSettings().getDataDirectory(), fileName); - boolean repoEmpty = !repoFile.isFile() || repoFile.length() <= 1L; - if (repoEmpty) { - // utilize the snapshot hosted suppression file + // Try remote update if enabled and stale or forced by user + File repoFile = tryRemoteHostedSuppressionsFetchIfConfigured(engine); + + // If still empty after update attempt; utilize the snapshot hosted suppression file + // + // Note that this local fallback will run regardless of whether hosted suppressions are "enabled" or the + // value of autoUpdate, forceupdate etc since this is an offline operation similar to regular "base" suppressions. + if (isRepoEmpty(repoFile)) { + LOGGER.debug("Hosted suppressions not found locally; attempting fallback to store packaged snapshot from this Dependency-Check release at {}...", repoFile.toPath()); URL hostedSuppressionSnapshotURL = getPackagedFile(HOSTED_SUPPRESSION_SNAPSHOT_FILE); try (InputStream in = hostedSuppressionSnapshotURL.openStream()) { Files.copy(in, repoFile.toPath(), StandardCopyOption.REPLACE_EXISTING); - repoEmpty = false; - LOGGER.debug("Copied hosted suppression snapshot file to {}", repoFile.toPath()); - } catch (IOException ex) { - LOGGER.warn("Unable to copy the hosted suppression snapshot file to {}, results may contain false positives " - + "already resolved by the DependencyCheck project", repoFile.toPath(), ex); } + LOGGER.info(falsePositivesDueTo("Hosted suppressions using snapshot as of this Dependency-Check release")); } - if (!repoEmpty) { - loadCachedHostedSuppressionsRules(parser, repoFile, engine); - } else { - LOGGER.warn("Empty Hosted Suppression file after update, results may contain false positives " - + "already resolved by the DependencyCheck project due to failed download of the hosted suppression file"); - } + + loadCachedHostedSuppressionsRules(repoFile, engine); + } catch (IOException | InitializationException ex) { - LOGGER.warn("Unable to load hosted suppressions", ex); + LOGGER.warn(falsePositivesDueTo("Unable to load hosted suppressions from either remote source or packaged snapshot"), ex); } } + private static boolean isRepoEmpty(File repoFile) { + return !repoFile.isFile() || repoFile.length() <= 1L; + } + + /** + * If configured to do so, try fetching hosted suppressions from the configured remote source. + * @return The local cached repoFile the suppressions are to be loaded from. Note that on return this may still not be created. + * @throws IOException only if settings are invalid to handle hosted suppressions either remotely or locally + */ + private File tryRemoteHostedSuppressionsFetchIfConfigured(Engine engine) throws IOException { + HostedSuppressionsDataSource ds = new HostedSuppressionsDataSource(); + try { + ds.updateUnhandled(engine); + } catch (UpdateException ex) { + LOGGER.warn(falsePositivesDueTo("Failed to update hosted suppressions file from remote source"), ex); + } + return ds.validatedRepoFile(); + } + /** * Load the hosted suppression file from the web resource * - * @param parser The suppressionParser to use for loading * @param repoFile The cached web resource * @param engine a reference the dependency-check engine * * @throws InitializationException When errors occur trying to create a * defensive copy of the web resource before loading */ - private void loadCachedHostedSuppressionsRules(final SuppressionParser parser, final File repoFile, final Engine engine) + private void loadCachedHostedSuppressionsRules(final File repoFile, final Engine engine) throws InitializationException { // take a defensive copy to avoid a risk of corrupted file by a competing parallel new download. final Path defensiveCopy; - try (WriteLock lock = new WriteLock(getSettings(), true, repoFile.getName() + ".lock")) { + try (WriteLock ignored = new WriteLock(getSettings(), true, repoFile.getName() + ".lock")) { defensiveCopy = Files.createTempFile("dc-basesuppressions", ".xml"); LOGGER.debug("copying hosted suppressions file {} to {}", repoFile.toPath(), defensiveCopy); Files.copy(repoFile.toPath(), defensiveCopy, StandardCopyOption.REPLACE_EXISTING); @@ -314,19 +293,12 @@ private void loadCachedHostedSuppressionsRules(final SuppressionParser parser, f try (InputStream in = Files.newInputStream(defensiveCopy)) { final List ruleList; - ruleList = parser.parseSuppressionRules(in); - if (!ruleList.isEmpty()) { - if (engine.hasObject(SUPPRESSION_OBJECT_KEY)) { - @SuppressWarnings("unchecked") - final List rules = (List) engine.getObject(SUPPRESSION_OBJECT_KEY); - rules.addAll(ruleList); - } else { - engine.putObject(SUPPRESSION_OBJECT_KEY, ruleList); - } - } + ruleList = new SuppressionParser().parseSuppressionRules(in); + LOGGER.debug("{} hosted suppression rules were loaded.", ruleList.size()); + appendRules(engine, ruleList); + } catch (SAXException | IOException ex) { - LOGGER.warn("Unable to parse the hosted suppressions data file, results may contain false positives already resolved " - + "by the DependencyCheck project", ex); + LOGGER.warn(falsePositivesDueTo("Unable to parse the hosted suppressions data file at {}"), repoFile.getPath(), ex); } try { Files.delete(defensiveCopy); @@ -335,6 +307,18 @@ private void loadCachedHostedSuppressionsRules(final SuppressionParser parser, f } } + private void appendRules(Engine engine, List ruleList) { + if (!ruleList.isEmpty()) { + if (engine.hasObject(SUPPRESSION_OBJECT_KEY)) { + @SuppressWarnings("unchecked") + final List rules = (List) engine.getObject(SUPPRESSION_OBJECT_KEY); + rules.addAll(ruleList); + } else { + engine.putObject(SUPPRESSION_OBJECT_KEY, ruleList); + } + } + } + /** * Load a single suppression rules file from the path provided using the * parser provided. @@ -367,20 +351,20 @@ private List loadSuppressionFile(final SuppressionParser parser Downloader.getInstance().fetchFile(url, file, true, Settings.KEYS.SUPPRESSION_FILE_USER, Settings.KEYS.SUPPRESSION_FILE_PASSWORD, Settings.KEYS.SUPPRESSION_FILE_BEARER_TOKEN); } catch (TooManyRequestsException ex1) { - throw new SuppressionParseException("Unable to download supression file `" + file + throw new SuppressionParseException("Unable to download suppression file `" + file + "`; received 429 - too many requests", ex1); } catch (ResourceNotFoundException ex1) { - throw new SuppressionParseException("Unable to download supression file `" + file + throw new SuppressionParseException("Unable to download suppression file `" + file + "`; received 404 - resource not found", ex1); } catch (InterruptedException ex1) { Thread.currentThread().interrupt(); - throw new SuppressionParseException("Unable to download supression file `" + file + "`", ex1); + throw new SuppressionParseException("Unable to download suppression file `" + file + "`", ex1); } } catch (TooManyRequestsException ex) { - throw new SuppressionParseException("Unable to download supression file `" + file + throw new SuppressionParseException("Unable to download suppression file `" + file + "`; received 429 - too many requests", ex); } catch (ResourceNotFoundException ex) { - throw new SuppressionParseException("Unable to download supression file `" + file + "`; received 404 - resource not found", ex); + throw new SuppressionParseException("Unable to download suppression file `" + file + "`; received 404 - resource not found", ex); } } else { file = new File(suppressionFilePath); @@ -435,7 +419,7 @@ private List loadSuppressionFile(final SuppressionParser parser * SuppressionParseException */ private void throwSuppressionParseException(String message, Exception exception, String suppressionFilePath) throws SuppressionParseException { - LOGGER.warn(String.format(message + " '%s'", suppressionFilePath)); + LOGGER.warn("{} [{}]", message, suppressionFilePath); LOGGER.debug("", exception); throw new SuppressionParseException(message, exception); } diff --git a/core/src/main/java/org/owasp/dependencycheck/data/update/HostedSuppressionsDataSource.java b/core/src/main/java/org/owasp/dependencycheck/data/update/HostedSuppressionsDataSource.java index 09d7627b7b2..b05dd28f267 100644 --- a/core/src/main/java/org/owasp/dependencycheck/data/update/HostedSuppressionsDataSource.java +++ b/core/src/main/java/org/owasp/dependencycheck/data/update/HostedSuppressionsDataSource.java @@ -17,10 +17,12 @@ */ package org.owasp.dependencycheck.data.update; +import org.jspecify.annotations.NonNull; import org.owasp.dependencycheck.Engine; import org.owasp.dependencycheck.data.update.exception.UpdateException; import org.owasp.dependencycheck.exception.WriteLockException; import org.owasp.dependencycheck.utils.Downloader; +import org.owasp.dependencycheck.utils.InvalidSettingException; import org.owasp.dependencycheck.utils.ResourceNotFoundException; import org.owasp.dependencycheck.utils.Settings; import org.owasp.dependencycheck.utils.TooManyRequestsException; @@ -35,6 +37,10 @@ import java.nio.file.Files; public class HostedSuppressionsDataSource extends LocalDataSource { + /** + * The default URL to the Hosted Suppressions file. + */ + public static final String DEFAULT_SUPPRESSIONS_URL = "https://dependency-check.github.io/DependencyCheck/suppressions/publishedSuppressions.xml"; /** * Static logger. @@ -45,51 +51,85 @@ public class HostedSuppressionsDataSource extends LocalDataSource { * The configured settings. */ private Settings settings; - /** - * The default URL to the Hosted Suppressions file. - */ - public static final String DEFAULT_SUPPRESSIONS_URL = "https://dependency-check.github.io/DependencyCheck/suppressions/publishedSuppressions.xml"; /** - * Downloads the current Hosted suppressions file. + * Makes a best effort to download the current Hosted suppressions file if configured to do so. * * @param engine a reference to the ODC Engine * @return returns false as no updates are made to the database, just web * resources cached locally - * @throws UpdateException thrown if the update encountered fatal errors + * @throws UpdateException thrown only if the update encountered fatal configuration errors */ @Override public boolean update(Engine engine) throws UpdateException { - this.settings = engine.getSettings(); - final String configuredUrl = settings.getString(Settings.KEYS.HOSTED_SUPPRESSIONS_URL, DEFAULT_SUPPRESSIONS_URL); - final boolean autoupdate = settings.getBoolean(Settings.KEYS.AUTO_UPDATE, true); - final boolean forceupdate = settings.getBoolean(Settings.KEYS.HOSTED_SUPPRESSIONS_FORCEUPDATE, false); - final boolean cpeSuppressionEnabled = settings.getBoolean(Settings.KEYS.ANALYZER_CPE_SUPPRESSION_ENABLED, true); - final boolean vulnSuppressionEnabled = settings.getBoolean(Settings.KEYS.ANALYZER_VULNERABILITY_SUPPRESSION_ENABLED, true); - boolean enabled = settings.getBoolean(Settings.KEYS.HOSTED_SUPPRESSIONS_ENABLED, true); - enabled = enabled && (cpeSuppressionEnabled || vulnSuppressionEnabled); try { - final URL url = new URL(configuredUrl); - final File filepath = new File(url.getPath()); - final File repoFile = new File(settings.getDataDirectory(), filepath.getName()); - final boolean proceed = enabled && (forceupdate || (autoupdate && shouldUpdate(repoFile))); - if (proceed) { - LOGGER.debug("Begin Hosted Suppressions file update"); - fetchHostedSuppressions(settings, url, repoFile); - saveLastUpdated(repoFile, System.currentTimeMillis() / 1000); - } + updateUnhandled(engine); } catch (UpdateException ex) { // only emit a warning, DependencyCheck will continue without taking the latest hosted suppressions into account. - LOGGER.warn("Failed to update hosted suppressions file, results may contain false positives already resolved by the " - + "DependencyCheck project", ex); - } catch (MalformedURLException ex) { - throw new UpdateException(String.format("Invalid URL for Hosted Suppressions file (%s)", configuredUrl), ex); + LOGGER.warn(falsePositivesDueTo("Failed to update hosted suppressions file from remote source"), ex); } catch (IOException ex) { - throw new UpdateException("Unable to get the data directory", ex); + // Unhandled IOExceptions are fatal configuration errors of a sort + throw new UpdateException("Unable to determine the local location to cache hosted suppressions", ex); } return false; } + public static @NonNull String falsePositivesDueTo(String reason) { + return reason + ", results may contain false positives already resolved by the DependencyCheck project"; + } + + /** + * Updates the current Hosted Suppressions file if configured to do so; failing if it cannot be done + * + * @param engine a reference to the ODC Engine + * @throws IOException if there is an error determining the local location to cache hosted suppressions + * @throws UpdateException if the remote update failed for any reason + */ + public void updateUnhandled(Engine engine) throws IOException, UpdateException { + this.settings = engine.getSettings(); + final URL url = validatedUrl(); + final File repoFile = validatedRepoFileFrom(url); + + if (isEnabled() && shouldUpdateFromRemote(repoFile)) { + LOGGER.debug("Begin Hosted Suppressions file update from remote source"); + fetchHostedSuppressions(url, repoFile); + saveLastUpdated(repoFile, System.currentTimeMillis() / 1000); + } + } + + private @NonNull URL validatedUrl() throws InvalidSettingException { + final String configuredUrl = settings.getString(Settings.KEYS.HOSTED_SUPPRESSIONS_URL, DEFAULT_SUPPRESSIONS_URL); + try { + return new URL(configuredUrl); + } catch (MalformedURLException ex) { + throw new InvalidSettingException(String.format("Invalid URL for Hosted Suppressions file (%s)", configuredUrl), ex); + } + } + + public @NonNull File validatedRepoFile() throws IOException { + return validatedRepoFileFrom(validatedUrl()); + } + + private @NonNull File validatedRepoFileFrom(URL url) throws IOException { + String fileName = new File(url.getPath()).getName(); + if (fileName.isBlank()) { + throw new InvalidSettingException("Hosted Suppression URL must imply a filename; even if disabled."); + } + return new File(settings.getDataDirectory(), fileName); + } + + private boolean isEnabled() { + return settings.getBoolean(Settings.KEYS.HOSTED_SUPPRESSIONS_ENABLED, true) && ( + settings.getBoolean(Settings.KEYS.ANALYZER_CPE_SUPPRESSION_ENABLED, true) || + settings.getBoolean(Settings.KEYS.ANALYZER_VULNERABILITY_SUPPRESSION_ENABLED, true)); + } + + private boolean shouldUpdateFromRemote(File repoFile) { + boolean forceupdate = settings.getBoolean(Settings.KEYS.HOSTED_SUPPRESSIONS_FORCEUPDATE, false); + boolean autoupdate = settings.getBoolean(Settings.KEYS.AUTO_UPDATE, true); + return forceupdate || (autoupdate && isStale(repoFile)); + } + /** * Determines if the we should update the Hosted Suppressions file. * @@ -99,8 +139,8 @@ public boolean update(Engine engine) throws UpdateException { * @throws NumberFormatException thrown if an invalid value is contained in * the database properties */ - protected boolean shouldUpdate(File repo) throws NumberFormatException { - boolean proceed = true; + protected boolean isStale(File repo) throws NumberFormatException { + boolean stale = true; if (repo != null && repo.isFile()) { final int validForHours = settings.getInt(Settings.KEYS.HOSTED_SUPPRESSIONS_VALID_FOR_HOURS, 2); final long lastUpdatedOn = getLastUpdated(repo); @@ -108,18 +148,17 @@ protected boolean shouldUpdate(File repo) throws NumberFormatException { LOGGER.debug("Last updated: {}", lastUpdatedOn); LOGGER.debug("Now: {}", now); final long msValid = validForHours * 60L * 60L * 1000L; - proceed = (now - lastUpdatedOn) > msValid; - if (!proceed) { + stale = (now - lastUpdatedOn) > msValid; + if (!stale) { LOGGER.info("Skipping Hosted Suppressions file update since last update was within {} hours.", validForHours); } } - return proceed; + return stale; } /** * Fetches the hosted suppressions file * - * @param settings a reference to the dependency-check settings * @param repoUrl the URL to the hosted suppressions file to use * @param repoFile the local file where the hosted suppressions file is to * be placed @@ -127,7 +166,7 @@ protected boolean shouldUpdate(File repo) throws NumberFormatException { * initialization */ @SuppressWarnings("try") - private void fetchHostedSuppressions(Settings settings, URL repoUrl, File repoFile) throws UpdateException { + private void fetchHostedSuppressions(URL repoUrl, File repoFile) throws UpdateException { try (WriteLock ignored = new WriteLock(settings, true, repoFile.getName() + ".lock")) { if (LOGGER.isDebugEnabled()) { LOGGER.debug("Hosted Suppressions URL: {}", repoUrl.toExternalForm()); @@ -144,17 +183,14 @@ public boolean purge(Engine engine) { this.settings = engine.getSettings(); boolean result = true; try { - final URL repoUrl = new URL(settings.getString(Settings.KEYS.HOSTED_SUPPRESSIONS_URL, - DEFAULT_SUPPRESSIONS_URL)); - final String filename = new File(repoUrl.getPath()).getName(); - final File repo = new File(settings.getDataDirectory(), filename); + final File repo = validatedRepoFile(); if (repo.exists()) { - try (WriteLock ignored = new WriteLock(settings, true, filename + ".lock")) { + try (WriteLock ignored = new WriteLock(settings, true, repo.getName() + ".lock")) { result = deleteCachedFile(repo); } } } catch (WriteLockException | IOException ex) { - LOGGER.error("Unable to delete the Hosted suppression file - invalid configuration"); + LOGGER.error("Unable to delete the Hosted suppression file - invalid configuration: {}", ex.toString()); result = false; } return result; @@ -163,8 +199,9 @@ public boolean purge(Engine engine) { private boolean deleteCachedFile(final File repo) { boolean deleted = true; try { - Files.delete(repo.toPath()); - LOGGER.info("Hosted suppression file removed successfully"); + if (Files.deleteIfExists(repo.toPath())) { + LOGGER.info("Hosted suppression file removed successfully"); + } } catch (IOException ex) { LOGGER.error("Unable to delete '{}'; please delete the file manually", repo.getAbsolutePath(), ex); deleted = false; diff --git a/core/src/test/java/org/owasp/dependencycheck/BaseDBTestCase.java b/core/src/test/java/org/owasp/dependencycheck/BaseDBTestCase.java index 4c0b7b10ebb..6a29dd07627 100644 --- a/core/src/test/java/org/owasp/dependencycheck/BaseDBTestCase.java +++ b/core/src/test/java/org/owasp/dependencycheck/BaseDBTestCase.java @@ -68,7 +68,7 @@ public void ensureDBExists() throws Exception { if (!dataPath.exists() || !dataFile.exists()) { LOGGER.trace("Extracting database to {}", dataPath); dataPath.mkdirs(); - File path = new File(BaseDBTestCase.class.getClassLoader().getResource("data.zip").toURI().getPath()); + File path = BaseTest.getResourceAsFile(this, "data.zip"); try (FileInputStream fis = new FileInputStream(path); ZipInputStream zin = new ZipInputStream(new BufferedInputStream(fis))) { ZipEntry entry; diff --git a/core/src/test/java/org/owasp/dependencycheck/BaseTest.java b/core/src/test/java/org/owasp/dependencycheck/BaseTest.java index daffc98c6da..a7b283a11dc 100644 --- a/core/src/test/java/org/owasp/dependencycheck/BaseTest.java +++ b/core/src/test/java/org/owasp/dependencycheck/BaseTest.java @@ -15,14 +15,19 @@ */ package org.owasp.dependencycheck; +import org.jspecify.annotations.NonNull; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.owasp.dependencycheck.utils.Settings; import java.io.File; +import java.io.IOException; import java.io.InputStream; +import java.net.MalformedURLException; +import java.net.URI; import java.net.URISyntaxException; +import java.nio.charset.StandardCharsets; import java.util.Objects; /** @@ -67,29 +72,70 @@ public static void tearDownClass() { /** * Returns the given resource as an InputStream using the object's class loader. * - * @param o the object used to obtain a reference to the class loader + * @param o the object used to obtain a reference to the class loader * @param resource the name of the resource to load * @return the resource as an InputStream */ - public static InputStream getResourceAsStream(Object o, String resource) { + public static @NonNull InputStream getResourceAsStream(Object o, String resource) { return Objects.requireNonNull(o.getClass().getClassLoader().getResourceAsStream(resource), resource + " not found on classpath"); } /** * Returns the given resource as a File using the object's class loader. * - * @param o the object used to obtain a reference to the class loader + * @param o the object used to obtain a reference to the class loader * @param resource the name of the resource to load - * @return the resource as an File + * @return the resource as a File */ - public static File getResourceAsFile(Object o, String resource) { + public static @NonNull File getResourceAsFile(Object o, String resource) { + return new File(getResourceAsURI(o, resource).getPath()); + } + + /** + * Returns the given resource as a URI using the object's class loader. + * + * @param o the object used to obtain a reference to the class loader + * @param resource the name of the resource to load + * @return the resource as a URI + */ + public static @NonNull URI getResourceAsURI(Object o, String resource) { try { - return new File(Objects.requireNonNull(o.getClass().getClassLoader().getResource(resource), resource + " not found on classpath").toURI().getPath()); + return Objects.requireNonNull(o.getClass().getClassLoader().getResource(resource), resource + " not found on classpath").toURI(); } catch (URISyntaxException e) { throw new UnsupportedOperationException(e); } } + /** + * Returns the given resource as a URL string using the object's class loader. + * + * @param o the object used to obtain a reference to the class loader + * @param resource the name of the resource to load + * @return the resource as a URL string + */ + public static @NonNull String getResourceAsUrlString(Object o, String resource) { + try { + return getResourceAsURI(o, resource).toURL().toString(); + } catch (MalformedURLException e) { + throw new UnsupportedOperationException(e); + } + } + + /** + * Returns the given resource content using the object's class loader. + * + * @param o the object used to obtain a reference to the class loader + * @param resource the name of the resource to load + * @return the resource as a String + */ + public static @NonNull String getResourceAsContentString(Object o, String resource) { + try (InputStream is = getResourceAsStream(o, resource)) { + return new String(is.readAllBytes(), StandardCharsets.UTF_8); + } catch (IOException e) { + throw new UnsupportedOperationException(e); + } + } + /** * @return the settings for the test cases. */ diff --git a/core/src/test/java/org/owasp/dependencycheck/analyzer/AbstractSuppressionAnalyzerTest.java b/core/src/test/java/org/owasp/dependencycheck/analyzer/AbstractSuppressionAnalyzerTest.java index 619a54cbe7b..48c973eda4e 100644 --- a/core/src/test/java/org/owasp/dependencycheck/analyzer/AbstractSuppressionAnalyzerTest.java +++ b/core/src/test/java/org/owasp/dependencycheck/analyzer/AbstractSuppressionAnalyzerTest.java @@ -18,30 +18,33 @@ package org.owasp.dependencycheck.analyzer; import org.jspecify.annotations.NonNull; -import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.owasp.dependencycheck.BaseTest; import org.owasp.dependencycheck.Engine; import org.owasp.dependencycheck.Engine.Mode; +import org.owasp.dependencycheck.data.update.HostedSuppressionsDataSource; import org.owasp.dependencycheck.dependency.Dependency; import org.owasp.dependencycheck.exception.InitializationException; import org.owasp.dependencycheck.utils.Downloader; import org.owasp.dependencycheck.utils.InvalidSettingException; import org.owasp.dependencycheck.utils.Settings; import org.owasp.dependencycheck.utils.Settings.KEYS; +import org.owasp.dependencycheck.xml.suppression.SuppressionParseException; import org.owasp.dependencycheck.xml.suppression.SuppressionRule; import java.util.List; -import java.util.Set; import java.util.stream.Collectors; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.empty; +import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.not; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.owasp.dependencycheck.analyzer.AbstractSuppressionAnalyzer.SUPPRESSION_OBJECT_KEY; @@ -53,84 +56,156 @@ class AbstractSuppressionAnalyzerTest extends BaseTest { /** * A second suppression file to test with. */ - private static final String OTHER_SUPPRESSIONS_FILE = "other-suppressions.xml"; + private static final String OTHER_TEST_SUPPRESSIONS_FILE = "other-suppressions.xml"; /** * Suppression file to test with. */ - private static final String SUPPRESSIONS_FILE = "suppressions.xml"; + private static final String TEST_SUPPRESSIONS_FILE = "suppressions.xml"; + private static final int TEST_SUPPRESSIONS_EXPECTED_COUNT = 5; - private AbstractSuppressionAnalyzer instance; + private Engine engine; - @BeforeEach - void createObjectUnderTest() { - instance = new AbstractSuppressionAnalyzerImpl(); + @AfterEach + void cleanUp() { + if (engine != null) { + new HostedSuppressionsDataSource().purge(engine); + engine.close(); + } } - /** - * Test of getSupportedExtensions method, of class - * AbstractSuppressionAnalyzer. - */ - @Test - void testGetSupportedExtensions() { - Set result = instance.getSupportedExtensions(); - assertNull(result); + private String testSuppressionsFileUrl() { + return BaseTest.getResourceAsUrlString(this, TEST_SUPPRESSIONS_FILE); } - /** - * Test of getRules method, of class AbstractSuppressionAnalyzer for - * suppression file declared as URL. - */ - @Test - void testGetRulesFromSuppressionFileFromURL() throws Exception { - final String fileUrl = getClass().getClassLoader().getResource(SUPPRESSIONS_FILE).toURI().toURL().toString(); - final int numberOfExtraLoadedRules = getNumberOfRulesLoadedFromPath(fileUrl) - getNumberOfRulesLoadedInCoreFile(); - assertEquals(5, numberOfExtraLoadedRules, "Expected 5 extra rules in the given path"); + @Nested + class BasePackagedSuppressionsLoading { + @Test + void packagedBaseSuppressions() throws Exception { + engine = prepareBaseSuppressionsOnly(); + @SuppressWarnings("unchecked") List rules = (List) engine.getObject(SUPPRESSION_OBJECT_KEY); + assertThat(rules, not(empty())); + assertThat("Expected all suppressions in base file to be marked as base", allRulesNotMarkedAsBase(rules), empty()); + } } - /** - * Test of getRules method, of class AbstractSuppressionAnalyzer for - * suppression file on the class path. - */ - @Test - void testGetRulesFromSuppressionFileInClasspath() throws Exception { - final int numberOfExtraLoadedRules = getNumberOfRulesLoadedFromPath(SUPPRESSIONS_FILE) - getNumberOfRulesLoadedInCoreFile(); - assertEquals(5, numberOfExtraLoadedRules, "Expected 5 extra rules in the given file"); - } + @Nested + class HostedSuppressionsLoading { + @Test + void packagedSnapshotHostedSuppressionsLoadedEvenIfRemoteHostedSuppressionsDisabled() throws Exception { + getSettings().setBoolean(KEYS.HOSTED_SUPPRESSIONS_ENABLED, false); + engine = prepareHostedSuppressionsOnly(); + @SuppressWarnings("unchecked") List rules = (List) engine.getObject(SUPPRESSION_OBJECT_KEY); + assertThat(rules, not(empty())); + assertThat("Expected all suppressions in hosted suppressions snapshot file to be marked as base", allRulesNotMarkedAsBase(rules), empty()); + } - /** - * Assert that rules are loaded from multiple files if multiple files are - * defined in the {@link Settings}. - */ - @Test - void testGetRulesFromMultipleSuppressionFiles() throws Exception { - final int rulesInCoreFile = getNumberOfRulesLoadedInCoreFile(); - - // GIVEN suppression rules from one file - final int rulesInFirstFile = getNumberOfRulesLoadedFromPath(SUPPRESSIONS_FILE) - rulesInCoreFile; - - // AND suppression rules from another file - final int rulesInSecondFile = getNumberOfRulesLoadedFromPath(OTHER_SUPPRESSIONS_FILE) - rulesInCoreFile; - - // WHEN initializing with both suppression files - final String[] suppressionFiles = {SUPPRESSIONS_FILE, OTHER_SUPPRESSIONS_FILE}; - getSettings().setArrayIfNotEmpty(KEYS.SUPPRESSION_FILE, suppressionFiles); - instance.initialize(getSettings()); - Engine engine = new Engine(getSettings()); - instance.prepare(engine); - - // THEN rules from both files were loaded - final int expectedSize = rulesInFirstFile + rulesInSecondFile + rulesInCoreFile; - assertThat("Expected suppressions from both files", instance.getRuleCount(engine), is(expectedSize)); + @Test + void packagedSnapshotHostedSuppressionsLoadedIfAutoUpdateDisabled() throws Exception { + getSettings().setBoolean(KEYS.HOSTED_SUPPRESSIONS_ENABLED, true); + getSettings().setBoolean(KEYS.AUTO_UPDATE, false); + engine = prepareHostedSuppressionsOnly(); + assertThat(AbstractSuppressionAnalyzer.getRuleCount(engine), greaterThan(0)); + } + @Test + void packagedSnapshotHostedSuppressionsLoadedIfRemoteHostedSuppressionsFail() throws Exception { + getSettings().setBoolean(KEYS.HOSTED_SUPPRESSIONS_ENABLED, true); + getSettings().setBoolean(KEYS.AUTO_UPDATE, true); + getSettings().setString(KEYS.HOSTED_SUPPRESSIONS_URL, "file:///does-not-exist.xml"); + engine = prepareHostedSuppressionsOnly(); + assertThat(AbstractSuppressionAnalyzer.getRuleCount(engine), greaterThan(0)); + } + + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void ignoresHostedSuppressionsIfUrlDoesntIncludeAFileNameRegardlessOfEnabledState(boolean hostedSuppressionsEnabled) throws Exception { + getSettings().setBoolean(KEYS.HOSTED_SUPPRESSIONS_ENABLED, hostedSuppressionsEnabled); + getSettings().setString(KEYS.HOSTED_SUPPRESSIONS_URL, "https://valid.url.but.no.file/"); + engine = prepareHostedSuppressionsOnly(); + assertThat(AbstractSuppressionAnalyzer.getRuleCount(engine), is(0)); + } + + @Test + void ignoresHostedSuppressionsIfCannotBeParsedFromRemote() throws Exception { + getSettings().setBoolean(KEYS.HOSTED_SUPPRESSIONS_ENABLED, true); + getSettings().setString(KEYS.HOSTED_SUPPRESSIONS_URL, BaseTest.getResourceAsUrlString(this, "suppressions-invalid.xml")); + engine = prepareHostedSuppressionsOnly(); + assertThat(AbstractSuppressionAnalyzer.getRuleCount(engine), is(0)); + } + + @Test + void prefersRemoteHostedSuppressionsIfEnabled() throws Exception { + getSettings().setBoolean(KEYS.HOSTED_SUPPRESSIONS_ENABLED, true); + getSettings().setBoolean(KEYS.AUTO_UPDATE, true); + getSettings().setString(KEYS.HOSTED_SUPPRESSIONS_URL, testSuppressionsFileUrl()); + engine = prepareHostedSuppressionsOnly(); + assertThat(AbstractSuppressionAnalyzer.getRuleCount(engine), is(TEST_SUPPRESSIONS_EXPECTED_COUNT)); + } + + @Test + void prefersRemoteHostedSuppressionsIfEnabledAndForced() throws Exception { + getSettings().setBoolean(KEYS.HOSTED_SUPPRESSIONS_ENABLED, true); + getSettings().setBoolean(KEYS.AUTO_UPDATE, false); + getSettings().setBoolean(KEYS.HOSTED_SUPPRESSIONS_FORCEUPDATE, true); + getSettings().setString(KEYS.HOSTED_SUPPRESSIONS_URL, testSuppressionsFileUrl()); + engine = prepareHostedSuppressionsOnly(); + assertThat(AbstractSuppressionAnalyzer.getRuleCount(engine), is(TEST_SUPPRESSIONS_EXPECTED_COUNT)); + } } - @Test - void testFailureToLocateSuppressionFileAnywhere() { - getSettings().setString(Settings.KEYS.SUPPRESSION_FILE, "doesnotexist.xml"); - instance.initialize(getSettings()); - Engine engine = new Engine(Mode.EVIDENCE_COLLECTION, getSettings()); - assertThrows(InitializationException.class, () -> - instance.prepare(engine)); + @Nested + class UserSuppressionsLoading { + /** + * Test of getRules method, of class AbstractSuppressionAnalyzer for + * suppression file declared as URL. + */ + @Test + void testGetRulesFromSuppressionFileFromURL() throws Exception { + final int numberOfExtraLoadedRules = getNumberOfRulesLoadedFromPath(testSuppressionsFileUrl()) - getNumberOfRulesLoadedInCoreFile(); + assertEquals(TEST_SUPPRESSIONS_EXPECTED_COUNT, numberOfExtraLoadedRules, "Wrong # of expected extra user suppression rules"); + } + + /** + * Test of getRules method, of class AbstractSuppressionAnalyzer for + * suppression file on the class path. + */ + @Test + void testGetRulesFromSuppressionFileInClasspath() throws Exception { + final int numberOfExtraLoadedRules = getNumberOfRulesLoadedFromPath(TEST_SUPPRESSIONS_FILE) - getNumberOfRulesLoadedInCoreFile(); + assertEquals(TEST_SUPPRESSIONS_EXPECTED_COUNT, numberOfExtraLoadedRules, "Wrong # of expected extra user suppression rules"); + } + + /** + * Assert that rules are loaded from multiple files if multiple files are + * defined in the {@link Settings}. + */ + @Test + void testGetRulesFromMultipleSuppressionFiles() throws Exception { + final int rulesInCoreFile = getNumberOfRulesLoadedInCoreFile(); + + // GIVEN suppression rules from one file + final int rulesInFirstFile = getNumberOfRulesLoadedFromPath(TEST_SUPPRESSIONS_FILE) - rulesInCoreFile; + + // AND suppression rules from another file + final int rulesInSecondFile = getNumberOfRulesLoadedFromPath(OTHER_TEST_SUPPRESSIONS_FILE) - rulesInCoreFile; + + // WHEN initializing with both suppression files + final String[] suppressionFiles = {TEST_SUPPRESSIONS_FILE, OTHER_TEST_SUPPRESSIONS_FILE}; + getSettings().setArrayIfNotEmpty(KEYS.SUPPRESSION_FILE, suppressionFiles); + engine = new Engine(getSettings()); + newAnalyzer().prepare(engine); + + // THEN rules from both files were loaded + final int expectedSize = rulesInFirstFile + rulesInSecondFile + rulesInCoreFile; + assertThat("Expected suppressions from both files", AbstractSuppressionAnalyzer.getRuleCount(engine), is(expectedSize)); + } + + @Test + void testFailureToLocateSuppressionFileAnywhere() { + getSettings().setString(Settings.KEYS.SUPPRESSION_FILE, "doesnotexist.xml"); + engine = new Engine(Mode.EVIDENCE_COLLECTION, getSettings()); + assertThrows(InitializationException.class, () -> newAnalyzer().prepare(engine)); + } } /** @@ -142,7 +217,7 @@ void testFailureToLocateSuppressionFileAnywhere() { */ private int getNumberOfRulesLoadedInCoreFile() throws Exception { getSettings().removeProperty(KEYS.SUPPRESSION_FILE); - Engine engine = prepareSuppressions(); + engine = prepareSuppressions(); return AbstractSuppressionAnalyzer.getRuleCount(engine); } @@ -156,70 +231,54 @@ private int getNumberOfRulesLoadedInCoreFile() throws Exception { */ private int getNumberOfRulesLoadedFromPath(final String path) throws Exception { getSettings().setString(KEYS.SUPPRESSION_FILE, path); - Engine engine = prepareSuppressions(); + engine = prepareSuppressions(); return AbstractSuppressionAnalyzer.getRuleCount(engine); } private @NonNull Engine prepareSuppressions() throws InvalidSettingException, InitializationException { - final AbstractSuppressionAnalyzerImpl fileAnalyzer = new AbstractSuppressionAnalyzerImpl(); - fileAnalyzer.initialize(getSettings()); - Downloader.getInstance().configure(getSettings()); - Engine engine = new Engine(Mode.EVIDENCE_COLLECTION, getSettings()); - fileAnalyzer.prepare(engine); + engine = new Engine(Mode.EVIDENCE_COLLECTION, getSettings()); + newAnalyzer().prepare(engine); return engine; } - @Nested - class CoreSuppressionsLoading { - @Test - void testLoadCorePackagedSuppressions() throws Exception { - List baseRules = assertAllBaseSuppressionRulesAreMarkedCorrectly(); - - assertAllHostedSnapshotSuppressionsAreMarkedAsBase(baseRules); - } - - private @NonNull List assertAllBaseSuppressionRulesAreMarkedCorrectly() throws InvalidSettingException, InitializationException { - getSettings().setBoolean(KEYS.HOSTED_SUPPRESSIONS_ENABLED, false); - Engine engine = prepareSuppressions(); - - @SuppressWarnings("unchecked") List baseRules = (List) engine.getObject(SUPPRESSION_OBJECT_KEY); - assertThat(baseRules, not(empty())); - assertThat("Expected all suppressions in base file to be marked as base", allRulesNotMarkedAsBase(baseRules), empty()); - return baseRules; - } - - private void assertAllHostedSnapshotSuppressionsAreMarkedAsBase(List baseRules) throws InvalidSettingException, InitializationException { - getSettings().setBoolean(KEYS.HOSTED_SUPPRESSIONS_ENABLED, true); - getSettings().setString(KEYS.HOSTED_SUPPRESSIONS_URL, "https://intentionally-bad-url/hosted-suppressions.xml"); - Engine engine = prepareSuppressions(); + private @NonNull Engine prepareBaseSuppressionsOnly() throws InvalidSettingException, SuppressionParseException { + engine = new Engine(Mode.EVIDENCE_COLLECTION, getSettings()); + newAnalyzer().loadPackagedBaseSuppressionData(engine); + return engine; + } - @SuppressWarnings("unchecked") List allRules = (List) engine.getObject(SUPPRESSION_OBJECT_KEY); + private @NonNull Engine prepareHostedSuppressionsOnly() throws InvalidSettingException { + engine = new Engine(Mode.EVIDENCE_COLLECTION, getSettings()); + newAnalyzer().loadHostedSuppressionBaseData(engine); + return engine; + } - List hostedSnapshotRules = allRules.stream().filter(r -> !baseRules.contains(r)).collect(Collectors.toList()); - assertThat(hostedSnapshotRules, not(empty())); - assertThat("Expected all suppressions in hosted suppressions snapshot file to be marked as base", allRulesNotMarkedAsBase(hostedSnapshotRules), empty()); - } + private @NonNull AbstractSuppressionAnalyzerImpl newAnalyzer() throws InvalidSettingException { + final AbstractSuppressionAnalyzerImpl fileAnalyzer = new AbstractSuppressionAnalyzerImpl(); + fileAnalyzer.initialize(getSettings()); + Downloader.getInstance().configure(getSettings()); + return fileAnalyzer; + } - private @NonNull List allRulesNotMarkedAsBase(List baseRules) { - return baseRules.stream().filter(r -> !r.isBase()).collect(Collectors.toList()); - } + private @NonNull List allRulesNotMarkedAsBase(List baseRules) { + return baseRules.stream().filter(r -> !r.isBase()).collect(Collectors.toList()); } public static class AbstractSuppressionAnalyzerImpl extends AbstractSuppressionAnalyzer { @Override public void analyzeDependency(Dependency dependency, Engine engine) { - throw new UnsupportedOperationException("Not supported yet."); //To change body of generated methods, choose Tools | Templates. + throw new UnsupportedOperationException("Not supported yet."); } @Override public String getName() { - throw new UnsupportedOperationException("Not supported yet."); //To change body of generated methods, choose Tools | Templates. + throw new UnsupportedOperationException("Not supported yet."); } @Override public AnalysisPhase getAnalysisPhase() { - throw new UnsupportedOperationException("Not supported yet."); //To change body of generated methods, choose Tools | Templates. + throw new UnsupportedOperationException("Not supported yet."); } @Override diff --git a/core/src/test/java/org/owasp/dependencycheck/data/update/HostedSuppressionsDataSourceTest.java b/core/src/test/java/org/owasp/dependencycheck/data/update/HostedSuppressionsDataSourceTest.java new file mode 100644 index 00000000000..a65957ebf9d --- /dev/null +++ b/core/src/test/java/org/owasp/dependencycheck/data/update/HostedSuppressionsDataSourceTest.java @@ -0,0 +1,225 @@ +/* + * This file is part of dependency-check-core. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * Copyright (c) 2026 Chad Wilson. All Rights Reserved. + */ +package org.owasp.dependencycheck.data.update; + +import org.jspecify.annotations.NonNull; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Disabled; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; +import org.owasp.dependencycheck.BaseTest; +import org.owasp.dependencycheck.Engine; +import org.owasp.dependencycheck.data.update.exception.UpdateException; +import org.owasp.dependencycheck.utils.InvalidSettingException; +import org.owasp.dependencycheck.utils.Settings; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.attribute.FileTime; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.instanceOf; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.not; +import static org.junit.jupiter.api.Assertions.assertThrowsExactly; + +class HostedSuppressionsDataSourceTest extends BaseTest { + + private static final String TEST_SUPPRESSIONS_FILE = "suppressions.xml"; + + private HostedSuppressionsDataSource dataSource; + private Engine engine; + + @BeforeEach + public void createEngine() { + dataSource = new HostedSuppressionsDataSource(); + engine = new Engine(Engine.Mode.EVIDENCE_COLLECTION, getSettings()); + } + + @AfterEach + void closeEngine() { + if (engine != null) { + dataSource.purge(engine); + engine.close(); + } + } + + @Nested + class Update { + @Test + void doesNothingIfRemoteHostedSuppressionsDisabled() throws Exception { + getSettings().setBoolean(Settings.KEYS.HOSTED_SUPPRESSIONS_ENABLED, false); + dataSource.update(engine); + assertNoCachedHostedSuppressions(); + } + + @Test + void doesNothingIfNoSuppressionAnalyzersEnabled() throws Exception { + getSettings().setBoolean(Settings.KEYS.HOSTED_SUPPRESSIONS_ENABLED, true); + getSettings().setBoolean(Settings.KEYS.ANALYZER_VULNERABILITY_SUPPRESSION_ENABLED, false); + getSettings().setBoolean(Settings.KEYS.ANALYZER_CPE_SUPPRESSION_ENABLED, false); + dataSource.update(engine); + assertNoCachedHostedSuppressions(); + } + + @Test + void doesNothingIfAutoUpdateDisabled() throws Exception { + getSettings().setBoolean(Settings.KEYS.HOSTED_SUPPRESSIONS_ENABLED, true); + getSettings().setBoolean(Settings.KEYS.AUTO_UPDATE, false); + getSettings().setBoolean(Settings.KEYS.HOSTED_SUPPRESSIONS_FORCEUPDATE, false); + dataSource.update(engine); + assertNoCachedHostedSuppressions(); + } + + @Test + void ignoresHostedSuppressionsIfRemoteHostedSuppressionsFail() throws Exception { + getSettings().setBoolean(Settings.KEYS.HOSTED_SUPPRESSIONS_ENABLED, true); + getSettings().setBoolean(Settings.KEYS.AUTO_UPDATE, true); + getSettings().setString(Settings.KEYS.HOSTED_SUPPRESSIONS_URL, "file:///does-not-exist.xml"); + dataSource.update(engine); + assertNoCachedHostedSuppressions(); + } + + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void failsIfSuppressionsUrlDoesntIncludeAFileNameRegardlessOfEnabledState(boolean hostedSuppressionsEnabled) throws Exception { + getSettings().setBoolean(Settings.KEYS.HOSTED_SUPPRESSIONS_ENABLED, hostedSuppressionsEnabled); + getSettings().setString(Settings.KEYS.HOSTED_SUPPRESSIONS_URL, "https://valid.url.but.no.file/"); + var ex = assertThrowsExactly(UpdateException.class, () -> dataSource.update(engine)); + assertThat(ex.getMessage(), containsString("Unable to determine the local location to cache hosted suppressions")); + assertThat(ex.getCause(), instanceOf(InvalidSettingException.class)); + assertThat(ex.getCause().getMessage(), containsString("Hosted Suppression URL must imply a filename")); + } + + @Test + void failsIfSuppressionsUrlIsMalformed() { + getSettings().setBoolean(Settings.KEYS.HOSTED_SUPPRESSIONS_ENABLED, true); + getSettings().setString(Settings.KEYS.HOSTED_SUPPRESSIONS_URL, "bad-url"); + var ex = assertThrowsExactly(UpdateException.class, () -> dataSource.update(engine)); + assertThat(ex.getMessage(), containsString("Unable to determine the local location to cache hosted suppressions")); + assertThat(ex.getCause(), instanceOf(InvalidSettingException.class)); + assertThat(ex.getCause().getMessage(), containsString("Invalid URL for Hosted Suppressions")); + } + + @Test + void loadsRemoteHostedSuppressionsIfEnabledAndForced() throws Exception { + getSettings().setBoolean(Settings.KEYS.HOSTED_SUPPRESSIONS_ENABLED, true); + getSettings().setBoolean(Settings.KEYS.AUTO_UPDATE, false); + getSettings().setBoolean(Settings.KEYS.HOSTED_SUPPRESSIONS_FORCEUPDATE, true); + getSettings().setString(Settings.KEYS.HOSTED_SUPPRESSIONS_URL, testSuppressionsFileUrl()); + dataSource.update(engine); + assertThat(Files.readString(cachedRepoFile()), is(testSuppressionsFileContent())); + assertThat(Files.exists(cachedRepoFileProperties()), is(true)); + } + + @Test + void loadsRemoteHostedSuppressionsIfEnabledWithAutoUpdate() throws Exception { + getSettings().setBoolean(Settings.KEYS.HOSTED_SUPPRESSIONS_ENABLED, true); + getSettings().setBoolean(Settings.KEYS.AUTO_UPDATE, true); + getSettings().setString(Settings.KEYS.HOSTED_SUPPRESSIONS_URL, testSuppressionsFileUrl()); + dataSource.update(engine); + assertThat(Files.readString(cachedRepoFile()), is(testSuppressionsFileContent())); + assertThat(Files.exists(cachedRepoFileProperties()), is(true)); + } + + @Test + @Disabled("stale time calculation is broken") + void doesNothingIfRemoteHostedSuppressionsIsNotStale() throws Exception { + getSettings().setBoolean(Settings.KEYS.HOSTED_SUPPRESSIONS_ENABLED, true); + getSettings().setBoolean(Settings.KEYS.AUTO_UPDATE, true); + getSettings().setString(Settings.KEYS.HOSTED_SUPPRESSIONS_URL, testSuppressionsFileUrl()); + getSettings().setInt(Settings.KEYS.HOSTED_SUPPRESSIONS_VALID_FOR_HOURS, 1); + dataSource.update(engine); + + // Update again immediately + String firstUpdateProperties = Files.readString(cachedRepoFileProperties()); + FileTime firstUpdatePropertiesModified = Files.getLastModifiedTime(cachedRepoFileProperties()); + dataSource.update(engine); + + assertThat(Files.readString(cachedRepoFileProperties()), is(firstUpdateProperties)); + assertThat(Files.getLastModifiedTime(cachedRepoFileProperties()), is(firstUpdatePropertiesModified)); + } + + @Test + void reloadsRemoteHostedSuppressionsIfStale() throws Exception { + getSettings().setBoolean(Settings.KEYS.HOSTED_SUPPRESSIONS_ENABLED, true); + getSettings().setBoolean(Settings.KEYS.AUTO_UPDATE, true); + getSettings().setString(Settings.KEYS.HOSTED_SUPPRESSIONS_URL, testSuppressionsFileUrl()); + getSettings().setInt(Settings.KEYS.HOSTED_SUPPRESSIONS_VALID_FOR_HOURS, 0); + dataSource.update(engine); + + // Reset to force an update + Files.writeString(cachedRepoFile(), "stale content"); + + dataSource.update(engine); + assertThat(Files.readString(cachedRepoFile()), not("stale content")); + } + } + + @Nested + class Purge { + @Test + void purgeRemovesCachedFiles() throws Exception { + getSettings().setBoolean(Settings.KEYS.HOSTED_SUPPRESSIONS_ENABLED, true); + getSettings().setBoolean(Settings.KEYS.AUTO_UPDATE, true); + getSettings().setString(Settings.KEYS.HOSTED_SUPPRESSIONS_URL, testSuppressionsFileUrl()); + dataSource.update(engine); + + assertThat(Files.exists(cachedRepoFile()), is(true)); + dataSource.purge(engine); + assertThat(Files.exists(cachedRepoFile()), is(false)); + } + + @Test + void doesNothingIfNoCachedFile() throws Exception { + dataSource.purge(engine); + assertThat(Files.exists(cachedRepoFile()), is(false)); + } + + @Test + void doesNothingIfSuppressionsUrlIsMalformed() { + getSettings().setString(Settings.KEYS.HOSTED_SUPPRESSIONS_URL, "bad-url"); + dataSource.purge(engine); + } + } + + private @NonNull Path cachedRepoFile() throws IOException { + return dataSource.validatedRepoFile().toPath(); + } + + private @NonNull Path cachedRepoFileProperties() throws IOException { + return Path.of(cachedRepoFile() + ".properties"); + } + + private @NonNull String testSuppressionsFileUrl() { + return BaseTest.getResourceAsUrlString(this, TEST_SUPPRESSIONS_FILE); + } + + private @NonNull String testSuppressionsFileContent() { + return BaseTest.getResourceAsContentString(this, TEST_SUPPRESSIONS_FILE); + } + + private void assertNoCachedHostedSuppressions() throws IOException { + assertThat("hosted suppression repo file should not exist", dataSource.validatedRepoFile().exists(), is(false)); + } +} diff --git a/core/src/test/resources/suppressions-invalid.xml b/core/src/test/resources/suppressions-invalid.xml new file mode 100644 index 00000000000..efd7b9ead1e --- /dev/null +++ b/core/src/test/resources/suppressions-invalid.xml @@ -0,0 +1,7 @@ + + + + \ No newline at end of file diff --git a/maven/src/main/java/org/owasp/dependencycheck/maven/BaseDependencyCheckMojo.java b/maven/src/main/java/org/owasp/dependencycheck/maven/BaseDependencyCheckMojo.java index ae24ca6af9c..43b40496ba2 100644 --- a/maven/src/main/java/org/owasp/dependencycheck/maven/BaseDependencyCheckMojo.java +++ b/maven/src/main/java/org/owasp/dependencycheck/maven/BaseDependencyCheckMojo.java @@ -1158,7 +1158,7 @@ public abstract class BaseDependencyCheckMojo extends AbstractMojo implements Ma @Parameter(property = "hostedSuppressionsForceUpdate") private Boolean hostedSuppressionsForceUpdate; /** - * Whether the hosted suppressions file will be used. + * Whether the hosted suppressions will be updated from the configured URL. */ @SuppressWarnings("CanBeFinal") @Parameter(property = "hostedSuppressionsEnabled") diff --git a/maven/src/site/markdown/configuration.md b/maven/src/site/markdown/configuration.md index ec9f299d6eb..8bbb43f6325 100644 --- a/maven/src/site/markdown/configuration.md +++ b/maven/src/site/markdown/configuration.md @@ -175,7 +175,7 @@ Note that any passwords in the below configuration could be exposed if you use ` | serverId | The id of a server defined in the settings.xml; this can be used to encrypt the database password. See [password encryption](http://maven.apache.org/guides/mini/guide-encryption.html) for more information. |   | | databaseUser | The username used when connecting to the database. |   | | databasePassword | The password used when connecting to the database. |   | -| hostedSuppressionsEnabled | Whether the hosted suppressions file will be used. | true | +| hostedSuppressionsEnabled | Whether the hosted suppressions will be updated from the configured URL. | true | | hostedSuppressionsForceUpdate | Whether the hosted suppressions file will update regardless of the `autoupdate` setting. | false | | hostedSuppressionsUrl | The URL to a mirrored copy of the hosted suppressions file for internet-constrained environments. | https://dependency-check.github.io/DependencyCheck/suppressions/publishedSuppressions.xml | | hostedSuppressionsValidForHours | Sets the number of hours to wait before checking for new updates of the hosted suppressions file. | 2 | diff --git a/src/site/markdown/dependency-check-gradle/configuration-aggregate.md b/src/site/markdown/dependency-check-gradle/configuration-aggregate.md index f1f658d8ae8..2e50a9ece2b 100644 --- a/src/site/markdown/dependency-check-gradle/configuration-aggregate.md +++ b/src/site/markdown/dependency-check-gradle/configuration-aggregate.md @@ -90,7 +90,7 @@ The following properties can be configured in the dependencyCheck task. However, | data | password | The password used when connecting to the database. |   | | slack | enabled | Whether or not slack notifications are enabled. | false | | slack | webhookUrl | The custom incoming webhook URL to receive notifications. Note that the current implementation only notifies about build failures so this should be used in combination with failBuildOnCVSS. |   | -| hostedSuppressions | enabled | Whether the hosted suppressions file will be used. | true | +| hostedSuppressions | enabled | Whether the hosted suppressions will be updated from the configured URL. | true | | hostedSuppressions | forceupdate | Sets whether hosted suppressions file will update regardless of the `autoupdate` setting. | false | | hostedSuppressions | url | The URL to a mirrored copy of the hosted suppressions file for internet-constrained environments. | https://dependency-check.github.io/DependencyCheck/suppressions/publishedSuppressions.xml | | hostedSuppressions | user | Credentials used for basic authentication for the hosted suppressions file. |   | diff --git a/src/site/markdown/dependency-check-gradle/configuration-update.md b/src/site/markdown/dependency-check-gradle/configuration-update.md index 17d6fc9ca42..2406e3123c7 100644 --- a/src/site/markdown/dependency-check-gradle/configuration-update.md +++ b/src/site/markdown/dependency-check-gradle/configuration-update.md @@ -62,7 +62,7 @@ The following properties can be configured in the dependencyCheck task. However, | data | connectionString | The connection string used to connect to the database. See using a [database server](../data/database.html). |   | | data | username | The username used when connecting to the database. |   | | data | password | The password used when connecting to the database. |   | -| hostedSuppressions | enabled | Whether the hosted suppressions file will be used. | true | +| hostedSuppressions | enabled | Whether the hosted suppressions will be updated from the configured URL. | true | | hostedSuppressions | forceupdate | Sets whether hosted suppressions file will update regardless of the `autoupdate` setting. | false | | hostedSuppressions | url | The URL to (a mirror of) the hosted suppressions file. | https://dependency-check.github.io/DependencyCheck/suppressions/publishedSuppressions.xml | | hostedSuppressions | user | Credentials used for basic authentication for the hosted suppressions file. |   | diff --git a/src/site/markdown/dependency-check-gradle/configuration.md b/src/site/markdown/dependency-check-gradle/configuration.md index 4c93f3e753c..36768686dd4 100644 --- a/src/site/markdown/dependency-check-gradle/configuration.md +++ b/src/site/markdown/dependency-check-gradle/configuration.md @@ -90,7 +90,7 @@ The following properties can be configured in the dependencyCheck task. However, | data | password | The password used when connecting to the database. |   | | slack | enabled | Whether or not slack notifications are enabled. | false | | slack | webhookUrl | The custom incoming webhook URL to receive notifications. Note that the current implementation only notifies about build failures so this should be used in combination with failBuildOnCVSS. |   | -| hostedSuppressions | enabled | Whether the hosted suppressions file will be used. | true | +| hostedSuppressions | enabled | Whether the hosted suppressions will be updated from the configured URL. | true | | hostedSuppressions | forceupdate | Sets whether hosted suppressions file will update regardless of the `autoupdate` setting. | false | | hostedSuppressions | url | The URL to a mirrored copy of the hosted suppressions file for internet-constrained environments. | https://dependency-check.github.io/DependencyCheck/suppressions/publishedSuppressions.xml | | hostedSuppressions | user | Credentials used for basic authentication for the hosted suppressions file. |   | From 0b55b57dda789cab35f7a3ca9f7e3529ea875d38 Mon Sep 17 00:00:00 2001 From: Chad Wilson <29788154+chadlwilson@users.noreply.github.com> Date: Fri, 8 May 2026 02:23:46 +0800 Subject: [PATCH 2/5] fix: correct stale/shouldUpdate checks for LocalDataSources After the switch from DB storage the logic was storing seconds since epoch, but comparing to milliseconds, so files were always considered stale, causing excessive updates. Signed-off-by: Chad Wilson <29788154+chadlwilson@users.noreply.github.com> --- .../update/HostedSuppressionsDataSource.java | 32 ++-------- .../data/update/LocalDataSource.java | 46 +++++++++++---- .../data/update/RetireJSDataSource.java | 58 +++++-------------- .../HostedSuppressionsDataSourceTest.java | 2 - 4 files changed, 53 insertions(+), 85 deletions(-) diff --git a/core/src/main/java/org/owasp/dependencycheck/data/update/HostedSuppressionsDataSource.java b/core/src/main/java/org/owasp/dependencycheck/data/update/HostedSuppressionsDataSource.java index b05dd28f267..70c9d94f8a9 100644 --- a/core/src/main/java/org/owasp/dependencycheck/data/update/HostedSuppressionsDataSource.java +++ b/core/src/main/java/org/owasp/dependencycheck/data/update/HostedSuppressionsDataSource.java @@ -35,6 +35,7 @@ import java.net.MalformedURLException; import java.net.URL; import java.nio.file.Files; +import java.time.Duration; public class HostedSuppressionsDataSource extends LocalDataSource { /** @@ -93,7 +94,7 @@ public void updateUnhandled(Engine engine) throws IOException, UpdateException { if (isEnabled() && shouldUpdateFromRemote(repoFile)) { LOGGER.debug("Begin Hosted Suppressions file update from remote source"); fetchHostedSuppressions(url, repoFile); - saveLastUpdated(repoFile, System.currentTimeMillis() / 1000); + saveLastUpdated(repoFile); } } @@ -127,33 +128,8 @@ private boolean isEnabled() { private boolean shouldUpdateFromRemote(File repoFile) { boolean forceupdate = settings.getBoolean(Settings.KEYS.HOSTED_SUPPRESSIONS_FORCEUPDATE, false); boolean autoupdate = settings.getBoolean(Settings.KEYS.AUTO_UPDATE, true); - return forceupdate || (autoupdate && isStale(repoFile)); - } - - /** - * Determines if the we should update the Hosted Suppressions file. - * - * @param repo the Hosted Suppressions file. - * @return true if an update to the Hosted Suppressions file - * should be performed; otherwise false - * @throws NumberFormatException thrown if an invalid value is contained in - * the database properties - */ - protected boolean isStale(File repo) throws NumberFormatException { - boolean stale = true; - if (repo != null && repo.isFile()) { - final int validForHours = settings.getInt(Settings.KEYS.HOSTED_SUPPRESSIONS_VALID_FOR_HOURS, 2); - final long lastUpdatedOn = getLastUpdated(repo); - final long now = System.currentTimeMillis(); - LOGGER.debug("Last updated: {}", lastUpdatedOn); - LOGGER.debug("Now: {}", now); - final long msValid = validForHours * 60L * 60L * 1000L; - stale = (now - lastUpdatedOn) > msValid; - if (!stale) { - LOGGER.info("Skipping Hosted Suppressions file update since last update was within {} hours.", validForHours); - } - } - return stale; + Duration validFor = Duration.ofHours(settings.getInt(Settings.KEYS.HOSTED_SUPPRESSIONS_VALID_FOR_HOURS, 2)); + return forceupdate || (autoupdate && isStale(repoFile, validFor)); } /** diff --git a/core/src/main/java/org/owasp/dependencycheck/data/update/LocalDataSource.java b/core/src/main/java/org/owasp/dependencycheck/data/update/LocalDataSource.java index 88d51840668..a258d968af9 100644 --- a/core/src/main/java/org/owasp/dependencycheck/data/update/LocalDataSource.java +++ b/core/src/main/java/org/owasp/dependencycheck/data/update/LocalDataSource.java @@ -17,15 +17,18 @@ */ package org.owasp.dependencycheck.data.update; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import java.io.File; import java.io.FileInputStream; import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.time.Duration; +import java.time.Instant; import java.util.Properties; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; /** * @@ -39,16 +42,15 @@ public abstract class LocalDataSource implements CachedWebDataSource { private static final Logger LOGGER = LoggerFactory.getLogger(LocalDataSource.class); /** - * Saves the timestamp in a properties file next to the provided repo file + * Saves the timestamp in a properties file adjacent to the provided repo file * * @param repo the local file data source - * @param timestamp the epoch timestamp to store */ - protected void saveLastUpdated(File repo, long timestamp) { + protected void saveLastUpdated(@NonNull File repo) { final File timestampFile = new File(repo + ".properties"); try (OutputStream out = new FileOutputStream(timestampFile)) { final Properties prop = new Properties(); - prop.setProperty("LAST_UPDATED", String.valueOf(timestamp)); + prop.setProperty("LAST_UPDATED", String.valueOf(System.currentTimeMillis())); prop.store(out, null); } catch (IOException ex) { throw new RuntimeException(ex); @@ -60,24 +62,46 @@ protected void saveLastUpdated(File repo, long timestamp) { * next to the repo file). * * @param repo the local file data source - * @return the epoch timestamp of the last updated date/time + * @return the instant of the last updated date/time */ - protected long getLastUpdated(File repo) { + protected Instant getLastUpdated(@NonNull File repo) { long lastUpdatedOn = 0; final File timestampFile = new File(repo + ".properties"); if (timestampFile.isFile()) { try (InputStream is = new FileInputStream(timestampFile)) { final Properties props = new Properties(); props.load(is); - lastUpdatedOn = Integer.parseInt(props.getProperty("LAST_UPDATED", "0")); + lastUpdatedOn = Long.parseLong(props.getProperty("LAST_UPDATED", "0")); } catch (IOException | NumberFormatException ex) { LOGGER.debug("error reading timestamp file", ex); } if (lastUpdatedOn <= 0) { - //fall back on conversion from file last modified to storing in the db. + //fall back on conversion from file last modified lastUpdatedOn = repo.lastModified(); } } - return lastUpdatedOn; + return Instant.ofEpochMilli(lastUpdatedOn); + } + + /** + * Determines if we should update the local data source. + * + * @param repo the local file data source + * @param validFor the duration for which the local data source should be considered valid + * @return true if an update to the data source should be performed; otherwise false. + * If the repo does not exist, or is an empty file, it is considered stale. + */ + protected boolean isStale(File repo, Duration validFor) { + boolean stale = true; + if (repo != null && repo.isFile()) { + final Instant lastUpdatedOn = getLastUpdated(repo); + final Instant now = Instant.now(); + LOGGER.debug("{} last updated: {}, now: {}", getClass().getSimpleName(), lastUpdatedOn, now); + stale = lastUpdatedOn.plus(validFor).isBefore(now); + if (!stale) { + LOGGER.info("Should skip {} update since last update was within period {}.", getClass().getSimpleName(), validFor); + } + } + return stale; } } diff --git a/core/src/main/java/org/owasp/dependencycheck/data/update/RetireJSDataSource.java b/core/src/main/java/org/owasp/dependencycheck/data/update/RetireJSDataSource.java index 7dcbf994b0d..20434830bfe 100644 --- a/core/src/main/java/org/owasp/dependencycheck/data/update/RetireJSDataSource.java +++ b/core/src/main/java/org/owasp/dependencycheck/data/update/RetireJSDataSource.java @@ -17,12 +17,6 @@ */ package org.owasp.dependencycheck.data.update; -import java.io.File; -import java.io.IOException; -import java.net.MalformedURLException; -import java.net.URL; -import javax.annotation.concurrent.ThreadSafe; - import org.owasp.dependencycheck.Engine; import org.owasp.dependencycheck.data.update.exception.UpdateException; import org.owasp.dependencycheck.exception.WriteLockException; @@ -34,6 +28,13 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import javax.annotation.concurrent.ThreadSafe; +import java.io.File; +import java.io.IOException; +import java.net.MalformedURLException; +import java.net.URL; +import java.time.Duration; + /** * Downloads a local copy of the RetireJS repository. * @@ -41,23 +42,18 @@ */ @ThreadSafe public class RetireJSDataSource extends LocalDataSource { - /** - * Static logger. + * The default URL to the RetireJS JavaScript repository. */ - private static final Logger LOGGER = LoggerFactory.getLogger(RetireJSDataSource.class); + public static final String DEFAULT_JS_URL = "https://raw.githubusercontent.com/Retirejs/retire.js/master/repository/jsrepository.json"; /** - * The property key indicating when the last update occurred. + * Static logger. */ - public static final String RETIREJS_UPDATED_ON = "RetireJSUpdatedOn"; + private static final Logger LOGGER = LoggerFactory.getLogger(RetireJSDataSource.class); /** * The configured settings. */ private Settings settings; - /** - * The default URL to the RetireJS JavaScript repository. - */ - public static final String DEFAULT_JS_URL = "https://raw.githubusercontent.com/Retirejs/retire.js/master/repository/jsrepository.json"; /** * Constructs a new engine version check utility. @@ -79,15 +75,16 @@ public boolean update(Engine engine) throws UpdateException { final boolean autoupdate = settings.getBoolean(Settings.KEYS.AUTO_UPDATE, true); final boolean forceupdate = settings.getBoolean(Settings.KEYS.ANALYZER_RETIREJS_FORCEUPDATE, false); final boolean enabled = settings.getBoolean(Settings.KEYS.ANALYZER_RETIREJS_ENABLED, true); + Duration validFor = Duration.ofHours(settings.getInt(Settings.KEYS.ANALYZER_RETIREJS_REPO_VALID_FOR_HOURS, 24)); try { final URL url = new URL(configuredUrl); final File filepath = new File(url.getPath()); final File repoFile = new File(settings.getDataDirectory(), filepath.getName()); - final boolean proceed = enabled && (forceupdate || (autoupdate && shouldUpdate(repoFile))); + final boolean proceed = enabled && (forceupdate || (autoupdate && isStale(repoFile, validFor))); if (proceed) { LOGGER.debug("Begin RetireJS Update"); initializeRetireJsRepo(settings, url, repoFile); - saveLastUpdated(repoFile, System.currentTimeMillis() / 1000); + saveLastUpdated(repoFile); } } catch (MalformedURLException ex) { throw new UpdateException(String.format("Invalid URL for RetireJS repository (%s)", configuredUrl), ex); @@ -97,33 +94,6 @@ public boolean update(Engine engine) throws UpdateException { return false; } - /** - * Determines if the we should update the RetireJS database. - * - * @param repo the retire JS repository. - * @return true if an updated to the RetireJS database should - * be performed; otherwise false - * @throws NumberFormatException thrown if an invalid value is contained in - * the database properties - */ - protected boolean shouldUpdate(File repo) throws NumberFormatException { - boolean proceed = true; - if (repo != null && repo.isFile()) { - final int validForHours = settings.getInt(Settings.KEYS.ANALYZER_RETIREJS_REPO_VALID_FOR_HOURS, 0); - final long lastUpdatedOn = getLastUpdated(repo); - final long now = System.currentTimeMillis(); - LOGGER.debug("Last updated: {}", lastUpdatedOn); - LOGGER.debug("Now: {}", now); - final long msValid = validForHours * 60L * 60L * 1000L; - proceed = (now - lastUpdatedOn) > msValid; - if (!proceed) { - LOGGER.info("Skipping RetireJS update since last update was within {} hours.", validForHours); - } - } - return proceed; - } - - /** * Initializes the local RetireJS repository * diff --git a/core/src/test/java/org/owasp/dependencycheck/data/update/HostedSuppressionsDataSourceTest.java b/core/src/test/java/org/owasp/dependencycheck/data/update/HostedSuppressionsDataSourceTest.java index a65957ebf9d..530e112df46 100644 --- a/core/src/test/java/org/owasp/dependencycheck/data/update/HostedSuppressionsDataSourceTest.java +++ b/core/src/test/java/org/owasp/dependencycheck/data/update/HostedSuppressionsDataSourceTest.java @@ -20,7 +20,6 @@ import org.jspecify.annotations.NonNull; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; @@ -143,7 +142,6 @@ void loadsRemoteHostedSuppressionsIfEnabledWithAutoUpdate() throws Exception { } @Test - @Disabled("stale time calculation is broken") void doesNothingIfRemoteHostedSuppressionsIsNotStale() throws Exception { getSettings().setBoolean(Settings.KEYS.HOSTED_SUPPRESSIONS_ENABLED, true); getSettings().setBoolean(Settings.KEYS.AUTO_UPDATE, true); From 54c5e6318338c3f8b36723558161f92dcb57f209 Mon Sep 17 00:00:00 2001 From: Chad Wilson <29788154+chadlwilson@users.noreply.github.com> Date: Fri, 8 May 2026 02:29:20 +0800 Subject: [PATCH 3/5] refactor: simplify readability of RetireJSDataSource update logic, similar to Hosted Suppressions Signed-off-by: Chad Wilson <29788154+chadlwilson@users.noreply.github.com> --- .../data/update/RetireJSDataSource.java | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/core/src/main/java/org/owasp/dependencycheck/data/update/RetireJSDataSource.java b/core/src/main/java/org/owasp/dependencycheck/data/update/RetireJSDataSource.java index 20434830bfe..cc6fd8df0f0 100644 --- a/core/src/main/java/org/owasp/dependencycheck/data/update/RetireJSDataSource.java +++ b/core/src/main/java/org/owasp/dependencycheck/data/update/RetireJSDataSource.java @@ -72,16 +72,11 @@ public RetireJSDataSource() { public boolean update(Engine engine) throws UpdateException { this.settings = engine.getSettings(); final String configuredUrl = settings.getString(Settings.KEYS.ANALYZER_RETIREJS_REPO_JS_URL, DEFAULT_JS_URL); - final boolean autoupdate = settings.getBoolean(Settings.KEYS.AUTO_UPDATE, true); - final boolean forceupdate = settings.getBoolean(Settings.KEYS.ANALYZER_RETIREJS_FORCEUPDATE, false); - final boolean enabled = settings.getBoolean(Settings.KEYS.ANALYZER_RETIREJS_ENABLED, true); - Duration validFor = Duration.ofHours(settings.getInt(Settings.KEYS.ANALYZER_RETIREJS_REPO_VALID_FOR_HOURS, 24)); try { final URL url = new URL(configuredUrl); final File filepath = new File(url.getPath()); final File repoFile = new File(settings.getDataDirectory(), filepath.getName()); - final boolean proceed = enabled && (forceupdate || (autoupdate && isStale(repoFile, validFor))); - if (proceed) { + if (isEnabled() && shouldUpdateFromRemote(repoFile)) { LOGGER.debug("Begin RetireJS Update"); initializeRetireJsRepo(settings, url, repoFile); saveLastUpdated(repoFile); @@ -94,6 +89,17 @@ public boolean update(Engine engine) throws UpdateException { return false; } + private boolean isEnabled() { + return settings.getBoolean(Settings.KEYS.ANALYZER_RETIREJS_ENABLED, true); + } + + private boolean shouldUpdateFromRemote(File repoFile) { + boolean forceupdate = settings.getBoolean(Settings.KEYS.ANALYZER_RETIREJS_FORCEUPDATE, false); + boolean autoupdate = settings.getBoolean(Settings.KEYS.AUTO_UPDATE, true); + Duration validFor = Duration.ofHours(settings.getInt(Settings.KEYS.ANALYZER_RETIREJS_REPO_VALID_FOR_HOURS, 24)); + return forceupdate || (autoupdate && isStale(repoFile, validFor)); + } + /** * Initializes the local RetireJS repository * From e453a0e468b30307902291a016461c2f77aad825 Mon Sep 17 00:00:00 2001 From: Chad Wilson <29788154+chadlwilson@users.noreply.github.com> Date: Fri, 8 May 2026 14:49:45 +0800 Subject: [PATCH 4/5] refactor: use common logic for determing non-empty files Signed-off-by: Chad Wilson <29788154+chadlwilson@users.noreply.github.com> --- .../analyzer/AbstractSuppressionAnalyzer.java | 7 +-- .../analyzer/LibmanAnalyzer.java | 4 +- .../analyzer/NodeAuditAnalyzer.java | 4 +- .../analyzer/NodePackageAnalyzer.java | 4 +- .../analyzer/PinnedMavenInstallAnalyzer.java | 8 ++-- .../dependencycheck/analyzer/PipAnalyzer.java | 4 +- .../analyzer/PipfileAnalyzer.java | 4 +- .../analyzer/PipfilelockAnalyzer.java | 4 +- .../analyzer/PnpmAuditAnalyzer.java | 4 +- .../analyzer/YarnAuditAnalyzer.java | 7 +-- .../data/update/LocalDataSource.java | 7 ++- .../data/update/RetireJSDataSource.java | 46 +++++++++++++------ .../AbstractSuppressionAnalyzerTest.java | 35 ++++++-------- .../dependencycheck/utils/FileUtils.java | 24 ++++++---- .../dependencycheck/utils/FileUtilsTest.java | 21 +++++++++ 15 files changed, 122 insertions(+), 61 deletions(-) diff --git a/core/src/main/java/org/owasp/dependencycheck/analyzer/AbstractSuppressionAnalyzer.java b/core/src/main/java/org/owasp/dependencycheck/analyzer/AbstractSuppressionAnalyzer.java index d4210ef9532..a9069c9eb27 100644 --- a/core/src/main/java/org/owasp/dependencycheck/analyzer/AbstractSuppressionAnalyzer.java +++ b/core/src/main/java/org/owasp/dependencycheck/analyzer/AbstractSuppressionAnalyzer.java @@ -54,6 +54,7 @@ import java.util.regex.Pattern; import static org.owasp.dependencycheck.data.update.HostedSuppressionsDataSource.falsePositivesDueTo; +import static org.owasp.dependencycheck.utils.FileUtils.existsWithContent; /** * Abstract base suppression analyzer that contains methods for parsing the @@ -235,7 +236,7 @@ void loadHostedSuppressionBaseData(final Engine engine) { // // Note that this local fallback will run regardless of whether hosted suppressions are "enabled" or the // value of autoUpdate, forceupdate etc since this is an offline operation similar to regular "base" suppressions. - if (isRepoEmpty(repoFile)) { + if (!existsWithContent(repoFile)) { LOGGER.debug("Hosted suppressions not found locally; attempting fallback to store packaged snapshot from this Dependency-Check release at {}...", repoFile.toPath()); URL hostedSuppressionSnapshotURL = getPackagedFile(HOSTED_SUPPRESSION_SNAPSHOT_FILE); try (InputStream in = hostedSuppressionSnapshotURL.openStream()) { @@ -251,10 +252,6 @@ void loadHostedSuppressionBaseData(final Engine engine) { } } - private static boolean isRepoEmpty(File repoFile) { - return !repoFile.isFile() || repoFile.length() <= 1L; - } - /** * If configured to do so, try fetching hosted suppressions from the configured remote source. * @return The local cached repoFile the suppressions are to be loaded from. Note that on return this may still not be created. diff --git a/core/src/main/java/org/owasp/dependencycheck/analyzer/LibmanAnalyzer.java b/core/src/main/java/org/owasp/dependencycheck/analyzer/LibmanAnalyzer.java index 8013e704239..1c946194494 100644 --- a/core/src/main/java/org/owasp/dependencycheck/analyzer/LibmanAnalyzer.java +++ b/core/src/main/java/org/owasp/dependencycheck/analyzer/LibmanAnalyzer.java @@ -47,6 +47,8 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; +import static org.owasp.dependencycheck.utils.FileUtils.existsWithContent; + /** * Analyzer which parses a libman.json file to gather module information. * @@ -161,7 +163,7 @@ public void analyzeDependency(Dependency dependency, Engine engine) throws Analy final File dependencyFile = dependency.getActualFile(); - if (!dependencyFile.isFile() || dependencyFile.length() == 0) { + if (!existsWithContent(dependencyFile)) { return; } diff --git a/core/src/main/java/org/owasp/dependencycheck/analyzer/NodeAuditAnalyzer.java b/core/src/main/java/org/owasp/dependencycheck/analyzer/NodeAuditAnalyzer.java index c6de926ed56..51307bbf5ac 100644 --- a/core/src/main/java/org/owasp/dependencycheck/analyzer/NodeAuditAnalyzer.java +++ b/core/src/main/java/org/owasp/dependencycheck/analyzer/NodeAuditAnalyzer.java @@ -45,6 +45,8 @@ import java.nio.file.Files; import java.util.List; +import static org.owasp.dependencycheck.utils.FileUtils.existsWithContent; + /** * Used to analyze Node Package Manager (npm) package-lock.json and * npm-shrinkwrap.json files via NPM Audit API. @@ -135,7 +137,7 @@ protected void analyzeDependency(Dependency dependency, Engine engine) throws An LOGGER.debug("Skipping {} because shrinkwrap lock file exists", dependency.getFilePath()); return; } - if (!packageLock.isFile() || packageLock.length() == 0 || !shouldProcess(packageLock)) { + if (!existsWithContent(packageLock) || !shouldProcess(packageLock)) { return; } final File packageJson = new File(packageLock.getParentFile(), "package.json"); diff --git a/core/src/main/java/org/owasp/dependencycheck/analyzer/NodePackageAnalyzer.java b/core/src/main/java/org/owasp/dependencycheck/analyzer/NodePackageAnalyzer.java index 5b29f43e5e3..7c5da572d00 100644 --- a/core/src/main/java/org/owasp/dependencycheck/analyzer/NodePackageAnalyzer.java +++ b/core/src/main/java/org/owasp/dependencycheck/analyzer/NodePackageAnalyzer.java @@ -54,6 +54,8 @@ import java.util.Map; import java.util.Objects; +import static org.owasp.dependencycheck.utils.FileUtils.existsWithContent; + /** * Used to analyze Node Package Manager (npm) package.json files, and collect * information that can be used to determine the associated CPE. @@ -223,7 +225,7 @@ private boolean noLockFileExists(File dependencyFile) { @Override protected void analyzeDependency(Dependency dependency, Engine engine) throws AnalysisException { final File dependencyFile = dependency.getActualFile(); - if (!dependencyFile.isFile() || dependencyFile.length() == 0 || !shouldProcess(dependencyFile)) { + if (!existsWithContent(dependencyFile) || !shouldProcess(dependencyFile)) { return; } if (isNodeAuditEnabled(engine) diff --git a/core/src/main/java/org/owasp/dependencycheck/analyzer/PinnedMavenInstallAnalyzer.java b/core/src/main/java/org/owasp/dependencycheck/analyzer/PinnedMavenInstallAnalyzer.java index 0be4239329f..cc156eba679 100644 --- a/core/src/main/java/org/owasp/dependencycheck/analyzer/PinnedMavenInstallAnalyzer.java +++ b/core/src/main/java/org/owasp/dependencycheck/analyzer/PinnedMavenInstallAnalyzer.java @@ -25,8 +25,6 @@ import com.github.packageurl.MalformedPackageURLException; import com.github.packageurl.PackageURL; import com.github.packageurl.PackageURLBuilder; -import java.util.Map; -import java.util.stream.Collectors; import org.owasp.dependencycheck.Engine; import org.owasp.dependencycheck.analyzer.exception.AnalysisException; import org.owasp.dependencycheck.data.nvd.ecosystem.Ecosystem; @@ -45,8 +43,12 @@ import java.io.IOException; import java.util.Collections; import java.util.List; +import java.util.Map; import java.util.Objects; import java.util.regex.Pattern; +import java.util.stream.Collectors; + +import static org.owasp.dependencycheck.utils.FileUtils.existsWithContent; /** * Used to analyze Maven pinned dependency files named {@code *install*.json}, a @@ -116,7 +118,7 @@ protected void analyzeDependency(Dependency dependency, Engine engine) throws An LOGGER.debug("Checking file {}", dependency.getActualFilePath()); final File dependencyFile = dependency.getActualFile(); - if (!dependencyFile.isFile() || dependencyFile.length() == 0) { + if (!existsWithContent(dependencyFile)) { return; } diff --git a/core/src/main/java/org/owasp/dependencycheck/analyzer/PipAnalyzer.java b/core/src/main/java/org/owasp/dependencycheck/analyzer/PipAnalyzer.java index e4ceb68bf25..bf7b88e8f7f 100644 --- a/core/src/main/java/org/owasp/dependencycheck/analyzer/PipAnalyzer.java +++ b/core/src/main/java/org/owasp/dependencycheck/analyzer/PipAnalyzer.java @@ -44,6 +44,8 @@ import java.util.regex.Pattern; import java.util.stream.Collectors; +import static org.owasp.dependencycheck.utils.FileUtils.existsWithContent; + /** * Used to analyze pip dependency files named requirements.txt. * @@ -131,7 +133,7 @@ protected void analyzeDependency(Dependency dependency, Engine engine) throws An engine.removeDependency(dependency); } final File dependencyFile = dependency.getActualFile(); - if (!dependencyFile.isFile() || dependencyFile.length() == 0) { + if (!existsWithContent(dependencyFile)) { return; } diff --git a/core/src/main/java/org/owasp/dependencycheck/analyzer/PipfileAnalyzer.java b/core/src/main/java/org/owasp/dependencycheck/analyzer/PipfileAnalyzer.java index bb5c66fc75e..318bc003ebe 100644 --- a/core/src/main/java/org/owasp/dependencycheck/analyzer/PipfileAnalyzer.java +++ b/core/src/main/java/org/owasp/dependencycheck/analyzer/PipfileAnalyzer.java @@ -43,6 +43,8 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; +import static org.owasp.dependencycheck.utils.FileUtils.existsWithContent; + /** * Used to analyze dependencies defined in Pipfile. This analyzer works in * tandem with the `PipfilelockAnalyzer` - and both analyzers use the same key @@ -141,7 +143,7 @@ protected void analyzeDependency(Dependency dependency, Engine engine) throws An } final File dependencyFile = dependency.getActualFile(); - if (!dependencyFile.isFile() || dependencyFile.length() == 0) { + if (!existsWithContent(dependencyFile)) { return; } diff --git a/core/src/main/java/org/owasp/dependencycheck/analyzer/PipfilelockAnalyzer.java b/core/src/main/java/org/owasp/dependencycheck/analyzer/PipfilelockAnalyzer.java index d2a3f8f4719..993c2b302d9 100644 --- a/core/src/main/java/org/owasp/dependencycheck/analyzer/PipfilelockAnalyzer.java +++ b/core/src/main/java/org/owasp/dependencycheck/analyzer/PipfilelockAnalyzer.java @@ -50,6 +50,8 @@ import java.nio.file.Files; import java.util.Set; +import static org.owasp.dependencycheck.utils.FileUtils.existsWithContent; + /** * Used to analyze dependencies defined in Pipfile.lock. This analyzer works in * tandem with the `PipfileAnalyzer` - and both analyzers use the same key to @@ -134,7 +136,7 @@ protected void analyzeDependency(Dependency dependency, Engine engine) throws An engine.removeDependency(dependency); final File dependencyFile = dependency.getActualFile(); - if (!dependencyFile.isFile() || dependencyFile.length() == 0) { + if (!existsWithContent(dependencyFile)) { return; } diff --git a/core/src/main/java/org/owasp/dependencycheck/analyzer/PnpmAuditAnalyzer.java b/core/src/main/java/org/owasp/dependencycheck/analyzer/PnpmAuditAnalyzer.java index a9a88c78835..ac42f445006 100644 --- a/core/src/main/java/org/owasp/dependencycheck/analyzer/PnpmAuditAnalyzer.java +++ b/core/src/main/java/org/owasp/dependencycheck/analyzer/PnpmAuditAnalyzer.java @@ -47,6 +47,8 @@ import java.util.ArrayList; import java.util.List; +import static org.owasp.dependencycheck.utils.FileUtils.existsWithContent; + @ThreadSafe public class PnpmAuditAnalyzer extends AbstractNpmAnalyzer { @@ -90,7 +92,7 @@ protected void analyzeDependency(Dependency dependency, Engine engine) throws An engine.removeDependency(dependency); } final File packageLock = dependency.getActualFile(); - if (!packageLock.isFile() || packageLock.length() == 0 || !shouldProcess(packageLock)) { + if (!existsWithContent(packageLock) || !shouldProcess(packageLock)) { return; } final List advisories; diff --git a/core/src/main/java/org/owasp/dependencycheck/analyzer/YarnAuditAnalyzer.java b/core/src/main/java/org/owasp/dependencycheck/analyzer/YarnAuditAnalyzer.java index 8a17a1077b4..a8a9f8d94fe 100644 --- a/core/src/main/java/org/owasp/dependencycheck/analyzer/YarnAuditAnalyzer.java +++ b/core/src/main/java/org/owasp/dependencycheck/analyzer/YarnAuditAnalyzer.java @@ -41,12 +41,11 @@ import org.slf4j.LoggerFactory; import us.springett.parsers.cpe.exceptions.CpeValidationException; +import javax.annotation.concurrent.ThreadSafe; import jakarta.json.Json; import jakarta.json.JsonException; import jakarta.json.JsonObject; import jakarta.json.JsonReader; - -import javax.annotation.concurrent.ThreadSafe; import java.io.File; import java.io.FileFilter; import java.io.IOException; @@ -57,6 +56,8 @@ import java.util.List; import java.util.stream.Stream; +import static org.owasp.dependencycheck.utils.FileUtils.existsWithContent; + @ThreadSafe public class YarnAuditAnalyzer extends AbstractNpmAnalyzer { @@ -233,7 +234,7 @@ protected void analyzeDependency(Dependency dependency, Engine engine) throws An engine.removeDependency(dependency); } final File packageLock = dependency.getActualFile(); - if (!packageLock.isFile() || packageLock.length() == 0 || !shouldProcess(packageLock)) { + if (!existsWithContent(packageLock) || !shouldProcess(packageLock)) { return; } File dependencyDirectory = getDependencyDirectory(packageLock); diff --git a/core/src/main/java/org/owasp/dependencycheck/data/update/LocalDataSource.java b/core/src/main/java/org/owasp/dependencycheck/data/update/LocalDataSource.java index a258d968af9..78c8a24dc83 100644 --- a/core/src/main/java/org/owasp/dependencycheck/data/update/LocalDataSource.java +++ b/core/src/main/java/org/owasp/dependencycheck/data/update/LocalDataSource.java @@ -17,6 +17,7 @@ */ package org.owasp.dependencycheck.data.update; +import org.jspecify.annotations.NonNull; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -30,6 +31,8 @@ import java.time.Instant; import java.util.Properties; +import static org.owasp.dependencycheck.utils.FileUtils.existsWithContent; + /** * * @author Jeremy Long @@ -91,9 +94,9 @@ protected Instant getLastUpdated(@NonNull File repo) { * @return true if an update to the data source should be performed; otherwise false. * If the repo does not exist, or is an empty file, it is considered stale. */ - protected boolean isStale(File repo, Duration validFor) { + protected boolean isStale(@NonNull File repo, @NonNull Duration validFor) { boolean stale = true; - if (repo != null && repo.isFile()) { + if (existsWithContent(repo)) { final Instant lastUpdatedOn = getLastUpdated(repo); final Instant now = Instant.now(); LOGGER.debug("{} last updated: {}, now: {}", getClass().getSimpleName(), lastUpdatedOn, now); diff --git a/core/src/main/java/org/owasp/dependencycheck/data/update/RetireJSDataSource.java b/core/src/main/java/org/owasp/dependencycheck/data/update/RetireJSDataSource.java index cc6fd8df0f0..723f6c90706 100644 --- a/core/src/main/java/org/owasp/dependencycheck/data/update/RetireJSDataSource.java +++ b/core/src/main/java/org/owasp/dependencycheck/data/update/RetireJSDataSource.java @@ -17,10 +17,12 @@ */ package org.owasp.dependencycheck.data.update; +import org.jspecify.annotations.NonNull; import org.owasp.dependencycheck.Engine; import org.owasp.dependencycheck.data.update.exception.UpdateException; import org.owasp.dependencycheck.exception.WriteLockException; import org.owasp.dependencycheck.utils.Downloader; +import org.owasp.dependencycheck.utils.InvalidSettingException; import org.owasp.dependencycheck.utils.ResourceNotFoundException; import org.owasp.dependencycheck.utils.Settings; import org.owasp.dependencycheck.utils.TooManyRequestsException; @@ -35,6 +37,8 @@ import java.net.URL; import java.time.Duration; +import static org.owasp.dependencycheck.utils.FileUtils.existsWithContent; + /** * Downloads a local copy of the RetireJS repository. * @@ -71,22 +75,39 @@ public RetireJSDataSource() { @Override public boolean update(Engine engine) throws UpdateException { this.settings = engine.getSettings(); + final URL url = validatedUrl(); + final File repoFile = validatedRepoFileFrom(url); + if (isEnabled() && shouldUpdateFromRemote(repoFile)) { + LOGGER.debug("Begin RetireJS Update"); + initializeRetireJsRepo(settings, url, repoFile); + saveLastUpdated(repoFile); + } + return false; + } + + private @NonNull URL validatedUrl() throws UpdateException { final String configuredUrl = settings.getString(Settings.KEYS.ANALYZER_RETIREJS_REPO_JS_URL, DEFAULT_JS_URL); try { - final URL url = new URL(configuredUrl); - final File filepath = new File(url.getPath()); - final File repoFile = new File(settings.getDataDirectory(), filepath.getName()); - if (isEnabled() && shouldUpdateFromRemote(repoFile)) { - LOGGER.debug("Begin RetireJS Update"); - initializeRetireJsRepo(settings, url, repoFile); - saveLastUpdated(repoFile); - } + return new URL(configuredUrl); } catch (MalformedURLException ex) { throw new UpdateException(String.format("Invalid URL for RetireJS repository (%s)", configuredUrl), ex); + } + } + + public @NonNull File validatedRepoFile() throws UpdateException { + return validatedRepoFileFrom(validatedUrl()); + } + + private @NonNull File validatedRepoFileFrom(URL url) throws UpdateException { + try { + String fileName = new File(url.getPath()).getName(); + if (fileName.isBlank()) { + throw new InvalidSettingException("RetireJS URL must imply a filename."); + } + return new File(settings.getDataDirectory(), fileName); } catch (IOException ex) { - throw new UpdateException("Unable to get the data directory", ex); + throw new UpdateException("Unable to determine RetireJS local repository location", ex); } - return false; } private boolean isEnabled() { @@ -97,7 +118,7 @@ private boolean shouldUpdateFromRemote(File repoFile) { boolean forceupdate = settings.getBoolean(Settings.KEYS.ANALYZER_RETIREJS_FORCEUPDATE, false); boolean autoupdate = settings.getBoolean(Settings.KEYS.AUTO_UPDATE, true); Duration validFor = Duration.ofHours(settings.getInt(Settings.KEYS.ANALYZER_RETIREJS_REPO_VALID_FOR_HOURS, 24)); - return forceupdate || (autoupdate && isStale(repoFile, validFor)); + return forceupdate || !existsWithContent(repoFile) || (autoupdate && isStale(repoFile, validFor)); } /** @@ -106,8 +127,7 @@ private boolean shouldUpdateFromRemote(File repoFile) { * @param settings a reference to the dependency-check settings * @param repoUrl the URL to the RetireJS repository to use * @param repoFile the filename to use for the RetireJS repository - * @throws UpdateException thrown if there is an exception during - * initialization + * @throws UpdateException thrown if there is an exception during initialization */ @SuppressWarnings("try") private void initializeRetireJsRepo(Settings settings, URL repoUrl, File repoFile) throws UpdateException { diff --git a/core/src/test/java/org/owasp/dependencycheck/analyzer/AbstractSuppressionAnalyzerTest.java b/core/src/test/java/org/owasp/dependencycheck/analyzer/AbstractSuppressionAnalyzerTest.java index 48c973eda4e..88677f9cd9d 100644 --- a/core/src/test/java/org/owasp/dependencycheck/analyzer/AbstractSuppressionAnalyzerTest.java +++ b/core/src/test/java/org/owasp/dependencycheck/analyzer/AbstractSuppressionAnalyzerTest.java @@ -82,7 +82,7 @@ private String testSuppressionsFileUrl() { class BasePackagedSuppressionsLoading { @Test void packagedBaseSuppressions() throws Exception { - engine = prepareBaseSuppressionsOnly(); + prepareBaseSuppressionsOnly(); @SuppressWarnings("unchecked") List rules = (List) engine.getObject(SUPPRESSION_OBJECT_KEY); assertThat(rules, not(empty())); assertThat("Expected all suppressions in base file to be marked as base", allRulesNotMarkedAsBase(rules), empty()); @@ -94,7 +94,7 @@ class HostedSuppressionsLoading { @Test void packagedSnapshotHostedSuppressionsLoadedEvenIfRemoteHostedSuppressionsDisabled() throws Exception { getSettings().setBoolean(KEYS.HOSTED_SUPPRESSIONS_ENABLED, false); - engine = prepareHostedSuppressionsOnly(); + prepareHostedSuppressionsOnly(); @SuppressWarnings("unchecked") List rules = (List) engine.getObject(SUPPRESSION_OBJECT_KEY); assertThat(rules, not(empty())); assertThat("Expected all suppressions in hosted suppressions snapshot file to be marked as base", allRulesNotMarkedAsBase(rules), empty()); @@ -104,7 +104,7 @@ void packagedSnapshotHostedSuppressionsLoadedEvenIfRemoteHostedSuppressionsDisab void packagedSnapshotHostedSuppressionsLoadedIfAutoUpdateDisabled() throws Exception { getSettings().setBoolean(KEYS.HOSTED_SUPPRESSIONS_ENABLED, true); getSettings().setBoolean(KEYS.AUTO_UPDATE, false); - engine = prepareHostedSuppressionsOnly(); + prepareHostedSuppressionsOnly(); assertThat(AbstractSuppressionAnalyzer.getRuleCount(engine), greaterThan(0)); } @Test @@ -112,7 +112,7 @@ void packagedSnapshotHostedSuppressionsLoadedIfRemoteHostedSuppressionsFail() th getSettings().setBoolean(KEYS.HOSTED_SUPPRESSIONS_ENABLED, true); getSettings().setBoolean(KEYS.AUTO_UPDATE, true); getSettings().setString(KEYS.HOSTED_SUPPRESSIONS_URL, "file:///does-not-exist.xml"); - engine = prepareHostedSuppressionsOnly(); + prepareHostedSuppressionsOnly(); assertThat(AbstractSuppressionAnalyzer.getRuleCount(engine), greaterThan(0)); } @@ -121,7 +121,7 @@ void packagedSnapshotHostedSuppressionsLoadedIfRemoteHostedSuppressionsFail() th void ignoresHostedSuppressionsIfUrlDoesntIncludeAFileNameRegardlessOfEnabledState(boolean hostedSuppressionsEnabled) throws Exception { getSettings().setBoolean(KEYS.HOSTED_SUPPRESSIONS_ENABLED, hostedSuppressionsEnabled); getSettings().setString(KEYS.HOSTED_SUPPRESSIONS_URL, "https://valid.url.but.no.file/"); - engine = prepareHostedSuppressionsOnly(); + prepareHostedSuppressionsOnly(); assertThat(AbstractSuppressionAnalyzer.getRuleCount(engine), is(0)); } @@ -129,7 +129,7 @@ void ignoresHostedSuppressionsIfUrlDoesntIncludeAFileNameRegardlessOfEnabledStat void ignoresHostedSuppressionsIfCannotBeParsedFromRemote() throws Exception { getSettings().setBoolean(KEYS.HOSTED_SUPPRESSIONS_ENABLED, true); getSettings().setString(KEYS.HOSTED_SUPPRESSIONS_URL, BaseTest.getResourceAsUrlString(this, "suppressions-invalid.xml")); - engine = prepareHostedSuppressionsOnly(); + prepareHostedSuppressionsOnly(); assertThat(AbstractSuppressionAnalyzer.getRuleCount(engine), is(0)); } @@ -138,7 +138,7 @@ void prefersRemoteHostedSuppressionsIfEnabled() throws Exception { getSettings().setBoolean(KEYS.HOSTED_SUPPRESSIONS_ENABLED, true); getSettings().setBoolean(KEYS.AUTO_UPDATE, true); getSettings().setString(KEYS.HOSTED_SUPPRESSIONS_URL, testSuppressionsFileUrl()); - engine = prepareHostedSuppressionsOnly(); + prepareHostedSuppressionsOnly(); assertThat(AbstractSuppressionAnalyzer.getRuleCount(engine), is(TEST_SUPPRESSIONS_EXPECTED_COUNT)); } @@ -148,7 +148,7 @@ void prefersRemoteHostedSuppressionsIfEnabledAndForced() throws Exception { getSettings().setBoolean(KEYS.AUTO_UPDATE, false); getSettings().setBoolean(KEYS.HOSTED_SUPPRESSIONS_FORCEUPDATE, true); getSettings().setString(KEYS.HOSTED_SUPPRESSIONS_URL, testSuppressionsFileUrl()); - engine = prepareHostedSuppressionsOnly(); + prepareHostedSuppressionsOnly(); assertThat(AbstractSuppressionAnalyzer.getRuleCount(engine), is(TEST_SUPPRESSIONS_EXPECTED_COUNT)); } } @@ -192,8 +192,7 @@ void testGetRulesFromMultipleSuppressionFiles() throws Exception { // WHEN initializing with both suppression files final String[] suppressionFiles = {TEST_SUPPRESSIONS_FILE, OTHER_TEST_SUPPRESSIONS_FILE}; getSettings().setArrayIfNotEmpty(KEYS.SUPPRESSION_FILE, suppressionFiles); - engine = new Engine(getSettings()); - newAnalyzer().prepare(engine); + prepareSuppressions(); // THEN rules from both files were loaded final int expectedSize = rulesInFirstFile + rulesInSecondFile + rulesInCoreFile; @@ -203,8 +202,7 @@ void testGetRulesFromMultipleSuppressionFiles() throws Exception { @Test void testFailureToLocateSuppressionFileAnywhere() { getSettings().setString(Settings.KEYS.SUPPRESSION_FILE, "doesnotexist.xml"); - engine = new Engine(Mode.EVIDENCE_COLLECTION, getSettings()); - assertThrows(InitializationException.class, () -> newAnalyzer().prepare(engine)); + assertThrows(InitializationException.class, AbstractSuppressionAnalyzerTest.this::prepareSuppressions); } } @@ -217,7 +215,7 @@ void testFailureToLocateSuppressionFileAnywhere() { */ private int getNumberOfRulesLoadedInCoreFile() throws Exception { getSettings().removeProperty(KEYS.SUPPRESSION_FILE); - engine = prepareSuppressions(); + prepareSuppressions(); return AbstractSuppressionAnalyzer.getRuleCount(engine); } @@ -231,26 +229,23 @@ private int getNumberOfRulesLoadedInCoreFile() throws Exception { */ private int getNumberOfRulesLoadedFromPath(final String path) throws Exception { getSettings().setString(KEYS.SUPPRESSION_FILE, path); - engine = prepareSuppressions(); + prepareSuppressions(); return AbstractSuppressionAnalyzer.getRuleCount(engine); } - private @NonNull Engine prepareSuppressions() throws InvalidSettingException, InitializationException { + private void prepareSuppressions() throws InvalidSettingException, InitializationException { engine = new Engine(Mode.EVIDENCE_COLLECTION, getSettings()); newAnalyzer().prepare(engine); - return engine; } - private @NonNull Engine prepareBaseSuppressionsOnly() throws InvalidSettingException, SuppressionParseException { + private void prepareBaseSuppressionsOnly() throws InvalidSettingException, SuppressionParseException { engine = new Engine(Mode.EVIDENCE_COLLECTION, getSettings()); newAnalyzer().loadPackagedBaseSuppressionData(engine); - return engine; } - private @NonNull Engine prepareHostedSuppressionsOnly() throws InvalidSettingException { + private void prepareHostedSuppressionsOnly() throws InvalidSettingException { engine = new Engine(Mode.EVIDENCE_COLLECTION, getSettings()); newAnalyzer().loadHostedSuppressionBaseData(engine); - return engine; } private @NonNull AbstractSuppressionAnalyzerImpl newAnalyzer() throws InvalidSettingException { diff --git a/utils/src/main/java/org/owasp/dependencycheck/utils/FileUtils.java b/utils/src/main/java/org/owasp/dependencycheck/utils/FileUtils.java index 88428dee453..5a55f16ad72 100644 --- a/utils/src/main/java/org/owasp/dependencycheck/utils/FileUtils.java +++ b/utils/src/main/java/org/owasp/dependencycheck/utils/FileUtils.java @@ -17,6 +17,14 @@ */ package org.owasp.dependencycheck.utils; +import org.apache.commons.io.FilenameUtils; +import org.apache.commons.lang3.StringUtils; +import org.apache.commons.lang3.SystemUtils; +import org.jspecify.annotations.NonNull; +import org.jspecify.annotations.Nullable; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import java.io.Closeable; import java.io.File; import java.io.FileInputStream; @@ -30,14 +38,6 @@ import java.util.UUID; import java.util.stream.Stream; -import org.apache.commons.io.FilenameUtils; -import org.apache.commons.lang3.StringUtils; -import org.apache.commons.lang3.SystemUtils; -import org.jspecify.annotations.NonNull; -import org.jspecify.annotations.Nullable; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - /** * A collection of utilities for processing information about files. * @@ -195,4 +195,12 @@ public static File getResourceAsFile(final String resource) { } return new File(path); } + + /** + * @param file the file to check + * @return true if the passed file is a file with more than 1 byte of content + */ + public static boolean existsWithContent(@NonNull File file) { + return file.isFile() && file.length() > 1; + } } diff --git a/utils/src/test/java/org/owasp/dependencycheck/utils/FileUtilsTest.java b/utils/src/test/java/org/owasp/dependencycheck/utils/FileUtilsTest.java index 5c63b668099..35918b7fe56 100644 --- a/utils/src/test/java/org/owasp/dependencycheck/utils/FileUtilsTest.java +++ b/utils/src/test/java/org/owasp/dependencycheck/utils/FileUtilsTest.java @@ -18,12 +18,17 @@ package org.owasp.dependencycheck.utils; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.owasp.dependencycheck.utils.FileUtils.existsWithContent; /** * @@ -31,6 +36,9 @@ */ class FileUtilsTest extends BaseTest { + @TempDir + Path tempDir; + /** * Test of getFileExtension method, of class FileUtils. */ @@ -74,4 +82,17 @@ void testDeleteWithSubDirectories() throws Exception { assertTrue(status, "delete returned a failed status"); assertFalse(file.exists(), "Temporary file exists after attempting deletion"); } + + @Test + void testExistsWithContent() throws IOException { + assertFalse(existsWithContent(new File("doesnt-exist"))); + assertFalse(existsWithContent(new File(".")), "directory shouldn't be considered as existing with content"); + + Path tempFile = Files.createTempFile(tempDir, "", ""); + assertFalse(existsWithContent(tempFile.toFile()), "empty file shouldn't be considered as existing with content"); + Files.writeString(tempFile, " "); + assertFalse(existsWithContent(tempFile.toFile()), "1 byte file shouldn't be considered as existing with content"); + Files.writeString(tempFile, " "); + assertTrue(existsWithContent(tempFile.toFile())); + } } From 49e9ab8d93b23ca7d44bcba45b9b3d196799bf2e Mon Sep 17 00:00:00 2001 From: Chad Wilson <29788154+chadlwilson@users.noreply.github.com> Date: Fri, 8 May 2026 14:53:46 +0800 Subject: [PATCH 5/5] refactor: refactor RetireJSAnalyzer to reuse RetireJSDataSource logic Avoid duplicating all the logic for validating URLs and determining whether the repository should be updated Signed-off-by: Chad Wilson <29788154+chadlwilson@users.noreply.github.com> --- .../analyzer/RetireJsAnalyzer.java | 57 ++--- .../data/update/RetireJSDataSource.java | 4 +- .../analyzer/RetireJsAnalyzerTest.java | 143 +++++++++++ .../data/update/RetireJSDataSourceTest.java | 233 ++++++++++++++++++ .../retirejs/jsrepository-invalid.json | 3 + .../test/resources/retirejs/jsrepository.json | 37 +++ 6 files changed, 442 insertions(+), 35 deletions(-) create mode 100644 core/src/test/java/org/owasp/dependencycheck/analyzer/RetireJsAnalyzerTest.java create mode 100644 core/src/test/java/org/owasp/dependencycheck/data/update/RetireJSDataSourceTest.java create mode 100644 core/src/test/resources/retirejs/jsrepository-invalid.json create mode 100644 core/src/test/resources/retirejs/jsrepository.json diff --git a/core/src/main/java/org/owasp/dependencycheck/analyzer/RetireJsAnalyzer.java b/core/src/main/java/org/owasp/dependencycheck/analyzer/RetireJsAnalyzer.java index af9319a4c4d..d73fd3e7201 100644 --- a/core/src/main/java/org/owasp/dependencycheck/analyzer/RetireJsAnalyzer.java +++ b/core/src/main/java/org/owasp/dependencycheck/analyzer/RetireJsAnalyzer.java @@ -20,10 +20,12 @@ import com.esotericsoftware.minlog.Log; import com.github.packageurl.MalformedPackageURLException; import com.github.packageurl.PackageURLBuilder; +import com.google.common.annotations.VisibleForTesting; import com.h3xstream.retirejs.repo.JsLibraryResult; import com.h3xstream.retirejs.repo.ScannerFacade; import com.h3xstream.retirejs.repo.VulnerabilitiesRepository; import com.h3xstream.retirejs.repo.VulnerabilitiesRepositoryLoader; +import org.apache.commons.io.IOUtils; import org.apache.commons.lang3.StringUtils; import org.apache.commons.validator.routines.UrlValidator; import org.json.JSONException; @@ -58,19 +60,18 @@ import java.io.FileInputStream; import java.io.IOException; import java.io.InputStream; -import java.net.URL; import java.nio.file.Files; +import java.nio.file.StandardCopyOption; import java.util.HashSet; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Optional; +import java.util.OptionalInt; import java.util.Set; import java.util.stream.Collectors; -import org.apache.commons.io.IOUtils; - import static org.owasp.dependencycheck.analyzer.RetireJsLibrary.KnownIdentifierTypes.CVE; import static org.owasp.dependencycheck.analyzer.RetireJsLibrary.KnownIdentifierTypes.GITHUB_SECURITY_ADVISORY; import static org.owasp.dependencycheck.analyzer.RetireJsLibrary.KnownIdentifierTypes.SECONDARY_NAME_TYPES; @@ -193,40 +194,13 @@ protected void prepareFileTypeAnalyzer(Engine engine) throws InitializationExcep // it aligns with other analyzers that don't log such information. Log.set(Log.LEVEL_WARN); - File repoFile = null; - boolean repoEmpty = false; - try { - final String configuredUrl = getSettings().getString(Settings.KEYS.ANALYZER_RETIREJS_REPO_JS_URL, RetireJSDataSource.DEFAULT_JS_URL); - final URL url = new URL(configuredUrl); - final File filepath = new File(url.getPath()); - repoFile = new File(getSettings().getDataDirectory(), filepath.getName()); - if (!repoFile.isFile() || repoFile.length() <= 1L) { - LOGGER.warn("Retire JS repository is empty or missing - attempting to force the update"); - repoEmpty = true; - getSettings().setBoolean(Settings.KEYS.ANALYZER_RETIREJS_FORCEUPDATE, true); - } - } catch (IOException ex) { - this.setEnabled(false); - throw new InitializationException("Failed to initialize the RetireJS", ex); - } + File repoFile = tryRemoteFetchIfConfigured(engine); - final boolean autoupdate = getSettings().getBoolean(Settings.KEYS.AUTO_UPDATE, true); - final boolean forceupdate = getSettings().getBoolean(Settings.KEYS.ANALYZER_RETIREJS_FORCEUPDATE, false); - if ((!autoupdate && forceupdate) || (autoupdate && repoEmpty)) { - final RetireJSDataSource ds = new RetireJSDataSource(); - try { - ds.update(engine); - } catch (UpdateException ex) { - throw new InitializationException("Unable to initialize the Retire JS repository", ex); - } - } - - //several users are reporting that the retire js repository is getting corrupted. try (WriteLock ignored = new WriteLock(getSettings(), true, repoFile.getName() + ".lock")) { final File temp = getSettings().getTempDirectory(); final File tempRepo = new File(temp, repoFile.getName()); - LOGGER.debug("copying retireJs repo {} to {}", repoFile.toPath(), tempRepo.toPath()); - Files.copy(repoFile.toPath(), tempRepo.toPath()); + LOGGER.debug("copying RetireJS repo {} to {}", repoFile.toPath(), tempRepo.toPath()); + Files.copy(repoFile.toPath(), tempRepo.toPath(), StandardCopyOption.REPLACE_EXISTING); repoFile = tempRepo; } catch (WriteLockException | IOException ex) { this.setEnabled(false); @@ -245,6 +219,17 @@ protected void prepareFileTypeAnalyzer(Engine engine) throws InitializationExcep } } + private File tryRemoteFetchIfConfigured(Engine engine) throws InitializationException { + RetireJSDataSource ds = new RetireJSDataSource(); + try { + ds.update(engine); + return ds.validatedRepoFile(); + } catch (UpdateException ex) { + this.setEnabled(false); + throw new InitializationException("Failed to initialize the RetireJS repo", ex); + } + } + /** * Returns the name of the analyzer. * @@ -324,6 +309,12 @@ public void analyzeDependency(Dependency dependency, Engine engine) throws Analy protected void closeAnalyzer() throws Exception { Log.set(Log.LEVEL_INFO); } + + @SuppressWarnings("SameParameterValue") + @VisibleForTesting + OptionalInt knownLibraryCountFor(String fileName) { + return jsRepository == null ? OptionalInt.empty() : OptionalInt.of(jsRepository.findByFilename(fileName).size()); + } } class RetireJsLibrary { diff --git a/core/src/main/java/org/owasp/dependencycheck/data/update/RetireJSDataSource.java b/core/src/main/java/org/owasp/dependencycheck/data/update/RetireJSDataSource.java index 723f6c90706..74280d08224 100644 --- a/core/src/main/java/org/owasp/dependencycheck/data/update/RetireJSDataSource.java +++ b/core/src/main/java/org/owasp/dependencycheck/data/update/RetireJSDataSource.java @@ -49,7 +49,7 @@ public class RetireJSDataSource extends LocalDataSource { /** * The default URL to the RetireJS JavaScript repository. */ - public static final String DEFAULT_JS_URL = "https://raw.githubusercontent.com/Retirejs/retire.js/master/repository/jsrepository.json"; + private static final String DEFAULT_JS_URL = "https://raw.githubusercontent.com/Retirejs/retire.js/master/repository/jsrepository.json"; /** * Static logger. */ @@ -106,7 +106,7 @@ public boolean update(Engine engine) throws UpdateException { } return new File(settings.getDataDirectory(), fileName); } catch (IOException ex) { - throw new UpdateException("Unable to determine RetireJS local repository location", ex); + throw new UpdateException("Unable to determine the local location to cache RetireJS repo", ex); } } diff --git a/core/src/test/java/org/owasp/dependencycheck/analyzer/RetireJsAnalyzerTest.java b/core/src/test/java/org/owasp/dependencycheck/analyzer/RetireJsAnalyzerTest.java new file mode 100644 index 00000000000..ca3bc5e3388 --- /dev/null +++ b/core/src/test/java/org/owasp/dependencycheck/analyzer/RetireJsAnalyzerTest.java @@ -0,0 +1,143 @@ +/* + * This file is part of dependency-check-core. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * Copyright (c) 2026 Chad Wilson. All Rights Reserved. + */ +package org.owasp.dependencycheck.analyzer; + +import org.apache.commons.lang3.exception.ExceptionUtils; +import org.json.JSONException; +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.owasp.dependencycheck.BaseTest; +import org.owasp.dependencycheck.Engine; +import org.owasp.dependencycheck.data.update.RetireJSDataSource; +import org.owasp.dependencycheck.exception.InitializationException; +import org.owasp.dependencycheck.utils.Downloader; +import org.owasp.dependencycheck.utils.InvalidSettingException; +import org.owasp.dependencycheck.utils.Settings; + +import java.nio.file.NoSuchFileException; + +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.instanceOf; +import static org.junit.jupiter.api.Assertions.assertThrows; + +class RetireJsAnalyzerTest extends BaseTest { + private static final String TEST_RETIRE_JS_REPOSITORY_FILE = "retirejs/jsrepository.json"; + + private RetireJsAnalyzer analyzer; + private Engine engine; + + @BeforeEach + void settings() { + getSettings().setBoolean(Settings.KEYS.ANALYZER_RETIREJS_ENABLED, true); + getSettings().setBoolean(Settings.KEYS.ANALYZER_RETIREJS_FORCEUPDATE, false); + getSettings().setString(Settings.KEYS.ANALYZER_RETIREJS_REPO_JS_URL, testRepositoryFileUrl()); + } + + @AfterEach + void cleanUp() { + if (engine != null) { + new RetireJSDataSource().purge(engine); + engine.close(); + } + } + + private String testRepositoryFileUrl() { + return BaseTest.getResourceAsUrlString(this, TEST_RETIRE_JS_REPOSITORY_FILE); + } + + @Nested + class RepositoryLoading { + + @Test + void loadsRemoteIfEmptyWithAutoUpdateDisabled() throws Exception { + getSettings().setBoolean(Settings.KEYS.AUTO_UPDATE, false); + prepareRetireJs(); + assertThat(analyzer.knownLibraryCountFor("retire-example-0.0.1.js").orElseThrow(), is(1)); + } + + @Test + void loadsRemoteIfPresentAndForced() throws Exception { + prepareRetireJs(); + + // try again with repo present; but with a URL that will fail when invoked so we know it tried + getSettings().setBoolean(Settings.KEYS.ANALYZER_RETIREJS_FORCEUPDATE, true); + getSettings().setString(Settings.KEYS.ANALYZER_RETIREJS_REPO_JS_URL, testRepositoryFileUrl().replace(TEST_RETIRE_JS_REPOSITORY_FILE, "doesnt-exist/" + TEST_RETIRE_JS_REPOSITORY_FILE)); + + var ex = assertThrows(InitializationException.class, RetireJsAnalyzerTest.this::prepareRetireJs); + assertThat(ex.getMessage(), containsString("Failed to initialize the RetireJS repo")); + assertThat(ExceptionUtils.getRootCause(ex), instanceOf(NoSuchFileException.class)); + } + + @Test + void doesNothingIfPresentAndAutoUpdateDisabled() throws Exception { + prepareRetireJs(); + + getSettings().setBoolean(Settings.KEYS.AUTO_UPDATE, false); + getSettings().setBoolean(Settings.KEYS.ANALYZER_RETIREJS_FORCEUPDATE, false); + + // try again with repo present; but with a URL that would fail if invoked + getSettings().setString(Settings.KEYS.ANALYZER_RETIREJS_REPO_JS_URL, testRepositoryFileUrl().replace(TEST_RETIRE_JS_REPOSITORY_FILE, "doesnt-exist/" + TEST_RETIRE_JS_REPOSITORY_FILE)); + prepareRetireJs(); + } + + @Test + void failsIfRemoteRetireJsRepoNotFound() { + getSettings().setBoolean(Settings.KEYS.AUTO_UPDATE, true); + getSettings().setString(Settings.KEYS.ANALYZER_RETIREJS_REPO_JS_URL, "file:///does-not-exist.xml"); + var ex = assertThrows(InitializationException.class, RetireJsAnalyzerTest.this::prepareRetireJs); + assertThat(ex.getMessage(), containsString("Failed to initialize the RetireJS repo")); + assertThat(ExceptionUtils.getRootCause(ex), instanceOf(NoSuchFileException.class)); + assertThat(analyzer.isEnabled(), is(false)); + } + + @Test + void failsIfRemoteRetireJsCannotBeParsed() { + getSettings().setString(Settings.KEYS.ANALYZER_RETIREJS_REPO_JS_URL, BaseTest.getResourceAsUrlString(this, "retirejs/jsrepository-invalid.json")); + var ex = assertThrows(InitializationException.class, RetireJsAnalyzerTest.this::prepareRetireJs); + assertThat(ex.getMessage(), containsString("Failed to initialize the RetireJS repo")); + assertThat(ExceptionUtils.getRootCause(ex), instanceOf(JSONException.class)); + assertThat(analyzer.isEnabled(), is(false)); + } + + @Test + void prefersRemoteRetireJsIfEnabled() throws Exception { + getSettings().setBoolean(Settings.KEYS.AUTO_UPDATE, true); + getSettings().setString(Settings.KEYS.ANALYZER_RETIREJS_REPO_JS_URL, testRepositoryFileUrl()); + prepareRetireJs(); + assertThat(analyzer.knownLibraryCountFor("retire-example-0.0.1.js").orElseThrow(), is(1)); + } + } + + private void prepareRetireJs() throws InvalidSettingException, InitializationException { + engine = new Engine(Engine.Mode.EVIDENCE_COLLECTION, getSettings()); + analyzer = newAnalyzer(); + analyzer.prepareFileTypeAnalyzer(engine); + } + + private @NonNull RetireJsAnalyzer newAnalyzer() throws InvalidSettingException { + final RetireJsAnalyzer fileAnalyzer = new RetireJsAnalyzer(); + fileAnalyzer.initialize(getSettings()); + Downloader.getInstance().configure(getSettings()); + return fileAnalyzer; + } +} \ No newline at end of file diff --git a/core/src/test/java/org/owasp/dependencycheck/data/update/RetireJSDataSourceTest.java b/core/src/test/java/org/owasp/dependencycheck/data/update/RetireJSDataSourceTest.java new file mode 100644 index 00000000000..e9f07856f8f --- /dev/null +++ b/core/src/test/java/org/owasp/dependencycheck/data/update/RetireJSDataSourceTest.java @@ -0,0 +1,233 @@ +/* + * This file is part of dependency-check-core. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * Copyright (c) 2026 Chad Wilson. All Rights Reserved. + */ +package org.owasp.dependencycheck.data.update; + +import org.apache.commons.lang3.exception.ExceptionUtils; +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.owasp.dependencycheck.BaseTest; +import org.owasp.dependencycheck.Engine; +import org.owasp.dependencycheck.data.update.exception.UpdateException; +import org.owasp.dependencycheck.utils.InvalidSettingException; +import org.owasp.dependencycheck.utils.Settings; + +import java.net.MalformedURLException; +import java.nio.file.Files; +import java.nio.file.NoSuchFileException; +import java.nio.file.Path; +import java.nio.file.attribute.FileTime; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.instanceOf; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.not; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertThrowsExactly; + +class RetireJSDataSourceTest extends BaseTest { + private static final String TEST_RETIRE_JS_REPOSITORY_FILE = "retirejs/jsrepository.json"; + + private RetireJSDataSource dataSource; + private Engine engine; + + @BeforeEach + public void createEngine() { + dataSource = new RetireJSDataSource(); + engine = new Engine(Engine.Mode.EVIDENCE_COLLECTION, getSettings()); + } + + @AfterEach + void closeEngine() { + if (engine != null) { + dataSource.purge(engine); + engine.close(); + } + } + + @Nested + class Update { + @Test + void doesNothingIfRetireJsDisabled() throws Exception { + getSettings().setBoolean(Settings.KEYS.ANALYZER_RETIREJS_ENABLED, false); + dataSource.update(engine); + assertNoCachedRetireJs(); + } + + @Test + void loadsRemoteIfEmptyWithAutoUpdateDisabled() throws Exception { + getSettings().setBoolean(Settings.KEYS.ANALYZER_RETIREJS_ENABLED, true); + getSettings().setBoolean(Settings.KEYS.AUTO_UPDATE, false); + dataSource.update(engine); + assertCachedRetireJs(); + } + + @Test + void doesNothingIfPresentAndAutoUpdateDisabled() throws Exception { + dataSource.update(engine); + + getSettings().setBoolean(Settings.KEYS.ANALYZER_RETIREJS_ENABLED, true); + getSettings().setBoolean(Settings.KEYS.AUTO_UPDATE, false); + getSettings().setBoolean(Settings.KEYS.ANALYZER_RETIREJS_FORCEUPDATE, false); + + // try again with repo present; but with a URL that would fail if invoked + getSettings().setString(Settings.KEYS.ANALYZER_RETIREJS_REPO_JS_URL, testRepositoryFileUrl().replace(TEST_RETIRE_JS_REPOSITORY_FILE, "doesnt-exist/" + TEST_RETIRE_JS_REPOSITORY_FILE)); + dataSource.update(engine); + } + + @Test + void failsIfRemoteRetireJsRepoNotFound() throws Exception { + getSettings().setBoolean(Settings.KEYS.ANALYZER_RETIREJS_ENABLED, true); + getSettings().setBoolean(Settings.KEYS.AUTO_UPDATE, true); + getSettings().setString(Settings.KEYS.ANALYZER_RETIREJS_REPO_JS_URL, "file:///does-not-exist.xml"); + var ex = assertThrows(UpdateException.class, () -> dataSource.update(engine)); + assertThat(ex.getMessage(), containsString("Failed to initialize the RetireJS repo")); + assertThat(ExceptionUtils.getRootCause(ex), instanceOf(NoSuchFileException.class)); + assertNoCachedRetireJs(); + } + + @Test + void failsIfUrlDoesntIncludeAFileName() { + getSettings().setBoolean(Settings.KEYS.ANALYZER_RETIREJS_ENABLED, true); + getSettings().setString(Settings.KEYS.ANALYZER_RETIREJS_REPO_JS_URL, "https://valid.url.but.no.file/"); + var ex = assertThrowsExactly(UpdateException.class, () -> dataSource.update(engine)); + assertThat(ex.getMessage(), containsString("Unable to determine the local location to cache")); + assertThat(ex.getCause(), instanceOf(InvalidSettingException.class)); + assertThat(ex.getCause().getMessage(), containsString("RetireJS URL must imply a filename")); + } + + @Test + void failsIfUrlIsMalformed() { + getSettings().setBoolean(Settings.KEYS.ANALYZER_RETIREJS_ENABLED, true); + getSettings().setString(Settings.KEYS.ANALYZER_RETIREJS_REPO_JS_URL, "bad-url"); + var ex = assertThrowsExactly(UpdateException.class, () -> dataSource.update(engine)); + assertThat(ex.getMessage(), containsString("Invalid URL for RetireJS repository (bad-url)")); + assertThat(ex.getCause(), instanceOf(MalformedURLException.class)); + } + + @Test + void loadsRemoteRetireJsIfEnabledAndForced() throws Exception { + getSettings().setBoolean(Settings.KEYS.ANALYZER_RETIREJS_ENABLED, true); + getSettings().setBoolean(Settings.KEYS.AUTO_UPDATE, false); + getSettings().setBoolean(Settings.KEYS.ANALYZER_RETIREJS_FORCEUPDATE, true); + getSettings().setString(Settings.KEYS.ANALYZER_RETIREJS_REPO_JS_URL, testRepositoryFileUrl()); + dataSource.update(engine); + + // try again with repo present; but with a URL that fails when invoked + getSettings().setString(Settings.KEYS.ANALYZER_RETIREJS_REPO_JS_URL, testRepositoryFileUrl().replace(TEST_RETIRE_JS_REPOSITORY_FILE, "doesnt-exist/" + TEST_RETIRE_JS_REPOSITORY_FILE)); + var ex = assertThrowsExactly(UpdateException.class, () -> dataSource.update(engine)); + assertThat(ExceptionUtils.getRootCause(ex), instanceOf(NoSuchFileException.class)); + } + + @Test + void loadsRemoteRetireJsIfEnabledWithAutoUpdate() throws Exception { + getSettings().setBoolean(Settings.KEYS.ANALYZER_RETIREJS_ENABLED, true); + getSettings().setBoolean(Settings.KEYS.AUTO_UPDATE, true); + getSettings().setString(Settings.KEYS.ANALYZER_RETIREJS_REPO_JS_URL, testRepositoryFileUrl()); + dataSource.update(engine); + assertThat(Files.readString(cachedRepoFile()), is(testRepositoryFileContent())); + assertThat(Files.exists(cachedRepoFileProperties()), is(true)); + } + + @Test + void doesNothingIfRemoteRetireJsIsNotStale() throws Exception { + getSettings().setBoolean(Settings.KEYS.ANALYZER_RETIREJS_ENABLED, true); + getSettings().setBoolean(Settings.KEYS.AUTO_UPDATE, true); + getSettings().setString(Settings.KEYS.ANALYZER_RETIREJS_REPO_JS_URL, testRepositoryFileUrl()); + getSettings().setInt(Settings.KEYS.ANALYZER_RETIREJS_REPO_VALID_FOR_HOURS, 1); + dataSource.update(engine); + + // Update again immediately + String firstUpdateProperties = Files.readString(cachedRepoFileProperties()); + FileTime firstUpdatePropertiesModified = Files.getLastModifiedTime(cachedRepoFileProperties()); + dataSource.update(engine); + + assertThat(Files.readString(cachedRepoFileProperties()), is(firstUpdateProperties)); + assertThat(Files.getLastModifiedTime(cachedRepoFileProperties()), is(firstUpdatePropertiesModified)); + } + + @Test + void reloadsRemoteRetireJsIfStale() throws Exception { + getSettings().setBoolean(Settings.KEYS.ANALYZER_RETIREJS_ENABLED, true); + getSettings().setBoolean(Settings.KEYS.AUTO_UPDATE, true); + getSettings().setString(Settings.KEYS.ANALYZER_RETIREJS_REPO_JS_URL, testRepositoryFileUrl()); + getSettings().setInt(Settings.KEYS.ANALYZER_RETIREJS_REPO_VALID_FOR_HOURS, 0); + dataSource.update(engine); + + // Reset to force an update + Files.writeString(cachedRepoFile(), "stale content"); + + dataSource.update(engine); + assertThat(Files.readString(cachedRepoFile()), not("stale content")); + } + } + + @Nested + class Purge { + @Test + void purgeRemovesCachedFiles() throws Exception { + getSettings().setBoolean(Settings.KEYS.ANALYZER_RETIREJS_ENABLED, true); + getSettings().setBoolean(Settings.KEYS.AUTO_UPDATE, true); + getSettings().setString(Settings.KEYS.ANALYZER_RETIREJS_REPO_JS_URL, testRepositoryFileUrl()); + dataSource.update(engine); + + assertThat(Files.exists(cachedRepoFile()), is(true)); + dataSource.purge(engine); + assertThat(Files.exists(cachedRepoFile()), is(false)); + } + + @Test + void doesNothingIfNoCachedFile() throws Exception { + dataSource.purge(engine); + assertThat(Files.exists(cachedRepoFile()), is(false)); + } + + @Test + void doesNothingIfUrlIsMalformed() { + getSettings().setString(Settings.KEYS.ANALYZER_RETIREJS_REPO_JS_URL, "bad-url"); + dataSource.purge(engine); + } + } + + private @NonNull Path cachedRepoFile() throws UpdateException { + return dataSource.validatedRepoFile().toPath(); + } + + private @NonNull Path cachedRepoFileProperties() throws UpdateException { + return Path.of(cachedRepoFile() + ".properties"); + } + + private @NonNull String testRepositoryFileUrl() { + return BaseTest.getResourceAsUrlString(this, TEST_RETIRE_JS_REPOSITORY_FILE); + } + + private @NonNull String testRepositoryFileContent() { + return BaseTest.getResourceAsContentString(this, TEST_RETIRE_JS_REPOSITORY_FILE); + } + + private void assertNoCachedRetireJs() throws UpdateException { + assertThat("RetireJS repo file should not exist", dataSource.validatedRepoFile().exists(), is(false)); + } + + private void assertCachedRetireJs() throws UpdateException { + assertThat("RetireJS repo file should exist", dataSource.validatedRepoFile().exists(), is(true)); + } +} diff --git a/core/src/test/resources/retirejs/jsrepository-invalid.json b/core/src/test/resources/retirejs/jsrepository-invalid.json new file mode 100644 index 00000000000..e076eea9296 --- /dev/null +++ b/core/src/test/resources/retirejs/jsrepository-invalid.json @@ -0,0 +1,3 @@ +{ + "bad": "syntax" +} diff --git a/core/src/test/resources/retirejs/jsrepository.json b/core/src/test/resources/retirejs/jsrepository.json new file mode 100644 index 00000000000..1dcaf60ee7f --- /dev/null +++ b/core/src/test/resources/retirejs/jsrepository.json @@ -0,0 +1,37 @@ +{ + "retire-example": { + "vulnerabilities": [ + { + "below": "0.0.2", + "severity": "low", + "cwe": [ + "CWE-477" + ], + "identifiers": { + "summary": "bug summary", + "CVE": [ + "CVE-XXXX-XXXX" + ], + "bug": "1234" + }, + "info": [ + "http://github.com/eoftedal/retire.js/" + ] + } + ], + "extractors": { + "func": [ + "retire.VERSION" + ], + "filename": [ + "retire-example-(§§version§§)(.min)?\\.js" + ], + "filecontent": [ + "/\\*!? Retire-example v(§§version§§)" + ], + "hashes": { + "07f8b94c8d601a24a1914a1a92bec0e4fafda964": "0.0.1" + } + } + } +}