Skip to content

Commit c1d19ba

Browse files
eranturgemaneranturgeman
authored
Filter out non-applicable violations if requested by policy (#275)
1 parent 333ac33 commit c1d19ba

File tree

11 files changed

+204
-22
lines changed

11 files changed

+204
-22
lines changed

Diff for: ‎audit_test.go

+4-7
Original file line numberDiff line numberDiff line change
@@ -808,7 +808,7 @@ type auditCommandTestParams struct {
808808
// --project flag value if provided.
809809
ProjectKey string
810810
// --fail flag value if provided, must be provided with 'createWatchesFuncs' to create watches for the test
811-
FailOnFailedBuildFlag bool
811+
DisableFailOnFailedBuildFlag bool
812812
// -- vuln flag 'True' value must be provided with 'createWatchesFuncs' to create watches for the test
813813
WithVuln bool
814814
// --licenses flag value if provided
@@ -836,12 +836,9 @@ func testAuditCommand(t *testing.T, testCli *coreTests.JfrogCli, params auditCom
836836
if len(params.Watches) > 0 {
837837
args = append(args, "--watches="+strings.Join(params.Watches, ","))
838838
}
839-
if params.FailOnFailedBuildFlag {
840-
if len(params.Watches) == 0 {
841-
// Verify params consistency no fail flag
842-
assert.False(t, params.FailOnFailedBuildFlag, "Fail flag provided without watches")
843-
}
844-
args = append(args, "--fail")
839+
// Default value for --fail flag is 'true'. Unless we directly pass DisableFailOnFailedBuildFlag=true, the flow will fail when security issues are found
840+
if params.DisableFailOnFailedBuildFlag {
841+
args = append(args, "--fail=false")
845842
}
846843
if params.WithVuln {
847844
args = append(args, "--vuln")

Diff for: ‎commands/audit/scarunner.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ func runScaWithTech(tech techutils.Technology, params *AuditParams, serverDetail
169169
if err != nil {
170170
return
171171
}
172-
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)))
172+
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)))
173173
techResults = sca.BuildImpactPathsForScanResponse(techResults, fullDependencyTrees)
174174
return
175175
}

Diff for: ‎go.mod

+1-1
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ require (
112112
gopkg.in/warnings.v0 v0.1.2 // indirect
113113
)
114114

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

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

Diff for: ‎go.sum

+2-2
Original file line numberDiff line numberDiff line change
@@ -129,8 +129,8 @@ github.com/jfrog/jfrog-apps-config v1.0.1 h1:mtv6k7g8A8BVhlHGlSveapqf4mJfonwvXYL
129129
github.com/jfrog/jfrog-apps-config v1.0.1/go.mod h1:8AIIr1oY9JuH5dylz2S6f8Ym2MaadPLR6noCBO4C22w=
130130
github.com/jfrog/jfrog-cli-core/v2 v2.31.1-0.20250101110857-b26e9a6644c6 h1:/i1sIQS0q0gRN531ChVToQWcjaVZOKZ4KuGk7j7vDTc=
131131
github.com/jfrog/jfrog-cli-core/v2 v2.31.1-0.20250101110857-b26e9a6644c6/go.mod h1:LfKvCRXbvwgE0V6aX3/GabkzCedghXq0Y6lmsEuxr44=
132-
github.com/jfrog/jfrog-client-go v1.28.1-0.20241230154616-e342ed5065f1 h1:JQvbTSPDkPNpts1NLHGTKvtG4cMFY1ptBHTNMYFyMhs=
133-
github.com/jfrog/jfrog-client-go v1.28.1-0.20241230154616-e342ed5065f1/go.mod h1:2ySOMva54L3EYYIlCBYBTcTgqfrrQ19gtpA/MWfA/ec=
132+
github.com/jfrog/jfrog-client-go v1.28.1-0.20250106143359-de902d8b8495 h1:cPugIRHCJxE+QWW9TlvOlTWPcVI1wTRgAujQZk4I4VI=
133+
github.com/jfrog/jfrog-client-go v1.28.1-0.20250106143359-de902d8b8495/go.mod h1:2ySOMva54L3EYYIlCBYBTcTgqfrrQ19gtpA/MWfA/ec=
134134
github.com/k0kubun/colorstring v0.0.0-20150214042306-9440f1994b88/go.mod h1:3w7q1U84EfirKl04SVQ/s7nPm1ZPhiXd34z40TNz36k=
135135
github.com/k0kubun/pp v3.0.1+incompatible/go.mod h1:GWse8YhT0p8pT4ir3ZgBbfZild3tgzSScAn6HmfYukg=
136136
github.com/kevinburke/ssh_config v1.2.0 h1:x584FjTGwHzMwvHx18PXxbBVzfnxogHaAReU4gf13a4=

