Skip to content
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.

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

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

Merged
merged 15 commits into from
Dec 20, 2024

Conversation

ftiercelin
Copy link
Contributor

Fixes Issue

#7165

Description of Change

add a flag, failBuildOnUnusedSuppressionRule, which will cause the build to fail if there are unused suppression rules.

Have test cases been added to cover the new functionality?

yes, a new test class was created: org.owasp.dependencycheck.analyzer.UnusedSuppressionRuleAnalyzerTest

@boring-cyborg boring-cyborg bot added ant changes to ant core changes to core documentation site documentation maven changes to the maven plugin tests test cases utils changes to utils labels Dec 12, 2024
@ftiercelin ftiercelin changed the title Add a flag to fail when one or more suppression rules are not used feat: Add a flag to fail when one or more suppression rules are not used Dec 12, 2024
@ftiercelin
Copy link
Contributor Author

ftiercelin commented Dec 13, 2024

to demonstrate the functionality:
using this pom.xml

<project xmlns="http://maven.apache.org/POM/4.0.0"
xmlns:xsi="https://www.w3.org/2001/XMLSchema-instance" 
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
  <modelVersion>4.0.0</modelVersion>
  <groupId>test</groupId>
  <artifactId>DemoProject</artifactId>
  <version>0.0.1-SNAPSHOT</version>
  <name>DemoProject</name>
  <description>DemoProject</description>

  <dependencies>
     <dependency>
        <groupId>org.json</groupId>
        <artifactId>json</artifactId>
        <version>... version ...</version>
     </dependency>
     <dependency>
        <groupId>org.apache.commons</groupId>
        <artifactId>commons-lang3</artifactId>
        <version>3.14.0</version>
     </dependency>
  </dependencies>

  <build>
        <plugins>
              <plugin>
                 <groupId>org.owasp</groupId>
                 <artifactId>dependency-check-maven</artifactId>
                 <version>11.1.2-SNAPSHOT</version>
                 <configuration>
                    <failBuildOnUnusedSuppressionRule> ... flag ...</failBuildOnUnusedSuppressionRule>
                    <skipProvidedScope>true</skipProvidedScope>
                    <skipTestScope>true</skipTestScope>
                    <failOnError>true</failOnError>
                    <suppressionFile>src/owasp.suppression.xml</suppressionFile>
                    <nvdApiKey> ... api key ...</nvdApiKey>
                    <autoUpdate>false</autoUpdate>
                 </configuration>
                 <executions>
                    <execution>
                       <goals>
                          <goal>check</goal>
                       </goals>
                    </execution>
                 </executions>
              </plugin>
        </plugins>
  </build>
</project>

and this suppression file listing a CVE for org.json version 20230618:

<?xml version="1.0" encoding="UTF-8"?>
<suppressions xmlns="https://jeremylong.github.io/DependencyCheck/dependency-suppression.1.3.xsd">
<suppress>
   <notes><![CDATA[
   file name: json-20230618.jar
   ]]></notes>
   <packageUrl regex="true">^pkg:maven/org\.json/json@.*$</packageUrl>
   <vulnerabilityName>CVE-2023-5072</vulnerabilityName>
</suppress>
</suppressions>

Setting the version of the JSON library and the value of the flag, running each time mvn dependency-check:check

flag = true flag =false no failBuildOnUnusedSuppressionRule configuration item
json version=20230618 build succeeds without any message about unused rules build succeeds without any message about unused rules build succeeds without any message about unused rules
json version=20240303 build displays a message about unused rule and fails with exception "There are 1 unused suppression rule(s): check logs." succeeds, with a message about an unused rule succeeds, with a message about an unused rule
no json dependency build displays a message about unused rule and fails with exception "There are 1 unused suppression rule(s): check logs." succeeds, with a message about an unused rule succeeds, with a message about an unused rule

without the suppressionFile configuration item but with failBuildOnCVSS set to 0 (fail for any vulnerability):

flag = true flag =false no failBuildOnUnusedSuppressionRule configuration item
json version=20230618 fails because of vuln found fails because of vuln found fails because of vuln found
json version=20240303 succeeds succeeds succeeds
no json dependency succeeds succeeds succeeds

@chadlwilson
Copy link
Contributor

Thank you for the work here! Haven't looked at the detail - but this would be great/useful! :-)

Copy link
Owner

@jeremylong jeremylong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please add this to the CLI as well, so that if there are unused suppression rules it will return a positive, non-zero error code?

ant/src/site/markdown/configuration.md Show resolved Hide resolved
Copy link
Owner

@jeremylong jeremylong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jeremylong jeremylong merged commit 8c8c19d into jeremylong:main Dec 20, 2024
9 checks passed
@jeremylong jeremylong added this to the 12.0.0 milestone Dec 20, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 20, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
ant changes to ant core changes to core documentation site documentation maven changes to the maven plugin tests test cases utils changes to utils
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants