Scan result policy requiring MR approval when no pipelines have been run when looking at Pre-Existing Vulnerabilities
Summary
When a Scan result policy is created that is only looking for Pre-Existing Vulnerabilities. Merge requests will always require approval until a scan is run on source branch.
Steps to reproduce
- Create security results policy to only looking at pre-existing vulnerabilities
- run scan on default branch
- create feature branch
- create merge into default branch
- approval will be required
Example Project
here (will need admin)
What is the current bug behavior?
- Approval is required even if no pre-existing security vulnerabilities are present
What is the expected correct behavior?
Approval would not be required
Relevant logs and/or screenshots
Output of checks
This bug happens on GitLab.com
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)
Implementation Plan
-
backend Introduce a new worker Security::ScanResultPolicies::SyncPreemptiveApprovalsWorker
that gets triggered whenever a MR is refreshed (MergeRequests::RefreshService
) and invoke the same worker fromMergeRequests::AfterCreateService
andSecurity::SecurityOrchestrationPolicies::SyncOpenedMergeRequestsService
-
backend Within the worker, extract the pre-existing vulnerability states approval rules and update approvals based on the results.
def perform(approval_rules)
rules_with_preexisting_states = approval_rules.reject do |rule|
include_newly_detected?(rule)
end
update_scan_finding_rules_with_preexisting_states(rules_with_preexisting_states)
end
def update_scan_finding_rules_with_preexisting_states(approval_rules)
violated_rules, unviolated_rules = approval_rules.partition do |rule|
preexisting_findings_count_violated?(rule)
end
update_required_approvals(violated_rules, unviolated_rules)
violations.add(violated_rules)
end
def preexisting_findings_count_violated?(approval_rule)
vulnerabilities_count = vulnerabilities_count_for_uuids(approval_rule)
vulnerabilities_count[:exceeded_allowed_count]
end
-
backend Create new worker Security::ScanResultPolicies::SyncTargetBranchMergeRequestsWorker
similar to this and invoke it fromSecurity::StoreScansService
so that the approvals are synced after a vulnerability is introduced in the default branch
Verification Steps
-
Create a project with security vulnerabilities from a security scan -
Create a scan result policy to require approval on pre-existing vulnerabilities
name: Pre-Existing Vulnerabilities
description: ''
enabled: true
actions:
- type: require_approval
approvals_required: 1
role_approvers:
- maintainer
rules:
- type: scan_finding
scanners: []
vulnerabilities_allowed: 0
severity_levels: []
vulnerability_states:
- detected
- confirmed
- dismissed
- resolved
branch_type: protected
approval_settings:
block_branch_modification: false
prevent_pushing_and_force_pushing: false
-
Enable security_policies_sync_preexisting_state
feature flag for the project -
Create an MR with a commit to skip CI by adding [skip ci]
prefix to commit -
Verify that the MR requires approval from the created policy -
Create an another MR without skipping the CI and verify that the approval is required even before the pipeline is completed, also verify the bot message.
Edited by Sashi Kumar Kumaresan