Certain failures in securityFindingDismiss GraphQL mutation do not populate errors in the response.

Everyone can contribute. Help move this issue forward while earning points, leveling up and collecting rewards.

Summary

When failures occur from the Vulnerabilities::Findings::FindOrCreateFromSecurityFindingService during the securityFindingDismiss GraphQL mutation the errors are not surfaced in the GraphQL response. This can happen for example in the case where all the report artifacts containing the security finding are expired and removed from the system.

This was reported in a support request for help

Steps to reproduce

  • Run a pipeline which introduces a new finding(use static reports template project for ease)
  • Note the UUID of the vulnerability
  • Remove the artifacts of the CI job from the system
  • Run the securityFindingDismiss GraphQL mutation with the UUID of the finding

You will see no errors in the GraphQL mutation response.

Example Project

https://gitlab.com/gitlab-org/govern/demos/sandbox/minac/static-reports/with-expiring-artifacts

What is the current bug behavior?

securityFindingDismiss GraphQL query does not bubble up errors to the client

What is the expected correct behavior?

securityFindingDismiss GraphQL query has to bubble up errors to the client if the report finding is not found.

Output of checks

This bug happens on GitLab.com

Possible fixes

The logic is failing here and returning error response to the caller but the logic laying on the upper level of the stack is not checking the error response.

We should check the return value from FindOrCreateFromSecurityFindingService and bubble and error that occur there.

Proof of concept regression test

Note we should consider reworking this test file as a request spec when we work on this as GQL unit tests are deprecated. The diff below is a proof of concept only and not a recommended implementation for a new test.

diff --git a/ee/spec/graphql/mutations/security/finding/dismiss_spec.rb b/ee/spec/graphql/mutations/security/finding/dismiss_spec.rb
index aba11e277607..2250ff73c55e 100644
--- a/ee/spec/graphql/mutations/security/finding/dismiss_spec.rb
+++ b/ee/spec/graphql/mutations/security/finding/dismiss_spec.rb
@@ -74,6 +74,22 @@
             expect(subject[:errors]).to match_array(['error'])
           end
         end
+
+        context 'when Vulnerability::Finding find or create fails' do
+          let_it_be(:error_result) do
+            ServiceResponse.error(message: "error")
+          end
+
+          before do
+            allow_next_instance_of(::Vulnerabilities::Findings::FindOrCreateFromSecurityFindingService) do |service|
+              allow(service).to receive(:execute).and_return(error_result)
+            end
+          end
+
+          it 'raises an error' do
+            expect(subject[:errors]).to match_array(['error'])
+          end
+        end
       end
     end
Edited by 🤖 GitLab Bot 🤖