Disable suggestions in errors message

[Sytten]

Context

We have some mutation defined:

  extend type Mutation {
    updateUserDetail(input: UpdateUserDetailsInput!): UpdateUserDetailsPayload
  }

The user sends the following query:

mutation {
  updateUserDetil {
    user {
        id
    }
  }
}

The server will response with an error maintaining in the message: "message": "Cannot query field \"updateUserDetil\" on type \"Mutation\". Did you mean \"updateUserDetail\"?",

The problem

In case of a private API, we generally want to avoid leaking information about our API. Disabling the introspection is a good step, but the recommendations are leaking some information that can be used by attackers. This talk discuss this issue (from the perspective of a pentester).

Propositions

1. Remove suggestions for any environment that is not development
2. Add a setting to force override the previous change

[Sytten]

The tool Shapeshifter uses that leakage of information to identify fields automatically. View the tool in action later in the same talk as previously linked.

[supermonkeybrainz]

This is a hard requirement from the security team at my client. If no resolution is found we my need to abandon Apollo server, which would be devastating to our project. Please make this available if possible, or provide documentation on how to configure. Thanks!

[Sytten]

@supermonkeybrainz honestly I think you probably better off summiting a fix for that. I don't really think the Apollo team cares about you since you don't bring them any revenue (no offence). If this client plans to use the commercial offering of Apollo and this is a deal breaker, I suggest you contact their customer reps.

[supermonkeybrainz]

@Sytten I was admittedly a little overdramatic in my post, but I wanted to convey the seriousness of this issue. There are workarounds (express middleware, error formatter, etc.). I did look into fixing this. After further digging I discovered the issue is in the graphql library. There is an existing issue. Not much the Apollo folks (or myself) can do here until that is fixed.

[enriquedacostacambio]

@supermonkeybrainz sort of a hack, but you could use either fork graphql-js or use patch-package to add this single line:

diff --git a/node_modules/graphql/jsutils/didYouMean.js b/node_modules/graphql/jsutils/didYouMean.js
index 43640da..da8daf9 100644
--- a/node_modules/graphql/jsutils/didYouMean.js
+++ b/node_modules/graphql/jsutils/didYouMean.js
@@ -11,6 +11,7 @@ var MAX_SUGGESTIONS = 5;
 
 // eslint-disable-next-line no-redeclare
 function didYouMean(firstArg, secondArg) {
+  return ''; // until https://github.com/graphql/graphql-js/issues/2247 or https://github.com/apollographql/apollo-server/issues/3919 are resolved.
   var _ref = typeof firstArg === 'string' ? [firstArg, secondArg] : [undefined, firstArg],
       subMessage = _ref[0],
       suggestions = _ref[1];

[DeeJEarl88]

Just found a work around for this and thought i'd share. Use the FormatError feature and mask any errors you don't want the client seeing. In my case i mask them all.

const apolloServer = new ApolloServer({
  typeDefs: globalTypeDefs,
  resolvers: globalResolvers,
  context:  () => {},
  datasources: () => {},
  ...
  // Mask errors from clients to mitigate security attacks
  formatError: (err) => {
    if (process.env.APP_ENV === 'production') {
      captureException(err);
      return new Error('Internal Server Error');
    }
    return err;
  },
});

[abenhamdine]

perhaps a more suited workaround should be :

import { ValidationError } from "apollo-server-express"
import { GraphQLError, GraphQLFormattedError } from "graphql"
import { NODE_ENV } from "../constants"

export const formatGqlError = (error: GraphQLError): GraphQLFormattedError<Recor
    if (process.env.NODE_ENV !== NODE_ENV.DEV) {
        if (error instanceof ValidationError) {
            return new ValidationError("Invalid request.")
        }
    }

[glasser]

This fix needs to be implemented in graphql-js. There's an issue tracking it in graphql/graphql-js#2247

If that is implemented, we would change introspection: false to automatically disable "did you know" messages when graphql-js allows that.

This is not currently actionable, so I'm going to close it; if the graphql-js issue gets fixed and we don't already do this, feel free to ping or open a new issue.