BE: Support new Status filtering options
Why are we doing this work
To support the frontend effort for #396985 (closed), we need to understand what changes are required on the backend to support new filtering options for the Status
field. With this new option, we can require approvals when new vulnerabilities are found with Dismissed
or Needs Triage
status.
Relevant links
Non-functional requirements
-
Documentation: Needs to be updated to reflect new option in Policy YAML !120376 (merged) -
Feature flag: No need for a feature flag as it will be a new option in Policy YAML -
Performance: -
Testing:
Implementation plan
-
backend Add new_needs_triage
andnew_dismissed
options tovulnerability_states
inee/app/validators/json_schemas/security_orchestration_policy.json
-
backend Update Gitlab::Ci::Reports::Security::Concerns::ScanFinding
to check for the new states and use them to calculate count:
diff --git a/ee/app/models/concerns/approval_rule_like.rb b/ee/app/models/concerns/approval_rule_like.rb
index d1698641ac4f..cd8a52f95ca4 100644
--- a/ee/app/models/concerns/approval_rule_like.rb
+++ b/ee/app/models/concerns/approval_rule_like.rb
@@ -10,6 +10,8 @@ module ApprovalRuleLike
APPROVALS_REQUIRED_MAX = 100
ALL_MEMBERS = 'All Members'
NEWLY_DETECTED = 'newly_detected'
+ NEW_NEEDS_TRIAGE = 'new_needs_triage'
+ NEW_DISMISSED = 'new_dismissed'
diff --git a/ee/lib/gitlab/ci/reports/security/concerns/scan_finding.rb b/ee/lib/gitlab/ci/reports/security/concerns/scan_finding.rb
index 92956f017833..9c934d024e6a 100644
--- a/ee/lib/gitlab/ci/reports/security/concerns/scan_finding.rb
+++ b/ee/lib/gitlab/ci/reports/security/concerns/scan_finding.rb
@@ -25,17 +25,25 @@ def violates_default_policy_against?(
preexisting_count > vulnerabilities_allowed
end
- def unsafe_findings_uuids(severity_levels, report_types)
- findings.select { |finding| finding.unsafe?(severity_levels, report_types) }.map(&:uuid)
+ def unsafe_findings_uuids(severity_levels, report_types, vulnerability_states)
+ if vulnerability_states.exclude?(ApprovalProjectRule::NEW_DISMISSED)
+ dismissed_uuids = pipeline.security_findings.undismissed_by_vulnerability.pluck(:uuid)
+ end
+
+ findings.select do |finding|
+ dismissed_uuids.exclude?(finding.uuid) && finding.unsafe?(severity_levels, report_types)
+ end.map(&:uuid)
end
private
def unsafe_findings_count(target_reports, severity_levels, vulnerability_states, report_types)
- pipeline_uuids = unsafe_findings_uuids(severity_levels, report_types)
+ pipeline_uuids = unsafe_findings_uuids(severity_levels, report_types, [])
pipeline_count = count_by_uuid(pipeline_uuids, vulnerability_states)
- new_uuids = pipeline_uuids - target_reports&.unsafe_findings_uuids(severity_levels, report_types).to_a
+ new_uuids = pipeline_uuids - target_reports&.unsafe_findings_uuids(
+ severity_levels, report_types, vulnerability_states
+ ).to_a
pipeline_count += new_uuids.count if vulnerability_states.include?(ApprovalProjectRule::NEWLY_DETECTED)
@@ -48,7 +56,13 @@ def preexisting_findings_count(target_reports, severity_levels, vulnerability_st
end
def count_by_uuid(uuids, states)
- states_without_newly_detected = states.reject { |state| ApprovalProjectRule::NEWLY_DETECTED == state }
+ states_without_newly_detected = states.reject do |state|
+ [
+ ApprovalProjectRule::NEWLY_DETECTED,
+ ApprovalProjectRule::NEW_NEEDS_TRIAGE,
+ ApprovalProjectRule::NEW_DISMISSED
+ ].include?(state)
+ end
uuids.each_slice(COUNT_BATCH_SIZE).sum do |uuids_batch|
pipeline
-
backend Update Security::ScanResultPolicies::UpdateApprovalsService
to check for the new states and use them to calculate uuids:
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 3a681f9bafc4..ba63abff62d6 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
@@ -49,7 +49,7 @@ def target_pipeline_security_findings
def findings_count_violated?(approval_rule, target_pipeline_uuids)
vulnerabilities_allowed = approval_rule.vulnerabilities_allowed
- pipeline_uuids = uuids_from_findings(pipeline_security_findings, approval_rule)
+ pipeline_uuids = uuids_from_findings(pipeline_security_findings, approval_rule, true)
new_uuids = pipeline_uuids - target_pipeline_uuids
if only_newly_detected?(approval_rule)
@@ -76,9 +76,18 @@ def preexisting_findings_count_violated?(approval_rule, target_pipeline_uuids)
vulnerabilities_count[:exceeded_allowed_count]
end
- def uuids_from_findings(security_findings, approval_rule)
+ def uuids_from_findings(security_findings, approval_rule, check_dismissed = false)
+ vulnerability_states = approval_rule.vulnerability_states_for_branch
+
findings = security_findings.by_severity_levels(approval_rule.severity_levels)
findings = findings.by_report_types(approval_rule.scanners) if approval_rule.scanners.present?
+
+ if check_dismissed &&
+ vulnerability_states.exclude?(ApprovalProjectRule::NEW_DISMISSED) &&
+ vulnerability_states.include?(ApprovalProjectRule::NEW_NEEDS_TRIAGE)
+ findings = findings.undismissed_by_vulnerability
+ end
+
findings.fetch_uuids
end
Verification steps
-
Follow the test cases mentioned in spreadsheet(internal) for Status(1)
andStatus(2)
columns
Edited by Marcos Rocha