Persist scan_finding violation data
Why are we doing this work
In Add violation_data to scan_result_policy_violat... (#433390 - closed) we're adding a new column violation_data
to be able to save details about what caused policy violations.
In this issue, we want to persist violation data for scan_finding
policies. For this reason, we should extend Security::ScanResultPolicies::UpdateApprovalsService and:
- Save
uuid
that caused the policy violations. - Save
SCAN_REMOVED
error
Relevant links
Non-functional requirements
-
Documentation: -
Feature flag: we should do these changes behind feature flag -
Performance: -
Testing:
Implementation plan
- Extend
UpdateApprovalsService
and useset_violation_data
method fromUpdateViolationsService
to store details about what caused violations:- UUIDs
- Errors (e.g. scan removed)
- Evaluated pipelines
A rough diff:
diff --git a/ee/app/services/security/scan_result_policies/update_approvals_service.rb b/ee/app/services/security/scan_result_policies/update_approvals_service.rb
index 82d9f0a64d0801e2b7cb7ec0161bfe30507a1a5a..6ee8af43ee65b726b5241f9f830055aa07954dba 100644
--- a/ee/app/services/security/scan_result_policies/update_approvals_service.rb
+++ b/ee/app/services/security/scan_result_policies/update_approvals_service.rb
@@ -59,6 +59,7 @@ def partition_rules(approval_rules)
approval_rule_name: approval_rule.name,
missing_scans: missing_scans(approval_rule)
)
+ violations.set_violation_data(approval_rule.scan_result_policy_id, { error: 'SCAN_REMOVED', missing_scans: missing_scans(approval_rule) })
next true
end
@@ -89,9 +90,15 @@ def log_update_approval_rule(message, **attributes)
def violates_approval_rule?(approval_rule)
target_pipeline_uuids = target_pipeline_findings_uuids(approval_rule)
+ pipeline_uuids = pipeline_findings_uuids(approval_rule)
+
+ return true if findings_count_violated?(approval_rule, pipeline_uuids, target_pipeline_uuids)
- return true if findings_count_violated?(approval_rule, target_pipeline_uuids)
- return true if preexisting_findings_count_violated?(approval_rule, target_pipeline_uuids)
+ if preexisting_findings_count_violated?(approval_rule, target_pipeline_uuids)
+ violations.set_violation_data(approval_rule.scan_result_policy_id,
+ { violations: { scan_finding: { uuids: { previously_existing: target_pipeline_uuids }, pipeline_ids: related_pipeline_ids, target_pipeline_ids: related_target_pipeline_ids } } })
+ return true
+ end
false
end
@@ -124,24 +131,36 @@ def target_pipeline
end
strong_memoize_attr :target_pipeline
- def findings_count_violated?(approval_rule, target_pipeline_uuids)
+ def findings_count_violated?(approval_rule, pipeline_uuids, target_pipeline_uuids)
vulnerabilities_allowed = approval_rule.vulnerabilities_allowed
- pipeline_uuids = pipeline_findings_uuids(approval_rule)
new_uuids = pipeline_uuids - target_pipeline_uuids
if only_newly_detected?(approval_rule)
- new_uuids.count > vulnerabilities_allowed
+ violated = new_uuids.count > vulnerabilities_allowed
+ if violated
+ violations.set_violation_data(approval_rule.scan_result_policy_id,
+ { violations: { scan_finding: { uuids: { newly_detected: new_uuids }, pipeline_ids: related_pipeline_ids, target_pipeline_ids: related_target_pipeline_ids } } })
+ end
+
+ violated
else
vulnerabilities_count = vulnerabilities_count_for_uuids(pipeline_uuids, approval_rule)
if vulnerabilities_count[:exceeded_allowed_count]
+ violations.set_violation_data(approval_rule.scan_result_policy_id,
+ { violations: { scan_finding: { uuids: { previously_existing: pipeline_uuids }, pipeline_ids: related_pipeline_ids, target_pipeline_ids: related_target_pipeline_ids } } })
true
- else
- total_count = vulnerabilities_count[:count]
- total_count += new_uuids.count if include_newly_detected?(approval_rule)
+ elsif include_newly_detected?(approval_rule)
+ total_count = vulnerabilities_count[:count] + new_uuids.count
+
+ violated = total_count > vulnerabilities_allowed
+ if violated
+ violations.set_violation_data(approval_rule.scan_result_policy_id,
+ { violations: { scan_finding: { uuids: { newly_detected: new_uuids }, pipeline_ids: related_pipeline_ids, target_pipeline_ids: related_target_pipeline_ids } } })
+ end
- total_count > vulnerabilities_allowed
+ violated
end
end
end
Verification steps
- Create a scan finding policy
- Configure
.gitlab-ci.yml
with appropriate scanners - Create MR causing violations (for example, adding a leaked secret)
- Check that
violation_data
in thescan_result_policy_violations
table contains correct data
Edited by Martin Čavoj