Bound GraphQL nested-filter recursion depth - #3776
Bound GraphQL nested-filter recursion depth#3776Souvik Ghosh (souvikghosh04) wants to merge 2 commits into
Conversation
Thread a nesting-level counter through the recursive GraphQL filter parser and reject filters whose relationship nesting exceeds runtime.graphql.depth-limit (when set and stricter) or a hardcoded safety ceiling. Prevents nested-filter depth-bomb amplification into deeply correlated EXISTS subqueries, which the HotChocolate execution-depth rule does not cover.
There was a problem hiding this comment.
Pull request overview
This PR adds a guardrail to the GraphQL filter parser to cap relationship-nesting depth in filter:{ rel:{ rel:{ ... }}} scenarios, preventing small requests from expanding into very expensive nested query plans.
Changes:
- Introduces a maximum nested-filter depth (hard ceiling of 20) and enforces it during recursive filter parsing, optionally honoring a stricter
runtime.graphql.depth-limit. - Threads a
nestingLevelcounter through recursive parsing paths (Parse,HandleNestedFilterForSql/Cosmos,ParseAndOr) and returns HTTP 400 on violations. - Adds unit tests validating effective limit selection and the enforcement helper.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/Core/Models/GraphQLFilterParsers.cs |
Adds max-depth resolution/enforcement and propagates nesting depth through recursive parsing. |
src/Service.Tests/UnitTests/GraphQLFilterParserUnitTests.cs |
Adds unit tests for depth-limit resolution and guard enforcement behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Address review feedback: keep the original 4-parameter public GQLFilterParser.Parse for binary compatibility and move the nesting-depth-tracking logic into a separate private 5-parameter overload used for recursion.
Aniruddh Munde (Aniruddh25)
left a comment
There was a problem hiding this comment.
The depth guard itself looks sound, but the over-limit path does not produce the promised HTTP 400 response.
EnsureWithinNestedFilterDepth throws DataApiBuilderException with SubStatusCodes.BadRequest. The GraphQL error filter exposes that substatus as the error code, while DetermineStatusCodeMiddleware only maps SubStatusCodes.DatabaseInputError to HTTP 400. Consequently, an over-limit filter remains a normal GraphQL execution-error response rather than receiving the intended 400 status.
Please either use DatabaseInputError for this client input error or extend the status middleware to map BadRequest, and add an endpoint-level boundary test proving that 20 relationship levels succeed while 21 returns HTTP 400 with the expected GraphQL error.
Summary
Bounds the relationship-nesting depth of GraphQL filter arguments (e.g.
filter:{rel:{rel:{...}}}). Deeply nested filters expand into correlatedEXISTSsubqueries that the HotChocolate execution-depth rule does not cover, so a small deeply-nested request could amplify into a very expensive query.Change
GQLFilterParser.Parse/HandleNestedFilter*/ParseAndOr).runtime.graphql.depth-limit(when set and stricter) or a hardcoded safety ceiling (20) with an HTTP 400.Parseparameter is optional; external callers are unchanged.Tests
depth-limit = -1(unlimited) case still capped at the ceiling.Note
Filters nesting beyond 20 relationship levels now return 400 instead of executing; this is far above typical usage.