From 8e91ec422d71205fce3e5346739c041042095d99 Mon Sep 17 00:00:00 2001 From: attiasas Date: Tue, 8 Oct 2024 17:55:39 +0300 Subject: [PATCH 1/5] Scan PR - Add option to scan source and most common ancestor target commit --- scanpullrequest/scanpullrequest.go | 82 ++++++++++++++++++++++- utils/consts.go | 1 + utils/git.go | 104 +++++++++++++++++++++++++++++ utils/params.go | 6 ++ 4 files changed, 190 insertions(+), 3 deletions(-) diff --git a/scanpullrequest/scanpullrequest.go b/scanpullrequest/scanpullrequest.go index ecc1634cb..bbec866b5 100644 --- a/scanpullrequest/scanpullrequest.go +++ b/scanpullrequest/scanpullrequest.go @@ -179,7 +179,7 @@ func auditPullRequestInProject(repoConfig *utils.Repository, scanDetails *utils. return } - // Set JAS output flags + // // Set JAS output flags sourceScanResults := sourceResults.ExtendedScanResults repoConfig.OutputWriter.SetJasOutputFlags(sourceScanResults.EntitledForJas, len(sourceScanResults.ApplicabilityScanResults) > 0) @@ -204,8 +204,7 @@ func auditTargetBranch(repoConfig *utils.Repository, scanDetails *utils.ScanDeta // Download target branch (if needed) cleanupTarget := func() error { return nil } if !repoConfig.IncludeAllVulnerabilities { - targetBranchInfo := repoConfig.PullRequestDetails.Target - if targetBranchWd, cleanupTarget, err = utils.DownloadRepoToTempDir(scanDetails.Client(), targetBranchInfo.Owner, targetBranchInfo.Repository, targetBranchInfo.Name); err != nil { + if targetBranchWd, cleanupTarget, err = prepareTargetForScan(repoConfig.PullRequestDetails, scanDetails); err != nil { return } } @@ -227,6 +226,83 @@ func auditTargetBranch(repoConfig *utils.Repository, scanDetails *utils.ScanDeta return } +func prepareTargetForScan(pullRequestDetails vcsclient.PullRequestInfo, scanDetails *utils.ScanDetails) (targetBranchWd string, cleanupTarget func() error, err error) { + target := pullRequestDetails.Target + // Download target branch + if targetBranchWd, cleanupTarget, err = utils.DownloadRepoToTempDir(scanDetails.Client(), target.Owner, target.Repository, target.Name); err != nil { + return + } + if !scanDetails.Git.UseMostCommonAncestorAsTarget { + return + } + log.Debug("Using most common ancestor commit as target branch commit") + // Get common parent commit between source and target and use it (checkout) to the target branch commit + if e := tryCheckoutToMostCommonAncestor(scanDetails, pullRequestDetails.Source.Name, target.Name, targetBranchWd); e != nil { + log.Warn(fmt.Sprintf("Failed to get best common ancestor commit between source branch: %s and target branch: %s, defaulting to target branch commit. Error: %s", pullRequestDetails.Source.Name, target.Name, e.Error())) + } + return +} + +func tryCheckoutToMostCommonAncestor(scanDetails *utils.ScanDetails, baseBranch, headBranch, targetBranchWd string) (err error) { + repositoryInfo, err := scanDetails.Client().GetRepositoryInfo(context.Background(), scanDetails.RepoOwner, scanDetails.RepoName) + if err != nil { + return + } + log.Info(fmt.Sprintf("RepositoryCloneUrl: %s", repositoryInfo.CloneInfo.HTTP)) + scanDetails.Git.RepositoryCloneUrl = repositoryInfo.CloneInfo.HTTP + + bestAncestorHash, err := getMostCommonAncestorCommitHash(scanDetails, baseBranch, headBranch) + if err != nil { + return + } + return checkoutToCommitAtTempWorkingDir(scanDetails, bestAncestorHash, targetBranchWd) + + // gitManager, err := utils.NewGitManager().SetAuth(scanDetails.Username, scanDetails.Token).SetRemoteGitUrl(scanDetails.Git.RepositoryCloneUrl) + // if err != nil { + // return + // } + // log.Info(fmt.Sprintf("RepositoryCloneUrl After: %s", scanDetails.Git.RepositoryCloneUrl)) + + // bestAncestorHash, err := gitManager.GetMostCommonAncestorHash(baseBranch, headBranch) + // if err != nil { + // return + // } + // cwd, err := os.Getwd() + // if err != nil { + // return + // } + // if err = os.Chdir(targetBranchWd); err != nil { + // return + // } + // defer os.Chdir(cwd) + // log.Debug("Checking out to best common ancestor commit hash: " + bestAncestorHash) + // return gitManager.CheckoutToHash(bestAncestorHash, targetBranchWd) +} + +func getMostCommonAncestorCommitHash(scanDetails *utils.ScanDetails, baseBranch, headBranch string) (hash string, err error) { + gitManager, err := utils.NewGitManager().SetAuth(scanDetails.Username, scanDetails.Token).SetRemoteGitUrl(scanDetails.Git.RepositoryCloneUrl) + if err != nil { + return + } + return gitManager.GetMostCommonAncestorHash(baseBranch, headBranch) +} + +func checkoutToCommitAtTempWorkingDir(scanDetails *utils.ScanDetails, commitHash, wd string) (err error) { + cwd, err := os.Getwd() + if err != nil { + return + } + if err = os.Chdir(wd); err != nil { + return + } + defer os.Chdir(cwd) + gitManager, err := utils.NewGitManager().SetAuth(scanDetails.Username, scanDetails.Token).SetRemoteGitUrl(scanDetails.Git.RepositoryCloneUrl) + if err != nil { + return + } + return gitManager.CheckoutToHash(commitHash, wd) +} + func getAllIssues(results *securityutils.Results, allowedLicenses []string) (*utils.IssuesCollection, error) { log.Info("Frogbot is configured to show all vulnerabilities") scanResults := results.ExtendedScanResults diff --git a/utils/consts.go b/utils/consts.go index 4e1e005f9..2f50e58b1 100644 --- a/utils/consts.go +++ b/utils/consts.go @@ -45,6 +45,7 @@ const ( CommitMessageTemplateEnv = "JF_COMMIT_MESSAGE_TEMPLATE" PullRequestTitleTemplateEnv = "JF_PULL_REQUEST_TITLE_TEMPLATE" PullRequestCommentTitleEnv = "JF_PR_COMMENT_TITLE" + UseMostCommonAncestorAsTargetEnv = "JF_USE_MOST_COMMON_ANCESTOR_AS_TARGET" // Repository environment variables - Ignored if the frogbot-config.yml file is used InstallCommandEnv = "JF_INSTALL_DEPS_CMD" diff --git a/utils/git.go b/utils/git.go index 986a3fa87..4b2cc69d9 100644 --- a/utils/git.go +++ b/utils/git.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" "net/http" + "os/exec" "regexp" "strings" "time" @@ -154,6 +155,89 @@ func (gm *GitManager) Checkout(branchName string) error { return nil } +func (gm *GitManager) CheckoutToHash(hash, targetBranchWd string) error { + if err := gm.Fetch(); err != nil { + return err + } + log.Debug("Running git checkout to hash:", hash) + if err := gm.createBranchAndCheckoutToHash(hash, false); err != nil { + return fmt.Errorf("'git checkout %s' failed with error: %s", hash, err.Error()) + } + return nil + // worktree, err := gm.localGitRepository.Worktree() + // if err != nil { + // return err + // } + // if err = worktree.Checkout(&git.CheckoutOptions{Hash: plumbing.NewHash(hash), Keep: true}); err != nil { + // return fmt.Errorf("'git checkout %s' failed with error: %s", hash, err.Error()) + // } + // cmd := exec.Command("git", "checkout", hash) + // cmd.Dir = targetBranchWd + // if output, err := cmd.CombinedOutput(); err != nil { + // return fmt.Errorf("'git checkout %s' failed with error: %s", hash, string(output)) + // } else if len(output) > 0 { + // log.Debug("git checkout output:", string(output)) + // } + // return nil +} + +func (gm *GitManager) Fetch() error { + log.Debug("Running git fetch...") + err := gm.localGitRepository.Fetch(&git.FetchOptions{ + RemoteName: gm.remoteName, + RemoteURL: gm.remoteGitUrl, + Auth: gm.auth, + }) + if err != nil && err != git.NoErrAlreadyUpToDate { + return fmt.Errorf("git fetch failed with error: %s", err.Error()) + } + log.Debug("Running local git fetch...") + cmd := exec.Command("git", "fetch", "--all") + if output, err := cmd.CombinedOutput(); err != nil { + return fmt.Errorf("git fetch failed with error: %s", string(output)) + } else if len(output) > 0 { + log.Debug("git fetch output:", string(output)) + } + return nil +} + +func (gm *GitManager) GetMostCommonAncestorHash(baseBranch, headBranch string) (string, error) { + // Make sure data is fetched + // if err := gm.Fetch(); err != nil { + // return "", err + // } + // Get the commit of the base branch + baseCommitHash, err := gm.localGitRepository.ResolveRevision(plumbing.Revision(baseBranch)) + if err != nil { + return "", err + } + baseCommit, err := gm.localGitRepository.CommitObject(*baseCommitHash) + if err != nil { + return "", err + } + // Get the commit of the head branch + headCommitHash, err := gm.localGitRepository.ResolveRevision(plumbing.Revision(headBranch)) + if err != nil { + return "", err + } + headCommit, err := gm.localGitRepository.CommitObject(*headCommitHash) + if err != nil { + return "", err + } + // Get the most common ancestor + log.Debug(fmt.Sprintf("Finding common ancestor between %s and %s...", baseBranch, headBranch)) + ancestorCommit, err := baseCommit.MergeBase(headCommit) + if err != nil { + return "", err + } + if len(ancestorCommit) == 0 { + return "", fmt.Errorf("no common ancestor found for %s and %s", baseBranch, headBranch) + } else if len(ancestorCommit) > 1 { + return "", fmt.Errorf("more than one common ancestor found for %s and %s", baseBranch, headBranch) + } + return ancestorCommit[0].Hash.String(), nil +} + func (gm *GitManager) Clone(destinationPath, branchName string) error { if gm.dryRun { // "Clone" the repository from the testdata folder @@ -201,6 +285,26 @@ func (gm *GitManager) CreateBranchAndCheckout(branchName string, keepLocalChange return err } +func (gm *GitManager) createBranchAndCheckoutToHash(hash string, keepLocalChanges bool) error { + var checkoutConfig *git.CheckoutOptions + if keepLocalChanges { + checkoutConfig = &git.CheckoutOptions{ + Hash: plumbing.NewHash(hash), + Keep: true, + } + } else { + checkoutConfig = &git.CheckoutOptions{ + Hash: plumbing.NewHash(hash), + Force: true, + } + } + worktree, err := gm.localGitRepository.Worktree() + if err != nil { + return err + } + return worktree.Checkout(checkoutConfig) +} + func (gm *GitManager) createBranchAndCheckout(branchName string, create bool, keepLocalChanges bool) error { var checkoutConfig *git.CheckoutOptions if keepLocalChanges { diff --git a/utils/params.go b/utils/params.go index 8e5295731..9fe0eef91 100644 --- a/utils/params.go +++ b/utils/params.go @@ -278,6 +278,7 @@ func (jp *JFrogPlatform) setDefaultsIfNeeded() (err error) { type Git struct { GitProvider vcsutils.VcsProvider vcsclient.VcsInfo + UseMostCommonAncestorAsTarget bool `yaml:"useMostCommonAncestorAsTarget,omitempty"` RepoOwner string RepoName string `yaml:"repoName,omitempty"` Branches []string `yaml:"branches,omitempty"` @@ -329,6 +330,11 @@ func (g *Git) extractScanPullRequestEnvParams(gitParamsFromEnv *Git) (err error) if g.PullRequestCommentTitle == "" { g.PullRequestCommentTitle = getTrimmedEnv(PullRequestCommentTitleEnv) } + if !g.UseMostCommonAncestorAsTarget { + if g.UseMostCommonAncestorAsTarget, err = getBoolEnv(UseMostCommonAncestorAsTargetEnv, true); err != nil { + return + } + } g.AvoidExtraMessages, err = getBoolEnv(AvoidExtraMessages, false) return } From 61b1ac97ccd74571a15b352dd3cfb0c53aac7261 Mon Sep 17 00:00:00 2001 From: attiasas Date: Tue, 8 Oct 2024 20:32:58 +0300 Subject: [PATCH 2/5] cleanup --- scanpullrequest/scanpullrequest.go | 27 +++------------------ utils/consts.go | 8 +++--- utils/git.go | 39 ++++++------------------------ utils/params.go | 24 +++++++++--------- 4 files changed, 26 insertions(+), 72 deletions(-) diff --git a/scanpullrequest/scanpullrequest.go b/scanpullrequest/scanpullrequest.go index bbec866b5..e962b6f3c 100644 --- a/scanpullrequest/scanpullrequest.go +++ b/scanpullrequest/scanpullrequest.go @@ -179,7 +179,7 @@ func auditPullRequestInProject(repoConfig *utils.Repository, scanDetails *utils. return } - // // Set JAS output flags + // Set JAS output flags sourceScanResults := sourceResults.ExtendedScanResults repoConfig.OutputWriter.SetJasOutputFlags(sourceScanResults.EntitledForJas, len(sourceScanResults.ApplicabilityScanResults) > 0) @@ -248,35 +248,12 @@ func tryCheckoutToMostCommonAncestor(scanDetails *utils.ScanDetails, baseBranch, if err != nil { return } - log.Info(fmt.Sprintf("RepositoryCloneUrl: %s", repositoryInfo.CloneInfo.HTTP)) scanDetails.Git.RepositoryCloneUrl = repositoryInfo.CloneInfo.HTTP - bestAncestorHash, err := getMostCommonAncestorCommitHash(scanDetails, baseBranch, headBranch) if err != nil { return } return checkoutToCommitAtTempWorkingDir(scanDetails, bestAncestorHash, targetBranchWd) - - // gitManager, err := utils.NewGitManager().SetAuth(scanDetails.Username, scanDetails.Token).SetRemoteGitUrl(scanDetails.Git.RepositoryCloneUrl) - // if err != nil { - // return - // } - // log.Info(fmt.Sprintf("RepositoryCloneUrl After: %s", scanDetails.Git.RepositoryCloneUrl)) - - // bestAncestorHash, err := gitManager.GetMostCommonAncestorHash(baseBranch, headBranch) - // if err != nil { - // return - // } - // cwd, err := os.Getwd() - // if err != nil { - // return - // } - // if err = os.Chdir(targetBranchWd); err != nil { - // return - // } - // defer os.Chdir(cwd) - // log.Debug("Checking out to best common ancestor commit hash: " + bestAncestorHash) - // return gitManager.CheckoutToHash(bestAncestorHash, targetBranchWd) } func getMostCommonAncestorCommitHash(scanDetails *utils.ScanDetails, baseBranch, headBranch string) (hash string, err error) { @@ -288,6 +265,7 @@ func getMostCommonAncestorCommitHash(scanDetails *utils.ScanDetails, baseBranch, } func checkoutToCommitAtTempWorkingDir(scanDetails *utils.ScanDetails, commitHash, wd string) (err error) { + // Change working directory to the temp target branch directory cwd, err := os.Getwd() if err != nil { return @@ -296,6 +274,7 @@ func checkoutToCommitAtTempWorkingDir(scanDetails *utils.ScanDetails, commitHash return } defer os.Chdir(cwd) + // Load .git info in directory and Checkout to the commit hash gitManager, err := utils.NewGitManager().SetAuth(scanDetails.Username, scanDetails.Token).SetRemoteGitUrl(scanDetails.Git.RepositoryCloneUrl) if err != nil { return diff --git a/utils/consts.go b/utils/consts.go index 2f50e58b1..f23bcf3f7 100644 --- a/utils/consts.go +++ b/utils/consts.go @@ -41,10 +41,10 @@ const ( GitUsernameEnv = "JF_GIT_USERNAME" // Git naming template environment variables - BranchNameTemplateEnv = "JF_BRANCH_NAME_TEMPLATE" - CommitMessageTemplateEnv = "JF_COMMIT_MESSAGE_TEMPLATE" - PullRequestTitleTemplateEnv = "JF_PULL_REQUEST_TITLE_TEMPLATE" - PullRequestCommentTitleEnv = "JF_PR_COMMENT_TITLE" + BranchNameTemplateEnv = "JF_BRANCH_NAME_TEMPLATE" + CommitMessageTemplateEnv = "JF_COMMIT_MESSAGE_TEMPLATE" + PullRequestTitleTemplateEnv = "JF_PULL_REQUEST_TITLE_TEMPLATE" + PullRequestCommentTitleEnv = "JF_PR_COMMENT_TITLE" UseMostCommonAncestorAsTargetEnv = "JF_USE_MOST_COMMON_ANCESTOR_AS_TARGET" // Repository environment variables - Ignored if the frogbot-config.yml file is used diff --git a/utils/git.go b/utils/git.go index 4b2cc69d9..f0350cffb 100644 --- a/utils/git.go +++ b/utils/git.go @@ -4,7 +4,7 @@ import ( "errors" "fmt" "net/http" - "os/exec" + // "os/exec" "regexp" "strings" "time" @@ -156,6 +156,7 @@ func (gm *GitManager) Checkout(branchName string) error { } func (gm *GitManager) CheckoutToHash(hash, targetBranchWd string) error { + log.Debug("Running git fetch...") if err := gm.Fetch(); err != nil { return err } @@ -164,48 +165,22 @@ func (gm *GitManager) CheckoutToHash(hash, targetBranchWd string) error { return fmt.Errorf("'git checkout %s' failed with error: %s", hash, err.Error()) } return nil - // worktree, err := gm.localGitRepository.Worktree() - // if err != nil { - // return err - // } - // if err = worktree.Checkout(&git.CheckoutOptions{Hash: plumbing.NewHash(hash), Keep: true}); err != nil { - // return fmt.Errorf("'git checkout %s' failed with error: %s", hash, err.Error()) - // } - // cmd := exec.Command("git", "checkout", hash) - // cmd.Dir = targetBranchWd - // if output, err := cmd.CombinedOutput(); err != nil { - // return fmt.Errorf("'git checkout %s' failed with error: %s", hash, string(output)) - // } else if len(output) > 0 { - // log.Debug("git checkout output:", string(output)) - // } - // return nil } func (gm *GitManager) Fetch() error { log.Debug("Running git fetch...") err := gm.localGitRepository.Fetch(&git.FetchOptions{ RemoteName: gm.remoteName, - RemoteURL: gm.remoteGitUrl, + RemoteURL: gm.remoteGitUrl, Auth: gm.auth, }) if err != nil && err != git.NoErrAlreadyUpToDate { return fmt.Errorf("git fetch failed with error: %s", err.Error()) } - log.Debug("Running local git fetch...") - cmd := exec.Command("git", "fetch", "--all") - if output, err := cmd.CombinedOutput(); err != nil { - return fmt.Errorf("git fetch failed with error: %s", string(output)) - } else if len(output) > 0 { - log.Debug("git fetch output:", string(output)) - } return nil } func (gm *GitManager) GetMostCommonAncestorHash(baseBranch, headBranch string) (string, error) { - // Make sure data is fetched - // if err := gm.Fetch(); err != nil { - // return "", err - // } // Get the commit of the base branch baseCommitHash, err := gm.localGitRepository.ResolveRevision(plumbing.Revision(baseBranch)) if err != nil { @@ -289,13 +264,13 @@ func (gm *GitManager) createBranchAndCheckoutToHash(hash string, keepLocalChange var checkoutConfig *git.CheckoutOptions if keepLocalChanges { checkoutConfig = &git.CheckoutOptions{ - Hash: plumbing.NewHash(hash), - Keep: true, + Hash: plumbing.NewHash(hash), + Keep: true, } } else { checkoutConfig = &git.CheckoutOptions{ - Hash: plumbing.NewHash(hash), - Force: true, + Hash: plumbing.NewHash(hash), + Force: true, } } worktree, err := gm.localGitRepository.Worktree() diff --git a/utils/params.go b/utils/params.go index 9fe0eef91..9ba7f4f21 100644 --- a/utils/params.go +++ b/utils/params.go @@ -279,18 +279,18 @@ type Git struct { GitProvider vcsutils.VcsProvider vcsclient.VcsInfo UseMostCommonAncestorAsTarget bool `yaml:"useMostCommonAncestorAsTarget,omitempty"` - RepoOwner string - RepoName string `yaml:"repoName,omitempty"` - Branches []string `yaml:"branches,omitempty"` - BranchNameTemplate string `yaml:"branchNameTemplate,omitempty"` - CommitMessageTemplate string `yaml:"commitMessageTemplate,omitempty"` - PullRequestTitleTemplate string `yaml:"pullRequestTitleTemplate,omitempty"` - PullRequestCommentTitle string `yaml:"pullRequestCommentTitle,omitempty"` - AvoidExtraMessages bool `yaml:"avoidExtraMessages,omitempty"` - EmailAuthor string `yaml:"emailAuthor,omitempty"` - AggregateFixes bool `yaml:"aggregateFixes,omitempty"` - PullRequestDetails vcsclient.PullRequestInfo - RepositoryCloneUrl string + RepoOwner string + RepoName string `yaml:"repoName,omitempty"` + Branches []string `yaml:"branches,omitempty"` + BranchNameTemplate string `yaml:"branchNameTemplate,omitempty"` + CommitMessageTemplate string `yaml:"commitMessageTemplate,omitempty"` + PullRequestTitleTemplate string `yaml:"pullRequestTitleTemplate,omitempty"` + PullRequestCommentTitle string `yaml:"pullRequestCommentTitle,omitempty"` + AvoidExtraMessages bool `yaml:"avoidExtraMessages,omitempty"` + EmailAuthor string `yaml:"emailAuthor,omitempty"` + AggregateFixes bool `yaml:"aggregateFixes,omitempty"` + PullRequestDetails vcsclient.PullRequestInfo + RepositoryCloneUrl string } func (g *Git) setDefaultsIfNeeded(gitParamsFromEnv *Git, commandName string) (err error) { From 4b1c02e8070444031a1bfb4452d88cc87877db1f Mon Sep 17 00:00:00 2001 From: attiasas Date: Thu, 10 Oct 2024 10:57:21 +0300 Subject: [PATCH 3/5] Update dependencies --- go.mod | 7 ++++--- go.sum | 12 ++++++------ 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/go.mod b/go.mod index 2f489caa7..d9aee55f0 100644 --- a/go.mod +++ b/go.mod @@ -9,9 +9,9 @@ require ( github.com/jfrog/build-info-go v1.10.1 github.com/jfrog/froggit-go v1.16.1 github.com/jfrog/gofrog v1.7.6 - github.com/jfrog/jfrog-cli-core/v2 v2.56.1 + github.com/jfrog/jfrog-cli-core/v2 v2.56.0 github.com/jfrog/jfrog-cli-security v1.10.1 - github.com/jfrog/jfrog-client-go v1.47.1 + github.com/jfrog/jfrog-client-go v1.47.0 github.com/jordan-wright/email v4.0.1-0.20210109023952-943e75fe5223+incompatible github.com/owenrumney/go-sarif/v2 v2.3.1 github.com/stretchr/testify v1.9.0 @@ -119,7 +119,8 @@ require ( gopkg.in/warnings.v0 v0.1.2 // indirect ) -// replace github.com/jfrog/jfrog-cli-security => github.com/jfrog/jfrog-cli-security dev +// hadarshjfrog:am_ver_1_9_4 +replace github.com/jfrog/jfrog-cli-security => github.com/hadarshjfrog/jfrog-cli-security v0.0.0-20241010072437-f2240586d479 // replace github.com/jfrog/jfrog-cli-core/v2 => github.com/jfrog/jfrog-cli-core/v2 dev diff --git a/go.sum b/go.sum index 941addce2..4163967a1 100644 --- a/go.sum +++ b/go.sum @@ -871,6 +871,8 @@ github.com/grokify/mogo v0.62.6/go.mod h1:gK6Qf761S7iOxI3LrILjoTOJWdQPgs8LxSPdDm github.com/grpc-ecosystem/grpc-gateway v1.16.0/go.mod h1:BDjrQk3hbvj6Nolgz8mAMFbcEtjT1g+wF4CSlocrBnw= github.com/grpc-ecosystem/grpc-gateway/v2 v2.7.0/go.mod h1:hgWBS7lorOAVIJEQMi4ZsPv9hVvWI6+ch50m39Pf2Ks= github.com/grpc-ecosystem/grpc-gateway/v2 v2.11.3/go.mod h1:o//XUCC/F+yRGJoPO/VU0GSB0f8Nhgmxx0VIRUvaC0w= +github.com/hadarshjfrog/jfrog-cli-security v0.0.0-20241010072437-f2240586d479 h1:0wV0K4gaJ0nzdNhZ98B1RmsRUpa6kcHRNEPqkTK9pxE= +github.com/hadarshjfrog/jfrog-cli-security v0.0.0-20241010072437-f2240586d479/go.mod h1:0vBYBP1jztDf5e25Ww3CkQAA1C609CAccz9NJLoSoRk= github.com/hashicorp/go-cleanhttp v0.5.2 h1:035FKYIWjmULyFRBKPs8TBQoi0x6d9G4xc9neXJWAZQ= github.com/hashicorp/go-cleanhttp v0.5.2/go.mod h1:kO/YDlP8L1346E6Sodw+PrpBSV4/SoxCXGY6BqNFT48= github.com/hashicorp/go-hclog v0.9.2/go.mod h1:5CU+agLiy3J7N7QjHK5d05KxGsuXiQLrjA0H7acj2lQ= @@ -899,12 +901,10 @@ github.com/jfrog/gofrog v1.7.6 h1:QmfAiRzVyaI7JYGsB7cxfAJePAZTzFz0gRWZSE27c6s= github.com/jfrog/gofrog v1.7.6/go.mod h1:ntr1txqNOZtHplmaNd7rS4f8jpA5Apx8em70oYEe7+4= github.com/jfrog/jfrog-apps-config v1.0.1 h1:mtv6k7g8A8BVhlHGlSveapqf4mJfonwvXYLipdsOFMY= github.com/jfrog/jfrog-apps-config v1.0.1/go.mod h1:8AIIr1oY9JuH5dylz2S6f8Ym2MaadPLR6noCBO4C22w= -github.com/jfrog/jfrog-cli-core/v2 v2.56.1 h1:+Me+RQx8BYKib+RZLFtGWFftLjEd3NrjVVxJbSYElKU= -github.com/jfrog/jfrog-cli-core/v2 v2.56.1/go.mod h1:+a9VRDizwc+SK2Io6e4Yp8j7hkTeQstQTmNVwrxdh6Q= -github.com/jfrog/jfrog-cli-security v1.10.1 h1:0YfDosXXazUJVQRBPmeoUwvrmEotMSGyE+3ICELmFJE= -github.com/jfrog/jfrog-cli-security v1.10.1/go.mod h1:Z4hS3Ge6LDqOF2vXeO6duuNZyPCEaKjoyoeJ7vGoy54= -github.com/jfrog/jfrog-client-go v1.47.1 h1:VT2v28/usTSP56+i3MC3fgRvZoh6vjRgQgs8xTk+sYU= -github.com/jfrog/jfrog-client-go v1.47.1/go.mod h1:7M/vgei7VGcLjUxwQ/3r9pH3lvDHlt6Q+Gw+YMis/mc= +github.com/jfrog/jfrog-cli-core/v2 v2.56.0 h1:rCNKhfESgsq0o6//gU1mNCvuCboE5BMfycj/RM/gq8k= +github.com/jfrog/jfrog-cli-core/v2 v2.56.0/go.mod h1:D8m0L8GCZiYCY9MjhnWY4egCqyVlU2iZsVA0yysBsVw= +github.com/jfrog/jfrog-client-go v1.47.0 h1:OBMB6TxqziBByjuk6hm0BM30pQwOb3XzjZKf/cmwCeM= +github.com/jfrog/jfrog-client-go v1.47.0/go.mod h1:UxzL9Q4pDoM+HQjSuQiGNakyoJNuxqPSs35/amBJvdY= github.com/jordan-wright/email v4.0.1-0.20210109023952-943e75fe5223+incompatible h1:jdpOPRN1zP63Td1hDQbZW73xKmzDvZHzVdNYxhnTMDA= github.com/jordan-wright/email v4.0.1-0.20210109023952-943e75fe5223+incompatible/go.mod h1:1c7szIrayyPPB/987hsnvNzLushdWf4o/79s3P08L8A= github.com/jstemmer/go-junit-report v0.0.0-20190106144839-af01ea7f8024/go.mod h1:6v2b51hI/fHJwM22ozAgKL4VKDeJcHhJFhtBdhmNjmU= From cc04ab0b4231a5206983fcfac036a9bb3224b810 Mon Sep 17 00:00:00 2001 From: attiasas Date: Wed, 30 Oct 2024 14:39:40 +0200 Subject: [PATCH 4/5] merge fix --- go.mod | 3 +- go.sum | 2 -- scanpullrequest/scanpullrequest.go | 56 ------------------------------ utils/params.go | 2 +- 4 files changed, 2 insertions(+), 61 deletions(-) diff --git a/go.mod b/go.mod index 01947f4be..5b9841a41 100644 --- a/go.mod +++ b/go.mod @@ -116,8 +116,7 @@ require ( gopkg.in/warnings.v0 v0.1.2 // indirect ) -// hadarshjfrog:am_ver_1_9_4 -replace github.com/jfrog/jfrog-cli-security => github.com/hadarshjfrog/jfrog-cli-security v0.0.0-20241010072437-f2240586d479 +// replace github.com/jfrog/jfrog-cli-security => github.com/jfrog/jfrog-cli-security dev // replace github.com/jfrog/jfrog-cli-core/v2 => github.com/jfrog/jfrog-cli-core/v2 dev diff --git a/go.sum b/go.sum index 2de259c2e..8a609dcb1 100644 --- a/go.sum +++ b/go.sum @@ -110,8 +110,6 @@ github.com/gookit/color v1.5.4 h1:FZmqs7XOyGgCAxmWyPslpiok1k05wmY3SJTytgvYFs0= github.com/gookit/color v1.5.4/go.mod h1:pZJOeOS8DM43rXbp4AZo1n9zCU2qjpcRko0b6/QJi9w= github.com/grokify/mogo v0.64.12 h1:BNrZ1qBFuX4qu5722CW6qtqu/mrrsZ3bhKu/w1KowKg= github.com/grokify/mogo v0.64.12/go.mod h1:lDhfYIiOhJo7C2U3aL00PlUU9gLvmTONi4MdIWoGmGM= -github.com/hadarshjfrog/jfrog-cli-security v0.0.0-20241010072437-f2240586d479 h1:0wV0K4gaJ0nzdNhZ98B1RmsRUpa6kcHRNEPqkTK9pxE= -github.com/hadarshjfrog/jfrog-cli-security v0.0.0-20241010072437-f2240586d479/go.mod h1:0vBYBP1jztDf5e25Ww3CkQAA1C609CAccz9NJLoSoRk= github.com/hashicorp/go-cleanhttp v0.5.2 h1:035FKYIWjmULyFRBKPs8TBQoi0x6d9G4xc9neXJWAZQ= github.com/hashicorp/go-cleanhttp v0.5.2/go.mod h1:kO/YDlP8L1346E6Sodw+PrpBSV4/SoxCXGY6BqNFT48= github.com/hashicorp/go-hclog v1.6.3 h1:Qr2kF+eVWjTiYmU7Y31tYlP1h0q/X3Nl3tPGdaB11/k= diff --git a/scanpullrequest/scanpullrequest.go b/scanpullrequest/scanpullrequest.go index 8348245e4..38576945b 100644 --- a/scanpullrequest/scanpullrequest.go +++ b/scanpullrequest/scanpullrequest.go @@ -246,62 +246,6 @@ func prepareTargetForScan(pullRequestDetails vcsclient.PullRequestInfo, scanDeta return } -func prepareTargetForScan(pullRequestDetails vcsclient.PullRequestInfo, scanDetails *utils.ScanDetails) (targetBranchWd string, cleanupTarget func() error, err error) { - target := pullRequestDetails.Target - // Download target branch - if targetBranchWd, cleanupTarget, err = utils.DownloadRepoToTempDir(scanDetails.Client(), target.Owner, target.Repository, target.Name); err != nil { - return - } - if !scanDetails.Git.UseMostCommonAncestorAsTarget { - return - } - log.Debug("Using most common ancestor commit as target branch commit") - // Get common parent commit between source and target and use it (checkout) to the target branch commit - if e := tryCheckoutToMostCommonAncestor(scanDetails, pullRequestDetails.Source.Name, target.Name, targetBranchWd); e != nil { - log.Warn(fmt.Sprintf("Failed to get best common ancestor commit between source branch: %s and target branch: %s, defaulting to target branch commit. Error: %s", pullRequestDetails.Source.Name, target.Name, e.Error())) - } - return -} - -func tryCheckoutToMostCommonAncestor(scanDetails *utils.ScanDetails, baseBranch, headBranch, targetBranchWd string) (err error) { - repositoryInfo, err := scanDetails.Client().GetRepositoryInfo(context.Background(), scanDetails.RepoOwner, scanDetails.RepoName) - if err != nil { - return - } - scanDetails.Git.RepositoryCloneUrl = repositoryInfo.CloneInfo.HTTP - bestAncestorHash, err := getMostCommonAncestorCommitHash(scanDetails, baseBranch, headBranch) - if err != nil { - return - } - return checkoutToCommitAtTempWorkingDir(scanDetails, bestAncestorHash, targetBranchWd) -} - -func getMostCommonAncestorCommitHash(scanDetails *utils.ScanDetails, baseBranch, headBranch string) (hash string, err error) { - gitManager, err := utils.NewGitManager().SetAuth(scanDetails.Username, scanDetails.Token).SetRemoteGitUrl(scanDetails.Git.RepositoryCloneUrl) - if err != nil { - return - } - return gitManager.GetMostCommonAncestorHash(baseBranch, headBranch) -} - -func checkoutToCommitAtTempWorkingDir(scanDetails *utils.ScanDetails, commitHash, wd string) (err error) { - // Change working directory to the temp target branch directory - cwd, err := os.Getwd() - if err != nil { - return - } - if err = os.Chdir(wd); err != nil { - return - } - defer os.Chdir(cwd) - // Load .git info in directory and Checkout to the commit hash - gitManager, err := utils.NewGitManager().SetAuth(scanDetails.Username, scanDetails.Token).SetRemoteGitUrl(scanDetails.Git.RepositoryCloneUrl) - if err != nil { - return - } - return gitManager.CheckoutToHash(commitHash, wd) -} - func tryCheckoutToMostCommonAncestor(scanDetails *utils.ScanDetails, baseBranch, headBranch, targetBranchWd string) (err error) { repositoryInfo, err := scanDetails.Client().GetRepositoryInfo(context.Background(), scanDetails.RepoOwner, scanDetails.RepoName) if err != nil { diff --git a/utils/params.go b/utils/params.go index 5de937781..54f4255f1 100644 --- a/utils/params.go +++ b/utils/params.go @@ -310,7 +310,7 @@ type Git struct { AggregateFixes bool `yaml:"aggregateFixes,omitempty"` PullRequestDetails vcsclient.PullRequestInfo RepositoryCloneUrl string - UseLocalRepository bool + UseLocalRepository bool } func (g *Git) setDefaultsIfNeeded(gitParamsFromEnv *Git, commandName string) (err error) { From 460ed876e4c13857be344e8be0d2a02636f04d37 Mon Sep 17 00:00:00 2001 From: attiasas Date: Wed, 30 Oct 2024 15:08:26 +0200 Subject: [PATCH 5/5] CR changes --- scanpullrequest/scanpullrequest.go | 4 +++- utils/git.go | 13 ++++++------- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/scanpullrequest/scanpullrequest.go b/scanpullrequest/scanpullrequest.go index 38576945b..5cf790306 100644 --- a/scanpullrequest/scanpullrequest.go +++ b/scanpullrequest/scanpullrequest.go @@ -276,7 +276,9 @@ func checkoutToCommitAtTempWorkingDir(scanDetails *utils.ScanDetails, commitHash if err = os.Chdir(wd); err != nil { return } - defer os.Chdir(cwd) + defer func() { + err = errors.Join(err, os.Chdir(cwd)) + }() // Load .git info in directory and Checkout to the commit hash gitManager, err := utils.NewGitManager().SetAuth(scanDetails.Username, scanDetails.Token).SetRemoteGitUrl(scanDetails.Git.RepositoryCloneUrl) if err != nil { diff --git a/utils/git.go b/utils/git.go index 1bd63da79..0a17f59a7 100644 --- a/utils/git.go +++ b/utils/git.go @@ -162,7 +162,6 @@ func (gm *GitManager) Checkout(branchName string) error { } func (gm *GitManager) CheckoutToHash(hash, targetBranchWd string) error { - log.Debug("Running git fetch...") if err := gm.Fetch(); err != nil { return err } @@ -186,7 +185,7 @@ func (gm *GitManager) Fetch() error { return nil } -func (gm *GitManager) GetMostCommonAncestorHash(baseBranch, headBranch string) (string, error) { +func (gm *GitManager) GetMostCommonAncestorHash(baseBranch, targetBranch string) (string, error) { // Get the commit of the base branch baseCommitHash, err := gm.localGitRepository.ResolveRevision(plumbing.Revision(baseBranch)) if err != nil { @@ -196,8 +195,8 @@ func (gm *GitManager) GetMostCommonAncestorHash(baseBranch, headBranch string) ( if err != nil { return "", err } - // Get the commit of the head branch - headCommitHash, err := gm.localGitRepository.ResolveRevision(plumbing.Revision(headBranch)) + // Get the HEAD commit of the target branch + headCommitHash, err := gm.localGitRepository.ResolveRevision(plumbing.Revision(targetBranch)) if err != nil { return "", err } @@ -206,15 +205,15 @@ func (gm *GitManager) GetMostCommonAncestorHash(baseBranch, headBranch string) ( return "", err } // Get the most common ancestor - log.Debug(fmt.Sprintf("Finding common ancestor between %s and %s...", baseBranch, headBranch)) + log.Debug(fmt.Sprintf("Finding common ancestor between %s and %s...", baseBranch, targetBranch)) ancestorCommit, err := baseCommit.MergeBase(headCommit) if err != nil { return "", err } if len(ancestorCommit) == 0 { - return "", fmt.Errorf("no common ancestor found for %s and %s", baseBranch, headBranch) + return "", fmt.Errorf("no common ancestor found for %s and %s", baseBranch, targetBranch) } else if len(ancestorCommit) > 1 { - return "", fmt.Errorf("more than one common ancestor found for %s and %s", baseBranch, headBranch) + return "", fmt.Errorf("more than one common ancestor found for %s and %s", baseBranch, targetBranch) } return ancestorCommit[0].Hash.String(), nil }