Skip to content
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

Provide a config to supress the 'Did you mean' message for GRAPHQL_VALIDATION_FAILED #5897

Open
zsolt-sb opened this issue Aug 27, 2024 · 8 comments

Comments

@zsolt-sb
Copy link

Is your feature request related to a problem? Please describe.

Remove any ability to introspect for security reasons

Using the getting started server and providing the following operation:

query ExampleQuery {
  __type
  me {
    id
  }
}

We get the response (Note the did you mean message)

{
  "errors": [
    {
      "message": "Field \"__type\" argument \"name\" of type \"String!\" is required, but it was not provided.",
      "locations": [
        {
          "line": 2,
          "column": 3
        }
      ],
      "extensions": {
        "code": "GRAPHQL_VALIDATION_FAILED"
      }
    },
    {
      "message": "Field \"__type\" of type \"Query\" must have a selection of subfields. Did you mean \"__type { ... }\"?",
      "locations": [
        {
          "line": 2,
          "column": 3
        }
      ],
      "extensions": {
        "code": "GRAPHQL_VALIDATION_FAILED"
      }
    }
  ]
}

Very recently the apollo server was given a config to disable this message.

Describe the solution you'd like

Would it be possible to provide a config to supress this message in the same way apollo server does?

Describe alternatives you've considered

We tried

supergraph:
  introspection: false

But this did not effect the message.

Additional context

Add any other context or screenshots about the feature request here.

@goto-bus-stop
Copy link
Member

Hi, can you confirm which Router version you are using?

For normal queries, we already use a validation implementation that does not produce "Did you mean" suggestions. So the Router does not leak information about your graph that way.

Introspection queries are as of today still resolved with graphql-js, which does include "Did you mean" suggestions. We are replacing this with a Rust-based implementation right now, and the suggestions will be removed once that is complete (within a few weeks).

@zsolt-sb
Copy link
Author

./router --version
1.52.0

Thanks for the update!

@goto-bus-stop
Copy link
Member

Note if introspection is disabled, you should already not get these messages today. Introspection is disabled by default in production environments.

@joemccall86
Copy link

I just disabled introspection in my router running locally, and I still got the "Did you mean" messages. Router v1.58.1

@goto-bus-stop
Copy link
Member

There are different kinds of "did you mean" messages that graphql-js originally introduced. We no longer use graphql-js for validation or introspection, but we mimick some of the error messages to avoid breaking customer snapshot tests and the like.

One of those error messages graphql-js "helpfully" provides is suggestions if you typoed a field name, listing out other field names in the schema that do exist. This was not ported to our Rust implementations and so they can not occur any more.

The error message saying "did you mean __type { ... }" does contain the text "did you mean" but it leaks no information at all about your schema on top of what the main error text says, so it is not a goal to remove those.

If you are concerned about any error message, please share the query that caused it and the whole error message, and what part of it is concerning (and ideally also the affected bits of the schema).

@joemccall86
Copy link

That makes sense. The original issue is indeed with the validation error message, which a bad actor with a valid token could use to determine the shape of the graph. After re-reading the guide we could mitigate this with either a Rhai script or by enabling persisted queries.

@zsolt-sb
Copy link
Author

The error message saying "did you mean __type { ... }" does contain the text "did you mean" but it leaks no information

I will have to set the server back up because originally it was not the did you mean part, but was the message

 "Field \"__type\" argument \"name\" of type \"String!\" is required, 

What I don't currently know is if this is a generic message or a legitimate field for the server. Will get back to you.

@goto-bus-stop
Copy link
Member

Got it, thank you. Missing required arguments to fields do indeed still get reported like that, both for introspection queries and any type of query against your schema.

Every validation message contains some information about the schema, the main concern with the graphql-js messages was that it would happily tell you about other things in the schema that you would otherwise never know.

There could be an argument for a feature that rejects invalid queries with no information on what exactly was invalid. I'm not sure it's easy to achieve with a Rhai script today, as there is no place in-between JSON parsing/serialization and GraphQL validation that you can hook into today. We've long wanted to add a JSON stage to the pipeline, if we had that you could easily redact any errors with the GRAPHQL_VALIDATION_FAILED error code.

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

No branches or pull requests

3 participants