License approval policies work incorrectly for MRs with both branch and MR pipelines
Summary
When an MR has both branch and MR pipeline, with security scan in one of the pipelines, the merge request approval policies are enforced incorrectly even if the conditions are not met.
This happens because of a race condition between Security::UnenforceablePolicyRulesPipelineNotificationWorker
& Ci::SyncReportsToReportApprovalRulesWorker
. When an MR's head pipeline is set to the one without the security reports, Security::UnenforceablePolicyRulesPipelineNotificationWorker
picks that and generates a violation comment considering that the pipeline is missing a security scan. When the pipeline with security reports is not the head_pipeline
, pipeline.merge_requests_as_head_pipeline
returns empty array, so the approvals are not removed resulting in false approvals.
Steps to reproduce
- Create a project with both branch & MR pipelines and create a dependency scan in MR pipeline
- Create a license approval policy to require approval on newly detected licenses
- Create an MR with changes in README file and observe that the approval is required
Example Project
https://gitlab.com/gitlab-org/govern/demos/sandbox/alan/gisolf-verification/slow-mr
What is the current bug behavior?
- False approvals are enforced
What is the expected correct behavior?
- Approvals should not be enforced
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
- Update
Security::UnenforceablePolicyRulesPipelineNotificationWorker
to useSecurity::RelatedPipelinesFinder
instead ofmerge_request.actual_head_pipeline
- Update
Security::SyncLicenseScanningRulesService
to consider all pipelines that ran on the recent commit instead ofpipeline.merge_requests_as_head_pipeline
Verification Steps
- Create a project with both merge request and branch pipelines with MR pipeline taking a longer time (add sleep). Example project: https://gitlab.com/gitlab-org/govern/demos/sandbox/alan/gisolf-verification/slow-mr
- Create license approval policy and a scan execution policy to add dependency scanning scan to all pipeline with policy like:
name: DS Scan
description: ''
enabled: true
actions:
- scan: dependency_scanning
rules:
- type: pipeline
branch_type: all
name: License policy
description: ''
enabled: true
actions:
- type: require_approval
approvals_required: 1
group_approvers_ids:
- 22
rules:
- type: license_finding
match_on_inclusion: false
license_types:
- MIT License
- unknown
license_states:
- newly_detected
branch_type: protected
approval_settings:
block_branch_modification: false
prevent_pushing_and_force_pushing: false
- Create a MR that updates the readme and verify if dependency_scanning job is added to the branch pipeline and wait for both the pipelines to complete
- Verify that the approval is not enforced and also no comment from the policy violation bot