History

Reject unknown OpenAPI tool arguments

Rhys Sullivan committed 4 months ago

Commit 39abea31 parent dee68b34 files · +456 −4Browse files

Unified · Split

4 files
  1. e2e/scenarios/openapi-unknown-args.test.ts +162 −0
  2. packages/plugins/openapi/src/sdk/errors.ts +1 −1
  3. packages/plugins/openapi/src/sdk/invoke.ts +69 −0
  4. packages/plugins/openapi/src/sdk/plugin.test.ts +224 −3
e2e/scenarios/openapi-unknown-args.test.ts+162−0
@@ -0,0 +1,162 @@
1+import { randomBytes } from "node:crypto";
2+import { createServer } from "node:http";
3+
4+import { expect } from "@effect/vitest";
5+import { Effect } from "effect";
6+import { composePluginApi } from "@executor-js/api/server";
7+import { openApiHttpPlugin } from "@executor-js/plugin-openapi/api";
8+import { AuthTemplateSlug, ConnectionName, IntegrationSlug } from "@executor-js/sdk/shared";
9+
10+import { scenario } from "../src/scenario";
11+import { Api, Target } from "../src/services";
12+
13+const api = composePluginApi([openApiHttpPlugin()] as const);
14+
15+const serveRecordingUpstream = () =>
16+ Effect.acquireRelease(
17+ Effect.callback<{
18+ readonly url: string;
19+ readonly requests: () => readonly string[];
20+ readonly close: () => void;
21+ }>((resume) => {
22+ const recorded: string[] = [];
23+ const server = createServer((request, response) => {
24+ recorded.push(request.url ?? "");
25+ response.writeHead(200, { "content-type": "application/json" });
26+ response.end(JSON.stringify({ id: "2", name: "Gadget" }));
27+ });
28+ server.listen(0, "127.0.0.1", () => {
29+ const address = server.address();
30+ const port = typeof address === "object" && address ? address.port : 0;
31+ resume(
32+ Effect.succeed({
33+ url: `http://127.0.0.1:${port}`,
34+ requests: () => [...recorded],
35+ close: () => {
36+ server.close();
37+ server.closeAllConnections();
38+ },
39+ }),
40+ );
41+ });
42+ }),
43+ (server) => Effect.sync(server.close),
44+ );
45+
46+const itemsSpec = (baseUrl: string): string =>
47+ JSON.stringify({
48+ openapi: "3.0.3",
49+ info: { title: "Items API", version: "1.0.0" },
50+ servers: [{ url: baseUrl }],
51+ paths: {
52+ "/items/{itemId}": {
53+ get: {
54+ operationId: "getItem",
55+ summary: "Fetch one item",
56+ parameters: [{ name: "itemId", in: "path", required: true, schema: { type: "string" } }],
57+ responses: { "200": { description: "the item" } },
58+ },
59+ },
60+ },
61+ });
62+
63+const invokeByAddressCode = (address: string, args: unknown) => `
64+const segments = ${JSON.stringify(address)}.split(".").slice(1);
65+let node = tools;
66+for (const segment of segments) node = node[segment];
67+const result = await node(${JSON.stringify(args)});
68+return JSON.stringify(result);
69+`;
70+
71+type ToolEnvelope = {
72+ readonly ok: boolean;
73+ readonly error?: {
74+ readonly code?: string;
75+ readonly message?: string;
76+ };
77+};
78+
79+scenario(
80+ "OpenAPI: unknown tool arguments fail locally before any upstream request",
81+ {},
82+ Effect.scoped(
83+ Effect.gen(function* () {
84+ const target = yield* Target;
85+ const { client: makeClient } = yield* Api;
86+ const identity = yield* target.newIdentity();
87+ const client = yield* makeClient(api, identity);
88+ const upstream = yield* serveRecordingUpstream();
89+ const slug = `openapi_unknown_args_${randomBytes(4).toString("hex")}`;
90+
91+ yield* Effect.ensuring(
92+ Effect.gen(function* () {
93+ yield* client.openapi.addSpec({
94+ payload: {
95+ spec: { kind: "blob", value: itemsSpec(upstream.url) },
96+ slug,
97+ baseUrl: upstream.url,
98+ authenticationTemplate: [
99+ {
100+ slug: "apiKey",
101+ type: "apiKey",
102+ headers: { authorization: ["Bearer ", { type: "variable", name: "token" }] },
103+ },
104+ ],
105+ },
106+ });
107+ yield* client.connections.create({
108+ payload: {
109+ owner: "org",
110+ name: ConnectionName.make("main"),
111+ integration: IntegrationSlug.make(slug),
112+ template: AuthTemplateSlug.make("apiKey"),
113+ value: "tok_unknown_args",
114+ },
115+ });
116+
117+ const tools = yield* client.tools.list({ query: {} });
118+ const address = tools
119+ .filter((tool) => String(tool.integration) === slug)
120+ .map((tool) => String(tool.address))
121+ .find((candidate) => candidate.endsWith("getItem"));
122+ expect(address, "the getItem operation is available").toBeDefined();
123+
124+ const executed = yield* client.executions.execute({
125+ payload: {
126+ code: invokeByAddressCode(address!, { itemId: "2", doesNotExist: "nope" }),
127+ autoApprove: true,
128+ },
129+ });
130+ expect(executed.status, "the sandbox returns the tool failure as a value").toBe(
131+ "completed",
132+ );
133+ const outcome = JSON.parse(executed.text) as ToolEnvelope;
134+ expect(outcome.ok, "the unknown argument fails the tool call").toBe(false);
135+ expect(outcome.error?.code, "the failure is classified as invalid arguments").toBe(
136+ "invalid_tool_arguments",
137+ );
138+ expect(
139+ outcome.error?.message,
140+ "the failure names the argument the caller must remove",
141+ ).toContain("doesNotExist");
142+ expect(
143+ upstream.requests(),
144+ "argument validation stops the call before upstream dispatch",
145+ ).toEqual([]);
146+ }),
147+ Effect.gen(function* () {
148+ yield* client.connections
149+ .remove({
150+ params: {
151+ owner: "org",
152+ integration: IntegrationSlug.make(slug),
153+ name: ConnectionName.make("main"),
154+ },
155+ })
156+ .pipe(Effect.ignore);
157+ yield* client.openapi.removeSpec({ params: { slug } }).pipe(Effect.ignore);
158+ }),
159+ );
160+ }),
161+ ),
162+);
packages/plugins/openapi/src/sdk/errors.ts+1−1
@@ -28,7 +28,7 @@ export class OpenApiExtractionError extends Schema.TaggedErrorClass<OpenApiExtra · 27 unmodified lines
2828 export class OpenApiInvocationError extends Data.TaggedError("OpenApiInvocationError")<{
2929 readonly message: string;
3030 readonly statusCode: Option.Option<number>;
31− readonly reason?: "response_headers_timeout";
31+ readonly reason?: "response_headers_timeout" | "unknown_arguments";
3232 readonly cause?: unknown;
3333 }> {}
3434
packages/plugins/openapi/src/sdk/invoke.ts+69−0
@@ -835,12 +835,64 @@ const applyRequestBody = ( · 834 unmodified lines
835835 // `invoke` applies (one code path, not a parallel re-implementation).
836836 // ---------------------------------------------------------------------------
837837
838+const acceptedArgumentNames = (operation: OperationBinding): readonly string[] => {
839+ const accepted = new Set<string>();
840+
841+ for (const param of operation.parameters) {
842+ accepted.add(param.name);
843+ for (const container of CONTAINER_KEYS[param.location] ?? []) accepted.add(container);
844+ }
845+
846+ for (const match of operation.pathTemplate.matchAll(/\{([^{}]+)\}/g)) {
847+ if (match[1]) accepted.add(match[1]);
848+ }
849+
850+ if (Option.isSome(operation.requestBody)) {
851+ const requestBody = operation.requestBody.value;
852+ const contents = Option.getOrUndefined(requestBody.contents);
853+ accepted.add("body");
854+ accepted.add("input");
855+ if (
856+ isOctetStream(requestBody.contentType) ||
857+ contents?.some((content) => isOctetStream(content.contentType))
858+ ) {
859+ accepted.add("bodyBase64");
860+ }
861+ if (contents && contents.length > 1) accepted.add("contentType");
862+ }
863+
864+ const servers = operation.servers ?? [];
865+ const hasServerVariables = servers.some(
866+ (server) => Object.keys(Option.getOrUndefined(server.variables) ?? {}).length > 0,
867+ );
868+ if (servers.length > 1 || hasServerVariables) accepted.add("server");
869+
870+ return [...accepted];
871+};
872+
838873 export const buildRequest = Effect.fn("OpenApi.buildRequest")(function* (
839874 operation: OperationBinding,
840875 args: Record<string, unknown>,
841876 resolvedHeaders: Record<string, string>,
842877 integrationQueryParams: Record<string, string> = {},
843878 ) {
879+ const accepted = acceptedArgumentNames(operation);
880+ const acceptedSet = new Set(accepted);
881+ const unknown = Object.keys(args).filter((name) => !acceptedSet.has(name));
882+ if (unknown.length > 0) {
883+ const label = unknown.length === 1 ? "Unknown argument" : "Unknown arguments";
884+ const names = unknown.map((name) => JSON.stringify(name)).join(", ");
885+ const acceptedMessage =
886+ accepted.length > 0
887+ ? `This operation accepts: ${accepted.join(", ")}.`
888+ : "This operation accepts no arguments.";
889+ return yield* new OpenApiInvocationError({
890+ message: `${label} ${names}. ${acceptedMessage}`,
891+ statusCode: Option.none(),
892+ reason: "unknown_arguments",
893+ });
894+ }
895+
844896 const resolvedPath = yield* resolvePath(operation.pathTemplate, args, operation.parameters);
845897
846898 const path = resolvedPath.startsWith("/") ? resolvedPath : `/${resolvedPath}`;
@@ -870,6 +922,14 @@ export const buildRequest = Effect.fn("OpenApi.buildRequest")(function* ( · 23 unmodified lines
870922 request = HttpClientRequest.setHeader(request, param.name, String(value));
871923 }
872924
925+ const cookieValues: string[] = [];
926+ for (const param of operation.parameters) {
927+ if (param.location !== "cookie") continue;
928+ const value = readParamValue(args, param);
929+ if (value === undefined || value === null) continue;
930+ cookieValues.push(`${param.name}=${primitiveToString(value)}`);
931+ }
932+
873933 if (Option.isSome(operation.requestBody)) {
874934 const rb = operation.requestBody.value;
875935 const contentsOpt = Option.getOrUndefined(rb.contents);
@@ -986,6 +1046,15 @@ export const buildRequest = Effect.fn("OpenApi.buildRequest")(function* ( · 110 unmodified lines
9861046 }
9871047
9881048 request = applyHeaders(request, resolvedHeaders);
1049+ if (cookieValues.length > 0) {
1050+ const existingCookie = request.headers.cookie;
1051+ const serializedCookies = cookieValues.join("; ");
1052+ request = HttpClientRequest.setHeader(
1053+ request,
1054+ "Cookie",
1055+ existingCookie ? `${existingCookie}; ${serializedCookies}` : serializedCookies,
1056+ );
1057+ }
9891058
9901059 return request;
9911060 });
packages/plugins/openapi/src/sdk/plugin.test.ts+224−3
@@ -42,7 +42,9 @@ import { type AuthenticationInput } from "./types"; · 41 unmodified lines
4242 import {
4343 addOpenApiTestConnection,
4444 makeOpenApiHttpApiTestIntegrationConfig,
45+ makeOpenApiTestSpecJson,
4546 serveMutableOpenApiSpecTestServer,
47+ serveOpenApiEchoTestServer,
4648 serveOpenApiHttpApiTestServer,
4749 unwrapInvocation,
4850 } from "../testing";
@@ -258,6 +260,40 @@ const servePluginTestApi = () => · 209 unmodified lines
258260 handlersLayer: ItemsGroupLive,
259261 });
260262
263+const recordValue = (value: unknown): Record<string, unknown> | undefined =>
264+ typeof value === "object" && value !== null && !Array.isArray(value)
265+ ? Object.fromEntries(Object.entries(value))
266+ : undefined;
267+
268+const withEchoCookieParameter = (spec: Record<string, unknown>): Record<string, unknown> => {
269+ const paths = recordValue(spec.paths);
270+ const pathItem = recordValue(paths?.["/echo-headers"]);
271+ const get = recordValue(pathItem?.get);
272+ if (!paths || !pathItem || !get) return spec;
273+ const parameters = Array.isArray(get.parameters) ? get.parameters : [];
274+ return {
275+ ...spec,
276+ paths: {
277+ ...paths,
278+ "/echo-headers": {
279+ ...pathItem,
280+ get: {
281+ ...get,
282+ parameters: [
283+ ...parameters,
284+ {
285+ name: "sessionId",
286+ in: "cookie",
287+ required: true,
288+ schema: { type: "string" },
289+ },
290+ ],
291+ },
292+ },
293+ },
294+ };
295+};
296+
261297 // An apiKey auth template that places the connection value into `x-api-key`.
262298 const apiKeyTemplate: AuthenticationInput = {
263299 slug: AuthTemplateSlug.make("apiKey"),
@@ -599,6 +635,72 @@ describe("OpenAPI Plugin", () => { · 335 unmodified lines
599635 ),
600636 );
601637
638+ it.effect("rejects unknown args before raising the approval elicitation", () =>
639+ Effect.scoped(
640+ Effect.gen(function* () {
641+ const server = yield* servePluginTestApi();
642+ const executor = yield* createExecutor(makeTestConfig({ plugins: testPlugins() }));
643+
644+ const conn = yield* addOpenApiTestConnection(executor, server, { slug: "test" });
645+ const calls = { count: 0 };
646+ const failure = yield* executor
647+ .execute(
648+ conn.address("items.createItem"),
649+ { body: { name: "New item" }, doesNotExist: "nope" },
650+ {
651+ onElicitation: () =>
652+ Effect.sync(() => {
653+ calls.count++;
654+ return { action: "accept" as const, content: {} };
655+ }),
656+ },
657+ )
658+ .pipe(Effect.flip);
659+
660+ expect(calls.count).toBe(0);
661+ expect(Predicate.isTagged(failure, "ToolInvocationError")).toBe(true);
662+ // oxlint-disable-next-line executor/no-unknown-error-message -- boundary: asserts the exact caller-facing message the pre-flight failure carries
663+ const message = (failure as { message: string }).message;
664+ expect(message).toContain('Unknown argument "doesNotExist".');
665+ expect(message).toContain("This operation accepts:");
666+ expect(message).toContain("body");
667+ }),
668+ ),
669+ );
670+
671+ it.effect("redirects requestBody callers to the accepted body argument", () =>
672+ Effect.scoped(
673+ Effect.gen(function* () {
674+ const server = yield* servePluginTestApi();
675+ const executor = yield* createExecutor(makeTestConfig({ plugins: testPlugins() }));
676+
677+ const conn = yield* addOpenApiTestConnection(executor, server, { slug: "test" });
678+ const calls = { count: 0 };
679+ const failure = yield* executor
680+ .execute(
681+ conn.address("items.createItem"),
682+ { requestBody: { name: "New item" } },
683+ {
684+ onElicitation: () =>
685+ Effect.sync(() => {
686+ calls.count++;
687+ return { action: "accept" as const, content: {} };
688+ }),
689+ },
690+ )
691+ .pipe(Effect.flip);
692+
693+ expect(calls.count).toBe(0);
694+ expect(Predicate.isTagged(failure, "ToolInvocationError")).toBe(true);
695+ // oxlint-disable-next-line executor/no-unknown-error-message -- boundary: asserts the exact caller-facing message the pre-flight failure carries
696+ const message = (failure as { message: string }).message;
697+ expect(message).toContain('Unknown argument "requestBody".');
698+ expect(message).toContain("This operation accepts:");
699+ expect(message).toContain("body");
700+ }),
701+ ),
702+ );
703+
602704 it.effect("describes OpenAPI invocation results payload-first with http meta beside data", () =>
603705 Effect.scoped(
604706 Effect.gen(function* () {
@@ -657,6 +759,128 @@ describe("OpenAPI Plugin", () => { · 52 unmodified lines
657759 ),
658760 );
659761
762+ it.effect("rejects unknown GET arguments locally", () =>
763+ Effect.scoped(
764+ Effect.gen(function* () {
765+ const server = yield* servePluginTestApi();
766+ const executor = yield* createExecutor(makeTestConfig({ plugins: testPlugins() }));
767+
768+ const conn = yield* addOpenApiTestConnection(executor, server, { slug: "test" });
769+ const failure = yield* executor
770+ .execute(conn.address("items.getItem"), {
771+ itemId: "2",
772+ doesNotExist: "nope",
773+ })
774+ .pipe(Effect.flip);
775+
776+ expect(Predicate.isTagged(failure, "ToolInvocationError")).toBe(true);
777+ // oxlint-disable-next-line executor/no-unknown-error-message -- boundary: asserts the exact caller-facing message the pre-flight failure carries
778+ const message = (failure as { message: string }).message;
779+ expect(message).toContain('Unknown argument "doesNotExist".');
780+ expect(message).toContain("This operation accepts:");
781+ expect(message).toContain("itemId");
782+ }),
783+ ),
784+ );
785+
786+ it.effect("accepts the server selector for multi-server operations", () =>
787+ Effect.scoped(
788+ Effect.gen(function* () {
789+ const server = yield* servePluginTestApi();
790+ const executor = yield* createExecutor(makeTestConfig({ plugins: testPlugins() }));
791+ const multiServer = {
792+ ...server,
793+ specJson: makeOpenApiTestSpecJson(TestApi, {
794+ transformSpec: (spec) => ({
795+ ...spec,
796+ servers: [{ url: "https://unused.example" }, { url: server.baseUrl }],
797+ }),
798+ }),
799+ };
800+
801+ const conn = yield* addOpenApiTestConnection(
802+ executor,
803+ multiServer,
804+ { slug: "multi_server", baseUrl: null },
805+ { value: "token" },
806+ );
807+ const result = unwrapInvocation(
808+ yield* executor.execute(conn.address("items.getItem"), {
809+ itemId: "2",
810+ server: { url: server.baseUrl },
811+ }),
812+ );
813+
814+ expect(result.error).toBeNull();
815+ expect(result.data).toEqual({ id: 2, name: "Gadget" });
816+ }),
817+ ),
818+ );
819+
820+ it.effect("serializes cookie parameters and container aliases after configured cookies", () =>
821+ Effect.scoped(
822+ Effect.gen(function* () {
823+ const server = yield* serveOpenApiEchoTestServer({
824+ transformSpec: withEchoCookieParameter,
825+ });
826+ const executor = yield* createExecutor(makeTestConfig({ plugins: testPlugins() }));
827+ const conn = yield* addOpenApiTestConnection(executor, server, {
828+ slug: "cookie_params",
829+ headers: { Cookie: "configured=yes" },
830+ });
831+ yield* server.clearRequests;
832+
833+ const result = unwrapInvocation(
834+ yield* executor.execute(conn.address("items.echoHeaders"), {
835+ sessionId: "session-123",
836+ }),
837+ );
838+ const aliasResult = unwrapInvocation(
839+ yield* executor.execute(conn.address("items.echoHeaders"), {
840+ cookies: { sessionId: "session-456" },
841+ }),
842+ );
843+ const requests = yield* server.requests;
844+
845+ expect(result.error).toBeNull();
846+ expect(aliasResult.error).toBeNull();
847+ expect(requests).toHaveLength(2);
848+ expect(requests[0]?.headers.cookie).toBe("configured=yes; sessionId=session-123");
849+ expect(requests[1]?.headers.cookie).toBe("configured=yes; sessionId=session-456");
850+ }),
851+ ),
852+ );
853+
854+ it.effect("rejects only unknown arguments beside declared cookie parameters", () =>
855+ Effect.scoped(
856+ Effect.gen(function* () {
857+ const server = yield* serveOpenApiEchoTestServer({
858+ transformSpec: withEchoCookieParameter,
859+ });
860+ const executor = yield* createExecutor(makeTestConfig({ plugins: testPlugins() }));
861+ const conn = yield* addOpenApiTestConnection(executor, server, {
862+ slug: "cookie_unknown",
863+ });
864+ yield* server.clearRequests;
865+
866+ const failure = yield* executor
867+ .execute(conn.address("items.echoHeaders"), {
868+ sessionId: "session-123",
869+ doesNotExist: "nope",
870+ })
871+ .pipe(Effect.flip);
872+ const requests = yield* server.requests;
873+
874+ expect(Predicate.isTagged(failure, "ToolInvocationError")).toBe(true);
875+ // oxlint-disable-next-line executor/no-unknown-error-message -- boundary: asserts the exact caller-facing message the pre-flight failure carries
876+ const message = (failure as { message: string }).message;
877+ expect(message).toMatch(/^Unknown argument "doesNotExist"\./);
878+ expect(message).toContain("sessionId");
879+ expect(requests).toEqual([]);
880+ }),
881+ ),
882+ );
883+
660884 it.effect("surfaces structured validation errors from OpenAPI tool calls", () =>
661885 Effect.scoped(
662886 Effect.gen(function* () {
@@ -668,9 +892,6 @@ describe("OpenAPI Plugin", () => { · 5 unmodified lines
668892 const result = unwrapInvocation(
669893 yield* executor.execute(conn.address("items.queryRows"), {
670894 entryTypeId: "18538",
671− query: JSON.stringify([{ DisplayName: "Example" }]),
672− limit: 10,
673− skip: 0,
674895 }),
675896 );
676897

Keyboard shortcuts

General

Show keyboard shortcuts
?
Focus the current search
/

Issues and pull requests

Open filters
f

Issues, pull requests and Inbox

Select next item
j↓
Select previous item
k↑
Open selected item
Enter
Dismiss filters or clear selection
Esc

Repository code

Go to file
t