diff --git a/go.mod b/go.mod index 34a74fdcd..8bc18f2ff 100644 --- a/go.mod +++ b/go.mod @@ -121,11 +121,8 @@ require ( ) // eranturgeman:jas-violations-support -replace github.com/jfrog/jfrog-cli-security => ../jfrog-cli-security - -// replace github.com/jfrog/jfrog-cli-security => github.com/eranturgeman/jfrog-cli-security v0.0.0-20241204143029-e901cd468c75 - -// replace github.com/jfrog/jfrog-cli-security => github.com/eranturgeman/jfrog-cli-security v0.0.0-20241124185605-a69b532152fc +// replace github.com/jfrog/jfrog-cli-security => ../jfrog-cli-security +replace github.com/jfrog/jfrog-cli-security => github.com/eranturgeman/jfrog-cli-security v0.0.0-20241211070414-848d30bb3042 // replace github.com/jfrog/jfrog-cli-core/v2 => github.com/jfrog/jfrog-cli-core/v2 dev diff --git a/go.sum b/go.sum index 60660ac4b..e60b7a09c 100644 --- a/go.sum +++ b/go.sum @@ -59,6 +59,8 @@ github.com/elazarl/goproxy v0.0.0-20230808193330-2592e75ae04a h1:mATvB/9r/3gvcej github.com/elazarl/goproxy v0.0.0-20230808193330-2592e75ae04a/go.mod h1:Ro8st/ElPeALwNFlcTpWmkr6IoMFfkjXAvTHpevnDsM= github.com/emirpasic/gods v1.18.1 h1:FXtiHYKDGKCW2KzwZKx0iC0PQmdlorYgdFG9jPXJ1Bc= github.com/emirpasic/gods v1.18.1/go.mod h1:8tpGGwCnJ5H4r6BWwaV6OrWmMoPhUl5jm/FMNAnJvWQ= +github.com/eranturgeman/jfrog-cli-security v0.0.0-20241211070414-848d30bb3042 h1:0jz+yrWt4B5oS1jGMU78qgywpOr+Is4cLkGCZjlzpQ8= +github.com/eranturgeman/jfrog-cli-security v0.0.0-20241211070414-848d30bb3042/go.mod h1:c4GycFVqXqYOjI/1FdJ1lUf//kqQBGEzds1iSn73OaM= github.com/fatih/color v1.16.0 h1:zmkK9Ngbjj+K0yRhTVONQh1p/HknKYSlNT+vZCzyokM= github.com/fatih/color v1.16.0/go.mod h1:fL2Sau1YI5c0pdGEVCbKQbLXB6edEj1ZgiY4NijnWvE= github.com/forPelevin/gomoji v1.2.0 h1:9k4WVSSkE1ARO/BWywxgEUBvR/jMnao6EZzrql5nxJ8= diff --git a/scanpullrequest/scanpullrequest.go b/scanpullrequest/scanpullrequest.go index e6a21fc35..4b7baf784 100644 --- a/scanpullrequest/scanpullrequest.go +++ b/scanpullrequest/scanpullrequest.go @@ -41,7 +41,7 @@ func (cmd *ScanPullRequestCmd) Run(configAggregator utils.RepoAggregator, client return } } - repoConfig.OutputWriter.SetHasInternetConnection( /*false*/ frogbotRepoConnection.IsConnected()) + repoConfig.OutputWriter.SetHasInternetConnection(frogbotRepoConnection.IsConnected()) if repoConfig.PullRequestDetails, err = client.GetPullRequestByID(context.Background(), repoConfig.RepoOwner, repoConfig.RepoName, int(repoConfig.PullRequestDetails.ID)); err != nil { return } @@ -310,7 +310,7 @@ func checkoutToCommitAtTempWorkingDir(scanDetails *utils.ScanDetails, commitHash } func getAllIssues(cmdResults *results.SecurityCommandResults, allowedLicenses []string, includeVulnerabilities, hasViolationContext bool) (*issues.ScansIssuesCollection, error) { - log.Info("Frogbot is configured to show all vulnerabilities") + log.Info("Frogbot is configured to show all issues") simpleJsonResults, err := conversion.NewCommandResultsConvertor(conversion.ResultConvertParams{ IncludeVulnerabilities: includeVulnerabilities, HasViolationContext: hasViolationContext, diff --git a/utils/comment.go b/utils/comment.go index 3c8b01380..9fe183697 100644 --- a/utils/comment.go +++ b/utils/comment.go @@ -32,6 +32,7 @@ const ( commentRemovalErrorMsg = "An error occurred while attempting to remove older Frogbot pull request comments:" ) +// In Scan PR, if there is an error, a comment will be added to the PR with the error message. func HandlePullRequestErrorComment(issues *issues.ScansIssuesCollection, repo *Repository, client vcsclient.VcsClient, pullRequestID int, scanError error) (err error) { if issues == nil { log.Debug("Can't generate error comment without issues collection") @@ -46,6 +47,7 @@ func HandlePullRequestErrorComment(issues *issues.ScansIssuesCollection, repo *R return } +// In Scan PR, if there are no issues, comments will be added to the PR with a message that there are no issues. func HandlePullRequestCommentsAfterScan(issues *issues.ScansIssuesCollection, repo *Repository, client vcsclient.VcsClient, pullRequestID int) (err error) { if !repo.Params.AvoidPreviousPrCommentsDeletion { // The removal of comments may fail for various reasons, @@ -91,7 +93,7 @@ func DeleteExistingPullRequestComments(repository *Repository, client vcsclient. "failed to get comments. the following details were used in order to fetch the comments: <%s/%s> pull request #%d. the error received: %s", repository.RepoOwner, repository.RepoName, int(repository.PullRequestDetails.ID), err.Error()) } - commentsToDelete := getFrogbotComments(repository.OutputWriter, comments) + commentsToDelete := getFrogbotComments(comments) // Delete if len(commentsToDelete) > 0 { for _, commentToDelete := range commentsToDelete { @@ -178,7 +180,7 @@ func addReviewComments(repo *Repository, pullRequestID int, client vcsclient.Vcs log.Debug("creating a review comment for", comment.Type, comment.Location.File, comment.Location.StartLine, comment.Location.StartColumn) if e := client.AddPullRequestReviewComments(context.Background(), repo.RepoOwner, repo.RepoName, pullRequestID, comment.CommentInfo); e != nil { log.Debug("couldn't add pull request review comment, fallback to regular comment: " + e.Error()) - if err = client.AddPullRequestComment(context.Background(), repo.RepoOwner, repo.RepoName, outputwriter.GetFallbackReviewCommentContent(comment.CommentInfo.Content, comment.Location, repo.OutputWriter), pullRequestID); err != nil { + if err = client.AddPullRequestComment(context.Background(), repo.RepoOwner, repo.RepoName, outputwriter.GetFallbackReviewCommentContent(comment.CommentInfo.Content, comment.Location), pullRequestID); err != nil { err = errors.New("couldn't add pull request comment, fallback to comment: " + err.Error()) return } @@ -197,7 +199,7 @@ func DeleteExistingPullRequestReviewComments(repo *Repository, pullRequestID int } // Delete old review comments if len(existingComments) > 0 { - if err = client.DeletePullRequestReviewComments(context.Background(), repo.RepoOwner, repo.RepoName, pullRequestID, getFrogbotComments(repo.OutputWriter, existingComments)...); err != nil { + if err = client.DeletePullRequestReviewComments(context.Background(), repo.RepoOwner, repo.RepoName, pullRequestID, getFrogbotComments(existingComments)...); err != nil { err = errors.New("couldn't delete pull request review comment: " + err.Error()) return } @@ -205,7 +207,7 @@ func DeleteExistingPullRequestReviewComments(repo *Repository, pullRequestID int return } -func getFrogbotComments(writer outputwriter.OutputWriter, existingComments []vcsclient.CommentInfo) (reviewComments []vcsclient.CommentInfo) { +func getFrogbotComments(existingComments []vcsclient.CommentInfo) (reviewComments []vcsclient.CommentInfo) { for _, comment := range existingComments { if outputwriter.IsFrogbotComment(comment.Content) { log.Debug("Deleting comment id:", comment.ID) @@ -278,26 +280,6 @@ func groupSimilarJasIssues(issues []formats.SourceCodeRow) (groupedIssues []simi return } -// // We group issues by their watches, so we can add all the watches to the same comment. -// func groupSimilarIssues(issues []formats.SourceCodeRow) (groupedIssues []formats.SourceCodeRow, issuesWatches map[string][]formats.ViolationContext) { -// issuesWatches = make(map[string][]formats.ViolationContext) -// for _, issue := range issues { -// if issue.Watch == "" { -// // no violation context, just add to the list -// groupedIssues = append(groupedIssues, issue) -// continue -// } -// id := getSourceCodeRowId(issue) -// if watches, ok := issuesWatches[id]; ok { -// issuesWatches[id] = append(watches, issue.ViolationContext) -// continue -// } -// groupedIssues = append(groupedIssues, issue) -// issuesWatches[id] = []formats.ViolationContext{issue.ViolationContext} -// } -// return groupedIssues, issuesWatches -// } - // We show different comments for each location and rule ID. (we group similar issues/violations to the same comment) func getSourceCodeRowId(issue formats.SourceCodeRow) string { return issue.RuleId + issue.Location.ToString() @@ -321,18 +303,6 @@ func generateApplicabilityReviewContent(issue issues.ApplicableEvidences, writer return outputwriter.GenerateReviewCommentContent(outputwriter.ApplicableCveReviewContent(issue, writer), writer) } -// func generateSourceCodeVulnerabilityReviewContent(commentType ReviewCommentType, issue formats.SourceCodeRow, writer outputwriter.OutputWriter) (content string) { -// switch commentType { -// case IacComment: -// return outputwriter.GenerateReviewCommentContent(outputwriter.IacReviewContent(issue, writer), writer) -// case SastComment: -// return outputwriter.GenerateReviewCommentContent(outputwriter.SastReviewContent(issue, writer), writer) -// case SecretComment: -// return outputwriter.GenerateReviewCommentContent(outputwriter.SecretReviewContent(issue, writer), writer) -// } -// return -// } - func generateSourceCodeReviewContent(commentType ReviewCommentType, violation bool, writer outputwriter.OutputWriter, similarIssues ...formats.SourceCodeRow) (content string) { switch commentType { case IacComment: diff --git a/utils/comment_test.go b/utils/comment_test.go index 79790c3f6..c0f6a0fe0 100644 --- a/utils/comment_test.go +++ b/utils/comment_test.go @@ -12,7 +12,6 @@ import ( ) func TestGetFrogbotReviewComments(t *testing.T) { - writer := &outputwriter.StandardOutput{} testCases := []struct { name string existingComments []vcsclient.CommentInfo @@ -44,7 +43,7 @@ func TestGetFrogbotReviewComments(t *testing.T) { } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - output := getFrogbotComments(writer, tc.existingComments) + output := getFrogbotComments(tc.existingComments) assert.ElementsMatch(t, tc.expectedOutput, output) }) } @@ -205,8 +204,6 @@ func TestGroupSimilarJasIssues(t *testing.T) { func TestGetNewReviewComments(t *testing.T) { writer := &outputwriter.StandardOutput{} - - // repo := &Repository{OutputWriter: &outputwriter.StandardOutput{}} testCases := []struct { name string generateSecretsComments bool diff --git a/utils/issues/issuescollection.go b/utils/issues/issuescollection.go index 85296ebdc..faacc6de8 100644 --- a/utils/issues/issuescollection.go +++ b/utils/issues/issuescollection.go @@ -8,7 +8,6 @@ import ( "github.com/jfrog/jfrog-cli-security/utils/severityutils" ) -// TODO: after refactor, move this to security-cli as a new formats or remove this and use the existing formats // Group issues by scan type type ScansIssuesCollection struct { formats.ScanStatus @@ -264,60 +263,6 @@ func (ic *ScansIssuesCollection) GetApplicableEvidences() (evidences []Applicabl evidences = append(evidences, evidence) } return - - // issueIdToApplicableInfo := map[string]formats.Applicability{} - // issueIdToIssue := map[string]formats.VulnerabilityOrViolationRow{} - // // Collect evidences from Violations - // for _, securityViolation := range ic.ScaViolations { - // issueId := results.GetIssueIdentifier(securityViolation.Cves, securityViolation.IssueId, "-") - // if _, exists := issueIdToIssue[issueId]; exists { - // // No need to add the same issue twice - // continue - // } - // for _, cve := range securityViolation.Cves { - // if cve.Applicability != nil && cve.Applicability.Status == jasutils.Applicable.String() { - // // We only want applicable issues - // issueIdToIssue[issueId] = securityViolation - // issueIdToApplicableInfo[issueId] = *cve.Applicability - // } - // } - // } - // // Collect evidences from Vulnerabilities - // for _, vulnerability := range ic.ScaVulnerabilities { - // issueId := results.GetIssueIdentifier(vulnerability.Cves, vulnerability.IssueId, "-") - // if _, exists := issueIdToIssue[issueId]; exists { - // // No need to add the same issue twice - // continue - // } - // for _, cve := range vulnerability.Cves { - // if cve.Applicability != nil && cve.Applicability.Status == jasutils.Applicable.String() { - // // We only want applicable issues - // issueIdToIssue[issueId] = vulnerability - // issueIdToApplicableInfo[issueId] = *cve.Applicability - // } - // } - // } - // // Create ApplicableEvidences from collected data - // for issueId := range maps.Keys(issueIdToApplicableInfo) { - // issue := issueIdToIssue[issueId] - // applicableInfo := issueIdToApplicableInfo[issueId] - // remediation := "" - // if issue.JfrogResearchInformation != nil { - // remediation = issue.JfrogResearchInformation.Remediation - // } - // for _, evidence := range applicableInfo.Evidence { - // evidences = append(evidences, ApplicableEvidences{ - // Evidence: evidence, - // Severity: issue.Severity, - // ScannerDescription: applicableInfo.ScannerDescription, - // IssueId: results.GetIssueIdentifier(issue.Cves, issue.IssueId, ","), - // CveSummary: issue.Summary, - // ImpactedDependency: results.GetDependencyId(issue.ImpactedDependencyName, issue.ImpactedDependencyVersion), - // Remediation: remediation, - // }) - // } - // } - // return } // Violations @@ -343,68 +288,3 @@ func (ic *ScansIssuesCollection) GetTotalVulnerabilities(includeSecrets bool) in } return total } - -// func (ic *ScansIssuesCollection) GetTotal() - -// --------------------------------------- - -// func (ic *ScansIssuesCollection) GetScaIssues() (unique []formats.VulnerabilityOrViolationRow) { -// return append(ic.ScaVulnerabilities, ic.ScaViolations...) -// } - -// func (ic *ScansIssuesCollection) GetUniqueIacIssues() (unique []formats.SourceCodeRow) { -// return getUniqueJasIssues(ic.IacVulnerabilities, ic.IacViolations) -// } - -// func (ic *ScansIssuesCollection) GetUniqueSecretsIssues() (unique []formats.SourceCodeRow) { -// return getUniqueJasIssues(ic.SecretsVulnerabilities, ic.SecretsViolations) -// } - -// func (ic *ScansIssuesCollection) GetUniqueSastIssues() (unique []formats.SourceCodeRow) { -// return getUniqueJasIssues(ic.SastVulnerabilities, ic.SastViolations) -// } - -// func getUniqueJasIssues(vulnerabilities, violations []formats.SourceCodeRow) (unique []formats.SourceCodeRow) { -// parsedIssues := datastructures.MakeSet[string]() -// for _, violation := range violations { -// issueId := violation.Location.ToString() + "|" + violation.Finding -// if parsedIssues.Exists(issueId) { -// continue -// } -// parsedIssues.Add(issueId) -// unique = append(unique, violation) -// } -// for _, vulnerability := range vulnerabilities { -// issueId := vulnerability.Location.ToString() + "|" + vulnerability.Finding -// if parsedIssues.Exists(issueId) { -// continue -// } -// parsedIssues.Add(issueId) -// unique = append(unique, vulnerability) -// } -// return -// } - -// func (ic *ScansIssuesCollection) LicensesViolationsExists() bool { -// return len(ic.LicensesViolations) > 0 -// } - -// func (ic *ScansIssuesCollection) PresentableIssuesExists() bool { -// return ic.ScaIssuesExists() || ic.IacIssuesExists() || ic.LicensesViolationsExists() || ic.SastIssuesExists() -// } - -// func (ic *ScansIssuesCollection) ViolationsExists() bool { -// return len(ic.ScaViolations) > 0 || len(ic.IacViolations) > 0 || len(ic.SecretsViolations) > 0 || len(ic.SastViolations) > 0 || len(ic.LicensesViolations) > 0 -// } - -// func (ic *ScansIssuesCollection) CountIssuesCollectionFindings() int { -// count := 0 - -// count += len(ic.GetScaIssues()) -// count += len(ic.GetUniqueIacIssues()) -// count += len(ic.GetUniqueSecretsIssues()) -// count += len(ic.GetUniqueSastIssues()) -// count += len(ic.LicensesViolations) - -// return count -// } diff --git a/utils/outputwriter/markdowntable.go b/utils/outputwriter/markdowntable.go index b254f39bb..f4a14b27f 100644 --- a/utils/outputwriter/markdowntable.go +++ b/utils/outputwriter/markdowntable.go @@ -6,12 +6,6 @@ import ( ) const ( - - // tableRowFirstColumnSeparator = "| :---------------------: |" - // tableRowColumnSeparator = " :-----------------------------------: |" - // cellFirstCellPlaceholder = "| %s |" - // cellCellPlaceholder = " %s |" - cellDefaultValue = "-" firstCellPlaceholder = "| %s |" diff --git a/utils/outputwriter/outputcontent.go b/utils/outputwriter/outputcontent.go index 40efb186c..68463552b 100644 --- a/utils/outputwriter/outputcontent.go +++ b/utils/outputwriter/outputcontent.go @@ -63,7 +63,7 @@ func GenerateReviewCommentContent(content string, writer OutputWriter) string { } // When can't create review comment, create a fallback comment by adding the location description to the content as a prefix -func GetFallbackReviewCommentContent(content string, location formats.Location, writer OutputWriter) string { +func GetFallbackReviewCommentContent(content string, location formats.Location) string { var contentBuilder strings.Builder contentBuilder.WriteString(MarkdownComment(ReviewCommentId)) WriteContent(&contentBuilder, getFallbackCommentLocationDescription(location), content) @@ -293,7 +293,7 @@ func PolicyViolationsContent(issues issues.ScansIssuesCollection, writer OutputW return ConvertContentToComments(policyViolationContent, writer, getDecoratorWithPolicyViolationTitle(writer)) } -func getDecoratorWithPolicyViolationTitle(writer OutputWriter) func(int, string) string { +func getDecoratorWithPolicyViolationTitle(writer OutputWriter) CommentDecorator { return func(commentCount int, content string) string { contentBuilder := strings.Builder{} // Decorate each part of the split content with a title as prefix and return the content @@ -314,7 +314,7 @@ func getSecurityViolationsContent(issues issues.ScansIssuesCollection, writer Ou return ConvertContentToComments(content, writer, getDecoratorWithSecurityViolationTitle(writer)) } -func getDecoratorWithSecurityViolationTitle(writer OutputWriter) func(int, string) string { +func getDecoratorWithSecurityViolationTitle(writer OutputWriter) CommentDecorator { return func(commentCount int, content string) string { contentBuilder := strings.Builder{} // Decorate each part of the split content with a title as prefix and return the content @@ -364,7 +364,7 @@ func getLicenseViolationsContent(issues issues.ScansIssuesCollection, writer Out return ConvertContentToComments(content, writer, getDecoratorWithLicenseViolationTitle(writer)) } -func getDecoratorWithLicenseViolationTitle(writer OutputWriter) func(int, string) string { +func getDecoratorWithLicenseViolationTitle(writer OutputWriter) CommentDecorator { return func(commentCount int, content string) string { contentBuilder := strings.Builder{} // Decorate each part of the split content with a title as prefix and return the content @@ -450,7 +450,7 @@ func GetVulnerabilitiesContent(vulnerabilities []formats.VulnerabilityOrViolatio return ConvertContentToComments(content, writer, getDecoratorWithScaVulnerabilitiesTitle(writer)) } -func getDecoratorWithScaVulnerabilitiesTitle(writer OutputWriter) func(int, string) string { +func getDecoratorWithScaVulnerabilitiesTitle(writer OutputWriter) CommentDecorator { return func(commentCount int, content string) string { contentBuilder := strings.Builder{} // Decorate each part of the split content with a title as prefix and return the content diff --git a/utils/outputwriter/outputcontent_test.go b/utils/outputwriter/outputcontent_test.go index f56954c21..d2a81479f 100644 --- a/utils/outputwriter/outputcontent_test.go +++ b/utils/outputwriter/outputcontent_test.go @@ -810,7 +810,7 @@ func TestGenerateReviewComment(t *testing.T) { expectedOutput := GetExpectedTestOutput(t, test) output := GenerateReviewCommentContent(content, test.writer) if tc.location != nil { - output = GetFallbackReviewCommentContent(content, *tc.location, test.writer) + output = GetFallbackReviewCommentContent(content, *tc.location) } assert.Equal(t, expectedOutput, output) })