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

Hide "did you mean" suggestions via internal plugin to avoid leaking schema information #7916

Merged

Conversation

andrewmcgivery
Copy link
Contributor

@andrewmcgivery andrewmcgivery commented Aug 7, 2024

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

Copy link

netlify bot commented Aug 7, 2024

👷 Deploy request for apollo-server-docs pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit 03e9b8b

Copy link

codesandbox-ci bot commented Aug 7, 2024

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.

@andrewmcgivery andrewmcgivery changed the title ApolloServerPluginDisableSuggestions plugin initial implementation Hide "did you mean" suggestions via internal plugin to avoid leaking schema information Aug 7, 2024
@andrewmcgivery andrewmcgivery marked this pull request as ready for review August 7, 2024 22:12
Copy link
Member

@trevor-scheer trevor-scheer left a 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.

Copy link
Member

@trevor-scheer trevor-scheer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@trevor-scheer trevor-scheer merged commit 4686454 into apollographql:main Aug 8, 2024
19 checks passed
@github-actions github-actions bot mentioned this pull request Aug 8, 2024
trevor-scheer pushed a commit that referenced this pull request Aug 8, 2024
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>
@Meschreiber
Copy link
Contributor

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.

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.

@Meschreiber Meschreiber mentioned this pull request Aug 9, 2024
@JonHX
Copy link

JonHX commented Aug 29, 2024

thanks for this!

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

Successfully merging this pull request may close these issues.

Disable suggestions in errors message
5 participants