From 39abea39f0040f0e1ebdb2b62d122d954a36f9e4 Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Thu, 9 Jul 2026 19:00:52 -0700 Subject: [PATCH] Reject unknown OpenAPI tool arguments --- e2e/scenarios/openapi-unknown-args.test.ts | 162 +++++++++++++ packages/plugins/openapi/src/sdk/errors.ts | 2 +- packages/plugins/openapi/src/sdk/invoke.ts | 69 ++++++ .../plugins/openapi/src/sdk/plugin.test.ts | 227 +++++++++++++++++- 4 files changed, 456 insertions(+), 4 deletions(-) create mode 100644 e2e/scenarios/openapi-unknown-args.test.ts diff --git a/e2e/scenarios/openapi-unknown-args.test.ts b/e2e/scenarios/openapi-unknown-args.test.ts new file mode 100644 index 000000000..faca4b460 --- /dev/null +++ b/e2e/scenarios/openapi-unknown-args.test.ts @@ -0,0 +1,162 @@ +import { randomBytes } from "node:crypto"; +import { createServer } from "node:http"; + +import { expect } from "@effect/vitest"; +import { Effect } from "effect"; +import { composePluginApi } from "@executor-js/api/server"; +import { openApiHttpPlugin } from "@executor-js/plugin-openapi/api"; +import { AuthTemplateSlug, ConnectionName, IntegrationSlug } from "@executor-js/sdk/shared"; + +import { scenario } from "../src/scenario"; +import { Api, Target } from "../src/services"; + +const api = composePluginApi([openApiHttpPlugin()] as const); + +const serveRecordingUpstream = () => + Effect.acquireRelease( + Effect.callback<{ + readonly url: string; + readonly requests: () => readonly string[]; + readonly close: () => void; + }>((resume) => { + const recorded: string[] = []; + const server = createServer((request, response) => { + recorded.push(request.url ?? ""); + response.writeHead(200, { "content-type": "application/json" }); + response.end(JSON.stringify({ id: "2", name: "Gadget" })); + }); + server.listen(0, "127.0.0.1", () => { + const address = server.address(); + const port = typeof address === "object" && address ? address.port : 0; + resume( + Effect.succeed({ + url: `http://127.0.0.1:${port}`, + requests: () => [...recorded], + close: () => { + server.close(); + server.closeAllConnections(); + }, + }), + ); + }); + }), + (server) => Effect.sync(server.close), + ); + +const itemsSpec = (baseUrl: string): string => + JSON.stringify({ + openapi: "3.0.3", + info: { title: "Items API", version: "1.0.0" }, + servers: [{ url: baseUrl }], + paths: { + "/items/{itemId}": { + get: { + operationId: "getItem", + summary: "Fetch one item", + parameters: [{ name: "itemId", in: "path", required: true, schema: { type: "string" } }], + responses: { "200": { description: "the item" } }, + }, + }, + }, + }); + +const invokeByAddressCode = (address: string, args: unknown) => ` +const segments = ${JSON.stringify(address)}.split(".").slice(1); +let node = tools; +for (const segment of segments) node = node[segment]; +const result = await node(${JSON.stringify(args)}); +return JSON.stringify(result); +`; + +type ToolEnvelope = { + readonly ok: boolean; + readonly error?: { + readonly code?: string; + readonly message?: string; + }; +}; + +scenario( + "OpenAPI: unknown tool arguments fail locally before any upstream request", + {}, + Effect.scoped( + Effect.gen(function* () { + const target = yield* Target; + const { client: makeClient } = yield* Api; + const identity = yield* target.newIdentity(); + const client = yield* makeClient(api, identity); + const upstream = yield* serveRecordingUpstream(); + const slug = `openapi_unknown_args_${randomBytes(4).toString("hex")}`; + + yield* Effect.ensuring( + Effect.gen(function* () { + yield* client.openapi.addSpec({ + payload: { + spec: { kind: "blob", value: itemsSpec(upstream.url) }, + slug, + baseUrl: upstream.url, + authenticationTemplate: [ + { + slug: "apiKey", + type: "apiKey", + headers: { authorization: ["Bearer ", { type: "variable", name: "token" }] }, + }, + ], + }, + }); + yield* client.connections.create({ + payload: { + owner: "org", + name: ConnectionName.make("main"), + integration: IntegrationSlug.make(slug), + template: AuthTemplateSlug.make("apiKey"), + value: "tok_unknown_args", + }, + }); + + const tools = yield* client.tools.list({ query: {} }); + const address = tools + .filter((tool) => String(tool.integration) === slug) + .map((tool) => String(tool.address)) + .find((candidate) => candidate.endsWith("getItem")); + expect(address, "the getItem operation is available").toBeDefined(); + + const executed = yield* client.executions.execute({ + payload: { + code: invokeByAddressCode(address!, { itemId: "2", doesNotExist: "nope" }), + autoApprove: true, + }, + }); + expect(executed.status, "the sandbox returns the tool failure as a value").toBe( + "completed", + ); + const outcome = JSON.parse(executed.text) as ToolEnvelope; + expect(outcome.ok, "the unknown argument fails the tool call").toBe(false); + expect(outcome.error?.code, "the failure is classified as invalid arguments").toBe( + "invalid_tool_arguments", + ); + expect( + outcome.error?.message, + "the failure names the argument the caller must remove", + ).toContain("doesNotExist"); + expect( + upstream.requests(), + "argument validation stops the call before upstream dispatch", + ).toEqual([]); + }), + Effect.gen(function* () { + yield* client.connections + .remove({ + params: { + owner: "org", + integration: IntegrationSlug.make(slug), + name: ConnectionName.make("main"), + }, + }) + .pipe(Effect.ignore); + yield* client.openapi.removeSpec({ params: { slug } }).pipe(Effect.ignore); + }), + ); + }), + ), +); diff --git a/packages/plugins/openapi/src/sdk/errors.ts b/packages/plugins/openapi/src/sdk/errors.ts index 2952955ad..076cb2921 100644 --- a/packages/plugins/openapi/src/sdk/errors.ts +++ b/packages/plugins/openapi/src/sdk/errors.ts @@ -28,7 +28,7 @@ export class OpenApiExtractionError extends Schema.TaggedErrorClass; - readonly reason?: "response_headers_timeout"; + readonly reason?: "response_headers_timeout" | "unknown_arguments"; readonly cause?: unknown; }> {} diff --git a/packages/plugins/openapi/src/sdk/invoke.ts b/packages/plugins/openapi/src/sdk/invoke.ts index 5bbeb1517..c25805d78 100644 --- a/packages/plugins/openapi/src/sdk/invoke.ts +++ b/packages/plugins/openapi/src/sdk/invoke.ts @@ -835,12 +835,64 @@ const applyRequestBody = ( // `invoke` applies (one code path, not a parallel re-implementation). // --------------------------------------------------------------------------- +const acceptedArgumentNames = (operation: OperationBinding): readonly string[] => { + const accepted = new Set(); + + for (const param of operation.parameters) { + accepted.add(param.name); + for (const container of CONTAINER_KEYS[param.location] ?? []) accepted.add(container); + } + + for (const match of operation.pathTemplate.matchAll(/\{([^{}]+)\}/g)) { + if (match[1]) accepted.add(match[1]); + } + + if (Option.isSome(operation.requestBody)) { + const requestBody = operation.requestBody.value; + const contents = Option.getOrUndefined(requestBody.contents); + accepted.add("body"); + accepted.add("input"); + if ( + isOctetStream(requestBody.contentType) || + contents?.some((content) => isOctetStream(content.contentType)) + ) { + accepted.add("bodyBase64"); + } + if (contents && contents.length > 1) accepted.add("contentType"); + } + + const servers = operation.servers ?? []; + const hasServerVariables = servers.some( + (server) => Object.keys(Option.getOrUndefined(server.variables) ?? {}).length > 0, + ); + if (servers.length > 1 || hasServerVariables) accepted.add("server"); + + return [...accepted]; +}; + export const buildRequest = Effect.fn("OpenApi.buildRequest")(function* ( operation: OperationBinding, args: Record, resolvedHeaders: Record, integrationQueryParams: Record = {}, ) { + const accepted = acceptedArgumentNames(operation); + const acceptedSet = new Set(accepted); + const unknown = Object.keys(args).filter((name) => !acceptedSet.has(name)); + if (unknown.length > 0) { + const label = unknown.length === 1 ? "Unknown argument" : "Unknown arguments"; + const names = unknown.map((name) => JSON.stringify(name)).join(", "); + const acceptedMessage = + accepted.length > 0 + ? `This operation accepts: ${accepted.join(", ")}.` + : "This operation accepts no arguments."; + return yield* new OpenApiInvocationError({ + message: `${label} ${names}. ${acceptedMessage}`, + statusCode: Option.none(), + reason: "unknown_arguments", + }); + } + const resolvedPath = yield* resolvePath(operation.pathTemplate, args, operation.parameters); const path = resolvedPath.startsWith("/") ? resolvedPath : `/${resolvedPath}`; @@ -870,6 +922,14 @@ export const buildRequest = Effect.fn("OpenApi.buildRequest")(function* ( request = HttpClientRequest.setHeader(request, param.name, String(value)); } + const cookieValues: string[] = []; + for (const param of operation.parameters) { + if (param.location !== "cookie") continue; + const value = readParamValue(args, param); + if (value === undefined || value === null) continue; + cookieValues.push(`${param.name}=${primitiveToString(value)}`); + } + if (Option.isSome(operation.requestBody)) { const rb = operation.requestBody.value; const contentsOpt = Option.getOrUndefined(rb.contents); @@ -986,6 +1046,15 @@ export const buildRequest = Effect.fn("OpenApi.buildRequest")(function* ( } request = applyHeaders(request, resolvedHeaders); + if (cookieValues.length > 0) { + const existingCookie = request.headers.cookie; + const serializedCookies = cookieValues.join("; "); + request = HttpClientRequest.setHeader( + request, + "Cookie", + existingCookie ? `${existingCookie}; ${serializedCookies}` : serializedCookies, + ); + } return request; }); diff --git a/packages/plugins/openapi/src/sdk/plugin.test.ts b/packages/plugins/openapi/src/sdk/plugin.test.ts index 0db4043bb..1617e8b55 100644 --- a/packages/plugins/openapi/src/sdk/plugin.test.ts +++ b/packages/plugins/openapi/src/sdk/plugin.test.ts @@ -42,7 +42,9 @@ import { type AuthenticationInput } from "./types"; import { addOpenApiTestConnection, makeOpenApiHttpApiTestIntegrationConfig, + makeOpenApiTestSpecJson, serveMutableOpenApiSpecTestServer, + serveOpenApiEchoTestServer, serveOpenApiHttpApiTestServer, unwrapInvocation, } from "../testing"; @@ -258,6 +260,40 @@ const servePluginTestApi = () => handlersLayer: ItemsGroupLive, }); +const recordValue = (value: unknown): Record | undefined => + typeof value === "object" && value !== null && !Array.isArray(value) + ? Object.fromEntries(Object.entries(value)) + : undefined; + +const withEchoCookieParameter = (spec: Record): Record => { + const paths = recordValue(spec.paths); + const pathItem = recordValue(paths?.["/echo-headers"]); + const get = recordValue(pathItem?.get); + if (!paths || !pathItem || !get) return spec; + const parameters = Array.isArray(get.parameters) ? get.parameters : []; + return { + ...spec, + paths: { + ...paths, + "/echo-headers": { + ...pathItem, + get: { + ...get, + parameters: [ + ...parameters, + { + name: "sessionId", + in: "cookie", + required: true, + schema: { type: "string" }, + }, + ], + }, + }, + }, + }; +}; + // An apiKey auth template that places the connection value into `x-api-key`. const apiKeyTemplate: AuthenticationInput = { slug: AuthTemplateSlug.make("apiKey"), @@ -599,6 +635,72 @@ describe("OpenAPI Plugin", () => { ), ); + it.effect("rejects unknown args before raising the approval elicitation", () => + Effect.scoped( + Effect.gen(function* () { + const server = yield* servePluginTestApi(); + const executor = yield* createExecutor(makeTestConfig({ plugins: testPlugins() })); + + const conn = yield* addOpenApiTestConnection(executor, server, { slug: "test" }); + const calls = { count: 0 }; + const failure = yield* executor + .execute( + conn.address("items.createItem"), + { body: { name: "New item" }, doesNotExist: "nope" }, + { + onElicitation: () => + Effect.sync(() => { + calls.count++; + return { action: "accept" as const, content: {} }; + }), + }, + ) + .pipe(Effect.flip); + + expect(calls.count).toBe(0); + expect(Predicate.isTagged(failure, "ToolInvocationError")).toBe(true); + // oxlint-disable-next-line executor/no-unknown-error-message -- boundary: asserts the exact caller-facing message the pre-flight failure carries + const message = (failure as { message: string }).message; + expect(message).toContain('Unknown argument "doesNotExist".'); + expect(message).toContain("This operation accepts:"); + expect(message).toContain("body"); + }), + ), + ); + + it.effect("redirects requestBody callers to the accepted body argument", () => + Effect.scoped( + Effect.gen(function* () { + const server = yield* servePluginTestApi(); + const executor = yield* createExecutor(makeTestConfig({ plugins: testPlugins() })); + + const conn = yield* addOpenApiTestConnection(executor, server, { slug: "test" }); + const calls = { count: 0 }; + const failure = yield* executor + .execute( + conn.address("items.createItem"), + { requestBody: { name: "New item" } }, + { + onElicitation: () => + Effect.sync(() => { + calls.count++; + return { action: "accept" as const, content: {} }; + }), + }, + ) + .pipe(Effect.flip); + + expect(calls.count).toBe(0); + expect(Predicate.isTagged(failure, "ToolInvocationError")).toBe(true); + // oxlint-disable-next-line executor/no-unknown-error-message -- boundary: asserts the exact caller-facing message the pre-flight failure carries + const message = (failure as { message: string }).message; + expect(message).toContain('Unknown argument "requestBody".'); + expect(message).toContain("This operation accepts:"); + expect(message).toContain("body"); + }), + ), + ); + it.effect("describes OpenAPI invocation results payload-first with http meta beside data", () => Effect.scoped( Effect.gen(function* () { @@ -657,6 +759,128 @@ describe("OpenAPI Plugin", () => { ), ); + it.effect("rejects unknown GET arguments locally", () => + Effect.scoped( + Effect.gen(function* () { + const server = yield* servePluginTestApi(); + const executor = yield* createExecutor(makeTestConfig({ plugins: testPlugins() })); + + const conn = yield* addOpenApiTestConnection(executor, server, { slug: "test" }); + const failure = yield* executor + .execute(conn.address("items.getItem"), { + itemId: "2", + doesNotExist: "nope", + }) + .pipe(Effect.flip); + + expect(Predicate.isTagged(failure, "ToolInvocationError")).toBe(true); + // oxlint-disable-next-line executor/no-unknown-error-message -- boundary: asserts the exact caller-facing message the pre-flight failure carries + const message = (failure as { message: string }).message; + expect(message).toContain('Unknown argument "doesNotExist".'); + expect(message).toContain("This operation accepts:"); + expect(message).toContain("itemId"); + }), + ), + ); + + it.effect("accepts the server selector for multi-server operations", () => + Effect.scoped( + Effect.gen(function* () { + const server = yield* servePluginTestApi(); + const executor = yield* createExecutor(makeTestConfig({ plugins: testPlugins() })); + const multiServer = { + ...server, + specJson: makeOpenApiTestSpecJson(TestApi, { + transformSpec: (spec) => ({ + ...spec, + servers: [{ url: "https://unused.example" }, { url: server.baseUrl }], + }), + }), + }; + + const conn = yield* addOpenApiTestConnection( + executor, + multiServer, + { slug: "multi_server", baseUrl: null }, + { value: "token" }, + ); + const result = unwrapInvocation( + yield* executor.execute(conn.address("items.getItem"), { + itemId: "2", + server: { url: server.baseUrl }, + }), + ); + + expect(result.error).toBeNull(); + expect(result.data).toEqual({ id: 2, name: "Gadget" }); + }), + ), + ); + + it.effect("serializes cookie parameters and container aliases after configured cookies", () => + Effect.scoped( + Effect.gen(function* () { + const server = yield* serveOpenApiEchoTestServer({ + transformSpec: withEchoCookieParameter, + }); + const executor = yield* createExecutor(makeTestConfig({ plugins: testPlugins() })); + const conn = yield* addOpenApiTestConnection(executor, server, { + slug: "cookie_params", + headers: { Cookie: "configured=yes" }, + }); + yield* server.clearRequests; + + const result = unwrapInvocation( + yield* executor.execute(conn.address("items.echoHeaders"), { + sessionId: "session-123", + }), + ); + const aliasResult = unwrapInvocation( + yield* executor.execute(conn.address("items.echoHeaders"), { + cookies: { sessionId: "session-456" }, + }), + ); + const requests = yield* server.requests; + + expect(result.error).toBeNull(); + expect(aliasResult.error).toBeNull(); + expect(requests).toHaveLength(2); + expect(requests[0]?.headers.cookie).toBe("configured=yes; sessionId=session-123"); + expect(requests[1]?.headers.cookie).toBe("configured=yes; sessionId=session-456"); + }), + ), + ); + + it.effect("rejects only unknown arguments beside declared cookie parameters", () => + Effect.scoped( + Effect.gen(function* () { + const server = yield* serveOpenApiEchoTestServer({ + transformSpec: withEchoCookieParameter, + }); + const executor = yield* createExecutor(makeTestConfig({ plugins: testPlugins() })); + const conn = yield* addOpenApiTestConnection(executor, server, { + slug: "cookie_unknown", + }); + yield* server.clearRequests; + + const failure = yield* executor + .execute(conn.address("items.echoHeaders"), { + sessionId: "session-123", + doesNotExist: "nope", + }) + .pipe(Effect.flip); + const requests = yield* server.requests; + + expect(Predicate.isTagged(failure, "ToolInvocationError")).toBe(true); + // oxlint-disable-next-line executor/no-unknown-error-message -- boundary: asserts the exact caller-facing message the pre-flight failure carries + const message = (failure as { message: string }).message; + expect(message).toMatch(/^Unknown argument "doesNotExist"\./); + expect(message).toContain("sessionId"); + expect(requests).toEqual([]); + }), + ), + ); + it.effect("surfaces structured validation errors from OpenAPI tool calls", () => Effect.scoped( Effect.gen(function* () { @@ -668,9 +892,6 @@ describe("OpenAPI Plugin", () => { const result = unwrapInvocation( yield* executor.execute(conn.address("items.queryRows"), { entryTypeId: "18538", - query: JSON.stringify([{ DisplayName: "Example" }]), - limit: 10, - skip: 0, }), );