Dismissed vulnerabilities showing up in Merge Request Security Widget
Summary
Follow-up of the discussion in this issue, it happens sometimes that previously Dismissed
findings appear in the Security Widget. They're not even strike through.
Steps to reproduce
Apparently, location changes in code can trigger this kind of behavior.
Example Project
!60385 (merged) (Until expiration of artifacts)
What is the current bug behavior?
Vulnerabilities dismissed a year ago should not appear in the Security Widget. They can't be fixed
or new
and dismissed
at the same time, it just doesn't make sense.
It would make sense if I took the action to dismiss them in the context of the MR, but this is different.
What is the expected correct behavior?
Previously dismissed vulnerabilities not showing up.
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)
Possible fixes
Initial investigation of the problem points to the following root cause:
This is related to project_fingerprint
and Feedback
model. We are going to get rid of Feedback
model as part of this &5629 (closed). As mentioned here #290238 (comment 636602020), this is also dependent on project_fingerprint
.
Implementation plan
- Adjust
lib/gitlab/ci/reports/security/vulnerability_reports_comparer.rb
to:- Gather UUIDs of
added
andfixed
vulnerabilities - Essentially perform a
SELECT FROM vulnerability_feedback WHERE finding_uuid IN (list of UUIDs here) AND feedback_type = 0
- Filter out matches
- 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.