From 12e30adf672c9668fb03d8bc75d8d76e5fd6dc19 Mon Sep 17 00:00:00 2001 From: Benjie Date: Fri, 21 Jun 2024 14:03:03 +0100 Subject: [PATCH] backport[v15]: Introduce "recommended" validation rules (#4120) Co-authored-by: enisdenjo --- integrationTests/ts/test.js | 2 + src/index.d.ts | 1 + src/index.js | 2 + .../MaxIntrospectionDepthRule-test.js | 554 ++++++++++++++++++ src/validation/index.d.ts | 2 +- src/validation/index.js | 4 +- .../rules/MaxIntrospectionDepthRule.d.ts | 6 + .../rules/MaxIntrospectionDepthRule.js | 90 +++ src/validation/specifiedRules.d.ts | 6 + src/validation/specifiedRules.js | 10 + 10 files changed, 675 insertions(+), 2 deletions(-) create mode 100644 src/validation/__tests__/MaxIntrospectionDepthRule-test.js create mode 100644 src/validation/rules/MaxIntrospectionDepthRule.d.ts create mode 100644 src/validation/rules/MaxIntrospectionDepthRule.js diff --git a/integrationTests/ts/test.js b/integrationTests/ts/test.js index 158aee4cf6..fe5c9fa195 100644 --- a/integrationTests/ts/test.js +++ b/integrationTests/ts/test.js @@ -1,5 +1,6 @@ 'use strict'; +const fs = require('fs'); const path = require('path'); const childProcess = require('child_process'); @@ -13,5 +14,6 @@ for (const version of tsVersions) { console.log(`Testing on ${version} ...`); const tscPath = path.join(__dirname, 'node_modules', version, 'bin/tsc'); + fs.chmodSync(tscPath, 0o755); childProcess.execSync(tscPath, { stdio: 'inherit' }); } diff --git a/src/index.d.ts b/src/index.d.ts index e0707adeaa..cf4b0281ca 100644 --- a/src/index.d.ts +++ b/src/index.d.ts @@ -317,6 +317,7 @@ export { ValidationContext, // All validation rules in the GraphQL Specification. specifiedRules, + recommendedRules, // Individual validation rules. ExecutableDefinitionsRule, FieldsOnCorrectTypeRule, diff --git a/src/index.js b/src/index.js index 4387ce14fa..72ba381d23 100644 --- a/src/index.js +++ b/src/index.js @@ -303,6 +303,7 @@ export { ValidationContext, // All validation rules in the GraphQL Specification. specifiedRules, + recommendedRules, // Individual validation rules. ExecutableDefinitionsRule, FieldsOnCorrectTypeRule, @@ -330,6 +331,7 @@ export { ValuesOfCorrectTypeRule, VariablesAreInputTypesRule, VariablesInAllowedPositionRule, + MaxIntrospectionDepthRule, // SDL-specific validation rules LoneSchemaDefinitionRule, UniqueOperationTypesRule, diff --git a/src/validation/__tests__/MaxIntrospectionDepthRule-test.js b/src/validation/__tests__/MaxIntrospectionDepthRule-test.js new file mode 100644 index 0000000000..f8a979bda5 --- /dev/null +++ b/src/validation/__tests__/MaxIntrospectionDepthRule-test.js @@ -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).to.deep.equal([]); +} + +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 + } + } + } + } + } + } + } + } + `).to.deep.equal([ + { + 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 + } + } + } + } + } + } + `).to.deep.equal([ + { + 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 + } + } + } + } + } + } + `).to.deep.equal([ + { + 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 + } + } + } + } + } + } + } + } + } + `).to.deep.equal([ + { + 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 + } + } + } + } + } + } + } + } + `).to.deep.equal([ + { + 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 + } + } + } + } + } + } + } + } + `).to.deep.equal([ + { + 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 + } + } + } + } + } + } + } + } + `).to.deep.equal([ + { + 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 + } + } + } + } + } + } + } + } + } + `).to.deep.equal([ + { + 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 + } + } + } + } + } + } + } + } + } + } + } + } + `).to.deep.equal([ + { + 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 + } + } + `).to.deep.equal([ + { + 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 } } } } } } } + } + } + `).to.deep.equal([ + { + 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.d.ts b/src/validation/index.d.ts index f049bf397e..0ab588cc78 100644 --- a/src/validation/index.d.ts +++ b/src/validation/index.d.ts @@ -2,7 +2,7 @@ export { validate } from './validate'; export { ValidationContext, ValidationRule } from './ValidationContext'; -export { specifiedRules } from './specifiedRules'; +export { specifiedRules, recommendedRules } from './specifiedRules'; // Spec Section: "Executable Definitions" export { ExecutableDefinitionsRule } from './rules/ExecutableDefinitionsRule'; diff --git a/src/validation/index.js b/src/validation/index.js index c0f24a0316..6956c3c818 100644 --- a/src/validation/index.js +++ b/src/validation/index.js @@ -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'; @@ -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.d.ts b/src/validation/rules/MaxIntrospectionDepthRule.d.ts new file mode 100644 index 0000000000..0c8e173507 --- /dev/null +++ b/src/validation/rules/MaxIntrospectionDepthRule.d.ts @@ -0,0 +1,6 @@ +import { ASTVisitor } from '../../language/visitor'; +import { SDLValidationContext } from '../ValidationContext'; + +export function MaxIntrospectionDepthRule( + context: SDLValidationContext, +): ASTVisitor; diff --git a/src/validation/rules/MaxIntrospectionDepthRule.js b/src/validation/rules/MaxIntrospectionDepthRule.js new file mode 100644 index 0000000000..49c7af4f26 --- /dev/null +++ b/src/validation/rules/MaxIntrospectionDepthRule.js @@ -0,0 +1,90 @@ +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 | null, + __proto__: null, + } = 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] = null; + } + } + + 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') + ) { + // $FlowFixMe[reassign-const] why are argument parameters treated as const in flow? + depth++; // eslint-disable-line no-param-reassign + 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', [node]), + ); + return false; + } + } + }, + }; +} diff --git a/src/validation/specifiedRules.d.ts b/src/validation/specifiedRules.d.ts index ffb5570894..e7264bd1a5 100644 --- a/src/validation/specifiedRules.d.ts +++ b/src/validation/specifiedRules.d.ts @@ -1,5 +1,11 @@ import { ValidationRule, SDLValidationRule } from './ValidationContext'; +/** + * Technically these aren't part of the spec but they are strongly encouraged + * validation rules. + */ +export const recommendedRules: ReadonlyArray; + /** * This set includes all validation rules defined by the GraphQL spec. * diff --git a/src/validation/specifiedRules.js b/src/validation/specifiedRules.js index 72f25d9490..5c3e78b9a2 100644 --- a/src/validation/specifiedRules.js +++ b/src/validation/specifiedRules.js @@ -82,6 +82,9 @@ import { OverlappingFieldsCanBeMergedRule } from './rules/OverlappingFieldsCanBe // Spec Section: "Input Object Field Uniqueness" import { UniqueInputFieldNamesRule } from './rules/UniqueInputFieldNamesRule'; +// TODO: Spec Section +import { MaxIntrospectionDepthRule } from './rules/MaxIntrospectionDepthRule'; + // SDL-specific validation rules import { LoneSchemaDefinitionRule } from './rules/LoneSchemaDefinitionRule'; import { UniqueOperationTypesRule } from './rules/UniqueOperationTypesRule'; @@ -91,6 +94,12 @@ import { UniqueFieldDefinitionNamesRule } from './rules/UniqueFieldDefinitionNam import { UniqueDirectiveNamesRule } from './rules/UniqueDirectiveNamesRule'; import { PossibleTypeExtensionsRule } from './rules/PossibleTypeExtensionsRule'; +/** + * 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. * @@ -124,6 +133,7 @@ export const specifiedRules = Object.freeze([ VariablesInAllowedPositionRule, OverlappingFieldsCanBeMergedRule, UniqueInputFieldNamesRule, + ...recommendedRules, ]); /**