Skip to content

feat: Add a flag to fail when one or more suppression rules are not used #7244

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jeremylong merged 15 commits into from
Dec 20, 2024
Merged

feat: Add a flag to fail when one or more suppression rules are not used #7244

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
b63cc60
turn into UTC the last modified date retrieved from database as requi…
ftiercelin Dec 4, 2024
1bcda3b
Merge branch 'jeremylong:main' into main
ftiercelin Dec 4, 2024
24c8e5a
Update NvdApiDataSource.java
ftiercelin Dec 4, 2024
db522c5
style: checkstyle would complain...
jeremylong Dec 4, 2024
59408b9
Merge branch 'jeremylong:main' into main
ftiercelin Dec 10, 2024
8466ad4
add a flag to fail when one or more suppression rules are not used
ftiercelin Dec 11, 2024
0d85ce0
Merge pull request #1 from ftiercelin/failBuildOnUnusedSuppressionRule
ftiercelin Dec 12, 2024
98670f2
Merge branch 'main' into main
ftiercelin Dec 12, 2024
5c13289
Merge branch 'main' into main
ftiercelin Dec 16, 2024
47c7b08
Merge branch 'main' into main
ftiercelin Dec 16, 2024
b3e718e
Merge branch 'main' into main
ftiercelin Dec 16, 2024
3c3b0ab
add switch to ant client for unused suppression rules failure
ftiercelin Dec 16, 2024
c055230
add setter
ftiercelin Dec 16, 2024
1d68f83
lint
ftiercelin Dec 16, 2024
55a5828
Merge pull request #2 from ftiercelin/failBuildOnUnusedSuppressionRule
ftiercelin Dec 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions ant/src/main/java/org/owasp/dependencycheck/taskdefs/Check.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -449,6 +449,11 @@ public class Check extends Update {
*/
private Reference refId = null;

/**
* whether an unsused suppression rule should get force the build to fail
*/
private boolean failBuildOnUnusedSuppressionRule = false;

/**
* Returns whether the version check is enabled.
*
Expand Down Expand Up @@ -2092,6 +2097,24 @@ public String getArtifactoryAnalyzerBearerToken() {
public void setArtifactoryAnalyzerBearerToken(String artifactoryAnalyzerBearerToken) {
this.artifactoryAnalyzerBearerToken = artifactoryAnalyzerBearerToken;
}

/**
* Returns the value of failBuildOnUnusedSuppressionRule.
* @return whether an unsused suppression rule should get force the build to fail
*/
public boolean getFailBuildOnUnusedSuppressionRule() {
return failBuildOnUnusedSuppressionRule;
}

/**
* Set the value of failBuildOnUnusedSuppressionRule.
*
* @param failBuildOnUnusedSuppressionRule new value of
* failBuildOnUnusedSuppressionRule
*/
public void setFailBuildOnUnusedSuppressionRule(boolean failBuildOnUnusedSuppressionRule) {
this.failBuildOnUnusedSuppressionRule = failBuildOnUnusedSuppressionRule;
}

//see note on `dealWithReferences()` for information on this suppression
@SuppressWarnings("squid:RedundantThrowsDeclarationCheck")
Expand Down Expand Up @@ -2280,6 +2303,7 @@ protected void populateSettings() throws BuildException {
getSettings().setBooleanIfNotNull(Settings.KEYS.ANALYZER_OSSINDEX_USE_CACHE, ossindexAnalyzerUseCache);
getSettings().setBooleanIfNotNull(Settings.KEYS.ANALYZER_OSSINDEX_WARN_ONLY_ON_REMOTE_ERRORS, ossIndexAnalyzerWarnOnlyOnRemoteErrors);
getSettings().setFloat(Settings.KEYS.JUNIT_FAIL_ON_CVSS, junitFailOnCVSS);
getSettings().setBooleanIfNotNull(Settings.KEYS.FAIL_ON_UNUSED_SUPPRESSION_RULE, failBuildOnUnusedSuppressionRule);
}

/**
Expand Down
43 changes: 22 additions & 21 deletions ant/src/site/markdown/configuration.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,27 +30,28 @@ Configuration: dependency-check Task
--------------------
The following properties can be set on the dependency-check task.

Property | Description | Default Value
----------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|-----------------
autoUpdate | Sets whether auto-updating of the NVD CVE/CPE data is enabled. It is not recommended that this be turned to false. | true
failOnError | Whether the build should fail if there is an error executing the dependency-check analysis | true
failBuildOnCVSS | Specifies if the build should be failed if a CVSS score equal to or above a specified level is identified. The default is 11 which means since the CVSS scores are 0-10, by default the build will never fail. More information on CVSS scores can be found at the [NVD](https://nvd.nist.gov/vuln-metrics/cvss)| 11
junitFailOnCVSS | If using the JUNIT report format the junitFailOnCVSS sets the CVSS score threshold that is considered a failure. | 0
prettyPrint | Whether the XML and JSON formatted reports should be pretty printed. | false
projectName | The name of the project being scanned. | Dependency-Check
reportFormat | The report format to be generated (HTML, XML, CSV, JSON, JUNIT, SARIF, JENKINS, GITLAB, ALL). | HTML
reportOutputDirectory | The location to write the report(s). Note, this is not used if generating the report as part of a `mvn site` build | 'target'
hintsFile | The file path to the XML hints file \- used to resolve [false negatives](../general/hints.html) |  
proxyServer | The Proxy Server; see the [proxy configuration](../data/proxy.html) page for more information. |  
proxyPort | The Proxy Port. |  
proxyUsername | Defines the proxy user name. |  
proxyPassword | Defines the proxy password. |  
nonProxyHosts | Defines the hosts that will not be proxied. |  
connectionTimeout | The URL Connection Timeout. |  
enableExperimental | Enable the [experimental analyzers](../analyzers/index.html). If not enabled the experimental analyzers (see below) will not be loaded or used. | false
enableRetired | Enable the [retired analyzers](../analyzers/index.html). If not enabled the retired analyzers (see below) will not be loaded or used. | false
suppressionFile | The file path to the XML suppression file \- used to suppress [false positives](../general/suppression.html). The parameter value can be a local file path, a URL to a suppression file, or even a reference to a file on the class path (see https://github.com/jeremylong/DependencyCheck/issues/1878#issuecomment-487533799) |  
junitFailOnCVSS | If using the JUNIT report format the junitFailOnCVSS sets the CVSS score threshold that is considered a failure. | 0
Property | Description | Default Value
---------------------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|-----------------
autoUpdate | Sets whether auto-updating of the NVD CVE/CPE data is enabled. It is not recommended that this be turned to false. | true
failOnError | Whether the build should fail if there is an error executing the dependency-check analysis | true
failBuildOnCVSS | Specifies if the build should be failed if a CVSS score equal to or above a specified level is identified. The default is 11 which means since the CVSS scores are 0-10, by default the build will never fail. More information on CVSS scores can be found at the [NVD](https://nvd.nist.gov/vuln-metrics/cvss)| 11
junitFailOnCVSS | If using the JUNIT report format the junitFailOnCVSS sets the CVSS score threshold that is considered a failure. | 0
prettyPrint | Whether the XML and JSON formatted reports should be pretty printed. | false
projectName | The name of the project being scanned. | Dependency-Check
reportFormat | The report format to be generated (HTML, XML, CSV, JSON, JUNIT, SARIF, JENKINS, GITLAB, ALL). | HTML
reportOutputDirectory | The location to write the report(s). Note, this is not used if generating the report as part of a `mvn site` build | 'target'
hintsFile | The file path to the XML hints file \- used to resolve [false negatives](../general/hints.html) |  
proxyServer | The Proxy Server; see the [proxy configuration](../data/proxy.html) page for more information. |  
proxyPort | The Proxy Port. |  
proxyUsername | Defines the proxy user name. |  
proxyPassword | Defines the proxy password. |  
nonProxyHosts | Defines the hosts that will not be proxied. |  
connectionTimeout | The URL Connection Timeout. |  
enableExperimental | Enable the [experimental analyzers](../analyzers/index.html). If not enabled the experimental analyzers (see below) will not be loaded or used. | false
enableRetired | Enable the [retired analyzers](../analyzers/index.html). If not enabled the retired analyzers (see below) will not be loaded or used. | false
suppressionFile | The file path to the XML suppression file \- used to suppress [false positives](../general/suppression.html). The parameter value can be a local file path, a URL to a suppression file, or even a reference to a file on the class path (see https://github.com/jeremylong/DependencyCheck/issues/1878#issuecomment-487533799) |  
junitFailOnCVSS | If using the JUNIT report format the junitFailOnCVSS sets the CVSS score threshold that is considered a failure. | 0
failBuildOnUnusedSuppressionRule | Specifies that if any unused suppression rule is found, the build will fail. | false

The following nested elements can be set on the dependency-check task.

Expand Down
23 changes: 23 additions & 0 deletions core/src/main/java/org/owasp/dependencycheck/agent/DependencyCheckScanAgent.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,10 @@ public class DependencyCheckScanAgent {
* Whether the Maven Central analyzer is enabled.
*/
private boolean centralAnalyzerEnabled = true;
/**
* Whether the build should fail if there are unused suppression rules.
*/
private boolean failOnUnusedSuppressionRule = false;
/**
* The URL of Maven Central.
*/
Expand Down Expand Up @@ -619,6 +623,24 @@ public String getCpeStartsWithFilter() {
return cpeStartsWithFilter;
}

/**
* Get the value of failOnUnusedSuppressionRule.
*
* @return the value of failOnUnusedSuppressionRule
*/
public boolean isFailOnUnusedSuppressionRule() {
return failOnUnusedSuppressionRule;
}

/**
* Set the value of failOnUnusedSuppressionRule.
*
* @param failOnUnusedSuppressionRule new value of failOnUnusedSuppressionRule
*/
public void setFailOnUnusedSuppressionRule(boolean failOnUnusedSuppressionRule) {
this.failOnUnusedSuppressionRule = failOnUnusedSuppressionRule;
}

/**
* Get the value of centralAnalyzerEnabled.
*
Expand Down Expand Up @@ -952,6 +974,7 @@ private void populateSettings() {
settings.setStringIfNotEmpty(Settings.KEYS.ADDITIONAL_ZIP_EXTENSIONS, zipExtensions);
settings.setStringIfNotEmpty(Settings.KEYS.NVD_API_KEY, nvdApiKey);
settings.setStringIfNotEmpty(Settings.KEYS.ANALYZER_ASSEMBLY_DOTNET_PATH, pathToCore);
settings.setBoolean(Settings.KEYS.FAIL_ON_UNUSED_SUPPRESSION_RULE, failOnUnusedSuppressionRule);
}

/**
Expand Down
61 changes: 55 additions & 6 deletions core/src/main/java/org/owasp/dependencycheck/analyzer/UnusedSuppressionRuleAnalyzer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@
* @author Jeremy Long
*/
public class UnusedSuppressionRuleAnalyzer extends AbstractAnalyzer {

protected static final String EXCEPTION_MSG = "There are %d unused suppression rule(s): check logs.";

/**
* The Logger for use throughout the class.
*/
Expand All @@ -43,27 +44,54 @@ public class UnusedSuppressionRuleAnalyzer extends AbstractAnalyzer {
* been reported.
*/
private boolean reported = false;
/**
* A flag indicating whether build should fail on unused suppression rule
*/
private boolean shouldFailForUnusedSuppressionRule = false;
/**
* unused suppression rule count
*/
private int unusedSuppressionRuleCount = 0;

@Override
public synchronized void initialize(Settings settings) {
super.initialize(settings);
if (settings.getBoolean(Settings.KEYS.FAIL_ON_UNUSED_SUPPRESSION_RULE, false)) {
this.shouldFailForUnusedSuppressionRule = true;
}
}

@Override
protected void analyzeDependency(Dependency dependency, Engine engine) throws AnalysisException {
if (!reported) {
logUnusedRules(engine);
reported = true;
checkUnusedRules(engine);
reported = true;
if(unusedSuppressionRuleCount > 0 && failsForUnusedSuppressionRule()) {
final String message = String.format(EXCEPTION_MSG, unusedSuppressionRuleCount);
LOGGER.error(message);
throw new AnalysisException(message);
}
}
}

/**
* Logs unused suppression RULES.
* check unused suppression RULES.
*
* @param engine a reference to the ODC engine
*/
private void logUnusedRules(Engine engine) {
protected void checkUnusedRules(Engine engine) {
if (engine.hasObject(SUPPRESSION_OBJECT_KEY)) {
@SuppressWarnings("unchecked")
final List<SuppressionRule> rules = (List<SuppressionRule>) engine.getObject(SUPPRESSION_OBJECT_KEY);
rules.forEach((rule) -> {
if (!rule.isMatched() && !rule.isBase()) {
LOGGER.info("Suppression Rule had zero matches: {}", rule.toString());
final String message = String.format("Suppression Rule had zero matches: %s", rule);
if(failsForUnusedSuppressionRule()) {
LOGGER.error(message);
} else {
LOGGER.info(message);
}
increaseUnusedSuppressionRuleCount();
}
});
}
Expand All @@ -89,4 +117,25 @@ public AnalysisPhase getAnalysisPhase() {
public boolean supportsParallelProcessing() {
return false;
}

/**
* increases the count of unused suppression rules
*/
public void increaseUnusedSuppressionRuleCount() {
unusedSuppressionRuleCount++;
}

/**
* @return the count of unused suppression rules
*/
public int getUnusedSuppressionRuleCount() {
return unusedSuppressionRuleCount;
}

/**
* @return whether the analyzer will fail for a unused suppression rule
*/
public boolean failsForUnusedSuppressionRule() {
return shouldFailForUnusedSuppressionRule;
}
}
1 change: 1 addition & 0 deletions core/src/main/resources/dependencycheck.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ database.batchinsert.maxsize=1000
analyzer.artifactory.enabled=false
analyzer.libman.enabled=true
odc.reports.pretty.print=false
analyzer.suppression.unused.fail=false

hosted.suppressions.enabled=true
hosted.suppressions.url=https://jeremylong.github.io/DependencyCheck/suppressions/publishedSuppressions.xml
Expand Down
Loading
Loading