From a7e7808bc35a264613d449bcee3f5084e2d9beee Mon Sep 17 00:00:00 2001 From: Imam Hossain Date: Tue, 17 Jun 2025 02:27:58 +0200 Subject: [PATCH 01/11] Audit event on security policy violations A stream only audit event is recorded when policy violations are detected in merge requests. --- .../types/policy_violations_detected.yml | 10 + config/sidekiq_queues.yml | 2 + doc/user/compliance/audit_event_types.md | 1 + .../security/scan_result_policy_violation.rb | 2 + ...violations_detected_audit_event_service.rb | 90 ++++++ .../update_violations_service.rb | 13 + ee/app/workers/all_queues.yml | 10 + ..._violations_detected_audit_event_worker.rb | 26 ++ ...lect_mr_policy_violations_audit_events.yml | 10 + .../scan_result_policy_violation_spec.rb | 9 + ...tions_detected_audit_event_service_spec.rb | 290 ++++++++++++++++++ .../update_violations_service_spec.rb | 51 +++ ...ations_detected_audit_event_worker_spec.rb | 32 ++ 13 files changed, 546 insertions(+) create mode 100644 config/audit_events/types/policy_violations_detected.yml create mode 100644 ee/app/services/merge_requests/policy_violations_detected_audit_event_service.rb create mode 100644 ee/app/workers/merge_requests/policy_violations_detected_audit_event_worker.rb create mode 100644 ee/config/feature_flags/gitlab_com_derisk/collect_mr_policy_violations_audit_events.yml create mode 100644 ee/spec/services/merge_requests/policy_violations_detected_audit_event_service_spec.rb create mode 100644 ee/spec/workers/merge_requests/policy_violations_detected_audit_event_worker_spec.rb diff --git a/config/audit_events/types/policy_violations_detected.yml b/config/audit_events/types/policy_violations_detected.yml new file mode 100644 index 00000000000000..d101592c8882c8 --- /dev/null +++ b/config/audit_events/types/policy_violations_detected.yml @@ -0,0 +1,10 @@ +--- +name: policy_violations_detected +description: Security policy violation is detected in the merge request +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/work_items/549811 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/193482 +milestone: '18.2' +feature_category: security_policy_management +saved_to_database: true +streamed: true +scope: [Project] diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml index 6d82a99197a3df..f2b363b5db2ebf 100644 --- a/config/sidekiq_queues.yml +++ b/config/sidekiq_queues.yml @@ -621,6 +621,8 @@ - 1 - - merge_requests_notify_approvers - 1 +- - merge_requests_policy_violations_detected_audit_event + - 1 - - merge_requests_process_auto_merge_from_event - 1 - - merge_requests_process_draft_note_published diff --git a/doc/user/compliance/audit_event_types.md b/doc/user/compliance/audit_event_types.md index 90ecb8469049b5..d481dd23fbb63f 100644 --- a/doc/user/compliance/audit_event_types.md +++ b/doc/user/compliance/audit_event_types.md @@ -533,6 +533,7 @@ Audit event types belong to the following product categories. | [`security_policy_create`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/192797) | A security policy is created | {{< icon name="check-circle" >}} Yes | GitLab [18.1](https://gitlab.com/gitlab-org/gitlab/-/issues/539230) | Project | | [`security_policy_delete`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/192797) | A security policy is deleted | {{< icon name="check-circle" >}} Yes | GitLab [18.1](https://gitlab.com/gitlab-org/gitlab/-/issues/539230) | Project | | [`security_policy_update`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/192797) | A security policy is updated | {{< icon name="check-circle" >}} Yes | GitLab [18.1](https://gitlab.com/gitlab-org/gitlab/-/issues/539230) | Project | +| [`policy_violations_detected`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/193482) | Security policy violation is detected in the merge request | {{< icon name="check-circle" >}} Yes | GitLab [18.2](https://gitlab.com/gitlab-org/gitlab/-/work_items/549811) | Project | ### Security testing configuration diff --git a/ee/app/models/security/scan_result_policy_violation.rb b/ee/app/models/security/scan_result_policy_violation.rb index 72bbd854ec9fb2..2e346187302957 100644 --- a/ee/app/models/security/scan_result_policy_violation.rb +++ b/ee/app/models/security/scan_result_policy_violation.rb @@ -17,6 +17,8 @@ class ScanResultPolicyViolation < ApplicationRecord validates :scan_result_policy_id, uniqueness: { scope: %i[merge_request_id] } validates :violation_data, json_schema: { filename: 'scan_result_policy_violation_data' }, allow_blank: true + scope :running, -> { where(status: :running) } + enum :status, { running: 0, failed: 1, diff --git a/ee/app/services/merge_requests/policy_violations_detected_audit_event_service.rb b/ee/app/services/merge_requests/policy_violations_detected_audit_event_service.rb new file mode 100644 index 00000000000000..7c6c682a88fe36 --- /dev/null +++ b/ee/app/services/merge_requests/policy_violations_detected_audit_event_service.rb @@ -0,0 +1,90 @@ +# frozen_string_literal: true + +module MergeRequests + class PolicyViolationsDetectedAuditEventService + include Gitlab::Utils::StrongMemoize + + def initialize(merge_request) + @merge_request = merge_request + end + + def execute + return unless violations.present? && all_policy_violations_data_populated? + + ::Gitlab::Audit::Auditor.audit(audit_context) + end + + private + + attr_reader :merge_request + + def violations + merge_request.scan_result_policy_violations + end + strong_memoize_attr :violations + + def all_policy_violations_data_populated? + violations.without_violation_data.blank? + end + + def audit_context + { + name: 'policy_violations_detected', + message: "#{violations.count} merge request approval policy violation(s) detected in merge request " \ + "with title \"#{merge_request.title}\" in \"#{merge_request.project.name}\" project", + author: merge_request.author, + scope: policy_configuration.security_policy_management_project, + target: merge_request, + additional_details: additional_details + } + end + + def additional_details + { + merge_request_title: merge_request.title, + merge_request_id: merge_request.id, + merge_request_iid: merge_request.iid, + source_branch: merge_request.source_branch, + target_branch: merge_request.target_branch, + project_id: merge_request.project.id, + project_name: merge_request.project.name, + project_full_path: merge_request.project.full_path, + violated_policies: violated_policies, + violation_details: violation_details, + security_policy_management_project_id: policy_configuration.security_policy_management_project_id + } + end + + def policy_configuration + merge_request.project.security_orchestration_policy_configuration + end + + def details + ::Security::ScanResultPolicies::PolicyViolationDetails.new(merge_request) + end + strong_memoize_attr :details + + def violation_details + { + fail_open_policies: details.fail_open_policies.as_json, + fail_closed_policies: details.fail_closed_policies.as_json, + new_scan_finding_violations: details.new_scan_finding_violations.as_json, + previous_scan_finding_violations: details.previous_scan_finding_violations.as_json, + license_scanning_violations: details.license_scanning_violations.as_json, + any_merge_request_violations: details.any_merge_request_violations.as_json, + errors: details.errors.as_json, + comparison_pipelines: details.comparison_pipelines.as_json + } + end + + def violated_policies + violations.map do |violation| + security_policy = violation.security_policy + { + policy_name: security_policy&.name, + policy_type: security_policy&.type + } + end + end + end +end diff --git a/ee/app/services/security/security_orchestration_policies/update_violations_service.rb b/ee/app/services/security/security_orchestration_policies/update_violations_service.rb index 1f7fedabe4b70c..b9b292618ccec5 100644 --- a/ee/app/services/security/security_orchestration_policies/update_violations_service.rb +++ b/ee/app/services/security/security_orchestration_policies/update_violations_service.rb @@ -60,6 +60,7 @@ def execute end publish_violations_updated_event if publish_event? + record_violations_detected_audit_event if record_violations_detected_audit_event? [violated_policies.clear, unviolated_policies.clear, skipped_policies.clear] end @@ -70,6 +71,18 @@ def publish_event? unviolated_policies.any? || violated_policies.any? || skipped_policies.any? end + def record_violations_detected_audit_event? + return false unless ::Feature.enabled?(:collect_mr_policy_violations_audit_events, merge_request.project) + + violations = merge_request.scan_result_policy_violations + + violations.any? && violations.running.blank? + end + + def record_violations_detected_audit_event + ::MergeRequests::PolicyViolationsDetectedAuditEventWorker.perform_async(merge_request.id) + end + # rubocop: disable CodeReuse/ActiveRecord, Database/AvoidUsingPluckWithoutLimit -- unviolated_policies is a Set def delete_violations Security::ScanResultPolicyViolation diff --git a/ee/app/workers/all_queues.yml b/ee/app/workers/all_queues.yml index 3131e405b8e209..b70ba752bed560 100644 --- a/ee/app/workers/all_queues.yml +++ b/ee/app/workers/all_queues.yml @@ -2603,6 +2603,16 @@ :idempotent: true :tags: [] :queue_namespace: +- :name: merge_requests_policy_violations_detected_audit_event + :worker_name: MergeRequests::PolicyViolationsDetectedAuditEventWorker + :feature_category: :security_policy_management + :has_external_dependencies: true + :urgency: :low + :resource_boundary: :unknown + :weight: 1 + :idempotent: true + :tags: [] + :queue_namespace: - :name: merge_requests_process_merge_audit_event :worker_name: MergeRequests::ProcessMergeAuditEventWorker :feature_category: :compliance_management diff --git a/ee/app/workers/merge_requests/policy_violations_detected_audit_event_worker.rb b/ee/app/workers/merge_requests/policy_violations_detected_audit_event_worker.rb new file mode 100644 index 00000000000000..8f8e966f8bbbf0 --- /dev/null +++ b/ee/app/workers/merge_requests/policy_violations_detected_audit_event_worker.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +module MergeRequests + class PolicyViolationsDetectedAuditEventWorker + include ApplicationWorker + + data_consistency :sticky + + feature_category :security_policy_management + urgency :low + idempotent! + defer_on_database_health_signal :gitlab_main + + worker_has_external_dependencies! + + def perform(merge_request_id) + merge_request = MergeRequest.find_by_id(merge_request_id) + unless merge_request + logger.info structured_payload(message: 'Merge request not found.', merge_request_id: merge_request_id) + return + end + + PolicyViolationsDetectedAuditEventService.new(merge_request).execute + end + end +end diff --git a/ee/config/feature_flags/gitlab_com_derisk/collect_mr_policy_violations_audit_events.yml b/ee/config/feature_flags/gitlab_com_derisk/collect_mr_policy_violations_audit_events.yml new file mode 100644 index 00000000000000..0e3c5be9a25a70 --- /dev/null +++ b/ee/config/feature_flags/gitlab_com_derisk/collect_mr_policy_violations_audit_events.yml @@ -0,0 +1,10 @@ +--- +name: collect_mr_policy_violations_audit_events +description: +feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/539231 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/193482 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/549868 +milestone: '18.2' +group: group::security policies +type: gitlab_com_derisk +default_enabled: false diff --git a/ee/spec/models/security/scan_result_policy_violation_spec.rb b/ee/spec/models/security/scan_result_policy_violation_spec.rb index 8232014f871dcb..98282099eb23a4 100644 --- a/ee/spec/models/security/scan_result_policy_violation_spec.rb +++ b/ee/spec/models/security/scan_result_policy_violation_spec.rb @@ -162,4 +162,13 @@ it { is_expected.to be_empty } end end + + describe '.running' do + let_it_be(:running_violation) { create(:scan_result_policy_violation, :running) } + let_it_be(:failed_violation) { create(:scan_result_policy_violation, :failed) } + + it 'returns only running violations' do + expect(described_class.running).to contain_exactly(running_violation) + end + end end diff --git a/ee/spec/services/merge_requests/policy_violations_detected_audit_event_service_spec.rb b/ee/spec/services/merge_requests/policy_violations_detected_audit_event_service_spec.rb new file mode 100644 index 00000000000000..cc6ed93c77e57b --- /dev/null +++ b/ee/spec/services/merge_requests/policy_violations_detected_audit_event_service_spec.rb @@ -0,0 +1,290 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe MergeRequests::PolicyViolationsDetectedAuditEventService, feature_category: :security_policy_management do + let_it_be(:project) { create(:project, :repository) } + let_it_be(:policy_project) { create(:project, :repository) } + let_it_be(:policy_configuration) do + create(:security_orchestration_policy_configuration, project: project, + security_policy_management_project: policy_project) + end + + let_it_be_with_reload(:merge_request) do + create(:merge_request, title: "Test MR", source_project: project, target_project: project) + end + + let_it_be_with_reload(:security_policy) do + create(:security_policy, name: 'Test Policy', + security_orchestration_policy_configuration: policy_configuration) + end + + let_it_be(:uuid) { SecureRandom.uuid } + let_it_be(:uuid_previous) { SecureRandom.uuid } + + let(:service) { described_class.new(merge_request) } + + def build_violation_details(policy, data, status = :failed) + create(:scan_result_policy_violation, status, project: project, merge_request: merge_request, + scan_result_policy_read: policy, approval_policy_rule: approval_policy_rule, violation_data: data) + end + + def build_violation_with_error(policy, error, status = :failed, **extra_data) + build_violation_details(policy, { 'errors' => [{ 'error' => error, **extra_data }] }, status) + end + + describe '#execute' do + subject(:execute_service) { service.execute } + + let(:audit_context) do + { + name: 'policy_violations_detected', + author: merge_request.author, + scope: policy_project, + target: merge_request, + message: "#{violations_count} merge request approval policy violation(s) detected in merge request " \ + "with title \"Test MR\" in \"#{project.name}\" project", + additional_details: { + merge_request_title: merge_request.title, + merge_request_id: merge_request.id, + merge_request_iid: merge_request.iid, + source_branch: merge_request.source_branch, + target_branch: merge_request.target_branch, + project_id: project.id, + project_name: project.name, + project_full_path: project.full_path, + violated_policies: [ + { + policy_name: 'Test Policy', + policy_type: 'approval_policy' + } + ], + violation_details: violation_details, + security_policy_management_project_id: policy_project.id + } + } + end + + shared_examples 'recording the audit event' do + it 'records a policy_violations_detected audit event' do + expect(::Gitlab::Audit::Auditor).to receive(:audit).with(audit_context) + + execute_service + end + end + + context 'when there are scan finding violations' do + let_it_be(:approval_policy_rule) do + create(:approval_policy_rule, :scan_finding, security_policy: security_policy) + end + + let_it_be(:policy) do + create(:scan_result_policy_read, project: project, + security_orchestration_policy_configuration: policy_configuration) + end + + let_it_be(:approver_rule_policy) do + create(:report_approver_rule, :scan_finding, + merge_request: merge_request, + scan_result_policy_read: policy + ) + end + + let_it_be(:pipeline) do + create(:ee_ci_pipeline, :with_dependency_scanning_report, :success, project: project, + ref: merge_request.source_branch, sha: merge_request.diff_head_sha, + merge_requests_as_head_pipeline: [merge_request]) + end + + let_it_be(:policy_violation) do + build_violation_details(policy, + context: { pipeline_ids: [pipeline.id] }, + violations: { scan_finding: { uuids: { newly_detected: [uuid], previously_existing: [uuid_previous] } } } + ) + end + + let_it_be(:ci_build) { pipeline.builds.first } + let_it_be(:scanner) { create(:vulnerabilities_scanner, project: project) } + + let_it_be(:new_security_finding) do + pipeline_scan = create(:security_scan, :succeeded, build: ci_build, scan_type: 'dependency_scanning') + create(:security_finding, :with_finding_data, scan: pipeline_scan, scanner: scanner, severity: 'high', + uuid: uuid, finding_data: { name: 'New Test Finding', location: { file: '.env', start_line: 3 } }) + end + + let_it_be(:existing_vulnerability_finding) do + create(:vulnerabilities_finding, :with_secret_detection, project: project, scanner: scanner, + uuid: uuid_previous, name: 'Existing AWS API key', severity: :critical) + end + + let(:expected_new_finding_json) do + { + 'name' => 'New Test Finding', + 'report_type' => 'dependency_scanning', + 'severity' => 'high', + 'location' => new_security_finding.location.as_json, + 'path' => new_security_finding.present.blob_url + } + end + + let(:expected_existing_finding_json) do + { + 'name' => 'AWS API key', + 'report_type' => 'secret_detection', + 'severity' => 'critical', + 'location' => existing_vulnerability_finding.location.as_json, + 'path' => existing_vulnerability_finding.vulnerability.present.location_link + } + end + + let(:violations_count) { 1 } + + let(:violation_details) do + { + fail_open_policies: [], + fail_closed_policies: ['Test Policy'], + new_scan_finding_violations: [expected_new_finding_json], + previous_scan_finding_violations: [expected_existing_finding_json], + license_scanning_violations: [], + any_merge_request_violations: [], + errors: [], + comparison_pipelines: [{ "report_type" => "scan_finding", "source" => [pipeline.id], "target" => [] }] + } + end + + it_behaves_like 'recording the audit event' + end + + context 'when there are any merge request violations' do + let_it_be(:approval_policy_rule) do + create(:approval_policy_rule, :any_merge_request, security_policy: security_policy) + end + + let(:any_merge_request_violation_json) do + { + 'name' => 'Test Policy', + 'commits' => true + } + end + + let(:violations_count) { 1 } + + let(:violation_details) do + { + fail_open_policies: [], + fail_closed_policies: ['Test Policy'], + new_scan_finding_violations: [], + previous_scan_finding_violations: [], + license_scanning_violations: [], + any_merge_request_violations: [any_merge_request_violation_json], + errors: [], + comparison_pipelines: [] + } + end + + before do + policy = create(:scan_result_policy_read, project: project, + security_orchestration_policy_configuration: policy_configuration) + + create(:report_approver_rule, :any_merge_request, merge_request: merge_request, + scan_result_policy_read: policy) + + build_violation_details(policy, violations: { any_merge_request: { commits: true } }) + end + + it_behaves_like 'recording the audit event' + end + + context 'when there are license scanning violations' do + let_it_be(:approval_policy_rule) do + create(:approval_policy_rule, :license_finding, security_policy: security_policy) + end + + let(:license_scanning_violations_json) do + { + "dependencies" => %w[A B], + "license" => "MIT License", + "url" => "https://spdx.org/licenses/MIT.html" + } + end + + let(:violations_count) { 1 } + + let(:violation_details) do + { + fail_open_policies: [], + fail_closed_policies: ['Test Policy'], + new_scan_finding_violations: [], + previous_scan_finding_violations: [], + license_scanning_violations: [license_scanning_violations_json], + any_merge_request_violations: [], + errors: [], + comparison_pipelines: [] + } + end + + before do + policy = create(:scan_result_policy_read, project: project, + security_orchestration_policy_configuration: policy_configuration) + + create(:report_approver_rule, :license_scanning, merge_request: merge_request, + scan_result_policy_read: policy) + + build_violation_details(policy, + violations: { license_scanning: { 'MIT License' => %w[A B] } } + ) + end + + it_behaves_like 'recording the audit event' + end + + context "when there are errors" do + let_it_be(:approval_policy_rule) do + create(:approval_policy_rule, :scan_finding, security_policy: security_policy) + end + + let_it_be(:policy) do + create(:scan_result_policy_read, project: project, + security_orchestration_policy_configuration: policy_configuration) + end + + let_it_be(:approver_rule_policy) do + create(:report_approver_rule, :scan_finding, + merge_request: merge_request, + scan_result_policy_read: policy + ) + end + + let_it_be(:errors_json) do + { "data" => { "missing_scans" => ["sast"] }, + "error" => "SCAN_REMOVED", + "message" => "There is a mismatch between the scans of the source and target pipelines. " \ + "The following scans are missing: Sast", + "report_type" => "scan_finding", + "warning" => false } + end + + let(:violations_count) { 1 } + + let(:violation_details) do + { + fail_open_policies: [], + fail_closed_policies: ['Test Policy'], + new_scan_finding_violations: [], + previous_scan_finding_violations: [], + license_scanning_violations: [], + any_merge_request_violations: [], + errors: [errors_json], + comparison_pipelines: [] + } + end + + before do + build_violation_with_error(policy, + Security::ScanResultPolicyViolation::ERRORS[:scan_removed], 'missing_scans' => %w[sast]) + end + + it_behaves_like 'recording the audit event' + end + end +end diff --git a/ee/spec/services/security/security_orchestration_policies/update_violations_service_spec.rb b/ee/spec/services/security/security_orchestration_policies/update_violations_service_spec.rb index e00600d82cd67f..98af9d50bd7e10 100644 --- a/ee/spec/services/security/security_orchestration_policies/update_violations_service_spec.rb +++ b/ee/spec/services/security/security_orchestration_policies/update_violations_service_spec.rb @@ -197,6 +197,57 @@ def last_violation expect { service.execute }.not_to publish_event(MergeRequests::ViolationsUpdatedEvent) end end + + describe 'violations detected audit event' do + shared_examples 'not recording audit event' do + it 'does not enqueue MergeRequests::PolicyViolationsDetectedAuditEventWorker' do + expect { service.execute }.not_to change { + ::MergeRequests::PolicyViolationsDetectedAuditEventWorker.jobs.size + } + end + end + + context 'when there are violations' do + before do + service.add([policy_a], []) + service.execute + end + + it 'enqueues MergeRequests::PolicyViolationsDetectedAuditEventWorker' do + expect(::MergeRequests::PolicyViolationsDetectedAuditEventWorker).to receive(:perform_async).with( + merge_request.id + ) + + service.execute + end + end + + context "when there are only running violations" do + let_it_be(:running_violation) do + create(:scan_result_policy_violation, :running, scan_result_policy_read: policy_a, + merge_request: merge_request) + end + + let_it_be(:failed_violation) do + create(:scan_result_policy_violation, :failed, scan_result_policy_read: policy_b, + merge_request: merge_request) + end + + it_behaves_like 'not recording audit event' + end + + context "when there are no violations" do + it_behaves_like 'not recording audit event' + end + + context 'when feature flag is disabled' do + before do + stub_feature_flags(collect_mr_policy_violations_audit_events: false) + end + + it_behaves_like 'not recording audit event' + end + end end describe '#add_violation' do diff --git a/ee/spec/workers/merge_requests/policy_violations_detected_audit_event_worker_spec.rb b/ee/spec/workers/merge_requests/policy_violations_detected_audit_event_worker_spec.rb new file mode 100644 index 00000000000000..99884b70612d93 --- /dev/null +++ b/ee/spec/workers/merge_requests/policy_violations_detected_audit_event_worker_spec.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe MergeRequests::PolicyViolationsDetectedAuditEventWorker, feature_category: :security_policy_management do + describe '#perform' do + let(:worker) { described_class.new } + + context 'when a merge request is not found' do + it 'logs and does not call PolicyViolationsDetectedAuditEventService' do + expect(Sidekiq.logger).to receive(:info).with( + hash_including('message' => 'Merge request not found.', 'merge_request_id' => non_existing_record_id) + ) + expect(MergeRequests::PolicyViolationsDetectedAuditEventService).not_to receive(:new).with(anything) + + worker.perform(non_existing_record_id) + end + end + + context 'when a merge request is found' do + let!(:merge_request) { create(:merge_request) } + + it 'calls PolicyViolationsDetectedAuditEventService' do + expect_next_instance_of(MergeRequests::PolicyViolationsDetectedAuditEventService, merge_request) do |service| + expect(service).to receive(:execute) + end + + worker.perform(merge_request.id) + end + end + end +end -- GitLab From 1b1f2a3eb659eede3d92abbc87b7dbf900335802 Mon Sep 17 00:00:00 2001 From: Imam Hossain Date: Tue, 17 Jun 2025 11:12:41 +0200 Subject: [PATCH 02/11] Update specs for audit event service --- ...violations_detected_audit_event_service.rb | 6 +-- ...tions_detected_audit_event_service_spec.rb | 38 ++++++++++++++++++- 2 files changed, 38 insertions(+), 6 deletions(-) diff --git a/ee/app/services/merge_requests/policy_violations_detected_audit_event_service.rb b/ee/app/services/merge_requests/policy_violations_detected_audit_event_service.rb index 7c6c682a88fe36..f4f53af1c5ba95 100644 --- a/ee/app/services/merge_requests/policy_violations_detected_audit_event_service.rb +++ b/ee/app/services/merge_requests/policy_violations_detected_audit_event_service.rb @@ -9,7 +9,7 @@ def initialize(merge_request) end def execute - return unless violations.present? && all_policy_violations_data_populated? + return if violations.blank? || violations.running.any? ::Gitlab::Audit::Auditor.audit(audit_context) end @@ -23,10 +23,6 @@ def violations end strong_memoize_attr :violations - def all_policy_violations_data_populated? - violations.without_violation_data.blank? - end - def audit_context { name: 'policy_violations_detected', diff --git a/ee/spec/services/merge_requests/policy_violations_detected_audit_event_service_spec.rb b/ee/spec/services/merge_requests/policy_violations_detected_audit_event_service_spec.rb index cc6ed93c77e57b..f3e1567eef8559 100644 --- a/ee/spec/services/merge_requests/policy_violations_detected_audit_event_service_spec.rb +++ b/ee/spec/services/merge_requests/policy_violations_detected_audit_event_service_spec.rb @@ -5,7 +5,7 @@ RSpec.describe MergeRequests::PolicyViolationsDetectedAuditEventService, feature_category: :security_policy_management do let_it_be(:project) { create(:project, :repository) } let_it_be(:policy_project) { create(:project, :repository) } - let_it_be(:policy_configuration) do + let_it_be_with_reload(:policy_configuration) do create(:security_orchestration_policy_configuration, project: project, security_policy_management_project: policy_project) end @@ -73,6 +73,14 @@ def build_violation_with_error(policy, error, status = :failed, **extra_data) end end + shared_examples 'not recording the audit event' do + it 'does not record a policy_violations_detected audit event' do + expect(::Gitlab::Audit::Auditor).not_to receive(:audit).with(anything) + + execute_service + end + end + context 'when there are scan finding violations' do let_it_be(:approval_policy_rule) do create(:approval_policy_rule, :scan_finding, security_policy: security_policy) @@ -286,5 +294,33 @@ def build_violation_with_error(policy, error, status = :failed, **extra_data) it_behaves_like 'recording the audit event' end + + context 'when there are running violations' do + let_it_be(:approval_policy_rule) do + create(:approval_policy_rule, :scan_finding, security_policy: security_policy) + end + + let_it_be(:policy) do + create(:scan_result_policy_read, project: project, + security_orchestration_policy_configuration: policy_configuration) + end + + let_it_be(:approver_rule_policy) do + create(:report_approver_rule, :scan_finding, + merge_request: merge_request, + scan_result_policy_read: policy + ) + end + + let_it_be(:running_violation) do + build_violation_details(policy, {}, :running) + end + + it_behaves_like 'not recording the audit event' + end + + context 'when there are no violations' do + it_behaves_like 'not recording the audit event' + end end end -- GitLab From ecdf48343bf51a2ea456641d5a2a1e3617b0c156 Mon Sep 17 00:00:00 2001 From: Imam Hossain Date: Tue, 17 Jun 2025 11:18:19 +0200 Subject: [PATCH 03/11] Update specs for UpdateViolatationsService --- ...violations_detected_audit_event_service_spec.rb | 5 +++-- .../update_violations_service_spec.rb | 14 +++++++------- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/ee/spec/services/merge_requests/policy_violations_detected_audit_event_service_spec.rb b/ee/spec/services/merge_requests/policy_violations_detected_audit_event_service_spec.rb index f3e1567eef8559..c5fdd2440aa2b0 100644 --- a/ee/spec/services/merge_requests/policy_violations_detected_audit_event_service_spec.rb +++ b/ee/spec/services/merge_requests/policy_violations_detected_audit_event_service_spec.rb @@ -93,6 +93,7 @@ def build_violation_with_error(policy, error, status = :failed, **extra_data) let_it_be(:approver_rule_policy) do create(:report_approver_rule, :scan_finding, + name: 'Test Policy', merge_request: merge_request, scan_result_policy_read: policy ) @@ -195,7 +196,7 @@ def build_violation_with_error(policy, error, status = :failed, **extra_data) security_orchestration_policy_configuration: policy_configuration) create(:report_approver_rule, :any_merge_request, merge_request: merge_request, - scan_result_policy_read: policy) + scan_result_policy_read: policy, name: 'Test Policy') build_violation_details(policy, violations: { any_merge_request: { commits: true } }) end @@ -236,7 +237,7 @@ def build_violation_with_error(policy, error, status = :failed, **extra_data) security_orchestration_policy_configuration: policy_configuration) create(:report_approver_rule, :license_scanning, merge_request: merge_request, - scan_result_policy_read: policy) + scan_result_policy_read: policy, name: 'Test Policy') build_violation_details(policy, violations: { license_scanning: { 'MIT License' => %w[A B] } } diff --git a/ee/spec/services/security/security_orchestration_policies/update_violations_service_spec.rb b/ee/spec/services/security/security_orchestration_policies/update_violations_service_spec.rb index 98af9d50bd7e10..368b495a565026 100644 --- a/ee/spec/services/security/security_orchestration_policies/update_violations_service_spec.rb +++ b/ee/spec/services/security/security_orchestration_policies/update_violations_service_spec.rb @@ -198,8 +198,8 @@ def last_violation end end - describe 'violations detected audit event' do - shared_examples 'not recording audit event' do + describe 'policy_violations_detected audit event' do + shared_examples 'not enqueuing the PolicyViolationsDetectedAuditEventWorker' do it 'does not enqueue MergeRequests::PolicyViolationsDetectedAuditEventWorker' do expect { service.execute }.not_to change { ::MergeRequests::PolicyViolationsDetectedAuditEventWorker.jobs.size @@ -207,7 +207,7 @@ def last_violation end end - context 'when there are violations' do + context 'when there are policy violations' do before do service.add([policy_a], []) service.execute @@ -222,7 +222,7 @@ def last_violation end end - context "when there are only running violations" do + context "when there are running violations" do let_it_be(:running_violation) do create(:scan_result_policy_violation, :running, scan_result_policy_read: policy_a, merge_request: merge_request) @@ -233,11 +233,11 @@ def last_violation merge_request: merge_request) end - it_behaves_like 'not recording audit event' + it_behaves_like 'not enqueuing the PolicyViolationsDetectedAuditEventWorker' end context "when there are no violations" do - it_behaves_like 'not recording audit event' + it_behaves_like 'not enqueuing the PolicyViolationsDetectedAuditEventWorker' end context 'when feature flag is disabled' do @@ -245,7 +245,7 @@ def last_violation stub_feature_flags(collect_mr_policy_violations_audit_events: false) end - it_behaves_like 'not recording audit event' + it_behaves_like 'not enqueuing the PolicyViolationsDetectedAuditEventWorker' end end end -- GitLab From 58dff39f84211b2e391e4151117e24f4b4b74bc5 Mon Sep 17 00:00:00 2001 From: Imam Hossain Date: Tue, 17 Jun 2025 12:32:23 +0200 Subject: [PATCH 04/11] Update feature flag --- .../update_violations_service.rb | 4 +++- ...ect_security_policy_violations_detected_audit_event.yml} | 6 +++--- .../policy_violations_detected_audit_event_service_spec.rb | 3 ++- .../update_violations_service_spec.rb | 2 +- 4 files changed, 9 insertions(+), 6 deletions(-) rename ee/config/feature_flags/gitlab_com_derisk/{collect_mr_policy_violations_audit_events.yml => collect_security_policy_violations_detected_audit_event.yml} (65%) diff --git a/ee/app/services/security/security_orchestration_policies/update_violations_service.rb b/ee/app/services/security/security_orchestration_policies/update_violations_service.rb index b9b292618ccec5..f99df856897267 100644 --- a/ee/app/services/security/security_orchestration_policies/update_violations_service.rb +++ b/ee/app/services/security/security_orchestration_policies/update_violations_service.rb @@ -72,7 +72,9 @@ def publish_event? end def record_violations_detected_audit_event? - return false unless ::Feature.enabled?(:collect_mr_policy_violations_audit_events, merge_request.project) + return false unless ::Feature.enabled?( + :collect_security_policy_violations_detected_audit_events, merge_request.project + ) violations = merge_request.scan_result_policy_violations diff --git a/ee/config/feature_flags/gitlab_com_derisk/collect_mr_policy_violations_audit_events.yml b/ee/config/feature_flags/gitlab_com_derisk/collect_security_policy_violations_detected_audit_event.yml similarity index 65% rename from ee/config/feature_flags/gitlab_com_derisk/collect_mr_policy_violations_audit_events.yml rename to ee/config/feature_flags/gitlab_com_derisk/collect_security_policy_violations_detected_audit_event.yml index 0e3c5be9a25a70..928d0f140f8a45 100644 --- a/ee/config/feature_flags/gitlab_com_derisk/collect_mr_policy_violations_audit_events.yml +++ b/ee/config/feature_flags/gitlab_com_derisk/collect_security_policy_violations_detected_audit_event.yml @@ -1,7 +1,7 @@ --- -name: collect_mr_policy_violations_audit_events -description: -feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/539231 +name: collect_security_policy_violations_detected_audit_events +description: Collects audit events for security policy violations detected in merge requests. +feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/549811 introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/193482 rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/549868 milestone: '18.2' diff --git a/ee/spec/services/merge_requests/policy_violations_detected_audit_event_service_spec.rb b/ee/spec/services/merge_requests/policy_violations_detected_audit_event_service_spec.rb index c5fdd2440aa2b0..daefc896a076d7 100644 --- a/ee/spec/services/merge_requests/policy_violations_detected_audit_event_service_spec.rb +++ b/ee/spec/services/merge_requests/policy_violations_detected_audit_event_service_spec.rb @@ -260,7 +260,8 @@ def build_violation_with_error(policy, error, status = :failed, **extra_data) let_it_be(:approver_rule_policy) do create(:report_approver_rule, :scan_finding, merge_request: merge_request, - scan_result_policy_read: policy + scan_result_policy_read: policy, + name: 'Test Policy' ) end diff --git a/ee/spec/services/security/security_orchestration_policies/update_violations_service_spec.rb b/ee/spec/services/security/security_orchestration_policies/update_violations_service_spec.rb index 368b495a565026..2b16c66457fad5 100644 --- a/ee/spec/services/security/security_orchestration_policies/update_violations_service_spec.rb +++ b/ee/spec/services/security/security_orchestration_policies/update_violations_service_spec.rb @@ -242,7 +242,7 @@ def last_violation context 'when feature flag is disabled' do before do - stub_feature_flags(collect_mr_policy_violations_audit_events: false) + stub_feature_flags(collect_security_policy_violations_detected_audit_events: false) end it_behaves_like 'not enqueuing the PolicyViolationsDetectedAuditEventWorker' -- GitLab From 9026fc6cd81e42bd4684bdecd9fb216b4c23ef06 Mon Sep 17 00:00:00 2001 From: Imam Hossain Date: Tue, 17 Jun 2025 12:42:26 +0200 Subject: [PATCH 05/11] Update audit event doc --- config/audit_events/types/policy_violations_detected.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/audit_events/types/policy_violations_detected.yml b/config/audit_events/types/policy_violations_detected.yml index d101592c8882c8..ec13bc72f403de 100644 --- a/config/audit_events/types/policy_violations_detected.yml +++ b/config/audit_events/types/policy_violations_detected.yml @@ -5,6 +5,6 @@ introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/work_items/549811 introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/193482 milestone: '18.2' feature_category: security_policy_management -saved_to_database: true +saved_to_database: false streamed: true scope: [Project] -- GitLab From e21f5d6a52e9299e7d9fca61e586a18e5f9416a7 Mon Sep 17 00:00:00 2001 From: Imam Hossain Date: Tue, 17 Jun 2025 12:47:38 +0200 Subject: [PATCH 06/11] Update audit event doc --- doc/user/compliance/audit_event_types.md | 2 +- ...violations_detected_audit_event_service.rb | 19 ++++++++++--------- ...licy_violations_detected_audit_events.yml} | 0 3 files changed, 11 insertions(+), 10 deletions(-) rename ee/config/feature_flags/gitlab_com_derisk/{collect_security_policy_violations_detected_audit_event.yml => collect_security_policy_violations_detected_audit_events.yml} (100%) diff --git a/doc/user/compliance/audit_event_types.md b/doc/user/compliance/audit_event_types.md index d481dd23fbb63f..3d072e10729f64 100644 --- a/doc/user/compliance/audit_event_types.md +++ b/doc/user/compliance/audit_event_types.md @@ -533,7 +533,7 @@ Audit event types belong to the following product categories. | [`security_policy_create`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/192797) | A security policy is created | {{< icon name="check-circle" >}} Yes | GitLab [18.1](https://gitlab.com/gitlab-org/gitlab/-/issues/539230) | Project | | [`security_policy_delete`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/192797) | A security policy is deleted | {{< icon name="check-circle" >}} Yes | GitLab [18.1](https://gitlab.com/gitlab-org/gitlab/-/issues/539230) | Project | | [`security_policy_update`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/192797) | A security policy is updated | {{< icon name="check-circle" >}} Yes | GitLab [18.1](https://gitlab.com/gitlab-org/gitlab/-/issues/539230) | Project | -| [`policy_violations_detected`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/193482) | Security policy violation is detected in the merge request | {{< icon name="check-circle" >}} Yes | GitLab [18.2](https://gitlab.com/gitlab-org/gitlab/-/work_items/549811) | Project | +| [`policy_violations_detected`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/193482) | Security policy violation is detected in the merge request | {{< icon name="dotted-circle" >}} No | GitLab [18.2](https://gitlab.com/gitlab-org/gitlab/-/work_items/549811) | Project | ### Security testing configuration diff --git a/ee/app/services/merge_requests/policy_violations_detected_audit_event_service.rb b/ee/app/services/merge_requests/policy_violations_detected_audit_event_service.rb index f4f53af1c5ba95..2cbc7a42398bc0 100644 --- a/ee/app/services/merge_requests/policy_violations_detected_audit_event_service.rb +++ b/ee/app/services/merge_requests/policy_violations_detected_audit_event_service.rb @@ -23,6 +23,16 @@ def violations end strong_memoize_attr :violations + def policy_configuration + merge_request.project.security_orchestration_policy_configuration + end + strong_memoize_attr :policy_configuration + + def details + ::Security::ScanResultPolicies::PolicyViolationDetails.new(merge_request) + end + strong_memoize_attr :details + def audit_context { name: 'policy_violations_detected', @@ -51,15 +61,6 @@ def additional_details } end - def policy_configuration - merge_request.project.security_orchestration_policy_configuration - end - - def details - ::Security::ScanResultPolicies::PolicyViolationDetails.new(merge_request) - end - strong_memoize_attr :details - def violation_details { fail_open_policies: details.fail_open_policies.as_json, diff --git a/ee/config/feature_flags/gitlab_com_derisk/collect_security_policy_violations_detected_audit_event.yml b/ee/config/feature_flags/gitlab_com_derisk/collect_security_policy_violations_detected_audit_events.yml similarity index 100% rename from ee/config/feature_flags/gitlab_com_derisk/collect_security_policy_violations_detected_audit_event.yml rename to ee/config/feature_flags/gitlab_com_derisk/collect_security_policy_violations_detected_audit_events.yml -- GitLab From 12ceb5357d304353f7c9e34cecc35cb09683ca0e Mon Sep 17 00:00:00 2001 From: Imam Hossain Date: Tue, 17 Jun 2025 13:22:13 +0200 Subject: [PATCH 07/11] Update sidekiq worker and specs --- .../policy_violations_detected_audit_event_worker.rb | 4 +++- .../policy_violations_detected_audit_event_worker_spec.rb | 8 +++++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/ee/app/workers/merge_requests/policy_violations_detected_audit_event_worker.rb b/ee/app/workers/merge_requests/policy_violations_detected_audit_event_worker.rb index 8f8e966f8bbbf0..33bc64c8e058df 100644 --- a/ee/app/workers/merge_requests/policy_violations_detected_audit_event_worker.rb +++ b/ee/app/workers/merge_requests/policy_violations_detected_audit_event_worker.rb @@ -9,8 +9,10 @@ class PolicyViolationsDetectedAuditEventWorker feature_category :security_policy_management urgency :low idempotent! - defer_on_database_health_signal :gitlab_main + deduplicate :until_executed + 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! def perform(merge_request_id) diff --git a/ee/spec/workers/merge_requests/policy_violations_detected_audit_event_worker_spec.rb b/ee/spec/workers/merge_requests/policy_violations_detected_audit_event_worker_spec.rb index 99884b70612d93..7c27967d840db9 100644 --- a/ee/spec/workers/merge_requests/policy_violations_detected_audit_event_worker_spec.rb +++ b/ee/spec/workers/merge_requests/policy_violations_detected_audit_event_worker_spec.rb @@ -6,6 +6,12 @@ describe '#perform' do let(:worker) { described_class.new } + include_examples 'an idempotent worker' do + let_it_be(:merge_request) { create(:merge_request) } + + let(:job_args) { merge_request.id } + end + context 'when a merge request is not found' do it 'logs and does not call PolicyViolationsDetectedAuditEventService' do expect(Sidekiq.logger).to receive(:info).with( @@ -18,7 +24,7 @@ end context 'when a merge request is found' do - let!(:merge_request) { create(:merge_request) } + let_it_be(:merge_request) { create(:merge_request) } it 'calls PolicyViolationsDetectedAuditEventService' do expect_next_instance_of(MergeRequests::PolicyViolationsDetectedAuditEventService, merge_request) do |service| -- GitLab From d8acc563307bf6147db66cb013217f970f33f376 Mon Sep 17 00:00:00 2001 From: Imam Hossain Date: Wed, 18 Jun 2025 12:05:07 +0200 Subject: [PATCH 08/11] Resolve MR reviews --- ...tions_detected_audit_event_service_spec.rb | 22 ++++++++++--------- .../update_violations_service_spec.rb | 6 ++--- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/ee/spec/services/merge_requests/policy_violations_detected_audit_event_service_spec.rb b/ee/spec/services/merge_requests/policy_violations_detected_audit_event_service_spec.rb index daefc896a076d7..c6d1b148507c62 100644 --- a/ee/spec/services/merge_requests/policy_violations_detected_audit_event_service_spec.rb +++ b/ee/spec/services/merge_requests/policy_violations_detected_audit_event_service_spec.rb @@ -14,8 +14,10 @@ create(:merge_request, title: "Test MR", source_project: project, target_project: project) end + let_it_be(:security_policy_name) { 'Test Policy' } let_it_be_with_reload(:security_policy) do - create(:security_policy, name: 'Test Policy', + create(:security_policy, :approval_policy, + name: security_policy_name, security_orchestration_policy_configuration: policy_configuration) end @@ -93,7 +95,7 @@ def build_violation_with_error(policy, error, status = :failed, **extra_data) let_it_be(:approver_rule_policy) do create(:report_approver_rule, :scan_finding, - name: 'Test Policy', + name: security_policy_name, merge_request: merge_request, scan_result_policy_read: policy ) @@ -151,7 +153,7 @@ def build_violation_with_error(policy, error, status = :failed, **extra_data) let(:violation_details) do { fail_open_policies: [], - fail_closed_policies: ['Test Policy'], + fail_closed_policies: [security_policy_name], new_scan_finding_violations: [expected_new_finding_json], previous_scan_finding_violations: [expected_existing_finding_json], license_scanning_violations: [], @@ -171,7 +173,7 @@ def build_violation_with_error(policy, error, status = :failed, **extra_data) let(:any_merge_request_violation_json) do { - 'name' => 'Test Policy', + 'name' => security_policy_name, 'commits' => true } end @@ -181,7 +183,7 @@ def build_violation_with_error(policy, error, status = :failed, **extra_data) let(:violation_details) do { fail_open_policies: [], - fail_closed_policies: ['Test Policy'], + fail_closed_policies: [security_policy_name], new_scan_finding_violations: [], previous_scan_finding_violations: [], license_scanning_violations: [], @@ -196,7 +198,7 @@ def build_violation_with_error(policy, error, status = :failed, **extra_data) security_orchestration_policy_configuration: policy_configuration) create(:report_approver_rule, :any_merge_request, merge_request: merge_request, - scan_result_policy_read: policy, name: 'Test Policy') + scan_result_policy_read: policy, name: security_policy_name) build_violation_details(policy, violations: { any_merge_request: { commits: true } }) end @@ -222,7 +224,7 @@ def build_violation_with_error(policy, error, status = :failed, **extra_data) let(:violation_details) do { fail_open_policies: [], - fail_closed_policies: ['Test Policy'], + fail_closed_policies: [security_policy_name], new_scan_finding_violations: [], previous_scan_finding_violations: [], license_scanning_violations: [license_scanning_violations_json], @@ -237,7 +239,7 @@ def build_violation_with_error(policy, error, status = :failed, **extra_data) security_orchestration_policy_configuration: policy_configuration) create(:report_approver_rule, :license_scanning, merge_request: merge_request, - scan_result_policy_read: policy, name: 'Test Policy') + scan_result_policy_read: policy, name: security_policy_name) build_violation_details(policy, violations: { license_scanning: { 'MIT License' => %w[A B] } } @@ -261,7 +263,7 @@ def build_violation_with_error(policy, error, status = :failed, **extra_data) create(:report_approver_rule, :scan_finding, merge_request: merge_request, scan_result_policy_read: policy, - name: 'Test Policy' + name: security_policy_name ) end @@ -279,7 +281,7 @@ def build_violation_with_error(policy, error, status = :failed, **extra_data) let(:violation_details) do { fail_open_policies: [], - fail_closed_policies: ['Test Policy'], + fail_closed_policies: [security_policy_name], new_scan_finding_violations: [], previous_scan_finding_violations: [], license_scanning_violations: [], diff --git a/ee/spec/services/security/security_orchestration_policies/update_violations_service_spec.rb b/ee/spec/services/security/security_orchestration_policies/update_violations_service_spec.rb index 2b16c66457fad5..c5e8e2adedcb03 100644 --- a/ee/spec/services/security/security_orchestration_policies/update_violations_service_spec.rb +++ b/ee/spec/services/security/security_orchestration_policies/update_violations_service_spec.rb @@ -201,9 +201,9 @@ def last_violation describe 'policy_violations_detected audit event' do shared_examples 'not enqueuing the PolicyViolationsDetectedAuditEventWorker' do it 'does not enqueue MergeRequests::PolicyViolationsDetectedAuditEventWorker' do - expect { service.execute }.not_to change { - ::MergeRequests::PolicyViolationsDetectedAuditEventWorker.jobs.size - } + expect(::MergeRequests::PolicyViolationsDetectedAuditEventWorker).not_to receive(:perform_async) + + service.execute end end -- GitLab From 4a37f52b10845c2910651754d5413b875fb69018 Mon Sep 17 00:00:00 2001 From: Imam Hossain Date: Wed, 18 Jun 2025 13:36:24 +0200 Subject: [PATCH 09/11] Use single qoute for names --- .../policy_violations_detected_audit_event_service.rb | 2 +- .../policy_violations_detected_audit_event_service_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ee/app/services/merge_requests/policy_violations_detected_audit_event_service.rb b/ee/app/services/merge_requests/policy_violations_detected_audit_event_service.rb index 2cbc7a42398bc0..5ade57ce7590b0 100644 --- a/ee/app/services/merge_requests/policy_violations_detected_audit_event_service.rb +++ b/ee/app/services/merge_requests/policy_violations_detected_audit_event_service.rb @@ -37,7 +37,7 @@ def audit_context { name: 'policy_violations_detected', message: "#{violations.count} merge request approval policy violation(s) detected in merge request " \ - "with title \"#{merge_request.title}\" in \"#{merge_request.project.name}\" project", + "with title '#{merge_request.title}' in '#{merge_request.project.name}' project", author: merge_request.author, scope: policy_configuration.security_policy_management_project, target: merge_request, diff --git a/ee/spec/services/merge_requests/policy_violations_detected_audit_event_service_spec.rb b/ee/spec/services/merge_requests/policy_violations_detected_audit_event_service_spec.rb index c6d1b148507c62..c902b960cc8eb3 100644 --- a/ee/spec/services/merge_requests/policy_violations_detected_audit_event_service_spec.rb +++ b/ee/spec/services/merge_requests/policy_violations_detected_audit_event_service_spec.rb @@ -45,7 +45,7 @@ def build_violation_with_error(policy, error, status = :failed, **extra_data) scope: policy_project, target: merge_request, message: "#{violations_count} merge request approval policy violation(s) detected in merge request " \ - "with title \"Test MR\" in \"#{project.name}\" project", + "with title 'Test MR' in '#{project.name}' project", additional_details: { merge_request_title: merge_request.title, merge_request_id: merge_request.id, -- GitLab From fddb0c8b17d466dd2c20f276d88664d699f20aa3 Mon Sep 17 00:00:00 2001 From: Imam Hossain Date: Wed, 18 Jun 2025 19:05:19 +0200 Subject: [PATCH 10/11] Add warn mode policies attribute --- .../policy_violations_detected_audit_event_service.rb | 1 + .../policy_violations_detected_audit_event_service_spec.rb | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/ee/app/services/merge_requests/policy_violations_detected_audit_event_service.rb b/ee/app/services/merge_requests/policy_violations_detected_audit_event_service.rb index 5ade57ce7590b0..6fe5ce398a2903 100644 --- a/ee/app/services/merge_requests/policy_violations_detected_audit_event_service.rb +++ b/ee/app/services/merge_requests/policy_violations_detected_audit_event_service.rb @@ -65,6 +65,7 @@ def violation_details { fail_open_policies: details.fail_open_policies.as_json, fail_closed_policies: details.fail_closed_policies.as_json, + warn_mode_policies: details.warn_mode_policies.as_json, new_scan_finding_violations: details.new_scan_finding_violations.as_json, previous_scan_finding_violations: details.previous_scan_finding_violations.as_json, license_scanning_violations: details.license_scanning_violations.as_json, diff --git a/ee/spec/services/merge_requests/policy_violations_detected_audit_event_service_spec.rb b/ee/spec/services/merge_requests/policy_violations_detected_audit_event_service_spec.rb index c902b960cc8eb3..3471ad1351eb58 100644 --- a/ee/spec/services/merge_requests/policy_violations_detected_audit_event_service_spec.rb +++ b/ee/spec/services/merge_requests/policy_violations_detected_audit_event_service_spec.rb @@ -154,6 +154,7 @@ def build_violation_with_error(policy, error, status = :failed, **extra_data) { fail_open_policies: [], fail_closed_policies: [security_policy_name], + warn_mode_policies: [], new_scan_finding_violations: [expected_new_finding_json], previous_scan_finding_violations: [expected_existing_finding_json], license_scanning_violations: [], @@ -184,6 +185,7 @@ def build_violation_with_error(policy, error, status = :failed, **extra_data) { fail_open_policies: [], fail_closed_policies: [security_policy_name], + warn_mode_policies: [], new_scan_finding_violations: [], previous_scan_finding_violations: [], license_scanning_violations: [], @@ -225,6 +227,7 @@ def build_violation_with_error(policy, error, status = :failed, **extra_data) { fail_open_policies: [], fail_closed_policies: [security_policy_name], + warn_mode_policies: [], new_scan_finding_violations: [], previous_scan_finding_violations: [], license_scanning_violations: [license_scanning_violations_json], @@ -282,6 +285,7 @@ def build_violation_with_error(policy, error, status = :failed, **extra_data) { fail_open_policies: [], fail_closed_policies: [security_policy_name], + warn_mode_policies: [], new_scan_finding_violations: [], previous_scan_finding_violations: [], license_scanning_violations: [], -- GitLab From 44fe53ac7633b3826708006e4b58afb54ccbe270 Mon Sep 17 00:00:00 2001 From: Imam Hossain Date: Wed, 18 Jun 2025 21:29:49 +0200 Subject: [PATCH 11/11] Reorder the methods position --- .../update_violations_service.rb | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/ee/app/services/security/security_orchestration_policies/update_violations_service.rb b/ee/app/services/security/security_orchestration_policies/update_violations_service.rb index f99df856897267..fc4793c4f35d86 100644 --- a/ee/app/services/security/security_orchestration_policies/update_violations_service.rb +++ b/ee/app/services/security/security_orchestration_policies/update_violations_service.rb @@ -71,20 +71,6 @@ def publish_event? unviolated_policies.any? || violated_policies.any? || skipped_policies.any? end - def record_violations_detected_audit_event? - return false unless ::Feature.enabled?( - :collect_security_policy_violations_detected_audit_events, merge_request.project - ) - - violations = merge_request.scan_result_policy_violations - - violations.any? && violations.running.blank? - end - - def record_violations_detected_audit_event - ::MergeRequests::PolicyViolationsDetectedAuditEventWorker.perform_async(merge_request.id) - end - # rubocop: disable CodeReuse/ActiveRecord, Database/AvoidUsingPluckWithoutLimit -- unviolated_policies is a Set def delete_violations Security::ScanResultPolicyViolation @@ -127,6 +113,20 @@ def publish_violations_updated_event ) ) end + + def record_violations_detected_audit_event? + return false unless ::Feature.enabled?( + :collect_security_policy_violations_detected_audit_events, merge_request.project + ) + + violations = merge_request.scan_result_policy_violations + + violations.any? && violations.running.empty? + end + + def record_violations_detected_audit_event + ::MergeRequests::PolicyViolationsDetectedAuditEventWorker.perform_async(merge_request.id) + end end end end -- GitLab