From 0c393436c2bdca79b59eb9bed59204b0efadd139 Mon Sep 17 00:00:00 2001
From: Brian Williams <bwilliams@gitlab.com>
Date: Tue, 4 Mar 2025 14:11:35 -0600
Subject: [PATCH] Don't auto-resolve dismissed vulnerabilities
If the vulnerability is dismissed, the auto-resolve policy will
transition it to resolved. This is not intended as dismissed
vulnerabilities should stay dismissed. This change updates the
auto-resolve service to exclude dismissed vulnerabilities.
Changelog: fixed
EE: true
---
ee/app/models/vulnerabilities/read.rb | 1 -
.../vulnerabilities/auto_resolve_service.rb | 12 ++++++++++--
ee/spec/models/vulnerabilities/read_spec.rb | 13 -------------
.../vulnerabilities/auto_resolve_service_spec.rb | 4 +++-
4 files changed, 13 insertions(+), 17 deletions(-)
diff --git a/ee/app/models/vulnerabilities/read.rb b/ee/app/models/vulnerabilities/read.rb
index 59be3639f2f74..3f17fc223a42e 100644
--- a/ee/app/models/vulnerabilities/read.rb
+++ b/ee/app/models/vulnerabilities/read.rb
@@ -75,7 +75,6 @@ class << self
scope :traversal_ids_gteq, ->(traversal_ids) { where(arel_table[:traversal_ids].gteq(traversal_ids)) }
scope :traversal_ids_lt, ->(traversal_ids) { where(arel_table[:traversal_ids].lt(traversal_ids)) }
scope :unarchived, -> { where(archived: false) }
- scope :unresolved, -> { with_states(::Enums::Vulnerability.vulnerability_states.except(:resolved).values) }
scope :order_traversal_ids_asc, -> do
reorder(Gitlab::Pagination::Keyset::Order.build([
Gitlab::Pagination::Keyset::ColumnOrderDefinition.new(
diff --git a/ee/app/services/vulnerabilities/auto_resolve_service.rb b/ee/app/services/vulnerabilities/auto_resolve_service.rb
index f782e98f888ee..6fece9ca2fe22 100644
--- a/ee/app/services/vulnerabilities/auto_resolve_service.rb
+++ b/ee/app/services/vulnerabilities/auto_resolve_service.rb
@@ -7,7 +7,7 @@ class AutoResolveService
def initialize(project, vulnerability_ids, budget)
@project = project
- @vulnerability_reads = Vulnerabilities::Read.by_vulnerabilities(vulnerability_ids).unresolved
+ @vulnerability_ids = vulnerability_ids
@budget = budget
end
@@ -25,7 +25,15 @@ def execute
private
- attr_reader :project, :vulnerability_reads, :budget
+ attr_reader :project, :vulnerability_ids, :budget
+
+ def vulnerability_reads
+ Vulnerabilities::Read.by_vulnerabilities(vulnerability_ids).with_states(auto_resolve_states)
+ end
+
+ def auto_resolve_states
+ ::Enums::Vulnerability.vulnerability_states.except(:resolved, :dismissed).values
+ end
def vulnerabilities_to_resolve
rules_by_vulnerability.keys.first(budget)
diff --git a/ee/spec/models/vulnerabilities/read_spec.rb b/ee/spec/models/vulnerabilities/read_spec.rb
index e777da9ca1b54..91a78e4b6ff2d 100644
--- a/ee/spec/models/vulnerabilities/read_spec.rb
+++ b/ee/spec/models/vulnerabilities/read_spec.rb
@@ -841,19 +841,6 @@
it { is_expected.to contain_exactly(unarchived_vulnerability_read) }
end
- describe '.unresolved' do
- let_it_be(:resolved_vulnerability) { create(:vulnerability_read, state: :resolved, project: project) }
- let_it_be(:unresolved_vulnerabilities) do
- ::Enums::Vulnerability.vulnerability_states.except(:resolved).keys.map do |state|
- create(:vulnerability_read, state: state, project: project)
- end
- end
-
- subject(:unresolved) { described_class.unresolved }
-
- it { is_expected.to match_array(unresolved_vulnerabilities) }
- end
-
describe '.order_traversal_ids_asc' do
let_it_be(:group_1) { create(:group) }
let_it_be(:group_3) { create(:group) }
diff --git a/ee/spec/services/vulnerabilities/auto_resolve_service_spec.rb b/ee/spec/services/vulnerabilities/auto_resolve_service_spec.rb
index 201bb4e57dbd5..244c7f8890026 100644
--- a/ee/spec/services/vulnerabilities/auto_resolve_service_spec.rb
+++ b/ee/spec/services/vulnerabilities/auto_resolve_service_spec.rb
@@ -8,6 +8,7 @@
let_it_be_with_reload(:project) { create(:project, namespace: namespace) }
let_it_be(:vulnerability) { create(:vulnerability, :with_findings, :detected, :high_severity, project: project) }
let_it_be(:resolved_vulnerability) { create(:vulnerability, :with_findings, :resolved, project: project) }
+ let_it_be(:dismissed_vulnerability) { create(:vulnerability, :with_findings, :dismissed, project: project) }
let_it_be(:policy) { create(:security_policy, :vulnerability_management_policy, linked_projects: [project]) }
let_it_be(:policy_rule) do
create(:vulnerability_management_policy_rule,
@@ -41,7 +42,7 @@
end
describe '#execute' do
- it 'resolves unresolved vulnerabilities', :freeze_time do
+ it 'resolves vulnerabilities that are not resolved or dismissed', :freeze_time do
service.execute
vulnerability.reload
@@ -54,6 +55,7 @@
# This causes the timestamp to be rounded down to the nearest microsecond when the record is reloaded.
# We need to make the comparison in microseconds to avoid a false-negative.
expect { resolved_vulnerability.reload }.not_to change { resolved_vulnerability.updated_at.floor(6) }
+ expect { dismissed_vulnerability.reload }.not_to change { dismissed_vulnerability.updated_at.floor(6) }
end
it 'inserts a state transition for each vulnerability' do
--
GitLab