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.
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
masterbranch failed (runner host rebooted, lost network connection, ...).
- Some advisory was not available when the last check on
masterwas launched (could be something added to Gemnasium DB, or a new SAST rule).
- Artifacts expired on
masterdoesn'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.
- 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.
|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].|
|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.
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
- 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
- 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
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.