From 4a4b169c0e8caf9e93819a6d9b795aec236555cf Mon Sep 17 00:00:00 2001 From: Imam Hossain Date: Fri, 27 Jun 2025 20:20:12 +0200 Subject: [PATCH 1/7] Add audit event yml --- config/audit_events/types/policies_limit_exceeded.yml | 10 ++++++++++ doc/user/compliance/audit_event_types.md | 1 + 2 files changed, 11 insertions(+) create mode 100644 config/audit_events/types/policies_limit_exceeded.yml diff --git a/config/audit_events/types/policies_limit_exceeded.yml b/config/audit_events/types/policies_limit_exceeded.yml new file mode 100644 index 00000000000000..a54917b2d70968 --- /dev/null +++ b/config/audit_events/types/policies_limit_exceeded.yml @@ -0,0 +1,10 @@ +--- +name: policies_limit_exceeded +description: Enabled policies count exceeded the maximum allowed limit for policy type +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/work_items/550891 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/196005 +feature_category: security_policy_management +milestone: '18.2' +saved_to_database: true +streamed: true +scope: [Project] diff --git a/doc/user/compliance/audit_event_types.md b/doc/user/compliance/audit_event_types.md index 78db3be8cffe18..f3198a315a2e8e 100644 --- a/doc/user/compliance/audit_event_types.md +++ b/doc/user/compliance/audit_event_types.md @@ -540,6 +540,7 @@ Audit event types belong to the following product categories. | [`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 | | [`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_policy_violations`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/195775) | A merge request merged with security policy violations | {{< icon name="check-circle" >}} Yes | GitLab [18.2](https://gitlab.com/gitlab-org/gitlab/-/work_items/549813) | Project | +| [`policies_limit_exceeded`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/196005) | Enabled policies count exceeded the maximum allowed limit for policy type | {{< icon name="check-circle" >}} Yes | GitLab [18.2](https://gitlab.com/gitlab-org/gitlab/-/work_items/550891) | 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 | | [`policy_violations_resolved`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/193482) | Security policy violations are resolved in the merge request | {{< icon name="dotted-circle" >}} No | GitLab [18.2](https://gitlab.com/gitlab-org/gitlab/-/issues/549812) | Project | | [`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 | -- GitLab From cf2101c95012ecf0f486503ba27edb022f10d784 Mon Sep 17 00:00:00 2001 From: Imam Hossain Date: Fri, 27 Jun 2025 21:13:27 +0200 Subject: [PATCH 2/7] Add sidekiq queues yml files --- config/sidekiq_queues.yml | 2 ++ ee/app/workers/all_queues.yml | 10 ++++++++++ 2 files changed, 12 insertions(+) diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml index 981416224f204a..90ecc1cf996ee4 100644 --- a/config/sidekiq_queues.yml +++ b/config/sidekiq_queues.yml @@ -931,6 +931,8 @@ - 1 - - security_analyzers_status_setting_changed_update - 1 +- - security_collect_policies_limit_audit_events + - 1 - - security_configuration_set_group_secret_push_protection - 1 - - security_create_security_policy_project diff --git a/ee/app/workers/all_queues.yml b/ee/app/workers/all_queues.yml index fd039a95e7b887..fc6f275f11a2ee 100644 --- a/ee/app/workers/all_queues.yml +++ b/ee/app/workers/all_queues.yml @@ -3444,6 +3444,16 @@ :idempotent: true :tags: [] :queue_namespace: +- :name: security_collect_policies_limit_audit_events + :worker_name: Security::CollectPoliciesLimitAuditEventsWorker + :feature_category: :security_policy_management + :has_external_dependencies: false + :urgency: :low + :resource_boundary: :unknown + :weight: 1 + :idempotent: true + :tags: [] + :queue_namespace: - :name: security_configuration_set_group_secret_push_protection :worker_name: Security::Configuration::SetGroupSecretPushProtectionWorker :feature_category: :security_testing_configuration -- GitLab From 2661c395c472e518d3be338e5e8f95c6c4937931 Mon Sep 17 00:00:00 2001 From: Imam Hossain Date: Tue, 1 Jul 2025 18:19:53 +0200 Subject: [PATCH 3/7] Add audit events for exceeded security policies limits Add audit events when security policies limits are exceeded. --- .../ci_component_publishing_policy.rb | 1 + .../security/pipeline_execution_policy.rb | 6 +- .../pipeline_execution_schedule_policy.rb | 1 + .../security/scan_execution_policy.rb | 1 + .../concerns/security/scan_result_policy.rb | 1 + .../vulnerability_management_policy.rb | 1 + .../orchestration_policy_configuration.rb | 46 +++- ...ect_policies_limit_audit_events_service.rb | 76 +++++++ ...lect_policies_limit_audit_events_worker.rb | 18 ++ .../persist_security_policies_worker.rb | 4 + .../collect_policies_limit_audit_events.yml | 10 + ...orchestration_policy_configuration_spec.rb | 85 ++++++- ...olicies_limit_audit_events_service_spec.rb | 215 ++++++++++++++++++ ...policies_limit_audit_events_worker_spec.rb | 38 ++++ .../persist_security_policies_worker_spec.rb | 23 ++ 15 files changed, 522 insertions(+), 4 deletions(-) create mode 100644 ee/app/services/security/security_orchestration_policies/collect_policies_limit_audit_events_service.rb create mode 100644 ee/app/workers/security/collect_policies_limit_audit_events_worker.rb create mode 100644 ee/config/feature_flags/gitlab_com_derisk/collect_policies_limit_audit_events.yml create mode 100644 ee/spec/services/security/security_orchestration_policies/collect_policies_limit_audit_events_service_spec.rb create mode 100644 ee/spec/workers/security/collect_policies_limit_audit_events_worker_spec.rb diff --git a/ee/app/models/concerns/security/ci_component_publishing_policy.rb b/ee/app/models/concerns/security/ci_component_publishing_policy.rb index e287c980ca38e2..1b17e38c61dee9 100644 --- a/ee/app/models/concerns/security/ci_component_publishing_policy.rb +++ b/ee/app/models/concerns/security/ci_component_publishing_policy.rb @@ -3,6 +3,7 @@ module Security module CiComponentPublishingPolicy POLICY_LIMIT = 5 + POLICY_TYPE_NAME = 'CI component publishing policy' def active_ci_component_publishing_policies ci_component_publishing_policy.select { |config| config[:enabled] }.first(POLICY_LIMIT) diff --git a/ee/app/models/concerns/security/pipeline_execution_policy.rb b/ee/app/models/concerns/security/pipeline_execution_policy.rb index ca411c2ee8fdea..1622c75a2e6755 100644 --- a/ee/app/models/concerns/security/pipeline_execution_policy.rb +++ b/ee/app/models/concerns/security/pipeline_execution_policy.rb @@ -2,8 +2,10 @@ module Security module PipelineExecutionPolicy + POLICY_TYPE_NAME = 'Pipeline execution policy' + def active_pipeline_execution_policies - pipeline_execution_policy.select { |config| config[:enabled] }.first(policy_limit) + pipeline_execution_policy.select { |config| config[:enabled] }.first(pipeline_execution_policy_limit) end def pipeline_execution_policy @@ -12,7 +14,7 @@ def pipeline_execution_policy private - def policy_limit + def pipeline_execution_policy_limit Security::SecurityOrchestrationPolicies::LimitService .new(container: source) .pipeline_execution_policies_per_configuration_limit diff --git a/ee/app/models/concerns/security/pipeline_execution_schedule_policy.rb b/ee/app/models/concerns/security/pipeline_execution_schedule_policy.rb index 5711fc01f91776..df530d77654999 100644 --- a/ee/app/models/concerns/security/pipeline_execution_schedule_policy.rb +++ b/ee/app/models/concerns/security/pipeline_execution_schedule_policy.rb @@ -3,6 +3,7 @@ module Security module PipelineExecutionSchedulePolicy POLICY_LIMIT = 1 + POLICY_TYPE_NAME = 'Pipeline execution schedule policy' def active_pipeline_execution_schedule_policies pipeline_execution_schedule_policy.select { |config| config[:enabled] }.first(POLICY_LIMIT) diff --git a/ee/app/models/concerns/security/scan_execution_policy.rb b/ee/app/models/concerns/security/scan_execution_policy.rb index c9b51b8661e524..7a1a0d2829f54d 100644 --- a/ee/app/models/concerns/security/scan_execution_policy.rb +++ b/ee/app/models/concerns/security/scan_execution_policy.rb @@ -6,6 +6,7 @@ module ScanExecutionPolicy include ::Gitlab::Utils::StrongMemoize POLICY_LIMIT = 5 + POLICY_TYPE_NAME = 'Scan execution policy' RULE_TYPES = { pipeline: 'pipeline', diff --git a/ee/app/models/concerns/security/scan_result_policy.rb b/ee/app/models/concerns/security/scan_result_policy.rb index 34424a12350b57..4481c84520c66e 100644 --- a/ee/app/models/concerns/security/scan_result_policy.rb +++ b/ee/app/models/concerns/security/scan_result_policy.rb @@ -13,6 +13,7 @@ module ScanResultPolicy APPROVAL_RULES_BATCH_SIZE = 5000 + POLICY_TYPE_NAME = 'Merge request approval policy' SCAN_FINDING = 'scan_finding' LICENSE_SCANNING = 'license_scanning' LICENSE_FINDING = 'license_finding' diff --git a/ee/app/models/concerns/security/vulnerability_management_policy.rb b/ee/app/models/concerns/security/vulnerability_management_policy.rb index 0e23907a03be94..ffac2ddaa48756 100644 --- a/ee/app/models/concerns/security/vulnerability_management_policy.rb +++ b/ee/app/models/concerns/security/vulnerability_management_policy.rb @@ -3,6 +3,7 @@ module Security module VulnerabilityManagementPolicy POLICY_LIMIT = 5 + POLICY_TYPE_NAME = 'Vulnerability management policy' def active_vulnerability_management_policies vulnerability_management_policy.select { |config| config[:enabled] }.first(POLICY_LIMIT) diff --git a/ee/app/models/security/orchestration_policy_configuration.rb b/ee/app/models/security/orchestration_policy_configuration.rb index 84b2091a73135e..a01330aa0ba1e6 100644 --- a/ee/app/models/security/orchestration_policy_configuration.rb +++ b/ee/app/models/security/orchestration_policy_configuration.rb @@ -24,7 +24,7 @@ class OrchestrationPolicyConfiguration < ApplicationRecord # json_schemer computes an $id fallback property for schemas lacking one. # But this schema is kept anonymous on purpose, so the $id is stripped. POLICY_SCHEMA_JSON = POLICY_SCHEMA.value.except('$id') - AVAILABLE_POLICY_TYPES = %i[approval_policy scan_execution_policy pipeline_execution_policy pipeline_execution_schedule_policy vulnerability_management_policy].freeze + AVAILABLE_POLICY_TYPES = %i[approval_policy scan_execution_policy pipeline_execution_policy pipeline_execution_schedule_policy vulnerability_management_policy ci_component_publishing_policy].freeze JSON_SCHEMA_VALIDATION_TIMEOUT = 5.seconds ALL_PROJECT_IDS_BATCH_SIZE = 1000 @@ -126,6 +126,44 @@ def latest_commit_before_configured_at end strong_memoize_attr :latest_commit_before_configured_at + def policy_limit_by_type(policy_type) + validate_policy_type(policy_type) + + case policy_type.to_sym + when :approval_policy + approval_policies_limit + when :pipeline_execution_policy + pipeline_execution_policy_limit + when :scan_execution_policy + ScanExecutionPolicy::POLICY_LIMIT + when :pipeline_execution_schedule_policy + PipelineExecutionSchedulePolicy::POLICY_LIMIT + when :vulnerability_management_policy + VulnerabilityManagementPolicy::POLICY_LIMIT + when :ci_component_publishing_policy + CiComponentPublishingPolicy::POLICY_LIMIT + end + end + + def policy_type_name_by_type(policy_type) + validate_policy_type(policy_type) + + case policy_type.to_sym + when :approval_policy + ScanResultPolicy::POLICY_TYPE_NAME + when :scan_execution_policy + ScanExecutionPolicy::POLICY_TYPE_NAME + when :pipeline_execution_policy + PipelineExecutionPolicy::POLICY_TYPE_NAME + when :pipeline_execution_schedule_policy + PipelineExecutionSchedulePolicy::POLICY_TYPE_NAME + when :vulnerability_management_policy + VulnerabilityManagementPolicy::POLICY_TYPE_NAME + when :ci_component_publishing_policy + CiComponentPublishingPolicy::POLICY_TYPE_NAME + end + end + def policy_by_type(type_or_types) return [] if policy_hash.blank? @@ -259,6 +297,12 @@ def first_configuration_for_the_management_project? private + def validate_policy_type(policy_type) + return if AVAILABLE_POLICY_TYPES.include?(policy_type.to_sym) + + raise ArgumentError, "Invalid policy type: #{policy_type}" + end + def available_policy_types policies_to_exclude = experiment_enabled?(:pipeline_execution_schedule_policy) ? [] : [:pipeline_execution_schedule_policy] diff --git a/ee/app/services/security/security_orchestration_policies/collect_policies_limit_audit_events_service.rb b/ee/app/services/security/security_orchestration_policies/collect_policies_limit_audit_events_service.rb new file mode 100644 index 00000000000000..d62025b0e26bc3 --- /dev/null +++ b/ee/app/services/security/security_orchestration_policies/collect_policies_limit_audit_events_service.rb @@ -0,0 +1,76 @@ +# frozen_string_literal: true + +module Security + module SecurityOrchestrationPolicies + class CollectPoliciesLimitAuditEventsService + include Gitlab::Utils::StrongMemoize + + def initialize(policy_configuration) + @policy_configuration = policy_configuration + end + + def execute + OrchestrationPolicyConfiguration::AVAILABLE_POLICY_TYPES.each do |policy_type| + policy_limit = policy_configuration.policy_limit_by_type(policy_type) + policies = policy_configuration.policy_by_type(policy_type) + enabled_policies = filter_enabled_policies(policies) + + next if enabled_policies.count <= policy_limit + + ::Gitlab::Audit::Auditor.audit(audit_context(policy_type, policy_limit, enabled_policies)) + end + + ServiceResponse.success(message: 'Collected policies limit audit events') + end + + private + + attr_reader :policy_configuration + + def filter_enabled_policies(policies) + policies.select { |policy| policy[:enabled] } + end + + def policy_type_name(policy_type) + policy_configuration.policy_type_name_by_type(policy_type) + end + + def audit_context(policy_type, policy_limit, policies) + policy_names = policies.pluck(:name) # rubocop:disable CodeReuse/ActiveRecord, Database/AvoidUsingPluckWithoutLimit -- pluck used on hash + { + name: 'policies_limit_exceeded', + author: commit&.author || Gitlab::Audit::DeletedAuthor.new(id: -3, name: 'Unknown User'), + scope: policy_management_project, + target: policy_management_project, + message: audit_message(policy_type_name(policy_type), policy_limit), + additional_details: { + policy_type: policy_type, + policy_type_limit: policy_limit, + policies_count: policies.count, + skipped_policies_count: policies.count - policy_limit, + active_policy_names: policy_names.first(policy_limit), + skipped_policy_names: policy_names.drop(policy_limit), + security_policy_project_commit_sha: commit&.sha, + security_policy_management_project_id: policy_management_project.id, + security_orchestration_policy_configuration_id: policy_configuration.id, + security_policy_configured_at: policy_configuration.configured_at + } + } + end + + def commit + policy_configuration.latest_commit_before_configured_at + end + + def policy_management_project + policy_configuration.security_policy_management_project + end + strong_memoize_attr :policy_management_project + + def audit_message(type_name, policy_limit) + "Policies limit exceeded for '#{type_name}' type. " \ + "Only the first #{policy_limit} enabled policies will be applied" + end + end + end +end diff --git a/ee/app/workers/security/collect_policies_limit_audit_events_worker.rb b/ee/app/workers/security/collect_policies_limit_audit_events_worker.rb new file mode 100644 index 00000000000000..acde928da22ca1 --- /dev/null +++ b/ee/app/workers/security/collect_policies_limit_audit_events_worker.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +module Security + class CollectPoliciesLimitAuditEventsWorker + include ApplicationWorker + + data_consistency :sticky + idempotent! + deduplicate :until_executed + feature_category :security_policy_management + + def perform(configuration_id) + configuration = Security::OrchestrationPolicyConfiguration.find_by_id(configuration_id) || return + + Security::SecurityOrchestrationPolicies::CollectPoliciesLimitAuditEventsService.new(configuration).execute + end + end +end diff --git a/ee/app/workers/security/persist_security_policies_worker.rb b/ee/app/workers/security/persist_security_policies_worker.rb index b54c5466c9b039..4d58d4c452571c 100644 --- a/ee/app/workers/security/persist_security_policies_worker.rb +++ b/ee/app/workers/security/persist_security_policies_worker.rb @@ -32,6 +32,10 @@ def perform(configuration_id, params = {}) Security::SecurityOrchestrationPolicies::SyncScanResultPoliciesService.new(configuration).execute track_csp_usage(configuration) + + return unless Feature.enabled?(:collect_policies_limit_audit_events, configuration.project) + + Security::CollectPoliciesLimitAuditEventsWorker.perform_async(configuration.id) end private diff --git a/ee/config/feature_flags/gitlab_com_derisk/collect_policies_limit_audit_events.yml b/ee/config/feature_flags/gitlab_com_derisk/collect_policies_limit_audit_events.yml new file mode 100644 index 00000000000000..07df706b1506ef --- /dev/null +++ b/ee/config/feature_flags/gitlab_com_derisk/collect_policies_limit_audit_events.yml @@ -0,0 +1,10 @@ +--- +name: collect_policies_limit_audit_events +description: +feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/work_items/550891 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/196005 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/553047 +milestone: '18.2' +group: group::security policies +type: gitlab_com_derisk +default_enabled: false diff --git a/ee/spec/models/security/orchestration_policy_configuration_spec.rb b/ee/spec/models/security/orchestration_policy_configuration_spec.rb index bb0ac22ea2861c..ce50afd53e7e68 100644 --- a/ee/spec/models/security/orchestration_policy_configuration_spec.rb +++ b/ee/spec/models/security/orchestration_policy_configuration_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' RSpec.describe Security::OrchestrationPolicyConfiguration, feature_category: :security_policy_management do + using RSpec::Parameterized::TableSyntax + let_it_be(:security_policy_management_project) { create(:project, :repository) } let(:security_orchestration_policy_configuration) do @@ -2757,7 +2759,7 @@ before do allow(Security::SecurityOrchestrationPolicies::PolicyScopeChecker).to receive(:new).with(project: project).and_return(policy_scope_checker) - allow(policy_configuration).to receive(:approval_policies_limit).and_return(3) + allow(policy_configuration).to receive(:approval_policy_limit).and_return(3) end context 'when there are no policies' do @@ -4368,4 +4370,85 @@ def resolve_refs(schema, root_schema = nil) %w[properties vulnerability_management_policy items properties rules items] end end + + describe '#policy_limit_by_type' do + subject { security_orchestration_policy_configuration.policy_limit_by_type(policy_type) } + + context 'with a valid policy type' do + context 'for :approval_policy' do + let(:policy_type) { :approval_policy } + + it 'returns the limit from settings' do + stub_application_setting(security_approval_policies_limit: 15) + + expect(subject).to eq(15) + end + end + + context 'for :pipeline_execution_policy' do + let(:policy_type) { :pipeline_execution_policy } + let(:limit_service) { instance_double(Security::SecurityOrchestrationPolicies::LimitService) } + + before do + allow(Security::SecurityOrchestrationPolicies::LimitService) + .to receive(:new) + .with(container: security_orchestration_policy_configuration.source) + .and_return(limit_service) + allow(limit_service).to receive(:pipeline_execution_policies_per_configuration_limit).and_return(10) + end + + it 'returns the policy limit from LimitService' do + expect(subject).to eq(10) + end + end + + where(:policy_type, :expected_policy_limit) do + :scan_execution_policy | lazy { Security::ScanExecutionPolicy::POLICY_LIMIT } + :pipeline_execution_schedule_policy | lazy { Security::PipelineExecutionSchedulePolicy::POLICY_LIMIT } + :vulnerability_management_policy | lazy { Security::VulnerabilityManagementPolicy::POLICY_LIMIT } + :ci_component_publishing_policy | lazy { Security::CiComponentPublishingPolicy::POLICY_LIMIT } + end + + with_them do + it 'returns the policy limit' do + expect(subject).to eq(expected_policy_limit) + end + end + end + + context 'with an invalid policy type' do + let(:policy_type) { :invalid_policy } + + it 'raises an ArgumentError' do + expect { subject }.to raise_error(ArgumentError, 'Invalid policy type: invalid_policy') + end + end + end + + describe '#policy_type_name_by_type' do + subject { security_orchestration_policy_configuration.policy_type_name_by_type(policy_type) } + + where(:policy_type, :expected_policy_type_name) do + :approval_policy | 'Merge request approval policy' + :scan_execution_policy | 'Scan execution policy' + :pipeline_execution_policy | 'Pipeline execution policy' + :pipeline_execution_schedule_policy | 'Pipeline execution schedule policy' + :vulnerability_management_policy | 'Vulnerability management policy' + :ci_component_publishing_policy | 'CI component publishing policy' + end + + with_them do + it 'returns the policy type name' do + expect(subject).to eq(expected_policy_type_name) + end + end + + context 'with an invalid policy type' do + let(:policy_type) { :invalid_policy } + + it 'raises an ArgumentError' do + expect { subject }.to raise_error(ArgumentError, 'Invalid policy type: invalid_policy') + end + end + end end diff --git a/ee/spec/services/security/security_orchestration_policies/collect_policies_limit_audit_events_service_spec.rb b/ee/spec/services/security/security_orchestration_policies/collect_policies_limit_audit_events_service_spec.rb new file mode 100644 index 00000000000000..0b97b5f3cf6239 --- /dev/null +++ b/ee/spec/services/security/security_orchestration_policies/collect_policies_limit_audit_events_service_spec.rb @@ -0,0 +1,215 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Security::SecurityOrchestrationPolicies::CollectPoliciesLimitAuditEventsService, feature_category: :security_policy_management do + let_it_be(:user) { create(:user) } + let_it_be(:policy_management_project) { create(:project, :repository) } + let_it_be_with_reload(:policy_configuration) do + create(:security_orchestration_policy_configuration, security_policy_management_project: policy_management_project) + end + + let(:approval_policy_limit) { 5 } + let(:scan_execution_policy_limit) { 5 } + let(:latest_commit) { build(:commit) } + + subject(:service) { described_class.new(policy_configuration) } + + def mock_other_policy_type_limits(policy_types) + other_policy_types = Security::OrchestrationPolicyConfiguration::AVAILABLE_POLICY_TYPES - policy_types + other_policy_types.each do |type| + allow(policy_configuration).to receive(:policy_limit_by_type).with(type).and_return(0) + end + end + + describe '#execute' do + subject(:execute_service) { service.execute } + + before do + policy_configuration.clear_memoization(:policy_blob) + + allow_next_instance_of(Repository) do |repository| + allow(repository).to receive_messages(blob_data_at: policy_yaml) + end + + allow(policy_configuration).to receive(:latest_commit_before_configured_at).and_return(latest_commit) + end + + context 'when policy limits are not exceeded' do + before do + allow(policy_configuration).to receive(:policy_limit_by_type) + .with(:approval_policy).and_return(approval_policy_limit) + + allow(policy_configuration).to receive(:policy_limit_by_type) + .with(:scan_execution_policy).and_return(scan_execution_policy_limit) + + mock_other_policy_type_limits([:approval_policy, :scan_execution_policy]) + end + + let(:approval_policies) { build_list(:approval_policy, approval_policy_limit) } + let(:scan_execution_policies) { build_list(:scan_execution_policy, scan_execution_policy_limit) } + + let(:policy_yaml) do + build(:orchestration_policy_yaml, approval_policy: approval_policies, + scan_execution_policy: scan_execution_policies) + end + + it 'does not create any audit events' do + expect(::Gitlab::Audit::Auditor).not_to receive(:audit) + + execute_service + end + + it 'returns a success response' do + result = execute_service + + expect(result).to be_success + expect(result.message).to eq('Collected policies limit audit events') + end + end + + context 'when policy limits are exceeded' do + let(:audit_context) do + { + name: 'policies_limit_exceeded', + message: "Policies limit exceeded for '#{type_name}' type. " \ + "Only the first #{mock_policy_limit} enabled policies will be applied", + scope: policy_management_project, + target: policy_management_project, + author: latest_commit.author, + additional_details: { + policy_type: policy_type, + policy_type_limit: mock_policy_limit, + policies_count: policies.count, + skipped_policies_count: policies.count - mock_policy_limit, + active_policy_names: policies.first(mock_policy_limit).pluck(:name), + skipped_policy_names: policies.drop(mock_policy_limit).pluck(:name), + security_policy_project_commit_sha: latest_commit.sha, + security_policy_management_project_id: policy_management_project.id, + security_orchestration_policy_configuration_id: policy_configuration.id, + security_policy_configured_at: policy_configuration.configured_at + } + } + end + + shared_examples 'creating an audit event for the exceeded policy type' do + it 'creates an audit event for the exceeded policy type' do + expect(::Gitlab::Audit::Auditor).to receive(:audit).once.with(audit_context) + + execute_service + end + end + + where(:policy_type) do + Security::OrchestrationPolicyConfiguration::AVAILABLE_POLICY_TYPES + end + + with_them do + let(:mock_policy_limit) { rand(2..6) } + let(:type_name) { policy_configuration.policy_type_name_by_type(policy_type) } + let(:policies) { build_list(policy_type, mock_policy_limit + 1) } + let(:policy_yaml) do + build(:orchestration_policy_yaml, policy_type => policies) + end + + before do + allow(policy_configuration).to receive(:policy_limit_by_type) + .with(policy_type).and_return(mock_policy_limit) + + mock_other_policy_type_limits([policy_type]) + end + + it_behaves_like 'creating an audit event for the exceeded policy type' + end + + context 'for multiple policy types' do + let(:mock_policy_limit) { 2 } + let(:scan_execution_policies) { build_list(:scan_execution_policy, mock_policy_limit + 1) } + let(:approval_policies) { build_list(:approval_policy, mock_policy_limit + 1) } + let(:policies) { build_list(policy_type, mock_policy_limit + 1) } + + let(:policy_yaml) do + build(:orchestration_policy_yaml, scan_execution_policy: scan_execution_policies, + approval_policy: approval_policies) + end + + before do + allow(policy_configuration).to receive(:policy_limit_by_type) + .with(:scan_execution_policy).and_return(mock_policy_limit) + allow(policy_configuration).to receive(:policy_limit_by_type) + .with(:approval_policy).and_return(mock_policy_limit) + + mock_other_policy_type_limits([:scan_execution_policy, :approval_policy]) + end + + it 'creates an audit event for each exceeded policy type' do + expect(::Gitlab::Audit::Auditor).to receive(:audit).twice + + execute_service + end + end + + context 'with disabled policies' do + let(:mock_policy_limit) { 2 } + let(:enabled_approval_policies) { build_list(:approval_policy, mock_policy_limit) } + let(:disabled_approval_policies) { build_list(:approval_policy, 2, enabled: false) } + let(:approval_policies) { enabled_approval_policies + disabled_approval_policies } + let(:policy_yaml) do + build(:orchestration_policy_yaml, approval_policy: approval_policies) + end + + before do + allow(policy_configuration).to receive(:policy_limit_by_type) + .with(:approval_policy).and_return(mock_policy_limit) + + mock_other_policy_type_limits([:approval_policy]) + end + + it 'does not create an audit event if enabled policies are within the limit' do + expect(::Gitlab::Audit::Auditor).not_to receive(:audit) + + execute_service + end + + context 'when enabled policies exceed the limit' do + let(:enabled_approval_policies) { build_list(:approval_policy, mock_policy_limit + 1) } + + it 'creates an audit event' do + expect(::Gitlab::Audit::Auditor).to receive(:audit).once + + execute_service + end + end + end + end + + context 'when commit author is not available' do + let(:mock_policy_limit) { 2 } + let(:policy_type) { :approval_policy } + let(:policies) { build_list(:approval_policy, mock_policy_limit + 1) } + + let(:policy_yaml) do + build(:orchestration_policy_yaml, approval_policy: policies) + end + + before do + allow(policy_configuration).to receive(:policy_limit_by_type) + .with(:approval_policy).and_return(mock_policy_limit) + mock_other_policy_type_limits([:approval_policy]) + + allow(policy_configuration).to receive(:latest_commit_before_configured_at).and_return(nil) + end + + it 'audits with a deleted author' do + expect(::Gitlab::Audit::Auditor).to receive(:audit).with( + a_hash_including( + author: an_instance_of(Gitlab::Audit::DeletedAuthor), + additional_details: a_hash_including(security_policy_project_commit_sha: nil) + ) + ) + + execute_service + end + end + end +end diff --git a/ee/spec/workers/security/collect_policies_limit_audit_events_worker_spec.rb b/ee/spec/workers/security/collect_policies_limit_audit_events_worker_spec.rb new file mode 100644 index 00000000000000..e9089e61575a83 --- /dev/null +++ b/ee/spec/workers/security/collect_policies_limit_audit_events_worker_spec.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Security::CollectPoliciesLimitAuditEventsWorker, feature_category: :security_policy_management do + describe '#perform' do + let_it_be(:configuration) { create(:security_orchestration_policy_configuration) } + let(:configuration_id) { configuration.id } + + subject(:perform) { described_class.new.perform(configuration_id) } + + it_behaves_like 'an idempotent worker' do + let(:job_args) { [configuration_id] } + end + + it 'calls CollectPoliciesLimitAuditEventsService with the correct configuration' do + service = instance_double(Security::SecurityOrchestrationPolicies::CollectPoliciesLimitAuditEventsService) + + expect(Security::SecurityOrchestrationPolicies::CollectPoliciesLimitAuditEventsService) + .to receive(:new).with(configuration).and_return(service) + expect(service).to receive(:execute) + + perform + end + + context 'when configuration is not found' do + let(:configuration_id) { non_existing_record_id } + + it { expect { perform }.not_to raise_error } + + it 'does not call CollectPoliciesLimitAuditEventsService' do + expect(Security::SecurityOrchestrationPolicies::CollectPoliciesLimitAuditEventsService).not_to receive(:new) + + perform + end + end + end +end diff --git a/ee/spec/workers/security/persist_security_policies_worker_spec.rb b/ee/spec/workers/security/persist_security_policies_worker_spec.rb index 7d8a2616be1fdb..486c3a20019d19 100644 --- a/ee/spec/workers/security/persist_security_policies_worker_spec.rb +++ b/ee/spec/workers/security/persist_security_policies_worker_spec.rb @@ -138,6 +138,29 @@ perform end + + it 'calls CollectPoliciesLimitAuditEventsWorker' do + expect(Security::CollectPoliciesLimitAuditEventsWorker).to receive(:perform_async) + .with(policy_configuration.id).exactly(IdempotentWorkerHelper::WORKER_EXEC_TIMES) + + perform + end + + context 'if the collect_policies_limit_audit_events feature is disabled' do + before do + stub_feature_flags(collect_policies_limit_audit_events: false) + end + + it 'persists policies' do + expect { perform }.to change { policy_configuration.security_policies.count }.from(0).to(14) + end + + it 'does not call CollectPoliciesLimitAuditEventsWorker' do + expect(Security::CollectPoliciesLimitAuditEventsWorker).not_to receive(:perform_async) + + perform + end + end end describe 'CSP usage tracking' do -- GitLab From e56f4e78423e853e6e07ec664c869f2a7b5ddf4f Mon Sep 17 00:00:00 2001 From: Imam Hossain Date: Tue, 1 Jul 2025 19:05:12 +0200 Subject: [PATCH 4/7] Update project for feature check --- ee/app/workers/security/persist_security_policies_worker.rb | 4 +++- .../security/orchestration_policy_configuration_spec.rb | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/ee/app/workers/security/persist_security_policies_worker.rb b/ee/app/workers/security/persist_security_policies_worker.rb index 4d58d4c452571c..35dfc21e444a7b 100644 --- a/ee/app/workers/security/persist_security_policies_worker.rb +++ b/ee/app/workers/security/persist_security_policies_worker.rb @@ -33,7 +33,9 @@ def perform(configuration_id, params = {}) track_csp_usage(configuration) - return unless Feature.enabled?(:collect_policies_limit_audit_events, configuration.project) + return unless Feature.enabled?(:collect_policies_limit_audit_events, + configuration.security_policy_management_project + ) Security::CollectPoliciesLimitAuditEventsWorker.perform_async(configuration.id) end diff --git a/ee/spec/models/security/orchestration_policy_configuration_spec.rb b/ee/spec/models/security/orchestration_policy_configuration_spec.rb index ce50afd53e7e68..2a810b07cd3f4f 100644 --- a/ee/spec/models/security/orchestration_policy_configuration_spec.rb +++ b/ee/spec/models/security/orchestration_policy_configuration_spec.rb @@ -2759,7 +2759,7 @@ before do allow(Security::SecurityOrchestrationPolicies::PolicyScopeChecker).to receive(:new).with(project: project).and_return(policy_scope_checker) - allow(policy_configuration).to receive(:approval_policy_limit).and_return(3) + allow(policy_configuration).to receive(:approval_policies_limit).and_return(3) end context 'when there are no policies' do -- GitLab From 24fc32209301e670833a41737f071768e5f289be Mon Sep 17 00:00:00 2001 From: Imam Hossain Date: Thu, 3 Jul 2025 19:37:20 +0200 Subject: [PATCH 5/7] Update unknown user ID' --- .../collect_policies_limit_audit_events_service.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/app/services/security/security_orchestration_policies/collect_policies_limit_audit_events_service.rb b/ee/app/services/security/security_orchestration_policies/collect_policies_limit_audit_events_service.rb index d62025b0e26bc3..6f0a7b923b14ad 100644 --- a/ee/app/services/security/security_orchestration_policies/collect_policies_limit_audit_events_service.rb +++ b/ee/app/services/security/security_orchestration_policies/collect_policies_limit_audit_events_service.rb @@ -39,7 +39,7 @@ def audit_context(policy_type, policy_limit, policies) policy_names = policies.pluck(:name) # rubocop:disable CodeReuse/ActiveRecord, Database/AvoidUsingPluckWithoutLimit -- pluck used on hash { name: 'policies_limit_exceeded', - author: commit&.author || Gitlab::Audit::DeletedAuthor.new(id: -3, name: 'Unknown User'), + author: commit&.author || Gitlab::Audit::DeletedAuthor.new(id: -4, name: 'Unknown User'), scope: policy_management_project, target: policy_management_project, message: audit_message(policy_type_name(policy_type), policy_limit), -- GitLab From 69cec65c9e23f3efbb0187bff9ae057674ee88e6 Mon Sep 17 00:00:00 2001 From: Imam Hossain Date: Fri, 4 Jul 2025 15:58:06 +0200 Subject: [PATCH 6/7] Resolve MR reviews --- .../collect_policies_limit_audit_events_service.rb | 6 +++--- .../collect_policies_limit_audit_events_service_spec.rb | 6 +++--- .../security/persist_security_policies_worker_spec.rb | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/ee/app/services/security/security_orchestration_policies/collect_policies_limit_audit_events_service.rb b/ee/app/services/security/security_orchestration_policies/collect_policies_limit_audit_events_service.rb index 6f0a7b923b14ad..0f551abd914dc0 100644 --- a/ee/app/services/security/security_orchestration_policies/collect_policies_limit_audit_events_service.rb +++ b/ee/app/services/security/security_orchestration_policies/collect_policies_limit_audit_events_service.rb @@ -47,9 +47,9 @@ def audit_context(policy_type, policy_limit, policies) policy_type: policy_type, policy_type_limit: policy_limit, policies_count: policies.count, - skipped_policies_count: policies.count - policy_limit, - active_policy_names: policy_names.first(policy_limit), - skipped_policy_names: policy_names.drop(policy_limit), + active_skipped_policies_count: policies.count - policy_limit, + active_policies_names: policy_names.first(policy_limit), + active_skipped_policies_names: policy_names.drop(policy_limit), security_policy_project_commit_sha: commit&.sha, security_policy_management_project_id: policy_management_project.id, security_orchestration_policy_configuration_id: policy_configuration.id, diff --git a/ee/spec/services/security/security_orchestration_policies/collect_policies_limit_audit_events_service_spec.rb b/ee/spec/services/security/security_orchestration_policies/collect_policies_limit_audit_events_service_spec.rb index 0b97b5f3cf6239..447da87cd39d8a 100644 --- a/ee/spec/services/security/security_orchestration_policies/collect_policies_limit_audit_events_service_spec.rb +++ b/ee/spec/services/security/security_orchestration_policies/collect_policies_limit_audit_events_service_spec.rb @@ -81,9 +81,9 @@ def mock_other_policy_type_limits(policy_types) policy_type: policy_type, policy_type_limit: mock_policy_limit, policies_count: policies.count, - skipped_policies_count: policies.count - mock_policy_limit, - active_policy_names: policies.first(mock_policy_limit).pluck(:name), - skipped_policy_names: policies.drop(mock_policy_limit).pluck(:name), + active_skipped_policies_count: policies.count - mock_policy_limit, + active_policies_names: policies.first(mock_policy_limit).pluck(:name), + active_skipped_policies_names: policies.drop(mock_policy_limit).pluck(:name), security_policy_project_commit_sha: latest_commit.sha, security_policy_management_project_id: policy_management_project.id, security_orchestration_policy_configuration_id: policy_configuration.id, diff --git a/ee/spec/workers/security/persist_security_policies_worker_spec.rb b/ee/spec/workers/security/persist_security_policies_worker_spec.rb index 486c3a20019d19..eb2392d0a1164e 100644 --- a/ee/spec/workers/security/persist_security_policies_worker_spec.rb +++ b/ee/spec/workers/security/persist_security_policies_worker_spec.rb @@ -152,7 +152,7 @@ end it 'persists policies' do - expect { perform }.to change { policy_configuration.security_policies.count }.from(0).to(14) + expect { perform }.to change { policy_configuration.security_policies.count } end it 'does not call CollectPoliciesLimitAuditEventsWorker' do -- GitLab From 42d40bdc118735b0c2129bd40062fbfad2da7402 Mon Sep 17 00:00:00 2001 From: Imam Hossain Date: Wed, 9 Jul 2025 17:56:58 +0200 Subject: [PATCH 7/7] Collect policies limit audit events service improvements Remove success response and add database health signal and external dependencies for the worker. --- .../collect_policies_limit_audit_events_service.rb | 2 -- ee/app/workers/all_queues.yml | 2 +- .../security/collect_policies_limit_audit_events_worker.rb | 5 +++++ .../collect_policies_limit_audit_events_service_spec.rb | 7 ------- 4 files changed, 6 insertions(+), 10 deletions(-) diff --git a/ee/app/services/security/security_orchestration_policies/collect_policies_limit_audit_events_service.rb b/ee/app/services/security/security_orchestration_policies/collect_policies_limit_audit_events_service.rb index 0f551abd914dc0..879407f399c837 100644 --- a/ee/app/services/security/security_orchestration_policies/collect_policies_limit_audit_events_service.rb +++ b/ee/app/services/security/security_orchestration_policies/collect_policies_limit_audit_events_service.rb @@ -19,8 +19,6 @@ def execute ::Gitlab::Audit::Auditor.audit(audit_context(policy_type, policy_limit, enabled_policies)) end - - ServiceResponse.success(message: 'Collected policies limit audit events') end private diff --git a/ee/app/workers/all_queues.yml b/ee/app/workers/all_queues.yml index fc6f275f11a2ee..9ae3d903297410 100644 --- a/ee/app/workers/all_queues.yml +++ b/ee/app/workers/all_queues.yml @@ -3447,7 +3447,7 @@ - :name: security_collect_policies_limit_audit_events :worker_name: Security::CollectPoliciesLimitAuditEventsWorker :feature_category: :security_policy_management - :has_external_dependencies: false + :has_external_dependencies: true :urgency: :low :resource_boundary: :unknown :weight: 1 diff --git a/ee/app/workers/security/collect_policies_limit_audit_events_worker.rb b/ee/app/workers/security/collect_policies_limit_audit_events_worker.rb index acde928da22ca1..57f04a84ea813d 100644 --- a/ee/app/workers/security/collect_policies_limit_audit_events_worker.rb +++ b/ee/app/workers/security/collect_policies_limit_audit_events_worker.rb @@ -9,6 +9,11 @@ class CollectPoliciesLimitAuditEventsWorker deduplicate :until_executed feature_category :security_policy_management + 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(configuration_id) configuration = Security::OrchestrationPolicyConfiguration.find_by_id(configuration_id) || return diff --git a/ee/spec/services/security/security_orchestration_policies/collect_policies_limit_audit_events_service_spec.rb b/ee/spec/services/security/security_orchestration_policies/collect_policies_limit_audit_events_service_spec.rb index 447da87cd39d8a..7154ab5b2f5c98 100644 --- a/ee/spec/services/security/security_orchestration_policies/collect_policies_limit_audit_events_service_spec.rb +++ b/ee/spec/services/security/security_orchestration_policies/collect_policies_limit_audit_events_service_spec.rb @@ -59,13 +59,6 @@ def mock_other_policy_type_limits(policy_types) execute_service end - - it 'returns a success response' do - result = execute_service - - expect(result).to be_success - expect(result.message).to eq('Collected policies limit audit events') - end end context 'when policy limits are exceeded' do -- GitLab