Merge branch 'add_vulnerability_states_into_security_rules' into 'master'

See merge request gitlab-org/gitlab!72405

app/models/concerns/enums/vulnerability.rb

@@ -37,6 +37,15 @@ module Enums
       security_audit: 4
     }.with_indifferent_access.freeze
 
+    # keep the order of the values in the state enum, it is used in state_order method to properly order vulnerabilities based on state
+    # remember to recreate index_vulnerabilities_on_state_case_id index when you update or extend this enum
+    VULNERABILITY_STATES = {
+      detected: 1,
+      confirmed: 4,
+      resolved: 3,
+      dismissed: 2
+    }.with_indifferent_access.freeze
+
     def self.confidence_levels
       CONFIDENCE_LEVELS
     end
@@ -52,6 +61,10 @@ module Enums
     def self.detection_methods
       DETECTION_METHODS
     end
+
+    def self.vulnerability_states
+      VULNERABILITY_STATES
+    end
   end
 end
 

db/migrate/20211013192749_add_states_into_approval_project_rules.rb

+# frozen_string_literal: true
+
+class AddStatesIntoApprovalProjectRules < Gitlab::Database::Migration[1.0]
+  def up
+    add_column :approval_project_rules, :vulnerability_states, :text, array: true, null: false, default: ['newly_detected']
+  end
+
+  def down
+    remove_column :approval_project_rules, :vulnerability_states
+  end
+end

db/schema_migrations/20211013192749

+eeda27c42a80d23851bb58b00cee79feeffbe9ae1fef76b3034f92c8610a8aaf
\ No newline at end of file

db/structure.sql

@@ -10579,7 +10579,8 @@ CREATE TABLE approval_project_rules (
     scanners text[],
     vulnerabilities_allowed smallint DEFAULT 0 NOT NULL,
     severity_levels text[] DEFAULT '{}'::text[] NOT NULL,
-    report_type smallint
+    report_type smallint,
+    vulnerability_states text[] DEFAULT '{newly_detected}'::text[] NOT NULL
 );
 
 CREATE TABLE approval_project_rules_groups (

ee/app/assets/javascripts/approvals/components/rule_form.vue

@@ -14,6 +14,7 @@ import {
   VULNERABILITY_CHECK_NAME,
   COVERAGE_CHECK_NAME,
   APPROVAL_DIALOG_I18N,
+  APPROVAL_VULNERABILITY_STATES,
 } from '../constants';
 import ApproversList from './approvers_list.vue';
 import ApproversSelect from './approvers_select.vue';
@@ -72,6 +73,10 @@ export default {
       serverValidationErrors: [],
       scanners: [],
       severityLevels: [],
+      vulnerabilityStates: [],
+      approvalVulnerabilityStatesKeys: Object.keys(APPROVAL_VULNERABILITY_STATES),
+      reportTypesKeys: Object.keys(REPORT_TYPES),
+      severityLevelsKeys: Object.keys(SEVERITY_LEVELS),
       ...this.getInitialData(),
     };
   },
