SONARGO-40 Vulnerability imported through external report Golangci li…

…nt should be set to Security in Software Quality (#556)

checkstyle-import/src/main/java/org/sonarsource/slang/externalreport/CheckstyleFormatImporter.java

@@ -20,6 +20,7 @@
 import java.io.FileInputStream;
 import java.io.IOException;
 import java.io.InputStream;
+import java.util.List;
 import javax.annotation.Nullable;
 import javax.xml.namespace.QName;
 import javax.xml.stream.XMLEventReader;
@@ -35,6 +36,7 @@
 import org.sonar.api.batch.sensor.SensorContext;
 import org.sonar.api.batch.sensor.issue.NewExternalIssue;
 import org.sonar.api.batch.sensor.issue.NewIssueLocation;
+import org.sonar.api.issue.impact.SoftwareQuality;
 import org.sonar.api.rule.RuleKey;
 import org.sonar.api.rules.RuleType;
 import org.sonarsource.analyzer.commons.xml.SafeStaxParserFactory;
@@ -67,7 +69,7 @@ public class CheckstyleFormatImporter {
   private InputFile inputFile = null;
 
   /**
-   * @param context, the context where issues will be sent
+   * @param context,   the context where issues will be sent
    * @param linterKey, used to specify the rule repository
    */
   public CheckstyleFormatImporter(SensorContext context, String linterKey) {
@@ -77,6 +79,7 @@ public CheckstyleFormatImporter(SensorContext context, String linterKey) {
 
   /**
    * "importFile" parses the given report file and imports the content into SonarQube
+   *
    * @param reportPath, path of the xml file
    */
   public void importFile(File reportPath) {
@@ -146,6 +149,11 @@ private void saveIssue(String line, String severity, String source, String messa
       .severity(ruleSeverity)
       .remediationEffortMinutes(effort(ruleKey.rule()));
 
+    var impacts = impacts(severity, source);
+    for (Impact impact : impacts) {
+      newExternalIssue.addImpact(impact.softwareQuality(), impact.severity());
+    }
+
     NewIssueLocation primaryLocation = newExternalIssue.newLocation()
       .message(message)
       .on(inputFile);
@@ -199,9 +207,21 @@ protected Long effort(String ruleKey) {
     return DEFAULT_CONSTANT_DEBT_MINUTES;
   }
 
+  /**
+   * Return list of {@link Impact}s. By default empty list is returned for backward compatibility.
+   * @param severity "severity" attribute's value of the report. Ex: "info", "error".
+   * @param source "source" attribute's value of the report. Ex: "gosec", "detekt.MagicNumber".
+   * @return list of {@link Impact}s defined by the given parameters.
+   */
+  protected List<Impact> impacts(String severity, String source) {
+    return List.of();
+  }
+
   private static String getAttributeValue(StartElement element, QName attributeName) {
     Attribute attribute = element.getAttributeByName(attributeName);
     return attribute != null ? attribute.getValue() : "";
   }
 
+  public record Impact(SoftwareQuality softwareQuality, org.sonar.api.issue.impact.Severity severity) {
+  }
 }

checkstyle-import/src/test/java/org/sonarsource/slang/externalreport/CheckstyleFormatImporterTest.java

@@ -28,13 +28,16 @@
 import org.sonar.api.batch.fs.InputFile;
 import org.sonar.api.batch.fs.internal.TestInputFileBuilder;
 import org.sonar.api.batch.rule.Severity;
+import org.sonar.api.batch.sensor.SensorContext;
 import org.sonar.api.batch.sensor.internal.SensorContextTester;
 import org.sonar.api.batch.sensor.issue.ExternalIssue;
+import org.sonar.api.issue.impact.SoftwareQuality;
 import org.sonar.api.rules.RuleType;
 import org.sonarsource.slang.testing.ThreadLocalLogTester;
 
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.entry;
 
 class CheckstyleFormatImporterTest {
 
@@ -55,6 +58,7 @@ void import_detekt_issues() throws IOException {
     assertThat(first.ruleKey().rule()).isEqualTo("detekt.EmptyIfBlock");
     assertThat(first.type()).isEqualTo(RuleType.CODE_SMELL);
     assertThat(first.severity()).isEqualTo(Severity.MINOR);
+    assertThat(first.impacts()).isEmpty();
     assertThat(first.primaryLocation().message()).isEqualTo("This empty block of code can be removed.");
     assertThat(first.primaryLocation().textRange().start().line()).isEqualTo(3);
 
@@ -63,6 +67,7 @@ void import_detekt_issues() throws IOException {
     assertThat(second.ruleKey().rule()).isEqualTo("detekt.MagicNumber");
     assertThat(second.type()).isEqualTo(RuleType.CODE_SMELL);
     assertThat(second.severity()).isEqualTo(Severity.MAJOR);
+    assertThat(first.impacts()).isEmpty();
     assertThat(second.remediationEffort().longValue()).isEqualTo(5L);
     assertThat(second.primaryLocation().message()).isEqualTo("This expression contains a magic number. Consider defining it to a well named constant.");
     assertThat(second.primaryLocation().textRange().start().line()).isEqualTo(3);
@@ -72,6 +77,7 @@ void import_detekt_issues() throws IOException {
     assertThat(third.ruleKey().rule()).isEqualTo("detekt.EqualsWithHashCodeExist");
     assertThat(third.type()).isEqualTo(RuleType.BUG);
     assertThat(third.severity()).isEqualTo(Severity.MAJOR);
+    assertThat(first.impacts()).isEmpty();
     assertThat(third.primaryLocation().message()).isEqualTo("A class should always override hashCode when overriding equals and the other way around.");
     assertThat(third.primaryLocation().textRange().start().line()).isEqualTo(3);
 
@@ -80,14 +86,15 @@ void import_detekt_issues() throws IOException {
 
   @Test
   void import_golandci_lint_issues() throws IOException {
-    List<ExternalIssue> externalIssues = importIssues("golandci-lint-report.xml");
+    List<ExternalIssue> externalIssues = importIssuesTestImporter("golandci-lint-report.xml");
     assertThat(externalIssues).hasSize(1);
 
     ExternalIssue first = externalIssues.get(0);
     assertThat(first.primaryLocation().inputComponent().key()).isEqualTo("externalreport-project:TabCharacter.go");
     assertThat(first.ruleKey().rule()).isEqualTo("deadcode");
     assertThat(first.type()).isEqualTo(RuleType.BUG);
     assertThat(first.severity()).isEqualTo(Severity.MAJOR);
+    assertThat(first.impacts()).contains(entry(SoftwareQuality.SECURITY, org.sonar.api.issue.impact.Severity.MEDIUM));
     assertThat(first.primaryLocation().message()).isEqualTo("`three` is unused");
     assertThat(first.primaryLocation().textRange().start().line()).isEqualTo(4);
 
@@ -131,6 +138,7 @@ void issues_when_xml_file_has_errors() throws IOException {
     assertThat(first.ruleKey().rule()).isEqualTo("detekt.UnknownRuleKey");
     assertThat(first.type()).isEqualTo(RuleType.CODE_SMELL);
     assertThat(first.severity()).isEqualTo(Severity.MAJOR);
+    assertThat(first.impacts()).isEmpty();
     assertThat(first.primaryLocation().message()).isEqualTo("Error at file level with an unknown rule key.");
     assertThat(first.primaryLocation().textRange()).isNull();
 
@@ -148,6 +156,13 @@ private List<ExternalIssue> importIssues(String fileName) throws IOException {
     return new ArrayList<>(context.allExternalIssues());
   }
 
+  private List<ExternalIssue> importIssuesTestImporter(String fileName) throws IOException {
+    SensorContextTester context = createContext();
+    CheckstyleFormatImporter importer = new TestImporter(context, LINTER_KEY);
+    importer.importFile(PROJECT_DIR.resolve(fileName).toAbsolutePath().toFile());
+    return new ArrayList<>(context.allExternalIssues());
+  }
+
   static SensorContextTester createContext() throws IOException {
     SensorContextTester context = SensorContextTester.create(PROJECT_DIR);
     Files.list(PROJECT_DIR)
@@ -175,4 +190,15 @@ static String onlyOneLogElement(List<String> elements) {
     return elements.get(0);
   }
 
+  static class TestImporter extends CheckstyleFormatImporter {
+
+    public TestImporter(SensorContext context, String linterKey) {
+      super(context, linterKey);
+    }
+
+    @Override
+    protected List<Impact> impacts(String severity, String source) {
+      return List.of(new Impact(SoftwareQuality.SECURITY, org.sonar.api.issue.impact.Severity.MEDIUM));
+    }
+  }
 }