From c72b0acd3f9ac8d8de209967cc32a09983f82cc0 Mon Sep 17 00:00:00 2001 From: Yury Smolski <140245+ysmolski@users.noreply.github.com> Date: Thu, 23 Oct 2025 12:50:06 +0300 Subject: [PATCH 1/4] fix: omit introspection queries for complexity limits This is disabled by default and should stay like that for now. There is a constant that can enable it again, but it is not intended to be tweaked by the clients. --- go.work.sum | 2 + .../operation_complexity.go | 39 ++++++++++++------- .../operation_complexity_test.go | 21 +++++++++- 3 files changed, 48 insertions(+), 14 deletions(-) diff --git a/go.work.sum b/go.work.sum index f13214003b..d66f874bf0 100644 --- a/go.work.sum +++ b/go.work.sum @@ -305,6 +305,7 @@ golang.org/x/mod v0.12.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= golang.org/x/mod v0.15.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c= golang.org/x/mod v0.17.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c= golang.org/x/mod v0.25.0/go.mod h1:IXM97Txy2VM4PJ3gI61r1YEk/gAj6zAHN3AdZt6S9Ww= +golang.org/x/mod v0.28.0/go.mod h1:yfB/L0NOf/kmEbXjzCPOx1iK1fRutOydrCMsqRhEBxI= golang.org/x/net v0.6.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs= golang.org/x/net v0.10.0/go.mod h1:0qNGK6F8kojg2nk9dLZ2mShWaEBan6FAoqfSigmmuDg= golang.org/x/net v0.15.0/go.mod h1:idbUs1IY1+zTqbi8yxTbhexhEEk5ur9LInksu6HrEpk= @@ -357,6 +358,7 @@ golang.org/x/tools v0.6.0/go.mod h1:Xwgl3UAJ/d3gWutnCtw505GrjyAbvKui8lOU390QaIU= golang.org/x/tools v0.13.0/go.mod h1:HvlwmtVNQAhOuCjW7xxvovg8wbNq7LwfXh/k7wXUl58= golang.org/x/tools v0.21.1-0.20240508182429-e35e4ccd0d2d/go.mod h1:aiJjzUbINMkxbQROHiO6hDPo2LHcIPhhQsa9DLh0yGk= golang.org/x/tools v0.34.0/go.mod h1:pAP9OwEaY1CAW3HOmg3hLZC5Z0CCmzjAF2UQMSqNARg= +golang.org/x/tools v0.37.0/go.mod h1:MBN5QPQtLMHVdvsbtarmTNukZDdgwdwlO5qGacAzF0w= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 h1:go1bK/D/BFZV2I8cIQd1NKEZ+0owSTG1fDTci4IqFcE= gonum.org/v1/plot v0.10.1 h1:dnifSs43YJuNMDzB7v8wV64O4ABBHReuAVAoBxqBqS4= diff --git a/v2/pkg/middleware/operation_complexity/operation_complexity.go b/v2/pkg/middleware/operation_complexity/operation_complexity.go index 23d0e5edd2..50c3122c6f 100644 --- a/v2/pkg/middleware/operation_complexity/operation_complexity.go +++ b/v2/pkg/middleware/operation_complexity/operation_complexity.go @@ -1,23 +1,26 @@ /* -package operation_complexity implements two common algorithms used by GitHub to calculate GraphQL query complexity +Package operation_complexity implements two common algorithms used by GitHub to calculate +GraphQL query complexity: -1. Node count, the maximum number of Nodes a query may return -2. Complexity, the maximum number of Node requests that might be needed to execute the query + 1. Node count, the maximum number of Nodes a query may return + 2. Complexity, the maximum number of Node requests that might be needed to execute the query -OperationComplexityEstimator takes a schema definition and a query and then walks recursively through the query to calculate both variables. +OperationComplexityEstimator takes a schema definition and a query and then +walks recursively through the query to calculate both variables. -The calculation can be influenced by integer arguments on fields that indicate the amount of Nodes returned by a field. +The calculation can be influenced by integer arguments on fields that indicate +the number of Nodes returned by a field. -To help the algorithm understand your schema you could make use of these two directives: +To help the algorithm understand the schema make use of these two directives: -- directive @nodeCountMultiply on ARGUMENT_DEFINITION -- directive @nodeCountSkip on FIELD + - directive @nodeCountMultiply on ARGUMENT_DEFINITION + - directive @nodeCountSkip on FIELD -nodeCountMultiply: -Indicates that the Int value the directive is applied on should be used as a Node multiplier +"nodeCountMultiply" indicates that the Int value the directive is applied on +should be used as a Node multiplier. -nodeCountSkip: -Indicates that the algorithm should skip this Node. This is useful to whitelist certain query paths, e.g. for introspection. +"nodeCountSkip" indicates that the algorithm should skip this Node. +It can be used to allowlist certain query paths, e.g. for introspection. */ package operation_complexity @@ -45,6 +48,12 @@ var ( nodeCountSkip = []byte("nodeCountSkip") ) +const ( + skipIntrospection = true + dashSchema = "__schema" + dashType = "__type" +) + type OperationComplexityEstimator struct { walker *astvisitor.Walker visitor *complexityVisitor @@ -184,12 +193,16 @@ func (c *complexityVisitor) EnterField(ref int) { return } - if _, exits := c.definition.FieldDefinitionDirectiveByName(definition, nodeCountSkip); exits { + if _, skip := c.definition.FieldDefinitionDirectiveByName(definition, nodeCountSkip); skip { c.SkipNode() return } typeName, fieldName, alias := c.extractFieldRelatedNames(ref, definition) + if skipIntrospection && (fieldName == dashSchema || fieldName == dashType) { + c.SkipNode() + return + } if c.isRootType(typeName) { c.resetCurrentRootFieldComplexity(typeName, fieldName, alias) } diff --git a/v2/pkg/middleware/operation_complexity/operation_complexity_test.go b/v2/pkg/middleware/operation_complexity/operation_complexity_test.go index 256c2c72c0..7e3e3f1bcc 100644 --- a/v2/pkg/middleware/operation_complexity/operation_complexity_test.go +++ b/v2/pkg/middleware/operation_complexity/operation_complexity_test.go @@ -39,6 +39,7 @@ func TestCalculateOperationComplexity(t *testing.T) { run(t, testDefinition, ` { users(first: 1) { + __typename id balance name @@ -66,6 +67,23 @@ func TestCalculateOperationComplexity(t *testing.T) { }, ) }) + t.Run("skipped node", func(t *testing.T) { + run(t, testDefinition, ` + { + activeUsers { + id + balance + name + } + }`, + OperationStats{ + NodeCount: 0, + Complexity: 0, + Depth: 0, + }, + []RootFieldStats{}, + ) + }) t.Run("one user with inline fragments", func(t *testing.T) { run(t, testDefinition, ` { @@ -592,11 +610,12 @@ input NewUserInput { } type Query { - __schema: __Schema! @nodeCountSkip + __schema: __Schema! user(id: ID!): User users(first: Int! @nodeCountMultiply, afterID: ID): [User] transactions(first: Int! @nodeCountMultiply, afterID: ID): [Transaction] currentPeriod: String + activeUsers: [User] @nodeCountSkip } type Mutation { From c989b226d8bc49ea531b77a29622158654bbf5c8 Mon Sep 17 00:00:00 2001 From: Yury Smolski <140245+ysmolski@users.noreply.github.com> Date: Thu, 23 Oct 2025 12:53:58 +0300 Subject: [PATCH 2/4] fix indentation --- .../operation_complexity/operation_complexity_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/v2/pkg/middleware/operation_complexity/operation_complexity_test.go b/v2/pkg/middleware/operation_complexity/operation_complexity_test.go index 7e3e3f1bcc..14d316aa14 100644 --- a/v2/pkg/middleware/operation_complexity/operation_complexity_test.go +++ b/v2/pkg/middleware/operation_complexity/operation_complexity_test.go @@ -71,9 +71,9 @@ func TestCalculateOperationComplexity(t *testing.T) { run(t, testDefinition, ` { activeUsers { - id - balance - name + id + balance + name } }`, OperationStats{ From c1b446202795bda9490f9c04e577ba8501cd5443 Mon Sep 17 00:00:00 2001 From: Yury Smolski <140245+ysmolski@users.noreply.github.com> Date: Thu, 23 Oct 2025 15:13:30 +0300 Subject: [PATCH 3/4] rename const --- .../middleware/operation_complexity/operation_complexity.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/v2/pkg/middleware/operation_complexity/operation_complexity.go b/v2/pkg/middleware/operation_complexity/operation_complexity.go index 50c3122c6f..74b78bd663 100644 --- a/v2/pkg/middleware/operation_complexity/operation_complexity.go +++ b/v2/pkg/middleware/operation_complexity/operation_complexity.go @@ -50,8 +50,8 @@ var ( const ( skipIntrospection = true - dashSchema = "__schema" - dashType = "__type" + __schemaLiteral = "__schema" + __typeLiteral = "__type" ) type OperationComplexityEstimator struct { @@ -199,7 +199,7 @@ func (c *complexityVisitor) EnterField(ref int) { } typeName, fieldName, alias := c.extractFieldRelatedNames(ref, definition) - if skipIntrospection && (fieldName == dashSchema || fieldName == dashType) { + if skipIntrospection && (fieldName == __schemaLiteral || fieldName == __typeLiteral) { c.SkipNode() return } From 18a977b34441ee0b688775d146d2be7180efcc0e Mon Sep 17 00:00:00 2001 From: Yury Smolski <140245+ysmolski@users.noreply.github.com> Date: Thu, 23 Oct 2025 15:28:09 +0300 Subject: [PATCH 4/4] refine commments --- .coderabbit.yaml | 3 +++ .../middleware/operation_complexity/operation_complexity.go | 5 ++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/.coderabbit.yaml b/.coderabbit.yaml index 01bc777d0c..28211c20fe 100644 --- a/.coderabbit.yaml +++ b/.coderabbit.yaml @@ -15,5 +15,8 @@ reviews: path_filters: # Exclude these paths (prefix with !) - "!pkg/**" + pre_merge_checks: + docstrings: + mode: off chat: art: false diff --git a/v2/pkg/middleware/operation_complexity/operation_complexity.go b/v2/pkg/middleware/operation_complexity/operation_complexity.go index 74b78bd663..dba06c7fc8 100644 --- a/v2/pkg/middleware/operation_complexity/operation_complexity.go +++ b/v2/pkg/middleware/operation_complexity/operation_complexity.go @@ -20,7 +20,10 @@ To help the algorithm understand the schema make use of these two directives: should be used as a Node multiplier. "nodeCountSkip" indicates that the algorithm should skip this Node. -It can be used to allowlist certain query paths, e.g. for introspection. +It can be used to allowlist certain query paths. + +Note: Introspection fields (__schema and __type) are automatically skipped +from complexity calculations by default. */ package operation_complexity