Skip to content

Commit

Permalink
review changes
Browse files Browse the repository at this point in the history
  • Loading branch information
@attiasas
attiasas committed Aug 30, 2023
1 parent 1568e84 commit b1383d2
Show file tree
Hide file tree
Showing 12 changed files with 106 additions and 161 deletions.
10 changes: 7 additions & 3 deletions xray/audit/jas/applicabilitymanager.go
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
package jas

import (
"path/filepath"
"strings"

"github.com/jfrog/gofrog/datastructures"
"github.com/jfrog/jfrog-cli-core/v2/utils/coreutils"
"github.com/jfrog/jfrog-cli-core/v2/xray/utils"
Expand All @@ -10,7 +13,6 @@ import (
"github.com/owenrumney/go-sarif/v2/sarif"
"golang.org/x/exp/maps"
"golang.org/x/exp/slices"
"strings"
)

const (
Expand Down Expand Up @@ -109,7 +111,9 @@ func (a *ApplicabilityScanManager) Run(wd string) (err error) {
return
}
var workingDirResults map[string]string
workingDirResults, err = a.getScanResults()
if workingDirResults, err = a.getScanResults(); err != nil {
return
}
for cve, result := range workingDirResults {
a.applicabilityScanResults[cve] = result
}
Expand Down Expand Up @@ -156,7 +160,7 @@ func (a *ApplicabilityScanManager) createConfigFile(workingDir string) error {
// 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.scanner.analyzerManager.Exec(a.scanner.configFileName, applicabilityScanCommand, a.scanner.analyzerManager.GetAnalyzerManagerDir(), a.scanner.serverDetails)
return a.scanner.analyzerManager.Exec(a.scanner.configFileName, applicabilityScanCommand, filepath.Dir(a.scanner.analyzerManager.AnalyzerManagerFullPath), a.scanner.serverDetails)
}

func (a *ApplicabilityScanManager) getScanResults() (map[string]string, error) {
Expand Down
188 changes: 62 additions & 126 deletions xray/audit/jas/applicabilitymanager_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,48 +12,37 @@ import (
)

func TestNewApplicabilityScanManager_InputIsValid(t *testing.T) {
scanner, cleanUp := initJasTest(t)
defer cleanUp()
// Act
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, mockDirectDependencies, scanner)

// Assert
assert.NotEmpty(t, applicabilityManager)
assert.NotEmpty(t, applicabilityManager.scanner.configFileName)
assert.NotEmpty(t, applicabilityManager.scanner.resultsFileName)
assert.Equal(t, applicabilityManager.directDependenciesCves.Size(), 5)
if assert.NotNil(t, applicabilityManager) {
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) {
scanner, cleanUp := initJasTest(t)
defer cleanUp()
// Act
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.NotEmpty(t, applicabilityManager)
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)
if assert.NotNil(t, applicabilityManager) {
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",
Expand All @@ -75,47 +64,36 @@ func TestNewApplicabilityScanManager_NoDirectDependenciesInScan(t *testing.T) {
fakeBasicXrayResults[0].Violations[0].Components["issueId_2_non_direct_dependency"] = services.Component{}

// Act
scanner, err := NewAdvancedSecurityScanner(nil, &fakeServerDetails)
assert.NoError(t, err)
defer func() {
if scanner.scannerDirCleanupFunc != nil {
assert.NoError(t, scanner.scannerDirCleanupFunc())
}
}()
scanner, cleanUp := initJasTest(t)
defer cleanUp()
applicabilityManager := newApplicabilityScanManager(noDirectDependenciesResults, mockDirectDependencies, scanner)

// Assert
assert.NotEmpty(t, applicabilityManager)
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())
if assert.NotNil(t, applicabilityManager) {
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())

scanner, cleanUp := initJasTest(t)
defer cleanUp()
// Act
scanner, err := NewAdvancedSecurityScanner(nil, &fakeServerDetails)
assert.NoError(t, err)
defer func() {
if scanner.scannerDirCleanupFunc != nil {
assert.NoError(t, scanner.scannerDirCleanupFunc())
}
}()
applicabilityManager := newApplicabilityScanManager(fakeBasicXrayResults, mockMultiRootDirectDependencies, scanner)

// Assert
assert.NotEmpty(t, applicabilityManager)
assert.NotEmpty(t, applicabilityManager.scanner.configFileName)
assert.NotEmpty(t, applicabilityManager.scanner.resultsFileName)
assert.Equal(t, 5, applicabilityManager.directDependenciesCves.Size())
if assert.NotNil(t, applicabilityManager) {
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",
Expand All @@ -126,28 +104,22 @@ func TestNewApplicabilityScanManager_ViolationsDontExistInResults(t *testing.T)
},
},
}
scanner, cleanUp := initJasTest(t)
defer cleanUp()

// Act
scanner, err := NewAdvancedSecurityScanner(nil, &fakeServerDetails)
assert.NoError(t, err)
defer func() {
if scanner.scannerDirCleanupFunc != nil {
assert.NoError(t, scanner.scannerDirCleanupFunc())
}
}()
applicabilityManager := newApplicabilityScanManager(noViolationScanResponse, mockDirectDependencies, scanner)

// Assert
assert.NoError(t, err)
assert.NotEmpty(t, applicabilityManager)
assert.NotEmpty(t, applicabilityManager.scanner.configFileName)
assert.NotEmpty(t, applicabilityManager.scanner.resultsFileName)
assert.Equal(t, 3, applicabilityManager.directDependenciesCves.Size())
if assert.NotNil(t, applicabilityManager) {
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",
Expand All @@ -158,33 +130,24 @@ func TestNewApplicabilityScanManager_VulnerabilitiesDontExist(t *testing.T) {
},
},
}
scanner, cleanUp := initJasTest(t)
defer cleanUp()

// Act
scanner, err := NewAdvancedSecurityScanner(nil, &fakeServerDetails)
assert.NoError(t, err)
defer func() {
if scanner.scannerDirCleanupFunc != nil {
assert.NoError(t, scanner.scannerDirCleanupFunc())
}
}()
applicabilityManager := newApplicabilityScanManager(noVulnerabilitiesScanResponse, mockDirectDependencies, scanner)

// Assert
assert.NotEmpty(t, applicabilityManager)
assert.NotEmpty(t, applicabilityManager.scanner.configFileName)
assert.NotEmpty(t, applicabilityManager.scanner.resultsFileName)
assert.Equal(t, 2, applicabilityManager.directDependenciesCves.Size())
if assert.NotNil(t, applicabilityManager) {
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) {
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())
}
}()
scanner, cleanUp := initJasTest(t)
defer cleanUp()

