From cb4f43e6112804671c40d4c6e9e352a7a612e18f Mon Sep 17 00:00:00 2001 From: mc_rocha Date: Thu, 3 Jul 2025 18:28:04 -0400 Subject: [PATCH 1/7] Add failed pipelines with security policy jobs audit event Changelog: added EE: true --- config/sidekiq_queues.yml | 2 + doc/user/compliance/audit_event_types.md | 1 + .../security/pipeline_execution_policy.rb | 4 + ee/app/models/ee/ci/pipeline.rb | 24 +++ ee/app/workers/all_queues.yml | 10 ++ .../policies/failed_pipelines_audit_worker.rb | 28 +++ .../types/policy_pipeline_failed.yml | 10 ++ ...y_policy_failed_pipelines_audit_events.yml | 10 ++ .../pipeline_auditor.rb | 107 ++++++++++++ .../pipeline_failed_auditor.rb | 17 ++ .../pipeline_failed_auditor_spec.rb | 10 ++ ee/spec/models/ci/pipeline_spec.rb | 59 +++++++ .../pipeline_auditor_shared_examples.rb | 159 ++++++++++++++++++ .../failed_pipelines_audit_worker_spec.rb | 58 +++++++ 14 files changed, 499 insertions(+) create mode 100644 ee/app/workers/security/policies/failed_pipelines_audit_worker.rb create mode 100644 ee/config/audit_events/types/policy_pipeline_failed.yml create mode 100644 ee/config/feature_flags/gitlab_com_derisk/collect_security_policy_failed_pipelines_audit_events.yml create mode 100644 ee/lib/security/security_orchestration_policies/pipeline_auditor.rb create mode 100644 ee/lib/security/security_orchestration_policies/pipeline_failed_auditor.rb create mode 100644 ee/spec/lib/security/security_orchestration_policies/pipeline_failed_auditor_spec.rb create mode 100644 ee/spec/support/shared_examples/lib/security/security_orchestration_policies/pipeline_auditor_shared_examples.rb create mode 100644 ee/spec/workers/security/policies/failed_pipelines_audit_worker_spec.rb diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml index fe4fbf36e59d49..d3ea963d87a323 100644 --- a/config/sidekiq_queues.yml +++ b/config/sidekiq_queues.yml @@ -961,6 +961,8 @@ - 1 - - security_pipeline_execution_policies_run_schedule - 1 +- - security_policies_failed_pipelines_audit + - 1 - - security_policies_project_transfer - 1 - - security_policies_skip_pipelines_audit diff --git a/doc/user/compliance/audit_event_types.md b/doc/user/compliance/audit_event_types.md index d9f5398ff2b228..dd4a4454de2c1a 100644 --- a/doc/user/compliance/audit_event_types.md +++ b/doc/user/compliance/audit_event_types.md @@ -529,6 +529,7 @@ Audit event types belong to the following product categories. | Type name | Event triggered when | Saved to database | Introduced in | Scope | |:----------|:---------------------|:------------------|:--------------|:------| +| [`policy_pipeline_failed`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/195325) | A pipeline with security policy jobs failed | {{< icon name="dotted-circle" >}} No | GitLab [18.2](https://gitlab.com/gitlab-org/gitlab/-/issues/539232) | Project | | [`policy_project_updated`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/102154) | The security policy project is updated for a project | {{< icon name="check-circle" >}} Yes | GitLab [15.6](https://gitlab.com/gitlab-org/gitlab/-/issues/377877) | Group, Project | | [`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 | diff --git a/ee/app/models/concerns/security/pipeline_execution_policy.rb b/ee/app/models/concerns/security/pipeline_execution_policy.rb index e8106ede1f9c45..25cb09ffab4619 100644 --- a/ee/app/models/concerns/security/pipeline_execution_policy.rb +++ b/ee/app/models/concerns/security/pipeline_execution_policy.rb @@ -12,6 +12,10 @@ def active_pipeline_execution_policy_names active_pipeline_execution_policies.pluck(:name) # rubocop:disable Database/AvoidUsingPluckWithoutLimit -- not an ActiveRecord model and active_pipeline_execution_policies has limit end + def active_pipeline_execution_policy_names + active_pipeline_execution_policies.pluck(:name) # rubocop:disable Database/AvoidUsingPluckWithoutLimit -- not an ActiveRecord model and active_pipeline_execution_policies has limit + end + def pipeline_execution_policy policy_by_type(:pipeline_execution_policy) end diff --git a/ee/app/models/ee/ci/pipeline.rb b/ee/app/models/ee/ci/pipeline.rb index 5d48dccbe55d3d..14fb28ef101538 100644 --- a/ee/app/models/ee/ci/pipeline.rb +++ b/ee/app/models/ee/ci/pipeline.rb @@ -57,6 +57,9 @@ module Pipeline cyclonedx: %i[dependency_scanning container_scanning] }.freeze + PIPELINE_EXECUTION_POLICIES_BUILD_SOURCES = %w[pipeline_execution_policy scan_execution_policy].freeze + SECURITY_POLICIES_PIPELINE_SOURCES = %w[security_orchestration_policy security_policies_default_source pipeline_execution_policy_forced pipeline_execution_policy_schedule].freeze + def self.latest_limited_pipeline_ids_per_source(pipelines, sha) pipelines_for_sha = pipelines.complete_or_manual.for_sha(sha).order(id: :desc).limit(LATEST_PIPELINES_LIMIT) @@ -135,6 +138,14 @@ def self.latest_limited_pipeline_ids_per_source(pipelines, sha) end end end + + after_transition any => :failed do |pipeline| + pipeline.run_after_commit do + if should_audit_security_policy_pipeline_failure?(pipeline) + Security::Policies::FailedPipelinesAuditWorker.perform_async(pipeline.id) + end + end + end end end @@ -353,6 +364,19 @@ def security_builds def sbom_report_ingestion_errors_redis_key "sbom_report_ingestion_errors/#{id}" end + + def should_audit_security_policy_pipeline_failure?(pipeline) + ::Feature.enabled?(:collect_security_policy_failed_pipelines_audit_events, pipeline.project) && + (pipeline_created_by_security_policies?(pipeline.source) || pipeline_with_security_policy_jobs?) + end + + def pipeline_created_by_security_policies?(source) + SECURITY_POLICIES_PIPELINE_SOURCES.include?(source) + end + + def pipeline_with_security_policy_jobs? + builds.any? { |build| PIPELINE_EXECUTION_POLICIES_BUILD_SOURCES.include?(build.source) } + end end end end diff --git a/ee/app/workers/all_queues.yml b/ee/app/workers/all_queues.yml index 7f3373da70e2e9..b29bab1461b425 100644 --- a/ee/app/workers/all_queues.yml +++ b/ee/app/workers/all_queues.yml @@ -3604,6 +3604,16 @@ :idempotent: true :tags: [] :queue_namespace: +- :name: security_policies_failed_pipelines_audit + :worker_name: Security::Policies::FailedPipelinesAuditWorker + :feature_category: :security_policy_management + :has_external_dependencies: true + :urgency: :low + :resource_boundary: :unknown + :weight: 1 + :idempotent: true + :tags: [] + :queue_namespace: - :name: security_policies_project_transfer :worker_name: Security::Policies::ProjectTransferWorker :feature_category: :security_policy_management diff --git a/ee/app/workers/security/policies/failed_pipelines_audit_worker.rb b/ee/app/workers/security/policies/failed_pipelines_audit_worker.rb new file mode 100644 index 00000000000000..11ccb1c03c8950 --- /dev/null +++ b/ee/app/workers/security/policies/failed_pipelines_audit_worker.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +module Security + module Policies + class FailedPipelinesAuditWorker + include ApplicationWorker + + data_consistency :sticky + + feature_category :security_policy_management + urgency :low + idempotent! + 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(pipeline_id) + pipeline = Ci::Pipeline.find_by_id(pipeline_id) + return unless pipeline + return unless pipeline.project.licensed_feature_available?(:security_orchestration_policies) + + Security::SecurityOrchestrationPolicies::PipelineFailedAuditor.new(pipeline: pipeline).audit + end + end + end +end diff --git a/ee/config/audit_events/types/policy_pipeline_failed.yml b/ee/config/audit_events/types/policy_pipeline_failed.yml new file mode 100644 index 00000000000000..fcdb9843f7032f --- /dev/null +++ b/ee/config/audit_events/types/policy_pipeline_failed.yml @@ -0,0 +1,10 @@ +--- +name: policy_pipeline_failed +description: A pipeline with security policy jobs failed +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/539232 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/195325 +feature_category: security_policy_management +milestone: '18.2' +saved_to_database: false +streamed: true +scope: [Project] diff --git a/ee/config/feature_flags/gitlab_com_derisk/collect_security_policy_failed_pipelines_audit_events.yml b/ee/config/feature_flags/gitlab_com_derisk/collect_security_policy_failed_pipelines_audit_events.yml new file mode 100644 index 00000000000000..b242b0c4e5344e --- /dev/null +++ b/ee/config/feature_flags/gitlab_com_derisk/collect_security_policy_failed_pipelines_audit_events.yml @@ -0,0 +1,10 @@ +--- +name: collect_security_policy_failed_pipelines_audit_events +description: Collects audit events for failed pipelines with security policy jobs or pipelines created by security policies. +feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/539232 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/196628 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/554064 +milestone: '18.2' +group: group::security policies +type: gitlab_com_derisk +default_enabled: false diff --git a/ee/lib/security/security_orchestration_policies/pipeline_auditor.rb b/ee/lib/security/security_orchestration_policies/pipeline_auditor.rb new file mode 100644 index 00000000000000..29c22bd98cba2d --- /dev/null +++ b/ee/lib/security/security_orchestration_policies/pipeline_auditor.rb @@ -0,0 +1,107 @@ +# frozen_string_literal: true + +module Security + module SecurityOrchestrationPolicies + class PipelineAuditor + include Gitlab::Utils::StrongMemoize + + def initialize(pipeline:) + @pipeline = pipeline + end + + def audit + return unless pipeline + return unless security_orchestration_policy_configurations.present? + return unless skipped_policies.present? + + ::Gitlab::Audit::Auditor.audit(audit_context) + end + + private + + attr_reader :pipeline + + def audit_context + { + name: event_name, + author: pipeline.user, + scope: project, + target: pipeline, + target_details: pipeline.id.to_s, + message: event_message, + additional_details: additional_details + } + end + + def additional_details + { + commit_sha: pipeline.sha, + 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, + skipped_policies: skipped_policies + }.compact + end + + def skipped_policies + pipeline_execution_policies, scan_execution_policies = active_execution_policies + + skipped_seps = format_skipped_policies(scan_execution_policies, 'scan_execution_policy') + skipped_peps = format_skipped_policies(pipeline_execution_policies, 'pipeline_execution_policy') + + skipped_seps + skipped_peps + end + + def format_skipped_policies(policies, type) + policies.map { |name| { name: name, policy_type: type } } + end + + def active_execution_policies + scan_execution_policies = Set.new + pipeline_execution_policies = Set.new + + security_orchestration_policy_configurations.each do |policy| + active_seps_for_policy = if target_branch_ref + policy.active_scan_execution_policy_names(target_branch_ref, project) + else + [] + end + + active_peps_for_policy = policy.active_pipeline_execution_policy_names + + scan_execution_policies.merge(active_seps_for_policy) if active_seps_for_policy.present? + pipeline_execution_policies.merge(active_peps_for_policy) if active_peps_for_policy.present? + end + + [pipeline_execution_policies, scan_execution_policies] + end + + strong_memoize_attr :skipped_policies + + def security_orchestration_policy_configurations + project.all_security_orchestration_policy_configurations + end + strong_memoize_attr :security_orchestration_policy_configurations + + def project + pipeline.project + end + strong_memoize_attr :project + + def merge_request + pipeline.merge_request + end + strong_memoize_attr :merge_request + + def target_branch_ref + merge_request&.target_branch_ref + end + strong_memoize_attr :target_branch_ref + end + end +end diff --git a/ee/lib/security/security_orchestration_policies/pipeline_failed_auditor.rb b/ee/lib/security/security_orchestration_policies/pipeline_failed_auditor.rb new file mode 100644 index 00000000000000..c885c9aa8361a2 --- /dev/null +++ b/ee/lib/security/security_orchestration_policies/pipeline_failed_auditor.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +module Security + module SecurityOrchestrationPolicies + class PipelineFailedAuditor < PipelineAuditor + private + + def event_name + 'policy_pipeline_failed' + end + + def event_message + "Pipeline: #{pipeline.id} created by security policies or with security policy jobs failed" + end + end + end +end diff --git a/ee/spec/lib/security/security_orchestration_policies/pipeline_failed_auditor_spec.rb b/ee/spec/lib/security/security_orchestration_policies/pipeline_failed_auditor_spec.rb new file mode 100644 index 00000000000000..bc2db51c611c24 --- /dev/null +++ b/ee/spec/lib/security/security_orchestration_policies/pipeline_failed_auditor_spec.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Security::SecurityOrchestrationPolicies::PipelineFailedAuditor, feature_category: :security_policy_management do + it_behaves_like 'pipeline auditor' do + let(:event_name) { 'policy_pipeline_failed' } + let(:event_message) { "Pipeline: #{pipeline.id} created by security policies or with security policy jobs failed" } + end +end diff --git a/ee/spec/models/ci/pipeline_spec.rb b/ee/spec/models/ci/pipeline_spec.rb index 3c776abe3d9ff2..dfdb2963bc9f87 100644 --- a/ee/spec/models/ci/pipeline_spec.rb +++ b/ee/spec/models/ci/pipeline_spec.rb @@ -727,6 +727,65 @@ end end end + + context 'Security::Policies::FailedPipelinesAuditWorker' do + let(:source) { :push } + let(:pipeline) { create(:ci_empty_pipeline, project: project, status: from_status, source: source) } + let(:from_status) { Ci::HasStatus::ACTIVE_STATUSES[-1] } + + context 'on pipeline failed' do + subject(:transition_pipeline) { pipeline.drop } + + shared_examples 'does not enqueue FailedPipelinesAuditWorker' do + specify do + expect(Security::Policies::FailedPipelinesAuditWorker).not_to receive(:perform_async).with(pipeline.id) + + transition_pipeline + end + end + + shared_examples 'enqueues FailedPipelinesAuditWorker' do + specify do + expect(Security::Policies::FailedPipelinesAuditWorker).to receive(:perform_async).with(pipeline.id) + + transition_pipeline + end + end + + context 'when the feature flag `collect_security_policy_skipped_pipelines_audit_events` is disabled' do + before do + stub_feature_flags(collect_security_policy_failed_pipelines_audit_events: false) + end + + it_behaves_like 'does not enqueue FailedPipelinesAuditWorker' + end + + context 'when the pipeline was not created by a security policy' do + let(:source) { :web } + + context 'when the pipeline has no builds created by security policies' do + it_behaves_like 'does not enqueue FailedPipelinesAuditWorker' + end + + context 'when the pipeline has builds created by security policies' do + let!(:build) { create(:ci_build, pipeline: pipeline, project: project) } + let!(:build_source) { create(:ci_build_source, build: build, source: 'scan_execution_policy') } + + it_behaves_like 'enqueues FailedPipelinesAuditWorker' + end + end + + context 'when the pipeline was created by a security policy' do + let(:source) { :security_orchestration_policy } + + it 'enqueue FailedPipelinesAuditWorker' do + expect(Security::Policies::FailedPipelinesAuditWorker).to receive(:perform_async).with(pipeline.id) + + transition_pipeline + end + end + end + end end describe '#latest_merged_result_pipeline?' do diff --git a/ee/spec/support/shared_examples/lib/security/security_orchestration_policies/pipeline_auditor_shared_examples.rb b/ee/spec/support/shared_examples/lib/security/security_orchestration_policies/pipeline_auditor_shared_examples.rb new file mode 100644 index 00000000000000..2d1e6466f404f1 --- /dev/null +++ b/ee/spec/support/shared_examples/lib/security/security_orchestration_policies/pipeline_auditor_shared_examples.rb @@ -0,0 +1,159 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.shared_examples_for 'pipeline auditor' do + let_it_be(:project) { build(:project) } + + describe '#audit' do + subject(:audit) { described_class.new(pipeline: pipeline).audit } + + shared_examples 'does not call Gitlab::Audit::Auditor' do + specify do + expect(::Gitlab::Audit::Auditor).not_to receive(:audit) + + audit + end + end + + context 'when pipeline is nil' do + let_it_be(:pipeline) { nil } + + it_behaves_like 'does not call Gitlab::Audit::Auditor' + end + + context 'when the pipeline is present' do + let_it_be(:pipeline) { build(:ci_pipeline, project: project) } + + context 'when there is no security_orchestration_policy_configuration assigned to project' do + it_behaves_like 'does not call Gitlab::Audit::Auditor' + end + + context 'when there is a security_orchestration_policy_configuration assigned to project' do + let_it_be(:security_orchestration_policy_configuration) do + build(:security_orchestration_policy_configuration, project: project) + end + + before do + allow(project).to receive(:all_security_orchestration_policy_configurations).and_return( + [security_orchestration_policy_configuration]) + + allow(security_orchestration_policy_configuration).to receive(:active_scan_execution_policy_names).with( + merge_request&.target_branch_ref, project).and_return(active_scan_execution_policy_names) + + allow(security_orchestration_policy_configuration).to receive(:active_pipeline_execution_policy_names) + .and_return(active_pipeline_execution_policy_names) + end + + context 'when there are no active policies' do + let(:active_scan_execution_policy_names) { [] } + let(:active_pipeline_execution_policy_names) { [] } + let(:merge_request) { nil } + + it_behaves_like 'does not call Gitlab::Audit::Auditor' + end + + context 'when there are active policies' do + shared_examples_for 'calls Gitlab::Audit::Auditor.audit with the expected context' do + specify do + expect(::Gitlab::Audit::Auditor).to receive(:audit) do |context| + expect(context[:name]).to eq(event_name) + expect(context[:author]).to eq(pipeline.user) + expect(context[:scope]).to eq(project) + expect(context[:target]).to eq(pipeline) + expect(context[:target_details]).to eq(pipeline.id.to_s) + expect(context[:message]).to eq(event_message) + expect(context[:additional_details]).to eq(additional_details) + end + + audit + end + end + + shared_examples_for 'when the merge_request is present' do + context 'when the merge_request is present' do + let_it_be(:merge_request) do + build(:merge_request, id: 1, iid: 1, source_project: project, target_project: project) + end + + let(:additional_details) do + { + commit_sha: pipeline.sha, + 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, + skipped_policies: skipped_policies_details + } + end + + before do + pipeline.merge_request = merge_request + end + + it_behaves_like 'calls Gitlab::Audit::Auditor.audit with the expected context' + end + end + + context 'when there are active scan_execution_policies policies' do + let(:skipped_policy_name) { 'Skipped sep policy' } + let(:skipped_policies_details) { [{ name: skipped_policy_name, policy_type: 'scan_execution_policy' }] } + + let(:active_scan_execution_policy_names) { [skipped_policy_name] } + let(:active_pipeline_execution_policy_names) { [] } + + it_behaves_like 'when the merge_request is present' + end + + context 'when there are active pipeline_execution_policies policies' do + let(:skipped_policy_name) { 'Skipped pep policy' } + let(:skipped_policies_details) { [{ name: skipped_policy_name, policy_type: 'pipeline_execution_policy' }] } + + let(:active_scan_execution_policy_names) { [] } + let(:active_pipeline_execution_policy_names) { [skipped_policy_name] } + + it_behaves_like 'when the merge_request is present' + + context 'when merge_request is nil' do + let(:merge_request) { nil } + let(:additional_details) do + { + commit_sha: pipeline.sha, + project_id: project.id, + project_name: project.name, + project_full_path: project.full_path, + skipped_policies: skipped_policies_details + } + end + + before do + pipeline.merge_request = merge_request + end + + it_behaves_like 'calls Gitlab::Audit::Auditor.audit with the expected context' + end + end + + context 'when there are active scan_execution and pipeline_execution policies' do + let(:skipped_sep_policy_name) { 'Skipped sep policy' } + let(:skipped_pep_policy_name) { 'Skipped pep policy' } + let(:skipped_policies_details) do + [{ name: skipped_sep_policy_name, policy_type: 'scan_execution_policy' }, + { name: skipped_pep_policy_name, + policy_type: 'pipeline_execution_policy' }] + end + + let(:active_scan_execution_policy_names) { [skipped_sep_policy_name] } + let(:active_pipeline_execution_policy_names) { [skipped_pep_policy_name] } + + it_behaves_like 'when the merge_request is present' + end + end + end + end + end +end diff --git a/ee/spec/workers/security/policies/failed_pipelines_audit_worker_spec.rb b/ee/spec/workers/security/policies/failed_pipelines_audit_worker_spec.rb new file mode 100644 index 00000000000000..3b32c002c98e6e --- /dev/null +++ b/ee/spec/workers/security/policies/failed_pipelines_audit_worker_spec.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Security::Policies::FailedPipelinesAuditWorker, feature_category: :security_policy_management do + describe '#perform' do + let_it_be(:pipeline) { create(:ci_pipeline) } + + subject(:run_worker) { described_class.new.perform(pipeline_id) } + + shared_examples_for 'does not call PipelineFailedAuditor' do + specify do + expect(Security::SecurityOrchestrationPolicies::PipelineFailedAuditor).not_to receive(:new) + + run_worker + end + end + + context 'when pipeline is not found' do + let(:pipeline_id) { non_existing_record_id } + + it_behaves_like 'does not call PipelineFailedAuditor' + end + + context 'when pipeline exist' do + let(:pipeline_id) { pipeline.id } + + context 'when security_orchestration_policies feature is available' do + before do + stub_licensed_features(security_orchestration_policies: true) + end + + it 'calls PipelineFailedAuditor' do + expect_next_instance_of(Security::SecurityOrchestrationPolicies::PipelineFailedAuditor, + pipeline: pipeline) do |auditor| + expect(auditor).to receive(:audit) + end + + run_worker + end + + it_behaves_like 'an idempotent worker' do + let(:job_args) { pipeline.id } + end + end + + context 'when security_orchestration_policies feature is not available' do + let(:pipeline_id) { pipeline.id } + + before do + stub_licensed_features(security_orchestration_policies: false) + end + + it_behaves_like 'does not call PipelineFailedAuditor' + end + end + end +end -- GitLab From 2d54bce4c994caf54886a0b945569df15cd9d1b1 Mon Sep 17 00:00:00 2001 From: mc_rocha Date: Tue, 8 Jul 2025 14:19:31 -0400 Subject: [PATCH 2/7] Address reviewer suggestion --- doc/user/compliance/audit_event_types.md | 2 +- .../types/policy_pipeline_failed.yml | 2 +- .../pipeline_auditor.rb | 12 ++++++-- .../pipeline_auditor_shared_examples.rb | 30 ++++++++++++------- 4 files changed, 32 insertions(+), 14 deletions(-) diff --git a/doc/user/compliance/audit_event_types.md b/doc/user/compliance/audit_event_types.md index dd4a4454de2c1a..3d95b90a1bd833 100644 --- a/doc/user/compliance/audit_event_types.md +++ b/doc/user/compliance/audit_event_types.md @@ -529,7 +529,7 @@ Audit event types belong to the following product categories. | Type name | Event triggered when | Saved to database | Introduced in | Scope | |:----------|:---------------------|:------------------|:--------------|:------| -| [`policy_pipeline_failed`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/195325) | A pipeline with security policy jobs failed | {{< icon name="dotted-circle" >}} No | GitLab [18.2](https://gitlab.com/gitlab-org/gitlab/-/issues/539232) | Project | +| [`policy_pipeline_failed`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/196628) | A pipeline with security policy jobs failed | {{< icon name="dotted-circle" >}} No | GitLab [18.2](https://gitlab.com/gitlab-org/gitlab/-/issues/539232) | Project | | [`policy_project_updated`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/102154) | The security policy project is updated for a project | {{< icon name="check-circle" >}} Yes | GitLab [15.6](https://gitlab.com/gitlab-org/gitlab/-/issues/377877) | Group, Project | | [`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 | diff --git a/ee/config/audit_events/types/policy_pipeline_failed.yml b/ee/config/audit_events/types/policy_pipeline_failed.yml index fcdb9843f7032f..bd4c8fa9539a8d 100644 --- a/ee/config/audit_events/types/policy_pipeline_failed.yml +++ b/ee/config/audit_events/types/policy_pipeline_failed.yml @@ -2,7 +2,7 @@ name: policy_pipeline_failed description: A pipeline with security policy jobs failed introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/539232 -introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/195325 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/196628 feature_category: security_policy_management milestone: '18.2' saved_to_database: false diff --git a/ee/lib/security/security_orchestration_policies/pipeline_auditor.rb b/ee/lib/security/security_orchestration_policies/pipeline_auditor.rb index 29c22bd98cba2d..1cd5bdf44b321c 100644 --- a/ee/lib/security/security_orchestration_policies/pipeline_auditor.rb +++ b/ee/lib/security/security_orchestration_policies/pipeline_auditor.rb @@ -21,10 +21,18 @@ def audit attr_reader :pipeline + def event_name + raise NoMethodError, "#{self.class} must implement the method #{__method__}" + end + + def event_message + raise NoMethodError, "#{self.class} must implement the method #{__method__}" + end + def audit_context { name: event_name, - author: pipeline.user, + author: pipeline.user || Gitlab::Audit::DeletedAuthor.new(id: -4, name: 'Unknown User'), scope: project, target: pipeline, target_details: pipeline.id.to_s, @@ -94,7 +102,7 @@ def project strong_memoize_attr :project def merge_request - pipeline.merge_request + pipeline.all_merge_requests.first end strong_memoize_attr :merge_request diff --git a/ee/spec/support/shared_examples/lib/security/security_orchestration_policies/pipeline_auditor_shared_examples.rb b/ee/spec/support/shared_examples/lib/security/security_orchestration_policies/pipeline_auditor_shared_examples.rb index 2d1e6466f404f1..50015b34ee376c 100644 --- a/ee/spec/support/shared_examples/lib/security/security_orchestration_policies/pipeline_auditor_shared_examples.rb +++ b/ee/spec/support/shared_examples/lib/security/security_orchestration_policies/pipeline_auditor_shared_examples.rb @@ -23,7 +23,8 @@ end context 'when the pipeline is present' do - let_it_be(:pipeline) { build(:ci_pipeline, project: project) } + let_it_be(:user) { build(:user) } + let_it_be(:pipeline) { build(:ci_pipeline, project: project, user: user) } context 'when there is no security_orchestration_policy_configuration assigned to project' do it_behaves_like 'does not call Gitlab::Audit::Auditor' @@ -56,15 +57,19 @@ context 'when there are active policies' do shared_examples_for 'calls Gitlab::Audit::Auditor.audit with the expected context' do specify do - expect(::Gitlab::Audit::Auditor).to receive(:audit) do |context| - expect(context[:name]).to eq(event_name) - expect(context[:author]).to eq(pipeline.user) - expect(context[:scope]).to eq(project) - expect(context[:target]).to eq(pipeline) - expect(context[:target_details]).to eq(pipeline.id.to_s) - expect(context[:message]).to eq(event_message) - expect(context[:additional_details]).to eq(additional_details) - end + expect(::Gitlab::Audit::Auditor).to receive(:audit).with( + hash_including( + { + name: event_name, + author: user, + scope: project, + target: pipeline, + target_details: pipeline.id.to_s, + message: event_message, + additional_details: additional_details + } + ) + ) audit end @@ -93,6 +98,11 @@ before do pipeline.merge_request = merge_request + + merge_requests_relation = instance_double(ActiveRecord::Relation) + + allow(pipeline).to receive(:all_merge_requests).and_return(merge_requests_relation) + allow(merge_requests_relation).to receive(:first).and_return(merge_request) end it_behaves_like 'calls Gitlab::Audit::Auditor.audit with the expected context' -- GitLab From 794ece184ff965e6497c42d358aaccf3d861a0aa Mon Sep 17 00:00:00 2001 From: mc_rocha Date: Wed, 9 Jul 2025 14:20:12 -0400 Subject: [PATCH 3/7] Sent one audit event per policy configuration --- .../security/pipeline_execution_policy.rb | 4 - .../pipeline_auditor.rb | 68 +++++++------ .../pipeline_auditor_shared_examples.rb | 99 ++++++++++++++++++- 3 files changed, 132 insertions(+), 39 deletions(-) diff --git a/ee/app/models/concerns/security/pipeline_execution_policy.rb b/ee/app/models/concerns/security/pipeline_execution_policy.rb index 25cb09ffab4619..e8106ede1f9c45 100644 --- a/ee/app/models/concerns/security/pipeline_execution_policy.rb +++ b/ee/app/models/concerns/security/pipeline_execution_policy.rb @@ -12,10 +12,6 @@ def active_pipeline_execution_policy_names active_pipeline_execution_policies.pluck(:name) # rubocop:disable Database/AvoidUsingPluckWithoutLimit -- not an ActiveRecord model and active_pipeline_execution_policies has limit end - def active_pipeline_execution_policy_names - active_pipeline_execution_policies.pluck(:name) # rubocop:disable Database/AvoidUsingPluckWithoutLimit -- not an ActiveRecord model and active_pipeline_execution_policies has limit - end - def pipeline_execution_policy policy_by_type(:pipeline_execution_policy) end diff --git a/ee/lib/security/security_orchestration_policies/pipeline_auditor.rb b/ee/lib/security/security_orchestration_policies/pipeline_auditor.rb index 1cd5bdf44b321c..4dcce1a0f07c87 100644 --- a/ee/lib/security/security_orchestration_policies/pipeline_auditor.rb +++ b/ee/lib/security/security_orchestration_policies/pipeline_auditor.rb @@ -12,9 +12,14 @@ def initialize(pipeline:) def audit return unless pipeline return unless security_orchestration_policy_configurations.present? - return unless skipped_policies.present? - ::Gitlab::Audit::Auditor.audit(audit_context) + security_orchestration_policy_configurations.each do |policy_configuration| + skipped_policies = skipped_policies(policy_configuration) + + next if skipped_policies.blank? + + ::Gitlab::Audit::Auditor.audit(audit_context(policy_configuration, skipped_policies)) + end end private @@ -29,19 +34,23 @@ def event_message raise NoMethodError, "#{self.class} must implement the method #{__method__}" end - def audit_context + def audit_context(policy_configuration, skipped_policies) { name: event_name, - author: pipeline.user || Gitlab::Audit::DeletedAuthor.new(id: -4, name: 'Unknown User'), - scope: project, + author: pipeline_author, + scope: policy_configuration.security_policy_management_project, target: pipeline, target_details: pipeline.id.to_s, message: event_message, - additional_details: additional_details + additional_details: additional_details(skipped_policies) } end - def additional_details + def additional_details(skipped_policies) + additional_details_base.merge({ skipped_policies: skipped_policies }) + end + + def additional_details_base { commit_sha: pipeline.sha, merge_request_title: merge_request&.title, @@ -51,13 +60,13 @@ def additional_details target_branch: merge_request&.target_branch, project_id: project.id, project_name: project.name, - project_full_path: project.full_path, - skipped_policies: skipped_policies + project_full_path: project.full_path }.compact end + strong_memoize_attr :additional_details_base - def skipped_policies - pipeline_execution_policies, scan_execution_policies = active_execution_policies + def skipped_policies(policy_configuration) + pipeline_execution_policies, scan_execution_policies = active_execution_policies(policy_configuration) skipped_seps = format_skipped_policies(scan_execution_policies, 'scan_execution_policy') skipped_peps = format_skipped_policies(pipeline_execution_policies, 'pipeline_execution_policy') @@ -65,31 +74,21 @@ def skipped_policies skipped_seps + skipped_peps end - def format_skipped_policies(policies, type) - policies.map { |name| { name: name, policy_type: type } } - end - - def active_execution_policies - scan_execution_policies = Set.new - pipeline_execution_policies = Set.new + def active_execution_policies(policy_configuration) + active_seps_for_policy = if target_branch_ref + policy_configuration.active_scan_execution_policy_names(target_branch_ref, project) + else + [] + end - security_orchestration_policy_configurations.each do |policy| - active_seps_for_policy = if target_branch_ref - policy.active_scan_execution_policy_names(target_branch_ref, project) - else - [] - end + active_peps_for_policy = policy_configuration.active_pipeline_execution_policy_names || [] - active_peps_for_policy = policy.active_pipeline_execution_policy_names - - scan_execution_policies.merge(active_seps_for_policy) if active_seps_for_policy.present? - pipeline_execution_policies.merge(active_peps_for_policy) if active_peps_for_policy.present? - end - - [pipeline_execution_policies, scan_execution_policies] + [active_peps_for_policy, active_seps_for_policy] end - strong_memoize_attr :skipped_policies + def format_skipped_policies(policies, type) + policies.map { |name| { name: name, policy_type: type } } + end def security_orchestration_policy_configurations project.all_security_orchestration_policy_configurations @@ -110,6 +109,11 @@ def target_branch_ref merge_request&.target_branch_ref end strong_memoize_attr :target_branch_ref + + def pipeline_author + pipeline.user || Gitlab::Audit::DeletedAuthor.new(id: -4, name: 'Unknown User') + end + strong_memoize_attr :pipeline_author end end end diff --git a/ee/spec/support/shared_examples/lib/security/security_orchestration_policies/pipeline_auditor_shared_examples.rb b/ee/spec/support/shared_examples/lib/security/security_orchestration_policies/pipeline_auditor_shared_examples.rb index 50015b34ee376c..89dc8287fc5fa5 100644 --- a/ee/spec/support/shared_examples/lib/security/security_orchestration_policies/pipeline_auditor_shared_examples.rb +++ b/ee/spec/support/shared_examples/lib/security/security_orchestration_policies/pipeline_auditor_shared_examples.rb @@ -31,13 +31,17 @@ end context 'when there is a security_orchestration_policy_configuration assigned to project' do + let_it_be(:security_policy_management_project) { build(:project) } let_it_be(:security_orchestration_policy_configuration) do - build(:security_orchestration_policy_configuration, project: project) + build(:security_orchestration_policy_configuration, project: project, + security_policy_management_project: security_policy_management_project) end + let_it_be(:all_policies) { [security_orchestration_policy_configuration] } + before do allow(project).to receive(:all_security_orchestration_policy_configurations).and_return( - [security_orchestration_policy_configuration]) + all_policies) allow(security_orchestration_policy_configuration).to receive(:active_scan_execution_policy_names).with( merge_request&.target_branch_ref, project).and_return(active_scan_execution_policy_names) @@ -62,7 +66,7 @@ { name: event_name, author: user, - scope: project, + scope: security_policy_management_project, target: pipeline, target_details: pipeline.id.to_s, message: event_message, @@ -161,6 +165,95 @@ let(:active_pipeline_execution_policy_names) { [skipped_pep_policy_name] } it_behaves_like 'when the merge_request is present' + + context 'when there are inherited security_orchestration_policy_configurations' do + let_it_be(:inherited_security_policy_management_project) { build(:project) } + let(:inherited_skipped_sep_policy_name) { 'Inherited skipped sep policy' } + let(:inherited_skipped_pep_policy_name) { 'Inherited skipped pep policy' } + let(:inherited_active_scan_execution_policy_names) { [inherited_skipped_sep_policy_name] } + let(:inherited_active_pipeline_execution_policy_names) { [inherited_skipped_pep_policy_name] } + let(:inherited_skipped_policies_details) do + [{ name: inherited_skipped_sep_policy_name, policy_type: 'scan_execution_policy' }, + { name: inherited_skipped_pep_policy_name, + policy_type: 'pipeline_execution_policy' }] + end + + let_it_be(:parent_group) { build(:group) } + let_it_be(:inherited_security_orchestration_policy_configuration) do + build(:security_orchestration_policy_configuration, namespace: parent_group, + security_policy_management_project: inherited_security_policy_management_project) + end + + let_it_be(:all_policies) do + [security_orchestration_policy_configuration, inherited_security_orchestration_policy_configuration] + end + + before do + allow(inherited_security_orchestration_policy_configuration) + .to receive(:active_scan_execution_policy_names).with( + merge_request&.target_branch_ref, project).and_return(inherited_active_scan_execution_policy_names) + + allow(inherited_security_orchestration_policy_configuration) + .to receive(:active_pipeline_execution_policy_names) + .and_return(inherited_active_pipeline_execution_policy_names) + end + + context 'when the merge_request is present' do + let_it_be(:merge_request) do + build(:merge_request, id: 1, iid: 1, source_project: project, target_project: project) + end + + let(:additional_details) do + { + commit_sha: pipeline.sha, + 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 + } + end + + before do + pipeline.merge_request = merge_request + + merge_requests_relation = instance_double(ActiveRecord::Relation) + + allow(pipeline).to receive(:all_merge_requests).and_return(merge_requests_relation) + allow(merge_requests_relation).to receive(:first).and_return(merge_request) + end + + it 'calls Gitlab::Audit::Auditor.audit for each policy configuration with the expected context' do + security_policy_projects = [security_policy_management_project, + inherited_security_policy_management_project] + additional_details_including_policies = [ + additional_details.merge({ skipped_policies: skipped_policies_details }), + additional_details.merge({ skipped_policies: inherited_skipped_policies_details }) + ] + + all_policies.size.times do |index| + expect(::Gitlab::Audit::Auditor).to receive(:audit).with( + hash_including( + { + name: event_name, + author: user, + scope: security_policy_projects[index], + target: pipeline, + target_details: pipeline.id.to_s, + message: event_message, + additional_details: additional_details_including_policies[index] + } + ) + ) + end + + audit + end + end + end end end end -- GitLab From 23cc3c622bff87e26c3ae6526d2aeece5c1019d7 Mon Sep 17 00:00:00 2001 From: mc_rocha Date: Fri, 11 Jul 2025 10:42:19 -0400 Subject: [PATCH 4/7] Refactor PipelineSkippedAuditor class Changelog: changed EE: true --- .../pipeline_auditor.rb | 2 +- .../pipeline_skipped_auditor.rb | 98 +---------- .../pipeline_skipped_auditor_spec.rb | 159 +----------------- 3 files changed, 9 insertions(+), 250 deletions(-) diff --git a/ee/lib/security/security_orchestration_policies/pipeline_auditor.rb b/ee/lib/security/security_orchestration_policies/pipeline_auditor.rb index 4dcce1a0f07c87..7facd657700e56 100644 --- a/ee/lib/security/security_orchestration_policies/pipeline_auditor.rb +++ b/ee/lib/security/security_orchestration_policies/pipeline_auditor.rb @@ -91,7 +91,7 @@ def format_skipped_policies(policies, type) end def security_orchestration_policy_configurations - project.all_security_orchestration_policy_configurations + project&.all_security_orchestration_policy_configurations end strong_memoize_attr :security_orchestration_policy_configurations diff --git a/ee/lib/security/security_orchestration_policies/pipeline_skipped_auditor.rb b/ee/lib/security/security_orchestration_policies/pipeline_skipped_auditor.rb index c8b5c1717f074c..59f47780f9b872 100644 --- a/ee/lib/security/security_orchestration_policies/pipeline_skipped_auditor.rb +++ b/ee/lib/security/security_orchestration_policies/pipeline_skipped_auditor.rb @@ -2,104 +2,16 @@ module Security module SecurityOrchestrationPolicies - class PipelineSkippedAuditor - include Gitlab::Utils::StrongMemoize - - def initialize(pipeline:) - @pipeline = pipeline - end - - def audit - return unless pipeline - return unless security_orchestration_policy_configurations.present? - return unless skipped_policies.present? - - ::Gitlab::Audit::Auditor.audit(audit_context) - end - + class PipelineSkippedAuditor < PipelineAuditor private - attr_reader :pipeline - - def audit_context - { - name: 'policy_pipeline_skipped', - author: pipeline.user, - scope: project, - target: pipeline, - target_details: pipeline.commit.present? ? pipeline.commit.title : pipeline.name, - message: "Pipeline: #{pipeline.id} with security policy jobs skipped", - additional_details: additional_details - } - end - - def additional_details - { - commit_sha: pipeline.sha, - 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, - skipped_policies: skipped_policies - }.compact - end - - def skipped_policies - pipeline_execution_policies, scan_execution_policies = active_execution_policies - - skipped_seps = format_skipped_policies(scan_execution_policies, 'scan_execution_policy') - skipped_peps = format_skipped_policies(pipeline_execution_policies, 'pipeline_execution_policy') - - skipped_seps + skipped_peps - end - - def format_skipped_policies(policies, type) - policies.map { |name| { name: name, policy_type: type } } - end - - def active_execution_policies - scan_execution_policies = Set.new - pipeline_execution_policies = Set.new - - security_orchestration_policy_configurations.each do |policy| - if target_branch_ref - active_seps_for_policy = policy.active_scan_execution_policy_names(target_branch_ref, project) - end - - active_peps_for_policy = policy.active_pipeline_execution_policy_names - - scan_execution_policies.merge(active_seps_for_policy) if active_seps_for_policy.present? - pipeline_execution_policies.merge(active_peps_for_policy) if active_peps_for_policy.present? - end - - [pipeline_execution_policies, scan_execution_policies] - end - - strong_memoize_attr :skipped_policies - - def security_orchestration_policy_configurations - project.all_security_orchestration_policy_configurations - end - strong_memoize_attr :security_orchestration_policy_configurations - - def project - pipeline.project - end - strong_memoize_attr :project - - def merge_request - pipeline.merge_request + def event_name + 'policy_pipeline_skipped' end - strong_memoize_attr :merge_request - def target_branch_ref - merge_request&.target_branch_ref + def event_message + "Pipeline: #{pipeline.id} with security policy jobs skipped" end - strong_memoize_attr :target_branch_ref end end end diff --git a/ee/spec/lib/security/security_orchestration_policies/pipeline_skipped_auditor_spec.rb b/ee/spec/lib/security/security_orchestration_policies/pipeline_skipped_auditor_spec.rb index 07ebb22c997b73..eb8941327f69b6 100644 --- a/ee/spec/lib/security/security_orchestration_policies/pipeline_skipped_auditor_spec.rb +++ b/ee/spec/lib/security/security_orchestration_policies/pipeline_skipped_auditor_spec.rb @@ -3,161 +3,8 @@ require 'spec_helper' RSpec.describe Security::SecurityOrchestrationPolicies::PipelineSkippedAuditor, feature_category: :security_policy_management do - let_it_be(:project) { build(:project) } - - describe '#audit' do - subject(:audit) { described_class.new(pipeline: pipeline).audit } - - shared_examples 'does not call Gitlab::Audit::Auditor' do - specify do - expect(::Gitlab::Audit::Auditor).not_to receive(:audit) - - audit - end - end - - context 'when pipeline is nil' do - let_it_be(:pipeline) { nil } - - it_behaves_like 'does not call Gitlab::Audit::Auditor' - end - - context 'when the pipeline is present' do - let_it_be(:commit_title) { '[skip ci]Add .gitlab-ci.yml' } - let_it_be(:pipeline) { build(:ci_pipeline, project: project, id: 42) } - let_it_be(:commit) { build(:commit, safe_message: commit_title) } - - context 'when there is no security_orchestration_policy_configuration assigned to project' do - it_behaves_like 'does not call Gitlab::Audit::Auditor' - end - - context 'when there is a security_orchestration_policy_configuration assigned to project' do - let_it_be(:security_orchestration_policy_configuration) do - build(:security_orchestration_policy_configuration, project: project) - end - - before do - allow(project).to receive(:all_security_orchestration_policy_configurations).and_return( - [security_orchestration_policy_configuration]) - - allow(security_orchestration_policy_configuration).to receive(:active_scan_execution_policy_names).with( - merge_request&.target_branch_ref, project).and_return(active_scan_execution_policy_names) - - allow(security_orchestration_policy_configuration).to receive(:active_pipeline_execution_policy_names) - .and_return(active_pipeline_execution_policy_names) - - allow(pipeline).to receive(:commit).and_return(commit) - end - - context 'when there are no active policies' do - let(:active_scan_execution_policy_names) { [] } - let(:active_pipeline_execution_policy_names) { [] } - let(:merge_request) { nil } - - it_behaves_like 'does not call Gitlab::Audit::Auditor' - end - - context 'when there are active policies' do - shared_examples_for 'calls Gitlab::Audit::Auditor.audit with the expected context' do - specify do - expect(::Gitlab::Audit::Auditor).to receive(:audit) do |context| - expect(context[:name]).to eq('policy_pipeline_skipped') - expect(context[:author]).to eq(pipeline.user) - expect(context[:scope]).to eq(project) - expect(context[:target]).to eq(pipeline) - expect(context[:target_details]).to eq(commit_title) - expect(context[:message]).to eq("Pipeline: #{pipeline.id} with security policy jobs skipped") - expect(context[:additional_details]).to eq(additional_details) - end - - audit - end - end - - shared_examples_for 'when the merge_request is present' do - context 'when the merge_request is present' do - let_it_be(:merge_request) do - build(:merge_request, id: 1, iid: 1, source_project: project, target_project: project) - end - - let(:additional_details) do - { - commit_sha: pipeline.sha, - 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, - skipped_policies: skipped_policies_details - } - end - - before do - pipeline.merge_request = merge_request - end - - it_behaves_like 'calls Gitlab::Audit::Auditor.audit with the expected context' - end - end - - context 'when there are active scan_execution_policies policies' do - let(:skipped_policy_name) { 'Skipped sep policy' } - let(:skipped_policies_details) { [{ name: skipped_policy_name, policy_type: 'scan_execution_policy' }] } - - let(:active_scan_execution_policy_names) { [skipped_policy_name] } - let(:active_pipeline_execution_policy_names) { [] } - - it_behaves_like 'when the merge_request is present' - end - - context 'when there are active pipeline_execution_policies policies' do - let(:skipped_policy_name) { 'Skipped pep policy' } - let(:skipped_policies_details) { [{ name: skipped_policy_name, policy_type: 'pipeline_execution_policy' }] } - - let(:active_scan_execution_policy_names) { [] } - let(:active_pipeline_execution_policy_names) { [skipped_policy_name] } - - it_behaves_like 'when the merge_request is present' - - context 'when merge_request is nil' do - let(:merge_request) { nil } - let(:additional_details) do - { - commit_sha: pipeline.sha, - project_id: project.id, - project_name: project.name, - project_full_path: project.full_path, - skipped_policies: skipped_policies_details - } - end - - before do - pipeline.merge_request = merge_request - end - - it_behaves_like 'calls Gitlab::Audit::Auditor.audit with the expected context' - end - end - - context 'when there are active scan_execution and pipeline_execution policies' do - let(:skipped_sep_policy_name) { 'Skipped sep policy' } - let(:skipped_pep_policy_name) { 'Skipped pep policy' } - let(:skipped_policies_details) do - [{ name: skipped_sep_policy_name, policy_type: 'scan_execution_policy' }, - { name: skipped_pep_policy_name, - policy_type: 'pipeline_execution_policy' }] - end - - let(:active_scan_execution_policy_names) { [skipped_sep_policy_name] } - let(:active_pipeline_execution_policy_names) { [skipped_pep_policy_name] } - - it_behaves_like 'when the merge_request is present' - end - end - end - end + it_behaves_like 'pipeline auditor' do + let(:event_name) { 'policy_pipeline_skipped' } + let(:event_message) { "Pipeline: #{pipeline.id} with security policy jobs skipped" } end end -- GitLab From c767beb17488ed8e83b005cc7bc21be93621fdfa Mon Sep 17 00:00:00 2001 From: Marcos Rocha Date: Mon, 14 Jul 2025 10:36:16 +0000 Subject: [PATCH 5/7] Apply reviewer suggestions --- doc/user/compliance/audit_event_types.md | 2 +- ee/app/models/ee/ci/pipeline.rb | 2 +- ee/config/audit_events/types/policy_pipeline_failed.yml | 2 +- .../collect_security_policy_failed_pipelines_audit_events.yml | 2 +- ee/spec/models/ci/pipeline_spec.rb | 2 +- .../pipeline_auditor_shared_examples.rb | 4 ++-- 6 files changed, 7 insertions(+), 7 deletions(-) diff --git a/doc/user/compliance/audit_event_types.md b/doc/user/compliance/audit_event_types.md index 3d95b90a1bd833..1bf3a71934cd61 100644 --- a/doc/user/compliance/audit_event_types.md +++ b/doc/user/compliance/audit_event_types.md @@ -529,7 +529,7 @@ Audit event types belong to the following product categories. | Type name | Event triggered when | Saved to database | Introduced in | Scope | |:----------|:---------------------|:------------------|:--------------|:------| -| [`policy_pipeline_failed`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/196628) | A pipeline with security policy jobs failed | {{< icon name="dotted-circle" >}} No | GitLab [18.2](https://gitlab.com/gitlab-org/gitlab/-/issues/539232) | Project | +| [`policy_pipeline_failed`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/196628) | A pipeline with security policy jobs failed | {{< icon name="dotted-circle" >}} No | GitLab [18.3](https://gitlab.com/gitlab-org/gitlab/-/issues/539232) | Project | | [`policy_project_updated`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/102154) | The security policy project is updated for a project | {{< icon name="check-circle" >}} Yes | GitLab [15.6](https://gitlab.com/gitlab-org/gitlab/-/issues/377877) | Group, Project | | [`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 | diff --git a/ee/app/models/ee/ci/pipeline.rb b/ee/app/models/ee/ci/pipeline.rb index 14fb28ef101538..e12305bf151e51 100644 --- a/ee/app/models/ee/ci/pipeline.rb +++ b/ee/app/models/ee/ci/pipeline.rb @@ -375,7 +375,7 @@ def pipeline_created_by_security_policies?(source) end def pipeline_with_security_policy_jobs? - builds.any? { |build| PIPELINE_EXECUTION_POLICIES_BUILD_SOURCES.include?(build.source) } + builds.joins(:build_source).where(p_ci_build_sources: { source: PIPELINE_EXECUTION_POLICIES_BUILD_SOURCES }).exists? end end end diff --git a/ee/config/audit_events/types/policy_pipeline_failed.yml b/ee/config/audit_events/types/policy_pipeline_failed.yml index bd4c8fa9539a8d..7cfeecd0979e69 100644 --- a/ee/config/audit_events/types/policy_pipeline_failed.yml +++ b/ee/config/audit_events/types/policy_pipeline_failed.yml @@ -4,7 +4,7 @@ description: A pipeline with security policy jobs failed introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/539232 introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/196628 feature_category: security_policy_management -milestone: '18.2' +milestone: '18.3' saved_to_database: false streamed: true scope: [Project] diff --git a/ee/config/feature_flags/gitlab_com_derisk/collect_security_policy_failed_pipelines_audit_events.yml b/ee/config/feature_flags/gitlab_com_derisk/collect_security_policy_failed_pipelines_audit_events.yml index b242b0c4e5344e..4715dc89edccf3 100644 --- a/ee/config/feature_flags/gitlab_com_derisk/collect_security_policy_failed_pipelines_audit_events.yml +++ b/ee/config/feature_flags/gitlab_com_derisk/collect_security_policy_failed_pipelines_audit_events.yml @@ -4,7 +4,7 @@ description: Collects audit events for failed pipelines with security policy job feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/539232 introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/196628 rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/554064 -milestone: '18.2' +milestone: '18.3' group: group::security policies type: gitlab_com_derisk default_enabled: false diff --git a/ee/spec/models/ci/pipeline_spec.rb b/ee/spec/models/ci/pipeline_spec.rb index dfdb2963bc9f87..df638611228c8a 100644 --- a/ee/spec/models/ci/pipeline_spec.rb +++ b/ee/spec/models/ci/pipeline_spec.rb @@ -778,7 +778,7 @@ context 'when the pipeline was created by a security policy' do let(:source) { :security_orchestration_policy } - it 'enqueue FailedPipelinesAuditWorker' do + it 'enqueues FailedPipelinesAuditWorker' do expect(Security::Policies::FailedPipelinesAuditWorker).to receive(:perform_async).with(pipeline.id) transition_pipeline diff --git a/ee/spec/support/shared_examples/lib/security/security_orchestration_policies/pipeline_auditor_shared_examples.rb b/ee/spec/support/shared_examples/lib/security/security_orchestration_policies/pipeline_auditor_shared_examples.rb index 89dc8287fc5fa5..961910fe57667a 100644 --- a/ee/spec/support/shared_examples/lib/security/security_orchestration_policies/pipeline_auditor_shared_examples.rb +++ b/ee/spec/support/shared_examples/lib/security/security_orchestration_policies/pipeline_auditor_shared_examples.rb @@ -47,7 +47,7 @@ merge_request&.target_branch_ref, project).and_return(active_scan_execution_policy_names) allow(security_orchestration_policy_configuration).to receive(:active_pipeline_execution_policy_names) - .and_return(active_pipeline_execution_policy_names) + .and_return(active_pipeline_execution_policy_names) end context 'when there are no active policies' do @@ -195,7 +195,7 @@ allow(inherited_security_orchestration_policy_configuration) .to receive(:active_pipeline_execution_policy_names) - .and_return(inherited_active_pipeline_execution_policy_names) + .and_return(inherited_active_pipeline_execution_policy_names) end context 'when the merge_request is present' do -- GitLab From 26c4514311d2e9ad10ef2282d0868552a3cd6e05 Mon Sep 17 00:00:00 2001 From: mc_rocha Date: Mon, 14 Jul 2025 12:44:05 -0400 Subject: [PATCH 6/7] Fix undercoverage error --- .../pipeline_auditor_spec.rb | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 ee/spec/lib/security/security_orchestration_policies/pipeline_auditor_spec.rb diff --git a/ee/spec/lib/security/security_orchestration_policies/pipeline_auditor_spec.rb b/ee/spec/lib/security/security_orchestration_policies/pipeline_auditor_spec.rb new file mode 100644 index 00000000000000..fe6d6549310813 --- /dev/null +++ b/ee/spec/lib/security/security_orchestration_policies/pipeline_auditor_spec.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Security::SecurityOrchestrationPolicies::PipelineAuditor, feature_category: :security_policy_management do + let_it_be(:auditor) { described_class.new(pipeline: build(:ci_pipeline)) } + + describe '#event_name' do + it 'raises a NoMethodError with custom message' do + expect do + auditor.send(:event_name) + end.to raise_error(NoMethodError, "#{described_class} must implement the method event_name") + end + end + + describe '#event_message' do + it 'raises a NoMethodError with custom message' do + expect do + auditor.send(:event_message) + end.to raise_error(NoMethodError, "#{described_class} must implement the method event_message") + end + end +end -- GitLab From 9ede0b8296665aba82299ed8e929f8bb98027732 Mon Sep 17 00:00:00 2001 From: mc_rocha Date: Mon, 14 Jul 2025 13:49:01 -0400 Subject: [PATCH 7/7] Apply maintainer suggestion --- .../security/pipeline_execution_policy.rb | 11 +++++-- .../pipeline_auditor.rb | 2 +- ...orchestration_policy_configuration_spec.rb | 32 +++++++++++++++---- 3 files changed, 35 insertions(+), 10 deletions(-) diff --git a/ee/app/models/concerns/security/pipeline_execution_policy.rb b/ee/app/models/concerns/security/pipeline_execution_policy.rb index e8106ede1f9c45..51d9726d216e89 100644 --- a/ee/app/models/concerns/security/pipeline_execution_policy.rb +++ b/ee/app/models/concerns/security/pipeline_execution_policy.rb @@ -8,8 +8,15 @@ def active_pipeline_execution_policies pipeline_execution_policy.select { |config| config[:enabled] }.first(pipeline_execution_policy_limit) end - def active_pipeline_execution_policy_names - active_pipeline_execution_policies.pluck(:name) # rubocop:disable Database/AvoidUsingPluckWithoutLimit -- not an ActiveRecord model and active_pipeline_execution_policies has limit + def active_pipeline_execution_policy_names(project) + policy_scope_checker = ::Security::SecurityOrchestrationPolicies::PolicyScopeChecker.new(project: project) + + applicable_policies = active_pipeline_execution_policies + .select { |policy| policy_scope_checker.policy_applicable?(policy) } + + return [] if applicable_policies.empty? + + applicable_policies.pluck(:name) # rubocop:disable Database/AvoidUsingPluckWithoutLimit -- not an ActiveRecord model and active_pipeline_execution_policies has limit end def pipeline_execution_policy diff --git a/ee/lib/security/security_orchestration_policies/pipeline_auditor.rb b/ee/lib/security/security_orchestration_policies/pipeline_auditor.rb index 7facd657700e56..2fd6e35a0dd79e 100644 --- a/ee/lib/security/security_orchestration_policies/pipeline_auditor.rb +++ b/ee/lib/security/security_orchestration_policies/pipeline_auditor.rb @@ -81,7 +81,7 @@ def active_execution_policies(policy_configuration) [] end - active_peps_for_policy = policy_configuration.active_pipeline_execution_policy_names || [] + active_peps_for_policy = policy_configuration.active_pipeline_execution_policy_names(project) || [] [active_peps_for_policy, active_seps_for_policy] end diff --git a/ee/spec/models/security/orchestration_policy_configuration_spec.rb b/ee/spec/models/security/orchestration_policy_configuration_spec.rb index fb7308f993125f..8f564639b19808 100644 --- a/ee/spec/models/security/orchestration_policy_configuration_spec.rb +++ b/ee/spec/models/security/orchestration_policy_configuration_spec.rb @@ -3713,21 +3713,39 @@ end describe '#active_pipeline_execution_policy_names' do + include_context 'for policies with pipeline and scheduled rules' + let(:policy_yaml) { fixture_file('security_orchestration.yml', dir: 'ee') } - subject(:active_pipeline_execution_policy_names) { security_orchestration_policy_configuration.active_pipeline_execution_policy_names } + subject(:active_pipeline_execution_policy_names) { security_orchestration_policy_configuration.active_pipeline_execution_policy_names(project) } before do allow(security_policy_management_project).to receive(:repository).and_return(repository) allow(repository).to receive(:blob_data_at).with(default_branch, Security::OrchestrationPolicyConfiguration::POLICY_PATH).and_return(policy_yaml) end - it 'returns active pipeline execution policy names' do - expect(active_pipeline_execution_policy_names).to contain_exactly('Run custom pipeline configuration', - 'Second pipeline execution policy', - 'Third pipeline execution policy', - 'Fourth pipeline execution policy', - 'Fifth pipeline execution policy') + context 'when policies are applicable to the project' do + it 'returns active pipeline execution policy names' do + expect(active_pipeline_execution_policy_names).to contain_exactly('Run custom pipeline configuration', + 'Second pipeline execution policy', + 'Third pipeline execution policy', + 'Fourth pipeline execution policy', + 'Fifth pipeline execution policy') + end + end + + context 'when policies are not applicable to the project' do + let(:policy_scope_checker) { instance_double(Security::SecurityOrchestrationPolicies::PolicyScopeChecker) } + + before do + allow(Security::SecurityOrchestrationPolicies::PolicyScopeChecker).to receive(:new) + .with(project: project).and_return(policy_scope_checker) + allow(policy_scope_checker).to receive(:policy_applicable?).and_return(false) + end + + it 'returns active pipeline execution policy names' do + expect(active_pipeline_execution_policy_names).to be_empty + end end end -- GitLab