Skip to content

Commit 0d8dfcc

Browse files
committed
test: Ensure packaged suppressions and hints are validated for loading consistently
Uses the actual analyzers to load them and ensure they can be parsed correctly Signed-off-by: Chad Wilson <29788154+chadlwilson@users.noreply.github.com>
1 parent f07d729 commit 0d8dfcc

File tree

6 files changed

+122
-83
lines changed

6 files changed

+122
-83
lines changed

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

Lines changed: 14 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -263,6 +263,9 @@ private void loadHostedSuppressionBaseData(final SuppressionParser parser, final
263263
HostedSuppressionsDataSource.DEFAULT_SUPPRESSIONS_URL);
264264
final URL url = new URL(configuredUrl);
265265
final String fileName = new File(url.getPath()).getName();
266+
if (fileName.isBlank()) {
267+
throw new IllegalArgumentException("Hosted Suppression URL must imply a filename");
268+
}
266269
final File repoFile = new File(getSettings().getDataDirectory(), fileName);
267270
boolean repoEmpty = !repoFile.isFile() || repoFile.length() <= 1L;
268271
if (repoEmpty) {
@@ -333,18 +336,6 @@ private void loadCachedHostedSuppressionsRules(final SuppressionParser parser, f
333336
}
334337
}
335338

