Skip to content

Commit

Permalink
Fix scan-and-fix-repos faulty checkout (#437)
Browse files Browse the repository at this point in the history
  • Loading branch information
EyalDelarea committed Aug 14, 2023
1 parent 33d33fd commit 1021885
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 28 deletions.
1 change: 1 addition & 0 deletions .github/ISSUE_TEMPLATE/bug_report.yml
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ body:
- GitHub
- Bitbucket Server
- GitLab
- Azure DevOps
validations:
required: true

Expand Down
5 changes: 4 additions & 1 deletion commands/scanrepository/scanmultiplerepositories.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,5 +55,8 @@ func (saf *ScanMultipleRepositories) downloadAndRunScanAndFix(repository *utils.
}()

cfp := ScanRepositoryCmd{dryRun: saf.dryRun, dryRunRepoPath: wd}
return cfp.scanAndFixRepository(repository, branch, client)
if err = cfp.setCommandPrerequisites(repository, branch, client); err != nil {
return
}
return cfp.scanAndFixRepository(repository)
}
43 changes: 21 additions & 22 deletions commands/scanrepository/scanrepository.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,23 +50,23 @@ func (cfp *ScanRepositoryCmd) Run(repoAggregator utils.RepoAggregator, client vc
}
repository := repoAggregator[0]
for _, branch := range repository.Branches {
if err = cfp.scanAndFixRepository(&repository, branch, client); err != nil {
if err = cfp.setCommandPrerequisites(&repository, branch, client); err != nil {
return
}
if err = cfp.gitManager.Checkout(branch); err != nil {
return
}
if err = cfp.scanAndFixRepository(&repository); err != nil {
return
}
}
return
}

