From 50e4bafacd3e4dfe636f0ea5d95eb6a1d015eb58 Mon Sep 17 00:00:00 2001 From: =?utf8?q?L=C3=A9o=20Geoffroy?= Date: Tue, 15 Aug 2023 14:40:53 +0200 Subject: [PATCH] SONAR-20021 Add clean code information to tainted vulnerability endpoint --- .../ce/task/projectanalysis/issue/Rule.java | 1 - .../task/projectanalysis/issue/RuleImpl.java | 1 - .../pushevent/PushEventFactory.java | 22 +++++++++ .../pushevent/TaintVulnerabilityRaised.java | 48 +++++++++++++++++++ .../pushevent/PushEventFactoryTest.java | 37 +++++++++++++- 5 files changed, 106 insertions(+), 3 deletions(-) diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/Rule.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/Rule.java index 53e41475825..94c0e3eee9a 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/Rule.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/Rule.java @@ -71,6 +71,5 @@ public interface Rule { Map getDefaultImpacts(); - @CheckForNull CleanCodeAttribute cleanCodeAttribute(); } diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/RuleImpl.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/RuleImpl.java index 42c5d5c26c3..2299f409a7e 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/RuleImpl.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/RuleImpl.java @@ -153,7 +153,6 @@ public class RuleImpl implements Rule { return defaultImpacts; } - @CheckForNull @Override public CleanCodeAttribute cleanCodeAttribute() { return cleanCodeAttribute; diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/pushevent/PushEventFactory.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/pushevent/PushEventFactory.java index 8c3a5c7580e..58c5e0ddeea 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/pushevent/PushEventFactory.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/pushevent/PushEventFactory.java @@ -21,10 +21,15 @@ package org.sonar.ce.task.projectanalysis.pushevent; import com.google.gson.Gson; import com.google.gson.GsonBuilder; +import java.util.EnumMap; +import java.util.List; +import java.util.Map; import java.util.Optional; import java.util.Set; import org.jetbrains.annotations.NotNull; import org.sonar.api.ce.ComputeEngineSide; +import org.sonar.api.issue.impact.Severity; +import org.sonar.api.issue.impact.SoftwareQuality; import org.sonar.api.rules.RuleType; import org.sonar.ce.task.projectanalysis.analysis.AnalysisMetadataHolder; import org.sonar.ce.task.projectanalysis.component.Component; @@ -139,9 +144,26 @@ public class PushEventFactory { event.setMainLocation(prepareMainLocation(component, issue)); event.setFlows(flowGenerator.convertFlows(component.getName(), requireNonNull(issue.getLocations()))); issue.getRuleDescriptionContextKey().ifPresent(event::setRuleDescriptionContextKey); + + Rule rule = ruleRepository.getByKey(issue.getRuleKey()); + event.setCleanCodeAttribute(rule.cleanCodeAttribute().name()); + event.setCleanCodeAttributeCategory(rule.cleanCodeAttribute().getAttributeCategory().name()); + event.setImpacts(computeEffectiveImpacts(rule.getDefaultImpacts(), issue.impacts())); return event; } + private static List computeEffectiveImpacts(Map defaultImpacts, Map impacts) { + Map impactMap = new EnumMap<>(defaultImpacts); + impacts.forEach((softwareQuality, severity) -> impactMap.computeIfPresent(softwareQuality, (existingSoftwareQuality, existingSeverity) -> severity)); + return impactMap.entrySet().stream() + .map(e -> { + TaintVulnerabilityRaised.Impact impact = new TaintVulnerabilityRaised.Impact(); + impact.setSoftwareQuality(e.getKey().name()); + impact.setSeverity(e.getValue().name()); + return impact; + }).toList(); + } + private static Location prepareMainLocation(Component component, DefaultIssue issue) { DbIssues.Locations issueLocations = requireNonNull(issue.getLocations()); TextRange mainLocationTextRange = getTextRange(issueLocations.getTextRange(), issueLocations.getChecksum()); diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/pushevent/TaintVulnerabilityRaised.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/pushevent/TaintVulnerabilityRaised.java index 003718bdfc4..30e57047e0f 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/pushevent/TaintVulnerabilityRaised.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/pushevent/TaintVulnerabilityRaised.java @@ -36,6 +36,9 @@ public class TaintVulnerabilityRaised extends IssueEvent { private Location mainLocation; private List flows; private String ruleDescriptionContextKey; + private String cleanCodeAttribute; + private String cleanCodeAttributeCategory; + private List impacts; public TaintVulnerabilityRaised() { // nothing to do @@ -109,4 +112,49 @@ public class TaintVulnerabilityRaised extends IssueEvent { public void setRuleDescriptionContextKey(String ruleDescriptionContextKey) { this.ruleDescriptionContextKey = ruleDescriptionContextKey; } + + public String getCleanCodeAttribute() { + return cleanCodeAttribute; + } + + public void setCleanCodeAttribute(String cleanCodeAttribute) { + this.cleanCodeAttribute = cleanCodeAttribute; + } + + public String getCleanCodeAttributeCategory() { + return cleanCodeAttributeCategory; + } + + public void setCleanCodeAttributeCategory(String cleanCodeAttributeCategory) { + this.cleanCodeAttributeCategory = cleanCodeAttributeCategory; + } + + public List getImpacts() { + return impacts; + } + + public void setImpacts(List impacts) { + this.impacts = impacts; + } + + public static class Impact { + String softwareQuality; + String severity; + + public String getSoftwareQuality() { + return softwareQuality; + } + + public void setSoftwareQuality(String softwareQuality) { + this.softwareQuality = softwareQuality; + } + + public String getSeverity() { + return severity; + } + + public void setSeverity(String severity) { + this.severity = severity; + } + } } diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/pushevent/PushEventFactoryTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/pushevent/PushEventFactoryTest.java index 028a0c11843..c76207cfec2 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/pushevent/PushEventFactoryTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/pushevent/PushEventFactoryTest.java @@ -23,11 +23,15 @@ import com.google.gson.Gson; import java.nio.charset.StandardCharsets; import java.util.Date; import java.util.Set; +import org.assertj.core.groups.Tuple; import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.sonar.api.issue.Issue; +import org.sonar.api.issue.impact.Severity; +import org.sonar.api.issue.impact.SoftwareQuality; import org.sonar.api.rule.RuleKey; +import org.sonar.api.rules.CleanCodeAttribute; import org.sonar.api.rules.RuleType; import org.sonar.api.utils.DateUtils; import org.sonar.ce.task.projectanalysis.analysis.AnalysisMetadataHolderRule; @@ -39,6 +43,8 @@ import org.sonar.ce.task.projectanalysis.issue.RuleRepository; import org.sonar.ce.task.projectanalysis.locations.flow.FlowGenerator; import org.sonar.core.issue.DefaultIssue; import org.sonar.core.issue.FieldDiffs; +import org.sonar.core.util.Uuids; +import org.sonar.db.issue.ImpactDto; import org.sonar.db.protobuf.DbCommons; import org.sonar.db.protobuf.DbIssues; import org.sonar.db.rule.RuleDto; @@ -47,6 +53,7 @@ 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.assertj.core.api.Assertions.tuple; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -104,11 +111,36 @@ public class PushEventFactoryTest { assertThat(taintVulnerabilityRaised.getRuleKey()).isEqualTo(defaultIssue.ruleKey().toString()); assertThat(taintVulnerabilityRaised.getType()).isEqualTo(defaultIssue.type().name()); assertThat(taintVulnerabilityRaised.getBranch()).isEqualTo(BRANCH_NAME); + assertThat(taintVulnerabilityRaised.getCleanCodeAttribute()).isEqualTo(CleanCodeAttribute.CONVENTIONAL.name()); + assertThat(taintVulnerabilityRaised.getCleanCodeAttributeCategory()).isEqualTo(CleanCodeAttribute.CONVENTIONAL.getAttributeCategory().name()); + assertThat(taintVulnerabilityRaised.getImpacts()).extracting(TaintVulnerabilityRaised.Impact::getSoftwareQuality, TaintVulnerabilityRaised.Impact::getSeverity) + .containsExactlyInAnyOrder(Tuple.tuple(SoftwareQuality.MAINTAINABILITY.name(), Severity.MEDIUM.name()), + Tuple.tuple(SoftwareQuality.RELIABILITY.name(), Severity.HIGH.name())); + String ruleDescriptionContextKey = taintVulnerabilityRaised.getRuleDescriptionContextKey().orElseGet(() -> fail("No rule description " + - "context key")); + "context key")); assertThat(ruleDescriptionContextKey).isEqualTo(defaultIssue.getRuleDescriptionContextKey().orElse(null)); } + @Test + public void raiseEventOnIssue_whenNewTaintVulnerabilityWithImpactAtRuleAndIssueLevel_shouldMergeImpacts() { + DefaultIssue defaultIssue = createDefaultIssue() + .setNew(true) + .addImpact(SoftwareQuality.MAINTAINABILITY, Severity.HIGH) + .setRuleDescriptionContextKey(randomAlphabetic(6)); + + when(taintChecker.isTaintVulnerability(any())).thenReturn(true); + + assertThat(underTest.raiseEventOnIssue("some-project-uuid", defaultIssue)) + .isNotEmpty() + .hasValueSatisfying(pushEventDto -> { + TaintVulnerabilityRaised taintVulnerabilityRaised = gson.fromJson(new String(pushEventDto.getPayload(), StandardCharsets.UTF_8), + TaintVulnerabilityRaised.class); + assertThat(taintVulnerabilityRaised.getImpacts()).extracting(TaintVulnerabilityRaised.Impact::getSoftwareQuality, TaintVulnerabilityRaised.Impact::getSeverity) + .containsExactlyInAnyOrder(tuple(SoftwareQuality.MAINTAINABILITY.name(), Severity.HIGH.name()), tuple(SoftwareQuality.RELIABILITY.name(), Severity.HIGH.name())); + }); + } + @Test public void raiseEventOnIssue_whenReopenedTaintVulnerability_shouldCreateRaisedEvent() { DefaultIssue defaultIssue = createDefaultIssue() @@ -379,6 +411,9 @@ public class PushEventFactoryTest { RuleDto ruleDto = new RuleDto(); ruleDto.setRuleKey(RuleKey.of("javasecurity", "S123")); ruleDto.setSecurityStandards(Set.of("owasp-a1")); + ruleDto.setCleanCodeAttribute(CleanCodeAttribute.CONVENTIONAL); + ruleDto.addDefaultImpact(new ImpactDto().setUuid(Uuids.createFast()).setSoftwareQuality(SoftwareQuality.MAINTAINABILITY).setSeverity(Severity.MEDIUM)); + ruleDto.addDefaultImpact(new ImpactDto().setUuid(Uuids.createFast()).setSoftwareQuality(SoftwareQuality.RELIABILITY).setSeverity(Severity.HIGH)); return new org.sonar.ce.task.projectanalysis.issue.RuleImpl(ruleDto); } } -- 2.39.5