ryandens/appscan message text (#423)

- **:recycle: provide messageText to all RuleSarifFactory impls** - **Bind AppScan sarif to rule by rule name from message text**
pixee · Jul 18, 2024 · ff519dc · ff519dc
1 parent 901144f
commit ff519dc
Show file tree

Hide file tree

Showing 10 changed files with 47 additions and 40 deletions.
diff --git a/framework/codemodder-base/src/main/java/io/codemodder/DefaultSarifParser.java b/framework/codemodder-base/src/main/java/io/codemodder/DefaultSarifParser.java
@@ -1,6 +1,5 @@
 package io.codemodder;
 
-import com.contrastsecurity.sarif.ReportingDescriptor;
 import com.contrastsecurity.sarif.Result;
 import com.contrastsecurity.sarif.Run;
 import com.contrastsecurity.sarif.SarifSchema210;
@@ -9,6 +8,7 @@
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.*;
+import java.util.stream.Collectors;
 import java.util.stream.Stream;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -28,12 +28,13 @@ private Optional<SarifSchema210> readSarifFile(final Path sarifFile) {
   /** Send the arguments to all factories and returns the first that built something. */
   private Optional<Map.Entry<String, RuleSarif>> tryToBuild(
       final String toolName,
-      final String rule,
+      final RuleDescriptor rule,
       final SarifSchema210 sarif,
       final CodeDirectory codeDirectory,
       final List<RuleSarifFactory> factories) {
     for (final var factory : factories) {
-      final var maybeRuleSarif = factory.build(toolName, rule, sarif, codeDirectory);
+      final var maybeRuleSarif =
+          factory.build(toolName, rule.ruleId, rule.messageText, sarif, codeDirectory);
       if (maybeRuleSarif.isPresent()) {
         return Optional.of(Map.entry(toolName, maybeRuleSarif.get()));
       }
@@ -43,7 +44,9 @@ private Optional<Map.Entry<String, RuleSarif>> tryToBuild(
     return Optional.empty();
   }
 
-  private String extractRuleId(final Result result, final Run run) {
+  private record RuleDescriptor(String ruleId, String messageText) {}
+
+  private RuleDescriptor extractRuleId(final Result result, final Run run) {
     if (result.getRuleId() == null) {
       var toolIndex = result.getRule().getToolComponent().getIndex();
       var ruleIndex = result.getRule().getIndex();
@@ -52,15 +55,16 @@ private String extractRuleId(final Result result, final Run run) {
               .skip(toolIndex)
               .findFirst()
               .flatMap(tool -> tool.getRules().stream().skip(ruleIndex).findFirst())
-              .map(ReportingDescriptor::getId);
+              .map(descriptor -> new RuleDescriptor(descriptor.getId(), null));
+
       if (maybeRule.isPresent()) {
         return maybeRule.get();
       } else {
         LOG.info("Could not find rule id for result.");
         return null;
       }
     }
-    return result.getRuleId();
+    return new RuleDescriptor(result.getRuleId(), result.getMessage().getText());
   }
 
   private Stream<Map.Entry<String, RuleSarif>> fromSarif(
@@ -77,8 +81,11 @@ private Stream<Map.Entry<String, RuleSarif>> fromSarif(
             ? runResults.stream()
                 .map(result -> extractRuleId(result, run))
                 .filter(Objects::nonNull)
-                .distinct()
-            : Stream.<String>empty();
+                .filter(ruleDescriptor -> ruleDescriptor.ruleId != null)
+                .collect(Collectors.toMap(r -> r.ruleId, r -> r, (r1, r2) -> r1))
+                .values()
+                .stream()
+            : Stream.<RuleDescriptor>empty();
 
     return allResults.flatMap(
         rule -> tryToBuild(toolName, rule, sarif, codeDirectory, factories).stream());

diff --git a/framework/codemodder-base/src/main/java/io/codemodder/RuleSarifFactory.java b/framework/codemodder-base/src/main/java/io/codemodder/RuleSarifFactory.java
@@ -8,5 +8,9 @@ public interface RuleSarifFactory {
 
   /** Builds {@link RuleSarif}s if it supports {@code toolName}. */
   Optional<RuleSarif> build(
-      String toolName, String rule, SarifSchema210 sarif, CodeDirectory codeDirectory);
+      String toolName,
+      String rule,
+      String messageText,
+      SarifSchema210 sarif,
+      CodeDirectory codeDirectory);
 }
diff --git a/...der-plugin-appscan/src/main/java/io/codemodder/providers/sarif/appscan/AppScanModule.java b/...der-plugin-appscan/src/main/java/io/codemodder/providers/sarif/appscan/AppScanModule.java
@@ -41,22 +41,10 @@ protected void configure() {
               .findFirst();
 
       annotation.ifPresent(
-          providedAppScanScan -> {
-            RuleSarif sarif = null;
-            for (final var ruleId : providedAppScanScan.ruleIds()) {
-              final var value = map.get(ruleId);
-              if (value != null) {
-                sarif = value;
-                break;
-              }
-            }
-
-            if (sarif == null) {
-              sarif = RuleSarif.EMPTY;
-            }
-
-            bind(RuleSarif.class).annotatedWith(providedAppScanScan).toInstance(sarif);
-          });
+          providedAppScanScan ->
+              bind(RuleSarif.class)
+                  .annotatedWith(providedAppScanScan)
+                  .toInstance(map.getOrDefault(providedAppScanScan.ruleName(), RuleSarif.EMPTY)));
     }
   }
 }
diff --git a/...-plugin-appscan/src/main/java/io/codemodder/providers/sarif/appscan/AppScanRuleSarif.java b/...-plugin-appscan/src/main/java/io/codemodder/providers/sarif/appscan/AppScanRuleSarif.java
@@ -12,7 +12,7 @@
 final class AppScanRuleSarif implements RuleSarif {
 
   private final SarifSchema210 sarif;
-  private final String ruleId;
+  private final String messageText;
   private final Map<Path, List<Result>> resultsCache;
   private final List<String> locations;
 
@@ -24,9 +24,9 @@ final class AppScanRuleSarif implements RuleSarif {
    * locations, which are strange combinations of class name and file path, into predictable paths.
    */
   AppScanRuleSarif(
-      final String ruleId, final SarifSchema210 sarif, final CodeDirectory codeDirectory) {
+      final String messageText, final SarifSchema210 sarif, final CodeDirectory codeDirectory) {
     this.sarif = Objects.requireNonNull(sarif);
-    this.ruleId = Objects.requireNonNull(ruleId);
+    this.messageText = Objects.requireNonNull(messageText);
     this.resultsCache = new HashMap<>();
     this.locations =
         sarif.getRuns().get(0).getArtifacts().stream()
@@ -76,7 +76,7 @@ public List<Result> getResultsByLocationPath(final Path path) {
         p ->
             sarif.getRuns().stream()
                 .flatMap(run -> run.getResults().stream())
-                .filter(result -> result.getRuleId().equals(ruleId))
+                .filter(result -> result.getMessage().getText().equals(messageText))
                 .filter(
                     result ->
                         artifactLocationIndices.get(path) != null
@@ -113,7 +113,7 @@ public SarifSchema210 rawDocument() {
    */
   @Override
   public String getRule() {
-    return ruleId;
+    return messageText;
   }
 
   static final String toolName = "HCL AppScan Static Analyzer";

diff --git a/...-appscan/src/main/java/io/codemodder/providers/sarif/appscan/AppScanRuleSarifFactory.java b/...-appscan/src/main/java/io/codemodder/providers/sarif/appscan/AppScanRuleSarifFactory.java
@@ -13,10 +13,11 @@ public final class AppScanRuleSarifFactory implements RuleSarifFactory {
   public Optional<RuleSarif> build(
       final String toolName,
       final String rule,
+      final String messageText,
       final SarifSchema210 sarif,
       final CodeDirectory codeDirectory) {
     if (AppScanRuleSarif.toolName.equals(toolName)) {
-      return Optional.of(new AppScanRuleSarif(rule, sarif, codeDirectory));
+      return Optional.of(new AppScanRuleSarif(messageText, sarif, codeDirectory));
     }
     return Optional.empty();
   }

diff --git a/...ugin-appscan/src/main/java/io/codemodder/providers/sarif/appscan/ProvidedAppScanScan.java b/...ugin-appscan/src/main/java/io/codemodder/providers/sarif/appscan/ProvidedAppScanScan.java
@@ -13,9 +13,6 @@
 @Target(ElementType.PARAMETER)
 public @interface ProvidedAppScanScan {
 
-  /**
-   * The AppScan rule "id" field from the sarif. If multiple are provided, we look for the first ID
-   * in the SARIF before looking up alternative rule IDs
-   */
-  String[] ruleIds();
+  /** The AppScan rule name, which shows up as the "message text" in the SARIF results. */
+  String ruleName();
 }
diff --git a/...plugin-appscan/src/test/java/io/codemodder/providers/sarif/appscan/AppScanModuleTest.java b/...plugin-appscan/src/test/java/io/codemodder/providers/sarif/appscan/AppScanModuleTest.java
@@ -28,7 +28,7 @@ static class AppScanSarifTestCodemod implements CodeChanger {
     private final RuleSarif ruleSarif;
 
     @Inject
-    AppScanSarifTestCodemod(@ProvidedAppScanScan(ruleIds = {"SA2813462719"}) RuleSarif ruleSarif) {
+    AppScanSarifTestCodemod(@ProvidedAppScanScan(ruleName = "SQL Injection") RuleSarif ruleSarif) {
       this.ruleSarif = ruleSarif;
     }
 
@@ -86,7 +86,11 @@ void it_works_with_appscan_sarif(@TempDir final Path repoDir) throws IOException
     AppScanRuleSarifFactory ruleSarifFactory = new AppScanRuleSarifFactory();
     Optional<RuleSarif> ruleSarif =
         ruleSarifFactory.build(
-            "HCL AppScan Static Analyzer", "SA2813462719", rawSarif, CodeDirectory.from(repoDir));
+            "HCL AppScan Static Analyzer",
+            "SA2813462719",
+            "SQL Injection",
+            rawSarif,
+            CodeDirectory.from(repoDir));
     assertThat(ruleSarif.isPresent(), is(true));
     AppScanModule module =
         new AppScanModule(List.of(AppScanSarifTestCodemod.class), List.of(ruleSarif.get()));

diff --git a/...scan/src/test/java/io/codemodder/providers/sarif/appscan/AppScanRuleSarifFactoryTest.java b/...scan/src/test/java/io/codemodder/providers/sarif/appscan/AppScanRuleSarifFactoryTest.java
@@ -43,12 +43,16 @@ void it_parses_sarif_and_maps_java_locations(@TempDir final Path tmpDir) throws
                 new File("src/test/resources/webgoat_2023_8_binary.sarif"), SarifSchema210.class);
     Optional<RuleSarif> sarifRef =
         appScanRuleSarifFactory.build(
-            "HCL AppScan Static Analyzer", "SA2813462719", rawSarif, CodeDirectory.from(tmpDir));
+            "HCL AppScan Static Analyzer",
+            "SA2813462719",
+            "SQL Injection",
+            rawSarif,
+            CodeDirectory.from(tmpDir));
     assertThat(sarifRef.isPresent()).isTrue();
     RuleSarif ruleSarif = sarifRef.get();
 
     // verify the rule sarif has the right stuff
-    assertThat(ruleSarif.getRule()).isEqualTo("SA2813462719");
+    assertThat(ruleSarif.getRule()).isEqualTo("SQL Injection");
     assertThat(ruleSarif.getDriver()).isEqualTo("HCL AppScan Static Analyzer");
     assertThat(ruleSarif.rawDocument()).isEqualTo(rawSarif);
 

diff --git a/...gin-codeql/src/main/java/io/codemodder/providers/sarif/codeql/CodeQLRuleSarifFactory.java b/...gin-codeql/src/main/java/io/codemodder/providers/sarif/codeql/CodeQLRuleSarifFactory.java
@@ -13,6 +13,7 @@ public final class CodeQLRuleSarifFactory implements RuleSarifFactory {
   public Optional<RuleSarif> build(
       final String toolName,
       final String rule,
+      final String messageText,
       final SarifSchema210 sarif,
       final CodeDirectory codeDirectory) {
     if (CodeQLRuleSarif.toolName.equals(toolName)) {

diff --git a/...-semgrep/src/main/java/io/codemodder/providers/sarif/semgrep/SemgrepRuleSarifFactory.java b/...-semgrep/src/main/java/io/codemodder/providers/sarif/semgrep/SemgrepRuleSarifFactory.java
@@ -15,6 +15,7 @@ public class SemgrepRuleSarifFactory implements RuleSarifFactory {
   public Optional<RuleSarif> build(
       final String toolName,
       final String rule,
+      final String messageText,
       final SarifSchema210 sarif,
       final CodeDirectory codeDirectory) {