diff --git a/doc/user/application_security/policies/scan-result-policies.md b/doc/user/application_security/policies/scan-result-policies.md index 51d5dff90b2bb25b7288bd00c5f6e89741a6d7a7..e666457fd9b1a17a8b3baf61eec4504df724011d 100644 --- a/doc/user/application_security/policies/scan-result-policies.md +++ b/doc/user/application_security/policies/scan-result-policies.md @@ -31,7 +31,7 @@ The following video gives you an overview of GitLab scan result policies: - You must add the respective [security scanning tools](../index.md#application-coverage). Otherwise, scan result policies do not have any effect. -- The maximum number of scan result policies is five per security policy project. +- The maximum number of scan result policies is five per security policy project. - Each policy can have a maximum of five rules. - All configured scanners must be present in the merge request's latest pipeline. If not, approvals are required even if some vulnerability criteria have not been met. - Scan result policies evaluate findings and determine approval requirements based on the job artifact reports published in a completed pipeline. However, scan result policies do not check the integrity or authenticity of the scan results generated in the artifact reports. @@ -302,8 +302,11 @@ actions: ### Scope of scan result policy comparison -- To determine when approval is required on a merge request, we compare the latest completed pipelines for each supported pipeline source for the source and target branch (for example, `feature`/`main`). This ensures the most comprehensive evaluation of scan results. -- We compare findings from the latest completed pipelines that ran on `HEAD` of the source and target branch. +- To determine when approval is required on a merge request, we compare completed pipelines for each supported pipeline source for the source and target branch (for example, `feature`/`main`). This ensures the most comprehensive evaluation of scan results. +- For the source branch, the comparison pipeline is its latest completed `HEAD` pipeline. +- For the target branch, the comparison pipeline differs for `license_finding` rules: + - For `license_finding` rules, we compare to a common ancestor's latest completed pipeline. + - For all other rules, we compare to the target branch's latest completed `HEAD` pipeline. - Scan result policies considers all supported pipeline sources (based on the [`CI_PIPELINE_SOURCE` variable](../../../ci/variables/predefined_variables.md)) when comparing results from both the source and target branches when determining if a merge request requires approval. Pipeline sources `webide` and `parent_pipeline` are not supported. ### Accepting risk and ignoring vulnerabilities in future merge requests diff --git a/ee/app/models/ee/merge_request.rb b/ee/app/models/ee/merge_request.rb index 6a31b467d9022b100cf412f2fae31391318f4718..a7854fae1f5c35b9a74e58aa5fe5ec76dc05c46b 100644 --- a/ee/app/models/ee/merge_request.rb +++ b/ee/app/models/ee/merge_request.rb @@ -13,12 +13,8 @@ module MergeRequest ::Ci::CompareMetricsReportsService => ->(_project) { true }, ::Ci::CompareCodequalityReportsService => ->(_project) { true }, ::Ci::CompareSecurityReportsService => ->(_project) { true }, - ::Ci::CompareLicenseScanningReportsCollapsedService => ->(project) do - ::Feature.enabled?(:scan_result_policy_license_scanning_merge_base_pipeline, project) - end, - ::Ci::CompareLicenseScanningReportsService => ->(project) do - ::Feature.enabled?(:scan_result_policy_license_scanning_merge_base_pipeline, project) - end + ::Ci::CompareLicenseScanningReportsCollapsedService => ->(_project) { true }, + ::Ci::CompareLicenseScanningReportsService => ->(_project) { true } }.freeze MAX_CHECKED_PIPELINES_FOR_SECURITY_REPORT_COMPARISON = 10 diff --git a/ee/app/services/security/sync_license_scanning_rules_service.rb b/ee/app/services/security/sync_license_scanning_rules_service.rb index 1bf1f7b6f2805e3e28042929a8f738d5f086a381..f05dc5a42ea03f31f18ddb10567efa5b37fed204 100644 --- a/ee/app/services/security/sync_license_scanning_rules_service.rb +++ b/ee/app/services/security/sync_license_scanning_rules_service.rb @@ -136,11 +136,7 @@ def target_branch_report(merge_request) end def target_branch_pipeline(merge_request) - if Feature.enabled?(:scan_result_policy_license_scanning_merge_base_pipeline, project) - merge_request.latest_comparison_pipeline_with_sbom_reports - else - merge_request.latest_finished_target_branch_pipeline_for_scan_result_policy - end + merge_request.latest_comparison_pipeline_with_sbom_reports end def new_dependency_names(target_branch_report) diff --git a/ee/config/feature_flags/development/scan_result_policy_license_scanning_merge_base_pipeline.yml b/ee/config/feature_flags/development/scan_result_policy_license_scanning_merge_base_pipeline.yml deleted file mode 100644 index c4ad1b544e7c9e1224d0a5c0bc16fae18226fb65..0000000000000000000000000000000000000000 --- a/ee/config/feature_flags/development/scan_result_policy_license_scanning_merge_base_pipeline.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: scan_result_policy_license_scanning_merge_base_pipeline -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/136695 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/435655 -milestone: '16.8' -type: development -group: group::security policies -default_enabled: false diff --git a/ee/spec/models/merge_request_spec.rb b/ee/spec/models/merge_request_spec.rb index 05451fe01feb3edb6dfd33c0ab9dae09b4827652..b4cb4fd3ade03ce93a931e1f1601e562126c5519 100644 --- a/ee/spec/models/merge_request_spec.rb +++ b/ee/spec/models/merge_request_spec.rb @@ -1410,28 +1410,12 @@ let(:service_class) { ::Ci::CompareLicenseScanningReportsCollapsedService } it { is_expected.to eq(true) } - - context 'with feature disabled' do - before do - stub_feature_flags(scan_result_policy_license_scanning_merge_base_pipeline: false) - end - - it { is_expected.to eq(false) } - end end context 'when service class is Ci::CompareLicenseScanningReportsService' do let(:service_class) { ::Ci::CompareLicenseScanningReportsService } it { is_expected.to eq(true) } - - context 'with feature disabled' do - before do - stub_feature_flags(scan_result_policy_license_scanning_merge_base_pipeline: false) - end - - it { is_expected.to eq(false) } - end end context 'when service class is different' do diff --git a/ee/spec/services/security/sync_license_scanning_rules_service_spec.rb b/ee/spec/services/security/sync_license_scanning_rules_service_spec.rb index f43c77536314e5f1710ea2cd4a0cf08bf458dc17..b9c387826a1b83106374556e19b342ddee3d1dbb 100644 --- a/ee/spec/services/security/sync_license_scanning_rules_service_spec.rb +++ b/ee/spec/services/security/sync_license_scanning_rules_service_spec.rb @@ -129,19 +129,6 @@ end it_behaves_like 'triggers policy bot comment', :license_scanning, true, requires_approval: true - - context 'with feature disabled' do - before do - stub_feature_flags(scan_result_policy_license_scanning_merge_base_pipeline: false) - end - - it 'uses the most recent security orchestration pipeline for comparison' do - expect(::Gitlab::LicenseScanning).to receive(:scanner_for_pipeline).with(project, - pipeline_without_sbom).and_return(sbom_scanner) - - execute - end - end end context 'with merge base pipeline and SBOM report present' do