Skip to content

Commit

Permalink
Remove old comments in pull requests (#400)
Browse files Browse the repository at this point in the history
  • Loading branch information
@omerzi
omerzi authored Jul 30, 2023
1 parent 25d5521 commit 0720844
Show file tree
Hide file tree
Showing 21 changed files with 333 additions and 133 deletions.
16 changes: 7 additions & 9 deletions commands/createfixpullrequests.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -310,11 +310,11 @@ func (cfp *CreateFixPullRequestsCmd) openFixingPullRequest(fixBranchName string,
if err = cfp.gitManager.Push(false, fixBranchName); err != nil {
return
}
scanHash, err := utils.VulnerabilityDetailsToMD5Hash(vulnDetails)
scanHash, err := utils.VulnerabilityDetailsToMD5Hash(vulnDetails.VulnerabilityOrViolationRow)
if err != nil {
return err
}
pullRequestTitle, prBody := cfp.preparePullRequestDetails(scanHash, []formats.VulnerabilityOrViolationRow{*vulnDetails.VulnerabilityOrViolationRow})
pullRequestTitle, prBody := cfp.preparePullRequestDetails(scanHash, []formats.VulnerabilityOrViolationRow{vulnDetails.VulnerabilityOrViolationRow})
log.Debug("Creating Pull Request form:", fixBranchName, " to:", cfp.details.Branch())
return cfp.details.Client().CreatePullRequest(context.Background(), cfp.details.RepoOwner, cfp.details.RepoName, fixBranchName, cfp.details.Branch(), pullRequestTitle, prBody)
}
Expand All @@ -329,14 +329,11 @@ func (cfp *CreateFixPullRequestsCmd) openAggregatedPullRequest(fixBranchName str
if err = cfp.gitManager.Push(true, fixBranchName); err != nil {
return
}
scanHash, err := utils.VulnerabilityDetailsToMD5Hash(vulnerabilities...)
vulnerabilityRows := utils.ExtractVunerabilitiesDetailsToRows(vulnerabilities)
scanHash, err := utils.VulnerabilityDetailsToMD5Hash(vulnerabilityRows...)
if err != nil {
return
}
var vulnerabilityRows []formats.VulnerabilityOrViolationRow
for _, vulnerability := range vulnerabilities {
vulnerabilityRows = append(vulnerabilityRows, *vulnerability.VulnerabilityOrViolationRow)
}
pullRequestTitle, prBody := cfp.preparePullRequestDetails(scanHash, vulnerabilityRows)
if pullRequestInfo == nil {
log.Info("Creating Pull Request from:", fixBranchName, "to:", cfp.details.Branch())
Expand Down Expand Up @@ -454,7 +451,7 @@ func (cfp *CreateFixPullRequestsCmd) addVulnerabilityToFixVersionsMap(vulnerabil
return err
}
// First appearance of a version that fixes the current impacted package
newVulnDetails := utils.NewVulnerabilityDetails(vulnerability, vulnFixVersion)
newVulnDetails := utils.NewVulnerabilityDetails(*vulnerability, vulnFixVersion)
newVulnDetails.SetIsDirectDependency(isDirectDependency)
vulnerabilitiesMap[vulnerability.ImpactedDependencyName] = newVulnDetails
}
Expand Down Expand Up @@ -561,7 +558,8 @@ func (cfp *CreateFixPullRequestsCmd) isUpdateRequired(fixedVulnerabilities []*ut
}
log.Info("Aggregated pull request already exists, verifying if update is needed...")
log.Debug("Comparing current scan results to existing", prInfo.Target.Name, "scan results")
currentScanHash, err := utils.VulnerabilityDetailsToMD5Hash(fixedVulnerabilities...)
fixedVulnerabilitiesRows := utils.ExtractVunerabilitiesDetailsToRows(fixedVulnerabilities)
currentScanHash, err := utils.VulnerabilityDetailsToMD5Hash(fixedVulnerabilitiesRows...)
if err != nil {
return
}
Expand Down
8 changes: 4 additions & 4 deletions commands/createfixpullrequests_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -520,7 +520,7 @@ func TestUpdatePackageToFixedVersion(t *testing.T) {
var testScan CreateFixPullRequestsCmd
for tech, buildToolsDependencies := range utils.BuildToolsDependenciesMap {
for _, impactedDependency := range buildToolsDependencies {
vulnDetails := &utils.VulnerabilityDetails{SuggestedFixedVersion: "3.3.3", VulnerabilityOrViolationRow: &formats.VulnerabilityOrViolationRow{Technology: tech, ImpactedDependencyName: impactedDependency}, IsDirectDependency: true}
vulnDetails := &utils.VulnerabilityDetails{SuggestedFixedVersion: "3.3.3", VulnerabilityOrViolationRow: formats.VulnerabilityOrViolationRow{Technology: tech, ImpactedDependencyName: impactedDependency}, IsDirectDependency: true}
err := testScan.updatePackageToFixedVersion(vulnDetails)
assert.Error(t, err, "Expected error to occur")
assert.IsType(t, &utils.ErrUnsupportedFix{}, err, "Expected unsupported fix error")
Expand Down Expand Up @@ -634,7 +634,7 @@ func TestPreparePullRequestDetails(t *testing.T) {
Cves: []formats.CveRow{{Id: "CVE-2022-1234"}},
},
}
expectedPrBody := "[![](https://raw.githubusercontent.com/jfrog/frogbot/master/resources/v2/vulnerabilitiesFixBannerPR.png)](https://github.com/jfrog/frogbot#readme)\n\n## 📦 Vulnerable Dependencies \n\n### ✍️ Summary\n\n<div align=\"center\">\n\n\n| SEVERITY | DIRECT DEPENDENCIES | IMPACTED DEPENDENCY | FIXED VERSIONS |\n| :---------------------: | :----------------------------------: | :-----------------------------------: | :---------------------------------: | \n| ![](https://raw.githubusercontent.com/jfrog/frogbot/master/resources/v2/applicableHighSeverity.png)<br> High | | package1:1.0.0 | 1.0.0<br><br>2.0.0 |\n\n</div>\n\n## 👇 Details\n\n\n\n\n- **Severity** 🔥 High\n- **Package Name:** package1\n- **Current Version:** 1.0.0\n- **Fixed Versions:** 1.0.0,2.0.0\n- **CVE:** CVE-2022-1234\n\n**Description:**\n\nsummary\n\n\n\n\n---\n\n<div align=\"center\">\n\n**Frogbot** also supports **Contextual Analysis**. This feature is included as part of the [JFrog Advanced Security](https://jfrog.com/xray/) package, which isn't enabled on your system.\n\n</div>\n\n<div align=\"center\">\n\n[JFrog Frogbot](https://github.com/jfrog/frogbot#readme)\n\n</div>\n"
expectedPrBody := "<div align='center'>\n\n[![](https://raw.githubusercontent.com/jfrog/frogbot/master/resources/v2/vulnerabilitiesFixBannerPR.png)](https://github.com/jfrog/frogbot#readme)\n\n</div>\n\n\n\n## 📦 Vulnerable Dependencies \n\n### ✍️ Summary\n\n<div align=\"center\">\n\n\n| SEVERITY | DIRECT DEPENDENCIES | IMPACTED DEPENDENCY | FIXED VERSIONS |\n| :---------------------: | :----------------------------------: | :-----------------------------------: | :---------------------------------: | \n| ![](https://raw.githubusercontent.com/jfrog/frogbot/master/resources/v2/applicableHighSeverity.png)<br> High | | package1:1.0.0 | 1.0.0<br><br>2.0.0 |\n\n</div>\n\n## 👇 Details\n\n\n\n\n- **Severity** 🔥 High\n- **Package Name:** package1\n- **Current Version:** 1.0.0\n- **Fixed Versions:** 1.0.0,2.0.0\n- **CVE:** CVE-2022-1234\n\n**Description:**\n\nsummary\n\n\n\n\n---\n\n<div align=\"center\">\n\n**Frogbot** also supports **Contextual Analysis, Secret Detection and IaC Vulnerabilities Scanning**. This features are included as part of the [JFrog Advanced Security](https://jfrog.com/xray/) package, which isn't enabled on your system.\n\n</div>\n\n<div align=\"center\">\n\n[JFrog Frogbot](https://github.com/jfrog/frogbot#readme)\n\n</div>\n"
prTitle, prBody := cfp.preparePullRequestDetails("hash", vulnerabilities)
assert.Equal(t, "[🐸 Frogbot] Update version of package1 to 1.0.0", prTitle)
assert.Equal(t, expectedPrBody, prBody)
Expand All @@ -647,12 +647,12 @@ func TestPreparePullRequestDetails(t *testing.T) {
Cves: []formats.CveRow{{Id: "CVE-2022-4321"}},
})
cfp.aggregateFixes = true
expectedPrBody = "[![](https://raw.githubusercontent.com/jfrog/frogbot/master/resources/v2/vulnerabilitiesFixBannerPR.png)](https://github.com/jfrog/frogbot#readme)\n\n## 📦 Vulnerable Dependencies \n\n### ✍️ Summary\n\n<div align=\"center\">\n\n\n| SEVERITY | DIRECT DEPENDENCIES | IMPACTED DEPENDENCY | FIXED VERSIONS |\n| :---------------------: | :----------------------------------: | :-----------------------------------: | :---------------------------------: | \n| ![](https://raw.githubusercontent.com/jfrog/frogbot/master/resources/v2/applicableHighSeverity.png)<br> High | | package1:1.0.0 | 1.0.0<br><br>2.0.0 |\n| ![](https://raw.githubusercontent.com/jfrog/frogbot/master/resources/v2/applicableCriticalSeverity.png)<br>Critical | | package2:2.0.0 | 2.0.0<br><br>3.0.0 |\n\n</div>\n\n## 👇 Details\n\n\n<details>\n<summary> <b>package1 1.0.0</b> </summary>\n<br>\n\n- **Severity** 🔥 High\n- **Package Name:** package1\n- **Current Version:** 1.0.0\n- **Fixed Versions:** 1.0.0,2.0.0\n- **CVE:** CVE-2022-1234\n\n**Description:**\n\nsummary\n\n\n\n</details>\n\n\n<details>\n<summary> <b>package2 2.0.0</b> </summary>\n<br>\n\n- **Severity** 💀 Critical\n- **Package Name:** package2\n- **Current Version:** 2.0.0\n- **Fixed Versions:** 2.0.0,3.0.0\n- **CVE:** CVE-2022-4321\n\n**Description:**\n\nsummary\n\n\n\n</details>\n\n\n---\n\n<div align=\"center\">\n\n**Frogbot** also supports **Contextual Analysis**. This feature is included as part of the [JFrog Advanced Security](https://jfrog.com/xray/) package, which isn't enabled on your system.\n\n</div>\n\n<div align=\"center\">\n\n[JFrog Frogbot](https://github.com/jfrog/frogbot#readme)\n\n</div>\n\n[comment]: <> (Checksum: hash)\n"
expectedPrBody = "<div align='center'>\n\n[![](https://raw.githubusercontent.com/jfrog/frogbot/master/resources/v2/vulnerabilitiesFixBannerPR.png)](https://github.com/jfrog/frogbot#readme)\n\n</div>\n\n\n\n## 📦 Vulnerable Dependencies \n\n### ✍️ Summary\n\n<div align=\"center\">\n\n\n| SEVERITY | DIRECT DEPENDENCIES | IMPACTED DEPENDENCY | FIXED VERSIONS |\n| :---------------------: | :----------------------------------: | :-----------------------------------: | :---------------------------------: | \n| ![](https://raw.githubusercontent.com/jfrog/frogbot/master/resources/v2/applicableHighSeverity.png)<br> High | | package1:1.0.0 | 1.0.0<br><br>2.0.0 |\n| ![](https://raw.githubusercontent.com/jfrog/frogbot/master/resources/v2/applicableCriticalSeverity.png)<br>Critical | | package2:2.0.0 | 2.0.0<br><br>3.0.0 |\n\n</div>\n\n## 👇 Details\n\n\n<details>\n<summary> <b>package1 1.0.0</b> </summary>\n<br>\n\n- **Severity** 🔥 High\n- **Package Name:** package1\n- **Current Version:** 1.0.0\n- **Fixed Versions:** 1.0.0,2.0.0\n- **CVE:** CVE-2022-1234\n\n**Description:**\n\nsummary\n\n\n\n</details>\n\n\n<details>\n<summary> <b>package2 2.0.0</b> </summary>\n<br>\n\n- **Severity** 💀 Critical\n- **Package Name:** package2\n- **Current Version:** 2.0.0\n- **Fixed Versions:** 2.0.0,3.0.0\n- **CVE:** CVE-2022-4321\n\n**Description:**\n\nsummary\n\n\n\n</details>\n\n\n---\n\n<div align=\"center\">\n\n**Frogbot** also supports **Contextual Analysis, Secret Detection and IaC Vulnerabilities Scanning**. This features are included as part of the [JFrog Advanced Security](https://jfrog.com/xray/) package, which isn't enabled on your system.\n\n</div>\n\n<div align=\"center\">\n\n[JFrog Frogbot](https://github.com/jfrog/frogbot#readme)\n\n</div>\n\n[comment]: <> (Checksum: hash)\n"
prTitle, prBody = cfp.preparePullRequestDetails("hash", vulnerabilities)
assert.Equal(t, utils.GetAggregatedPullRequestTitle(""), prTitle)
assert.Equal(t, expectedPrBody, prBody)
cfp.OutputWriter = &utils.SimplifiedOutput{}
expectedPrBody = "**🚨 This automated pull request was created by Frogbot and fixes the below:**\n\n\n---\n## 📦 Vulnerable Dependencies\n---\n\n### ✍️ Summary \n\n\n| SEVERITY | DIRECT DEPENDENCIES | IMPACTED DEPENDENCY | FIXED VERSIONS |\n| :---------------------: | :----------------------------------: | :-----------------------------------: | :---------------------------------: | \n| High | | package1:1.0.0 | 1.0.0, 2.0.0 |\n| Critical | | package2:2.0.0 | 2.0.0, 3.0.0 |\n\n---\n### 👇 Details\n---\n\n\n#### package1 1.0.0\n\n\n- **Severity** 🔥 High\n- **Package Name:** package1\n- **Current Version:** 1.0.0\n- **Fixed Versions:** 1.0.0,2.0.0\n- **CVE:** CVE-2022-1234\n\n**Description:**\n\nsummary\n\n\n\n\n#### package2 2.0.0\n\n\n- **Severity** 💀 Critical\n- **Package Name:** package2\n- **Current Version:** 2.0.0\n- **Fixed Versions:** 2.0.0,3.0.0\n- **CVE:** CVE-2022-4321\n\n**Description:**\n\nsummary\n\n\n\n\n---\n\n\n**Frogbot** also supports **Contextual Analysis**. This feature is included as part of the [JFrog Advanced Security](https://jfrog.com/xray/) package, which isn't enabled on your system.\n\n[JFrog Frogbot](https://github.com/jfrog/frogbot#readme)\n[comment]: <> (Checksum: hash)\n"
expectedPrBody = "**🚨 This automated pull request was created by Frogbot and fixes the below:**\n\n\n---\n## 📦 Vulnerable Dependencies\n---\n\n### ✍️ Summary \n\n\n| SEVERITY | DIRECT DEPENDENCIES | IMPACTED DEPENDENCY | FIXED VERSIONS |\n| :---------------------: | :----------------------------------: | :-----------------------------------: | :---------------------------------: | \n| High | | package1:1.0.0 | 1.0.0, 2.0.0 |\n| Critical | | package2:2.0.0 | 2.0.0, 3.0.0 |\n\n---\n### 👇 Details\n---\n\n\n#### package1 1.0.0\n\n\n- **Severity** 🔥 High\n- **Package Name:** package1\n- **Current Version:** 1.0.0\n- **Fixed Versions:** 1.0.0,2.0.0\n- **CVE:** CVE-2022-1234\n\n**Description:**\n\nsummary\n\n\n\n\n#### package2 2.0.0\n\n\n- **Severity** 💀 Critical\n- **Package Name:** package2\n- **Current Version:** 2.0.0\n- **Fixed Versions:** 2.0.0,3.0.0\n- **CVE:** CVE-2022-4321\n\n**Description:**\n\nsummary\n\n\n\n\n---\n\n\n**Frogbot** also supports **Contextual Analysis, Secret Detection and IaC Vulnerabilities Scanning**. This features are included as part of the [JFrog Advanced Security](https://jfrog.com/xray/) package, which isn't enabled on your system.\n\n[JFrog Frogbot](https://github.com/jfrog/frogbot#readme)\n[comment]: <> (Checksum: hash)\n"
prTitle, prBody = cfp.preparePullRequestDetails("hash", vulnerabilities)
assert.Equal(t, utils.GetAggregatedPullRequestTitle(""), prTitle)
assert.Equal(t, expectedPrBody, prBody)
Expand Down
36 changes: 31 additions & 5 deletions commands/scanpullrequest.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,25 +58,30 @@ func (cmd *ScanPullRequestCmd) Run(configAggregator utils.RepoAggregator, client
// a. Audit the dependencies of the source and the target branches.
// b. Compare the vulnerabilities found in source and target branches, and show only the new vulnerabilities added by the pull request.
// Otherwise, only the source branch is scanned and all found vulnerabilities are being displayed.
func scanPullRequest(repoConfig *utils.Repository, client vcsclient.VcsClient, pullRequestDetails vcsclient.PullRequestInfo) error {
func scanPullRequest(repo *utils.Repository, client vcsclient.VcsClient, pullRequestDetails vcsclient.PullRequestInfo) error {
log.Info("Scanning Pull Request ID:", pullRequestDetails.ID, "Source:", pullRequestDetails.Source.Name, "Target:", pullRequestDetails.Target.Name)
log.Info("-----------------------------------------------------------")
// Audit PR code
vulnerabilitiesRows, iacRows, err := auditPullRequest(repoConfig, client, pullRequestDetails)
vulnerabilitiesRows, iacRows, err := auditPullRequest(repo, client, pullRequestDetails)
if err != nil {
return err
}

// Delete previous Frogbot pull request message if exists
if err = deletePreviousPullRequestMessages(repo, client); err != nil {
return err
}

// Create a pull request message
message := createPullRequestMessage(vulnerabilitiesRows, iacRows, repoConfig.OutputWriter)
message := createPullRequestMessage(vulnerabilitiesRows, iacRows, repo.OutputWriter)

// Add comment to the pull request
if err = client.AddPullRequestComment(context.Background(), repoConfig.RepoOwner, repoConfig.RepoName, message, repoConfig.PullRequestID); err != nil {
if err = client.AddPullRequestComment(context.Background(), repo.RepoOwner, repo.RepoName, message, repo.PullRequestID); err != nil {
return errors.New("couldn't add pull request comment: " + err.Error())
}

// Fail the Frogbot task if a security issue is found and Frogbot isn't configured to avoid the failure.
if repoConfig.FailOnSecurityIssues != nil && *repoConfig.FailOnSecurityIssues && len(vulnerabilitiesRows) > 0 {
if repo.FailOnSecurityIssues != nil && *repo.FailOnSecurityIssues && len(vulnerabilitiesRows) > 0 {
err = errors.New(securityIssueFoundErr)
}
return err
Expand Down Expand Up @@ -368,3 +373,24 @@ func createPullRequestMessage(vulnerabilitiesRows []formats.VulnerabilityOrViola
}
return writer.VulnerabiltiesTitle(true) + writer.VulnerabilitiesContent(vulnerabilitiesRows) + writer.IacContent(iacRows) + writer.UntitledForJasMsg() + writer.Footer()
}

func deletePreviousPullRequestMessages(repository *utils.Repository, client vcsclient.VcsClient) error {
comments, err := utils.GetSortedPullRequestComments(client, repository.RepoOwner, repository.RepoName, repository.PullRequestID)
if err != nil {
return err
}

var commentID int
for _, comment := range comments {
if repository.OutputWriter.IsFrogbotResultComment(comment.Content) {
commentID = int(comment.ID)
break
}
}

if e := client.DeletePullRequestComment(context.Background(), repository.RepoOwner, repository.RepoName, repository.PullRequestID, commentID); e != nil {
err = errors.Join(err, e)
}

return err
}
Loading

0 comments on commit 0720844

Please sign in to comment.