From 266881788f7ce7c582cb1021c36133509abb3c87 Mon Sep 17 00:00:00 2001 From: =?utf8?q?S=C3=A9bastien=20Lesaint?= Date: Wed, 18 Dec 2019 18:04:50 +0100 Subject: [PATCH] SONAR-12726 add startup warning when hotspot rule desc can't be parsed --- .../server/rule}/HotspotRuleDescription.java | 14 ++- .../sonar/server/rule/index/RuleIndexer.java | 21 ++++- .../rule}/HotspotRuleDescriptionTest.java | 2 +- .../server/rule/index/RuleIndexerTest.java | 90 ++++++++++++++++++- .../sonar/server/hotspot/ws/ShowAction.java | 1 + 5 files changed, 124 insertions(+), 4 deletions(-) rename server/{sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws => sonar-server-common/src/main/java/org/sonar/server/rule}/HotspotRuleDescription.java (90%) rename server/{sonar-webserver-webapi/src/test/java/org/sonar/server/hotspot/ws => sonar-server-common/src/test/java/org/sonar/server/rule}/HotspotRuleDescriptionTest.java (99%) diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/HotspotRuleDescription.java b/server/sonar-server-common/src/main/java/org/sonar/server/rule/HotspotRuleDescription.java similarity index 90% rename from server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/HotspotRuleDescription.java rename to server/sonar-server-common/src/main/java/org/sonar/server/rule/HotspotRuleDescription.java index 205bc723dbc..f66e1938293 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/HotspotRuleDescription.java +++ b/server/sonar-server-common/src/main/java/org/sonar/server/rule/HotspotRuleDescription.java @@ -17,12 +17,13 @@ * along with this program; if not, write to the Free Software Foundation, * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ -package org.sonar.server.hotspot.ws; +package org.sonar.server.rule; import java.util.Optional; import javax.annotation.CheckForNull; import javax.annotation.Nullable; import org.sonar.db.rule.RuleDefinitionDto; +import org.sonar.db.rule.RuleForIndexingDto; import static java.lang.Character.isWhitespace; import static java.util.Optional.ofNullable; @@ -45,6 +46,14 @@ public class HotspotRuleDescription { public static HotspotRuleDescription from(RuleDefinitionDto dto) { String description = dto.getDescription(); + return from(description); + } + + public static HotspotRuleDescription from(RuleForIndexingDto dto) { + return from(dto.getDescription()); + } + + private static HotspotRuleDescription from(@Nullable String description) { if (description == null) { return NO_DESCRIPTION; } @@ -111,6 +120,9 @@ public class HotspotRuleDescription { return ofNullable(fixIt); } + public boolean isComplete() { + return risk != null && vulnerable != null && fixIt != null; + } @Override public String toString() { return "HotspotRuleDescription{" + diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/rule/index/RuleIndexer.java b/server/sonar-server-common/src/main/java/org/sonar/server/rule/index/RuleIndexer.java index d5ff6345cb6..4a4dac17eca 100644 --- a/server/sonar-server-common/src/main/java/org/sonar/server/rule/index/RuleIndexer.java +++ b/server/sonar-server-common/src/main/java/org/sonar/server/rule/index/RuleIndexer.java @@ -26,6 +26,8 @@ import java.util.List; import java.util.Optional; import java.util.Set; import java.util.stream.Stream; +import org.sonar.api.rules.RuleType; +import org.sonar.api.utils.log.Logger; import org.sonar.api.utils.log.Loggers; import org.sonar.core.util.stream.MoreCollectors; import org.sonar.db.DbClient; @@ -42,6 +44,7 @@ import org.sonar.server.es.IndexingListener; import org.sonar.server.es.IndexingResult; import org.sonar.server.es.OneToOneResilientIndexingListener; import org.sonar.server.es.ResilientIndexer; +import org.sonar.server.rule.HotspotRuleDescription; import org.sonar.server.security.SecurityStandards; import static com.google.common.base.Preconditions.checkArgument; @@ -55,6 +58,8 @@ import static org.sonar.server.rule.index.RuleIndexDefinition.TYPE_RULE_EXTENSIO import static org.sonar.server.security.SecurityStandards.SQ_CATEGORY_KEYS_ORDERING; public class RuleIndexer implements ResilientIndexer { + private static final Logger LOG = Loggers.get(RuleIndexer.class); + private final EsClient esClient; private final DbClient dbClient; @@ -195,7 +200,7 @@ public class RuleIndexer implements ResilientIndexer { private RuleDoc ruleDocOf(RuleForIndexingDto dto) { SecurityStandards securityStandards = SecurityStandards.fromSecurityStandards(dto.getSecurityStandards()); if (!securityStandards.getIgnoredSQCategories().isEmpty()) { - Loggers.get(RuleIndexer.class).warn( + LOG.warn( "Rule {} with CWEs '{}' maps to multiple SQ Security Categories: {}", dto.getRuleKey(), String.join(", ", securityStandards.getCwe()), @@ -204,9 +209,23 @@ public class RuleIndexer implements ResilientIndexer { .sorted(SQ_CATEGORY_KEYS_ORDERING) .collect(joining(", "))); } + if (dto.getTypeAsRuleType() == RuleType.SECURITY_HOTSPOT) { + HotspotRuleDescription ruleDescription = HotspotRuleDescription.from(dto); + if (!ruleDescription.isComplete()) { + LOG.warn( + "Description of Security Hotspot Rule {} can't be fully parsed: What is the risk?={}, Are you vulnerable?={}, How to fix it={}", + dto.getRuleKey(), + toOkMissing(ruleDescription.getRisk()), toOkMissing(ruleDescription.getVulnerable()), + toOkMissing(ruleDescription.getFixIt())); + } + } return RuleDoc.of(dto, securityStandards); } + private static String toOkMissing(Optional field) { + return field.map(t -> "ok").orElse("missing"); + } + private BulkIndexer createBulkIndexer(Size bulkSize, IndexingListener listener) { return new BulkIndexer(esClient, TYPE_RULE, bulkSize, listener); } diff --git a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/hotspot/ws/HotspotRuleDescriptionTest.java b/server/sonar-server-common/src/test/java/org/sonar/server/rule/HotspotRuleDescriptionTest.java similarity index 99% rename from server/sonar-webserver-webapi/src/test/java/org/sonar/server/hotspot/ws/HotspotRuleDescriptionTest.java rename to server/sonar-server-common/src/test/java/org/sonar/server/rule/HotspotRuleDescriptionTest.java index 1a35b069c1b..511a516b293 100644 --- a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/hotspot/ws/HotspotRuleDescriptionTest.java +++ b/server/sonar-server-common/src/test/java/org/sonar/server/rule/HotspotRuleDescriptionTest.java @@ -17,7 +17,7 @@ * along with this program; if not, write to the Free Software Foundation, * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ -package org.sonar.server.hotspot.ws; +package org.sonar.server.rule; import com.tngtech.java.junit.dataprovider.DataProvider; import com.tngtech.java.junit.dataprovider.DataProviderRunner; diff --git a/server/sonar-server-common/src/test/java/org/sonar/server/rule/index/RuleIndexerTest.java b/server/sonar-server-common/src/test/java/org/sonar/server/rule/index/RuleIndexerTest.java index beae75e6e24..5bba89e4db3 100644 --- a/server/sonar-server-common/src/test/java/org/sonar/server/rule/index/RuleIndexerTest.java +++ b/server/sonar-server-common/src/test/java/org/sonar/server/rule/index/RuleIndexerTest.java @@ -28,6 +28,7 @@ import java.util.Random; import java.util.Set; import java.util.stream.IntStream; import java.util.stream.Stream; +import javax.annotation.Nullable; import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; @@ -55,6 +56,7 @@ import static java.util.Collections.emptyList; import static java.util.Collections.emptySet; import static java.util.stream.Collectors.joining; import static java.util.stream.Collectors.toSet; +import static org.apache.commons.lang.RandomStringUtils.randomAlphabetic; import static org.assertj.core.api.Assertions.assertThat; import static org.elasticsearch.index.query.QueryBuilders.termQuery; import static org.sonar.server.rule.index.RuleIndexDefinition.TYPE_RULE; @@ -65,6 +67,11 @@ import static org.sonar.server.security.SecurityStandards.SQ_CATEGORY_KEYS_ORDER @RunWith(DataProviderRunner.class) public class RuleIndexerTest { + public static final String VALID_HOTSPOT_RULE_DESCRIPTION = "acme\n" + + "