func (cfp *ScanRepositoryCmd) scanAndFixRepository(repository *utils.Repository, branch string, client vcsclient.VcsClient) (err error) {
func (cfp *ScanRepositoryCmd) scanAndFixRepository(repository *utils.Repository) (err error) {
if cfp.baseWd, err = os.Getwd(); err != nil {
return
}
if err = cfp.setCommandPrerequisites(repository, branch, client); err != nil {
return
}
if err = cfp.gitManager.Checkout(branch); err != nil {
return fmt.Errorf("failed to checkout to %s branch before scanning. The following error has been received:\n%s", branch, err.Error())
}
for i := range repository.Projects {
cfp.details.Project = &repository.Projects[i]
cfp.projectTech = ""
Expand All @@ -81,7 +81,7 @@ func (cfp *ScanRepositoryCmd) setCommandPrerequisites(repository *utils.Reposito
cfp.details = utils.NewScanDetails(client, &repository.Server, &repository.Git).
SetXrayGraphScanParams(repository.Watches, repository.JFrogProjectKey).
SetFailOnInstallationErrors(*repository.FailOnSecurityIssues).
SetBranch(branch).
SetBaseBranch(branch).
SetFixableOnly(repository.FixableOnly).
SetMinSeverity(repository.MinSeverity)
cfp.aggregateFixes = repository.Git.AggregateFixes
Expand All @@ -104,7 +104,7 @@ func (cfp *ScanRepositoryCmd) scanAndFixProject(repository *utils.Repository) er
}

if !cfp.dryRun {
if err = utils.UploadScanToGitProvider(scanResults, repository, cfp.details.Branch(), cfp.details.Client()); err != nil {
if err = utils.UploadScanToGitProvider(scanResults, repository, cfp.details.BaseBranch(), cfp.details.Client()); err != nil {
log.Warn(err)
}
}
Expand Down Expand Up @@ -153,7 +153,7 @@ func (cfp *ScanRepositoryCmd) getVulnerabilitiesMap(scanResults *xrayutils.Exten
}

func (cfp *ScanRepositoryCmd) fixVulnerablePackages(vulnerabilitiesByWdMap map[string]map[string]*utils.VulnerabilityDetails) (err error) {
clonedRepoDir, restoreBaseDir, err := cfp.cloneRepository()
clonedRepoDir, restoreBaseDir, err := cfp.cloneRepositoryAndCheckoutToBranch()
if err != nil {
return
}
Expand Down Expand Up @@ -202,8 +202,7 @@ func (cfp *ScanRepositoryCmd) fixProjectVulnerabilities(fullProjectPath string,
}

// After fixing the current vulnerability, checkout to the base branch to start fixing the next vulnerability
log.Debug("Running git checkout to base branch:", cfp.details.Branch())
if e := cfp.gitManager.Checkout(cfp.details.Branch()); e != nil {
if e := cfp.gitManager.Checkout(cfp.details.BaseBranch()); e != nil {
err = errors.Join(err, cfp.handleUpdatePackageErrors(e))
return
}
Expand Down Expand Up @@ -270,7 +269,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", vulnDetails.ImpactedDependencyName, "with", fixVersion)
fixBranchName, err := cfp.gitManager.GenerateFixBranchName(cfp.details.Branch(), vulnDetails.ImpactedDependencyName, fixVersion)
fixBranchName, err := cfp.gitManager.GenerateFixBranchName(cfp.details.BaseBranch(), vulnDetails.ImpactedDependencyName, fixVersion)
if err != nil {
return
}
Expand Down Expand Up @@ -317,8 +316,8 @@ func (cfp *ScanRepositoryCmd) openFixingPullRequest(fixBranchName string, vulnDe
return err
}
pullRequestTitle, prBody := cfp.preparePullRequestDetails(scanHash, []formats.VulnerabilityOrViolationRow{vulnDetails.VulnerabilityOrViolationRow})
log.Debug("Creating Pull Request form:", fixBranchName, " to:", cfp.details.Branch())
return cfp.details.Client().CreatePullRequest(context.Background(), cfp.details.RepoOwner, cfp.details.RepoName, fixBranchName, cfp.details.Branch(), pullRequestTitle, prBody)
log.Debug("Creating Pull Request form:", fixBranchName, " to:", cfp.details.BaseBranch())
return cfp.details.Client().CreatePullRequest(context.Background(), cfp.details.RepoOwner, cfp.details.RepoName, fixBranchName, cfp.details.BaseBranch(), pullRequestTitle, prBody)
}

// openAggregatedPullRequest handles the opening or updating of a pull request when the aggregate mode is active.
Expand All @@ -338,10 +337,10 @@ func (cfp *ScanRepositoryCmd) openAggregatedPullRequest(fixBranchName string, pu
}
pullRequestTitle, prBody := cfp.preparePullRequestDetails(scanHash, vulnerabilityRows)
if pullRequestInfo == nil {
log.Info("Creating Pull Request from:", fixBranchName, "to:", cfp.details.Branch())
return cfp.details.Client().CreatePullRequest(context.Background(), cfp.details.RepoOwner, cfp.details.RepoName, fixBranchName, cfp.details.Branch(), pullRequestTitle, prBody)
log.Info("Creating Pull Request from:", fixBranchName, "to:", cfp.details.BaseBranch())
return cfp.details.Client().CreatePullRequest(context.Background(), cfp.details.RepoOwner, cfp.details.RepoName, fixBranchName, cfp.details.BaseBranch(), pullRequestTitle, prBody)
}
log.Info("Updating Pull Request from:", fixBranchName, "to:", cfp.details.Branch())
log.Info("Updating Pull Request from:", fixBranchName, "to:", cfp.details.BaseBranch())
return cfp.details.Client().UpdatePullRequest(context.Background(), cfp.details.RepoOwner, cfp.details.RepoName, pullRequestTitle, prBody, "", int(pullRequestInfo.ID), vcsutils.Open)
}

Expand All @@ -361,7 +360,7 @@ func (cfp *ScanRepositoryCmd) preparePullRequestDetails(scanHash string, vulnera
return pullRequestTitle, prBody
}

func (cfp *ScanRepositoryCmd) cloneRepository() (tempWd string, restoreDir func() error, err error) {
func (cfp *ScanRepositoryCmd) cloneRepositoryAndCheckoutToBranch() (tempWd string, restoreDir func() error, err error) {
if cfp.dryRunRepoPath != "" {
tempWd, err = cfp.getDryRunClonedRepo()
} else {
Expand All @@ -374,7 +373,7 @@ func (cfp *ScanRepositoryCmd) cloneRepository() (tempWd string, restoreDir func(
log.Debug("Created temp working directory:", tempWd)

// Clone the content of the repo to the new working directory
if err = cfp.gitManager.Clone(tempWd, cfp.details.Branch()); err != nil {
if err = cfp.gitManager.Clone(tempWd, cfp.details.BaseBranch()); err != nil {
return
}

Expand Down Expand Up @@ -545,7 +544,7 @@ func (cfp *ScanRepositoryCmd) aggregateFixAndOpenPullRequest(vulnerabilitiesMap
}
}
log.Info("-----------------------------------------------------------------")
if e := cfp.gitManager.Checkout(cfp.details.Branch()); e != nil {
if e := cfp.gitManager.Checkout(cfp.details.BaseBranch()); e != nil {
err = errors.Join(err, e)
}
return
Expand Down
10 changes: 5 additions & 5 deletions commands/utils/scandetails.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ type ScanDetails struct {
failOnInstallationErrors bool
fixableOnly bool
minSeverityFilter string
branch string
baseBranch string
}

func NewScanDetails(client vcsclient.VcsClient, server *config.ServerDetails, git *Git) *ScanDetails {
Expand Down Expand Up @@ -59,17 +59,17 @@ func (sc *ScanDetails) SetMinSeverity(minSeverity string) *ScanDetails {
return sc
}

func (sc *ScanDetails) SetBranch(branch string) *ScanDetails {
sc.branch = branch
func (sc *ScanDetails) SetBaseBranch(branch string) *ScanDetails {
sc.baseBranch = branch
return sc
}

func (sc *ScanDetails) Client() vcsclient.VcsClient {
return sc.client
}

func (sc *ScanDetails) Branch() string {
return sc.branch
func (sc *ScanDetails) BaseBranch() string {
return sc.baseBranch
}

func (sc *ScanDetails) FailOnInstallationErrors() bool {
Expand Down

0 comments on commit 1021885

Please sign in to comment.