]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-17671 Add description context key for taint vulnerabilities
authorAntoine Vinot <antoine.vinot@sonarsource.com>
Wed, 14 Dec 2022 10:46:51 +0000 (11:46 +0100)
committersonartech <sonartech@sonarsource.com>
Thu, 15 Dec 2022 20:03:33 +0000 (20:03 +0000)
server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/pushevent/PushEventFactory.java
server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/pushevent/TaintVulnerabilityRaised.java
server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/pushevent/PushEventFactoryTest.java
server/sonar-db-dao/src/main/resources/org/sonar/db/issue/IssueMapper.xml
server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/ws/BasePullAction.java
server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/ws/pull/PullTaintActionProtobufObjectGenerator.java
server/sonar-webserver-webapi/src/main/resources/org/sonar/server/issue/ws/pull-taint-example.proto
server/sonar-webserver-webapi/src/test/java/org/sonar/server/issue/ws/PullTaintActionTest.java
sonar-ws/src/main/protobuf/ws-issues.proto

index 30b1221456226b919979f55d8f9752e15f33a478..85e5218b402504f7da77d486a9e8fcf88fbc39b8 100644 (file)
@@ -99,22 +99,22 @@ public class PushEventFactory {
     event.setSeverity(issue.severity());
     event.setRuleKey(issue.getRuleKey().toString());
     event.setType(issue.type().name());
-
     event.setBranch(analysisMetadataHolder.getBranch().getName());
+    event.setMainLocation(prepareMainLocation(component, issue));
+    event.setFlows(flowGenerator.convertFlows(component.getName(), requireNonNull(issue.getLocations())));
+    issue.getRuleDescriptionContextKey().ifPresent(event::setRuleDescriptionContextKey);
+    return event;
+  }
 
