Fix #59: Bump dependency-check (#66)

* Bump Dependency-Check * Tolerant POM implementation. --------- Co-authored-by: Andrea Fantasticini <[email protected]> Co-authored-by: Roberto Polli <[email protected]>
par-tec · Nov 15, 2023 · 746a559 · 746a559
1 parent 9cd3c17
commit 746a559
Show file tree

Hide file tree

Showing 7 changed files with 277 additions and 75 deletions.
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
@@ -26,9 +26,12 @@ repos:
         - --remove-unused-variables
         - --remove-all-unused-imports
 -   repo: https://github.com/psf/black
-    rev: 23.1.0
+    rev: 23.11.0
     hooks:
     -   id: black
+        # Since pre-commit passes each file individually to black,
+        #   config options like extend-exclude don't work.
+        exclude: 'tests/data'
 - repo: https://github.com/pycqa/isort
   rev: 5.12.0
   hooks:
@@ -47,6 +50,7 @@ repos:
   rev: 6.0.0
   hooks:
   - id: flake8
+    exclude: 'tests/data'
 - repo: https://github.com/PyCQA/bandit
   rev: 1.7.4
   hooks:

diff --git a/entrypoint.py b/entrypoint.py
@@ -21,6 +21,8 @@
 from sys import stderr, stdout
 from xml.etree import ElementTree
 
+from packaging.version import parse as parse_version
+
 # Log to stdout
 # for both stdout and stderr.
 logging.basicConfig(
@@ -134,9 +136,10 @@ def plugins(self):
     def plugins_tag(self):
         return self.findall(".//build/plugins")[0]
 
-    def add_plugins(self, plugins):
+    def add_plugins(self, plugins) -> list:
         if not plugins:
             return
+        errors = []
         build = self.find("build")
         # Add build tag if it doesn't exist.
         if not build:
@@ -157,13 +160,28 @@ def add_plugins(self, plugins):
                 log.info(f"Adding plugin {POM.plugin_id(plugin)}")
                 plugins_tag.append(plugin)
                 continue
-            if existing_version == target_version:
-                log.info(f"Plugin {POM.plugin_id(plugin)} already exists. Skipping.")
+
+            target_version = parse_version(target_version)
+            existing_version = parse_version(existing_version)
+
+            if target_version < existing_version:
+                log.info(
+                    f"Plugin {POM.plugin_id(plugin)} already exists with a newer version {existing_version}. Skipping."
+                )
                 continue
-            if existing_version != target_version:
-                raise ValueError(
-                    f"Plugin {target_plugin} already exists with version {existing_version}"
+            if target_version == existing_version:
+                log.info(
+                    f"Plugin {POM.plugin_id(plugin)}@{target_version} already present. Skipping."
                 )
+                continue
+            if target_version > existing_version:
+                # At the moment, we don't do this, since upgrading plugins can break builds.
+                errors.append(
+                    ValueError(
+                        f"Plugin {target_plugin} already exists with an older version {existing_version}. Cannot add {target_version}."
+                    )
+                )
+        return errors
 
     def validate_plugins(self, plugins):
         dst_plugins = {(g, a): v for g, a, v in map(POM.plugin_id, self.plugins())}
@@ -187,10 +205,10 @@ def replace_ns(element, new_ns):
         for child in element:
             POM.replace_ns(child, new_ns)
 
-    def add_plugins_from_pom(self, src_pom_xml: str):
+    def add_plugins_from_pom(self, src_pom_xml: str) -> list:
         src_pom = POM(src_pom_xml)
         src_plugins = src_pom.plugins()
-        self.add_plugins(src_plugins)
+        return self.add_plugins(src_plugins)
 
     def write(self, fpath: str):
         ElementTree.register_namespace("", self.ns)
@@ -224,6 +242,7 @@ def exists_python_code(dir):
 
 def run_sast(tool, command, env, config_dir, log_file=stdout, run_all=True):
     log.info(f"Preparing {tool}")
+    errors = None
 
     var_enabled = f"RUN_{tool.upper()}"
     var_args = f"{tool.upper()}_ARGS"
@@ -258,7 +277,6 @@ def run_sast(tool, command, env, config_dir, log_file=stdout, run_all=True):
         config_file=config_file,
         maven_args=maven_args,
     )
-
     if cmd.startswith("mvn "):
         if " -f " in cmd:
             log.error(f"Skipping {tool} because it uses a custom pom.xml: {cmd}")
@@ -269,7 +287,10 @@ def run_sast(tool, command, env, config_dir, log_file=stdout, run_all=True):
         dst_pom = POM("pom.xml")
         tmpfile = f".pom.xml.{uuid.uuid4()}"
         log.info("Creating runtime pom.xml in %r", (tmpfile,))
-        dst_pom.add_plugins_from_pom("/app/java-validators/pom.xml")
+
+        # Track plugins that have not been added.
+        #   They will be listed at the end of the run, which will fail.
+        errors = dst_pom.add_plugins_from_pom("/app/java-validators/pom.xml")
         dst_pom.write(tmpfile)
         cmd = f"mvn -f {tmpfile} {cmd[3:]}"
 
@@ -285,7 +306,9 @@ def run_sast(tool, command, env, config_dir, log_file=stdout, run_all=True):
     )
     if status.returncode != 0:
         log.error(f"{tool} failed with status {status.returncode}")
