Issues with new modal on MR Widget
Context
When utilizing the new standalone finding modal ee/app/assets/javascripts/security_dashboard/components/pipeline/vulnerability_finding_modal.vue
(already used from the pipeline security tab) in the MR security widget, the following 3 specs fail in qa/qa/specs/features/ee/browser_ui/10_govern/vulnerability_management_spec.rb
:
- vulnerability_management_spec.rb:74 - 'can dismiss a vulnerability with a reason from mr security widget'
- vulnerability_management_spec.rb:93 - 'can create an issue from a vulnerability from mr security widget'
- vulnerability_management_spec.rb:105 - 'can create an auto-remediation MR from mr security widget'
This standalone modal is rendered instead of the old modal (ee/app/assets/javascripts/vue_shared/security_reports/components/modal.vue
) when the standalone_finding_modal_merge_request_widget
feature flag is enabled. Implemented in this MR: Integrate standalone finding modal with MR widget (!132381 - merged) • Lorenz van Herwaarden • 16.5
Additional info
1. can dismiss a vulnerability with a reason from mr security widget
Error (only on new modal)
With a new finding found in the merge request (that does not have a vulnerability attached), clicking "Confirm dismissal" results in a 500 error in mutation securityFindingDismiss
: undefined method 'update!' for nil:NilClass\n\n vulnerability.vulnerability_read.update!
Error message
{
"errors": [
{
"message": "Internal server error: undefined method update!' for nil:NilClass\n\n vulnerability.vulnerability_read.update!(dismissal_reason: params[:dismissal_reason])\n ^^^^^^^^", "raisedAt": "/Users/lorenzvanherwaarden/gitlab-development-kit/gitlab/ee/app/services/vulnerabilities/find_or_create_from_security_finding_service.rb:90:in
block in update_existing_state_transition' \u003c-- /Users/lorenzvanherwaarden/gitlab-development-kit/gitlab/app/models/concerns/cross_database_modification.rb:92:in block in transaction' \u003c-- /Users/lorenzvanherwaarden/.asdf/installs/ruby/3.1.4/lib/ruby/gems/3.1.0/gems/activerecord-7.0.8/lib/active_record/connection_adapters/abstract/database_statements.rb:314:in
transaction' \u003c-- /Users/lorenzvanherwaarden/gitlab-development-kit/gitlab/lib/gitlab/database/load_balancing/connection_proxy.rb:127:in public_send' \u003c-- /Users/lorenzvanherwaarden/gitlab-development-kit/gitlab/lib/gitlab/database/load_balancing/connection_proxy.rb:127:in
block in write_using_load_balancer' \u003c-- /Users/lorenzvanherwaarden/gitlab-development-kit/gitlab/lib/gitlab/database/load_balancing/load_balancer.rb:137:in block in read_write' \u003c-- /Users/lorenzvanherwaarden/gitlab-development-kit/gitlab/lib/gitlab/database/load_balancing/load_balancer.rb:224:in
retry_with_backoff' \u003c-- /Users/lorenzvanherwaarden/gitlab-development-kit/gitlab/lib/gitlab/database/load_balancing/load_balancer.rb:126:in read_write' \u003c-- /Users/lorenzvanherwaarden/gitlab-development-kit/gitlab/lib/gitlab/database/load_balancing/connection_proxy.rb:126:in
write_using_load_balancer' \u003c-- /Users/lorenzvanherwaarden/gitlab-development-kit/gitlab/lib/gitlab/database/load_balancing/connection_proxy.rb:78:in transaction' \u003c-- /Users/lorenzvanherwaarden/.asdf/installs/ruby/3.1.4/lib/ruby/gems/3.1.0/gems/activerecord-7.0.8/lib/active_record/transactions.rb:209:in
transaction'"
}
]
}
Reproduce
- Go to https://staging.gitlab.com/govern-team-test/verify-413516/-/merge_requests/2
- Expand security reports widget and Click on a container scanning finding (is a new finding)
- Click "Dismiss vulnerability"
- Select a dismissal reason
- Click "Confirm dismissal"
Spec related issue
This fails because the new modal allows a finding to be dismissed with a proper dismissal reason. Hence, instead of clicking the "Dismiss with comment" button and providing the comment:
- Click the "Dismiss" button. This opens a form to supply the dismissal reason and comment
- Choose a dismissal reason and provide a comment
- Click "Confirm dismissal"
Fix dismissal reason modal based on FF (!134345 - merged) • Harsha Muralidhar • 16.8 implements a fix for the flow.
2. can create an issue from a vulnerability from mr security widget
This functionality is also available on the new modal, but the condition that shows the button is changed:
- old modal:
this.mr.createVulnerabilityFeedbackIssuePath || this.modalData?.vulnerability?.create_jira_issue_url
- new modal:
finding?.vulnerability?.userPermissions?.createVulnerabilityFeedback
With a new finding found in the merge request (that does not have a vulnerability attached), the "Create issue" button will not show.
Reproduce
- Go to https://staging.gitlab.com/govern-team-test/verify-413516/-/merge_requests/2
- Expand security reports widget and Click on a container scanning finding (is a new finding)
- Click on blue-dropdown button
- No "Create issue" option
3. can create an auto-remediation MR from mr security widget
This functionality is also available on the new modal, but the API that is called to create the remediation is changed:
- old modal: REST POST to
create_vulnerability_feedback_merge_request_path
- new modal: GraphQL mutation:
securityFindingCreateMergeRequest
Error (on new and old modal)
Clicking "Resolve with merge request" results in an error in the securityFindingCreateMergeRequest
mutation: "Unable to apply patch". It does however create a branch remediate/...
with the diff applied. This is also the case with the old modal.
The patch_result
in ee/app/services/merge_requests/create_from_vulnerability_data_service.rb:22
is the following:
"{:message=\u003e\"9:Patch failed at 0001 Fix Vulnerability - CVE-2022-27774 in curl-7.79.1-1.amzn2.0.1\\nWhen you have resolved this problem, run \\\"git am --continue\\\".\\nIf you prefer to skip this patch, run \\\"git am --skip\\\" instead.\\nTo restore the original branch and stop patching, run \\\"git am --abort\\\".\\n.\", :status=\u003e:error}"
Reproduce
- Go to https://staging.gitlab.com/govern-team-test/verify-413516/-/merge_requests/2
- Expand security reports widget and Click on a container scanning finding (these have remediations)
- Click on "Resolve with merge request"