Security Approvals required even when no new vulnerabilities have been found
Summary
Merge requests not introducing any new vulnerability (Security Widget is green) still require the approval of someone in the Vulnerability-Check
approval group.
Steps to reproduce
(How one can reproduce the issue - this is very important)
- Create a new project with vulnerabilities
- Create a Merge Request changing text files (no new vulnerabilities)
Example Project
What is the current bug behavior?
-
Requires approval from Vulnerability-Check.
andSecurity scanning detected no new vulnerabilities
.
What is the expected correct behavior?
No approval required.
Relevant logs and/or screenshots
(Paste any relevant logs - please use code blocks (```) to format console output, logs, and code as it's tough to read otherwise.)
Output of checks
N/A
Results of GitLab environment info
N/A
Results of GitLab application Check
N/A
Possible fixes
Since the pipeline Security tab has some findings, I wonder if we compare correctly. Maybe only these findings are used.
Implementation plan
Security::SyncReportsToApprovalRulesService
(https://gitlab.com/gitlab-org/gitlab/blob/master/ee/app/services/security/sync_reports_to_approval_rules_service.rb#L46) is taking care of syncing security report results to approval rules, we need to take a look at sync_vulnerability_rules
and reports.violates_default_policy?
methods defined in https://gitlab.com/gitlab-org/gitlab/blob/master/ee/lib/gitlab/ci/reports/security/reports.rb#L22. It looks like the logic that is defined there is causing this bug.
-
backend Investigate and fix Security::SyncReportsToApprovalRulesService
(https://gitlab.com/gitlab-org/gitlab/blob/master/ee/app/services/security/sync_reports_to_approval_rules_service.rb#L46) andGitlab::Ci::Reports::Security::Reports#violates_default_policy?
method (https://gitlab.com/gitlab-org/gitlab/blob/master/ee/lib/gitlab/ci/reports/security/reports.rb#L22) to verify if there are any new vulnerabilities found
/cc @thiagocsf @lkerr @matt_wilson for prioritization