Skip to content

Commit 0e33c5e

Browse files
RhysSullivanRhysSullivan
authored
Reject unknown OpenAPI tool arguments (#1388)
1 parent 4fd94dc commit 0e33c5e

4 files changed

Lines changed: 456 additions & 4 deletions

File tree

‎e2e/scenarios/openapi-unknown-args.test.ts‎

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -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‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ export class OpenApiExtractionError extends Schema.TaggedErrorClass<OpenApiExtra
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‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -821,12 +821,64 @@ const applyRequestBody = (
821821
// `invoke` applies (one code path, not a parallel re-implementation).
822822
// ---------------------------------------------------------------------------
823823

824+
const acceptedArgumentNames = (operation: OperationBinding): readonly string[] => {
825+
const accepted = new Set<string>();
826+
827+
for (const param of operation.parameters) {
828+
accepted.add(param.name);
829+
for (const container of CONTAINER_KEYS[param.location] ?? []) accepted.add(container);
830+
}
831+
832+
for (const match of operation.pathTemplate.matchAll(/\{([^{}]+)\}/g)) {
833+
if (match[1]) accepted.add(match[1]);
834+
}
835+
836+
if (Option.isSome(operation.requestBody)) {
837+
const requestBody = operation.requestBody.value;
838+
const contents = Option.getOrUndefined(requestBody.contents);
839+
accepted.add("body");
840+
accepted.add("input");
841+
if (
842+
isOctetStream(requestBody.contentType) ||
843+
contents?.some((content) => isOctetStream(content.contentType))
844+
) {
845+
accepted.add("bodyBase64");
846+
}
847+
if (contents && contents.length > 1) accepted.add("contentType");
848+
}
849+
850+
const servers = operation.servers ?? [];
851+
const hasServerVariables = servers.some(
852+
(server) => Object.keys(Option.getOrUndefined(server.variables) ?? {}).length > 0,
853+
);
854+
if (servers.length > 1 || hasServerVariables) accepted.add("server");
855+
856+
return [...accepted];
857+
};
858+
824859
export const buildRequest = Effect.fn("OpenApi.buildRequest")(function* (
825860
operation: OperationBinding,
826861
args: Record<string, unknown>,
827862
resolvedHeaders: Record<string, string>,
828863
integrationQueryParams: Record<string, string> = {},
829864
) {
865+
const accepted = acceptedArgumentNames(operation);
866+
const acceptedSet = new Set(accepted);
867+
const unknown = Object.keys(args).filter((name) => !acceptedSet.has(name));
868+
if (unknown.length > 0) {
869+
const label = unknown.length === 1 ? "Unknown argument" : "Unknown arguments";
870+
const names = unknown.map((name) => JSON.stringify(name)).join(", ");
871+
const acceptedMessage =
872+
accepted.length > 0
873+
? `This operation accepts: ${accepted.join(", ")}.`
874+
: "This operation accepts no arguments.";
875+
return yield* new OpenApiInvocationError({
876+
message: `${label} ${names}. ${acceptedMessage}`,
877+
statusCode: Option.none(),
878+
reason: "unknown_arguments",
879+
});
880+
}
881+
830882
const resolvedPath = yield* resolvePath(operation.pathTemplate, args, operation.parameters);
831883

832884
const path = resolvedPath.startsWith("/") ? resolvedPath : `/${resolvedPath}`;
@@ -856,6 +908,14 @@ export const buildRequest = Effect.fn("OpenApi.buildRequest")(function* (
856908
request = HttpClientRequest.setHeader(request, param.name, String(value));
857909
}
858910

911+
const cookieValues: string[] = [];
912+
for (const param of operation.parameters) {
913+
if (param.location !== "cookie") continue;
914+
const value = readParamValue(args, param);
915+
if (value === undefined || value === null) continue;
916+
cookieValues.push(`${param.name}=${primitiveToString(value)}`);
917+
}
918+
859919
if (Option.isSome(operation.requestBody)) {
860920
const rb = operation.requestBody.value;
861921
const contentsOpt = Option.getOrUndefined(rb.contents);
@@ -972,6 +1032,15 @@ export const buildRequest = Effect.fn("OpenApi.buildRequest")(function* (
9721032
}
9731033

9741034
request = applyHeaders(request, resolvedHeaders);
1035+
if (cookieValues.length > 0) {
1036+
const existingCookie = request.headers.cookie;
1037+
const serializedCookies = cookieValues.join("; ");
1038+
request = HttpClientRequest.setHeader(
1039+
request,
1040+
"Cookie",
1041+
existingCookie ? `${existingCookie}; ${serializedCookies}` : serializedCookies,
1042+
);
1043+
}
9751044

9761045
return request;
9771046
});

0 commit comments

Comments
 (0)