+  private static Location prepareMainLocation(Component component, DefaultIssue issue) {
     DbIssues.Locations issueLocations = requireNonNull(issue.getLocations());
+    TextRange mainLocationTextRange = getTextRange(issueLocations.getTextRange(), issueLocations.getChecksum());
 
     Location mainLocation = new Location();
-    mainLocation.setMessage(issue.getMessage());
-
+    Optional.ofNullable(issue.getMessage()).ifPresent(mainLocation::setMessage);
     mainLocation.setFilePath(component.getName());
-
-    TextRange mainLocationTextRange = getTextRange(issueLocations.getTextRange(), issueLocations.getChecksum());
     mainLocation.setTextRange(mainLocationTextRange);
-    event.setMainLocation(mainLocation);
-
-    event.setFlows(flowGenerator.convertFlows(component.getName(), issueLocations));
-    return event;
+    return mainLocation;
   }
 
   private static PushEventDto raiseTaintVulnerabilityClosedEvent(String projectUuid, DefaultIssue issue) {
index a3ddd139ef6486d38dde0685b947b0d9b81e419c..d490bcf7fb7d7e2c378df1c75bdc9292d3f76dd6 100644 (file)
@@ -20,6 +20,7 @@
 package org.sonar.ce.task.projectanalysis.pushevent;
 
 import java.util.List;
+import java.util.Optional;
 import org.sonar.ce.task.projectanalysis.locations.flow.Flow;
 import org.sonar.ce.task.projectanalysis.locations.flow.Location;
 
@@ -32,10 +33,9 @@ public class TaintVulnerabilityRaised {
   private String ruleKey;
   private String severity;
   private String type;
-
   private Location mainLocation;
-
   private List<Flow> flows;
+  private String ruleDescriptionContextKey;
 
   public TaintVulnerabilityRaised() {
     // nothing to do
@@ -112,4 +112,12 @@ public class TaintVulnerabilityRaised {
   public void setFlows(List<Flow> flows) {
     this.flows = flows;
   }
+
+  public Optional<String> getRuleDescriptionContextKey() {
+    return Optional.ofNullable(ruleDescriptionContextKey);
+  }
+
+  public void setRuleDescriptionContextKey(String ruleDescriptionContextKey) {
+    this.ruleDescriptionContextKey = ruleDescriptionContextKey;
+  }
 }
index 1b4a04e3d5d87a6ce30d1377801361cfc6fc5637..55959eb9a8fbc7f08f90cea4265dfea54fffc68d 100644 (file)
@@ -19,6 +19,8 @@
  */
 package org.sonar.ce.task.projectanalysis.pushevent;
 
+import com.google.gson.Gson;
+import java.nio.charset.StandardCharsets;
 import java.util.Date;
 import java.util.List;
 import org.junit.Before;
@@ -39,19 +41,24 @@ import org.sonar.db.protobuf.DbCommons;
 import org.sonar.db.protobuf.DbIssues;
 import org.sonar.server.issue.TaintChecker;
 
+import static org.apache.commons.lang.RandomStringUtils.randomAlphabetic;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.fail;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
 public class PushEventFactoryTest {
 
+  private static final Gson gson = new Gson();
+  private static final String BRANCH_NAME = "develop";
+
   private final TaintChecker taintChecker = mock(TaintChecker.class);
   @Rule
   public MutableTreeRootHolderRule treeRootHolder = new MutableTreeRootHolderRule();
   @Rule
   public AnalysisMetadataHolderRule analysisMetadataHolder = new AnalysisMetadataHolderRule()
-    .setBranch(new TestBranch("develop"));
+    .setBranch(new TestBranch(BRANCH_NAME));
 
   private final FlowGenerator flowGenerator = new FlowGenerator(treeRootHolder);
   private final PushEventFactory underTest = new PushEventFactory(treeRootHolder, analysisMetadataHolder, taintChecker, flowGenerator);
@@ -67,19 +74,35 @@ public class PushEventFactoryTest {
   @Test
   public void raise_event_to_repository_if_taint_vulnerability_is_new() {
     DefaultIssue defaultIssue = createDefaultIssue()
-      .setNew(true);
+      .setNew(true)
+      .setRuleDescriptionContextKey(randomAlphabetic(6));
 
     assertThat(underTest.raiseEventOnIssue("some-project-uuid", defaultIssue))
       .isNotEmpty()
       .hasValueSatisfying(pushEventDto -> {
         assertThat(pushEventDto.getName()).isEqualTo("TaintVulnerabilityRaised");
-        assertThat(pushEventDto.getPayload()).isNotNull();
+        verifyPayload(pushEventDto.getPayload(), defaultIssue);
         assertThat(pushEventDto.getLanguage()).isEqualTo("java");
         assertThat(pushEventDto.getProjectUuid()).isEqualTo("some-project-uuid");
       });
 
   }
 
+  private static void verifyPayload(byte[] payload, DefaultIssue defaultIssue) {
+    assertThat(payload).isNotNull();
+
+    TaintVulnerabilityRaised taintVulnerabilityRaised = gson.fromJson(new String(payload, StandardCharsets.UTF_8), TaintVulnerabilityRaised.class);
+    assertThat(taintVulnerabilityRaised.getProjectKey()).isEqualTo(defaultIssue.projectKey());
+    assertThat(taintVulnerabilityRaised.getCreationDate()).isEqualTo(defaultIssue.creationDate().getTime());
+    assertThat(taintVulnerabilityRaised.getKey()).isEqualTo(defaultIssue.key());
+    assertThat(taintVulnerabilityRaised.getSeverity()).isEqualTo(defaultIssue.severity());
+    assertThat(taintVulnerabilityRaised.getRuleKey()).isEqualTo(defaultIssue.ruleKey().toString());
+    assertThat(taintVulnerabilityRaised.getType()).isEqualTo(defaultIssue.type().name());
+    assertThat(taintVulnerabilityRaised.getBranch()).isEqualTo(BRANCH_NAME);
+    String ruleDescriptionContextKey = taintVulnerabilityRaised.getRuleDescriptionContextKey().orElseGet(() -> fail("No rule description context key"));
+    assertThat(ruleDescriptionContextKey).isEqualTo(defaultIssue.getRuleDescriptionContextKey().orElse(null));
+  }
+
   @Test
   public void raise_event_to_repository_if_taint_vulnerability_is_reopened() {
     DefaultIssue defaultIssue = createDefaultIssue()
index 655c142af2e6af18bcf2ece8d5619bce71477079..b8126c88f96db765dcc7ca0a92830fceba473755 100644 (file)
     i.issue_type as type,
     i.locations as locations,
     i.component_uuid as component_uuid,
-    i.assignee as assigneeUuid
+    i.assignee as assigneeUuid,
+    i.rule_description_context_key as ruleDescriptionContextKey
   </sql>
 
   <select id="selectByBranch" parameterType="map" resultType="Issue">
index c7cfe1503098f51037915c11aa3cb5f72b3b1788..4503c9e67b4d161feef25063ca696c7bb84c497b 100644 (file)
@@ -86,7 +86,7 @@ public abstract class BasePullAction implements IssuesWsAction {
       .setInternal(true)
       .setResponseExample(getClass().getResource(resourceExample))
       .setDescription(format("This endpoint fetches and returns all (unless filtered by optional params) the %s for a given branch. " +
-        "The %s returned are not paginated, so the response size can be big.", issueType, issueType))
+        "The %s returned are not paginated, so the response size can be big. Requires project 'Browse' permission.", issueType, issueType))
       .setSince(sinceVersion);
 
     action.createParam(PROJECT_KEY_PARAM)
index 0647cc9880c05d2f0c73a6224682cc1540ede9e5..3767094ad103b84e8e9a011cbe844624589c17d7 100644 (file)
@@ -96,6 +96,7 @@ public class PullTaintActionProtobufObjectGenerator implements ProtobufObjectGen
     taintBuilder.setType(Common.RuleType.forNumber(issueDto.getType()));
     taintBuilder.setClosed(false);
     taintBuilder.setMainLocation(locationBuilder.build());
+    issueDto.getOptionalRuleDescriptionContextKey().ifPresent(taintBuilder::setRuleDescriptionContextKey);
 
     return taintBuilder.build();
   }
index fc9d872c01a4ee633b3f89d3eb21cbba00db065f..05fb3bf1677846e977df1a808329f0bcc4628bfc 100644 (file)
@@ -13,6 +13,8 @@ message TaintLite {
   optional Location mainLocation = 7;
   optional bool closed = 8;
   optional Flow flows = 9;
+  optional bool assignedToSubscribedUser = 10;
+  optional string ruleDescriptionContextKey = 11;
 }
 
 message Location {
index 5a8ad622255d7c40476cc12a8c25cf7d101dced7..7ec6fa652c7037e698213ed1cfe7517498312c7e 100644 (file)
@@ -23,6 +23,7 @@ import java.io.IOException;
 import java.io.InputStream;
 import java.util.ArrayList;
 import java.util.List;
+import org.apache.commons.lang.RandomStringUtils;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
@@ -54,6 +55,7 @@ import org.sonarqube.ws.Common;
 import org.sonarqube.ws.Issues;
 
 import static java.lang.String.format;
+import static org.apache.commons.lang.RandomStringUtils.randomAlphabetic;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.mockito.Mockito.mock;
@@ -387,6 +389,7 @@ public class PullTaintActionTest {
     RuleDto javaRule = db.rules().insert(r -> r.setRepositoryKey("javasecurity"));
     RuleDto javaScriptRule = db.rules().insert(r -> r.setRepositoryKey("javascript"));
 
+    String ruledescriptionContextKey = randomAlphabetic(6);
     IssueDto issueDto = issueDbTester.insertIssue(p -> p.setSeverity("MINOR")
       .setManualSeverity(true)
       .setMessage("openIssue")
@@ -395,7 +398,8 @@ public class PullTaintActionTest {
       .setStatus(Issue.STATUS_OPEN)
       .setProject(correctProject)
       .setComponent(correctFile)
-      .setType(2));
+      .setType(2)
+      .setRuleDescriptionContextKey(ruledescriptionContextKey));
 
     //this one should not be returned - it is a normal issue, no taint
     issueDbTester.insertIssue(p -> p.setSeverity("MINOR")
@@ -416,7 +420,9 @@ public class PullTaintActionTest {
     List<Issues.TaintVulnerabilityLite> taints = readAllTaint(response);
 
     assertThat(taints).hasSize(1);
-    assertThat(taints.get(0).getKey()).isEqualTo(issueDto.getKey());
+    Issues.TaintVulnerabilityLite taintVulnerabilityLite = taints.get(0);
+    assertThat(taintVulnerabilityLite.getKey()).isEqualTo(issueDto.getKey());
+    assertThat(taintVulnerabilityLite.getRuleDescriptionContextKey()).isEqualTo(ruledescriptionContextKey);
   }
 
   @Test
index 3a99824f6b49a256bed9d5b5a82f268c465bd32f..b03e72200b13355fb7047f96fabe5f1716f5bcf9 100644 (file)
@@ -286,6 +286,7 @@ message TaintVulnerabilityLite {
   optional bool closed = 8;
   repeated Flow flows = 9;
   optional bool assignedToSubscribedUser = 10;
+  optional string ruleDescriptionContextKey = 11;
 }
 
 message Flow {