Ask Yourself Whether

\n" + + "bar\n" + + "

Recommended Secure Coding Practices

\n" + + "foo"; @Rule public EsTester es = EsTester.create(); @Rule @@ -181,7 +188,10 @@ public class RuleIndexerTest { .flatMap(t -> CWES_BY_SQ_CATEGORY.get(t).stream().map(e -> "cwe:" + e)) .collect(toSet()); SecurityStandards securityStandards = SecurityStandards.fromSecurityStandards(standards); - RuleDefinitionDto rule = dbTester.rules().insert(RuleTesting.newRule().setType(RuleType.SECURITY_HOTSPOT).setSecurityStandards(standards)); + RuleDefinitionDto rule = dbTester.rules().insert(RuleTesting.newRule() + .setType(RuleType.SECURITY_HOTSPOT) + .setSecurityStandards(standards) + .setDescription(VALID_HOTSPOT_RULE_DESCRIPTION)); OrganizationDto organization = dbTester.organizations().insert(); underTest.commitAndIndex(dbTester.getSession(), rule.getId(), organization); @@ -211,4 +221,82 @@ public class RuleIndexerTest { {sqCategory1, sqCategory2} }; } + + @Test + @UseDataProvider("nullEmptyOrNoTitleDescription") + public void log_a_warning_when_hotspot_rule_description_is_null_or_empty_or_has_none_of_the_key_titles(@Nullable String description) { + RuleDefinitionDto rule = dbTester.rules().insert(RuleTesting.newRule() + .setType(RuleType.SECURITY_HOTSPOT) + .setDescription(description)); + OrganizationDto organization = dbTester.organizations().insert(); + underTest.commitAndIndex(dbTester.getSession(), rule.getId(), organization); + + assertThat(logTester.getLogs()).hasSize(1); + assertThat(logTester.logs(LoggerLevel.WARN).get(0)) + .isEqualTo(format( + "Description of Security Hotspot Rule %s can't be fully parsed: What is the risk?=missing, Are you vulnerable?=missing, How to fix it=missing", + rule.getKey())); + } + + @DataProvider + public static Object[][] nullEmptyOrNoTitleDescription() { + return new Object[][] { + {null}, + {""}, + {" "}, + {randomAlphabetic(30)} + }; + } + + @Test + public void log_a_warning_when_hotspot_rule_description_is_missing_fixIt_tab_content() { + RuleDefinitionDto rule = dbTester.rules().insert(RuleTesting.newRule() + .setType(RuleType.SECURITY_HOTSPOT) + .setDescription("bar\n" + + "

