diff --git a/scanpullrequest/scanpullrequest.go b/scanpullrequest/scanpullrequest.go index a869fdbd4..1fcc49829 100644 --- a/scanpullrequest/scanpullrequest.go +++ b/scanpullrequest/scanpullrequest.go @@ -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 } diff --git a/utils/outputwriter/outputwriter.go b/utils/outputwriter/outputwriter.go index 9c3d79876..dc886d8e0 100644 --- a/utils/outputwriter/outputwriter.go +++ b/utils/outputwriter/outputwriter.go @@ -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| :---------------------: | :----------------------------------: | :-----------------------------------: | :---------------------------------: |" @@ -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 @@ -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 { @@ -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 } @@ -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" diff --git a/utils/outputwriter/simplifiedoutput.go b/utils/outputwriter/simplifiedoutput.go index 013afc6af..acfe06f74 100644 --- a/utils/outputwriter/simplifiedoutput.go +++ b/utils/outputwriter/simplifiedoutput.go @@ -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 + " |" @@ -129,7 +129,7 @@ Finding: %s %s `, - smo.FormattedSeverity(severity, "Applicable"), + smo.FormattedSeverity(severity, "Applicable", false), finding, fullDetails, cveDetails) @@ -146,7 +146,7 @@ Finding: %s %s `, - smo.FormattedSeverity(severity, "Applicable"), + smo.FormattedSeverity(severity, "Applicable", false), finding, fullDetails) } @@ -169,7 +169,7 @@ Finding: %s #### Vulnerable data flows `, - smo.FormattedSeverity(severity, "Applicable"), + smo.FormattedSeverity(severity, "Applicable", false), finding, fullDetails, )) @@ -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 } diff --git a/utils/outputwriter/standardoutput.go b/utils/outputwriter/standardoutput.go index 64a94772a..2213c7ae2 100644 --- a/utils/outputwriter/standardoutput.go +++ b/utils/outputwriter/standardoutput.go @@ -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 + " | " } @@ -130,15 +130,14 @@ func (so *StandardOutput) ApplicableCveReviewContent(severity, finding, fullDeta return fmt.Sprintf(` ### 📦🔍 Applicable dependency CVE Vulnerability -Severity: %s - -Finding: %s +%s #### 👇 Details
Description
+ %s
@@ -146,6 +145,7 @@ Finding: %s
CVE details
+ %s
@@ -153,13 +153,13 @@ Finding: %s
Remediation
+ %s
`, - so.FormattedSeverity(severity, "Applicable"), - finding, + GetJasMarkdownDescription(so.FormattedSeverity(severity, "Applicable", false),finding), fullDetails, cveDetails, remediation) @@ -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
Full description
+ %s
`, - 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 +
-Finding: %s +### 🎯 Static Application Security Testing (SAST) Vulnerability + +%s #### 👇 Details
Full description
+ %s
`, - 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(`
- %d. Vulnerable data flow analysis result + Vulnerable data flow analysis result
`, 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), )) @@ -278,12 +277,27 @@ func (so *StandardOutput) Footer() string { `, CommentGeneratedByFrogbot) } +func (so *StandardOutput) ReviewFooter() string { + return fmt.Sprintf(` +--- +
+ +%s + +
+`, ReviewCommentGeneratedByFrogbot) +} + func (so *StandardOutput) Separator() string { return "

" } -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 { @@ -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() +} \ No newline at end of file diff --git a/utils/reviewcomment.go b/utils/reviewcomment.go index 9b1ba51bb..9033fef80 100644 --- a/utils/reviewcomment.go +++ b/utils/reviewcomment.go @@ -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 @@ -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) @@ -303,7 +308,7 @@ func generateReviewCommentContent(id string, commentType ReviewCommentType, loca ) } - content += outputwriter.ReviewCommentGeneratedByFrogbot + content += writer.ReviewFooter() return } diff --git a/utils/utils.go b/utils/utils.go index 9e7dc81b2..3c77beae7 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -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) -}