Validation errors can be used to get schema details

[ravangen]

Similar to introspection, another way to probe a server for details about its schema is to submit invalid GraphQL documents so that the default validation rules provide data back. A variety of rules use didYouMean.js to give the developer suggestions about possible changes to get a valid document. This is ideal when developing against an API, but also is a channel for leaking information (like prototype features) with validation rules like FieldsOnCorrectType.

In production, some companies with internal schemas elect to disable introspection, where internally they can acquire their schema via other channels. Seemingly security through obscurity.

The validation rules still need to run, but it would be ideal if we could toggle the ability to provide suggestions as part of the error message. The ValidationContext does provide onError handler, but it seems a bit reactive and wasteful to then remove the computed suggestionList. This also ties into some ideas outlined in #2074.

For example, Apollo Server provides a convenient constructor option introspection flag to determine if an introspection query should be allowed. It would be an improved developer experience if this flag could inform the underlying GraphQL.js validation rules to restrict its error messaging without any additional configuration required.

[glasser]

We've gotten more requests for this from users of Apollo Server. Would graphql-js be open to a PR that adds a flag to validate's options allowing you to disable the didYouMean suggestions? The alternatives would be to provide alternate versions of all of the specified rules that provide suggestions, or manually stripping didYouMean from returned errors, neither of which feels particularly maintainable.

[leebyron]

Based on the OP intent, should there be a single flag to disable introspection that also disables didYouMean?

[ravangen]

I favour a more granular mechanism to disable didYouMean functionality if desired, whether for the introspection angle, or if there is a performance angle for large schemas (I haven't done any profiling).

[glasser]

@leebyron I think that would satisfy the particular use cases we've heard recommended, though it might also be nice to have the more granular version (eg, for people using validation rules exported by graphql-js directly).

For what it's worth, Apollo Server implements "no introspection" by adding the following validation rule:

const NoIntrospection = (context: ValidationContext) => ({
  Field(node: FieldDefinitionNode) {
    if (node.name.value === '__schema' || node.name.value === '__type') {
      context.reportError(
        new GraphQLError(
          'GraphQL introspection is not allowed by Apollo Server, but the query contained __schema or __type. To enable introspection, pass introspection: true to ApolloServer in production',
          [node],
        ),
      );
    }
  },
});

Is your proposal a flag to validate that would add a rule like the above and change the behavior of didYouMean rules?

[leebyron]

I don't really have a crisp proposal - just trying to clarify the need.

That seems like a reasonable way to implement introspection blocking, though I would worry about a theoretical misuse that creates a security risk where a query skips validation in some way and is still executed and exposes introspection. It might be nice if this was implemented within GraphQL.js that in addition to the clear error, it also simply wasn't possible to execute introspection queries on a schema with introspection disabled.

My concern with granular controls is that a user would disable introspection but not consider disabling didYouMean and inadvertently open this internal information leak. I would expect that a schema with introspection disabled would also disable didYouMean.

I can't think of a reason why you would want to disable introspection but enable didYouKnow or vice-versa.

[glasser]

Yes, I would expect that Apollo Server would take its introspection option and use it to control both of these features if they are separate. So for the AS use case it doesn't really matter too much if it's one feature or two... though if the features were separate we might consider still using our own NoIntrospection rule just to keep the behavior that the error message tells the user what option to pass to our API to control it?

Re your theoretical risk, I do feel that if disableIntrospection were an option to validate, then structurally it would be pretty clear that you need to run validate in order to disable introspection? So my inclination would be to add disableIntrospection as an option to validate and allow that to be enough.

If it were an execute option too then I guess the behavior of resolving an introspection field without validation would be to just leave the __schema field out of the response, just as it appears execution currently does for unknown fields without validation?

[sahanatroam]

I'm happy to help here and I do agree with @leebyron where if introspection is set to false didYouMean method should not return any suggestions.

Can the options object be used here to accept the introspection value from the consumer?

export function validate(
  schema: GraphQLSchema,
  documentAST: DocumentNode,
  rules: ReadonlyArray<ValidationRule> = specifiedRules,
  options?: { maxErrors?: number },

  /** @deprecated will be removed in 17.0.0 */
  typeInfo: TypeInfo = new TypeInfo(schema),
)

In addition to this we could also create a method getSpecifiedRules({ introspection: false }) so that consumers could pass in option so that errors returned would respect the options passed in.

Please let me know if this needs to go through an RFC process and gather feedback.