-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Hide "did you mean" suggestions via internal plugin to avoid leaking schema information #7916
Hide "did you mean" suggestions via internal plugin to avoid leaking schema information #7916
Conversation
👷 Deploy request for apollo-server-docs pending review.Visit the deploys page to approve it
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great overall!
In the past, for security-related config options we've added (this is arguably less serious - it's been known and people either are ok with it or work around it), we went and updated all code examples in docs to include that config option. Wondering if we should do that for this case. I'll field @Meschreiber 's input on this one.
packages/server/src/__tests__/plugin/disableSuggestions/disableSuggestions.test.ts
Outdated
Show resolved
Hide resolved
…o disable spell checking everywhere!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @apollo/server@4.11.0 ### Minor Changes - [#7916](#7916) [`4686454`](4686454) Thanks [@andrewmcgivery](https://github.com/andrewmcgivery)! - Add `hideSchemaDetailsFromClientErrors` option to ApolloServer to allow hiding 'did you mean' suggestions from validation errors. Even with introspection disabled, it is possible to "fuzzy test" a graph manually or with automated tools to try to determine the shape of your schema. This is accomplished by taking advantage of the default behavior where a misspelt field in an operation will be met with a validation error that includes a helpful "did you mean" as part of the error text. For example, with this option set to `true`, an error would read `Cannot query field "help" on type "Query".` whereas with this option set to `false` it would read `Cannot query field "help" on type "Query". Did you mean "hello"?`. We recommend enabling this option in production to avoid leaking information about your schema to malicious actors. To enable, set this option to `true` in your `ApolloServer` options: ```javascript const server = new ApolloServer({ typeDefs, resolvers, hideSchemaDetailsFromClientErrors: true, }); ``` ## @apollo/server-integration-testsuite@4.11.0 ### Patch Changes - Updated dependencies \[[`4686454`](4686454)]: - @apollo/server@4.11.0 Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sorry for the delayed response on this, and glad to see y'all got this out the door! Agreed that best practice dictates including recommended configs. At the same time, it can be distracting in code samples that are trying to illustrate just one thing. Therefore, I think it only makes sense to include in more comprehensive samples. It would be nice to just have a consolidated list of these that we can auto-inject into an samples tagged "comprehensive". Creating a docs ticket for that. |
thanks for this! |
It was previously discussed (see: #3919) to wait for graphql/graphql-js#2247 to close, however, that issue has not moved in years and in the mean time libraries and frameworks seem to have opted for implementing their own solutions (E.g. https://github.com/Escape-Technologies/graphql-armor/blob/main/packages/plugins/block-field-suggestions/src/index.ts).
This should be a very low impact change that achieves the goal that would also be easy enough to rip out if this gets properly implemented in graphql-js later.
Adds
hideSchemaDetailsFromClientErrors
option to ApolloServer to allow hiding of these suggestions.Before:
Cannot query field "helloo" on type "Query". Did you mean "hello"?
After:
Cannot query field "helloo" on type "Query".
Fixes #3919