From 8d07a9970ea3be7432b9b1dadf431a01289ecf87 Mon Sep 17 00:00:00 2001 From: Omer Zidkoni <50792403+omerzi@users.noreply.github.com> Date: Mon, 21 Aug 2023 08:10:07 +0300 Subject: [PATCH] Refactor advanced security scanners (#894) --- xray/audit/jas/applicabilitymanager.go | 92 +++------- xray/audit/jas/applicabilitymanager_test.go | 191 ++++++++++++++------ xray/audit/jas/iacscanner.go | 86 +++------ xray/audit/jas/iacscanner_test.go | 68 ++++--- xray/audit/jas/jasmanager.go | 86 ++++++--- xray/audit/jas/jasmanager_test.go | 41 ++--- xray/audit/jas/secretsscanner.go | 104 +++-------- xray/audit/jas/secretsscanner_test.go | 105 +++++++---- xray/commands/audit/generic/auditmanager.go | 2 +- xray/utils/analyzermanager.go | 68 +++---- xray/utils/resultwriter.go | 4 +- 11 files changed, 424 insertions(+), 423 deletions(-) diff --git a/xray/audit/jas/applicabilitymanager.go b/xray/audit/jas/applicabilitymanager.go index 927944b57..788114c24 100644 --- a/xray/audit/jas/applicabilitymanager.go +++ b/xray/audit/jas/applicabilitymanager.go @@ -1,19 +1,15 @@ package jas import ( - "errors" "github.com/jfrog/gofrog/datastructures" - "github.com/jfrog/jfrog-cli-core/v2/utils/config" "github.com/jfrog/jfrog-cli-core/v2/utils/coreutils" "github.com/jfrog/jfrog-cli-core/v2/xray/utils" "github.com/jfrog/jfrog-client-go/utils/errorutils" - "github.com/jfrog/jfrog-client-go/utils/io/fileutils" "github.com/jfrog/jfrog-client-go/utils/log" "github.com/jfrog/jfrog-client-go/xray/services" xrayUtils "github.com/jfrog/jfrog-client-go/xray/services/utils" "github.com/owenrumney/go-sarif/v2/sarif" "golang.org/x/exp/maps" - "path/filepath" "strings" ) @@ -32,63 +28,37 @@ const ( // map[string]string: A map containing the applicability result of each XRAY CVE. // bool: true if the user is entitled to the applicability scan, false otherwise. // error: An error object (if any). -func getApplicabilityScanResults(results []services.ScanResponse, dependencyTrees []*xrayUtils.GraphNode, - serverDetails *config.ServerDetails, scannedTechnologies []coreutils.Technology, workingDirs []string, analyzerManager utils.AnalyzerManagerInterface) (map[string]string, bool, error) { - applicabilityScanManager, cleanupFunc, err := newApplicabilityScanManager(results, dependencyTrees, serverDetails, workingDirs, analyzerManager) - if err != nil { - return nil, false, utils.Applicability.FormattedError(err) - } - defer func() { - if cleanupFunc != nil { - err = errors.Join(err, cleanupFunc()) - } - }() +func getApplicabilityScanResults(xrayResults []services.ScanResponse, dependencyTrees []*xrayUtils.GraphNode, + scannedTechnologies []coreutils.Technology, scanner *AdvancedSecurityScanner) (results map[string]string, err error) { + applicabilityScanManager := newApplicabilityScanManager(xrayResults, dependencyTrees, scanner) if !applicabilityScanManager.shouldRunApplicabilityScan(scannedTechnologies) { log.Debug("The technologies that have been scanned are currently not supported for contextual analysis scanning, or we couldn't find any vulnerable direct dependencies. Skipping....") - return nil, false, nil + return } - if err = applicabilityScanManager.run(); err != nil { - return nil, false, utils.ParseAnalyzerManagerError(utils.Applicability, err) + if err = applicabilityScanManager.scanner.Run(applicabilityScanManager); err != nil { + err = utils.ParseAnalyzerManagerError(utils.Applicability, err) + return } - return applicabilityScanManager.applicabilityScanResults, true, nil + results = applicabilityScanManager.applicabilityScanResults + return } type ApplicabilityScanManager struct { applicabilityScanResults map[string]string directDependenciesCves *datastructures.Set[string] xrayResults []services.ScanResponse - configFileName string - resultsFileName string - analyzerManager utils.AnalyzerManagerInterface - serverDetails *config.ServerDetails - workingDirs []string + scanner *AdvancedSecurityScanner } -func newApplicabilityScanManager(xrayScanResults []services.ScanResponse, dependencyTrees []*xrayUtils.GraphNode, - serverDetails *config.ServerDetails, workingDirs []string, analyzerManager utils.AnalyzerManagerInterface) (manager *ApplicabilityScanManager, cleanup func() error, err error) { +func newApplicabilityScanManager(xrayScanResults []services.ScanResponse, dependencyTrees []*xrayUtils.GraphNode, scanner *AdvancedSecurityScanner) (manager *ApplicabilityScanManager) { directDependencies := getDirectDependenciesSet(dependencyTrees) - tempDir, err := fileutils.CreateTempDir() - if err != nil { - return - } - cleanup = func() error { - return fileutils.RemoveTempDir(tempDir) - } - fullPathWorkingDirs, err := utils.GetFullPathsWorkingDirs(workingDirs) - if err != nil { - return nil, cleanup, err - } directDependenciesCves := extractDirectDependenciesCvesFromScan(xrayScanResults, directDependencies) return &ApplicabilityScanManager{ applicabilityScanResults: map[string]string{}, directDependenciesCves: directDependenciesCves, - configFileName: filepath.Join(tempDir, "config.yaml"), - resultsFileName: filepath.Join(tempDir, "results.sarif"), xrayResults: xrayScanResults, - analyzerManager: analyzerManager, - serverDetails: serverDetails, - workingDirs: fullPathWorkingDirs, - }, cleanup, nil + scanner: scanner, + } } // This function gets a list of xray scan responses that contain direct and indirect vulnerabilities and returns only direct @@ -135,31 +105,19 @@ func getDirectDependenciesSet(dependencyTrees []*xrayUtils.GraphNode) *datastruc return directDependencies } -func (a *ApplicabilityScanManager) run() (err error) { - log.Info("Running applicability scanning for the identified vulnerable direct dependencies...") - for _, workingDir := range a.workingDirs { - var workingDirResults map[string]string - if workingDirResults, err = a.runApplicabilityScan(workingDir); err != nil { - return - } - for cve, result := range workingDirResults { - a.applicabilityScanResults[cve] = result - } - } - return -} - -func (a *ApplicabilityScanManager) runApplicabilityScan(workingDir string) (results map[string]string, err error) { - defer func() { - err = errors.Join(err, deleteJasProcessFiles(a.configFileName, a.resultsFileName)) - }() - if err = a.createConfigFile(workingDir); err != nil { +func (a *ApplicabilityScanManager) Run(wd string) (err error) { + log.Info("Running applicability scanning in the", wd, "directory...") + if err = a.createConfigFile(wd); err != nil { return } if err = a.runAnalyzerManager(); err != nil { return } - results, err = a.getScanResults() + var workingDirResults map[string]string + workingDirResults, err = a.getScanResults() + for cve, result := range workingDirResults { + a.applicabilityScanResults[cve] = result + } return } @@ -189,7 +147,7 @@ func (a *ApplicabilityScanManager) createConfigFile(workingDir string) error { Scans: []scanConfiguration{ { Roots: []string{workingDir}, - Output: a.resultsFileName, + Output: a.scanner.resultsFileName, Type: applicabilityScanType, GrepDisable: false, CveWhitelist: a.directDependenciesCves.ToSlice(), @@ -197,17 +155,17 @@ func (a *ApplicabilityScanManager) createConfigFile(workingDir string) error { }, }, } - return createScannersConfigFile(a.configFileName, configFileContent) + return createScannersConfigFile(a.scanner.configFileName, configFileContent) } // Runs the analyzerManager app and returns a boolean to indicate whether the user is entitled for // advance security feature func (a *ApplicabilityScanManager) runAnalyzerManager() error { - return a.analyzerManager.Exec(a.configFileName, applicabilityScanCommand, a.serverDetails) + return a.scanner.analyzerManager.Exec(a.scanner.configFileName, applicabilityScanCommand, a.scanner.serverDetails) } func (a *ApplicabilityScanManager) getScanResults() (map[string]string, error) { - report, err := sarif.Open(a.resultsFileName) + report, err := sarif.Open(a.scanner.resultsFileName) if errorutils.CheckError(err) != nil { return nil, err } diff --git a/xray/audit/jas/applicabilitymanager_test.go b/xray/audit/jas/applicabilitymanager_test.go index 95e68b4e9..23891f51e 100644 --- a/xray/audit/jas/applicabilitymanager_test.go +++ b/xray/audit/jas/applicabilitymanager_test.go @@ -1,8 +1,8 @@ package jas import ( - "errors" "github.com/jfrog/gofrog/datastructures" + rtutils "github.com/jfrog/jfrog-cli-core/v2/artifactory/utils" "github.com/jfrog/jfrog-cli-core/v2/utils/coreutils" "github.com/jfrog/jfrog-cli-core/v2/xray/utils" "github.com/jfrog/jfrog-client-go/xray/services" @@ -15,30 +15,47 @@ import ( func TestNewApplicabilityScanManager_InputIsValid(t *testing.T) { // Act - applicabilityManager, _, err := newApplicabilityScanManager(fakeBasicXrayResults, fakeBasicDependencyGraph, &fakeServerDetails, nil, &analyzerManagerMock{}) + assert.NoError(t, rtutils.DownloadAnalyzerManagerIfNeeded()) + scanner, err := NewAdvancedSecurityScanner(nil, &fakeServerDetails) + assert.NoError(t, err) + defer func() { + if scanner.scannerDirCleanupFunc != nil { + assert.NoError(t, scanner.scannerDirCleanupFunc()) + } + }() + applicabilityManager := newApplicabilityScanManager(fakeBasicXrayResults, fakeBasicDependencyGraph, scanner) // Assert - assert.NoError(t, err) assert.NotEmpty(t, applicabilityManager) - assert.NotEmpty(t, applicabilityManager.configFileName) - assert.NotEmpty(t, applicabilityManager.resultsFileName) + assert.NotEmpty(t, applicabilityManager.scanner.configFileName) + assert.NotEmpty(t, applicabilityManager.scanner.resultsFileName) assert.Equal(t, applicabilityManager.directDependenciesCves.Size(), 5) } func TestNewApplicabilityScanManager_DependencyTreeDoesntExist(t *testing.T) { // Act - applicabilityManager, _, err := newApplicabilityScanManager(fakeBasicXrayResults, nil, &fakeServerDetails, nil, &analyzerManagerMock{}) + assert.NoError(t, rtutils.DownloadAnalyzerManagerIfNeeded()) + scanner, err := NewAdvancedSecurityScanner(nil, &fakeServerDetails) + assert.NoError(t, err) + defer func() { + if scanner.scannerDirCleanupFunc != nil { + assert.NoError(t, scanner.scannerDirCleanupFunc()) + } + }() + applicabilityManager := newApplicabilityScanManager(fakeBasicXrayResults, nil, scanner) // Assert - assert.NoError(t, err) assert.NotEmpty(t, applicabilityManager) - assert.NotEmpty(t, applicabilityManager.configFileName) - assert.NotEmpty(t, applicabilityManager.resultsFileName) + assert.NotNil(t, applicabilityManager.scanner.scannerDirCleanupFunc) + assert.Len(t, applicabilityManager.scanner.workingDirs, 1) + assert.NotEmpty(t, applicabilityManager.scanner.configFileName) + assert.NotEmpty(t, applicabilityManager.scanner.resultsFileName) assert.Equal(t, applicabilityManager.directDependenciesCves.Size(), 0) } func TestNewApplicabilityScanManager_NoDirectDependenciesInScan(t *testing.T) { // Arrange + assert.NoError(t, rtutils.DownloadAnalyzerManagerIfNeeded()) var noDirectDependenciesResults = []services.ScanResponse{ { ScanId: "scanId_1", @@ -60,34 +77,48 @@ func TestNewApplicabilityScanManager_NoDirectDependenciesInScan(t *testing.T) { fakeBasicXrayResults[0].Violations[0].Components["issueId_2_non_direct_dependency"] = services.Component{} // Act - applicabilityManager, _, err := newApplicabilityScanManager(noDirectDependenciesResults, fakeBasicDependencyGraph, &fakeServerDetails, nil, &analyzerManagerMock{}) + scanner, err := NewAdvancedSecurityScanner(nil, &fakeServerDetails) + assert.NoError(t, err) + defer func() { + if scanner.scannerDirCleanupFunc != nil { + assert.NoError(t, scanner.scannerDirCleanupFunc()) + } + }() + applicabilityManager := newApplicabilityScanManager(noDirectDependenciesResults, fakeBasicDependencyGraph, scanner) // Assert - assert.NoError(t, err) assert.NotEmpty(t, applicabilityManager) - assert.NotEmpty(t, applicabilityManager.configFileName) - assert.NotEmpty(t, applicabilityManager.resultsFileName) + assert.NotEmpty(t, applicabilityManager.scanner.configFileName) + assert.NotEmpty(t, applicabilityManager.scanner.resultsFileName) // Non-direct dependencies should not be added assert.Equal(t, 0, applicabilityManager.directDependenciesCves.Size()) } func TestNewApplicabilityScanManager_MultipleDependencyTrees(t *testing.T) { // Arrange + assert.NoError(t, rtutils.DownloadAnalyzerManagerIfNeeded()) multipleDependencyTrees := []*xrayUtils.GraphNode{multipleFakeBasicDependencyGraph[0], multipleFakeBasicDependencyGraph[1]} // Act - applicabilityManager, _, err := newApplicabilityScanManager(fakeBasicXrayResults, multipleDependencyTrees, &fakeServerDetails, nil, &analyzerManagerMock{}) + scanner, err := NewAdvancedSecurityScanner(nil, &fakeServerDetails) + assert.NoError(t, err) + defer func() { + if scanner.scannerDirCleanupFunc != nil { + assert.NoError(t, scanner.scannerDirCleanupFunc()) + } + }() + applicabilityManager := newApplicabilityScanManager(fakeBasicXrayResults, multipleDependencyTrees, scanner) // Assert - assert.NoError(t, err) assert.NotEmpty(t, applicabilityManager) - assert.NotEmpty(t, applicabilityManager.configFileName) - assert.NotEmpty(t, applicabilityManager.resultsFileName) + assert.NotEmpty(t, applicabilityManager.scanner.configFileName) + assert.NotEmpty(t, applicabilityManager.scanner.resultsFileName) assert.Equal(t, 5, applicabilityManager.directDependenciesCves.Size()) } func TestNewApplicabilityScanManager_ViolationsDontExistInResults(t *testing.T) { // Arrange + assert.NoError(t, rtutils.DownloadAnalyzerManagerIfNeeded()) noViolationScanResponse := []services.ScanResponse{ { ScanId: "scanId_1", @@ -100,18 +131,26 @@ func TestNewApplicabilityScanManager_ViolationsDontExistInResults(t *testing.T) } // Act - applicabilityManager, _, err := newApplicabilityScanManager(noViolationScanResponse, fakeBasicDependencyGraph, &fakeServerDetails, nil, &analyzerManagerMock{}) + scanner, err := NewAdvancedSecurityScanner(nil, &fakeServerDetails) + assert.NoError(t, err) + defer func() { + if scanner.scannerDirCleanupFunc != nil { + assert.NoError(t, scanner.scannerDirCleanupFunc()) + } + }() + applicabilityManager := newApplicabilityScanManager(noViolationScanResponse, fakeBasicDependencyGraph, scanner) // Assert assert.NoError(t, err) assert.NotEmpty(t, applicabilityManager) - assert.NotEmpty(t, applicabilityManager.configFileName) - assert.NotEmpty(t, applicabilityManager.resultsFileName) + assert.NotEmpty(t, applicabilityManager.scanner.configFileName) + assert.NotEmpty(t, applicabilityManager.scanner.resultsFileName) assert.Equal(t, 3, applicabilityManager.directDependenciesCves.Size()) } func TestNewApplicabilityScanManager_VulnerabilitiesDontExist(t *testing.T) { // Arrange + assert.NoError(t, rtutils.DownloadAnalyzerManagerIfNeeded()) noVulnerabilitiesScanResponse := []services.ScanResponse{ { ScanId: "scanId_1", @@ -124,31 +163,50 @@ func TestNewApplicabilityScanManager_VulnerabilitiesDontExist(t *testing.T) { } // Act - applicabilityManager, _, err := newApplicabilityScanManager(noVulnerabilitiesScanResponse, fakeBasicDependencyGraph, &fakeServerDetails, nil, &analyzerManagerMock{}) + scanner, err := NewAdvancedSecurityScanner(nil, &fakeServerDetails) + assert.NoError(t, err) + defer func() { + if scanner.scannerDirCleanupFunc != nil { + assert.NoError(t, scanner.scannerDirCleanupFunc()) + } + }() + applicabilityManager := newApplicabilityScanManager(noVulnerabilitiesScanResponse, fakeBasicDependencyGraph, scanner) // Assert - assert.NoError(t, err) assert.NotEmpty(t, applicabilityManager) - assert.NotEmpty(t, applicabilityManager.configFileName) - assert.NotEmpty(t, applicabilityManager.resultsFileName) + assert.NotEmpty(t, applicabilityManager.scanner.configFileName) + assert.NotEmpty(t, applicabilityManager.scanner.resultsFileName) assert.Equal(t, 2, applicabilityManager.directDependenciesCves.Size()) } func TestApplicabilityScanManager_ShouldRun_TechnologiesNotEligibleForScan(t *testing.T) { - analyzerManagerExecuter = &analyzerManagerMock{} - applicabilityManager, eligible, err := getApplicabilityScanResults(fakeBasicXrayResults, fakeBasicDependencyGraph, - &fakeServerDetails, []coreutils.Technology{coreutils.Nuget, coreutils.Go}, nil, &analyzerManagerMock{}) + assert.NoError(t, rtutils.DownloadAnalyzerManagerIfNeeded()) + scanner, err := NewAdvancedSecurityScanner(nil, &fakeServerDetails) + assert.NoError(t, err) + defer func() { + if scanner.scannerDirCleanupFunc != nil { + assert.NoError(t, scanner.scannerDirCleanupFunc()) + } + }() + results, err := getApplicabilityScanResults(fakeBasicXrayResults, fakeBasicDependencyGraph, + []coreutils.Technology{coreutils.Nuget, coreutils.Go}, scanner) // Assert - assert.Nil(t, applicabilityManager) + assert.Nil(t, results) assert.NoError(t, err) - assert.False(t, eligible) } func TestApplicabilityScanManager_ShouldRun_ScanResultsAreEmpty(t *testing.T) { // Arrange - analyzerManagerExecuter = &analyzerManagerMock{} - applicabilityManager, _, err := newApplicabilityScanManager(nil, fakeBasicDependencyGraph, &fakeServerDetails, nil, &analyzerManagerMock{}) + assert.NoError(t, rtutils.DownloadAnalyzerManagerIfNeeded()) + scanner, err := NewAdvancedSecurityScanner(nil, &fakeServerDetails) + assert.NoError(t, err) + defer func() { + if scanner.scannerDirCleanupFunc != nil { + assert.NoError(t, scanner.scannerDirCleanupFunc()) + } + }() + applicabilityManager := newApplicabilityScanManager(nil, fakeBasicDependencyGraph, scanner) assert.NoError(t, err) // Assert eligible := applicabilityManager.shouldRunApplicabilityScan([]coreutils.Technology{coreutils.Npm}) @@ -274,9 +332,15 @@ func TestGetDirectDependenciesList(t *testing.T) { func TestCreateConfigFile_VerifyFileWasCreated(t *testing.T) { // Arrange - analyzerManagerExecuter = &analyzerManagerMock{} - applicabilityManager, _, applicabilityManagerError := newApplicabilityScanManager(fakeBasicXrayResults, fakeBasicDependencyGraph, &fakeServerDetails, nil, &analyzerManagerMock{}) - assert.NoError(t, applicabilityManagerError) + assert.NoError(t, rtutils.DownloadAnalyzerManagerIfNeeded()) + scanner, err := NewAdvancedSecurityScanner(nil, &fakeServerDetails) + assert.NoError(t, err) + defer func() { + if scanner.scannerDirCleanupFunc != nil { + assert.NoError(t, scanner.scannerDirCleanupFunc()) + } + }() + applicabilityManager := newApplicabilityScanManager(fakeBasicXrayResults, fakeBasicDependencyGraph, scanner) currWd, err := coreutils.GetWorkingDirectory() assert.NoError(t, err) @@ -284,28 +348,34 @@ func TestCreateConfigFile_VerifyFileWasCreated(t *testing.T) { assert.NoError(t, err) defer func() { - err = os.Remove(applicabilityManager.configFileName) + err = os.Remove(applicabilityManager.scanner.configFileName) assert.NoError(t, err) }() - _, fileNotExistError := os.Stat(applicabilityManager.configFileName) + _, fileNotExistError := os.Stat(applicabilityManager.scanner.configFileName) assert.NoError(t, fileNotExistError) - fileContent, err := os.ReadFile(applicabilityManager.configFileName) + fileContent, err := os.ReadFile(applicabilityManager.scanner.configFileName) assert.NoError(t, err) assert.True(t, len(fileContent) > 0) } func TestParseResults_EmptyResults_AllCvesShouldGetUnknown(t *testing.T) { // Arrange - analyzerManagerExecuter = &analyzerManagerMock{} - applicabilityManager, _, applicabilityManagerError := newApplicabilityScanManager(fakeBasicXrayResults, fakeBasicDependencyGraph, &fakeServerDetails, nil, &analyzerManagerMock{}) - applicabilityManager.resultsFileName = filepath.Join("..", "..", "commands", "testdata", "applicability-scan", "empty-results.sarif") + assert.NoError(t, rtutils.DownloadAnalyzerManagerIfNeeded()) + scanner, err := NewAdvancedSecurityScanner(nil, &fakeServerDetails) + assert.NoError(t, err) + defer func() { + if scanner.scannerDirCleanupFunc != nil { + assert.NoError(t, scanner.scannerDirCleanupFunc()) + } + }() + applicabilityManager := newApplicabilityScanManager(fakeBasicXrayResults, fakeBasicDependencyGraph, scanner) + applicabilityManager.scanner.resultsFileName = filepath.Join("..", "..", "commands", "testdata", "applicability-scan", "empty-results.sarif") // Act results, err := applicabilityManager.getScanResults() // Assert - assert.NoError(t, applicabilityManagerError) assert.NoError(t, err) assert.Equal(t, 5, len(results)) for _, cveResult := range results { @@ -315,15 +385,21 @@ func TestParseResults_EmptyResults_AllCvesShouldGetUnknown(t *testing.T) { func TestParseResults_ApplicableCveExist(t *testing.T) { // Arrange - analyzerManagerExecuter = &analyzerManagerMock{} - applicabilityManager, _, applicabilityManagerError := newApplicabilityScanManager(fakeBasicXrayResults, fakeBasicDependencyGraph, &fakeServerDetails, nil, &analyzerManagerMock{}) - applicabilityManager.resultsFileName = filepath.Join("..", "..", "commands", "testdata", "applicability-scan", "applicable-cve-results.sarif") + assert.NoError(t, rtutils.DownloadAnalyzerManagerIfNeeded()) + scanner, err := NewAdvancedSecurityScanner(nil, &fakeServerDetails) + assert.NoError(t, err) + defer func() { + if scanner.scannerDirCleanupFunc != nil { + assert.NoError(t, scanner.scannerDirCleanupFunc()) + } + }() + applicabilityManager := newApplicabilityScanManager(fakeBasicXrayResults, fakeBasicDependencyGraph, scanner) + applicabilityManager.scanner.resultsFileName = filepath.Join("..", "..", "commands", "testdata", "applicability-scan", "applicable-cve-results.sarif") // Act results, err := applicabilityManager.getScanResults() // Assert - assert.NoError(t, applicabilityManagerError) assert.NoError(t, err) assert.Equal(t, 5, len(results)) assert.Equal(t, utils.ApplicableStringValue, results["testCve1"]) @@ -332,15 +408,21 @@ func TestParseResults_ApplicableCveExist(t *testing.T) { func TestParseResults_AllCvesNotApplicable(t *testing.T) { // Arrange - analyzerManagerExecuter = &analyzerManagerMock{} - applicabilityManager, _, applicabilityManagerError := newApplicabilityScanManager(fakeBasicXrayResults, fakeBasicDependencyGraph, &fakeServerDetails, nil, &analyzerManagerMock{}) - applicabilityManager.resultsFileName = filepath.Join("..", "..", "commands", "testdata", "applicability-scan", "no-applicable-cves-results.sarif") + assert.NoError(t, rtutils.DownloadAnalyzerManagerIfNeeded()) + scanner, err := NewAdvancedSecurityScanner(nil, &fakeServerDetails) + assert.NoError(t, err) + defer func() { + if scanner.scannerDirCleanupFunc != nil { + assert.NoError(t, scanner.scannerDirCleanupFunc()) + } + }() + applicabilityManager := newApplicabilityScanManager(fakeBasicXrayResults, fakeBasicDependencyGraph, scanner) + applicabilityManager.scanner.resultsFileName = filepath.Join("..", "..", "commands", "testdata", "applicability-scan", "no-applicable-cves-results.sarif") // Act results, err := applicabilityManager.getScanResults() // Assert - assert.NoError(t, applicabilityManagerError) assert.NoError(t, err) assert.Equal(t, 5, len(results)) for _, cveResult := range results { @@ -349,19 +431,12 @@ func TestParseResults_AllCvesNotApplicable(t *testing.T) { } func TestGetExtendedScanResults_AnalyzerManagerReturnsError(t *testing.T) { - defer func() { - analyzerManagerExecutionError = nil - }() - // Arrange - analyzerManagerErrorMessage := "analyzer manager failure message" - analyzerManagerExecutionError = errors.New(analyzerManagerErrorMessage) - analyzerManagerExecuter = &analyzerManagerMock{} - // Act + assert.NoError(t, rtutils.DownloadAnalyzerManagerIfNeeded()) extendedResults, err := GetExtendedScanResults(fakeBasicXrayResults, fakeBasicDependencyGraph, &fakeServerDetails, []coreutils.Technology{coreutils.Npm}, nil) // Assert assert.Error(t, err) - assert.Equal(t, "failed to run Applicability scan. Exit code received: analyzer manager failure message", err.Error()) + assert.ErrorContains(t, err, "failed to run Applicability scan") assert.Nil(t, extendedResults) } diff --git a/xray/audit/jas/iacscanner.go b/xray/audit/jas/iacscanner.go index 831acbb6e..629cd384f 100644 --- a/xray/audit/jas/iacscanner.go +++ b/xray/audit/jas/iacscanner.go @@ -1,28 +1,18 @@ package jas import ( - "errors" - "fmt" - "github.com/jfrog/jfrog-cli-core/v2/utils/config" "github.com/jfrog/jfrog-cli-core/v2/xray/utils" - "github.com/jfrog/jfrog-client-go/utils/io/fileutils" "github.com/jfrog/jfrog-client-go/utils/log" - "path/filepath" ) const ( - iacScannerType = "iac-scan-modules" - iacScanFailureMessage = "failed to run Infrastructure as Code scan. Cause: %s" - iacScanCommand = "iac" + iacScannerType = "iac-scan-modules" + iacScanCommand = "iac" ) type IacScanManager struct { iacScannerResults []utils.IacOrSecretResult - configFileName string - resultsFileName string - analyzerManager utils.AnalyzerManagerInterface - serverDetails *config.ServerDetails - workingDirs []string + scanner *AdvancedSecurityScanner } // The getIacScanResults function runs the iac scan flow, which includes the following steps: @@ -33,72 +23,38 @@ type IacScanManager struct { // []utils.IacOrSecretResult: a list of the iac violations that were found. // bool: true if the user is entitled to iac scan, false otherwise. // error: An error object (if any). -func getIacScanResults(serverDetails *config.ServerDetails, workingDirs []string, analyzerManager utils.AnalyzerManagerInterface) ([]utils.IacOrSecretResult, - bool, error) { - iacScanManager, cleanupFunc, err := newIacScanManager(serverDetails, workingDirs, analyzerManager) - if err != nil { - return nil, false, fmt.Errorf(iacScanFailureMessage, err.Error()) - } - defer func() { - if cleanupFunc != nil { - err = errors.Join(err, cleanupFunc()) - } - }() +func getIacScanResults(scanner *AdvancedSecurityScanner) (results []utils.IacOrSecretResult, err error) { + iacScanManager := newIacScanManager(scanner) log.Info("Running IaC scanning...") - if err = iacScanManager.run(); err != nil { - return nil, false, utils.ParseAnalyzerManagerError(utils.IaC, err) + if err = iacScanManager.scanner.Run(iacScanManager); err != nil { + err = utils.ParseAnalyzerManagerError(utils.IaC, err) + return } if len(iacScanManager.iacScannerResults) > 0 { log.Info("Found", len(iacScanManager.iacScannerResults), "IaC vulnerabilities") } - return iacScanManager.iacScannerResults, true, nil + results = iacScanManager.iacScannerResults + return } -func newIacScanManager(serverDetails *config.ServerDetails, workingDirs []string, analyzerManager utils.AnalyzerManagerInterface) (manager *IacScanManager, - cleanup func() error, err error) { - tempDir, err := fileutils.CreateTempDir() - if err != nil { - return - } - cleanup = func() error { - return fileutils.RemoveTempDir(tempDir) - } - fullPathWorkingDirs, err := utils.GetFullPathsWorkingDirs(workingDirs) - if err != nil { - return nil, cleanup, err - } +func newIacScanManager(scanner *AdvancedSecurityScanner) (manager *IacScanManager) { return &IacScanManager{ iacScannerResults: []utils.IacOrSecretResult{}, - configFileName: filepath.Join(tempDir, "config.yaml"), - resultsFileName: filepath.Join(tempDir, "results.sarif"), - analyzerManager: analyzerManager, - serverDetails: serverDetails, - workingDirs: fullPathWorkingDirs, - }, cleanup, nil -} - -func (iac *IacScanManager) run() (err error) { - for _, workingDir := range iac.workingDirs { - var currWdResults []utils.IacOrSecretResult - if currWdResults, err = iac.runIacScan(workingDir); err != nil { - return - } - iac.iacScannerResults = append(iac.iacScannerResults, currWdResults...) + scanner: scanner, } - return } -func (iac *IacScanManager) runIacScan(workingDir string) (results []utils.IacOrSecretResult, err error) { - defer func() { - err = errors.Join(err, deleteJasProcessFiles(iac.configFileName, iac.resultsFileName)) - }() - if err = iac.createConfigFile(workingDir); err != nil { +func (iac *IacScanManager) Run(wd string) (err error) { + scanner := iac.scanner + if err = iac.createConfigFile(wd); err != nil { return } if err = iac.runAnalyzerManager(); err != nil { return } - results, err = getIacOrSecretsScanResults(iac.resultsFileName, workingDir, false) + var workingDirResults []utils.IacOrSecretResult + workingDirResults, err = getIacOrSecretsScanResults(scanner.resultsFileName, wd, false) + iac.iacScannerResults = append(iac.iacScannerResults, workingDirResults...) return } @@ -118,15 +74,15 @@ func (iac *IacScanManager) createConfigFile(currentWd string) error { Scans: []iacScanConfiguration{ { Roots: []string{currentWd}, - Output: iac.resultsFileName, + Output: iac.scanner.resultsFileName, Type: iacScannerType, SkippedDirs: skippedDirs, }, }, } - return createScannersConfigFile(iac.configFileName, configFileContent) + return createScannersConfigFile(iac.scanner.configFileName, configFileContent) } func (iac *IacScanManager) runAnalyzerManager() error { - return iac.analyzerManager.Exec(iac.configFileName, iacScanCommand, iac.serverDetails) + return iac.scanner.analyzerManager.Exec(iac.scanner.configFileName, iacScanCommand, iac.scanner.serverDetails) } diff --git a/xray/audit/jas/iacscanner_test.go b/xray/audit/jas/iacscanner_test.go index b4b956482..430b75fdb 100644 --- a/xray/audit/jas/iacscanner_test.go +++ b/xray/audit/jas/iacscanner_test.go @@ -1,8 +1,8 @@ package jas import ( + rtutils "github.com/jfrog/jfrog-cli-core/v2/artifactory/utils" "github.com/jfrog/jfrog-cli-core/v2/utils/coreutils" - "github.com/jfrog/jfrog-cli-core/v2/xray/utils" "github.com/stretchr/testify/assert" "os" "path/filepath" @@ -11,65 +11,89 @@ import ( func TestNewIacScanManager(t *testing.T) { // Act - iacScanManager, _, err := newIacScanManager(&fakeServerDetails, []string{"currentDir"}, &analyzerManagerMock{}) + assert.NoError(t, rtutils.DownloadAnalyzerManagerIfNeeded()) + scanner, err := NewAdvancedSecurityScanner([]string{"currentDir"}, &fakeServerDetails) + assert.NoError(t, err) + defer func() { + if scanner.scannerDirCleanupFunc != nil { + assert.NoError(t, scanner.scannerDirCleanupFunc()) + } + }() + iacScanManager := newIacScanManager(scanner) // Assert assert.NoError(t, err) assert.NotEmpty(t, iacScanManager) - assert.NotEmpty(t, iacScanManager.configFileName) - assert.NotEmpty(t, iacScanManager.resultsFileName) - assert.NotEmpty(t, iacScanManager.workingDirs) - assert.Equal(t, &fakeServerDetails, iacScanManager.serverDetails) + assert.NotEmpty(t, iacScanManager.scanner.configFileName) + assert.NotEmpty(t, iacScanManager.scanner.resultsFileName) + assert.NotEmpty(t, iacScanManager.scanner.workingDirs) + assert.Equal(t, &fakeServerDetails, iacScanManager.scanner.serverDetails) } func TestIacScan_CreateConfigFile_VerifyFileWasCreated(t *testing.T) { - iacScanManager, _, iacManagerError := newIacScanManager(&fakeServerDetails, []string{"currentDir"}, &analyzerManagerMock{}) - assert.NoError(t, iacManagerError) + assert.NoError(t, rtutils.DownloadAnalyzerManagerIfNeeded()) + scanner, err := NewAdvancedSecurityScanner([]string{"currentDir"}, &fakeServerDetails) + assert.NoError(t, err) + defer func() { + if scanner.scannerDirCleanupFunc != nil { + assert.NoError(t, scanner.scannerDirCleanupFunc()) + } + }() + iacScanManager := newIacScanManager(scanner) currWd, err := coreutils.GetWorkingDirectory() assert.NoError(t, err) err = iacScanManager.createConfigFile(currWd) defer func() { - err = os.Remove(iacScanManager.configFileName) + err = os.Remove(iacScanManager.scanner.configFileName) assert.NoError(t, err) }() - _, fileNotExistError := os.Stat(iacScanManager.configFileName) + _, fileNotExistError := os.Stat(iacScanManager.scanner.configFileName) assert.NoError(t, fileNotExistError) - fileContent, err := os.ReadFile(iacScanManager.configFileName) + fileContent, err := os.ReadFile(iacScanManager.scanner.configFileName) assert.NoError(t, err) assert.True(t, len(fileContent) > 0) } func TestIacParseResults_EmptyResults(t *testing.T) { // Arrange - fullPathWorkingDirs, err := utils.GetFullPathsWorkingDirs(nil) + assert.NoError(t, rtutils.DownloadAnalyzerManagerIfNeeded()) + scanner, err := NewAdvancedSecurityScanner(nil, &fakeServerDetails) assert.NoError(t, err) - iacScanManager, _, iacManagerError := newIacScanManager(&fakeServerDetails, fullPathWorkingDirs, &analyzerManagerMock{}) - iacScanManager.resultsFileName = filepath.Join("..", "..", "commands", "testdata", "iac-scan", "no-violations.sarif") + defer func() { + if scanner.scannerDirCleanupFunc != nil { + assert.NoError(t, scanner.scannerDirCleanupFunc()) + } + }() + iacScanManager := newIacScanManager(scanner) + iacScanManager.scanner.resultsFileName = filepath.Join("..", "..", "commands", "testdata", "iac-scan", "no-violations.sarif") // Act - iacScanManager.iacScannerResults, err = getIacOrSecretsScanResults(iacScanManager.resultsFileName, fullPathWorkingDirs[0], false) + iacScanManager.iacScannerResults, err = getIacOrSecretsScanResults(iacScanManager.scanner.resultsFileName, scanner.workingDirs[0], false) // Assert - assert.NoError(t, iacManagerError) assert.NoError(t, err) assert.Empty(t, iacScanManager.iacScannerResults) } -func TestIacParseResults_ResultsContainSecrets(t *testing.T) { +func TestIacParseResults_ResultsContainIacViolations(t *testing.T) { // Arrange - fullPathWorkingDirs, err := utils.GetFullPathsWorkingDirs(nil) + scanner, err := NewAdvancedSecurityScanner(nil, &fakeServerDetails) assert.NoError(t, err) - iacScanManager, _, iacManagerError := newIacScanManager(&fakeServerDetails, fullPathWorkingDirs, &analyzerManagerMock{}) - iacScanManager.resultsFileName = filepath.Join("..", "..", "commands", "testdata", "iac-scan", "contains-iac-violations.sarif") + defer func() { + if scanner.scannerDirCleanupFunc != nil { + assert.NoError(t, scanner.scannerDirCleanupFunc()) + } + }() + iacScanManager := newIacScanManager(scanner) + iacScanManager.scanner.resultsFileName = filepath.Join("..", "..", "commands", "testdata", "iac-scan", "contains-iac-violations.sarif") // Act - iacScanManager.iacScannerResults, err = getIacOrSecretsScanResults(iacScanManager.resultsFileName, fullPathWorkingDirs[0], false) + iacScanManager.iacScannerResults, err = getIacOrSecretsScanResults(iacScanManager.scanner.resultsFileName, scanner.workingDirs[0], false) // Assert - assert.NoError(t, iacManagerError) assert.NoError(t, err) assert.NotEmpty(t, iacScanManager.iacScannerResults) assert.Equal(t, 4, len(iacScanManager.iacScannerResults)) diff --git a/xray/audit/jas/jasmanager.go b/xray/audit/jas/jasmanager.go index 10e10c3e6..441b398cd 100644 --- a/xray/audit/jas/jasmanager.go +++ b/xray/audit/jas/jasmanager.go @@ -1,6 +1,7 @@ package jas import ( + "errors" "github.com/jfrog/jfrog-cli-core/v2/utils/config" "github.com/jfrog/jfrog-cli-core/v2/utils/coreutils" "github.com/jfrog/jfrog-cli-core/v2/xray/utils" @@ -12,50 +13,93 @@ import ( "github.com/owenrumney/go-sarif/v2/sarif" "gopkg.in/yaml.v3" "os" + "path/filepath" ) var ( - analyzerManagerExecuter utils.AnalyzerManagerInterface = &utils.AnalyzerManager{} - skippedDirs = []string{"**/*test*/**", "**/*venv*/**", "**/*node_modules*/**", "**/*target*/**"} + skippedDirs = []string{"**/*test*/**", "**/*venv*/**", "**/*node_modules*/**", "**/*target*/**"} ) +type ScannerCmd interface { + Run(wd string) (err error) +} + +type AdvancedSecurityScanner struct { + configFileName string + resultsFileName string + analyzerManager utils.AnalyzerManager + serverDetails *config.ServerDetails + workingDirs []string + scannerDirCleanupFunc func() error +} + +func NewAdvancedSecurityScanner(workingDirs []string, serverDetails *config.ServerDetails) (scanner *AdvancedSecurityScanner, err error) { + scanner = &AdvancedSecurityScanner{} + if scanner.analyzerManager.AnalyzerManagerFullPath, err = utils.GetAnalyzerManagerExecutable(); err != nil { + return + } + var tempDir string + if tempDir, err = fileutils.CreateTempDir(); err != nil { + return + } + scanner.scannerDirCleanupFunc = func() error { + return fileutils.RemoveTempDir(tempDir) + } + scanner.serverDetails = serverDetails + scanner.configFileName = filepath.Join(tempDir, "config.yaml") + scanner.resultsFileName = filepath.Join(tempDir, "results.sarif") + scanner.workingDirs, err = utils.GetFullPathsWorkingDirs(workingDirs) + return +} + +func (a *AdvancedSecurityScanner) Run(scannerCmd ScannerCmd) (err error) { + for _, workingDir := range a.workingDirs { + func() { + defer func() { + err = errors.Join(err, deleteJasProcessFiles(a.configFileName, a.resultsFileName)) + }() + if err = scannerCmd.Run(workingDir); err != nil { + return + } + }() + } + return +} + func GetExtendedScanResults(xrayResults []services.ScanResponse, dependencyTrees []*xrayUtils.GraphNode, serverDetails *config.ServerDetails, scannedTechnologies []coreutils.Technology, workingDirs []string) (*utils.ExtendedScanResults, error) { if serverDetails == nil || len(serverDetails.Url) == 0 { log.Warn("To include 'Advanced Security' scan as part of the audit output, please run the 'jf c add' command before running this command.") return &utils.ExtendedScanResults{XrayResults: xrayResults}, nil } - analyzerManagerExist, err := analyzerManagerExecuter.ExistLocally() + scanner, err := NewAdvancedSecurityScanner(workingDirs, serverDetails) if err != nil { - return &utils.ExtendedScanResults{XrayResults: xrayResults}, err - } - if !analyzerManagerExist { - log.Debug("Since the 'Analyzer Manager' doesn't exist locally, its execution is skipped.") - return &utils.ExtendedScanResults{XrayResults: xrayResults}, nil + return nil, err } - applicabilityScanResults, eligibleForApplicabilityScan, err := getApplicabilityScanResults(xrayResults, - dependencyTrees, serverDetails, scannedTechnologies, workingDirs, analyzerManagerExecuter) + defer func() { + cleanup := scanner.scannerDirCleanupFunc + err = errors.Join(err, cleanup()) + }() + applicabilityScanResults, err := getApplicabilityScanResults( + xrayResults, dependencyTrees, scannedTechnologies, scanner) if err != nil { return nil, err } - secretsScanResults, eligibleForSecretsScan, err := getSecretsScanResults(serverDetails, workingDirs, analyzerManagerExecuter) + secretsScanResults, err := getSecretsScanResults(scanner) if err != nil { return nil, err } - iacScanResults, eligibleForIacScan, err := getIacScanResults(serverDetails, workingDirs, analyzerManagerExecuter) + iacScanResults, err := getIacScanResults(scanner) if err != nil { return nil, err } return &utils.ExtendedScanResults{ - EntitledForJas: true, - XrayResults: xrayResults, - ScannedTechnologies: scannedTechnologies, - ApplicabilityScanResults: applicabilityScanResults, - EligibleForApplicabilityScan: eligibleForApplicabilityScan, - SecretsScanResults: secretsScanResults, - EligibleForSecretScan: eligibleForSecretsScan, - IacScanResults: iacScanResults, - EligibleForIacScan: eligibleForIacScan, + EntitledForJas: true, + XrayResults: xrayResults, + ScannedTechnologies: scannedTechnologies, + ApplicabilityScanResults: applicabilityScanResults, + SecretsScanResults: secretsScanResults, + IacScanResults: iacScanResults, }, nil } diff --git a/xray/audit/jas/jasmanager_test.go b/xray/audit/jas/jasmanager_test.go index c8350a505..df4622ba8 100644 --- a/xray/audit/jas/jasmanager_test.go +++ b/xray/audit/jas/jasmanager_test.go @@ -1,30 +1,17 @@ package jas import ( + "os" + "testing" + "github.com/jfrog/jfrog-cli-core/v2/utils/config" "github.com/jfrog/jfrog-cli-core/v2/utils/coreutils" + "github.com/jfrog/jfrog-client-go/utils/io/fileutils" "github.com/jfrog/jfrog-client-go/xray/services" xrayUtils "github.com/jfrog/jfrog-client-go/xray/services/utils" "github.com/stretchr/testify/assert" - "testing" -) - -var ( - analyzerManagerExecutionError error = nil - analyzerManagerExists = true ) -type analyzerManagerMock struct { -} - -func (am *analyzerManagerMock) Exec(string, string, *config.ServerDetails) error { - return analyzerManagerExecutionError -} - -func (am *analyzerManagerMock) ExistLocally() (bool, error) { - return analyzerManagerExists, nil -} - var fakeBasicXrayResults = []services.ScanResponse{ { ScanId: "scanId_1", @@ -75,18 +62,20 @@ var fakeServerDetails = config.ServerDetails{ } func TestGetExtendedScanResults_AnalyzerManagerDoesntExist(t *testing.T) { - // Arrange - analyzerManagerExists = false - analyzerManagerExecuter = &analyzerManagerMock{} - - // Act + tmpDir, err := fileutils.CreateTempDir() + defer func() { + assert.NoError(t, fileutils.RemoveTempDir(tmpDir)) + }() + assert.NoError(t, err) + assert.NoError(t, os.Setenv(coreutils.HomeDir, tmpDir)) + defer func() { + assert.NoError(t, os.Unsetenv(coreutils.HomeDir)) + }() extendedResults, err := GetExtendedScanResults(fakeBasicXrayResults, fakeBasicDependencyGraph, &fakeServerDetails, []coreutils.Technology{coreutils.Yarn}, nil) // Assert - assert.NoError(t, err) - assert.False(t, extendedResults.EntitledForJas) - assert.Equal(t, 1, len(extendedResults.XrayResults)) - assert.Nil(t, extendedResults.ApplicabilityScanResults) + assert.Error(t, err) + assert.Nil(t, extendedResults) } func TestGetExtendedScanResults_ServerNotValid(t *testing.T) { diff --git a/xray/audit/jas/secretsscanner.go b/xray/audit/jas/secretsscanner.go index 5cfda696a..cd1159a49 100644 --- a/xray/audit/jas/secretsscanner.go +++ b/xray/audit/jas/secretsscanner.go @@ -1,29 +1,20 @@ package jas import ( - "errors" - "fmt" - "path/filepath" + "strings" - "github.com/jfrog/jfrog-cli-core/v2/utils/config" "github.com/jfrog/jfrog-cli-core/v2/xray/utils" - "github.com/jfrog/jfrog-client-go/utils/io/fileutils" "github.com/jfrog/jfrog-client-go/utils/log" ) const ( - secretsScanCommand = "sec" - secretsScannerType = "secrets-scan" - secScanFailureMessage = "failed to run secrets scan. Cause: %s" + secretsScanCommand = "sec" + secretsScannerType = "secrets-scan" ) type SecretScanManager struct { secretsScannerResults []utils.IacOrSecretResult - configFileName string - resultsFileName string - analyzerManager utils.AnalyzerManagerInterface - serverDetails *config.ServerDetails - workingDirs []string + scanner *AdvancedSecurityScanner } // The getSecretsScanResults function runs the secrets scan flow, which includes the following steps: @@ -32,74 +23,39 @@ type SecretScanManager struct { // Parsing the analyzer manager results. // Return values: // []utils.IacOrSecretResult: a list of the secrets that were found. -// bool: true if the user is entitled to secrets scan, false otherwise. // error: An error object (if any). -func getSecretsScanResults(serverDetails *config.ServerDetails, workingDirs []string, analyzerManager utils.AnalyzerManagerInterface) ([]utils.IacOrSecretResult, - bool, error) { - secretScanManager, cleanupFunc, err := newSecretsScanManager(serverDetails, workingDirs, analyzerManager) - if err != nil { - return nil, false, fmt.Errorf(secScanFailureMessage, err.Error()) - } - defer func() { - if cleanupFunc != nil { - err = errors.Join(err, cleanupFunc()) - } - }() +func getSecretsScanResults(scanner *AdvancedSecurityScanner) (results []utils.IacOrSecretResult, err error) { + secretScanManager := newSecretsScanManager(scanner) log.Info("Running secrets scanning...") - if err = secretScanManager.run(); err != nil { - return nil, false, utils.ParseAnalyzerManagerError(utils.Secrets, err) + if err = secretScanManager.scanner.Run(secretScanManager); err != nil { + err = utils.ParseAnalyzerManagerError(utils.Secrets, err) + return } - if len(secretScanManager.secretsScannerResults) > 0 { - log.Info(len(secretScanManager.secretsScannerResults), "secrets were found") + results = secretScanManager.secretsScannerResults + if len(results) > 0 { + log.Info(len(results), "secrets were found") } - return secretScanManager.secretsScannerResults, true, nil + return } -func newSecretsScanManager(serverDetails *config.ServerDetails, workingDirs []string, analyzerManager utils.AnalyzerManagerInterface) (manager *SecretScanManager, - cleanup func() error, err error) { - tempDir, err := fileutils.CreateTempDir() - if err != nil { - return - } - cleanup = func() error { - return fileutils.RemoveTempDir(tempDir) - } - fullPathWorkingDirs, err := utils.GetFullPathsWorkingDirs(workingDirs) - if err != nil { - return nil, cleanup, err - } +func newSecretsScanManager(scanner *AdvancedSecurityScanner) (manager *SecretScanManager) { return &SecretScanManager{ secretsScannerResults: []utils.IacOrSecretResult{}, - configFileName: filepath.Join(tempDir, "config.yaml"), - resultsFileName: filepath.Join(tempDir, "results.sarif"), - analyzerManager: analyzerManager, - serverDetails: serverDetails, - workingDirs: fullPathWorkingDirs, - }, cleanup, nil -} - -func (s *SecretScanManager) run() (err error) { - for _, workingDir := range s.workingDirs { - var workingDirResults []utils.IacOrSecretResult - if workingDirResults, err = s.runSecretsScan(workingDir); err != nil { - return - } - s.secretsScannerResults = append(s.secretsScannerResults, workingDirResults...) + scanner: scanner, } - return } -func (s *SecretScanManager) runSecretsScan(workingDir string) (results []utils.IacOrSecretResult, err error) { - defer func() { - err = errors.Join(err, deleteJasProcessFiles(s.configFileName, s.resultsFileName)) - }() - if err = s.createConfigFile(workingDir); err != nil { +func (s *SecretScanManager) Run(wd string) (err error) { + scanner := s.scanner + if err = s.createConfigFile(wd); err != nil { return } if err = s.runAnalyzerManager(); err != nil { return } - results, err = getIacOrSecretsScanResults(s.resultsFileName, workingDir, true) + var workingDirResults []utils.IacOrSecretResult + workingDirResults, err = getIacOrSecretsScanResults(scanner.resultsFileName, wd, true) + s.secretsScannerResults = append(s.secretsScannerResults, workingDirResults...) return } @@ -119,32 +75,22 @@ func (s *SecretScanManager) createConfigFile(currentWd string) error { Scans: []secretsScanConfiguration{ { Roots: []string{currentWd}, - Output: s.resultsFileName, + Output: s.scanner.resultsFileName, Type: secretsScannerType, SkippedDirs: skippedDirs, }, }, } - return createScannersConfigFile(s.configFileName, configFileContent) + return createScannersConfigFile(s.scanner.configFileName, configFileContent) } func (s *SecretScanManager) runAnalyzerManager() error { - return s.analyzerManager.Exec(s.configFileName, secretsScanCommand, s.serverDetails) + return s.scanner.analyzerManager.Exec(s.scanner.configFileName, secretsScanCommand, s.scanner.serverDetails) } func hideSecret(secret string) string { if len(secret) <= 3 { return "***" } - hiddenSecret := "" - i := 0 - for i < 3 { // Show first 3 digits - hiddenSecret += string(secret[i]) - i++ - } - for i < 15 { - hiddenSecret += "*" - i++ - } - return hiddenSecret + return secret[:3] + strings.Repeat("*", 12) } diff --git a/xray/audit/jas/secretsscanner_test.go b/xray/audit/jas/secretsscanner_test.go index 4118a5576..9ec80fa61 100644 --- a/xray/audit/jas/secretsscanner_test.go +++ b/xray/audit/jas/secretsscanner_test.go @@ -1,7 +1,7 @@ package jas import ( - "errors" + rtutils "github.com/jfrog/jfrog-cli-core/v2/artifactory/utils" "github.com/jfrog/jfrog-cli-core/v2/utils/coreutils" "github.com/stretchr/testify/assert" "os" @@ -10,20 +10,33 @@ import ( ) func TestNewSecretsScanManager(t *testing.T) { - // Act - secretScanManager, _, err := newSecretsScanManager(&fakeServerDetails, nil, &analyzerManagerMock{}) + assert.NoError(t, rtutils.DownloadAnalyzerManagerIfNeeded()) + scanner, err := NewAdvancedSecurityScanner(nil, &fakeServerDetails) + assert.NoError(t, err) + defer func() { + if scanner.scannerDirCleanupFunc != nil { + assert.NoError(t, scanner.scannerDirCleanupFunc()) + } + }() + secretScanManager := newSecretsScanManager(scanner) - // Assert assert.NoError(t, err) assert.NotEmpty(t, secretScanManager) - assert.NotEmpty(t, secretScanManager.configFileName) - assert.NotEmpty(t, secretScanManager.resultsFileName) - assert.Equal(t, &fakeServerDetails, secretScanManager.serverDetails) + assert.NotEmpty(t, secretScanManager.scanner.configFileName) + assert.NotEmpty(t, secretScanManager.scanner.resultsFileName) + assert.Equal(t, &fakeServerDetails, secretScanManager.scanner.serverDetails) } func TestSecretsScan_CreateConfigFile_VerifyFileWasCreated(t *testing.T) { - secretScanManager, _, secretsManagerError := newSecretsScanManager(&fakeServerDetails, nil, &analyzerManagerMock{}) - assert.NoError(t, secretsManagerError) + assert.NoError(t, rtutils.DownloadAnalyzerManagerIfNeeded()) + scanner, err := NewAdvancedSecurityScanner(nil, &fakeServerDetails) + assert.NoError(t, err) + defer func() { + if scanner.scannerDirCleanupFunc != nil { + assert.NoError(t, scanner.scannerDirCleanupFunc()) + } + }() + secretScanManager := newSecretsScanManager(scanner) currWd, err := coreutils.GetWorkingDirectory() assert.NoError(t, err) @@ -31,13 +44,13 @@ func TestSecretsScan_CreateConfigFile_VerifyFileWasCreated(t *testing.T) { assert.NoError(t, err) defer func() { - err = os.Remove(secretScanManager.configFileName) + err = os.Remove(secretScanManager.scanner.configFileName) assert.NoError(t, err) }() - _, fileNotExistError := os.Stat(secretScanManager.configFileName) + _, fileNotExistError := os.Stat(secretScanManager.scanner.configFileName) assert.NoError(t, fileNotExistError) - fileContent, err := os.ReadFile(secretScanManager.configFileName) + fileContent, err := os.ReadFile(secretScanManager.scanner.configFileName) assert.NoError(t, err) assert.True(t, len(fileContent) > 0) } @@ -45,71 +58,83 @@ func TestSecretsScan_CreateConfigFile_VerifyFileWasCreated(t *testing.T) { func TestRunAnalyzerManager_ReturnsGeneralError(t *testing.T) { defer func() { os.Clearenv() - analyzerManagerExecutionError = nil }() // Arrange - analyzerManagerExecutionError = errors.New("analyzer manager error") - secretScanManager, _, secretsManagerError := newSecretsScanManager(&fakeServerDetails, nil, &analyzerManagerMock{}) + assert.NoError(t, rtutils.DownloadAnalyzerManagerIfNeeded()) + scanner, err := NewAdvancedSecurityScanner(nil, &fakeServerDetails) + assert.NoError(t, err) + defer func() { + if scanner.scannerDirCleanupFunc != nil { + assert.NoError(t, scanner.scannerDirCleanupFunc()) + } + }() + secretScanManager := newSecretsScanManager(scanner) // Act - err := secretScanManager.runAnalyzerManager() + err = secretScanManager.runAnalyzerManager() // Assert - assert.NoError(t, secretsManagerError) assert.Error(t, err) - assert.Equal(t, analyzerManagerExecutionError.Error(), err.Error()) } func TestParseResults_EmptyResults(t *testing.T) { // Arrange - secretScanManager, _, secretsManagerError := newSecretsScanManager(&fakeServerDetails, []string{"am_versions_for_leap"}, &analyzerManagerMock{}) - secretScanManager.resultsFileName = filepath.Join("..", "..", "commands", "testdata", "secrets-scan", "no-secrets.sarif") + assert.NoError(t, rtutils.DownloadAnalyzerManagerIfNeeded()) + scanner, err := NewAdvancedSecurityScanner(nil, &fakeServerDetails) + assert.NoError(t, err) + defer func() { + if scanner.scannerDirCleanupFunc != nil { + assert.NoError(t, scanner.scannerDirCleanupFunc()) + } + }() + secretScanManager := newSecretsScanManager(scanner) + secretScanManager.scanner.resultsFileName = filepath.Join("..", "..", "commands", "testdata", "secrets-scan", "no-secrets.sarif") // Act - var err error - secretScanManager.secretsScannerResults, err = getIacOrSecretsScanResults(secretScanManager.resultsFileName, "am_versions_for_leap", false) + secretScanManager.secretsScannerResults, err = getIacOrSecretsScanResults(secretScanManager.scanner.resultsFileName, scanner.workingDirs[0], false) // Assert - assert.NoError(t, secretsManagerError) assert.NoError(t, err) assert.Empty(t, secretScanManager.secretsScannerResults) } func TestParseResults_ResultsContainSecrets(t *testing.T) { // Arrange - - secretScanManager, _, secretsManagerError := newSecretsScanManager(&fakeServerDetails, []string{"secrets_scanner"}, &analyzerManagerMock{}) - secretScanManager.resultsFileName = filepath.Join("..", "..", "commands", "testdata", "secrets-scan", "contain-secrets.sarif") + assert.NoError(t, rtutils.DownloadAnalyzerManagerIfNeeded()) + scanner, err := NewAdvancedSecurityScanner(nil, &fakeServerDetails) + assert.NoError(t, err) + defer func() { + if scanner.scannerDirCleanupFunc != nil { + assert.NoError(t, scanner.scannerDirCleanupFunc()) + } + }() + secretScanManager := newSecretsScanManager(scanner) + secretScanManager.scanner.resultsFileName = filepath.Join("..", "..", "commands", "testdata", "secrets-scan", "contain-secrets.sarif") // Act - var err error - secretScanManager.secretsScannerResults, err = getIacOrSecretsScanResults(secretScanManager.resultsFileName, "secrets_scanner", false) + secretScanManager.secretsScannerResults, err = getIacOrSecretsScanResults(secretScanManager.scanner.resultsFileName, scanner.workingDirs[0], false) // Assert - assert.NoError(t, secretsManagerError) assert.NoError(t, err) assert.NotEmpty(t, secretScanManager.secretsScannerResults) assert.Equal(t, 7, len(secretScanManager.secretsScannerResults)) } func TestGetSecretsScanResults_AnalyzerManagerReturnsError(t *testing.T) { + assert.NoError(t, rtutils.DownloadAnalyzerManagerIfNeeded()) + scanner, err := NewAdvancedSecurityScanner(nil, &fakeServerDetails) + assert.NoError(t, err) defer func() { - analyzerManagerExecutionError = nil + if scanner.scannerDirCleanupFunc != nil { + assert.NoError(t, scanner.scannerDirCleanupFunc()) + } }() + secretsResults, err := getSecretsScanResults(scanner) - // Arrange - analyzerManagerErrorMessage := "analyzer manager failure message" - analyzerManagerExecutionError = errors.New(analyzerManagerErrorMessage) - - // Act - secretsResults, entitledForSecrets, err := getSecretsScanResults(&fakeServerDetails, nil, &analyzerManagerMock{}) - - // Assert assert.Error(t, err) - assert.Equal(t, "failed to run Secrets scan. Exit code received: analyzer manager failure message", err.Error()) + assert.ErrorContains(t, err, "failed to run Secrets scan") assert.Nil(t, secretsResults) - assert.False(t, entitledForSecrets) } func TestHideSecret(t *testing.T) { diff --git a/xray/commands/audit/generic/auditmanager.go b/xray/commands/audit/generic/auditmanager.go index 7fe2f65fd..5418e1e17 100644 --- a/xray/commands/audit/generic/auditmanager.go +++ b/xray/commands/audit/generic/auditmanager.go @@ -151,7 +151,7 @@ func RunAudit(auditParams *Params) (results *Results, err error) { if isEntitled { xrayScanResults := results.ExtendedScanResults.XrayResults scannedTechnologies := results.ScannedTechnologies - results.ExtendedScanResults, err = jas.GetExtendedScanResults(xrayScanResults, auditParams.FullDependenciesTree(), serverDetails, scannedTechnologies, auditParams.WorkingDirs()) + results.ExtendedScanResults, err = jas.GetExtendedScanResults(xrayScanResults, auditParams.FullDependenciesTree(), serverDetails, scannedTechnologies, auditParams.workingDirs) } return } diff --git a/xray/utils/analyzermanager.go b/xray/utils/analyzermanager.go index b6f61cda9..e81a90cd3 100644 --- a/xray/utils/analyzermanager.go +++ b/xray/utils/analyzermanager.go @@ -3,6 +3,13 @@ package utils import ( "errors" "fmt" + "os" + "os/exec" + "path" + "path/filepath" + "strconv" + "strings" + "github.com/jfrog/jfrog-cli-core/v2/utils/config" "github.com/jfrog/jfrog-cli-core/v2/utils/coreutils" "github.com/jfrog/jfrog-client-go/utils/errorutils" @@ -10,12 +17,6 @@ import ( "github.com/jfrog/jfrog-client-go/utils/log" "github.com/jfrog/jfrog-client-go/xray/services" "github.com/owenrumney/go-sarif/v2/sarif" - "os" - "os/exec" - "path" - "path/filepath" - "strconv" - "strings" ) var ( @@ -79,52 +80,27 @@ type IacOrSecretResult struct { } type ExtendedScanResults struct { - XrayResults []services.ScanResponse - ScannedTechnologies []coreutils.Technology - ApplicabilityScanResults map[string]string - SecretsScanResults []IacOrSecretResult - IacScanResults []IacOrSecretResult - EntitledForJas bool - EligibleForApplicabilityScan bool - EligibleForSecretScan bool - EligibleForIacScan bool + XrayResults []services.ScanResponse + ScannedTechnologies []coreutils.Technology + ApplicabilityScanResults map[string]string + SecretsScanResults []IacOrSecretResult + IacScanResults []IacOrSecretResult + EntitledForJas bool } func (e *ExtendedScanResults) getXrayScanResults() []services.ScanResponse { return e.XrayResults } -// AnalyzerManagerInterface represents the analyzer manager executable file that exists locally as a Jfrog dependency. -// It triggers JAS capabilities by verifying user's entitlements and running the JAS scanners. -// Analyzer manager input: -// - scan command: ca (contextual analysis) / sec (secrets) / iac -// - path to configuration file -// -// Analyzer manager output: -// - sarif file containing the scan results -type AnalyzerManagerInterface interface { - ExistLocally() (bool, error) - Exec(string, string, *config.ServerDetails) error -} - type AnalyzerManager struct { - analyzerManagerFullPath string -} - -func (am *AnalyzerManager) ExistLocally() (bool, error) { - analyzerManagerPath, err := getAnalyzerManagerExecutable() - if err != nil { - return false, err - } - am.analyzerManagerFullPath = analyzerManagerPath - return fileutils.IsFileExists(analyzerManagerPath, false) + AnalyzerManagerFullPath string } func (am *AnalyzerManager) Exec(configFile, scanCommand string, serverDetails *config.ServerDetails) (err error) { if err = SetAnalyzerManagerEnvVariables(serverDetails); err != nil { return err } - cmd := exec.Command(am.analyzerManagerFullPath, scanCommand, configFile) + cmd := exec.Command(am.AnalyzerManagerFullPath, scanCommand, configFile) defer func() { if !cmd.ProcessState.Exited() { if killProcessError := cmd.Process.Kill(); errorutils.CheckError(killProcessError) != nil { @@ -132,7 +108,7 @@ func (am *AnalyzerManager) Exec(configFile, scanCommand string, serverDetails *c } } }() - cmd.Dir = filepath.Dir(am.analyzerManagerFullPath) + cmd.Dir = filepath.Dir(am.AnalyzerManagerFullPath) err = cmd.Run() return errorutils.CheckError(err) } @@ -153,12 +129,20 @@ func GetAnalyzerManagerDirAbsolutePath() (string, error) { return filepath.Join(jfrogDir, analyzerManagerDirName), nil } -func getAnalyzerManagerExecutable() (string, error) { +func GetAnalyzerManagerExecutable() (analyzerManagerPath string, err error) { analyzerManagerDir, err := GetAnalyzerManagerDirAbsolutePath() if err != nil { return "", err } - return filepath.Join(analyzerManagerDir, GetAnalyzerManagerExecutableName()), nil + analyzerManagerPath = filepath.Join(analyzerManagerDir, GetAnalyzerManagerExecutableName()) + var exists bool + if exists, err = fileutils.IsFileExists(analyzerManagerPath, false); err != nil { + return + } + if !exists { + err = errors.New("unable to locate the analyzer manager package. Advanced security scans cannot be performed without this package") + } + return analyzerManagerPath, err } func GetAnalyzerManagerExecutableName() string { diff --git a/xray/utils/resultwriter.go b/xray/utils/resultwriter.go index 90cc2fafc..b01900082 100644 --- a/xray/utils/resultwriter.go +++ b/xray/utils/resultwriter.go @@ -108,10 +108,10 @@ func printScanResultsTables(results *ExtendedScanResults, scan, includeVulnerabi return } } - if err = PrintSecretsTable(results.SecretsScanResults, results.EligibleForSecretScan); err != nil { + if err = PrintSecretsTable(results.SecretsScanResults, results.EntitledForJas); err != nil { return } - return PrintIacTable(results.IacScanResults, results.EligibleForIacScan) + return PrintIacTable(results.IacScanResults, results.EntitledForJas) } func printMessages(messages []string) {