@@ -144,11 +149,7 @@ export default {
       return '';
     },
     invalidScanners() {
-      if (this.scanners.length <= 0) {
-        return APPROVAL_DIALOG_I18N.validations.scannersRequired;
-      }
-
-      return '';
+      return this.scanners.length <= 0;
     },
     invalidVulnerabilitiesAllowedError() {
       if (!isNumber(this.vulnerabilitiesAllowed)) {
@@ -161,11 +162,10 @@ export default {
       return '';
     },
     invalidSeverityLevels() {
-      if (this.severityLevels.length === 0) {
-        return APPROVAL_DIALOG_I18N.validations.severityLevelsRequired;
-      }
-
-      return '';
+      return this.severityLevels.length === 0;
+    },
+    invalidVulnerabilityStates() {
+      return this.vulnerabilityStates.length === 0;
     },
     isValid() {
       return (
@@ -175,7 +175,8 @@ export default {
         this.isValidApprovers &&
         this.areValidScanners &&
         this.isValidVulnerabilitiesAllowed &&
-        this.areValidSeverityLevels
+        this.areValidSeverityLevels &&
+        this.areValidVulnerabilityStates
       );
     },
     isValidName() {
@@ -203,6 +204,9 @@ export default {
     areValidSeverityLevels() {
       return !this.showValidation || !this.isVulnerabilityCheck || !this.invalidSeverityLevels;
     },
+    areValidVulnerabilityStates() {
+      return !this.showValidation || !this.isVulnerabilityCheck || !this.invalidVulnerabilityStates;
+    },
     isMultiSubmission() {
       return this.settings.allowMultiRule && !this.isFallbackSubmission;
     },
@@ -242,6 +246,7 @@ export default {
         protectedBranchIds: this.branches.map((x) => x.id),
         scanners: this.scanners,
         severityLevels: this.severityLevels,
+        vulnerabilityStates: this.vulnerabilityStates,
       };
     },
     isEditing() {
@@ -251,11 +256,11 @@ export default {
       return VULNERABILITY_CHECK_NAME === this.name;
     },
     areAllScannersSelected() {
-      return this.scanners.length === Object.values(this.$options.REPORT_TYPES).length;
+      return this.scanners.length === this.reportTypesKeys.length;
     },
     scannersText() {
       switch (this.scanners.length) {
-        case Object.values(this.$options.REPORT_TYPES).length:
+        case this.reportTypesKeys.length:
           return APPROVAL_DIALOG_I18N.form.allScannersSelectedLabel;
         case 0:
           return APPROVAL_DIALOG_I18N.form.scannersSelectLabel;
@@ -269,11 +274,11 @@ export default {
       }
     },
     areAllSeverityLevelsSelected() {
-      return this.severityLevels.length === Object.values(this.$options.SEVERITY_LEVELS).length;
+      return this.severityLevels.length === this.severityLevelsKeys.length;
     },
     severityLevelsText() {
       switch (this.severityLevels.length) {
-        case Object.keys(this.$options.SEVERITY_LEVELS).length:
+        case this.severityLevelsKeys.length:
           return APPROVAL_DIALOG_I18N.form.allSeverityLevelsSelectedLabel;
         case 0:
           return APPROVAL_DIALOG_I18N.form.severityLevelsSelectLabel;
@@ -286,6 +291,24 @@ export default {
           });
       }
     },
+    vulnerabilityStatesText() {
+      switch (this.vulnerabilityStates.length) {
+        case this.approvalVulnerabilityStatesKeys.length:
+          return APPROVAL_DIALOG_I18N.form.allVulnerabilityStatesSelectedLabel;
+        case 0:
+          return APPROVAL_DIALOG_I18N.form.vulnerabilityStatesSelectLabel;
+        case 1:
+          return APPROVAL_VULNERABILITY_STATES[this.vulnerabilityStates[0]];
+        default:
+          return sprintf(APPROVAL_DIALOG_I18N.form.multipleSelectedLabel, {
+            firstLabel: APPROVAL_VULNERABILITY_STATES[this.vulnerabilityStates[0]],
+            numberOfAdditionalLabels: this.vulnerabilityStates.length - 1,
+          });
+      }
+    },
+    areAllVulnerabilityStatesSelected() {
+      return this.vulnerabilityStates.length === this.approvalVulnerabilityStatesKeys.length;
+    },
   },
   watch: {
     approversToAdd(value) {
@@ -404,10 +427,11 @@ export default {
         scanners: this.initRule.scanners || [],
         vulnerabilitiesAllowed: this.initRule.vulnerabilitiesAllowed || 0,
         severityLevels: this.initRule.severityLevels || [],
+        vulnerabilityStates: this.initRule.vulnerabilityStates || [],
       };
     },
     setAllSelectedScanners() {
-      this.scanners = this.areAllScannersSelected ? [] : Object.keys(this.$options.REPORT_TYPES);
+      this.scanners = this.areAllScannersSelected ? [] : this.reportTypesKeys;
     },
     isScannerSelected(scanner) {
       return this.scanners.includes(scanner);
@@ -421,9 +445,7 @@ export default {
       }
     },
     setAllSelectedSeverityLevels() {
-      this.severityLevels = this.areAllSeverityLevelsSelected
-        ? []
-        : Object.keys(this.$options.SEVERITY_LEVELS);
+      this.severityLevels = this.areAllSeverityLevelsSelected ? [] : this.severityLevelsKeys;
     },
     isSeveritySelected(severity) {
       return this.severityLevels.includes(severity);
@@ -436,10 +458,27 @@ export default {
         this.severityLevels.splice(pos, 1);
       }
     },
+    setAllSelectedVulnerabilityStates() {
+      this.vulnerabilityStates = this.areAllVulnerabilityStatesSelected
+        ? []
+        : this.approvalVulnerabilityStatesKeys;
+    },
+    isVulnerabilityStateSelected(vulnerability) {
+      return this.vulnerabilityStates.includes(vulnerability);
+    },
+    setVulnerabilityState(vulnerability) {
+      const pos = this.vulnerabilityStates.indexOf(vulnerability);
+      if (pos === -1) {
+        this.vulnerabilityStates.push(vulnerability);
+      } else {
+        this.vulnerabilityStates.splice(pos, 1);
+      }
+    },
   },
   APPROVAL_DIALOG_I18N,
   REPORT_TYPES: omit(REPORT_TYPES, EXCLUDED_REPORT_TYPE),
   SEVERITY_LEVELS,
+  APPROVAL_VULNERABILITY_STATES,
 };
 </script>
 
@@ -466,7 +505,7 @@ export default {
       :label="$options.APPROVAL_DIALOG_I18N.form.scannersLabel"
       :description="$options.APPROVAL_DIALOG_I18N.form.scannersDescription"
       :state="areValidScanners"
-      :invalid-feedback="invalidScanners"
+      :invalid-feedback="$options.APPROVAL_DIALOG_I18N.validations.scannersRequired"
       data-testid="scanners-group"
     >
       <gl-dropdown :text="scannersText">
@@ -504,6 +543,34 @@ export default {
         :selected-branches="branches"
       />
     </gl-form-group>
+    <gl-form-group
+      v-if="isVulnerabilityCheck"
+      :label="$options.APPROVAL_DIALOG_I18N.form.vulnerabilityStatesLabel"
+      :description="$options.APPROVAL_DIALOG_I18N.form.vulnerabilityStatesDescription"
+      :state="areValidVulnerabilityStates"
+      :invalid-feedback="$options.APPROVAL_DIALOG_I18N.validations.vulnerabilityStatesRequired"
+      data-testid="vulnerability-states-group"
+    >
+      <gl-dropdown :text="vulnerabilityStatesText">
+        <gl-dropdown-item
+          key="all"
+          is-check-item
+          :is-checked="areAllVulnerabilityStatesSelected"
+          @click.native.capture.stop="setAllSelectedVulnerabilityStates"
+        >
+          <gl-truncate :text="$options.APPROVAL_DIALOG_I18N.form.selectAllLabel" />
+        </gl-dropdown-item>
+        <gl-dropdown-item
+          v-for="(value, key) in $options.APPROVAL_VULNERABILITY_STATES"
+          :key="key"
+          is-check-item
+          :is-checked="isVulnerabilityStateSelected(key)"
+          @click.native.capture.stop="setVulnerabilityState(key)"
+        >
+          <gl-truncate :text="value" />
+        </gl-dropdown-item>
+      </gl-dropdown>
+    </gl-form-group>
     <gl-form-group
       v-if="isVulnerabilityCheck"
       :label="$options.APPROVAL_DIALOG_I18N.form.vulnerabilitiesAllowedLabel"
@@ -526,7 +593,7 @@ export default {
       :label="$options.APPROVAL_DIALOG_I18N.form.severityLevelsLabel"
       :description="$options.APPROVAL_DIALOG_I18N.form.severityLevelsDescription"
       :state="areValidSeverityLevels"
-      :invalid-feedback="invalidSeverityLevels"
+      :invalid-feedback="$options.APPROVAL_DIALOG_I18N.validations.severityLevelsRequired"
       data-testid="severity-levels-group"
     >
       <gl-dropdown :text="severityLevelsText">

ee/app/assets/javascripts/approvals/constants.js

@@ -118,6 +118,12 @@ export const APPROVAL_DIALOG_I18N = {
     vulnerabilitiesAllowedDescription: s__(
       'ApprovalRule|Number of vulnerabilities allowed before approval rule is triggered.',
     ),
+    vulnerabilityStatesLabel: s__('ApprovalRule|Vulnerability states'),
+    vulnerabilityStatesDescription: s__(
+      'ApprovalRule|Apply this approval rule to consider only the selected vulnerability states.',
+    ),
+    vulnerabilityStatesSelectLabel: s__('ApprovalRule|Select vulnerability states'),
+    allVulnerabilityStatesSelectedLabel: s__('ApprovalRule|All vulnerability states'),
     severityLevelsLabel: s__('ApprovalRule|Severity levels'),
     severityLevelsDescription: s__(
       'ApprovalRule|Apply this approval rule to consider only the selected severity levels.',
@@ -140,5 +146,14 @@ export const APPROVAL_DIALOG_I18N = {
       'ApprovalRule|Please enter a number equal or greater than zero',
     ),
     severityLevelsRequired: s__('ApprovalRule|Please select at least one severity level'),
+    vulnerabilityStatesRequired: s__('ApprovalRule|Please select at least one vulnerability state'),
   },
 };
+
+export const APPROVAL_VULNERABILITY_STATES = {
+  newly_detected: s__('ApprovalRule|Newly detected'),
+  detected: s__('ApprovalRule|Previously detected'),
+  confirmed: s__('ApprovalRule|Confirmed'),
+  dismissed: s__('ApprovalRule|Dismissed'),
+  resolved: s__('ApprovalRule|Resolved'),
+};

ee/app/assets/javascripts/approvals/mappers.js

-import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils';
+import {
+  convertObjectPropsToCamelCase,
+  convertObjectPropsToSnakeCase,
+} from '~/lib/utils/common_utils';
 import {
   RULE_TYPE_REGULAR,
   RULE_TYPE_ANY_APPROVER,
@@ -39,15 +42,7 @@ function reportTypeFromName(ruleName) {
 }
 
 export const mapApprovalRuleRequest = (req) => ({
-  name: req.name,
-  approvals_required: req.approvalsRequired,
-  users: req.users,
-  groups: req.groups,
-  remove_hidden_groups: req.removeHiddenGroups,
-  protected_branch_ids: req.protectedBranchIds,
-  scanners: req.scanners,
-  vulnerabilities_allowed: req.vulnerabilitiesAllowed,
-  severity_levels: req.severityLevels,
+  ...convertObjectPropsToSnakeCase(req),
   report_type: reportTypeFromName(req.name),
   rule_type: ruleTypeFromName(req.name),
 });
@@ -57,21 +52,9 @@ export const mapApprovalFallbackRuleRequest = (req) => ({
 });
 
 export const mapApprovalRuleResponse = (res) => ({
-  id: res.id,
+  ...convertObjectPropsToCamelCase(res),
   hasSource: Boolean(res.source_rule),
-  name: res.name,
-  approvalsRequired: res.approvals_required,
   minApprovalsRequired: 0,
-  approvers: res.approvers,
-  containsHiddenGroups: res.contains_hidden_groups,
-  users: res.users,
-  groups: res.groups,
-  ruleType: res.rule_type,
-  protectedBranches: res.protected_branches,
-  overridden: res.overridden,
-  scanners: res.scanners,
-  vulnerabilitiesAllowed: res.vulnerabilities_allowed,
-  severityLevels: res.severity_levels,
 });
 
 export const mapApprovalSettingsResponse = (res) => ({

ee/app/models/approval_project_rule.rb

@@ -7,6 +7,9 @@ class ApprovalProjectRule < ApplicationRecord
   UNSUPPORTED_SCANNER = 'cluster_image_scanning'
   SUPPORTED_SCANNERS = (::Ci::JobArtifact::SECURITY_REPORT_FILE_TYPES - [UNSUPPORTED_SCANNER]).freeze
   DEFAULT_SEVERITIES = %w[unknown high critical].freeze
+  NEWLY_DETECTED = 'newly_detected'
+  NEWLY_DETECTED_STATE = { NEWLY_DETECTED.to_sym => 0 }.freeze
+  APPROVAL_VULNERABILITY_STATES = ::Enums::Vulnerability.vulnerability_states.merge(NEWLY_DETECTED_STATE).freeze
 
   belongs_to :project
   has_and_belongs_to_many :protected_branches
@@ -37,6 +40,8 @@ class ApprovalProjectRule < ApplicationRecord
   validates :severity_levels, inclusion: { in: ::Enums::Vulnerability.severity_levels.keys }
   default_value_for :severity_levels, allows_nil: false, value: DEFAULT_SEVERITIES
 
+  validates :vulnerability_states, inclusion: { in: APPROVAL_VULNERABILITY_STATES.keys }
+
   def applies_to_branch?(branch)
     return true if protected_branches.empty?
 
@@ -65,6 +70,14 @@ class ApprovalProjectRule < ApplicationRecord
     push_audit_event("Removed #{model.class.name} #{model.name} from approval group on #{self.name} rule")
   end
 
+  def vulnerability_states_for_branch(branch = project.default_branch)
+    if applies_to_branch?(branch)
+      self.vulnerability_states
+    else
+      self.vulnerability_states.select { |state| NEWLY_DETECTED == state }
+    end
+  end
+
   private
 
   def report_approver_attributes

ee/app/models/ee/vulnerability.rb

@@ -57,9 +57,7 @@ module EE
       has_many :notes, as: :noteable, dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent
       has_many :user_mentions, class_name: 'VulnerabilityUserMention'
 
-      # keep the order of the values in the state enum, it is used in state_order method to properly order vulnerabilities based on state
-      # remember to recreate index_vulnerabilities_on_state_case_id index when you update or extend this enum
-      enum state: { detected: 1, confirmed: 4, resolved: 3, dismissed: 2 }
+      enum state: ::Enums::Vulnerability.vulnerability_states
       enum severity: ::Enums::Vulnerability.severity_levels, _prefix: :severity
       enum confidence: ::Enums::Vulnerability.confidence_levels, _prefix: :confidence
       enum report_type: ::Enums::Vulnerability.report_types
@@ -74,6 +72,7 @@ module EE
 
       scope :with_author_and_project, -> { includes(:author, :project) }
       scope :with_findings, -> { includes(:findings) }
+      scope :with_findings_by_uuid_and_state, -> (uuid, state) { with_findings.where(findings: { uuid: uuid }, state: state) }
       scope :with_findings_and_scanner, -> { includes(findings: :scanner) }
       scope :with_findings_scanner_and_identifiers, -> { includes(findings: [:scanner, :identifiers, finding_identifiers: :identifier]) }
       scope :with_created_issue_links_and_issues, -> { includes(created_issue_links: :issue) }

ee/app/services/ci/sync_reports_to_approval_rules_service.rb

@@ -23,7 +23,7 @@ module Ci
       log_error(payload)
       error("Failed to update approval rules")
     ensure
-      [:project_rule_vulnerabilities_allowed, :project_rule_scanners, :project_rule_severity_levels, :project_vulnerability_report, :reports].each do |memoization|
+      [:project_rule_vulnerabilities_allowed, :project_rule_scanners, :project_rule_severity_levels, :project_vulnerability_report, :reports, :project_rule_vulnerability_states].each do |memoization|
         clear_memoization(memoization)
       end
     end
@@ -86,7 +86,7 @@ module Ci
 
     def merge_requests_approved_security_reports
       pipeline.merge_requests_as_head_pipeline.reject do |merge_request|
-        reports.present? && reports.violates_default_policy_against?(merge_request.base_pipeline&.security_reports, project_rule_vulnerabilities_allowed, project_rule_severity_levels)
+        reports.present? && reports.violates_default_policy_against?(merge_request.base_pipeline&.security_reports, project_rule_vulnerabilities_allowed, project_rule_severity_levels, project_rule_vulnerability_states)
       end
     end
 
@@ -102,6 +102,12 @@ module Ci
       end
     end
 
+    def project_rule_vulnerability_states
+      strong_memoize(:project_rule_vulnerability_states) do
+        project_vulnerability_report&.vulnerability_states_for_branch
+      end
+    end
+
     def project_vulnerability_report
       strong_memoize(:project_vulnerability_report) do
         pipeline.project.vulnerability_report_rule

ee/lib/api/helpers/project_approval_rules_helpers.rb

@@ -16,6 +16,7 @@ module API
         optional :vulnerabilities_allowed, type: Integer, desc: 'The number of vulnerabilities allowed for this rule'
         optional :severity_levels, type: Array[String], desc: 'The security levels to be considered by the approval rule'
         optional :report_type, type: String, desc: 'The type of the report required when rule type equals to report_approver'
+        optional :vulnerability_states, type: Array[String], desc: 'The vulnerability states to be considered by the approval rule'
       end
 
       params :update_project_approval_rule do
@@ -29,6 +30,7 @@ module API
         optional :scanners, type: Array[String], desc: 'The security scanners to be considered by the approval rule'
         optional :vulnerabilities_allowed, type: Integer, desc: 'The number of vulnerabilities allowed for this rule'
         optional :severity_levels, type: Array[String], desc: 'The security levels to be considered by the approval rule'
+        optional :vulnerability_states, type: Array[String], desc: 'The vulnerability states to be considered by the approval rule'
       end
 
       params :delete_project_approval_rule do

ee/lib/ee/api/entities/project_approval_setting_rule.rb

@@ -12,6 +12,7 @@ module EE
         expose :scanners, override: true
         expose :vulnerabilities_allowed, override: true
         expose :severity_levels, override: true
+        expose :vulnerability_states, override: true
       end
     end
   end

ee/lib/ee/gitlab/ci/reports/security/reports.rb

+# frozen_string_literal: true
+
+module EE
+  module Gitlab
+    module Ci
+      module Reports
+        module Security
+          module Reports
+            extend ::Gitlab::Utils::Override
+
+            private
+
+            override :unsafe_findings_count
+            def unsafe_findings_count(target_reports, severity_levels, vulnerability_states)
+              pipeline_uuids = unsafe_findings_uuids(severity_levels)
+              pipeline_count = count_by_uuid(pipeline_uuids, vulnerability_states)
+
+              new_uuids = pipeline_uuids - target_reports&.unsafe_findings_uuids(severity_levels).to_a
+
+              if vulnerability_states.include?(ApprovalProjectRule::NEWLY_DETECTED)
+                pipeline_count += new_uuids.count
+              end
+
+              pipeline_count
+            end
+
+            def count_by_uuid(uuids, states)
+              pipeline.project.vulnerabilities.with_findings_by_uuid_and_state(uuids, states.reject { |state| ApprovalProjectRule::NEWLY_DETECTED == state }).count
+            end
+          end
+        end
+      end
+    end
+  end
+end

ee/spec/fixtures/api/schemas/public_api/v4/project_approval_setting.json

@@ -46,6 +46,12 @@
       "items": {
         "type": "string"
       }
+    },
+    "vulnerability_states":{
+      "type": "array",
+      "items": {
+        "type": "string"
+      }
     }
   },
   "additionalProperties": false

ee/spec/frontend/approvals/components/rule_form_spec.js

-import { GlFormGroup, GlFormInput, GlTruncate } from '@gitlab/ui';
+import { GlDropdown, GlFormGroup, GlFormInput, GlTruncate } from '@gitlab/ui';
 import { shallowMount } from '@vue/test-utils';
 import Vue, { nextTick } from 'vue';
 import Vuex from 'vuex';
@@ -13,6 +13,8 @@ import {
   TYPE_GROUP,
   TYPE_HIDDEN_GROUPS,
   VULNERABILITY_CHECK_NAME,
+  APPROVAL_VULNERABILITY_STATES,
+  APPROVAL_DIALOG_I18N,
 } from 'ee/approvals/constants';
 import { createStoreOptions } from 'ee/approvals/stores';
 import projectSettingsModule from 'ee/approvals/stores/modules/project_settings';
@@ -21,28 +23,14 @@ import ProtectedBranchesSelector from 'ee/vue_shared/components/branches_selecto
 import { stubComponent } from 'helpers/stub_component';
 import { extendedWrapper } from 'helpers/vue_test_utils_helper';
 import waitForPromises from 'helpers/wait_for_promises';
+import {
+  TEST_RULE,
+  TEST_RULE_VULNERABILITY_CHECK,
+  TEST_PROTECTED_BRANCHES,
+  TEST_RULE_WITH_PROTECTED_BRANCHES,
+} from '../mocks';
 
 const TEST_PROJECT_ID = '7';
-const TEST_RULE = {
-  id: 10,
-  name: 'QA',
-  approvalsRequired: 2,
-  users: [{ id: 1 }, { id: 2 }, { id: 3 }],
-  groups: [{ id: 1 }, { id: 2 }],
-};
-const TEST_PROTECTED_BRANCHES = [{ id: 2 }, { id: 3 }, { id: 4 }];
-const TEST_RULE_WITH_PROTECTED_BRANCHES = {
-  ...TEST_RULE,
-  protectedBranches: TEST_PROTECTED_BRANCHES,
-};
-const TEST_RULE_VULNERABILITY_CHECK = {
-  ...TEST_RULE,
-  id: null,
-  name: VULNERABILITY_CHECK_NAME,
-  scanners: ['sast', 'dast'],
-  vulnerabilitiesAllowed: 0,
-  severityLevels: ['high'],
-};
 const TEST_APPROVERS = [{ id: 7, type: TYPE_USER }];
 const TEST_APPROVALS_REQUIRED = 3;
 const TEST_FALLBACK_RULE = {
@@ -101,6 +89,9 @@ describe('EE Approvals RuleForm', () => {
   const findScannersGroup = () => wrapper.findByTestId('scanners-group');
   const findVulnerabilityFormGroup = () => wrapper.findByTestId('vulnerability-amount-group');
   const findSeverityLevelsGroup = () => wrapper.findByTestId('severity-levels-group');
+  const findVulnerabilityStatesGroup = () => wrapper.findByTestId('vulnerability-states-group');
+  const findVulnerabilityStatesDropdown = () =>
+    findVulnerabilityStatesGroup().findComponent(GlDropdown);
 
   const inputsAreValid = (inputs) => inputs.every((x) => x.props('state'));
 
@@ -207,6 +198,7 @@ describe('EE Approvals RuleForm', () => {
             scanners: [],
             severityLevels: [],
             protectedBranchIds: branches.map((x) => x.id),
+            vulnerabilityStates: [],
           };
 
           await findNameInput().vm.$emit('input', expected.name);
@@ -287,6 +279,7 @@ describe('EE Approvals RuleForm', () => {
           scanners: [],
           severityLevels: [],
           protectedBranchIds: branches.map((x) => x.id),
+          vulnerabilityStates: [],
         };
 
         beforeEach(async () => {
@@ -368,6 +361,7 @@ describe('EE Approvals RuleForm', () => {
           scanners: [],
           severityLevels: [],
           protectedBranchIds: [],
+          vulnerabilityStates: [],
         };
 
         it('on submit, puts rule', async () => {
@@ -720,6 +714,95 @@ describe('EE Approvals RuleForm', () => {
           );
         });
       });
+
+      describe('without any vulnerability state selected', () => {
+        beforeEach(() => {
+          createComponent({
+            initRule: {
+              ...TEST_RULE_VULNERABILITY_CHECK,
+              vulnerabilityStates: [],
+            },
+          });
+          findForm().trigger('submit');
+        });
+
+        it('does not dispatch the action on submit', () => {
+          expect(actions.postRule).not.toHaveBeenCalled();
+        });
+
+        it('changes vulnerability states dropdown text to select vulnerability states', () => {
+          expect(findVulnerabilityStatesDropdown().props('text')).toBe(
+            APPROVAL_DIALOG_I18N.form.vulnerabilityStatesSelectLabel,
+          );
+        });
+
+        it('shows error message in regards to vulnerability states selection', () => {
+          expect(findVulnerabilityStatesGroup().props('invalidFeedback')).toBe(
+            APPROVAL_DIALOG_I18N.validations.vulnerabilityStatesRequired,
+          );
+        });
+      });
+
+      describe('with one vulnerability state selected', () => {
+        beforeEach(() => {
+          createComponent({
+            initRule: {
+              ...TEST_RULE_VULNERABILITY_CHECK,
+              vulnerabilityStates: ['newly_detected'],
+            },
+          });
+          findForm().trigger('submit');
+        });
+
+        it('dispatches the action on submit', () => {
+          expect(actions.postRule).toHaveBeenCalledWith(
+            expect.anything(),
+            expect.objectContaining({
+              vulnerabilityStates: TEST_RULE_VULNERABILITY_CHECK.vulnerabilityStates,
+            }),
+          );
+        });
+
+        it('changes vulnerability states dropdown text to its name', () => {
+          expect(findVulnerabilityStatesDropdown().props('text')).toBe(
+            APPROVAL_VULNERABILITY_STATES.newly_detected,
+          );
+        });
+      });
+
+      describe('with all vulnerability states selected', () => {
+        beforeEach(() => {
+          createComponent({
+            initRule: {
+              ...TEST_RULE_VULNERABILITY_CHECK,
+              vulnerabilityStates: Object.keys(APPROVAL_VULNERABILITY_STATES),
+            },
+          });
+        });
+
+        it('changes vulnerability states dropdown text to all selected', () => {
+          expect(findVulnerabilityStatesDropdown().props('text')).toBe(
+            APPROVAL_DIALOG_I18N.form.allVulnerabilityStatesSelectedLabel,
+          );
+        });
+      });
+
+      describe('with all but one vulnerability state selected', () => {
+        beforeEach(() => {
+          createComponent({
+            initRule: {
+              ...TEST_RULE_VULNERABILITY_CHECK,
+              vulnerabilityStates: Object.keys(APPROVAL_VULNERABILITY_STATES).splice(1),
+            },
+          });
+        });
+
+        it('changes vulnerability states dropdown text to all selected', () => {
+          expect(findVulnerabilityStatesDropdown().props('text')).toBe(
+            'Previously detected +3 more',
+          );
+        });
+      });
     });
   });
 

ee/spec/frontend/approvals/mocks.js

@@ -93,3 +93,28 @@ export const createGroupApprovalsState = (locked = null) => ({
     },
   },
 });
+
+export const TEST_PROTECTED_BRANCHES = [{ id: 2 }, { id: 3 }, { id: 4 }];
+
+export const TEST_RULE = {
+  id: 10,
+  name: 'QA',
+  approvalsRequired: 2,
+  users: [{ id: 1 }, { id: 2 }, { id: 3 }],
+  groups: [{ id: 1 }, { id: 2 }],
+};
+
+export const TEST_RULE_VULNERABILITY_CHECK = {
+  ...TEST_RULE,
+  id: null,
+  name: 'Vulnerability-Check',
+  scanners: ['sast', 'dast'],
+  vulnerabilitiesAllowed: 0,
+  severityLevels: ['high'],
+  vulnerabilityStates: ['newly_detected'],
+};
+
+export const TEST_RULE_WITH_PROTECTED_BRANCHES = {
+  ...TEST_RULE,
+  protectedBranches: TEST_PROTECTED_BRANCHES,
+};

ee/spec/lib/ee/gitlab/ci/reports/security/reports_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::Ci::Reports::Security::Reports do
+  let_it_be(:pipeline) { create(:ci_pipeline) }
+  let_it_be(:artifact) { create(:ci_job_artifact, :sast) }
+
+  let(:security_reports) { described_class.new(pipeline) }
+
+  describe "#violates_default_policy_against?" do
+    let(:high_severity_dast) { build(:ci_reports_security_finding, severity: 'high', report_type: :dast) }
+    let(:vulnerabilities_allowed) { 0 }
+    let(:severity_levels) { %w(critical high) }
+    let(:vulnerability_states) { %w(newly_detected)}
+
+    subject { security_reports.violates_default_policy_against?(target_reports, vulnerabilities_allowed, severity_levels, vulnerability_states) }
+
+    before do
+      security_reports.get_report('sast', artifact).add_finding(high_severity_dast)
+    end
+
+    context 'when the target_reports is `nil`' do
+      let(:target_reports) { nil }
+
+      it { is_expected.to be(true) }
+
+      context 'with existing vulnerabilities' do
+        let!(:finding) { create(:vulnerabilities_finding, :detected, report_type: :sast, project: pipeline.project, uuid: high_severity_dast.uuid) }
+
+        it { is_expected.to be(true) }
+
+        context 'with vulnerability states matching existing vulnerabilities' do
+          let(:vulnerability_states) { %w(detected)}
+
+          it { is_expected.to be(true) }
+        end
+
+        context 'with vulnerability states not matching existing vulnerabilities' do
+          let(:vulnerability_states) { %w(resolved)}
+
+          it { is_expected.to be(false) }
+        end
+      end
+    end
+
+    context 'when the target_reports is not `nil`' do
+      let(:target_reports) { described_class.new(pipeline) }
+
+      it { is_expected.to be(true) }
+
+      context "when none of the reports have a new unsafe vulnerability" do
+        before do
+          target_reports.get_report('sast', artifact).add_finding(high_severity_dast)
+        end
+
+        it { is_expected.to be(false) }
+
+        context 'with existing vulnerabilities' do
+          let!(:finding) { create(:vulnerabilities_finding, :detected, report_type: :sast, project: pipeline.project, uuid: high_severity_dast.uuid) }
+
+          it { is_expected.to be(false) }
+
+          context 'with vulnerability states matching existing vulnerability' do
+            let(:vulnerability_states) { %w(detected)}
+
+            it { is_expected.to be(true) }
+          end
+
+          context 'with vulnerability states not matching existing vulnerabilities' do
+            let(:vulnerability_states) { %w(resolved)}
+
+            it { is_expected.to be(false) }
+          end
+        end
+      end
+    end
+  end
+end

ee/spec/models/approval_project_rule_spec.rb

@@ -15,6 +15,12 @@ RSpec.describe ApprovalProjectRule do
         expect(::Enums::Vulnerability.severity_levels.keys).to include(*described_class::DEFAULT_SEVERITIES)
       end
     end
+
+    context 'APPROVAL_VULNERABILITY_STATES' do
+      it 'contains all vulnerability states' do
+        expect(described_class::APPROVAL_VULNERABILITY_STATES).to include(*::Enums::Vulnerability.vulnerability_states.keys)
+      end
+    end
   end
 
   describe 'associations' do
@@ -177,20 +183,21 @@ RSpec.describe ApprovalProjectRule do
       context "with a `Vulnerability-Check` rule" do
         using RSpec::Parameterized::TableSyntax
 
-        where(:is_valid, :scanners, :vulnerabilities_allowed, :severity_levels) do
-          true  | []                                     | 0     | []
-          true  | %w(dast)                               | 1     | %w(critical high medium)
-          true  | %w(dast sast)                          | 10    | %w(critical high)
-          true  | %w(dast dast)                          | 100   | %w(critical)
-          false | %w(dast dast)                          | 100   | %w(unknown_severity)
-          false | %w(dast unknown_scanner)               | 100   | %w(critical)
-          false | [described_class::UNSUPPORTED_SCANNER] | 100   | %w(critical)
-          false | %w(dast sast)                          | 1.1   | %w(critical)
-          false | %w(dast sast)                          | 'one' | %w(critical)
+        where(:is_valid, :scanners, :vulnerabilities_allowed, :severity_levels, :vulnerability_states) do
+          true  | []                                     | 0     | []                       | %w(newly_detected)
+          true  | %w(dast)                               | 1     | %w(critical high medium) | %w(newly_detected resolved)
+          true  | %w(dast sast)                          | 10    | %w(critical high)        | %w(resolved detected)
+          true  | %w(dast dast)                          | 100   | %w(critical)             | %w(detected dismissed)
+          false | %w(dast dast)                          | 100   | %w(critical)             | %w(dismissed unknown)
+          false | %w(dast dast)                          | 100   | %w(unknown_severity)     | %w(detected dismissed)
+          false | %w(dast unknown_scanner)               | 100   | %w(critical)             | %w(detected dismissed)
+          false | [described_class::UNSUPPORTED_SCANNER] | 100   | %w(critical)             | %w(detected dismissed)
+          false | %w(dast sast)                          | 1.1   | %w(critical)             | %w(detected dismissed)
+          false | %w(dast sast)                          | 'one' | %w(critical)             | %w(detected dismissed)
         end
 
         with_them do
-          let(:vulnerability_check_rule) { build(:approval_project_rule, :vulnerability, scanners: scanners, vulnerabilities_allowed: vulnerabilities_allowed, severity_levels: severity_levels) }
+          let(:vulnerability_check_rule) { build(:approval_project_rule, :vulnerability, scanners: scanners, vulnerabilities_allowed: vulnerabilities_allowed, severity_levels: severity_levels, vulnerability_states: vulnerability_states) }
 
           specify { expect(vulnerability_check_rule.valid?).to be(is_valid) }
         end
@@ -273,5 +280,27 @@ RSpec.describe ApprovalProjectRule do
 
       it_behaves_like 'auditable'
     end
+
+    describe '#vulnerability_states_for_branch' do
+      let(:project) { create(:project, :repository) }
+      let(:branch_name) { project.default_branch }
+      let!(:rule) { build(:approval_project_rule, project: project, protected_branches: protected_branches, vulnerability_states: %w(newly_detected resolved)) }
+
+      context 'with protected branch set to any' do
+        let(:protected_branches) { [] }
+
+        it 'returns all content of vulnerability states' do
+          expect(rule.vulnerability_states_for_branch).to contain_exactly('newly_detected', 'resolved')
+        end
+      end
+
+      context 'with protected branch set to a custom branch' do
+        let(:protected_branches) { [create(:protected_branch, project: project, name: 'custom_branch')] }
+
+        it 'returns only the content of vulnerability states' do
+          expect(rule.vulnerability_states_for_branch).to contain_exactly('newly_detected')
+        end
+      end
+    end
   end
 end

ee/spec/models/ee/vulnerability_spec.rb

@@ -826,4 +826,26 @@ RSpec.describe Vulnerability do
       )
     end
   end
+
+  describe '.with_findings_by_uuid_and_state scope' do
+    let_it_be(:vulnerability) { create(:vulnerability, state: :detected) }
+
+    let(:uuid) { [SecureRandom.uuid] }
+
+    subject { described_class.with_findings_by_uuid_and_state(uuid, ["detected"]) }
+
+    it { is_expected.to be_empty }
+
+    context 'with findings' do
+      let_it_be(:finding) { create(:vulnerabilities_finding, vulnerability: vulnerability) }
+
+      it { is_expected.to be_empty }
+
+      context 'with matching uuid' do
+        let(:uuid) { [finding.uuid] }
+
+        it { is_expected.to contain_exactly(vulnerability) }
+      end
+    end
+  end
 end

ee/spec/services/ci/sync_reports_to_approval_rules_service_spec.rb

@@ -22,9 +22,10 @@ RSpec.describe Ci::SyncReportsToApprovalRulesService, '#execute' do
     let(:scanners) { %w[dependency_scanning] }
     let(:vulnerabilities_allowed) { 0 }
     let(:severity_levels) { %w[high unknown] }
+    let(:vulnerability_states) { %w(newly_detected) }
 
     before do
-      create(:approval_project_rule, :vulnerability, project: project, approvals_required: 2, scanners: scanners, vulnerabilities_allowed: vulnerabilities_allowed, severity_levels: severity_levels)
+      create(:approval_project_rule, :vulnerability, project: project, approvals_required: 2, scanners: scanners, vulnerabilities_allowed: vulnerabilities_allowed, severity_levels: severity_levels, vulnerability_states: vulnerability_states)
     end
 
     context 'when there are security reports' do
@@ -78,6 +79,15 @@ RSpec.describe Ci::SyncReportsToApprovalRulesService, '#execute' do
                 .to change { report_approver_rule.reload.approvals_required }.from(2).to(0)
             end
           end
+
+          context 'without any vulnerability state related to the security reports' do
+            let(:vulnerability_states) { %w(resolved) }
+
+            it 'lowers approvals_required count to zero' do
+              expect { subject }
+                .to change { report_approver_rule.reload.approvals_required }.from(2).to(0)
+            end
+          end
         end
 
         context 'when only low-severity vulnerabilities are present' do

lib/gitlab/ci/reports/security/reports.rb

@@ -22,21 +22,24 @@ module Gitlab
             reports.values.flat_map(&:findings)
           end
 
-          def violates_default_policy_against?(target_reports, vulnerabilities_allowed, severity_levels)
-            unsafe_findings_count(target_reports, severity_levels) > vulnerabilities_allowed
+          def violates_default_policy_against?(target_reports, vulnerabilities_allowed, severity_levels, vulnerability_states)
+            unsafe_findings_count(target_reports, severity_levels, vulnerability_states) > vulnerabilities_allowed
           end
 
-          private
-
-          def findings_diff(target_reports)
-            findings - target_reports&.findings.to_a
+          def unsafe_findings_uuids(severity_levels)
+            findings.select { |finding| finding.unsafe?(severity_levels) }.map(&:uuid)
           end
 
-          def unsafe_findings_count(target_reports, severity_levels)
-            findings_diff(target_reports).count {|finding| finding.unsafe?(severity_levels)}
+          private
+
+          def unsafe_findings_count(target_reports, severity_levels, vulnerability_states)
+            new_uuids = unsafe_findings_uuids(severity_levels) - target_reports&.unsafe_findings_uuids(severity_levels).to_a
+            new_uuids.count
           end
         end
       end
     end
   end
 end
+
+Gitlab::Ci::Reports::Security::Reports.prepend_mod_with('Gitlab::Ci::Reports::Security::Reports')

locale/gitlab.pot

@@ -4223,12 +4223,18 @@ msgstr ""
 msgid "ApprovalRule|All severity levels"
 msgstr ""
 
+msgid "ApprovalRule|All vulnerability states"
+msgstr ""
+
 msgid "ApprovalRule|Apply this approval rule to consider only the selected security scanners."
 msgstr ""
 
 msgid "ApprovalRule|Apply this approval rule to consider only the selected severity levels."
 msgstr ""
 
+msgid "ApprovalRule|Apply this approval rule to consider only the selected vulnerability states."
+msgstr ""
+
 msgid "ApprovalRule|Approval rules"
 msgstr ""
 
@@ -4241,12 +4247,21 @@ msgstr ""
 msgid "ApprovalRule|Approvers"
 msgstr ""
 
+msgid "ApprovalRule|Confirmed"
+msgstr ""
+
+msgid "ApprovalRule|Dismissed"
+msgstr ""
+
 msgid "ApprovalRule|Examples: QA, Security."
 msgstr ""
 
 msgid "ApprovalRule|Name"
 msgstr ""
 
+msgid "ApprovalRule|Newly detected"
+msgstr ""
+
 msgid "ApprovalRule|Number of vulnerabilities allowed before approval rule is triggered."
 msgstr ""
 
@@ -4259,6 +4274,15 @@ msgstr ""
 msgid "ApprovalRule|Please select at least one severity level"
 msgstr ""
 
+msgid "ApprovalRule|Please select at least one vulnerability state"
+msgstr ""
+
+msgid "ApprovalRule|Previously detected"
+msgstr ""
+
+msgid "ApprovalRule|Resolved"
+msgstr ""
+
 msgid "ApprovalRule|Rule name"
 msgstr ""
 
@@ -4274,6 +4298,9 @@ msgstr ""
 msgid "ApprovalRule|Select severity levels"
 msgstr ""
 
+msgid "ApprovalRule|Select vulnerability states"
+msgstr ""
+
 msgid "ApprovalRule|Severity levels"
 msgstr ""
 
@@ -4283,6 +4310,9 @@ msgstr ""
 msgid "ApprovalRule|Vulnerabilities allowed"
 msgstr ""
 
+msgid "ApprovalRule|Vulnerability states"
+msgstr ""
+
 msgid "ApprovalSettings|Merge request approval settings have been updated."
 msgstr ""
 

spec/lib/gitlab/ci/reports/security/reports_spec.rb

@@ -57,8 +57,9 @@ RSpec.describe Gitlab::Ci::Reports::Security::Reports do
     let(:high_severity_dast) { build(:ci_reports_security_finding, severity: 'high', report_type: :dast) }
     let(:vulnerabilities_allowed) { 0 }
     let(:severity_levels) { %w(critical high) }
+    let(:vulnerability_states) { %w(newly_detected)}
 
-    subject { security_reports.violates_default_policy_against?(target_reports, vulnerabilities_allowed, severity_levels) }
+    subject { security_reports.violates_default_policy_against?(target_reports, vulnerabilities_allowed, severity_levels, vulnerability_states) }
 
     before do
       security_reports.get_report('sast', artifact).add_finding(high_severity_dast)