Skip to content

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

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
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

Edited by Marcos Rocha