feat: optionally mask parse and validation errors (ardatan#1351)

* feat: mask parse and validation errors * Update .changeset/light-eagles-promise.md * feat: rename config option + add context * Apply suggestions from code review
klippx · Mar 31, 2022 · d5115b4 · d5115b4
1 parent ada7fb0
commit d5115b4
Show file tree

Hide file tree

Showing 7 changed files with 145 additions and 33 deletions.
diff --git a/.changeset/light-eagles-promise.md b/.changeset/light-eagles-promise.md
@@ -0,0 +1,16 @@
+---
+'@envelop/core': minor
+---
+
+allow masking validation and parse errors with `useMaskedErrors`.
+
+```ts
+useMaskedErrors({
+  handleParseErrors: true,
+  handleValidateErrors: true,
+});
+```
+
+This option is disabled by default as validation and parse errors are expected errors that help the API consumer instead of leaking secret information.
+
+If you want to avoid leaking schema suggestions, we recommend using persisted operations.
diff --git a/.changeset/sweet-badgers-bake.md b/.changeset/sweet-badgers-bake.md
@@ -0,0 +1,6 @@
+---
+'@envelop/core': minor
+'@envelop/types': minor
+---
+
+add `setResult` to `AfterValidateEventPayload` for altering the validation errors.
diff --git a/packages/core/src/orchestrator.ts b/packages/core/src/orchestrator.ts
@@ -257,6 +257,9 @@ export function createEnvelopOrchestrator<PluginsContext extends DefaultContext>
             extendContext: extension => {
               Object.assign(context, extension);
             },
+            setResult: newResult => {
+              result = newResult;
+            },
           });
         }
 