Diff for: ‎jas/runner/jasrunner.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,7 @@ func runContextualScan(securityParallelRunner *utils.SecurityParallelRunner, sca
193193
securityParallelRunner.ResultsMu.Lock()
194194
defer securityParallelRunner.ResultsMu.Unlock()
195195
// 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
196-
scanResults.JasResults.NewApplicabilityScanResults(jas.GetAnalyzerManagerExitCode(err), caScanResults...)
196+
scanResults.JasResults.AddApplicabilityScanResults(jas.GetAnalyzerManagerExitCode(err), caScanResults...)
197197
if err = jas.ParseAnalyzerManagerError(jasutils.Applicability, err); err != nil {
198198
return fmt.Errorf("%s%s", clientutils.GetLogMsgPrefix(threadId, false), err.Error())
199199
}

Diff for: ‎tests/utils/test_utils.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -297,11 +297,11 @@ func CreateSecurityPolicy(t *testing.T, policyName string, rules ...xrayUtils.Po
297297
}
298298
}
299299

300-
func CreateTestSecurityPolicy(t *testing.T, policyName string, severity xrayUtils.Severity, failBuild bool) (string, func()) {
300+
func CreateTestSecurityPolicy(t *testing.T, policyName string, severity xrayUtils.Severity, failBuild bool, skipNotApplicable bool) (string, func()) {
301301
return CreateSecurityPolicy(t, policyName,
302302
xrayUtils.PolicyRule{
303303
Name: "sca_rule",
304-
Criteria: *xrayUtils.CreateSeverityPolicyCriteria(severity),
304+
Criteria: *xrayUtils.CreateSeverityPolicyCriteria(severity, skipNotApplicable),
305305
Actions: getBuildFailAction(failBuild),
306306
Priority: 1,
307307
},
@@ -382,7 +382,7 @@ func CreateTestPolicyAndWatch(t *testing.T, policyName, watchName string, severi
382382
Type: xrayUtils.Security,
383383
Rules: []xrayUtils.PolicyRule{{
384384
Name: "sec_rule",
385-
Criteria: *xrayUtils.CreateSeverityPolicyCriteria(severity),
385+
Criteria: *xrayUtils.CreateSeverityPolicyCriteria(severity, false),
386386
Priority: 1,
387387
Actions: &xrayUtils.PolicyAction{
388388
FailBuild: clientUtils.Pointer(true),

Diff for: ‎utils/results/common.go

+26-1
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ func ApplyHandlerToScaVulnerabilities(target ScanTarget, vulnerabilities []servi
116116
return nil
117117
}
118118

119-
// 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.
119+
// Allows to iterate over the provided SCA violations and call the provided handler for each impacted component/package with a violation to process it.
120120
func ApplyHandlerToScaViolations(target ScanTarget, violations []services.Violation, entitledForJas bool, applicabilityRuns []*sarif.Run, securityHandler ParseScaViolationFunc, licenseHandler ParseScaViolationFunc, operationalRiskHandler ParseScaViolationFunc) (watches []string, failBuild bool, err error) {
121121
if securityHandler == nil && licenseHandler == nil && operationalRiskHandler == nil {
122122
return
@@ -145,6 +145,13 @@ func ApplyHandlerToScaViolations(target ScanTarget, violations []services.Violat
145145
// No handler was provided for security violations
146146
continue
147147
}
148+
149+
var skipNotApplicable bool
150+
if skipNotApplicable, err = shouldSkipNotApplicable(violation, applicabilityStatus); skipNotApplicable {
151+
log.Debug("A non-applicable violation was found and will be removed from final results as requested by its policies")
152+
continue
153+
}
154+
148155
for compIndex := 0; compIndex < len(impactedPackagesNames); compIndex++ {
149156
if e := securityHandler(
150157
violation, cves, applicabilityStatus, severity,
@@ -655,3 +662,21 @@ func GetIssueTechnology(responseTechnology string, targetTech techutils.Technolo
655662
// if no technology is provided, use the target technology
656663
return targetTech
657664
}
665+
666+
// Checks if the violation's applicability status is NotApplicable and if all of its policies states that non-applicable CVEs should be skipped
667+
func shouldSkipNotApplicable(violation services.Violation, applicabilityStatus jasutils.ApplicabilityStatus) (bool, error) {
668+
if applicabilityStatus != jasutils.NotApplicable {
669+
return false, nil
670+
}
671+
672+
if len(violation.Policies) == 0 {
673+
return false, errors.New("A violation with no policies was provided")
674+
}
675+
676+
for _, policy := range violation.Policies {
677+
if !policy.SkipNotApplicable {
678+
return false, nil
679+
}
680+
}
681+
return true, nil
682+
}

Diff for: ‎utils/results/common_test.go

+106
Original file line numberDiff line numberDiff line change
@@ -701,3 +701,109 @@ func TestGetFinalApplicabilityStatus(t *testing.T) {
701701
})
702702
}
703703
}
704+
705+
func TestShouldSkipNotApplicable(t *testing.T) {
706+
testCases := []struct {
707+
name string
708+
violation services.Violation
709+
applicabilityStatus jasutils.ApplicabilityStatus
710+
shouldSkip bool
711+
errorExpected bool
712+
}{
713+
{
714+
name: "Applicable CVE - should NOT skip",
715+
violation: services.Violation{},
716+
applicabilityStatus: jasutils.Applicable,
717+
shouldSkip: false,
718+
errorExpected: false,
719+
},
720+
{
721+
name: "Undetermined CVE - should NOT skip",
722+
violation: services.Violation{},
723+
applicabilityStatus: jasutils.ApplicabilityUndetermined,
724+
shouldSkip: false,
725+
errorExpected: false,
726+
},
727+
{
728+
name: "Not covered CVE - should NOT skip",
729+
violation: services.Violation{},
730+
applicabilityStatus: jasutils.NotCovered,
731+
shouldSkip: false,
732+
errorExpected: false,
733+
},
734+
{
735+
name: "Missing Context CVE - should NOT skip",
736+
violation: services.Violation{},
737+
applicabilityStatus: jasutils.MissingContext,
738+
shouldSkip: false,
739+
errorExpected: false,
740+
},
741+
{
742+
name: "Not scanned CVE - should NOT skip",
743+
violation: services.Violation{},
744+
applicabilityStatus: jasutils.NotScanned,
745+
shouldSkip: false,
746+
errorExpected: false,
747+
},
748+
{
749+
name: "Non applicable CVE with skip-non-applicable in ALL policies - SHOULD skip",
750+
violation: services.Violation{
751+
Policies: []services.Policy{
752+
{
753+
Policy: "policy-1",
754+
SkipNotApplicable: true,
755+
},
756+
{
757+
Policy: "policy-2",
758+
SkipNotApplicable: true,
759+
},
760+
},
761+
},
762+
applicabilityStatus: jasutils.NotApplicable,
763+
shouldSkip: true,
764+
errorExpected: false,
765+
},
766+
{
767+
name: "Non applicable CVE with skip-non-applicable in SOME policies - should NOT skip",
768+
violation: services.Violation{
769+
Policies: []services.Policy{
770+
{
771+
Policy: "policy-1",
772+
SkipNotApplicable: true,
773+
},
774+
{
775+
Policy: "policy-2",
776+
SkipNotApplicable: false,
777+
},
778+
},
779+
},
780+
applicabilityStatus: jasutils.NotApplicable,
781+
shouldSkip: false,
782+
errorExpected: false,
783+
},
784+
{
785+
name: "Violation without policy - error expected",
786+
violation: services.Violation{},
787+
applicabilityStatus: jasutils.NotApplicable,
788+
shouldSkip: false,
789+
errorExpected: true,
790+
},
791+
}
792+
793+
for _, tc := range testCases {
794+
t.Run(tc.name, func(t *testing.T) {
795+
shouldSkip, err := shouldSkipNotApplicable(tc.violation, tc.applicabilityStatus)
796+
if tc.errorExpected {
797+
assert.Error(t, err)
798+
} else {
799+
assert.NoError(t, err)
800+
}
801+
802+
if tc.shouldSkip {
803+
assert.True(t, shouldSkip)
804+
} else {
805+
assert.False(t, shouldSkip)
806+
}
807+
})
808+
}
809+
}

Diff for: ‎utils/results/conversion/convertor_test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -335,7 +335,7 @@ func getAuditTestResults(unique bool) (*results.SecurityCommandResults, validati
335335
ScannedStatus: "completed",
336336
})
337337
// Contextual analysis scan results
338-
npmTargetResults.JasResults.NewApplicabilityScanResults(0,
338+
npmTargetResults.JasResults.AddApplicabilityScanResults(0,
339339
&sarif.Run{
340340
Tool: sarif.Tool{
341341
Driver: sarifutils.CreateDummyDriver(validations.ContextualAnalysisToolName,
@@ -537,7 +537,7 @@ func getDockerScanTestResults(unique bool) (*results.SecurityCommandResults, val
537537
ScannedStatus: "completed",
538538
})
539539
// Contextual analysis scan results
540-
dockerImageTarget.JasResults.NewApplicabilityScanResults(0,
540+
dockerImageTarget.JasResults.AddApplicabilityScanResults(0,
541541
&sarif.Run{
542542
Tool: sarif.Tool{
543543
Driver: sarifutils.CreateDummyDriver(validations.ContextualAnalysisToolName,

Diff for: ‎utils/results/results.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -442,7 +442,7 @@ func (ssr *ScaScanResults) HasFindings() bool {
442442
return false
443443
}
444444

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

Diff for: ‎xsc_test.go

+57-3
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package main
33
import (
44
"encoding/json"
55
"errors"
6+
"os"
67
"path/filepath"
78
"testing"
89

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

104+
func TestXrayAuditJasSkipNotApplicableCvesViolations(t *testing.T) {
105+
// Init XSC tests also enabled analytics reporting.
106+
_, _, cleanUpXsc := integration.InitXscTest(t, func() { securityTestUtils.ValidateXrayVersion(t, services.MinXrayVersionGitRepoKey) })
107+
defer cleanUpXsc()
108+
// Create the audit command with git repo context injected.
109+
cliToRun, cleanUpHome := integration.InitTestWithMockCommandOrParams(t, false, getAuditCommandWithXscGitContext(validations.TestMockGitInfo))
110+
defer cleanUpHome()
111+
// Create the project to scan
112+
_, cleanUpProject := securityTestUtils.CreateTestProjectEnvAndChdir(t, filepath.Join(filepath.FromSlash(tests.GetTestResourcesPath()), "projects", "jas", "jas"))
113+
defer cleanUpProject()
114+
// Create policy and watch for the git repo so we will also get violations - This watch DO NOT skip not-applicable results
115+
var firstPolicyCleaned, firstWatchCleaned bool
116+
policyName, cleanUpPolicy := securityTestUtils.CreateTestSecurityPolicy(t, "without-skip-non-applicable-policy", utils.Low, false, false)
117+
defer func() {
118+
if !firstPolicyCleaned {
119+
cleanUpPolicy()
120+
}
121+
}()
122+
watchName, cleanUpWatch := securityTestUtils.CreateWatchForTests(t, policyName, "without-skip-not-applicable-watch", xscutils.GetGitRepoUrlKey(validations.TestMockGitInfo.GitRepoHttpsCloneUrl))
123+
defer func() {
124+
if !firstWatchCleaned {
125+
cleanUpWatch()
126+
}
127+
}()
128+
// Run the audit command with git repo and verify violations are reported to the platform.
129+
output, err := testAuditCommand(t, cliToRun, auditCommandTestParams{Format: string(format.SimpleJson), Watches: []string{watchName}, DisableFailOnFailedBuildFlag: true})
130+
assert.NoError(t, err)
131+
validations.VerifySimpleJsonResults(t, output, validations.ValidationParams{
132+
Violations: &validations.ViolationCount{ValidateScan: &validations.ScanCount{Sca: 17, Sast: 1, Secrets: 15}},
133+
ExactResultsMatch: true,
134+
})
135+
136+
// 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
137+
cleanUpWatch()
138+
firstWatchCleaned = true
139+
cleanUpPolicy()
140+
firstPolicyCleaned = true
141+
142+
// Create policy and watch for the git repo so we will also get violations - This watch DO NOT skip not-applicable results
143+
skipPolicyName, skipCleanUpPolicy := securityTestUtils.CreateTestSecurityPolicy(t, "skip-non-applicable-policy", utils.Low, false, true)
144+
defer skipCleanUpPolicy()
145+
skipWatchName, skipCleanUpWatch := securityTestUtils.CreateWatchForTests(t, skipPolicyName, "skip-not-applicable-watch", xscutils.GetGitRepoUrlKey(validations.TestMockGitInfo.GitRepoHttpsCloneUrl))
146+
defer skipCleanUpWatch()
147+
output, err = testAuditCommand(t, cliToRun, auditCommandTestParams{Format: string(format.SimpleJson), Watches: []string{skipWatchName}, DisableFailOnFailedBuildFlag: true})
148+
assert.NoError(t, err)
149+
validations.VerifySimpleJsonResults(t, output, validations.ValidationParams{
150+
Violations: &validations.ViolationCount{ValidateScan: &validations.ScanCount{Sca: 12, Sast: 1, Secrets: 15}},
151+
ExactResultsMatch: true,
152+
})
153+
154+
}
155+
103156
func TestAuditJasViolationsProjectKeySimpleJson(t *testing.T) {
104157
_, _, cleanUpXsc := integration.InitXscTest(t, func() { securityTestUtils.ValidateXrayVersion(t, services.MinXrayVersionGitRepoKey) })
105158
defer cleanUpXsc()
106-
if tests.TestJfrogPlatformProjectKeyEnvVar == "" {
159+
testsJfrogPlatformProjectKey := os.Getenv(tests.TestJfrogPlatformProjectKeyEnvVar)
160+
if testsJfrogPlatformProjectKey == "" {
107161
t.Skipf("skipping test. %s is not set.", tests.TestJfrogPlatformProjectKeyEnvVar)
108162
}
109163
// Create the audit command with git repo context injected.
@@ -114,7 +168,7 @@ func TestAuditJasViolationsProjectKeySimpleJson(t *testing.T) {
114168
_, cleanUpProject := securityTestUtils.CreateTestProjectEnvAndChdir(t, filepath.Join(filepath.FromSlash(tests.GetTestResourcesPath()), "projects", "jas", "jas"))
115169
defer cleanUpProject()
116170
// Create policy and watch for the project so we will get violations (unknown = all vulnerabilities will be reported as violations)
117-
policyName, cleanUpPolicy := securityTestUtils.CreateTestSecurityPolicy(t, "project-key-jas-violations-policy", utils.Unknown, true)
171+
policyName, cleanUpPolicy := securityTestUtils.CreateTestSecurityPolicy(t, "project-key-jas-violations-policy", utils.Unknown, true, false)
118172
defer cleanUpPolicy()
119173
_, cleanUpWatch := securityTestUtils.CreateTestProjectKeyWatch(t, policyName, "project-key-jas-violations-watch", *tests.JfrogTestProjectKey)
120174
defer cleanUpWatch()

0 commit comments

Comments
 (0)