336-
private static boolean forceUpdateHostedSuppressions(final Engine engine, final File repoFile) {
337-
final HostedSuppressionsDataSource ds = new HostedSuppressionsDataSource();
338-
boolean repoEmpty = true;
339-
try {
340-
ds.update(engine);
341-
repoEmpty = !repoFile.isFile() || repoFile.length() <= 1L;
342-
} catch (UpdateException ex) {
343-
LOGGER.warn("Failed to update the Hosted Suppression file", ex);
344-
}
345-
return repoEmpty;
346-
}
347-
348339
/**
349340
* Load a single suppression rules file from the path provided using the
350341
* parser provided.
@@ -407,19 +398,17 @@ private List<SuppressionRule> loadSuppressionFile(final SuppressionParser parser
407398
}
408399
}
409400
}
410-
if (file != null) {
411-
if (!file.exists()) {
412-
final String msg = String.format("Suppression file '%s' does not exist", file.getPath());
413-
LOGGER.warn(msg);
414-
throw new SuppressionParseException(msg);
415-
}
416-
try {
417-
list.addAll(parser.parseSuppressionRules(file));
418-
} catch (SuppressionParseException ex) {
419-
LOGGER.warn("Unable to parse suppression xml file '{}'", file.getPath());
420-
LOGGER.warn(ex.getMessage());
421-
throw ex;
422-
}
401+
if (!file.exists()) {
402+
final String msg = String.format("Suppression file '%s' does not exist", file.getPath());
403+
LOGGER.warn(msg);
404+
throw new SuppressionParseException(msg);
405+
}
406+
try {
407+
list.addAll(parser.parseSuppressionRules(file));
408+
} catch (SuppressionParseException ex) {
409+
LOGGER.warn("Unable to parse suppression xml file '{}'", file.getPath());
410+
LOGGER.warn(ex.getMessage());
411+
throw ex;
423412
}
424413
} catch (DownloadFailedException ex) {
425414
throwSuppressionParseException("Unable to fetch the configured suppression file", ex, suppressionFilePath);

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

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

20+
import org.jetbrains.annotations.VisibleForTesting;
2021
import org.owasp.dependencycheck.Engine;
2122
import org.owasp.dependencycheck.analyzer.exception.AnalysisException;
2223
import org.owasp.dependencycheck.dependency.Dependency;
@@ -327,4 +328,14 @@ private void loadHintRules() throws HintParseException {
327328
LOGGER.debug("{} hint rules were loaded.", hints.length);
328329
LOGGER.debug("{} duplicating hint rules were loaded.", vendorHints.length);
329330
}
331+
332+
@VisibleForTesting
333+
HintRule[] getHintRules() {
334+
return hints;
335+
}
336+
337+
@VisibleForTesting
338+
VendorDuplicatingHintRule[] getVendorDuplicatingHintRules() {
339+
return vendorHints;
340+
}
330341
}

core/src/main/java/org/owasp/dependencycheck/xml/suppression/SuppressionRule.java

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import java.util.HashSet;
2323
import java.util.Iterator;
2424
import java.util.List;
25+
import java.util.Objects;
2526
import java.util.Set;
2627
import javax.annotation.concurrent.NotThreadSafe;
2728
import org.apache.commons.lang3.time.DateFormatUtils;
@@ -856,4 +857,30 @@ public String toString() {
856857
sb.append('}');
857858
return sb.toString();
858859
}
860+
861+
@Override
862+
public boolean equals(Object o) {
863+
if (o == null || getClass() != o.getClass()) return false;
864+
SuppressionRule that = (SuppressionRule) o;
865+
return base == that.base
866+
&& Objects.equals(filePath, that.filePath)
867+
&& Objects.equals(sha1, that.sha1)
868+
&& Objects.equals(cpe, that.cpe)
869+
&& Objects.equals(cvssBelow, that.cvssBelow)
870+
&& Objects.equals(cvssV2Below, that.cvssV2Below)
871+
&& Objects.equals(cvssV3Below, that.cvssV3Below)
872+
&& Objects.equals(cvssV4Below, that.cvssV4Below)
873+
&& Objects.equals(cwe, that.cwe)
874+
&& Objects.equals(cve, that.cve)
875+
&& Objects.equals(vulnerabilityNames, that.vulnerabilityNames)
876+
&& Objects.equals(gav, that.gav)
877+
&& Objects.equals(packageUrl, that.packageUrl)
878+
&& Objects.equals(notes, that.notes)
879+
&& Objects.equals(until, that.until);
880+
}
881+
882+
@Override
883+
public int hashCode() {
884+
return Objects.hash(filePath, sha1, cpe, cvssBelow, cvssV2Below, cvssV3Below, cvssV4Below, cwe, cve, vulnerabilityNames, gav, packageUrl, notes, base, until);
885+
}
859886
}

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

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

20+
import org.jspecify.annotations.NonNull;
2021
import org.junit.jupiter.api.BeforeEach;
22+
import org.junit.jupiter.api.Nested;
2123
import org.junit.jupiter.api.Test;
2224
import org.owasp.dependencycheck.BaseTest;
2325
import org.owasp.dependencycheck.Engine;
2426
import org.owasp.dependencycheck.Engine.Mode;
2527
import org.owasp.dependencycheck.dependency.Dependency;
2628
import org.owasp.dependencycheck.exception.InitializationException;
2729
import org.owasp.dependencycheck.utils.Downloader;
30+
import org.owasp.dependencycheck.utils.InvalidSettingException;
2831
import org.owasp.dependencycheck.utils.Settings;
2932
import org.owasp.dependencycheck.utils.Settings.KEYS;
3033
import org.owasp.dependencycheck.xml.suppression.SuppressionRule;
3134

35+
import java.util.List;
3236
import java.util.Set;
37+
import java.util.stream.Collectors;
3338

3439
import static org.hamcrest.CoreMatchers.is;
3540
import static org.hamcrest.MatcherAssert.assertThat;
41+
import static org.hamcrest.Matchers.empty;
42+
import static org.hamcrest.Matchers.not;
3643
import static org.junit.jupiter.api.Assertions.assertEquals;
3744
import static org.junit.jupiter.api.Assertions.assertNull;
3845
import static org.junit.jupiter.api.Assertions.assertThrows;
46+
import static org.owasp.dependencycheck.analyzer.AbstractSuppressionAnalyzer.SUPPRESSION_OBJECT_KEY;
3947

4048
/**
4149
* @author Jeremy Long
@@ -134,12 +142,8 @@ void testFailureToLocateSuppressionFileAnywhere() {
134142
*/
135143
private int getNumberOfRulesLoadedInCoreFile() throws Exception {
136144
getSettings().removeProperty(KEYS.SUPPRESSION_FILE);
137-
final AbstractSuppressionAnalyzerImpl coreFileAnalyzer = new AbstractSuppressionAnalyzerImpl();
138-
coreFileAnalyzer.initialize(getSettings());
139-
Engine engine = new Engine(Mode.EVIDENCE_COLLECTION, getSettings());
140-
coreFileAnalyzer.prepare(engine);
141-
int count = AbstractSuppressionAnalyzer.getRuleCount(engine);
142-
return count;
145+
Engine engine = prepareSuppressions();
146+
return AbstractSuppressionAnalyzer.getRuleCount(engine);
143147
}
144148

