From 3bd76606269c67d49674f83b61db74d4cff762ac Mon Sep 17 00:00:00 2001 From: Marcin Stachniuk Date: Tue, 10 Dec 2024 15:48:38 +0100 Subject: [PATCH 1/5] SONARGO-40 Vulnerability imported through external report Golangci lint should be set to Security in Software Quality --- settings.gradle | 3 ++- .../go/externalreport/GolangCILintReportSensor.java | 10 ++++++++++ .../externalreport/GolangCILintReportSensorTest.java | 4 ++++ 3 files changed, 16 insertions(+), 1 deletion(-) diff --git a/settings.gradle b/settings.gradle index baae4825..2a9083ec 100644 --- a/settings.gradle +++ b/settings.gradle @@ -6,7 +6,8 @@ pluginManagement { } dependencyResolutionManagement { - def slangDependenciesVersion = '1.17.0.6351' + //TODO change to released version before merge + def slangDependenciesVersion = '1.18.0.6372' def analyzerCommonsVersion = '2.16.0.3141' def pluginApiVersion = '10.10.0.2391' def sonarqubeVersion = '10.0.0.68432' diff --git a/sonar-go-plugin/src/main/java/org/sonar/go/externalreport/GolangCILintReportSensor.java b/sonar-go-plugin/src/main/java/org/sonar/go/externalreport/GolangCILintReportSensor.java index 8fff2b08..040a3c8b 100644 --- a/sonar-go-plugin/src/main/java/org/sonar/go/externalreport/GolangCILintReportSensor.java +++ b/sonar-go-plugin/src/main/java/org/sonar/go/externalreport/GolangCILintReportSensor.java @@ -17,11 +17,13 @@ package org.sonar.go.externalreport; import java.io.File; +import java.util.List; import java.util.Locale; import java.util.function.Consumer; import javax.annotation.Nullable; import org.sonar.api.batch.rule.Severity; import org.sonar.api.batch.sensor.SensorContext; +import org.sonar.api.issue.impact.SoftwareQuality; import org.sonar.api.notifications.AnalysisWarnings; import org.sonar.api.rule.RuleKey; import org.sonar.api.rules.RuleType; @@ -79,5 +81,13 @@ protected RuleKey createRuleKey(String source, RuleType ruleType, Severity ruleS ruleSeverity.toString().toLowerCase(Locale.ROOT)); return RuleKey.of(linterKey, ruleKey); } + + @Override + protected List impacts(String severity, String source) { + if ("gosec".equals(source)) { + return List.of(new Impact(SoftwareQuality.SECURITY, org.sonar.api.issue.impact.Severity.MEDIUM)); + } + return List.of(new Impact(SoftwareQuality.MAINTAINABILITY, org.sonar.api.issue.impact.Severity.MEDIUM)); + } } } diff --git a/sonar-go-plugin/src/test/java/org/sonar/go/externalreport/GolangCILintReportSensorTest.java b/sonar-go-plugin/src/test/java/org/sonar/go/externalreport/GolangCILintReportSensorTest.java index 164f80e3..185abedd 100644 --- a/sonar-go-plugin/src/test/java/org/sonar/go/externalreport/GolangCILintReportSensorTest.java +++ b/sonar-go-plugin/src/test/java/org/sonar/go/externalreport/GolangCILintReportSensorTest.java @@ -27,10 +27,12 @@ import org.sonar.api.batch.sensor.internal.DefaultSensorDescriptor; 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 org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.entry; import static org.sonar.go.externalreport.ExternalLinterSensorHelper.REPORT_BASE_PATH; class GolangCILintReportSensorTest { @@ -69,6 +71,7 @@ void issues_with_sonarqube() throws IOException { assertThat(first.severity()).isEqualTo(Severity.MAJOR); assertThat(first.ruleKey().repository()).isEqualTo("external_golangci-lint"); assertThat(first.ruleKey().rule()).isEqualTo("deadcode.bug.major"); + assertThat(first.impacts()).contains(entry(SoftwareQuality.MAINTAINABILITY, org.sonar.api.issue.impact.Severity.MEDIUM)); assertThat(first.primaryLocation().message()).isEqualTo("`three` is unused"); assertThat(first.primaryLocation().textRange().start().line()).isEqualTo(3); @@ -77,6 +80,7 @@ void issues_with_sonarqube() throws IOException { assertThat(second.severity()).isEqualTo(Severity.MAJOR); assertThat(second.ruleKey().repository()).isEqualTo("external_golangci-lint"); assertThat(second.ruleKey().rule()).isEqualTo("gosec"); + assertThat(second.impacts()).contains(entry(SoftwareQuality.SECURITY, org.sonar.api.issue.impact.Severity.MEDIUM)); assertThat(second.primaryLocation().message()).isEqualTo("G402: TLS InsecureSkipVerify set true."); assertThat(second.primaryLocation().inputComponent().key()).isEqualTo("module:main.go"); assertThat(second.primaryLocation().textRange().start().line()).isEqualTo(4); From d5c35fe91d26e993c64f9a77a23a749481f3bf0d Mon Sep 17 00:00:00 2001 From: Marcin Stachniuk Date: Tue, 10 Dec 2024 16:31:21 +0100 Subject: [PATCH 2/5] Fix code smell --- .../sonar/go/externalreport/GolangCILintReportSensor.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/sonar-go-plugin/src/main/java/org/sonar/go/externalreport/GolangCILintReportSensor.java b/sonar-go-plugin/src/main/java/org/sonar/go/externalreport/GolangCILintReportSensor.java index 040a3c8b..fd3442ec 100644 --- a/sonar-go-plugin/src/main/java/org/sonar/go/externalreport/GolangCILintReportSensor.java +++ b/sonar-go-plugin/src/main/java/org/sonar/go/externalreport/GolangCILintReportSensor.java @@ -49,6 +49,8 @@ public Consumer reportConsumer(SensorContext context) { private static class GolangCILintCheckstyleFormatImporter extends CheckstyleFormatImporter { + private static final String GOSEC = "gosec"; + public GolangCILintCheckstyleFormatImporter(SensorContext context, String linterKey) { super(context, linterKey); } @@ -65,7 +67,7 @@ public GolangCILintCheckstyleFormatImporter(SensorContext context, String linter */ @Override protected RuleType ruleType(@Nullable String severity, String source) { - if ("gosec".equals(source)) { + if (GOSEC.equals(source)) { return RuleType.VULNERABILITY; } return super.ruleType(severity, source); @@ -73,7 +75,7 @@ protected RuleType ruleType(@Nullable String severity, String source) { @Override protected RuleKey createRuleKey(String source, RuleType ruleType, Severity ruleSeverity) { - if ("gosec".equals(source)) { + if (GOSEC.equals(source)) { // gosec issues are exclusively "major vulnerability", keeping "gosec" as rule key. return RuleKey.of(linterKey, source); } @@ -84,7 +86,7 @@ protected RuleKey createRuleKey(String source, RuleType ruleType, Severity ruleS @Override protected List impacts(String severity, String source) { - if ("gosec".equals(source)) { + if (GOSEC.equals(source)) { return List.of(new Impact(SoftwareQuality.SECURITY, org.sonar.api.issue.impact.Severity.MEDIUM)); } return List.of(new Impact(SoftwareQuality.MAINTAINABILITY, org.sonar.api.issue.impact.Severity.MEDIUM)); From a99b300e8f83adbd6944b9348d2322683b11462e Mon Sep 17 00:00:00 2001 From: Marcin Stachniuk Date: Wed, 11 Dec 2024 11:50:34 +0100 Subject: [PATCH 3/5] Avoid setting Impact for SonarQube Cloud --- .../GolangCILintReportSensor.java | 15 ++++-- .../GolangCILintReportSensorTest.java | 47 +++++++++++++++++-- 2 files changed, 56 insertions(+), 6 deletions(-) diff --git a/sonar-go-plugin/src/main/java/org/sonar/go/externalreport/GolangCILintReportSensor.java b/sonar-go-plugin/src/main/java/org/sonar/go/externalreport/GolangCILintReportSensor.java index fd3442ec..2b289568 100644 --- a/sonar-go-plugin/src/main/java/org/sonar/go/externalreport/GolangCILintReportSensor.java +++ b/sonar-go-plugin/src/main/java/org/sonar/go/externalreport/GolangCILintReportSensor.java @@ -21,6 +21,8 @@ import java.util.Locale; import java.util.function.Consumer; import javax.annotation.Nullable; +import org.sonar.api.SonarEdition; +import org.sonar.api.SonarProduct; import org.sonar.api.batch.rule.Severity; import org.sonar.api.batch.sensor.SensorContext; import org.sonar.api.issue.impact.SoftwareQuality; @@ -50,9 +52,11 @@ public Consumer reportConsumer(SensorContext context) { private static class GolangCILintCheckstyleFormatImporter extends CheckstyleFormatImporter { private static final String GOSEC = "gosec"; + private final SensorContext context; public GolangCILintCheckstyleFormatImporter(SensorContext context, String linterKey) { super(context, linterKey); + this.context = context; } /** @@ -86,10 +90,15 @@ protected RuleKey createRuleKey(String source, RuleType ruleType, Severity ruleS @Override protected List impacts(String severity, String source) { - if (GOSEC.equals(source)) { - return List.of(new Impact(SoftwareQuality.SECURITY, org.sonar.api.issue.impact.Severity.MEDIUM)); + var isSonarCloud = context.runtime().getProduct() == SonarProduct.SONARQUBE && context.runtime().getEdition() == SonarEdition.SONARCLOUD; + if (!isSonarCloud) { + // SonarQube Cloud does not yet support the `impact` field for external issues + if (GOSEC.equals(source)) { + return List.of(new Impact(SoftwareQuality.SECURITY, org.sonar.api.issue.impact.Severity.MEDIUM)); + } + return List.of(new Impact(SoftwareQuality.MAINTAINABILITY, org.sonar.api.issue.impact.Severity.MEDIUM)); } - return List.of(new Impact(SoftwareQuality.MAINTAINABILITY, org.sonar.api.issue.impact.Severity.MEDIUM)); + return List.of(); } } } diff --git a/sonar-go-plugin/src/test/java/org/sonar/go/externalreport/GolangCILintReportSensorTest.java b/sonar-go-plugin/src/test/java/org/sonar/go/externalreport/GolangCILintReportSensorTest.java index 185abedd..6dcc4240 100644 --- a/sonar-go-plugin/src/test/java/org/sonar/go/externalreport/GolangCILintReportSensorTest.java +++ b/sonar-go-plugin/src/test/java/org/sonar/go/externalreport/GolangCILintReportSensorTest.java @@ -22,17 +22,23 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; +import org.mockito.Mockito; import org.slf4j.event.Level; +import org.sonar.api.SonarEdition; +import org.sonar.api.SonarProduct; +import org.sonar.api.SonarRuntime; import org.sonar.api.batch.rule.Severity; import org.sonar.api.batch.sensor.internal.DefaultSensorDescriptor; 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.sonar.api.utils.Version; import org.sonarsource.slang.testing.ThreadLocalLogTester; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.entry; +import static org.mockito.Mockito.when; import static org.sonar.go.externalreport.ExternalLinterSensorHelper.REPORT_BASE_PATH; class GolangCILintReportSensorTest { @@ -48,7 +54,7 @@ void setup() { public ThreadLocalLogTester logTester = new ThreadLocalLogTester(); @Test - void test_descriptor() { + void shouldValidateDescriptor() { DefaultSensorDescriptor sensorDescriptor = new DefaultSensorDescriptor(); golangCILintReportSensor().describe(sensorDescriptor); assertThat(sensorDescriptor.name()).isEqualTo("Import of GolangCI-Lint issues"); @@ -60,7 +66,7 @@ private GolangCILintReportSensor golangCILintReportSensor() { } @Test - void issues_with_sonarqube() throws IOException { + void shouldValidateWithSonarqube() throws IOException { SensorContextTester context = ExternalLinterSensorHelper.createContext(); context.settings().setProperty("sonar.go.golangci-lint.reportPaths", REPORT_BASE_PATH.resolve("golandci-lint-report.xml").toString()); List externalIssues = ExternalLinterSensorHelper.executeSensor(golangCILintReportSensor(), context); @@ -88,9 +94,44 @@ void issues_with_sonarqube() throws IOException { assertThat(logTester.logs(Level.ERROR)).isEmpty(); } + @Test + void shouldValidateWithSonarcloud() throws IOException { + SensorContextTester context = ExternalLinterSensorHelper.createContext(); + var sonarRuntime = Mockito.mock(SonarRuntime.class); + when(sonarRuntime.getProduct()).thenReturn(SonarProduct.SONARQUBE); + when(sonarRuntime.getEdition()).thenReturn(SonarEdition.SONARCLOUD); + when(sonarRuntime.getApiVersion()).thenReturn(Version.create(7,2)); + context.setRuntime(sonarRuntime); + context.settings().setProperty("sonar.go.golangci-lint.reportPaths", REPORT_BASE_PATH.resolve("golandci-lint-report.xml").toString()); + List externalIssues = ExternalLinterSensorHelper.executeSensor(golangCILintReportSensor(), context); + assertThat(externalIssues).hasSize(2); + + org.sonar.api.batch.sensor.issue.ExternalIssue first = externalIssues.get(0); + assertThat(first.type()).isEqualTo(RuleType.BUG); + assertThat(first.severity()).isEqualTo(Severity.MAJOR); + assertThat(first.ruleKey().repository()).isEqualTo("external_golangci-lint"); + assertThat(first.ruleKey().rule()).isEqualTo("deadcode.bug.major"); + // For SonarQube Cloud the impact should be empty as it is not supported + assertThat(first.impacts()).isEmpty(); + assertThat(first.primaryLocation().message()).isEqualTo("`three` is unused"); + assertThat(first.primaryLocation().textRange().start().line()).isEqualTo(3); + + ExternalIssue second = externalIssues.get(1); + assertThat(second.type()).isEqualTo(RuleType.VULNERABILITY); + assertThat(second.severity()).isEqualTo(Severity.MAJOR); + assertThat(second.ruleKey().repository()).isEqualTo("external_golangci-lint"); + assertThat(second.ruleKey().rule()).isEqualTo("gosec"); + assertThat(first.impacts()).isEmpty(); + assertThat(second.primaryLocation().message()).isEqualTo("G402: TLS InsecureSkipVerify set true."); + assertThat(second.primaryLocation().inputComponent().key()).isEqualTo("module:main.go"); + assertThat(second.primaryLocation().textRange().start().line()).isEqualTo(4); + + assertThat(logTester.logs(Level.ERROR)).isEmpty(); + } + @Test - void import_check_style_report_same_source_different_key() throws IOException { + void shouldImportSameSourceDifferentKey() throws IOException { // Check that rules have different key based on the severity SensorContextTester context = ExternalLinterSensorHelper.createContext(); context.settings().setProperty("sonar.go.golangci-lint.reportPaths", REPORT_BASE_PATH.resolve("checkstyle-different-severity.xml").toString()); From ba01903c2cd74dcfbb0e7c203aa356f2b9c4ab3e Mon Sep 17 00:00:00 2001 From: Marcin Stachniuk Date: Wed, 11 Dec 2024 14:04:29 +0100 Subject: [PATCH 4/5] Code review remarks Co-authored-by: Peter Trifanov --- .../go/externalreport/GolangCILintReportSensor.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/sonar-go-plugin/src/main/java/org/sonar/go/externalreport/GolangCILintReportSensor.java b/sonar-go-plugin/src/main/java/org/sonar/go/externalreport/GolangCILintReportSensor.java index 2b289568..49926474 100644 --- a/sonar-go-plugin/src/main/java/org/sonar/go/externalreport/GolangCILintReportSensor.java +++ b/sonar-go-plugin/src/main/java/org/sonar/go/externalreport/GolangCILintReportSensor.java @@ -91,14 +91,14 @@ protected RuleKey createRuleKey(String source, RuleType ruleType, Severity ruleS @Override protected List impacts(String severity, String source) { var isSonarCloud = context.runtime().getProduct() == SonarProduct.SONARQUBE && context.runtime().getEdition() == SonarEdition.SONARCLOUD; - if (!isSonarCloud) { + if (isSonarCloud) { // SonarQube Cloud does not yet support the `impact` field for external issues - if (GOSEC.equals(source)) { - return List.of(new Impact(SoftwareQuality.SECURITY, org.sonar.api.issue.impact.Severity.MEDIUM)); - } - return List.of(new Impact(SoftwareQuality.MAINTAINABILITY, org.sonar.api.issue.impact.Severity.MEDIUM)); + return List.of(); } - return List.of(); + if (GOSEC.equals(source)) { + return List.of(new Impact(SoftwareQuality.SECURITY, org.sonar.api.issue.impact.Severity.MEDIUM)); + } + return List.of(new Impact(SoftwareQuality.MAINTAINABILITY, org.sonar.api.issue.impact.Severity.MEDIUM)); } } } From 3ca91c3c08ed873599d02ed73683499c8981976b Mon Sep 17 00:00:00 2001 From: Marcin Stachniuk Date: Wed, 11 Dec 2024 14:05:46 +0100 Subject: [PATCH 5/5] Fix slang version --- settings.gradle | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/settings.gradle b/settings.gradle index 2a9083ec..d0d509f9 100644 --- a/settings.gradle +++ b/settings.gradle @@ -6,8 +6,7 @@ pluginManagement { } dependencyResolutionManagement { - //TODO change to released version before merge - def slangDependenciesVersion = '1.18.0.6372' + def slangDependenciesVersion = '1.18.0.6375' def analyzerCommonsVersion = '2.16.0.3141' def pluginApiVersion = '10.10.0.2391' def sonarqubeVersion = '10.0.0.68432'