diff --git a/go.mod b/go.mod index 2f489caa7..cfb3ff36d 100644 --- a/go.mod +++ b/go.mod @@ -119,7 +119,7 @@ require ( gopkg.in/warnings.v0 v0.1.2 // indirect ) -// replace github.com/jfrog/jfrog-cli-security => github.com/jfrog/jfrog-cli-security dev +// replace github.com/jfrog/jfrog-cli-security => github.com/attiasas/jfrog-cli-security v0.0.0-20240801093546-f19002fc5efa // replace github.com/jfrog/jfrog-cli-core/v2 => github.com/jfrog/jfrog-cli-core/v2 dev diff --git a/scanpullrequest/scanpullrequest.go b/scanpullrequest/scanpullrequest.go index ecc1634cb..d6af90001 100644 --- a/scanpullrequest/scanpullrequest.go +++ b/scanpullrequest/scanpullrequest.go @@ -39,6 +39,7 @@ func (cmd *ScanPullRequestCmd) Run(configAggregator utils.RepoAggregator, client } } repoConfig.OutputWriter.SetHasInternetConnection(frogbotRepoConnection.IsConnected()) + if repoConfig.PullRequestDetails, err = client.GetPullRequestByID(context.Background(), repoConfig.RepoOwner, repoConfig.RepoName, int(repoConfig.PullRequestDetails.ID)); err != nil { return } @@ -83,12 +84,17 @@ func verifyGitHubFrogbotEnvironment(client vcsclient.VcsClient, repoConfig *util // Otherwise, only the source branch is scanned and all found vulnerabilities are being displayed. func scanPullRequest(repo *utils.Repository, client vcsclient.VcsClient) (err error) { pullRequestDetails := repo.PullRequestDetails + log.Info(fmt.Sprintf("Pull request details URL: %s", pullRequestDetails.URL)) log.Info(fmt.Sprintf("Scanning Pull Request #%d (from source branch: <%s/%s/%s> to target branch: <%s/%s/%s>)", pullRequestDetails.ID, pullRequestDetails.Source.Owner, pullRequestDetails.Source.Repository, pullRequestDetails.Source.Name, pullRequestDetails.Target.Owner, pullRequestDetails.Target.Repository, pullRequestDetails.Target.Name)) log.Info("-----------------------------------------------------------") - + repositoryInfo, err := client.GetRepositoryInfo(context.Background(), repo.RepoOwner, repo.RepoName) + if err != nil { + return + } + repo.Git.RepositoryCloneUrl = repositoryInfo.CloneInfo.HTTP analyticsService := utils.AddAnalyticsGeneralEvent(nil, &repo.Server, analyticsScanPrScanType) defer func() { analyticsService.UpdateAndSendXscAnalyticsGeneralEventFinalize(err) @@ -200,12 +206,39 @@ func auditPullRequestInProject(repoConfig *utils.Repository, scanDetails *utils. 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 + } + // Get common parent commit between source and target and use it (checkout) to the target branch commit + if e := tryCheckoutToBestCommonAncestor(scanDetails, pullRequestDetails.Source.Name, target.Name); 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 tryCheckoutToBestCommonAncestor(scanDetails *utils.ScanDetails, baseBranch, headBranch string) (err error) { + log.Info(fmt.Sprintf("RepositoryCloneUrl: %s", scanDetails.Git.RepositoryCloneUrl)) + gitManager, err := utils.NewGitManager().SetAuth(scanDetails.Username, scanDetails.Token).SetRemoteGitUrl(scanDetails.Git.RepositoryCloneUrl) + if err != nil { + return + } + + bestAncestorHash, err := gitManager.GetBestCommonAncestorHash(baseBranch, headBranch) + if err != nil { + return + } + log.Debug("Checking out to best common ancestor commit hash: " + bestAncestorHash) + return gitManager.CheckoutToHash(bestAncestorHash) +} + func auditTargetBranch(repoConfig *utils.Repository, scanDetails *utils.ScanDetails, sourceScanResults *securityutils.Results) (newIssues *utils.IssuesCollection, targetBranchWd string, err error) { // 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 } } diff --git a/utils/git.go b/utils/git.go index 986a3fa87..e52243895 100644 --- a/utils/git.go +++ b/utils/git.go @@ -201,6 +201,68 @@ func (gm *GitManager) CreateBranchAndCheckout(branchName string, keepLocalChange return err } +func (gm *GitManager) CheckoutToHash(hash string) error { + 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) GetBestCommonAncestorHash(baseBranch, headBranch string) (string, error) { + // Fetch the latest changes from the remote + // if err := gm. + // return "", err + // } + // Get the commit object of the base branch + + baseRef, err := gm.localGitRepository.Reference(plumbing.NewBranchReferenceName(baseBranch), true) + if err != nil { + return "", err + } + baseCommit, err := gm.localGitRepository.CommitObject(baseRef.Hash()) + if err != nil { + return "", err + } + log.Debug(fmt.Sprintf("Found commit %s for base branch %s", baseCommit.Hash.String(), baseBranch)) + // Fetch the head branch and its commits + // TODO: fetch only if not in ref list (go over first -> extract to func) + + if err = gm.localGitRepository.Fetch(&git.FetchOptions{ + RemoteName: gm.remoteName, + Auth: gm.auth, + RefSpecs: []config.RefSpec{config.RefSpec(fmt.Sprintf(refFormat, headBranch))}, + }); err != nil { + return "", err + } + headRef, err := gm.localGitRepository.Reference(plumbing.NewBranchReferenceName(headBranch), true) + if err != nil { + return "", err + } + headCommit, err := gm.localGitRepository.CommitObject(headRef.Hash()) + if err != nil { + return "", err + } + log.Debug(fmt.Sprintf("Found commit %s for head branch %s", headCommit.Hash.String(), headBranch)) + // Preform a merge base between the two branches to find the common parent commit + // TODO: stop here, run merge-base in terminal on folder and see result hash + mergeBase, err := baseCommit.MergeBase(headCommit) + if err != nil { + return "", err + } + if len(mergeBase) == 0 { + return "", fmt.Errorf("failed to find a common parent commit between %s and %s", baseBranch, headBranch) + } else if len(mergeBase) > 1 { + log.Debug(fmt.Sprintf("Found %d common parent commits between %s and %s, using the first one", len(mergeBase), baseBranch, headBranch)) + } + // Return the first common parent commit + return mergeBase[0].Hash.String(), nil +} + func (gm *GitManager) createBranchAndCheckout(branchName string, create bool, keepLocalChanges bool) error { var checkoutConfig *git.CheckoutOptions if keepLocalChanges {