145149
/**
@@ -152,13 +156,53 @@ private int getNumberOfRulesLoadedInCoreFile() throws Exception {
152156
*/
153157
private int getNumberOfRulesLoadedFromPath(final String path) throws Exception {
154158
getSettings().setString(KEYS.SUPPRESSION_FILE, path);
159+
Engine engine = prepareSuppressions();
160+
return AbstractSuppressionAnalyzer.getRuleCount(engine);
161+
}
162+
163+
private @NonNull Engine prepareSuppressions() throws InvalidSettingException, InitializationException {
155164
final AbstractSuppressionAnalyzerImpl fileAnalyzer = new AbstractSuppressionAnalyzerImpl();
156165
fileAnalyzer.initialize(getSettings());
157166
Downloader.getInstance().configure(getSettings());
158167
Engine engine = new Engine(Mode.EVIDENCE_COLLECTION, getSettings());
159168
fileAnalyzer.prepare(engine);
160-
int count = AbstractSuppressionAnalyzer.getRuleCount(engine);
161-
return count;
169+
return engine;
170+
}
171+
172+
@Nested
173+
class CoreSuppressionsLoading {
174+
@Test
175+
void testLoadCorePackagedSuppressions() throws Exception {
176+
List<SuppressionRule> baseRules = assertAllBaseSuppressionRulesAreMarkedCorrectly();
177+
178+
assertAllHostedSnapshotSuppressionsAreMarkedAsBase(baseRules);
179+
}
180+
181+
private void assertAllHostedSnapshotSuppressionsAreMarkedAsBase(List<SuppressionRule> baseRules) throws InvalidSettingException, InitializationException {
182+
getSettings().setBoolean(KEYS.HOSTED_SUPPRESSIONS_ENABLED, true);
183+
getSettings().setString(KEYS.HOSTED_SUPPRESSIONS_URL, "https://intentionally-bad-url/hosted-suppressions.xml");
184+
Engine engine = prepareSuppressions();
185+
186+
@SuppressWarnings("unchecked") List<SuppressionRule> allRules = (List<SuppressionRule>) engine.getObject(SUPPRESSION_OBJECT_KEY);
187+
188+
List<SuppressionRule> hostedSnapshotRules = allRules.stream().filter(r -> !baseRules.contains(r)).collect(Collectors.toList());
189+
assertThat(hostedSnapshotRules, not(empty()));
190+
assertThat("Expected all suppressions in hosted suppressions snapshot file to be marked as base", allRulesNotMarkedAsBase(hostedSnapshotRules), empty());
191+
}
192+
193+
private @NonNull List<SuppressionRule> assertAllBaseSuppressionRulesAreMarkedCorrectly() throws InvalidSettingException, InitializationException {
194+
getSettings().setBoolean(KEYS.HOSTED_SUPPRESSIONS_ENABLED, false);
195+
Engine engine = prepareSuppressions();
196+
197+
@SuppressWarnings("unchecked") List<SuppressionRule> baseRules = (List<SuppressionRule>) engine.getObject(SUPPRESSION_OBJECT_KEY);
198+
assertThat(baseRules, not(empty()));
199+
assertThat("Expected all suppressions in base file to be marked as base", allRulesNotMarkedAsBase(baseRules), empty());
200+
return baseRules;
201+
}
202+
203+
private @NonNull List<SuppressionRule> allRulesNotMarkedAsBase(List<SuppressionRule> baseRules) {
204+
return baseRules.stream().filter(r -> !r.isBase()).collect(Collectors.toList());
205+
}
162206
}
163207

164208
public static class AbstractSuppressionAnalyzerImpl extends AbstractSuppressionAnalyzer {

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
*/
1616
package org.owasp.dependencycheck.analyzer;
1717

18+
import org.junit.jupiter.api.Nested;
1819
import org.junit.jupiter.api.Test;
1920
import org.owasp.dependencycheck.BaseDBTestCase;
2021
import org.owasp.dependencycheck.BaseTest;
@@ -26,7 +27,11 @@
2627
import org.owasp.dependencycheck.utils.Settings;
2728

2829
import java.io.File;
30+
import java.util.List;
2931

32+
import static org.hamcrest.MatcherAssert.assertThat;
33+
import static org.hamcrest.Matchers.empty;
34+
import static org.hamcrest.Matchers.not;
3035
import static org.junit.jupiter.api.Assertions.assertEquals;
3136
import static org.junit.jupiter.api.Assertions.assertFalse;
3237
import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -130,6 +135,19 @@ void testAnalyze_1() throws Exception {
130135
assertEquals(1, d.getEvidence(EvidenceType.VENDOR).size(), "vendor evidence mismatch");
131136
assertEquals(1, d.getEvidence(EvidenceType.PRODUCT).size(), "product evidence mismatch");
132137
assertEquals(2, d.getEvidence(EvidenceType.VERSION).size(), "version evidence mismatch");
138+
}
139+
140+
@Nested
141+
class CoreHintsLoading {
142+
@Test
143+
void testLoadCorePackagedHints() throws Exception {
144+
getSettings().removeProperty(Settings.KEYS.HINTS_FILE);
145+
HintAnalyzer instance = new HintAnalyzer();
146+
instance.initialize(getSettings());
147+
instance.prepare(null);
133148

149+
assertThat(List.of(instance.getHintRules()), not(empty()));
150+
assertThat(List.of(instance.getVendorDuplicatingHintRules()), not(empty()));
151+
}
134152
}
135153
}

core/src/test/java/org/owasp/dependencycheck/resources/DependencyCheckBaseSuppressionTest.java

Lines changed: 0 additions & 50 deletions
This file was deleted.

0 commit comments

Comments
 (0)