From 52ec789c20cb7f83ef8dd3a92eb09d8891dcb153 Mon Sep 17 00:00:00 2001 From: GeiserX <9169332+GeiserX@users.noreply.github.com> Date: Fri, 14 Aug 2026 00:43:20 +0200 Subject: [PATCH] fix(graphql): refuse an empty credential value instead of dialing with it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Omitting a required credential input and supplying it as "" are the same state — there is no usable credential — but only the omission was caught. The empty one was rendered onto the request, the upstream answered 401, and the operator was told authentication failed rather than that their credential was empty. The OpenAPI backing already treats "" as missing, at both of its checks. This brings GraphQL's check in line with it. Whitespace is deliberately left alone: judging a value someone supplied is a different decision from judging one that is absent, and OpenAPI does not do it either. The new test fails without the change. The four existing cases in that file, which dial with real values, are the control. --- .../src/sdk/multi-placement-auth.test.ts | 50 +++++++++++++++++++ packages/plugins/graphql/src/sdk/plugin.ts | 9 +++- 2 files changed, 58 insertions(+), 1 deletion(-) diff --git a/packages/plugins/graphql/src/sdk/multi-placement-auth.test.ts b/packages/plugins/graphql/src/sdk/multi-placement-auth.test.ts index 437fde0ff2..d4ec659122 100644 --- a/packages/plugins/graphql/src/sdk/multi-placement-auth.test.ts +++ b/packages/plugins/graphql/src/sdk/multi-placement-auth.test.ts @@ -203,4 +203,54 @@ describe("GraphQL multi-placement auth", () => { expect(String(result.error?.message ?? "")).toContain("b"); }), ); + + it.effect("an EMPTY input is missing too, not a value to dial with", () => + Effect.gen(function* () { + // Supplying "" and omitting the input are the same state: no usable + // credential. Only the omission used to be caught, so an empty value went + // out on the wire and came back as an upstream 401 — an error that names + // authentication rather than the empty input that caused it. The OpenAPI + // backing already refused "", so this is the behaviour the plugins share. + const server = yield* serveGreetingServer; + const executor = yield* makeExecutor(); + + yield* executor.graphql.addIntegration({ + endpoint: server.endpoint, + slug: "empty_gql", + name: "Empty-input GraphQL", + authenticationTemplate: [ + { + slug: "two_inputs", + kind: "apikey", + placements: [ + { carrier: "header", name: "Authorization", prefix: "Bearer ", variable: "a" }, + { carrier: "query", name: "team_id", variable: "b" }, + ], + }, + ], + }); + + yield* executor.connections.create({ + owner: "org", + name: ConnectionName.make("empty"), + integration: IntegrationSlug.make("empty_gql"), + template: AuthTemplateSlug.make("two_inputs"), + values: { a: "tok_A", b: "" }, + }); + + const result = (yield* executor.execute(toolAddr("empty_gql", "empty", "query.hello"), { + name: "Ada", + })) as { ok: boolean; error?: { code?: string; message?: string } }; + expect(result.ok).toBe(false); + expect(result.error).toMatchObject({ code: "connection_value_missing" }); + expect(String(result.error?.message ?? "")).toContain("b"); + + // The server must not have been dialed at all — refusing after sending the + // request would still leak an empty credential onto the wire. + // Deliberately not asserted here: connect-time introspection dials before + // this guard, and does so whether the input is empty or absent — checked + // both ways. That is pre-existing behaviour of a different stage, so + // pinning it in this test would tie the fix to something it does not do. + }), + ); }); diff --git a/packages/plugins/graphql/src/sdk/plugin.ts b/packages/plugins/graphql/src/sdk/plugin.ts index 6e162d29df..e669b98db3 100644 --- a/packages/plugins/graphql/src/sdk/plugin.ts +++ b/packages/plugins/graphql/src/sdk/plugin.ts @@ -1354,7 +1354,14 @@ export const graphqlPlugin = definePlugin((options?: GraphqlPluginOptions) => { method.kind === "oauth2" ? [TOKEN_VARIABLE] : requiredPlacementVariables(method.placements) - ).filter((variable) => credential.values[variable] == null); + ) + // An empty value is as unusable as an absent one, and forwarding it + // sends an empty credential upstream — the 401 that follows names the + // wrong problem. Matches the OpenAPI backing's check. + .filter((variable) => { + const value = credential.values[variable]; + return value == null || value === ""; + }); if (missing.length > 0) { return yield* new GraphqlAuthRequiredError({ code: