Blockers to the usage of Merge Request Security Approvals

Follow-up of this discussion on a closed issue (which was a priority1 / severity1 but the underlying problems were not fixed):

The Merge Request Security Widget is failing in some edge cases, which makes it 100% reliable, and will prevent from using the Merge Request Security Approvals, especially for the upcoming Security Champions Program.

As explained in our doc:

GitLab checks the SAST report, compares the found vulnerabilities between the source and target branches.

Details of the vulnerabilities found are included in the merge request.

sast_v13_2

It means we compare the report generated in the pipeline of the branching commit with the report of the HEAD of the current branch. That way, we should be able to detect the consequences of a branch, in other words, what was introduced, and what was fixed. And that's true for all our Secure Products, not just SAST.

That's the theory. Now comes reality, with its share of edge cases.
There are multiple things that can impact the results of this comparison:

  • The reports are generated at different points in time, in a non-reproducible way. It means we can detect different things depending on WHEN we run the analyzers. If we had a new advisory to the Gemnasium DB between the original and the new report, we can "introduce" vulnerabilities in dependencies if one of them is affected. The same goes for SAST. Rules can be updated anytime, and that leads to different results. One way to fix this is to freeze the rules set when branching. We have a feature in the widget to notify the user if the original report is too old (>1 week), and invite them to run again the pipeline there.
  • Some of the reports might not have been generated in the branching commit pipeline. The wording starts to get complex. If the user doesn't read (very) carefully, they can get confused easily.
  • Some code might have been moved to a new location. GitLab is currently unable to detect and track these changes (note that this issue has been created almost 2 years ago). This leads to having the vulnerabilities on the moved coded to be "fixed" and "introduced" at the same time. If merged, they will create new Standalone Vulnerabilities in the dashboard, even though someone already dismissed them.
  • Some analyzers might not return deterministic results. Due to their nature, they can report slightly different results. Imagine a DAST scan, for example, hitting an application when some network issues occur.

/cc @thiagocsf @matt_wilson @lkerr