Skip to content

docs: add guide for operation complexity controls #4402

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: 16.x.x
Choose a base branch
from

Conversation

sarahxsanders
Copy link
Contributor

@sarahxsanders sarahxsanders commented May 20, 2025

Adds guide "Operation Complexity Controls"

Copy link
Member

@benjie benjie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is really good, and I'm excited to see us addressing this topic! However, ideally we should not promote a particular module for complexity analysis (unless it's one we control), otherwise we open ourselves to adding that to GraphQL.js' security surface area, and we artificially limit competition/innovation within the ecosystem. Instead we should talk about it in more general terms (and perhaps give a few examples of modules that can be used for this). In addition:

  1. We should strongly encourage the use of trusted documents over complexity limits at runtime
  2. We should note that complexity limits can be subject to attack themselves (for example, via fragment cycles), so it's generally best to run them after validation but before execution unless we know they're safe to run alongside validation
  3. The validation phase doesn't have access to variables (validation tends to happen once per document independent of variables) so complexity limits depending on variables will need special plumbing
  4. Certain field types make operations more complex, in particular lists (and, to a lesser extent, abstract types); we should comment how these "multiply up" the complexity.
  5. We should note that this should form part of a layered security approach - referencing https://graphql.org/learn/security/
  6. We should note that with trusted documents, complexity analysis can still be valuable to warn engineers about expensive operations, and can mostly be done at build-time rather than run-time.
  7. We should be careful with the use of the word "query", because we're talking about mutations and subscriptions too really. Consider "operation" or "document" as appropriate (glossary), or simply elide "query" entirely: "complexity controls".

I don't think you need to change the document very much to accommodate this feedback - e.g. you can keep all the examples, just make it very clear that they're just examples from one of the modules and other approaches are available.

@sarahxsanders sarahxsanders changed the title docs: add guide for query complexity controls docs: add guide for operation complexity controls May 21, 2025
@sarahxsanders
Copy link
Contributor Author

@benjie thanks for much for the thorough feedback! My next commit has the following updates:

  • Renamed the guide to "Operation Complexity Controls" and replaced mentions of "query complexity"
  • Clarified tool neutrality, making it clear that graphql-query-complexity is one example among several (happy to expand on this more if needed)
  • Added some callouts to address some of your other points (trusted documents, cycle safety and validation timing, etc.)
  • Reworked the best practices to align with updates from your feedback
  • Tried to position complexity analysis as part of a layered security approach instead of a standalone solution

Please let me know if there's anything else we should update! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants