Skip to content

Project Export Approval Rule Filter

Summary

When creating a project export, by default all approval rules are included in the export. In most cases, that is okay, but for security policies, it can be an issue because scan result policy rules would be included when importing a project elsewhere, causing an invalid or duplicate rule to show up in the MR UI widget.

In general, when you import a project into a new group, that group may not have the same security policy setup, so the rule won't make sense being there and could cause weird bugs.

Steps to reproduce

  1. Create a new group with at least 1 project (TEST_PROJECT) inside.
  2. Create a group level scan result policy that requires an approval if a high or critical vulnerability is detected in a SAST scan.
  3. Export the TEST_PROJECT, and import it into a new project with a different name (TEST_PROJECT_1).
  4. Add code to trigger the SAST scan with high/critical vulnerabilities in TEST_PROJECT_1, commit the changes, and create an MR.
  5. You'll see in the MR UI that a duplicate approval rule will show up either as optional or invalid.

Example Project

What is the current bug behavior?

A duplicate or invalid approval rule shows up in the MR view because it is either adding an approval rule to what was exported or referencing an approval rule that no longer exists, respectively.

What is the expected correct behavior?

We should only see the single security approval rule created in the MR UI. The export should probably not include security approval rules since you don't know what the setup of the group will be wherever the project is imported. image

Relevant logs and/or screenshots

Output of checks

Results of GitLab environment info

Expand for output related to GitLab environment info 

(For installations with omnibus-gitlab package run and paste the output of:
`sudo gitlab-rake gitlab:env:info`)

(For installations from source run and paste the output of:
`sudo -u git -H bundle exec rake gitlab:env:info RAILS_ENV=production`)

Results of GitLab application Check

Expand for output related to the GitLab application check 
(For installations with omnibus-gitlab package run and paste the output of:
sudo gitlab-rake gitlab:check SANITIZE=true)


(For installations from source run and paste the output of:
sudo -u git -H bundle exec rake gitlab:check RAILS_ENV=production SANITIZE=true)


(we will only investigate if the tests are passing)

Possible fixes

We can prevent importing ApprovalProjectRule if it belongs to a security policy configuration and use the orchestration_policy_idx field to identify this.

diff --git a/ee/app/models/approval_project_rule.rb b/ee/app/models/approval_project_rule.rb
index 0f01c8fbbc94..0102e58012af 100644
--- a/ee/app/models/approval_project_rule.rb
+++ b/ee/app/models/approval_project_rule.rb
@@ -3,6 +3,7 @@
 class ApprovalProjectRule < ApplicationRecord
   include ApprovalRuleLike
   include Auditable
+  include Importable
   extend ::Gitlab::Utils::Override
 
   UNSUPPORTED_SCANNER = 'cluster_image_scanning'
@@ -49,6 +50,7 @@ class ApprovalProjectRule < ApplicationRecord
   validates :severity_levels, inclusion: { in: ::Enums::Vulnerability.severity_levels.keys }
   validates :vulnerability_states, inclusion: { in: APPROVAL_VULNERABILITY_STATES.keys }
   validates :protected_branches, presence: true, if: -> { scan_finding? && !applies_to_all_protected_branches? }
+  validates :orchestration_policy_idx, inclusion: { in: [nil] }, if: :importing?
 
   delegate :vulnerability_attributes, to: :scan_result_policy_read, allow_nil: true

Workarounds

For self-managed users, the following rails console script can be used to address the issue caused by project transfer to another group:

Project.joins(:approval_rules).where(approval_rules: { report_type: %i[scan_finding license_scanning] }).where.not(approval_rules: { security_orchestration_policy_configuration_id: nil }).find_in_batches.flat_map { |batch| batch.map { |project| [project, project.approval_rules.where(report_type: %i[scan_finding license_scanning]).pluck(:security_orchestration_policy_configuration_id).uniq] }.uniq.map { |project, configuration_ids| [project, configuration_ids - project.all_security_orchestration_policy_configurations.pluck(:id)] }.select { |_project, configuration_ids| configuration_ids.any? } }.each { |project, configuration_ids| Security::OrchestrationPolicyConfiguration.where(id: configuration_ids).each { |configuration| configuration.delete_scan_finding_rules_for_project(project.id) }; Security::ScanResultPolicies::SyncProjectWorker.perform_async(project.id) }

The script will check for orphaned approval rules and delete them for all the projects, no need to replace anything. Simply copy/paste to the rails console.

Edited by Martin Čavoj