History

Route tool defects opaquely; remove legacy envelope shim (#826)

* feat(tool-result): add ToolResult union and dual-shape invoker Adds `ToolResult<T>` / `ToolError` to @executor-js/sdk and a `ToolResult.ok` / `ToolResult.fail` constructor pair. Updates the sandbox tool dispatcher to accept both the new typed union (passes through unchanged) and the legacy `{ data, error }` envelope. The legacy branch now walks known upstream shapes (Microsoft Graph, DealCloud, JSON:API, plain message-bearing bodies) before falling back to a clamped JSON.stringify of the body, so structured 4xx payloads no longer collapse to 'Tool execution failed'. The cause/defect branch keeps its strict .message-only discipline. * feat(plugins): emit ToolResult from openapi, graphql, mcp, google-discovery Migrates each dynamic plugin's invokeTool to wrap its result in the typed ToolResult<T> union. OpenAPI and Google Discovery map non-2xx responses to ToolResult.fail with an extracted upstream message and the raw body in error.details; 2xx responses to ToolResult.ok with { status, headers, data }. GraphQL maps 200-with-errors bodies to ToolResult.fail({ code: 'graphql_errors', ... }) and bare data to ToolResult.ok. MCP maps { isError: true } to ToolResult.fail and content arrays to ToolResult.ok. Static plugin tools (executor.openapi.previewSpec / addSource, executor.graphql.addSource) now also wrap their plain values in ToolResult.ok for shape consistency. Updates plugin tests that asserted on the old { status, headers, data, error } envelope or the bare MCP { content } shape. Tests that consume the OpenAPI plugin's invocation result use the new unwrapInvocation helper from @executor-js/plugin-openapi/testing. * feat(tool-result): make ToolResult mandatory and route defects opaquely Removes the legacy { data, error } envelope shim from the sandbox tool dispatcher. The invoker now passes ToolResult<T> through unchanged and wraps any other plain-value plugin return in ToolResult.ok so the sandbox surface is uniform. Plugin/infra defects no longer pass their message into the sandbox. The dispatcher generates a short hex correlation id, logs the full cause with that id under executor.correlation_id, and rejects with `Internal tool error [<corrId>]`. The QuickJS bridge and the dynamic-worker module template defensively re-stamp the same opaque shape; the MCP host server's top-level execute failure path does the same. ExecutionToolError in-band messages from the execution package's built-in validators (tools.search arg checks, etc.) are still passed through at the QuickJS bridge so model-facing input errors keep their useful diagnostic. Tests: - ToolResult.ok / fail / isToolResult constructor unit tests. - repro tests assert structured upstream payloads now reach the sandbox through ToolResult.error.details (not through .message). - leak tests pin the new invariant: plugin defects only escape as the opaque generic + correlation id; no token / connection string / file path leaks into Error.message. - QuickJS end-to-end defect test confirms the same shape at the sandbox boundary. - Cloud HTTP integration tests, MCP host tests, dynamic-worker invocation tests, and plugin tests updated for the new ToolResult wire shape and the opaque-generic defect contract. * Fix tool result contracts

Rhys Sullivan committed 6 months ago

Commit 278ca7c1 parent 8d986ff35 files · +1322 −340Browse files

Showing the changes to packages/kernel/runtime-quickjs/src (2 of 35 files). Show all

Unified · Split

2 files
  1. packages/kernel/runtime-quickjs/src/index.test.ts +46 −0
  2. packages/kernel/runtime-quickjs/src/index.ts +35 −1
packages/kernel/runtime-quickjs/src/index.test.ts+46−0
@@ -121,6 +121,52 @@ describe("quickjs executor", () => { · 120 unmodified lines
121121 }),
122122 );
123123
124+ it.effect("internal defects reach the sandbox as an opaque generic only", () =>
125+ Effect.gen(function* () {
126+ // Plugin defect carrying sensitive context. The bridge's reject
127+ // path must strip everything except the canonical
128+ // "Internal tool error [<corrId>]" shape — or fall back to the
129+ // bare generic if the upstream invoker hasn't already stamped
130+ // the correlation id (this test exercises the latter path
131+ // because it bypasses makeExecutorToolInvoker).
132+ const invoker: SandboxToolInvoker = {
133+ invoke: () =>
134+ Effect.fail(
135+ Object.assign(
136+ new Error("Authorization: Bearer SECRET_TOKEN_xyz failed against host 10.0.0.5"),
137+ {
138+ stack: "Error\n at /home/svc/executor/packages/plugins/foo:142:11",
139+ },
140+ ) as never,
141+ ),
142+ };
143+
144+ const result = yield* executor.execute(
145+ `
146+ try {
147+ await tools.leaky.call({});
148+ return "should not reach";
149+ } catch (e) {
150+ return e.message;
151+ }
152+ `,
153+ invoker,
154+ );
155+
156+ expect(result.error).toBeUndefined();
157+ const message = String(result.result);
158+ // Either the canonical opaque generic with a correlation id, or
159+ // the bare fallback. Neither must contain any sensitive context.
160+ expect(
161+ message === "Internal tool error" || /^Internal tool error \[[0-9a-f]{8}\]$/.test(message),
162+ ).toBe(true);
163+ expect(message).not.toContain("SECRET_TOKEN_xyz");
164+ expect(message).not.toContain("Authorization");
165+ expect(message).not.toContain("10.0.0.5");
166+ expect(message).not.toContain("packages/plugins");
167+ }),
168+ );
169+
124170 it.effect("handles unknown tool path", () =>
125171 Effect.gen(function* () {
126172 const invoker = makeTestInvoker({});
packages/kernel/runtime-quickjs/src/index.ts+35−1
@@ -47,6 +47,30 @@ const EXECUTION_FILENAME = "executor-quickjs-runtime.js"; · 46 unmodified lines
4747 const toError = (cause: unknown): Error =>
4848 cause instanceof Error ? cause : new Error(String(cause));
4949
50+// Defect surfaced to the sandbox when a tool dispatch reaches the
51+// reject path. Two sources reach here:
52+// - `ExecutionToolError` emitted by the execution package — message
53+// is already either the canonical
54+// `Internal tool error [<corrId>]` opaque generic (plugin defects
55+// passed through `makeExecutorToolInvoker`'s catchCause) or an
56+// intentional public string (validators in engine.ts:
57+// "tools.search expects an object: ...").
58+// - Anything else — assume infra defect and emit a bare opaque
59+// generic.
60+// `_tag === "ExecutionToolError"` is the in-band signal that the
61+// message was generated by code we control and is safe to pass
62+// through; we never read `.cause` / `.stack`.
63+const sandboxDefectMessage = (cause: unknown): string => {
64+ if (cause !== null && typeof cause === "object") {
65+ const tagged = cause as { readonly _tag?: unknown; readonly message?: unknown };
66+ // oxlint-disable-next-line executor/no-manual-tag-check, executor/no-unknown-error-message -- boundary: QuickJS reject path receives an already-rendered ExecutionToolError; we pass its in-band public message through and otherwise emit the opaque generic
67+ if (tagged._tag === "ExecutionToolError" && typeof tagged.message === "string") {
68+ return tagged.message;
69+ }
70+ }
71+ return "Internal tool error";
72+};
73+
5074 const toErrorMessage = (cause: unknown): string => {
5175 if (typeof cause === "object" && cause !== null) {
5276 const message =
@@ -216,7 +240,17 @@ const createToolBridge = ( · 163 unmodified lines
216240 return;
217241 }
218242
219− const errorHandle = context.newError(toErrorMessage(cause));
243+ // The reject path is reserved for true infra defects. The
244+ // upstream tool-invoker has already replaced the message with
245+ // an opaque generic plus a correlation id, but defensively log
246+ // the cause here and emit a stable generic if upstream changes.
247+ const message = sandboxDefectMessage(cause);
248+ try {
249+ console.error("[executor:quickjs] tool dispatch defect", { path, cause });
250+ } catch {
251+ /* ignore logger failures */
252+ }
253+ const errorHandle = context.newError(message);
220254 deferred.reject(errorHandle);
221255 errorHandle.dispose();
222256 },