Ask Yourself Whether

\n" + + "foo")); + OrganizationDto organization = dbTester.organizations().insert(); + underTest.commitAndIndex(dbTester.getSession(), rule.getId(), organization); + + assertThat(logTester.getLogs()).hasSize(1); + assertThat(logTester.logs(LoggerLevel.WARN).get(0)) + .isEqualTo(format( + "Description of Security Hotspot Rule %s can't be fully parsed: What is the risk?=ok, Are you vulnerable?=ok, How to fix it=missing", + rule.getKey())); + } + + @Test + public void log_a_warning_when_hotspot_rule_description_is_missing_risk_tab_content() { + RuleDefinitionDto rule = dbTester.rules().insert(RuleTesting.newRule() + .setType(RuleType.SECURITY_HOTSPOT) + .setDescription("

Ask Yourself Whether

\n" + + "bar\n" + + "

Recommended Secure Coding Practices

\n" + + "foo")); + OrganizationDto organization = dbTester.organizations().insert(); + underTest.commitAndIndex(dbTester.getSession(), rule.getId(), organization); + + assertThat(logTester.getLogs()).hasSize(1); + assertThat(logTester.logs(LoggerLevel.WARN).get(0)) + .isEqualTo(format( + "Description of Security Hotspot Rule %s can't be fully parsed: What is the risk?=missing, Are you vulnerable?=ok, How to fix it=ok", + rule.getKey())); + } + + @Test + public void log_a_warning_when_hotspot_rule_description_is_missing_vulnerable_tab_content() { + RuleDefinitionDto rule = dbTester.rules().insert(RuleTesting.newRule() + .setType(RuleType.SECURITY_HOTSPOT) + .setDescription("bar\n" + + "

Recommended Secure Coding Practices

\n" + + "foo")); + OrganizationDto organization = dbTester.organizations().insert(); + underTest.commitAndIndex(dbTester.getSession(), rule.getId(), organization); + + assertThat(logTester.getLogs()).hasSize(1); + assertThat(logTester.logs(LoggerLevel.WARN).get(0)) + .isEqualTo(format( + "Description of Security Hotspot Rule %s can't be fully parsed: What is the risk?=ok, Are you vulnerable?=missing, How to fix it=ok", + rule.getKey())); + } } diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/ShowAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/ShowAction.java index 21ab159e185..2bf315ca165 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/ShowAction.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/ShowAction.java @@ -43,6 +43,7 @@ import org.sonar.server.issue.IssueChangeWSSupport.FormattingContext; import org.sonar.server.issue.IssueChangeWSSupport.Load; import org.sonar.server.issue.TextRangeResponseFormatter; import org.sonar.server.issue.ws.UserResponseFormatter; +import org.sonar.server.rule.HotspotRuleDescription; import org.sonar.server.security.SecurityStandards; import org.sonarqube.ws.Common; import org.sonarqube.ws.Hotspots; -- 2.39.5