Skip to content

Commit

Permalink
Apply allow-partial-results to JAS scans (#223)
Browse files Browse the repository at this point in the history
  • Loading branch information
@eranturgeman
eranturgeman authored Nov 6, 2024
1 parent ebfda25 commit 2359427
Show file tree
Hide file tree
Showing 5 changed files with 104 additions and 47 deletions.
23 changes: 13 additions & 10 deletions commands/audit/audit.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,30 +181,30 @@ 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)
// Add the JAS scans to the parallel runner
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()
auditParallelRunner.JasWg.Wait()
// 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()
}()
Expand Down Expand Up @@ -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())
}
Expand Down Expand Up @@ -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
Expand All @@ -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)
}
Expand Down
80 changes: 64 additions & 16 deletions commands/audit/audit_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -364,6 +369,7 @@ func TestAuditWithPartialResults(t *testing.T) {
name string
allowPartialResults bool
useJas bool
pipRequirementsFile string
testDirPath string
}{
{
Expand All @@ -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,
Expand All @@ -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}).
Expand Down
20 changes: 10 additions & 10 deletions commands/scan/scan.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand All @@ -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 {
Expand Down Expand Up @@ -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)
}
}
}()
Expand Down
15 changes: 8 additions & 7 deletions jas/runner/jasrunner.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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))
}
Expand All @@ -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
}
Expand Down
13 changes: 9 additions & 4 deletions utils/results/results.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down

0 comments on commit 2359427

Please sign in to comment.