diff --git a/commands/audit/audit.go b/commands/audit/audit.go index 83112c9e..1c3fdb26 100644 --- a/commands/audit/audit.go +++ b/commands/audit/audit.go @@ -181,7 +181,7 @@ func RunAudit(auditParams *AuditParams) (cmdResults *results.SecurityCommandResu } jfrogAppsConfig, err := jas.CreateJFrogAppsConfig(cmdResults.GetTargetsPaths()) if err != nil { - return cmdResults.AddGeneralError(fmt.Errorf("failed to create JFrogAppsConfig: %s", err.Error())) + return cmdResults.AddGeneralError(fmt.Errorf("failed to create JFrogAppsConfig: %s", err.Error()), false) } // Initialize the parallel runner auditParallelRunner := utils.CreateSecurityParallelRunner(auditParams.threads) @@ -189,14 +189,14 @@ func RunAudit(auditParams *AuditParams) (cmdResults *results.SecurityCommandResu var jasScanner *jas.JasScanner var generalJasScanErr error if jasScanner, generalJasScanErr = RunJasScans(auditParallelRunner, auditParams, cmdResults, jfrogAppsConfig); generalJasScanErr != nil { - cmdResults.AddGeneralError(fmt.Errorf("An error has occurred during JAS scan process. JAS scan is skipped for the following directories: %s\n%s", strings.Join(cmdResults.GetTargetsPaths(), ","), generalJasScanErr.Error())) + cmdResults.AddGeneralError(fmt.Errorf("An error has occurred during JAS scan process. JAS scan is skipped for the following directories: %s\n%s", strings.Join(cmdResults.GetTargetsPaths(), ","), generalJasScanErr.Error()), auditParams.AllowPartialResults()) } if auditParams.Progress() != nil { auditParams.Progress().SetHeadlineMsg("Scanning for issues") } // The sca scan doesn't require the analyzer manager, so it can run separately from the analyzer manager download routine. if generalScaScanError := buildDepTreeAndRunScaScan(auditParallelRunner, auditParams, cmdResults); generalScaScanError != nil { - cmdResults.AddGeneralError(fmt.Errorf("An error has occurred during SCA scan process. SCA scan is skipped for the following directories: %s\n%s", strings.Join(cmdResults.GetTargetsPaths(), ","), generalScaScanError.Error())) + cmdResults.AddGeneralError(fmt.Errorf("An error has occurred during SCA scan process. SCA scan is skipped for the following directories: %s\n%s", strings.Join(cmdResults.GetTargetsPaths(), ","), generalScaScanError.Error()), auditParams.AllowPartialResults()) } go func() { auditParallelRunner.ScaScansWg.Wait() @@ -204,7 +204,7 @@ func RunAudit(auditParams *AuditParams) (cmdResults *results.SecurityCommandResu // Wait for all jas scanners to complete before cleaning up scanners temp dir auditParallelRunner.JasScannersWg.Wait() if jasScanner != nil && jasScanner.ScannerDirCleanupFunc != nil { - cmdResults.AddGeneralError(jasScanner.ScannerDirCleanupFunc()) + cmdResults.AddGeneralError(jasScanner.ScannerDirCleanupFunc(), false) } auditParallelRunner.Runner.Done() }() @@ -242,7 +242,7 @@ func RunJasScans(auditParallelRunner *utils.SecurityParallelRunner, auditParams } auditParallelRunner.JasWg.Add(1) if _, jasErr := auditParallelRunner.Runner.AddTaskWithError(createJasScansTasks(auditParallelRunner, scanResults, serverDetails, auditParams, jasScanner, jfrogAppsConfig), func(taskErr error) { - generalError = errors.Join(generalError, fmt.Errorf("failed while adding JAS scan tasks: %s", taskErr.Error())) + scanResults.AddGeneralError(fmt.Errorf("failed while adding JAS scan tasks: %s", taskErr.Error()), auditParams.AllowPartialResults()) }); jasErr != nil { generalError = fmt.Errorf("failed to create JAS task: %s", jasErr.Error()) } @@ -281,9 +281,12 @@ func createJasScansTasks(auditParallelRunner *utils.SecurityParallelRunner, scan SignedDescriptions: auditParams.OutputFormat() == format.Sarif, ScanResults: targetResult, TargetOutputDir: auditParams.scanResultsOutputDir, + AllowPartialResults: auditParams.AllowPartialResults(), } - if generalError := runner.AddJasScannersTasks(params); generalError != nil { + if generalError = runner.AddJasScannersTasks(params); generalError != nil { _ = targetResult.AddTargetError(fmt.Errorf("%s failed to add JAS scan tasks: %s", logPrefix, generalError.Error()), auditParams.AllowPartialResults()) + // We assign nil to 'generalError' after handling it to prevent it to propagate further, so it will not be captured twice - once here, and once in the error handling function of createJasScansTasks + generalError = nil } } return @@ -295,20 +298,20 @@ func initAuditCmdResults(params *AuditParams) (cmdResults *results.SecurityComma // Initialize general information serverDetails, err := params.ServerDetails() if err != nil { - return cmdResults.AddGeneralError(err) + return cmdResults.AddGeneralError(err, false) } var xrayManager *xray.XrayServicesManager if xrayManager, params.xrayVersion, err = xrayutils.CreateXrayServiceManagerAndGetVersion(serverDetails); err != nil { - return cmdResults.AddGeneralError(err) + return cmdResults.AddGeneralError(err, false) } else { cmdResults.SetXrayVersion(params.xrayVersion) } if err = clientutils.ValidateMinimumVersion(clientutils.Xray, params.xrayVersion, scangraph.GraphScanMinXrayVersion); err != nil { - return cmdResults.AddGeneralError(err) + return cmdResults.AddGeneralError(err, false) } entitledForJas, err := isEntitledForJas(xrayManager, params) if err != nil { - return cmdResults.AddGeneralError(err) + return cmdResults.AddGeneralError(err, false) } else { cmdResults.SetEntitledForJas(entitledForJas) } diff --git a/commands/audit/audit_test.go b/commands/audit/audit_test.go index 6dac6e19..6032ae80 100644 --- a/commands/audit/audit_test.go +++ b/commands/audit/audit_test.go @@ -2,6 +2,11 @@ package audit import ( "fmt" + commonCommands "github.com/jfrog/jfrog-cli-core/v2/common/commands" + "github.com/jfrog/jfrog-cli-core/v2/utils/coreutils" + configTests "github.com/jfrog/jfrog-cli-security/tests" + securityTestUtils "github.com/jfrog/jfrog-cli-security/tests/utils" + clientTests "github.com/jfrog/jfrog-client-go/utils/tests" "net/http" "path/filepath" "sort" @@ -364,6 +369,7 @@ func TestAuditWithPartialResults(t *testing.T) { name string allowPartialResults bool useJas bool + pipRequirementsFile string testDirPath string }{ { @@ -378,6 +384,13 @@ func TestAuditWithPartialResults(t *testing.T) { useJas: false, testDirPath: filepath.Join("..", "..", "tests", "testdata", "projects", "package-managers", "npm", "npm-project"), }, + { + name: "Failure in JAS scans", + allowPartialResults: false, + useJas: true, + pipRequirementsFile: "requirements.txt", + testDirPath: filepath.Join("..", "..", "tests", "testdata", "projects", "jas", "jas", "npm-project"), + }, { name: "Skip failure in SCA during dependency tree construction", allowPartialResults: true, @@ -390,35 +403,70 @@ func TestAuditWithPartialResults(t *testing.T) { useJas: false, testDirPath: filepath.Join("..", "..", "tests", "testdata", "projects", "package-managers", "npm", "npm-project"), }, - // TODO when applying allow-partial-results to JAS make sure to add a test case that checks failures in JAS scans + add some JAS api call to the mock server + { + name: "Skip failure in JAS scans", + allowPartialResults: true, + useJas: true, + pipRequirementsFile: "requirements.txt", + testDirPath: filepath.Join("..", "..", "tests", "testdata", "projects", "jas", "jas", "npm-project"), + }, } - serverMock, serverDetails := validations.CreateXrayRestsMockServer(func(w http.ResponseWriter, r *http.Request) { - if r.RequestURI == "/xray/api/v1/system/version" { - _, err := w.Write([]byte(fmt.Sprintf(`{"xray_version": "%s", "xray_revision": "xxx"}`, scangraph.GraphScanMinXrayVersion))) - if !assert.NoError(t, err) { - return - } - } - if strings.HasPrefix(r.RequestURI, "/xray/api/v1/scan/graph") && r.Method == http.MethodPost { - // We set SCA scan graph API to fail - w.WriteHeader(http.StatusBadRequest) - } - }) - defer serverMock.Close() - for _, testcase := range testcases { t.Run(testcase.name, func(t *testing.T) { + serverMock, serverDetails := validations.CreateXrayRestsMockServer(func(w http.ResponseWriter, r *http.Request) { + if r.RequestURI == "/xray/api/v1/system/version" { + _, err := w.Write([]byte(fmt.Sprintf(`{"xray_version": "%s", "xray_revision": "xxx"}`, utils.EntitlementsMinVersion))) + if !assert.NoError(t, err) { + return + } + } + // All endpoints required to test failures in SCA scan + if !testcase.useJas { + if strings.Contains(r.RequestURI, "/xray/api/v1/scan/graph") && r.Method == http.MethodPost { + // We set SCA scan graph API to fail + w.WriteHeader(http.StatusBadRequest) + } + } + + // All endpoints required to test failures in JAS + if testcase.useJas { + if strings.Contains(r.RequestURI, "/xsc-gen-exe-analyzer-manager-local/v1") { + w.WriteHeader(http.StatusBadRequest) + } + if strings.Contains(r.RequestURI, "api/v1/entitlements/feature/contextual_analysis") && r.Method == http.MethodGet { + _, err := w.Write([]byte(`{"entitled":true,"feature_id":"contextual_analysis"}`)) + if !assert.NoError(t, err) { + return + } + } + } + + }) + defer serverMock.Close() + tempDirPath, createTempDirCallback := coreTests.CreateTempDirWithCallbackAndAssert(t) defer createTempDirCallback() + if testcase.useJas { + // In order to simulate failure in Jas process we fail the AM download by using the mock server for the download and failing the endpoint call there + clientTests.SetEnvAndAssert(t, coreutils.HomeDir, filepath.Join(tempDirPath, configTests.Out, "jfroghome")) + err := commonCommands.NewConfigCommand(commonCommands.AddOrEdit, "testServer").SetDetails(serverDetails).SetInteractive(false).SetEncPassword(false).Run() + assert.NoError(t, err) + defer securityTestUtils.CleanTestsHomeEnv() + + callbackEnv := clientTests.SetEnvWithCallbackAndAssert(t, coreutils.ReleasesRemoteEnv, "testServer/testRemoteRepo") + defer callbackEnv() + } + assert.NoError(t, biutils.CopyDir(testcase.testDirPath, tempDirPath, false, nil)) auditBasicParams := (&utils.AuditBasicParams{}). SetServerDetails(serverDetails). SetOutputFormat(format.Table). SetUseJas(testcase.useJas). - SetAllowPartialResults(testcase.allowPartialResults) + SetAllowPartialResults(testcase.allowPartialResults). + SetPipRequirementsFile(testcase.pipRequirementsFile) auditParams := NewAuditParams(). SetWorkingDirs([]string{tempDirPath}). diff --git a/commands/scan/scan.go b/commands/scan/scan.go index 02972313..6ee5a7c5 100644 --- a/commands/scan/scan.go +++ b/commands/scan/scan.go @@ -252,7 +252,7 @@ func (scanCmd *ScanCommand) RunAndRecordResults(cmdType utils.CommandType, recor } if err = recordResFunc(cmdResults); err != nil { - cmdResults.AddGeneralError(fmt.Errorf("failed to record results: %s", err.Error())) + cmdResults.AddGeneralError(fmt.Errorf("failed to record results: %s", err.Error()), false) } if err = cmdResults.GetErrors(); err != nil { return @@ -283,7 +283,7 @@ func (scanCmd *ScanCommand) RunScan(cmdType utils.CommandType) (cmdResults *resu } // Initialize the Xray Indexer if indexerPath, indexerTempDir, cleanUp, err := initIndexer(xrayManager, cmdResults.XrayVersion); err != nil { - return cmdResults.AddGeneralError(err) + return cmdResults.AddGeneralError(err, false) } else { scanCmd.indexerPath = indexerPath scanCmd.indexerTempDir = indexerTempDir @@ -295,7 +295,7 @@ func (scanCmd *ScanCommand) RunScan(cmdType utils.CommandType) (cmdResults *resu } // Wait for the Download of the AnalyzerManager to complete. if err := errGroup.Wait(); err != nil { - cmdResults.AddGeneralError(errors.New("failed while trying to get Analyzer Manager: " + err.Error())) + cmdResults.AddGeneralError(errors.New("failed while trying to get Analyzer Manager: "+err.Error()), false) } fileProducerConsumer := parallel.NewRunner(threads, 20000, false) indexedFileProducerConsumer := parallel.NewRunner(threads, 20000, false) @@ -313,22 +313,22 @@ func initScanCmdResults(cmdType utils.CommandType, serverDetails *config.ServerD cmdResults = results.NewCommandResults(cmdType) xrayManager, xrayVersion, err := xray.CreateXrayServiceManagerAndGetVersion(serverDetails) if err != nil { - return xrayManager, cmdResults.AddGeneralError(err) + return xrayManager, cmdResults.AddGeneralError(err, false) } else { cmdResults.SetXrayVersion(xrayVersion) } // Validate Xray minimum version for graph scan command - if err := clientutils.ValidateMinimumVersion(clientutils.Xray, xrayVersion, scangraph.GraphScanMinXrayVersion); err != nil { - return xrayManager, cmdResults.AddGeneralError(err) + if err = clientutils.ValidateMinimumVersion(clientutils.Xray, xrayVersion, scangraph.GraphScanMinXrayVersion); err != nil { + return xrayManager, cmdResults.AddGeneralError(err, false) } if bypassArchiveLimits { // Validate Xray minimum version for BypassArchiveLimits flag for indexer - if err := clientutils.ValidateMinimumVersion(clientutils.Xray, xrayVersion, BypassArchiveLimitsMinXrayVersion); err != nil { - return xrayManager, cmdResults.AddGeneralError(err) + if err = clientutils.ValidateMinimumVersion(clientutils.Xray, xrayVersion, BypassArchiveLimitsMinXrayVersion); err != nil { + return xrayManager, cmdResults.AddGeneralError(err, false) } } if entitledForJas, err := isEntitledForJas(xrayManager, xrayVersion, useJas); err != nil { - return xrayManager, cmdResults.AddGeneralError(err) + return xrayManager, cmdResults.AddGeneralError(err, false) } else { cmdResults.SetEntitledForJas(entitledForJas) if entitledForJas { @@ -386,7 +386,7 @@ func (scanCmd *ScanCommand) prepareScanTasks(fileProducer, indexedFileProducer p taskHandler := getAddTaskToProducerFunc(fileProducer, artifactHandlerFunc) if generalError := collectFilesForIndexing(specFiles[i], taskHandler); generalError != nil { log.Error(generalError) - cmdResults.AddGeneralError(generalError) + cmdResults.AddGeneralError(generalError, false) } } }() diff --git a/jas/runner/jasrunner.go b/jas/runner/jasrunner.go index fdf3ef11..da33ab14 100644 --- a/jas/runner/jasrunner.go +++ b/jas/runner/jasrunner.go @@ -27,8 +27,9 @@ type JasRunnerParams struct { ServerDetails *config.ServerDetails Scanner *jas.JasScanner - Module jfrogappsconfig.Module - ConfigProfile *services.ConfigProfile + Module jfrogappsconfig.Module + ConfigProfile *services.ConfigProfile + AllowPartialResults bool ScansToPreform []utils.SubScanType @@ -101,7 +102,7 @@ func addJasScanTaskForModuleIfNeeded(params JasRunnerParams, subScan utils.SubSc return } if enabled { - generalError = addModuleJasScanTask(jasType, params.Runner, task, params.ScanResults) + generalError = addModuleJasScanTask(jasType, params.Runner, task, params.ScanResults, params.AllowPartialResults) } else { log.Debug(fmt.Sprintf("Skipping %s scan as requested by '%s' config profile...", jasType, params.ConfigProfile.ProfileName)) } @@ -111,15 +112,15 @@ func addJasScanTaskForModuleIfNeeded(params JasRunnerParams, subScan utils.SubSc log.Debug(fmt.Sprintf("Skipping %s scan as requested by local module config...", subScan)) return } - return addModuleJasScanTask(jasType, params.Runner, task, params.ScanResults) + return addModuleJasScanTask(jasType, params.Runner, task, params.ScanResults, params.AllowPartialResults) } -func addModuleJasScanTask(scanType jasutils.JasScanType, securityParallelRunner *utils.SecurityParallelRunner, task parallel.TaskFunc, scanResults *results.TargetResults) (generalError error) { +func addModuleJasScanTask(scanType jasutils.JasScanType, securityParallelRunner *utils.SecurityParallelRunner, task parallel.TaskFunc, scanResults *results.TargetResults, allowSkippingErrors bool) (generalError error) { securityParallelRunner.JasScannersWg.Add(1) if _, addTaskErr := securityParallelRunner.Runner.AddTaskWithError(task, func(err error) { - _ = scanResults.AddTargetError(fmt.Errorf("failed to run %s scan: %s", scanType, err.Error()), false) + _ = scanResults.AddTargetError(fmt.Errorf("failed to run %s scan: %s", scanType, err.Error()), allowSkippingErrors) }); addTaskErr != nil { - generalError = fmt.Errorf("failed to create %s scan task: %s", scanType, addTaskErr.Error()) + generalError = scanResults.AddTargetError(fmt.Errorf("error occurred while adding '%s' scan to parallel runner: %s", scanType, generalError.Error()), allowSkippingErrors) } return } diff --git a/utils/results/results.go b/utils/results/results.go index 2f454c87..51f93a4a 100644 --- a/utils/results/results.go +++ b/utils/results/results.go @@ -110,8 +110,13 @@ func (r *SecurityCommandResults) SetMultiScanId(multiScanId string) *SecurityCom } // --- Aggregated results for all targets --- - -func (r *SecurityCommandResults) AddGeneralError(err error) *SecurityCommandResults { +// Adds a general error to the command results in different phases of its execution. +// Notice that in some usages we pass constant 'false' to the 'allowSkippingError' parameter in some places, where we wish to force propagation of the error when it occurs. +func (r *SecurityCommandResults) AddGeneralError(err error, allowSkippingError bool) *SecurityCommandResults { + if allowSkippingError && err != nil { + log.Warn(fmt.Sprintf("Partial results are allowed, the error is skipped: %s", err.Error())) + return r + } r.GeneralError = errors.Join(r.GeneralError, err) return r } @@ -292,8 +297,8 @@ func (sr *TargetResults) HasFindings() bool { return false } -func (sr *TargetResults) AddTargetError(err error, allowPartialResults bool) error { - if allowPartialResults { +func (sr *TargetResults) AddTargetError(err error, allowSkippingError bool) error { + if allowSkippingError && err != nil { log.Warn(fmt.Sprintf("Partial results are allowed, the error is skipped in target '%s': %s", sr.String(), err.Error())) return nil }