-    return status.returncode
+    if errors:
+        log.error(f"{tool} setup failed with errors {errors}")
+    return status.returncode or bool(errors)
 
 
 def _show_environ(config_dir, dump_config=False):

diff --git a/pom.xml b/pom.xml
@@ -39,7 +39,7 @@ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
             <plugin>
                 <groupId>org.owasp</groupId>
                 <artifactId>dependency-check-maven</artifactId>
-                <version>8.0.0</version>
+                <version>8.4.0</version>
                 <configuration>
                     <cveStartYear>2018</cveStartYear>
                 </configuration>

diff --git a/tests/data/dest-pom-4.xml b/tests/data/dest-pom-4.xml
@@ -0,0 +1,76 @@
+<?xml version="1.0"?>
+<!-- A pom.xml of a project depending on org.owasp:dependency-check-maven -->
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://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>it.babel.devops</groupId>
+  <artifactId>jenkins-super-sast</artifactId>
+  <version>1.0-SNAPSHOT</version>
+  <build>
+    <plugins>
+      <!-- Add the spotbugs plugin -->
+      <plugin>
+        <groupId>com.github.spotbugs</groupId>
+        <artifactId>spotbugs-maven-plugin</artifactId>
+        <version>4.7.1</version>
+        <configuration>
+          <effort>Max</effort>
+          <threshold>Low</threshold>
+          <failOnError>true</failOnError>
+          <excludeFilterFile>spotbugs-exclude.xml</excludeFilterFile>
+          <plugins>
+            <plugin>
+              <groupId>com.h3xstream.findsecbugs</groupId>
+              <artifactId>findsecbugs-plugin</artifactId>
+              <version>1.12.0</version>
+            </plugin>
+          </plugins>
+        </configuration>
+        <executions>
+          <execution>
+            <goals>
+              <goal>help</goal>
+            </goals>
+          </execution>
+        </executions>
+      </plugin>
+      <plugin>
+        <groupId>org.owasp</groupId>
+        <artifactId>dependency-check-maven</artifactId>
+        <version>7.2.1</version>
+        <configuration>
+          <cveStartYear>2018</cveStartYear>
+        </configuration>
+        <executions>
+          <execution>
+            <goals>
+              <goal>update-only</goal>
+            </goals>
+          </execution>
+        </executions>
+      </plugin>
+      <plugin>
+        <groupId>com.diffplug.spotless</groupId>
+        <artifactId>spotless-maven-plugin</artifactId>
+        <version>2.0.2</version>
+        <executions>
+          <execution>
+            <goals>
+              <goal>check</goal>
+              <goal>apply</goal>
+            </goals>
+          </execution>
+        </executions>
+        <configuration>
+          <java>
+            <!-- no need to specify files, inferred automatically, but you can if you
+                        want -->
+            <!-- apply a specific flavor of google-java-format -->
+            <googleJavaFormat>
+              <version>1.8</version>
+            </googleJavaFormat>
+          </java>
+        </configuration>
+      </plugin>
+    </plugins>
+  </build>
+</project>
diff --git a/tests/data/dest-pom-5.xml b/tests/data/dest-pom-5.xml
@@ -0,0 +1,76 @@
+<?xml version="1.0"?>
+<!-- A pom.xml of a project depending on org.owasp:dependency-check-maven -->
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://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>it.babel.devops</groupId>
+  <artifactId>jenkins-super-sast</artifactId>
+  <version>1.0-SNAPSHOT</version>
+  <build>
+    <plugins>
+      <!-- Add the spotbugs plugin -->
+      <plugin>
+        <groupId>com.github.spotbugs</groupId>
+        <artifactId>spotbugs-maven-plugin</artifactId>
+        <version>4.8.1</version>
+        <configuration>
+          <effort>Max</effort>
+          <threshold>Low</threshold>
+          <failOnError>true</failOnError>
+          <excludeFilterFile>spotbugs-exclude.xml</excludeFilterFile>
+          <plugins>
+            <plugin>
+              <groupId>com.h3xstream.findsecbugs</groupId>
+              <artifactId>findsecbugs-plugin</artifactId>
+              <version>1.12.0</version>
+            </plugin>
+          </plugins>
+        </configuration>
+        <executions>
+          <execution>
+            <goals>
+              <goal>help</goal>
+            </goals>
+          </execution>
+        </executions>
+      </plugin>
+      <plugin>
+        <groupId>org.owasp</groupId>
+        <artifactId>dependency-check-maven</artifactId>
+        <version>8.2.1</version>
+        <configuration>
+          <cveStartYear>2018</cveStartYear>
+        </configuration>
+        <executions>
+          <execution>
+            <goals>
+              <goal>update-only</goal>
+            </goals>
+          </execution>
+        </executions>
+      </plugin>
+      <plugin>
+        <groupId>com.diffplug.spotless</groupId>
+        <artifactId>spotless-maven-plugin</artifactId>
+        <version>2.0.2</version>
+        <executions>
+          <execution>
+            <goals>
+              <goal>check</goal>
+              <goal>apply</goal>
+            </goals>
+          </execution>
+        </executions>
+        <configuration>
+          <java>
+            <!-- no need to specify files, inferred automatically, but you can if you
+                        want -->
+            <!-- apply a specific flavor of google-java-format -->
+            <googleJavaFormat>
+              <version>1.8</version>
+            </googleJavaFormat>
+          </java>
+        </configuration>
+      </plugin>
+    </plugins>
+  </build>
+</project>
diff --git a/tests/test_entrypoint.py b/tests/test_entrypoint.py
@@ -2,7 +2,7 @@
 
 import pytest
 
