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

useMaskedErrors hides error's path #2091

Open
4 tasks
Warxcell opened this issue Nov 22, 2023 · 5 comments · May be fixed by #2096
Open
4 tasks

useMaskedErrors hides error's path #2091

Warxcell opened this issue Nov 22, 2023 · 5 comments · May be fixed by #2096

Comments

@Warxcell
Copy link

Warxcell commented Nov 22, 2023

Issue workflow progress

Progress of the issue based on the
Contributor Workflow

  • 1. The issue provides a
    minimal reproduction available on
    Stackblitz.
    • Please install the latest @envelop/* packages that you are using.
    • Please make sure the reproduction is as small as possible.
  • 2. A failing test has been provided
  • 3. A local solution has been provided
  • 4. A pull request is pending review

Describe the bug

when using useMaskedErrors plugin - it hides the path

image

To Reproduce Steps to reproduce the behavior:

add useMaskedErrors

useMaskedErrors({
   errorMessage: 'Oops! Something went wrong.',
}),

Expected behavior

useMaskedErrors to replace only the message, not the error.path
image

Environment:

  • OS: archlinux
  • NodeJS: 20
  • @envelop/* versions:
    • @envelop/core: 5.0.0

Additional context

@Warxcell Warxcell linked a pull request Nov 27, 2023 that will close this issue
9 tasks
@ardatan
Copy link
Collaborator

ardatan commented Nov 27, 2023

I think this is intentional. The idea is not to expose any detail about an unexpected error. You can configure error masking on your own if you want to expose path.

@Warxcell
Copy link
Author

Warxcell commented Nov 27, 2023

I think this is intentional. The idea is not to expose any detail about an unexpected error. You can configure error masking on your own if you want to expose path.

but the path is not really confidential detail.
I think consumer should know where the error occured - because otherwise it cannot know whether data is missing because of error, or because its just not existing.

@EmrysMyrddin
Copy link
Collaborator

It should be preserved if it is a GraphqlError, which will have a path. Otherwise, if it is an exception, you probably don't want to give any information about which resolver thrown this error on consumer side by default.

You can either have some instrumentation server side to log/store the unexpected exception, or you should add a mechanism to allow your customer, under strict and restricted circumstances, get access to the unmasked errors (like in a sandbox environment, a special role, or any kind of allow list like this)

@Warxcell
Copy link
Author

Warxcell commented Dec 5, 2023

The idea is not to give consumer unmasked error, but to know that there is SOME error ar certain path. Basically to know that data is missing, because of error, not just because.

@EmrysMyrddin
Copy link
Collaborator

I understand your use case, but for this, you should either manually handle errors and throw a GraphqlError or customise the formatError function.

Providing the path for an unknown internal error is actually giving precious informations to a potential attack. It can be used to know exactly which field is leading to error and use this information to investigate further for potential security breach.

It can be seen as a bit strict, but we want to provide good default security configuration. This way, we enforce that the user explicitly understand that this kind information will be available to consumers.

Perhaps we can have better documentation on this point ? Like providing some examples on how to loosen the default behaviour ?

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 a pull request may close this issue.

3 participants