diff --git a/integrationutils.go b/integrationutils.go index d30230e36..a6dd4f5f4 100644 --- a/integrationutils.go +++ b/integrationutils.go @@ -3,6 +3,12 @@ package main import ( "context" "fmt" + "os" + "strconv" + "strings" + "testing" + "time" + "github.com/go-git/go-git/v5" githttp "github.com/go-git/go-git/v5/plumbing/transport/http" "github.com/jfrog/frogbot/v2/scanpullrequest" @@ -13,11 +19,6 @@ import ( "github.com/jfrog/froggit-go/vcsutils" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "os" - "strconv" - "strings" - "testing" - "time" ) const ( @@ -171,8 +172,8 @@ func runScanPullRequestCmd(t *testing.T, client vcsclient.VcsClient, testDetails defer unsetEnvs() err = Exec(&scanpullrequest.ScanPullRequestCmd{}, utils.ScanPullRequest) - // Validate that issues were found and the relevant error returned - require.Errorf(t, err, scanpullrequest.SecurityIssueFoundErr) + // Validate that no error is returned + require.NoError(t, err) validateResults(t, ctx, client, testDetails, prId) } diff --git a/scanpullrequest/scanpullrequest.go b/scanpullrequest/scanpullrequest.go index c51ee2653..52ea18769 100644 --- a/scanpullrequest/scanpullrequest.go +++ b/scanpullrequest/scanpullrequest.go @@ -24,8 +24,7 @@ import ( const ( SecurityIssueFoundErr = "issues were detected by Frogbot\n" + - "You can avoid marking the Frogbot scan as failed by setting the " + utils.FailOnSecurityIssuesEnv + " environment variable to false\n" + - "Note that even if " + utils.FailOnSecurityIssuesEnv + " is set to false, but a security violation with 'fail-pull-request' rule is found, Frogbot scan will fail as well" + "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" @@ -81,7 +80,7 @@ func verifyGitHubFrogbotEnvironment(client vcsclient.VcsClient, repoConfig *util return nil } -// By default, includeAllVulnerabilities is set to false and the scan goes as follows: +// 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. @@ -104,25 +103,14 @@ func scanPullRequest(repo *utils.Repository, client vcsclient.VcsClient) (err er return } - // Fail the Frogbot task if a security issue is found and Frogbot isn't configured to avoid the failure. - if toFailTaskStatus(repo, issues) { + // Fail the Frogbot task if a security violation is found and fail pr rule applied. + if issues.IsFailPrRuleApplied() { err = errors.New(SecurityIssueFoundErr) return } return } -func toFailTaskStatus(repo *utils.Repository, issues *issues.ScansIssuesCollection) bool { - failFlagSet := repo.Params.Scan.FailOnSecurityIssues != nil && *repo.Params.Scan.FailOnSecurityIssues - if failFlagSet { - // If the fail flag is set to true (JF_FAIL), we check if any security ISSUE exists (not just violations), and if so, we fail the build. - return issues.IssuesExists(repo.Params.Git.PullRequestSecretComments) - } else { - // When fail flag is set to false, we check for fail-pr rule in existing VIOLATIONS. If one exists, we fail the build as well. - return issues.IsFailPrRuleApplied() - } -} - func auditPullRequestAndReport(repoConfig *utils.Repository, client vcsclient.VcsClient) (issuesCollection *issues.ScansIssuesCollection, resultContext results.ResultContext, err error) { // Prepare scanDetails, err := createBaseScanDetails(repoConfig, client) @@ -171,7 +159,6 @@ func createBaseScanDetails(repoConfig *utils.Repository, client vcsclient.VcsCli SetFixableOnly(repoConfig.Params.Scan.FixableOnly). SetConfigProfile(repoConfig.Params.Scan.ConfigProfile). SetSkipAutoInstall(repoConfig.Params.Scan.SkipAutoInstall). - SetDisableJas(repoConfig.Params.Scan.DisableJas). SetXscPRGitInfoContext(repoConfig.Params.Git.Project, client, repoConfig.Params.Git.PullRequestDetails). SetDiffScan(!repoConfig.Params.JFrogPlatform.IncludeVulnerabilities). SetAllowPartialResults(repoConfig.Params.Scan.AllowPartialResults) @@ -212,15 +199,13 @@ func auditPullRequestCode(repoConfig *utils.Repository, scanDetails *utils.ScanD // Reset scan details for each project scanDetails.SetProject(&repoConfig.Params.Scan.Projects[i]).SetResultsToCompare(nil) // Scan target branch of the project - if !repoConfig.Params.JFrogPlatform.IncludeVulnerabilities { - 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) - } + 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...") @@ -282,7 +267,6 @@ func filterOutFailedScansIfAllowPartialResultsEnabled(targetResults, sourceResul return nil } if targetResults == nil { - // If IncludeAllVulnerabilities is applied, only sourceResults exists and we don't need to filter anything - we present results we have return nil } @@ -403,7 +387,6 @@ func filterOutScaResultsIfScanFailed(targetResult, sourceResult *results.TargetR // Sorts the Targets slice in both targetResults and sourceResults // by the physical location (Target field) of each scan target in ascending order. func sortTargetsByPhysicalLocation(targetResults, sourceResults *results.SecurityCommandResults) error { - // If !IncludeAllVulnerabilities we expect targetResults and sourceResults to be non-empty and to have the same amount of targets. if len(targetResults.Targets) != len(sourceResults.Targets) { return fmt.Errorf("amount of targets in target results is different than source results: %d vs %d", len(targetResults.Targets), len(sourceResults.Targets)) } diff --git a/scanpullrequest/scanpullrequest_test.go b/scanpullrequest/scanpullrequest_test.go index 234e4a4fd..e3b1819e1 100644 --- a/scanpullrequest/scanpullrequest_test.go +++ b/scanpullrequest/scanpullrequest_test.go @@ -220,60 +220,49 @@ func TestScanResultsToIssuesCollection(t *testing.T) { func TestScanPullRequest(t *testing.T) { tests := []struct { - testName string - projectName string - failOnSecurityIssues bool + testName string + projectName string }{ { - testName: "ScanPullRequest", - projectName: "test-proj", - failOnSecurityIssues: true, + testName: "ScanPullRequest", + projectName: "test-proj", }, { - testName: "ScanPullRequestNoFail", - projectName: "test-proj", - failOnSecurityIssues: false, + testName: "ScanPullRequestNoFail", + projectName: "test-proj", }, { - testName: "ScanPullRequestSubdir", - projectName: "test-proj-subdir", - failOnSecurityIssues: true, + testName: "ScanPullRequestSubdir", + projectName: "test-proj-subdir", }, { - testName: "ScanPullRequestNoIssues", - projectName: "clean-test-proj", - failOnSecurityIssues: false, + testName: "ScanPullRequestNoIssues", + projectName: "clean-test-proj", }, { - testName: "ScanPullRequestMultiWorkDir", - projectName: "multi-dir-test-proj", - failOnSecurityIssues: false, + testName: "ScanPullRequestMultiWorkDir", + projectName: "multi-dir-test-proj", }, { - testName: "ScanPullRequestMultiWorkDirNoFail", - projectName: "multi-dir-test-proj", - failOnSecurityIssues: true, + testName: "ScanPullRequestMultiWorkDirNoFail", + projectName: "multi-dir-test-proj", }, } for _, test := range tests { t.Run(test.testName, func(t *testing.T) { - testScanPullRequest(t, test.projectName, test.failOnSecurityIssues) + testScanPullRequest(t, test.projectName) }) } } -func testScanPullRequest(t *testing.T, projectName string, failOnSecurityIssues bool) { - config, client, cleanUp := preparePullRequestTest(t, projectName, failOnSecurityIssues) +func testScanPullRequest(t *testing.T, projectName string) { + config, client, cleanUp := preparePullRequestTest(t, projectName) defer cleanUp() // Run "frogbot scan pull request" var scanPullRequest ScanPullRequestCmd err := scanPullRequest.Run(config, client, utils.MockHasConnection()) - if failOnSecurityIssues { - assert.EqualErrorf(t, err, SecurityIssueFoundErr, "Error should be: %v, got: %v", SecurityIssueFoundErr, err) - } else { - assert.NoError(t, err) - } + assert.NoError(t, err) // Check env sanitize err = utils.SanitizeEnv() @@ -492,7 +481,7 @@ func TestAuditDiffInPullRequest(t *testing.T) { for _, test := range tests { t.Run(test.testName, func(t *testing.T) { - repoConfig, client, cleanUpTest := preparePullRequestTest(t, test.projectName, false) + repoConfig, client, cleanUpTest := preparePullRequestTest(t, test.projectName) defer cleanUpTest() issuesCollection, _, err := auditPullRequestAndReport(&repoConfig, client) @@ -514,7 +503,7 @@ func TestToFailTaskStatus(t *testing.T) { failureExpected bool }{ { - name: "fail flag set to false and no violations with fail_pr", + name: "no violations with fail_pr", setFailFlag: false, issuesCollection: issues.ScansIssuesCollection{ LicensesViolations: []formats.LicenseViolationRow{{ @@ -541,74 +530,7 @@ func TestToFailTaskStatus(t *testing.T) { failureExpected: false, }, { - name: "fail flag set to true, sca vulnerability", - setFailFlag: true, - issuesCollection: issues.ScansIssuesCollection{ - ScaVulnerabilities: []formats.VulnerabilityOrViolationRow{ - { - ImpactedDependencyDetails: formats.ImpactedDependencyDetails{ - ImpactedDependencyName: "impacted-name", - ImpactedDependencyVersion: "1.0.0", - SeverityDetails: formats.SeverityDetails{Severity: "High"}, - Components: []formats.ComponentRow{ - { - Name: "vuln-pack-name1", - Version: "1.0.0", - }, - { - Name: "vuln-pack-name1", - Version: "1.2.3", - }, - { - Name: "vuln-pack-name2", - Version: "1.2.3", - }, - }, - }, - Cves: []formats.CveRow{{ - Id: "CVE-2021-1234", - Applicability: &formats.Applicability{ - Status: "Applicable", - ScannerDescription: "scanner", - Evidence: []formats.Evidence{ - {Reason: "reason", Location: formats.Location{File: "file1", StartLine: 1, StartColumn: 2, EndLine: 3, EndColumn: 4, Snippet: "snippet1"}}, - {Reason: "other reason", Location: formats.Location{File: "file2", StartLine: 5, StartColumn: 6, EndLine: 7, EndColumn: 8, Snippet: "snippet2"}}, - }, - }, - }}, - JfrogResearchInformation: &formats.JfrogResearchInformation{ - Remediation: "remediation", - }, - Summary: "summary", - Applicable: "Applicable", - IssueId: "Xray-Id", - }, - { - ImpactedDependencyDetails: formats.ImpactedDependencyDetails{ - ImpactedDependencyName: "impacted-name2", - ImpactedDependencyVersion: "1.0.0", - SeverityDetails: formats.SeverityDetails{Severity: "Low"}, - Components: []formats.ComponentRow{ - { - Name: "vuln-pack-name3", - Version: "1.0.0", - }, - }, - }, - Cves: []formats.CveRow{{ - Id: "CVE-1111-2222", - Applicability: &formats.Applicability{Status: "Not Applicable"}, - }}, - Summary: "other summary", - Applicable: "Not Applicable", - IssueId: "Xray-Id2", - }, - }, - }, - failureExpected: true, - }, - { - name: "fail flag is set to false, fail_pr in licenses violation", + name: "fail_pr in licenses violation", setFailFlag: false, issuesCollection: issues.ScansIssuesCollection{ LicensesViolations: []formats.LicenseViolationRow{{ @@ -638,19 +560,7 @@ func TestToFailTaskStatus(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - failFlag := test.setFailFlag - repo := &utils.Repository{ - Params: utils.Params{ - Scan: utils.Scan{ - FailOnSecurityIssues: &failFlag, - }, - Git: utils.Git{ - PullRequestSecretComments: false, - }, - }, - } - - assert.Equal(t, test.failureExpected, toFailTaskStatus(repo, &test.issuesCollection)) + assert.Equal(t, test.failureExpected, test.issuesCollection.IsFailPrRuleApplied()) }) } } @@ -1138,14 +1048,11 @@ func TestFilterOutFailedScansIfAllowPartialResultsEnabled(t *testing.T) { } } -func preparePullRequestTest(t *testing.T, projectName string, failOnSecurityIssues bool) (utils.Repository, vcsclient.VcsClient, func()) { +func preparePullRequestTest(t *testing.T, projectName string) (utils.Repository, vcsclient.VcsClient, func()) { params, restoreEnv := utils.VerifyEnv(t) // Set test-specific environment variables envVars := map[string]string{} - if !failOnSecurityIssues { - envVars[utils.FailOnSecurityIssuesEnv] = "false" - } // Set working directories for multi-dir tests if projectName == "multi-dir-test-proj" { diff --git a/scanrepository/scanrepository.go b/scanrepository/scanrepository.go index 6c3d01ccc..d57431544 100644 --- a/scanrepository/scanrepository.go +++ b/scanrepository/scanrepository.go @@ -132,8 +132,7 @@ func (cfp *ScanRepositoryCmd) setCommandPrerequisites(repository *utils.Reposito SetFixableOnly(repository.Params.Scan.FixableOnly). SetConfigProfile(repository.Params.Scan.ConfigProfile). SetSkipAutoInstall(repository.Params.Scan.SkipAutoInstall). - SetAllowPartialResults(repository.Params.Scan.AllowPartialResults). - SetDisableJas(repository.Params.Scan.DisableJas) + SetAllowPartialResults(repository.Params.Scan.AllowPartialResults) if cfp.scanDetails, err = cfp.scanDetails.SetMinSeverity(repository.Params.Scan.MinSeverity); err != nil { return diff --git a/testdata/messages/summarycomment/structure/fix_mr_entitled.md b/testdata/messages/summarycomment/structure/fix_mr_entitled.md index 9dc24f0ff..fa70861d4 100644 --- a/testdata/messages/summarycomment/structure/fix_mr_entitled.md +++ b/testdata/messages/summarycomment/structure/fix_mr_entitled.md @@ -13,7 +13,6 @@ some content ``` - ---
diff --git a/testdata/messages/summarycomment/structure/fix_pr_entitled.md b/testdata/messages/summarycomment/structure/fix_pr_entitled.md index 9776c5958..bfedf5267 100644 --- a/testdata/messages/summarycomment/structure/fix_pr_entitled.md +++ b/testdata/messages/summarycomment/structure/fix_pr_entitled.md @@ -13,7 +13,6 @@ some content ``` - ---
diff --git a/testdata/messages/summarycomment/structure/fix_simplified_entitled.md b/testdata/messages/summarycomment/structure/fix_simplified_entitled.md index 0df981633..281624be8 100644 --- a/testdata/messages/summarycomment/structure/fix_simplified_entitled.md +++ b/testdata/messages/summarycomment/structure/fix_simplified_entitled.md @@ -8,6 +8,5 @@ some content ``` - --- [🐸 JFrog Frogbot](https://jfrog.com/help/r/jfrog-security-user-guide/shift-left-on-security/frogbot) \ No newline at end of file diff --git a/testdata/messages/summarycomment/structure/summary_comment_issues_mr_entitled.md b/testdata/messages/summarycomment/structure/summary_comment_issues_mr_entitled.md index 794c9e470..340abb1fa 100644 --- a/testdata/messages/summarycomment/structure/summary_comment_issues_mr_entitled.md +++ b/testdata/messages/summarycomment/structure/summary_comment_issues_mr_entitled.md @@ -13,7 +13,6 @@ some content ``` - ---
diff --git a/testdata/messages/summarycomment/structure/summary_comment_issues_mr_entitled_with_title.md b/testdata/messages/summarycomment/structure/summary_comment_issues_mr_entitled_with_title.md index ca58b0623..973de21a6 100644 --- a/testdata/messages/summarycomment/structure/summary_comment_issues_mr_entitled_with_title.md +++ b/testdata/messages/summarycomment/structure/summary_comment_issues_mr_entitled_with_title.md @@ -14,7 +14,6 @@ some content ``` - ---
diff --git a/testdata/messages/summarycomment/structure/summary_comment_issues_pr_entitled.md b/testdata/messages/summarycomment/structure/summary_comment_issues_pr_entitled.md index 27871fc44..cdfd55e5f 100644 --- a/testdata/messages/summarycomment/structure/summary_comment_issues_pr_entitled.md +++ b/testdata/messages/summarycomment/structure/summary_comment_issues_pr_entitled.md @@ -13,7 +13,6 @@ some content ``` - ---
diff --git a/testdata/messages/summarycomment/structure/summary_comment_issues_simplified_entitled.md b/testdata/messages/summarycomment/structure/summary_comment_issues_simplified_entitled.md index e81ee1d6c..cd0c28ce0 100644 --- a/testdata/messages/summarycomment/structure/summary_comment_issues_simplified_entitled.md +++ b/testdata/messages/summarycomment/structure/summary_comment_issues_simplified_entitled.md @@ -8,6 +8,5 @@ some content ``` - --- [🐸 JFrog Frogbot](https://jfrog.com/help/r/jfrog-security-user-guide/shift-left-on-security/frogbot) \ No newline at end of file diff --git a/testdata/messages/summarycomment/structure/summary_comment_issues_simplified_entitled_with_title.md b/testdata/messages/summarycomment/structure/summary_comment_issues_simplified_entitled_with_title.md index a37ddbf86..8b968284b 100644 --- a/testdata/messages/summarycomment/structure/summary_comment_issues_simplified_entitled_with_title.md +++ b/testdata/messages/summarycomment/structure/summary_comment_issues_simplified_entitled_with_title.md @@ -13,6 +13,5 @@ some content ``` - --- [🐸 JFrog Frogbot](https://jfrog.com/help/r/jfrog-security-user-guide/shift-left-on-security/frogbot) \ No newline at end of file diff --git a/testdata/messages/summarycomment/structure/summary_comment_no_issues_mr_entitled.md b/testdata/messages/summarycomment/structure/summary_comment_no_issues_mr_entitled.md index 7668951bf..4d2db540f 100644 --- a/testdata/messages/summarycomment/structure/summary_comment_no_issues_mr_entitled.md +++ b/testdata/messages/summarycomment/structure/summary_comment_no_issues_mr_entitled.md @@ -9,7 +9,6 @@
- ---
diff --git a/testdata/messages/summarycomment/structure/summary_comment_no_issues_pr_entitled.md b/testdata/messages/summarycomment/structure/summary_comment_no_issues_pr_entitled.md index a58195da0..dfa96ad45 100644 --- a/testdata/messages/summarycomment/structure/summary_comment_no_issues_pr_entitled.md +++ b/testdata/messages/summarycomment/structure/summary_comment_no_issues_pr_entitled.md @@ -9,7 +9,6 @@
- ---
diff --git a/testdata/messages/summarycomment/structure/summary_comment_no_issues_pr_entitled_with_title.md b/testdata/messages/summarycomment/structure/summary_comment_no_issues_pr_entitled_with_title.md index 1d25ceb52..8aa6ef929 100644 --- a/testdata/messages/summarycomment/structure/summary_comment_no_issues_pr_entitled_with_title.md +++ b/testdata/messages/summarycomment/structure/summary_comment_no_issues_pr_entitled_with_title.md @@ -10,7 +10,6 @@ ## **Custom title** - ---
diff --git a/testdata/messages/summarycomment/structure/summary_comment_no_issues_simplified_entitled.md b/testdata/messages/summarycomment/structure/summary_comment_no_issues_simplified_entitled.md index a7c3380b8..eb34f9dcb 100644 --- a/testdata/messages/summarycomment/structure/summary_comment_no_issues_simplified_entitled.md +++ b/testdata/messages/summarycomment/structure/summary_comment_no_issues_simplified_entitled.md @@ -4,6 +4,5 @@ **👍 Frogbot scanned this pull request and did not find any new security issues.** - --- [🐸 JFrog Frogbot](https://jfrog.com/help/r/jfrog-security-user-guide/shift-left-on-security/frogbot) \ No newline at end of file diff --git a/testdata/scanpullrequest/expected_response.md b/testdata/scanpullrequest/expected_response.md index 9b6750fc1..258fdc5db 100644 --- a/testdata/scanpullrequest/expected_response.md +++ b/testdata/scanpullrequest/expected_response.md @@ -70,7 +70,6 @@ This vulnerability can be triggered when the attacker-controlled input is parsed Add the `Object.freeze(Object.prototype);` directive once at the beginning of your main JS source code file (ex. `index.js`), preferably after all your `require` directives. This will prevent any changes to the prototype object, thus completely negating prototype pollution attacks. - ---
diff --git a/testdata/scanpullrequest/expected_response_multi_dir.md b/testdata/scanpullrequest/expected_response_multi_dir.md index aac807de5..be339b676 100644 --- a/testdata/scanpullrequest/expected_response_multi_dir.md +++ b/testdata/scanpullrequest/expected_response_multi_dir.md @@ -129,7 +129,6 @@ The vulnerability was disputed (and never fixed) since the maintainers claim tha A vulnerability was found in the minimatch package. This flaw allows a Regular Expression Denial of Service (ReDoS) when calling the braceExpand function with specific arguments, resulting in a Denial of Service.
- ---
diff --git a/utils/comment.go b/utils/comment.go index 6c1563a11..f34a551c5 100644 --- a/utils/comment.go +++ b/utils/comment.go @@ -35,16 +35,14 @@ const ( // In Scan PR, if there are no issues, comments will be added to the PR with a message that there are no issues. func HandlePullRequestCommentsAfterScan(issues *issues.ScansIssuesCollection, resultContext results.ResultContext, repo *Repository, client vcsclient.VcsClient, pullRequestID int) (err error) { - if !repo.Params.AvoidPreviousPrCommentsDeletion { - // The removal of comments may fail for various reasons, - // such as concurrent scanning of pull requests and attempts - // to delete comments that have already been removed in a different process. - // Since this task is not mandatory for a Frogbot run, - // we will not cause a Frogbot run to fail but will instead log the error. - log.Debug("Looking for an existing Frogbot pull request comment. Deleting it if it exists...") - if e := DeletePullRequestComments(repo, client, pullRequestID); e != nil { - log.Error(fmt.Sprintf("%s:\n%v", commentRemovalErrorMsg, e)) - } + // The removal of comments may fail for various reasons, + // such as concurrent scanning of pull requests and attempts + // to delete comments that have already been removed in a different process. + // Since this task is not mandatory for a Frogbot run, + // we will not cause a Frogbot run to fail but will instead log the error. + log.Debug("Looking for an existing Frogbot pull request comment. Deleting it if it exists...") + if e := DeletePullRequestComments(repo, client, pullRequestID); e != nil { + log.Error(fmt.Sprintf("%s:\n%v", commentRemovalErrorMsg, e)) } // Add summary (SCA, license) scan comment diff --git a/utils/consts.go b/utils/consts.go index 50703328d..977cc3584 100644 --- a/utils/consts.go +++ b/utils/consts.go @@ -24,15 +24,13 @@ 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" - JFrogTokenEnv = "JF_ACCESS_TOKEN" - JfrogUseConfigProfileEnv = "JF_USE_CONFIG_PROFILE" - JfrogConfigProfileEnv = "JF_CONFIG_PROFILE" + JFrogUserEnv = "JF_USER" + JFrogUrlEnv = "JF_URL" + jfrogXrayUrlEnv = "JF_XRAY_URL" + jfrogArtifactoryUrlEnv = "JF_ARTIFACTORY_URL" + jfrogReleasesRepoEnv = "JF_RELEASES_REPO" + JFrogPasswordEnv = "JF_PASSWORD" + JFrogTokenEnv = "JF_ACCESS_TOKEN" // Git environment variables GitProvider = "JF_GIT_PROVIDER" @@ -47,7 +45,6 @@ const ( BranchNameTemplateEnv = "JF_BRANCH_NAME_TEMPLATE" CommitMessageTemplateEnv = "JF_COMMIT_MESSAGE_TEMPLATE" PullRequestTitleTemplateEnv = "JF_PULL_REQUEST_TITLE_TEMPLATE" - PullRequestCommentTitleEnv = "JF_PR_COMMENT_TITLE" //#nosec G101 -- not a secret PullRequestSecretCommentsEnv = "JF_PR_SHOW_SECRETS_COMMENTS" @@ -61,21 +58,16 @@ const ( jfrogProjectEnv = "JF_PROJECT" // To include vulnerabilities and violations IncludeVulnerabilitiesEnv = "JF_INCLUDE_VULNERABILITIES" - // To include all the vulnerabilities in the source branch at PR scan - IncludeAllVulnerabilitiesEnv = "JF_INCLUDE_ALL_VULNERABILITIES" - AvoidPreviousPrCommentsDeletionEnv = "JF_AVOID_PREVIOUS_PR_COMMENTS_DELETION" - AddPrCommentOnSuccessEnv = "JF_PR_ADD_SUCCESS_COMMENT" - FailOnSecurityIssuesEnv = "JF_FAIL" - UseWrapperEnv = "JF_USE_WRAPPER" - DepsRepoEnv = "JF_DEPS_REPO" - MinSeverityEnv = "JF_MIN_SEVERITY" - FixableOnlyEnv = "JF_FIXABLE_ONLY" - DisableJasEnv = "JF_DISABLE_ADVANCED_SECURITY" - DetectionOnlyEnv = "JF_SKIP_AUTOFIX" - AllowedLicensesEnv = "JF_ALLOWED_LICENSES" - SkipAutoInstallEnv = "JF_SKIP_AUTO_INSTALL" - AllowPartialResultsEnv = "JF_ALLOW_PARTIAL_RESULTS" - WatchesDelimiter = "," + 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" + SkipAutoInstallEnv = "JF_SKIP_AUTO_INSTALL" + AllowPartialResultsEnv = "JF_ALLOW_PARTIAL_RESULTS" + WatchesDelimiter = "," //#nosec G101 -- False positive - no hardcoded credentials. GitTokenEnv = "JF_GIT_TOKEN" @@ -83,7 +75,6 @@ const ( GitPullRequestIDEnv = "JF_GIT_PULL_REQUEST_ID" GitApiEndpointEnv = "JF_GIT_API_ENDPOINT" GitAggregateFixesEnv = "JF_GIT_AGGREGATE_FIXES" - GitEmailAuthorEnv = "JF_GIT_EMAIL_AUTHOR" // The 'GITHUB_ACTIONS' environment variable exists when the CI is GitHub Actions GitHubActionsEnv = "GITHUB_ACTIONS" @@ -93,9 +84,6 @@ const ( FixVersionPlaceHolder = "{FIX_VERSION}" BranchHashPlaceHolder = "{BRANCH_NAME_HASH}" - // General flags - AvoidExtraMessages = "JF_AVOID_EXTRA_MESSAGES" - // Default naming templates BranchNameTemplate = "frogbot-" + PackagePlaceHolder + "-" + BranchHashPlaceHolder AggregatedBranchNameTemplate = "frogbot-update-" + BranchHashPlaceHolder + "-dependencies" @@ -104,7 +92,7 @@ const ( AggregatePullRequestTitleDefaultTemplate = outputwriter.FrogbotTitlePrefix + " Update %s dependencies" // Frogbot Git author details showed in commits frogbotAuthorName = "JFrog-Frogbot" - frogbotAuthorEmail = "eco-system+frogbot@jfrog.com" + frogbotAuthorEmail = "frogbot@jfrog.com" ) type UnsupportedErrorType string diff --git a/utils/outputwriter/outputcontent.go b/utils/outputwriter/outputcontent.go index 8dab36380..9476702a6 100644 --- a/utils/outputwriter/outputcontent.go +++ b/utils/outputwriter/outputcontent.go @@ -40,8 +40,7 @@ const ( ) var ( - CommentGeneratedByFrogbot = MarkAsLink("🐸 JFrog Frogbot", FrogbotDocumentationUrl) - jasFeaturesMsgWhenNotEnabled = MarkAsBold("Frogbot") + " also supports " + MarkAsBold("Contextual Analysis, Secret Detection, IaC and SAST Vulnerabilities Scanning") + ". These features are included as part of the " + MarkAsLink("JFrog Advanced Security", "https://jfrog.com/advanced-security") + " package, which isn't enabled on your system." + CommentGeneratedByFrogbot = MarkAsLink("🐸 JFrog Frogbot", FrogbotDocumentationUrl) ) // For review comment Frogbot creates on Scan PR @@ -92,7 +91,7 @@ func GetFrogbotCommentBaseDecorator(writer OutputWriter) CommentDecorator { } } -// Adding a banner, custom title and untitled Jas message to the content +// Adding a banner and custom title to the content func GetPRSummaryMainCommentDecorator(issuesExists, isComment bool, writer OutputWriter) CommentDecorator { return func(_ int, content string) string { comment := strings.Builder{} @@ -104,7 +103,6 @@ func GetPRSummaryMainCommentDecorator(issuesExists, isComment bool, writer Outpu if issuesExists { WriteContent(&comment, content) } - WriteContent(&comment, untitledForJasMsg(writer)) return comment.String() } } @@ -140,13 +138,6 @@ func fixCVETitleSrc(vcsProvider vcsutils.VcsProvider) ImageSource { return VulnerabilitiesFixPrBannerSource } -func untitledForJasMsg(writer OutputWriter) string { - if writer.AvoidExtraMessages() || writer.IsEntitledForJas() { - return "" - } - return writer.MarkAsDetails("Note", 0, fmt.Sprintf("\n%s\n%s", SectionDivider(), writer.MarkInCenter(jasFeaturesMsgWhenNotEnabled))) -} - func footer(writer OutputWriter) string { return fmt.Sprintf("%s\n%s", SectionDivider(), writer.MarkInCenter(CommentGeneratedByFrogbot)) } diff --git a/utils/outputwriter/outputcontent_test.go b/utils/outputwriter/outputcontent_test.go index f6ade06be..36981740e 100644 --- a/utils/outputwriter/outputcontent_test.go +++ b/utils/outputwriter/outputcontent_test.go @@ -27,54 +27,29 @@ func TestGetMainCommentContent(t *testing.T) { issuesExists: false, isComment: true, cases: []OutputTestCase{ - { - name: "Pull Request not entitled (Standard output)", - writer: &StandardOutput{MarkdownOutput{hasInternetConnection: true}}, - expectedOutputPath: []string{filepath.Join(testSummaryCommentDir, "structure", "summary_comment_no_issues_pr_not_entitled.md")}, - }, { name: "Pull Request entitled (Standard output)", writer: &StandardOutput{MarkdownOutput{hasInternetConnection: true, entitledForJas: true}}, expectedOutputPath: []string{filepath.Join(testSummaryCommentDir, "structure", "summary_comment_no_issues_pr_entitled.md")}, }, - { - name: "Merge Request not entitled (Standard output)", - writer: &StandardOutput{MarkdownOutput{hasInternetConnection: true, vcsProvider: vcsutils.GitLab}}, - expectedOutputPath: []string{filepath.Join(testSummaryCommentDir, "structure", "summary_comment_no_issues_mr_not_entitled.md")}, - }, { name: "Merge Request entitled (Standard output)", writer: &StandardOutput{MarkdownOutput{hasInternetConnection: true, vcsProvider: vcsutils.GitLab, entitledForJas: true}}, expectedOutputPath: []string{filepath.Join(testSummaryCommentDir, "structure", "summary_comment_no_issues_mr_entitled.md")}, }, - { - name: "Simplified output not entitled", - writer: &SimplifiedOutput{MarkdownOutput{hasInternetConnection: true}}, - expectedOutputPath: []string{filepath.Join(testSummaryCommentDir, "structure", "summary_comment_no_issues_simplified_not_entitled.md")}, - }, { name: "Simplified output entitled", writer: &SimplifiedOutput{MarkdownOutput{hasInternetConnection: true, entitledForJas: true}}, expectedOutputPath: []string{filepath.Join(testSummaryCommentDir, "structure", "summary_comment_no_issues_simplified_entitled.md")}, }, { - name: "Pull request not entitled custom title (Standard output)", + name: "Pull Request not entitled custom title (Standard output)", writer: &StandardOutput{MarkdownOutput{hasInternetConnection: true, pullRequestCommentTitle: "Custom title"}}, - expectedOutputPath: []string{filepath.Join(testSummaryCommentDir, "structure", "summary_comment_no_issues_pr_not_entitled_with_title.md")}, - }, - { - name: "Pull Request not entitled custom title avoid extra messages (Standard output)", - writer: &StandardOutput{MarkdownOutput{hasInternetConnection: true, pullRequestCommentTitle: "Custom title", avoidExtraMessages: true}}, expectedOutputPath: []string{filepath.Join(testSummaryCommentDir, "structure", "summary_comment_no_issues_pr_entitled_with_title.md")}, }, { - name: "Pull request not entitled custom title (Simplified output)", - writer: &SimplifiedOutput{MarkdownOutput{hasInternetConnection: true, pullRequestCommentTitle: "Custom title"}}, - expectedOutputPath: []string{filepath.Join(testSummaryCommentDir, "structure", "summary_comment_no_issues_simplified_not_entitled_with_title.md")}, - }, - { - name: "Merge Request not entitled avoid extra messages (Standard output)", - writer: &StandardOutput{MarkdownOutput{hasInternetConnection: true, vcsProvider: vcsutils.GitLab, avoidExtraMessages: true}}, + name: "Merge Request not entitled (Standard output)", + writer: &StandardOutput{MarkdownOutput{hasInternetConnection: true, vcsProvider: vcsutils.GitLab}}, expectedOutputPath: []string{filepath.Join(testSummaryCommentDir, "structure", "summary_comment_no_issues_mr_entitled.md")}, }, }, @@ -84,39 +59,24 @@ func TestGetMainCommentContent(t *testing.T) { issuesExists: true, isComment: true, cases: []OutputTestCase{ - { - name: "Pull Request not entitled (Standard output)", - writer: &StandardOutput{MarkdownOutput{hasInternetConnection: true}}, - expectedOutputPath: []string{filepath.Join(testSummaryCommentDir, "structure", "summary_comment_issues_pr_not_entitled.md")}, - }, { name: "Pull Request entitled (Standard output)", writer: &StandardOutput{MarkdownOutput{hasInternetConnection: true, entitledForJas: true}}, expectedOutputPath: []string{filepath.Join(testSummaryCommentDir, "structure", "summary_comment_issues_pr_entitled.md")}, }, - { - name: "Merge Request not entitled (Standard output)", - writer: &StandardOutput{MarkdownOutput{hasInternetConnection: true, vcsProvider: vcsutils.GitLab}}, - expectedOutputPath: []string{filepath.Join(testSummaryCommentDir, "structure", "summary_comment_issues_mr_not_entitled.md")}, - }, { name: "Merge Request entitled (Standard output)", writer: &StandardOutput{MarkdownOutput{hasInternetConnection: true, vcsProvider: vcsutils.GitLab, entitledForJas: true}}, expectedOutputPath: []string{filepath.Join(testSummaryCommentDir, "structure", "summary_comment_issues_mr_entitled.md")}, }, { - name: "Simplified output not entitled", - writer: &SimplifiedOutput{MarkdownOutput{hasInternetConnection: true}}, - expectedOutputPath: []string{filepath.Join(testSummaryCommentDir, "structure", "summary_comment_issues_simplified_not_entitled.md")}, - }, - { - name: "Pull Request not entitled avoid extra messages (Standard output)", - writer: &StandardOutput{MarkdownOutput{hasInternetConnection: true, avoidExtraMessages: true}}, + name: "Pull Request not entitled (Standard output)", + writer: &StandardOutput{MarkdownOutput{hasInternetConnection: true}}, expectedOutputPath: []string{filepath.Join(testSummaryCommentDir, "structure", "summary_comment_issues_pr_entitled.md")}, }, { - name: "Simplified output not entitled avoid extra messages", - writer: &SimplifiedOutput{MarkdownOutput{hasInternetConnection: true, avoidExtraMessages: true}}, + name: "Simplified output not entitled", + writer: &SimplifiedOutput{MarkdownOutput{hasInternetConnection: true}}, expectedOutputPath: []string{filepath.Join(testSummaryCommentDir, "structure", "summary_comment_issues_simplified_entitled.md")}, }, { @@ -129,11 +89,6 @@ func TestGetMainCommentContent(t *testing.T) { writer: &StandardOutput{MarkdownOutput{hasInternetConnection: true, vcsProvider: vcsutils.GitLab, entitledForJas: true, pullRequestCommentTitle: "Custom title"}}, expectedOutputPath: []string{filepath.Join(testSummaryCommentDir, "structure", "summary_comment_issues_mr_entitled_with_title.md")}, }, - { - name: "Pull Request not entitled custom title (Standard output)", - writer: &StandardOutput{MarkdownOutput{hasInternetConnection: true, pullRequestCommentTitle: "Custom title"}}, - expectedOutputPath: []string{filepath.Join(testSummaryCommentDir, "structure", "summary_comment_issues_pr_not_entitled_with_title.md")}, - }, { name: "Pull request entitled custom title (Simplified output)", writer: &SimplifiedOutput{MarkdownOutput{hasInternetConnection: true, entitledForJas: true, pullRequestCommentTitle: "Custom title"}}, @@ -146,39 +101,19 @@ func TestGetMainCommentContent(t *testing.T) { issuesExists: true, isComment: false, cases: []OutputTestCase{ - { - name: "Pull Request not entitled (Standard output)", - writer: &StandardOutput{MarkdownOutput{hasInternetConnection: true}}, - expectedOutputPath: []string{filepath.Join(testSummaryCommentDir, "structure", "fix_pr_not_entitled.md")}, - }, { name: "Pull Request entitled (Standard output)", writer: &StandardOutput{MarkdownOutput{hasInternetConnection: true, entitledForJas: true}}, expectedOutputPath: []string{filepath.Join(testSummaryCommentDir, "structure", "fix_pr_entitled.md")}, }, - { - name: "Merge Request not entitled (Standard output)", - writer: &StandardOutput{MarkdownOutput{hasInternetConnection: true, vcsProvider: vcsutils.GitLab}}, - expectedOutputPath: []string{filepath.Join(testSummaryCommentDir, "structure", "fix_mr_not_entitled.md")}, - }, { name: "Merge Request entitled (Standard output)", writer: &StandardOutput{MarkdownOutput{hasInternetConnection: true, vcsProvider: vcsutils.GitLab, entitledForJas: true}}, expectedOutputPath: []string{filepath.Join(testSummaryCommentDir, "structure", "fix_mr_entitled.md")}, }, { - name: "Simplified output not entitled", - writer: &SimplifiedOutput{MarkdownOutput{hasInternetConnection: true}}, - expectedOutputPath: []string{filepath.Join(testSummaryCommentDir, "structure", "fix_simplified_not_entitled.md")}, - }, - { - name: "Simplified output not entitled ", + name: "Simplified output entitled", writer: &SimplifiedOutput{MarkdownOutput{hasInternetConnection: true}}, - expectedOutputPath: []string{filepath.Join(testSummaryCommentDir, "structure", "fix_simplified_not_entitled.md")}, - }, - { - name: "Simplified output entitled avoid extra messages", - writer: &SimplifiedOutput{MarkdownOutput{hasInternetConnection: true, avoidExtraMessages: true}}, expectedOutputPath: []string{filepath.Join(testSummaryCommentDir, "structure", "fix_simplified_entitled.md")}, }, }, diff --git a/utils/outputwriter/outputwriter.go b/utils/outputwriter/outputwriter.go index ed8fbbc33..17a43d8b0 100644 --- a/utils/outputwriter/outputwriter.go +++ b/utils/outputwriter/outputwriter.go @@ -17,8 +17,6 @@ type OutputWriter interface { SetJasOutputFlags(entitled, showCaColumn bool) IsShowingCaColumn() bool IsEntitledForJas() bool - SetAvoidExtraMessages(avoidExtraMessages bool) - AvoidExtraMessages() bool SetPullRequestCommentTitle(pullRequestCommentTitle string) PullRequestCommentTitle() string SetHasInternetConnection(connected bool) @@ -40,7 +38,6 @@ type OutputWriter interface { type MarkdownOutput struct { pullRequestCommentTitle string - avoidExtraMessages bool showCaColumn bool entitledForJas bool hasInternetConnection bool @@ -59,14 +56,6 @@ func (mo *MarkdownOutput) VcsProvider() vcsutils.VcsProvider { return mo.vcsProvider } -func (mo *MarkdownOutput) SetAvoidExtraMessages(avoidExtraMessages bool) { - mo.avoidExtraMessages = avoidExtraMessages -} - -func (mo *MarkdownOutput) AvoidExtraMessages() bool { - return mo.avoidExtraMessages -} - func (mo *MarkdownOutput) SetHasInternetConnection(connected bool) { mo.hasInternetConnection = connected } diff --git a/utils/params.go b/utils/params.go index 3ae8d5ec6..04dfcfc7b 100644 --- a/utils/params.go +++ b/utils/params.go @@ -48,7 +48,6 @@ type Repository struct { func (r *Repository) setOutputWriterDetails() { r.OutputWriter = outputwriter.GetCompatibleOutputWriter(r.Params.Git.GitProvider) - r.OutputWriter.SetAvoidExtraMessages(r.Params.Git.AvoidExtraMessages) r.OutputWriter.SetPullRequestCommentTitle(r.Params.Git.PullRequestCommentTitle) } @@ -143,43 +142,24 @@ func (p *Project) GetTechFromInstallCmdIfExists() []string { } type Scan struct { - IncludeAllVulnerabilities bool `yaml:"includeAllVulnerabilities,omitempty"` - FixableOnly bool `yaml:"fixableOnly,omitempty"` - DetectionOnly bool `yaml:"skipAutoFix,omitempty"` - FailOnSecurityIssues *bool `yaml:"failOnSecurityIssues,omitempty"` - AvoidPreviousPrCommentsDeletion bool `yaml:"avoidPreviousPrCommentsDeletion,omitempty"` - MinSeverity string `yaml:"minSeverity,omitempty"` - DisableJas bool `yaml:"disableJas,omitempty"` - AddPrCommentOnSuccess bool `yaml:"addPrCommentOnSuccess,omitempty"` - AllowedLicenses []string `yaml:"allowedLicenses,omitempty"` - Projects []Project `yaml:"projects,omitempty"` - ConfigProfile *services.ConfigProfile - SkipAutoInstall bool - AllowPartialResults bool + 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.IncludeAllVulnerabilities { - if s.IncludeAllVulnerabilities, err = getBoolEnv(IncludeAllVulnerabilitiesEnv, false); err != nil { - return - } - } - if !s.AvoidPreviousPrCommentsDeletion { - if s.AvoidPreviousPrCommentsDeletion, err = getBoolEnv(AvoidPreviousPrCommentsDeletionEnv, false); err != nil { - return - } - } if !s.FixableOnly { if s.FixableOnly, err = getBoolEnv(FixableOnlyEnv, false); err != nil { return } } - if !s.DisableJas { - if s.DisableJas, err = getBoolEnv(DisableJasEnv, false); err != nil { - return - } - } if !s.AddPrCommentOnSuccess { if s.AddPrCommentOnSuccess, err = getBoolEnv(AddPrCommentOnSuccessEnv, true); err != nil { return @@ -190,13 +170,6 @@ func (s *Scan) setDefaultsIfNeeded() (err error) { return } } - if s.FailOnSecurityIssues == nil { - var failOnSecurityIssues bool - if failOnSecurityIssues, err = getBoolEnv(FailOnSecurityIssuesEnv, true); err != nil { - return - } - s.FailOnSecurityIssues = &failOnSecurityIssues - } if s.MinSeverity == "" { if err = readParamFromEnv(MinSeverityEnv, &s.MinSeverity); err != nil && !e.IsMissingEnvErr(err) { return @@ -276,7 +249,6 @@ type Git struct { PullRequestTitleTemplate string `yaml:"pullRequestTitleTemplate,omitempty"` PullRequestCommentTitle string `yaml:"pullRequestCommentTitle,omitempty"` PullRequestSecretComments bool `yaml:"pullRequestSecretComments,omitempty"` - AvoidExtraMessages bool `yaml:"avoidExtraMessages,omitempty"` EmailAuthor string `yaml:"emailAuthor,omitempty"` AggregateFixes bool `yaml:"aggregateFixes,omitempty"` PullRequestDetails vcsclient.PullRequestInfo @@ -310,9 +282,7 @@ func (g *Git) setDefaultsIfNeeded(gitParamsFromEnv *Git, commandName string) (er g.RepoName = gitParamsFromEnv.RepoName } if g.EmailAuthor == "" { - if g.EmailAuthor = getTrimmedEnv(GitEmailAuthorEnv); g.EmailAuthor == "" { - g.EmailAuthor = frogbotAuthorEmail - } + g.EmailAuthor = frogbotAuthorEmail } if commandName == ScanPullRequest { if err = g.extractScanPullRequestEnvParams(gitParamsFromEnv); err != nil { @@ -342,16 +312,12 @@ func (g *Git) extractScanPullRequestEnvParams(gitParamsFromEnv *Git) (err error) 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.PullRequestCommentTitle == "" { - g.PullRequestCommentTitle = getTrimmedEnv(PullRequestCommentTitleEnv) - } if !g.PullRequestSecretComments { if g.PullRequestSecretComments, err = getBoolEnv(PullRequestSecretCommentsEnv, false); err != nil { return } } - g.AvoidExtraMessages, err = getBoolEnv(AvoidExtraMessages, false) return } @@ -439,7 +405,7 @@ func GetFrogbotDetails(commandName string) (frogbotDetails *FrogbotDetails, err return } - configProfile, repoCloneUrl, err := getConfigProfileIfExistsAndValid(xrayVersion, jfrogServer, client, gitParamsFromEnv, repository.JFrogProjectKey) + configProfile, repoCloneUrl, err := getConfigProfileIfExistsAndValid(xrayVersion, jfrogServer, client, gitParamsFromEnv) if err != nil { return } @@ -651,33 +617,15 @@ func getBoolEnv(envKey string, defaultValue bool) (bool, error) { return defaultValue, nil } -// This function attempts to fetch a config profile if JF_USE_CONFIG_PROFILE is set to true. -// If we need to use a profile, we first try to get the profile by name that can be provided through JF_CONFIG_PROFILE. If name is provided but profile doesn't exist we return an error. -// If we need to use a profile, but name is not provided, we check if there is a config profile associated to the repo URL. +// 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 nor by name we return an error. -func getConfigProfileIfExistsAndValid(xrayVersion string, jfrogServer *coreconfig.ServerDetails, gitClient vcsclient.VcsClient, gitParams *Git, projectKey string) (configProfile *services.ConfigProfile, repoCloneUrl string, err error) { - var useConfigProfile bool - if useConfigProfile, err = getBoolEnv(JfrogUseConfigProfileEnv, false); err != nil || !useConfigProfile { - log.Debug(fmt.Sprintf("Configuration Profile usage is disabled. All configurations will be derived from environment variables and files.\nTo enable a Configuration Profile, please set %s to TRUE", JfrogUseConfigProfileEnv)) - return - } - +// 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 } - // Attempt to get the config profile by profile's name - profileName := getTrimmedEnv(JfrogConfigProfileEnv) - if profileName != "" { - log.Debug(fmt.Sprintf("Configuration profile was requested. Searching profile by provided name '%s'", profileName)) - if configProfile, err = xsc.GetConfigProfileByName(xrayVersion, jfrogServer, profileName, projectKey); err != nil || configProfile == nil { - return - } - err = verifyConfigProfileValidity(configProfile) - return - } // Getting repository's url in order to get repository HTTP url if repoCloneUrl, err = gitParams.GetRepositoryHttpsCloneUrl(gitClient); err != nil { return diff --git a/utils/params_test.go b/utils/params_test.go index 99f7fe373..c9fff151d 100644 --- a/utils/params_test.go +++ b/utils/params_test.go @@ -3,6 +3,7 @@ package utils import ( "context" "encoding/json" + "fmt" "os" "path/filepath" "testing" @@ -13,7 +14,6 @@ import ( "github.com/jfrog/froggit-go/vcsclient" "github.com/jfrog/jfrog-cli-core/v2/utils/config" - "github.com/jfrog/jfrog-client-go/utils/tests" "github.com/jfrog/jfrog-client-go/xsc/services" "github.com/jfrog/froggit-go/vcsutils" @@ -223,31 +223,25 @@ func TestExtractInstallationCommandFromEnv(t *testing.T) { 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", - IncludeAllVulnerabilitiesEnv: "true", - AvoidPreviousPrCommentsDeletionEnv: "true", - FailOnSecurityIssuesEnv: "false", - MinSeverityEnv: "medium", - FixableOnlyEnv: "true", - DisableJasEnv: "true", - DetectionOnlyEnv: "true", - AllowedLicensesEnv: "MIT, Apache-2.0", - AvoidExtraMessages: "true", - PullRequestCommentTitleEnv: "build 1323", + 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", }) defer func() { assert.NoError(t, SanitizeEnv()) @@ -282,10 +276,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, false, *repo.FailOnSecurityIssues) assert.Equal(t, "Medium", repo.MinSeverity) assert.Equal(t, true, repo.FixableOnly) - assert.Equal(t, true, repo.DisableJas) assert.Equal(t, true, repo.AddPrCommentOnSuccess) assert.Equal(t, true, repo.DetectionOnly) assert.ElementsMatch(t, []string{"MIT", "Apache-2.0"}, repo.AllowedLicenses) @@ -308,8 +300,7 @@ func validateBuildRepo(t *testing.T, repo *Repository, gitParams *Git, server *c if commandName == ScanPullRequest { assert.NotZero(t, repo.PullRequestDetails.ID) - assert.True(t, repo.AvoidExtraMessages) - assert.NotEmpty(t, repo.PullRequestCommentTitle) + assert.Empty(t, repo.PullRequestCommentTitle) } project := repo.Projects[0] @@ -384,94 +375,48 @@ func TestVerifyValidApiEndpoint(t *testing.T) { func TestGetConfigProfileIfExistsAndValid(t *testing.T) { testcases := []struct { name string - useProfile bool - profileName string xrayVersion string failureExpected bool profileWithRepo bool + mockRepoInfoErr bool }{ { name: "Deprecated Server - Xray version is too low", - useProfile: true, - profileName: ValidConfigProfile, xrayVersion: "3.110.0", failureExpected: true, }, { - name: "Profile usage is not required", - useProfile: false, - }, - { - name: "Profile by name - Valid ConfigProfile", - useProfile: true, - profileName: ValidConfigProfile, - xrayVersion: services.ConfigProfileNewSchemaMinXrayVersion, - failureExpected: false, - }, - { - name: "Profile by name - Invalid Path From Root ConfigProfile", - useProfile: true, - profileName: InvalidPathConfigProfile, - xrayVersion: services.ConfigProfileNewSchemaMinXrayVersion, - failureExpected: true, - }, - { - name: "Profile by name - Invalid Modules ConfigProfile", - useProfile: true, - profileName: InvalidModulesConfigProfile, - xrayVersion: services.ConfigProfileNewSchemaMinXrayVersion, - failureExpected: true, - }, - { - // We are not creating test cases for Profile by URL verifications since they are the same verifications as Profile by name name: "Profile by URL - Valid ConfigProfile", - useProfile: true, - profileName: "", xrayVersion: services.ConfigProfileNewSchemaMinXrayVersion, failureExpected: false, profileWithRepo: true, }, { - name: "Profile by Name - Non existing profile name", - useProfile: true, - profileName: NonExistingProfile, + name: "Profile by URL - Failed fetching repository info", xrayVersion: services.ConfigProfileNewSchemaMinXrayVersion, failureExpected: true, + profileWithRepo: true, + mockRepoInfoErr: true, }, } for _, testcase := range testcases { t.Run(testcase.name, func(t *testing.T) { - if testcase.useProfile { - useProfileEnvCallBackFunc := tests.SetEnvWithCallbackAndAssert(t, JfrogUseConfigProfileEnv, "true") - defer useProfileEnvCallBackFunc() - } - - if testcase.profileName != "" { - profileNameEnvCallbackFunc := tests.SetEnvWithCallbackAndAssert(t, JfrogConfigProfileEnv, testcase.profileName) - defer profileNameEnvCallbackFunc() - } - mockServer, serverDetails := CreateXscMockServerForConfigProfile(t, testcase.xrayVersion) defer mockServer.Close() var mockVcsClient *testdata.MockVcsClient var mockGitParams *Git if testcase.profileWithRepo { - mockVcsClient = createMockVcsClient(t, "myUser", "my-repo") + mockVcsClient = createMockVcsClient(t, "myUser", "my-repo", testcase.mockRepoInfoErr) mockGitParams = &Git{ RepoOwner: "myUser", RepoName: "my-repo", } } - configProfile, repoCloneUrl, err := getConfigProfileIfExistsAndValid(testcase.xrayVersion, serverDetails, mockVcsClient, mockGitParams, "") + configProfile, repoCloneUrl, err := getConfigProfileIfExistsAndValid(testcase.xrayVersion, serverDetails, mockVcsClient, mockGitParams) - if !testcase.useProfile { - assert.Nil(t, configProfile) - assert.Nil(t, err) - return - } if testcase.failureExpected { assert.Error(t, err) return @@ -493,14 +438,20 @@ func TestGetConfigProfileIfExistsAndValid(t *testing.T) { } } -func createMockVcsClient(t *testing.T, repoOwner, repoName string) *testdata.MockVcsClient { +func createMockVcsClient(t *testing.T, repoOwner, repoName string, withError bool) *testdata.MockVcsClient { mockVcsClient := testdata.NewMockVcsClient(gomock.NewController(t)) - mockVcsClient.EXPECT().GetRepositoryInfo(context.Background(), repoOwner, repoName).Return(vcsclient.RepositoryInfo{ - CloneInfo: vcsclient.CloneInfo{ - HTTP: "https://github.com/myUser/my-repo.git", - SSH: "git@github.com:myUser/my-repo.git", - }, - RepositoryVisibility: 0, - }, nil) + if withError { + mockVcsClient.EXPECT().GetRepositoryInfo(context.Background(), repoOwner, repoName).Return(vcsclient.RepositoryInfo{}, fmt.Errorf("failed to fetch repository info")) + } else { + mockVcsClient.EXPECT().GetRepositoryInfo(context.Background(), repoOwner, repoName).Return( + vcsclient.RepositoryInfo{ + CloneInfo: vcsclient.CloneInfo{ + HTTP: "https://github.com/myUser/my-repo.git", + SSH: "git@github.com:myUser/my-repo.git", + }, + RepositoryVisibility: 0, + }, nil, + ) + } return mockVcsClient } diff --git a/utils/scandetails.go b/utils/scandetails.go index c9e91b343..38924eba9 100644 --- a/utils/scandetails.go +++ b/utils/scandetails.go @@ -25,7 +25,6 @@ type ScanDetails struct { *config.ServerDetails client vcsclient.VcsClient fixableOnly bool - disableJas bool skipAutoInstall bool minSeverityFilter severityutils.Severity baseBranch string @@ -62,11 +61,6 @@ func (sc *ScanDetails) SetResultsToCompare(results *results.SecurityCommandResul return sc } -func (sc *ScanDetails) SetDisableJas(disable bool) *ScanDetails { - sc.disableJas = disable - return sc -} - func (sc *ScanDetails) SetProject(project *Project) *ScanDetails { sc.Project = project return sc @@ -126,10 +120,6 @@ func (sc *ScanDetails) FixableOnly() bool { return sc.fixableOnly } -func (sc *ScanDetails) DisableJas() bool { - return sc.disableJas -} - func (sc *ScanDetails) MinSeverityFilter() severityutils.Severity { return sc.minSeverityFilter } @@ -165,7 +155,7 @@ func (sc *ScanDetails) RunInstallAndAudit(workDirs ...string) (auditResults *res SetAllowPartialResults(sc.allowPartialResults). SetExclusions(sc.PathExclusions). SetIsRecursiveScan(sc.IsRecursiveScan). - SetUseJas(!sc.DisableJas()). + SetUseJas(true). SetConfigProfile(sc.configProfile) auditParams := audit.NewAuditParams().