-from entrypoint import POM, TOOLS_MAP, run_sast
+from entrypoint import TOOLS_MAP, run_sast
 
 DATA_DIR = Path(__file__).parent / "data"
 
@@ -56,64 +56,3 @@ def test_mvn_skip():
         config_dir=Path("/code/config"),
     )
     assert status == 0
-
-
-def test_pom_parse():
-    pom_xml = DATA_DIR / "src-pom.xml"
-    dst_xml = DATA_DIR / "dest-pom.xml"
-    src_pom = POM(pom_xml)
-    dst_pom = POM(dst_xml)
-    assert src_pom.ns
-    assert dst_pom.ns
-
-
-def test_pom_get_plugins():
-    testcases = [
-        ("src-pom.xml", 3),
-        ("dest-pom.xml", 4),
-    ]
-    for f, count in testcases:
-        pom_xml = DATA_DIR / f
-        pom = POM(pom_xml)
-        plugins = pom.plugins()
-        plugins_id = [
-            (pom.find("artifactId", p).text + ":" + pom.find("groupId", p).text)
-            for p in plugins
-        ]
-        assert len(plugins_id) == count
-
-
-def test_pom_replace_ns():
-    testcases = [("src-pom.xml", "dest-pom-1.xml"), ("src-pom.xml", "dest-pom-2.xml")]
-    for f, dst in testcases:
-        pom_xml = DATA_DIR / f
-        pom = POM(pom_xml)
-        dst_pom = POM(DATA_DIR / dst)
-        pom.replace_ns(pom.root, dst_pom.ns)
-        pom.ns = dst_pom.ns
-        src_plugins = pom.plugins()
-        assert len(src_plugins) == 3
-        dst_plugins_tag = dst_pom.plugins_tag()
-        for p in src_plugins:
-            dst_plugins_tag.append(p)
-        assert len(dst_pom.plugins()) == 7
-        dst_pom.write("tests/deleteme-out.xml")
-
-
-def test_pom_append_plugins():
-    testcases = [("dest-pom.xml", 4), ("dest-pom-1.xml", 4), ("dest-pom-2.xml", 4)]
-
-    pom_xml = DATA_DIR / "src-pom.xml"
-    for f, count in testcases:
-        dst_xml = DATA_DIR / f
-        dst_pom = POM(dst_xml)
-        dst_pom.add_plugins_from_pom(pom_xml.absolute().as_posix())
-
-        # Total plugins count.
-        dst_plugins = dst_pom.plugins()
-        assert len(dst_plugins) == 3 + count
-
-        # Use default namespace.
-        dst_pom.write("tests/deleteme-out.xml")
-        content = Path("tests/deleteme-out.xml").read_text()
-        assert "ns0:" not in content