diff --git a/packages/core/src/plugins/use-masked-errors.ts b/packages/core/src/plugins/use-masked-errors.ts
@@ -14,7 +14,12 @@ export type FormatErrorHandler = (error: GraphQLError | unknown, message: string
 
 export const formatError: FormatErrorHandler = (err, message, isDev) => {
   if (err instanceof GraphQLError) {
-    if (err.originalError && err.originalError instanceof EnvelopError === false) {
+    if (
+      /** execution error */
+      (err.originalError && err.originalError instanceof EnvelopError === false) ||
+      /** validate and parse errors */
+      (err.originalError === undefined && err instanceof EnvelopError === false)
+    ) {
       return new GraphQLError(
         message,
         err.nodes,
@@ -25,8 +30,8 @@ export const formatError: FormatErrorHandler = (err, message, isDev) => {
         isDev
           ? {
               originalError: {
-                message: err.originalError.message,
-                stack: err.originalError.stack,
+                message: err.originalError?.message ?? err.message,
+                stack: err.originalError?.stack ?? err.stack,
               },
             }
           : undefined
@@ -47,6 +52,25 @@ export type UseMaskedErrorsOpts = {
    * The default value is `process.env['NODE_ENV'] === 'development'`
    */
   isDev?: boolean;
+  /**
+   * Whether parse errors should be processed by this plugin.
+   * In general it is not recommend to set this flag to `true`
+   * as a `parse` error contains useful information for debugging a GraphQL operation.
+   * A `parse` error never contains any sensitive information.
+   * @default false
+   */
+  handleParseErrors?: boolean;
+  /**
+   * Whether validation errors should processed by this plugin.
+   * In general we recommend against setting this flag to `true`
+   * as a `validate` error contains useful information for debugging a GraphQL operation.
+   * A `validate` error contains "did you mean x" suggestions that make it easier
+   * to reverse-introspect a GraphQL schema whose introspection capabilities got disabled.
+   * Instead of disabling introspection and masking validation errors, using persisted operations
+   * is a safer solution for avoiding the execution of unwanted/arbitrary operations.
+   * @default false
+   */
+  handleValidationErrors?: boolean;
 };
 
 const makeHandleResult =
@@ -65,6 +89,26 @@ export const useMaskedErrors = (opts?: UseMaskedErrorsOpts): Plugin => {
   const handleResult = makeHandleResult(format, message, isDev);
 
   return {
+    onParse:
+      opts?.handleParseErrors === true
+        ? function onParse() {
+            return function onParseEnd({ result, replaceParseResult }) {
+              if (result instanceof Error) {
+                replaceParseResult(format(result, message, isDev));
+              }
+            };
+          }
+        : undefined,
+    onValidate:
+      opts?.handleValidationErrors === true
+        ? function onValidate() {
+            return function onValidateEnd({ valid, result, setResult }) {
+              if (valid === false) {
+                setResult(result.map(error => format(error, message, isDev)));
+              }
+            };
+          }
+        : undefined,
     onPluginInit(context) {
       context.registerContextErrorHandler(({ error, setError }) => {
         setError(formatError(error, message, isDev));

diff --git a/packages/core/test/plugins/use-masked-errors.spec.ts b/packages/core/test/plugins/use-masked-errors.spec.ts
@@ -184,8 +184,8 @@ describe('useMaskedErrors', () => {
     );
     try {
       await testInstance.execute(`query { secretWithExtensions }`);
-    } catch (err) {
-      expect(err).toMatchInlineSnapshot(`[GraphQLError: ${DEFAULT_ERROR_MESSAGE}]`);
+    } catch (err: any) {
+      expect(err.message).toEqual(DEFAULT_ERROR_MESSAGE);
     }
   });
 
@@ -211,11 +211,9 @@ describe('useMaskedErrors', () => {
     const result = await testInstance.execute(`subscription { instantError }`);
     assertSingleExecutionValue(result);
     expect(result.errors).toBeDefined();
-    expect(result.errors).toMatchInlineSnapshot(`
-Array [
-  [GraphQLError: ${DEFAULT_ERROR_MESSAGE}],
-]
-`);
+    expect(result.errors).toHaveLength(1);
+    const [error] = result.errors!;
+    expect(error.message).toEqual(DEFAULT_ERROR_MESSAGE);
   });
 
   it('Should mask subscribe (sync/promise) subscription errors with a custom error message', async () => {
@@ -224,10 +222,10 @@ Array [
     assertSingleExecutionValue(result);
     expect(result.errors).toBeDefined();
     expect(result.errors).toMatchInlineSnapshot(`
-Array [
-  [GraphQLError: My Custom subscription error message.],
-]
-`);
+      Array [
+        [GraphQLError: My Custom subscription error message.],
+      ]
+    `);
   });
 
   it('Should not mask subscribe (sync/promise) subscription envelop errors', async () => {
@@ -236,10 +234,10 @@ Array [
     assertSingleExecutionValue(result);
     expect(result.errors).toBeDefined();
     expect(result.errors).toMatchInlineSnapshot(`
-Array [
-  [GraphQLError: Noop],
-]
-`);
+      Array [
+        [GraphQLError: Noop],
+      ]
+    `);
   });
 
   it('Should mask subscribe (AsyncIterable) subscription errors', async () => {
@@ -249,8 +247,8 @@ Array [
     assertStreamExecutionValue(resultStream);
     try {
       await collectAsyncIteratorValues(resultStream);
-    } catch (err) {
-      expect(err).toMatchInlineSnapshot(`[GraphQLError: ${DEFAULT_ERROR_MESSAGE}]`);
+    } catch (err: any) {
+      expect(err.message).toEqual(DEFAULT_ERROR_MESSAGE);
     }
   });
 
@@ -285,11 +283,9 @@ Array [
     expect(allResults).toHaveLength(1);
     const [result] = allResults;
     expect(result.errors).toBeDefined();
-    expect(result.errors).toMatchInlineSnapshot(`
-Array [
-  [GraphQLError: ${DEFAULT_ERROR_MESSAGE}],
-]
-`);
+    expect(result.errors).toHaveLength(1);
+    const [error] = result.errors!;
+    expect(error.message).toEqual(DEFAULT_ERROR_MESSAGE);
   });
   it('Should mask resolve subscription errors with a custom error message', async () => {
     const testInstance = createTestkit([useMaskedErrors({ errorMessage: 'Custom resolve subscription errors.' })], schema);
@@ -300,10 +296,10 @@ Array [
     const [result] = allResults;
     expect(result.errors).toBeDefined();
     expect(result.errors).toMatchInlineSnapshot(`
-Array [
-  [GraphQLError: Custom resolve subscription errors.],
-]
-`);
+      Array [
+        [GraphQLError: Custom resolve subscription errors.],
+      ]
+    `);
   });
 
   it('Should not mask resolve subscription envelop errors', async () => {
@@ -315,10 +311,10 @@ Array [
     const [result] = allResults;
     expect(result.errors).toBeDefined();
     expect(result.errors).toMatchInlineSnapshot(`
-Array [
-  [GraphQLError: Noop],
-]
-`);
+      Array [
+        [GraphQLError: Noop],
+      ]
+    `);
   });
 
   it('Should not mask auth0 header errors', async () => {
@@ -344,4 +340,45 @@ Array [
       expect(err).toMatchInlineSnapshot(`[GraphQLError: Unsupported token type provided: "Something"!]`);
     }
   });
+
+  it('should not mask parse errors', async () => {
+    const testInstance = createTestkit([useMaskedErrors()], schema);
+    const result = await testInstance.execute(`query { a `, {});
+    assertSingleExecutionValue(result);
+    expect(result).toMatchInlineSnapshot(`
+      Object {
+        "errors": Array [
+          [GraphQLError: Syntax Error: Expected Name, found <EOF>.],
+        ],
+      }
+    `);
+  });
+  it('should mask parse errors with handleParseErrors option', async () => {
+    const testInstance = createTestkit([useMaskedErrors({ handleParseErrors: true })], schema);
+    const result = await testInstance.execute(`query { a `, {});
+    assertSingleExecutionValue(result);
+    expect(result.errors).toBeDefined();
+    const [error] = result.errors!;
+    expect(error.message).toEqual(DEFAULT_ERROR_MESSAGE);
+  });
+  it('should not mask validation errors', async () => {
+    const testInstance = createTestkit([useMaskedErrors()], schema);
+    const result = await testInstance.execute(`query { iDoNotExistsMyGuy }`, {});
+    assertSingleExecutionValue(result);
+    expect(result).toMatchInlineSnapshot(`
+      Object {
+        "errors": Array [
+          [GraphQLError: Cannot query field "iDoNotExistsMyGuy" on type "Query".],
+        ],
+      }
+    `);
+  });
+  it('should mask validation errors with handleValidationErrors option', async () => {
+    const testInstance = createTestkit([useMaskedErrors({ handleValidationErrors: true })], schema);
+    const result = await testInstance.execute(`query { iDoNotExistsMyGuy }`, {});
+    assertSingleExecutionValue(result);
+    expect(result.errors).toBeDefined();
+    const [error] = result.errors!;
+    expect(error.message).toEqual(DEFAULT_ERROR_MESSAGE);
+  });
 });
diff --git a/packages/core/test/validate.spec.ts b/packages/core/test/validate.spec.ts
@@ -29,6 +29,7 @@ describe('validate', () => {
       context: expect.any(Object),
       extendContext: expect.any(Function),
       result: [],
+      setResult: expect.any(Function),
       valid: true,
     });
   });
@@ -90,6 +91,7 @@ describe('validate', () => {
     expect(after).toHaveBeenCalledWith({
       valid: false,
       result: [e],
+      setResult: expect.any(Function),
       context: expect.any(Object),
       extendContext: expect.any(Function),
     });

diff --git a/packages/types/src/hooks.ts b/packages/types/src/hooks.ts
@@ -195,6 +195,10 @@ export type AfterValidateEventPayload<ContextType> = {
    * The array is empty if no errors were raised.
    */
   result: readonly GraphQLError[];
+  /**
+   * Replace the current error result with a new one.
+   */
+  setResult: (errors: GraphQLError[]) => void;
 };
 
 /**