From 2ec989f81d6e11d309f3cd3eacf6f6bbdad0cb1f Mon Sep 17 00:00:00 2001 From: Eran Turgeman Date: Mon, 26 Feb 2024 12:26:46 +0200 Subject: [PATCH 1/6] initial work and comments: params.go, scanrepository.go, git.go --- scanrepository/scanrepository.go | 56 ++++++++++++++++++++++---------- utils/git.go | 6 ++-- utils/params.go | 24 ++++++++++++++ 3 files changed, 66 insertions(+), 20 deletions(-) diff --git a/scanrepository/scanrepository.go b/scanrepository/scanrepository.go index 9bd04e677..0b9859776 100644 --- a/scanrepository/scanrepository.go +++ b/scanrepository/scanrepository.go @@ -83,10 +83,21 @@ func (cfp *ScanRepositoryCmd) scanAndFixBranch(repository *utils.Repository) (er if err = cfp.scanDetails.CreateMultiScanIdForScans(); err != nil { return err } + + // If we have several provided projects, we want to differentiate between them in order to avoid possible conflict in the 'fix' branch's name + // When fixing the same vulnerable dependency in two different projects + setUniqueProjectHash := len(repository.Projects) > 1 + for i := range repository.Projects { cfp.scanDetails.Project = &repository.Projects[i] + if setUniqueProjectHash { + err = cfp.scanDetails.Project.SetUniqueProjectHash() + if err != nil { + return + } + } cfp.projectTech = []coreutils.Technology{} - if err = cfp.scanAndFixProject(repository); err != nil { + if err = cfp.scanAndFixProject(repository); err != nil { //TODO ERAN here we split into projects return } } @@ -153,7 +164,7 @@ func (cfp *ScanRepositoryCmd) scanAndFixProject(repository *utils.Repository) er vulnerabilitiesByPathMap[fullPathWd] = currPathVulnerabilities } if fixNeeded { - return cfp.fixVulnerablePackages(vulnerabilitiesByPathMap) + return cfp.fixVulnerablePackages(vulnerabilitiesByPathMap) //TODO ERAN EP-A-B 1 } return nil } @@ -188,15 +199,15 @@ func (cfp *ScanRepositoryCmd) getVulnerabilitiesMap(scanResults *xrayutils.Resul func (cfp *ScanRepositoryCmd) fixVulnerablePackages(vulnerabilitiesByWdMap map[string]map[string]*utils.VulnerabilityDetails) (err error) { if cfp.aggregateFixes { - return cfp.fixIssuesSinglePR(vulnerabilitiesByWdMap) + return cfp.fixIssuesSinglePR(vulnerabilitiesByWdMap) //TODO ERAN EP-A 2 } - return cfp.fixIssuesSeparatePRs(vulnerabilitiesByWdMap) + return cfp.fixIssuesSeparatePRs(vulnerabilitiesByWdMap) //TODO ERAN EP-B 2 } func (cfp *ScanRepositoryCmd) 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 { + if e := cfp.fixProjectVulnerabilities(fullPath, vulnerabilities); e != nil { //TODO ERAN EP-B 3 err = errors.Join(err, fmt.Errorf("the following errors occured while fixing vulnerabilities in '%s':\n%s", fullPath, e)) } } @@ -220,7 +231,7 @@ func (cfp *ScanRepositoryCmd) fixProjectVulnerabilities(fullProjectPath string, // Fix every vulnerability in a separate pull request and branch for _, vulnerability := range vulnerabilities { - if e := cfp.fixSinglePackageAndCreatePR(vulnerability); e != nil { + if e := cfp.fixSinglePackageAndCreatePR(vulnerability); e != nil { //TODO ERAN EP-B 4 err = errors.Join(err, cfp.handleUpdatePackageErrors(e)) } @@ -260,18 +271,18 @@ func (cfp *ScanRepositoryCmd) fixMultiplePackages(fullProjectPath string, vulner return } -// fixIssuesSinglePR fixes all the vulnerabilities in a single aggregated pull request. +// Fixes all the vulnerabilities in a single aggregated pull request. // If an existing aggregated fix is present, it checks for different scan results. // 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 *ScanRepositoryCmd) fixIssuesSinglePR(vulnerabilitiesMap map[string]map[string]*utils.VulnerabilityDetails) (err error) { - aggregatedFixBranchName := cfp.gitManager.GenerateAggregatedFixBranchName(cfp.scanDetails.BaseBranch(), cfp.projectTech) + aggregatedFixBranchName := cfp.gitManager.GenerateAggregatedFixBranchName(cfp.scanDetails.BaseBranch(), cfp.projectTech) //TODO ERAN try apply fix to aggregated too existingPullRequestDetails, err := cfp.getOpenPullRequestBySourceBranch(aggregatedFixBranchName) if err != nil { return } - return cfp.aggregateFixAndOpenPullRequest(vulnerabilitiesMap, aggregatedFixBranchName, existingPullRequestDetails) + return cfp.aggregateFixAndOpenPullRequest(vulnerabilitiesMap, aggregatedFixBranchName, existingPullRequestDetails) //TODO ERAN EP-A 3 } // Handles possible error of update package operation @@ -297,10 +308,12 @@ func (cfp *ScanRepositoryCmd) handleUpdatePackageErrors(err error) error { func (cfp *ScanRepositoryCmd) fixSinglePackageAndCreatePR(vulnDetails *utils.VulnerabilityDetails) (err error) { fixVersion := vulnDetails.SuggestedFixedVersion log.Debug("Attempting to fix", fmt.Sprintf("%s:%s", vulnDetails.ImpactedDependencyName, vulnDetails.ImpactedDependencyVersion), "with", fixVersion) - fixBranchName, err := cfp.gitManager.GenerateFixBranchName(cfp.scanDetails.BaseBranch(), vulnDetails.ImpactedDependencyName, fixVersion) + fixBranchName, err := cfp.gitManager.GenerateFixBranchName(cfp.scanDetails.BaseBranch(), vulnDetails.ImpactedDependencyName, fixVersion, cfp.scanDetails.Project.UniqueProjectHash) if err != nil { return } + //TODO ERAN this is where we have some kind of block in duplicated fixes/ it we actually pushed the first fix so the branch already exist in the remote + // and therefore no new branch will be created at all existsInRemote, err := cfp.gitManager.BranchExistsInRemote(fixBranchName) if err != nil { return @@ -327,7 +340,7 @@ func (cfp *ScanRepositoryCmd) fixSinglePackageAndCreatePR(vulnDetails *utils.Vul if err = cfp.updatePackageToFixedVersion(vulnDetails); err != nil { return } - if err = cfp.openFixingPullRequest(fixBranchName, vulnDetails); err != nil { + if err = cfp.openFixingPullRequest(fixBranchName, vulnDetails); err != nil { //TODO ERAN EP-B 5 return errors.Join(fmt.Errorf("failed while creating a fixing pull request for: %s with version: %s with error: ", vulnDetails.ImpactedDependencyName, fixVersion), err) } log.Info(fmt.Sprintf("Created Pull Request updating dependency '%s' to version '%s'", vulnDetails.ImpactedDependencyName, vulnDetails.SuggestedFixedVersion)) @@ -336,7 +349,7 @@ func (cfp *ScanRepositoryCmd) fixSinglePackageAndCreatePR(vulnDetails *utils.Vul func (cfp *ScanRepositoryCmd) openFixingPullRequest(fixBranchName string, vulnDetails *utils.VulnerabilityDetails) (err error) { log.Debug("Checking if there are changes to commit") - isClean, err := cfp.gitManager.IsClean() + isClean, err := cfp.gitManager.IsClean() //TODO ERAN EP-B FAIL HERE if err != nil { return } @@ -362,7 +375,7 @@ func (cfp *ScanRepositoryCmd) openFixingPullRequest(fixBranchName string, vulnDe // 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 *ScanRepositoryCmd) openAggregatedPullRequest(fixBranchName string, pullRequestInfo *vcsclient.PullRequestInfo, vulnerabilities []*utils.VulnerabilityDetails) (err error) { - commitMessage := cfp.gitManager.GenerateAggregatedCommitMessage(cfp.projectTech) + commitMessage := cfp.gitManager.GenerateAggregatedCommitMessage(cfp.projectTech) // TODO ERAN EP-A FAIL HERE if err = cfp.gitManager.AddAllAndCommit(commitMessage); err != nil { return } @@ -494,11 +507,18 @@ func (cfp *ScanRepositoryCmd) updatePackageToFixedVersion(vulnDetails *utils.Vul cfp.handlers = make(map[coreutils.Technology]packagehandlers.PackageHandler) } - handler := cfp.handlers[vulnDetails.Technology] - if handler == nil { - handler = packagehandlers.GetCompatiblePackageHandler(vulnDetails, cfp.scanDetails) - cfp.handlers[vulnDetails.Technology] = handler - } else if _, unsupported := handler.(*packagehandlers.UnsupportedPackageHandler); unsupported { + /* + handler := cfp.handlers[vulnDetails.Technology] + if handler == nil { + handler = packagehandlers.GetCompatiblePackageHandler(vulnDetails, cfp.scanDetails) + cfp.handlers[vulnDetails.Technology] = handler + } else if _, unsupported := handler.(*packagehandlers.UnsupportedPackageHandler); unsupported { + return + } + */ + handler := packagehandlers.GetCompatiblePackageHandler(vulnDetails, cfp.scanDetails) + cfp.handlers[vulnDetails.Technology] = handler + if _, unsupported := handler.(*packagehandlers.UnsupportedPackageHandler); unsupported { return } diff --git a/utils/git.go b/utils/git.go index cd8a79251..e7b64869d 100644 --- a/utils/git.go +++ b/utils/git.go @@ -220,6 +220,7 @@ func (gm *GitManager) createBranchAndCheckout(branchName string, create bool, ke return worktree.Checkout(checkoutConfig) } +// TODO ERAN check if this func still in use... func getCurrentBranch(repository *git.Repository) (string, error) { head, err := repository.Head() if err != nil { @@ -394,8 +395,9 @@ func formatStringWithPlaceHolders(str, impactedPackage, fixVersion, hash, baseBr return str } -func (gm *GitManager) GenerateFixBranchName(branch string, impactedPackage string, fixVersion string) (string, error) { - hash, err := Md5Hash("frogbot", branch, impactedPackage, fixVersion) +// TODO ERAN fix test +func (gm *GitManager) GenerateFixBranchName(branch string, impactedPackage string, fixVersion string, uniqueProjectHash string) (string, error) { + hash, err := Md5Hash("frogbot", branch, impactedPackage, fixVersion, uniqueProjectHash) if err != nil { return "", err } diff --git a/utils/params.go b/utils/params.go index 42a63f5e0..38a7c6711 100644 --- a/utils/params.go +++ b/utils/params.go @@ -86,6 +86,7 @@ type Project struct { InstallCommandName string InstallCommandArgs []string IsRecursiveScan bool + UniqueProjectHash string } func (p *Project) setDefaultsIfNeeded() error { @@ -127,6 +128,29 @@ func (p *Project) setDefaultsIfNeeded() error { return nil } +// Creates a unique hash code for a Project struct based on its fields. +// The creates hash is consistent as long as the Project's values doesn't change +func (p *Project) SetUniqueProjectHash() error { + // TODO Hash the entire object (the JSON that we unmarshall to the object) + // TODO verify the order is meaningless while creating the Hash + // TODO verify all lists comes in the same order in every execution + var collectedValues []string + collectedValues = append(collectedValues, p.InstallCommand) + collectedValues = append(collectedValues, p.PipRequirementsFile) + collectedValues = append(collectedValues, p.WorkingDirs...) + collectedValues = append(collectedValues, p.PathExclusions...) + collectedValues = append(collectedValues, p.DepsRepo) + //TODO ERAN what should we do if tomorrow we add another field to Project? + //TODO ERAN if one field has changed we break the HASH + + var err error + p.UniqueProjectHash, err = Md5Hash(collectedValues...) + if err != nil { + err = fmt.Errorf("failed to create a unique hash for one of the provided projects: %s", err.Error()) + } + return err +} + type Scan struct { IncludeAllVulnerabilities bool `yaml:"includeAllVulnerabilities,omitempty"` FixableOnly bool `yaml:"fixableOnly,omitempty"` From 8faa2fc7bf189e408c24ec8a020b130a1957f4f7 Mon Sep 17 00:00:00 2001 From: Eran Turgeman Date: Wed, 28 Feb 2024 15:24:32 +0200 Subject: [PATCH 2/6] updated scan-repository flow with a bug fix using for aggregatedFix: false --- scanrepository/scanrepository.go | 36 +++++++-------------- scanrepository/scanrepository_test.go | 17 ++++++++-- schema/frogbot-schema.json | 5 +++ utils/git.go | 8 +++-- utils/git_test.go | 45 +++++++++++++++++++-------- utils/params.go | 29 ++--------------- utils/scandetails.go | 6 ++-- 7 files changed, 75 insertions(+), 71 deletions(-) diff --git a/scanrepository/scanrepository.go b/scanrepository/scanrepository.go index 0b9859776..7d3be1fb8 100644 --- a/scanrepository/scanrepository.go +++ b/scanrepository/scanrepository.go @@ -84,20 +84,10 @@ func (cfp *ScanRepositoryCmd) scanAndFixBranch(repository *utils.Repository) (er return err } - // If we have several provided projects, we want to differentiate between them in order to avoid possible conflict in the 'fix' branch's name - // When fixing the same vulnerable dependency in two different projects - setUniqueProjectHash := len(repository.Projects) > 1 - for i := range repository.Projects { cfp.scanDetails.Project = &repository.Projects[i] - if setUniqueProjectHash { - err = cfp.scanDetails.Project.SetUniqueProjectHash() - if err != nil { - return - } - } cfp.projectTech = []coreutils.Technology{} - if err = cfp.scanAndFixProject(repository); err != nil { //TODO ERAN here we split into projects + if err = cfp.scanAndFixProject(repository); err != nil { return } } @@ -164,7 +154,7 @@ func (cfp *ScanRepositoryCmd) scanAndFixProject(repository *utils.Repository) er vulnerabilitiesByPathMap[fullPathWd] = currPathVulnerabilities } if fixNeeded { - return cfp.fixVulnerablePackages(vulnerabilitiesByPathMap) //TODO ERAN EP-A-B 1 + return cfp.fixVulnerablePackages(vulnerabilitiesByPathMap) } return nil } @@ -199,15 +189,15 @@ func (cfp *ScanRepositoryCmd) getVulnerabilitiesMap(scanResults *xrayutils.Resul func (cfp *ScanRepositoryCmd) fixVulnerablePackages(vulnerabilitiesByWdMap map[string]map[string]*utils.VulnerabilityDetails) (err error) { if cfp.aggregateFixes { - return cfp.fixIssuesSinglePR(vulnerabilitiesByWdMap) //TODO ERAN EP-A 2 + return cfp.fixIssuesSinglePR(vulnerabilitiesByWdMap) } - return cfp.fixIssuesSeparatePRs(vulnerabilitiesByWdMap) //TODO ERAN EP-B 2 + return cfp.fixIssuesSeparatePRs(vulnerabilitiesByWdMap) } func (cfp *ScanRepositoryCmd) 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 { //TODO ERAN EP-B 3 + 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)) } } @@ -231,7 +221,7 @@ func (cfp *ScanRepositoryCmd) fixProjectVulnerabilities(fullProjectPath string, // Fix every vulnerability in a separate pull request and branch for _, vulnerability := range vulnerabilities { - if e := cfp.fixSinglePackageAndCreatePR(vulnerability); e != nil { //TODO ERAN EP-B 4 + if e := cfp.fixSinglePackageAndCreatePR(vulnerability); e != nil { err = errors.Join(err, cfp.handleUpdatePackageErrors(e)) } @@ -277,12 +267,12 @@ func (cfp *ScanRepositoryCmd) fixMultiplePackages(fullProjectPath string, vulner // 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 *ScanRepositoryCmd) fixIssuesSinglePR(vulnerabilitiesMap map[string]map[string]*utils.VulnerabilityDetails) (err error) { - aggregatedFixBranchName := cfp.gitManager.GenerateAggregatedFixBranchName(cfp.scanDetails.BaseBranch(), cfp.projectTech) //TODO ERAN try apply fix to aggregated too + aggregatedFixBranchName := cfp.gitManager.GenerateAggregatedFixBranchName(cfp.scanDetails.BaseBranch(), cfp.projectTech) existingPullRequestDetails, err := cfp.getOpenPullRequestBySourceBranch(aggregatedFixBranchName) if err != nil { return } - return cfp.aggregateFixAndOpenPullRequest(vulnerabilitiesMap, aggregatedFixBranchName, existingPullRequestDetails) //TODO ERAN EP-A 3 + return cfp.aggregateFixAndOpenPullRequest(vulnerabilitiesMap, aggregatedFixBranchName, existingPullRequestDetails) } // Handles possible error of update package operation @@ -312,8 +302,6 @@ func (cfp *ScanRepositoryCmd) fixSinglePackageAndCreatePR(vulnDetails *utils.Vul if err != nil { return } - //TODO ERAN this is where we have some kind of block in duplicated fixes/ it we actually pushed the first fix so the branch already exist in the remote - // and therefore no new branch will be created at all existsInRemote, err := cfp.gitManager.BranchExistsInRemote(fixBranchName) if err != nil { return @@ -340,7 +328,7 @@ func (cfp *ScanRepositoryCmd) fixSinglePackageAndCreatePR(vulnDetails *utils.Vul if err = cfp.updatePackageToFixedVersion(vulnDetails); err != nil { return } - if err = cfp.openFixingPullRequest(fixBranchName, vulnDetails); err != nil { //TODO ERAN EP-B 5 + if err = cfp.openFixingPullRequest(fixBranchName, vulnDetails); err != nil { return errors.Join(fmt.Errorf("failed while creating a fixing pull request for: %s with version: %s with error: ", vulnDetails.ImpactedDependencyName, fixVersion), err) } log.Info(fmt.Sprintf("Created Pull Request updating dependency '%s' to version '%s'", vulnDetails.ImpactedDependencyName, vulnDetails.SuggestedFixedVersion)) @@ -349,7 +337,7 @@ func (cfp *ScanRepositoryCmd) fixSinglePackageAndCreatePR(vulnDetails *utils.Vul func (cfp *ScanRepositoryCmd) openFixingPullRequest(fixBranchName string, vulnDetails *utils.VulnerabilityDetails) (err error) { log.Debug("Checking if there are changes to commit") - isClean, err := cfp.gitManager.IsClean() //TODO ERAN EP-B FAIL HERE + isClean, err := cfp.gitManager.IsClean() if err != nil { return } @@ -375,7 +363,7 @@ func (cfp *ScanRepositoryCmd) openFixingPullRequest(fixBranchName string, vulnDe // 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 *ScanRepositoryCmd) openAggregatedPullRequest(fixBranchName string, pullRequestInfo *vcsclient.PullRequestInfo, vulnerabilities []*utils.VulnerabilityDetails) (err error) { - commitMessage := cfp.gitManager.GenerateAggregatedCommitMessage(cfp.projectTech) // TODO ERAN EP-A FAIL HERE + commitMessage := cfp.gitManager.GenerateAggregatedCommitMessage(cfp.projectTech) if err = cfp.gitManager.AddAllAndCommit(commitMessage); err != nil { return } @@ -410,7 +398,7 @@ func (cfp *ScanRepositoryCmd) preparePullRequestDetails(vulnerabilitiesDetails . } // In separate pull requests there is only one vulnerability vulnDetails := vulnerabilitiesDetails[0] - pullRequestTitle := cfp.gitManager.GeneratePullRequestTitle(vulnDetails.ImpactedDependencyName, vulnDetails.SuggestedFixedVersion) + pullRequestTitle := cfp.gitManager.GeneratePullRequestTitle(vulnDetails.ImpactedDependencyName, vulnDetails.SuggestedFixedVersion, cfp.scanDetails.UniqueProjectHash) return pullRequestTitle, prBody, nil } diff --git a/scanrepository/scanrepository_test.go b/scanrepository/scanrepository_test.go index e8f3faf1c..5f884a90d 100644 --- a/scanrepository/scanrepository_test.go +++ b/scanrepository/scanrepository_test.go @@ -353,7 +353,7 @@ func TestGenerateFixBranchName(t *testing.T) { gitManager := utils.GitManager{} for _, test := range tests { t.Run(test.expectedName, func(t *testing.T) { - branchName, err := gitManager.GenerateFixBranchName(test.baseBranch, test.impactedPackage, test.fixVersion) + branchName, err := gitManager.GenerateFixBranchName(test.baseBranch, test.impactedPackage, test.fixVersion, "") assert.NoError(t, err) assert.Equal(t, test.expectedName, branchName) }) @@ -596,7 +596,11 @@ random body } func TestPreparePullRequestDetails(t *testing.T) { - cfp := ScanRepositoryCmd{OutputWriter: &outputwriter.StandardOutput{}, gitManager: &utils.GitManager{}} + cfp := ScanRepositoryCmd{ + OutputWriter: &outputwriter.StandardOutput{}, + gitManager: &utils.GitManager{}, scanDetails: &utils.ScanDetails{ + Project: &utils.Project{UniqueProjectHash: ""}, + }} cfp.OutputWriter.SetJasOutputFlags(true, false) vulnerabilities := []*utils.VulnerabilityDetails{ { @@ -618,6 +622,14 @@ func TestPreparePullRequestDetails(t *testing.T) { assert.NoError(t, err) assert.Equal(t, "[🐸 Frogbot] Update version of package1 to 1.0.0", prTitle) assert.Equal(t, expectedPrBody, prBody) + + cfp.scanDetails.UniqueProjectHash = "my-unique-identifier" + expectedPrBody = utils.GenerateFixPullRequestDetails(utils.ExtractVulnerabilitiesDetailsToRows(vulnerabilities), cfp.OutputWriter) + prTitle, prBody, err = cfp.preparePullRequestDetails(vulnerabilities...) + assert.NoError(t, err) + assert.Equal(t, "[🐸 Frogbot] Update version of package1 to 1.0.0 (my-unique-identifier)", prTitle) + assert.Equal(t, expectedPrBody, prBody) + vulnerabilities = append(vulnerabilities, &utils.VulnerabilityDetails{ VulnerabilityOrViolationRow: formats.VulnerabilityOrViolationRow{ Summary: "summary", @@ -637,6 +649,7 @@ func TestPreparePullRequestDetails(t *testing.T) { assert.NoError(t, err) assert.Equal(t, cfp.gitManager.GenerateAggregatedPullRequestTitle([]coreutils.Technology{}), prTitle) assert.Equal(t, expectedPrBody, prBody) + cfp.OutputWriter = &outputwriter.SimplifiedOutput{} expectedPrBody = utils.GenerateFixPullRequestDetails(utils.ExtractVulnerabilitiesDetailsToRows(vulnerabilities), cfp.OutputWriter) + outputwriter.MarkdownComment("Checksum: bec823edaceb5d0478b789798e819bde") prTitle, prBody, err = cfp.preparePullRequestDetails(vulnerabilities...) diff --git a/schema/frogbot-schema.json b/schema/frogbot-schema.json index 048453b4d..2e3787b07 100644 --- a/schema/frogbot-schema.json +++ b/schema/frogbot-schema.json @@ -219,6 +219,11 @@ "type": "string", "title": "Virtual Artifactory Repository", "description": "Name of a Virtual Repository in Artifactory to resolve (download) the project dependencies from" + }, + "uniqueIdentifier": { + "type": "string", + "title": "Unique Hash Identifier", + "description": "Unique hash identifier for each project that is used to create separation between projects" } } } diff --git a/utils/git.go b/utils/git.go index e7b64869d..3cda949f2 100644 --- a/utils/git.go +++ b/utils/git.go @@ -220,7 +220,6 @@ func (gm *GitManager) createBranchAndCheckout(branchName string, create bool, ke return worktree.Checkout(checkoutConfig) } -// TODO ERAN check if this func still in use... func getCurrentBranch(repository *git.Repository) (string, error) { head, err := repository.Head() if err != nil { @@ -392,10 +391,10 @@ func formatStringWithPlaceHolders(str, impactedPackage, fixVersion, hash, baseBr if baseBranch != "" { str += "-" + baseBranch } + return str } -// TODO ERAN fix test func (gm *GitManager) GenerateFixBranchName(branch string, impactedPackage string, fixVersion string, uniqueProjectHash string) (string, error) { hash, err := Md5Hash("frogbot", branch, impactedPackage, fixVersion, uniqueProjectHash) if err != nil { @@ -410,8 +409,11 @@ func (gm *GitManager) GenerateFixBranchName(branch string, impactedPackage strin return formatStringWithPlaceHolders(branchFormat, fixedPackageName, fixVersion, hash, "", false), nil } -func (gm *GitManager) GeneratePullRequestTitle(impactedPackage string, version string) string { +func (gm *GitManager) GeneratePullRequestTitle(impactedPackage string, version string, uniqueProjectHash string) string { template := PullRequestTitleTemplate + if uniqueProjectHash != "" { + template += " (" + uniqueProjectHash + ")" + } pullRequestFormat := gm.customTemplates.pullRequestTitleTemplate if pullRequestFormat != "" { template = pullRequestFormat diff --git a/utils/git_test.go b/utils/git_test.go index 633a74883..0ee42abbb 100644 --- a/utils/git_test.go +++ b/utils/git_test.go @@ -66,11 +66,12 @@ func TestGitManager_GenerateCommitMessage(t *testing.T) { func TestGitManager_GenerateFixBranchName(t *testing.T) { testCases := []struct { - gitManager GitManager - impactedPackage string - fixVersion VulnerabilityDetails - expected string - description string + gitManager GitManager + impactedPackage string + fixVersion VulnerabilityDetails + expected string + description string + uniqueProjectIdentifier string }{ { gitManager: GitManager{customTemplates: CustomTemplates{branchNameTemplate: "[Feature]-${IMPACTED_PACKAGE}-${BRANCH_NAME_HASH}"}}, @@ -85,17 +86,26 @@ func TestGitManager_GenerateFixBranchName(t *testing.T) { 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{SuggestedFixedVersion: "3.4.5"}, expected: "just-a-branch-41b1f45136b25e3624b15999bd57a476", description: "Custom template without inputs", }, + { + gitManager: GitManager{customTemplates: CustomTemplates{branchNameTemplate: "just-a-branch-${BRANCH_NAME_HASH}"}}, + impactedPackage: "mquery", + fixVersion: VulnerabilityDetails{SuggestedFixedVersion: "3.4.5"}, + expected: "just-a-branch-576bdc1ddb2ad676551efd7a47f48ece", + description: "Custom template without inputs", + uniqueProjectIdentifier: "my-identifier", + }, } for _, test := range testCases { t.Run(test.expected, func(t *testing.T) { - commitMessage, err := test.gitManager.GenerateFixBranchName("md5Branch", test.impactedPackage, test.fixVersion.SuggestedFixedVersion) + commitMessage, err := test.gitManager.GenerateFixBranchName("md5Branch", test.impactedPackage, test.fixVersion.SuggestedFixedVersion, test.uniqueProjectIdentifier) assert.NoError(t, err) assert.Equal(t, test.expected, commitMessage) }) @@ -104,11 +114,12 @@ func TestGitManager_GenerateFixBranchName(t *testing.T) { func TestGitManager_GeneratePullRequestTitle(t *testing.T) { testCases := []struct { - gitManager GitManager - impactedPackage string - fixVersion VulnerabilityDetails - expected string - description string + gitManager GitManager + impactedPackage string + fixVersion VulnerabilityDetails + expected string + description string + uniqueProjectIdentifier string }{ { gitManager: GitManager{customTemplates: CustomTemplates{pullRequestTitleTemplate: "[CustomPR] update ${IMPACTED_PACKAGE} to ${FIX_VERSION}"}}, @@ -131,10 +142,18 @@ func TestGitManager_GeneratePullRequestTitle(t *testing.T) { expected: "[🐸 Frogbot] Update version of mquery to 3.4.5", description: "No prefix", }, + { + gitManager: GitManager{customTemplates: CustomTemplates{pullRequestTitleTemplate: ""}}, + impactedPackage: "mquery", + fixVersion: VulnerabilityDetails{SuggestedFixedVersion: "3.4.5"}, + expected: "[🐸 Frogbot] Update version of mquery to 3.4.5 (my-identifier)", + description: "No prefix", + uniqueProjectIdentifier: "my-identifier", + }, } for _, test := range testCases { t.Run(test.expected, func(t *testing.T) { - titleOutput := test.gitManager.GeneratePullRequestTitle(test.impactedPackage, test.fixVersion.SuggestedFixedVersion) + titleOutput := test.gitManager.GeneratePullRequestTitle(test.impactedPackage, test.fixVersion.SuggestedFixedVersion, test.uniqueProjectIdentifier) assert.Equal(t, test.expected, titleOutput) }) } diff --git a/utils/params.go b/utils/params.go index 38a7c6711..2b137a7ac 100644 --- a/utils/params.go +++ b/utils/params.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "github.com/jfrog/jfrog-cli-security/commands/audit/jas" "net/http" "net/url" "os" @@ -12,7 +13,6 @@ import ( "strings" "github.com/jfrog/frogbot/v2/utils/outputwriter" - "github.com/jfrog/jfrog-cli-security/commands/audit" xrutils "github.com/jfrog/jfrog-cli-security/utils" "github.com/jfrog/build-info-go/utils" @@ -86,7 +86,7 @@ type Project struct { InstallCommandName string InstallCommandArgs []string IsRecursiveScan bool - UniqueProjectHash string + UniqueProjectHash string `yaml:"uniqueIdentifier,omitempty"` } func (p *Project) setDefaultsIfNeeded() error { @@ -103,7 +103,7 @@ func (p *Project) setDefaultsIfNeeded() error { } if len(p.PathExclusions) == 0 { if p.PathExclusions, _ = readArrayParamFromEnv(PathExclusionsEnv, ";"); len(p.PathExclusions) == 0 { - p.PathExclusions = audit.DefaultExcludePatterns + p.PathExclusions = jas.DefaultExcludePatterns } } if p.UseWrapper == nil { @@ -128,29 +128,6 @@ func (p *Project) setDefaultsIfNeeded() error { return nil } -// Creates a unique hash code for a Project struct based on its fields. -// The creates hash is consistent as long as the Project's values doesn't change -func (p *Project) SetUniqueProjectHash() error { - // TODO Hash the entire object (the JSON that we unmarshall to the object) - // TODO verify the order is meaningless while creating the Hash - // TODO verify all lists comes in the same order in every execution - var collectedValues []string - collectedValues = append(collectedValues, p.InstallCommand) - collectedValues = append(collectedValues, p.PipRequirementsFile) - collectedValues = append(collectedValues, p.WorkingDirs...) - collectedValues = append(collectedValues, p.PathExclusions...) - collectedValues = append(collectedValues, p.DepsRepo) - //TODO ERAN what should we do if tomorrow we add another field to Project? - //TODO ERAN if one field has changed we break the HASH - - var err error - p.UniqueProjectHash, err = Md5Hash(collectedValues...) - if err != nil { - err = fmt.Errorf("failed to create a unique hash for one of the provided projects: %s", err.Error()) - } - return err -} - type Scan struct { IncludeAllVulnerabilities bool `yaml:"includeAllVulnerabilities,omitempty"` FixableOnly bool `yaml:"fixableOnly,omitempty"` diff --git a/utils/scandetails.go b/utils/scandetails.go index f7b6d654c..1c2990d67 100644 --- a/utils/scandetails.go +++ b/utils/scandetails.go @@ -120,11 +120,11 @@ func (sc *ScanDetails) RunInstallAndAudit(workDirs ...string) (auditResults *xra auditParams := audit.NewAuditParams(). SetXrayGraphScanParams(sc.XrayGraphScanParams). SetWorkingDirs(workDirs). - SetExclusions(sc.PathExclusions). SetMinSeverityFilter(sc.MinSeverityFilter()). SetFixableOnly(sc.FixableOnly()). - SetGraphBasicParams(auditBasicParams). - SetIsRecursiveScan(sc.IsRecursiveScan) + SetGraphBasicParams(auditBasicParams) + + auditParams.SetExclusions(sc.PathExclusions).SetIsRecursiveScan(sc.IsRecursiveScan) auditResults, err = audit.RunAudit(auditParams) if auditResults != nil { From f270f9ad227ffc8e54a514c08037444165ff2220 Mon Sep 17 00:00:00 2001 From: Eran Turgeman Date: Wed, 28 Feb 2024 16:04:39 +0200 Subject: [PATCH 3/6] updated test --- scanrepository/scanrepository_test.go | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/scanrepository/scanrepository_test.go b/scanrepository/scanrepository_test.go index 5f884a90d..01b72064f 100644 --- a/scanrepository/scanrepository_test.go +++ b/scanrepository/scanrepository_test.go @@ -341,19 +341,21 @@ func TestParseVersionChangeString(t *testing.T) { func TestGenerateFixBranchName(t *testing.T) { tests := []struct { - baseBranch string - impactedPackage string - fixVersion string - expectedName string + baseBranch string + impactedPackage string + fixVersion string + expectedName string + uniqueIdentifier string }{ - {"dev", "gopkg.in/yaml.v3", "3.0.0", "frogbot-gopkg.in/yaml.v3-d61bde82dc594e5ccc5a042fe224bf7c"}, - {"master", "gopkg.in/yaml.v3", "3.0.0", "frogbot-gopkg.in/yaml.v3-41405528994061bd108e3bbd4c039a03"}, - {"dev", "replace:colons:colons", "3.0.0", "frogbot-replace_colons_colons-89e555131b4a70a32fe9d9c44d6ff0fc"}, + {"dev", "gopkg.in/yaml.v3", "3.0.0", "frogbot-gopkg.in/yaml.v3-d61bde82dc594e5ccc5a042fe224bf7c", ""}, + {"master", "gopkg.in/yaml.v3", "3.0.0", "frogbot-gopkg.in/yaml.v3-41405528994061bd108e3bbd4c039a03", ""}, + {"dev", "replace:colons:colons", "3.0.0", "frogbot-replace_colons_colons-89e555131b4a70a32fe9d9c44d6ff0fc", ""}, + {"master", "mquery", "3.4.5", "frogbot-mquery-75fc8c6d4e3368833be20a46fe5b5cb0", "my-identifier"}, } gitManager := utils.GitManager{} for _, test := range tests { t.Run(test.expectedName, func(t *testing.T) { - branchName, err := gitManager.GenerateFixBranchName(test.baseBranch, test.impactedPackage, test.fixVersion, "") + branchName, err := gitManager.GenerateFixBranchName(test.baseBranch, test.impactedPackage, test.fixVersion, test.uniqueIdentifier) assert.NoError(t, err) assert.Equal(t, test.expectedName, branchName) }) From 75ec721a95346fde07cd254d1679ef4662cf02a8 Mon Sep 17 00:00:00 2001 From: Eran Turgeman Date: Thu, 29 Feb 2024 16:05:08 +0200 Subject: [PATCH 4/6] . --- scanrepository/scanrepository.go | 4 ++-- scanrepository/scanrepository_test.go | 4 ++-- utils/git.go | 10 +++++----- utils/params.go | 20 ++++++++++---------- 4 files changed, 19 insertions(+), 19 deletions(-) diff --git a/scanrepository/scanrepository.go b/scanrepository/scanrepository.go index 7d3be1fb8..b8d692be0 100644 --- a/scanrepository/scanrepository.go +++ b/scanrepository/scanrepository.go @@ -298,7 +298,7 @@ func (cfp *ScanRepositoryCmd) handleUpdatePackageErrors(err error) error { func (cfp *ScanRepositoryCmd) fixSinglePackageAndCreatePR(vulnDetails *utils.VulnerabilityDetails) (err error) { fixVersion := vulnDetails.SuggestedFixedVersion log.Debug("Attempting to fix", fmt.Sprintf("%s:%s", vulnDetails.ImpactedDependencyName, vulnDetails.ImpactedDependencyVersion), "with", fixVersion) - fixBranchName, err := cfp.gitManager.GenerateFixBranchName(cfp.scanDetails.BaseBranch(), vulnDetails.ImpactedDependencyName, fixVersion, cfp.scanDetails.Project.UniqueProjectHash) + fixBranchName, err := cfp.gitManager.GenerateFixBranchName(cfp.scanDetails.BaseBranch(), vulnDetails.ImpactedDependencyName, fixVersion, cfp.scanDetails.Project.UniqueProjectIdentifier) if err != nil { return } @@ -398,7 +398,7 @@ func (cfp *ScanRepositoryCmd) preparePullRequestDetails(vulnerabilitiesDetails . } // In separate pull requests there is only one vulnerability vulnDetails := vulnerabilitiesDetails[0] - pullRequestTitle := cfp.gitManager.GeneratePullRequestTitle(vulnDetails.ImpactedDependencyName, vulnDetails.SuggestedFixedVersion, cfp.scanDetails.UniqueProjectHash) + pullRequestTitle := cfp.gitManager.GeneratePullRequestTitle(vulnDetails.ImpactedDependencyName, vulnDetails.SuggestedFixedVersion, cfp.scanDetails.UniqueProjectIdentifier) return pullRequestTitle, prBody, nil } diff --git a/scanrepository/scanrepository_test.go b/scanrepository/scanrepository_test.go index 01b72064f..7caf63f7a 100644 --- a/scanrepository/scanrepository_test.go +++ b/scanrepository/scanrepository_test.go @@ -601,7 +601,7 @@ func TestPreparePullRequestDetails(t *testing.T) { cfp := ScanRepositoryCmd{ OutputWriter: &outputwriter.StandardOutput{}, gitManager: &utils.GitManager{}, scanDetails: &utils.ScanDetails{ - Project: &utils.Project{UniqueProjectHash: ""}, + Project: &utils.Project{UniqueProjectIdentifier: ""}, }} cfp.OutputWriter.SetJasOutputFlags(true, false) vulnerabilities := []*utils.VulnerabilityDetails{ @@ -625,7 +625,7 @@ func TestPreparePullRequestDetails(t *testing.T) { assert.Equal(t, "[🐸 Frogbot] Update version of package1 to 1.0.0", prTitle) assert.Equal(t, expectedPrBody, prBody) - cfp.scanDetails.UniqueProjectHash = "my-unique-identifier" + cfp.scanDetails.UniqueProjectIdentifier = "my-unique-identifier" expectedPrBody = utils.GenerateFixPullRequestDetails(utils.ExtractVulnerabilitiesDetailsToRows(vulnerabilities), cfp.OutputWriter) prTitle, prBody, err = cfp.preparePullRequestDetails(vulnerabilities...) assert.NoError(t, err) diff --git a/utils/git.go b/utils/git.go index 3cda949f2..efb88a085 100644 --- a/utils/git.go +++ b/utils/git.go @@ -395,8 +395,8 @@ func formatStringWithPlaceHolders(str, impactedPackage, fixVersion, hash, baseBr return str } -func (gm *GitManager) GenerateFixBranchName(branch string, impactedPackage string, fixVersion string, uniqueProjectHash string) (string, error) { - hash, err := Md5Hash("frogbot", branch, impactedPackage, fixVersion, uniqueProjectHash) +func (gm *GitManager) GenerateFixBranchName(branch string, impactedPackage string, fixVersion string, uniqueProjectIdentifier string) (string, error) { + hash, err := Md5Hash("frogbot", branch, impactedPackage, fixVersion, uniqueProjectIdentifier) if err != nil { return "", err } @@ -409,10 +409,10 @@ func (gm *GitManager) GenerateFixBranchName(branch string, impactedPackage strin return formatStringWithPlaceHolders(branchFormat, fixedPackageName, fixVersion, hash, "", false), nil } -func (gm *GitManager) GeneratePullRequestTitle(impactedPackage string, version string, uniqueProjectHash string) string { +func (gm *GitManager) GeneratePullRequestTitle(impactedPackage string, version string, uniqueProjectIdentifier string) string { template := PullRequestTitleTemplate - if uniqueProjectHash != "" { - template += " (" + uniqueProjectHash + ")" + if uniqueProjectIdentifier != "" { + template += " (" + uniqueProjectIdentifier + ")" } pullRequestFormat := gm.customTemplates.pullRequestTitleTemplate if pullRequestFormat != "" { diff --git a/utils/params.go b/utils/params.go index 2b137a7ac..cc0b76ee0 100644 --- a/utils/params.go +++ b/utils/params.go @@ -77,16 +77,16 @@ func (p *Params) setDefaultsIfNeeded(gitParamsFromEnv *Git, commandName string) } type Project struct { - InstallCommand string `yaml:"installCommand,omitempty"` - PipRequirementsFile string `yaml:"pipRequirementsFile,omitempty"` - WorkingDirs []string `yaml:"workingDirs,omitempty"` - PathExclusions []string `yaml:"pathExclusions,omitempty"` - UseWrapper *bool `yaml:"useWrapper,omitempty"` - DepsRepo string `yaml:"repository,omitempty"` - InstallCommandName string - InstallCommandArgs []string - IsRecursiveScan bool - UniqueProjectHash string `yaml:"uniqueIdentifier,omitempty"` + InstallCommand string `yaml:"installCommand,omitempty"` + PipRequirementsFile string `yaml:"pipRequirementsFile,omitempty"` + WorkingDirs []string `yaml:"workingDirs,omitempty"` + PathExclusions []string `yaml:"pathExclusions,omitempty"` + UseWrapper *bool `yaml:"useWrapper,omitempty"` + DepsRepo string `yaml:"repository,omitempty"` + InstallCommandName string + InstallCommandArgs []string + IsRecursiveScan bool + UniqueProjectIdentifier string `yaml:"uniqueIdentifier,omitempty"` } func (p *Project) setDefaultsIfNeeded() error { From af46eaf6f9651e6652081761c13d311ed968d4f4 Mon Sep 17 00:00:00 2001 From: Eran Turgeman Date: Tue, 5 Mar 2024 11:19:36 +0200 Subject: [PATCH 5/6] CR fixes --- scanrepository/scanrepository.go | 6 ++-- scanrepository/scanrepository_test.go | 4 +-- schema/frogbot-schema.json | 6 ++-- utils/git.go | 10 +++--- utils/git_test.go | 52 +++++++++++++-------------- utils/params.go | 20 +++++------ 6 files changed, 49 insertions(+), 49 deletions(-) diff --git a/scanrepository/scanrepository.go b/scanrepository/scanrepository.go index b8d692be0..594f35d56 100644 --- a/scanrepository/scanrepository.go +++ b/scanrepository/scanrepository.go @@ -298,7 +298,7 @@ func (cfp *ScanRepositoryCmd) handleUpdatePackageErrors(err error) error { func (cfp *ScanRepositoryCmd) fixSinglePackageAndCreatePR(vulnDetails *utils.VulnerabilityDetails) (err error) { fixVersion := vulnDetails.SuggestedFixedVersion log.Debug("Attempting to fix", fmt.Sprintf("%s:%s", vulnDetails.ImpactedDependencyName, vulnDetails.ImpactedDependencyVersion), "with", fixVersion) - fixBranchName, err := cfp.gitManager.GenerateFixBranchName(cfp.scanDetails.BaseBranch(), vulnDetails.ImpactedDependencyName, fixVersion, cfp.scanDetails.Project.UniqueProjectIdentifier) + fixBranchName, err := cfp.gitManager.GenerateFixBranchName(cfp.scanDetails.BaseBranch(), vulnDetails.ImpactedDependencyName, fixVersion, cfp.scanDetails.Project.ProjectName) if err != nil { return } @@ -398,7 +398,7 @@ func (cfp *ScanRepositoryCmd) preparePullRequestDetails(vulnerabilitiesDetails . } // In separate pull requests there is only one vulnerability vulnDetails := vulnerabilitiesDetails[0] - pullRequestTitle := cfp.gitManager.GeneratePullRequestTitle(vulnDetails.ImpactedDependencyName, vulnDetails.SuggestedFixedVersion, cfp.scanDetails.UniqueProjectIdentifier) + pullRequestTitle := cfp.gitManager.GeneratePullRequestTitle(vulnDetails.ImpactedDependencyName, vulnDetails.SuggestedFixedVersion, cfp.scanDetails.ProjectName) return pullRequestTitle, prBody, nil } @@ -505,10 +505,10 @@ func (cfp *ScanRepositoryCmd) updatePackageToFixedVersion(vulnDetails *utils.Vul } */ handler := packagehandlers.GetCompatiblePackageHandler(vulnDetails, cfp.scanDetails) - cfp.handlers[vulnDetails.Technology] = handler if _, unsupported := handler.(*packagehandlers.UnsupportedPackageHandler); unsupported { return } + cfp.handlers[vulnDetails.Technology] = handler return cfp.handlers[vulnDetails.Technology].UpdateDependency(vulnDetails) } diff --git a/scanrepository/scanrepository_test.go b/scanrepository/scanrepository_test.go index 7caf63f7a..e64628d4d 100644 --- a/scanrepository/scanrepository_test.go +++ b/scanrepository/scanrepository_test.go @@ -601,7 +601,7 @@ func TestPreparePullRequestDetails(t *testing.T) { cfp := ScanRepositoryCmd{ OutputWriter: &outputwriter.StandardOutput{}, gitManager: &utils.GitManager{}, scanDetails: &utils.ScanDetails{ - Project: &utils.Project{UniqueProjectIdentifier: ""}, + Project: &utils.Project{ProjectName: ""}, }} cfp.OutputWriter.SetJasOutputFlags(true, false) vulnerabilities := []*utils.VulnerabilityDetails{ @@ -625,7 +625,7 @@ func TestPreparePullRequestDetails(t *testing.T) { assert.Equal(t, "[🐸 Frogbot] Update version of package1 to 1.0.0", prTitle) assert.Equal(t, expectedPrBody, prBody) - cfp.scanDetails.UniqueProjectIdentifier = "my-unique-identifier" + cfp.scanDetails.ProjectName = "my-unique-identifier" expectedPrBody = utils.GenerateFixPullRequestDetails(utils.ExtractVulnerabilitiesDetailsToRows(vulnerabilities), cfp.OutputWriter) prTitle, prBody, err = cfp.preparePullRequestDetails(vulnerabilities...) assert.NoError(t, err) diff --git a/schema/frogbot-schema.json b/schema/frogbot-schema.json index 2e3787b07..7fd828e9f 100644 --- a/schema/frogbot-schema.json +++ b/schema/frogbot-schema.json @@ -220,10 +220,10 @@ "title": "Virtual Artifactory Repository", "description": "Name of a Virtual Repository in Artifactory to resolve (download) the project dependencies from" }, - "uniqueIdentifier": { + "projectName": { "type": "string", - "title": "Unique Hash Identifier", - "description": "Unique hash identifier for each project that is used to create separation between projects" + "title": "Unique Project Name", + "description": "A unique name assigned to the project to establish separation between projects." } } } diff --git a/utils/git.go b/utils/git.go index efb88a085..711afe1b1 100644 --- a/utils/git.go +++ b/utils/git.go @@ -395,8 +395,8 @@ func formatStringWithPlaceHolders(str, impactedPackage, fixVersion, hash, baseBr return str } -func (gm *GitManager) GenerateFixBranchName(branch string, impactedPackage string, fixVersion string, uniqueProjectIdentifier string) (string, error) { - hash, err := Md5Hash("frogbot", branch, impactedPackage, fixVersion, uniqueProjectIdentifier) +func (gm *GitManager) GenerateFixBranchName(branch string, impactedPackage string, fixVersion string, projectName string) (string, error) { + hash, err := Md5Hash("frogbot", branch, impactedPackage, fixVersion, projectName) if err != nil { return "", err } @@ -409,10 +409,10 @@ func (gm *GitManager) GenerateFixBranchName(branch string, impactedPackage strin return formatStringWithPlaceHolders(branchFormat, fixedPackageName, fixVersion, hash, "", false), nil } -func (gm *GitManager) GeneratePullRequestTitle(impactedPackage string, version string, uniqueProjectIdentifier string) string { +func (gm *GitManager) GeneratePullRequestTitle(impactedPackage string, version string, projectName string) string { template := PullRequestTitleTemplate - if uniqueProjectIdentifier != "" { - template += " (" + uniqueProjectIdentifier + ")" + if projectName != "" { + template += " (" + projectName + ")" } pullRequestFormat := gm.customTemplates.pullRequestTitleTemplate if pullRequestFormat != "" { diff --git a/utils/git_test.go b/utils/git_test.go index 0ee42abbb..6592e27ca 100644 --- a/utils/git_test.go +++ b/utils/git_test.go @@ -66,12 +66,12 @@ func TestGitManager_GenerateCommitMessage(t *testing.T) { func TestGitManager_GenerateFixBranchName(t *testing.T) { testCases := []struct { - gitManager GitManager - impactedPackage string - fixVersion VulnerabilityDetails - expected string - description string - uniqueProjectIdentifier string + gitManager GitManager + impactedPackage string + fixVersion VulnerabilityDetails + expected string + description string + projectName string }{ { gitManager: GitManager{customTemplates: CustomTemplates{branchNameTemplate: "[Feature]-${IMPACTED_PACKAGE}-${BRANCH_NAME_HASH}"}}, @@ -95,17 +95,17 @@ func TestGitManager_GenerateFixBranchName(t *testing.T) { description: "Custom template without inputs", }, { - gitManager: GitManager{customTemplates: CustomTemplates{branchNameTemplate: "just-a-branch-${BRANCH_NAME_HASH}"}}, - impactedPackage: "mquery", - fixVersion: VulnerabilityDetails{SuggestedFixedVersion: "3.4.5"}, - expected: "just-a-branch-576bdc1ddb2ad676551efd7a47f48ece", - description: "Custom template without inputs", - uniqueProjectIdentifier: "my-identifier", + gitManager: GitManager{customTemplates: CustomTemplates{branchNameTemplate: "just-a-branch-${BRANCH_NAME_HASH}"}}, + impactedPackage: "mquery", + fixVersion: VulnerabilityDetails{SuggestedFixedVersion: "3.4.5"}, + expected: "just-a-branch-576bdc1ddb2ad676551efd7a47f48ece", + description: "Custom template without inputs", + projectName: "my-identifier", }, } for _, test := range testCases { t.Run(test.expected, func(t *testing.T) { - commitMessage, err := test.gitManager.GenerateFixBranchName("md5Branch", test.impactedPackage, test.fixVersion.SuggestedFixedVersion, test.uniqueProjectIdentifier) + commitMessage, err := test.gitManager.GenerateFixBranchName("md5Branch", test.impactedPackage, test.fixVersion.SuggestedFixedVersion, test.projectName) assert.NoError(t, err) assert.Equal(t, test.expected, commitMessage) }) @@ -114,12 +114,12 @@ func TestGitManager_GenerateFixBranchName(t *testing.T) { func TestGitManager_GeneratePullRequestTitle(t *testing.T) { testCases := []struct { - gitManager GitManager - impactedPackage string - fixVersion VulnerabilityDetails - expected string - description string - uniqueProjectIdentifier string + gitManager GitManager + impactedPackage string + fixVersion VulnerabilityDetails + expected string + description string + projectName string }{ { gitManager: GitManager{customTemplates: CustomTemplates{pullRequestTitleTemplate: "[CustomPR] update ${IMPACTED_PACKAGE} to ${FIX_VERSION}"}}, @@ -143,17 +143,17 @@ func TestGitManager_GeneratePullRequestTitle(t *testing.T) { description: "No prefix", }, { - gitManager: GitManager{customTemplates: CustomTemplates{pullRequestTitleTemplate: ""}}, - impactedPackage: "mquery", - fixVersion: VulnerabilityDetails{SuggestedFixedVersion: "3.4.5"}, - expected: "[🐸 Frogbot] Update version of mquery to 3.4.5 (my-identifier)", - description: "No prefix", - uniqueProjectIdentifier: "my-identifier", + gitManager: GitManager{customTemplates: CustomTemplates{pullRequestTitleTemplate: ""}}, + impactedPackage: "mquery", + fixVersion: VulnerabilityDetails{SuggestedFixedVersion: "3.4.5"}, + expected: "[🐸 Frogbot] Update version of mquery to 3.4.5 (my-identifier)", + description: "No prefix", + projectName: "my-identifier", }, } for _, test := range testCases { t.Run(test.expected, func(t *testing.T) { - titleOutput := test.gitManager.GeneratePullRequestTitle(test.impactedPackage, test.fixVersion.SuggestedFixedVersion, test.uniqueProjectIdentifier) + titleOutput := test.gitManager.GeneratePullRequestTitle(test.impactedPackage, test.fixVersion.SuggestedFixedVersion, test.projectName) assert.Equal(t, test.expected, titleOutput) }) } diff --git a/utils/params.go b/utils/params.go index cc0b76ee0..4984c35bc 100644 --- a/utils/params.go +++ b/utils/params.go @@ -77,16 +77,16 @@ func (p *Params) setDefaultsIfNeeded(gitParamsFromEnv *Git, commandName string) } type Project struct { - InstallCommand string `yaml:"installCommand,omitempty"` - PipRequirementsFile string `yaml:"pipRequirementsFile,omitempty"` - WorkingDirs []string `yaml:"workingDirs,omitempty"` - PathExclusions []string `yaml:"pathExclusions,omitempty"` - UseWrapper *bool `yaml:"useWrapper,omitempty"` - DepsRepo string `yaml:"repository,omitempty"` - InstallCommandName string - InstallCommandArgs []string - IsRecursiveScan bool - UniqueProjectIdentifier string `yaml:"uniqueIdentifier,omitempty"` + InstallCommand string `yaml:"installCommand,omitempty"` + PipRequirementsFile string `yaml:"pipRequirementsFile,omitempty"` + WorkingDirs []string `yaml:"workingDirs,omitempty"` + PathExclusions []string `yaml:"pathExclusions,omitempty"` + UseWrapper *bool `yaml:"useWrapper,omitempty"` + DepsRepo string `yaml:"repository,omitempty"` + InstallCommandName string + InstallCommandArgs []string + IsRecursiveScan bool + ProjectName string `yaml:"projectName,omitempty"` } func (p *Project) setDefaultsIfNeeded() error { From b09f28bf7891414cb120d1194d301c218ef5e9a7 Mon Sep 17 00:00:00 2001 From: Eran Turgeman Date: Tue, 5 Mar 2024 14:17:25 +0200 Subject: [PATCH 6/6] CR fixes - left a comment --- scanrepository/scanrepository.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/scanrepository/scanrepository.go b/scanrepository/scanrepository.go index 594f35d56..394327b8d 100644 --- a/scanrepository/scanrepository.go +++ b/scanrepository/scanrepository.go @@ -40,8 +40,8 @@ type ScanRepositoryCmd struct { aggregateFixes bool // The current project technology projectTech []coreutils.Technology - // Stores all package manager handlers for detected issues - handlers map[coreutils.Technology]packagehandlers.PackageHandler + // Stores all package manager handler for detected issues + handler packagehandlers.PackageHandler } func (cfp *ScanRepositoryCmd) Run(repoAggregator utils.RepoAggregator, client vcsclient.VcsClient, frogbotRepoConnection *utils.UrlAccessChecker) (err error) { @@ -491,15 +491,15 @@ func (cfp *ScanRepositoryCmd) updatePackageToFixedVersion(vulnDetails *utils.Vul return } - if cfp.handlers == nil { - cfp.handlers = make(map[coreutils.Technology]packagehandlers.PackageHandler) - } - /* - handler := cfp.handlers[vulnDetails.Technology] + if cfp.handler == nil { + cfp.handler = make(map[coreutils.Technology]packagehandlers.PackageHandler) + } + + handler := cfp.handler[vulnDetails.Technology] if handler == nil { handler = packagehandlers.GetCompatiblePackageHandler(vulnDetails, cfp.scanDetails) - cfp.handlers[vulnDetails.Technology] = handler + cfp.handler[vulnDetails.Technology] = handler } else if _, unsupported := handler.(*packagehandlers.UnsupportedPackageHandler); unsupported { return } @@ -508,9 +508,9 @@ func (cfp *ScanRepositoryCmd) updatePackageToFixedVersion(vulnDetails *utils.Vul if _, unsupported := handler.(*packagehandlers.UnsupportedPackageHandler); unsupported { return } - cfp.handlers[vulnDetails.Technology] = handler + cfp.handler = handler - return cfp.handlers[vulnDetails.Technology].UpdateDependency(vulnDetails) + return cfp.handler.UpdateDependency(vulnDetails) } // The getRemoteBranchScanHash function extracts the checksum written inside the pull request body and returns it.