Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Exclude JAS Scan Flag #836

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

Exclude JAS Scan Flag #836

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion xray/audit/jas/applicabilitymanager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,8 @@ func newApplicabilityScanManager(xrayScanResults []services.ScanResponse, depend
}

func (a *ApplicabilityScanManager) eligibleForApplicabilityScan() bool {
return resultsIncludeEligibleTechnologies(getXrayVulnerabilities(a.xrayResults), getXrayViolations(a.xrayResults))
return resultsIncludeEligibleTechnologies(getXrayVulnerabilities(a.xrayResults), getXrayViolations(a.xrayResults)) &&
!utils.ExcludeScan(scannersToExclude, ApplicabilityFeatureId)
}

// This function gets a liat of xray scan responses that contains direct and indirect violations, and returns only direct
Expand Down
2 changes: 1 addition & 1 deletion xray/audit/jas/applicabilitymanager_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,7 @@ func TestGetExtendedScanResults_AnalyzerManagerReturnsError(t *testing.T) {
analyzerManagerExecuter = &analyzerManagerMock{}

// Act
extendedResults, err := GetExtendedScanResults(fakeBasicXrayResults, fakeBasicDependencyGraph, &fakeServerDetails)
extendedResults, err := GetExtendedScanResults(fakeBasicXrayResults, fakeBasicDependencyGraph, &fakeServerDetails, "")

// Assert
assert.Error(t, err)
Expand Down
3 changes: 3 additions & 0 deletions xray/audit/jas/iacscanner.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ func getIacScanResults(serverDetails *config.ServerDetails, analyzerManager util
err = errors.Join(err, cleanupFunc())
}
}()
if utils.ExcludeScan(scannersToExclude, iacScanCommand) {
return nil, false, nil
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's add a debug level log that the IaC scan is skipped

}
if err = iacScanManager.run(); err != nil {
if utils.IsNotEntitledError(err) || utils.IsUnsupportedCommandError(err) {
return nil, false, nil
Expand Down
5 changes: 4 additions & 1 deletion xray/audit/jas/jasmanager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,19 @@ import (
"github.com/jfrog/jfrog-client-go/xray/services"
xrayUtils "github.com/jfrog/jfrog-client-go/xray/services/utils"
"os"
"strings"
)

const serverDetailsErrorMessage = "cant get xray server details"

var (
analyzerManagerExecuter utils.AnalyzerManagerInterface = &utils.AnalyzerManager{}
skippedDirs = []string{"**/*test*/**", "**/*venv*/**", "**/*node_modules*/**", "**/*target*/**"}
scannersToExclude = []string{}
Comment on lines 19 to +21
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment is not related to this PR, but is more general -
Please prefer not to use global variables. Instead, the recommended way is to use a new JasManager struct.

One design pattern is to pass a pointer of the struct when needed to each one of the scanners.
Another design pattern is to use inheritance/decoration, whereby every scanner contains the JasManager struct.

)

func GetExtendedScanResults(xrayResults []services.ScanResponse, dependencyTrees []*xrayUtils.GraphNode,
serverDetails *config.ServerDetails) (*utils.ExtendedScanResults, error) {
serverDetails *config.ServerDetails, excludeScan string) (*utils.ExtendedScanResults, error) {
if serverDetails == nil {
return nil, errors.New(serverDetailsErrorMessage)
}
Expand All @@ -39,6 +41,7 @@ func GetExtendedScanResults(xrayResults []services.ScanResponse, dependencyTrees
if err = utils.CreateAnalyzerManagerLogDir(); err != nil {
return nil, err
}
scannersToExclude = strings.Split(excludeScan, ";")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's also trim strings to allow for example: --exclude-scan=contextual_analysis; secrets

applicabilityScanResults, eligibleForApplicabilityScan, err := getApplicabilityScanResults(xrayResults,
dependencyTrees, serverDetails, analyzerManagerExecuter)
if err != nil {
Expand Down
18 changes: 16 additions & 2 deletions xray/audit/jas/jasmanager_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func TestGetExtendedScanResults_AnalyzerManagerDoesntExist(t *testing.T) {
analyzerManagerExecuter = &analyzerManagerMock{}

// Act
extendedResults, err := GetExtendedScanResults(fakeBasicXrayResults, fakeBasicDependencyGraph, &fakeServerDetails)
extendedResults, err := GetExtendedScanResults(fakeBasicXrayResults, fakeBasicDependencyGraph, &fakeServerDetails, "")

// Assert
assert.NoError(t, err)
Expand All @@ -74,10 +74,24 @@ func TestGetExtendedScanResults_AnalyzerManagerDoesntExist(t *testing.T) {

func TestGetExtendedScanResults_ServerNotValid(t *testing.T) {
// Act
extendedResults, err := GetExtendedScanResults(fakeBasicXrayResults, fakeBasicDependencyGraph, nil)
extendedResults, err := GetExtendedScanResults(fakeBasicXrayResults, fakeBasicDependencyGraph, nil, "")

// Assert
assert.Nil(t, extendedResults)
assert.Error(t, err)
assert.Equal(t, "cant get xray server details", err.Error())
}

func TestGetExtendedScanResults_ExcludeScanners(t *testing.T) {
// Arrange
excludeScansInput := "contextual_analysis;secrets;iac"

// Act
extendedResults, err := GetExtendedScanResults(fakeBasicXrayResults, fakeBasicDependencyGraph, &fakeServerDetails, excludeScansInput)

// Assert
assert.NoError(t, err)
assert.False(t, extendedResults.EligibleForSecretScan)
assert.False(t, extendedResults.EligibleForApplicabilityScan)
assert.False(t, extendedResults.EligibleForIacScan)
}
4 changes: 4 additions & 0 deletions xray/audit/jas/secretsscanner.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ const (
secretsScannersNames = "tokens, entropy"
secretsScannerType = "secrets-scan"
secScanFailureMessage = "failed to run secrets scan. Cause: %s"
secretsFeatureName = "secrets"
)

type SecretScanManager struct {
Expand Down Expand Up @@ -49,6 +50,9 @@ func getSecretsScanResults(serverDetails *config.ServerDetails, analyzerManager
err = errors.Join(err, cleanupFunc())
}
}()
if utils.ExcludeScan(scannersToExclude, secretsFeatureName) {
return nil, false, nil
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's add a debug level log that the secret scan is skipped

}
if err = secretScanManager.run(); err != nil {
if utils.IsNotEntitledError(err) || utils.IsUnsupportedCommandError(err) {
return nil, false, nil
Expand Down
8 changes: 7 additions & 1 deletion xray/commands/audit/generic/auditmanager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ type Params struct {
installFunc func(tech string) error
fixableOnly bool
minSeverityFilter string
excludeJasScan string
*clientUtils.GraphBasicParams
xrayVersion string
}
Expand Down Expand Up @@ -106,6 +107,11 @@ func (params *Params) SetXrayVersion(version string) *Params {
return params
}

func (params *Params) SetExcludeJasScan(excludeScan string) *Params {
params.excludeJasScan = excludeScan
return params
}

// Runs an audit scan based on the provided auditParams.
// Returns an audit Results object containing all the scan results.
// If the current server is entitled for JAS, the advanced security results will be included in the scan results.
Expand Down Expand Up @@ -137,7 +143,7 @@ func RunAudit(auditParams *Params) (results *Results, err error) {
extendedScanResults := &clientUtils.ExtendedScanResults{XrayResults: scanResults}
// Try to run contextual analysis only if the user is entitled for advance security
if isEntitled {
extendedScanResults, err = jas.GetExtendedScanResults(scanResults, auditParams.FullDependenciesTree(), serverDetails)
extendedScanResults, err = jas.GetExtendedScanResults(scanResults, auditParams.FullDependenciesTree(), serverDetails, auditParams.excludeJasScan)
if err != nil {
return
}
Expand Down
9 changes: 8 additions & 1 deletion xray/commands/audit/generic/generic.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ type GenericAuditCommand struct {
watches []string
projectKey string
targetRepoPath string
excludeJasScan string
IncludeVulnerabilities bool
IncludeLicenses bool
Fail bool
Expand Down Expand Up @@ -64,6 +65,11 @@ func (auditCmd *GenericAuditCommand) SetPrintExtendedTable(printExtendedTable bo
return auditCmd
}

func (auditCmd *GenericAuditCommand) SetExcludeJasScan(excludeScan string) *GenericAuditCommand {
auditCmd.excludeJasScan = excludeScan
return auditCmd
}

func (auditCmd *GenericAuditCommand) CreateXrayGraphScanParams() *services.XrayGraphScanParams {
params := &services.XrayGraphScanParams{
RepoPath: auditCmd.targetRepoPath,
Expand All @@ -86,7 +92,8 @@ func (auditCmd *GenericAuditCommand) Run() (err error) {
SetWorkingDirs(auditCmd.workingDirs).
SetMinSeverityFilter(auditCmd.minSeverityFilter).
SetFixableOnly(auditCmd.fixableOnly).
SetGraphBasicParams(auditCmd.GraphBasicParams)
SetGraphBasicParams(auditCmd.GraphBasicParams).
SetExcludeJasScan(auditCmd.excludeJasScan)
auditResults, err := RunAudit(auditParams)
if err != nil {
return err
Expand Down
9 changes: 9 additions & 0 deletions xray/utils/analyzermanager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,3 +243,12 @@ func GetResultSeverity(result *sarif.Result) string {
}
return SeverityDefaultValue
}

func ExcludeScan(scansToBeExcluded []string, scan string) bool {
for _, s := range scansToBeExcluded {
if s == scan {
return true
}
}
return false
Comment on lines +248 to +253
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can be inlined:

Suggested change
for _, s := range scansToBeExcluded {
if s == scan {
return true
}
}
return false
return slices.Contains(scansToBeExcluded, scan)

}
17 changes: 17 additions & 0 deletions xray/utils/analyzermanager_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,3 +133,20 @@ func TestGetResultSeverity(t *testing.T) {
assert.Equal(t, test.expectedSeverity, GetResultSeverity(test.result))
}
}

func TestExcludeScan(t *testing.T) {
tests := []struct {
excludedScanList []string
scanToCheck string
expectedResult bool
}{
{excludedScanList: []string{"secrets", "iac", "contextual_analysis"}, scanToCheck: "contextual_analysis", expectedResult: true},
{excludedScanList: []string{"secrets", "iac"}, scanToCheck: "contextual_analysis", expectedResult: false},
{excludedScanList: []string{}, scanToCheck: "contextual_analysis", expectedResult: false},
{excludedScanList: []string{"", "unsupported_Scan_type", "contextual_analysis"}, scanToCheck: "contextual_analysis", expectedResult: true},
}

for _, test := range tests {
assert.Equal(t, test.expectedResult, ExcludeScan(test.excludedScanList, test.scanToCheck))
}
}