diff --git a/commands.go b/commands.go index ba0431297..1db68028d 100644 --- a/commands.go +++ b/commands.go @@ -5,14 +5,14 @@ import ( "fmt" "os" + "github.com/jfrog/froggit-go/vcsclient" + "github.com/jfrog/jfrog-client-go/utils/io/fileutils" + "github.com/jfrog/jfrog-client-go/utils/log" + "github.com/jfrog/frogbot/v2/scanpullrequest" "github.com/jfrog/frogbot/v2/scanrepository" "github.com/jfrog/frogbot/v2/utils" "github.com/jfrog/frogbot/v2/utils/outputwriter" - "github.com/jfrog/froggit-go/vcsclient" - "github.com/jfrog/jfrog-cli-core/v2/utils/coreutils" - "github.com/jfrog/jfrog-client-go/utils/io/fileutils" - "github.com/jfrog/jfrog-client-go/utils/log" "github.com/jfrog/jfrog-cli-security/utils/xsc" clitool "github.com/urfave/cli/v2" @@ -65,17 +65,6 @@ func Exec(command FrogbotCommand, commandName string) (err error) { err = errors.Join(err, os.Setenv(utils.JfrogHomeDirEnv, originalJfrogHomeDir), fileutils.RemoveTempDir(tempJFrogHomeDir)) }() - // Set releases remote repository env if needed - previousReleasesRepoEnv := os.Getenv(coreutils.ReleasesRemoteEnv) - if frogbotDetails.ReleasesRepo != "" { - if err = os.Setenv(coreutils.ReleasesRemoteEnv, fmt.Sprintf("frogbot/%s", frogbotDetails.ReleasesRepo)); err != nil { - return - } - defer func() { - err = errors.Join(err, os.Setenv(coreutils.ReleasesRemoteEnv, previousReleasesRepoEnv)) - }() - } - // Invoke the command interface log.Info(fmt.Sprintf("Running Frogbot %q command", commandName)) err = command.Run(frogbotDetails.Repository, frogbotDetails.GitClient, frogbotRepoConnection) diff --git a/go.mod b/go.mod index 4e6bc6736..daf6b5664 100644 --- a/go.mod +++ b/go.mod @@ -3,6 +3,7 @@ module github.com/jfrog/frogbot/v2 go 1.25.5 require ( + github.com/CycloneDX/cyclonedx-go v0.9.3 github.com/go-git/go-git/v5 v5.16.3 github.com/golang/mock v1.6.0 github.com/google/go-github/v45 v45.2.0 @@ -12,7 +13,7 @@ require ( github.com/jfrog/jfrog-cli-artifactory v0.8.1-0.20251210074251-c15fabe27f7f github.com/jfrog/jfrog-cli-core/v2 v2.60.1-0.20251125083543-e689762c4ff0 github.com/jfrog/jfrog-cli-security v1.24.0 - github.com/jfrog/jfrog-client-go v1.55.1-0.20251209090954-d6b1c70d3a5e + github.com/jfrog/jfrog-client-go v1.55.1-0.20251212095223-711b02c53736 github.com/owenrumney/go-sarif/v3 v3.2.3 github.com/stretchr/testify v1.11.1 github.com/urfave/cli/v2 v2.27.7 @@ -22,7 +23,6 @@ require ( require ( dario.cat/mergo v1.0.2 // indirect github.com/BurntSushi/toml v1.5.0 // indirect - github.com/CycloneDX/cyclonedx-go v0.9.3 // indirect github.com/Microsoft/go-winio v0.6.2 // indirect github.com/ProtonMail/go-crypto v1.3.0 // indirect github.com/andybalholm/brotli v1.2.0 // indirect @@ -134,7 +134,7 @@ replace github.com/jfrog/jfrog-cli-security => github.com/kerenr-jfrog/jfrog-cli // replace github.com/jfrog/build-info-go => github.com/jfrog/build-info-go dev -// attiasas:fix_cdx_remediation_api -replace github.com/jfrog/jfrog-client-go => github.com/attiasas/jfrog-client-go v0.0.0-20251210093930-2b29e73a9eb0 +// eranturgeman:update-config-profile-structure +replace github.com/jfrog/jfrog-client-go => github.com/eranturgeman/jfrog-client-go v0.0.0-20251215075212-69a133d2b04e // replace github.com/jfrog/froggit-go => github.com/jfrog/froggit-go master diff --git a/go.sum b/go.sum index 40b622f63..6a3e50750 100644 --- a/go.sum +++ b/go.sum @@ -21,8 +21,6 @@ github.com/anmitsu/go-shlex v0.0.0-20200514113438-38f4b401e2be h1:9AeTilPcZAjCFI github.com/anmitsu/go-shlex v0.0.0-20200514113438-38f4b401e2be/go.mod h1:ySMOLuWl6zY27l47sB3qLNK6tF2fkHG55UZxx8oIVo4= github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5 h1:0CwZNZbxp69SHPdPJAN/hZIm0C4OItdklCFmMRWYpio= github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5/go.mod h1:wHh0iHkYZB8zMSxRWpUBQtwG5a7fFgvEO+odwuTv2gs= -github.com/attiasas/jfrog-client-go v0.0.0-20251210093930-2b29e73a9eb0 h1:Xc39FpfwsS2F0eVKmQ2KyMpb/7Yluk56jgLVCd90mT4= -github.com/attiasas/jfrog-client-go v0.0.0-20251210093930-2b29e73a9eb0/go.mod h1:WQ5Y+oKYyHFAlCbHN925bWhnShTd2ruxZ6YTpb76fpU= github.com/bradleyjkemp/cupaloy/v2 v2.8.0 h1:any4BmKE+jGIaMpnU8YgH/I2LPiLBufr6oMMlVBbn9M= github.com/bradleyjkemp/cupaloy/v2 v2.8.0/go.mod h1:bm7JXdkRd4BHJk9HpwqAI8BoAY1lps46Enkdqw6aRX0= github.com/bufbuild/protocompile v0.4.0 h1:LbFKd2XowZvQ/kajzguUp2DC9UEIQhIq77fZZlaQsNA= @@ -62,6 +60,8 @@ github.com/elazarl/goproxy v1.7.2 h1:Y2o6urb7Eule09PjlhQRGNsqRfPmYI3KKQLFpCAV3+o github.com/elazarl/goproxy v1.7.2/go.mod h1:82vkLNir0ALaW14Rc399OTTjyNREgmdL2cVoIbS6XaE= github.com/emirpasic/gods v1.18.1 h1:FXtiHYKDGKCW2KzwZKx0iC0PQmdlorYgdFG9jPXJ1Bc= github.com/emirpasic/gods v1.18.1/go.mod h1:8tpGGwCnJ5H4r6BWwaV6OrWmMoPhUl5jm/FMNAnJvWQ= +github.com/eranturgeman/jfrog-client-go v0.0.0-20251215075212-69a133d2b04e h1:Wivac8wJVoq7wg83MeaOV24RxCisS/l98PQSBroQD68= +github.com/eranturgeman/jfrog-client-go v0.0.0-20251215075212-69a133d2b04e/go.mod h1:WQ5Y+oKYyHFAlCbHN925bWhnShTd2ruxZ6YTpb76fpU= github.com/fatih/color v1.13.0/go.mod h1:kLAiJbzzSOZDVNGyDpeOxJ47H46qBXwg5ILebYFFOfk= github.com/fatih/color v1.16.0 h1:zmkK9Ngbjj+K0yRhTVONQh1p/HknKYSlNT+vZCzyokM= github.com/fatih/color v1.16.0/go.mod h1:fL2Sau1YI5c0pdGEVCbKQbLXB6edEj1ZgiY4NijnWvE= diff --git a/integrationutils.go b/integrationutils.go index a6dd4f5f4..a3e02a3ae 100644 --- a/integrationutils.go +++ b/integrationutils.go @@ -11,14 +11,15 @@ import ( "github.com/go-git/go-git/v5" githttp "github.com/go-git/go-git/v5/plumbing/transport/http" - "github.com/jfrog/frogbot/v2/scanpullrequest" - "github.com/jfrog/frogbot/v2/scanrepository" - "github.com/jfrog/frogbot/v2/utils" - "github.com/jfrog/frogbot/v2/utils/outputwriter" "github.com/jfrog/froggit-go/vcsclient" "github.com/jfrog/froggit-go/vcsutils" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/jfrog/frogbot/v2/scanpullrequest" + "github.com/jfrog/frogbot/v2/scanrepository" + "github.com/jfrog/frogbot/v2/utils" + "github.com/jfrog/frogbot/v2/utils/outputwriter" ) const ( @@ -77,18 +78,15 @@ func setIntegrationTestEnvs(t *testing.T, testDetails *IntegrationTestDetails) f // so we restore them at the end of the test to avoid collisions with other tests envRestoreFunc := getJfrogEnvRestoreFunc(t) unsetEnvs := utils.SetEnvsAndAssertWithCallback(t, map[string]string{ - utils.RequirementsFileEnv: "requirements.txt", - utils.GitPullRequestIDEnv: testDetails.PullRequestID, - utils.GitProvider: testDetails.GitProvider, - utils.GitTokenEnv: testDetails.GitToken, - utils.GitRepoEnv: testDetails.RepoName, - utils.GitRepoOwnerEnv: testDetails.RepoOwner, - utils.BranchNameTemplateEnv: testDetails.CustomBranchName, - utils.GitApiEndpointEnv: testDetails.ApiEndpoint, - utils.GitProjectEnv: testDetails.GitProject, - utils.GitUsernameEnv: testDetails.GitUsername, - utils.GitBaseBranchEnv: mainBranch, - utils.GitUseLocalRepositoryEnv: fmt.Sprintf("%t", testDetails.UseLocalRepo), + utils.GitPullRequestIDEnv: testDetails.PullRequestID, + utils.GitProvider: testDetails.GitProvider, + utils.GitTokenEnv: testDetails.GitToken, + utils.GitRepoEnv: testDetails.RepoName, + utils.GitRepoOwnerEnv: testDetails.RepoOwner, + utils.GitApiEndpointEnv: testDetails.ApiEndpoint, + utils.GitAzureProjectEnv: testDetails.GitProject, + utils.GitBitBucketUsernameEnv: testDetails.GitUsername, + utils.GitBaseBranchEnv: mainBranch, }) return func() { envRestoreFunc() diff --git a/packagehandlers/commonpackagehandler.go b/packagehandlers/commonpackagehandler.go index a5643af33..934ef14b8 100644 --- a/packagehandlers/commonpackagehandler.go +++ b/packagehandlers/commonpackagehandler.go @@ -8,10 +8,11 @@ import ( "regexp" "strings" - "github.com/jfrog/frogbot/v2/utils" "github.com/jfrog/jfrog-cli-core/v2/utils/config" "github.com/jfrog/jfrog-cli-security/utils/techutils" "github.com/jfrog/jfrog-client-go/utils/log" + + "github.com/jfrog/frogbot/v2/utils" ) // PackageHandler interface to hold operations on packages @@ -33,7 +34,7 @@ func GetCompatiblePackageHandler(vulnDetails *utils.VulnerabilityDetails, detail case techutils.Yarn: handler = &YarnPackageHandler{} case techutils.Pip: - handler = &PythonPackageHandler{pipRequirementsFile: details.PipRequirementsFile} + handler = &PythonPackageHandler{pipRequirementsFile: defaultRequirementFile} case techutils.Maven: handler = NewMavenPackageHandler(details) case techutils.Nuget: @@ -47,7 +48,6 @@ func GetCompatiblePackageHandler(vulnDetails *utils.VulnerabilityDetails, detail default: handler = &UnsupportedPackageHandler{} } - handler.SetCommonParams(details.ServerDetails, details.DepsRepo) return } diff --git a/packagehandlers/mavenpackagehandler.go b/packagehandlers/mavenpackagehandler.go index 6796acbeb..f5bb70f69 100644 --- a/packagehandlers/mavenpackagehandler.go +++ b/packagehandlers/mavenpackagehandler.go @@ -5,13 +5,15 @@ import ( "encoding/xml" "errors" "fmt" - "github.com/jfrog/frogbot/v2/utils" - "github.com/jfrog/jfrog-cli-security/sca/bom/buildinfo/technologies/java" - "github.com/jfrog/jfrog-client-go/utils/log" - "golang.org/x/exp/slices" "os" "path/filepath" "strings" + + "github.com/jfrog/jfrog-cli-security/sca/bom/buildinfo/technologies/java" + "github.com/jfrog/jfrog-client-go/utils/log" + "golang.org/x/exp/slices" + + "github.com/jfrog/frogbot/v2/utils" ) const MavenVersionNotAvailableErrorFormat = "Version %s is not available for artifact" @@ -134,7 +136,6 @@ type pomDependencyDetails struct { func NewMavenPackageHandler(scanDetails *utils.ScanDetails) *MavenPackageHandler { depTreeParams := &java.DepTreeParams{ Server: scanDetails.ServerDetails, - DepsRepo: scanDetails.DepsRepo, IsMavenDepTreeInstalled: true, } // The mvn-dep-tree plugin has already been installed during the audit dependency tree build phase, diff --git a/packagehandlers/pythonpackagehandler.go b/packagehandlers/pythonpackagehandler.go index 3ff6569ac..e05fa7c8f 100644 --- a/packagehandlers/pythonpackagehandler.go +++ b/packagehandlers/pythonpackagehandler.go @@ -8,12 +8,13 @@ import ( "regexp" "strings" - "github.com/jfrog/frogbot/v2/utils" "github.com/jfrog/jfrog-cli-security/utils/techutils" + + "github.com/jfrog/frogbot/v2/utils" ) const ( - + defaultRequirementFile = "requirements.txt" // Package names are case-insensitive with this prefix PythonPackageRegexPrefix = "(?i)" // Match all possible operators and versions syntax diff --git a/scanpullrequest/scanpullrequest.go b/scanpullrequest/scanpullrequest.go index 4b529e873..6224ba00c 100644 --- a/scanpullrequest/scanpullrequest.go +++ b/scanpullrequest/scanpullrequest.go @@ -4,15 +4,11 @@ import ( "context" "errors" "fmt" - "os" "path/filepath" "sort" "strings" - "github.com/jfrog/frogbot/v2/utils" - "github.com/jfrog/frogbot/v2/utils/issues" "github.com/jfrog/froggit-go/vcsclient" - "github.com/jfrog/froggit-go/vcsutils" "github.com/jfrog/jfrog-cli-security/utils/formats" "github.com/jfrog/jfrog-cli-security/utils/formats/violationutils" "github.com/jfrog/jfrog-cli-security/utils/jasutils" @@ -20,13 +16,14 @@ import ( "github.com/jfrog/jfrog-cli-security/utils/results/conversion" "github.com/jfrog/jfrog-cli-security/utils/xsc" "github.com/jfrog/jfrog-client-go/utils/log" + + "github.com/jfrog/frogbot/v2/utils" + "github.com/jfrog/frogbot/v2/utils/issues" ) const ( SecurityIssueFoundErr = "issues were detected by Frogbot\n" + "Security violation with 'fail-pull-request' rule is found" - noGitHubEnvErr = "frogbot did not scan this PR, because a GitHub Environment named 'frogbot' does not exist. Please refer to the Frogbot documentation for instructions on how to create the Environment" - noGitHubEnvReviewersErr = "frogbot did not scan this PR, because the existing GitHub Environment named 'frogbot' doesn't have reviewers selected. Please refer to the Frogbot documentation for instructions on how to create the Environment" analyticsScanPrScanType = "PR" vulnerabilitiesFilteringErrorMessage = "%s scan has completed with errors. Vulnerabilities results will be removed from final report" violationsFilteringErrorMessage = "%s scan has completed with errors. Violations results will be removed from final report" @@ -34,77 +31,28 @@ const ( type ScanPullRequestCmd struct{} -func (cmd *ScanPullRequestCmd) Run(repository utils.Repository, client vcsclient.VcsClient, frogbotRepoConnection *utils.UrlAccessChecker) (err error) { +func (pr *ScanPullRequestCmd) Run(repository utils.Repository, client vcsclient.VcsClient, frogbotRepoConnection *utils.UrlAccessChecker) (err error) { repoConfig := &repository - if repoConfig.Params.Git.GitProvider == vcsutils.GitHub { - if err = verifyGitHubFrogbotEnvironment(client, repoConfig); err != nil { - return - } - } repoConfig.OutputWriter.SetHasInternetConnection(frogbotRepoConnection.IsConnected()) - if repoConfig.Params.Git.PullRequestDetails, err = client.GetPullRequestByID(context.Background(), repoConfig.Params.Git.RepoOwner, repoConfig.Params.Git.RepoName, int(repoConfig.Params.Git.PullRequestDetails.ID)); err != nil { + if repoConfig.Params.Git.PullRequestDetails, err = client.GetPullRequestByID(context.Background(), + repoConfig.Params.Git.RepoOwner, repoConfig.Params.Git.RepoName, int(repoConfig.Params.Git.PullRequestDetails.ID)); err != nil { return } - return scanPullRequest(repoConfig, client) -} - -// Verify that the 'frogbot' GitHub environment was properly configured on the repository -func verifyGitHubFrogbotEnvironment(client vcsclient.VcsClient, repoConfig *utils.Repository) error { - if repoConfig.Params.Git.APIEndpoint != "" && repoConfig.Params.Git.APIEndpoint != "https://api.github.com" { - // Don't verify 'frogbot' environment on GitHub on-prem - return nil - } - if _, exist := os.LookupEnv(utils.GitHubActionsEnv); !exist { - // Don't verify 'frogbot' environment on non GitHub Actions CI - return nil - } - - // If the repository is not public, using 'frogbot' environment is not mandatory - repoInfo, err := client.GetRepositoryInfo(context.Background(), repoConfig.Params.Git.RepoOwner, repoConfig.Params.Git.RepoName) - if err != nil { - return err - } - if repoInfo.RepositoryVisibility != vcsclient.Public { - return nil - } - - // Get the 'frogbot' environment info and make sure it exists and includes reviewers - repoEnvInfo, err := client.GetRepositoryEnvironmentInfo(context.Background(), repoConfig.Params.Git.RepoOwner, repoConfig.Params.Git.RepoName, "frogbot") - if err != nil { - return errors.New(err.Error() + "\n" + noGitHubEnvErr) - } - if len(repoEnvInfo.Reviewers) == 0 { - return errors.New(noGitHubEnvReviewersErr) - } - - return nil -} - -// By default, the scan goes as follows: -// a. Audit the dependencies of the source and the target branches. -// b. Compare the vulnerabilities found in source and target branches, and show only the new vulnerabilities added by the pull request. -// 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.Params.Git.PullRequestDetails + pullRequestDetails := &repository.Params.Git.PullRequestDetails 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("-----------------------------------------------------------") - // Audit PR code - issues, resultContext, err := auditPullRequestAndReport(repo, client) + pullRequestIssues, resultContext, err := auditPullRequestAndReport(&repository, client) if err != nil { return } - - // Handle PR comments for scan output - if err = utils.HandlePullRequestCommentsAfterScan(issues, resultContext, repo, client, int(pullRequestDetails.ID)); err != nil { + if err = utils.HandlePullRequestCommentsAfterScan(pullRequestIssues, resultContext, &repository, client, int(pullRequestDetails.ID)); err != nil { return } - - // Fail the Frogbot task if a security violation is found and fail pr rule applied. - if issues.IsFailPrRuleApplied() { + if pullRequestIssues.IsFailPrRuleApplied() { err = errors.New(SecurityIssueFoundErr) return } @@ -112,20 +60,18 @@ func scanPullRequest(repo *utils.Repository, client vcsclient.VcsClient) (err er } func auditPullRequestAndReport(repoConfig *utils.Repository, client vcsclient.VcsClient) (issuesCollection *issues.ScansIssuesCollection, resultContext results.ResultContext, err error) { - // Prepare scanDetails, err := createBaseScanDetails(repoConfig, client) if err != nil { return } resultContext = scanDetails.ResultContext - sourceBranchWd, targetBranchWd, cleanup, err := prepareSourceCodeForScan(repoConfig, scanDetails) + sourceBranchWd, targetBranchWd, cleanup, err := downloadSourceAndTarget(repoConfig, scanDetails) if err != nil { return } defer func() { err = errors.Join(err, cleanup()) }() - // Report scanDetails.MultiScanId, scanDetails.StartTime = xsc.SendNewScanEvent( scanDetails.XrayVersion, scanDetails.XscVersion, @@ -143,7 +89,6 @@ func auditPullRequestAndReport(repoConfig *utils.Repository, client vcsclient.Vc ) } }() - // Audit PR code issuesCollection, err = auditPullRequestCode(repoConfig, scanDetails, sourceBranchWd, targetBranchWd) return } @@ -153,18 +98,14 @@ func createBaseScanDetails(repoConfig *utils.Repository, client vcsclient.VcsCli if err != nil { return } - scanDetails = utils.NewScanDetails(client, &repoConfig.Server, &repoConfig.Params.Git). + return utils.NewScanDetails(client, &repoConfig.Server, &repoConfig.Params.Git). SetJfrogVersions(repoConfig.Params.XrayVersion, repoConfig.Params.XscVersion). - SetResultsContext(repositoryCloneUrl, repoConfig.Params.JFrogPlatform.Watches, repoConfig.Params.JFrogPlatform.JFrogProjectKey, repoConfig.Params.JFrogPlatform.IncludeVulnerabilities, len(repoConfig.Params.Scan.AllowedLicenses) > 0). - SetFixableOnly(repoConfig.Params.Scan.FixableOnly). - SetConfigProfile(repoConfig.Params.Scan.ConfigProfile). - SetXscPRGitInfoContext(repoConfig.Params.Git.Project, client, repoConfig.Params.Git.PullRequestDetails). - SetDiffScan(!repoConfig.Params.JFrogPlatform.IncludeVulnerabilities). - SetAllowPartialResults(repoConfig.Params.Scan.AllowPartialResults) - return scanDetails.SetMinSeverity(repoConfig.Params.Scan.MinSeverity) + SetResultsContext(repositoryCloneUrl, repoConfig.Params.JFrogPlatform.JFrogProjectKey, false). + SetConfigProfile(repoConfig.Params.ConfigProfile). + SetXscPRGitInfoContext(repoConfig.Params.Git.Project, client, repoConfig.Params.Git.PullRequestDetails), nil } -func prepareSourceCodeForScan(repoConfig *utils.Repository, scanDetails *utils.ScanDetails) (sourceBranchWd, targetBranchWd string, cleanup func() error, err error) { +func downloadSourceAndTarget(repoConfig *utils.Repository, scanDetails *utils.ScanDetails) (sourceBranchWd, targetBranchWd string, cleanup func() error, err error) { cleanupSource := func() error { return nil } cleanupTarget := func() error { return nil } cleanup = func() error { return errors.Join(cleanupSource(), cleanupTarget()) } @@ -178,11 +119,6 @@ func prepareSourceCodeForScan(repoConfig *utils.Repository, scanDetails *utils.S err = fmt.Errorf("failed to download source branch code. Error: %s", err.Error()) return } - if repoConfig.Params.JFrogPlatform.IncludeVulnerabilities { - // No need to download target branch - log.Info("Frogbot is configured to show all issues at source branch") - return - } target := repoConfig.Params.Git.PullRequestDetails.Target if targetBranchWd, cleanupTarget, err = utils.DownloadRepoToTempDir(scanDetails.Client(), target.Owner, target.Repository, target.Name); err != nil { err = fmt.Errorf("failed to download target branch code. Error: %s", err.Error()) @@ -193,44 +129,34 @@ func prepareSourceCodeForScan(repoConfig *utils.Repository, scanDetails *utils.S func auditPullRequestCode(repoConfig *utils.Repository, scanDetails *utils.ScanDetails, sourceBranchWd, targetBranchWd string) (issuesCollection *issues.ScansIssuesCollection, err error) { issuesCollection = &issues.ScansIssuesCollection{} - - for i := range repoConfig.Params.Scan.Projects { - // Reset scan details for each project - scanDetails.SetProject(&repoConfig.Params.Scan.Projects[i]).SetResultsToCompare(nil) - // Scan target branch of the project - log.Debug("Scanning target branch code...") - if targetScanResults, e := auditPullRequestTargetCode(scanDetails, targetBranchWd); e != nil { - issuesCollection.AppendStatus(getResultScanStatues(targetScanResults)) - err = errors.Join(err, fmt.Errorf("failed to audit target branch code for %v project. Error: %s", repoConfig.Params.Scan.Projects[i].WorkingDirs, e.Error())) - continue - } else { - scanDetails.SetResultsToCompare(targetScanResults) - } - // Scan source branch of the project - log.Debug("Scanning source branch code...") - if issues, e := auditPullRequestSourceCode(repoConfig, scanDetails, sourceBranchWd, targetBranchWd); e == nil { - issuesCollection.Append(issues) - continue - } else { - if issues != nil { - // Scan error, report the scan status - issuesCollection.AppendStatus(issues.ScanStatus) - } - err = errors.Join(err, fmt.Errorf("failed to audit source branch code for %v project. Error: %s", repoConfig.Params.Scan.Projects[i].WorkingDirs, e.Error())) + log.Debug("Scanning target branch code...") + if targetScanResults, e := auditPullRequestTargetCode(scanDetails, targetBranchWd); e != nil { + issuesCollection.AppendStatus(getResultScanStatues(targetScanResults)) + return issuesCollection, fmt.Errorf("failed to audit target branch. Error: %s", e.Error()) + } else { + scanDetails.SetResultsToCompare(targetScanResults) + } + log.Debug("Scanning source branch code...") + pullRequestIssues, e := auditPullRequestSourceCode(repoConfig, scanDetails, sourceBranchWd, targetBranchWd) + if e != nil { + if pullRequestIssues != nil { + // Scan error, report the scan status + issuesCollection.AppendStatus(pullRequestIssues.ScanStatus) } + return issuesCollection, fmt.Errorf("failed to audit source branch code. Error: %s", e.Error()) } - + issuesCollection.Append(pullRequestIssues) return } func auditPullRequestTargetCode(scanDetails *utils.ScanDetails, targetBranchWd string) (scanResults *results.SecurityCommandResults, err error) { - scanResults = scanDetails.RunInstallAndAudit(utils.GetFullPathWorkingDirs(scanDetails.Project.WorkingDirs, targetBranchWd)...) + scanResults = scanDetails.Audit(targetBranchWd) err = scanResults.GetErrors() return } func auditPullRequestSourceCode(repoConfig *utils.Repository, scanDetails *utils.ScanDetails, sourceBranchWd, targetBranchWd string) (issuesCollection *issues.ScansIssuesCollection, err error) { - scanResults := scanDetails.RunInstallAndAudit(utils.GetFullPathWorkingDirs(scanDetails.Project.WorkingDirs, sourceBranchWd)...) + scanResults := scanDetails.Audit(sourceBranchWd) if err = scanResults.GetErrors(); err != nil { issuesCollection = &issues.ScansIssuesCollection{ScanStatus: getResultScanStatues(scanResults)} return @@ -238,31 +164,26 @@ func auditPullRequestSourceCode(repoConfig *utils.Repository, scanDetails *utils // Set JAS output flags based on the scan results repoConfig.OutputWriter.SetJasOutputFlags(scanResults.EntitledForJas, scanResults.HasJasScansResults(jasutils.Applicability)) workingDirs := []string{strings.TrimPrefix(sourceBranchWd, string(filepath.Separator))} - if !repoConfig.Params.JFrogPlatform.IncludeVulnerabilities && targetBranchWd != "" && scanDetails.ResultsToCompare != nil { - // Diff scan - calculated at audit source scan, make sure to include target branch working dir when converting to issues + if targetBranchWd != "" && scanDetails.ResultsToCompare != nil { log.Debug("Diff scan - converting to new issues...") workingDirs = append(workingDirs, strings.TrimPrefix(targetBranchWd, string(filepath.Separator))) } - if err = filterOutFailedScansIfAllowPartialResultsEnabled(scanDetails.ResultsToCompare, scanResults, repoConfig.Params.Scan.AllowPartialResults); err != nil { + if err = filterOutFailedScansIfFailUponScanErrorDisabled(scanDetails.ResultsToCompare, scanResults, repoConfig.Params.ConfigProfile.GeneralConfig.FailUponAnyScannerError); err != nil { return } - - // Convert to issues - if issues, e := scanResultsToIssuesCollection(scanResults, workingDirs...); e == nil { - issuesCollection = issues - return - } else { - err = errors.Join(err, fmt.Errorf("failed to get all issues for %v project. Error: %s", scanDetails.Project.WorkingDirs, e.Error())) + issuesCollection, e := scanResultsToIssuesCollection(scanResults, workingDirs...) + if e != nil { + err = errors.Join(err, fmt.Errorf("failed to get issues for pull request. Error: %s", e.Error())) } return } -// When allowPartialResults is enabled, and we are performing a diff scan (both source & target results exist), we filter out a scanner results +// When failUponScanError is disabled, and we are performing a diff scan (both source & target results exist), we filter out a scanner results // if we found any error in any of its results (non-zero status code) in either source or target results. // This logic prevents us from presenting incorrect results due to an incomplete scan that produced incomplete results that might affect the diff process. -func filterOutFailedScansIfAllowPartialResultsEnabled(targetResults, sourceResults *results.SecurityCommandResults, allowPartialResults bool) error { - if !allowPartialResults { +func filterOutFailedScansIfFailUponScanErrorDisabled(targetResults, sourceResults *results.SecurityCommandResults, failUponScanError bool) error { + if failUponScanError { return nil } if targetResults == nil { diff --git a/scanpullrequest/scanpullrequest_test.go b/scanpullrequest/scanpullrequest_test.go index 28eba9108..4718734c4 100644 --- a/scanpullrequest/scanpullrequest_test.go +++ b/scanpullrequest/scanpullrequest_test.go @@ -15,7 +15,6 @@ import ( "time" "github.com/golang/mock/gomock" - "github.com/jfrog/frogbot/v2/testdata" securityutils "github.com/jfrog/jfrog-cli-security/utils" "github.com/jfrog/jfrog-cli-security/utils/formats/sarifutils" "github.com/jfrog/jfrog-cli-security/utils/formats/violationutils" @@ -25,9 +24,8 @@ import ( "github.com/jfrog/jfrog-client-go/xray/services" "github.com/owenrumney/go-sarif/v3/pkg/report/v210/sarif" - "github.com/jfrog/frogbot/v2/utils" - "github.com/jfrog/frogbot/v2/utils/issues" - "github.com/jfrog/frogbot/v2/utils/outputwriter" + "github.com/jfrog/frogbot/v2/testdata" + "github.com/jfrog/froggit-go/vcsclient" "github.com/jfrog/froggit-go/vcsutils" coreconfig "github.com/jfrog/jfrog-cli-core/v2/utils/config" @@ -36,6 +34,10 @@ import ( "github.com/jfrog/jfrog-client-go/utils/io/fileutils" "github.com/jfrog/jfrog-client-go/utils/log" "github.com/stretchr/testify/assert" + + "github.com/jfrog/frogbot/v2/utils" + "github.com/jfrog/frogbot/v2/utils/issues" + "github.com/jfrog/frogbot/v2/utils/outputwriter" ) //go:generate go run github.com/golang/mock/mockgen@v1.6.0 -destination=../testdata/vcsclientmock.go -package=testdata github.com/jfrog/froggit-go/vcsclient VcsClient @@ -260,61 +262,6 @@ func testScanPullRequest(t *testing.T, projectName string) { utils.AssertSanitizedEnv(t) } -func TestVerifyGitHubFrogbotEnvironment(t *testing.T) { - // Init mock - client := CreateMockVcsClient(t) - environment := "frogbot" - client.EXPECT().GetRepositoryInfo(context.Background(), gitParams.RepoOwner, gitParams.RepoName).Return(vcsclient.RepositoryInfo{}, nil) - client.EXPECT().GetRepositoryEnvironmentInfo(context.Background(), gitParams.RepoOwner, gitParams.RepoName, environment).Return(vcsclient.RepositoryEnvironmentInfo{Reviewers: []string{"froggy"}}, nil) - assert.NoError(t, os.Setenv(utils.GitHubActionsEnv, "true")) - - // Run verifyGitHubFrogbotEnvironment - err := verifyGitHubFrogbotEnvironment(client, gitParams) - assert.NoError(t, err) -} - -func TestVerifyGitHubFrogbotEnvironmentNoEnv(t *testing.T) { - // Redirect log to avoid negative output - previousLogger := redirectLogOutputToNil() - defer log.SetLogger(previousLogger) - - // Init mock - client := CreateMockVcsClient(t) - environment := "frogbot" - client.EXPECT().GetRepositoryInfo(context.Background(), gitParams.RepoOwner, gitParams.RepoName).Return(vcsclient.RepositoryInfo{}, nil) - client.EXPECT().GetRepositoryEnvironmentInfo(context.Background(), gitParams.RepoOwner, gitParams.RepoName, environment).Return(vcsclient.RepositoryEnvironmentInfo{}, errors.New("404")) - assert.NoError(t, os.Setenv(utils.GitHubActionsEnv, "true")) - - // Run verifyGitHubFrogbotEnvironment - err := verifyGitHubFrogbotEnvironment(client, gitParams) - assert.ErrorContains(t, err, noGitHubEnvErr) -} - -func TestVerifyGitHubFrogbotEnvironmentNoReviewers(t *testing.T) { - // Init mock - client := CreateMockVcsClient(t) - environment := "frogbot" - client.EXPECT().GetRepositoryInfo(context.Background(), gitParams.RepoOwner, gitParams.RepoName).Return(vcsclient.RepositoryInfo{}, nil) - client.EXPECT().GetRepositoryEnvironmentInfo(context.Background(), gitParams.RepoOwner, gitParams.RepoName, environment).Return(vcsclient.RepositoryEnvironmentInfo{}, nil) - assert.NoError(t, os.Setenv(utils.GitHubActionsEnv, "true")) - - // Run verifyGitHubFrogbotEnvironment - err := verifyGitHubFrogbotEnvironment(client, gitParams) - assert.ErrorContains(t, err, noGitHubEnvReviewersErr) -} - -func TestVerifyGitHubFrogbotEnvironmentOnPrem(t *testing.T) { - repoConfig := &utils.Repository{ - Params: utils.Params{Git: utils.Git{ - VcsInfo: vcsclient.VcsInfo{APIEndpoint: "https://acme.vcs.io"}}, - }, - } - - // Run verifyGitHubFrogbotEnvironment - err := verifyGitHubFrogbotEnvironment(&vcsclient.GitHubClient{}, repoConfig) - assert.NoError(t, err) -} - func prepareConfigAndClient(t *testing.T, xrayVersion, xscVersion string, server *httptest.Server, serverParams coreconfig.ServerDetails, gitServerParams GitServerParams) (utils.Repository, vcsclient.VcsClient) { gitTestParams := &utils.Git{ GitProvider: vcsutils.GitHub, @@ -934,7 +881,7 @@ func TestFilterOutFailedScansIfAllowPartialResultsEnabled(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - err := filterOutFailedScansIfAllowPartialResultsEnabled(test.targetResults, test.sourceResults, true) + err := filterOutFailedScansIfFailUponScanErrorDisabled(test.targetResults, test.sourceResults, true) assert.NoError(t, err) sourceTarget := test.sourceResults.Targets[0] @@ -965,12 +912,6 @@ func preparePullRequestTest(t *testing.T, projectName string) (utils.Repository, // Set test-specific environment variables envVars := map[string]string{} - // Set working directories for multi-dir tests - if projectName == "multi-dir-test-proj" { - envVars[utils.WorkingDirectoryEnv] = "sub1,sub3/sub4,sub2" - envVars[utils.RequirementsFileEnv] = "requirements.txt" - } - if len(envVars) > 0 { utils.SetEnvAndAssert(t, envVars) } diff --git a/scanrepository/scanrepository.go b/scanrepository/scanrepository.go index 277570d4b..db08e9d39 100644 --- a/scanrepository/scanrepository.go +++ b/scanrepository/scanrepository.go @@ -12,9 +12,6 @@ import ( "github.com/go-git/go-git/v5" biutils "github.com/jfrog/build-info-go/utils" - "github.com/jfrog/frogbot/v2/packagehandlers" - "github.com/jfrog/frogbot/v2/utils" - "github.com/jfrog/frogbot/v2/utils/outputwriter" "github.com/jfrog/froggit-go/vcsclient" "github.com/jfrog/froggit-go/vcsutils" "github.com/jfrog/gofrog/version" @@ -28,6 +25,10 @@ import ( "github.com/jfrog/jfrog-client-go/utils/log" "golang.org/x/exp/maps" "golang.org/x/exp/slices" + + "github.com/jfrog/frogbot/v2/packagehandlers" + "github.com/jfrog/frogbot/v2/utils" + "github.com/jfrog/frogbot/v2/utils/outputwriter" ) const analyticsScanRepositoryScanType = "monitor" @@ -45,275 +46,208 @@ type ScanRepositoryCmd struct { 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 []techutils.Technology // Stores all package manager handlers for detected issues handlers map[techutils.Technology]packagehandlers.PackageHandler + customTemplates utils.CustomTemplates + XrayVersion string XscVersion string } -func (cfp *ScanRepositoryCmd) Run(repository utils.Repository, client vcsclient.VcsClient, frogbotRepoConnection *utils.UrlAccessChecker) (err error) { +func (sr *ScanRepositoryCmd) Run(repository utils.Repository, client vcsclient.VcsClient, frogbotRepoConnection *utils.UrlAccessChecker) (err error) { repository.OutputWriter.SetHasInternetConnection(frogbotRepoConnection.IsConnected()) - cfp.XrayVersion = repository.Params.XrayVersion - cfp.XscVersion = repository.Params.XscVersion - return cfp.scanAndFixRepository(&repository, client) + sr.XrayVersion = repository.Params.XrayVersion + sr.XscVersion = repository.Params.XscVersion + if err = sr.setCommandPrerequisites(&repository, client); err != nil { + return + } + log.Debug(fmt.Sprintf("Detected branches for scan: %s", strings.Join(repository.Params.Git.Branches, ", "))) + for _, branch := range repository.Params.Git.Branches { + log.Debug(fmt.Sprintf("Scanning '%s' branch...", branch)) + sr.scanDetails.SetBaseBranch(branch) + sr.scanDetails.SetXscGitInfoContext(branch, repository.Params.Git.Project, client) + if err = sr.prepareEnvAndScanBranch(&repository); err != nil { + return + } + } + return } -func (cfp *ScanRepositoryCmd) scanAndFixRepository(repository *utils.Repository, client vcsclient.VcsClient) (err error) { - if err = cfp.setCommandPrerequisites(repository, client); err != nil { +func (sr *ScanRepositoryCmd) scanAndFixRepository(repository *utils.Repository, client vcsclient.VcsClient) (err error) { + if err = sr.setCommandPrerequisites(repository, client); err != nil { return } log.Debug(fmt.Sprintf("Detected branches for scan: %s", strings.Join(repository.Params.Git.Branches, ", "))) for _, branch := range repository.Params.Git.Branches { log.Debug(fmt.Sprintf("Scanning '%s' branch...", branch)) - cfp.scanDetails.SetBaseBranch(branch) - cfp.scanDetails.SetXscGitInfoContext(branch, repository.Params.Git.Project, client) - if err = cfp.scanAndFixBranch(repository); err != nil { + sr.scanDetails.SetBaseBranch(branch) + sr.scanDetails.SetXscGitInfoContext(branch, repository.Params.Git.Project, client) + if err = sr.prepareEnvAndScanBranch(repository); err != nil { return } } return } -func (cfp *ScanRepositoryCmd) scanAndFixBranch(repository *utils.Repository) (err error) { - repoDir, restoreBaseDir, err := cfp.cloneRepositoryOrUseLocalAndCheckoutToBranch() +func (sr *ScanRepositoryCmd) prepareEnvAndScanBranch(repository *utils.Repository) (err error) { + repoDir, restoreBaseDir, err := sr.checkoutToBranch() if err != nil { return } - cfp.baseWd = repoDir + sr.baseWd = repoDir defer func() { // On dry run don't delete the folder as we want to validate results - if cfp.dryRun { + if sr.dryRun { return } err = errors.Join(err, restoreBaseDir(), fileutils.RemoveTempDir(repoDir)) }() - cfp.scanDetails.MultiScanId, cfp.scanDetails.StartTime = xsc.SendNewScanEvent( - cfp.scanDetails.XrayVersion, - cfp.scanDetails.XscVersion, - cfp.scanDetails.ServerDetails, - utils.CreateScanEvent(cfp.scanDetails.ServerDetails, cfp.scanDetails.XscGitInfoContext, analyticsScanRepositoryScanType), + sr.scanDetails.MultiScanId, sr.scanDetails.StartTime = xsc.SendNewScanEvent(sr.scanDetails.XrayVersion, sr.scanDetails.XscVersion, + sr.scanDetails.ServerDetails, utils.CreateScanEvent(sr.scanDetails.ServerDetails, sr.scanDetails.XscGitInfoContext, analyticsScanRepositoryScanType), repository.Params.JFrogPlatform.JFrogProjectKey, ) - totalFindings := 0 - + findings := 0 defer func() { - xsc.SendScanEndedEvent(cfp.scanDetails.XrayVersion, cfp.scanDetails.XscVersion, cfp.scanDetails.ServerDetails, cfp.scanDetails.MultiScanId, cfp.scanDetails.StartTime, totalFindings, &cfp.scanDetails.ResultContext, err) + xsc.SendScanEndedEvent(sr.scanDetails.XrayVersion, sr.scanDetails.XscVersion, sr.scanDetails.ServerDetails, sr.scanDetails.MultiScanId, sr.scanDetails.StartTime, findings, &sr.scanDetails.ResultContext, err) }() - - for i := range repository.Params.Scan.Projects { - cfp.scanDetails.Project = &repository.Params.Scan.Projects[i] - cfp.projectTech = []techutils.Technology{} - if findings, e := cfp.scanAndFixProject(repository); e != nil { - return e - } else { - totalFindings += findings - } - } - + findings, err = sr.scanAndFixBranch(repository) return } -func (cfp *ScanRepositoryCmd) setCommandPrerequisites(repository *utils.Repository, client vcsclient.VcsClient) (err error) { +func (sr *ScanRepositoryCmd) setCommandPrerequisites(repository *utils.Repository, client vcsclient.VcsClient) (err error) { repositoryCloneUrl, err := repository.Params.Git.GetRepositoryHttpsCloneUrl(client) if err != nil { return } - // Set the scan details - cfp.scanDetails = utils.NewScanDetails(client, &repository.Server, &repository.Params.Git). - SetJfrogVersions(cfp.XrayVersion, cfp.XscVersion). - SetResultsContext(repositoryCloneUrl, repository.Params.JFrogPlatform.Watches, repository.Params.JFrogPlatform.JFrogProjectKey, repository.Params.JFrogPlatform.IncludeVulnerabilities, len(repository.Params.Scan.AllowedLicenses) > 0). - SetFixableOnly(repository.Params.Scan.FixableOnly). - SetConfigProfile(repository.Params.Scan.ConfigProfile). - SetAllowPartialResults(repository.Params.Scan.AllowPartialResults) - - if cfp.scanDetails, err = cfp.scanDetails.SetMinSeverity(repository.Params.Scan.MinSeverity); err != nil { - return - } + sr.scanDetails = utils.NewScanDetails(client, &repository.Server, &repository.Params.Git). + SetJfrogVersions(sr.XrayVersion, sr.XscVersion). + SetResultsContext(repositoryCloneUrl, repository.Params.JFrogPlatform.JFrogProjectKey, false). + SetConfigProfile(repository.Params.ConfigProfile) - // Set the flag for aggregating fixes to generate a unified pull request for fixing vulnerabilities - cfp.aggregateFixes = repository.Params.Git.AggregateFixes // Set the outputwriter interface for the relevant vcs git provider - cfp.OutputWriter = outputwriter.GetCompatibleOutputWriter(repository.Params.Git.GitProvider) - cfp.OutputWriter.SetSizeLimit(client) + sr.OutputWriter = outputwriter.GetCompatibleOutputWriter(repository.Params.Git.GitProvider) + sr.OutputWriter.SetSizeLimit(client) // Set the git client to perform git operations - cfp.gitManager, err = utils.NewGitManager(). - SetAuth(cfp.scanDetails.Username, cfp.scanDetails.Token). - SetDryRun(cfp.dryRun, cfp.dryRunRepoPath). + sr.gitManager, err = utils.NewGitManager(). + SetAuth(sr.scanDetails.Username, sr.scanDetails.Token). + SetDryRun(sr.dryRun, sr.dryRunRepoPath). SetRemoteGitUrl(repositoryCloneUrl) if err != nil { return } - _, err = cfp.gitManager.SetGitParams(cfp.scanDetails.Git) + if sr.customTemplates, err = utils.LoadCustomTemplates(repository.ConfigProfile.FrogbotConfig.CommitMessageTemplate, + repository.ConfigProfile.FrogbotConfig.BranchNameTemplate, repository.ConfigProfile.FrogbotConfig.PrTitleTemplate); err != nil { + return + } + sr.gitManager.SetGitParams(sr.scanDetails.Git) return } -func (cfp *ScanRepositoryCmd) scanAndFixProject(repository *utils.Repository) (int, error) { - var fixNeeded bool +func (sr *ScanRepositoryCmd) scanAndFixBranch(repository *utils.Repository) (int, error) { totalFindings := 0 - // A map that contains the full project paths as a keys - // 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) - for _, fullPathWd := range projectFullPathWorkingDirs { - scanResults, err := cfp.scan(fullPathWd) - if err != nil { - if err = utils.CreateErrorIfPartialResultsDisabled(cfp.scanDetails.AllowPartialResults(), fmt.Sprintf("An error occurred during Audit execution for '%s' working directory. Fixes will be skipped for this working directory", fullPathWd), err); err != nil { - return totalFindings, err - } - continue + scanResults, err := sr.scan() + if err != nil { + if err = utils.CreateErrorIfFailUponScannerErrorEnabled(repository.GeneralConfig.FailUponAnyScannerError, fmt.Sprintf("An error occurred during Audit execution for '%s' branch. Fixes will be skipped for this branch", sr.scanDetails.BaseBranch()), err); err != nil { + return 0, err } - if summary, err := conversion.NewCommandResultsConvertor(conversion.ResultConvertParams{IncludeVulnerabilities: scanResults.IncludesVulnerabilities(), HasViolationContext: scanResults.HasViolationContext()}).ConvertToSummary(scanResults); err != nil { - return totalFindings, err - } else { - findingCount := summary.GetTotalViolations() - if findingCount == 0 { - findingCount = summary.GetTotalVulnerabilities() - } - totalFindings += findingCount + } + if summary, err := conversion.NewCommandResultsConvertor(conversion.ResultConvertParams{IncludeVulnerabilities: scanResults.IncludesVulnerabilities(), HasViolationContext: scanResults.HasViolationContext()}).ConvertToSummary(scanResults); err != nil { + return 0, err + } else { + findingCount := summary.GetTotalViolations() + if findingCount == 0 { + findingCount = summary.GetTotalVulnerabilities() } + totalFindings = findingCount + } + sr.uploadResultsToGithubDashboardsIfNeeded(repository, err, scanResults) - if repository.Params.Git.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 { - log.Warn(err) - } - - if *repository.Params.Git.UploadSbomToVcs && scanResults.EntitledForJas { - if err = utils.UploadSbomSnapshotToGithubDependencyGraph(repository.Params.Git.RepoOwner, repository.Params.Git.RepoName, scanResults, cfp.scanDetails.Client(), cfp.scanDetails.BaseBranch()); err != nil { - log.Warn(err) - } - } + if !repository.Params.FrogbotConfig.CreateAutoFixPr { + log.Info(fmt.Sprintf("This command is running in detection mode only. To enable automatic fixing of issues, set the '%s' flag under the repository's coniguration settings in Jfrog platform")) // todo add configuration name + return totalFindings, nil + } + vulnerabilitiesByPathMap, err := sr.createVulnerabilitiesMap(repository.GeneralConfig.FailUponAnyScannerError, scanResults) + if err != nil { + if err = utils.CreateErrorIfFailUponScannerErrorEnabled(repository.GeneralConfig.FailUponAnyScannerError, fmt.Sprintf("An error occurred while preparing the vulnerabilities map for branch '%s'.", sr.scanDetails.BaseBranch()), err); err != nil { + return 0, err } - if repository.Params.Scan.DetectionOnly { - continue + } + if len(vulnerabilitiesByPathMap) == 0 { + log.Info("Didn't find vulnerable dependencies with existing fix versions for", sr.scanDetails.RepoName) + return totalFindings, nil + } + return totalFindings, sr.fixVulnerablePackages(repository, vulnerabilitiesByPathMap) +} + +func (sr *ScanRepositoryCmd) uploadResultsToGithubDashboardsIfNeeded(repository *utils.Repository, err error, scanResults *results.SecurityCommandResults) { + if repository.Params.Git.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, sr.scanDetails.BaseBranch(), sr.scanDetails.Client()); err != nil { + log.Warn(err) } - // Prepare the vulnerabilities map for each working dir path - currPathVulnerabilities, err := cfp.getVulnerabilitiesMap(scanResults) - if err != nil { - if err = utils.CreateErrorIfPartialResultsDisabled(cfp.scanDetails.AllowPartialResults(), fmt.Sprintf("An error occurred while preparing the vulnerabilities map for '%s' working directory. Fixes will be skipped for this working directory", fullPathWd), err); err != nil { - return totalFindings, err + + if *repository.Params.Git.UploadSbomToVcs && scanResults.EntitledForJas { + if err = utils.UploadSbomSnapshotToGithubDependencyGraph(repository.Params.Git.RepoOwner, repository.Params.Git.RepoName, scanResults, sr.scanDetails.Client(), sr.scanDetails.BaseBranch()); err != nil { + log.Warn(err) } - continue } - if len(currPathVulnerabilities) > 0 { - fixNeeded = true - } - vulnerabilitiesByPathMap[fullPathWd] = currPathVulnerabilities - } - if repository.Params.Scan.DetectionOnly { - log.Info(fmt.Sprintf("This command is running in detection mode only. To enable automatic fixing of issues, set the '%s' environment variable to 'false'.", utils.DetectionOnlyEnv)) - } else if fixNeeded { - return totalFindings, cfp.fixVulnerablePackages(repository, vulnerabilitiesByPathMap) } - return totalFindings, nil } // Audit the dependencies of the current commit. -func (cfp *ScanRepositoryCmd) scan(currentWorkingDir string) (*results.SecurityCommandResults, error) { - // Audit commit code - auditResults := cfp.scanDetails.RunInstallAndAudit(currentWorkingDir) +func (sr *ScanRepositoryCmd) scan() (*results.SecurityCommandResults, error) { + auditResults := sr.scanDetails.Audit(sr.baseWd) if err := auditResults.GetErrors(); err != nil { return nil, err } log.Info("Xray scan completed") - cfp.OutputWriter.SetJasOutputFlags(auditResults.EntitledForJas, auditResults.HasJasScansResults(jasutils.Applicability)) - cfp.projectTech = auditResults.GetTechnologies(cfp.projectTech...) + sr.OutputWriter.SetJasOutputFlags(auditResults.EntitledForJas, auditResults.HasJasScansResults(jasutils.Applicability)) + sr.projectTech = auditResults.GetTechnologies(sr.projectTech...) return auditResults, nil } -func (cfp *ScanRepositoryCmd) getVulnerabilitiesMap(scanResults *results.SecurityCommandResults) (map[string]*utils.VulnerabilityDetails, error) { - vulnerabilitiesMap, err := cfp.createVulnerabilitiesMap(scanResults) - if err != nil { - return nil, err - } - - // Nothing to fix, return - if len(vulnerabilitiesMap) == 0 { - log.Info("Didn't find vulnerable dependencies with existing fix versions for", cfp.scanDetails.RepoName) - } - return vulnerabilitiesMap, nil -} - -func (cfp *ScanRepositoryCmd) fixVulnerablePackages(repository *utils.Repository, vulnerabilitiesByWdMap map[string]map[string]*utils.VulnerabilityDetails) (err error) { - if cfp.aggregateFixes { - err = cfp.fixIssuesSinglePR(repository, vulnerabilitiesByWdMap) +func (sr *ScanRepositoryCmd) fixVulnerablePackages(repository *utils.Repository, vulnerabilitiesMap map[string]*utils.VulnerabilityDetails) (err error) { + if repository.FrogbotConfig.AggregateFixes { + aggregatedFixBranchName, e := sr.gitManager.GenerateAggregatedFixBranchName(sr.scanDetails.BaseBranch(), sr.projectTech) + if e != nil { + return + } + existingPullRequestDetails, e := sr.getOpenPullRequestBySourceBranch(aggregatedFixBranchName) + if e != nil { + return + } + e = sr.aggregateFixAndOpenPullRequest(repository, vulnerabilitiesMap, aggregatedFixBranchName, existingPullRequestDetails) } else { - err = cfp.fixIssuesSeparatePRs(repository, vulnerabilitiesByWdMap) + if e := sr.fixProjectVulnerabilities(repository, vulnerabilitiesMap); e != nil { + err = errors.Join(err, fmt.Errorf("the following errors occured while fixing vulnerabilities in '%s':\n%s", sr.scanDetails.BaseBranch(), e)) + } } if err != nil { - return utils.CreateErrorIfPartialResultsDisabled(cfp.scanDetails.AllowPartialResults(), fmt.Sprintf("failed to fix vulnerable dependencies: %s", err.Error()), err) + return utils.CreateErrorIfFailUponScannerErrorEnabled(repository.GeneralConfig.FailUponAnyScannerError, fmt.Sprintf("failed to fix vulnerable dependencies: %s", err.Error()), err) } return } -func (cfp *ScanRepositoryCmd) fixIssuesSeparatePRs(repository *utils.Repository, vulnerabilitiesMap map[string]map[string]*utils.VulnerabilityDetails) error { - var err error - for fullPath, vulnerabilities := range vulnerabilitiesMap { - if e := cfp.fixProjectVulnerabilities(repository, fullPath, vulnerabilities); e != nil { - err = errors.Join(err, fmt.Errorf("the following errors occured while fixing vulnerabilities in '%s':\n%s", fullPath, e)) - } - } - return err -} - -func (cfp *ScanRepositoryCmd) fixProjectVulnerabilities(repository *utils.Repository, fullProjectPath string, vulnerabilities map[string]*utils.VulnerabilityDetails) (err error) { - // Update the working directory to the project's current working directory - projectWorkingDir := utils.GetRelativeWd(fullProjectPath, cfp.baseWd) - - // 'CD' into the relevant working directory - if projectWorkingDir != "" { - var restoreDirFunc func() error - if restoreDirFunc, err = utils.Chdir(projectWorkingDir); err != nil { - return - } - defer func() { - err = errors.Join(err, restoreDirFunc()) - }() - } - +func (sr *ScanRepositoryCmd) fixProjectVulnerabilities(repository *utils.Repository, vulnerabilities map[string]*utils.VulnerabilityDetails) (err error) { // Fix every vulnerability in a separate pull request and branch for _, vulnerability := range vulnerabilities { - if e := cfp.fixSinglePackageAndCreatePR(repository, vulnerability); e != nil { - err = errors.Join(err, cfp.handleUpdatePackageErrors(e)) - } - - // After fixing the current vulnerability, checkout to the base branch to start fixing the next vulnerability - if e := cfp.gitManager.Checkout(cfp.scanDetails.BaseBranch()); e != nil { - err = errors.Join(err, cfp.handleUpdatePackageErrors(e)) - return + if e := sr.fixSinglePackageAndCreatePR(repository, vulnerability); e != nil { + err = errors.Join(err, sr.handleUpdatePackageErrors(e)) } } - return } -func (cfp *ScanRepositoryCmd) fixMultiplePackages(fullProjectPath string, vulnerabilities map[string]*utils.VulnerabilityDetails) (fixedVulnerabilities []*utils.VulnerabilityDetails, err error) { - // Update the working directory to the project's current working directory - projectWorkingDir := utils.GetRelativeWd(fullProjectPath, cfp.baseWd) - - // 'CD' into the relevant working directory - if projectWorkingDir != "" { - var restoreDir func() error - restoreDir, err = utils.Chdir(projectWorkingDir) - if err != nil { - return nil, err - } - defer func() { - err = errors.Join(err, restoreDir()) - }() - } +func (sr *ScanRepositoryCmd) fixMultiplePackages(vulnerabilities map[string]*utils.VulnerabilityDetails) (fixedVulnerabilities []*utils.VulnerabilityDetails, err error) { for _, vulnDetails := range vulnerabilities { - if e := cfp.updatePackageToFixedVersion(vulnDetails); e != nil { - err = errors.Join(err, cfp.handleUpdatePackageErrors(e)) + if e := sr.updatePackageToFixedVersion(vulnDetails); e != nil { + err = errors.Join(err, sr.handleUpdatePackageErrors(e)) continue } fixedVulnerabilities = append(fixedVulnerabilities, vulnDetails) @@ -322,27 +256,10 @@ func (cfp *ScanRepositoryCmd) fixMultiplePackages(fullProjectPath string, vulner return } -// Fixes all the vulnerabilities in a single aggregated pull request. -// If an existing aggregated fix is present, it checks for different scan results. -// If the scan results are the same, no action is taken. -// Otherwise, it performs a force push to the same branch and reopens the pull request if it was closed. -// Only one aggregated pull request should remain open at all times. -func (cfp *ScanRepositoryCmd) fixIssuesSinglePR(repository *utils.Repository, vulnerabilitiesMap map[string]map[string]*utils.VulnerabilityDetails) (err error) { - aggregatedFixBranchName, err := cfp.gitManager.GenerateAggregatedFixBranchName(cfp.scanDetails.BaseBranch(), cfp.projectTech) - if err != nil { - return - } - existingPullRequestDetails, err := cfp.getOpenPullRequestBySourceBranch(aggregatedFixBranchName) - if err != nil { - return - } - return cfp.aggregateFixAndOpenPullRequest(repository, vulnerabilitiesMap, aggregatedFixBranchName, existingPullRequestDetails) -} - // Handles possible error of update package operation // When the expected custom error occurs, log to debug. // else, return the error -func (cfp *ScanRepositoryCmd) handleUpdatePackageErrors(err error) error { +func (sr *ScanRepositoryCmd) handleUpdatePackageErrors(err error) error { var errUnsupportedFix *utils.ErrUnsupportedFix var errNoChangesToCommit *utils.ErrNothingToCommit @@ -359,14 +276,14 @@ func (cfp *ScanRepositoryCmd) handleUpdatePackageErrors(err error) error { // Creates a branch for the fixed package and open pull request against the target branch. // In case a branch already exists on remote, we skip it. -func (cfp *ScanRepositoryCmd) fixSinglePackageAndCreatePR(repository *utils.Repository, vulnDetails *utils.VulnerabilityDetails) (err error) { +func (sr *ScanRepositoryCmd) fixSinglePackageAndCreatePR(repository *utils.Repository, vulnDetails *utils.VulnerabilityDetails) (err error) { fixVersion := vulnDetails.SuggestedFixedVersion log.Debug("Attempting to fix", fmt.Sprintf("%s:%s", vulnDetails.ImpactedDependencyName, vulnDetails.ImpactedDependencyVersion), "with", fixVersion) - fixBranchName, err := cfp.gitManager.GenerateFixBranchName(cfp.scanDetails.BaseBranch(), vulnDetails.ImpactedDependencyName, fixVersion) + fixBranchName, err := sr.gitManager.GenerateFixBranchName(sr.scanDetails.BaseBranch(), vulnDetails.ImpactedDependencyName, fixVersion) if err != nil { return } - existsInRemote, err := cfp.gitManager.BranchExistsInRemote(fixBranchName) + existsInRemote, err := sr.gitManager.BranchExistsInRemote(fixBranchName) if err != nil { return } @@ -375,33 +292,33 @@ func (cfp *ScanRepositoryCmd) fixSinglePackageAndCreatePR(repository *utils.Repo return } - workTreeIsClean, err := cfp.gitManager.IsClean() + workTreeIsClean, err := sr.gitManager.IsClean() if err != nil { return } if !workTreeIsClean { // If there are local changes, such as files generated after running an 'install' command, we aim to preserve them in the new branch - err = cfp.gitManager.CreateBranchAndCheckout(fixBranchName, true) + err = sr.gitManager.CreateBranchAndCheckout(fixBranchName, true) } else { - err = cfp.gitManager.CreateBranchAndCheckout(fixBranchName, false) + err = sr.gitManager.CreateBranchAndCheckout(fixBranchName, false) } if err != nil { return } - if err = cfp.updatePackageToFixedVersion(vulnDetails); err != nil { + if err = sr.updatePackageToFixedVersion(vulnDetails); err != nil { return } - if err = cfp.openFixingPullRequest(repository, fixBranchName, vulnDetails); err != nil { + if err = sr.openFixingPullRequest(repository, fixBranchName, vulnDetails); err != nil { return errors.Join(fmt.Errorf("failed while creating a fixing pull request for: %s with version: %s with error: ", vulnDetails.ImpactedDependencyName, fixVersion), err) } log.Info(fmt.Sprintf("Created Pull Request updating dependency '%s' to version '%s'", vulnDetails.ImpactedDependencyName, vulnDetails.SuggestedFixedVersion)) return } -func (cfp *ScanRepositoryCmd) openFixingPullRequest(repository *utils.Repository, fixBranchName string, vulnDetails *utils.VulnerabilityDetails) (err error) { +func (sr *ScanRepositoryCmd) openFixingPullRequest(repository *utils.Repository, fixBranchName string, vulnDetails *utils.VulnerabilityDetails) (err error) { log.Debug("Checking if there are changes to commit") - isClean, err := cfp.gitManager.IsClean() + isClean, err := sr.gitManager.IsClean() if err != nil { return } @@ -409,32 +326,32 @@ func (cfp *ScanRepositoryCmd) openFixingPullRequest(repository *utils.Repository // In instances where a fix is required that Frogbot does not support, the worktree will remain clean, and there will be nothing to push return &utils.ErrNothingToCommit{PackageName: vulnDetails.ImpactedDependencyName} } - commitMessage := cfp.gitManager.GenerateCommitMessage(vulnDetails.ImpactedDependencyName, vulnDetails.SuggestedFixedVersion) - if err = cfp.cleanNewFilesMissingInRemote(); err != nil { - log.Warn(fmt.Sprintf("failed fo clean untracked files from '%s' due to the following errors: %s", cfp.baseWd, err.Error())) + commitMessage := sr.gitManager.GenerateCommitMessage(vulnDetails.ImpactedDependencyName, vulnDetails.SuggestedFixedVersion) + if err = sr.cleanNewFilesMissingInRemote(); err != nil { + log.Warn(fmt.Sprintf("failed fo clean untracked files from '%s' due to the following errors: %s", sr.baseWd, err.Error())) } - if err = cfp.gitManager.AddAllAndCommit(commitMessage, vulnDetails.ImpactedDependencyName); err != nil { + if err = sr.gitManager.AddAllAndCommit(commitMessage, vulnDetails.ImpactedDependencyName); err != nil { return } - if err = cfp.gitManager.Push(false, fixBranchName); err != nil { + if err = sr.gitManager.Push(false, fixBranchName); err != nil { return } - return cfp.handleFixPullRequestContent(repository, fixBranchName, nil, vulnDetails) + return sr.handleFixPullRequestContent(repository, fixBranchName, nil, vulnDetails) } -func (cfp *ScanRepositoryCmd) handleFixPullRequestContent(repository *utils.Repository, fixBranchName string, pullRequestInfo *vcsclient.PullRequestInfo, vulnerabilities ...*utils.VulnerabilityDetails) (err error) { - pullRequestTitle, prBody, extraComments, err := cfp.preparePullRequestDetails(vulnerabilities...) +func (sr *ScanRepositoryCmd) handleFixPullRequestContent(repository *utils.Repository, fixBranchName string, pullRequestInfo *vcsclient.PullRequestInfo, vulnerabilities ...*utils.VulnerabilityDetails) (err error) { + pullRequestTitle, prBody, extraComments, err := sr.preparePullRequestDetails(repository.FrogbotConfig.AggregateFixes, vulnerabilities...) if err != nil { return } // Update PR description - if pullRequestInfo, err = cfp.createOrUpdatePullRequest(repository, pullRequestInfo, fixBranchName, pullRequestTitle, prBody); err != nil { + if pullRequestInfo, err = sr.createOrUpdatePullRequest(repository, pullRequestInfo, fixBranchName, pullRequestTitle, prBody); err != nil { return } // Update PR extra comments - client := cfp.scanDetails.Client() + client := sr.scanDetails.Client() for _, comment := range extraComments { - if err = client.AddPullRequestComment(context.Background(), cfp.scanDetails.RepoOwner, cfp.scanDetails.RepoName, comment, int(pullRequestInfo.ID)); err != nil { + if err = client.AddPullRequestComment(context.Background(), sr.scanDetails.RepoOwner, sr.scanDetails.RepoName, comment, int(pullRequestInfo.ID)); err != nil { err = errors.New("couldn't add pull request comment: " + err.Error()) return } @@ -442,41 +359,41 @@ func (cfp *ScanRepositoryCmd) handleFixPullRequestContent(repository *utils.Repo return } -func (cfp *ScanRepositoryCmd) createOrUpdatePullRequest(repository *utils.Repository, pullRequestInfo *vcsclient.PullRequestInfo, fixBranchName, pullRequestTitle, prBody string) (prInfo *vcsclient.PullRequestInfo, err error) { +func (sr *ScanRepositoryCmd) createOrUpdatePullRequest(repository *utils.Repository, pullRequestInfo *vcsclient.PullRequestInfo, fixBranchName, pullRequestTitle, prBody string) (prInfo *vcsclient.PullRequestInfo, err error) { if pullRequestInfo == nil { - log.Info("Creating Pull Request from:", fixBranchName, "to:", cfp.scanDetails.BaseBranch()) - if err = cfp.scanDetails.Client().CreatePullRequest(context.Background(), cfp.scanDetails.RepoOwner, cfp.scanDetails.RepoName, fixBranchName, cfp.scanDetails.BaseBranch(), pullRequestTitle, prBody); err != nil { + log.Info("Creating Pull Request from:", fixBranchName, "to:", sr.scanDetails.BaseBranch()) + if err = sr.scanDetails.Client().CreatePullRequest(context.Background(), sr.scanDetails.RepoOwner, sr.scanDetails.RepoName, fixBranchName, sr.scanDetails.BaseBranch(), pullRequestTitle, prBody); err != nil { return } - return cfp.getOpenPullRequestBySourceBranch(fixBranchName) + return sr.getOpenPullRequestBySourceBranch(fixBranchName) } - log.Info("Updating Pull Request from:", fixBranchName, "to:", cfp.scanDetails.BaseBranch()) - if err = cfp.scanDetails.Client().UpdatePullRequest(context.Background(), cfp.scanDetails.RepoOwner, cfp.scanDetails.RepoName, pullRequestTitle, prBody, pullRequestInfo.Target.Name, int(pullRequestInfo.ID), vcsutils.Open); err != nil { + log.Info("Updating Pull Request from:", fixBranchName, "to:", sr.scanDetails.BaseBranch()) + if err = sr.scanDetails.Client().UpdatePullRequest(context.Background(), sr.scanDetails.RepoOwner, sr.scanDetails.RepoName, pullRequestTitle, prBody, pullRequestInfo.Target.Name, int(pullRequestInfo.ID), vcsutils.Open); err != nil { return } // Delete old extra comments - return pullRequestInfo, utils.DeletePullRequestComments(repository, cfp.scanDetails.Client(), int(pullRequestInfo.ID)) + return pullRequestInfo, utils.DeletePullRequestComments(repository, sr.scanDetails.Client(), int(pullRequestInfo.ID)) } // Handles the opening or updating of a pull request when the aggregate mode is active. // If a pull request is already open, Frogbot will update the branch and the pull request body. -func (cfp *ScanRepositoryCmd) openAggregatedPullRequest(repository *utils.Repository, fixBranchName string, pullRequestInfo *vcsclient.PullRequestInfo, vulnerabilities []*utils.VulnerabilityDetails) (err error) { - commitMessage := cfp.gitManager.GenerateAggregatedCommitMessage(cfp.projectTech) - if err = cfp.cleanNewFilesMissingInRemote(); err != nil { +func (sr *ScanRepositoryCmd) openAggregatedPullRequest(repository *utils.Repository, fixBranchName string, pullRequestInfo *vcsclient.PullRequestInfo, vulnerabilities []*utils.VulnerabilityDetails) (err error) { + commitMessage := sr.gitManager.GenerateAggregatedCommitMessage(sr.projectTech) + if err = sr.cleanNewFilesMissingInRemote(); err != nil { return } - if err = cfp.gitManager.AddAllAndCommit(commitMessage, ""); err != nil { + if err = sr.gitManager.AddAllAndCommit(commitMessage, ""); err != nil { return } - if err = cfp.gitManager.Push(true, fixBranchName); err != nil { + if err = sr.gitManager.Push(true, fixBranchName); err != nil { return } - return cfp.handleFixPullRequestContent(repository, fixBranchName, pullRequestInfo, vulnerabilities...) + return sr.handleFixPullRequestContent(repository, fixBranchName, pullRequestInfo, vulnerabilities...) } -func (cfp *ScanRepositoryCmd) cleanNewFilesMissingInRemote() error { +func (sr *ScanRepositoryCmd) cleanNewFilesMissingInRemote() error { // Open the local repository - localRepo, err := git.PlainOpen(cfp.baseWd) + localRepo, err := git.PlainOpen(sr.baseWd) if err != nil { return err } @@ -496,7 +413,7 @@ func (cfp *ScanRepositoryCmd) cleanNewFilesMissingInRemote() error { for relativeFilePath, status := range gitStatus { if status.Worktree == git.Untracked { log.Debug(fmt.Sprintf("Untracking file '%s' that was created locally during the scan/fix process", relativeFilePath)) - fileDeletionErr := os.Remove(filepath.Join(cfp.baseWd, relativeFilePath)) + fileDeletionErr := os.Remove(filepath.Join(sr.baseWd, relativeFilePath)) if fileDeletionErr != nil { err = errors.Join(err, fmt.Errorf("file '%s': %s", relativeFilePath, fileDeletionErr.Error())) continue @@ -506,67 +423,57 @@ func (cfp *ScanRepositoryCmd) cleanNewFilesMissingInRemote() error { return err } -func (cfp *ScanRepositoryCmd) preparePullRequestDetails(vulnerabilitiesDetails ...*utils.VulnerabilityDetails) (prTitle, prBody string, otherComments []string, err error) { - if cfp.dryRun && cfp.aggregateFixes { +func (sr *ScanRepositoryCmd) preparePullRequestDetails(aggregateFixes bool, vulnerabilitiesDetails ...*utils.VulnerabilityDetails) (prTitle, prBody string, otherComments []string, err error) { + if sr.dryRun && aggregateFixes { // For testings, don't compare pull request body as scan results order may change. - return cfp.gitManager.GenerateAggregatedPullRequestTitle(cfp.projectTech), "", []string{}, nil + return sr.gitManager.GenerateAggregatedPullRequestTitle(sr.projectTech), "", []string{}, nil } vulnerabilitiesRows := utils.ExtractVulnerabilitiesDetailsToRows(vulnerabilitiesDetails) - prBody, extraComments := utils.GenerateFixPullRequestDetails(vulnerabilitiesRows, cfp.OutputWriter) + prBody, extraComments := utils.GenerateFixPullRequestDetails(vulnerabilitiesRows, sr.OutputWriter) - if cfp.aggregateFixes { + if aggregateFixes { var scanHash string if scanHash, err = utils.VulnerabilityDetailsToMD5Hash(vulnerabilitiesRows...); err != nil { return } - return cfp.gitManager.GenerateAggregatedPullRequestTitle(cfp.projectTech), prBody + outputwriter.MarkdownComment(fmt.Sprintf("Checksum: %s", scanHash)), extraComments, nil + return sr.gitManager.GenerateAggregatedPullRequestTitle(sr.projectTech), prBody + outputwriter.MarkdownComment(fmt.Sprintf("Checksum: %s", scanHash)), extraComments, nil } // In separate pull requests there is only one vulnerability vulnDetails := vulnerabilitiesDetails[0] - pullRequestTitle := cfp.gitManager.GeneratePullRequestTitle(vulnDetails.ImpactedDependencyName, vulnDetails.SuggestedFixedVersion) + pullRequestTitle := sr.gitManager.GeneratePullRequestTitle(vulnDetails.ImpactedDependencyName, vulnDetails.SuggestedFixedVersion) return pullRequestTitle, prBody, extraComments, nil } -func (cfp *ScanRepositoryCmd) cloneRepositoryOrUseLocalAndCheckoutToBranch() (tempWd string, restoreDir func() error, err error) { - if cfp.dryRun { - tempWd = filepath.Join(cfp.dryRunRepoPath, cfp.scanDetails.RepoName) +func (sr *ScanRepositoryCmd) checkoutToBranch() (tempWd string, restoreDir func() error, err error) { + if sr.dryRun { + tempWd = filepath.Join(sr.dryRunRepoPath, sr.scanDetails.RepoName) } else { - // Create temp working directory if tempWd, err = fileutils.CreateTempDir(); err != nil { return } } log.Debug("Created temp working directory:", tempWd) - if cfp.scanDetails.UseLocalRepository { - var curDir string - if curDir, err = os.Getwd(); err != nil { - return - } - if err = biutils.CopyDir(curDir, tempWd, true, nil); err != nil { - return - } - // 'CD' into the temp working directory - restoreDir, err = utils.Chdir(tempWd) - if err != nil { - return - } - // Set the current copied local dir as the local git repository we are working with - err = cfp.gitManager.SetLocalRepository() - } else { - // Clone the content of the repo to the new working directory - if err = cfp.gitManager.Clone(tempWd, cfp.scanDetails.BaseBranch()); err != nil { - return - } - // 'CD' into the temp working directory - restoreDir, err = utils.Chdir(tempWd) + var curDir string + if curDir, err = os.Getwd(); err != nil { + return + } + if err = biutils.CopyDir(curDir, tempWd, true, nil); err != nil { + return + } + // 'CD' into the temp working directory + restoreDir, err = utils.Chdir(tempWd) + if err != nil { + return } + // Set the current copied local dir as the local git repository we are working with + err = sr.gitManager.SetLocalRepository() return } // Create a vulnerabilities map - a map with 'impacted package' as a key and all the necessary information of this vulnerability as value. -func (cfp *ScanRepositoryCmd) createVulnerabilitiesMap(scanResults *results.SecurityCommandResults) (map[string]*utils.VulnerabilityDetails, error) { +func (sr *ScanRepositoryCmd) createVulnerabilitiesMap(failUponError bool, scanResults *results.SecurityCommandResults) (map[string]*utils.VulnerabilityDetails, error) { vulnerabilitiesMap := map[string]*utils.VulnerabilityDetails{} simpleJsonResult, err := conversion.NewCommandResultsConvertor(conversion.ResultConvertParams{IncludeVulnerabilities: scanResults.IncludesVulnerabilities(), HasViolationContext: scanResults.HasViolationContext()}).ConvertToSimpleJson(scanResults) if err != nil { @@ -574,13 +481,13 @@ func (cfp *ScanRepositoryCmd) createVulnerabilitiesMap(scanResults *results.Secu } if len(simpleJsonResult.Vulnerabilities) > 0 { for i := range simpleJsonResult.Vulnerabilities { - if err = cfp.addVulnerabilityToFixVersionsMap(&simpleJsonResult.Vulnerabilities[i], vulnerabilitiesMap); err != nil { + if err = sr.addVulnerabilityToFixVersionsMap(failUponError, &simpleJsonResult.Vulnerabilities[i], vulnerabilitiesMap); err != nil { return nil, err } } } else if len(simpleJsonResult.SecurityViolations) > 0 { for i := range simpleJsonResult.SecurityViolations { - if err = cfp.addVulnerabilityToFixVersionsMap(&simpleJsonResult.SecurityViolations[i], vulnerabilitiesMap); err != nil { + if err = sr.addVulnerabilityToFixVersionsMap(failUponError, &simpleJsonResult.SecurityViolations[i], vulnerabilitiesMap); err != nil { return nil, err } } @@ -591,12 +498,12 @@ func (cfp *ScanRepositoryCmd) createVulnerabilitiesMap(scanResults *results.Secu return vulnerabilitiesMap, nil } -func (cfp *ScanRepositoryCmd) addVulnerabilityToFixVersionsMap(vulnerability *formats.VulnerabilityOrViolationRow, vulnerabilitiesMap map[string]*utils.VulnerabilityDetails) error { +func (sr *ScanRepositoryCmd) addVulnerabilityToFixVersionsMap(failUponError bool, vulnerability *formats.VulnerabilityOrViolationRow, vulnerabilitiesMap map[string]*utils.VulnerabilityDetails) error { if len(vulnerability.FixedVersions) == 0 { return nil } - if len(cfp.projectTech) == 0 { - cfp.projectTech = []techutils.Technology{vulnerability.Technology} + if len(sr.projectTech) == 0 { + sr.projectTech = []techutils.Technology{vulnerability.Technology} } vulnFixVersion := getMinimalFixVersion(vulnerability.ImpactedDependencyVersion, vulnerability.FixedVersions) if vulnFixVersion == "" { @@ -609,11 +516,10 @@ func (cfp *ScanRepositoryCmd) addVulnerabilityToFixVersionsMap(vulnerability *fo } else { isDirectDependency, err := utils.IsDirectDependency(vulnerability.ImpactPaths) if err != nil { - if cfp.scanDetails.AllowPartialResults() { - log.Warn(fmt.Sprintf("An error occurred while determining if the dependency '%s' is direct: %s.\nAs partial results are permitted, the vulnerability will not be fixed", vulnerability.ImpactedDependencyName, err.Error())) - } else { + if failUponError { return err } + log.Warn(fmt.Sprintf("An error occurred while determining if the dependency '%s' is direct: %s.\nAs fail upon scan configuration is permitted, the vulnerability will not be fixed", vulnerability.ImpactedDependencyName, err.Error())) } // First appearance of a version that fixes the current impacted package newVulnDetails := utils.NewVulnerabilityDetails(*vulnerability, vulnFixVersion) @@ -626,28 +532,28 @@ func (cfp *ScanRepositoryCmd) addVulnerabilityToFixVersionsMap(vulnerability *fo } // Updates impacted package, can return ErrUnsupportedFix. -func (cfp *ScanRepositoryCmd) updatePackageToFixedVersion(vulnDetails *utils.VulnerabilityDetails) (err error) { +func (sr *ScanRepositoryCmd) updatePackageToFixedVersion(vulnDetails *utils.VulnerabilityDetails) (err error) { if err = isBuildToolsDependency(vulnDetails); err != nil { return } - if cfp.handlers == nil { - cfp.handlers = make(map[techutils.Technology]packagehandlers.PackageHandler) + if sr.handlers == nil { + sr.handlers = make(map[techutils.Technology]packagehandlers.PackageHandler) } - handler := cfp.handlers[vulnDetails.Technology] + handler := sr.handlers[vulnDetails.Technology] if handler == nil { - handler = packagehandlers.GetCompatiblePackageHandler(vulnDetails, cfp.scanDetails) - cfp.handlers[vulnDetails.Technology] = handler + handler = packagehandlers.GetCompatiblePackageHandler(vulnDetails, sr.scanDetails) + sr.handlers[vulnDetails.Technology] = handler } else if _, unsupported := handler.(*packagehandlers.UnsupportedPackageHandler); unsupported { return } - return cfp.handlers[vulnDetails.Technology].UpdateDependency(vulnDetails) + return sr.handlers[vulnDetails.Technology].UpdateDependency(vulnDetails) } // The getRemoteBranchScanHash function extracts the checksum written inside the pull request body and returns it. -func (cfp *ScanRepositoryCmd) getRemoteBranchScanHash(prBody string) string { +func (sr *ScanRepositoryCmd) getRemoteBranchScanHash(prBody string) string { // The pattern matches the string "Checksum: ", followed by one or more word characters (letters, digits, or underscores). re := regexp.MustCompile(`Checksum: (\w+)`) match := re.FindStringSubmatch(prBody) @@ -662,8 +568,8 @@ func (cfp *ScanRepositoryCmd) getRemoteBranchScanHash(prBody string) string { return match[1] } -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) +func (sr *ScanRepositoryCmd) getOpenPullRequestBySourceBranch(branchName string) (prInfo *vcsclient.PullRequestInfo, err error) { + list, err := sr.scanDetails.Client().ListOpenPullRequestsWithBody(context.Background(), sr.scanDetails.RepoOwner, sr.scanDetails.RepoName) if err != nil { return } @@ -677,51 +583,45 @@ func (cfp *ScanRepositoryCmd) getOpenPullRequestBySourceBranch(branchName string return } -func (cfp *ScanRepositoryCmd) aggregateFixAndOpenPullRequest(repository *utils.Repository, vulnerabilitiesMap map[string]map[string]*utils.VulnerabilityDetails, aggregatedFixBranchName string, existingPullRequestInfo *vcsclient.PullRequestInfo) (err error) { +func (sr *ScanRepositoryCmd) aggregateFixAndOpenPullRequest(repository *utils.Repository, vulnerabilitiesMap map[string]*utils.VulnerabilityDetails, aggregatedFixBranchName string, existingPullRequestInfo *vcsclient.PullRequestInfo) (err error) { log.Info("-----------------------------------------------------------------") log.Info("Starting aggregated dependencies fix") - workTreeIsClean, err := cfp.gitManager.IsClean() + workTreeIsClean, err := sr.gitManager.IsClean() if err != nil { return } if !workTreeIsClean { // If there are local changes, such as files generated after running an 'install' command, we aim to preserve them in the new branch - err = cfp.gitManager.CreateBranchAndCheckout(aggregatedFixBranchName, true) + err = sr.gitManager.CreateBranchAndCheckout(aggregatedFixBranchName, true) } else { - err = cfp.gitManager.CreateBranchAndCheckout(aggregatedFixBranchName, false) + err = sr.gitManager.CreateBranchAndCheckout(aggregatedFixBranchName, false) } if err != nil { return } - // Fix all packages in the same branch if expected error accrued, log and continue. - var fixedVulnerabilities []*utils.VulnerabilityDetails - for fullPath, vulnerabilities := range vulnerabilitiesMap { - currentFixes, e := cfp.fixMultiplePackages(fullPath, vulnerabilities) - if e != nil { - err = errors.Join(err, fmt.Errorf("the following errors occurred while fixing vulnerabilities in %s:\n%s", fullPath, e)) - continue - } - fixedVulnerabilities = append(fixedVulnerabilities, currentFixes...) + fixedVulnerabilities, err := sr.fixMultiplePackages(vulnerabilitiesMap) + if err != nil { + return err } - updateRequired, e := cfp.isUpdateRequired(fixedVulnerabilities, existingPullRequestInfo) + updateRequired, e := sr.isUpdateRequired(fixedVulnerabilities, existingPullRequestInfo) if e != nil { err = errors.Join(err, e) return } if !updateRequired { - err = errors.Join(err, cfp.gitManager.Checkout(cfp.scanDetails.BaseBranch())) + err = errors.Join(err, sr.gitManager.Checkout(sr.scanDetails.BaseBranch())) log.Info("The existing pull request is in sync with the latest scan, and no further updates are required.") return } if len(fixedVulnerabilities) > 0 { - if e = cfp.openAggregatedPullRequest(repository, aggregatedFixBranchName, existingPullRequestInfo, fixedVulnerabilities); e != nil { + if e = sr.openAggregatedPullRequest(repository, aggregatedFixBranchName, existingPullRequestInfo, fixedVulnerabilities); e != nil { err = errors.Join(err, fmt.Errorf("failed while creating aggregated pull request. Error: \n%s", e.Error())) } } log.Info("-----------------------------------------------------------------") - err = errors.Join(err, cfp.gitManager.Checkout(cfp.scanDetails.BaseBranch())) + err = errors.Join(err, sr.gitManager.Checkout(sr.scanDetails.BaseBranch())) return } @@ -729,8 +629,8 @@ func (cfp *ScanRepositoryCmd) aggregateFixAndOpenPullRequest(repository *utils.R // First, checks if the working tree is clean. If so, no update is required. // Second, checks if there is an already open pull request for the fix. If so, no update is needed. // Lastly, performs a comparison of Xray scan result hashes between an existing pull request's remote source branch and the current source branch to identify any differences. -func (cfp *ScanRepositoryCmd) isUpdateRequired(fixedVulnerabilities []*utils.VulnerabilityDetails, prInfo *vcsclient.PullRequestInfo) (updateRequired bool, err error) { - isClean, err := cfp.gitManager.IsClean() +func (sr *ScanRepositoryCmd) isUpdateRequired(fixedVulnerabilities []*utils.VulnerabilityDetails, prInfo *vcsclient.PullRequestInfo) (updateRequired bool, err error) { + isClean, err := sr.gitManager.IsClean() if err != nil { return } @@ -751,7 +651,7 @@ func (cfp *ScanRepositoryCmd) isUpdateRequired(fixedVulnerabilities []*utils.Vul if err != nil { return } - remoteBranchScanHash := cfp.getRemoteBranchScanHash(prInfo.Body) + remoteBranchScanHash := sr.getRemoteBranchScanHash(prInfo.Body) updateRequired = currentScanHash != remoteBranchScanHash if updateRequired { log.Info("The existing pull request is not in sync with the latest scan, updating pull request...") diff --git a/scanrepository/scanrepository_test.go b/scanrepository/scanrepository_test.go index f696c2088..db85af517 100644 --- a/scanrepository/scanrepository_test.go +++ b/scanrepository/scanrepository_test.go @@ -21,8 +21,6 @@ import ( "github.com/google/go-github/v45/github" biutils "github.com/jfrog/build-info-go/utils" - "github.com/jfrog/frogbot/v2/utils" - "github.com/jfrog/frogbot/v2/utils/outputwriter" "github.com/jfrog/froggit-go/vcsclient" "github.com/jfrog/froggit-go/vcsutils" "github.com/jfrog/jfrog-cli-core/v2/utils/coreutils" @@ -34,6 +32,9 @@ import ( "github.com/jfrog/jfrog-client-go/xray/services" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/jfrog/frogbot/v2/utils" + "github.com/jfrog/frogbot/v2/utils/outputwriter" ) const rootTestDir = "scanrepository" @@ -154,33 +155,7 @@ func TestScanRepositoryCmd_Run(t *testing.T) { for _, test := range tests { t.Run(test.testName, func(t *testing.T) { // Prepare - serverParams, restoreEnv := utils.VerifyEnv(t) - defer restoreEnv() - if test.aggregateFixes { - assert.NoError(t, os.Setenv(utils.GitAggregateFixesEnv, "true")) - defer func() { - assert.NoError(t, os.Setenv(utils.GitAggregateFixesEnv, "false")) - }() - } - if test.allowPartialResults { - assert.NoError(t, os.Setenv(utils.AllowPartialResultsEnv, "true")) - defer func() { - assert.NoError(t, os.Setenv(utils.AllowPartialResultsEnv, "false")) - }() - } - // Set working directories for multi-dir/multi-project tests - switch test.testName { - case "aggregate-multi-dir": - assert.NoError(t, os.Setenv(utils.WorkingDirectoryEnv, "npm1,npm2")) - defer func() { - assert.NoError(t, os.Unsetenv(utils.WorkingDirectoryEnv)) - }() - case "partial-results-enabled": - assert.NoError(t, os.Setenv(utils.WorkingDirectoryEnv, ".,inner-project")) - defer func() { - assert.NoError(t, os.Unsetenv(utils.WorkingDirectoryEnv)) - }() - } + serverParams, _ := utils.VerifyEnv(t) xrayVersion, xscVersion, err := xsc.GetJfrogServicesVersion(&serverParams) assert.NoError(t, err) @@ -316,12 +291,6 @@ pr body server := httptest.NewServer(createScanRepoGitHubHandler(t, &port, test.mockPullRequestResponse, test.testName)) defer server.Close() port = server.URL[strings.LastIndex(server.URL, ":")+1:] - - assert.NoError(t, os.Setenv(utils.GitAggregateFixesEnv, "true")) - defer func() { - assert.NoError(t, os.Setenv(utils.GitAggregateFixesEnv, "false")) - }() - gitTestParams := &utils.Git{ GitProvider: vcsutils.GitHub, RepoOwner: "jfrog", @@ -406,10 +375,7 @@ func TestPackageTypeFromScan(t *testing.T) { assert.NoError(t, err) testScan := &ScanRepositoryCmd{OutputWriter: &outputwriter.StandardOutput{}} - trueVal := true - params := utils.Params{ - Scan: utils.Scan{Projects: []utils.Project{{UseWrapper: &trueVal}}}, - } + params := utils.Params{} var frogbotParams = utils.Repository{ Server: environmentVars, Params: params, @@ -428,22 +394,18 @@ func TestPackageTypeFromScan(t *testing.T) { assert.NoError(t, os.Chmod(filepath.Join(tmpDir, "gradlew"), 0777)) assert.NoError(t, os.Chmod(filepath.Join(tmpDir, "gradlew.bat"), 0777)) } - frogbotParams.Projects[0].WorkingDirs = []string{tmpDir} files, err := fileutils.ListFiles(tmpDir, true) assert.NoError(t, err) for _, file := range files { log.Info(file) } - frogbotParams.Projects[0].InstallCommandName = pkg.commandName - frogbotParams.Projects[0].InstallCommandArgs = pkg.commandArgs scanSetup := utils.ScanDetails{ XrayVersion: xrayVersion, XscVersion: xscVersion, - Project: &frogbotParams.Projects[0], ServerDetails: &frogbotParams.Server, } testScan.scanDetails = &scanSetup - scanResponse, err := testScan.scan(tmpDir) + scanResponse, err := testScan.scan() require.NoError(t, err) verifyTechnologyNaming(t, scanResponse.GetScaScansXrayResults(), pkg.packageType) }) @@ -535,7 +497,7 @@ func TestCreateVulnerabilitiesMap(t *testing.T) { for _, testCase := range testCases { t.Run(testCase.name, func(t *testing.T) { - fixVersionsMap, err := cfp.createVulnerabilitiesMap(testCase.scanResults) + fixVersionsMap, err := cfp.createVulnerabilitiesMap(false, testCase.scanResults) assert.NoError(t, err) for name, expectedVuln := range testCase.expectedMap { actualVuln, exists := fixVersionsMap[name] @@ -597,7 +559,7 @@ func TestPreparePullRequestDetails(t *testing.T) { }, } expectedPrBody, expectedExtraComments := utils.GenerateFixPullRequestDetails(utils.ExtractVulnerabilitiesDetailsToRows(vulnerabilities), cfp.OutputWriter) - prTitle, prBody, extraComments, err := cfp.preparePullRequestDetails(vulnerabilities...) + prTitle, prBody, extraComments, err := cfp.preparePullRequestDetails(false, vulnerabilities...) assert.NoError(t, err) assert.Equal(t, "[šŸø Frogbot] Update version of package1 to 1.0.0", prTitle) assert.Equal(t, expectedPrBody, prBody) @@ -615,10 +577,9 @@ func TestPreparePullRequestDetails(t *testing.T) { }, SuggestedFixedVersion: "2.0.0", }) - cfp.aggregateFixes = true expectedPrBody, expectedExtraComments = utils.GenerateFixPullRequestDetails(utils.ExtractVulnerabilitiesDetailsToRows(vulnerabilities), cfp.OutputWriter) expectedPrBody += outputwriter.MarkdownComment("Checksum: bec823edaceb5d0478b789798e819bde") - prTitle, prBody, extraComments, err = cfp.preparePullRequestDetails(vulnerabilities...) + prTitle, prBody, extraComments, err = cfp.preparePullRequestDetails(true, vulnerabilities...) assert.NoError(t, err) assert.Equal(t, cfp.gitManager.GenerateAggregatedPullRequestTitle([]techutils.Technology{}), prTitle) assert.Equal(t, expectedPrBody, prBody) @@ -626,7 +587,7 @@ func TestPreparePullRequestDetails(t *testing.T) { cfp.OutputWriter = &outputwriter.SimplifiedOutput{} expectedPrBody, expectedExtraComments = utils.GenerateFixPullRequestDetails(utils.ExtractVulnerabilitiesDetailsToRows(vulnerabilities), cfp.OutputWriter) expectedPrBody += outputwriter.MarkdownComment("Checksum: bec823edaceb5d0478b789798e819bde") - prTitle, prBody, extraComments, err = cfp.preparePullRequestDetails(vulnerabilities...) + prTitle, prBody, extraComments, err = cfp.preparePullRequestDetails(true, vulnerabilities...) assert.NoError(t, err) assert.Equal(t, cfp.gitManager.GenerateAggregatedPullRequestTitle([]techutils.Technology{}), prTitle) assert.Equal(t, expectedPrBody, prBody) diff --git a/utils/comment.go b/utils/comment.go index f34a551c5..e2543147b 100644 --- a/utils/comment.go +++ b/utils/comment.go @@ -5,14 +5,14 @@ import ( "errors" "fmt" "sort" - "strings" - "github.com/jfrog/frogbot/v2/utils/issues" - "github.com/jfrog/frogbot/v2/utils/outputwriter" "github.com/jfrog/froggit-go/vcsclient" "github.com/jfrog/jfrog-cli-security/utils/formats" "github.com/jfrog/jfrog-cli-security/utils/results" "github.com/jfrog/jfrog-client-go/utils/log" + + "github.com/jfrog/frogbot/v2/utils/issues" + "github.com/jfrog/frogbot/v2/utils/outputwriter" ) type ReviewCommentType string @@ -29,7 +29,6 @@ const ( SastComment ReviewCommentType = "Sast" SecretComment ReviewCommentType = "Secrets" - RescanRequestComment = "rescan" commentRemovalErrorMsg = "An error occurred while attempting to remove older Frogbot pull request comments:" ) @@ -45,9 +44,9 @@ func HandlePullRequestCommentsAfterScan(issues *issues.ScansIssuesCollection, re log.Error(fmt.Sprintf("%s:\n%v", commentRemovalErrorMsg, e)) } - // Add summary (SCA, license) scan comment - if issues.IssuesExists(repo.PullRequestSecretComments) || repo.AddPrCommentOnSuccess { - for _, comment := range generatePullRequestSummaryComment(*issues, resultContext, repo.PullRequestSecretComments, repo.OutputWriter) { + // Add summary scan comment + if issues.IssuesExists(repo.FrogbotConfig.ShowSecretsAsPrComment) || !repo.FrogbotConfig.HideSuccessBannerForNoIssues { + for _, comment := range generatePullRequestSummaryComment(*issues, resultContext, repo.FrogbotConfig.ShowSecretsAsPrComment, repo.OutputWriter) { if err = client.AddPullRequestComment(context.Background(), repo.RepoOwner, repo.RepoName, comment, pullRequestID); err != nil { err = errors.New("couldn't add pull request comment: " + err.Error()) return @@ -111,7 +110,6 @@ func GenerateFixPullRequestDetails(vulnerabilities []formats.VulnerabilityOrViol func generatePullRequestSummaryComment(issuesCollection issues.ScansIssuesCollection, resultContext results.ResultContext, includeSecrets bool, writer outputwriter.OutputWriter) []string { if !issuesCollection.IssuesExists(includeSecrets) { - // No Issues return outputwriter.GetMainCommentContent([]string{}, false, true, writer) } // Summary @@ -127,10 +125,6 @@ func generatePullRequestSummaryComment(issuesCollection issues.ScansIssuesCollec return outputwriter.GetMainCommentContent(content, true, true, writer) } -func IsFrogbotRescanComment(comment string) bool { - return strings.Contains(strings.ToLower(comment), RescanRequestComment) -} - func GetSortedPullRequestComments(client vcsclient.VcsClient, repoOwner, repoName string, prID int) ([]vcsclient.CommentInfo, error) { pullRequestsComments, err := client.ListPullRequestComments(context.Background(), repoOwner, repoName, prID) if err != nil { @@ -213,7 +207,7 @@ func getNewReviewComments(repo *Repository, issues *issues.ScansIssuesCollection } } // Secrets review comments - if !repo.Params.PullRequestSecretComments { + if !repo.FrogbotConfig.ShowSecretsAsPrComment { return } for _, secret := range issues.SecretsVulnerabilities { diff --git a/utils/comment_test.go b/utils/comment_test.go index 9c7fb29f7..831ceb896 100644 --- a/utils/comment_test.go +++ b/utils/comment_test.go @@ -3,12 +3,14 @@ package utils import ( "testing" - "github.com/jfrog/frogbot/v2/utils/issues" - "github.com/jfrog/frogbot/v2/utils/outputwriter" "github.com/jfrog/froggit-go/vcsclient" "github.com/jfrog/jfrog-cli-security/utils/formats" "github.com/jfrog/jfrog-cli-security/utils/techutils" + "github.com/jfrog/jfrog-client-go/xsc/services" "github.com/stretchr/testify/assert" + + "github.com/jfrog/frogbot/v2/utils/issues" + "github.com/jfrog/frogbot/v2/utils/outputwriter" ) func TestGetFrogbotReviewComments(t *testing.T) { @@ -615,7 +617,17 @@ func TestGetNewReviewComments(t *testing.T) { } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - repo := &Repository{OutputWriter: writer, Params: Params{Git: Git{PullRequestSecretComments: tc.generateSecretsComments}}} + repo := &Repository{ + OutputWriter: writer, + Params: Params{ + ConfigProfile: &services.ConfigProfile{ + FrogbotConfig: services.FrogbotConfig{ + ShowSecretsAsPrComment: tc.generateSecretsComments, + }, + }, + Git: Git{}, + }, + } output := getNewReviewComments(repo, tc.issues) assert.ElementsMatch(t, tc.expectedOutput, output) }) diff --git a/utils/consts.go b/utils/consts.go index 83148332b..c0bc37e88 100644 --- a/utils/consts.go +++ b/utils/consts.go @@ -24,59 +24,28 @@ const ( azurePipelines ciProvider = "azure-pipelines" // JFrog platform environment variables - JFrogUserEnv = "JF_USER" JFrogUrlEnv = "JF_URL" jfrogXrayUrlEnv = "JF_XRAY_URL" jfrogArtifactoryUrlEnv = "JF_ARTIFACTORY_URL" jfrogReleasesRepoEnv = "JF_RELEASES_REPO" - JFrogPasswordEnv = "JF_PASSWORD" + JFrogUserEnv = "JF_USER" // TODO SHOULD ADD SUPPORT ONCE CATALOG ENRICH API IS FIXED + JFrogPasswordEnv = "JF_PASSWORD" // TODO SHOULD ADD SUPPORT ONCE CATALOG ENRICH API IS FIXED JFrogTokenEnv = "JF_ACCESS_TOKEN" + jfrogProjectEnv = "JF_PROJECT_KEY" // Git environment variables GitProvider = "JF_GIT_PROVIDER" GitRepoOwnerEnv = "JF_GIT_OWNER" GitRepoEnv = "JF_GIT_REPO" - GitProjectEnv = "JF_GIT_PROJECT" - GitUsernameEnv = "JF_GIT_USERNAME" - GitUseLocalRepositoryEnv = "JF_USE_LOCAL_REPOSITORY" + GitAzureProjectEnv = "JF_GIT_AZURE_PROJECT" + GitBitBucketUsernameEnv = "JF_GIT_BB_USERNAME" GitDependencyGraphSubmissionEnv = "JF_UPLOAD_SBOM_TO_VCS" - // Git naming template environment variables - BranchNameTemplateEnv = "JF_BRANCH_NAME_TEMPLATE" - CommitMessageTemplateEnv = "JF_COMMIT_MESSAGE_TEMPLATE" - PullRequestTitleTemplateEnv = "JF_PULL_REQUEST_TITLE_TEMPLATE" - //#nosec G101 -- not a secret - PullRequestSecretCommentsEnv = "JF_PR_SHOW_SECRETS_COMMENTS" - - // Repository environment variables - InstallCommandEnv = "JF_INSTALL_DEPS_CMD" - MaxPnpmTreeDepthEnv = "JF_PNPM_MAX_TREE_DEPTH" - RequirementsFileEnv = "JF_REQUIREMENTS_FILE" - WorkingDirectoryEnv = "JF_WORKING_DIR" - PathExclusionsEnv = "JF_PATH_EXCLUSIONS" - jfrogWatchesEnv = "JF_WATCHES" - jfrogProjectEnv = "JF_PROJECT" - // To include vulnerabilities and violations - IncludeVulnerabilitiesEnv = "JF_INCLUDE_VULNERABILITIES" - AddPrCommentOnSuccessEnv = "JF_PR_ADD_SUCCESS_COMMENT" - UseWrapperEnv = "JF_USE_WRAPPER" - DepsRepoEnv = "JF_DEPS_REPO" - MinSeverityEnv = "JF_MIN_SEVERITY" - FixableOnlyEnv = "JF_FIXABLE_ONLY" - DetectionOnlyEnv = "JF_SKIP_AUTOFIX" - AllowedLicensesEnv = "JF_ALLOWED_LICENSES" - AllowPartialResultsEnv = "JF_ALLOW_PARTIAL_RESULTS" - WatchesDelimiter = "," - //#nosec G101 -- False positive - no hardcoded credentials. - GitTokenEnv = "JF_GIT_TOKEN" - GitBaseBranchEnv = "JF_GIT_BASE_BRANCH" - GitPullRequestIDEnv = "JF_GIT_PULL_REQUEST_ID" - GitApiEndpointEnv = "JF_GIT_API_ENDPOINT" - GitAggregateFixesEnv = "JF_GIT_AGGREGATE_FIXES" - - // The 'GITHUB_ACTIONS' environment variable exists when the CI is GitHub Actions - GitHubActionsEnv = "GITHUB_ACTIONS" + GitTokenEnv = "JF_GIT_TOKEN" + GitBaseBranchEnv = "JF_GIT_BASE_BRANCH" + GitPullRequestIDEnv = "JF_GIT_PULL_REQUEST_ID" + GitApiEndpointEnv = "JF_GIT_API_ENDPOINT" // Placeholders for templates PackagePlaceHolder = "{IMPACTED_PACKAGE}" diff --git a/utils/getconfiguration.go b/utils/getconfiguration.go new file mode 100644 index 000000000..47ca98a72 --- /dev/null +++ b/utils/getconfiguration.go @@ -0,0 +1,376 @@ +package utils + +import ( + "context" + "errors" + "fmt" + "net/url" + "os" + "strconv" + "strings" + + clientutils "github.com/jfrog/jfrog-client-go/utils" + + "github.com/jfrog/jfrog-cli-security/utils/xsc" + "github.com/jfrog/jfrog-client-go/xsc/services" + + "github.com/jfrog/frogbot/v2/utils/outputwriter" + + "github.com/jfrog/froggit-go/vcsclient" + "github.com/jfrog/froggit-go/vcsutils" + coreconfig "github.com/jfrog/jfrog-cli-core/v2/utils/config" + "github.com/jfrog/jfrog-client-go/utils/log" +) + +const configProfileV3MinXrayVersion = "1.0.0" // TODO REAL XRAY VERSION + +type FrogbotDetails struct { + XrayVersion string + XscVersion string + Repository Repository + ServerDetails *coreconfig.ServerDetails + GitClient vcsclient.VcsClient + ReleasesRepo string +} + +type Repository struct { + Params `yaml:"params,omitempty"` + OutputWriter outputwriter.OutputWriter + Server coreconfig.ServerDetails +} + +func (r *Repository) setOutputWriterDetails() { + r.OutputWriter = outputwriter.GetCompatibleOutputWriter(r.Params.Git.GitProvider) +} + +type Params struct { + *services.ConfigProfile + Git + JFrogPlatform +} + +type JFrogPlatform struct { + XrayVersion string + XscVersion string + JFrogProjectKey string `yaml:"jfrogProjectKey,omitempty"` +} + +func (jp *JFrogPlatform) setJfProjectKeyIfExists() (err error) { + e := &ErrMissingEnv{} + if jp.JFrogProjectKey == "" { + if err = readParamFromEnv(jfrogProjectEnv, &jp.JFrogProjectKey); err != nil && !e.IsMissingEnvErr(err) { + return + } + // We don't want to return an error from this function if the error is of type ErrMissingEnv because JFrogPlatform environment variables are not mandatory. + err = nil + } + return +} + +type Git struct { + GitProvider vcsutils.VcsProvider + vcsclient.VcsInfo + RepoOwner string + RepoName string `yaml:"repoName,omitempty"` + Branches []string `yaml:"branches,omitempty"` + PullRequestDetails vcsclient.PullRequestInfo + RepositoryCloneUrl string + UploadSbomToVcs *bool `yaml:"uploadSbomToVcs,omitempty"` +} + +func (g *Git) GetRepositoryHttpsCloneUrl(gitClient vcsclient.VcsClient) (string, error) { + if g.RepositoryCloneUrl != "" { + return g.RepositoryCloneUrl, nil + } + // If the repository clone URL is not cached, we fetch it from the VCS provider + repositoryInfo, err := gitClient.GetRepositoryInfo(context.Background(), g.RepoOwner, g.RepoName) + if err != nil { + return "", fmt.Errorf("failed to fetch the repository clone URL. %s", err.Error()) + } + g.RepositoryCloneUrl = repositoryInfo.CloneInfo.HTTP + return g.RepositoryCloneUrl, nil +} + +func (g *Git) setDefaultsIfNeeded(gitParamsFromEnv *Git, commandName string) (err error) { + g.RepoOwner = gitParamsFromEnv.RepoOwner + g.GitProvider = gitParamsFromEnv.GitProvider + g.VcsInfo = gitParamsFromEnv.VcsInfo + g.PullRequestDetails = gitParamsFromEnv.PullRequestDetails + if g.RepoName == "" { + if gitParamsFromEnv.RepoName == "" { + return fmt.Errorf("repository name is missing. please set the %s environment variable", GitRepoEnv) + } + g.RepoName = gitParamsFromEnv.RepoName + } + if commandName == ScanPullRequest { + if gitParamsFromEnv.PullRequestDetails.ID == 0 { + return errors.New("no Pull Request ID has been provided. Please configure it by using the `JF_GIT_PULL_REQUEST_ID` environment variable") + } + } + if commandName == ScanRepository { + noBranchesProvidedViaConfig := len(g.Branches) == 0 + noBranchesProvidedViaEnv := len(gitParamsFromEnv.Branches) == 0 + if noBranchesProvidedViaConfig { + if noBranchesProvidedViaEnv { + return errors.New("no branches were provided. Please set your branches using the `JF_GIT_BASE_BRANCH` environment variable") + } + g.Branches = gitParamsFromEnv.Branches + } + } + envValue, err := getBoolEnv(GitDependencyGraphSubmissionEnv, true) + if err != nil { + return err + } + g.UploadSbomToVcs = &envValue + return +} + +func GetFrogbotDetails(commandName string) (frogbotDetails *FrogbotDetails, err error) { + // Get server and git details + jfrogServer, err := extractJFrogCredentialsFromEnvs() + if err != nil { + return + } + xrayVersion, xscVersion, err := xsc.GetJfrogServicesVersion(jfrogServer) + if err != nil { + return + } + + gitParams, err := extractGitParamsFromEnvs() + if err != nil { + return + } + + defer func() { + err = errors.Join(err, SanitizeEnv()) + }() + + // Build a version control client for REST API requests + client, err := vcsclient. + NewClientBuilder(gitParams.GitProvider). + ApiEndpoint(strings.TrimSuffix(gitParams.APIEndpoint, "/")). + Token(gitParams.Token). + Project(gitParams.Project). + Logger(log.GetLogger()). + Username(gitParams.Username). + Build() + if err != nil { + return + } + + repository, err := BuildRepository(xrayVersion, xscVersion, client, gitParams, jfrogServer, commandName) + if err != nil { + return + } + + configProfile, repoCloneUrl, err := getConfigProfileIfExistsAndValid(xrayVersion, jfrogServer, client, gitParams) + if err != nil { + return + } + + // We apply the configProfile to the repository. If no config profile was fetched, a nil value is passed + repository.ConfigProfile = configProfile + repository.Git.RepositoryCloneUrl = repoCloneUrl + + frogbotDetails = createFrogbotDetails(frogbotDetails, xrayVersion, xscVersion, repository, client, jfrogServer) + return +} + +func createFrogbotDetails(frogbotDetails *FrogbotDetails, xrayVersion string, xscVersion string, repository Repository, client vcsclient.VcsClient, jfrogServer *coreconfig.ServerDetails) *FrogbotDetails { + frogbotDetails = &FrogbotDetails{XrayVersion: xrayVersion, XscVersion: xscVersion, Repository: repository, GitClient: client, ServerDetails: jfrogServer} + frogbotDetails.ReleasesRepo = os.Getenv(jfrogReleasesRepoEnv) + if frogbotDetails.ReleasesRepo == "" { + frogbotDetails.ReleasesRepo = repository.GeneralConfig.ScannersDownloadPath + } + return frogbotDetails +} + +// Builds a Repository from environment variables only +// Returns a Repository instance with all the defaults and necessary fields. +func BuildRepository(xrayVersion, xscVersion string, gitClient vcsclient.VcsClient, gitParamsFromEnv *Git, server *coreconfig.ServerDetails, commandName string) (repository Repository, err error) { + // Create a single repository from environment variables + repository = Repository{} + repository.Server = *server + repository.Params.XrayVersion = xrayVersion + repository.Params.XscVersion = xscVersion + if err = repository.Params.Git.setDefaultsIfNeeded(gitParamsFromEnv, commandName); err != nil { + return + } + if err = repository.Params.JFrogPlatform.setJfProjectKeyIfExists(); err != nil { + return + } + repository.setOutputWriterDetails() + repository.OutputWriter.SetSizeLimit(gitClient) + return repository, nil +} + +func extractJFrogCredentialsFromEnvs() (*coreconfig.ServerDetails, error) { + server := coreconfig.ServerDetails{} + platformUrl := strings.TrimSuffix(getTrimmedEnv(JFrogUrlEnv), "/") + xrUrl := strings.TrimSuffix(getTrimmedEnv(jfrogXrayUrlEnv), "/") + rtUrl := strings.TrimSuffix(getTrimmedEnv(jfrogArtifactoryUrlEnv), "/") + if xrUrl != "" && rtUrl != "" { + server.XrayUrl = xrUrl + "/" + server.ArtifactoryUrl = rtUrl + "/" + } else { + if platformUrl == "" { + return nil, fmt.Errorf("%s or %s and %s environment variables are missing", JFrogUrlEnv, jfrogXrayUrlEnv, jfrogArtifactoryUrlEnv) + } + server.Url = platformUrl + "/" + server.XrayUrl = platformUrl + "/xray/" + server.ArtifactoryUrl = platformUrl + "/artifactory/" + } + if accessToken := getTrimmedEnv(JFrogTokenEnv); accessToken != "" { + server.AccessToken = accessToken + } else { + return nil, fmt.Errorf("%s environment variable is missing", JFrogTokenEnv) + } + return &server, nil +} + +func extractGitParamsFromEnvs() (*Git, error) { + e := &ErrMissingEnv{} + var err error + gitEnvParams := &Git{} + // Branch & Repo names are mandatory variables. + // Must be set as environment variables. + // Validation performed later + // Set the base branch name + var branch string + if err = readParamFromEnv(GitBaseBranchEnv, &branch); err != nil && !e.IsMissingEnvErr(err) { + return nil, err + } + if branch != "" { + gitEnvParams.Branches = []string{branch} + } + // Non-mandatory Git Api Endpoint, if not set, default values will be used. + if err = readParamFromEnv(GitApiEndpointEnv, &gitEnvParams.APIEndpoint); err != nil && !e.IsMissingEnvErr(err) { + return nil, err + } + if err = verifyValidApiEndpoint(gitEnvParams.APIEndpoint); err != nil { + return nil, err + } + // [Mandatory] Set the Git provider + if gitEnvParams.GitProvider, err = extractVcsProviderFromEnv(); err != nil { + return nil, err + } + // [Mandatory] Set the git repository owner name (organization) + if err = readParamFromEnv(GitRepoOwnerEnv, &gitEnvParams.RepoOwner); err != nil { + return nil, err + } + // [Mandatory] Set the access token to the git provider + if err = readParamFromEnv(GitTokenEnv, &gitEnvParams.Token); err != nil { + return nil, err + } + + // [Mandatory] Set the repository name, except for multi repository. + if err = readParamFromEnv(GitRepoEnv, &gitEnvParams.RepoName); err != nil { + return nil, err + } + + // Set Bitbucket Server username + // Mandatory only for Bitbucket Server, this authentication detail is required for performing git operations. + if err = readParamFromEnv(GitBitBucketUsernameEnv, &gitEnvParams.Username); err != nil && !e.IsMissingEnvErr(err) { + return nil, err + } + // Set Azure Repos Project name + // Mandatory for Azure Repos only + if err = readParamFromEnv(GitAzureProjectEnv, &gitEnvParams.Project); err != nil && gitEnvParams.GitProvider == vcsutils.AzureRepos { + return nil, err + } + if envPrId := getTrimmedEnv(GitPullRequestIDEnv); envPrId != "" { + var convertedPrId int + if convertedPrId, err = strconv.Atoi(envPrId); err != nil { + return nil, fmt.Errorf("failed parsing %s environment variable as a number. The received environment is : %s", GitPullRequestIDEnv, envPrId) + } + gitEnvParams.PullRequestDetails = vcsclient.PullRequestInfo{ID: int64(convertedPrId)} + } + + return gitEnvParams, nil +} + +func verifyValidApiEndpoint(apiEndpoint string) error { + // Empty string will resolve to default values. + if apiEndpoint == "" { + return nil + } + parsedUrl, err := url.Parse(apiEndpoint) + if err != nil { + return err + } + if parsedUrl.Scheme == "" { + return errors.New("the given API endpoint is invalid. Please note that the API endpoint format should be provided with the 'HTTPS' protocol as a prefix") + } + return nil +} + +func readParamFromEnv(envKey string, paramValue *string) error { + *paramValue = getTrimmedEnv(envKey) + if *paramValue == "" { + return &ErrMissingEnv{VariableName: envKey} + } + return nil +} + +func getTrimmedEnv(envKey string) string { + return strings.TrimSpace(os.Getenv(envKey)) +} + +func extractVcsProviderFromEnv() (vcsutils.VcsProvider, error) { + vcsProvider := getTrimmedEnv(GitProvider) + switch vcsProvider { + case string(GitHub): + return vcsutils.GitHub, nil + case string(GitLab): + return vcsutils.GitLab, nil + // For backward compatibility, we are accepting also "bitbucket server" + case string(BitbucketServer), "bitbucket server": + return vcsutils.BitbucketServer, nil + case string(AzureRepos): + return vcsutils.AzureRepos, nil + } + return 0, fmt.Errorf("%s should be one of: '%s', '%s', '%s' or '%s'", GitProvider, GitHub, GitLab, BitbucketServer, AzureRepos) +} + +func SanitizeEnv() error { + for _, env := range os.Environ() { + if !strings.HasPrefix(env, "JF_") { + continue + } + envSplit := strings.Split(env, "=") + if err := os.Unsetenv(envSplit[0]); err != nil { + return err + } + } + return nil +} + +func getBoolEnv(envKey string, defaultValue bool) (bool, error) { + envValue := getTrimmedEnv(envKey) + if envValue != "" { + parsedEnv, err := strconv.ParseBool(envValue) + if err != nil { + return false, fmt.Errorf("the value of the %s environment is expected to be either TRUE or FALSE. The value received however is %s", envKey, envValue) + } + return parsedEnv, nil + } + + return defaultValue, nil +} + +func getConfigProfileIfExistsAndValid(xrayVersion string, jfrogServer *coreconfig.ServerDetails, gitClient vcsclient.VcsClient, gitParams *Git) (configProfile *services.ConfigProfile, repoCloneUrl string, err error) { + if err = clientutils.ValidateMinimumVersion(clientutils.Xray, xrayVersion, configProfileV3MinXrayVersion); err != nil { + log.Info(fmt.Sprintf("The utilized Frogbot version requires a higher version of Xray than %s in order to use Config Profile. Please upgrade Xray to version %s and above. Frogbot configurations will be derived from environment variables only.", xrayVersion, configProfileV3MinXrayVersion)) + return + } + if repoCloneUrl, err = gitParams.GetRepositoryHttpsCloneUrl(gitClient); err != nil { + return + } + log.Debug(fmt.Sprintf("Searching central configuration associated to repository '%s'", jfrogServer.Url)) + if configProfile, err = xsc.GetConfigProfileByUrl(xrayVersion, jfrogServer, repoCloneUrl); err != nil || configProfile == nil { + return + } + + log.Info(fmt.Sprintf("Using Config profile '%s'", configProfile.ProfileName)) + return +} diff --git a/utils/params_test.go b/utils/getconfiguration_test.go similarity index 75% rename from utils/params_test.go rename to utils/getconfiguration_test.go index 4ee276e15..2c1b75b2c 100644 --- a/utils/params_test.go +++ b/utils/getconfiguration_test.go @@ -189,60 +189,14 @@ func extractAndAssertParamsFromEnv(t *testing.T, platformUrl, basicAuth bool, co } } -func TestExtractInstallationCommandFromEnv(t *testing.T) { - defer func() { - assert.NoError(t, SanitizeEnv()) - }() - - project := &Project{} - err := project.setDefaultsIfNeeded() - assert.NoError(t, err) - assert.Empty(t, project.InstallCommandName) - assert.Empty(t, project.InstallCommandArgs) - - project = &Project{} - SetEnvAndAssert(t, map[string]string{InstallCommandEnv: "a"}) - err = project.setDefaultsIfNeeded() - assert.NoError(t, err) - assert.Equal(t, "a", project.InstallCommandName) - assert.Empty(t, project.InstallCommandArgs) - - project = &Project{} - SetEnvAndAssert(t, map[string]string{InstallCommandEnv: "a b"}) - err = project.setDefaultsIfNeeded() - assert.NoError(t, err) - assert.Equal(t, "a", project.InstallCommandName) - assert.Equal(t, []string{"b"}, project.InstallCommandArgs) - - project = &Project{} - SetEnvAndAssert(t, map[string]string{InstallCommandEnv: "a b --flagName=flagValue"}) - err = project.setDefaultsIfNeeded() - assert.NoError(t, err) - assert.Equal(t, "a", project.InstallCommandName) - assert.Equal(t, []string{"b", "--flagName=flagValue"}, project.InstallCommandArgs) -} - func TestGenerateConfigFromEnv(t *testing.T) { SetEnvAndAssert(t, map[string]string{ - JFrogUrlEnv: "", - jfrogArtifactoryUrlEnv: "http://127.0.0.1:8081/artifactory", - jfrogXrayUrlEnv: "http://127.0.0.1:8081/xray", - JFrogUserEnv: "admin", - JFrogPasswordEnv: "password", - BranchNameTemplateEnv: "branch-${BRANCH_NAME_HASH}", - CommitMessageTemplateEnv: "commit", - PullRequestTitleTemplateEnv: "pr-title", - InstallCommandEnv: "nuget restore", - UseWrapperEnv: "false", - RequirementsFileEnv: "requirements.txt", - WorkingDirectoryEnv: "a/b", - jfrogProjectEnv: "projectKey", - jfrogWatchesEnv: "watch-1, watch-2, watch-3", - DepsRepoEnv: "deps-remote", - MinSeverityEnv: "medium", - FixableOnlyEnv: "true", - DetectionOnlyEnv: "true", - AllowedLicensesEnv: "MIT, Apache-2.0", + JFrogUrlEnv: "", + jfrogArtifactoryUrlEnv: "http://127.0.0.1:8081/artifactory", + jfrogXrayUrlEnv: "http://127.0.0.1:8081/xray", + JFrogUserEnv: "admin", + JFrogPasswordEnv: "password", + jfrogProjectEnv: "projectKey", }) defer func() { assert.NoError(t, SanitizeEnv()) @@ -276,12 +230,8 @@ func TestGenerateConfigFromEnv(t *testing.T) { func validateBuildRepo(t *testing.T, repo *Repository, gitParams *Git, server *config.ServerDetails, commandName string) { assert.Equal(t, "repoName", repo.RepoName) - assert.ElementsMatch(t, repo.Watches, []string{"watch-1", "watch-2", "watch-3"}) - assert.Equal(t, "Medium", repo.MinSeverity) - assert.Equal(t, true, repo.FixableOnly) assert.Equal(t, true, repo.AddPrCommentOnSuccess) assert.Equal(t, true, repo.DetectionOnly) - assert.ElementsMatch(t, []string{"MIT", "Apache-2.0"}, repo.AllowedLicenses) assert.Equal(t, gitParams.RepoOwner, repo.RepoOwner) assert.Equal(t, gitParams.Token, repo.Token) assert.Equal(t, gitParams.APIEndpoint, repo.APIEndpoint) @@ -301,51 +251,8 @@ func validateBuildRepo(t *testing.T, repo *Repository, gitParams *Git, server *c if commandName == ScanPullRequest { assert.NotZero(t, repo.PullRequestDetails.ID) - assert.Empty(t, repo.PullRequestCommentTitle) } - project := repo.Projects[0] - assert.Equal(t, []string{"a/b"}, project.WorkingDirs) - assert.False(t, *project.UseWrapper) - assert.Equal(t, "requirements.txt", project.PipRequirementsFile) - assert.Equal(t, "nuget", project.InstallCommandName) - assert.Equal(t, []string{"restore"}, project.InstallCommandArgs) - assert.Equal(t, "deps-remote", project.DepsRepo) -} - -func TestExtractProjectParamsFromEnv(t *testing.T) { - project := &Project{} - defer func() { - assert.NoError(t, SanitizeEnv()) - }() - - // Test default values - err := project.setDefaultsIfNeeded() - assert.NoError(t, err) - assert.True(t, *project.UseWrapper) - assert.Equal(t, []string{RootDir}, project.WorkingDirs) - assert.Equal(t, "", project.PipRequirementsFile) - assert.Equal(t, "", project.InstallCommandName) - assert.Equal(t, []string(nil), project.InstallCommandArgs) - - // Test value extraction - SetEnvAndAssert(t, map[string]string{ - WorkingDirectoryEnv: "b/c", - RequirementsFileEnv: "r.txt", - UseWrapperEnv: "false", - InstallCommandEnv: "nuget restore", - DepsRepoEnv: "repository", - }) - - project = &Project{} - err = project.setDefaultsIfNeeded() - assert.NoError(t, err) - assert.Equal(t, []string{"b/c"}, project.WorkingDirs) - assert.Equal(t, "r.txt", project.PipRequirementsFile) - assert.False(t, *project.UseWrapper) - assert.Equal(t, "nuget", project.InstallCommandName) - assert.Equal(t, []string{"restore"}, project.InstallCommandArgs) - assert.Equal(t, "repository", project.DepsRepo) } func TestVerifyValidApiEndpoint(t *testing.T) { diff --git a/utils/git.go b/utils/git.go index 0b6c89505..49cacb30b 100644 --- a/utils/git.go +++ b/utils/git.go @@ -4,7 +4,6 @@ import ( "errors" "fmt" "net/http" - "regexp" "strings" "time" @@ -130,20 +129,8 @@ func (gm *GitManager) SetLocalRepository() error { return err } -func (gm *GitManager) SetGitParams(gitParams *Git) (*GitManager, error) { - var err error - if gm.customTemplates, err = loadCustomTemplates(gitParams.CommitMessageTemplate, gitParams.BranchNameTemplate, gitParams.PullRequestTitleTemplate); err != nil { - return nil, err - } +func (gm *GitManager) SetGitParams(gitParams *Git) *GitManager { gm.git = gitParams - return gm, nil -} - -func (gm *GitManager) SetEmailAuthor(emailAuthor string) *GitManager { - if gm.git == nil { - gm.git = &Git{} - } - gm.git.EmailAuthor = emailAuthor return gm } @@ -337,7 +324,7 @@ func (gm *GitManager) commit(commitMessage string) error { _, err = worktree.Commit(commitMessage, &git.CommitOptions{ Author: &object.Signature{ Name: frogbotAuthorName, - Email: gm.git.EmailAuthor, + Email: frogbotAuthorEmail, When: time.Now(), }, }) @@ -551,7 +538,7 @@ func GetFullBranchName(branchName string) plumbing.ReferenceName { return plumbing.NewBranchReferenceName(plumbing.ReferenceName(branchName).Short()) } -func loadCustomTemplates(commitMessageTemplate, branchNameTemplate, pullRequestTitleTemplate string) (customTemplates CustomTemplates, err error) { +func LoadCustomTemplates(commitMessageTemplate, branchNameTemplate, pullRequestTitleTemplate string) (customTemplates CustomTemplates, err error) { customTemplates = CustomTemplates{ commitMessageTemplate: commitMessageTemplate, branchNameTemplate: branchNameTemplate, diff --git a/utils/issues/issuescollection.go b/utils/issues/issuescollection.go index cdebd87fa..e4df23583 100644 --- a/utils/issues/issuescollection.go +++ b/utils/issues/issuescollection.go @@ -2,6 +2,7 @@ package issues import ( "fmt" + "github.com/jfrog/jfrog-cli-security/utils" "github.com/jfrog/jfrog-cli-security/utils/formats" "github.com/jfrog/jfrog-cli-security/utils/jasutils" @@ -211,35 +212,30 @@ func (ic *ScansIssuesCollection) SastIssuesExists() bool { // Checkes if a 'fail_pr' rule is applied to any of the violations in the collection func (ic *ScansIssuesCollection) IsFailPrRuleApplied() bool { - // Checking Sca Violations for _, scaViolation := range ic.ScaViolations { if scaViolation.FailPr { log.Debug(fmt.Sprintf(FailPrRuleMessage, scaViolation.ViolationContext.Watch)) return true } } - // Checking Licenses Violations for _, licenseViolation := range ic.LicensesViolations { if licenseViolation.FailPr { log.Debug(fmt.Sprintf(FailPrRuleMessage, licenseViolation.ViolationContext.Watch)) return true } } - // Checking Iac Violations for _, iacViolation := range ic.IacViolations { if iacViolation.FailPr { log.Debug(fmt.Sprintf(FailPrRuleMessage, iacViolation.ViolationContext.Watch)) return true } } - // Checking Sast Violations for _, sastViolation := range ic.SastViolations { if sastViolation.FailPr { log.Debug(fmt.Sprintf(FailPrRuleMessage, sastViolation.ViolationContext.Watch)) return true } } - // Checking Secrets Violations for _, secretViolation := range ic.SecretsViolations { if secretViolation.FailPr { log.Debug(fmt.Sprintf(FailPrRuleMessage, secretViolation.ViolationContext.Watch)) diff --git a/utils/params.go b/utils/params.go deleted file mode 100644 index 51c33c16e..000000000 --- a/utils/params.go +++ /dev/null @@ -1,648 +0,0 @@ -package utils - -import ( - "context" - "errors" - "fmt" - "net/url" - "os" - "strconv" - "strings" - - clientutils "github.com/jfrog/jfrog-client-go/utils" - - "github.com/jfrog/jfrog-cli-security/utils/techutils" - "github.com/jfrog/jfrog-cli-security/utils/xsc" - "github.com/jfrog/jfrog-client-go/xsc/services" - "golang.org/x/exp/slices" - - securityutils "github.com/jfrog/jfrog-cli-security/utils" - "github.com/jfrog/jfrog-cli-security/utils/severityutils" - - "github.com/jfrog/frogbot/v2/utils/outputwriter" - - "github.com/jfrog/froggit-go/vcsclient" - "github.com/jfrog/froggit-go/vcsutils" - coreconfig "github.com/jfrog/jfrog-cli-core/v2/utils/config" - "github.com/jfrog/jfrog-client-go/utils/log" -) - -type FrogbotDetails struct { - XrayVersion string - XscVersion string - Repository Repository - ServerDetails *coreconfig.ServerDetails - GitClient vcsclient.VcsClient - ReleasesRepo string -} - -// Returns an initialized Repository with an empty repository -func newRepository() Repository { - return Repository{Params: Params{Scan: Scan{Projects: []Project{{}}}}} -} - -type Repository struct { - Params `yaml:"params,omitempty"` - OutputWriter outputwriter.OutputWriter - Server coreconfig.ServerDetails -} - -func (r *Repository) setOutputWriterDetails() { - r.OutputWriter = outputwriter.GetCompatibleOutputWriter(r.Params.Git.GitProvider) - r.OutputWriter.SetPullRequestCommentTitle(r.Params.Git.PullRequestCommentTitle) -} - -type Params struct { - Scan `yaml:"scan,omitempty"` - Git `yaml:"git,omitempty"` - JFrogPlatform `yaml:"jfrogPlatform,omitempty"` -} - -func (p *Params) setDefaultsIfNeeded(gitParamsFromEnv *Git, commandName string) error { - if err := p.Git.setDefaultsIfNeeded(gitParamsFromEnv, commandName); err != nil { - return err - } - if err := p.JFrogPlatform.setDefaultsIfNeeded(); err != nil { - return err - } - return p.Scan.setDefaultsIfNeeded() -} - -type Project struct { - InstallCommand string `yaml:"installCommand,omitempty"` - PipRequirementsFile string `yaml:"pipRequirementsFile,omitempty"` - WorkingDirs []string `yaml:"workingDirs,omitempty"` - PathExclusions []string `yaml:"pathExclusions,omitempty"` - UseWrapper *bool `yaml:"useWrapper,omitempty"` - MaxPnpmTreeDepth string `yaml:"maxPnpmTreeDepth,omitempty"` - DepsRepo string `yaml:"repository,omitempty"` - InstallCommandName string - InstallCommandArgs []string -} - -func (p *Project) setDefaultsIfNeeded() error { - if len(p.WorkingDirs) == 0 { - workingDir := getTrimmedEnv(WorkingDirectoryEnv) - if workingDir == "" { - - // If no working directories are provided, and none exist in the environment variable, we designate the project's root directory as our sole working directory. - // We then execute a recursive scan across the entire project, commencing from the root. - workingDir = RootDir - p.WorkingDirs = append(p.WorkingDirs, workingDir) - } else { - workingDirs := strings.Split(workingDir, ",") - p.WorkingDirs = append(p.WorkingDirs, workingDirs...) - } - } - if len(p.PathExclusions) == 0 { - if p.PathExclusions, _ = readArrayParamFromEnv(PathExclusionsEnv, ";"); len(p.PathExclusions) == 0 { - p.PathExclusions = securityutils.DefaultScaExcludePatterns - } - } - if p.UseWrapper == nil { - useWrapper, err := getBoolEnv(UseWrapperEnv, true) - if err != nil { - return err - } - p.UseWrapper = &useWrapper - } - if p.InstallCommand == "" { - p.InstallCommand = getTrimmedEnv(InstallCommandEnv) - } - if p.InstallCommand != "" { - setProjectInstallCommand(p.InstallCommand, p) - } - if p.PipRequirementsFile == "" { - p.PipRequirementsFile = getTrimmedEnv(RequirementsFileEnv) - } - if p.DepsRepo == "" { - p.DepsRepo = getTrimmedEnv(DepsRepoEnv) - } - if p.MaxPnpmTreeDepth == "" { - p.MaxPnpmTreeDepth = getTrimmedEnv(MaxPnpmTreeDepthEnv) - } - - return nil -} - -func (p *Project) GetTechFromInstallCmdIfExists() []string { - var technologies []string - if p.InstallCommandName != "" { - if !slices.Contains(techutils.AllTechnologiesStrings, p.InstallCommandName) { - log.Warn(fmt.Sprintf("The technology ā€˜%sā€™ was inferred from the provided install command but is not listed among the supported technologies. Please provide an install command for one of the following supported technologies: %s", p.InstallCommandName, techutils.AllTechnologiesStrings)) - return technologies - } - technologies = append(technologies, p.InstallCommandName) - if strings.ToLower(p.InstallCommandName) == "dotnet" { - technologies = append(technologies, "nuget") - } - } - return technologies -} - -type Scan struct { - FixableOnly bool `yaml:"fixableOnly,omitempty"` - DetectionOnly bool `yaml:"skipAutoFix,omitempty"` - MinSeverity string `yaml:"minSeverity,omitempty"` - AddPrCommentOnSuccess bool `yaml:"addPrCommentOnSuccess,omitempty"` - AllowedLicenses []string `yaml:"allowedLicenses,omitempty"` - Projects []Project `yaml:"projects,omitempty"` - ConfigProfile *services.ConfigProfile - SkipAutoInstall bool - AllowPartialResults bool -} - -func (s *Scan) setDefaultsIfNeeded() (err error) { - e := &ErrMissingEnv{} - if !s.FixableOnly { - if s.FixableOnly, err = getBoolEnv(FixableOnlyEnv, false); err != nil { - return - } - } - if !s.AddPrCommentOnSuccess { - if s.AddPrCommentOnSuccess, err = getBoolEnv(AddPrCommentOnSuccessEnv, true); err != nil { - return - } - } - if !s.DetectionOnly { - if s.DetectionOnly, err = getBoolEnv(DetectionOnlyEnv, false); err != nil { - return - } - } - if s.MinSeverity == "" { - if err = readParamFromEnv(MinSeverityEnv, &s.MinSeverity); err != nil && !e.IsMissingEnvErr(err) { - return - } - } - if s.MinSeverity != "" { - var severity severityutils.Severity - if severity, err = severityutils.ParseSeverity(s.MinSeverity, false); err != nil { - return - } - s.MinSeverity = severity.String() - } - if len(s.Projects) == 0 { - s.Projects = append(s.Projects, Project{}) - } - if len(s.AllowedLicenses) == 0 { - if s.AllowedLicenses, err = readArrayParamFromEnv(AllowedLicensesEnv, ","); err != nil && !e.IsMissingEnvErr(err) { - return - } - } - if !s.AllowPartialResults { - if s.AllowPartialResults, err = getBoolEnv(AllowPartialResultsEnv, true); err != nil { - return - } - } - for i := range s.Projects { - if err = s.Projects[i].setDefaultsIfNeeded(); err != nil { - return - } - } - return -} - -type JFrogPlatform struct { - XrayVersion string - XscVersion string - Watches []string `yaml:"watches,omitempty"` - IncludeVulnerabilities bool `yaml:"includeVulnerabilities,omitempty"` - JFrogProjectKey string `yaml:"jfrogProjectKey,omitempty"` -} - -func (jp *JFrogPlatform) setDefaultsIfNeeded() (err error) { - e := &ErrMissingEnv{} - if len(jp.Watches) == 0 { - if jp.Watches, err = readArrayParamFromEnv(jfrogWatchesEnv, WatchesDelimiter); err != nil && !e.IsMissingEnvErr(err) { - return - } - } - if jp.JFrogProjectKey == "" { - if err = readParamFromEnv(jfrogProjectEnv, &jp.JFrogProjectKey); err != nil && !e.IsMissingEnvErr(err) { - return - } - // We don't want to return an error from this function if the error is of type ErrMissingEnv because JFrogPlatform environment variables are not mandatory. - err = nil - } - if !jp.IncludeVulnerabilities { - if jp.IncludeVulnerabilities, err = getBoolEnv(IncludeVulnerabilitiesEnv, false); err != nil { - return - } - } - return -} - -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"` - PullRequestSecretComments bool `yaml:"pullRequestSecretComments,omitempty"` - EmailAuthor string `yaml:"emailAuthor,omitempty"` - AggregateFixes bool `yaml:"aggregateFixes,omitempty"` - PullRequestDetails vcsclient.PullRequestInfo - RepositoryCloneUrl string - UseLocalRepository bool - UploadSbomToVcs *bool `yaml:"uploadSbomToVcs,omitempty"` -} - -func (g *Git) GetRepositoryHttpsCloneUrl(gitClient vcsclient.VcsClient) (string, error) { - if g.RepositoryCloneUrl != "" { - return g.RepositoryCloneUrl, nil - } - // If the repository clone URL is not cached, we fetch it from the VCS provider - repositoryInfo, err := gitClient.GetRepositoryInfo(context.Background(), g.RepoOwner, g.RepoName) - if err != nil { - return "", fmt.Errorf("failed to fetch the repository clone URL. %s", err.Error()) - } - g.RepositoryCloneUrl = repositoryInfo.CloneInfo.HTTP - return g.RepositoryCloneUrl, nil -} - -func (g *Git) setDefaultsIfNeeded(gitParamsFromEnv *Git, commandName string) (err error) { - g.RepoOwner = gitParamsFromEnv.RepoOwner - g.GitProvider = gitParamsFromEnv.GitProvider - g.VcsInfo = gitParamsFromEnv.VcsInfo - g.PullRequestDetails = gitParamsFromEnv.PullRequestDetails - if g.RepoName == "" { - if gitParamsFromEnv.RepoName == "" { - return fmt.Errorf("repository name is missing. please set the %s environment variable", GitRepoEnv) - } - g.RepoName = gitParamsFromEnv.RepoName - } - if g.EmailAuthor == "" { - g.EmailAuthor = frogbotAuthorEmail - } - if commandName == ScanPullRequest { - if err = g.extractScanPullRequestEnvParams(gitParamsFromEnv); err != nil { - return - } - } - if commandName == ScanRepository { - if err = g.extractScanRepositoryEnvParams(gitParamsFromEnv); err != nil { - return - } - } - - // We don't need to examine gitParamsFromEnv since GitDependencyGraphSubmissionEnv value is not fetched upon gitParamsFromEnv creation - if g.UploadSbomToVcs == nil { - envValue, err := getBoolEnv(GitDependencyGraphSubmissionEnv, true) - if err != nil { - return err - } - g.UploadSbomToVcs = &envValue - } - - return -} - -func (g *Git) extractScanPullRequestEnvParams(gitParamsFromEnv *Git) (err error) { - // The Pull Request ID is a mandatory requirement for Frogbot to properly identify and scan the relevant pull request - if gitParamsFromEnv.PullRequestDetails.ID == 0 { - return errors.New("no Pull Request ID has been provided. Please configure it by using the `JF_GIT_PULL_REQUEST_ID` environment variable") - } - if !g.PullRequestSecretComments { - if g.PullRequestSecretComments, err = getBoolEnv(PullRequestSecretCommentsEnv, false); err != nil { - return - } - } - - return -} - -func (g *Git) extractScanRepositoryEnvParams(gitParamsFromEnv *Git) (err error) { - // Continue to extract ScanRepository related env params - noBranchesProvidedViaConfig := len(g.Branches) == 0 - noBranchesProvidedViaEnv := len(gitParamsFromEnv.Branches) == 0 - if noBranchesProvidedViaConfig { - if noBranchesProvidedViaEnv { - return errors.New("no branches were provided. Please set your branches using the `JF_GIT_BASE_BRANCH` environment variable") - } - g.Branches = gitParamsFromEnv.Branches - } - if g.BranchNameTemplate == "" { - branchTemplate := getTrimmedEnv(BranchNameTemplateEnv) - if err = validateHashPlaceHolder(branchTemplate); err != nil { - return - } - g.BranchNameTemplate = branchTemplate - } - if g.CommitMessageTemplate == "" { - g.CommitMessageTemplate = getTrimmedEnv(CommitMessageTemplateEnv) - } - if g.PullRequestTitleTemplate == "" { - g.PullRequestTitleTemplate = getTrimmedEnv(PullRequestTitleTemplateEnv) - } - if !g.AggregateFixes { - if g.AggregateFixes, err = getBoolEnv(GitAggregateFixesEnv, false); err != nil { - return - } - } - if !g.UseLocalRepository { - if g.UseLocalRepository, err = getBoolEnv(GitUseLocalRepositoryEnv, false); err != nil { - return - } - } - return -} - -func validateHashPlaceHolder(template string) error { - if template == "" { - return nil - } - if !strings.Contains(template, BranchHashPlaceHolder) { - return fmt.Errorf("branch name template must contain %s, provided: %s", BranchHashPlaceHolder, template) - } - return nil -} - -func GetFrogbotDetails(commandName string) (frogbotDetails *FrogbotDetails, err error) { - // Get server and git details - jfrogServer, err := extractJFrogCredentialsFromEnvs() - if err != nil { - return - } - xrayVersion, xscVersion, err := xsc.GetJfrogServicesVersion(jfrogServer) - if err != nil { - return - } - - gitParamsFromEnv, err := extractGitParamsFromEnvs() - if err != nil { - return - } - - defer func() { - err = errors.Join(err, SanitizeEnv()) - }() - - // Build a version control client for REST API requests - client, err := vcsclient. - NewClientBuilder(gitParamsFromEnv.GitProvider). - ApiEndpoint(strings.TrimSuffix(gitParamsFromEnv.APIEndpoint, "/")). - Token(gitParamsFromEnv.Token). - Project(gitParamsFromEnv.Project). - Logger(log.GetLogger()). - Username(gitParamsFromEnv.Username). - Build() - if err != nil { - return - } - - repository, err := BuildRepository(xrayVersion, xscVersion, client, gitParamsFromEnv, jfrogServer, commandName) - if err != nil { - return - } - - configProfile, repoCloneUrl, err := getConfigProfileIfExistsAndValid(xrayVersion, jfrogServer, client, gitParamsFromEnv) - if err != nil { - return - } - - // We apply the configProfile to the repository. If no config profile was fetched, a nil value is passed - repository.Scan.ConfigProfile = configProfile - repository.Git.RepositoryCloneUrl = repoCloneUrl - - frogbotDetails = &FrogbotDetails{XrayVersion: xrayVersion, XscVersion: xscVersion, Repository: repository, GitClient: client, ServerDetails: jfrogServer, ReleasesRepo: os.Getenv(jfrogReleasesRepoEnv)} - return -} - -// Builds a Repository from environment variables only -// Returns a Repository instance with all the defaults and necessary fields. -func BuildRepository(xrayVersion, xscVersion string, gitClient vcsclient.VcsClient, gitParamsFromEnv *Git, server *coreconfig.ServerDetails, commandName string) (repository Repository, err error) { - // Create a single repository from environment variables - repository = newRepository() - repository.Server = *server - repository.Params.XrayVersion = xrayVersion - repository.Params.XscVersion = xscVersion - if err = repository.Params.setDefaultsIfNeeded(gitParamsFromEnv, commandName); err != nil { - return - } - repository.setOutputWriterDetails() - repository.OutputWriter.SetSizeLimit(gitClient) - return repository, nil -} - -func extractJFrogCredentialsFromEnvs() (*coreconfig.ServerDetails, error) { - server := coreconfig.ServerDetails{} - platformUrl := strings.TrimSuffix(getTrimmedEnv(JFrogUrlEnv), "/") - xrUrl := strings.TrimSuffix(getTrimmedEnv(jfrogXrayUrlEnv), "/") - rtUrl := strings.TrimSuffix(getTrimmedEnv(jfrogArtifactoryUrlEnv), "/") - if xrUrl != "" && rtUrl != "" { - server.XrayUrl = xrUrl + "/" - server.ArtifactoryUrl = rtUrl + "/" - } else { - if platformUrl == "" { - return nil, fmt.Errorf("%s or %s and %s environment variables are missing", JFrogUrlEnv, jfrogXrayUrlEnv, jfrogArtifactoryUrlEnv) - } - server.Url = platformUrl + "/" - server.XrayUrl = platformUrl + "/xray/" - server.ArtifactoryUrl = platformUrl + "/artifactory/" - } - - password := getTrimmedEnv(JFrogPasswordEnv) - user := getTrimmedEnv(JFrogUserEnv) - if password != "" && user != "" { - server.User = user - server.Password = password - } else if accessToken := getTrimmedEnv(JFrogTokenEnv); accessToken != "" { - server.AccessToken = accessToken - } else { - return nil, fmt.Errorf("%s and %s or %s environment variables are missing", JFrogUserEnv, JFrogPasswordEnv, JFrogTokenEnv) - } - return &server, nil -} - -func extractGitParamsFromEnvs() (*Git, error) { - e := &ErrMissingEnv{} - var err error - gitEnvParams := &Git{} - // Branch & Repo names are mandatory variables. - // Must be set as environment variables. - // Validation performed later - // Set the base branch name - var branch string - if err = readParamFromEnv(GitBaseBranchEnv, &branch); err != nil && !e.IsMissingEnvErr(err) { - return nil, err - } - if branch != "" { - gitEnvParams.Branches = []string{branch} - } - // Non-mandatory Git Api Endpoint, if not set, default values will be used. - if err = readParamFromEnv(GitApiEndpointEnv, &gitEnvParams.APIEndpoint); err != nil && !e.IsMissingEnvErr(err) { - return nil, err - } - if err = verifyValidApiEndpoint(gitEnvParams.APIEndpoint); err != nil { - return nil, err - } - // [Mandatory] Set the Git provider - if gitEnvParams.GitProvider, err = extractVcsProviderFromEnv(); err != nil { - return nil, err - } - // [Mandatory] Set the git repository owner name (organization) - if err = readParamFromEnv(GitRepoOwnerEnv, &gitEnvParams.RepoOwner); err != nil { - return nil, err - } - // [Mandatory] Set the access token to the git provider - if err = readParamFromEnv(GitTokenEnv, &gitEnvParams.Token); err != nil { - return nil, err - } - - // [Mandatory] Set the repository name, except for multi repository. - if err = readParamFromEnv(GitRepoEnv, &gitEnvParams.RepoName); err != nil { - return nil, err - } - - // Set Bitbucket Server username - // Mandatory only for Bitbucket Server, this authentication detail is required for performing git operations. - if err = readParamFromEnv(GitUsernameEnv, &gitEnvParams.Username); err != nil && !e.IsMissingEnvErr(err) { - return nil, err - } - // Set Azure Repos Project name - // Mandatory for Azure Repos only - if err = readParamFromEnv(GitProjectEnv, &gitEnvParams.Project); err != nil && gitEnvParams.GitProvider == vcsutils.AzureRepos { - return nil, err - } - if envPrId := getTrimmedEnv(GitPullRequestIDEnv); envPrId != "" { - var convertedPrId int - if convertedPrId, err = strconv.Atoi(envPrId); err != nil { - return nil, fmt.Errorf("failed parsing %s environment variable as a number. The received environment is : %s", GitPullRequestIDEnv, envPrId) - } - gitEnvParams.PullRequestDetails = vcsclient.PullRequestInfo{ID: int64(convertedPrId)} - } - - return gitEnvParams, nil -} - -func verifyValidApiEndpoint(apiEndpoint string) error { - // Empty string will resolve to default values. - if apiEndpoint == "" { - return nil - } - parsedUrl, err := url.Parse(apiEndpoint) - if err != nil { - return err - } - if parsedUrl.Scheme == "" { - return errors.New("the given API endpoint is invalid. Please note that the API endpoint format should be provided with the 'HTTPS' protocol as a prefix") - } - return nil -} - -func readArrayParamFromEnv(envKey, delimiter string) ([]string, error) { - var envValue string - var err error - e := &ErrMissingEnv{} - if err = readParamFromEnv(envKey, &envValue); err != nil && !e.IsMissingEnvErr(err) { - return nil, err - } - if envValue == "" { - return nil, &ErrMissingEnv{VariableName: envKey} - } - // Remove spaces if exists - envValue = strings.ReplaceAll(envValue, " ", "") - return strings.Split(envValue, delimiter), nil -} - -func readParamFromEnv(envKey string, paramValue *string) error { - *paramValue = getTrimmedEnv(envKey) - if *paramValue == "" { - return &ErrMissingEnv{VariableName: envKey} - } - return nil -} - -func getTrimmedEnv(envKey string) string { - return strings.TrimSpace(os.Getenv(envKey)) -} - -func extractVcsProviderFromEnv() (vcsutils.VcsProvider, error) { - vcsProvider := getTrimmedEnv(GitProvider) - switch vcsProvider { - case string(GitHub): - return vcsutils.GitHub, nil - case string(GitLab): - return vcsutils.GitLab, nil - // For backward compatibility, we are accepting also "bitbucket server" - case string(BitbucketServer), "bitbucket server": - return vcsutils.BitbucketServer, nil - case string(AzureRepos): - return vcsutils.AzureRepos, nil - } - return 0, fmt.Errorf("%s should be one of: '%s', '%s', '%s' or '%s'", GitProvider, GitHub, GitLab, BitbucketServer, AzureRepos) -} - -func SanitizeEnv() error { - for _, env := range os.Environ() { - if !strings.HasPrefix(env, "JF_") { - continue - } - envSplit := strings.Split(env, "=") - if err := os.Unsetenv(envSplit[0]); err != nil { - return err - } - } - return nil -} - -func setProjectInstallCommand(installCommand string, project *Project) { - parts := strings.Fields(installCommand) - if len(parts) > 1 { - project.InstallCommandArgs = parts[1:] - } - project.InstallCommandName = parts[0] -} - -func getBoolEnv(envKey string, defaultValue bool) (bool, error) { - envValue := getTrimmedEnv(envKey) - if envValue != "" { - parsedEnv, err := strconv.ParseBool(envValue) - if err != nil { - return false, fmt.Errorf("the value of the %s environment is expected to be either TRUE or FALSE. The value received however is %s", envKey, envValue) - } - return parsedEnv, nil - } - - return defaultValue, nil -} - -// This function attempts to fetch a config profile, we check if there is a config profile associated to the repo URL. -// When a profile is found we verify several conditions on it. -// If a profile was requested but not found by url we return an error. -func getConfigProfileIfExistsAndValid(xrayVersion string, jfrogServer *coreconfig.ServerDetails, gitClient vcsclient.VcsClient, gitParams *Git) (configProfile *services.ConfigProfile, repoCloneUrl string, err error) { - if err = clientutils.ValidateMinimumVersion(clientutils.Xray, xrayVersion, services.ConfigProfileNewSchemaMinXrayVersion); err != nil { - log.Info(fmt.Sprintf("The utilized Frogbot version requires a higher version of Xray than %s in order to use Config Profile. Please upgrade Xray to version %s and above or downgrade Frogbot to prior versions", xrayVersion, services.ConfigProfileNewSchemaMinXrayVersion)) - return - } - - // Getting repository's url in order to get repository HTTP url - if repoCloneUrl, err = gitParams.GetRepositoryHttpsCloneUrl(gitClient); err != nil { - return - } - // Attempt to get a config profile associated with the repo URL - log.Debug(fmt.Sprintf("Configuration profile was requested. Searching profile associated to repository '%s'", jfrogServer.Url)) - if configProfile, err = xsc.GetConfigProfileByUrl(xrayVersion, jfrogServer, repoCloneUrl); err != nil || configProfile == nil { - return - } - err = verifyConfigProfileValidity(configProfile) - return -} - -func verifyConfigProfileValidity(configProfile *services.ConfigProfile) (err error) { - // Currently, only a single Module that represents the entire project is supported - if len(configProfile.Modules) != 1 { - err = fmt.Errorf("more than one module was found '%s' profile. Frogbot currently supports only one module per config profile", configProfile.ProfileName) - return - } - if configProfile.Modules[0].PathFromRoot != "." { - err = fmt.Errorf("module '%s' in profile '%s' contains the following path from root: '%s'. Frogbot currently supports only a single module with a '.' path from root", configProfile.Modules[0].ModuleName, configProfile.ProfileName, configProfile.Modules[0].PathFromRoot) - return - } - log.Info(fmt.Sprintf("Using Config profile '%s'. jfrog-apps-config will be ignored if exists", configProfile.ProfileName)) - return -} diff --git a/utils/scandetails.go b/utils/scandetails.go index 3ff0e9b57..7f6912917 100644 --- a/utils/scandetails.go +++ b/utils/scandetails.go @@ -13,26 +13,20 @@ import ( "github.com/jfrog/jfrog-cli-security/sca/bom/xrayplugin" "github.com/jfrog/jfrog-cli-security/sca/scan/enrich" "github.com/jfrog/jfrog-cli-security/utils/results" - "github.com/jfrog/jfrog-cli-security/utils/severityutils" "github.com/jfrog/jfrog-client-go/utils/log" xscservices "github.com/jfrog/jfrog-client-go/xsc/services" ) type ScanDetails struct { - *Project *Git *xscservices.XscGitInfoContext *config.ServerDetails - client vcsclient.VcsClient - fixableOnly bool - minSeverityFilter severityutils.Severity - baseBranch string - configProfile *xscservices.ConfigProfile - allowPartialResults bool - + client vcsclient.VcsClient + baseBranch string diffScan bool ResultsToCompare *results.SecurityCommandResults + configProfile *xscservices.ConfigProfile results.ResultContext MultiScanId string @@ -61,35 +55,8 @@ func (sc *ScanDetails) SetResultsToCompare(results *results.SecurityCommandResul return sc } -func (sc *ScanDetails) SetProject(project *Project) *ScanDetails { - sc.Project = project - return sc -} - -func (sc *ScanDetails) SetResultsContext(httpCloneUrl string, watches []string, jfrogProjectKey string, includeVulnerabilities, includeLicenses bool) *ScanDetails { - sc.ResultContext = audit.CreateAuditResultsContext(sc.ServerDetails, sc.XrayVersion, watches, sc.RepoPath, jfrogProjectKey, httpCloneUrl, includeVulnerabilities, includeLicenses, false) - return sc -} - -func (sc *ScanDetails) SetFixableOnly(fixable bool) *ScanDetails { - sc.fixableOnly = fixable - return sc -} - -func (sc *ScanDetails) SetMinSeverity(minSeverity string) (*ScanDetails, error) { - if minSeverity == "" { - return sc, nil - } - if severity, err := severityutils.ParseSeverity(minSeverity, false); err != nil { - return sc, err - } else { - sc.minSeverityFilter = severity - } - return sc, nil -} - -func (sc *ScanDetails) SetAllowPartialResults(allowPartialResults bool) *ScanDetails { - sc.allowPartialResults = allowPartialResults +func (sc *ScanDetails) SetResultsContext(httpCloneUrl string, jfrogProjectKey string, includeVulnerabilities bool) *ScanDetails { + sc.ResultContext = audit.CreateAuditResultsContext(sc.ServerDetails, sc.XrayVersion, []string{}, sc.RepoPath, jfrogProjectKey, httpCloneUrl, includeVulnerabilities, true, false) return sc } @@ -111,14 +78,6 @@ func (sc *ScanDetails) BaseBranch() string { return sc.baseBranch } -func (sc *ScanDetails) FixableOnly() bool { - return sc.fixableOnly -} - -func (sc *ScanDetails) MinSeverityFilter() severityutils.Severity { - return sc.minSeverityFilter -} - func (sc *ScanDetails) SetRepoOwner(owner string) *ScanDetails { sc.RepoOwner = owner return sc @@ -129,25 +88,13 @@ func (sc *ScanDetails) SetRepoName(repoName string) *ScanDetails { return sc } -func (sc *ScanDetails) AllowPartialResults() bool { - return sc.allowPartialResults -} - -func (sc *ScanDetails) RunInstallAndAudit(workDirs ...string) (auditResults *results.SecurityCommandResults) { +func (sc *ScanDetails) Audit(workDirs ...string) (auditResults *results.SecurityCommandResults) { auditBasicParams := (&audit.AuditBasicParams{}). SetXrayVersion(sc.XrayVersion). SetXscVersion(sc.XscVersion). - SetPipRequirementsFile(sc.PipRequirementsFile). - SetUseWrapper(*sc.UseWrapper). - SetMaxTreeDepth(sc.MaxPnpmTreeDepth). - SetDepsRepo(sc.DepsRepo). - SetIgnoreConfigFile(true). SetServerDetails(sc.ServerDetails). - SetInstallCommandName(sc.InstallCommandName). - SetInstallCommandArgs(sc.InstallCommandArgs). - SetTechnologies(sc.GetTechFromInstallCmdIfExists()). - SetAllowPartialResults(sc.allowPartialResults). - SetExclusions(sc.PathExclusions). + SetAllowPartialResults(!sc.configProfile.GeneralConfig.FailUponAnyScannerError). + SetExclusions(sc.configProfile.GeneralConfig.GeneralExcludePatterns). SetUseJas(true). SetConfigProfile(sc.configProfile) @@ -158,8 +105,6 @@ func (sc *ScanDetails) RunInstallAndAudit(workDirs ...string) (auditResults *res SetGitContext(sc.XscGitInfoContext). SetRtResultRepository(frogbotUploadRtRepoPath). SetWorkingDirs(workDirs). - SetMinSeverityFilter(sc.MinSeverityFilter()). - SetFixableOnly(sc.FixableOnly()). SetGraphBasicParams(auditBasicParams). SetResultsContext(sc.ResultContext). SetDiffMode(sc.diffScan). diff --git a/utils/testsutils.go b/utils/testsutils.go index f957eec71..a9dea02f1 100644 --- a/utils/testsutils.go +++ b/utils/testsutils.go @@ -26,7 +26,6 @@ import ( ) const ( - ValidConfigProfile = "default-profile" InvalidPathConfigProfile = "invalid-path-from-root-profile" InvalidModulesConfigProfile = "invalid-modules-profile" NonExistingProfile = "non-existing-profile" @@ -113,11 +112,10 @@ func VerifyEnv(t *testing.T) (server config.ServerDetails, restoreFunc func()) { server.AccessToken = token restoreFunc = func() { SetEnvAndAssert(t, map[string]string{ - JFrogUrlEnv: url, - JFrogTokenEnv: token, - JFrogUserEnv: username, - JFrogPasswordEnv: password, - GitAggregateFixesEnv: "FALSE", + JFrogUrlEnv: url, + JFrogTokenEnv: token, + JFrogUserEnv: username, + JFrogPasswordEnv: password, }) } return @@ -156,12 +154,10 @@ func CreateXscMockServerForConfigProfile(t *testing.T, xrayVersion string) (mock } secondModule := services.Module{ - ModuleId: 999, - ModuleName: "second-module", - PathFromRoot: ".", - ExcludePatterns: nil, - ScanConfig: services.ScanConfig{}, - DepsRepo: "", + ModuleId: 999, + ModuleName: "second-module", + PathFromRoot: ".", + ScanConfig: services.ScanConfig{}, } switch { diff --git a/utils/utils.go b/utils/utils.go index 50d32b498..e9225be70 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -188,7 +188,7 @@ func VulnerabilityDetailsToMD5Hash(vulnerabilities ...formats.VulnerabilityOrVio } func UploadSarifResultsToGithubSecurityTab(scanResults *results.SecurityCommandResults, repo *Repository, branch string, client vcsclient.VcsClient) error { - report, err := GenerateFrogbotSarifReport(scanResults, repo.AllowedLicenses) + report, err := GenerateFrogbotSarifReport(scanResults) if err != nil { return err } @@ -237,7 +237,7 @@ func UploadSbomSnapshotToGithubDependencyGraph(owner, repo string, scanResults * return nil } -func GenerateFrogbotSarifReport(extendedResults *results.SecurityCommandResults, allowedLicenses []string) (string, error) { +func GenerateFrogbotSarifReport(extendedResults *results.SecurityCommandResults) (string, error) { convertor := conversion.NewCommandResultsConvertor(conversion.ResultConvertParams{ IncludeVulnerabilities: extendedResults.IncludesVulnerabilities(), HasViolationContext: extendedResults.HasViolationContext(), @@ -453,9 +453,10 @@ func isUrlAccessible(url string) bool { return resp != nil && resp.StatusCode == http.StatusOK } -// This function checks if partial results are allowed by the user. If so instead of returning an error we log the error and continue as if we didn't have an error -func CreateErrorIfPartialResultsDisabled(allowPartial bool, messageForLog string, err error) error { - if allowPartial { +// CreateErrorIfFailUponScannerErrorEnabled This function checks if fail upn scanner error configuration is enabled by the user. +// If not - instead of returning an error we log the error and continue as if we didn't have an error +func CreateErrorIfFailUponScannerErrorEnabled(fail bool, messageForLog string, err error) error { + if !fail { log.Warn(messageForLog) return nil }