Do not show Findings dismissed on the default branch in the Security scanning MR widget
Why are we doing this work?
The discussion on Dismissed vulnerabilities showing up in Merge R... (#330042 - closed) yielded the following description of the desired behavior of the Security scanning MR widget
best described as follows:
- finding dismissed in default branch: always hide in MR widget
- finding dismissed on branch: show in MR widget with a badge/strikethrough
Essentially we need to hide Findings that are present on the default branch and have been dismissed and expose Findings that are not present on the default branch and have been dismissed
Steps to reproduce
See #330042 (comment 1263880687)
1st Implementation plan
Read More
- Adjust
lib/gitlab/ci/reports/security/vulnerability_reports_comparer.rb
to:- Gather UUIDs of
added
andfixed
vulnerabilities SELECT vulnerability_id INNER JOIN vulnerabilties ON vulnerabilties_occurrences.vulnerability_id = vulnerabilities.id AND vulnerabilities.present_on_default_branch = true FROM vulnerabilties_occurrences WHERE uuid IN (list of UUIDS here) AND vulnerabilities.state = 2
- Remove rows matching the above query from the result
- Gather UUIDs of
Explanation: both app/models/merge_request.rb
and ee/app/models/ee/merge_request.rb
call ee/app/services/ci/compare_security_reports_service.rb
which uses lib/gitlab/ci/reports/security/vulnerability_reports_comparer.rb
under the hood. This class has added
and fixed
methods which could be changed to filter out dismissed Vulnerabilities.
I don't think we can do it further down the stack because we use Vulnerabilities::FindingEntity
inside of Vulnerabilities::FindingReportsComparerEntity
to serialize the results and the serializer does the expose :dismissal_feedback, using: Vulnerabilities::FeedbackEntity
for us.
1st Implementation Result
Upon delivery of the first implementation in which we would take the UUID's of the vulnerabilities taken from the vulnerability scan job artifacts, and check them against known dismissed UUID's to filter them from the sets that would be presented in the MR Secuirty comparison widget.
Unfortunately that fix proved to be unsuccessful due to a separate problem with the current implementation of the vulnerability comparison report in which the UUID's of vulnerabilities detected in job artifacts do not match with the UUID's of persisted vulnerabilities in the database, meaning the prior fixes's implementation for filtering dismissed vulnerabilities will not work in all instances. Unfortunately, to reconcile all the possible UUID's for a report and then filter off the dismissed instances would be extremely expensive and prohibitive to calculate with the current architecture. As a result, we need to rework the vulnerability_reports_comparison to use persisted vulnerability information to avoid this expensive and make this filtering appropriately feasible.
This was unfortunately not an expected result of our first attempt at fixing this.
2nd Implementation Plan
Current expectation is that the implementation of Use security_findings for security MR widget re... (#390185) • Michael Becker • 17.1 • At risk will probably fix this implicitly.