results, err := getApplicabilityScanResults(fakeBasicXrayResults, mockDirectDependencies,
[]coreutils.Technology{coreutils.Nuget, coreutils.Go}, scanner)

Expand All @@ -195,16 +158,11 @@ func TestApplicabilityScanManager_ShouldRun_TechnologiesNotEligibleForScan(t *te

func TestApplicabilityScanManager_ShouldRun_ScanResultsAreEmpty(t *testing.T) {
// Arrange
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())
}
}()
scanner, cleanUp := initJasTest(t)
defer cleanUp()

applicabilityManager := newApplicabilityScanManager(nil, mockDirectDependencies, scanner)
assert.NoError(t, err)

// Assert
eligible := applicabilityManager.shouldRunApplicabilityScan([]coreutils.Technology{coreutils.Npm})
assert.False(t, eligible)
Expand Down Expand Up @@ -284,14 +242,9 @@ func TestExtractXrayDirectVulnerabilities(t *testing.T) {

func TestCreateConfigFile_VerifyFileWasCreated(t *testing.T) {
// Arrange
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())
}
}()
scanner, cleanUp := initJasTest(t)
defer cleanUp()

applicabilityManager := newApplicabilityScanManager(fakeBasicXrayResults, []string{"issueId_1_direct_dependency", "issueId_2_direct_dependency"}, scanner)

currWd, err := coreutils.GetWorkingDirectory()
Expand All @@ -313,14 +266,9 @@ func TestCreateConfigFile_VerifyFileWasCreated(t *testing.T) {

func TestParseResults_EmptyResults_AllCvesShouldGetUnknown(t *testing.T) {
// Arrange
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())
}
}()
scanner, cleanUp := initJasTest(t)
defer cleanUp()

