diff --git a/.changeset/tough-weeks-yawn.md b/.changeset/tough-weeks-yawn.md new file mode 100644 index 00000000..151f02db --- /dev/null +++ b/.changeset/tough-weeks-yawn.md @@ -0,0 +1,5 @@ +--- +'@escape.tech/graphql-armor-max-aliases': minor +--- + +add fragmentRecursionCost diff --git a/.changeset/wicked-shrimps-arrive.md b/.changeset/wicked-shrimps-arrive.md new file mode 100644 index 00000000..17514c50 --- /dev/null +++ b/.changeset/wicked-shrimps-arrive.md @@ -0,0 +1,7 @@ +--- +'@escape.tech/graphql-armor-max-aliases': patch +'@escape.tech/graphql-armor-cost-limit': patch +'@escape.tech/graphql-armor-max-depth': patch +--- + +graceful handler for recursive fragments diff --git a/examples/apollo/test/index.spec.ts b/examples/apollo/test/index.spec.ts index b46b6248..bd537760 100644 --- a/examples/apollo/test/index.spec.ts +++ b/examples/apollo/test/index.spec.ts @@ -56,7 +56,9 @@ describe('startup', () => { }`, }); expect(query.errors).toBeDefined(); - expect(query.errors?.map((e) => e.message)).toContain('Syntax Error: Query Cost limit of 100 exceeded, found 138.'); + expect(query.errors?.map((e) => e.message)).toContain( + 'Syntax Error: Query Cost limit of 100 exceeded, found 5023.', + ); }); it('should block field suggestion', async () => { diff --git a/examples/yoga/test/index.spec.ts b/examples/yoga/test/index.spec.ts index 5de3857b..316a263a 100644 --- a/examples/yoga/test/index.spec.ts +++ b/examples/yoga/test/index.spec.ts @@ -68,7 +68,7 @@ describe('startup', () => { const body = JSON.parse(response.text); expect(body.data?.books).toBeUndefined(); expect(body.errors).toBeDefined(); - expect(body.errors?.map((e) => e.message)).toContain('Syntax Error: Query Cost limit of 100 exceeded, found 138.'); + expect(body.errors?.map((e) => e.message)).toContain('Syntax Error: Query Cost limit of 100 exceeded, found 5023.'); }); it('should disable field suggestion', async () => { diff --git a/packages/plugins/cost-limit/src/index.ts b/packages/plugins/cost-limit/src/index.ts index d0f293f4..8ae15bcc 100644 --- a/packages/plugins/cost-limit/src/index.ts +++ b/packages/plugins/cost-limit/src/index.ts @@ -17,12 +17,14 @@ export type CostLimitOptions = { scalarCost?: number; depthCostFactor?: number; ignoreIntrospection?: boolean; + fragmentRecursionCost?: number; } & GraphQLArmorCallbackConfiguration; const costLimitDefaultOptions: Required = { maxCost: 5000, objectCost: 2, scalarCost: 1, depthCostFactor: 1.5, + fragmentRecursionCost: 1000, ignoreIntrospection: true, onAccept: [], onReject: [], @@ -34,6 +36,7 @@ class CostLimitVisitor { private readonly context: ValidationContext; private readonly config: Required; + private readonly visitedFragments: Set = new Set(); constructor(context: ValidationContext, options?: CostLimitOptions) { this.context = context; @@ -93,6 +96,10 @@ class CostLimitVisitor { } if (node.kind == Kind.FRAGMENT_SPREAD) { + if (this.visitedFragments.has(node.name.value)) { + return this.config.fragmentRecursionCost; + } + this.visitedFragments.add(node.name.value); const fragment = this.context.getFragment(node.name.value); if (fragment) { cost += this.config.depthCostFactor * this.computeComplexity(fragment, depth + 1); diff --git a/packages/plugins/cost-limit/test/index.spec.ts b/packages/plugins/cost-limit/test/index.spec.ts index 7a4f45c8..0e701acb 100644 --- a/packages/plugins/cost-limit/test/index.spec.ts +++ b/packages/plugins/cost-limit/test/index.spec.ts @@ -139,4 +139,36 @@ describe('global', () => { `Syntax Error: Query Cost limit of ${maxCost} exceeded, found ${maxCost + 1}.`, ]); }); + + it('should not crash on recursive fragment', async () => { + const testkit = createTestkit( + [ + costLimitPlugin({ + maxCost: 50, + objectCost: 4, + scalarCost: 2, + depthCostFactor: 2, + ignoreIntrospection: true, + }), + ], + schema, + ); + const result = await testkit.execute(`query { + ...A + } + + fragment A on Query { + ...B + } + + fragment B on Query { + ...A + } + `); + assertSingleExecutionValue(result); + expect(result.errors).toBeDefined(); + expect(result.errors?.map((error) => error.message)).toContain( + 'Syntax Error: Query Cost limit of 50 exceeded, found 16050.', + ); + }); }); diff --git a/packages/plugins/max-aliases/src/index.ts b/packages/plugins/max-aliases/src/index.ts index 9ece3053..9d9a2077 100644 --- a/packages/plugins/max-aliases/src/index.ts +++ b/packages/plugins/max-aliases/src/index.ts @@ -6,6 +6,7 @@ import { FragmentSpreadNode, GraphQLError, InlineFragmentNode, + Kind, OperationDefinitionNode, ValidationContext, } from 'graphql'; @@ -23,6 +24,7 @@ class MaxAliasesVisitor { private readonly context: ValidationContext; private readonly config: Required; + private readonly visitedFragments: Set = new Set(); constructor(context: ValidationContext, options?: MaxAliasesOptions) { this.context = context; @@ -64,10 +66,14 @@ class MaxAliasesVisitor { ++aliases; } if ('selectionSet' in node && node.selectionSet) { - for (let child of node.selectionSet.selections) { + for (const child of node.selectionSet.selections) { aliases += this.countAliases(child); } - } else if (node.kind === 'FragmentSpread') { + } else if (node.kind === Kind.FRAGMENT_SPREAD) { + if (this.visitedFragments.has(node.name.value)) { + return 0; + } + this.visitedFragments.add(node.name.value); const fragment = this.context.getFragment(node.name.value); if (fragment) { aliases += this.countAliases(fragment); diff --git a/packages/plugins/max-aliases/test/index.spec.ts b/packages/plugins/max-aliases/test/index.spec.ts index 5a3fc60c..ff7cf4f3 100644 --- a/packages/plugins/max-aliases/test/index.spec.ts +++ b/packages/plugins/max-aliases/test/index.spec.ts @@ -103,4 +103,23 @@ describe('global', () => { `Syntax Error: Aliases limit of ${maxAliases} exceeded, found ${maxAliases + 1}.`, ]); }); + + it('should not crash on recursive fragment', async () => { + const testkit = createTestkit([maxAliasesPlugin({ n: 3 })], schema); + const result = await testkit.execute(`query { + ...A + } + + fragment A on Query { + ...B + } + + fragment B on Query { + ...A + } + `); + assertSingleExecutionValue(result); + expect(result.errors).toBeDefined(); + expect(result.errors?.map((error) => error.message)).toContain('Cannot spread fragment "A" within itself via "B".'); + }); }); diff --git a/packages/plugins/max-depth/src/index.ts b/packages/plugins/max-depth/src/index.ts index c7bf0dfe..4238f8c7 100644 --- a/packages/plugins/max-depth/src/index.ts +++ b/packages/plugins/max-depth/src/index.ts @@ -25,6 +25,7 @@ class MaxDepthVisitor { private readonly context: ValidationContext; private readonly config: Required; + private readonly visitedFragments: Set = new Set(); constructor(context: ValidationContext, options?: MaxDepthOptions) { this.context = context; @@ -60,7 +61,7 @@ class MaxDepthVisitor { private countDepth( node: FieldNode | FragmentDefinitionNode | InlineFragmentNode | OperationDefinitionNode | FragmentSpreadNode, - parentDepth: number = 0, + parentDepth = 0, ): number { if (this.config.ignoreIntrospection && 'name' in node && node.name?.value === '__schema') { return 0; @@ -68,10 +69,14 @@ class MaxDepthVisitor { let depth = parentDepth; if ('selectionSet' in node && node.selectionSet) { - for (let child of node.selectionSet.selections) { + for (const child of node.selectionSet.selections) { depth = Math.max(depth, this.countDepth(child, parentDepth + 1)); } } else if (node.kind == Kind.FRAGMENT_SPREAD) { + if (this.visitedFragments.has(node.name.value)) { + return 0; + } + this.visitedFragments.add(node.name.value); const fragment = this.context.getFragment(node.name.value); if (fragment) { depth = Math.max(depth, this.countDepth(fragment, parentDepth + 1)); diff --git a/packages/plugins/max-depth/test/index.spec.ts b/packages/plugins/max-depth/test/index.spec.ts index 355c227f..4641553f 100644 --- a/packages/plugins/max-depth/test/index.spec.ts +++ b/packages/plugins/max-depth/test/index.spec.ts @@ -119,4 +119,25 @@ describe('global', () => { expect(result.errors).toBeUndefined(); expect(result.data?.__schema).toBeDefined(); }); + + it('should not crash on recursive fragment', async () => { + const testkit = createTestkit([maxDepthPlugin({ n: 3 })], schema); + const result = await testkit.execute(`query { + ...A + } + + fragment A on Query { + ...B + } + + fragment B on Query { + ...A + } + `); + assertSingleExecutionValue(result); + expect(result.errors).toBeDefined(); + expect(result.errors?.map((error) => error.message)).toContain( + 'Syntax Error: Query depth limit of 3 exceeded, found 4.', + ); + }); });