✨ Support CodeTF Findings Without IDs (#443)

When a finding has no ID, we prefer to reflect this in CodeTF vs make up
a new ID that is not significant to the tool that produced the finding.

/towards ISS-1837

Author: gilday

core-codemods/src/main/java/io/codemodder/codemods/MavenSecureURLCodemod.java

@@ -104,8 +104,7 @@ private CodemodFileScanningResult processXml(final Path file, final List<Result>
                               })
                           .findFirst();
                   if (matchingResult.isPresent()) {
-                    String id =
-                        SarifFindingKeyUtil.buildFindingId(matchingResult.get(), file, line);
+                    String id = SarifFindingKeyUtil.buildFindingId(matchingResult.get());
                     Integer sarifLine =
                         matchingResult
                             .get()

framework/codemodder-base/build.gradle.kts

@@ -21,7 +21,7 @@ dependencies {
     api(libs.java.security.toolkit)
     api(libs.commons.lang3)
 
-    api("io.codemodder:codetf-java:4.1.3")
+    api("io.codemodder:codetf-java:4.2.1")
     api(libs.slf4j.api)
     api(libs.javaparser.core)
     api(libs.javaparser.symbolsolver.core)

framework/codemodder-base/src/main/java/io/codemodder/SarifFindingKeyUtil.java

@@ -1,11 +1,6 @@
 package io.codemodder;
 
-import com.contrastsecurity.sarif.Fingerprints;
 import com.contrastsecurity.sarif.Result;
-import java.nio.file.Path;
-import java.util.Collection;
-import java.util.HashMap;
-import java.util.Map;
 import org.apache.commons.lang3.StringUtils;
 
 /** Utility class for building keys for SARIF findings. */
@@ -14,27 +9,20 @@ public final class SarifFindingKeyUtil {
   private SarifFindingKeyUtil() {}
 
   /**
-   * Builds a finding ID for a SARIF finding based on the provided result, file path, and line
-   * number.
+   * Builds a finding ID for a SARIF finding based on the provided result.
+   *
+   * <p>Individual results are identified by the {@code guid} property, if present. Multiple results
+   * across scans are identified by the {@code correlationGuid} property. We prefer to identify the
+   * result by its {@code guid} if present, and fall back to the {@code correlationGuid} if not. We
+   * can be reasonably certain that the {@code correlationGuid} is unique within a single {@code
+   * run}.
    */
-  public static String buildFindingId(final Result result, final Path path, final int line) {
-    // prefer the guid, then the correlation guid
+  public static String buildFindingId(final Result result) {
     if (!StringUtils.isBlank(result.getGuid())) {
       return result.getGuid().trim();
     } else if (!StringUtils.isBlank(result.getCorrelationGuid())) {
       return result.getCorrelationGuid().trim();
     }
-
-    // use a fingerprint, as at least that is some guarantee of uniqueness
-    final Fingerprints fingerprints = result.getFingerprints();
-    final Map<String, String> fingerPrintProperties =
-        fingerprints != null ? fingerprints.getAdditionalProperties() : new HashMap<>();
-    if (!fingerPrintProperties.isEmpty()) {
-      Collection<String> values = fingerPrintProperties.values();
-      return values.iterator().next();
-    }
-
-    // ultimate fallback, some composite ID that will not represent anything
-    return String.format("%s-%s-%d", result.getRuleId(), path.getFileName(), line);
+    return null;
   }
 }

framework/codemodder-base/src/main/java/io/codemodder/SarifPluginJavaParserChanger.java

@@ -134,10 +134,7 @@ public CodemodFileScanningResult visit(
               if (changeSuccessful.areChangesApplied()) {
                 codemodChanges.add(
                     buildCodemodChange(
-                        context.path(),
-                        region.start().line(),
-                        changeSuccessful.getDependenciesRequired(),
-                        result));
+                        region.start().line(), changeSuccessful.getDependenciesRequired(), result));
               }
             }
           }
@@ -149,17 +146,13 @@ public CodemodFileScanningResult visit(
   }
 
   private CodemodChange buildCodemodChange(
-      final Path path,
-      final int line,
-      final List<DependencyGAV> dependencies,
-      final Result result) {
+      final int line, final List<DependencyGAV> dependencies, final Result result) {
     if (this instanceof FixOnlyCodeChanger fixOnlyCodeChanger) {
       return CodemodChange.from(
           line,
           dependencies,
           new FixedFinding(
-              SarifFindingKeyUtil.buildFindingId(result, path, line),
-              fixOnlyCodeChanger.detectorRule()));
+              SarifFindingKeyUtil.buildFindingId(result), fixOnlyCodeChanger.detectorRule()));
     }
 
     return CodemodChange.from(line, dependencies);

framework/codemodder-base/src/test/java/io/codemodder/SarifFindingKeyUtilTest.java

@@ -2,38 +2,39 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 
-import com.contrastsecurity.sarif.Fingerprints;
 import com.contrastsecurity.sarif.Result;
-import java.io.IOException;
-import java.nio.file.Files;
-import java.nio.file.Path;
+import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
-import org.junit.jupiter.api.io.TempDir;
 
+/** Unit tests for {@link SarifFindingKeyUtil}. */
 final class SarifFindingKeyUtilTest {
 
-  @Test
-  void it_creates_key(@TempDir final Path tmpDir) throws IOException {
-    Path tmpFile = tmpDir.resolve("my-code.txt");
-    Files.writeString(tmpFile, "my code");
+  private Result result;
 
-    Result result = new Result();
+  @BeforeEach
+  void before() {
+    result = new Result();
     result.setRuleId("my-rule");
+  }
 
-    assertThat(SarifFindingKeyUtil.buildFindingId(result, tmpFile, 1))
-        .isEqualTo("my-rule-my-code.txt-1");
-
-    Fingerprints fingerprints = new Fingerprints();
-    fingerprints.setAdditionalProperty("my-key", "my-fingerprint-value");
-    result.setFingerprints(fingerprints);
-    assertThat(SarifFindingKeyUtil.buildFindingId(result, tmpFile, 1))
-        .isEqualTo("my-fingerprint-value");
-
-    result.setCorrelationGuid(" my-correlation-guid ");
-    assertThat(SarifFindingKeyUtil.buildFindingId(result, tmpFile, 1))
-        .isEqualTo("my-correlation-guid");
+  @Test
+  void it_supports_findings_without_ids() {
+    final var id = SarifFindingKeyUtil.buildFindingId(result);
+    assertThat(id).isNull();
+  }
 
+  @Test
+  void it_prefers_guid() {
     result.setGuid("my-guid");
-    assertThat(SarifFindingKeyUtil.buildFindingId(result, tmpFile, 1)).isEqualTo("my-guid");
+    result.setCorrelationGuid("my-correlation-guid");
+    final var id = SarifFindingKeyUtil.buildFindingId(result);
+    assertThat(id).isEqualTo("my-guid");
+  }
+
+  @Test
+  void it_falls_back_to_correlation_guid() {
+    result.setCorrelationGuid("my-correlation-guid");
+    final var id = SarifFindingKeyUtil.buildFindingId(result);
+    assertThat(id).isEqualTo("my-correlation-guid");
   }
 }