Skip to content

Commit

Permalink
start clean
Browse files Browse the repository at this point in the history
  • Loading branch information
@attiasas
attiasas committed Dec 11, 2024
1 parent d37e974 commit 3e8b0c6
Show file tree
Hide file tree
Showing 9 changed files with 19 additions and 179 deletions.
7 changes: 2 additions & 5 deletions go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 2 additions & 0 deletions go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
4 changes: 2 additions & 2 deletions scanpullrequest/scanpullrequest.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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,
Expand Down
42 changes: 6 additions & 36 deletions utils/comment.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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,
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
Expand All @@ -197,15 +199,15 @@ 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
}
}
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)
Expand Down Expand Up @@ -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()
Expand All @@ -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:
Expand Down
5 changes: 1 addition & 4 deletions utils/comment_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (
)

func TestGetFrogbotReviewComments(t *testing.T) {
writer := &outputwriter.StandardOutput{}
testCases := []struct {
name string
existingComments []vcsclient.CommentInfo
Expand Down Expand Up @@ -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)
})
}
Expand Down Expand Up @@ -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
Expand Down
120 changes: 0 additions & 120 deletions utils/issues/issuescollection.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
// }
6 changes: 0 additions & 6 deletions utils/outputwriter/markdowntable.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,6 @@ import (
)

const (

// tableRowFirstColumnSeparator = "| :---------------------: |"
// tableRowColumnSeparator = " :-----------------------------------: |"
// cellFirstCellPlaceholder = "| %s |"
// cellCellPlaceholder = " %s |"

cellDefaultValue = "-"

firstCellPlaceholder = "| %s |"
Expand Down
10 changes: 5 additions & 5 deletions utils/outputwriter/outputcontent.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion utils/outputwriter/outputcontent_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
Expand Down

0 comments on commit 3e8b0c6

Please sign in to comment.