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.

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

Filter out non-applicable violations if requested by policy #275

Merged
merged 18 commits into from
Jan 6, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
11 changes: 4 additions & 7 deletions audit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -808,7 +808,7 @@ type auditCommandTestParams struct {
// --project flag value if provided.
ProjectKey string
// --fail flag value if provided, must be provided with 'createWatchesFuncs' to create watches for the test
FailOnFailedBuildFlag bool
DisableFailOnFailedBuildFlag bool
// -- vuln flag 'True' value must be provided with 'createWatchesFuncs' to create watches for the test
WithVuln bool
// --licenses flag value if provided
Expand Down Expand Up @@ -836,12 +836,9 @@ func testAuditCommand(t *testing.T, testCli *coreTests.JfrogCli, params auditCom
if len(params.Watches) > 0 {
args = append(args, "--watches="+strings.Join(params.Watches, ","))
}
if params.FailOnFailedBuildFlag {
if len(params.Watches) == 0 {
// Verify params consistency no fail flag
assert.False(t, params.FailOnFailedBuildFlag, "Fail flag provided without watches")
}
args = append(args, "--fail")
// Default value for --fail flag is 'true'. Unless we directly pass DisableFailOnFailedBuildFlag=true, the flow will fail when security issues are found
if params.DisableFailOnFailedBuildFlag {
args = append(args, "--fail=false")
}
if params.WithVuln {
args = append(args, "--vuln")
Expand Down
2 changes: 1 addition & 1 deletion commands/audit/scarunner.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ func runScaWithTech(tech techutils.Technology, params *AuditParams, serverDetail
if err != nil {
return
}
log.Debug(fmt.Sprintf("Finished '%s' dependency tree scan. %s", tech.ToFormal(), utils.GetScanFindingsLog(utils.ScaScan, len(techResults[0].Vulnerabilities), len(techResults[0].Violations), -1)))
log.Info(fmt.Sprintf("Finished '%s' dependency tree scan. %s", tech.ToFormal(), utils.GetScanFindingsLog(utils.ScaScan, len(techResults[0].Vulnerabilities), len(techResults[0].Violations), -1)))
techResults = sca.BuildImpactPathsForScanResponse(techResults, fullDependencyTrees)
return
}
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ require (
gopkg.in/warnings.v0 v0.1.2 // indirect
)

replace github.com/jfrog/jfrog-client-go => github.com/jfrog/jfrog-client-go v1.28.1-0.20241230154616-e342ed5065f1
replace github.com/jfrog/jfrog-client-go => github.com/jfrog/jfrog-client-go v1.28.1-0.20250106143359-de902d8b8495

replace github.com/jfrog/jfrog-cli-core/v2 => github.com/jfrog/jfrog-cli-core/v2 v2.31.1-0.20250101110857-b26e9a6644c6

Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,8 @@ github.com/jfrog/jfrog-apps-config v1.0.1 h1:mtv6k7g8A8BVhlHGlSveapqf4mJfonwvXYL
github.com/jfrog/jfrog-apps-config v1.0.1/go.mod h1:8AIIr1oY9JuH5dylz2S6f8Ym2MaadPLR6noCBO4C22w=
github.com/jfrog/jfrog-cli-core/v2 v2.31.1-0.20250101110857-b26e9a6644c6 h1:/i1sIQS0q0gRN531ChVToQWcjaVZOKZ4KuGk7j7vDTc=
github.com/jfrog/jfrog-cli-core/v2 v2.31.1-0.20250101110857-b26e9a6644c6/go.mod h1:LfKvCRXbvwgE0V6aX3/GabkzCedghXq0Y6lmsEuxr44=
github.com/jfrog/jfrog-client-go v1.28.1-0.20241230154616-e342ed5065f1 h1:JQvbTSPDkPNpts1NLHGTKvtG4cMFY1ptBHTNMYFyMhs=
github.com/jfrog/jfrog-client-go v1.28.1-0.20241230154616-e342ed5065f1/go.mod h1:2ySOMva54L3EYYIlCBYBTcTgqfrrQ19gtpA/MWfA/ec=
github.com/jfrog/jfrog-client-go v1.28.1-0.20250106143359-de902d8b8495 h1:cPugIRHCJxE+QWW9TlvOlTWPcVI1wTRgAujQZk4I4VI=
github.com/jfrog/jfrog-client-go v1.28.1-0.20250106143359-de902d8b8495/go.mod h1:2ySOMva54L3EYYIlCBYBTcTgqfrrQ19gtpA/MWfA/ec=
github.com/k0kubun/colorstring v0.0.0-20150214042306-9440f1994b88/go.mod h1:3w7q1U84EfirKl04SVQ/s7nPm1ZPhiXd34z40TNz36k=
github.com/k0kubun/pp v3.0.1+incompatible/go.mod h1:GWse8YhT0p8pT4ir3ZgBbfZild3tgzSScAn6HmfYukg=
github.com/kevinburke/ssh_config v1.2.0 h1:x584FjTGwHzMwvHx18PXxbBVzfnxogHaAReU4gf13a4=
Expand Down
2 changes: 1 addition & 1 deletion jas/runner/jasrunner.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ func runContextualScan(securityParallelRunner *utils.SecurityParallelRunner, sca
securityParallelRunner.ResultsMu.Lock()
defer securityParallelRunner.ResultsMu.Unlock()
// We first add the scan results and only then check for errors, so we can store the exit code in order to report it in the end
scanResults.JasResults.NewApplicabilityScanResults(jas.GetAnalyzerManagerExitCode(err), caScanResults...)
scanResults.JasResults.AddApplicabilityScanResults(jas.GetAnalyzerManagerExitCode(err), caScanResults...)
if err = jas.ParseAnalyzerManagerError(jasutils.Applicability, err); err != nil {
return fmt.Errorf("%s%s", clientutils.GetLogMsgPrefix(threadId, false), err.Error())
}
Expand Down
6 changes: 3 additions & 3 deletions tests/utils/test_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -297,11 +297,11 @@ func CreateSecurityPolicy(t *testing.T, policyName string, rules ...xrayUtils.Po
}
}

func CreateTestSecurityPolicy(t *testing.T, policyName string, severity xrayUtils.Severity, failBuild bool) (string, func()) {
func CreateTestSecurityPolicy(t *testing.T, policyName string, severity xrayUtils.Severity, failBuild bool, skipNotApplicable bool) (string, func()) {
return CreateSecurityPolicy(t, policyName,
xrayUtils.PolicyRule{
Name: "sca_rule",
Criteria: *xrayUtils.CreateSeverityPolicyCriteria(severity),
Criteria: *xrayUtils.CreateSeverityPolicyCriteria(severity, skipNotApplicable),
Actions: getBuildFailAction(failBuild),
Priority: 1,
},
Expand Down Expand Up @@ -382,7 +382,7 @@ func CreateTestPolicyAndWatch(t *testing.T, policyName, watchName string, severi
Type: xrayUtils.Security,
Rules: []xrayUtils.PolicyRule{{
Name: "sec_rule",
Criteria: *xrayUtils.CreateSeverityPolicyCriteria(severity),
Criteria: *xrayUtils.CreateSeverityPolicyCriteria(severity, false),
Priority: 1,
Actions: &xrayUtils.PolicyAction{
FailBuild: clientUtils.Pointer(true),
Expand Down
27 changes: 26 additions & 1 deletion utils/results/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ func ApplyHandlerToScaVulnerabilities(target ScanTarget, vulnerabilities []servi
return nil
}

// ApplyHandlerToScaViolations allows to iterate over the provided SCA violations and call the provided handler for each impacted component/package with a violation to process it.
// Allows to iterate over the provided SCA violations and call the provided handler for each impacted component/package with a violation to process it.
func ApplyHandlerToScaViolations(target ScanTarget, violations []services.Violation, entitledForJas bool, applicabilityRuns []*sarif.Run, securityHandler ParseScaViolationFunc, licenseHandler ParseScaViolationFunc, operationalRiskHandler ParseScaViolationFunc) (watches []string, failBuild bool, err error) {
if securityHandler == nil && licenseHandler == nil && operationalRiskHandler == nil {
return
Expand Down Expand Up @@ -145,6 +145,13 @@ func ApplyHandlerToScaViolations(target ScanTarget, violations []services.Violat
// No handler was provided for security violations
continue
}

var skipNotApplicable bool
if skipNotApplicable, err = shouldSkipNotApplicable(violation, applicabilityStatus); skipNotApplicable {
log.Debug("A non-applicable violation was found and will be removed from final results as requested by its policies")
continue
}

for compIndex := 0; compIndex < len(impactedPackagesNames); compIndex++ {
if e := securityHandler(
violation, cves, applicabilityStatus, severity,
Expand Down Expand Up @@ -655,3 +662,21 @@ func GetIssueTechnology(responseTechnology string, targetTech techutils.Technolo
// if no technology is provided, use the target technology
return targetTech
}

// Checks if the violation's applicability status is NotApplicable and if all of its policies states that non-applicable CVEs should be skipped
func shouldSkipNotApplicable(violation services.Violation, applicabilityStatus jasutils.ApplicabilityStatus) (bool, error) {
if applicabilityStatus != jasutils.NotApplicable {
return false, nil
}

if len(violation.Policies) == 0 {
return false, errors.New("A violation with no policies was provided")
}

for _, policy := range violation.Policies {
if !policy.SkipNotApplicable {
return false, nil
}
}
return true, nil
}
106 changes: 106 additions & 0 deletions utils/results/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -701,3 +701,109 @@ func TestGetFinalApplicabilityStatus(t *testing.T) {
})
}
}

func TestShouldSkipNotApplicable(t *testing.T) {
testCases := []struct {
name string
violation services.Violation
applicabilityStatus jasutils.ApplicabilityStatus
shouldSkip bool
errorExpected bool
}{
{
name: "Applicable CVE - should NOT skip",
violation: services.Violation{},
applicabilityStatus: jasutils.Applicable,
shouldSkip: false,
errorExpected: false,
},
{
name: "Undetermined CVE - should NOT skip",
violation: services.Violation{},
applicabilityStatus: jasutils.ApplicabilityUndetermined,
shouldSkip: false,
errorExpected: false,
},
{
name: "Not covered CVE - should NOT skip",
violation: services.Violation{},
applicabilityStatus: jasutils.NotCovered,
shouldSkip: false,
errorExpected: false,
},
{
name: "Missing Context CVE - should NOT skip",
violation: services.Violation{},
applicabilityStatus: jasutils.MissingContext,
shouldSkip: false,
errorExpected: false,
},
{
name: "Not scanned CVE - should NOT skip",
violation: services.Violation{},
applicabilityStatus: jasutils.NotScanned,
shouldSkip: false,
errorExpected: false,
},
{
name: "Non applicable CVE with skip-non-applicable in ALL policies - SHOULD skip",
violation: services.Violation{
Policies: []services.Policy{
{
Policy: "policy-1",
SkipNotApplicable: true,
},
{
Policy: "policy-2",
SkipNotApplicable: true,
},
},
},
applicabilityStatus: jasutils.NotApplicable,
shouldSkip: true,
errorExpected: false,
},
{
name: "Non applicable CVE with skip-non-applicable in SOME policies - should NOT skip",
violation: services.Violation{
Policies: []services.Policy{
{
Policy: "policy-1",
SkipNotApplicable: true,
},
{
Policy: "policy-2",
SkipNotApplicable: false,
},
},
},
applicabilityStatus: jasutils.NotApplicable,
shouldSkip: false,
errorExpected: false,
},
{
name: "Violation without policy - error expected",
violation: services.Violation{},
applicabilityStatus: jasutils.NotApplicable,
shouldSkip: false,
errorExpected: true,
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
shouldSkip, err := shouldSkipNotApplicable(tc.violation, tc.applicabilityStatus)
if tc.errorExpected {
assert.Error(t, err)
} else {
assert.NoError(t, err)
}

if tc.shouldSkip {
assert.True(t, shouldSkip)
} else {
assert.False(t, shouldSkip)
}
})
}
}
4 changes: 2 additions & 2 deletions utils/results/conversion/convertor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@ func getAuditTestResults(unique bool) (*results.SecurityCommandResults, validati
ScannedStatus: "completed",
})
// Contextual analysis scan results
npmTargetResults.JasResults.NewApplicabilityScanResults(0,
npmTargetResults.JasResults.AddApplicabilityScanResults(0,
&sarif.Run{
Tool: sarif.Tool{
Driver: sarifutils.CreateDummyDriver(validations.ContextualAnalysisToolName,
Expand Down Expand Up @@ -537,7 +537,7 @@ func getDockerScanTestResults(unique bool) (*results.SecurityCommandResults, val
ScannedStatus: "completed",
})
// Contextual analysis scan results
dockerImageTarget.JasResults.NewApplicabilityScanResults(0,
dockerImageTarget.JasResults.AddApplicabilityScanResults(0,
&sarif.Run{
Tool: sarif.Tool{
Driver: sarifutils.CreateDummyDriver(validations.ContextualAnalysisToolName,
Expand Down
2 changes: 1 addition & 1 deletion utils/results/results.go
Original file line number Diff line number Diff line change
Expand Up @@ -442,7 +442,7 @@ func (ssr *ScaScanResults) HasFindings() bool {
return false
}

func (jsr *JasScansResults) NewApplicabilityScanResults(exitCode int, runs ...*sarif.Run) {
func (jsr *JasScansResults) AddApplicabilityScanResults(exitCode int, runs ...*sarif.Run) {
jsr.ApplicabilityScanResults = append(jsr.ApplicabilityScanResults, ScanResult[[]*sarif.Run]{Scan: runs, StatusCode: exitCode})
}

Expand Down
60 changes: 57 additions & 3 deletions xsc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package main
import (
"encoding/json"
"errors"
"os"
"path/filepath"
"testing"

Expand Down Expand Up @@ -66,7 +67,7 @@ func TestXscAuditViolationsWithIgnoreRule(t *testing.T) {
_, cleanUpProject := securityTestUtils.CreateTestProjectEnvAndChdir(t, filepath.Join(filepath.FromSlash(tests.GetTestResourcesPath()), "projects", "jas", "jas"))
defer cleanUpProject()
// Create policy and watch for the git repo so we will also get violations (unknown = all vulnerabilities will be reported as violations)
policyName, cleanUpPolicy := securityTestUtils.CreateTestSecurityPolicy(t, "git-repo-ignore-rule-policy", utils.Unknown, true)
policyName, cleanUpPolicy := securityTestUtils.CreateTestSecurityPolicy(t, "git-repo-ignore-rule-policy", utils.Unknown, true, false)
defer cleanUpPolicy()
_, cleanUpWatch := securityTestUtils.CreateWatchForTests(t, policyName, "git-repo-ignore-rule-watch", xscutils.GetGitRepoUrlKey(validations.TestMockGitInfo.GitRepoHttpsCloneUrl))
defer cleanUpWatch()
Expand Down Expand Up @@ -100,10 +101,63 @@ func TestXscAuditViolationsWithIgnoreRule(t *testing.T) {
validations.VerifySimpleJsonResults(t, output, validations.ValidationParams{ExactResultsMatch: true, Total: &validations.TotalCount{}, Violations: &validations.ViolationCount{ValidateScan: &validations.ScanCount{}}})
}

func TestXrayAuditJasSkipNotApplicableCvesViolations(t *testing.T) {
// Init XSC tests also enabled analytics reporting.
_, _, cleanUpXsc := integration.InitXscTest(t, func() { securityTestUtils.ValidateXrayVersion(t, services.MinXrayVersionGitRepoKey) })
defer cleanUpXsc()
// Create the audit command with git repo context injected.
cliToRun, cleanUpHome := integration.InitTestWithMockCommandOrParams(t, false, getAuditCommandWithXscGitContext(validations.TestMockGitInfo))
defer cleanUpHome()
// Create the project to scan
_, cleanUpProject := securityTestUtils.CreateTestProjectEnvAndChdir(t, filepath.Join(filepath.FromSlash(tests.GetTestResourcesPath()), "projects", "jas", "jas"))
defer cleanUpProject()
// Create policy and watch for the git repo so we will also get violations - This watch DO NOT skip not-applicable results
var firstPolicyCleaned, firstWatchCleaned bool
policyName, cleanUpPolicy := securityTestUtils.CreateTestSecurityPolicy(t, "without-skip-non-applicable-policy", utils.Low, false, false)
defer func() {
if !firstPolicyCleaned {
cleanUpPolicy()
}
}()
watchName, cleanUpWatch := securityTestUtils.CreateWatchForTests(t, policyName, "without-skip-not-applicable-watch", xscutils.GetGitRepoUrlKey(validations.TestMockGitInfo.GitRepoHttpsCloneUrl))
defer func() {
if !firstWatchCleaned {
cleanUpWatch()
}
}()
// Run the audit command with git repo and verify violations are reported to the platform.
output, err := testAuditCommand(t, cliToRun, auditCommandTestParams{Format: string(format.SimpleJson), Watches: []string{watchName}, DisableFailOnFailedBuildFlag: true})
assert.NoError(t, err)
validations.VerifySimpleJsonResults(t, output, validations.ValidationParams{
Violations: &validations.ViolationCount{ValidateScan: &validations.ScanCount{Sca: 17, Sast: 1, Secrets: 15}},
ExactResultsMatch: true,
})

// We clean the initially created Policy and Watch that are related to the Git Repo resource, because we must have all related policies with skipNotApplicable=true
cleanUpWatch()
firstWatchCleaned = true
cleanUpPolicy()
firstPolicyCleaned = true

// Create policy and watch for the git repo so we will also get violations - This watch DO NOT skip not-applicable results
skipPolicyName, skipCleanUpPolicy := securityTestUtils.CreateTestSecurityPolicy(t, "skip-non-applicable-policy", utils.Low, false, true)
defer skipCleanUpPolicy()
skipWatchName, skipCleanUpWatch := securityTestUtils.CreateWatchForTests(t, skipPolicyName, "skip-not-applicable-watch", xscutils.GetGitRepoUrlKey(validations.TestMockGitInfo.GitRepoHttpsCloneUrl))
defer skipCleanUpWatch()
output, err = testAuditCommand(t, cliToRun, auditCommandTestParams{Format: string(format.SimpleJson), Watches: []string{skipWatchName}, DisableFailOnFailedBuildFlag: true})
assert.NoError(t, err)
validations.VerifySimpleJsonResults(t, output, validations.ValidationParams{
Violations: &validations.ViolationCount{ValidateScan: &validations.ScanCount{Sca: 12, Sast: 1, Secrets: 15}},
ExactResultsMatch: true,
})

}

func TestAuditJasViolationsProjectKeySimpleJson(t *testing.T) {
_, _, cleanUpXsc := integration.InitXscTest(t, func() { securityTestUtils.ValidateXrayVersion(t, services.MinXrayVersionGitRepoKey) })
defer cleanUpXsc()
if tests.TestJfrogPlatformProjectKeyEnvVar == "" {
testsJfrogPlatformProjectKey := os.Getenv(tests.TestJfrogPlatformProjectKeyEnvVar)
if testsJfrogPlatformProjectKey == "" {
t.Skipf("skipping test. %s is not set.", tests.TestJfrogPlatformProjectKeyEnvVar)
}
// Create the audit command with git repo context injected.
Expand All @@ -114,7 +168,7 @@ func TestAuditJasViolationsProjectKeySimpleJson(t *testing.T) {
_, cleanUpProject := securityTestUtils.CreateTestProjectEnvAndChdir(t, filepath.Join(filepath.FromSlash(tests.GetTestResourcesPath()), "projects", "jas", "jas"))
defer cleanUpProject()
// Create policy and watch for the project so we will get violations (unknown = all vulnerabilities will be reported as violations)
policyName, cleanUpPolicy := securityTestUtils.CreateTestSecurityPolicy(t, "project-key-jas-violations-policy", utils.Unknown, true)
policyName, cleanUpPolicy := securityTestUtils.CreateTestSecurityPolicy(t, "project-key-jas-violations-policy", utils.Unknown, true, false)
defer cleanUpPolicy()
_, cleanUpWatch := securityTestUtils.CreateTestProjectKeyWatch(t, policyName, "project-key-jas-violations-watch", *tests.JfrogTestProjectKey)
defer cleanUpWatch()
Expand Down
Loading