Limit the depth of introspection queries #6601
Conversation
The [schema-intropsection schema](https://spec.graphql.org/draft/#sec-Schema-Introspection.Schema-Introspection-Schema) is recursive: a client can query the fields of the types of some other fields, and so on arbitrarily deep. This can produce responses that grow much faster than the size of the request. To protect against abusive Router now refuses to execute introspection queries that nest list fields too deep and returns an error instead. The criteria matches `MaxIntrospectionDepthRule` in graphql-js, but may change in future versions. In case it rejects legitimate queries, this check can be disabled in Router configuration: ```yaml supergraph: introspection: true # Needed to enable schema introspection in the first place introspection_check_max_depth: false # Defaults to true ```
✅ Docs preview readyThe preview is ready to be viewed. View the preview File Changes 0 new, 1 changed, 0 removedBuild ID: 5f350051025f52f7c176ee7a URL: https://www.apollographql.com/docs/deploy-preview/5f350051025f52f7c176ee7a |
This comment has been minimized.
This comment has been minimized.
|
CI performance tests
|
There was a problem hiding this comment.
This is handled a little bit differently than our existing limits, and I am torn on whether it should be handled in a similar fashion. On one hand, it's definitely a limit configuration, but on the other hand, it's specific to graphql specification, and is closely tied to introspection implementation itself, which will make it hard to decouple the limiting functionality from introspection itself.
The one thing we do have to think about is the error we return regardless of where the actual limiting lives. Is there merit to returning the same http::StatusCode::PAYLOAD_TOO_LARGE error as the other limits do?
| /// Limit the depth of nested list fields in introspection queries | ||
| /// to protect avoid generating huge responses. | ||
| /// Default: false | ||
| pub(crate) introspection_check_max_depth: bool, | ||
|
|
There was a problem hiding this comment.
I am wondering if this should go here or under Limits config instead? It feels more of a limit, rather than a general supergraph configuration.
Before this comment I didn’t realize we have this guide already. I’ve added to it now. |
Co-authored-by: Iryna Shestak <[email protected]>
The schema-intropsection schema is recursive: a client can query the fields of the types of some other fields, and so on arbitrarily deep. This can produce responses that grow much faster than the size of the request.
To protect against abusive requests Router now refuses to execute introspection queries that nest list fields too deep and returns an error instead. The criteria matches
MaxIntrospectionDepthRulein graphql-js, but may change in future versions.In case it rejects legitimate queries, this check can be disabled in Router configuration:
Checklist
Complete the checklist (and note appropriate exceptions) before the PR is marked ready-for-review.
Exceptions
Note any exceptions here
Notes
Footnotes
It may be appropriate to bring upcoming changes to the attention of other (impacted) groups. Please endeavour to do this before seeking PR approval. The mechanism for doing this will vary considerably, so use your judgement as to how and when to do this. ↩
Configuration is an important part of many changes. Where applicable please try to document configuration examples. ↩
Tick whichever testing boxes are applicable. If you are adding Manual Tests, please document the manual testing (extensively) in the Exceptions. ↩