Merge pull request #450 from AikidoSec/graphql

Don't discover GraphQL queries from server-side rendering

Author: willem-delbare

library/sources/GraphQL.schema.test.ts

@@ -1,18 +1,22 @@
 import * as t from "tap";
 import { ReportingAPIForTesting } from "../agent/api/ReportingAPIForTesting";
-import { runWithContext } from "../agent/Context";
+import { Context, getContext, runWithContext } from "../agent/Context";
 import { GraphQL } from "./GraphQL";
 import { Token } from "../agent/api/Token";
 import { createTestAgent } from "../helpers/createTestAgent";
 
-function getTestContext() {
+function getTestContext(): Context {
   return {
     remoteAddress: "::1",
     method: "POST",
     url: "http://localhost:4000/graphql",
     query: {},
-    headers: {},
-    body: {},
+    headers: {
+      "content-type": "application/json",
+    },
+    body: {
+      query: '{ getFile(path: "/etc/bashrc") }',
+    },
     cookies: {},
     routeParams: {},
     source: "express",
@@ -108,6 +112,29 @@ type Author {
     }
   });
 
+  await runWithContext(
+    {
+      remoteAddress: "::1",
+      method: "POST",
+      url: "http://localhost:4000/server-rendered-page",
+      query: {},
+      headers: {},
+      body: {
+        query: "this is not a graphql query",
+      },
+      cookies: {},
+      routeParams: {},
+      source: "express",
+      route: "/server-rendered-page",
+    },
+    async () => {
+      await query({ id: "1" });
+
+      // Route is registered at the end of the request
+      agent.onRouteExecute(getContext()!);
+    }
+  );
+
   await agent.flushStats(1000);
 
   t.same(api.getEvents().length, 1);
@@ -134,6 +161,14 @@ type Author {
         apispec: {},
         graphQLSchema: dsl,
       },
+      {
+        method: "POST",
+        path: "/server-rendered-page",
+        hits: 1,
+        graphql: undefined,
+        apispec: {},
+        graphQLSchema: undefined,
+      },
     ],
   });
 });

library/sources/GraphQL.test.ts

@@ -1,6 +1,6 @@
 import * as t from "tap";
 import { ReportingAPIForTesting } from "../agent/api/ReportingAPIForTesting";
-import { getContext, runWithContext } from "../agent/Context";
+import { Context, getContext, runWithContext } from "../agent/Context";
 import { GraphQL } from "./GraphQL";
 import { Token } from "../agent/api/Token";
 import { createTestAgent } from "../helpers/createTestAgent";
@@ -56,13 +56,17 @@ t.test("it works", async () => {
   const schema = buildSchema(`
         type Query {
             getFile(path: String): String
+            anotherQuery: String
         }
     `);
 
   const root = {
     getFile: ({ path }: { path: string }) => {
       return "file content";
     },
+    anotherQuery: () => {
+      return "another query";
+    },
   };
 
   const query = async (path: string, variableValues?: Record<string, any>) => {
@@ -96,5 +100,33 @@ t.test("it works", async () => {
     await query("/etc/bashrc");
     const result = await query("/etc/bashrc");
     t.same(result.errors![0].message, "You are rate limited by Zen.");
+
+    // With operation name
+    t.same(
+      await graphql({
+        schema,
+        source: `
+        query getFile {
+          getFile(path: "/etc/bashrc")
+        }
+
+        query anotherQuery {
+          anotherQuery
+        }
+      `,
+        rootValue: root,
+        variableValues: {},
+        operationName: "anotherQuery",
+      }),
+      {
+        data: { anotherQuery: "another query" },
+      }
+    );
+  });
+
+  // Empty context
+  await runWithContext({} as Context, async () => {
+    const response = await query("/etc/bashrc");
+    t.same(response, { data: { getFile: "file content" } });
   });
 });

library/sources/GraphQL.ts

@@ -1,22 +1,22 @@
 /* eslint-disable prefer-rest-params */
 import { Agent } from "../agent/Agent";
-import { getContext, updateContext } from "../agent/Context";
+import { Context, getContext, updateContext } from "../agent/Context";
 import { Hooks } from "../agent/hooks/Hooks";
 import { WrapPackageInfo } from "../agent/hooks/WrapPackageInfo";
 import { Wrapper } from "../agent/Wrapper";
 import type { ExecutionArgs } from "graphql/execution/execute";
 import { isPlainObject } from "../helpers/isPlainObject";
 import { extractInputsFromDocument } from "./graphql/extractInputsFromDocument";
 import { extractTopLevelFieldsFromDocument } from "./graphql/extractTopLevelFieldsFromDocument";
+import { isGraphQLOverHTTP } from "./graphql/isGraphQLOverHTTP";
 import { shouldRateLimitOperation } from "./graphql/shouldRateLimitOperation";
 import { wrapExport } from "../agent/hooks/wrapExport";
 
 export class GraphQL implements Wrapper {
   private graphqlModule: typeof import("graphql") | undefined;
 
   private discoverGraphQLSchema(
-    method: string,
-    route: string,
+    context: Context,
     executeArgs: ExecutionArgs,
     agent: Agent
   ) {
@@ -28,31 +28,38 @@ export class GraphQL implements Wrapper {
       return;
     }
 
-    if (!agent.hasGraphQLSchema(method, route)) {
+    if (!context.method || !context.route) {
+      return;
+    }
+
+    if (!agent.hasGraphQLSchema(context.method, context.route)) {
       try {
         const schema = this.graphqlModule.printSchema(executeArgs.schema);
-        agent.onGraphQLSchema(method, route, schema);
+        agent.onGraphQLSchema(context.method, context.route, schema);
       } catch (e) {
         // Ignore errors
       }
     }
   }
 
   private discoverGraphQLQueryFields(
-    method: string,
-    route: string,
+    context: Context,
     executeArgs: ExecutionArgs,
     agent: Agent
   ) {
+    if (!context.method || !context.route) {
+      return;
+    }
+
     const topLevelFields = extractTopLevelFieldsFromDocument(
       executeArgs.document,
       executeArgs.operationName ? executeArgs.operationName : undefined
     );
 
     if (topLevelFields) {
       agent.onGraphQLExecute(
-        method,
-        route,
+        context.method,
+        context.route,
         topLevelFields.type,
         topLevelFields.fields.map((field) => field.name.value)
       );
@@ -76,19 +83,16 @@ export class GraphQL implements Wrapper {
       return;
     }
 
-    if (context.method && context.route) {
-      this.discoverGraphQLSchema(
-        context.method,
-        context.route,
-        executeArgs,
-        agent
-      );
-      this.discoverGraphQLQueryFields(
-        context.method,
-        context.route,
-        executeArgs,
-        agent
-      );
+    if (
+      context &&
+      context.method &&
+      context.route &&
+      isGraphQLOverHTTP(context)
+    ) {
+      // We only want to discover GraphQL over HTTP
+      // We should ignore queries coming from a GraphQL client in SSR mode
+      this.discoverGraphQLSchema(context, executeArgs, agent);
+      this.discoverGraphQLQueryFields(context, executeArgs, agent);
     }
 
     const userInputs = extractInputsFromDocument(