fix: return the top-level GraphQL errors of eight security mutations
What does this MR do?
The review of !3063, the joint merge request that came out of #2300, asked me to split two of its changes into merge requests of their own, and this is the second, now with the integration tests the review asked for. The first is !3065. Like the rest of that work, it comes from maintaining gitlab-mcp-server, an MCP server built on this library.
When GitLab refuses one of the five security attribute mutations or the three security category mutations, it answers with HTTP 200, a top-level errors array and a null payload, and the methods only read the payload's own errors field. So a refused mutation reached the caller as a success from five of them (DestroySecurityAttribute, DestroySecurityCategory and BulkUpdateSecurityAttributes with a nil error, CreateSecurityAttributes with an empty slice, ProjectUpdateSecurityAttribute with zero counts), and as a bare ErrNotFound, without GitLab's message, from the other three (UpdateSecurityAttribute, CreateSecurityCategory and UpdateSecurityCategory). All eight now return a *GraphQLResponseError carrying GitLab's errors, as the work item and saved view methods already do.
The two commits
- fix: return the top-level GraphQL errors of eight security mutations. The commit reviewed in !3063 as its commit 4, cherry-picked unchanged onto v3.15.0. Each method checks the top-level errors before the payload and returns them in a
*GraphQLResponseError. Each of the eight resolves what it names throughauthorized_find!, which raisesResourceNotAvailable, aGraphQL::ExecutionError, when the object does not exist or the caller may not act on it. authorize_resource.rb - test: add integration tests for the eight security mutations. New, from the review. Two lifecycle tests drive all eight mutations against a live instance, and eight refusals, one for each of the eight, check that GitLab's error comes back as a
*GraphQLResponseError; the testing section below has the detail.utils_test.gogainsSkipIfNotUltimate, since security attributes are an Ultimate feature andSkipIfNotLicensedalso admits Premium, andCreateTestSecurityCategory.
gitlab-mcp-server keeps the record of this defect, and of everything else it finds and contributes in the projects it depends on and in sibling projects, in upstream-bugs.md (entry 19). The server works around it by sending the same eight mutations itself and reading the top-level errors, and moves onto these methods once they do.
Is this a breaking change?
No signature changes. What changes is the error a refused mutation returns: five methods that returned success now return an error, and UpdateSecurityAttribute, CreateSecurityCategory and UpdateSecurityCategory return a *GraphQLResponseError where they returned ErrNotFound, so a caller matching ErrNotFound on a refusal from those three sees a different error. They still return ErrNotFound when GitLab answers with no error and no object.
How was this tested?
Unit tests, integration tests and lint
Unit tests. Each of the eight methods gains a _graphQLErrors test whose handler answers HTTP 200 with a top-level error and a null payload, the shape GitLab refuses with, and asserts a *GraphQLResponseError carrying GitLab's message.
Integration tests, in gitlab_test/:
TestSecurityCategoryLifeCyclecreates, updates and destroys a category.TestSecurityAttributeLifeCyclecreates two attributes in a category, updates one, adds both to a project and removes one, applies one in bulk, destroys one, and finally applies the destroyed attribute in bulk, which GitLab refuses.TestSecurityAttributeMutationsOnAttributeThatDoesNotExistdestroys and updates an attribute that does not exist, andTestDestroySecurityCategoryThatDoesNotExistdestroys a category that does not exist.TestSecurityCategoryMutationsOnNamespaceThatDoesNotExistcreates and updates a category in a namespace that does not exist, andTestSecurityAttributeMutationsOnNamespaceOrProjectThatDoesNotExistcreates attributes in a namespace that does not exist and adds an attribute to a project that does not exist.
Each refusal asserts that the method returns a *GraphQLResponseError whose GitLab message says the resource does not exist or may not be acted on, that the error's text carries that message, and that the response was HTTP 200. The two lifecycle tests need an Ultimate license and skip elsewhere; SkipIfNotUltimate reads the license on every call rather than through the cache SkipIfNotLicensed keeps, which the two, running in parallel, would read and write without synchronization. The seven refusals of an ID that does not exist need no license: each of the seven mutations they call resolves the attribute, category, namespace or project it names through authorized_find! first, as categories/update.rb and attributes/project_update.rb show, so an ID that resolves to nothing is refused before anything a license decides is checked. The GitLab behavior the lifecycle relies on (soft deletion, the refusal of a destroyed attribute in bulk) was read from the 19.4.1 source, and the second commit's message links it.
Run against a live instance on 2026-09-30: GitLab EE 19.4.1-ee (revision 26212baacad, the gitlab/gitlab-ee:19.4.1-ee.0 image) in Docker, with an administrator's personal access token of api scope, from the branch with Go 1.27.1:
GITLAB_BASE_URL=http://127.0.0.1:18095/api/v4 GITLAB_TOKEN_TEST=<token> \
go test -race -tags=integration -count=1 -v -run '^(TestSecurity|TestDestroySecurityCategory)' ./gitlab_test/- Without a license, the seven refusals of an ID that does not exist pass and the two lifecycle tests skip.
- With an Ultimate license installed on the same instance, all six tests pass, the lifecycle tests included.
- On v3.15.0 with only the second commit, the fix left out, against the same Ultimate instance, every refusal fails: the five methods that returned success give a nil error, the three that returned
ErrNotFoundgive it (404 Not Found) rather than a*GraphQLResponseError, and the attribute lifecycle fails at its last step, the bulk update of the destroyed attribute. The category lifecycle, which has no refusal, passes.
The project's tests:integration job does not run them yet, nor any other test in gitlab_test/: it exports GITLAB_TOKEN, while SetupIntegrationClient reads GITLAB_TOKEN_TEST, so in the latest scheduled main pipeline every test there skipped with GITLAB_TOKEN_TEST environment variable not set (job 16821904079). !3067 (merged) proposes the one-line fix.
Lint and the rest, on the branch with Go 1.27.1:
go test -race ./... ./config/...in workspace mode, andgo test ./...withGOWORK=off, pass, andgo vet -tags=integration ./... ./config/...is clean.golangci-lint runover both modules with golangci-lint 2.13.2, the version.tool-versionspins, reports 0 issues. Its configuration sets no build tags, so that run does not load theintegrationfiles;gofumpt -lreports nothing,gitlab_testincluded. Run with--build-tags=integration, golangci-lint reports nothing in the new code butusetestingon thecontext.Background()of its two cleanups, which is the pattern the existing integration tests use, sincet.Context()is already cancelled when a cleanup runs.- No
.protofile and no service interface changes, somake generatehas nothing to regenerate.
Related to #2300