From db589b7b296942a5285a541a3552d02b23be3749 Mon Sep 17 00:00:00 2001 From: enisdenjo Date: Mon, 20 May 2024 18:00:36 +0200 Subject: [PATCH 01/12] impl --- src/index.ts | 1 + .../MaxIntrospectionDepthRule-test.ts | 539 ++++++++++++++++++ src/validation/index.ts | 2 + .../rules/MaxIntrospectionDepthRule.ts | 69 +++ src/validation/specifiedRules.ts | 2 + 5 files changed, 613 insertions(+) create mode 100644 src/validation/__tests__/MaxIntrospectionDepthRule-test.ts create mode 100644 src/validation/rules/MaxIntrospectionDepthRule.ts diff --git a/src/index.ts b/src/index.ts index bb82b58164..597bfe310b 100644 --- a/src/index.ts +++ b/src/index.ts @@ -366,6 +366,7 @@ export { ValuesOfCorrectTypeRule, VariablesAreInputTypesRule, VariablesInAllowedPositionRule, + MaxIntrospectionDepthRule, // SDL-specific validation rules LoneSchemaDefinitionRule, UniqueOperationTypesRule, diff --git a/src/validation/__tests__/MaxIntrospectionDepthRule-test.ts b/src/validation/__tests__/MaxIntrospectionDepthRule-test.ts new file mode 100644 index 0000000000..ee6269857e --- /dev/null +++ b/src/validation/__tests__/MaxIntrospectionDepthRule-test.ts @@ -0,0 +1,539 @@ +import { describe, it } from 'mocha'; + +import { getIntrospectionQuery } from '../../utilities/getIntrospectionQuery'; + +import { MaxIntrospectionDepthRule } from '../rules/MaxIntrospectionDepthRule'; + +import { expectValidationErrors } from './harness'; + +function expectErrors(queryStr: string) { + return expectValidationErrors(MaxIntrospectionDepthRule, queryStr); +} + +function expectValid(queryStr: string) { + expectErrors(queryStr).toDeepEqual([]); +} + +describe('Validate: Max introspection nodes rule', () => { + it('default introspection query', () => { + expectValid(getIntrospectionQuery()); + }); + + it('all options introspection query', () => { + expectValid( + getIntrospectionQuery({ + descriptions: true, + specifiedByUrl: true, + directiveIsRepeatable: true, + schemaDescription: true, + inputValueDeprecation: true, + }), + ); + }); + + it('3 flat fields introspection query', () => { + expectValid(` + { + __type(name: "Query") { + trueFields: fields(includeDeprecated: true) { + name + } + falseFields: fields(includeDeprecated: false) { + name + } + omittedFields: fields { + name + } + } + } + `); + }); + + it('3 fields deep introspection query from __schema', () => { + expectErrors(` + { + __schema { + types { + fields { + type { + fields { + type { + fields { + name + } + } + } + } + } + } + } + } + `).toDeepEqual([ + { + message: 'Maximum introspection depth exceeded', + locations: [ + { + column: 7, + line: 3, + }, + ], + }, + ]); + }); + + it('3 interfaces deep introspection query from __schema', () => { + expectErrors(` + { + __schema { + types { + interfaces { + interfaces { + interfaces { + name + } + } + } + } + } + } + `).toDeepEqual([ + { + message: 'Maximum introspection depth exceeded', + locations: [ + { + column: 7, + line: 3, + }, + ], + }, + ]); + }); + + it('3 possibleTypes deep introspection query from __schema', () => { + expectErrors(` + { + __schema { + types { + possibleTypes { + possibleTypes { + possibleTypes { + name + } + } + } + } + } + } + `).toDeepEqual([ + { + message: 'Maximum introspection depth exceeded', + locations: [ + { + column: 7, + line: 3, + }, + ], + }, + ]); + }); + + it('3 inputFields deep introspection query from __schema', () => { + expectErrors(` + { + __schema { + types { + inputFields { + type { + inputFields { + type { + inputFields { + type { + name + } + } + } + } + } + } + } + } + } + `).toDeepEqual([ + { + message: 'Maximum introspection depth exceeded', + locations: [ + { + column: 7, + line: 3, + }, + ], + }, + ]); + }); + + it('3 fields deep introspection query from multiple __schema', () => { + expectErrors(` + { + one: __schema { + types { + fields { + type { + fields { + type { + fields { + name + } + } + } + } + } + } + } + two: __schema { + types { + fields { + type { + fields { + type { + fields { + name + } + } + } + } + } + } + } + three: __schema { + types { + fields { + type { + fields { + type { + fields { + name + } + } + } + } + } + } + } + } + `).toDeepEqual([ + { + message: 'Maximum introspection depth exceeded', + locations: [ + { + column: 7, + line: 3, + }, + ], + }, + { + locations: [ + { + column: 7, + line: 18, + }, + ], + message: 'Maximum introspection depth exceeded', + }, + { + locations: [ + { + column: 7, + line: 33, + }, + ], + message: 'Maximum introspection depth exceeded', + }, + ]); + }); + + it('3 fields deep introspection query from __type', () => { + expectErrors(` + { + __type(name: "Query") { + types { + fields { + type { + fields { + type { + fields { + name + } + } + } + } + } + } + } + } + `).toDeepEqual([ + { + message: 'Maximum introspection depth exceeded', + locations: [ + { + column: 7, + line: 3, + }, + ], + }, + ]); + }); + + it('3 fields deep introspection query from multiple __type', () => { + expectErrors(` + { + one: __type(name: "Query") { + types { + fields { + type { + fields { + type { + fields { + name + } + } + } + } + } + } + } + two: __type(name: "Query") { + types { + fields { + type { + fields { + type { + fields { + name + } + } + } + } + } + } + } + three: __type(name: "Query") { + types { + fields { + type { + fields { + type { + fields { + name + } + } + } + } + } + } + } + } + `).toDeepEqual([ + { + message: 'Maximum introspection depth exceeded', + locations: [ + { + column: 7, + line: 3, + }, + ], + }, + { + locations: [ + { + column: 7, + line: 18, + }, + ], + message: 'Maximum introspection depth exceeded', + }, + { + locations: [ + { + column: 7, + line: 33, + }, + ], + message: 'Maximum introspection depth exceeded', + }, + ]); + }); + + it('1 fields deep with 3 fields introspection query', () => { + expectValid(` + { + __schema { + types { + fields { + type { + oneFields: fields { + name + } + twoFields: fields { + name + } + threeFields: fields { + name + } + } + } + } + } + } + `); + }); + + it('3 fields deep from varying parents introspection query', () => { + expectErrors(` + { + __schema { + types { + fields { + type { + fields { + type { + ofType { + fields { + name + } + } + } + } + } + } + } + } + } + `).toDeepEqual([ + { + message: 'Maximum introspection depth exceeded', + locations: [ + { + column: 7, + line: 3, + }, + ], + }, + ]); + }); + + it('3 fields deep introspection query with inline fragments', () => { + expectErrors(` + query test { + __schema { + types { + ... on __Type { + fields { + type { + ... on __Type { + ofType { + fields { + type { + ... on __Type { + fields { + name + } + } + } + } + } + } + } + } + } + } + } + } + `).toDeepEqual([ + { + message: 'Maximum introspection depth exceeded', + locations: [ + { + column: 7, + line: 3, + }, + ], + }, + ]); + }); + + it('3 fields deep introspection query with fragments', () => { + expectErrors(` + query test { + __schema { + types { + ...One + } + } + } + + fragment One on __Type { + fields { + type { + ...Two + } + } + } + + fragment Two on __Type { + fields { + type { + ...Three + } + } + } + + fragment Three on __Type { + fields { + name + } + } + `).toDeepEqual([ + { + message: 'Maximum introspection depth exceeded', + locations: [ + { + column: 7, + line: 3, + }, + ], + }, + ]); + }); + + it('3 fields deep inside inline fragment on query', () => { + expectErrors(` + { + ... { + __schema { types { fields { type { fields { type { fields { name } } } } } } } + } + } + `).toDeepEqual([ + { + message: 'Maximum introspection depth exceeded', + locations: [ + { + column: 9, + line: 4, + }, + ], + }, + ]); + }); + + it('opts out if fragment is missing', () => { + expectValid(` + query test { + __schema { + types { + ...Missing + } + } + } + `); + }); +}); diff --git a/src/validation/index.ts b/src/validation/index.ts index 58cc012ee8..f92ee51aa1 100644 --- a/src/validation/index.ts +++ b/src/validation/index.ts @@ -84,6 +84,8 @@ export { VariablesAreInputTypesRule } from './rules/VariablesAreInputTypesRule'; // Spec Section: "All Variable Usages Are Allowed" export { VariablesInAllowedPositionRule } from './rules/VariablesInAllowedPositionRule'; +export { MaxIntrospectionDepthRule } from './rules/MaxIntrospectionDepthRule'; + // SDL-specific validation rules export { LoneSchemaDefinitionRule } from './rules/LoneSchemaDefinitionRule'; export { UniqueOperationTypesRule } from './rules/UniqueOperationTypesRule'; diff --git a/src/validation/rules/MaxIntrospectionDepthRule.ts b/src/validation/rules/MaxIntrospectionDepthRule.ts new file mode 100644 index 0000000000..fb38a58863 --- /dev/null +++ b/src/validation/rules/MaxIntrospectionDepthRule.ts @@ -0,0 +1,69 @@ +import { GraphQLError } from '../../error/GraphQLError'; + +import type { ASTNode } from '../../language/ast'; +import { Kind } from '../../language/kinds'; +import type { ASTVisitor } from '../../language/visitor'; + +import type { ASTValidationContext } from '../ValidationContext'; + +const MAX_LISTS_DEPTH = 3; + +export function MaxIntrospectionDepthRule( + context: ASTValidationContext, +): ASTVisitor { + /** + * Counts the depth of list fields in "__Type" recursively and + * returns `true` if the limit has been reached. + */ + function checkDepth(node: ASTNode, depth: number = 0): boolean { + if (node.kind === Kind.FRAGMENT_SPREAD) { + const fragment = context.getFragment(node.name.value); + if (!fragment) { + // missing fragments checks are handled by the `KnownFragmentNamesRule` + return false; + } + return checkDepth(fragment, depth); + } + + if ( + node.kind === Kind.FIELD && + // check all introspection lists + (node.name.value === 'fields' || + node.name.value === 'interfaces' || + node.name.value === 'possibleTypes' || + node.name.value === 'inputFields') + ) { + // eslint-disable-next-line no-param-reassign + depth++; + if (depth >= MAX_LISTS_DEPTH) { + return true; + } + } + + // handles fields and inline fragments + if ('selectionSet' in node && node.selectionSet) { + for (const child of node.selectionSet.selections) { + if (checkDepth(child, depth)) { + return true; + } + } + } + + return false; + } + + return { + Field(node) { + if (node.name.value === '__schema' || node.name.value === '__type') { + if (checkDepth(node)) { + context.reportError( + new GraphQLError('Maximum introspection depth exceeded', { + nodes: [node], + }), + ); + return false; + } + } + }, + }; +} diff --git a/src/validation/specifiedRules.ts b/src/validation/specifiedRules.ts index 16e555db8a..cc0c1f2495 100644 --- a/src/validation/specifiedRules.ts +++ b/src/validation/specifiedRules.ts @@ -19,6 +19,7 @@ import { KnownTypeNamesRule } from './rules/KnownTypeNamesRule'; import { LoneAnonymousOperationRule } from './rules/LoneAnonymousOperationRule'; // SDL-specific validation rules import { LoneSchemaDefinitionRule } from './rules/LoneSchemaDefinitionRule'; +import { MaxIntrospectionDepthRule } from './rules/MaxIntrospectionDepthRule'; // Spec Section: "Fragments must not form cycles" import { NoFragmentCyclesRule } from './rules/NoFragmentCyclesRule'; // Spec Section: "All Variable Used Defined" @@ -121,5 +122,6 @@ export const specifiedSDLRules: ReadonlyArray = KnownArgumentNamesOnDirectivesRule, UniqueArgumentNamesRule, UniqueInputFieldNamesRule, + MaxIntrospectionDepthRule, ProvidedRequiredArgumentsOnDirectivesRule, ]); From e3c6ec1a2ee3871cecf6cd3537069420c8602ae9 Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Thu, 23 May 2024 17:00:30 +0100 Subject: [PATCH 02/12] Add test to check how we handle infinite recursion --- .../__tests__/MaxIntrospectionDepthRule-test.ts | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/validation/__tests__/MaxIntrospectionDepthRule-test.ts b/src/validation/__tests__/MaxIntrospectionDepthRule-test.ts index ee6269857e..d6609941d4 100644 --- a/src/validation/__tests__/MaxIntrospectionDepthRule-test.ts +++ b/src/validation/__tests__/MaxIntrospectionDepthRule-test.ts @@ -536,4 +536,19 @@ describe('Validate: Max introspection nodes rule', () => { } `); }); + + it("doesn't infinitely recurse on fragment cycle", () => { + expectValid(` + query test { + __schema { + types { + ...Cycle + } + } + } + fragment Cycle on __Type { + ...Cycle + } + `); + }); }); From c2479a29d19718d2a9fd274a6256a2ab8fd867a8 Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Thu, 23 May 2024 17:08:12 +0100 Subject: [PATCH 03/12] Fix infinite recursion bug --- .../rules/MaxIntrospectionDepthRule.ts | 32 +++++++++++++++---- 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/src/validation/rules/MaxIntrospectionDepthRule.ts b/src/validation/rules/MaxIntrospectionDepthRule.ts index fb38a58863..a03374c5e2 100644 --- a/src/validation/rules/MaxIntrospectionDepthRule.ts +++ b/src/validation/rules/MaxIntrospectionDepthRule.ts @@ -15,14 +15,34 @@ export function MaxIntrospectionDepthRule( * Counts the depth of list fields in "__Type" recursively and * returns `true` if the limit has been reached. */ - function checkDepth(node: ASTNode, depth: number = 0): boolean { + function checkDepth( + node: ASTNode, + visitedFragments: Record, + depth: number = 0, + ): boolean { if (node.kind === Kind.FRAGMENT_SPREAD) { - const fragment = context.getFragment(node.name.value); + const fragmentName = node.name.value; + if (visitedFragments[fragmentName]) { + // Fragment cycles are handled by `NoFragmentCyclesRule`. + return false; + } + const fragment = context.getFragment(fragmentName); if (!fragment) { - // missing fragments checks are handled by the `KnownFragmentNamesRule` + // Missing fragments checks are handled by `KnownFragmentNamesRule`. return false; } - return checkDepth(fragment, depth); + + // Rather than following an immutable programming pattern which has + // significant memory and garbage collection overhead, we've opted to + // take a mutable approach for efficiency's sake. Importantly visiting a + // fragment twice is fine, so long as you don't do one visit inside the + // other. + try { + visitedFragments[fragmentName] = true; + return checkDepth(fragment, visitedFragments, depth); + } finally { + delete visitedFragments[fragmentName]; + } } if ( @@ -43,7 +63,7 @@ export function MaxIntrospectionDepthRule( // handles fields and inline fragments if ('selectionSet' in node && node.selectionSet) { for (const child of node.selectionSet.selections) { - if (checkDepth(child, depth)) { + if (checkDepth(child, visitedFragments, depth)) { return true; } } @@ -55,7 +75,7 @@ export function MaxIntrospectionDepthRule( return { Field(node) { if (node.name.value === '__schema' || node.name.value === '__type') { - if (checkDepth(node)) { + if (checkDepth(node, Object.create(null))) { context.reportError( new GraphQLError('Maximum introspection depth exceeded', { nodes: [node], From 18d6927e721d443ed919207475cc57d8eb008b93 Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Thu, 23 May 2024 17:09:28 +0100 Subject: [PATCH 04/12] Lint --- src/validation/rules/MaxIntrospectionDepthRule.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/validation/rules/MaxIntrospectionDepthRule.ts b/src/validation/rules/MaxIntrospectionDepthRule.ts index a03374c5e2..88b7485825 100644 --- a/src/validation/rules/MaxIntrospectionDepthRule.ts +++ b/src/validation/rules/MaxIntrospectionDepthRule.ts @@ -17,7 +17,7 @@ export function MaxIntrospectionDepthRule( */ function checkDepth( node: ASTNode, - visitedFragments: Record, + visitedFragments: { [fragmentName: string]: true }, depth: number = 0, ): boolean { if (node.kind === Kind.FRAGMENT_SPREAD) { From 784b74fa203acdb5424ee9854bd05262779da5b5 Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Thu, 23 May 2024 17:11:41 +0100 Subject: [PATCH 05/12] Excessive efficiency --- src/validation/rules/MaxIntrospectionDepthRule.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/validation/rules/MaxIntrospectionDepthRule.ts b/src/validation/rules/MaxIntrospectionDepthRule.ts index 88b7485825..c55b2dbae7 100644 --- a/src/validation/rules/MaxIntrospectionDepthRule.ts +++ b/src/validation/rules/MaxIntrospectionDepthRule.ts @@ -17,12 +17,12 @@ export function MaxIntrospectionDepthRule( */ function checkDepth( node: ASTNode, - visitedFragments: { [fragmentName: string]: true }, + visitedFragments: { [fragmentName: string]: true | undefined }, depth: number = 0, ): boolean { if (node.kind === Kind.FRAGMENT_SPREAD) { const fragmentName = node.name.value; - if (visitedFragments[fragmentName]) { + if (visitedFragments[fragmentName] === true) { // Fragment cycles are handled by `NoFragmentCyclesRule`. return false; } @@ -41,7 +41,7 @@ export function MaxIntrospectionDepthRule( visitedFragments[fragmentName] = true; return checkDepth(fragment, visitedFragments, depth); } finally { - delete visitedFragments[fragmentName]; + visitedFragments[fragmentName] = undefined; } } From 07062740236bb3d2e256c606582f9b5ccbd8cc2b Mon Sep 17 00:00:00 2001 From: enisdenjo Date: Mon, 3 Jun 2024 13:06:22 +0200 Subject: [PATCH 06/12] on create --- src/validation/rules/MaxIntrospectionDepthRule.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/validation/rules/MaxIntrospectionDepthRule.ts b/src/validation/rules/MaxIntrospectionDepthRule.ts index c55b2dbae7..0c2dbd3879 100644 --- a/src/validation/rules/MaxIntrospectionDepthRule.ts +++ b/src/validation/rules/MaxIntrospectionDepthRule.ts @@ -17,7 +17,9 @@ export function MaxIntrospectionDepthRule( */ function checkDepth( node: ASTNode, - visitedFragments: { [fragmentName: string]: true | undefined }, + visitedFragments: { + [fragmentName: string]: true | undefined; + } = Object.create(null), depth: number = 0, ): boolean { if (node.kind === Kind.FRAGMENT_SPREAD) { @@ -75,7 +77,7 @@ export function MaxIntrospectionDepthRule( return { Field(node) { if (node.name.value === '__schema' || node.name.value === '__type') { - if (checkDepth(node, Object.create(null))) { + if (checkDepth(node)) { context.reportError( new GraphQLError('Maximum introspection depth exceeded', { nodes: [node], From 0e0a7a8d51f98321785fe8cca3735eb119e46ba6 Mon Sep 17 00:00:00 2001 From: enisdenjo Date: Mon, 3 Jun 2024 13:07:14 +0200 Subject: [PATCH 07/12] in specified rules --- src/validation/specifiedRules.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/validation/specifiedRules.ts b/src/validation/specifiedRules.ts index cc0c1f2495..3cebe560bd 100644 --- a/src/validation/specifiedRules.ts +++ b/src/validation/specifiedRules.ts @@ -99,6 +99,7 @@ export const specifiedRules: ReadonlyArray = Object.freeze([ ValuesOfCorrectTypeRule, ProvidedRequiredArgumentsRule, VariablesInAllowedPositionRule, + MaxIntrospectionDepthRule, OverlappingFieldsCanBeMergedRule, UniqueInputFieldNamesRule, ]); @@ -122,6 +123,5 @@ export const specifiedSDLRules: ReadonlyArray = KnownArgumentNamesOnDirectivesRule, UniqueArgumentNamesRule, UniqueInputFieldNamesRule, - MaxIntrospectionDepthRule, ProvidedRequiredArgumentsOnDirectivesRule, ]); From bd6dcb5f0399cc321515bfd905dbd749c20b9b30 Mon Sep 17 00:00:00 2001 From: enisdenjo Date: Mon, 3 Jun 2024 13:08:58 +0200 Subject: [PATCH 08/12] reorder --- src/validation/specifiedRules.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/validation/specifiedRules.ts b/src/validation/specifiedRules.ts index 3cebe560bd..34a5faa8fa 100644 --- a/src/validation/specifiedRules.ts +++ b/src/validation/specifiedRules.ts @@ -17,9 +17,9 @@ import { KnownFragmentNamesRule } from './rules/KnownFragmentNamesRule'; import { KnownTypeNamesRule } from './rules/KnownTypeNamesRule'; // Spec Section: "Lone Anonymous Operation" import { LoneAnonymousOperationRule } from './rules/LoneAnonymousOperationRule'; +import { MaxIntrospectionDepthRule } from './rules/MaxIntrospectionDepthRule'; // SDL-specific validation rules import { LoneSchemaDefinitionRule } from './rules/LoneSchemaDefinitionRule'; -import { MaxIntrospectionDepthRule } from './rules/MaxIntrospectionDepthRule'; // Spec Section: "Fragments must not form cycles" import { NoFragmentCyclesRule } from './rules/NoFragmentCyclesRule'; // Spec Section: "All Variable Used Defined" @@ -99,9 +99,9 @@ export const specifiedRules: ReadonlyArray = Object.freeze([ ValuesOfCorrectTypeRule, ProvidedRequiredArgumentsRule, VariablesInAllowedPositionRule, - MaxIntrospectionDepthRule, OverlappingFieldsCanBeMergedRule, UniqueInputFieldNamesRule, + MaxIntrospectionDepthRule, ]); /** From c10935fa81e02e1d91487f209adaf49658884426 Mon Sep 17 00:00:00 2001 From: enisdenjo Date: Wed, 19 Jun 2024 11:52:02 +0200 Subject: [PATCH 09/12] comment --- src/validation/specifiedRules.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/validation/specifiedRules.ts b/src/validation/specifiedRules.ts index 34a5faa8fa..4a90f265f3 100644 --- a/src/validation/specifiedRules.ts +++ b/src/validation/specifiedRules.ts @@ -17,7 +17,6 @@ import { KnownFragmentNamesRule } from './rules/KnownFragmentNamesRule'; import { KnownTypeNamesRule } from './rules/KnownTypeNamesRule'; // Spec Section: "Lone Anonymous Operation" import { LoneAnonymousOperationRule } from './rules/LoneAnonymousOperationRule'; -import { MaxIntrospectionDepthRule } from './rules/MaxIntrospectionDepthRule'; // SDL-specific validation rules import { LoneSchemaDefinitionRule } from './rules/LoneSchemaDefinitionRule'; // Spec Section: "Fragments must not form cycles" @@ -68,6 +67,9 @@ import { VariablesAreInputTypesRule } from './rules/VariablesAreInputTypesRule'; import { VariablesInAllowedPositionRule } from './rules/VariablesInAllowedPositionRule'; import type { SDLValidationRule, ValidationRule } from './ValidationContext'; +// TODO: Spec Section +import { MaxIntrospectionDepthRule } from './rules/MaxIntrospectionDepthRule'; + /** * This set includes all validation rules defined by the GraphQL spec. * @@ -101,6 +103,7 @@ export const specifiedRules: ReadonlyArray = Object.freeze([ VariablesInAllowedPositionRule, OverlappingFieldsCanBeMergedRule, UniqueInputFieldNamesRule, + // Technically this isn't part of the spec but it's a strongly encouraged validation rule. MaxIntrospectionDepthRule, ]); From 8d14383344d2248c50c237c1ff983ecb3f5d13d8 Mon Sep 17 00:00:00 2001 From: enisdenjo Date: Mon, 20 May 2024 18:00:36 +0200 Subject: [PATCH 10/12] Introduce max introspection depth rule --- src/index.ts | 1 + .../MaxIntrospectionDepthRule-test.ts | 554 ++++++++++++++++++ src/validation/index.ts | 2 + .../rules/MaxIntrospectionDepthRule.ts | 91 +++ src/validation/specifiedRules.ts | 5 + 5 files changed, 653 insertions(+) create mode 100644 src/validation/__tests__/MaxIntrospectionDepthRule-test.ts create mode 100644 src/validation/rules/MaxIntrospectionDepthRule.ts diff --git a/src/index.ts b/src/index.ts index bb82b58164..597bfe310b 100644 --- a/src/index.ts +++ b/src/index.ts @@ -366,6 +366,7 @@ export { ValuesOfCorrectTypeRule, VariablesAreInputTypesRule, VariablesInAllowedPositionRule, + MaxIntrospectionDepthRule, // SDL-specific validation rules LoneSchemaDefinitionRule, UniqueOperationTypesRule, diff --git a/src/validation/__tests__/MaxIntrospectionDepthRule-test.ts b/src/validation/__tests__/MaxIntrospectionDepthRule-test.ts new file mode 100644 index 0000000000..d6609941d4 --- /dev/null +++ b/src/validation/__tests__/MaxIntrospectionDepthRule-test.ts @@ -0,0 +1,554 @@ +import { describe, it } from 'mocha'; + +import { getIntrospectionQuery } from '../../utilities/getIntrospectionQuery'; + +import { MaxIntrospectionDepthRule } from '../rules/MaxIntrospectionDepthRule'; + +import { expectValidationErrors } from './harness'; + +function expectErrors(queryStr: string) { + return expectValidationErrors(MaxIntrospectionDepthRule, queryStr); +} + +function expectValid(queryStr: string) { + expectErrors(queryStr).toDeepEqual([]); +} + +describe('Validate: Max introspection nodes rule', () => { + it('default introspection query', () => { + expectValid(getIntrospectionQuery()); + }); + + it('all options introspection query', () => { + expectValid( + getIntrospectionQuery({ + descriptions: true, + specifiedByUrl: true, + directiveIsRepeatable: true, + schemaDescription: true, + inputValueDeprecation: true, + }), + ); + }); + + it('3 flat fields introspection query', () => { + expectValid(` + { + __type(name: "Query") { + trueFields: fields(includeDeprecated: true) { + name + } + falseFields: fields(includeDeprecated: false) { + name + } + omittedFields: fields { + name + } + } + } + `); + }); + + it('3 fields deep introspection query from __schema', () => { + expectErrors(` + { + __schema { + types { + fields { + type { + fields { + type { + fields { + name + } + } + } + } + } + } + } + } + `).toDeepEqual([ + { + message: 'Maximum introspection depth exceeded', + locations: [ + { + column: 7, + line: 3, + }, + ], + }, + ]); + }); + + it('3 interfaces deep introspection query from __schema', () => { + expectErrors(` + { + __schema { + types { + interfaces { + interfaces { + interfaces { + name + } + } + } + } + } + } + `).toDeepEqual([ + { + message: 'Maximum introspection depth exceeded', + locations: [ + { + column: 7, + line: 3, + }, + ], + }, + ]); + }); + + it('3 possibleTypes deep introspection query from __schema', () => { + expectErrors(` + { + __schema { + types { + possibleTypes { + possibleTypes { + possibleTypes { + name + } + } + } + } + } + } + `).toDeepEqual([ + { + message: 'Maximum introspection depth exceeded', + locations: [ + { + column: 7, + line: 3, + }, + ], + }, + ]); + }); + + it('3 inputFields deep introspection query from __schema', () => { + expectErrors(` + { + __schema { + types { + inputFields { + type { + inputFields { + type { + inputFields { + type { + name + } + } + } + } + } + } + } + } + } + `).toDeepEqual([ + { + message: 'Maximum introspection depth exceeded', + locations: [ + { + column: 7, + line: 3, + }, + ], + }, + ]); + }); + + it('3 fields deep introspection query from multiple __schema', () => { + expectErrors(` + { + one: __schema { + types { + fields { + type { + fields { + type { + fields { + name + } + } + } + } + } + } + } + two: __schema { + types { + fields { + type { + fields { + type { + fields { + name + } + } + } + } + } + } + } + three: __schema { + types { + fields { + type { + fields { + type { + fields { + name + } + } + } + } + } + } + } + } + `).toDeepEqual([ + { + message: 'Maximum introspection depth exceeded', + locations: [ + { + column: 7, + line: 3, + }, + ], + }, + { + locations: [ + { + column: 7, + line: 18, + }, + ], + message: 'Maximum introspection depth exceeded', + }, + { + locations: [ + { + column: 7, + line: 33, + }, + ], + message: 'Maximum introspection depth exceeded', + }, + ]); + }); + + it('3 fields deep introspection query from __type', () => { + expectErrors(` + { + __type(name: "Query") { + types { + fields { + type { + fields { + type { + fields { + name + } + } + } + } + } + } + } + } + `).toDeepEqual([ + { + message: 'Maximum introspection depth exceeded', + locations: [ + { + column: 7, + line: 3, + }, + ], + }, + ]); + }); + + it('3 fields deep introspection query from multiple __type', () => { + expectErrors(` + { + one: __type(name: "Query") { + types { + fields { + type { + fields { + type { + fields { + name + } + } + } + } + } + } + } + two: __type(name: "Query") { + types { + fields { + type { + fields { + type { + fields { + name + } + } + } + } + } + } + } + three: __type(name: "Query") { + types { + fields { + type { + fields { + type { + fields { + name + } + } + } + } + } + } + } + } + `).toDeepEqual([ + { + message: 'Maximum introspection depth exceeded', + locations: [ + { + column: 7, + line: 3, + }, + ], + }, + { + locations: [ + { + column: 7, + line: 18, + }, + ], + message: 'Maximum introspection depth exceeded', + }, + { + locations: [ + { + column: 7, + line: 33, + }, + ], + message: 'Maximum introspection depth exceeded', + }, + ]); + }); + + it('1 fields deep with 3 fields introspection query', () => { + expectValid(` + { + __schema { + types { + fields { + type { + oneFields: fields { + name + } + twoFields: fields { + name + } + threeFields: fields { + name + } + } + } + } + } + } + `); + }); + + it('3 fields deep from varying parents introspection query', () => { + expectErrors(` + { + __schema { + types { + fields { + type { + fields { + type { + ofType { + fields { + name + } + } + } + } + } + } + } + } + } + `).toDeepEqual([ + { + message: 'Maximum introspection depth exceeded', + locations: [ + { + column: 7, + line: 3, + }, + ], + }, + ]); + }); + + it('3 fields deep introspection query with inline fragments', () => { + expectErrors(` + query test { + __schema { + types { + ... on __Type { + fields { + type { + ... on __Type { + ofType { + fields { + type { + ... on __Type { + fields { + name + } + } + } + } + } + } + } + } + } + } + } + } + `).toDeepEqual([ + { + message: 'Maximum introspection depth exceeded', + locations: [ + { + column: 7, + line: 3, + }, + ], + }, + ]); + }); + + it('3 fields deep introspection query with fragments', () => { + expectErrors(` + query test { + __schema { + types { + ...One + } + } + } + + fragment One on __Type { + fields { + type { + ...Two + } + } + } + + fragment Two on __Type { + fields { + type { + ...Three + } + } + } + + fragment Three on __Type { + fields { + name + } + } + `).toDeepEqual([ + { + message: 'Maximum introspection depth exceeded', + locations: [ + { + column: 7, + line: 3, + }, + ], + }, + ]); + }); + + it('3 fields deep inside inline fragment on query', () => { + expectErrors(` + { + ... { + __schema { types { fields { type { fields { type { fields { name } } } } } } } + } + } + `).toDeepEqual([ + { + message: 'Maximum introspection depth exceeded', + locations: [ + { + column: 9, + line: 4, + }, + ], + }, + ]); + }); + + it('opts out if fragment is missing', () => { + expectValid(` + query test { + __schema { + types { + ...Missing + } + } + } + `); + }); + + it("doesn't infinitely recurse on fragment cycle", () => { + expectValid(` + query test { + __schema { + types { + ...Cycle + } + } + } + fragment Cycle on __Type { + ...Cycle + } + `); + }); +}); diff --git a/src/validation/index.ts b/src/validation/index.ts index 58cc012ee8..f92ee51aa1 100644 --- a/src/validation/index.ts +++ b/src/validation/index.ts @@ -84,6 +84,8 @@ export { VariablesAreInputTypesRule } from './rules/VariablesAreInputTypesRule'; // Spec Section: "All Variable Usages Are Allowed" export { VariablesInAllowedPositionRule } from './rules/VariablesInAllowedPositionRule'; +export { MaxIntrospectionDepthRule } from './rules/MaxIntrospectionDepthRule'; + // SDL-specific validation rules export { LoneSchemaDefinitionRule } from './rules/LoneSchemaDefinitionRule'; export { UniqueOperationTypesRule } from './rules/UniqueOperationTypesRule'; diff --git a/src/validation/rules/MaxIntrospectionDepthRule.ts b/src/validation/rules/MaxIntrospectionDepthRule.ts new file mode 100644 index 0000000000..0c2dbd3879 --- /dev/null +++ b/src/validation/rules/MaxIntrospectionDepthRule.ts @@ -0,0 +1,91 @@ +import { GraphQLError } from '../../error/GraphQLError'; + +import type { ASTNode } from '../../language/ast'; +import { Kind } from '../../language/kinds'; +import type { ASTVisitor } from '../../language/visitor'; + +import type { ASTValidationContext } from '../ValidationContext'; + +const MAX_LISTS_DEPTH = 3; + +export function MaxIntrospectionDepthRule( + context: ASTValidationContext, +): ASTVisitor { + /** + * Counts the depth of list fields in "__Type" recursively and + * returns `true` if the limit has been reached. + */ + function checkDepth( + node: ASTNode, + visitedFragments: { + [fragmentName: string]: true | undefined; + } = Object.create(null), + depth: number = 0, + ): boolean { + if (node.kind === Kind.FRAGMENT_SPREAD) { + const fragmentName = node.name.value; + if (visitedFragments[fragmentName] === true) { + // Fragment cycles are handled by `NoFragmentCyclesRule`. + return false; + } + const fragment = context.getFragment(fragmentName); + if (!fragment) { + // Missing fragments checks are handled by `KnownFragmentNamesRule`. + return false; + } + + // Rather than following an immutable programming pattern which has + // significant memory and garbage collection overhead, we've opted to + // take a mutable approach for efficiency's sake. Importantly visiting a + // fragment twice is fine, so long as you don't do one visit inside the + // other. + try { + visitedFragments[fragmentName] = true; + return checkDepth(fragment, visitedFragments, depth); + } finally { + visitedFragments[fragmentName] = undefined; + } + } + + if ( + node.kind === Kind.FIELD && + // check all introspection lists + (node.name.value === 'fields' || + node.name.value === 'interfaces' || + node.name.value === 'possibleTypes' || + node.name.value === 'inputFields') + ) { + // eslint-disable-next-line no-param-reassign + depth++; + if (depth >= MAX_LISTS_DEPTH) { + return true; + } + } + + // handles fields and inline fragments + if ('selectionSet' in node && node.selectionSet) { + for (const child of node.selectionSet.selections) { + if (checkDepth(child, visitedFragments, depth)) { + return true; + } + } + } + + return false; + } + + return { + Field(node) { + if (node.name.value === '__schema' || node.name.value === '__type') { + if (checkDepth(node)) { + context.reportError( + new GraphQLError('Maximum introspection depth exceeded', { + nodes: [node], + }), + ); + return false; + } + } + }, + }; +} diff --git a/src/validation/specifiedRules.ts b/src/validation/specifiedRules.ts index 16e555db8a..4a90f265f3 100644 --- a/src/validation/specifiedRules.ts +++ b/src/validation/specifiedRules.ts @@ -67,6 +67,9 @@ import { VariablesAreInputTypesRule } from './rules/VariablesAreInputTypesRule'; import { VariablesInAllowedPositionRule } from './rules/VariablesInAllowedPositionRule'; import type { SDLValidationRule, ValidationRule } from './ValidationContext'; +// TODO: Spec Section +import { MaxIntrospectionDepthRule } from './rules/MaxIntrospectionDepthRule'; + /** * This set includes all validation rules defined by the GraphQL spec. * @@ -100,6 +103,8 @@ export const specifiedRules: ReadonlyArray = Object.freeze([ VariablesInAllowedPositionRule, OverlappingFieldsCanBeMergedRule, UniqueInputFieldNamesRule, + // Technically this isn't part of the spec but it's a strongly encouraged validation rule. + MaxIntrospectionDepthRule, ]); /** From c40e054dcf780f49192af3afe24673083db7100b Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Fri, 21 Jun 2024 10:03:21 +0100 Subject: [PATCH 11/12] Move recommended rules into their own array --- src/index.ts | 1 + src/validation/index.ts | 2 +- src/validation/specifiedRules.ts | 12 ++++++++---- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/index.ts b/src/index.ts index 597bfe310b..6185d9f444 100644 --- a/src/index.ts +++ b/src/index.ts @@ -339,6 +339,7 @@ export { ValidationContext, // All validation rules in the GraphQL Specification. specifiedRules, + recommendedRules, // Individual validation rules. ExecutableDefinitionsRule, FieldsOnCorrectTypeRule, diff --git a/src/validation/index.ts b/src/validation/index.ts index f92ee51aa1..587479e351 100644 --- a/src/validation/index.ts +++ b/src/validation/index.ts @@ -4,7 +4,7 @@ export { ValidationContext } from './ValidationContext'; export type { ValidationRule } from './ValidationContext'; // All validation rules in the GraphQL Specification. -export { specifiedRules } from './specifiedRules'; +export { specifiedRules, recommendedRules } from './specifiedRules'; // Spec Section: "Executable Definitions" export { ExecutableDefinitionsRule } from './rules/ExecutableDefinitionsRule'; diff --git a/src/validation/specifiedRules.ts b/src/validation/specifiedRules.ts index 4a90f265f3..c312c9839c 100644 --- a/src/validation/specifiedRules.ts +++ b/src/validation/specifiedRules.ts @@ -19,6 +19,8 @@ import { KnownTypeNamesRule } from './rules/KnownTypeNamesRule'; import { LoneAnonymousOperationRule } from './rules/LoneAnonymousOperationRule'; // SDL-specific validation rules import { LoneSchemaDefinitionRule } from './rules/LoneSchemaDefinitionRule'; +// TODO: Spec Section +import { MaxIntrospectionDepthRule } from './rules/MaxIntrospectionDepthRule'; // Spec Section: "Fragments must not form cycles" import { NoFragmentCyclesRule } from './rules/NoFragmentCyclesRule'; // Spec Section: "All Variable Used Defined" @@ -67,8 +69,11 @@ import { VariablesAreInputTypesRule } from './rules/VariablesAreInputTypesRule'; import { VariablesInAllowedPositionRule } from './rules/VariablesInAllowedPositionRule'; import type { SDLValidationRule, ValidationRule } from './ValidationContext'; -// TODO: Spec Section -import { MaxIntrospectionDepthRule } from './rules/MaxIntrospectionDepthRule'; +/** + * Technically these aren't part of the spec but they are strongly encouraged + * validation rules. + */ +export const recommendedRules = Object.freeze([MaxIntrospectionDepthRule]); /** * This set includes all validation rules defined by the GraphQL spec. @@ -103,8 +108,7 @@ export const specifiedRules: ReadonlyArray = Object.freeze([ VariablesInAllowedPositionRule, OverlappingFieldsCanBeMergedRule, UniqueInputFieldNamesRule, - // Technically this isn't part of the spec but it's a strongly encouraged validation rule. - MaxIntrospectionDepthRule, + ...recommendedRules, ]); /** From 294f1998594e280571cce32f5b872c39195ac7fc Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Fri, 21 Jun 2024 10:03:21 +0100 Subject: [PATCH 12/12] Move recommended rules into their own array --- src/index.ts | 1 + src/validation/index.ts | 2 +- src/validation/specifiedRules.ts | 12 ++++++++---- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/index.ts b/src/index.ts index 597bfe310b..6185d9f444 100644 --- a/src/index.ts +++ b/src/index.ts @@ -339,6 +339,7 @@ export { ValidationContext, // All validation rules in the GraphQL Specification. specifiedRules, + recommendedRules, // Individual validation rules. ExecutableDefinitionsRule, FieldsOnCorrectTypeRule, diff --git a/src/validation/index.ts b/src/validation/index.ts index f92ee51aa1..587479e351 100644 --- a/src/validation/index.ts +++ b/src/validation/index.ts @@ -4,7 +4,7 @@ export { ValidationContext } from './ValidationContext'; export type { ValidationRule } from './ValidationContext'; // All validation rules in the GraphQL Specification. -export { specifiedRules } from './specifiedRules'; +export { specifiedRules, recommendedRules } from './specifiedRules'; // Spec Section: "Executable Definitions" export { ExecutableDefinitionsRule } from './rules/ExecutableDefinitionsRule'; diff --git a/src/validation/specifiedRules.ts b/src/validation/specifiedRules.ts index 4a90f265f3..c312c9839c 100644 --- a/src/validation/specifiedRules.ts +++ b/src/validation/specifiedRules.ts @@ -19,6 +19,8 @@ import { KnownTypeNamesRule } from './rules/KnownTypeNamesRule'; import { LoneAnonymousOperationRule } from './rules/LoneAnonymousOperationRule'; // SDL-specific validation rules import { LoneSchemaDefinitionRule } from './rules/LoneSchemaDefinitionRule'; +// TODO: Spec Section +import { MaxIntrospectionDepthRule } from './rules/MaxIntrospectionDepthRule'; // Spec Section: "Fragments must not form cycles" import { NoFragmentCyclesRule } from './rules/NoFragmentCyclesRule'; // Spec Section: "All Variable Used Defined" @@ -67,8 +69,11 @@ import { VariablesAreInputTypesRule } from './rules/VariablesAreInputTypesRule'; import { VariablesInAllowedPositionRule } from './rules/VariablesInAllowedPositionRule'; import type { SDLValidationRule, ValidationRule } from './ValidationContext'; -// TODO: Spec Section -import { MaxIntrospectionDepthRule } from './rules/MaxIntrospectionDepthRule'; +/** + * Technically these aren't part of the spec but they are strongly encouraged + * validation rules. + */ +export const recommendedRules = Object.freeze([MaxIntrospectionDepthRule]); /** * This set includes all validation rules defined by the GraphQL spec. @@ -103,8 +108,7 @@ export const specifiedRules: ReadonlyArray = Object.freeze([ VariablesInAllowedPositionRule, OverlappingFieldsCanBeMergedRule, UniqueInputFieldNamesRule, - // Technically this isn't part of the spec but it's a strongly encouraged validation rule. - MaxIntrospectionDepthRule, + ...recommendedRules, ]); /**