<!--IssueSummary start--> <details> <summary> Everyone can contribute. [Help move this issue forward](https://handbook.gitlab.com/handbook/marketing/developer-relations/contributor-success/community-contributors-workflows/#contributor-links) while earning points, leveling up and collecting rewards. </summary> - [Close this issue](https://contributors.gitlab.com/manage-issue?action=close&projectId=278964&issueIid=454393) </details> <!--IssueSummary end--> ### 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](https://gitlab.com/gitlab-com/sec-sub-department/section-sec-request-for-help/-/issues/236) ### 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](https://gitlab.com/gitlab-org/gitlab/-/blob/7edbdeb8295ebf4c8b67a2380d7a39377633774a/ee/app/services/vulnerabilities/findings/find_or_create_from_security_finding_service.rb#L11) and returning error response to the caller but the [logic](https://gitlab.com/gitlab-org/gitlab/-/blob/7edbdeb8295ebf4c8b67a2380d7a39377633774a/ee/app/services/security/findings/dismiss_service.rb#L75) 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](https://docs.gitlab.com/ee/development/api_graphql_styleguide.html#writing-unit-tests-deprecated). The diff below is a proof of concept only and not a recommended implementation for a new test. ```diff 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 ```