Keep a query-carried credential out of GraphQL introspection's failure log

.changeset/graphql-introspection-credential-log.md

@@ -0,0 +1,11 @@
+---
+"executor": patch
+---
+
+**GraphQL introspection no longer logs a credential carried in the query string**
+
+`query` is a supported credential carrier, so a GraphQL endpoint can be reached with `?token=<secret>`. Introspection built its request from a URL **string**, and `HttpClientRequest.setUrl` keeps a string verbatim as `request.url`. Every `HttpClientError` renders `${method} ${request.url}` into its `message` getter, and introspection logs the raw failure cause — so on any transport failure or non-JSON response, the connection's secret was written to the process log.
+
+The request is now built from a URL **object**, which moves the query into `request.urlParams` and clears it from `request.url`. The secret is therefore absent from the error message, and from anything else that renders the request URL. Nothing changes on the wire: the client recombines url and urlParams when it executes the request.
+
+The endpoint's own query string is handled the same way, not just the separately-supplied query parameters, since a configured endpoint can carry a credential too.

packages/plugins/graphql/src/sdk/introspect-credential-logging.test.ts

@@ -0,0 +1,113 @@
+// ---------------------------------------------------------------------------
+// Introspection must not log a credential carried in the query string.
+//
+// `query` is a supported credential carrier, so a GraphQL endpoint can be
+// reached with `?token=<secret>`. Introspection logs the raw failure cause on
+// any transport error, and every `HttpClientError` renders `${method}
+// ${request.url}` into its message — so if the request is built from a URL
+// STRING, the secret is inside that message and goes straight to the log.
+//
+// Building the request from a URL OBJECT moves the query into
+// `request.urlParams`, out of `request.url` and therefore out of the message,
+// while the client still recombines the two when it executes.
+//
+// Both directions are asserted. A test that only checked "the secret is absent"
+// would pass just as happily against a logger that captured nothing at all, or
+// a change that stopped sending the parameter entirely.
+// ---------------------------------------------------------------------------
+
+import { describe, expect, it } from "@effect/vitest";
+import { Cause, Effect, Layer, Logger } from "effect";
+import { FetchHttpClient } from "effect/unstable/http";
+
+import { introspect } from "./introspect";
+
+const SECRET = "tok_live_introspection_MUST_NOT_LOG";
+const ENDPOINT = "https://graph.example.test/graphql";
+
+/** Collects everything a logger would have written, message and cause alike —
+ *  `Cause.pretty` is the renderer that rebuilds the first line from the error's
+ *  live `message` getter, which is the exact path the leak took. */
+const capturingLogger = (sink: Array<string>) =>
+  Logger.make<unknown, void>((options) => {
+    sink.push(String(options.message));
+    sink.push(Cause.pretty(options.cause));
+  });
+
+/** A fetch that records the URL it was handed and then fails at the transport
+ *  layer, which is what drives introspection down its logging path. */
+const failingFetch = (seen: Array<string>): typeof globalThis.fetch =>
+  (async (input: RequestInfo | URL) => {
+    seen.push(input instanceof Request ? input.url : String(input));
+    // oxlint-disable-next-line executor/no-try-catch-or-throw, executor/no-error-constructor -- boundary: simulates the platform fetch rejecting on a dead host
+    throw new Error("getaddrinfo ENOTFOUND graph.example.test");
+  }) as typeof globalThis.fetch;
+
+const clientLayer = (seen: Array<string>) =>
+  FetchHttpClient.layer.pipe(
+    Layer.provide(Layer.succeed(FetchHttpClient.Fetch)(failingFetch(seen))),
+  );
+
+describe("GraphQL introspection credential logging", () => {
+  it.effect("does not write a query-carried credential to the log", () =>
+    Effect.gen(function* () {
+      const logged: Array<string> = [];
+      const seen: Array<string> = [];
+
+      yield* introspect(ENDPOINT, undefined, { token: SECRET }).pipe(
+        Effect.flip,
+        Effect.provide(clientLayer(seen)),
+        Effect.provide(Logger.layer([capturingLogger(logged)])),
+      );
+
+      const output = logged.join("\n");
+
+      // Positive control FIRST: prove the logger actually captured the failure.
+      // Without this, an empty capture would satisfy every assertion below.
+      expect(output).toContain("graphql introspection request failed");
+      expect(output).toContain("graph.example.test");
+
+      // The credential is absent from everything that was logged.
+      expect(output).not.toContain(SECRET);
+      expect(output).not.toContain("token=");
+    }),
+  );
+
+  it.effect("still sends the query-carried credential on the wire", () =>
+    Effect.gen(function* () {
+      const logged: Array<string> = [];
+      const seen: Array<string> = [];
+
+      yield* introspect(ENDPOINT, undefined, { token: SECRET }).pipe(
+        Effect.flip,
+        Effect.provide(clientLayer(seen)),
+        Effect.provide(Logger.layer([capturingLogger(logged)])),
+      );
+
+      // Keeping it out of the log is only correct if it still reaches the
+      // upstream — otherwise this "fix" silently breaks authentication.
+      expect(seen).toHaveLength(1);
+      expect(seen[0]).toContain(`token=${SECRET}`);
+    }),
+  );
+
+  it.effect("keeps a credential carried in the endpoint's own query out of the log", () =>
+    Effect.gen(function* () {
+      // A configured endpoint can carry the secret itself, with no separate
+      // queryParams argument at all.
+      const logged: Array<string> = [];
+      const seen: Array<string> = [];
+
+      yield* introspect(`${ENDPOINT}?token=${SECRET}`).pipe(
+        Effect.flip,
+        Effect.provide(clientLayer(seen)),
+        Effect.provide(Logger.layer([capturingLogger(logged)])),
+      );
+
+      const output = logged.join("\n");
+      expect(output).toContain("graphql introspection request failed");
+      expect(output).not.toContain(SECRET);
+      expect(seen[0]).toContain(`token=${SECRET}`);
+    }),
+  );
+});

