Skip to content

Commit

Permalink
Fixed: CxFlow tries do delete a wrong SAST project when the project n…
Browse files Browse the repository at this point in the history 
…ame was defined in config-as-code (#384)

* Added the isUseConfigAsCodeFromDefaultBranch property to determine effective branch for getting config-as-code.

* Don't allow to delete SAST project if the corresponding branch is protected.

* Added test scenarios for config-as-code and SAST project deletion functionality.
  • Loading branch information
@alex-ko-dev
alex-ko-dev authored Aug 12, 2020
1 parent 913046a commit 36ff4fe
Show file tree
Hide file tree
Showing 14 changed files with 503 additions and 332 deletions.
6 changes: 5 additions & 1 deletion src/main/java/com/checkmarx/flow/config/GitHubProperties.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package com.checkmarx.flow.config;

import lombok.Getter;
import lombok.Setter;
import org.springframework.boot.context.properties.ConfigurationProperties;
import org.springframework.stereotype.Component;
import org.springframework.validation.annotation.Validated;
Expand All @@ -8,6 +10,9 @@
@ConfigurationProperties(prefix = "github")
@Validated
public class GitHubProperties extends RepoProperties {
@Getter
@Setter
private boolean useConfigAsCodeFromDefaultBranch;

public String getMergeNoteUri(String namespace, String repo, String mergeId){
String format = "%s/%s/%s/issues/%s/comments";
Expand All @@ -20,5 +25,4 @@ public String getGitUri(String namespace, String repo){
return String.format(format, getUrl(), namespace, repo);
//sample: https://github.com/namespace/repo.git
}

}
9 changes: 5 additions & 4 deletions src/main/java/com/checkmarx/flow/config/RepoProperties.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package com.checkmarx.flow.config;

import lombok.Getter;
import lombok.Setter;
import org.apache.commons.lang3.StringUtils;

import javax.annotation.PostConstruct;
Expand All @@ -12,6 +14,7 @@ public class RepoProperties {
private String apiUrl;
private String falsePositiveLabel = "false-positive";
private String configAsCode = "cx.config";

private String openTransition = "open";
private String closeTransition = "closed";
private String filePath = ".";
Expand Down Expand Up @@ -173,10 +176,8 @@ public void setCxSummaryHeader(String cxSummaryHeader) {

@PostConstruct
private void postConstruct() {
if(this.apiUrl != null){
if(this.apiUrl.endsWith("/")){
this.setApiUrl(StringUtils.chop(apiUrl));
}
if(apiUrl != null && apiUrl.endsWith("/")){
setApiUrl(StringUtils.chop(apiUrl));
}
}
}
10 changes: 7 additions & 3 deletions src/main/java/com/checkmarx/flow/controller/GitHubController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -419,21 +419,25 @@ public ResponseEntity<EventResponse> deleteBranchRequest(
.repoUrl(repository.getCloneUrl())
.repoType(ScanRequest.Repository.NA)
.branch(currentBranch)
.defaultBranch(repository.getDefaultBranch())
.refs(event.getRef())
.build();

request.setScanPresetOverride(false);

CxConfig cxConfig = gitHubService.getCxConfigOverride(request);
request = configOverrider.overrideScanRequestProperties(cxConfig, request);

//deletes a project which is not in the middle of a scan, otherwise it will not be deleted
sastScanner.deleteProject(request);

log.info("Process of delete branch has finished successfully");
final String MESSAGE = "Branch deletion event was handled successfully.";
log.info(MESSAGE);

return ResponseEntity.status(HttpStatus.OK).body(EventResponse.builder()
.message("Delete Branch Successfully finished")
.message(MESSAGE)
.success(true)
.build());

}


Expand Down
133 changes: 88 additions & 45 deletions src/main/java/com/checkmarx/flow/service/GitHubService.java
View file
Original file line number Diff line number Diff line change
@@ -1,33 +1,26 @@
package com.checkmarx.flow.service;

import com.checkmarx.flow.config.FindingSeverity;
import com.checkmarx.flow.config.FlowProperties;
import com.checkmarx.flow.config.GitHubProperties;
import com.checkmarx.flow.config.*;
import com.checkmarx.flow.dto.*;
import com.checkmarx.flow.dto.github.Content;
import com.checkmarx.flow.dto.report.AnalyticsReport;
import com.checkmarx.flow.dto.report.PullRequestReport;
import com.checkmarx.flow.exception.GitHubClientRunTimeException;
import com.checkmarx.flow.utils.HTMLHelper;
import com.checkmarx.flow.utils.ScanUtils;
import com.checkmarx.sdk.dto.CxConfig;
import com.checkmarx.sdk.dto.ScanResults;
import lombok.extern.slf4j.Slf4j;
import org.apache.commons.codec.binary.Base64;
import org.apache.commons.lang3.StringUtils;
import org.apache.commons.lang3.exception.ExceptionUtils;
import org.codehaus.jackson.JsonNode;
import org.codehaus.jackson.map.ObjectMapper;
import org.json.JSONObject;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Qualifier;
import org.springframework.http.HttpEntity;
import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpMethod;
import org.springframework.http.ResponseEntity;
import org.springframework.http.*;
import org.springframework.stereotype.Service;
import org.springframework.web.client.HttpClientErrorException;
import org.springframework.web.client.RestClientException;
import org.springframework.web.client.RestTemplate;
import org.springframework.web.client.*;
import org.springframework.web.util.DefaultUriBuilderFactory;

import java.io.IOException;
Expand All @@ -36,9 +29,8 @@
import java.util.*;

@Service
@Slf4j
public class GitHubService extends RepoService {
private static final Logger log = LoggerFactory.getLogger(GitHubService.class);

private static final String HTTP_BODY_IS_NULL = "HTTP Body is null for content api ";
private static final String CONTENT_NOT_FOUND_IN_RESPONSE = "Content not found in JSON response";
private static final String STATUSES_URL_KEY = "statuses_url";
Expand Down Expand Up @@ -196,9 +188,8 @@ private void statusExchange(ScanRequest request, HttpEntity<?> httpEntity, Strin
}

void endBlockMerge(ScanRequest request, ScanResults results, ScanDetails scanDetails) {
logPullRequestWithScaResults(request, results);

logPullRequestWithScaResutls(request, results);

if (properties.isBlockMerge()) {
String statusApiUrl = request.getAdditionalMetadata(STATUSES_URL_KEY);
if (ScanUtils.empty(statusApiUrl)) {
Expand All @@ -207,9 +198,9 @@ void endBlockMerge(ScanRequest request, ScanResults results, ScanDetails scanDet
}

PullRequestReport report = new PullRequestReport(scanDetails, request);

HttpEntity<String> httpEntity = getStatusRequestEntity(results, report);

logPullRequestWithSastOsa(results, report);

log.debug("Updating pull request status: {}", statusApiUrl);
Expand Down Expand Up @@ -239,26 +230,22 @@ private boolean hasSastOsaScan(ScanResults results) {
}

private void logPullRequestWithSastOsa(ScanResults results, PullRequestReport report) {

//Report pull request only if there was SAST/OSA scan
//Otherwise it would be only SCA
if(hasSastOsaScan(results)) {
report.log();
}
}

private void logPullRequestWithScaResutls(ScanRequest request, ScanResults results) {
private void logPullRequestWithScaResults(ScanRequest request, ScanResults results) {
if(results.getScaResults() != null ) {
PullRequestReport report = new PullRequestReport(results.getScaResults().getScanId(), request, PullRequestReport.SCA);
PullRequestReport report = new PullRequestReport(results.getScaResults().getScanId(), request, AnalyticsReport.SCA);
report.setFindingsPerSeveritySca(results);
report.setPullRequestResult(OperationResult.successful());
report.log();
}

}



private HttpEntity<String> getStatusRequestEntity(ScanResults results, PullRequestReport pullRequestReport) {
String statusForApi = MERGE_SUCCESS;

Expand Down Expand Up @@ -367,7 +354,10 @@ private Sources getRepoLanguagePercentages(ScanRequest request) {
}
for (Map.Entry<String,Long> entry : langs.entrySet()){
Long bytes = entry.getValue();
double percentage = (Double.valueOf(bytes) / Double.valueOf(total) * 100);
double percentage = 0;
if (total != 0L) {
percentage = (Double.valueOf(bytes) / (double) total * 100);
}
langsPercent.put(entry.getKey(), (int) percentage);
}
sources.setLanguageStats(langsPercent);
Expand Down Expand Up @@ -427,7 +417,7 @@ public CxConfig getCxConfigOverride(ScanRequest request) {
CxConfig result = null;
if (StringUtils.isNotBlank(properties.getConfigAsCode())) {
try {
result = loadCxConfigFromGitHub(properties.getConfigAsCode(), request);
result = loadConfigAsCode(properties.getConfigAsCode(), request);
} catch (NullPointerException e) {
log.warn(CONTENT_NOT_FOUND_IN_RESPONSE);
} catch (HttpClientErrorException.NotFound e) {
Expand All @@ -439,31 +429,84 @@ public CxConfig getCxConfigOverride(ScanRequest request) {
return result;
}

private CxConfig loadCxConfigFromGitHub(String filename, ScanRequest request) {
HttpHeaders headers = createAuthHeaders();
String urlTemplate = properties.getApiUrl().concat(FILE_CONTENT);
ResponseEntity<String> response = restTemplate.exchange(
urlTemplate,
HttpMethod.GET,
new HttpEntity<>(headers),
String.class,
request.getNamespace(),
request.getRepoName(),
filename,
request.getBranch()
);
if (response.getBody() == null) {
private CxConfig loadConfigAsCode(String filename, ScanRequest request) {
CxConfig result = null;

String effectiveBranch = determineConfigAsCodeBranch(request);
String fileContent = downloadFileContent(filename, request, effectiveBranch);
if (fileContent == null) {
log.warn(HTTP_BODY_IS_NULL);
return null;
} else {
JSONObject json = new JSONObject(response.getBody());
JSONObject json = new JSONObject(fileContent);
String content = json.getString("content");
if (ScanUtils.empty(content)) {
log.warn(CONTENT_NOT_FOUND_IN_RESPONSE);
return null;
} else {
String decodedContent = new String(Base64.decodeBase64(content.trim()));
result = com.checkmarx.sdk.utils.ScanUtils.getConfigAsCode(decodedContent);
}
String decodedContent = new String(Base64.decodeBase64(content.trim()));
return com.checkmarx.sdk.utils.ScanUtils.getConfigAsCode(decodedContent);
}

return result;
}

private String downloadFileContent(String filename, ScanRequest request, String branch) {
ResponseEntity<String> response = null;

if (StringUtils.isNotEmpty(branch)) {
HttpHeaders headers = createAuthHeaders();
String urlTemplate = properties.getApiUrl().concat(FILE_CONTENT);

response = restTemplate.exchange(
urlTemplate,
HttpMethod.GET,
new HttpEntity<>(headers),
String.class,
request.getNamespace(),
request.getRepoName(),
filename,
branch);
} else {
log.warn("Unable to load config-as-code.");
}

return response != null ? response.getBody() : null;
}

private String determineConfigAsCodeBranch(ScanRequest request) {
String result;
log.debug("Determining a branch to get config-as-code from.");
if (properties.isUseConfigAsCodeFromDefaultBranch()) {
result = tryGetDefaultBranch(request);
} else {
result = tryGetCurrentBranch(request);
}
return result;
}

private static String tryGetCurrentBranch(ScanRequest request) {
String result = null;
if (StringUtils.isNotEmpty(request.getBranch())) {
result = request.getBranch();
log.debug("Using the current branch ({}) to get config-as-code.", result);
}
else {
log.warn("Tried to use current branch to get config-as-code. " +
"However, current branch couldn't be determined.");
}
return result;
}

private static String tryGetDefaultBranch(ScanRequest request) {
String result = null;
if (StringUtils.isNotEmpty(request.getDefaultBranch())) {
result = request.getDefaultBranch();
log.debug("Using the default branch ({}) to get config-as-code.", result);
}
else {
log.warn("Configuration indicates that default branch must be used to get config-as-code. " +
"However, default branch couldn't be determined.");
}
return result;
}
}
Loading

0 comments on commit 36ff4fe

Please sign in to comment.