From ead365c8c7ad283a255a5e83baa429a2daf936ea Mon Sep 17 00:00:00 2001 From: Or Toren Date: Tue, 10 Feb 2026 13:07:26 +0200 Subject: [PATCH 1/4] added full path to the branch hash name --- scanrepository/scanrepository.go | 6 +++--- scanrepository/scanrepository_test.go | 30 +++++++++++++++++++++------ utils/git.go | 9 +++++--- utils/git_test.go | 2 +- 4 files changed, 34 insertions(+), 13 deletions(-) diff --git a/scanrepository/scanrepository.go b/scanrepository/scanrepository.go index 7e7bcc042..07d5f4b7e 100644 --- a/scanrepository/scanrepository.go +++ b/scanrepository/scanrepository.go @@ -309,7 +309,7 @@ func (cfp *ScanRepositoryCmd) fixProjectVulnerabilities(repository *utils.Reposi // Fix every vulnerability in a separate pull request and branch for _, vulnerability := range vulnerabilities { - if e := cfp.fixSinglePackageAndCreatePR(repository, vulnerability); e != nil { + if e := cfp.fixSinglePackageAndCreatePR(repository, fullProjectPath, vulnerability); e != nil { err = errors.Join(err, cfp.handleUpdatePackageErrors(e)) } @@ -386,10 +386,10 @@ func (cfp *ScanRepositoryCmd) 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 *ScanRepositoryCmd) fixSinglePackageAndCreatePR(repository *utils.Repository, vulnDetails *utils.VulnerabilityDetails) (err error) { +func (cfp *ScanRepositoryCmd) fixSinglePackageAndCreatePR(repository *utils.Repository, fullProjectPath string, 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, fullProjectPath) if err != nil { return } diff --git a/scanrepository/scanrepository_test.go b/scanrepository/scanrepository_test.go index 070d5765a..a63bb5014 100644 --- a/scanrepository/scanrepository_test.go +++ b/scanrepository/scanrepository_test.go @@ -15,8 +15,6 @@ import ( "github.com/CycloneDX/cyclonedx-go" "github.com/google/go-github/v45/github" biutils "github.com/jfrog/build-info-go/utils" - "github.com/jfrog/frogbot/v2/utils" - "github.com/jfrog/frogbot/v2/utils/outputwriter" "github.com/jfrog/froggit-go/vcsclient" "github.com/jfrog/froggit-go/vcsutils" "github.com/jfrog/jfrog-cli-core/v2/utils/coreutils" @@ -31,6 +29,9 @@ import ( "github.com/jfrog/jfrog-client-go/xray/services" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/jfrog/frogbot/v2/utils" + "github.com/jfrog/frogbot/v2/utils/outputwriter" ) const rootTestDir = "scanrepository" @@ -392,22 +393,39 @@ func TestGenerateFixBranchName(t *testing.T) { baseBranch string impactedPackage string fixVersion string + projectPath string expectedName 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"}, } 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.projectPath) assert.NoError(t, err) assert.Equal(t, test.expectedName, branchName) }) } } +func TestGenerateFixBranchName_UniquePerProject(t *testing.T) { + gitManager := utils.GitManager{} + branchName1, err := gitManager.GenerateFixBranchName("main", "requests", "2.25.3", "") + assert.NoError(t, err) + branchName2, err := gitManager.GenerateFixBranchName("main", "requests", "2.25.3", "subfolder") + assert.NoError(t, err) + branchName3, err := gitManager.GenerateFixBranchName("main", "requests", "2.25.3", "other/project") + assert.NoError(t, err) + assert.NotEqual(t, branchName1, branchName2, "root and subfolder projects should have different branch names") + assert.NotEqual(t, branchName1, branchName3, "root and other/project should have different branch names") + assert.NotEqual(t, branchName2, branchName3, "subfolder and other/project should have different branch names") + assert.Regexp(t, `^frogbot-requests-[a-f0-9]+$`, branchName1) + assert.Regexp(t, `^frogbot-requests-[a-f0-9]+$`, branchName2) + assert.Regexp(t, `^frogbot-requests-[a-f0-9]+$`, branchName3) +} + func TestPackageTypeFromScan(t *testing.T) { environmentVars, restoreEnv := utils.VerifyEnv(t) defer restoreEnv() diff --git a/utils/git.go b/utils/git.go index 22bd26f86..5e1375a1a 100644 --- a/utils/git.go +++ b/utils/git.go @@ -4,7 +4,6 @@ import ( "errors" "fmt" "net/http" - "regexp" "strings" "time" @@ -486,8 +485,12 @@ 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) +func (gm *GitManager) GenerateFixBranchName(branch string, impactedPackage string, fixVersion string, projectPath string) (string, error) { + hashInputs := []string{"frogbot", branch, impactedPackage, fixVersion} + if projectPath != "" { + hashInputs = append(hashInputs, projectPath) + } + hash, err := Md5Hash(hashInputs...) if err != nil { return "", err } diff --git a/utils/git_test.go b/utils/git_test.go index b1208a75e..d6c44da0b 100644 --- a/utils/git_test.go +++ b/utils/git_test.go @@ -96,7 +96,7 @@ func TestGitManager_GenerateFixBranchName(t *testing.T) { } 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, "") assert.NoError(t, err) assert.Equal(t, test.expected, commitMessage) }) From bae3f9caf7742f9cca0eae28cba9aa1e32a4b3f0 Mon Sep 17 00:00:00 2001 From: Or Toren Date: Sun, 15 Feb 2026 14:21:14 +0200 Subject: [PATCH 2/4] after code review --- scanrepository/scanrepository.go | 8 +++++--- scanrepository/scanrepository_test.go | 19 +++---------------- 2 files changed, 8 insertions(+), 19 deletions(-) diff --git a/scanrepository/scanrepository.go b/scanrepository/scanrepository.go index 07d5f4b7e..bdf9c8018 100644 --- a/scanrepository/scanrepository.go +++ b/scanrepository/scanrepository.go @@ -308,8 +308,9 @@ func (cfp *ScanRepositoryCmd) fixProjectVulnerabilities(repository *utils.Reposi } // Fix every vulnerability in a separate pull request and branch + projectPathFromRoot := projectWorkingDir for _, vulnerability := range vulnerabilities { - if e := cfp.fixSinglePackageAndCreatePR(repository, fullProjectPath, vulnerability); e != nil { + if e := cfp.fixSinglePackageAndCreatePR(repository, projectPathFromRoot, vulnerability); e != nil { err = errors.Join(err, cfp.handleUpdatePackageErrors(e)) } @@ -386,10 +387,11 @@ func (cfp *ScanRepositoryCmd) 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 *ScanRepositoryCmd) fixSinglePackageAndCreatePR(repository *utils.Repository, fullProjectPath string, vulnDetails *utils.VulnerabilityDetails) (err error) { +// projectPathFromRoot is the relative path of the project from the repository root (used for branch name uniqueness). +func (cfp *ScanRepositoryCmd) fixSinglePackageAndCreatePR(repository *utils.Repository, projectPathFromRoot string, 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, fullProjectPath) + fixBranchName, err := cfp.gitManager.GenerateFixBranchName(cfp.scanDetails.BaseBranch(), vulnDetails.ImpactedDependencyName, fixVersion, projectPathFromRoot) if err != nil { return } diff --git a/scanrepository/scanrepository_test.go b/scanrepository/scanrepository_test.go index a63bb5014..11840a109 100644 --- a/scanrepository/scanrepository_test.go +++ b/scanrepository/scanrepository_test.go @@ -399,6 +399,9 @@ func TestGenerateFixBranchName(t *testing.T) { {"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"}, + {"main", "requests", "2.25.3", "", "frogbot-requests-ae6fef399c0fdd96441b0215f28147d2"}, + {"main", "requests", "2.25.3", "subfolder", "frogbot-requests-28662794aa63a6250dd9a80f7618f841"}, + {"main", "requests", "2.25.3", "other/project", "frogbot-requests-61eeddf6eda4b867a2b75fa5630875e8"}, } gitManager := utils.GitManager{} for _, test := range tests { @@ -410,22 +413,6 @@ func TestGenerateFixBranchName(t *testing.T) { } } -func TestGenerateFixBranchName_UniquePerProject(t *testing.T) { - gitManager := utils.GitManager{} - branchName1, err := gitManager.GenerateFixBranchName("main", "requests", "2.25.3", "") - assert.NoError(t, err) - branchName2, err := gitManager.GenerateFixBranchName("main", "requests", "2.25.3", "subfolder") - assert.NoError(t, err) - branchName3, err := gitManager.GenerateFixBranchName("main", "requests", "2.25.3", "other/project") - assert.NoError(t, err) - assert.NotEqual(t, branchName1, branchName2, "root and subfolder projects should have different branch names") - assert.NotEqual(t, branchName1, branchName3, "root and other/project should have different branch names") - assert.NotEqual(t, branchName2, branchName3, "subfolder and other/project should have different branch names") - assert.Regexp(t, `^frogbot-requests-[a-f0-9]+$`, branchName1) - assert.Regexp(t, `^frogbot-requests-[a-f0-9]+$`, branchName2) - assert.Regexp(t, `^frogbot-requests-[a-f0-9]+$`, branchName3) -} - func TestPackageTypeFromScan(t *testing.T) { environmentVars, restoreEnv := utils.VerifyEnv(t) defer restoreEnv() From e9ba54840985d189eb820d43ee5dcd78ab60df63 Mon Sep 17 00:00:00 2001 From: Or Toren Date: Mon, 16 Feb 2026 09:37:12 +0200 Subject: [PATCH 3/4] go sec ignore --- utils/utils.go | 1 + 1 file changed, 1 insertion(+) diff --git a/utils/utils.go b/utils/utils.go index fac886853..9b3d43a40 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -561,6 +561,7 @@ func isUrlAccessible(url string) bool { return false } log.Debug(fmt.Sprintf("Sending HTTP %s request to: '%s'", req.Method, req.URL)) + // #nosec G107 -- URL is from Frogbot config (FrogbotRepoUrl), not user input resp, err := client.GetClient().Do(req) if errorutils.CheckError(err) != nil { log.Debug(fmt.Sprintf("Can't check access to '%s', error while sending request:\n%s", url, err.Error())) From 282096d8c300f20c44b9196371098594f11a37c8 Mon Sep 17 00:00:00 2001 From: Or Toren Date: Mon, 16 Feb 2026 10:16:05 +0200 Subject: [PATCH 4/4] go sec ignore --- utils/utils.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utils/utils.go b/utils/utils.go index 9b3d43a40..8d5b39cb5 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -561,7 +561,7 @@ func isUrlAccessible(url string) bool { return false } log.Debug(fmt.Sprintf("Sending HTTP %s request to: '%s'", req.Method, req.URL)) - // #nosec G107 -- URL is from Frogbot config (FrogbotRepoUrl), not user input + // #nosec G704 -- URL is from Frogbot config (FrogbotRepoUrl), not user input resp, err := client.GetClient().Do(req) if errorutils.CheckError(err) != nil { log.Debug(fmt.Sprintf("Can't check access to '%s', error while sending request:\n%s", url, err.Error()))