Skip to content

Hide resolve merge request for new findings

Lorenz van Herwaarden requested to merge hide-resolve-merge-request-for-new-findings-fe into master

What does this MR do and why?

Related #428890 (closed)

This MR makes sure the "Resolve with merge request" button is only visible in the finding modal when the finding has a vulnerability which is present on the default branch. The related issue describes in detail why we're making this change. In essence: when opening a finding modal, which does not have a vulnerability that is present on the default branch, this means it's a newly identified finding (introduced by an open MR). It does not make sense to create a MR to resolve this finding because its target branch will be the open MR itself.

note: this is only the case for the new vulnerability finding modal, which (in case of the MR security report widget), is only available behind the feature flag standalone_finding_modal_merge_request_widget

Screenshots or screen recordings

Screenshots are required for UI changes, and strongly recommended for all other merge requests.

Scenario Before After
Scenario 1 Screenshot_2023-11-22_at_15.59.31 Screenshot_2023-11-22_at_15.57.09
Scenario 2 Screenshot_2023-11-22_at_15.59.56 Screenshot_2023-11-22_at_15.57.30
Scenario 3 Screenshot_2023-11-22_at_15.59.56 Screenshot_2023-11-22_at_15.59.56

How to set up and validate locally

Setup

  1. You need an EE license
  2. You need to have runners enabled (See $2408961 for setting up a runner)
  3. Import https://gitlab.com/gitlab-org/govern/demos/sandbox/minac/test-remediations
  4. Do not run a pipeline yet on master!
  5. Enable feature flag: echo "Feature.enable(:standalone_finding_modal_merge_request_widget)" | gdk rails c
  6. In the imported test-remediations project
    1. go to reports/sast.json and remove both entries in vulnerabilities, leaving effectively an empty array
    2. commit and push this to master
    3. now, in reports/sast.json, add the removed vulnerabilities back again, but commit and push to a new branch, and create a merge request.

To validate properly, we want to test a couple scenarios:

  1. Newly introduced finding without an attached vulnerability
  2. Newly introduced finding with an attached vulnerability
  3. Finding which has made its way onto the default branch, i.e. vulnerability.presentOnDefaultBranch is true

Validate scenario 1

  1. Go to the created merge request
  2. in the mr security report widget, click on the only critical finding
  3. This is a new finding (no attached vulnerability and not present on default branch), but with a remediation
  4. Validate that is a dropdown button to Download patch to resolve" and "Create issue", but no "Resolve with merge request"

Validate scenario 2

  1. In the modal of that same finding, click "Create issue"
  2. This creates an attached vulnerability for the finding
  3. You're redirected to that issue, go back to the merge request and open the finding in the modal again
  4. Validate that now, there's only a button to "Download patch to resolve", still no "Resolve with merge request"

Validate scenario 3

  1. Merge this merge request
  2. Go to the general pipelines page and click on the latest pipeline and go go to the security tab
  3. Click on the critical finding
  4. Validate that now you see a dropdown button with the options to "Resolve with merge request" and "Download patch to resolve"

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Lorenz van Herwaarden

Merge request reports