Scan PR - Add option to scan source and most common ancestor target c… (

#763)

scanpullrequest/scanpullrequest.go

@@ -207,8 +207,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
 		}
 	}
@@ -230,6 +229,64 @@ 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
+	}
+	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 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 {
+		return
+	}
+	return gitManager.CheckoutToHash(commitHash, wd)
+}
+
 func getAllIssues(cmdResults *results.SecurityCommandResults, allowedLicenses []string, hasViolationContext bool) (*utils.IssuesCollection, error) {
 	log.Info("Frogbot is configured to show all vulnerabilities")
 	simpleJsonResults, err := conversion.NewCommandResultsConvertor(conversion.ResultConvertParams{

utils/consts.go

@@ -42,10 +42,11 @@ const (
 	GitUseLocalRepositoryEnv = "JF_USE_LOCAL_REPOSITORY"
 
 	// 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
 	InstallCommandEnv                  = "JF_INSTALL_DEPS_CMD"

utils/git.go

@@ -4,6 +4,7 @@ import (
 	"errors"
 	"fmt"
 	"net/http"
+	// "os/exec"
 	"regexp"
 	"strings"
 	"time"
@@ -160,6 +161,63 @@ 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
+}
+
+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())
+	}
+	return nil
+}
+
+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 {
+		return "", err
+	}
+	baseCommit, err := gm.localGitRepository.CommitObject(*baseCommitHash)
+	if err != nil {
+		return "", err
+	}
+	// Get the HEAD commit of the target branch
+	headCommitHash, err := gm.localGitRepository.ResolveRevision(plumbing.Revision(targetBranch))
+	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, 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, targetBranch)
+	} else if len(ancestorCommit) > 1 {
+		return "", fmt.Errorf("more than one common ancestor found for %s and %s", baseBranch, targetBranch)
+	}
+	return ancestorCommit[0].Hash.String(), nil
+}
+
 func (gm *GitManager) Clone(destinationPath, branchName string) error {
 	if gm.dryRun {
 		// "Clone" the repository from the testdata folder
@@ -207,6 +265,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 {

utils/params.go

@@ -297,19 +297,20 @@ func (jp *JFrogPlatform) setDefaultsIfNeeded() (err error) {
 type Git struct {
 	GitProvider vcsutils.VcsProvider
 	vcsclient.VcsInfo
-	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
-	UseLocalRepository       bool
+	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
+	UseLocalRepository            bool
 }
 
 func (g *Git) setDefaultsIfNeeded(gitParamsFromEnv *Git, commandName string) (err error) {
@@ -349,6 +350,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
 }