packages/plugins/graphql/src/sdk/introspect.ts

@@ -235,18 +235,32 @@ export const introspect = Effect.fn("GraphQL.introspect")(function* (
   queryParams?: Record<string, string>,
 ) {
   const client = yield* HttpClient.HttpClient;
-  const requestEndpoint =
-    queryParams && Object.keys(queryParams).length > 0
-      ? (() => {
-          const url = new URL(endpoint);
-          for (const [name, value] of Object.entries(queryParams)) {
-            url.searchParams.set(name, value);
-          }
-          return url.toString();
-        })()
-      : endpoint;
+  // Hand `post` a URL OBJECT rather than a string, deliberately.
+  //
+  // `HttpClientRequest.setUrl` keeps a string verbatim as `request.url`, and
+  // every `HttpClientError` renders `${method} ${request.url}` into its
+  // `message` getter. The `query` carrier is a supported credential placement,
+  // so an endpoint reached with `?token=…` put that secret inside the error
+  // message — and the `Effect.logError(…, cause)` below writes the message
+  // straight to the log on any transport failure or non-JSON response.
+  //
+  // Given a URL object, `setUrl` moves the query into `request.urlParams` and
+  // clears it from `request.url`, so the same failure logs the bare endpoint.
+  // Nothing is lost on the wire: the client recombines url + urlParams when it
+  // executes the request. Handling the endpoint's OWN query the same way (not
+  // just the `queryParams` argument) matters — a configured endpoint can carry
+  // a credential in its query string too.
+  const requestUrl: string | URL = URL.canParse(endpoint)
+    ? (() => {
+        const url = new URL(endpoint);
+        for (const [name, value] of Object.entries(queryParams ?? {})) {
+          url.searchParams.set(name, value);
+        }
+        return url;
+      })()
+    : endpoint;
 
-  let request = HttpClientRequest.post(requestEndpoint).pipe(
+  let request = HttpClientRequest.post(requestUrl).pipe(
     HttpClientRequest.setHeader("Content-Type", "application/json"),
     HttpClientRequest.setHeader("Accept", "application/json"),
     HttpClientRequest.setHeader("User-Agent", "executor-graphql"),