From ecc49cafdc1edcb2ff614c0206e90c06adf99558 Mon Sep 17 00:00:00 2001 From: Omer Zidkoni <50792403+omerzi@users.noreply.github.com> Date: Sun, 23 Jul 2023 12:18:36 +0300 Subject: [PATCH] Fix aggregated pull requests with multi project/working dirs (#386) --- .gitignore | 3 +- action/node_modules/.package-lock.json | 2 +- commands/createfixpullrequests.go | 373 +++++++++++------- commands/createfixpullrequests_test.go | 268 +++++++++---- commands/scanandfixrepos.go | 4 +- commands/scanandfixrepos_test.go | 2 +- commands/scanpullrequest.go | 12 +- commands/scanpullrequests.go | 2 +- commands/scanpullrequests_test.go | 4 +- .../.frogbot/frogbot-config.yml | 10 + .../aggregate-multi-dir/git/COMMIT_EDITMSG | 1 + .../aggregate-multi-dir/git/HEAD | 1 + .../aggregate-multi-dir/git/config | 7 + .../aggregate-multi-dir/git/description | 1 + .../aggregate-multi-dir/git/index | Bin 0 -> 412 bytes .../aggregate-multi-dir/git/info/exclude | 6 + .../aggregate-multi-dir/git/logs/HEAD | 1 + .../git/logs/refs/heads/main | 1 + .../19/01e308e74cb275753b496e24dc28ca3dc9dc15 | Bin 0 -> 57 bytes .../20/cfb0cbabf22f1c12d626eec6e6dd834f4e43d1 | 2 + .../61/a70d8baf4dd63a6b1b01f87791cb73425caf55 | Bin 0 -> 57 bytes .../9f/70c235abae1684cf8db2cadf2730087a1773cc | Bin 0 -> 127 bytes .../ac/2801a7e744aca172a5f4155f8f8670756f27e1 | Bin 0 -> 63 bytes .../ae/099789389636d0d9196ce5cdb1de2dab9fbc91 | Bin 0 -> 208 bytes .../b1/d50a083f2239a3ff3b3ed2d1bbaea16d17c1ee | Bin 0 -> 103 bytes .../be/180a62cd192cb4ef53647c6d907074d428d724 | Bin 0 -> 63 bytes .../aggregate-multi-dir/git/refs/heads/main | 1 + .../aggregate-multi-dir/npm1/package.json | 16 + .../aggregate-multi-dir/npm2/package.json | 5 + .../.frogbot/frogbot-config.yml | 12 + .../git/COMMIT_EDITMSG | 1 + .../aggregate-multi-project/git/HEAD | 1 + .../aggregate-multi-project/git/config | 7 + .../aggregate-multi-project/git/description | 1 + .../aggregate-multi-project/git/index | Bin 0 -> 418 bytes .../aggregate-multi-project/git/info/exclude | 6 + .../aggregate-multi-project/git/logs/HEAD | 1 + .../git/logs/refs/heads/main | 1 + .../19/01e308e74cb275753b496e24dc28ca3dc9dc15 | Bin 0 -> 57 bytes .../1d/ced3ffd48839041e403eb2548fcce4f8d24a86 | Bin 0 -> 63 bytes .../29/d343bde70ca929c71d042ab0564c89d4f1b88b | Bin 0 -> 61 bytes .../65/c96377f2ad26e3ebe7179e95f73202e3ea7666 | 2 + .../ae/099789389636d0d9196ce5cdb1de2dab9fbc91 | Bin 0 -> 208 bytes .../d9/a9828971df60b05f3f550b35c96c07991aef46 | 2 + .../db/63220ddfc3c49fd55d611720d5423ac966a1d0 | 2 + .../ee/6eb069e71d2a82dd7749fa7c8c935d7d014e2d | Bin 0 -> 104 bytes .../git/refs/heads/main | 1 + .../aggregate-multi-project/npm/package.json | 16 + .../pip/requirements.txt | 2 + .../aggregate/git/COMMIT_EDITMSG | 2 +- .../createfixpullrequests/aggregate/git/index | Bin 225 -> 225 bytes .../aggregate/git/logs/HEAD | 2 +- .../aggregate/git/logs/refs/heads/main | 2 +- .../0b/0bc897474ade0656ffb59bde78495cbb0215ed | 2 - .../48/181e917e611cb6c95db9119329859b7f39cc2a | Bin 0 -> 130 bytes .../aggregate/git/refs/heads/main | 2 +- .../scanpullrequest/expectedResponse.json | 2 +- .../sub1/package-lock.json | 96 +++++ commands/utils/consts.go | 10 +- commands/utils/git.go | 19 +- commands/utils/git_test.go | 35 +- commands/utils/outputwriter.go | 50 ++- commands/utils/outputwriter_test.go | 25 ++ .../packagehandlers/commonpackagehandler.go | 2 +- .../packagehandlers/mavenpackagehandler.go | 12 +- .../packagehandlers/npmpackagehandler.go | 2 +- .../packagehandlers/packagehandlers_test.go | 42 +- .../packagehandlers/pythonpackagehandler.go | 6 +- .../packagehandlers/yarnpackagehandler.go | 2 +- commands/utils/simplifiedoutput.go | 12 +- commands/utils/standardoutput.go | 16 +- commands/utils/utils.go | 28 +- commands/utils/utils_test.go | 34 +- go.mod | 6 +- go.sum | 8 +- 75 files changed, 829 insertions(+), 365 deletions(-) create mode 100644 commands/testdata/createfixpullrequests/aggregate-multi-dir/.frogbot/frogbot-config.yml create mode 100644 commands/testdata/createfixpullrequests/aggregate-multi-dir/git/COMMIT_EDITMSG create mode 100644 commands/testdata/createfixpullrequests/aggregate-multi-dir/git/HEAD create mode 100644 commands/testdata/createfixpullrequests/aggregate-multi-dir/git/config create mode 100644 commands/testdata/createfixpullrequests/aggregate-multi-dir/git/description create mode 100644 commands/testdata/createfixpullrequests/aggregate-multi-dir/git/index create mode 100644 commands/testdata/createfixpullrequests/aggregate-multi-dir/git/info/exclude create mode 100644 commands/testdata/createfixpullrequests/aggregate-multi-dir/git/logs/HEAD create mode 100644 commands/testdata/createfixpullrequests/aggregate-multi-dir/git/logs/refs/heads/main create mode 100644 commands/testdata/createfixpullrequests/aggregate-multi-dir/git/objects/19/01e308e74cb275753b496e24dc28ca3dc9dc15 create mode 100644 commands/testdata/createfixpullrequests/aggregate-multi-dir/git/objects/20/cfb0cbabf22f1c12d626eec6e6dd834f4e43d1 create mode 100644 commands/testdata/createfixpullrequests/aggregate-multi-dir/git/objects/61/a70d8baf4dd63a6b1b01f87791cb73425caf55 create mode 100644 commands/testdata/createfixpullrequests/aggregate-multi-dir/git/objects/9f/70c235abae1684cf8db2cadf2730087a1773cc create mode 100644 commands/testdata/createfixpullrequests/aggregate-multi-dir/git/objects/ac/2801a7e744aca172a5f4155f8f8670756f27e1 create mode 100644 commands/testdata/createfixpullrequests/aggregate-multi-dir/git/objects/ae/099789389636d0d9196ce5cdb1de2dab9fbc91 create mode 100644 commands/testdata/createfixpullrequests/aggregate-multi-dir/git/objects/b1/d50a083f2239a3ff3b3ed2d1bbaea16d17c1ee create mode 100644 commands/testdata/createfixpullrequests/aggregate-multi-dir/git/objects/be/180a62cd192cb4ef53647c6d907074d428d724 create mode 100644 commands/testdata/createfixpullrequests/aggregate-multi-dir/git/refs/heads/main create mode 100644 commands/testdata/createfixpullrequests/aggregate-multi-dir/npm1/package.json create mode 100644 commands/testdata/createfixpullrequests/aggregate-multi-dir/npm2/package.json create mode 100644 commands/testdata/createfixpullrequests/aggregate-multi-project/.frogbot/frogbot-config.yml create mode 100644 commands/testdata/createfixpullrequests/aggregate-multi-project/git/COMMIT_EDITMSG create mode 100644 commands/testdata/createfixpullrequests/aggregate-multi-project/git/HEAD create mode 100644 commands/testdata/createfixpullrequests/aggregate-multi-project/git/config create mode 100644 commands/testdata/createfixpullrequests/aggregate-multi-project/git/description create mode 100644 commands/testdata/createfixpullrequests/aggregate-multi-project/git/index create mode 100644 commands/testdata/createfixpullrequests/aggregate-multi-project/git/info/exclude create mode 100644 commands/testdata/createfixpullrequests/aggregate-multi-project/git/logs/HEAD create mode 100644 commands/testdata/createfixpullrequests/aggregate-multi-project/git/logs/refs/heads/main create mode 100644 commands/testdata/createfixpullrequests/aggregate-multi-project/git/objects/19/01e308e74cb275753b496e24dc28ca3dc9dc15 create mode 100644 commands/testdata/createfixpullrequests/aggregate-multi-project/git/objects/1d/ced3ffd48839041e403eb2548fcce4f8d24a86 create mode 100644 commands/testdata/createfixpullrequests/aggregate-multi-project/git/objects/29/d343bde70ca929c71d042ab0564c89d4f1b88b create mode 100644 commands/testdata/createfixpullrequests/aggregate-multi-project/git/objects/65/c96377f2ad26e3ebe7179e95f73202e3ea7666 create mode 100644 commands/testdata/createfixpullrequests/aggregate-multi-project/git/objects/ae/099789389636d0d9196ce5cdb1de2dab9fbc91 create mode 100644 commands/testdata/createfixpullrequests/aggregate-multi-project/git/objects/d9/a9828971df60b05f3f550b35c96c07991aef46 create mode 100644 commands/testdata/createfixpullrequests/aggregate-multi-project/git/objects/db/63220ddfc3c49fd55d611720d5423ac966a1d0 create mode 100644 commands/testdata/createfixpullrequests/aggregate-multi-project/git/objects/ee/6eb069e71d2a82dd7749fa7c8c935d7d014e2d create mode 100644 commands/testdata/createfixpullrequests/aggregate-multi-project/git/refs/heads/main create mode 100644 commands/testdata/createfixpullrequests/aggregate-multi-project/npm/package.json create mode 100644 commands/testdata/createfixpullrequests/aggregate-multi-project/pip/requirements.txt delete mode 100644 commands/testdata/createfixpullrequests/aggregate/git/objects/0b/0bc897474ade0656ffb59bde78495cbb0215ed create mode 100644 commands/testdata/createfixpullrequests/aggregate/git/objects/48/181e917e611cb6c95db9119329859b7f39cc2a create mode 100644 commands/testdata/scanpullrequest/multi-dir-test-proj/sub1/package-lock.json create mode 100644 commands/utils/outputwriter_test.go diff --git a/.gitignore b/.gitignore index a9b694eee..31b36bb2f 100644 --- a/.gitignore +++ b/.gitignore @@ -6,4 +6,5 @@ coverage .idea .vscode *.iml -.DS_Store \ No newline at end of file +.DS_Store +commands/testdata/**/hooks/ \ No newline at end of file diff --git a/action/node_modules/.package-lock.json b/action/node_modules/.package-lock.json index 74526b54b..24507b693 100644 --- a/action/node_modules/.package-lock.json +++ b/action/node_modules/.package-lock.json @@ -1,7 +1,7 @@ { "name": "@jfrog/frogbot", "version": "1.0.0", - "lockfileVersion": 2, + "lockfileVersion": 3, "requires": true, "packages": { "node_modules/@actions/core": { diff --git a/commands/createfixpullrequests.go b/commands/createfixpullrequests.go index 1628b41aa..699eeab97 100644 --- a/commands/createfixpullrequests.go +++ b/commands/createfixpullrequests.go @@ -18,6 +18,7 @@ import ( "golang.org/x/exp/maps" "golang.org/x/exp/slices" "os" + "regexp" "strings" ) @@ -30,21 +31,23 @@ type CreateFixPullRequestsCmd struct { dryRunRepoPath string // The details of the current scan details *utils.ScanDetails - // The current project working directory - projectWorkingDir string + // The base working directory + baseWd string // The git client the command performs git operations with gitManager *utils.GitManager // Determines whether to open a pull request for each vulnerability fix or to aggregate all fixes into one pull request aggregateFixes bool + // The current project technology + projectTech coreutils.Technology // Stores all package manager handlers for detected issues handlers map[coreutils.Technology]packagehandlers.PackageHandler } -func (cfp *CreateFixPullRequestsCmd) Run(configAggregator utils.RepoAggregator, client vcsclient.VcsClient) error { - if err := utils.ValidateSingleRepoConfiguration(&configAggregator); err != nil { +func (cfp *CreateFixPullRequestsCmd) Run(repoAggregator utils.RepoAggregator, client vcsclient.VcsClient) error { + if err := utils.ValidateSingleRepoConfiguration(&repoAggregator); err != nil { return err } - repository := configAggregator[0] + repository := repoAggregator[0] for _, branch := range repository.Branches { err := cfp.scanAndFixRepository(&repository, branch, client) if err != nil { @@ -54,11 +57,23 @@ func (cfp *CreateFixPullRequestsCmd) Run(configAggregator utils.RepoAggregator, return nil } -func (cfp *CreateFixPullRequestsCmd) scanAndFixRepository(repository *utils.Repository, branch string, client vcsclient.VcsClient) error { - baseWd, err := os.Getwd() +func (cfp *CreateFixPullRequestsCmd) scanAndFixRepository(repository *utils.Repository, branch string, client vcsclient.VcsClient) (err error) { + cfp.baseWd, err = os.Getwd() if err != nil { - return err + return } + cfp.setCommandPrerequisites(repository, branch, client) + for i := range repository.Projects { + cfp.details.Project = &repository.Projects[i] + cfp.projectTech = "" + if err = cfp.scanAndFixProject(repository); err != nil { + return + } + } + return +} + +func (cfp *CreateFixPullRequestsCmd) setCommandPrerequisites(repository *utils.Repository, branch string, client vcsclient.VcsClient) { cfp.details = utils.NewScanDetails(client, &repository.Server, &repository.Git). SetXrayGraphScanParams(repository.Watches, repository.JFrogProjectKey). SetFailOnInstallationErrors(*repository.FailOnSecurityIssues). @@ -66,29 +81,40 @@ func (cfp *CreateFixPullRequestsCmd) scanAndFixRepository(repository *utils.Repo SetFixableOnly(repository.FixableOnly). SetMinSeverity(repository.MinSeverity) cfp.aggregateFixes = repository.Git.AggregateFixes - cfp.OutputWriter = utils.GetCompatibleOutputWriter(cfp.details.GitProvider) - for i := range repository.Projects { - cfp.details.Project = &repository.Projects[i] - projectFullPathWorkingDirs := getFullPathWorkingDirs(cfp.details.Project.WorkingDirs, baseWd) - for _, fullPathWd := range projectFullPathWorkingDirs { - scanResults, err := cfp.scan(fullPathWd) - if err != nil { - return err - } - cfp.OutputWriter.SetEntitledForJas(scanResults.ExtendedScanResults.EntitledForJas) + cfp.OutputWriter = utils.GetCompatibleOutputWriter(repository.GitProvider) +} - if !cfp.dryRun { - if err = utils.UploadScanToGitProvider(scanResults, repository, cfp.details.Branch(), cfp.details.Client()); err != nil { - log.Warn(err) - } - } - // Update the working directory to the project current working directory - cfp.projectWorkingDir = utils.GetRelativeWd(fullPathWd, baseWd) - // Fix and create PRs - if err = cfp.fixImpactedPackagesAndCreatePRs(scanResults.ExtendedScanResults, scanResults.IsMultipleRootProject); err != nil { - return err +func (cfp *CreateFixPullRequestsCmd) scanAndFixProject(repository *utils.Repository) error { + var fixNeeded bool + // A map that contains the full project paths as a keys + // The value is a map of vulnerable package names -> the details of the vulnerable packages.x + // That means we have a map of all the vulnerabilities that were found in a specific folder, along with their full details. + vulnerabilitiesByPathMap := make(map[string]map[string]*utils.VulnerabilityDetails) + projectFullPathWorkingDirs := getFullPathWorkingDirs(cfp.details.Project.WorkingDirs, cfp.baseWd) + for _, fullPathWd := range projectFullPathWorkingDirs { + scanResults, err := cfp.scan(fullPathWd) + if err != nil { + return err + } + + if !cfp.dryRun { + if err = utils.UploadScanToGitProvider(scanResults, repository, cfp.details.Branch(), cfp.details.Client()); err != nil { + log.Warn(err) } } + + // Prepare the vulnerabilities map for each working dir path + currPathVulnerabilities, err := cfp.getVulnerabilitiesMap(scanResults.ExtendedScanResults, scanResults.IsMultipleRootProject) + if err != nil { + return err + } + if len(currPathVulnerabilities) > 0 { + fixNeeded = true + } + vulnerabilitiesByPathMap[fullPathWd] = currPathVulnerabilities + } + if fixNeeded { + return cfp.fixVulnerablePackages(vulnerabilitiesByPathMap) } return nil } @@ -101,28 +127,29 @@ func (cfp *CreateFixPullRequestsCmd) scan(currentWorkingDir string) (*audit.Resu return nil, err } log.Info("Xray scan completed") + cfp.OutputWriter.SetEntitledForJas(auditResults.ExtendedScanResults.EntitledForJas) return auditResults, nil } -func (cfp *CreateFixPullRequestsCmd) fixImpactedPackagesAndCreatePRs(scanResults *xrayutils.ExtendedScanResults, isMultipleRoots bool) (err error) { +func (cfp *CreateFixPullRequestsCmd) getVulnerabilitiesMap(scanResults *xrayutils.ExtendedScanResults, isMultipleRoots bool) (map[string]*utils.VulnerabilityDetails, error) { vulnerabilitiesMap, err := cfp.createVulnerabilitiesMap(scanResults, isMultipleRoots) if err != nil { - return err + return nil, err } // Nothing to fix, return if len(vulnerabilitiesMap) == 0 { log.Info("Didn't find vulnerable dependencies with existing fix versions for", cfp.details.RepoName) - return nil } - - return cfp.fixVulnerablePackages(vulnerabilitiesMap) + return vulnerabilitiesMap, nil } -func (cfp *CreateFixPullRequestsCmd) fixVulnerablePackages(vulnerabilitiesMap map[string]*utils.VulnerabilityDetails) (err error) { - cfp.gitManager, err = utils.NewGitManager(cfp.dryRun, cfp.dryRunRepoPath, ".", "origin", cfp.details.Token, cfp.details.Username, cfp.details.Git) - if err != nil { - return +func (cfp *CreateFixPullRequestsCmd) fixVulnerablePackages(vulnerabilitiesByWdMap map[string]map[string]*utils.VulnerabilityDetails) (err error) { + if cfp.gitManager == nil { + cfp.gitManager, err = utils.NewGitManager(cfp.dryRun, cfp.dryRunRepoPath, ".", "origin", cfp.details.Token, cfp.details.Username, cfp.details.Git) + if err != nil { + return + } } clonedRepoDir, restoreBaseDir, err := cfp.cloneRepository() if err != nil { @@ -136,24 +163,74 @@ func (cfp *CreateFixPullRequestsCmd) fixVulnerablePackages(vulnerabilitiesMap ma }() if cfp.aggregateFixes { - return cfp.fixIssuesSinglePR(vulnerabilitiesMap) + return cfp.fixIssuesSinglePR(vulnerabilitiesByWdMap) } - return cfp.fixIssuesSeparatePRs(vulnerabilitiesMap) + return cfp.fixIssuesSeparatePRs(vulnerabilitiesByWdMap) } -func (cfp *CreateFixPullRequestsCmd) fixIssuesSeparatePRs(vulnerabilitiesMap map[string]*utils.VulnerabilityDetails) (err error) { - if len(vulnerabilitiesMap) == 0 { - return +func (cfp *CreateFixPullRequestsCmd) fixIssuesSeparatePRs(vulnerabilitiesMap map[string]map[string]*utils.VulnerabilityDetails) error { + var err error + for fullPath, vulnerabilities := range vulnerabilitiesMap { + if e := cfp.fixProjectVulnerabilities(fullPath, vulnerabilities); e != nil { + err = errors.Join(err, fmt.Errorf("the following errors occured while fixing vulnerabilities in %s:\n%s", fullPath, e)) + } + } + return err +} + +func (cfp *CreateFixPullRequestsCmd) fixProjectVulnerabilities(fullProjectPath string, vulnerabilities map[string]*utils.VulnerabilityDetails) (err error) { + // Update the working directory to the project's current working directory + projectWorkingDir := utils.GetRelativeWd(fullProjectPath, cfp.baseWd) + + // 'CD' into the relevant working directory + if projectWorkingDir != "" { + restoreDir, err := utils.Chdir(projectWorkingDir) + if err != nil { + return err + } + defer func() { + err = errors.Join(err, restoreDir()) + }() } - for _, vulnDetails := range vulnerabilitiesMap { - if e := cfp.fixSinglePackageAndCreatePR(vulnDetails); e != nil { + + // Fix every vulnerability in a separate pull request and branch + for _, vulnerability := range vulnerabilities { + if e := cfp.fixSinglePackageAndCreatePR(vulnerability); e != nil { err = errors.Join(err, cfp.handleUpdatePackageErrors(e)) } - // After finishing to work on the current vulnerability, we go back to the base branch to start the next vulnerability fix + + // After fixing the current vulnerability, checkout to the base branch to start fixing the next vulnerability log.Debug("Running git checkout to base branch:", cfp.details.Branch()) if e := cfp.gitManager.CheckoutLocalBranch(cfp.details.Branch()); e != nil { - return errors.Join(err, e) + err = errors.Join(err, cfp.handleUpdatePackageErrors(e)) + return + } + } + + return +} + +func (cfp *CreateFixPullRequestsCmd) fixMultiplePackages(fullProjectPath string, vulnerabilities map[string]*utils.VulnerabilityDetails) (fixedVulnerabilities []*utils.VulnerabilityDetails, err error) { + // Update the working directory to the project's current working directory + projectWorkingDir := utils.GetRelativeWd(fullProjectPath, cfp.baseWd) + + // 'CD' into the relevant working directory + if projectWorkingDir != "" { + restoreDir, err := utils.Chdir(projectWorkingDir) + if err != nil { + return nil, err + } + defer func() { + err = errors.Join(err, restoreDir()) + }() + } + for _, vulnDetails := range vulnerabilities { + if e := cfp.updatePackageToFixedVersion(vulnDetails); e != nil { + err = errors.Join(err, cfp.handleUpdatePackageErrors(e)) + continue } + fixedVulnerabilities = append(fixedVulnerabilities, vulnDetails) + log.Info(fmt.Sprintf("Updated dependency '%s' to version '%s'", vulnDetails.ImpactedDependencyName, vulnDetails.SuggestedFixedVersion)) } return } @@ -163,9 +240,8 @@ func (cfp *CreateFixPullRequestsCmd) fixIssuesSeparatePRs(vulnerabilitiesMap map // If the scan results are the same, no action is taken. // Otherwise, it performs a force push to the same branch and reopens the pull request if it was closed. // Only one aggregated pull request should remain open at all times. -func (cfp *CreateFixPullRequestsCmd) fixIssuesSinglePR(vulnerabilities map[string]*utils.VulnerabilityDetails) (err error) { - log.Debug("Starting aggregated dependencies fix...") - aggregatedFixBranchName, err := cfp.gitManager.GenerateAggregatedFixBranchName() +func (cfp *CreateFixPullRequestsCmd) fixIssuesSinglePR(vulnerabilitiesMap map[string]map[string]*utils.VulnerabilityDetails) (err error) { + aggregatedFixBranchName, err := cfp.gitManager.GenerateAggregatedFixBranchName(cfp.projectTech) if err != nil { return } @@ -173,19 +249,7 @@ func (cfp *CreateFixPullRequestsCmd) fixIssuesSinglePR(vulnerabilities map[strin if err != nil { return } - if existingPullRequestDetails != nil { - log.Info("Aggregated pull request already exists, verifying if update is needed...") - identicalScanResults, err := cfp.compareScanResults(vulnerabilities, existingPullRequestDetails) - if err != nil { - return err - } - if identicalScanResults { - log.Info("The existing pull request is in sync with the latest Xray scan, and no further updates are required.") - return err - } - log.Info("The existing pull request is not in sync with the latest Xray scan, updating pull request...") - } - return cfp.aggregateFixAndOpenPullRequest(vulnerabilities, aggregatedFixBranchName, existingPullRequestDetails) + return cfp.aggregateFixAndOpenPullRequest(vulnerabilitiesMap, aggregatedFixBranchName, existingPullRequestDetails) } // Handles possible error of update package operation @@ -202,8 +266,8 @@ func (cfp *CreateFixPullRequestsCmd) handleUpdatePackageErrors(err error) error // Creates a branch for the fixed package and open pull request against the target branch. // In case a branch already exists on remote, we skip it. func (cfp *CreateFixPullRequestsCmd) fixSinglePackageAndCreatePR(vulnDetails *utils.VulnerabilityDetails) (err error) { - fixVersion := vulnDetails.FixVersion - log.Debug("Attempting to fix", vulnDetails.ImpactedDependencyName, "with version", fixVersion) + fixVersion := vulnDetails.SuggestedFixedVersion + log.Debug("Attempting to fix", vulnDetails.ImpactedDependencyName, "with", fixVersion) fixBranchName, err := cfp.gitManager.GenerateFixBranchName(cfp.details.Branch(), vulnDetails.ImpactedDependencyName, fixVersion) if err != nil { return @@ -213,7 +277,7 @@ func (cfp *CreateFixPullRequestsCmd) fixSinglePackageAndCreatePR(vulnDetails *ut return } if existsInRemote { - log.Info(fmt.Sprintf("A pull request updating the dependency '%s' to version '%s' already exists. Skipping...", vulnDetails.ImpactedDependencyName, vulnDetails.FixVersion)) + log.Info(fmt.Sprintf("A pull request updating the dependency '%s' to version '%s' already exists. Skipping...", vulnDetails.ImpactedDependencyName, vulnDetails.SuggestedFixedVersion)) return } if err = cfp.gitManager.CreateBranchAndCheckout(fixBranchName); err != nil { @@ -226,7 +290,7 @@ func (cfp *CreateFixPullRequestsCmd) fixSinglePackageAndCreatePR(vulnDetails *ut return fmt.Errorf("failed while creating a fixing pull request for: %s with version: %s with error: \n%s", vulnDetails.ImpactedDependencyName, fixVersion, err.Error()) } - log.Info(fmt.Sprintf("Created Pull Request updating dependency '%s' to version '%s'", vulnDetails.ImpactedDependencyName, vulnDetails.FixVersion)) + log.Info(fmt.Sprintf("Created Pull Request updating dependency '%s' to version '%s'", vulnDetails.ImpactedDependencyName, vulnDetails.SuggestedFixedVersion)) return } @@ -239,29 +303,41 @@ func (cfp *CreateFixPullRequestsCmd) openFixingPullRequest(fixBranchName string, if isClean { return fmt.Errorf("there were no changes to commit after fixing the package '%s'", vulnDetails.ImpactedDependencyName) } - commitMessage := cfp.gitManager.GenerateCommitMessage(vulnDetails.ImpactedDependencyName, vulnDetails.FixVersion) + commitMessage := cfp.gitManager.GenerateCommitMessage(vulnDetails.ImpactedDependencyName, vulnDetails.SuggestedFixedVersion) if err = cfp.gitManager.AddAllAndCommit(commitMessage); err != nil { return } if err = cfp.gitManager.Push(false, fixBranchName); err != nil { return } - pullRequestTitle, prBody := cfp.preparePullRequestDetails([]formats.VulnerabilityOrViolationRow{*vulnDetails.VulnerabilityOrViolationRow}) + scanHash, err := utils.VulnerabilityDetailsToMD5Hash(vulnDetails) + if err != nil { + return err + } + 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) } // openAggregatedPullRequest handles the opening or updating of a pull request when the aggregate mode is active. // If a pull request is already open, Frogbot will update the branch and the pull request body. -func (cfp *CreateFixPullRequestsCmd) openAggregatedPullRequest(fixBranchName string, pullRequestInfo *vcsclient.PullRequestInfo, vulnerabilities []formats.VulnerabilityOrViolationRow) (err error) { - commitMessage := cfp.gitManager.GenerateAggregatedCommitMessage() +func (cfp *CreateFixPullRequestsCmd) openAggregatedPullRequest(fixBranchName string, pullRequestInfo *vcsclient.PullRequestInfo, vulnerabilities []*utils.VulnerabilityDetails) (err error) { + commitMessage := cfp.gitManager.GenerateAggregatedCommitMessage(cfp.projectTech) if err = cfp.gitManager.AddAllAndCommit(commitMessage); err != nil { return } if err = cfp.gitManager.Push(true, fixBranchName); err != nil { return } - pullRequestTitle, prBody := cfp.preparePullRequestDetails(vulnerabilities) + scanHash, err := utils.VulnerabilityDetailsToMD5Hash(vulnerabilities...) + 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()) return cfp.details.Client().CreatePullRequest(context.Background(), cfp.details.RepoOwner, cfp.details.RepoName, fixBranchName, cfp.details.Branch(), pullRequestTitle, prBody) @@ -270,25 +346,25 @@ func (cfp *CreateFixPullRequestsCmd) openAggregatedPullRequest(fixBranchName str return cfp.details.Client().UpdatePullRequest(context.Background(), cfp.details.RepoOwner, cfp.details.RepoName, pullRequestTitle, prBody, "", int(pullRequestInfo.ID), vcsutils.Open) } -func (cfp *CreateFixPullRequestsCmd) preparePullRequestDetails(vulnerabilities []formats.VulnerabilityOrViolationRow) (pullRequestTitle string, prBody string) { +func (cfp *CreateFixPullRequestsCmd) preparePullRequestDetails(scanHash string, vulnerabilitiesRows []formats.VulnerabilityOrViolationRow) (string, string) { if cfp.dryRun && cfp.aggregateFixes { // For testings, don't compare pull request body as scan results order may change. - return utils.AggregatedPullRequestTitleTemplate, "" + return utils.GetAggregatedPullRequestTitle(cfp.projectTech), "" } - prBody = cfp.OutputWriter.VulnerabiltiesTitle(false) + "\n" + cfp.OutputWriter.VulnerabilitiesContent(vulnerabilities) + "\n---\n" + cfp.OutputWriter.UntitledForJasMsg() + cfp.OutputWriter.Footer() + + prBody := cfp.OutputWriter.VulnerabiltiesTitle(false) + "\n" + cfp.OutputWriter.VulnerabilitiesContent(vulnerabilitiesRows) + "\n---\n" + cfp.OutputWriter.UntitledForJasMsg() + cfp.OutputWriter.Footer() if cfp.aggregateFixes { - pullRequestTitle = utils.AggregatedPullRequestTitleTemplate - } else { - vulnDetails := vulnerabilities[0] - pullRequestTitle = cfp.gitManager.GeneratePullRequestTitle(vulnDetails.ImpactedDependencyName, vulnDetails.FixedVersions[0]) + return utils.GetAggregatedPullRequestTitle(cfp.projectTech), prBody + utils.MarkdownComment(fmt.Sprintf("Checksum: %s", scanHash)) } - return + // In separate pull requests there is only one vulnerability + vulnDetails := vulnerabilitiesRows[0] + pullRequestTitle := cfp.gitManager.GeneratePullRequestTitle(vulnDetails.ImpactedDependencyName, vulnDetails.FixedVersions[0]) + return pullRequestTitle, prBody } func (cfp *CreateFixPullRequestsCmd) cloneRepository() (tempWd string, restoreDir func() error, err error) { if cfp.dryRunRepoPath != "" { - // On dry run, create the temp folder nested in the current folder - tempWd, err = os.MkdirTemp(cfp.dryRunRepoPath, "nested-temp.") + tempWd, err = cfp.getDryRunClonedRepo() } else { // Create temp working directory tempWd, err = fileutils.CreateTempDir() @@ -308,6 +384,25 @@ func (cfp *CreateFixPullRequestsCmd) cloneRepository() (tempWd string, restoreDi return } +func (cfp *CreateFixPullRequestsCmd) getDryRunClonedRepo() (tempWd string, err error) { + // Check if we already cloned the repository before, for multi projects tests + // Return the existing folder if exists + var files []string + files, err = fileutils.ListFiles(cfp.dryRunRepoPath, true) + if err != nil { + return + } + for _, file := range files { + if strings.Contains(file, "nested-temp.") { + cfp.gitManager.SkipClone = true + tempWd = file + return + } + } + // Create the temp folder nested in the current folder + return os.MkdirTemp(cfp.dryRunRepoPath, "nested-temp.") +} + // Create a vulnerabilities map - a map with 'impacted package' as a key and all the necessary information of this vulnerability as value. func (cfp *CreateFixPullRequestsCmd) createVulnerabilitiesMap(scanResults *xrayutils.ExtendedScanResults, isMultipleRoots bool) (map[string]*utils.VulnerabilityDetails, error) { vulnerabilitiesMap := map[string]*utils.VulnerabilityDetails{} @@ -342,6 +437,9 @@ func (cfp *CreateFixPullRequestsCmd) addVulnerabilityToFixVersionsMap(vulnerabil if len(vulnerability.FixedVersions) == 0 { return nil } + if cfp.projectTech == "" { + cfp.projectTech = vulnerability.Technology + } vulnFixVersion := getMinimalFixVersion(vulnerability.ImpactedDependencyVersion, vulnerability.FixedVersions) if vulnFixVersion == "" { return nil @@ -361,23 +459,12 @@ func (cfp *CreateFixPullRequestsCmd) addVulnerabilityToFixVersionsMap(vulnerabil vulnerabilitiesMap[vulnerability.ImpactedDependencyName] = newVulnDetails } // Set the fixed version array to the relevant fixed version so that only that specific fixed version will be displayed - vulnerability.FixedVersions = []string{vulnerabilitiesMap[vulnerability.ImpactedDependencyName].FixVersion} + vulnerability.FixedVersions = []string{vulnerabilitiesMap[vulnerability.ImpactedDependencyName].SuggestedFixedVersion} return nil } // Updates impacted package, can return ErrUnsupportedFix. func (cfp *CreateFixPullRequestsCmd) updatePackageToFixedVersion(vulnDetails *utils.VulnerabilityDetails) (err error) { - // 'CD' into the relevant working directory - if cfp.projectWorkingDir != "" { - restoreDir, err := utils.Chdir(cfp.projectWorkingDir) - if err != nil { - return err - } - defer func() { - err = errors.Join(err, restoreDir()) - }() - } - if err = isBuildToolsDependency(vulnDetails); err != nil { return } @@ -397,32 +484,24 @@ func (cfp *CreateFixPullRequestsCmd) updatePackageToFixedVersion(vulnDetails *ut return cfp.handlers[vulnDetails.Technology].UpdateDependency(vulnDetails) } -// Computes the MD5 hash of a vulnerabilitiesMap originated from the remote branch scan results -func (cfp *CreateFixPullRequestsCmd) getRemoteBranchScanHash(remoteBranchName string) (hash string, err error) { - log.Debug("Scanning remote branch", remoteBranchName) - if err = cfp.gitManager.CheckoutRemoteBranch(remoteBranchName); err != nil { - return - } - defer func() { - err = cfp.gitManager.CheckoutLocalBranch(cfp.details.Branch()) - }() - wd, err := os.Getwd() - if err != nil { - return - } - res, err := cfp.scan(wd) - if err != nil { - return - } - vulnerabilitiesMap, err := cfp.createVulnerabilitiesMap(res.ExtendedScanResults, res.IsMultipleRootProject) - if err != nil { - return +// The getRemoteBranchScanHash function extracts the checksum written inside the pull request body and returns it. +func (cfp *CreateFixPullRequestsCmd) getRemoteBranchScanHash(prBody string) string { + // The pattern matches the string "Checksum: ", followed by one or more word characters (letters, digits, or underscores). + re := regexp.MustCompile(`Checksum: (\w+)`) + match := re.FindStringSubmatch(prBody) + + // The first element is the entire matched string, and the second element is the checksum value. + // If the length of match is not equal to 2, it means that the pattern was not found or the captured group is missing. + if len(match) != 2 { + log.Debug("Checksum not found in the aggregated pull request. Frogbot will proceed to update the existing pull request.") + return "" } - return utils.VulnerabilityDetailsToMD5Hash(vulnerabilitiesMap) + + return match[1] } func (cfp *CreateFixPullRequestsCmd) getOpenPullRequestBySourceBranch(branchName string) (prInfo *vcsclient.PullRequestInfo, err error) { - list, err := cfp.details.Client().ListOpenPullRequests(context.Background(), cfp.details.RepoOwner, cfp.details.RepoName) + list, err := cfp.details.Client().ListOpenPullRequestsWithBody(context.Background(), cfp.details.RepoOwner, cfp.details.RepoName) if err != nil { return } @@ -433,47 +512,65 @@ func (cfp *CreateFixPullRequestsCmd) getOpenPullRequestBySourceBranch(branchName } } log.Debug("No pull request found from source branch ", branchName) - return nil, nil + return } -func (cfp *CreateFixPullRequestsCmd) aggregateFixAndOpenPullRequest(vulnerabilities map[string]*utils.VulnerabilityDetails, aggregatedFixBranchName string, pullRequestInfo *vcsclient.PullRequestInfo) (err error) { - var atLeastOneFix bool +func (cfp *CreateFixPullRequestsCmd) aggregateFixAndOpenPullRequest(vulnerabilitiesMap map[string]map[string]*utils.VulnerabilityDetails, aggregatedFixBranchName string, existingPullRequestInfo *vcsclient.PullRequestInfo) (err error) { + log.Info("-----------------------------------------------------------------") + log.Info("Starting aggregated dependencies fix") if err = cfp.gitManager.CreateBranchAndCheckout(aggregatedFixBranchName); err != nil { return } // Fix all packages in the same branch if expected error accrued, log and continue. - var fixedVulnerabilities []formats.VulnerabilityOrViolationRow - for _, vulnDetails := range vulnerabilities { - if e := cfp.updatePackageToFixedVersion(vulnDetails); e != nil { - err = errors.Join(cfp.handleUpdatePackageErrors(err)) - } else { - vulnDetails.FixedVersions = []string{vulnDetails.FixVersion} - fixedVulnerabilities = append(fixedVulnerabilities, *vulnDetails.VulnerabilityOrViolationRow) - log.Info(fmt.Sprintf("Updated dependency '%s' to version '%s'", vulnDetails.ImpactedDependencyName, vulnDetails.FixVersion)) - atLeastOneFix = true + var fixedVulnerabilities []*utils.VulnerabilityDetails + for fullPath, vulnerabilities := range vulnerabilitiesMap { + currentFixes, e := cfp.fixMultiplePackages(fullPath, vulnerabilities) + if e != nil { + err = errors.Join(err, fmt.Errorf("the following errors occured while fixing vulnerabilities in %s:\n%s", fullPath, e)) + continue } + fixedVulnerabilities = append(fixedVulnerabilities, currentFixes...) } - if atLeastOneFix { - if e := cfp.openAggregatedPullRequest(aggregatedFixBranchName, pullRequestInfo, fixedVulnerabilities); e != nil { - err = errors.Join(err, fmt.Errorf("failed while creating aggreagted pull request. Error: \n%s", e.Error())) - return + updateRequired, e := cfp.isUpdateRequired(fixedVulnerabilities, existingPullRequestInfo) + if err != nil { + err = errors.Join(err, e) + return + } + if !updateRequired { + log.Info("The existing pull request is in sync with the latest scan, and no further updates are required.") + return + } + if len(fixedVulnerabilities) > 0 { + if e := cfp.openAggregatedPullRequest(aggregatedFixBranchName, existingPullRequestInfo, fixedVulnerabilities); e != nil { + err = errors.Join(err, fmt.Errorf("failed while creating aggreagted pull request. Error: \n%s", err.Error())) } } + log.Info("-----------------------------------------------------------------") + if e := cfp.gitManager.CheckoutLocalBranch(cfp.details.Branch()); e != nil { + err = errors.Join(err, e) + } return } -// Performs a comparison of the Xray scan results between an existing pull request's remote source branch +// Performs a comparison of the Xray scan results hashes between an existing pull request's remote source branch // and the current source branch to identify any differences. -func (cfp *CreateFixPullRequestsCmd) compareScanResults(vulnerabilityDetails map[string]*utils.VulnerabilityDetails, prInfo *vcsclient.PullRequestInfo) (identical bool, err error) { - currentScanHash, err := utils.VulnerabilityDetailsToMD5Hash(vulnerabilityDetails) - if err != nil { +func (cfp *CreateFixPullRequestsCmd) isUpdateRequired(fixedVulnerabilities []*utils.VulnerabilityDetails, prInfo *vcsclient.PullRequestInfo) (updateRequired bool, err error) { + if prInfo == nil { + updateRequired = true return } - remoteBranchScanHash, err := cfp.getRemoteBranchScanHash(prInfo.Target.Name) + 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...) if err != nil { return } - return currentScanHash == remoteBranchScanHash, err + remoteBranchScanHash := cfp.getRemoteBranchScanHash(prInfo.Body) + updateRequired = currentScanHash != remoteBranchScanHash + if updateRequired { + log.Info("The existing pull request is not in sync with the latest scan, updating pull request...") + } + return } // getMinimalFixVersion find the minimal version that fixes the current impactedPackage; @@ -514,7 +611,7 @@ func isBuildToolsDependency(vulnDetails *utils.VulnerabilityDetails) error { if slices.Contains(utils.BuildToolsDependenciesMap[vulnDetails.Technology], vulnDetails.ImpactedDependencyName) { return &utils.ErrUnsupportedFix{ PackageName: vulnDetails.ImpactedDependencyName, - FixedVersion: vulnDetails.FixVersion, + FixedVersion: vulnDetails.SuggestedFixedVersion, ErrorType: utils.BuildToolsDependencyFixNotSupported, } } diff --git a/commands/createfixpullrequests_test.go b/commands/createfixpullrequests_test.go index bd03052de..1828040b1 100644 --- a/commands/createfixpullrequests_test.go +++ b/commands/createfixpullrequests_test.go @@ -19,15 +19,10 @@ import ( "os" "os/exec" "path/filepath" - "runtime" "strings" "testing" ) -const ( - aggregatedBranchConstName = "frogbot-update-dependencies-0" -) - var testPackagesData = []struct { packageType coreutils.Technology commandName string @@ -76,45 +71,63 @@ var testPackagesData = []struct { // Make sure it is checked out to the main branch, replicating an actual run. func TestCreateFixPullRequestsCmd_Run(t *testing.T) { tests := []struct { - repoName string - testDir string - configPath string - expectedBranchName string - expectedDiff string - dependencyFileName string - aggregateFixes bool + repoName string + testDir string + configPath string + expectedDiff []string + expectedBranches []string + packageDescriptorPaths []string + aggregateFixes bool }{ { - repoName: "aggregate", - testDir: "createfixpullrequests/aggregate", - expectedBranchName: aggregatedBranchConstName, - expectedDiff: "diff --git a/package.json b/package.json\nindex c5ea932..1176f2d 100644\n--- a/package.json\n+++ b/package.json\n@@ -9,8 +9,8 @@\n \"author\": \"\",\n \"license\": \"ISC\",\n \"dependencies\": {\n- \"uuid\": \"^9.0.0\",\n- \"minimist\":\"1.2.5\",\n- \"mpath\": \"0.7.0\"\n+ \"minimist\": \"^1.2.6\",\n+ \"mpath\": \"^0.8.4\",\n+ \"uuid\": \"^9.0.0\"\n }\n-}\n\\ No newline at end of file\n+}\n", - dependencyFileName: "package.json", - aggregateFixes: true, + repoName: "aggregate", + testDir: "createfixpullrequests/aggregate", + expectedBranches: []string{"frogbot-update-npm-dependencies"}, + expectedDiff: []string{"diff --git a/package.json b/package.json\nindex c5ea932..1176f2d 100644\n--- a/package.json\n+++ b/package.json\n@@ -9,8 +9,8 @@\n \"author\": \"\",\n \"license\": \"ISC\",\n \"dependencies\": {\n- \"uuid\": \"^9.0.0\",\n- \"minimist\":\"1.2.5\",\n- \"mpath\": \"0.7.0\"\n+ \"minimist\": \"^1.2.6\",\n+ \"mpath\": \"^0.8.4\",\n+ \"uuid\": \"^9.0.0\"\n }\n-}\n\\ No newline at end of file\n+}\n"}, + packageDescriptorPaths: []string{"package.json"}, + aggregateFixes: true, + }, + { + repoName: "aggregate-multi-dir", + testDir: "createfixpullrequests/aggregate-multi-dir", + expectedBranches: []string{"frogbot-update-npm-dependencies"}, + expectedDiff: []string{"diff --git a/npm1/package.json b/npm1/package.json\nindex ae09978..286211d 100644\n--- a/npm1/package.json\n+++ b/npm1/package.json\n@@ -9,8 +9,8 @@\n \"author\": \"\",\n \"license\": \"ISC\",\n \"dependencies\": {\n- \"uuid\": \"^9.0.0\",\n- \"minimatch\":\"3.0.2\",\n- \"mpath\": \"0.7.0\"\n+ \"minimatch\": \"^3.0.5\",\n+ \"mpath\": \"^0.8.4\",\n+ \"uuid\": \"^9.0.0\"\n }\n-}\n\\ No newline at end of file\n+}\ndiff --git a/npm2/package.json b/npm2/package.json\nindex be180a6..14b5c7a 100644\n--- a/npm2/package.json\n+++ b/npm2/package.json\n@@ -1,5 +1,5 @@\n {\n \"dependencies\": {\n- \"minimist\": \"^1.2.5\"\n+ \"minimist\": \"^1.2.6\"\n }\n }\n"}, + packageDescriptorPaths: []string{"npm1/package.json", "npm2/package.json"}, + aggregateFixes: true, + configPath: "testdata/createfixpullrequests/aggregate-multi-dir/.frogbot/frogbot-config.yml", + }, + { + repoName: "aggregate-multi-project", + testDir: "createfixpullrequests/aggregate-multi-project", + expectedBranches: []string{"frogbot-update-npm-dependencies", "frogbot-update-pip-dependencies"}, + expectedDiff: []string{"diff --git a/npm/package.json b/npm/package.json\nindex ae09978..286211d 100644\n--- a/npm/package.json\n+++ b/npm/package.json\n@@ -9,8 +9,8 @@\n \"author\": \"\",\n \"license\": \"ISC\",\n \"dependencies\": {\n- \"uuid\": \"^9.0.0\",\n- \"minimatch\":\"3.0.2\",\n- \"mpath\": \"0.7.0\"\n+ \"minimatch\": \"^3.0.5\",\n+ \"mpath\": \"^0.8.4\",\n+ \"uuid\": \"^9.0.0\"\n }\n-}\n\\ No newline at end of file\n+}\n", "diff --git a/pip/requirements.txt b/pip/requirements.txt\nindex 65c9637..7788edc 100644\n--- a/pip/requirements.txt\n+++ b/pip/requirements.txt\n@@ -1,2 +1,2 @@\n pexpect==4.8.0\n-pyjwt==1.7.1\n\\ No newline at end of file\n+pyjwt==2.4.0\n\\ No newline at end of file\n"}, + packageDescriptorPaths: []string{"npm/package.json", "pip/requirements.txt"}, + aggregateFixes: true, + configPath: "testdata/createfixpullrequests/aggregate-multi-project/.frogbot/frogbot-config.yml", }, { - repoName: "aggregate-no-vul", - testDir: "createfixpullrequests/aggregate-no-vul", - expectedBranchName: "main", // No branch should be created - expectedDiff: "", - dependencyFileName: "package.json", - aggregateFixes: true, + repoName: "aggregate-no-vul", + testDir: "createfixpullrequests/aggregate-no-vul", + expectedBranches: []string{"main"}, // No branch should be created + expectedDiff: []string{""}, + packageDescriptorPaths: []string{"package.json"}, + aggregateFixes: true, }, { - repoName: "aggregate-cant-fix", - testDir: "createfixpullrequests/aggregate-cant-fix", - expectedBranchName: aggregatedBranchConstName, - expectedDiff: "", // No diff expected - dependencyFileName: "setup.py", // This is a build tool dependency which should not be fixed - aggregateFixes: true, + repoName: "aggregate-cant-fix", + testDir: "createfixpullrequests/aggregate-cant-fix", + expectedBranches: []string{"frogbot-update-pip-dependencies"}, + expectedDiff: []string{""}, // No diff expected + packageDescriptorPaths: []string{"setup.py"}, // This is a build tool dependency which should not be fixed + aggregateFixes: true, }, { - repoName: "non-aggregate", - testDir: "createfixpullrequests/non-aggregate", - expectedBranchName: "frogbot-minimist-e6e68f7e53c2b59c6bd946e00af797f7", - expectedDiff: "diff --git a/package.json b/package.json\nindex 5c4b711..134c416 100644\n--- a/package.json\n+++ b/package.json\n@@ -9,6 +9,6 @@\n \"author\": \"\",\n \"license\": \"ISC\",\n \"dependencies\": {\n- \"minimist\":\"1.2.5\"\n+ \"minimist\": \"^1.2.6\"\n }\n-}\n\\ No newline at end of file\n+}\n", - dependencyFileName: "package.json", - aggregateFixes: false, + repoName: "non-aggregate", + testDir: "createfixpullrequests/non-aggregate", + expectedBranches: []string{"frogbot-minimist-e6e68f7e53c2b59c6bd946e00af797f7"}, + expectedDiff: []string{"diff --git a/package.json b/package.json\nindex 5c4b711..134c416 100644\n--- a/package.json\n+++ b/package.json\n@@ -9,6 +9,6 @@\n \"author\": \"\",\n \"license\": \"ISC\",\n \"dependencies\": {\n- \"minimist\":\"1.2.5\"\n+ \"minimist\": \"^1.2.6\"\n }\n-}\n\\ No newline at end of file\n+}\n"}, + packageDescriptorPaths: []string{"package.json"}, + aggregateFixes: false, }, } for _, test := range tests { @@ -158,12 +171,16 @@ func TestCreateFixPullRequestsCmd_Run(t *testing.T) { err = cmd.Run(configAggregator, client) // Validate assert.NoError(t, err) - resultDiff, err := verifyDependencyFileDiff("main", test.expectedBranchName, test.dependencyFileName) - assert.NoError(t, err) - assert.Equal(t, test.expectedDiff, string(resultDiff)) + for _, branch := range test.expectedBranches { + resultDiff, err := verifyDependencyFileDiff("main", branch, test.packageDescriptorPaths...) + assert.NoError(t, err) + assert.Contains(t, test.expectedDiff, string(resultDiff)) + } // Defers - restoreEnv() - server.Close() + defer func() { + restoreEnv() + server.Close() + }() }) } } @@ -186,7 +203,11 @@ func TestAggregatePullRequestLifecycle(t *testing.T) { testDir: "createfixpullrequests/aggregate-dont-update-pr", expectedUpdate: false, mockPullRequestResponse: []vcsclient.PullRequestInfo{{ID: mockPrId, - Source: vcsclient.BranchInfo{Name: aggregatedBranchConstName}, + Body: ` +[comment]: <> (Checksum: 16cc29940fb50efb794f6a53bbf18f80) +pr body + `, + Source: vcsclient.BranchInfo{Name: "frogbot-update-npm-dependencies"}, Target: vcsclient.BranchInfo{Name: "main"}, }}, }, @@ -195,7 +216,11 @@ func TestAggregatePullRequestLifecycle(t *testing.T) { testDir: "createfixpullrequests/aggregate-update-pr", expectedUpdate: true, mockPullRequestResponse: []vcsclient.PullRequestInfo{{ID: mockPrId, - Source: vcsclient.BranchInfo{Name: aggregatedBranchConstName}, + Body: ` +[comment]: <> (Checksum: 01373ac4d2c32e7da9be22f3e4b4e665) +pr body + `, + Source: vcsclient.BranchInfo{Name: "frogbot-update-npm-dependencies"}, Target: vcsclient.BranchInfo{Name: "remoteMain"}, }}, }, @@ -221,9 +246,9 @@ func TestAggregatePullRequestLifecycle(t *testing.T) { } // Set up mock VCS responses client := mockVcsClient(t) - client.EXPECT().ListOpenPullRequests(context.Background(), "", gitTestParams.RepoName).Return(test.mockPullRequestResponse, nil) + client.EXPECT().ListOpenPullRequestsWithBody(context.Background(), "", gitTestParams.RepoName).Return(test.mockPullRequestResponse, nil) if test.expectedUpdate { - client.EXPECT().UpdatePullRequest(context.Background(), "", gitTestParams.RepoName, utils.AggregatedPullRequestTitleTemplate, "", "", int(mockPrId), vcsutils.Open).Return(nil) + client.EXPECT().UpdatePullRequest(context.Background(), "", gitTestParams.RepoName, utils.GetAggregatedPullRequestTitle(coreutils.Npm), "", "", int(mockPrId), vcsutils.Open).Return(nil) } // Load default configurations var configData []byte @@ -294,7 +319,7 @@ func TestGenerateFixBranchName(t *testing.T) { func TestPackageTypeFromScan(t *testing.T) { environmentVars, restoreEnv := verifyEnv(t) defer restoreEnv() - var testScan CreateFixPullRequestsCmd + testScan := &CreateFixPullRequestsCmd{OutputWriter: &utils.StandardOutput{}} trueVal := true params := utils.Params{ Scan: utils.Scan{Projects: []utils.Project{{UseWrapper: &trueVal}}}, @@ -412,13 +437,13 @@ func TestCreateVulnerabilitiesMap(t *testing.T) { }, expectedMap: map[string]*utils.VulnerabilityDetails{ "vuln1": { - FixVersion: "1.9.1", - IsDirectDependency: true, - Cves: []string{"CVE-2023-1234", "CVE-2023-4321"}, + SuggestedFixedVersion: "1.9.1", + IsDirectDependency: true, + Cves: []string{"CVE-2023-1234", "CVE-2023-4321"}, }, "vuln2": { - FixVersion: "2.4.1", - Cves: []string{"CVE-2022-1234", "CVE-2022-4321"}, + SuggestedFixedVersion: "2.4.1", + Cves: []string{"CVE-2022-1234", "CVE-2022-4321"}, }, }, }, @@ -462,13 +487,13 @@ func TestCreateVulnerabilitiesMap(t *testing.T) { }, expectedMap: map[string]*utils.VulnerabilityDetails{ "viol1": { - FixVersion: "1.9.1", - IsDirectDependency: true, - Cves: []string{"CVE-2023-1234", "CVE-2023-4321"}, + SuggestedFixedVersion: "1.9.1", + IsDirectDependency: true, + Cves: []string{"CVE-2023-1234", "CVE-2023-4321"}, }, "viol2": { - FixVersion: "2.4.1", - Cves: []string{"CVE-2022-1234", "CVE-2022-4321"}, + SuggestedFixedVersion: "2.4.1", + Cves: []string{"CVE-2022-1234", "CVE-2022-4321"}, }, }, }, @@ -482,7 +507,7 @@ func TestCreateVulnerabilitiesMap(t *testing.T) { actualVuln, exists := fixVersionsMap[name] require.True(t, exists) assert.Equal(t, expectedVuln.IsDirectDependency, actualVuln.IsDirectDependency) - assert.Equal(t, expectedVuln.FixVersion, actualVuln.FixVersion) + assert.Equal(t, expectedVuln.SuggestedFixedVersion, actualVuln.SuggestedFixedVersion) assert.ElementsMatch(t, expectedVuln.Cves, actualVuln.Cves) } }) @@ -495,7 +520,7 @@ func TestUpdatePackageToFixedVersion(t *testing.T) { var testScan CreateFixPullRequestsCmd for tech, buildToolsDependencies := range utils.BuildToolsDependenciesMap { for _, impactedDependency := range buildToolsDependencies { - vulnDetails := &utils.VulnerabilityDetails{FixVersion: "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") @@ -503,6 +528,100 @@ func TestUpdatePackageToFixedVersion(t *testing.T) { } } +func TestGetRemoteBranchScanHash(t *testing.T) { + prBody := ` +[![](https://raw.githubusercontent.com/jfrog/frogbot/master/resources/v2/vulnerabilitiesBannerMR.png)](https://github.com/jfrog/frogbot#readme) +## 📦 Vulnerable Dependencies + +### ✍️ Summary + +
+ +| SEVERITY | CONTEXTUAL ANALYSIS | DIRECT DEPENDENCIES | IMPACTED DEPENDENCY | FIXED VERSIONS | +| :---------------------: | :----------------------------------: | :----------------------------------: | :-----------------------------------: | :---------------------------------: | +| ![](https://raw.githubusercontent.com/jfrog/frogbot/master/resources/v2/applicableHighSeverity.png)
High | $\color{}{\textsf{Undetermined}}$ |github.com/nats-io/nats-streaming-server:v0.21.0 | github.com/nats-io/nats-streaming-server:v0.21.0 | [0.24.1] | +| ![](https://raw.githubusercontent.com/jfrog/frogbot/master/resources/v2/applicableHighSeverity.png)
High | $\color{}{\textsf{Undetermined}}$ |github.com/mholt/archiver/v3:v3.5.1 | github.com/mholt/archiver/v3:v3.5.1 | | +| ![](https://raw.githubusercontent.com/jfrog/frogbot/master/resources/v2/applicableMediumSeverity.png)
Medium | $\color{}{\textsf{Undetermined}}$ |github.com/nats-io/nats-streaming-server:v0.21.0 | github.com/nats-io/nats-streaming-server:v0.21.0 | [0.24.3] | + +
+ +## 👇 Details + + +
+ github.com/nats-io/nats-streaming-server v0.21.0 +
+ +- **Severity** 🔥 High +- **Contextual Analysis:** $\color{}{\textsf{Undetermined}}$ +- **Package Name:** github.com/nats-io/nats-streaming-server +- **Current Version:** v0.21.0 +- **Fixed Version:** [0.24.1] +- **CVEs:** CVE-2022-24450 + + +
+ + +
+ github.com/mholt/archiver/v3 v3.5.1 +
+ +- **Severity** 🔥 High +- **Contextual Analysis:** $\color{}{\textsf{Undetermined}}$ +- **Package Name:** github.com/mholt/archiver/v3 +- **Current Version:** v3.5.1 + + +
+ + +
+ github.com/nats-io/nats-streaming-server v0.21.0 +
+ +- **Severity** 🎃 Medium +- **Contextual Analysis:** $\color{}{\textsf{Undetermined}}$ +- **Package Name:** github.com/nats-io/nats-streaming-server +- **Current Version:** v0.21.0 +- **Fixed Version:** [0.24.3] +- **CVEs:** CVE-2022-26652 + + +
+ + +## 🛠️ Infrastructure as Code + +
+ + +| SEVERITY | FILE | LINE:COLUMN | FINDING | +| :---------------------: | :----------------------------------: | :-----------------------------------: | :---------------------------------: | +| ![](https://raw.githubusercontent.com/jfrog/frogbot/master/resources/v2/applicableLowSeverity.png)
Low | test.js | 1:20 | kms_key_id='' was detected | +| ![](https://raw.githubusercontent.com/jfrog/frogbot/master/resources/v2/applicableHighSeverity.png)
High | test2.js | 4:30 | Deprecated TLS version was detected | + +
+ + +
+ +[JFrog Frogbot](https://github.com/jfrog/frogbot#readme) + +
+ +[Comment]: <> (Checksum: myhash4321) +` + cfp := &CreateFixPullRequestsCmd{} + result := cfp.getRemoteBranchScanHash(prBody) + assert.Equal(t, "myhash4321", result) + prBody = ` +random body +` + result = cfp.getRemoteBranchScanHash(prBody) + assert.Equal(t, "", result) +} + func TestPreparePullRequestDetails(t *testing.T) { cfp := CreateFixPullRequestsCmd{OutputWriter: &utils.StandardOutput{}, gitManager: &utils.GitManager{}} vulnerabilities := []formats.VulnerabilityOrViolationRow{ @@ -515,8 +634,8 @@ 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
\n\n\n| SEVERITY | DIRECT DEPENDENCIES | IMPACTED DEPENDENCY | FIXED VERSIONS |\n| :---------------------: | :----------------------------------: | :-----------------------------------: | :---------------------------------: | \n| ![](https://raw.githubusercontent.com/jfrog/frogbot/master/resources/v2/applicableHighSeverity.png)
High | | package1:1.0.0 | 1.0.0

2.0.0 |\n\n
\n\n## 👇 Details\n\n\n\n\n- **Severity** 🔥 High\n- **Package Name:** package1\n- **Current Version:** 1.0.0\n- **Fixed Version:** 1.0.0,2.0.0\n- **CVE:** CVE-2022-1234\n\n**Description:**\n\nsummary\n\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
\n\n
\n\n[JFrog Frogbot](https://github.com/jfrog/frogbot#readme)\n\n
\n" - prTitle, prBody := cfp.preparePullRequestDetails(vulnerabilities) + 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
\n\n\n| SEVERITY | DIRECT DEPENDENCIES | IMPACTED DEPENDENCY | FIXED VERSIONS |\n| :---------------------: | :----------------------------------: | :-----------------------------------: | :---------------------------------: | \n| ![](https://raw.githubusercontent.com/jfrog/frogbot/master/resources/v2/applicableHighSeverity.png)
High | | package1:1.0.0 | 1.0.0

2.0.0 |\n\n
\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
\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
\n\n
\n\n[JFrog Frogbot](https://github.com/jfrog/frogbot#readme)\n\n
\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) vulnerabilities = append(vulnerabilities, formats.VulnerabilityOrViolationRow{ @@ -528,14 +647,14 @@ 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
\n\n\n| SEVERITY | DIRECT DEPENDENCIES | IMPACTED DEPENDENCY | FIXED VERSIONS |\n| :---------------------: | :----------------------------------: | :-----------------------------------: | :---------------------------------: | \n| ![](https://raw.githubusercontent.com/jfrog/frogbot/master/resources/v2/applicableHighSeverity.png)
High | | package1:1.0.0 | 1.0.0

2.0.0 |\n| ![](https://raw.githubusercontent.com/jfrog/frogbot/master/resources/v2/applicableCriticalSeverity.png)
Critical | | package2:2.0.0 | 2.0.0

3.0.0 |\n\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 Version:** 1.0.0,2.0.0\n- **CVE:** CVE-2022-1234\n\n**Description:**\n\nsummary\n\n\n\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 Version:** 2.0.0,3.0.0\n- **CVE:** CVE-2022-4321\n\n**Description:**\n\nsummary\n\n\n\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
\n\n
\n\n[JFrog Frogbot](https://github.com/jfrog/frogbot#readme)\n\n
\n" - prTitle, prBody = cfp.preparePullRequestDetails(vulnerabilities) - assert.Equal(t, "[🐸 Frogbot] Update dependencies versions", prTitle) + 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
\n\n\n| SEVERITY | DIRECT DEPENDENCIES | IMPACTED DEPENDENCY | FIXED VERSIONS |\n| :---------------------: | :----------------------------------: | :-----------------------------------: | :---------------------------------: | \n| ![](https://raw.githubusercontent.com/jfrog/frogbot/master/resources/v2/applicableHighSeverity.png)
High | | package1:1.0.0 | 1.0.0

2.0.0 |\n| ![](https://raw.githubusercontent.com/jfrog/frogbot/master/resources/v2/applicableCriticalSeverity.png)
Critical | | package2:2.0.0 | 2.0.0

3.0.0 |\n\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\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\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
\n\n
\n\n[JFrog Frogbot](https://github.com/jfrog/frogbot#readme)\n\n
\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 Version:** 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 Version:** 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)" - prTitle, prBody = cfp.preparePullRequestDetails(vulnerabilities) - assert.Equal(t, "[🐸 Frogbot] Update dependencies versions", prTitle) + 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" + prTitle, prBody = cfp.preparePullRequestDetails("hash", vulnerabilities) + assert.Equal(t, utils.GetAggregatedPullRequestTitle(""), prTitle) assert.Equal(t, expectedPrBody, prBody) } @@ -548,17 +667,20 @@ func verifyTechnologyNaming(t *testing.T, scanResponse []services.ScanResponse, } // Executing git diff to ensure that the intended changes to the dependent file have been made -func verifyDependencyFileDiff(baseBranch string, fixBranch string, dependencyFilename string) (output []byte, err error) { - var cmd *exec.Cmd - log.Debug(fmt.Sprintf("Checking differences in %s between branches %s and %s", dependencyFilename, baseBranch, fixBranch)) +func verifyDependencyFileDiff(baseBranch string, fixBranch string, packageDescriptorPaths ...string) (output []byte, err error) { + log.Debug(fmt.Sprintf("Checking differences in %s between branches %s and %s", packageDescriptorPaths, baseBranch, fixBranch)) // Suppress condition always false warning //goland:noinspection ALL - if runtime.GOOS == "windows" { - cmd = exec.Command("cmd", "/c", "git", "diff", baseBranch, fixBranch, "--", dependencyFilename) + var args []string + if coreutils.IsWindows() { + args = []string{"/c", "git", "diff", baseBranch, fixBranch} + args = append(args, packageDescriptorPaths...) + output, err = exec.Command("cmd", args...).Output() } else { - cmd = exec.Command("git", "diff", baseBranch, fixBranch, "--", dependencyFilename) + args = []string{"diff", baseBranch, fixBranch} + args = append(args, packageDescriptorPaths...) + output, err = exec.Command("git", args...).Output() } - output, err = cmd.Output() if exitError, ok := err.(*exec.ExitError); ok { err = errors.New("git error: " + string(exitError.Stderr)) } diff --git a/commands/scanandfixrepos.go b/commands/scanandfixrepos.go index 1d166efb5..872eb22b5 100644 --- a/commands/scanandfixrepos.go +++ b/commands/scanandfixrepos.go @@ -10,8 +10,6 @@ import ( type ScanAndFixRepositories struct { // dryRun is used for testing purposes, mocking part of the git commands that requires networking dryRun bool - // When dryRun is enabled, dryRunRepoPath specifies the repository local path to clone - dryRunRepoPath string } func (saf *ScanAndFixRepositories) Run(repoAggregator utils.RepoAggregator, client vcsclient.VcsClient) error { @@ -56,6 +54,6 @@ func (saf *ScanAndFixRepositories) downloadAndRunScanAndFix(repository *utils.Re err = errors.Join(err, restoreDir()) }() - cfp := CreateFixPullRequestsCmd{dryRun: saf.dryRun, dryRunRepoPath: saf.dryRunRepoPath} + cfp := CreateFixPullRequestsCmd{dryRun: saf.dryRun, dryRunRepoPath: wd} return cfp.scanAndFixRepository(repository, branch, client) } diff --git a/commands/scanandfixrepos_test.go b/commands/scanandfixrepos_test.go index 646865a32..cc3d384e7 100644 --- a/commands/scanandfixrepos_test.go +++ b/commands/scanandfixrepos_test.go @@ -61,7 +61,7 @@ func TestScanAndFixRepos(t *testing.T) { configAggregator, err := utils.BuildRepoAggregator(configData, &gitTestParams, &serverParams) assert.NoError(t, err) - var cmd = ScanAndFixRepositories{dryRun: true, dryRunRepoPath: tmpDir} + var cmd = ScanAndFixRepositories{dryRun: true} assert.NoError(t, cmd.Run(configAggregator, client)) } diff --git a/commands/scanpullrequest.go b/commands/scanpullrequest.go index f08d55451..98095b20d 100644 --- a/commands/scanpullrequest.go +++ b/commands/scanpullrequest.go @@ -343,14 +343,14 @@ func getNewViolations(targetScan, sourceScan services.ScanResponse, auditResults return violationsRows, err } for _, violation := range violationsRows { - existsViolationsMap[getUniqueID(violation)] = violation + existsViolationsMap[utils.GetUniqueID(violation)] = violation } violationsRows, _, _, err = xrayutils.PrepareViolations(sourceScan.Violations, auditResults.ExtendedScanResults, auditResults.IsMultipleRootProject, true) if err != nil { return newViolationsRows, err } for _, violation := range violationsRows { - if _, exists := existsViolationsMap[getUniqueID(violation)]; !exists { + if _, exists := existsViolationsMap[utils.GetUniqueID(violation)]; !exists { newViolationsRows = append(newViolationsRows, violation) } } @@ -364,24 +364,20 @@ func getNewVulnerabilities(targetScan, sourceScan services.ScanResponse, auditRe return newVulnerabilitiesRows, err } for _, vulnerability := range targetVulnerabilitiesRows { - targetVulnerabilitiesMap[getUniqueID(vulnerability)] = vulnerability + targetVulnerabilitiesMap[utils.GetUniqueID(vulnerability)] = vulnerability } sourceVulnerabilitiesRows, err := xrayutils.PrepareVulnerabilities(sourceScan.Vulnerabilities, auditResults.ExtendedScanResults, auditResults.IsMultipleRootProject, true) if err != nil { return newVulnerabilitiesRows, err } for _, vulnerability := range sourceVulnerabilitiesRows { - if _, exists := targetVulnerabilitiesMap[getUniqueID(vulnerability)]; !exists { + if _, exists := targetVulnerabilitiesMap[utils.GetUniqueID(vulnerability)]; !exists { newVulnerabilitiesRows = append(newVulnerabilitiesRows, vulnerability) } } return } -func getUniqueID(vulnerability formats.VulnerabilityOrViolationRow) string { - return vulnerability.ImpactedDependencyName + vulnerability.ImpactedDependencyVersion + vulnerability.IssueId -} - func createPullRequestMessage(vulnerabilitiesRows []formats.VulnerabilityOrViolationRow, iacRows []formats.IacSecretsRow, writer utils.OutputWriter) string { if len(vulnerabilitiesRows) == 0 && len(iacRows) == 0 { return writer.NoVulnerabilitiesTitle() + writer.UntitledForJasMsg() + writer.Footer() diff --git a/commands/scanpullrequests.go b/commands/scanpullrequests.go index 5261632d9..e96c716ea 100644 --- a/commands/scanpullrequests.go +++ b/commands/scanpullrequests.go @@ -84,7 +84,7 @@ func isFrogbotRescanComment(comment string) bool { return strings.Contains(strings.ToLower(strings.TrimSpace(comment)), utils.RescanRequestComment) } -func downloadAndScanPullRequest(pr vcsclient.PullRequestInfo, repo utils.Repository, client vcsclient.VcsClient) error { +func downloadAndScanPullRequest(pr vcsclient.PullRequestInfo, repo utils.Repository, client vcsclient.VcsClient) (err error) { // Download the pull request source ("from") branch params := utils.Params{ Git: utils.Git{ diff --git a/commands/scanpullrequests_test.go b/commands/scanpullrequests_test.go index e91ef4c4c..e5c4f7a19 100644 --- a/commands/scanpullrequests_test.go +++ b/commands/scanpullrequests_test.go @@ -147,7 +147,7 @@ func TestScanAllPullRequestsMultiRepo(t *testing.T) { err := scanAllPullRequestsCmd.Run(configAggregator, client) assert.NoError(t, err) assert.Len(t, frogbotMessages, 4) - expectedMessage := "[![](https://raw.githubusercontent.com/jfrog/frogbot/master/resources/v2/vulnerabilitiesBannerPR.png)](https://github.com/jfrog/frogbot#readme)\n## 📦 Vulnerable Dependencies \n\n### ✍️ Summary\n\n
\n\n| SEVERITY | CONTEXTUAL ANALYSIS | DIRECT DEPENDENCIES | IMPACTED DEPENDENCY | FIXED VERSIONS |\n| :---------------------: | :----------------------------------: | :----------------------------------: | :-----------------------------------: | :---------------------------------: | \n| ![](https://raw.githubusercontent.com/jfrog/frogbot/master/resources/v2/notApplicableCritical.png)
Critical | Not Applicable |minimist:1.2.5 | minimist:1.2.5 | [0.2.4]

[1.2.6] |\n\n
\n\n## 👇 Details\n\n\n\n\n- **Severity** 💀 Critical\n- **Contextual Analysis:** Not Applicable\n- **Package Name:** minimist\n- **Current Version:** 1.2.5\n- **Fixed Version:** [0.2.4],[1.2.6]\n- **CVE:** CVE-2021-44906\n\n**Description:**\n\n[Minimist](https://github.com/substack/minimist) is a simple and very popular argument parser. It is used by more than 14 million by Mar 2022. This package developers stopped developing it since April 2020 and its community released a [newer version](https://github.com/meszaros-lajos-gyorgy/minimist-lite) supported by the community.\n\n\nAn incomplete fix for [CVE-2020-7598](https://nvd.nist.gov/vuln/detail/CVE-2020-7598) partially blocked prototype pollution attacks. Researchers discovered that it does not check for constructor functions which means they can be overridden. This behavior can be triggered easily when using it insecurely (which is the common usage). For example:\n```\nvar argv = parse(['--_.concat.constructor.prototype.y', '123']);\nt.equal((function(){}).foo, undefined);\nt.equal(argv.y, undefined);\n```\nIn this example, `prototype.y` is assigned with `123` which will be derived to every newly created object. \n\nThis vulnerability can be triggered when the attacker-controlled input is parsed using Minimist without any validation. As always with prototype pollution, the impact depends on the code that follows the attack, but denial of service is almost always guaranteed.\n\n**Remediation:**\n\n##### Development mitigations\n\nAdd the `Object.freeze(Object.prototype);` directive once at the beginning of your main JS source code file (ex. `index.js`), preferably after all your `require` directives. This will prevent any changes to the prototype object, thus completely negating prototype pollution attacks.\n\n\n\n\n
\n\n[JFrog Frogbot](https://github.com/jfrog/frogbot#readme)\n\n
\n" + expectedMessage := "[![](https://raw.githubusercontent.com/jfrog/frogbot/master/resources/v2/vulnerabilitiesBannerPR.png)](https://github.com/jfrog/frogbot#readme)\n## 📦 Vulnerable Dependencies \n\n### ✍️ Summary\n\n
\n\n| SEVERITY | CONTEXTUAL ANALYSIS | DIRECT DEPENDENCIES | IMPACTED DEPENDENCY | FIXED VERSIONS |\n| :---------------------: | :----------------------------------: | :----------------------------------: | :-----------------------------------: | :---------------------------------: | \n| ![](https://raw.githubusercontent.com/jfrog/frogbot/master/resources/v2/notApplicableCritical.png)
Critical | Not Applicable |minimist:1.2.5 | minimist:1.2.5 | [0.2.4]

[1.2.6] |\n\n
\n\n## 👇 Details\n\n\n\n\n- **Severity** 💀 Critical\n- **Contextual Analysis:** Not Applicable\n- **Package Name:** minimist\n- **Current Version:** 1.2.5\n- **Fixed Versions:** [0.2.4],[1.2.6]\n- **CVE:** CVE-2021-44906\n\n**Description:**\n\n[Minimist](https://github.com/substack/minimist) is a simple and very popular argument parser. It is used by more than 14 million by Mar 2022. This package developers stopped developing it since April 2020 and its community released a [newer version](https://github.com/meszaros-lajos-gyorgy/minimist-lite) supported by the community.\n\n\nAn incomplete fix for [CVE-2020-7598](https://nvd.nist.gov/vuln/detail/CVE-2020-7598) partially blocked prototype pollution attacks. Researchers discovered that it does not check for constructor functions which means they can be overridden. This behavior can be triggered easily when using it insecurely (which is the common usage). For example:\n```\nvar argv = parse(['--_.concat.constructor.prototype.y', '123']);\nt.equal((function(){}).foo, undefined);\nt.equal(argv.y, undefined);\n```\nIn this example, `prototype.y` is assigned with `123` which will be derived to every newly created object. \n\nThis vulnerability can be triggered when the attacker-controlled input is parsed using Minimist without any validation. As always with prototype pollution, the impact depends on the code that follows the attack, but denial of service is almost always guaranteed.\n\n**Remediation:**\n\n##### Development mitigations\n\nAdd the `Object.freeze(Object.prototype);` directive once at the beginning of your main JS source code file (ex. `index.js`), preferably after all your `require` directives. This will prevent any changes to the prototype object, thus completely negating prototype pollution attacks.\n\n\n\n\n
\n\n[JFrog Frogbot](https://github.com/jfrog/frogbot#readme)\n\n
\n" assert.Equal(t, expectedMessage, frogbotMessages[0]) expectedMessage = "[![](https://raw.githubusercontent.com/jfrog/frogbot/master/resources/v2/noVulnerabilityBannerPR.png)](https://github.com/jfrog/frogbot#readme)\n
\n\n[JFrog Frogbot](https://github.com/jfrog/frogbot#readme)\n\n
\n" assert.Equal(t, expectedMessage, frogbotMessages[1]) @@ -188,7 +188,7 @@ func TestScanAllPullRequests(t *testing.T) { err := scanAllPullRequestsCmd.Run(paramsAggregator, client) assert.NoError(t, err) assert.Len(t, frogbotMessages, 2) - expectedMessage := "**🚨 Frogbot scanned this pull request and found the below:**\n\n---\n## 📦 Vulnerable Dependencies\n---\n\n### ✍️ Summary \n\n| SEVERITY | CONTEXTUAL ANALYSIS | DIRECT DEPENDENCIES | IMPACTED DEPENDENCY | FIXED VERSIONS |\n| :---------------------: | :----------------------------------: | :----------------------------------: | :-----------------------------------: | :---------------------------------: | \n| Critical | Not Applicable | minimist:1.2.5 | minimist:1.2.5 | [0.2.4], [1.2.6] |\n\n---\n### 👇 Details\n---\n\n\n#### minimist 1.2.5\n\n\n- **Severity** 💀 Critical\n- **Contextual Analysis:** Not Applicable\n- **Package Name:** minimist\n- **Current Version:** 1.2.5\n- **Fixed Version:** [0.2.4],[1.2.6]\n- **CVE:** CVE-2021-44906\n\n**Description:**\n\n[Minimist](https://github.com/substack/minimist) is a simple and very popular argument parser. It is used by more than 14 million by Mar 2022. This package developers stopped developing it since April 2020 and its community released a [newer version](https://github.com/meszaros-lajos-gyorgy/minimist-lite) supported by the community.\n\n\nAn incomplete fix for [CVE-2020-7598](https://nvd.nist.gov/vuln/detail/CVE-2020-7598) partially blocked prototype pollution attacks. Researchers discovered that it does not check for constructor functions which means they can be overridden. This behavior can be triggered easily when using it insecurely (which is the common usage). For example:\n```\nvar argv = parse(['--_.concat.constructor.prototype.y', '123']);\nt.equal((function(){}).foo, undefined);\nt.equal(argv.y, undefined);\n```\nIn this example, `prototype.y` is assigned with `123` which will be derived to every newly created object. \n\nThis vulnerability can be triggered when the attacker-controlled input is parsed using Minimist without any validation. As always with prototype pollution, the impact depends on the code that follows the attack, but denial of service is almost always guaranteed.\n\n**Remediation:**\n\n##### Development mitigations\n\nAdd the `Object.freeze(Object.prototype);` directive once at the beginning of your main JS source code file (ex. `index.js`), preferably after all your `require` directives. This will prevent any changes to the prototype object, thus completely negating prototype pollution attacks.\n\n\n\n\n\n[JFrog Frogbot](https://github.com/jfrog/frogbot#readme)" + expectedMessage := "**🚨 Frogbot scanned this pull request and found the below:**\n\n---\n## 📦 Vulnerable Dependencies\n---\n\n### ✍️ Summary \n\n| SEVERITY | CONTEXTUAL ANALYSIS | DIRECT DEPENDENCIES | IMPACTED DEPENDENCY | FIXED VERSIONS |\n| :---------------------: | :----------------------------------: | :----------------------------------: | :-----------------------------------: | :---------------------------------: | \n| Critical | Not Applicable | minimist:1.2.5 | minimist:1.2.5 | [0.2.4], [1.2.6] |\n\n---\n### 👇 Details\n---\n\n\n#### minimist 1.2.5\n\n\n- **Severity** 💀 Critical\n- **Contextual Analysis:** Not Applicable\n- **Package Name:** minimist\n- **Current Version:** 1.2.5\n- **Fixed Versions:** [0.2.4],[1.2.6]\n- **CVE:** CVE-2021-44906\n\n**Description:**\n\n[Minimist](https://github.com/substack/minimist) is a simple and very popular argument parser. It is used by more than 14 million by Mar 2022. This package developers stopped developing it since April 2020 and its community released a [newer version](https://github.com/meszaros-lajos-gyorgy/minimist-lite) supported by the community.\n\n\nAn incomplete fix for [CVE-2020-7598](https://nvd.nist.gov/vuln/detail/CVE-2020-7598) partially blocked prototype pollution attacks. Researchers discovered that it does not check for constructor functions which means they can be overridden. This behavior can be triggered easily when using it insecurely (which is the common usage). For example:\n```\nvar argv = parse(['--_.concat.constructor.prototype.y', '123']);\nt.equal((function(){}).foo, undefined);\nt.equal(argv.y, undefined);\n```\nIn this example, `prototype.y` is assigned with `123` which will be derived to every newly created object. \n\nThis vulnerability can be triggered when the attacker-controlled input is parsed using Minimist without any validation. As always with prototype pollution, the impact depends on the code that follows the attack, but denial of service is almost always guaranteed.\n\n**Remediation:**\n\n##### Development mitigations\n\nAdd the `Object.freeze(Object.prototype);` directive once at the beginning of your main JS source code file (ex. `index.js`), preferably after all your `require` directives. This will prevent any changes to the prototype object, thus completely negating prototype pollution attacks.\n\n\n\n\n\n[JFrog Frogbot](https://github.com/jfrog/frogbot#readme)" assert.Equal(t, expectedMessage, frogbotMessages[0]) expectedMessage = "**👍 Frogbot scanned this pull request and found that it did not add vulnerable dependencies.** \n\n\n[JFrog Frogbot](https://github.com/jfrog/frogbot#readme)" assert.Equal(t, expectedMessage, frogbotMessages[1]) diff --git a/commands/testdata/createfixpullrequests/aggregate-multi-dir/.frogbot/frogbot-config.yml b/commands/testdata/createfixpullrequests/aggregate-multi-dir/.frogbot/frogbot-config.yml new file mode 100644 index 000000000..9f70c235a --- /dev/null +++ b/commands/testdata/createfixpullrequests/aggregate-multi-dir/.frogbot/frogbot-config.yml @@ -0,0 +1,10 @@ +- params: + git: + branches: + - main + repoName: "aggregate-multi-dir" + scan: + projects: + - workingDirs: + - npm1 + - npm2 diff --git a/commands/testdata/createfixpullrequests/aggregate-multi-dir/git/COMMIT_EDITMSG b/commands/testdata/createfixpullrequests/aggregate-multi-dir/git/COMMIT_EDITMSG new file mode 100644 index 000000000..b2c0d99a7 --- /dev/null +++ b/commands/testdata/createfixpullrequests/aggregate-multi-dir/git/COMMIT_EDITMSG @@ -0,0 +1 @@ +fix tests diff --git a/commands/testdata/createfixpullrequests/aggregate-multi-dir/git/HEAD b/commands/testdata/createfixpullrequests/aggregate-multi-dir/git/HEAD new file mode 100644 index 000000000..b870d8262 --- /dev/null +++ b/commands/testdata/createfixpullrequests/aggregate-multi-dir/git/HEAD @@ -0,0 +1 @@ +ref: refs/heads/main diff --git a/commands/testdata/createfixpullrequests/aggregate-multi-dir/git/config b/commands/testdata/createfixpullrequests/aggregate-multi-dir/git/config new file mode 100644 index 000000000..6c9406b7d --- /dev/null +++ b/commands/testdata/createfixpullrequests/aggregate-multi-dir/git/config @@ -0,0 +1,7 @@ +[core] + repositoryformatversion = 0 + filemode = true + bare = false + logallrefupdates = true + ignorecase = true + precomposeunicode = true diff --git a/commands/testdata/createfixpullrequests/aggregate-multi-dir/git/description b/commands/testdata/createfixpullrequests/aggregate-multi-dir/git/description new file mode 100644 index 000000000..498b267a8 --- /dev/null +++ b/commands/testdata/createfixpullrequests/aggregate-multi-dir/git/description @@ -0,0 +1 @@ +Unnamed repository; edit this file 'description' to name the repository. diff --git a/commands/testdata/createfixpullrequests/aggregate-multi-dir/git/index b/commands/testdata/createfixpullrequests/aggregate-multi-dir/git/index new file mode 100644 index 0000000000000000000000000000000000000000..6c359bb82ebd30469488972bab6b554c8de79951 GIT binary patch literal 412 zcmZ?q402{*U|<4b=9C@ZFX>L=t%T8x3=I4%vN0DK7#f!_Ffe`vsu2O=_45l3nXX;W#YCfaZI?m~x7Sqfw+?33DdUoSI-PQB=Ok@zuE66p}FGx(zPE1eL%PP*#V*t5R zFZX&TjE0)0iE18$={^arq_dJbTiyq!)Z|VmD7m6>T?K5O5vqA1L9VVqXO=J+D;RTa zyvoI4uVlISzqQ?^i@Vn?%oRWQ4&(#|Lj?maNyf(<&wVzPmRft}soc>xWqb0DC{WS} zESb2Rw|l+sHLGlC#vkPqPZvAItPh0+4p_k&4aViqUDhltTKYvazQ3)YG++JU?ar>y V?5Z-mrsXv!r<%`s;bm>s3jn6blwJS; literal 0 HcmV?d00001 diff --git a/commands/testdata/createfixpullrequests/aggregate-multi-dir/git/info/exclude b/commands/testdata/createfixpullrequests/aggregate-multi-dir/git/info/exclude new file mode 100644 index 000000000..a5196d1be --- /dev/null +++ b/commands/testdata/createfixpullrequests/aggregate-multi-dir/git/info/exclude @@ -0,0 +1,6 @@ +# git ls-files --others --exclude-from=.git/info/exclude +# Lines that start with '#' are comments. +# For a project mostly in C, the following would be a good set of +# exclude patterns (uncomment them if you want to use them): +# *.[oa] +# *~ diff --git a/commands/testdata/createfixpullrequests/aggregate-multi-dir/git/logs/HEAD b/commands/testdata/createfixpullrequests/aggregate-multi-dir/git/logs/HEAD new file mode 100644 index 000000000..d4a6b8be3 --- /dev/null +++ b/commands/testdata/createfixpullrequests/aggregate-multi-dir/git/logs/HEAD @@ -0,0 +1 @@ +0000000000000000000000000000000000000000 20cfb0cbabf22f1c12d626eec6e6dd834f4e43d1 Omer Zidkoni 1689852711 +0300 commit (initial): fix tests diff --git a/commands/testdata/createfixpullrequests/aggregate-multi-dir/git/logs/refs/heads/main b/commands/testdata/createfixpullrequests/aggregate-multi-dir/git/logs/refs/heads/main new file mode 100644 index 000000000..d4a6b8be3 --- /dev/null +++ b/commands/testdata/createfixpullrequests/aggregate-multi-dir/git/logs/refs/heads/main @@ -0,0 +1 @@ +0000000000000000000000000000000000000000 20cfb0cbabf22f1c12d626eec6e6dd834f4e43d1 Omer Zidkoni 1689852711 +0300 commit (initial): fix tests diff --git a/commands/testdata/createfixpullrequests/aggregate-multi-dir/git/objects/19/01e308e74cb275753b496e24dc28ca3dc9dc15 b/commands/testdata/createfixpullrequests/aggregate-multi-dir/git/objects/19/01e308e74cb275753b496e24dc28ca3dc9dc15 new file mode 100644 index 0000000000000000000000000000000000000000..aaf0a91f04e7253ee58d2ef92fe6a83e62b3c0aa GIT binary patch literal 57 zcmV-90LK4#0V^p=O;s>4U@$Z=Ff%bxC`e4sPE1eL%PP*#V_3&Iz0+cv*@c^uIZw}S Pyr;W*{+@{df$kGVW=9z6 literal 0 HcmV?d00001 diff --git a/commands/testdata/createfixpullrequests/aggregate-multi-dir/git/objects/20/cfb0cbabf22f1c12d626eec6e6dd834f4e43d1 b/commands/testdata/createfixpullrequests/aggregate-multi-dir/git/objects/20/cfb0cbabf22f1c12d626eec6e6dd834f4e43d1 new file mode 100644 index 000000000..ef0858726 --- /dev/null +++ b/commands/testdata/createfixpullrequests/aggregate-multi-dir/git/objects/20/cfb0cbabf22f1c12d626eec6e6dd834f4e43d1 @@ -0,0 +1,2 @@ +xK +1 @] 4tD{wZ:+|/IkUigy.T5P)csFلޥåqkOyU8ʠQI p^er5ÎҔM7 7 \ No newline at end of file diff --git a/commands/testdata/createfixpullrequests/aggregate-multi-dir/git/objects/61/a70d8baf4dd63a6b1b01f87791cb73425caf55 b/commands/testdata/createfixpullrequests/aggregate-multi-dir/git/objects/61/a70d8baf4dd63a6b1b01f87791cb73425caf55 new file mode 100644 index 0000000000000000000000000000000000000000..af1a192778d8feafeb97f3957cdc3fe2cc26069a GIT binary patch literal 57 zcmV-90LK4#0V^p=O;s>4U@$Z=Ff%bxC`e4sPE1eL%PP*#W7sFbm2_58XUqHGl$zWL P1tnKBuB!k5aYPY1Pe&Ed literal 0 HcmV?d00001 diff --git a/commands/testdata/createfixpullrequests/aggregate-multi-dir/git/objects/9f/70c235abae1684cf8db2cadf2730087a1773cc b/commands/testdata/createfixpullrequests/aggregate-multi-dir/git/objects/9f/70c235abae1684cf8db2cadf2730087a1773cc new file mode 100644 index 0000000000000000000000000000000000000000..87edd16562c6a355d31813992520875b7ad7f37d GIT binary patch literal 127 zcmV-_0D%8^0acAL4#F@DMVWgFt9ynbA*9aC+`$Q!G1N}v2626wwn*T~djIRcIOPb# z`E&?S&{5~HgXLK2_5egjOLy9X76wohTYCUbC0|i#gdUll7%K&RO;y58ykA>$Li^4Y h&ksuVZ|4V=y!@Ff%bxNGr-uPs%URP0r6t%S_j+%*|n#UvS8D^*XVZ V^Szr+-B&l@s1h$e0|4?a6n`Gp8$JL4 literal 0 HcmV?d00001 diff --git a/commands/testdata/createfixpullrequests/aggregate-multi-dir/git/objects/ae/099789389636d0d9196ce5cdb1de2dab9fbc91 b/commands/testdata/createfixpullrequests/aggregate-multi-dir/git/objects/ae/099789389636d0d9196ce5cdb1de2dab9fbc91 new file mode 100644 index 0000000000000000000000000000000000000000..680b53c67205e0ac568a04cd6b2ea746754ef823 GIT binary patch literal 208 zcmV;>05AV|0ZorFZ^AGTg_-#kryDX=QGlT(E9%g>E2?CiBRPca$i7raRsK8Y7%JBH z?!Eio=^9S3-G8k9)&S^@BWwXp-$&|=5WZ#kmtx}Jv$Lt2x+!W$$wnUK;S|vsUhdd; zbgM6^sO2zeL4Ko+LP>HR*&)CYKVuBB1s_0?I*>-Pd}iv72vr4iV}T87bY6JOBtwWf zU+S-H$)|<<>-+Ge8%Y_wWqSELP2A<&&;Eah(m3|)jMxEN+^Ml$DzY&eQ4z4I-*t8d KSkHI+S5f?k_i4fa literal 0 HcmV?d00001 diff --git a/commands/testdata/createfixpullrequests/aggregate-multi-dir/git/objects/b1/d50a083f2239a3ff3b3ed2d1bbaea16d17c1ee b/commands/testdata/createfixpullrequests/aggregate-multi-dir/git/objects/b1/d50a083f2239a3ff3b3ed2d1bbaea16d17c1ee new file mode 100644 index 0000000000000000000000000000000000000000..fef22e8a2036e423706f7a9b51c4d10b82f38bb2 GIT binary patch literal 103 zcmV-t0GR)H0V^p=O;xZoXD~4U0tLOaqWtuv{1S#W8jQ=IyR2DQwDgN;e1BU(X}X7beKwVrT6^ZH+|f8?d-9GbT-t~saXD}Idf#hS+0u+Z$|s&Kc8Xaa J3IM)VCq1jcFg^eP literal 0 HcmV?d00001 diff --git a/commands/testdata/createfixpullrequests/aggregate-multi-dir/git/objects/be/180a62cd192cb4ef53647c6d907074d428d724 b/commands/testdata/createfixpullrequests/aggregate-multi-dir/git/objects/be/180a62cd192cb4ef53647c6d907074d428d724 new file mode 100644 index 0000000000000000000000000000000000000000..d83aed2c86b91f1f58d6f03c80b60afb84f03411 GIT binary patch literal 63 zcmb*;s7`hQ=if42)laYD9qe^Uak_orU)k zHpJV9a+{vaVV^1W-i<+8FRds)Jt@CLA4=;c=jWwmrt4Ma<^VN;K*~<3>#8#Abzn5q zd_Pq48MW4NPVcmsW_IDGWX{vG8}I3^p1)@zgFs$Eu6{vca&}^Rs$N!cejd;~kb6&@ z+6SYd<^`ad#~_`0GP(TITD8ZopNr3%`rU}>@vE{l29biy0{x=Y!qUv5)ZEm(l48A* ziV}!BLxNmgfvh41V+CWbcX=B!pUY}B-7WY0Rns#$wwBRP7w8lQLj?maNyf(<&wVzP zmRft}soc>xWqb0DD9~82oaSZcz0Y}8Y95zm(b^E^(|P6Nj&4{GffdM}yZrx3hb4=g lgWaZ({xeU0T=HsL|4h$E@yV0cB7yQerAPe2X0z@s0|3Q-mXiPg literal 0 HcmV?d00001 diff --git a/commands/testdata/createfixpullrequests/aggregate-multi-project/git/info/exclude b/commands/testdata/createfixpullrequests/aggregate-multi-project/git/info/exclude new file mode 100644 index 000000000..a5196d1be --- /dev/null +++ b/commands/testdata/createfixpullrequests/aggregate-multi-project/git/info/exclude @@ -0,0 +1,6 @@ +# git ls-files --others --exclude-from=.git/info/exclude +# Lines that start with '#' are comments. +# For a project mostly in C, the following would be a good set of +# exclude patterns (uncomment them if you want to use them): +# *.[oa] +# *~ diff --git a/commands/testdata/createfixpullrequests/aggregate-multi-project/git/logs/HEAD b/commands/testdata/createfixpullrequests/aggregate-multi-project/git/logs/HEAD new file mode 100644 index 000000000..60385a71b --- /dev/null +++ b/commands/testdata/createfixpullrequests/aggregate-multi-project/git/logs/HEAD @@ -0,0 +1 @@ +0000000000000000000000000000000000000000 db63220ddfc3c49fd55d611720d5423ac966a1d0 Omer Zidkoni 1689852747 +0300 commit (initial): fix tests diff --git a/commands/testdata/createfixpullrequests/aggregate-multi-project/git/logs/refs/heads/main b/commands/testdata/createfixpullrequests/aggregate-multi-project/git/logs/refs/heads/main new file mode 100644 index 000000000..60385a71b --- /dev/null +++ b/commands/testdata/createfixpullrequests/aggregate-multi-project/git/logs/refs/heads/main @@ -0,0 +1 @@ +0000000000000000000000000000000000000000 db63220ddfc3c49fd55d611720d5423ac966a1d0 Omer Zidkoni 1689852747 +0300 commit (initial): fix tests diff --git a/commands/testdata/createfixpullrequests/aggregate-multi-project/git/objects/19/01e308e74cb275753b496e24dc28ca3dc9dc15 b/commands/testdata/createfixpullrequests/aggregate-multi-project/git/objects/19/01e308e74cb275753b496e24dc28ca3dc9dc15 new file mode 100644 index 0000000000000000000000000000000000000000..aaf0a91f04e7253ee58d2ef92fe6a83e62b3c0aa GIT binary patch literal 57 zcmV-90LK4#0V^p=O;s>4U@$Z=Ff%bxC`e4sPE1eL%PP*#V_3&Iz0+cv*@c^uIZw}S Pyr;W*{+@{df$kGVW=9z6 literal 0 HcmV?d00001 diff --git a/commands/testdata/createfixpullrequests/aggregate-multi-project/git/objects/1d/ced3ffd48839041e403eb2548fcce4f8d24a86 b/commands/testdata/createfixpullrequests/aggregate-multi-project/git/objects/1d/ced3ffd48839041e403eb2548fcce4f8d24a86 new file mode 100644 index 0000000000000000000000000000000000000000..dc2cae5520d239bdc146337c6f1e5286cea1604f GIT binary patch literal 63 zcmV-F0Korv0V^p=O;s>4V=y!@Ff%bxNGr-uPs%URP0r6t%S_j+%*|oAxw5IV@P5LE Vc>7Rp(~~*uGo{|U0RZq36XZV+9lrno literal 0 HcmV?d00001 diff --git a/commands/testdata/createfixpullrequests/aggregate-multi-project/git/objects/29/d343bde70ca929c71d042ab0564c89d4f1b88b b/commands/testdata/createfixpullrequests/aggregate-multi-project/git/objects/29/d343bde70ca929c71d042ab0564c89d4f1b88b new file mode 100644 index 0000000000000000000000000000000000000000..4c8a8549be996f11b2072fef68eb6491325fa02e GIT binary patch literal 61 zcmV-D0K)%x0V^p=O;s>4VK6i>Ff%bxC`v6X%`8gIP0cGQ)+?zfVMslhT>fdT+T+*H T#pg}^Zp8HXRaqJU=4ut%pn)2F literal 0 HcmV?d00001 diff --git a/commands/testdata/createfixpullrequests/aggregate-multi-project/git/objects/65/c96377f2ad26e3ebe7179e95f73202e3ea7666 b/commands/testdata/createfixpullrequests/aggregate-multi-project/git/objects/65/c96377f2ad26e3ebe7179e95f73202e3ea7666 new file mode 100644 index 000000000..e72f2147a --- /dev/null +++ b/commands/testdata/createfixpullrequests/aggregate-multi-project/git/objects/65/c96377f2ad26e3ebe7179e95f73202e3ea7666 @@ -0,0 +1,2 @@ +xKOR02g(H(HM.5ѳ3**r } +K \ No newline at end of file diff --git a/commands/testdata/createfixpullrequests/aggregate-multi-project/git/objects/ae/099789389636d0d9196ce5cdb1de2dab9fbc91 b/commands/testdata/createfixpullrequests/aggregate-multi-project/git/objects/ae/099789389636d0d9196ce5cdb1de2dab9fbc91 new file mode 100644 index 0000000000000000000000000000000000000000..680b53c67205e0ac568a04cd6b2ea746754ef823 GIT binary patch literal 208 zcmV;>05AV|0ZorFZ^AGTg_-#kryDX=QGlT(E9%g>E2?CiBRPca$i7raRsK8Y7%JBH z?!Eio=^9S3-G8k9)&S^@BWwXp-$&|=5WZ#kmtx}Jv$Lt2x+!W$$wnUK;S|vsUhdd; zbgM6^sO2zeL4Ko+LP>HR*&)CYKVuBB1s_0?I*>-Pd}iv72vr4iV}T87bY6JOBtwWf zU+S-H$)|<<>-+Ge8%Y_wWqSELP2A<&&;Eah(m3|)jMxEN+^Ml$DzY&eQ4z4I-*t8d KSkHI+S5f?k_i4fa literal 0 HcmV?d00001 diff --git a/commands/testdata/createfixpullrequests/aggregate-multi-project/git/objects/d9/a9828971df60b05f3f550b35c96c07991aef46 b/commands/testdata/createfixpullrequests/aggregate-multi-project/git/objects/d9/a9828971df60b05f3f550b35c96c07991aef46 new file mode 100644 index 000000000..12bf427e3 --- /dev/null +++ b/commands/testdata/createfixpullrequests/aggregate-multi-project/git/objects/d9/a9828971df60b05f3f550b35c96c07991aef46 @@ -0,0 +1,2 @@ +x};1 D9}(jkDI `C~xWӌm@AصkY +&wL9P'B yT2ҌT0}9{fs<5碡X|AzѳPݑC}PF_D@ \ No newline at end of file diff --git a/commands/testdata/createfixpullrequests/aggregate-multi-project/git/objects/db/63220ddfc3c49fd55d611720d5423ac966a1d0 b/commands/testdata/createfixpullrequests/aggregate-multi-project/git/objects/db/63220ddfc3c49fd55d611720d5423ac966a1d0 new file mode 100644 index 000000000..3886d1b05 --- /dev/null +++ b/commands/testdata/createfixpullrequests/aggregate-multi-project/git/objects/db/63220ddfc3c49fd55d611720d5423ac966a1d0 @@ -0,0 +1,2 @@ +x +!@;sbt]G!?msƲp ZJ w ,0ِ>ifbV HkW]3Zb-'?{հGoR@oG7 \ No newline at end of file diff --git a/commands/testdata/createfixpullrequests/aggregate-multi-project/git/objects/ee/6eb069e71d2a82dd7749fa7c8c935d7d014e2d b/commands/testdata/createfixpullrequests/aggregate-multi-project/git/objects/ee/6eb069e71d2a82dd7749fa7c8c935d7d014e2d new file mode 100644 index 0000000000000000000000000000000000000000..1cebb03bc5d05a0f0e5e27fcb4ea06696bb15706 GIT binary patch literal 104 zcmV-u0GI!G0V^p=O;xZoWiT-S0tLOaqWtuv{1OJ)bC>^L>9AyxbFkYK(tqa3k4s){ z5G8p9xeStwk2#+EY$`3a_RLecqjAdifV>#WTp6fq{Vugj06fGx9ih)WK**1_pi>`B1-!Dwg6qezj_TuqsG_(qJjs g1nY?fV>#WTp6fq{Vugj2RH`XPUO<^dSZ$iTqQB60HkL={W%ZHpc;PEuF~lmgRG gsS6WJB1JAZ 1689690889 +0300 commit (initial): init commit +0000000000000000000000000000000000000000 48181e917e611cb6c95db9119329859b7f39cc2a Omer Zidkoni 1689861913 +0300 commit (initial): fix tests diff --git a/commands/testdata/createfixpullrequests/aggregate/git/logs/refs/heads/main b/commands/testdata/createfixpullrequests/aggregate/git/logs/refs/heads/main index efa827c7b..f7ac2d504 100644 --- a/commands/testdata/createfixpullrequests/aggregate/git/logs/refs/heads/main +++ b/commands/testdata/createfixpullrequests/aggregate/git/logs/refs/heads/main @@ -1 +1 @@ -0000000000000000000000000000000000000000 0b0bc897474ade0656ffb59bde78495cbb0215ed Omer Zidkoni 1689690889 +0300 commit (initial): init commit +0000000000000000000000000000000000000000 48181e917e611cb6c95db9119329859b7f39cc2a Omer Zidkoni 1689861913 +0300 commit (initial): fix tests diff --git a/commands/testdata/createfixpullrequests/aggregate/git/objects/0b/0bc897474ade0656ffb59bde78495cbb0215ed b/commands/testdata/createfixpullrequests/aggregate/git/objects/0b/0bc897474ade0656ffb59bde78495cbb0215ed deleted file mode 100644 index dfaa03991..000000000 --- a/commands/testdata/createfixpullrequests/aggregate/git/objects/0b/0bc897474ade0656ffb59bde78495cbb0215ed +++ /dev/null @@ -1,2 +0,0 @@ -xK -1P9E|&ݵF4<*xTeiF/|daѮ .ّ a".'Vpiõ/*e],Ƙ`Qvnϫ/|8 \ No newline at end of file diff --git a/commands/testdata/createfixpullrequests/aggregate/git/objects/48/181e917e611cb6c95db9119329859b7f39cc2a b/commands/testdata/createfixpullrequests/aggregate/git/objects/48/181e917e611cb6c95db9119329859b7f39cc2a new file mode 100644 index 0000000000000000000000000000000000000000..6e2f4e35413a1c76d57f58065172f4c4692500a2 GIT binary patch literal 130 zcmV-|0Db>>0hNtO3IZ_@06pgweHWyAlF0%h{=mDpHj~yUnLs9j_DTO_PXoX7PF=0.4.2" + } + }, + "node_modules/balanced-match": { + "version": "1.0.2", + "resolved": "https://registry.npmjs.org/balanced-match/-/balanced-match-1.0.2.tgz", + "integrity": "sha512-3oSeUO0TMV67hN1AmbXsK4yaqU7tjiHlbxRDZOpH0KW9+CeX4bRAaX0Anxt0tx2MrpRpWwQaPwIlISEJhYU5Pw==" + }, + "node_modules/brace-expansion": { + "version": "1.1.11", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.11.tgz", + "integrity": "sha512-iCuPHDFgrHX7H2vEI/5xpz07zSHB00TpugqhmYtVmMO6518mCuRMoOYFldEBl0g187ufozdaHgWKcYFb61qGiA==", + "dependencies": { + "balanced-match": "^1.0.0", + "concat-map": "0.0.1" + } + }, + "node_modules/concat-map": { + "version": "0.0.1", + "resolved": "https://registry.npmjs.org/concat-map/-/concat-map-0.0.1.tgz", + "integrity": "sha512-/Srv4dswyQNBfohGpz9o6Yb3Gz3SrUDqBH5rTuhGR7ahtlbYKnVxw2bCFMRljaA7EXHaXZ8wsHdodFvbkhKmqg==" + }, + "node_modules/minimatch": { + "version": "3.0.4", + "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-3.0.4.tgz", + "integrity": "sha512-yJHVQEhyqPLUTgt9B83PXu6W3rx4MvvHvSUvToogpwoGDOUQ+yDrR0HRot+yOCdCO7u4hX3pWft6kWBBcqh0UA==", + "dependencies": { + "brace-expansion": "^1.1.7" + }, + "engines": { + "node": "*" + } + }, + "node_modules/optimist": { + "version": "0.3.7", + "resolved": "https://registry.npmjs.org/optimist/-/optimist-0.3.7.tgz", + "integrity": "sha512-TCx0dXQzVtSCg2OgY/bO9hjM9cV4XYx09TVK+s3+FhkjT6LovsLe+pPMzpWf+6yXK/hUizs2gUoTw3jHM0VaTQ==", + "dependencies": { + "wordwrap": "~0.0.2" + } + }, + "node_modules/source-map": { + "version": "0.1.43", + "resolved": "https://registry.npmjs.org/source-map/-/source-map-0.1.43.tgz", + "integrity": "sha512-VtCvB9SIQhk3aF6h+N85EaqIaBFIAfZ9Cu+NJHHVvc8BbEcnvDcFw6sqQ2dQrT6SlOrZq3tIvyD9+EGq/lJryQ==", + "dependencies": { + "amdefine": ">=0.0.4" + }, + "engines": { + "node": ">=0.8.0" + } + }, + "node_modules/uglify-js": { + "version": "2.2.5", + "resolved": "https://registry.npmjs.org/uglify-js/-/uglify-js-2.2.5.tgz", + "integrity": "sha512-viLk+/8G0zm2aKt1JJAVcz5J/5ytdiNaIsKgrre3yvSUjwVG6ZUujGH7E2TiPigZUwLYCe7eaIUEP2Zka2VJPA==", + "dependencies": { + "optimist": "~0.3.5", + "source-map": "~0.1.7" + }, + "bin": { + "uglifyjs": "bin/uglifyjs" + }, + "engines": { + "node": ">=0.4.0" + } + }, + "node_modules/wordwrap": { + "version": "0.0.3", + "resolved": "https://registry.npmjs.org/wordwrap/-/wordwrap-0.0.3.tgz", + "integrity": "sha512-1tMA907+V4QmxV7dbRvb4/8MaRALK6q9Abid3ndMYnbyo8piisCmeONVqVSXqQA3KaP4SLt5b7ud6E2sqP8TFw==", + "engines": { + "node": ">=0.4.0" + } + } + } +} diff --git a/commands/utils/consts.go b/commands/utils/consts.go index ffeb2f19d..9595b53f0 100644 --- a/commands/utils/consts.go +++ b/commands/utils/consts.go @@ -99,11 +99,11 @@ const ( BranchHashPlaceHolder = "${BRANCH_NAME_HASH}" // Default naming templates - BranchNameTemplate = "frogbot-" + PackagePlaceHolder + "-" + BranchHashPlaceHolder - AggregatedBranchNameTemplate = "frogbot-update-dependencies-" + BranchHashPlaceHolder - CommitMessageTemplate = "Upgrade " + PackagePlaceHolder + " to " + FixVersionPlaceHolder - AggregatedPullRequestTitleTemplate = "[🐸 Frogbot] Update dependencies versions" - PullRequestTitleTemplate = "[🐸 Frogbot] Update version of " + PackagePlaceHolder + " to " + FixVersionPlaceHolder + BranchNameTemplate = "frogbot-" + PackagePlaceHolder + "-" + BranchHashPlaceHolder + AggregatedBranchNameTemplate = "frogbot-update-" + BranchHashPlaceHolder + "-dependencies" + CommitMessageTemplate = "Upgrade " + PackagePlaceHolder + " to " + FixVersionPlaceHolder + FrogbotPullRequestTitlePrefix = "[🐸 Frogbot]" + PullRequestTitleTemplate = FrogbotPullRequestTitlePrefix + " Update version of " + PackagePlaceHolder + " to " + FixVersionPlaceHolder // Frogbot Git author details showed in commits frogbotAuthorName = "JFrog-Frogbot" frogbotAuthorEmail = "eco-system+frogbot@jfrog.com" diff --git a/commands/utils/git.go b/commands/utils/git.go index 8eacfc24c..7b3a0ec66 100644 --- a/commands/utils/git.go +++ b/commands/utils/git.go @@ -8,6 +8,7 @@ import ( "github.com/go-git/go-git/v5/plumbing/transport" "github.com/go-git/go-git/v5/plumbing/transport/client" "github.com/jfrog/froggit-go/vcsutils" + "github.com/jfrog/jfrog-cli-core/v2/utils/coreutils" "github.com/jfrog/jfrog-client-go/utils/io/fileutils" "net/http" "os" @@ -35,10 +36,6 @@ const ( gitLabHttpsFormat = "%s/%s/%s.git" bitbucketServerHttpsFormat = "%s/scm/%s/%s.git" azureDevopsHttpsFormat = "https://%s@%s%s/_git/%s" - - // Aggregate branches name should always be the same name. - // We use a const to replace in the branch template ${BRANCH_NAME_HASH} - constAggregatedHash = "0" ) type GitManager struct { @@ -52,6 +49,8 @@ type GitManager struct { dryRun bool // When dryRun is enabled, dryRunRepoPath specifies the repository local path to clone dryRunRepoPath string + // When dryRun is enabled, skipClone allows skipping the cloning of a repository for testing purposes + SkipClone bool // Custom naming formats customTemplates CustomTemplates // Git details @@ -286,10 +285,10 @@ func (gm *GitManager) GenerateCommitMessage(impactedPackage string, fixVersion s return formatStringWithPlaceHolders(template, impactedPackage, fixVersion, "", true) } -func (gm *GitManager) GenerateAggregatedCommitMessage() string { +func (gm *GitManager) GenerateAggregatedCommitMessage(tech coreutils.Technology) string { template := gm.customTemplates.commitMessageTemplate if template == "" { - template = AggregatedPullRequestTitleTemplate + template = GetAggregatedPullRequestTitle(tech) } return formatStringWithPlaceHolders(template, "", "", "", true) } @@ -303,7 +302,6 @@ func formatStringWithPlaceHolders(str, impactedPackage, fixVersion, hash string, {FixVersionPlaceHolder, fixVersion}, {BranchHashPlaceHolder, hash}, } - for _, r := range replacements { str = strings.Replace(str, r.placeholder, r.value, 1) } @@ -338,17 +336,20 @@ func (gm *GitManager) GeneratePullRequestTitle(impactedPackage string, version s // GenerateAggregatedFixBranchName Generating a consistent branch name to enable branch updates // and to ensure that there is only one Frogbot branch in aggregated mode. -func (gm *GitManager) GenerateAggregatedFixBranchName() (fixBranchName string, err error) { +func (gm *GitManager) GenerateAggregatedFixBranchName(tech coreutils.Technology) (fixBranchName string, err error) { branchFormat := gm.customTemplates.branchNameTemplate if branchFormat == "" { branchFormat = AggregatedBranchNameTemplate } - return formatStringWithPlaceHolders(branchFormat, "", "", constAggregatedHash, false), nil + return formatStringWithPlaceHolders(branchFormat, "", "", tech.ToString(), false), nil } // dryRunClone clones an existing repository from our testdata folder into the destination folder for testing purposes. // We should call this function when the current working directory is the repository we want to clone. func (gm *GitManager) dryRunClone(destination string) error { + if gm.SkipClone { + return nil + } baseWd, err := os.Getwd() if err != nil { return err diff --git a/commands/utils/git_test.go b/commands/utils/git_test.go index 4441db8b3..98d6399a5 100644 --- a/commands/utils/git_test.go +++ b/commands/utils/git_test.go @@ -3,6 +3,7 @@ package utils import ( "github.com/jfrog/froggit-go/vcsclient" "github.com/jfrog/froggit-go/vcsutils" + "github.com/jfrog/jfrog-cli-core/v2/utils/coreutils" "github.com/stretchr/testify/assert" "testing" ) @@ -18,25 +19,25 @@ func TestGitManager_GenerateCommitMessage(t *testing.T) { { gitManager: GitManager{customTemplates: CustomTemplates{commitMessageTemplate: ": bump ${IMPACTED_PACKAGE}"}}, impactedPackage: "mquery", - fixVersion: VulnerabilityDetails{FixVersion: "3.4.5"}, + fixVersion: VulnerabilityDetails{SuggestedFixedVersion: "3.4.5"}, expected: ": bump mquery", description: "Custom prefix", }, { gitManager: GitManager{customTemplates: CustomTemplates{commitMessageTemplate: "[scope]: Upgrade package ${IMPACTED_PACKAGE} to ${FIX_VERSION}"}}, - impactedPackage: "mquery", fixVersion: VulnerabilityDetails{FixVersion: "3.4.5"}, + impactedPackage: "mquery", fixVersion: VulnerabilityDetails{SuggestedFixedVersion: "3.4.5"}, expected: "[scope]: Upgrade package mquery to 3.4.5", description: "Default template", }, { gitManager: GitManager{customTemplates: CustomTemplates{commitMessageTemplate: ""}}, - impactedPackage: "mquery", fixVersion: VulnerabilityDetails{FixVersion: "3.4.5"}, + impactedPackage: "mquery", fixVersion: VulnerabilityDetails{SuggestedFixedVersion: "3.4.5"}, expected: "Upgrade mquery to 3.4.5", description: "Default template", }, } for _, test := range tests { t.Run(test.expected, func(t *testing.T) { - commitMessage := test.gitManager.GenerateCommitMessage(test.impactedPackage, test.fixVersion.FixVersion) + commitMessage := test.gitManager.GenerateCommitMessage(test.impactedPackage, test.fixVersion.SuggestedFixedVersion) assert.Equal(t, test.expected, commitMessage) }) } @@ -53,27 +54,27 @@ func TestGitManager_GenerateFixBranchName(t *testing.T) { { gitManager: GitManager{customTemplates: CustomTemplates{branchNameTemplate: "[Feature]-${IMPACTED_PACKAGE}-${BRANCH_NAME_HASH}"}}, impactedPackage: "mquery", - fixVersion: VulnerabilityDetails{FixVersion: "3.4.5"}, + fixVersion: VulnerabilityDetails{SuggestedFixedVersion: "3.4.5"}, expected: "[Feature]-mquery-41b1f45136b25e3624b15999bd57a476", description: "Custom template", }, { gitManager: GitManager{customTemplates: CustomTemplates{branchNameTemplate: ""}}, impactedPackage: "mquery", - fixVersion: VulnerabilityDetails{FixVersion: "3.4.5"}, + fixVersion: VulnerabilityDetails{SuggestedFixedVersion: "3.4.5"}, expected: "frogbot-mquery-41b1f45136b25e3624b15999bd57a476", description: "No template", }, { gitManager: GitManager{customTemplates: CustomTemplates{branchNameTemplate: "just-a-branch-${BRANCH_NAME_HASH}"}}, impactedPackage: "mquery", - fixVersion: VulnerabilityDetails{FixVersion: "3.4.5"}, + fixVersion: VulnerabilityDetails{SuggestedFixedVersion: "3.4.5"}, expected: "just-a-branch-41b1f45136b25e3624b15999bd57a476", description: "Custom template without inputs", }, } for _, test := range tests { t.Run(test.expected, func(t *testing.T) { - commitMessage, err := test.gitManager.GenerateFixBranchName("md5Branch", test.impactedPackage, test.fixVersion.FixVersion) + commitMessage, err := test.gitManager.GenerateFixBranchName("md5Branch", test.impactedPackage, test.fixVersion.SuggestedFixedVersion) assert.NoError(t, err) assert.Equal(t, test.expected, commitMessage) }) @@ -91,28 +92,28 @@ func TestGitManager_GeneratePullRequestTitle(t *testing.T) { { gitManager: GitManager{customTemplates: CustomTemplates{pullRequestTitleTemplate: "[CustomPR] update ${IMPACTED_PACKAGE} to ${FIX_VERSION}"}}, impactedPackage: "mquery", - fixVersion: VulnerabilityDetails{FixVersion: "3.4.5"}, + fixVersion: VulnerabilityDetails{SuggestedFixedVersion: "3.4.5"}, expected: "[CustomPR] update mquery to 3.4.5", description: "Custom template", }, { gitManager: GitManager{customTemplates: CustomTemplates{pullRequestTitleTemplate: "[CustomPR] update ${IMPACTED_PACKAGE}"}}, impactedPackage: "mquery", - fixVersion: VulnerabilityDetails{FixVersion: "3.4.5"}, + fixVersion: VulnerabilityDetails{SuggestedFixedVersion: "3.4.5"}, expected: "[CustomPR] update mquery", description: "Custom template one var", }, { gitManager: GitManager{customTemplates: CustomTemplates{pullRequestTitleTemplate: ""}}, impactedPackage: "mquery", - fixVersion: VulnerabilityDetails{FixVersion: "3.4.5"}, + fixVersion: VulnerabilityDetails{SuggestedFixedVersion: "3.4.5"}, expected: "[🐸 Frogbot] Update version of mquery to 3.4.5", description: "No prefix", }, } for _, test := range tests { t.Run(test.expected, func(t *testing.T) { - titleOutput := test.gitManager.GeneratePullRequestTitle(test.impactedPackage, test.fixVersion.FixVersion) + titleOutput := test.gitManager.GeneratePullRequestTitle(test.impactedPackage, test.fixVersion.SuggestedFixedVersion) assert.Equal(t, test.expected, titleOutput) }) } @@ -125,19 +126,19 @@ func TestGitManager_GenerateAggregatedFixBranchName(t *testing.T) { desc string }{ { - expected: "frogbot-update-dependencies-0", + expected: "frogbot-update-go-dependencies", desc: "No template", gitManager: GitManager{}, }, { - expected: "[feature]-0", + expected: "[feature]-go", desc: "Custom template hash only", gitManager: GitManager{customTemplates: CustomTemplates{branchNameTemplate: "[feature]-${BRANCH_NAME_HASH}"}}, }, } for _, test := range tests { t.Run(test.desc, func(t *testing.T) { - titleOutput, err := test.gitManager.GenerateAggregatedFixBranchName() + titleOutput, err := test.gitManager.GenerateAggregatedFixBranchName(coreutils.Go) assert.NoError(t, err) assert.Equal(t, test.expected, titleOutput) }) @@ -149,12 +150,12 @@ func TestGitManager_GenerateAggregatedCommitMessage(t *testing.T) { gitManager GitManager expected string }{ - {gitManager: GitManager{}, expected: AggregatedPullRequestTitleTemplate}, + {gitManager: GitManager{}, expected: GetAggregatedPullRequestTitle(coreutils.Pipenv)}, {gitManager: GitManager{customTemplates: CustomTemplates{commitMessageTemplate: "custom_template"}}, expected: "custom_template"}, } for _, test := range tests { t.Run(test.expected, func(t *testing.T) { - commit := test.gitManager.GenerateAggregatedCommitMessage() + commit := test.gitManager.GenerateAggregatedCommitMessage(coreutils.Pipenv) assert.Equal(t, commit, test.expected) }) } diff --git a/commands/utils/outputwriter.go b/commands/utils/outputwriter.go index 1afbabffa..9bfffd0d8 100644 --- a/commands/utils/outputwriter.go +++ b/commands/utils/outputwriter.go @@ -3,6 +3,7 @@ package utils import ( "fmt" "github.com/jfrog/froggit-go/vcsutils" + "github.com/jfrog/jfrog-cli-core/v2/utils/coreutils" "github.com/jfrog/jfrog-cli-core/v2/xray/formats" "github.com/jfrog/jfrog-cli-core/v2/xray/utils" "strings" @@ -15,7 +16,7 @@ type OutputWriter interface { NoVulnerabilitiesTitle() string VulnerabiltiesTitle(isComment bool) string VulnerabilitiesTableHeader() string - VulnerabilitiesContent(vulnerabilitiesRows []formats.VulnerabilityOrViolationRow) string + VulnerabilitiesContent(vulnerabilities []formats.VulnerabilityOrViolationRow) string IacContent(iacRows []formats.IacSecretsRow) string Footer() string Seperator() string @@ -42,13 +43,14 @@ type descriptionBullet struct { value string } -func createVulnerabilityDescription(vulnerabilityDetails *formats.VulnerabilityOrViolationRow) string { +func createVulnerabilityDescription(vulnerability *formats.VulnerabilityOrViolationRow) string { var cves []string - for _, cve := range vulnerabilityDetails.Cves { + for _, cve := range vulnerability.Cves { cves = append(cves, cve.Id) } - if vulnerabilityDetails.JfrogResearchInformation == nil { - vulnerabilityDetails.JfrogResearchInformation = &formats.JfrogResearchInformation{Details: vulnerabilityDetails.Summary} + + if vulnerability.JfrogResearchInformation == nil { + vulnerability.JfrogResearchInformation = &formats.JfrogResearchInformation{Details: vulnerability.Summary} } cvesTitle := "**CVE:**" @@ -56,12 +58,17 @@ func createVulnerabilityDescription(vulnerabilityDetails *formats.VulnerabilityO cvesTitle = "**CVEs:**" } + fixedVersionsTitle := "**Fixed Version:**" + if len(vulnerability.FixedVersions) > 1 { + fixedVersionsTitle = "**Fixed Versions:**" + } + descriptionBullets := []descriptionBullet{ - {title: "**Severity**", value: fmt.Sprintf("%s %s", utils.GetSeverity(vulnerabilityDetails.Severity, utils.ApplicableStringValue).Emoji(), vulnerabilityDetails.Severity)}, - {title: "**Contextual Analysis:**", value: vulnerabilityDetails.Applicable}, - {title: "**Package Name:**", value: vulnerabilityDetails.ImpactedDependencyName}, - {title: "**Current Version:**", value: vulnerabilityDetails.ImpactedDependencyVersion}, - {title: "**Fixed Version:**", value: strings.Join(vulnerabilityDetails.FixedVersions, ",")}, + {title: "**Severity**", value: fmt.Sprintf("%s %s", utils.GetSeverity(vulnerability.Severity, utils.ApplicableStringValue).Emoji(), vulnerability.Severity)}, + {title: "**Contextual Analysis:**", value: vulnerability.Applicable}, + {title: "**Package Name:**", value: vulnerability.ImpactedDependencyName}, + {title: "**Current Version:**", value: vulnerability.ImpactedDependencyVersion}, + {title: fixedVersionsTitle, value: strings.Join(vulnerability.FixedVersions, ",")}, {title: cvesTitle, value: strings.Join(cves, ", ")}, } @@ -75,21 +82,21 @@ func createVulnerabilityDescription(vulnerabilityDetails *formats.VulnerabilityO } // Write description if exists: - if vulnerabilityDetails.JfrogResearchInformation.Details != "" { - descriptionBuilder.WriteString(fmt.Sprintf("\n**Description:**\n\n%s\n\n", vulnerabilityDetails.JfrogResearchInformation.Details)) + if vulnerability.JfrogResearchInformation.Details != "" { + descriptionBuilder.WriteString(fmt.Sprintf("\n**Description:**\n\n%s\n\n", vulnerability.JfrogResearchInformation.Details)) } // Write remediation if exists - if vulnerabilityDetails.JfrogResearchInformation.Remediation != "" { - descriptionBuilder.WriteString(fmt.Sprintf("**Remediation:**\n\n%s\n\n", vulnerabilityDetails.JfrogResearchInformation.Remediation)) + if vulnerability.JfrogResearchInformation.Remediation != "" { + descriptionBuilder.WriteString(fmt.Sprintf("**Remediation:**\n\n%s\n\n", vulnerability.JfrogResearchInformation.Remediation)) } return descriptionBuilder.String() } -func getVulnerabilitiesTableContent(vulnerabilitiesRows []formats.VulnerabilityOrViolationRow, writer OutputWriter) string { +func getVulnerabilitiesTableContent(vulnerabilities []formats.VulnerabilityOrViolationRow, writer OutputWriter) string { var tableContent string - for _, vulnerability := range vulnerabilitiesRows { + for _, vulnerability := range vulnerabilities { tableContent += "\n" + writer.VulnerabilitiesTableRow(vulnerability) } return tableContent @@ -102,3 +109,14 @@ func getIacTableContent(iacRows []formats.IacSecretsRow, writer OutputWriter) st } return tableContent } + +func MarkdownComment(text string) string { + return fmt.Sprintf("\n[comment]: <> (%s)\n", text) +} + +func GetAggregatedPullRequestTitle(tech coreutils.Technology) string { + if tech.ToString() == "" { + return FrogbotPullRequestTitlePrefix + " Update dependencies" + } + return fmt.Sprintf("%s Update %s dependencies", FrogbotPullRequestTitlePrefix, tech.ToFormal()) +} diff --git a/commands/utils/outputwriter_test.go b/commands/utils/outputwriter_test.go new file mode 100644 index 000000000..69d51327d --- /dev/null +++ b/commands/utils/outputwriter_test.go @@ -0,0 +1,25 @@ +package utils + +import ( + "github.com/jfrog/jfrog-cli-core/v2/utils/coreutils" + "github.com/stretchr/testify/assert" + "testing" +) + +func TestGetAggregatedPullRequestTitle(t *testing.T) { + tests := []struct { + tech coreutils.Technology + expected string + }{ + {tech: "", expected: "[🐸 Frogbot] Update dependencies"}, + {tech: coreutils.Maven, expected: "[🐸 Frogbot] Update Maven dependencies"}, + {tech: coreutils.Gradle, expected: "[🐸 Frogbot] Update Gradle dependencies"}, + {tech: coreutils.Npm, expected: "[🐸 Frogbot] Update npm dependencies"}, + {tech: coreutils.Yarn, expected: "[🐸 Frogbot] Update Yarn dependencies"}, + } + + for _, test := range tests { + title := GetAggregatedPullRequestTitle(test.tech) + assert.Equal(t, test.expected, title) + } +} diff --git a/commands/utils/packagehandlers/commonpackagehandler.go b/commands/utils/packagehandlers/commonpackagehandler.go index 747ced01a..b5b29cdeb 100644 --- a/commands/utils/packagehandlers/commonpackagehandler.go +++ b/commands/utils/packagehandlers/commonpackagehandler.go @@ -45,7 +45,7 @@ func (cph *CommonPackageHandler) UpdateDependency(vulnDetails *utils.Vulnerabili commandArgs := []string{vulnDetails.Technology.GetPackageInstallOperator()} commandArgs = append(commandArgs, extraArgs...) operator := vulnDetails.Technology.GetPackageOperator() - fixedPackage := impactedPackage + operator + vulnDetails.FixVersion + fixedPackage := impactedPackage + operator + vulnDetails.SuggestedFixedVersion commandArgs = append(commandArgs, fixedPackage) return runPackageMangerCommand(vulnDetails.Technology.GetExecCommandName(), commandArgs) } diff --git a/commands/utils/packagehandlers/mavenpackagehandler.go b/commands/utils/packagehandlers/mavenpackagehandler.go index ec2b0783c..9231e8f8e 100644 --- a/commands/utils/packagehandlers/mavenpackagehandler.go +++ b/commands/utils/packagehandlers/mavenpackagehandler.go @@ -11,7 +11,7 @@ import ( rtutils "github.com/jfrog/jfrog-cli-core/v2/artifactory/utils" "github.com/jfrog/jfrog-cli-core/v2/utils/config" mvnutils "github.com/jfrog/jfrog-cli-core/v2/utils/mvn" - audit "github.com/jfrog/jfrog-cli-core/v2/xray/commands/audit/generic" + "github.com/jfrog/jfrog-cli-core/v2/xray/audit/java" "github.com/jfrog/jfrog-client-go/utils/log" "golang.org/x/exp/slices" "io" @@ -180,15 +180,15 @@ func (mph *MavenPackageHandler) UpdateDependency(vulnDetails *utils.Vulnerabilit if depDetails, exists = mph.mavenDepToPropertyMap[impactedDependency]; !exists { return &utils.ErrUnsupportedFix{ PackageName: vulnDetails.ImpactedDependencyName, - FixedVersion: vulnDetails.FixVersion, + FixedVersion: vulnDetails.SuggestedFixedVersion, ErrorType: utils.IndirectDependencyFixNotSupported, } } if len(depDetails.properties) > 0 { - return mph.updateProperties(&depDetails, vulnDetails.FixVersion) + return mph.updateProperties(&depDetails, vulnDetails.SuggestedFixedVersion) } - return mph.updatePackageVersion(vulnDetails.ImpactedDependencyName, vulnDetails.FixVersion, depDetails.foundInDependencyManagement) + return mph.updatePackageVersion(vulnDetails.ImpactedDependencyName, vulnDetails.SuggestedFixedVersion, depDetails.foundInDependencyManagement) } func (mph *MavenPackageHandler) installMavenGavReader() (err error) { @@ -304,8 +304,8 @@ func (mph *MavenPackageHandler) runMvnCommand(goals []string) (readerOutput []by return } // Run the mvn command with the Maven Build-Info Extractor to download dependencies from Artifactory. - javaProps := audit.CreateJavaProps(mph.depsRepo, mph.ServerDetails) - vConfig, err := rtutils.ReadMavenConfig("", javaProps) + mvnProps := java.CreateMvnProps(mph.depsRepo, mph.ServerDetails) + vConfig, err := rtutils.ReadMavenConfig("", mvnProps) if err != nil { return } diff --git a/commands/utils/packagehandlers/npmpackagehandler.go b/commands/utils/packagehandlers/npmpackagehandler.go index 00167facf..28df0eb57 100644 --- a/commands/utils/packagehandlers/npmpackagehandler.go +++ b/commands/utils/packagehandlers/npmpackagehandler.go @@ -12,7 +12,7 @@ func (npm *NpmPackageHandler) UpdateDependency(vulnDetails *utils.VulnerabilityD } else { return &utils.ErrUnsupportedFix{ PackageName: vulnDetails.ImpactedDependencyName, - FixedVersion: vulnDetails.FixVersion, + FixedVersion: vulnDetails.SuggestedFixedVersion, ErrorType: utils.IndirectDependencyFixNotSupported, } } diff --git a/commands/utils/packagehandlers/packagehandlers_test.go b/commands/utils/packagehandlers/packagehandlers_test.go index 06852a1f5..e250aa550 100644 --- a/commands/utils/packagehandlers/packagehandlers_test.go +++ b/commands/utils/packagehandlers/packagehandlers_test.go @@ -39,21 +39,21 @@ func TestGoPackageHandler_UpdateDependency(t *testing.T) { testcases := []dependencyFixTest{ { vulnDetails: &utils.VulnerabilityDetails{ - FixVersion: "0.0.0-20201216223049-8b5274cf687f", + SuggestedFixedVersion: "0.0.0-20201216223049-8b5274cf687f", IsDirectDependency: false, VulnerabilityOrViolationRow: &formats.VulnerabilityOrViolationRow{Technology: coreutils.Go, ImpactedDependencyName: "golang.org/x/crypto"}, }, fixSupported: true, }, { vulnDetails: &utils.VulnerabilityDetails{ - FixVersion: "1.7.7", + SuggestedFixedVersion: "1.7.7", IsDirectDependency: true, VulnerabilityOrViolationRow: &formats.VulnerabilityOrViolationRow{Technology: coreutils.Go, ImpactedDependencyName: "github.com/gin-gonic/gin"}, }, fixSupported: true, }, { vulnDetails: &utils.VulnerabilityDetails{ - FixVersion: "1.3.0", + SuggestedFixedVersion: "1.3.0", IsDirectDependency: true, VulnerabilityOrViolationRow: &formats.VulnerabilityOrViolationRow{Technology: coreutils.Go, ImpactedDependencyName: "github.com/google/uuid"}, }, fixSupported: true, @@ -82,49 +82,49 @@ func TestPythonPackageHandler_UpdateDependency(t *testing.T) { testcases := []pythonIndirectDependencies{ {dependencyFixTest: dependencyFixTest{ vulnDetails: &utils.VulnerabilityDetails{ - FixVersion: "1.25.9", + SuggestedFixedVersion: "1.25.9", VulnerabilityOrViolationRow: &formats.VulnerabilityOrViolationRow{Technology: coreutils.Pip, ImpactedDependencyName: "urllib3"}, }}, requirementsPath: "requirements.txt"}, {dependencyFixTest: dependencyFixTest{ vulnDetails: &utils.VulnerabilityDetails{ - FixVersion: "1.25.9", + SuggestedFixedVersion: "1.25.9", VulnerabilityOrViolationRow: &formats.VulnerabilityOrViolationRow{Technology: coreutils.Poetry, ImpactedDependencyName: "urllib3"}}}, requirementsPath: "pyproejct.toml"}, {dependencyFixTest: dependencyFixTest{ vulnDetails: &utils.VulnerabilityDetails{ - FixVersion: "1.25.9", + SuggestedFixedVersion: "1.25.9", VulnerabilityOrViolationRow: &formats.VulnerabilityOrViolationRow{Technology: coreutils.Pipenv, ImpactedDependencyName: "urllib3"}}}, requirementsPath: "Pipfile"}, {dependencyFixTest: dependencyFixTest{ vulnDetails: &utils.VulnerabilityDetails{ - FixVersion: "2.4.0", + SuggestedFixedVersion: "2.4.0", VulnerabilityOrViolationRow: &formats.VulnerabilityOrViolationRow{Technology: coreutils.Pip, ImpactedDependencyName: "pyjwt"}, IsDirectDependency: true}, fixSupported: true}, requirementsPath: "requirements.txt"}, {dependencyFixTest: dependencyFixTest{ vulnDetails: &utils.VulnerabilityDetails{ - FixVersion: "2.4.0", + SuggestedFixedVersion: "2.4.0", VulnerabilityOrViolationRow: &formats.VulnerabilityOrViolationRow{Technology: coreutils.Pip, ImpactedDependencyName: "Pyjwt"}, IsDirectDependency: true}, fixSupported: true}, requirementsPath: "requirements.txt"}, {dependencyFixTest: dependencyFixTest{ vulnDetails: &utils.VulnerabilityDetails{ - FixVersion: "2.4.0", + SuggestedFixedVersion: "2.4.0", VulnerabilityOrViolationRow: &formats.VulnerabilityOrViolationRow{Technology: coreutils.Pip, ImpactedDependencyName: "pyjwt"}, IsDirectDependency: true}, fixSupported: true}, requirementsPath: "setup.py"}, {dependencyFixTest: dependencyFixTest{ vulnDetails: &utils.VulnerabilityDetails{ - FixVersion: "2.4.0", + SuggestedFixedVersion: "2.4.0", VulnerabilityOrViolationRow: &formats.VulnerabilityOrViolationRow{Technology: coreutils.Poetry, ImpactedDependencyName: "pyjwt"}, IsDirectDependency: true}, fixSupported: true}, requirementsPath: "pyproject.toml"}, {dependencyFixTest: dependencyFixTest{ vulnDetails: &utils.VulnerabilityDetails{ - FixVersion: "2.4.0", + SuggestedFixedVersion: "2.4.0", VulnerabilityOrViolationRow: &formats.VulnerabilityOrViolationRow{Technology: coreutils.Poetry, ImpactedDependencyName: "pyjwt"}, IsDirectDependency: true}, fixSupported: true}, @@ -176,13 +176,13 @@ func TestNpmPackageHandler_UpdateDependency(t *testing.T) { testcases := []dependencyFixTest{ { vulnDetails: &utils.VulnerabilityDetails{ - FixVersion: "0.8.4", + SuggestedFixedVersion: "0.8.4", VulnerabilityOrViolationRow: &formats.VulnerabilityOrViolationRow{Technology: coreutils.Npm, ImpactedDependencyName: "mpath"}, }, fixSupported: false, }, { vulnDetails: &utils.VulnerabilityDetails{ - FixVersion: "3.0.2", + SuggestedFixedVersion: "3.0.2", IsDirectDependency: true, VulnerabilityOrViolationRow: &formats.VulnerabilityOrViolationRow{Technology: coreutils.Npm, ImpactedDependencyName: "minimatch"}, }, fixSupported: true, @@ -211,13 +211,13 @@ func TestYarnPackageHandler_UpdateDependency(t *testing.T) { testcases := []dependencyFixTest{ { vulnDetails: &utils.VulnerabilityDetails{ - FixVersion: "1.2.6", + SuggestedFixedVersion: "1.2.6", VulnerabilityOrViolationRow: &formats.VulnerabilityOrViolationRow{Technology: coreutils.Yarn, ImpactedDependencyName: "minimist"}, }, fixSupported: false, }, { vulnDetails: &utils.VulnerabilityDetails{ - FixVersion: "1.2.6", + SuggestedFixedVersion: "1.2.6", IsDirectDependency: true, VulnerabilityOrViolationRow: &formats.VulnerabilityOrViolationRow{Technology: coreutils.Yarn, ImpactedDependencyName: "minimist"}, }, fixSupported: true, @@ -244,11 +244,11 @@ func TestYarnPackageHandler_UpdateDependency(t *testing.T) { func TestMavenPackageHandler_UpdateDependency(t *testing.T) { tests := []dependencyFixTest{ {vulnDetails: &utils.VulnerabilityDetails{ - FixVersion: "2.7", + SuggestedFixedVersion: "2.7", VulnerabilityOrViolationRow: &formats.VulnerabilityOrViolationRow{Technology: coreutils.Maven, ImpactedDependencyName: "commons-io:commons-io"}, IsDirectDependency: true}, fixSupported: true}, {vulnDetails: &utils.VulnerabilityDetails{ - FixVersion: "4.3.20", + SuggestedFixedVersion: "4.3.20", VulnerabilityOrViolationRow: &formats.VulnerabilityOrViolationRow{Technology: coreutils.Maven, ImpactedDependencyName: "org.springframework:spring-core"}, IsDirectDependency: false}, fixSupported: false}, } @@ -469,14 +469,14 @@ func TestFixVersionInfo_UpdateFixVersionIfMax(t *testing.T) { } testCases := []testCase{ - {fixVersionInfo: utils.VulnerabilityDetails{FixVersion: "1.2.3", IsDirectDependency: true}, newFixVersion: "1.2.4", expectedOutput: "1.2.4"}, - {fixVersionInfo: utils.VulnerabilityDetails{FixVersion: "1.2.3", IsDirectDependency: true}, newFixVersion: "1.0.4", expectedOutput: "1.2.3"}, + {fixVersionInfo: utils.VulnerabilityDetails{SuggestedFixedVersion: "1.2.3", IsDirectDependency: true}, newFixVersion: "1.2.4", expectedOutput: "1.2.4"}, + {fixVersionInfo: utils.VulnerabilityDetails{SuggestedFixedVersion: "1.2.3", IsDirectDependency: true}, newFixVersion: "1.0.4", expectedOutput: "1.2.3"}, } for _, tc := range testCases { t.Run(tc.expectedOutput, func(t *testing.T) { tc.fixVersionInfo.UpdateFixVersionIfMax(tc.newFixVersion) - assert.Equal(t, tc.expectedOutput, tc.fixVersionInfo.FixVersion) + assert.Equal(t, tc.expectedOutput, tc.fixVersionInfo.SuggestedFixedVersion) }) } } @@ -556,7 +556,7 @@ func assertFixVersionInPackageDescriptor(t *testing.T, test dependencyFixTest, p if !test.fixSupported { assert.NotContains(t, string(file), test.vulnDetails) } else { - assert.Contains(t, string(file), test.vulnDetails.FixVersion) + assert.Contains(t, string(file), test.vulnDetails.SuggestedFixedVersion) // Verify that case-sensitive packages in python are lowered assert.Contains(t, string(file), strings.ToLower(test.vulnDetails.ImpactedDependencyName)) } diff --git a/commands/utils/packagehandlers/pythonpackagehandler.go b/commands/utils/packagehandlers/pythonpackagehandler.go index 3336bfd6e..47c7978a4 100644 --- a/commands/utils/packagehandlers/pythonpackagehandler.go +++ b/commands/utils/packagehandlers/pythonpackagehandler.go @@ -32,7 +32,7 @@ func (py *PythonPackageHandler) UpdateDependency(vulnDetails *utils.Vulnerabilit return &utils.ErrUnsupportedFix{ PackageName: vulnDetails.ImpactedDependencyName, - FixedVersion: vulnDetails.FixVersion, + FixedVersion: vulnDetails.SuggestedFixedVersion, ErrorType: utils.IndirectDependencyFixNotSupported, } } @@ -62,7 +62,7 @@ func (py *PythonPackageHandler) handlePoetry(vulnDetails *utils.VulnerabilityDet func (py *PythonPackageHandler) handlePip(vulnDetails *utils.VulnerabilityDetails) (err error) { var fixedFile string // This function assumes that the version of the dependencies is statically pinned in the requirements file or inside the 'install_requires' array in the setup.py file - fixedPackage := vulnDetails.ImpactedDependencyName + "==" + vulnDetails.FixVersion + fixedPackage := vulnDetails.ImpactedDependencyName + "==" + vulnDetails.SuggestedFixedVersion if py.pipRequirementsFile == "" { py.pipRequirementsFile = "setup.py" } @@ -90,7 +90,7 @@ func (py *PythonPackageHandler) handlePip(vulnDetails *utils.VulnerabilityDetail return fmt.Errorf("impacted package %s not found, fix failed", vulnDetails.ImpactedDependencyName) } if err = os.WriteFile(py.pipRequirementsFile, []byte(fixedFile), 0600); err != nil { - err = fmt.Errorf("an error occured while writing the fixed version of %s to the requirements file:\n%s", vulnDetails.FixVersion, err.Error()) + err = fmt.Errorf("an error occured while writing the fixed version of %s to the requirements file:\n%s", vulnDetails.SuggestedFixedVersion, err.Error()) } return } diff --git a/commands/utils/packagehandlers/yarnpackagehandler.go b/commands/utils/packagehandlers/yarnpackagehandler.go index 6e11157b2..7db3d75e3 100644 --- a/commands/utils/packagehandlers/yarnpackagehandler.go +++ b/commands/utils/packagehandlers/yarnpackagehandler.go @@ -12,7 +12,7 @@ func (yarn *YarnPackageHandler) UpdateDependency(vulnDetails *utils.Vulnerabilit } else { return &utils.ErrUnsupportedFix{ PackageName: vulnDetails.ImpactedDependencyName, - FixedVersion: vulnDetails.FixVersion, + FixedVersion: vulnDetails.SuggestedFixedVersion, ErrorType: utils.IndirectDependencyFixNotSupported, } } diff --git a/commands/utils/simplifiedoutput.go b/commands/utils/simplifiedoutput.go index 374958afb..dfcb4426d 100644 --- a/commands/utils/simplifiedoutput.go +++ b/commands/utils/simplifiedoutput.go @@ -79,7 +79,7 @@ func (smo *SimplifiedOutput) EntitledForJas() bool { return smo.entitledForJas } -func (smo *SimplifiedOutput) VulnerabilitiesContent(vulnerabilitiesRows []formats.VulnerabilityOrViolationRow) string { +func (smo *SimplifiedOutput) VulnerabilitiesContent(vulnerabilities []formats.VulnerabilityOrViolationRow) string { var contentBuilder strings.Builder // Write summary table part contentBuilder.WriteString(fmt.Sprintf(` @@ -97,17 +97,17 @@ func (smo *SimplifiedOutput) VulnerabilitiesContent(vulnerabilitiesRows []format `, smo.VulnerabilitiesTableHeader(), - getVulnerabilitiesTableContent(vulnerabilitiesRows, smo))) - for i := range vulnerabilitiesRows { + getVulnerabilitiesTableContent(vulnerabilities, smo))) + for i := range vulnerabilities { contentBuilder.WriteString(fmt.Sprintf(` #### %s %s %s `, - vulnerabilitiesRows[i].ImpactedDependencyName, - vulnerabilitiesRows[i].ImpactedDependencyVersion, - createVulnerabilityDescription(&vulnerabilitiesRows[i]))) + vulnerabilities[i].ImpactedDependencyName, + vulnerabilities[i].ImpactedDependencyVersion, + createVulnerabilityDescription(&vulnerabilities[i]))) } return contentBuilder.String() diff --git a/commands/utils/standardoutput.go b/commands/utils/standardoutput.go index 10f282290..3976dc6ec 100644 --- a/commands/utils/standardoutput.go +++ b/commands/utils/standardoutput.go @@ -79,7 +79,7 @@ func (so *StandardOutput) EntitledForJas() bool { return so.entitledForJas } -func (so *StandardOutput) VulnerabilitiesContent(vulnerabilitiesRows []formats.VulnerabilityOrViolationRow) string { +func (so *StandardOutput) VulnerabilitiesContent(vulnerabilities []formats.VulnerabilityOrViolationRow) string { var contentBuilder strings.Builder // Write summary table part contentBuilder.WriteString(fmt.Sprintf(` @@ -97,15 +97,15 @@ func (so *StandardOutput) VulnerabilitiesContent(vulnerabilitiesRows []formats.V `, so.VulnerabilitiesTableHeader(), - getVulnerabilitiesTableContent(vulnerabilitiesRows, so))) + getVulnerabilitiesTableContent(vulnerabilities, so))) // Write details for each vulnerability - for i := range vulnerabilitiesRows { - if len(vulnerabilitiesRows) == 1 { + for i := range vulnerabilities { + if len(vulnerabilities) == 1 { contentBuilder.WriteString(fmt.Sprintf(` %s -`, createVulnerabilityDescription(&vulnerabilitiesRows[i]))) +`, createVulnerabilityDescription(&vulnerabilities[i]))) break } contentBuilder.WriteString(fmt.Sprintf(` @@ -117,9 +117,9 @@ func (so *StandardOutput) VulnerabilitiesContent(vulnerabilitiesRows []formats.V `, - vulnerabilitiesRows[i].ImpactedDependencyName, - vulnerabilitiesRows[i].ImpactedDependencyVersion, - createVulnerabilityDescription(&vulnerabilitiesRows[i]))) + vulnerabilities[i].ImpactedDependencyName, + vulnerabilities[i].ImpactedDependencyVersion, + createVulnerabilityDescription(&vulnerabilities[i]))) } return contentBuilder.String() } diff --git a/commands/utils/utils.go b/commands/utils/utils.go index e854d52e1..8c7064b03 100644 --- a/commands/utils/utils.go +++ b/commands/utils/utils.go @@ -71,7 +71,7 @@ func (err *ErrUnsupportedFix) Error() string { type VulnerabilityDetails struct { *formats.VulnerabilityOrViolationRow // Suggested fix version - FixVersion string + SuggestedFixedVersion string // States whether the dependency is direct or transitive IsDirectDependency bool // Cves as a list of string @@ -81,7 +81,7 @@ type VulnerabilityDetails struct { func NewVulnerabilityDetails(vulnerability *formats.VulnerabilityOrViolationRow, fixVersion string) *VulnerabilityDetails { vulnDetails := &VulnerabilityDetails{ VulnerabilityOrViolationRow: vulnerability, - FixVersion: fixVersion, + SuggestedFixedVersion: fixVersion, } vulnDetails.SetCves(vulnerability.Cves) return vulnDetails @@ -99,8 +99,8 @@ func (vd *VulnerabilityDetails) SetCves(cves []formats.CveRow) { func (vd *VulnerabilityDetails) UpdateFixVersionIfMax(fixVersion string) { // Update vd.FixVersion as the maximum version if found a new version that is greater than the previous maximum version. - if vd.FixVersion == "" || version.NewVersion(vd.FixVersion).Compare(fixVersion) > 0 { - vd.FixVersion = fixVersion + if vd.SuggestedFixedVersion == "" || version.NewVersion(vd.SuggestedFixedVersion).Compare(fixVersion) > 0 { + vd.SuggestedFixedVersion = fixVersion } } @@ -131,7 +131,7 @@ func Chdir(dir string) (cbk func() error, err error) { return nil, err } if err = os.Chdir(dir); err != nil { - return nil, err + return nil, fmt.Errorf("could not change dir to: %s\n%s", dir, err.Error()) } return func() error { return os.Chdir(wd) }, err } @@ -172,19 +172,19 @@ func Md5Hash(values ...string) (string, error) { // Generates MD5Hash from a vulnerabilityDetails // The map can be returned in different order from Xray, so we need to sort the strings before hashing. -func VulnerabilityDetailsToMD5Hash(vulnerabilityDetails map[string]*VulnerabilityDetails) (string, error) { - h := crypto.MD5.New() - keys := make([]string, 0, len(vulnerabilityDetails)) - for k, v := range vulnerabilityDetails { - keys = append(keys, k+v.FixVersion) +func VulnerabilityDetailsToMD5Hash(vulnerabilities ...*VulnerabilityDetails) (string, error) { + hash := crypto.MD5.New() + var keys []string + for _, vuln := range vulnerabilities { + keys = append(keys, GetUniqueID(*vuln.VulnerabilityOrViolationRow)) } sort.Strings(keys) for key, value := range keys { - if _, err := fmt.Fprint(h, key, value); err != nil { + if _, err := fmt.Fprint(hash, key, value); err != nil { return "", err } } - return hex.EncodeToString(h.Sum(nil)), nil + return hex.EncodeToString(hash.Sum(nil)), nil } // UploadScanToGitProvider uploads scan results to the relevant git provider in order to view the scan in the Git provider code scanning UI @@ -287,3 +287,7 @@ func BuildServerConfigFile(server *config.ServerDetails) (previousJFrogHomeDir, err = cc.Run() return } + +func GetUniqueID(vulnerability formats.VulnerabilityOrViolationRow) string { + return vulnerability.ImpactedDependencyName + vulnerability.ImpactedDependencyVersion + vulnerability.IssueId +} diff --git a/commands/utils/utils_test.go b/commands/utils/utils_test.go index 14656eb96..fe37cc1ff 100644 --- a/commands/utils/utils_test.go +++ b/commands/utils/utils_test.go @@ -131,34 +131,34 @@ func TestMd5Hash(t *testing.T) { func TestFixVersionsMapToMd5Hash(t *testing.T) { tests := []struct { - fixVersionMap map[string]*VulnerabilityDetails - expectedHash string + vulnerabilities []*VulnerabilityDetails + expectedHash string }{ { - fixVersionMap: map[string]*VulnerabilityDetails{ - "pkg": {FixVersion: "1.2.3", VulnerabilityOrViolationRow: &formats.VulnerabilityOrViolationRow{Technology: coreutils.Npm}, IsDirectDependency: false}}, - expectedHash: "0aa066970b613b114f8e21d11c74ff94", + vulnerabilities: []*VulnerabilityDetails{ + {SuggestedFixedVersion: "1.2.3", VulnerabilityOrViolationRow: &formats.VulnerabilityOrViolationRow{ImpactedDependencyName: "pkg", Technology: coreutils.Npm}, IsDirectDependency: false}}, + expectedHash: "8a5f1a3a7f11a825f3e0546f74d5dd18", }, { - fixVersionMap: map[string]*VulnerabilityDetails{ - "pkg": {FixVersion: "5.2.3", VulnerabilityOrViolationRow: &formats.VulnerabilityOrViolationRow{Technology: coreutils.Go}, IsDirectDependency: false}, - "pkg2": {FixVersion: "1.2.3", VulnerabilityOrViolationRow: &formats.VulnerabilityOrViolationRow{Technology: coreutils.Go}, IsDirectDependency: false}}, - expectedHash: "a0d4119dfe5fc5186d6c2cf1497f8c7c", + vulnerabilities: []*VulnerabilityDetails{ + {SuggestedFixedVersion: "5.2.3", VulnerabilityOrViolationRow: &formats.VulnerabilityOrViolationRow{ImpactedDependencyName: "pkg", Technology: coreutils.Go}, IsDirectDependency: false}, + {SuggestedFixedVersion: "1.2.3", VulnerabilityOrViolationRow: &formats.VulnerabilityOrViolationRow{ImpactedDependencyName: "pkg2", Technology: coreutils.Go}, IsDirectDependency: false}}, + expectedHash: "984b2585da84b62146f8d2ec0fd12e0e", }, { // The Same map with different order should be the same hash. - fixVersionMap: map[string]*VulnerabilityDetails{ - "pkg2": {FixVersion: "1.2.3", VulnerabilityOrViolationRow: &formats.VulnerabilityOrViolationRow{Technology: coreutils.Go}, IsDirectDependency: false}, - "pkg": {FixVersion: "5.2.3", VulnerabilityOrViolationRow: &formats.VulnerabilityOrViolationRow{Technology: coreutils.Go}, IsDirectDependency: false}}, - expectedHash: "a0d4119dfe5fc5186d6c2cf1497f8c7c", + vulnerabilities: []*VulnerabilityDetails{ + {SuggestedFixedVersion: "1.2.3", VulnerabilityOrViolationRow: &formats.VulnerabilityOrViolationRow{ImpactedDependencyName: "pkg2", Technology: coreutils.Go}, IsDirectDependency: false}, + {SuggestedFixedVersion: "5.2.3", VulnerabilityOrViolationRow: &formats.VulnerabilityOrViolationRow{ImpactedDependencyName: "pkg", Technology: coreutils.Go}, IsDirectDependency: false}}, + expectedHash: "984b2585da84b62146f8d2ec0fd12e0e", }, { - fixVersionMap: map[string]*VulnerabilityDetails{ - "myNuget": {FixVersion: "0.2.33", VulnerabilityOrViolationRow: &formats.VulnerabilityOrViolationRow{Technology: coreutils.Nuget}, IsDirectDependency: false}}, - expectedHash: "887ac2c931920c20956409702c0dfbc7", + vulnerabilities: []*VulnerabilityDetails{ + {SuggestedFixedVersion: "0.2.33", VulnerabilityOrViolationRow: &formats.VulnerabilityOrViolationRow{ImpactedDependencyName: "myNuget", Technology: coreutils.Nuget}, IsDirectDependency: false}}, + expectedHash: "1708946f489ed70c754dd1ceef49b50b", }, } for _, test := range tests { t.Run(test.expectedHash, func(t *testing.T) { - hash, err := VulnerabilityDetailsToMD5Hash(test.fixVersionMap) + hash, err := VulnerabilityDetailsToMD5Hash(test.vulnerabilities...) assert.NoError(t, err) assert.Equal(t, test.expectedHash, hash) }) diff --git a/go.mod b/go.mod index 19554be5f..d06e9bf00 100644 --- a/go.mod +++ b/go.mod @@ -6,9 +6,9 @@ require ( github.com/go-git/go-git/v5 v5.7.0 github.com/golang/mock v1.6.0 github.com/jfrog/build-info-go v1.9.6 - github.com/jfrog/froggit-go v1.8.1 + github.com/jfrog/froggit-go v1.9.0 github.com/jfrog/gofrog v1.3.0 - github.com/jfrog/jfrog-cli-core/v2 v2.39.2 + github.com/jfrog/jfrog-cli-core/v2 v2.39.3 github.com/jfrog/jfrog-client-go v1.31.2 github.com/mholt/archiver/v3 v3.5.1 github.com/stretchr/testify v1.8.4 @@ -114,6 +114,6 @@ require ( gopkg.in/warnings.v0 v0.1.2 // indirect ) -replace github.com/jfrog/jfrog-cli-core/v2 => github.com/jfrog/jfrog-cli-core/v2 v2.31.1-0.20230720085538-4befe41129f8 +//replace github.com/jfrog/jfrog-cli-core/v2 => github.com/jfrog/jfrog-cli-core/v2 v2.31.1-0.20230720085538-4befe41129f8 // replace github.com/jfrog/jfrog-client-go => github.com/jfrog/jfrog-client-go v1.28.1-0.20230611131847-a3b84a9004c3 diff --git a/go.sum b/go.sum index fe4094e4c..fa94e5e27 100644 --- a/go.sum +++ b/go.sum @@ -220,12 +220,12 @@ github.com/jedib0t/go-pretty/v6 v6.4.6 h1:v6aG9h6Uby3IusSSEjHaZNXpHFhzqMmjXcPq1R github.com/jedib0t/go-pretty/v6 v6.4.6/go.mod h1:Ndk3ase2CkQbXLLNf5QDHoYb6J9WtVfmHZu9n8rk2xs= github.com/jfrog/build-info-go v1.9.6 h1:lCJ2j5uXAlJsSwDe5J8WD7Co1f/hUlZvMfwfb5AzLJU= github.com/jfrog/build-info-go v1.9.6/go.mod h1:GbuFS+viHCKZYx9nWHYu7ab1DgQkFdtVN3BJPUNb2D4= -github.com/jfrog/froggit-go v1.8.1 h1:3eT1gQ7/Snft181Ig7EZlZSaXlr++tskdxgoCJKmAmw= -github.com/jfrog/froggit-go v1.8.1/go.mod h1:XTFf4RqWwZsF9pdERt0Ra5gO1O3iqIq7Npx2tEQSGAQ= +github.com/jfrog/froggit-go v1.9.0 h1:cLdQJHZOph+mEvS1QiF84X7qonDKP1paTyr2bQcuGr0= +github.com/jfrog/froggit-go v1.9.0/go.mod h1:XTFf4RqWwZsF9pdERt0Ra5gO1O3iqIq7Npx2tEQSGAQ= github.com/jfrog/gofrog v1.3.0 h1:o4zgsBZE4QyDbz2M7D4K6fXPTBJht+8lE87mS9bw7Gk= github.com/jfrog/gofrog v1.3.0/go.mod h1:IFMc+V/yf7rA5WZ74CSbXe+Lgf0iApEQLxRZVzKRUR0= -github.com/jfrog/jfrog-cli-core/v2 v2.31.1-0.20230720085538-4befe41129f8 h1:EfxIkjtuoLshViOJb3mBlNi3SwXs/4DY4bvOvxNTji0= -github.com/jfrog/jfrog-cli-core/v2 v2.31.1-0.20230720085538-4befe41129f8/go.mod h1:/HJ9mO3AZsACtQWxkwMj7REWPdXT3yHKjJXjPHlmB34= +github.com/jfrog/jfrog-cli-core/v2 v2.39.3 h1:GtBwEAchuvI4c8ZwaJ6CKN/KavMlEu5+DwNX9OesYMI= +github.com/jfrog/jfrog-cli-core/v2 v2.39.3/go.mod h1:/HJ9mO3AZsACtQWxkwMj7REWPdXT3yHKjJXjPHlmB34= github.com/jfrog/jfrog-client-go v1.31.2 h1:foy8owM2lS8jZL7zuBPtcx1RpF1GeIXaXF8hIufyr4I= github.com/jfrog/jfrog-client-go v1.31.2/go.mod h1:qEJxoe68sUtqHJ1YhXv/7pKYP/9p1D5tJrruzJKYeoI= github.com/jstemmer/go-junit-report v0.0.0-20190106144839-af01ea7f8024/go.mod h1:6v2b51hI/fHJwM22ozAgKL4VKDeJcHhJFhtBdhmNjmU=