Skip to content

Display in MR if security report is outdated

Problem to solve

Security reports in the Merge Request widget can be wrong because the target branch has outdated reports.

Further details

When I create a branch from master, commits a few changes like in the README.md file, and create a Merge Request for that, the report generated in the Security widget can report misleading information. If the last pipeline for master didn't detect anything for some reason, the widget will report new vulnerabilities introduced by my change, whereas I just updated a README file. It doesn't make sense, and it's confusing for the users.

This situation is unfortunately common for these reasons:

  • The secure jobs on the master branch failed (runner host rebooted, lost network connection, ...).
  • Some advisory was not available when the last check on master was launched (could be something added to Gemnasium DB, or a new SAST rule).
  • Artifacts expired on master
  • master doesn't have any report yet.

According to https://gitlab.com/gitlab-org/gitlab-ee/issues/5106, the diff is made between the base commit of the target branch, so having new commits in master won't affect the reports.

I think the last point is already covered, and we won't display any "introduced" or "fixed" notion if we don't have a report to compare to (@leipert @samdbeckham can you confirm please?)

Proposal

  • Make sure we have the impacted files in the security reports
  • Compare these files with the changed files of the commit
  • Indicate in the MR that security results are either fresh or outdated and indicate why & how to update

If we find a difference, that probably means the target branch security status is outdated, and a new pipeline must be run for the base commit.

Ideally, we should have something in the UI indicating that the target branch status is currently updating, with a "refresh" option once the new data is available.

Design

First iteration
Screen_Shot_2019-08-27_at_8.39.22_AM
Edge case Messaging
If an MR branch is behind the target branch and security reports are out of date Security report is out of date. Please incorporate latest changes from [target-branch-name].

Future iterations:

Ideal experience
Screen_Shot_2019-08-27_at_8.39.17_AM
We link the user to the last pipeline run for the target branch. instance/group/project/pipelines/pipeline_id

What does success look like, and how can we measure that?

Security widget always reports relevant and fresh data.

Documentation

Update DAST documentation page to describe the new MR widget behavior that security results will be indicated as either fresh or outdated with a description of why and how to update. Include screenshots where applicable to highlight the new behaviors.

Links / references

Automation issue: #13749 (closed)

Scope

  • inform a user that the target branch pipeline security report is outdated (generated more than 1 week ago)
  • inform a user that the target branch pipeline security report does not exist (there are many reasons this could be the case)
  • compare the branch security report to the base commit target security report, not the latest target security report

Changes required

  • BE will return vulnerability reports for the base commit target pipelines, not the latest
  • FE will make a new api call to determine if the security report is out of date
  • BE will implement a new api endpoint that determines if a branch security report is out of date
  • FE will show appropriate message to the user based on security report out of date test outcome

Old version:

Following #3995 (closed), we now have a nice diff view of security issues. The current implementation can be error prone for users, with misleading information. Reminder: SAST is running several tests, including on the "old" (yet still maintained) Gemnasium DB. Once an advisory is published in the DB, it will be immediately available for all SAST tests. That means we could report a new security issue, apparently introduced in the MR. There's no way for the user to know if it was the case, except checking the dependency files, and running the SAST job on the destination branch again.

I suggest to:

  • Determine a way to trigger SAST jobs retries
  • When a new advisory is added to the DB trigger necessary SAST jobs:
    • all open MRs using SAST
    • all destination branches
  • When a new MR is created, run SAST on destination first, to ensure freshness.

This tasks is depending of the integration of the DB directly in gitlab-ee, otherwise we won't be able to trigger anything. There is no issue for that step yet.

/cc @bikebilly

Edited by Andy Volpe