security: GraphQL depth/complexity/alias limits and disable GET transport (#14)#584
Merged
lakhansamani merged 2 commits intomainfrom Apr 7, 2026
Merged
security: GraphQL depth/complexity/alias limits and disable GET transport (#14)#584lakhansamani merged 2 commits intomainfrom
lakhansamani merged 2 commits intomainfrom
Conversation
…port (#14) The GraphQL endpoint had no query depth limit, no per-operation alias cap, and accepted GET requests. These open three denial-of-service / leakage vectors: 1. Deeply nested queries can blow the call stack and exhaust resolver work 2. Alias amplification (many aliases on the same expensive field) multiplies work without changing complexity score 3. transport.GET leaks queries (and any sensitive arguments) into proxy logs, server access logs, and browser history Changes: - Remove transport.GET — clients must POST - Add a queryLimits handler extension that walks the parsed AST and enforces configurable max depth and max alias count, returning a gqlerror before execution begins - Make the existing complexity limit configurable instead of hardcoded 300 - Cap the GraphQL request body size via http.MaxBytesReader (default 1 MB) New CLI flags (with safe defaults): - --graphql-max-complexity (default 300) - --graphql-max-depth (default 15) - --graphql-max-aliases (default 30) - --graphql-max-body-bytes (default 1048576)
lakhansamani
force-pushed
the
security/graphql-hardening
branch
from
1e702a3 to
4d0d57a
Compare
The previous implementation walked the operation's selection-set tree twice for legitimate traffic — once for max depth, once for total alias count. Both walks visit every node, so the second pass is pure duplicated work. Replace selectionSetDepth + countAliases with a single recursive walkSelectionSet that returns (depth, aliases) in one traversal. The behaviour is identical: - Field nodes contribute one level of depth and (if aliased) one to the alias count - Inline fragments and fragment spreads do not add a depth level but their nested aliases still count - Walks short-circuit naturally on the rejection path because the caller checks the limits in order after one pass For a typical operation (~10–100 nodes) this halves the per-request AST work added by this PR. No allocations beyond the recursion stack.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three related hardenings on the GraphQL endpoint:
transport.GET— GraphQL queries (and especially mutations) over GET leak into proxy logs, server access logs, and browser historyChanges
queryLimitshandler extension walks the parsed AST and enforces max depth + max alias count viagqlerror, before execution begins.300.internal/http_handlers/graphql.goremovestransport.GET{}and wraps the request body withhttp.MaxBytesReader.--graphql-max-complexity(300),--graphql-max-depth(15),--graphql-max-aliases(30),--graphql-max-body-bytes(1 MiB).Test plan
go build ./...TEST_DBS=sqlite go test -run "TestSignup|TestLogin" ./internal/integration_tests/curl -X GET '<host>/graphql?query={__typename}'returns 405 / 404