Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Encapsulate scan details for each command #483

Closed
wants to merge 11 commits into from
2 changes: 1 addition & 1 deletion action/node_modules/.package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions packagehandlers/commonpackagehandler.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ type PackageHandler interface {
UpdateDependency(details *utils.VulnerabilityDetails) error
}

func GetCompatiblePackageHandler(vulnDetails *utils.VulnerabilityDetails, details *utils.ScanDetails) (handler PackageHandler) {
func GetCompatiblePackageHandler(vulnDetails *utils.VulnerabilityDetails, details *utils.RepositoryScanDetails) (handler PackageHandler) {
switch vulnDetails.Technology {
case coreutils.Go:
handler = &GoPackageHandler{}
Expand All @@ -27,9 +27,9 @@ func GetCompatiblePackageHandler(vulnDetails *utils.VulnerabilityDetails, detail
case coreutils.Yarn:
handler = &YarnPackageHandler{}
case coreutils.Pip:
handler = &PythonPackageHandler{pipRequirementsFile: details.PipRequirementsFile}
handler = &PythonPackageHandler{pipRequirementsFile: details.Project().PipRequirementsFile}
case coreutils.Maven:
handler = &MavenPackageHandler{depsRepo: details.DepsRepo, ServerDetails: details.ServerDetails}
handler = &MavenPackageHandler{depsRepo: details.Project().DepsRepo, ServerDetails: details.ServerDetails()}
case coreutils.Nuget:
handler = &NugetPackageHandler{}
default:
Expand Down
26 changes: 15 additions & 11 deletions packagehandlers/packagehandlers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import (

type dependencyFixTest struct {
vulnDetails *utils.VulnerabilityDetails
scanDetails *utils.ScanDetails
project *utils.Project
fixSupported bool
specificTechVersion string
uniqueChecksExtraArgs []string
Expand Down Expand Up @@ -74,23 +74,23 @@ func TestUpdateDependency(t *testing.T) {
SuggestedFixedVersion: "1.25.9",
VulnerabilityOrViolationRow: formats.VulnerabilityOrViolationRow{Technology: coreutils.Pip, ImpactedDependencyName: "urllib3"},
},
scanDetails: &utils.ScanDetails{Project: &utils.Project{PipRequirementsFile: "requirements.txt"}},
project: &utils.Project{PipRequirementsFile: "requirements.txt"},
fixSupported: false,
},
{
vulnDetails: &utils.VulnerabilityDetails{
SuggestedFixedVersion: "1.25.9",
VulnerabilityOrViolationRow: formats.VulnerabilityOrViolationRow{Technology: coreutils.Poetry, ImpactedDependencyName: "urllib3"},
},
scanDetails: &utils.ScanDetails{Project: &utils.Project{PipRequirementsFile: "pyproejct.toml"}},
project: &utils.Project{PipRequirementsFile: "pyproejct.toml"},
fixSupported: false,
},
{
vulnDetails: &utils.VulnerabilityDetails{
SuggestedFixedVersion: "1.25.9",
VulnerabilityOrViolationRow: formats.VulnerabilityOrViolationRow{Technology: coreutils.Pipenv, ImpactedDependencyName: "urllib3"},
},
scanDetails: &utils.ScanDetails{Project: &utils.Project{PipRequirementsFile: "Pipfile"}},
project: &utils.Project{PipRequirementsFile: "Pipfile"},
fixSupported: false,
},
{
Expand All @@ -99,31 +99,31 @@ func TestUpdateDependency(t *testing.T) {
VulnerabilityOrViolationRow: formats.VulnerabilityOrViolationRow{Technology: coreutils.Pip, ImpactedDependencyName: "pyjwt"},
IsDirectDependency: true,
},
scanDetails: &utils.ScanDetails{Project: &utils.Project{PipRequirementsFile: "requirements.txt"}},
project: &utils.Project{PipRequirementsFile: "requirements.txt"},
fixSupported: true,
},
{
vulnDetails: &utils.VulnerabilityDetails{
SuggestedFixedVersion: "2.4.0",
VulnerabilityOrViolationRow: formats.VulnerabilityOrViolationRow{Technology: coreutils.Pip, ImpactedDependencyName: "Pyjwt"},
IsDirectDependency: true},
scanDetails: &utils.ScanDetails{Project: &utils.Project{PipRequirementsFile: "requirements.txt"}},
project: &utils.Project{PipRequirementsFile: "requirements.txt"},
fixSupported: true,
},
{
vulnDetails: &utils.VulnerabilityDetails{
SuggestedFixedVersion: "2.4.0",
VulnerabilityOrViolationRow: formats.VulnerabilityOrViolationRow{Technology: coreutils.Pip, ImpactedDependencyName: "pyjwt"},
IsDirectDependency: true},
scanDetails: &utils.ScanDetails{Project: &utils.Project{PipRequirementsFile: "setup.py"}},
project: &utils.Project{PipRequirementsFile: "setup.py"},
fixSupported: true,
},
{
vulnDetails: &utils.VulnerabilityDetails{
SuggestedFixedVersion: "2.4.0",
VulnerabilityOrViolationRow: formats.VulnerabilityOrViolationRow{Technology: coreutils.Poetry, ImpactedDependencyName: "pyjwt"},
IsDirectDependency: true},
scanDetails: &utils.ScanDetails{Project: &utils.Project{PipRequirementsFile: "pyproject.toml"}},
project: &utils.Project{PipRequirementsFile: "pyproject.toml"},
fixSupported: true,
},
},
Expand Down Expand Up @@ -185,15 +185,15 @@ func TestUpdateDependency(t *testing.T) {
SuggestedFixedVersion: "2.7",
VulnerabilityOrViolationRow: formats.VulnerabilityOrViolationRow{Technology: coreutils.Maven, ImpactedDependencyName: "commons-io:commons-io"},
IsDirectDependency: true},
scanDetails: &utils.ScanDetails{Project: &utils.Project{DepsRepo: ""}, ServerDetails: nil},
project: &utils.Project{DepsRepo: ""},
fixSupported: true,
},
{
vulnDetails: &utils.VulnerabilityDetails{
SuggestedFixedVersion: "4.3.20",
VulnerabilityOrViolationRow: formats.VulnerabilityOrViolationRow{Technology: coreutils.Maven, ImpactedDependencyName: "org.springframework:spring-core"},
IsDirectDependency: false},
scanDetails: &utils.ScanDetails{Project: &utils.Project{DepsRepo: ""}, ServerDetails: nil},
project: &utils.Project{DepsRepo: ""},
fixSupported: false,
},
},
Expand Down Expand Up @@ -222,7 +222,11 @@ func TestUpdateDependency(t *testing.T) {

for _, testBatch := range testCases {
for _, test := range testBatch {
packageHandler := GetCompatiblePackageHandler(test.vulnDetails, test.scanDetails)
repositoryScanDetails := utils.NewRepositoryScanDetails(nil, nil)
if test.project != nil {
repositoryScanDetails.SetProject(test.project)
}
packageHandler := GetCompatiblePackageHandler(test.vulnDetails, repositoryScanDetails)
t.Run(fmt.Sprintf("%s:%s direct:%s", test.vulnDetails.Technology.ToString()+test.specificTechVersion, test.vulnDetails.ImpactedDependencyName, strconv.FormatBool(test.vulnDetails.IsDirectDependency)),
func(t *testing.T) {
testDataDir := getTestDataDir(t, test.vulnDetails.IsDirectDependency)
Expand Down
11 changes: 3 additions & 8 deletions scanpullrequest/scanpullrequest.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,18 +164,13 @@ func auditPullRequest(repoConfig *utils.Repository, client vcsclient.VcsClient,
err = errors.Join(err, cleanupSource(), cleanupTarget())
}()

scanDetails := utils.NewScanDetails(client, &repoConfig.Server, &repoConfig.Git).
SetXrayGraphScanParams(repoConfig.Watches, repoConfig.JFrogProjectKey).
SetMinSeverity(repoConfig.MinSeverity).
SetFixableOnly(repoConfig.FixableOnly).
SetFailOnInstallationErrors(*repoConfig.FailOnSecurityIssues)

scanDetails := utils.NewPullRequestScanDetails(client, repoConfig)
for i := range repoConfig.Projects {
scanDetails.SetProject(&repoConfig.Projects[i])

// Audit source branch
var sourceResults *audit.Results
workingDirs := utils.GetFullPathWorkingDirs(scanDetails.Project.WorkingDirs, sourceBranchWd)
workingDirs := utils.GetFullPathWorkingDirs(scanDetails.Project().WorkingDirs, sourceBranchWd)
sourceResults, err = scanDetails.RunInstallAndAudit(workingDirs...)
if err != nil {
return
Expand All @@ -202,7 +197,7 @@ func auditPullRequest(repoConfig *utils.Repository, client vcsclient.VcsClient,

// Set target branch scan details
var targetResults *audit.Results
workingDirs = utils.GetFullPathWorkingDirs(scanDetails.Project.WorkingDirs, targetBranchWd)
workingDirs = utils.GetFullPathWorkingDirs(scanDetails.Project().WorkingDirs, targetBranchWd)
targetResults, err = scanDetails.RunInstallAndAudit(workingDirs...)

if err != nil {
Expand Down
54 changes: 25 additions & 29 deletions scanrepository/scanrepository.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,11 @@ type ScanRepositoryCmd struct {
// When dryRun is enabled, dryRunRepoPath specifies the repository local path to clone
dryRunRepoPath string
// The scanDetails of the current scan
scanDetails *utils.ScanDetails
scanDetails *utils.RepositoryScanDetails
// The base working directory
baseWd string
// The git client the command performs git operations with
gitManager *utils.GitManager
// Determines whether to open a pull request for each vulnerability fix or to aggregate all fixes into one pull request
aggregateFixes bool
// The current project technology
projectTech coreutils.Technology
// Stores all package manager handlers for detected issues
Expand All @@ -58,7 +56,6 @@ func (cfp *ScanRepositoryCmd) scanAndFixRepository(repository *utils.Repository,
if err = cfp.setCommandPrerequisites(repository, branch, client); err != nil {
return
}
cfp.scanDetails.SetXscGitInfoContext(branch, repository.Project, client)
if err = cfp.scanAndFixBranch(repository); err != nil {
return
}
Expand All @@ -80,7 +77,7 @@ func (cfp *ScanRepositoryCmd) scanAndFixBranch(repository *utils.Repository) (er
err = errors.Join(err, restoreBaseDir(), fileutils.RemoveTempDir(clonedRepoDir))
}()
for i := range repository.Projects {
cfp.scanDetails.Project = &repository.Projects[i]
cfp.scanDetails.SetProject(&repository.Projects[i])
cfp.projectTech = ""
if err = cfp.scanAndFixProject(repository); err != nil {
return
Expand All @@ -90,29 +87,28 @@ func (cfp *ScanRepositoryCmd) scanAndFixBranch(repository *utils.Repository) (er
}

func (cfp *ScanRepositoryCmd) setCommandPrerequisites(repository *utils.Repository, branch string, client vcsclient.VcsClient) (err error) {

cfp.scanDetails = utils.NewScanDetails(client, &repository.Server, &repository.Git).
SetXrayGraphScanParams(repository.Watches, repository.JFrogProjectKey).
SetFailOnInstallationErrors(*repository.FailOnSecurityIssues).
SetBaseBranch(branch).
SetFixableOnly(repository.FixableOnly).
SetMinSeverity(repository.MinSeverity)

cfp.aggregateFixes = repository.Git.AggregateFixes
cfp.OutputWriter = outputwriter.GetCompatibleOutputWriter(repository.GitProvider)
repositoryInfo, err := client.GetRepositoryInfo(context.Background(), cfp.scanDetails.RepoOwner, cfp.scanDetails.RepoName)
repositoryInfo, err := client.GetRepositoryInfo(context.Background(), repository.RepoOwner, repository.RepoName)
if err != nil {
return
}
cfp.scanDetails.Git.RepositoryCloneUrl = repositoryInfo.CloneInfo.HTTP

cfp.scanDetails = utils.NewRepositoryScanDetails(client, repository).
SetBaseBranch(branch).
SetAggregateFixes(repository.Git.AggregateFixes).
SetRepositoryCloneUrl(repositoryInfo.CloneInfo.HTTP).
SetXscGitInfoContext(branch, repository.Project, client)

cfp.gitManager, err = utils.NewGitManager().
SetAuth(cfp.scanDetails.Username, cfp.scanDetails.Token).
SetAuth(cfp.scanDetails.VcsInfo().Username, cfp.scanDetails.VcsInfo().Username).
SetDryRun(cfp.dryRun, cfp.dryRunRepoPath).
SetRemoteGitUrl(cfp.scanDetails.Git.RepositoryCloneUrl)
SetEmailAuthor(repository.EmailAuthor).
SetRemoteGitUrl(repositoryInfo.CloneInfo.HTTP)
if err != nil {
return
}
_, err = cfp.gitManager.SetGitParams(cfp.scanDetails.Git)
customTemplates := utils.NewCustomTemplates(repository.CommitMessageTemplate, repository.BranchNameTemplate, repository.PullRequestTitleTemplate)
cfp.gitManager, err = cfp.gitManager.SetTemplates(customTemplates)
return
}

Expand All @@ -122,7 +118,7 @@ func (cfp *ScanRepositoryCmd) scanAndFixProject(repository *utils.Repository) er
// The value is a map of vulnerable package names -> the scanDetails of the vulnerable packages.
// That means we have a map of all the vulnerabilities that were found in a specific folder, along with their full scanDetails.
vulnerabilitiesByPathMap := make(map[string]map[string]*utils.VulnerabilityDetails)
projectFullPathWorkingDirs := utils.GetFullPathWorkingDirs(cfp.scanDetails.Project.WorkingDirs, cfp.baseWd)
projectFullPathWorkingDirs := utils.GetFullPathWorkingDirs(cfp.scanDetails.Project().WorkingDirs, cfp.baseWd)
for _, fullPathWd := range projectFullPathWorkingDirs {
scanResults, err := cfp.scan(fullPathWd)
if err != nil {
Expand All @@ -132,7 +128,7 @@ func (cfp *ScanRepositoryCmd) scanAndFixProject(repository *utils.Repository) er
if repository.GitProvider.String() == vcsutils.GitHub.String() {
// Uploads Sarif results to GitHub in order to view the scan in the code scanning UI
// Currently available on GitHub only
if err = utils.UploadSarifResultsToGithubSecurityTab(scanResults, repository, cfp.scanDetails.BaseBranch(), cfp.scanDetails.Client()); err != nil {
if err = utils.UploadSarifResultsToGithubSecurityTab(scanResults, repository, cfp.scanDetails.BaseBranch(), cfp.scanDetails.GitClient()); err != nil {
log.Warn(err)
}
}
Expand Down Expand Up @@ -181,7 +177,7 @@ func (cfp *ScanRepositoryCmd) getVulnerabilitiesMap(scanResults *xrayutils.Exten
}

func (cfp *ScanRepositoryCmd) fixVulnerablePackages(vulnerabilitiesByWdMap map[string]map[string]*utils.VulnerabilityDetails) (err error) {
if cfp.aggregateFixes {
if cfp.scanDetails.AggregateFixes() {
return cfp.fixIssuesSinglePR(vulnerabilitiesByWdMap)
}
return cfp.fixIssuesSeparatePRs(vulnerabilitiesByWdMap)
Expand Down Expand Up @@ -334,7 +330,7 @@ func (cfp *ScanRepositoryCmd) openFixingPullRequest(fixBranchName string, vulnDe
return
}
log.Debug("Creating Pull Request form:", fixBranchName, " to:", cfp.scanDetails.BaseBranch())
return cfp.scanDetails.Client().CreatePullRequest(context.Background(), cfp.scanDetails.RepoOwner, cfp.scanDetails.RepoName, fixBranchName, cfp.scanDetails.BaseBranch(), pullRequestTitle, prBody)
return cfp.scanDetails.GitClient().CreatePullRequest(context.Background(), cfp.scanDetails.RepoOwner(), cfp.scanDetails.RepoName(), fixBranchName, cfp.scanDetails.BaseBranch(), pullRequestTitle, prBody)
}

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

func (cfp *ScanRepositoryCmd) preparePullRequestDetails(vulnerabilitiesDetails ...*utils.VulnerabilityDetails) (string, string, error) {
if cfp.dryRun && cfp.aggregateFixes {
if cfp.dryRun && cfp.scanDetails.AggregateFixes() {
// For testings, don't compare pull request body as scan results order may change.
return outputwriter.GetAggregatedPullRequestTitle(cfp.projectTech), "", nil
}
vulnerabilitiesRows := utils.ExtractVulnerabilitiesDetailsToRows(vulnerabilitiesDetails)
prBody := cfp.OutputWriter.VulnerabilitiesTitle(false) + "\n" + cfp.OutputWriter.VulnerabilitiesContent(vulnerabilitiesRows) + "\n---\n" + cfp.OutputWriter.UntitledForJasMsg() + cfp.OutputWriter.Footer()
if cfp.aggregateFixes {
if cfp.scanDetails.AggregateFixes() {
scanHash, err := utils.VulnerabilityDetailsToMD5Hash(vulnerabilitiesRows...)
if err != nil {
return "", "", err
Expand All @@ -381,7 +377,7 @@ func (cfp *ScanRepositoryCmd) preparePullRequestDetails(vulnerabilitiesDetails .

func (cfp *ScanRepositoryCmd) cloneRepositoryAndCheckoutToBranch() (tempWd string, restoreDir func() error, err error) {
if cfp.dryRun {
tempWd = filepath.Join(cfp.dryRunRepoPath, cfp.scanDetails.RepoName)
tempWd = filepath.Join(cfp.dryRunRepoPath, cfp.scanDetails.RepoName())
} else {
// Create temp working directory
if tempWd, err = fileutils.CreateTempDir(); err != nil {
Expand Down Expand Up @@ -500,7 +496,7 @@ func (cfp *ScanRepositoryCmd) getRemoteBranchScanHash(prBody string) string {
}

func (cfp *ScanRepositoryCmd) getOpenPullRequestBySourceBranch(branchName string) (prInfo *vcsclient.PullRequestInfo, err error) {
list, err := cfp.scanDetails.Client().ListOpenPullRequestsWithBody(context.Background(), cfp.scanDetails.RepoOwner, cfp.scanDetails.RepoName)
list, err := cfp.scanDetails.GitClient().ListOpenPullRequestsWithBody(context.Background(), cfp.scanDetails.RepoOwner(), cfp.scanDetails.RepoName())
if err != nil {
return
}
Expand Down
Loading
Loading