Skip to content

Commit

Permalink
fix sast
Browse files Browse the repository at this point in the history
  • Loading branch information
@attiasas
attiasas committed Sep 11, 2023
1 parent 8adb862 commit 77a72d8
Show file tree
Hide file tree
Showing 6 changed files with 81 additions and 46 deletions.
1 change: 1 addition & 0 deletions scanpullrequest/scanpullrequest.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,7 @@ func auditPullRequest(repoConfig *utils.Repository, client vcsclient.VcsClient,
applicableIssues = append(applicableIssues, filterNotApplicableResults(sourceScanResults.ApplicabilityScanResults)...)
iacIssues = append(iacIssues, sourceScanResults.IacScanResults...)
secretsIssues = append(secretsIssues, sourceScanResults.SecretsScanResults...)
sastIssues = append(sastIssues, sourceScanResults.SastScanResults...)
continue
}

Expand Down
13 changes: 10 additions & 3 deletions utils/outputwriter/outputwriter.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import (
const (
FrogbotTitlePrefix = "[🐸 Frogbot]"
CommentGeneratedByFrogbot = "[JFrog Frogbot](https://github.com/jfrog/frogbot#readme)"
ReviewCommentGeneratedByFrogbot = "[[🐸 JFrog Frogbot]](https://github.com/jfrog/frogbot#readme)"
ReviewCommentGeneratedByFrogbot = "[🐸 JFrog Frogbot](https://github.com/jfrog/frogbot#readme)"
vulnerabilitiesTableHeader = "\n| SEVERITY | DIRECT DEPENDENCIES | IMPACTED DEPENDENCY | FIXED VERSIONS |\n| :---------------------: | :----------------------------------: | :-----------------------------------: | :---------------------------------: |"
vulnerabilitiesTableHeaderWithContextualAnalysis = "| SEVERITY | CONTEXTUAL ANALYSIS | DIRECT DEPENDENCIES | IMPACTED DEPENDENCY | FIXED VERSIONS |\n| :---------------------: | :----------------------------------: | :----------------------------------: | :-----------------------------------: | :---------------------------------: |"
iacTableHeader = "\n| SEVERITY | FILE | LINE:COLUMN | FINDING |\n| :---------------------: | :----------------------------------: | :-----------------------------------: | :---------------------------------: |"
Expand Down Expand Up @@ -104,7 +104,7 @@ type OutputWriter interface {
IacContent(iacRows []formats.SourceCodeRow) string
Footer() string
Separator() string
FormattedSeverity(severity, applicability string) string
FormattedSeverity(severity, applicability string, addName bool) string
IsFrogbotResultComment(comment string) bool
SetJasOutputFlags(entitled, showCaColumn bool)
VcsProvider() vcsutils.VcsProvider
Expand All @@ -114,6 +114,7 @@ type OutputWriter interface {
ApplicableCveReviewContent(severity, finding, fullDetails, cveDetails, remediation string) string
IacReviewContent(severity, finding, fullDetails string) string
SastReviewContent(severity, finding, fullDetails string, codeFlows []*sarif.CodeFlow) string
ReviewFooter() string
}

func GetCompatibleOutputWriter(provider vcsutils.VcsProvider) OutputWriter {
Expand Down Expand Up @@ -196,7 +197,7 @@ func getVulnerabilitiesTableContent(vulnerabilities []formats.VulnerabilityOrVio
func getIacTableContent(iacRows []formats.SourceCodeRow, writer OutputWriter) string {
var tableContent string
for _, iac := range iacRows {
tableContent += fmt.Sprintf("\n| %s | %s | %s | %s |", writer.FormattedSeverity(iac.Severity, string(xrayutils.Applicable)), iac.File, iac.LineColumn, iac.Snippet)
tableContent += fmt.Sprintf("\n| %s | %s | %s | %s |", writer.FormattedSeverity(iac.Severity, string(xrayutils.Applicable), true), iac.File, iac.LineColumn, iac.Snippet)
}
return tableContent
}
Expand All @@ -209,6 +210,12 @@ func MarkAsQuote(s string) string {
return fmt.Sprintf("`%s`", s)
}

func GetJasMarkdownDescription(severity, finding string) string {
headerRow := "| Severity | Finding |\n"
separatorRow := "| :---: | :---: |\n"
return headerRow + separatorRow + fmt.Sprintf("| %s | %s |", severity, finding)
}

func GetAggregatedPullRequestTitle(tech coreutils.Technology) string {
if tech.ToString() == "" {
return FrogbotTitlePrefix + " Update dependencies"
Expand Down
20 changes: 15 additions & 5 deletions utils/outputwriter/simplifiedoutput.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ type SimplifiedOutput struct {
}

func (smo *SimplifiedOutput) VulnerabilitiesTableRow(vulnerability formats.VulnerabilityOrViolationRow) string {
row := fmt.Sprintf("| %s | ", smo.FormattedSeverity(vulnerability.Severity, vulnerability.Applicable))
row := fmt.Sprintf("| %s | ", smo.FormattedSeverity(vulnerability.Severity, vulnerability.Applicable, true))
directsRowFmt := directDependencyRow
if smo.showCaColumn {
row += vulnerability.Applicable + " |"
Expand Down Expand Up @@ -129,7 +129,7 @@ Finding: %s
%s
`,
smo.FormattedSeverity(severity, "Applicable"),
smo.FormattedSeverity(severity, "Applicable", false),
finding,
fullDetails,
cveDetails)
Expand All @@ -146,7 +146,7 @@ Finding: %s
%s
`,
smo.FormattedSeverity(severity, "Applicable"),
smo.FormattedSeverity(severity, "Applicable", false),
finding,
fullDetails)
}
Expand All @@ -169,7 +169,7 @@ Finding: %s
#### Vulnerable data flows
`,
smo.FormattedSeverity(severity, "Applicable"),
smo.FormattedSeverity(severity, "Applicable", false),
finding,
fullDetails,
))
Expand Down Expand Up @@ -230,11 +230,21 @@ func (smo *SimplifiedOutput) Footer() string {
return fmt.Sprintf("\n\n%s", CommentGeneratedByFrogbot)
}

func (smo *SimplifiedOutput) ReviewFooter() string {
return fmt.Sprintf(`
---
%s
`, ReviewCommentGeneratedByFrogbot)
}

func (smo *SimplifiedOutput) Separator() string {
return ", "
}

func (smo *SimplifiedOutput) FormattedSeverity(severity, _ string) string {
func (smo *SimplifiedOutput) FormattedSeverity(severity, _ string, _ bool) string {
return severity
}

Expand Down
75 changes: 49 additions & 26 deletions utils/outputwriter/standardoutput.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ func (so *StandardOutput) VulnerabilitiesTableRow(vulnerability formats.Vulnerab
directDependencies.WriteString(fmt.Sprintf("%s:%s%s", dependency.Name, dependency.Version, so.Separator()))
}

row := fmt.Sprintf("| %s | ", so.FormattedSeverity(vulnerability.Severity, vulnerability.Applicable))
row := fmt.Sprintf("| %s | ", so.FormattedSeverity(vulnerability.Severity, vulnerability.Applicable, true))
if so.showCaColumn {
row += vulnerability.Applicable + " | "
}
Expand Down Expand Up @@ -130,36 +130,36 @@ func (so *StandardOutput) ApplicableCveReviewContent(severity, finding, fullDeta
return fmt.Sprintf(`
### 📦🔍 Applicable dependency CVE Vulnerability
Severity: %s
Finding: %s
%s
#### 👇 Details
<details>
<summary> <b>Description</b> </summary>
<br>
%s
</details>
<details>
<summary> <b>CVE details</b> </summary>
<br>
%s
</details>
<details>
<summary> <b>Remediation</b> </summary>
<br>
%s
</details>
`,
so.FormattedSeverity(severity, "Applicable"),
finding,
GetJasMarkdownDescription(so.FormattedSeverity(severity, "Applicable", false),finding),
fullDetails,
cveDetails,
remediation)
Expand All @@ -169,68 +169,67 @@ func (so *StandardOutput) IacReviewContent(severity, finding, fullDetails string
return fmt.Sprintf(`
### 🛠️ Infrastructure as Code Vulnerability
Severity: %s
Finding: %s
#### 👇 Details
%s
<details>
<summary> <b>Full description</b> </summary>
<br>
%s
</details>
`,
so.FormattedSeverity(severity, "Applicable"),
MarkAsQuote(finding),
GetJasMarkdownDescription(so.FormattedSeverity(severity, "Applicable", false),finding),
fullDetails)
}

func (so *StandardOutput) SastReviewContent(severity, finding, fullDetails string, codeFlows []*sarif.CodeFlow) string {
var contentBuilder strings.Builder
contentBuilder.WriteString(fmt.Sprintf(`
### 🔐 Static Application Security Testing (SAST) Vulnerability
Severity: %s
<div align="center">
Finding: %s
### 🎯 Static Application Security Testing (SAST) Vulnerability
%s
#### 👇 Details
<details>
<summary> <b>Full description</b> </summary>
<br>
%s
</details>
`,
so.FormattedSeverity(severity, "Applicable"),
MarkAsQuote(finding),
GetJasMarkdownDescription(so.FormattedSeverity(severity, "Applicable", false),finding),
fullDetails,
))

if len(codeFlows) > 0 {
contentBuilder.WriteString(`
`)
dataFlowId := 1
for _, codeFlow := range codeFlows {
for _, threadFlow := range codeFlow.ThreadFlows {
contentBuilder.WriteString(fmt.Sprintf(`
<details>
<summary> <b>%d. Vulnerable data flow analysis result</b> </summary>
<summary> <b>Vulnerable data flow analysis result</b> </summary>
<br>
`,
dataFlowId,
))

for i, threadFlowLocation := range threadFlow.Locations {
for _, threadFlowLocation := range threadFlow.Locations {
contentBuilder.WriteString(fmt.Sprintf(`
%d. %s (at %s line %d)
%s. %s (at %s line %d)
`,
i+1,
xrayutils.GetLocationSnippet(threadFlowLocation.Location),
"↘️",
MarkAsQuote(xrayutils.GetLocationSnippet(threadFlowLocation.Location)),
xrayutils.GetLocationFileName(threadFlowLocation.Location),
xrayutils.GetLocationStartLine(threadFlowLocation.Location),
))
Expand Down Expand Up @@ -278,12 +277,27 @@ func (so *StandardOutput) Footer() string {
`, CommentGeneratedByFrogbot)
}

func (so *StandardOutput) ReviewFooter() string {
return fmt.Sprintf(`
---
<div align="center">
%s
</div>
`, ReviewCommentGeneratedByFrogbot)
}

func (so *StandardOutput) Separator() string {
return "<br><br>"
}

func (so *StandardOutput) FormattedSeverity(severity, applicability string) string {
return fmt.Sprintf("%s%8s", getSeverityTag(IconName(severity), applicability), severity)
func (so *StandardOutput) FormattedSeverity(severity, applicability string, addName bool) string {
s := fmt.Sprintf("%s", getSeverityTag(IconName(severity), applicability))
if addName {
s = fmt.Sprintf(s + "%8s", severity)
}
return s
}

func (so *StandardOutput) UntitledForJasMsg() string {
Expand All @@ -300,3 +314,12 @@ func (so *StandardOutput) UntitledForJasMsg() string {
}
return msg
}

func wrapInDetails(str, summary string) string {
var contentBuilder strings.Builder
contentBuilder.WriteString(`
`)

return contentBuilder.String()
}
7 changes: 6 additions & 1 deletion utils/reviewcomment.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,11 @@ func getCommentsToUpdate(repo *Repository, pullRequestID int, client vcsclient.V
err = errors.New("couldn't list existing review comments: " + err.Error())
return
}
log.Debug("Detected", len(existingComments), "existing comments")
existingApplicableComments, existingIacComments, existingSastComments := extractFrogbotReviewComments(existingComments)
log.Debug("Detected", len(existingApplicableComments), "JFrog Applicable review comments")
log.Debug("Detected", len(existingIacComments), "JFrog Iac review comments")
log.Debug("Detected", len(existingSastComments), "JFrog Sast review comments")
writer := repo.OutputWriter

// Get comments related to updates on applicable review
Expand Down Expand Up @@ -247,6 +251,7 @@ func extractRunReviewChanges(commentType ReviewCommentType, data *sarif.Run, exi
id := generateFrogbotReviewCommentId(commentType, location)
if _, exists := existingCommentsForType[id]; exists {
// Location review comment for this type exist already and not changed
log.Debug("Review comment still relevant", id)
delete(existingCommentsForType, id)
} else {
log.Debug("Adding new review comment", id)
Expand Down Expand Up @@ -303,7 +308,7 @@ func generateReviewCommentContent(id string, commentType ReviewCommentType, loca
)
}

content += outputwriter.ReviewCommentGeneratedByFrogbot
content += writer.ReviewFooter()
return
}

Expand Down
11 changes: 0 additions & 11 deletions utils/utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -354,14 +354,3 @@ func GetSortedPullRequestComments(client vcsclient.VcsClient, repoOwner, repoNam
})
return pullRequestsComments, nil
}

func GetJasMarkdownDescription(scanType xrayutils.JasScanType, location *sarif.Location, severity, content string) string {
dataColumnHeader := "Finding"
if scanType == xrayutils.Secrets {
dataColumnHeader = "Secret"
}
headerRow := fmt.Sprintf("| Severity | File | Line:Column | %s |\n", dataColumnHeader)
separatorRow := "| :---: | :---: | :---: | :---: |\n"
tableHeader := headerRow + separatorRow
return tableHeader + fmt.Sprintf("| %s | %s | %s | %s |", severity, xrayutils.GetLocationFileName(location), xrayutils.GetStartLocationInFile(location), content)
}

0 comments on commit 77a72d8

Please sign in to comment.