History

apps: harden the handle bridge dispatch and deepen the sandbox conformance suite

Make the HandleBridge dispatch strict (grafted from build A): reject an empty/malformed method path, a reserved root, an undeclared root, an out-of-range fan-out index, or an index on a single-connection role, all as typed errors rather than silent resolution. The tool sandbox invoker also rejects any bridge path other than the one reserved channel. Merge A's richer ToolSandbox conformance cases into B's suite: a db.sql write param round-trip (asserting the SQL text + interpolated params cross intact) and a per-index fan-out case (each connections() element addressed distinctly). Add a focused strict-dispatch unit test.

Rhys Sullivan committed 4 months ago

Commit 374f9231 parent 24a1c5e4 files · +236 −17Browse files

Unified · Split

4 files
  1. packages/plugins/apps/src/backing/quickjs-tool-sandbox.ts +10 −1
  2. packages/plugins/apps/src/plugin/bindings.strict.test.ts +80 −0
  3. packages/plugins/apps/src/plugin/bindings.ts +46 −16
  4. packages/plugins/apps/src/seams/tool-sandbox.conformance.ts +100 −0
packages/plugins/apps/src/backing/quickjs-tool-sandbox.ts+10−1
@@ -225,14 +225,23 @@ export const makeQuickjsToolSandbox = (options: QuickjsToolSandboxOptions = {}): · 224 unmodified lines
225 // bridge. Path 0 is `__handle__`; the single arg is {root, path, args}.225 // bridge. Path 0 is `__handle__`; the single arg is {root, path, args}.
226 const invoker: SandboxToolInvoker = {226 const invoker: SandboxToolInvoker = {
227 invoke: (input: { path: string; args: unknown }) => {227 invoke: (input: { path: string; args: unknown }) => {
228 // Strictness (grafted from A): the ONLY reserved bridge path the
229 // invoke phase accepts is `__handle__`. Anything else is a hard
230 // error, never silently ignored — a handler must not reach the host
231 // through an unexpected channel.
228 if (input.path !== "__handle__") {232 if (input.path !== "__handle__") {
229 return Effect.fail(new Error(`unexpected sandbox call: ${input.path}`)) as never;233 return Effect.fail(
234 new Error(`unexpected sandbox bridge path: ${input.path}`),
235 ) as never;
230 }236 }
231 const call = input.args as {237 const call = input.args as {
232 root: string;238 root: string;
233 path: readonly string[];239 path: readonly string[];
234 args: readonly unknown[];240 args: readonly unknown[];
235 };241 };
242 if (!call || typeof call.root !== "string" || !Array.isArray(call.path)) {
243 return Effect.fail(new Error("malformed sandbox bridge call")) as never;
244 }
236 return bridge.call({ root: call.root, path: call.path, args: call.args }) as never;245 return bridge.call({ root: call.root, path: call.path, args: call.args }) as never;
237 },246 },
238 };247 };
packages/plugins/apps/src/plugin/bindings.strict.test.ts+80−0
@@ -0,0 +1,80 @@
1import { describe, expect, it } from "vitest";
2import { Effect } from "effect";
3
4import { buildBridge, type ClientResolver } from "./bindings";
5import type { ConnectionDecl } from "../pipeline/descriptor";
6import type { ScopeDbHandle } from "../seams/scope-db";
7
8// ---------------------------------------------------------------------------
9// Strict HandleBridge dispatch (Fix 4, grafted from build A). The bridge is the
10// ONE channel out of the sandbox, so its dispatch must reject anything
11// malformed, reserved, undeclared, or out-of-range rather than resolve it.
12// ---------------------------------------------------------------------------
13
14const okDb: ScopeDbHandle = {
15 sql: () => Effect.succeed([]),
16 exec: () => Effect.succeed([]),
17 tableVersion: () => Effect.succeed(0),
18 versions: () => Effect.succeed(new Map()),
19};
20
21const okResolver: ClientResolver = {
22 call: () => Effect.succeed({ ok: true }),
23};
24
25const fails = <A, E>(effect: Effect.Effect<A, E>): Promise<boolean> =>
26 Effect.runPromiseExit(effect).then((exit) => exit._tag === "Failure");
27
28describe("HandleBridge strict dispatch", () => {
29 const declared: Record<string, ConnectionDecl> = {
30 gh: { kind: "single", integration: "github" },
31 inboxes: { kind: "array", integration: "gmail" },
32 };
33 const bindings = {
34 gh: { kind: "single", connection: "gh-main" } as const,
35 inboxes: { kind: "array", connections: ["a", "b"] } as const,
36 };
37 const bridge = buildBridge({ declared, bindings, db: okDb, resolver: okResolver });
38
39 it("rejects an empty method path", async () => {
40 expect(await fails(bridge.call({ root: "gh", path: [], args: [] }))).toBe(true);
41 });
42
43 it("rejects a reserved root", async () => {
44 expect(await fails(bridge.call({ root: "__proto__", path: ["x"], args: [] }))).toBe(true);
45 });
46
47 it("rejects an undeclared root", async () => {
48 expect(await fails(bridge.call({ root: "ghost", path: ["x"], args: [] }))).toBe(true);
49 });
50
51 it("rejects an out-of-range fan-out index", async () => {
52 expect(
53 await fails(bridge.call({ root: "inboxes#5", path: ["messages", "list"], args: [] })),
54 ).toBe(true);
55 });
56
57 it("rejects an index on a single-connection role", async () => {
58 expect(await fails(bridge.call({ root: "gh#0", path: ["repos", "list"], args: [] }))).toBe(
59 true,
60 );
61 });
62
63 it("rejects an unsupported db call", async () => {
64 expect(await fails(bridge.call({ root: "db", path: ["drop"], args: [] }))).toBe(true);
65 });
66
67 it("routes a valid single-connection call through the resolver", async () => {
68 const out = await Effect.runPromise(
69 bridge.call({ root: "gh", path: ["repos", "list"], args: [{}] }),
70 );
71 expect(out).toEqual({ ok: true });
72 });
73
74 it("routes a valid fan-out element through the resolver", async () => {
75 const out = await Effect.runPromise(
76 bridge.call({ root: "inboxes#1", path: ["messages", "list"], args: [{}] }),
77 );
78 expect(out).toEqual({ ok: true });
79 });
80});
packages/plugins/apps/src/plugin/bindings.ts+46−16
@@ -120,15 +120,37 @@ const parseRoot = (root: string): { role: string; index?: number } => { · 119 unmodified lines
120 return { role: root.slice(0, hash), index: Number(root.slice(hash + 1)) };120 return { role: root.slice(0, hash), index: Number(root.slice(hash + 1)) };
121};121};
122 122
123// Root names the bridge treats as reserved: nothing the sandbox sends may
124// address them as a connection role. `db` is handled explicitly first; any other
125// reserved prefix (a future internal handle) is rejected rather than silently
126// resolved. (Grafted strictness from A: reject unexpected/reserved calls.)
127const RESERVED_ROOTS = new Set(["__proto__", "constructor", "prototype"]);
128
129const invokeErr = (message: string): ToolSandboxError =>
130 new ToolSandboxError({ kind: "invoke", message });
131
123/**132/**
124 * Build the HandleBridge the sandbox calls out through. `db` routes to the133 * Build the HandleBridge the sandbox calls out through. `db` routes to the
125 * scope database; a declared role routes to its bound connection through the134 * scope database; a declared role routes to its bound connection through the
126 * `ClientResolver`. Undeclared roots are unreachable (the sandbox never injects135 * `ClientResolver`. The dispatch is STRICT: an empty/malformed path, a reserved
127 * them). A `.account` read on a client returns bound-connection metadata136 * root, an undeclared root, or an out-of-range fan-out index is a typed error,
137 * never a silent success (the cloud RPC backing must be able to trust the same
138 * shape). A `.account` read on a client returns bound-connection metadata
128 * without a round-trip.139 * without a round-trip.
129 */140 */
130export const buildBridge = (context: BindingContext): HandleBridge => ({141export const buildBridge = (context: BindingContext): HandleBridge => ({
131 call: ({ root, path, args }) => {142 call: ({ root, path, args }) => {
143 // Strictness: every bridge call must name a root and a non-empty method path.
144 if (typeof root !== "string" || root.length === 0) {
145 return Effect.fail(invokeErr("bridge call is missing a handle root"));
146 }
147 if (!Array.isArray(path) || path.length === 0) {
148 return Effect.fail(invokeErr(`bridge call to "${root}" has an empty method path`));
149 }
150 if (RESERVED_ROOTS.has(root)) {
151 return Effect.fail(invokeErr(`reserved handle root is not callable: ${root}`));
152 }
153
132 if (root === "db") {154 if (root === "db") {
133 // The scope db handle exposes `sql` as a tagged template; the injected155 // The scope db handle exposes `sql` as a tagged template; the injected
134 // client calls `db.sql(templateStrings, ...values)`. When routed through156 // client calls `db.sql(templateStrings, ...values)`. When routed through
@@ -143,29 +165,39 @@ export const buildBridge = (context: BindingContext): HandleBridge => ({ · 8 unmodified lines
143 ),165 ),
144 );166 );
145 }167 }
146 return Effect.fail(168 return Effect.fail(invokeErr(`unsupported db call: ${path.join(".")}`));
147 new ToolSandboxError({ kind: "invoke", message: `unsupported db call: ${path.join(".")}` }),
148 );
149 }169 }
150 170
151 const { role, index } = parseRoot(root);171 const { role, index } = parseRoot(root);
152 const decl = context.declared[role];172 const decl = context.declared[role];
153 if (!decl) {173 if (!decl) {
154 return Effect.fail(174 return Effect.fail(invokeErr(`undeclared handle root: ${root}`));
155 new ToolSandboxError({ kind: "invoke", message: `undeclared handle root: ${root}` }),
156 );
157 }175 }
158 if (decl.kind === "catalog") {176 if (decl.kind === "catalog") {
159 return Effect.fail(177 return Effect.fail(
160 new ToolSandboxError({178 invokeErr("catalog() open-world proxy execution is not implemented in this build"),
161 kind: "invoke",
162 message: "catalog() open-world proxy execution is not implemented in this build",
163 }),
164 );179 );
165 }180 }
166 181
167 // A `.account` read returns bound-connection metadata (safe, no creds).
168 const binding = context.bindings[role];182 const binding = context.bindings[role];
183 // Fan-out roots MUST carry a valid index within the bound array; a single
184 // role MUST NOT carry an index. (Grafted bounds-checking from A.)
185 if (decl.kind === "array") {
186 if (binding?.kind !== "array") {
187 return Effect.fail(invokeErr(`role "${role}" is a fan-out but is not bound to an array`));
188 }
189 if (
190 index === undefined ||
191 !Number.isInteger(index) ||
192 index < 0 ||
193 index >= binding.connections.length
194 ) {
195 return Effect.fail(invokeErr(`fan-out handle "${root}" addresses an out-of-range index`));
196 }
197 } else if (index !== undefined) {
198 return Effect.fail(invokeErr(`single-connection role "${role}" was addressed with an index`));
199 }
200
169 const connectionName =201 const connectionName =
170 binding?.kind === "array"202 binding?.kind === "array"
171 ? binding.connections[index ?? 0]203 ? binding.connections[index ?? 0]
@@ -173,9 +205,7 @@ export const buildBridge = (context: BindingContext): HandleBridge => ({ · 1 unmodified line
173 ? binding.connection205 ? binding.connection
174 : undefined;206 : undefined;
175 if (!connectionName) {207 if (!connectionName) {
176 return Effect.fail(208 return Effect.fail(invokeErr(`no binding for role ${role}`));
177 new ToolSandboxError({ kind: "invoke", message: `no binding for role ${role}` }),
178 );
179 }209 }
180 if (path.length === 1 && path[0] === "account") {210 if (path.length === 1 && path[0] === "account") {
181 // Clients read `.account.email` / `.account.login`; expose both keys.211 // Clients read `.account.email` / `.account.login`; expose both keys.
packages/plugins/apps/src/seams/tool-sandbox.conformance.ts+100−0
@@ -112,5 +112,105 @@ export default defineTool({ · 111 unmodified lines
112 expect(seen[0].path).toEqual(["messages", "search"]);112 expect(seen[0].path).toEqual(["messages", "search"]);
113 expect(seen[0].args[0]).toMatchObject({ q: "invoice" });113 expect(seen[0].args[0]).toMatchObject({ q: "invoice" });
114 });114 });
115
116 // --- richer assertions grafted from build A ---------------------------
117
118 it("round-trips a db.sql write with its parameters intact", async () => {
119 const sandbox = makeSandbox();
120 // A tool with a github client AND a db.sql write, asserting BOTH the
121 // connection call and the parameterized db statement cross the bridge with
122 // their args intact (the db.sql param round-trip A's suite covers).
123 const src = `import { defineTool, connection } from "executor:app";
124import { z } from "zod";
125export default defineTool({
126 description: "bridge",
127 connections: { gh: connection("github") },
128 input: z.object({}),
129 async handler(_i, { gh, db }) {
130 const repos = await gh.repos.listForAuthenticatedUser({ per_page: 5 });
131 await db.sql\`INSERT INTO log (n) VALUES (\${repos.length})\`;
132 return { count: repos.length };
133 },
134});`;
135 const b = await bundle("tools/bridge.ts", src);
136 const calls: { root: string; path: readonly string[]; args: readonly unknown[] }[] = [];
137 const bridge: HandleBridge = {
138 call: ({ root, path, args }) =>
139 Effect.sync(() => {
140 calls.push({ root, path, args });
141 if (root === "gh" && path.join(".") === "repos.listForAuthenticatedUser") {
142 return [{ full_name: "a/b" }, { full_name: "c/d" }];
143 }
144 return [];
145 }),
146 };
147 const result = await run(
148 sandbox.invoke(
149 b,
150 {
151 artifact: "bridge",
152 kind: "tool",
153 input: {},
154 roots: { gh: { kind: "single" }, db: { kind: "single" } },
155 },
156 bridge,
157 ),
158 );
159 expect(result.output).toEqual({ count: 2 });
160 // The github call crossed the bridge.
161 expect(calls.some((c) => c.root === "gh")).toBe(true);
162 // The db.sql write crossed as ["sql"] with the template strings + the
163 // interpolated parameter (2) intact.
164 const dbCall = calls.find((c) => c.root === "db");
165 expect(dbCall?.path).toEqual(["sql"]);
166 const [strings, ...params] = dbCall!.args as [readonly string[], ...unknown[]];
167 expect(strings.join("?")).toContain("INSERT INTO log");
168 expect(params).toEqual([2]);
169 });
170
171 it("fans out per-index, each element addressed distinctly", async () => {
172 const sandbox = makeSandbox();
173 // inbox i returns (i+1) messages; the handler pushes each count, so the
174 // ordered result proves each fan-out element was addressed with its index.
175 const src = `import { defineTool, connections } from "executor:app";
176import { z } from "zod";
177export default defineTool({
178 description: "fanout",
179 connections: { inboxes: connections("gmail") },
180 input: z.object({}),
181 async handler(_i, { inboxes }) {
182 const counts = [];
183 for (const inbox of inboxes) {
184 const r = await inbox.messages.search({ q: "x" });
185 counts.push(r.length);
186 }
187 return { inboxes: inboxes.length, counts };
188 },
189});`;
190 const b = await bundle("tools/fanout2.ts", src);
191 const bridge: HandleBridge = {
192 call: ({ root, path }) =>
193 Effect.sync(() => {
194 if (path.join(".") === "messages.search") {
195 const idx = Number(root.split("#")[1]);
196 return new Array(idx + 1).fill({ id: "m" });
197 }
198 return [];
199 }),
200 };
201 const result = await run(
202 sandbox.invoke(
203 b,
204 {
205 artifact: "fanout2",
206 kind: "tool",
207 input: {},
208 roots: { inboxes: { kind: "array", count: 3 } },
209 },
210 bridge,
211 ),
212 );
213 expect(result.output).toEqual({ inboxes: 3, counts: [1, 2, 3] });
214 });
115 });215 });
116};216};