diff --git a/scanpullrequest/scanpullrequest.go b/scanpullrequest/scanpullrequest.go index ec7717568..90a156fe1 100644 --- a/scanpullrequest/scanpullrequest.go +++ b/scanpullrequest/scanpullrequest.go @@ -91,14 +91,7 @@ func auditPullRequestAndReport(repoConfig *utils.Repository, client vcsclient.Vc repoConfig.Params.JFrogPlatform.JFrogProjectKey, ) defer func() { - if issuesCollection != nil { - xsc.SendScanEndedEvent( - scanDetails.XrayVersion, - scanDetails.XscVersion, - scanDetails.ServerDetails, - scanDetails.MultiScanId, scanDetails.StartTime, issuesCollection.GetAllIssuesCount(true), &scanDetails.ResultContext, err, - ) - } + xsc.SendScanEndedWithResults(scanDetails.ServerDetails, scanResults) }() issuesCollection, scanResults, err = auditPullRequestCode(repoConfig, scanDetails, sourceBranchWd, targetBranchWd) return @@ -144,7 +137,7 @@ func auditPullRequestCode(repoConfig *utils.Repository, scanDetails *utils.ScanD log.Debug("Scanning target branch code...") if targetScanResults, e := auditPullRequestTargetCode(scanDetails, targetBranchWd); e != nil { issuesCollection.AppendStatus(getResultScanStatues(targetScanResults)) - return issuesCollection, nil, fmt.Errorf("failed to audit target branch. Error: %s", e.Error()) + return issuesCollection, targetScanResults, fmt.Errorf("failed to audit target branch. Error: %s", e.Error()) } else { scanDetails.SetResultsToCompare(targetScanResults) } @@ -155,7 +148,7 @@ func auditPullRequestCode(repoConfig *utils.Repository, scanDetails *utils.ScanD // Scan error, report the scan status issuesCollection.AppendStatus(pullRequestIssues.ScanStatus) } - return issuesCollection, nil, fmt.Errorf("failed to audit source branch code. Error: %s", e.Error()) + return issuesCollection, sourceScanResults, fmt.Errorf("failed to audit source branch code. Error: %s", e.Error()) } issuesCollection.Append(pullRequestIssues) scanResults = sourceScanResults diff --git a/scanrepository/scanrepository.go b/scanrepository/scanrepository.go index 1fcba9bc6..66a52a4ed 100644 --- a/scanrepository/scanrepository.go +++ b/scanrepository/scanrepository.go @@ -106,11 +106,11 @@ func (sr *ScanRepositoryCmd) prepareEnvAndScanBranch(repository *utils.Repositor repository.Params.JFrogPlatform.JFrogProjectKey, ) - findings := 0 + var scanResults *results.SecurityCommandResults defer func() { - xsc.SendScanEndedEvent(sr.scanDetails.XrayVersion, sr.scanDetails.XscVersion, sr.scanDetails.ServerDetails, sr.scanDetails.MultiScanId, sr.scanDetails.StartTime, findings, &sr.scanDetails.ResultContext, err) + xsc.SendScanEndedWithResults(sr.scanDetails.ServerDetails, scanResults) }() - findings, err = sr.scanAndFixBranch(repository) + scanResults, err = sr.scanAndFixBranch(repository) return } @@ -143,19 +143,13 @@ func (sr *ScanRepositoryCmd) setCommandPrerequisites(repository *utils.Repositor return } -func (sr *ScanRepositoryCmd) scanAndFixBranch(repository *utils.Repository) (totalFindings int, err error) { - scanResults, err := sr.scan() +func (sr *ScanRepositoryCmd) scanAndFixBranch(repository *utils.Repository) (scanResults *results.SecurityCommandResults, err error) { + 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 + return scanResults, err } - } - if scanResults == nil { - err = fmt.Errorf("scan returned empty results for branch '%s'", sr.scanDetails.BaseBranch()) - if err = utils.CreateErrorIfFailUponScannerErrorEnabled(repository.GeneralConfig.FailUponAnyScannerError, fmt.Sprintf("An error occurred during Audit execution for '%s' branch. Downstream processing will be skipped", sr.scanDetails.BaseBranch()), err); err != nil { - return 0, err - } - return 0, nil + return scanResults, nil } defer func() { // Always check policy even if an error occurred during the scan @@ -165,42 +159,24 @@ func (sr *ScanRepositoryCmd) scanAndFixBranch(repository *utils.Repository) (tot } }() utils.PrintScanResultsTable(scanResults) - totalFindings = getTotalFindingsFromScanResults(scanResults) sr.uploadResultsToGithubDashboardsIfNeeded(repository, scanResults) sr.uploadGitLabScanResultsIfNeeded(repository, scanResults) 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 configuration settings in Jfrog platform", createAutoFixPrConfigNameInProfile)) - return totalFindings, nil + return scanResults, 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 + return scanResults, err } } if len(vulnerabilitiesByPathMap) == 0 { log.Info(fmt.Sprintf("Didn't find any vulnerable dependencies with existing fix versions or that are currently supported for fixing, for %s", sr.scanDetails.RepoName)) - return totalFindings, nil - } - return totalFindings, sr.fixVulnerablePackages(repository, scanResults.ResultsPlatformUrl, vulnerabilitiesByPathMap) -} - -func getTotalFindingsFromScanResults(scanResults *results.SecurityCommandResults) int { - if scanResults == nil { - return 0 + return scanResults, nil } - - summary, err := conversion.NewCommandResultsConvertor(conversion.ResultConvertParams{IncludeVulnerabilities: scanResults.IncludesVulnerabilities(), HasViolationContext: scanResults.HasViolationContext()}).ConvertToSummary(scanResults) - if err != nil { - log.Error("Failed to extract findings summary from scan results:", err) - return 0 - } - findingCount := summary.GetTotalViolations() - if findingCount == 0 { - findingCount = summary.GetTotalVulnerabilities() - } - return findingCount + return scanResults, sr.fixVulnerablePackages(repository, scanResults.ResultsPlatformUrl, vulnerabilitiesByPathMap) } func (sr *ScanRepositoryCmd) uploadGitLabScanResultsIfNeeded(repository *utils.Repository, scanResults *results.SecurityCommandResults) { @@ -233,7 +209,7 @@ func (sr *ScanRepositoryCmd) uploadResultsToGithubDashboardsIfNeeded(repository func (sr *ScanRepositoryCmd) scan() (*results.SecurityCommandResults, error) { auditResults := sr.scanDetails.Audit(sr.baseWd) if err := auditResults.GetErrors(); err != nil { - return nil, err + return auditResults, err } log.Info("Xray scan completed") sr.OutputWriter.SetJasOutputFlags(auditResults.Entitlements.Jas, auditResults.HasJasScansResults(jasutils.Applicability)) @@ -686,7 +662,7 @@ func (sr *ScanRepositoryCmd) aggregateFixAndOpenPullRequest(repository *utils.Re // Determines whether an update is necessary: // 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. +// Lastly, performs a comparison of Xray scans result hashes between an existing pull request's remote source branch and the current source branch to identify any differences. func (sr *ScanRepositoryCmd) isUpdateRequired(fixedVulnerabilities []*utils.VulnerabilityDetails, prInfo *vcsclient.PullRequestInfo) (updateRequired bool, err error) { isClean, err := sr.gitManager.IsClean() if err != nil { diff --git a/scanrepository/scanrepository_test.go b/scanrepository/scanrepository_test.go index b3fd9d96c..606774f75 100644 --- a/scanrepository/scanrepository_test.go +++ b/scanrepository/scanrepository_test.go @@ -638,116 +638,6 @@ func loadTestSBOM(t *testing.T, filename string) *cyclonedx.BOM { return bom } -func TestGetTotalFindingsFromScanResults(t *testing.T) { - testCases := []struct { - name string - scanResults *results.SecurityCommandResults - expectedCount int - }{ - { - name: "Nil scan results", - scanResults: nil, - expectedCount: 0, - }, - { - name: "No violations or vulnerabilities", - scanResults: &results.SecurityCommandResults{Targets: []*results.TargetResults{{ - ScanTarget: results.ScanTarget{Target: "target1"}, - }}}, - expectedCount: 0, - }, - { - name: "Vulnerabilities only", - scanResults: &results.SecurityCommandResults{ - ResultsMetaData: results.ResultsMetaData{ - ResultContext: results.ResultContext{IncludeVulnerabilities: true}}, - Targets: []*results.TargetResults{{ - ScanTarget: results.ScanTarget{Target: "target1", Technologies: []techutils.Technology{techutils.Npm}}, - ScaResults: &results.ScaScanResults{ - Sbom: loadTestSBOM(t, "sbom_with_vulnerabilities.json"), - }, - }}, - }, - expectedCount: 4, - }, - { - name: "Violations only", - scanResults: &results.SecurityCommandResults{ - ResultsMetaData: results.ResultsMetaData{ - ResultContext: results.ResultContext{Watches: []string{"w1"}}}, - Targets: []*results.TargetResults{{ - ScanTarget: results.ScanTarget{Target: "target1", Technologies: []techutils.Technology{techutils.Npm}}, - }}, - Violations: &violationutils.Violations{ - Sca: []violationutils.CveViolation{ - { - ScaViolation: violationutils.ScaViolation{ - ImpactedComponent: &cyclonedx.Component{ - BOMRef: "pkg:npm/viol1@1.0.0", - PackageURL: "pkg:npm/viol1@1.0.0", - }, - }, - CveVulnerability: cyclonedx.Vulnerability{BOMRef: "CVE-2023-1234"}, - }, - { - ScaViolation: violationutils.ScaViolation{ - ImpactedComponent: &cyclonedx.Component{ - BOMRef: "pkg:npm/viol2@2.0.0", - PackageURL: "pkg:npm/viol2@2.0.0", - }, - }, - CveVulnerability: cyclonedx.Vulnerability{BOMRef: "CVE-2022-1234"}, - }, - }, - }, - }, - expectedCount: 2, - }, - { - name: "Violations take precedence over vulnerabilities", - scanResults: &results.SecurityCommandResults{ - ResultsMetaData: results.ResultsMetaData{ - ResultContext: results.ResultContext{IncludeVulnerabilities: true, Watches: []string{"w1"}}}, - Targets: []*results.TargetResults{{ - ScanTarget: results.ScanTarget{Target: "target1", Technologies: []techutils.Technology{techutils.Npm}}, - ScaResults: &results.ScaScanResults{ - Sbom: loadTestSBOM(t, "sbom_with_vulnerabilities.json"), - }, - }}, - Violations: &violationutils.Violations{ - Sca: []violationutils.CveViolation{ - { - ScaViolation: violationutils.ScaViolation{ - ImpactedComponent: &cyclonedx.Component{ - BOMRef: "pkg:npm/viol1@1.0.0", - PackageURL: "pkg:npm/viol1@1.0.0", - }, - }, - CveVulnerability: cyclonedx.Vulnerability{BOMRef: "CVE-2023-1234"}, - }, - { - ScaViolation: violationutils.ScaViolation{ - ImpactedComponent: &cyclonedx.Component{ - BOMRef: "pkg:npm/viol2@2.0.0", - PackageURL: "pkg:npm/viol2@2.0.0", - }, - }, - CveVulnerability: cyclonedx.Vulnerability{BOMRef: "CVE-2022-1234"}, - }, - }, - }, - }, - expectedCount: 2, - }, - } - - for _, testCase := range testCases { - t.Run(testCase.name, func(t *testing.T) { - assert.Equal(t, testCase.expectedCount, getTotalFindingsFromScanResults(testCase.scanResults)) - }) - } -} - // Verifies unsupported packages return specific error // Other logic is implemented inside each package-handler. func TestUpdatePackageToFixedVersion(t *testing.T) {