applicabilityManager := newApplicabilityScanManager(fakeBasicXrayResults, mockDirectDependencies, scanner)
applicabilityManager.scanner.resultsFileName = filepath.Join("..", "..", "commands", "testdata", "applicability-scan", "empty-results.sarif")

Expand All @@ -337,14 +285,8 @@ func TestParseResults_EmptyResults_AllCvesShouldGetUnknown(t *testing.T) {

func TestParseResults_ApplicableCveExist(t *testing.T) {
// Arrange
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())
}
}()
scanner, cleanUp := initJasTest(t)
defer cleanUp()
applicabilityManager := newApplicabilityScanManager(fakeBasicXrayResults, mockDirectDependencies, scanner)
applicabilityManager.scanner.resultsFileName = filepath.Join("..", "..", "commands", "testdata", "applicability-scan", "applicable-cve-results.sarif")

Expand All @@ -360,14 +302,8 @@ func TestParseResults_ApplicableCveExist(t *testing.T) {

func TestParseResults_AllCvesNotApplicable(t *testing.T) {
// Arrange
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())
}
}()
scanner, cleanUp := initJasTest(t)
defer cleanUp()
applicabilityManager := newApplicabilityScanManager(fakeBasicXrayResults, mockDirectDependencies, scanner)
applicabilityManager.scanner.resultsFileName = filepath.Join("..", "..", "commands", "testdata", "applicability-scan", "no-applicable-cves-results.sarif")

Expand Down
8 changes: 6 additions & 2 deletions xray/audit/jas/iacscanner.go
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package jas

import (
"path/filepath"

"github.com/jfrog/jfrog-cli-core/v2/xray/utils"
"github.com/jfrog/jfrog-client-go/utils/log"
)
Expand Down Expand Up @@ -53,7 +55,9 @@ func (iac *IacScanManager) Run(wd string) (err error) {
return
}
var workingDirResults []utils.SourceCodeScanResult
workingDirResults, err = getSourceCodeScanResults(scanner.resultsFileName, wd, utils.IaC)
if workingDirResults, err = getSourceCodeScanResults(scanner.resultsFileName, wd, utils.IaC); err != nil {
return
}
iac.iacScannerResults = append(iac.iacScannerResults, workingDirResults...)
return
}
Expand Down Expand Up @@ -84,5 +88,5 @@ func (iac *IacScanManager) createConfigFile(currentWd string) error {
}

func (iac *IacScanManager) runAnalyzerManager() error {
return iac.scanner.analyzerManager.Exec(iac.scanner.configFileName, iacScanCommand, iac.scanner.analyzerManager.GetAnalyzerManagerDir(), iac.scanner.serverDetails)
return iac.scanner.analyzerManager.Exec(iac.scanner.configFileName, iacScanCommand, filepath.Dir(iac.scanner.analyzerManager.AnalyzerManagerFullPath), iac.scanner.serverDetails)
}
11 changes: 6 additions & 5 deletions xray/audit/jas/iacscanner_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,12 @@ func TestNewIacScanManager(t *testing.T) {
iacScanManager := newIacScanManager(scanner)

// Assert
assert.NotEmpty(t, iacScanManager)
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)
if assert.NotNil(t, iacScanManager) {
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) {
Expand Down
2 changes: 1 addition & 1 deletion xray/audit/jas/jasmanager.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ func deleteJasProcessFiles(configFile string, resultFile string) error {
return errorutils.CheckError(err)
}

func getSourceCodeScanResults(resultsFileName, workingDir string, scanType utils.ScanType) ([]utils.SourceCodeScanResult, error) {
func getSourceCodeScanResults(resultsFileName, workingDir string, scanType utils.JasScanType) ([]utils.SourceCodeScanResult, error) {
report, err := sarif.Open(resultsFileName)
if errorutils.CheckError(err) != nil {
return nil, err
Expand Down
Loading

0 comments on commit b1383d2

Please sign in to comment.