Configurable query depth limiting

[lennyburdette]

Is your feature request related to a problem? Please describe. The OWASP GraphQL security guide recommends limiting the allowed depth of operations: https://cheatsheetseries.owasp.org/cheatsheets/GraphQL_Cheat_Sheet.html#query-limiting-depth-amount

Describe the solution you'd like The apollo-parser crate has a with_recursion_limit that adds an error on the parser result when the recursion limit is exceeded: https://github.com/apollographql/apollo-rs/blob/d22bfcd6dc36e359e31d9cb68cbea38807fd3406/crates/apollo-parser/src/parser/mod.rs#L116-L120

It'd be great if the recursion limit for operation parsing was configurable, and the parser error is used to generate the appropriate error response.

Describe alternatives you've considered I half-wrote an extension that did this manually before Iryna pointed out that this was built into the parser already.

Additional context I know of a couple of customers that would configure this right away if it was available.

[garypen]

also see: #1268

[garypen]

@lennyburdette Does the config proposal in #1268 meet your requirements?

[lennyburdette]

Possibly! A couple of thoughts:

• This setting will affect parsing supergraph schemas as well as operation documents right? Does that matter? I'm really only looking for depth limiting on operations.
• I wonder how many folks will recognize recursion_limit when they're looking for "depth" limiting — that's the terminology I see most frequently.
• Will the response be spec-compliant GraphQL with { data: null, errors: [{ message: "something about recursion limits" }] }? If not, is there a hook for formatting the error?

[SimonSapin]

• The apollo-parser crate has a recursion limit in all cases, but the router can configure it differently for supergraph schemas and for operations.
• How about naming it operation_max_depth for example?
• Yes, all errors from the router are formatted in this way. There isn’t a formatting hook. (Is that also a need?)

[lennyburdette]

• operation_max_depth is perfect
• a formatting hook as a rhai script is a nice future ask (maybe this is actually possible already with router_service.map_response?)

[lennyburdette]

Just had an additional thought: the customer asking for this feature also requested the ability to customize metric labels based on the response body: https://github.com/apollographql/router/issues/1198

They use metrics to track types of GraphQL errors by inspecting attributes on the errors in the response. They'll also want to track the rate of depth limit errors, so maybe we do want a way to format the error.

Or we can standardize on an error like:

{
  "data": null,
  "errors": [
    {
      "message": "operation depth limit exceeded",
      "extensions": {
        "code": "OPERATION_DEPTH_LIMIT"
      }
    }
  ]
}

And they would add metrics config like:

telemetry:
  metrics: 
    common:
      attributes:
        router:
          response:
            - body:
                path: errors.extensions.code
                name: code  

To create metrics that look like:

http_requests_total{code="OPERATION_DEPTH_LIMIT",operation_name="AllPandas",status="200"} 1

But I don't know if we want to decide on a specific error extensions format, or provide a hook for capturing an operation depth limit error and formatting it however the customer sees fit.

[timbotnik]

@lennyburdette From the Studio side I feel pretty confident that standardizing on ways to categorize errors would be beneficial for a lot of folks. I'd be interested in watching this space and collaborating on an E2E solution here (of course proving that this feature has value in Router is a great place to start).

[SimonSapin]

I’ve updated the title to reflect discussion in https://github.com/apollographql/router/pull/1437 that the parser recursion limit is not quite the desired query depth limit.

[viktorvoltaire]

Any update on this? Is it something we can expect to be implemented?

[lennyburdette]

I have an example of a plugin for this: https://github.com/apollosolutions/router-basic-operation-cost/blob/main/src/plugins/basic_depth_limit.rs

There's a current caveat that it re-parses the schema on every request, but we'll hopefully improve that in a couple of weeks.

[viktorvoltaire]

Nice, its a start! 👍

[SimonSapin]

We shipped operation limits in Apollo Router 1.17:

• https://github.com/apollographql/router/blob/dev/CHANGELOG.md#1170---2023-05-04
• https://www.apollographql.com/blog/announcement/backend/prevent-graph-misuse-with-operation-size-and-complexity-limits/