chore: checkstyle configuration and suggested updates (#7003)

jeremylong · Oct 3, 2024 · 76051ca · 76051ca
1 parent ee2b032
commit 76051ca
Show file tree

Hide file tree

Showing 29 changed files with 322 additions and 235 deletions.
diff --git a/.github/workflows/pull_requests.yml b/.github/workflows/pull_requests.yml
@@ -10,6 +10,7 @@ jobs:
   test:
     name: Build and Test
     permissions: 
+      security-events: write
       contents: read
     runs-on: ubuntu-latest 
     steps:
@@ -43,10 +44,31 @@ jobs:
         with:
           # Command to be sent to SARIF Multitool
           command: 'validate core/target/test-reports/Report.sarif'
+      - name: Upload SARIF file
+        uses: github/codeql-action/upload-sarif@v3
+        with:
+          sarif_file: utils/target/spotbugsSarif.json
+          category: spotbugs-utils
+      - name: Upload SARIF file
+        uses: github/codeql-action/upload-sarif@v3
+        with:
+          sarif_file: cli/target/spotbugsSarif.json
+          category: spotbugs-cli
+      - name: Upload SARIF file
+        uses: github/codeql-action/upload-sarif@v3
+        with:
+          sarif_file: ant/target/spotbugsSarif.json
+          category: spotbugs-ant
+      - name: Upload SARIF file
+        uses: github/codeql-action/upload-sarif@v3
+        with:
+          sarif_file: core/target/spotbugsSarif.json
+          category: spotbugs-core
 
   maven:
     name: Regression Test Maven Plugin
     permissions: 
+      security-events: write
       contents: read
     runs-on: ubuntu-latest 
     steps:
@@ -76,7 +98,7 @@ jobs:
         env:
           NVD_API_KEY: ${{ secrets.NVD_API_KEY }}
         run: |
-            mvn -V -s settings.xml -pl utils,core,maven -am compile verify -DtestMavenPlugin -DreleaseTesting --no-transfer-progress --batch-mode
+            mvn -V -s settings.xml -pl maven -am compile verify -DtestMavenPlugin -DreleaseTesting --no-transfer-progress --batch-mode
       - name: Archive IT test logs
         id: archive-logs
         if: always()
@@ -85,32 +107,40 @@ jobs:
           name: it-test-logs
           retention-days: 7
           path: maven/target/it/**/build.log
-# this action has been failing - so I'm disabling it; we never really found anything anyways
-#  audit:
-#    runs-on: ubuntu-latest
-#    permissions: 
-#      contents: read
-#      pull-requests: write
-#    name: Audit
-#    steps:
-#      - uses: actions/checkout@v4
-#      - name: Check Maven Cache
-#        id: maven-cache
-#        uses: actions/cache@v4
-#        with:
-#          path: ~/.m2/repository/
-#          key: ${{ runner.os }}-maven-${{ hashFiles('**/pom.xml') }}
-#          restore-keys: |
-#            ${{ runner.os }}-maven-
-#      - name: Semgrep
-#        id: semgrep
-#        run: |
-#            docker run --rm -v "${PWD}:/src" returntocorp/semgrep semgrep --config "p/ci" --sarif > semgrep.sarif
-#      - name: Maven Site
-#        if: always()
-#        run: |
-#            mvn -s settings.xml package site -DskipTests=true --no-transfer-progress --batch-mode
-#      - name: Publish Comments
-#        if: always()
-#        run: |
-#            mvn se.bjurr.violations:violation-comments-to-github-maven-plugin:violation-comments --no-transfer-progress --batch-mode -DpullRequestId=${{ github.event.pull_request.number }} -DoAuth2Token=${{ secrets.GITHUB_TOKEN }}
+      - name: Upload SARIF file
+        uses: github/codeql-action/upload-sarif@v3
+        with:
+          sarif_file: maven/target/spotbugsSarif.json
+          category: spotbugs-maven
+
+  checkstyle:
+    name: Checkstyle Validation
+    permissions:
+      security-events: write
+      contents: read
+    runs-on: ubuntu-latest 
+    steps:
+      - uses: actions/checkout@v4
+      - name: Check Maven Cache
+        id: maven-cache
+        uses: actions/cache@v4
+        with:
+          path: ~/.m2/repository/
+          key: ${{ runner.os }}-maven-${{ hashFiles('**/pom.xml') }}
+          restore-keys: |
+            ${{ runner.os }}-maven-
+      - name: Set up JDK 11
+        id: jdk-11
+        uses: actions/setup-java@v4
+        with:
+          java-version: 11
+          distribution: 'zulu'
+      - name: Checkstyle
+        id: checkstyle
+        run: |
+            mvn -V -s settings.xml checkstyle:checkstyle-aggregate --no-transfer-progress --batch-mode
+      - name: Upload SARIF file
+        uses: github/codeql-action/upload-sarif@v3
+        with:
+          sarif_file: target/checkstyle-result.sarif
+          category: checkstyle
diff --git a/cli/src/main/java/org/owasp/dependencycheck/CliParser.java b/cli/src/main/java/org/owasp/dependencycheck/CliParser.java
@@ -374,7 +374,7 @@ private void addAdvancedOptions(final Options options) {
                         "Credentials for basic authentication to the NVD API Datafeed."))
                 .addOption(newOptionWithArg(ARGUMENT.NVD_API_DATAFEED_PASSWORD, "password",
                         "Credentials for basic authentication to the NVD API Datafeed."))
-                .addOption(newOptionWithArg(ARGUMENT.NVD_API_MAX_RETRY_COUNT,"count",
+                .addOption(newOptionWithArg(ARGUMENT.NVD_API_MAX_RETRY_COUNT, "count",
                         "The maximum number of retry requests for a single call to the NVD API."))
                 .addOption(newOptionWithArg(ARGUMENT.NVD_API_VALID_FOR_HOURS, "hours",
                         "The number of hours to wait before checking for new updates from the NVD."))
@@ -1144,16 +1144,17 @@ public static class ARGUMENT {
          */
         public static final String DATA_DIRECTORY = "data";
         /**
-         * The CLI argument name for setting the URL for the NVD API Endpoint
+         * The CLI argument name for setting the URL for the NVD API Endpoint.
          */
         public static final String NVD_API_ENDPOINT = "nvdApiEndpoint";
         /**
          * The CLI argument name for setting the URL for the NVD API Key.
          */
         public static final String NVD_API_KEY = "nvdApiKey";
         /**
-        * The CLI argument name for setting the maximum number of retry requests for a single call to the NVD API.
-        */
+         * The CLI argument name for setting the maximum number of retry
+         * requests for a single call to the NVD API.
+         */
         public static final String NVD_API_MAX_RETRY_COUNT = "nvdMaxRetryCount";
         /**
          * The CLI argument name for setting the number of hours to wait before

diff --git a/core/src/main/java/org/owasp/dependencycheck/analyzer/AbstractSuppressionAnalyzer.java b/core/src/main/java/org/owasp/dependencycheck/analyzer/AbstractSuppressionAnalyzer.java
@@ -346,12 +346,14 @@ private List<SuppressionRule> loadSuppressionFile(final SuppressionParser parser
                 file = getSettings().getTempFile("suppression", "xml");
                 final URL url = new URL(suppressionFilePath);
                 try {
-                    Downloader.getInstance().fetchFile(url, file, false, Settings.KEYS.SUPPRESSION_FILE_USER, Settings.KEYS.SUPPRESSION_FILE_PASSWORD);
+                    Downloader.getInstance().fetchFile(url, file, false, Settings.KEYS.SUPPRESSION_FILE_USER,
+                            Settings.KEYS.SUPPRESSION_FILE_PASSWORD);
                 } catch (DownloadFailedException ex) {
                     LOGGER.trace("Failed download suppression file - first attempt", ex);
                     try {
                         Thread.sleep(500);
-                        Downloader.getInstance().fetchFile(url, file, true, Settings.KEYS.SUPPRESSION_FILE_USER, Settings.KEYS.SUPPRESSION_FILE_PASSWORD);
+                        Downloader.getInstance().fetchFile(url, file, true, Settings.KEYS.SUPPRESSION_FILE_USER,
+                                Settings.KEYS.SUPPRESSION_FILE_PASSWORD);
                     } catch (TooManyRequestsException ex1) {
                         throw new SuppressionParseException("Unable to download supression file `" + file
                                 + "`; received 429 - too many requests", ex1);

diff --git a/core/src/main/java/org/owasp/dependencycheck/analyzer/CarthageAnalyzer.java b/core/src/main/java/org/owasp/dependencycheck/analyzer/CarthageAnalyzer.java
@@ -86,15 +86,16 @@ public class CarthageAnalyzer extends AbstractFileTypeAnalyzer {
     private static final FileFilter CARTHAGE_FILTER = FileFilterBuilder.newInstance().addFilenames(CARTFILE_RESOLVED).build();
 
     /**
-     * The capture group #1 is the dependency type, #2 is the name, #3 is dependency version.
-     * The version can be a commit ref, so we can't assume it's a number
+     * The capture group #1 is the dependency type, #2 is the name, #3 is
+     * dependency version. The version can be a commit ref, so we can't assume
+     * it's a number
      *
-     * Example values:
-     * - binary "https://dl.google.com/geosdk/GoogleMaps.json" "7.2.0"
-     * - git "https://gitlab.matrix.org/matrix-org/olm.git" "3.2.16"
-     * - github "alinradut/SwiftEntryKit" "95f4a08f41ddcf2c02e2b22789038774c8c94df5""
-     * - github "CocoaLumberjack/CocoaLumberjack" "3.8.5"
-     * - github "realm/realm-swift" "v10.44.0"
+     * Example values: - binary "https://dl.google.com/geosdk/GoogleMaps.json"
+     * "7.2.0" - git "https://gitlab.matrix.org/matrix-org/olm.git" "3.2.16" -
+     * github "alinradut/SwiftEntryKit"
+     * "95f4a08f41ddcf2c02e2b22789038774c8c94df5"" - github
+     * "CocoaLumberjack/CocoaLumberjack" "3.8.5" - github "realm/realm-swift"
+     * "v10.44.0"
      */
     private static final Pattern CARTFILE_RESOLVED_DEPENDENCY_PATTERN = Pattern.compile("(github|git|binary) \"([^\"]+)\" \"([^\"]+)\"");
 
@@ -106,9 +107,8 @@ public class CarthageAnalyzer extends AbstractFileTypeAnalyzer {
     /**
      * Capture group #1 is the dependency name.
      *
-     * Example values:
-     * - robbiehanson/XMPPFramework
-     * - CocoaLumberjack/CocoaLumberjack
+     * Example values: - robbiehanson/XMPPFramework -
+     * CocoaLumberjack/CocoaLumberjack
      */
     private static final Pattern CARTFILE_RESOLVED_GITHUB_DEPENDENCY = Pattern.compile("[a-zA-Z0-9-_]+/([a-zA-Z0-9\\-_\\.]+)");
 
@@ -120,11 +120,9 @@ public class CarthageAnalyzer extends AbstractFileTypeAnalyzer {
     /**
      * Capture group #1 is the dependency name.
      *
-     * Example values:
-     * - https://my.domain.com/release/MyFramework.json
-     * - file:///some/Path/MyFramework.json
-     * - relative/path/MyFramework.json
-     * - /absolute/path/MyFramework.json
+     * Example values: - https://my.domain.com/release/MyFramework.json -
+     * file:///some/Path/MyFramework.json - relative/path/MyFramework.json -
+     * /absolute/path/MyFramework.json
      */
     private static final Pattern CARTFILE_RESOLVED_BINARY_DEPENDENCY = Pattern.compile("([a-zA-Z0-9\\-_\\.]+).json");
 
@@ -186,6 +184,7 @@ protected void analyzeDependency(Dependency dependency, Engine engine)
      * Analyzes the Cartfile.resolved and adds the evidence to the dependency.
      *
      * @param cartfileResolved the dependency
+     * @param engine a reference to the dependency-check engine
      * @throws AnalysisException thrown if there is an error analyzing the
      * Cartfile
      */
@@ -210,8 +209,7 @@ private void analyzeCartfileResolvedDependency(Dependency cartfileResolved, Engi
             final Matcher versionMatcher = CARTFILE_VERSION_PATTERN.matcher(version);
             if (versionMatcher.find()) {
                 version = versionMatcher.group(1);
-            }
-            else {
+            } else {
                 // this is probably a git commit reference, so we'll default to 0.0.0.
                 // this will probably bubble up a ton of CVEs, but serves you right for
                 // not using semantic versioning.
@@ -224,15 +222,13 @@ private void analyzeCartfileResolvedDependency(Dependency cartfileResolved, Engi
                     continue;
                 }
                 name = nameMatcher.group(1);
-            }
-            else if (type.contentEquals("github")) {
+            } else if (type.contentEquals("github")) {
                 final Matcher nameMatcher = CARTFILE_RESOLVED_GITHUB_DEPENDENCY.matcher(name);
                 if (!nameMatcher.find()) {
                     continue;
                 }
                 name = nameMatcher.group(1);
-            }
-            else if (type.contentEquals("binary")) {
+            } else if (type.contentEquals("binary")) {
                 final Matcher nameMatcher = CARTFILE_RESOLVED_BINARY_DEPENDENCY.matcher(name);
                 if (!nameMatcher.find()) {
                     continue;

diff --git a/core/src/main/java/org/owasp/dependencycheck/analyzer/PinnedMavenInstallAnalyzer.java b/core/src/main/java/org/owasp/dependencycheck/analyzer/PinnedMavenInstallAnalyzer.java
@@ -123,11 +123,11 @@ protected void analyzeDependency(Dependency dependency, Engine engine) throws An
         final DependencyTree tree;
         List<MavenDependency> deps;
         try {
-            JsonNode jsonNode = MAPPER.readTree(dependencyFile);
-            JsonNode v2Version = jsonNode.path("version");
-            JsonNode v010Version = jsonNode.path("dependency_tree").path("version");
+            final JsonNode jsonNode = MAPPER.readTree(dependencyFile);
+            final JsonNode v2Version = jsonNode.path("version");
+            final JsonNode v010Version = jsonNode.path("dependency_tree").path("version");
 
-             if (v2Version.isTextual()) {
+            if (v2Version.isTextual()) {
                 final InstallFileV2 installFile = INSTALL_FILE_V2_READER.readValue(dependencyFile);
                 if (!Objects.equals(installFile.getAutogeneratedSentinel(), "THERE_IS_NO_DATA_ONLY_ZUUL")) {
                     return;
@@ -155,7 +155,6 @@ protected void analyzeDependency(Dependency dependency, Engine engine) throws An
                 return;
             }
 
-
         } catch (IOException e) {
             System.out.println("e");
             return;
@@ -325,11 +324,12 @@ public String getVersion() {
      * {@code .dependency_tree.dependencies}.
      */
     private static class MavenDependency {
-        public MavenDependency(String coord) {
+
+        MavenDependency(String coord) {
             this.coord = coord;
         }
 
-        public MavenDependency() {
+        MavenDependency() {
         }
         /**
          * The standard Maven coordinate string
@@ -352,7 +352,13 @@ public String getCoord() {
      * A reusable reader for {@link InstallFile}.
      */
     private static final ObjectReader INSTALL_FILE_READER;
+    /**
+     * A reusable reader for {@link InstallFileV2}.
+     */
     private static final ObjectReader INSTALL_FILE_V2_READER;
+    /**
+     * A reusable object mapper.
+     */
     private static final ObjectMapper MAPPER;
 
     static {
@@ -367,11 +373,12 @@ public String getCoord() {
      * file.
      *
      * <p>
-     * At the time of writing, the latest version is 2, and the dependencies
-     * are stored in {@code .artifacts}.
+     * At the time of writing, the latest version is 2, and the dependencies are
+     * stored in {@code .artifacts}.
      *
      * <p>
-     * The top-level keys we care about are {@code .artifacts}. {@code .version}.
+     * The top-level keys we care about are {@code .artifacts}.
+     * {@code .version}.
      */
     private static class InstallFileV2 {
 
@@ -382,8 +389,9 @@ private static class InstallFileV2 {
         private String version;
 
         /**
-         * A list of Maven dependencies made available. Note that this map is transitively closed and
-         * pinned to a specific version of each artifact.
+         * A list of Maven dependencies made available. Note that this map is
+         * transitively closed and pinned to a specific version of each
+         * artifact.
          * <p>
          * The key is the Maven coordinate string, less the version
          * {@code group:artifact[:optional classifier][:optional packaging]}.
@@ -394,8 +402,8 @@ private static class InstallFileV2 {
         private Map<String, Artifactv2> artifacts;
 
         /**
-         * A sentinel value placed in the file to indicate that it is an auto-generated pinned maven
-         * install file.
+         * A sentinel value placed in the file to indicate that it is an
+         * auto-generated pinned maven install file.
          */
         @JsonProperty("__AUTOGENERATED_FILE_DO_NOT_MODIFY_THIS_FILE_MANUALLY")
         private String autogeneratedSentinel;
@@ -427,23 +435,23 @@ public String getAutogeneratedSentinel() {
             return autogeneratedSentinel;
         }
     }
+
     private static class Artifactv2 {
 
-          /**
-          * The version of the artifact.
-          */
-          @JsonProperty("version")
-          private String version;
-
-          /**
-          * Returns the value of version.
-          *
-          * @return the value of version
-          */
-          public String getVersion() {
-              return version;
-          }
-    }
+        /**
+         * The version of the artifact.
+         */
+        @JsonProperty("version")
+        private String version;
 
+        /**
+         * Returns the value of version.
+         *
+         * @return the value of version
+         */
+        public String getVersion() {
+            return version;
+        }
+    }
 
 }
diff --git a/core/src/main/java/org/owasp/dependencycheck/analyzer/YarnAuditAnalyzer.java b/core/src/main/java/org/owasp/dependencycheck/analyzer/YarnAuditAnalyzer.java
@@ -280,7 +280,7 @@ private List<Advisory> analyzePackage(final File lockFile, final File packageFil
             final JsonObject lockJson = fetchYarnAuditJson(dependency, skipDevDependencies);
             // Retrieves the contents of package-lock.json from the Dependency
             final JsonObject packageJson;
-            try (final JsonReader packageReader = Json.createReader(Files.newInputStream(packageFile.toPath()))) {
+            try (JsonReader packageReader = Json.createReader(Files.newInputStream(packageFile.toPath()))) {
                 packageJson = packageReader.readObject();
             }
             // Modify the payload to meet the NPM Audit API requirements