Security approval in merge requests MVC
In order to allow teams to focus on the most critical security items and avoid things being lost due to volume of results, GitLab now allows customers to automatically approve specifically identified merge requests if there are no vulnerabilities introduced of a specified severity. This will enable a workflow that would allow Security teams to be involved in Merge Request Approval workflow without having to approve every single Merge Request.
Problem to solve
Security checks are performed during the pipeline execution, and reports are available in the merge request widget. GitLab is able to determine if new vulnerabilities have been introduced into the code because of the specific changes of the feature branch.
Our security paradigm states that we don't want to make a pipeline fail and block the entire development process if some vulnerability is found. They could be false positive, or just something that is not more important than the development velocity.
On the other side, we're getting a lot of feedback that unsecure code should not be allowed unless previously approved by the security team. This is considered a strict requirement by some customer, and also a must-have by analysts. Security teams don't want to be involved in each and every merge request, but only if security flaws have been found.
GitLab recently added the feature of Merge Request Approval rules (including adding multiple approval rules) that would enable approval being required by specific individuals on a merge request. The pain point is that in practice, a Security Team would have to review every single merge request, and what they are interested in doing is setting a threshold for when a review is required.
We strongly believe that our security paradigm is valid, but we should consider something that allows companies to deal with their compliance requirements and ensure they are able to use GitLab.
- Sam, Security Analyst, https://design.gitlab.com/research/personas#persona-sam
We recently released multiple approval rules, and we can leverage this feature to build some advanced logic on top of it.
We can use multiple merge request approval rules to create an MVC that auto-approves specifically identified Approvals if there are no vulnerabilities introduced of a certain severity. This will enable a workflow that would allow Security teams to be involved in Merge Request Approval without having to approve every single Merge Request.
- Default to displaying a
Vulnerability-CheckMerge Request Approval rule with no Approvers added with a tool-tip to point you to documentation on what the Merge Request Approval rule provides
- Administrators can update the number of Approvals required to activate the rule and add appropriate approvers using the standard Merge Request approval rule UI
- Merge requests themselves utilize existing approval rules UI
- The removing of the required approval is performed by lowering the
approvals_requiredto zero for the Vulnerability-Check approval rule when no
Unknownvulnerabilities are introduced
- Vulnerability Dismissal does not affect whether
approvals_requiredis adjusted to zero
- Usage ping with total number of projects with a
Vulnerability-Checkproject rule that exists with an approval count greater than 0
This avoids the confusing "Double Set of Approvers" or "Special Approvers" that was mentioned in the discovery.
Out of Scope
- Compliance focused reporting on merge approvals
- Selection Vulnerability Severity level
- Adding auto-approval to non-standard (in this case
We should be careful to communicate to users that one
Merge request approvals section checkbox might lead to unexpected behavior in this workflow. Allowing users to adjust Approvals in specific Merge Requests could enable un-authorized adjustment to approvers to subvert the workflow.
backend refactoring of existing Approval Rules implementation to support new rule types https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/13036
backend addition of
backend work nearly feature-complete, ongoing discussion on implementation and refactoring https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/13109
backend work to fix issue introduced where default rule_type was being overridden https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/30575
backend frontend MR to remove feature flag, activating feature by default https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/15087
Add default "security approver rule" row to UI(decision was made but missed in implementation, see below)
unknown-severity in vulnerability checks
undefined-severity in vulnerability checks
- "Dismissing feedback" will not be disabled when Security Approvals are enabled. Many edgecases need to be considered still
- Remove MR approval rules when project approval rule is deleted
- Do not create approval rules for open MRs when project approval rule is added (deferred due to complexity)
- The existing validation preventing override rules from being less strict than project rules will be bypassed and not apply to
We will introduce a
approval_project_rulesto reduce ~"technical debt" and better support complex rule types later
- We will not be shipping the placeholder row in the UI as this was missed and won't make the cutoff for %12.1. After research in ux-research#206 (closed) this should be reconsidered