Scan result policy does not work when a user does not have permission to update project approval rule

Summary

Scan Result Policies are not applied on MRs when author of the policy (last user that commited changes to security policy project) does not have permissions to edit_approval_rule (a user with Maintainer role in the project).

Steps to reproduce

  1. Create a project and enable security scans for this project (like include Security/Container-Scanning.gitlab-ci.yml template and use nginx:1.18.0 with DOCKER_IMAGE as a variable)
  2. Create a security policy project for this project
  3. Add new user as a member of the security policy project created in step 1. with Developer role
  4. As the user created in step 3. create new Scan Result Policy and merge the MR to apply the policy
  5. In project created in step 1. create new MR that will change the DOCKER_IMAGE variable set in gitlab-ci.yml file to something else (like nginx:1.17.0)
  6. Observer applied approval rules in the MR

Example Project

What is the current bug behavior?

Approval Rule created from Scan Result Policy is not applied.

What is the expected correct behavior?

Approval Rule created from Scan Result Policy is applied.

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)

Explanation

This happens when the user does not have permission to update an approval rule for the project, the sync from the policy to approval rules does not complete.

The current flow is:

  • Security::SyncScanPoliciesWorker ->
  • Security::SecurityOrchestrationPolicies::SyncScanResultPoliciesService ->
  • Security::ProcessScanResultPolicyWorker ->
  • Security::SecurityOrchestrationPolicies::ProcessScanResultPolicyService ->
  • ApprovalRules::CreateService (ApprovalRules::BaseService)
  • in ApprovalRules::BaseService we do can?(current_user, :edit_approval_rule, rule) ->
  • rule is the newly built (not persisted) ApprovalProjectRule from the policy,
  • if the user does not have edit_approval_rule permission, ApprovalRules::CreateService returns error, but in Security::SecurityOrchestrationPolicies::ProcessScanResultPolicyService the response is not considered which results in sync action incomplete

Possible fixes

Use security_policy_bot user when creating approval rules in database (use this bot in Security::SecurityOrchestrationPolicies::ProcessScanResultPolicyService as author) (see !103544 (merged))

  • backend Add skip_authorization param to ApprovalRules::CreateService to ignore permission check if the param is true
Edited by Sashi Kumar Kumaresan