diff --git a/config/audit_events/types/merge_request_merged_with_dismissed_security_policy.yml b/config/audit_events/types/merge_request_merged_with_dismissed_security_policy.yml new file mode 100644 index 0000000000000000000000000000000000000000..b415341a64f41a23afa20538a8171cd7b2cce4f6 --- /dev/null +++ b/config/audit_events/types/merge_request_merged_with_dismissed_security_policy.yml @@ -0,0 +1,11 @@ +--- +name: merge_request_merged_with_dismissed_security_policy +description: When a merge request violated a security policy in warn-mode that was + dismissed and the MR was merged +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/569628 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/205857 +feature_category: security_policy_management +milestone: '18.5' +saved_to_database: true +streamed: true +scope: [Project] diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml index e4f015a977531898a58976dd1a5533d3ac8fde87..6f5506066a4f89d483e69010ec169cd87be8c98a 100644 --- a/config/sidekiq_queues.yml +++ b/config/sidekiq_queues.yml @@ -993,6 +993,8 @@ - 1 - - security_pipeline_execution_policies_run_schedule - 1 +- - security_policies_dismissal_preserve + - 1 - - security_policies_failed_pipelines_audit - 1 - - security_policies_group_project_transfer diff --git a/db/migrate/20250920051349_add_status_to_security_policy_dismissals.rb b/db/migrate/20250920051349_add_status_to_security_policy_dismissals.rb new file mode 100644 index 0000000000000000000000000000000000000000..3340c3006d1c917f77d45258c5cff066bd9fd7ea --- /dev/null +++ b/db/migrate/20250920051349_add_status_to_security_policy_dismissals.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class AddStatusToSecurityPolicyDismissals < Gitlab::Database::Migration[2.3] + milestone '18.5' + + def change + add_column :security_policy_dismissals, :status, :smallint, default: 0, null: false + end +end diff --git a/db/schema_migrations/20250920051349 b/db/schema_migrations/20250920051349 new file mode 100644 index 0000000000000000000000000000000000000000..7e63be464396d2de23baa2c7de39b9684f293089 --- /dev/null +++ b/db/schema_migrations/20250920051349 @@ -0,0 +1 @@ +3383bea6d5a7bca37cbdcdf463d61cdf2991d4ac4f2497447153d6a2cd83919b \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 0039b5ff853051a468d72f9a3ea1ab0e36983b12..0338e56777d057fd5c188bef20f8f0fa6ce0a3d6 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -25743,6 +25743,7 @@ CREATE TABLE security_policy_dismissals ( security_findings_uuids text[] DEFAULT '{}'::text[], dismissal_types smallint[] DEFAULT '{}'::smallint[] NOT NULL, comment text, + status smallint DEFAULT 0 NOT NULL, CONSTRAINT check_654ff06528 CHECK ((char_length(comment) <= 255)) ); diff --git a/doc/user/compliance/audit_event_types.md b/doc/user/compliance/audit_event_types.md index adb563f70b264ee878411522d9da58c20caddc25..f9619c49fa92929aac6b448c79ca479cb2fdde27 100644 --- a/doc/user/compliance/audit_event_types.md +++ b/doc/user/compliance/audit_event_types.md @@ -558,6 +558,7 @@ Audit event types belong to the following product categories. | [`security_policy_yaml_invalidated`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/196721) | The policy YAML is invalidated in security policy project | {{< icon name="check-circle" >}} Yes | GitLab [18.2](https://gitlab.com/gitlab-org/gitlab/-/work_items/550892) | Project | | [`status_check_response_update`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/189293) | A user updates external status check response for merge request | {{< icon name="dotted-circle" >}} No | GitLab [18.2](https://gitlab.com/gitlab-org/gitlab/-/issues/413535) | Project | | [`merge_request_branch_bypassed_by_security_policy`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/195942) | The merge request's approval is bypassed by the branches configured in the security policy | {{< icon name="check-circle" >}} Yes | GitLab [18.2](https://gitlab.com/gitlab-org/gitlab/-/issues/549646) | Project | +| [`merge_request_merged_with_dismissed_security_policy`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/205857) | When a merge request violated a security policy in warn-mode that was dismissed and the MR was merged | {{< icon name="check-circle" >}} Yes | GitLab [18.5](https://gitlab.com/gitlab-org/gitlab/-/issues/569628) | Project | | [`security_policy_merge_request_bypass`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/205601) | A security policy is bypassed for a merge request | {{< icon name="check-circle" >}} Yes | GitLab [18.5](https://gitlab.com/gitlab-org/gitlab/-/issues/549797) | Project | ### Security testing configuration diff --git a/ee/app/events/security/policy_dismissal_preserved_event.rb b/ee/app/events/security/policy_dismissal_preserved_event.rb new file mode 100644 index 0000000000000000000000000000000000000000..e3518d74d23573f4b70f65afa609669eb3f0e3b8 --- /dev/null +++ b/ee/app/events/security/policy_dismissal_preserved_event.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +module Security + class PolicyDismissalPreservedEvent < ::Gitlab::EventStore::Event + def schema + { + 'type' => 'object', + 'properties' => { + 'security_policy_dismissal_id' => { 'type' => 'integer' } + }, + 'required' => %w[security_policy_dismissal_id] + } + end + end +end diff --git a/ee/app/models/ee/merge_request.rb b/ee/app/models/ee/merge_request.rb index 270575254ef0af820dfa59ed6853e797873ed00c..c7e5dffe8d75bbafaf9bc61f91ddda00d6d6bbf8 100644 --- a/ee/app/models/ee/merge_request.rb +++ b/ee/app/models/ee/merge_request.rb @@ -758,6 +758,12 @@ def duo_code_review_progress_note .find_by(system_note_metadata: { action: 'duo_code_review_started' }) end + def preserve_open_policy_dismissals! + return unless merged? + + policy_dismissals.with_status(:open).find_each(&:preserve!) + end + private def assigned_or_authored_by_with_access?(user) diff --git a/ee/app/models/security/policy_dismissal.rb b/ee/app/models/security/policy_dismissal.rb index 17acd2df49e80d2ea7c4f957190e80e737751b7f..289e6aca431de1e961b347af530c58abfb9f432c 100644 --- a/ee/app/models/security/policy_dismissal.rb +++ b/ee/app/models/security/policy_dismissal.rb @@ -2,6 +2,8 @@ module Security class PolicyDismissal < ApplicationRecord + include AfterCommitQueue + self.table_name = 'security_policy_dismissals' DISMISSAL_TYPES = { policy_false_positive: 0, @@ -24,8 +26,46 @@ class PolicyDismissal < ApplicationRecord where("security_findings_uuids && ARRAY[?]::text[]", security_findings_uuids) end + state_machine :status, initial: :open do + state :open, value: 0 + state :preserved, value: 1 + + event :preserve do + transition any - [:preserved] => :preserved + end + + after_transition open: :preserved do |dismissal, _| + next dismissal.destroy! unless dismissal.applicable_for_all_violations? + + dismissal.run_after_commit do + event = Security::PolicyDismissalPreservedEvent.new(data: { security_policy_dismissal_id: dismissal.id }) + ::Gitlab::EventStore.publish(event) + end + end + end + + def applicable_for_findings?(finding_uuids) + return false if security_findings_uuids.nil? + + finding_uuids.to_set.subset?(security_findings_uuids.to_set) + end + + def applicable_for_all_violations? + applicable_for_findings?(mr_violation_finding_uuids) + end + private + def scan_result_policy_violations + merge_request.scan_result_policy_violations + .joins(:security_policy) + .where(security_policies: { id: security_policy_id }) + end + + def mr_violation_finding_uuids + scan_result_policy_violations.flat_map(&:finding_uuids).uniq + end + def dismissal_types_are_valid invalid_values = Array(dismissal_types) - DISMISSAL_TYPES.values return if dismissal_types.present? && invalid_values.blank? diff --git a/ee/app/models/security/scan_result_policy_violation.rb b/ee/app/models/security/scan_result_policy_violation.rb index d1717c08ec5df9ac00f7d249af148fe1ba5a80b8..db9fd9f959aa9b2614889d1a2679e981ba061884 100644 --- a/ee/app/models/security/scan_result_policy_violation.rb +++ b/ee/app/models/security/scan_result_policy_violation.rb @@ -87,8 +87,7 @@ def dismissed? return false unless dismissal - dismissed_uuids = dismissal.security_findings_uuids.to_set - finding_uuids.all? { |uuid| dismissed_uuids.include?(uuid) } + dismissal.applicable_for_findings?(finding_uuids) end end end diff --git a/ee/app/workers/all_queues.yml b/ee/app/workers/all_queues.yml index d90d7e2bf79003bcd312e47e1d79f514fe02ace1..478e238f94fa2ffa5dc133358a2dc94473dc4b98 100644 --- a/ee/app/workers/all_queues.yml +++ b/ee/app/workers/all_queues.yml @@ -3744,6 +3744,16 @@ :idempotent: true :tags: [] :queue_namespace: +- :name: security_policies_dismissal_preserve + :worker_name: Security::Policies::DismissalPreserveWorker + :feature_category: :security_policy_management + :has_external_dependencies: true + :urgency: :low + :resource_boundary: :unknown + :weight: 1 + :idempotent: true + :tags: [] + :queue_namespace: - :name: security_policies_failed_pipelines_audit :worker_name: Security::Policies::FailedPipelinesAuditWorker :feature_category: :security_policy_management diff --git a/ee/app/workers/security/policies/dismissal_preserve_worker.rb b/ee/app/workers/security/policies/dismissal_preserve_worker.rb new file mode 100644 index 0000000000000000000000000000000000000000..906e50bc91a46a8436e2705e62077d5a7abb840c --- /dev/null +++ b/ee/app/workers/security/policies/dismissal_preserve_worker.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +module Security + module Policies + class DismissalPreserveWorker + include Gitlab::EventStore::Subscriber + + data_consistency :sticky + idempotent! + + defer_on_database_health_signal :gitlab_main, [:project_audit_events], 1.minute + + # Audit stream to external destination with HTTP request if configured + worker_has_external_dependencies! + + feature_category :security_policy_management + + def handle_event(event) + dismissal = Security::PolicyDismissal.find_by_id(event.data[:security_policy_dismissal_id]) + + return unless dismissal + + audit_context = { + name: 'merge_request_merged_with_dismissed_security_policy', + author: dismissal.user, + scope: dismissal.project, + target: dismissal.security_policy, + message: "Merge request #{dismissal.merge_request.to_reference} was merged with violated security policy." + } + + ::Gitlab::Audit::Auditor.audit(audit_context) + end + end + end +end diff --git a/ee/app/workers/security/scan_result_policies/cleanup_merge_request_violations_worker.rb b/ee/app/workers/security/scan_result_policies/cleanup_merge_request_violations_worker.rb index fae52558cc5c969872acfae6ed1478d1d3dca027..af13a176a6943eaa196fd5565a9789d8120ec69e 100644 --- a/ee/app/workers/security/scan_result_policies/cleanup_merge_request_violations_worker.rb +++ b/ee/app/workers/security/scan_result_policies/cleanup_merge_request_violations_worker.rb @@ -20,6 +20,7 @@ def handle_event(event) if event.is_a?(::MergeRequests::MergedEvent) log_running_violations_after_merge(merge_request) audit_merged_with_policy_violations(merge_request) + preserve_security_policy_dismissals(merge_request) end merge_request.scan_result_policy_violations.delete_all(:delete_all) @@ -45,6 +46,10 @@ def log_running_violations_after_merge(merge_request) def audit_merged_with_policy_violations(merge_request) MergeRequests::MergedWithPolicyViolationsAuditEventService.new(merge_request).execute end + + def preserve_security_policy_dismissals(merge_request) + merge_request.preserve_open_policy_dismissals! + end end end end diff --git a/ee/lib/ee/gitlab/event_store.rb b/ee/lib/ee/gitlab/event_store.rb index 3a7364e63adf3bb9d4483ad47ca9db1f9b08f36b..ef5ea2127e6ccb9fc737adb3e029ff740b1e99f7 100644 --- a/ee/lib/ee/gitlab/event_store.rb +++ b/ee/lib/ee/gitlab/event_store.rb @@ -109,6 +109,8 @@ def register_security_policy_subscribers(store) store.subscribe ::Security::ScanResultPolicies::ProcessPipelineCompletionWorker, to: ::Ci::PipelineFinishedEvent + + store.subscribe ::Security::Policies::DismissalPreserveWorker, to: ::Security::PolicyDismissalPreservedEvent end def register_threat_insights_subscribers(store) diff --git a/ee/spec/lib/ee/gitlab/event_store_spec.rb b/ee/spec/lib/ee/gitlab/event_store_spec.rb index 0514d08633a699044092b13ede539720d05b5306..a63256b92e1d0f14f627ede3e2c078753e204442 100644 --- a/ee/spec/lib/ee/gitlab/event_store_spec.rb +++ b/ee/spec/lib/ee/gitlab/event_store_spec.rb @@ -87,6 +87,7 @@ Security::PolicyUpdatedEvent, Security::PolicyDeletedEvent, Security::PolicyResyncEvent, + Security::PolicyDismissalPreservedEvent, ::Members::MembershipModifiedByAdminEvent, Repositories::ProtectedBranchCreatedEvent, Repositories::ProtectedBranchDestroyedEvent, diff --git a/ee/spec/models/merge_request_spec.rb b/ee/spec/models/merge_request_spec.rb index 4162cb5fc8a0adebaffca3896001fff86e1c51d8..6566e3c58f06b0aa0f2946ee65ffabbf32b8f74b 100644 --- a/ee/spec/models/merge_request_spec.rb +++ b/ee/spec/models/merge_request_spec.rb @@ -3818,6 +3818,41 @@ def stub_foss_conditions_met end end + describe '#preserve_open_policy_dismissals!' do + let_it_be(:project) { create(:project) } + let_it_be(:merge_request) { create(:merge_request, :merged, source_project: project, target_project: project) } + + subject(:preserve_open_policy_dismissals) { merge_request.preserve_open_policy_dismissals! } + + context 'when merge request is merged' do + context 'when there are open policy dismissals' do + let_it_be(:open_dismissal_1) { create(:policy_dismissal, merge_request: merge_request, project: project) } + let_it_be(:open_dismissal_2) { create(:policy_dismissal, merge_request: merge_request, project: project) } + let_it_be(:preserved_dismissal) { create(:policy_dismissal, merge_request: merge_request, project: project, status: 1) } + + it 'preserves all open policy dismissals' do + expect { preserve_open_policy_dismissals } + .to change { open_dismissal_1.reload.status }.from(0).to(1) + .and change { open_dismissal_2.reload.status }.from(0).to(1) + .and not_change { preserved_dismissal.reload.status } + end + end + end + + context 'when merge request is not merged' do + let_it_be(:open_dismissal) { create(:policy_dismissal, merge_request: merge_request, project: project) } + + before do + allow(merge_request).to receive(:merged?).and_return(false) + end + + it 'returns early without processing dismissals' do + expect { preserve_open_policy_dismissals } + .not_to change { open_dismissal.reload.status } + end + end + end + describe '#all_target_branch_pipelines' do let_it_be(:project) { create(:project, :public, :repository) } let_it_be_with_refind(:merge_request) do diff --git a/ee/spec/models/security/policy_dismissal_spec.rb b/ee/spec/models/security/policy_dismissal_spec.rb index 857766030031798ee22c5cea9684671df544c099..20c8b76e3b407c476b5053f3b3e4d6bee3a0ce69 100644 --- a/ee/spec/models/security/policy_dismissal_spec.rb +++ b/ee/spec/models/security/policy_dismissal_spec.rb @@ -132,4 +132,260 @@ end end end + + describe '#applicable_for_all_violations?' do + let_it_be(:project) { create(:project) } + let_it_be(:merge_request) { create(:merge_request, target_project: project, source_project: project) } + let_it_be(:policy_configuration) { create(:security_orchestration_policy_configuration, project: project) } + let_it_be(:security_policy) do + create(:security_policy, security_orchestration_policy_configuration: policy_configuration) + end + + let_it_be(:approval_policy_rule) { create(:approval_policy_rule, security_policy: security_policy) } + + context 'when dismissal covers all violation finding UUIDs' do + let_it_be(:violation_1) do + create(:scan_result_policy_violation, :new_scan_finding, + project: project, + merge_request: merge_request, + approval_policy_rule: approval_policy_rule, + uuids: %w[uuid-1 uuid-2]) + end + + let_it_be(:violation_2) do + create(:scan_result_policy_violation, :previous_scan_finding, + project: project, + merge_request: merge_request, + approval_policy_rule: approval_policy_rule, + uuids: ['uuid-3']) + end + + let_it_be(:policy_dismissal) do + create(:policy_dismissal, + project: project, + merge_request: merge_request, + security_policy: security_policy, + security_findings_uuids: %w[uuid-1 uuid-2 uuid-3 uuid-4]) + end + + it 'returns true when all violation finding UUIDs are covered' do + expect(policy_dismissal.applicable_for_all_violations?).to be true + end + end + + context 'when dismissal does not cover all violation finding UUIDs' do + let_it_be(:violation_1) do + create(:scan_result_policy_violation, :new_scan_finding, + project: project, + merge_request: merge_request, + approval_policy_rule: approval_policy_rule, + uuids: %w[uuid-1 uuid-2]) + end + + let_it_be(:violation_2) do + create(:scan_result_policy_violation, :previous_scan_finding, + project: project, + merge_request: merge_request, + approval_policy_rule: approval_policy_rule, + uuids: ['uuid-3']) + end + + let_it_be(:policy_dismissal) do + create(:policy_dismissal, + project: project, + merge_request: merge_request, + security_policy: security_policy, + security_findings_uuids: %w[uuid-1 uuid-2]) + end + + it 'returns false when some violation finding UUIDs are missing' do + expect(policy_dismissal.applicable_for_all_violations?).to be false + end + end + + context 'when there are no violations' do + let_it_be(:policy_dismissal) do + create(:policy_dismissal, + project: project, + merge_request: merge_request, + security_policy: security_policy, + security_findings_uuids: ['uuid-1']) + end + + it 'returns true when there are no violations to check' do + expect(policy_dismissal.applicable_for_all_violations?).to be true + end + end + + context 'when there are violations for other security policies' do + let_it_be(:other_security_policy) do + create(:security_policy, security_orchestration_policy_configuration: policy_configuration, policy_index: 1) + end + + let_it_be(:other_approval_policy_rule) { create(:approval_policy_rule, security_policy: other_security_policy) } + + let_it_be(:matching_violation) do + create(:scan_result_policy_violation, :new_scan_finding, + project: project, + merge_request: merge_request, + approval_policy_rule: approval_policy_rule, + uuids: ['uuid-1']) + end + + let_it_be(:different_policy_violation) do + create(:scan_result_policy_violation, :new_scan_finding, + project: project, + merge_request: merge_request, + approval_policy_rule: other_approval_policy_rule, + uuids: ['uuid-2']) + end + + let_it_be(:policy_dismissal) do + create(:policy_dismissal, + project: project, + merge_request: merge_request, + security_policy: security_policy, + security_findings_uuids: ['uuid-1']) + end + + it 'only considers violations for the same security policy' do + expect(policy_dismissal.applicable_for_all_violations?).to be true + end + end + + context 'when there are violations for other merge requests' do + let_it_be(:different_mr) do + create(:merge_request, target_project: project, source_project: project, source_branch: 'different-branch') + end + + let_it_be(:matching_violation) do + create(:scan_result_policy_violation, :new_scan_finding, + project: project, + merge_request: merge_request, + approval_policy_rule: approval_policy_rule, + uuids: ['uuid-1']) + end + + let_it_be(:different_mr_violation) do + create(:scan_result_policy_violation, :new_scan_finding, + project: project, + merge_request: different_mr, + approval_policy_rule: approval_policy_rule, + uuids: ['uuid-2']) + end + + let_it_be(:policy_dismissal) do + create(:policy_dismissal, + project: project, + merge_request: merge_request, + security_policy: security_policy, + security_findings_uuids: ['uuid-1']) + end + + it 'only considers violations for the same merge request' do + expect(policy_dismissal.applicable_for_all_violations?).to be true + end + end + + context 'when dismissal has empty security findings UUIDs' do + let_it_be(:violation) do + create(:scan_result_policy_violation, :new_scan_finding, + project: project, + merge_request: merge_request, + approval_policy_rule: approval_policy_rule, + uuids: ['uuid-1']) + end + + let_it_be(:empty_dismissal) do + create(:policy_dismissal, + project: project, + merge_request: merge_request, + security_policy: security_policy, + security_findings_uuids: []) + end + + it 'returns false when security_findings_uuids is empty and there are violations' do + expect(empty_dismissal.applicable_for_all_violations?).to be false + end + end + end + + describe '#applicable_for_findings?' do + let_it_be(:policy_dismissal) do + create(:policy_dismissal, security_findings_uuids: %w[uuid-1 uuid-2 uuid-3]) + end + + it 'returns true for subset of UUIDs' do + expect(policy_dismissal.applicable_for_findings?(%w[uuid-1 uuid-2])).to be true + end + + it 'returns true for exact match of UUIDs' do + expect(policy_dismissal.applicable_for_findings?(%w[uuid-1 uuid-2 uuid-3])).to be true + end + + it 'returns true for empty array' do + expect(policy_dismissal.applicable_for_findings?([])).to be true + end + + it 'returns false when some UUIDs are missing' do + expect(policy_dismissal.applicable_for_findings?(%w[uuid-1 uuid-4])).to be false + end + + context 'when dismissal has no security findings UUIDs' do + let_it_be(:empty_dismissal) { create(:policy_dismissal, security_findings_uuids: []) } + + it 'returns false for any provided UUIDs' do + expect(empty_dismissal.applicable_for_findings?(['uuid-1'])).to be false + end + + it 'returns true for empty array' do + expect(empty_dismissal.applicable_for_findings?([])).to be true + end + end + end + + describe '#preserve!' do + let_it_be(:project) { create(:project) } + let_it_be(:merge_request) { create(:merge_request, target_project: project, source_project: project) } + let(:policy_dismissal) { create(:policy_dismissal, project: project, merge_request: merge_request) } + + subject(:preserve) { policy_dismissal.preserve! } + + before do + allow(Gitlab::EventStore).to receive(:publish) + end + + context 'when dismissal is applicable for all violations' do + before do + allow(policy_dismissal).to receive(:applicable_for_all_violations?).and_return(true) + end + + it 'changes status to preserved and publishes event', :aggregate_failures do + expect { preserve }.to change { policy_dismissal.reload.status }.from(0).to(1) + + expect(Gitlab::EventStore).to have_received(:publish) do |event| + expect(event).to be_a(Security::PolicyDismissalPreservedEvent) + expect(event.data[:security_policy_dismissal_id]).to eq(policy_dismissal.id) + end + end + end + + context 'when dismissal is not applicable for all violations' do + before do + allow(policy_dismissal).to receive(:applicable_for_all_violations?).and_return(false) + end + + it 'destroys the dismissal instead of preserving it' do + policy_dismissal.id + + expect { preserve }.to change { described_class.count }.by(-1) + end + + it 'does not publish the preserved event when destroyed' do + preserve + + expect(Gitlab::EventStore).not_to have_received(:publish) + end + end + end end diff --git a/ee/spec/workers/security/policies/dismissal_preserve_worker_spec.rb b/ee/spec/workers/security/policies/dismissal_preserve_worker_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..3a3bbca39d137dce0ad9be72758da0a1b31cbc6f --- /dev/null +++ b/ee/spec/workers/security/policies/dismissal_preserve_worker_spec.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Security::Policies::DismissalPreserveWorker, feature_category: :security_policy_management do + let_it_be(:project) { create(:project) } + let_it_be(:merge_request) { create(:merge_request, source_project: project) } + let_it_be(:user) { create(:user) } + let_it_be(:security_policy) { create(:security_policy) } + let_it_be(:policy_dismissal) do + create(:policy_dismissal, + project: project, + merge_request: merge_request, + user: user, + security_policy: security_policy + ) + end + + let(:event) do + Security::PolicyDismissalPreservedEvent.new( + data: { security_policy_dismissal_id: policy_dismissal.id } + ) + end + + subject(:worker) { described_class.new } + + describe '#handle_event' do + context 'when policy dismissal exists' do + it 'creates an audit event with correct context' do + expected_audit_context = { + name: 'merge_request_merged_with_dismissed_security_policy', + author: user, + scope: project, + target: policy_dismissal.security_policy, + message: "Merge request !#{merge_request.iid} was merged with violated security policy." + } + + expect(::Gitlab::Audit::Auditor).to receive(:audit).with(expected_audit_context) + + worker.handle_event(event) + end + end + + context 'when policy dismissal does not exist' do + let(:event) do + Security::PolicyDismissalPreservedEvent.new( + data: { security_policy_dismissal_id: non_existing_record_id } + ) + end + + it 'does not create an audit event' do + expect(::Gitlab::Audit::Auditor).not_to receive(:audit) + + worker.handle_event(event) + end + end + end +end diff --git a/ee/spec/workers/security/scan_result_policies/cleanup_merge_request_violations_worker_spec.rb b/ee/spec/workers/security/scan_result_policies/cleanup_merge_request_violations_worker_spec.rb index 6a975d1153dc9885d6e77895e9639de408e31e95..e5e9e184bce2ec0d4bb9f0734d5812b2f594f432 100644 --- a/ee/spec/workers/security/scan_result_policies/cleanup_merge_request_violations_worker_spec.rb +++ b/ee/spec/workers/security/scan_result_policies/cleanup_merge_request_violations_worker_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe Security::ScanResultPolicies::CleanupMergeRequestViolationsWorker, '#perform', feature_category: :security_policy_management do - let_it_be(:merge_request) { create(:merge_request) } + let_it_be(:merge_request) { create(:merge_request, :merged) } let_it_be_with_reload(:merge_request_violation) do create(:scan_result_policy_violation, :running, merge_request: merge_request) @@ -101,6 +101,54 @@ perform end + it 'preserves security policy dismissals' do + policy_dismissal = create(:policy_dismissal, merge_request: merge_request) + + expect { perform }.to change { policy_dismissal.reload.status }.from(0).to(1) + end + + context 'when there are multiple policy dismissals with different statuses' do + let_it_be(:open_dismissal_1) { create(:policy_dismissal, merge_request: merge_request) } + let_it_be(:open_dismissal_2) { create(:policy_dismissal, merge_request: merge_request) } + let_it_be(:preserved_dismissal) { create(:policy_dismissal, merge_request: merge_request, status: 1) } + + it 'only preserves open policy dismissals' do + expect { perform }.to change { open_dismissal_1.reload.status }.from(0).to(1) + .and change { open_dismissal_2.reload.status }.from(0).to(1) + .and not_change { preserved_dismissal.reload.status } + end + end + + context 'when policy dismissal is no longer applicable' do + let_it_be(:project) { merge_request.project } + let_it_be(:policy_configuration) { create(:security_orchestration_policy_configuration, project: project) } + let_it_be(:security_policy) do + create(:security_policy, security_orchestration_policy_configuration: policy_configuration) + end + + let_it_be(:approval_policy_rule) { create(:approval_policy_rule, security_policy: security_policy) } + let_it_be(:violation) do + create(:scan_result_policy_violation, :new_scan_finding, + project: project, + merge_request: merge_request, + approval_policy_rule: approval_policy_rule, + uuids: %w[uuid-1 uuid-2]) + end + + let_it_be(:non_applicable_dismissal) do + create(:policy_dismissal, + project: project, + merge_request: merge_request, + security_policy: security_policy, + security_findings_uuids: ['uuid-1']) # Missing uuid-2 + end + + it 'destroys non-applicable policy dismissals during preservation' do + expect { perform }.to change { Security::PolicyDismissal.exists?(non_applicable_dismissal.id) } + .from(true).to(false) + end + end + context 'when there is no merge_request scan result policy violations' do before do merge_request_violation.update!(merge_request_id: unrelated_merge_request_violation.merge_request_id)