diff --git a/.changeset/secure-toolkit-workspace-policies.md b/.changeset/secure-toolkit-workspace-policies.md new file mode 100644 index 0000000000..9382c718e2 --- /dev/null +++ b/.changeset/secure-toolkit-workspace-policies.md @@ -0,0 +1,5 @@ +--- +"@executor-js/sdk": patch +--- + +Enforce workspace approval and block policies when tools run through a toolkit-scoped executor. diff --git a/packages/core/sdk/src/executor.ts b/packages/core/sdk/src/executor.ts index 6916233cb5..f5f05f3c45 100644 --- a/packages/core/sdk/src/executor.ts +++ b/packages/core/sdk/src/executor.ts @@ -153,6 +153,7 @@ import { } from "./oauth-client"; import type { FirstPartyOAuthClientConfig } from "./oauth-client"; import { + combineEffectivePolicies, comparePolicyRow, isValidPattern, matchPattern, @@ -5290,6 +5291,7 @@ export const createExecutor = EffectivePolicy; + readonly workspaceRows: readonly ToolPolicyRow[]; }; const compareProviderPolicyRule = ( @@ -5331,53 +5334,63 @@ export const createExecutor = => - activeToolPolicyProvider - ? // Batched per-operation resolver: fetch all policy + connection state - // once, then resolve every tool in this operation against that - // snapshot. Avoids the per-tool resolve N+1 on the list surface. - activeToolPolicyProvider.prepare - ? activeToolPolicyProvider.prepare().pipe( - Effect.map((resolve) => ({ - kind: "prepared" as const, - resolve, - })), - ) - : activeToolPolicyProvider.resolve - ? Effect.succeed({ - kind: "provider" as const, - provider: activeToolPolicyProvider, - rules: null, - }) - : activeToolPolicyProvider.list().pipe( - Effect.map((rules) => ({ - kind: "provider" as const, - provider: activeToolPolicyProvider!, - rules, - })), - ) - : core - .findMany("tool_policy", {}) - .pipe(Effect.map((rows) => ({ kind: "global" as const, rows }))); + Effect.gen(function* () { + // Fetch workspace policy once per operation, then reuse the provider's + // prepared snapshot for every tool on the list surface. + const workspaceRows = yield* core.findMany("tool_policy", {}); + if (!activeToolPolicyProvider) { + return { kind: "global" as const, rows: workspaceRows }; + } + if (activeToolPolicyProvider.prepare) { + const resolve = yield* activeToolPolicyProvider.prepare(); + return { kind: "prepared" as const, resolve, workspaceRows }; + } + if (activeToolPolicyProvider.resolve) { + return { + kind: "provider" as const, + provider: activeToolPolicyProvider, + rules: null, + workspaceRows, + }; + } + const rules = yield* activeToolPolicyProvider.list(); + return { + kind: "provider" as const, + provider: activeToolPolicyProvider, + rules, + workspaceRows, + }; + }); const resolvePolicyFromRuleSet = ( toolId: string, ruleSet: ActivePolicyRuleSet, defaultRequiresApproval?: boolean, ): Effect.Effect => - ruleSet.kind === "prepared" - ? Effect.succeed(ruleSet.resolve({ toolId, defaultRequiresApproval })) - : ruleSet.kind === "provider" - ? ruleSet.provider.resolve - ? ruleSet.provider.resolve({ toolId, defaultRequiresApproval }) - : Effect.succeed(resolveProviderPolicyFromRules(toolId, ruleSet.rules ?? [])) - : Effect.succeed( - resolveEffectivePolicy( - toolId, - ruleSet.rows, - ownerRankForRow, - defaultRequiresApproval, - ), - ); + Effect.gen(function* () { + if (ruleSet.kind === "global") { + return resolveEffectivePolicy( + toolId, + ruleSet.rows, + ownerRankForRow, + defaultRequiresApproval, + ); + } + + const workspacePolicy = resolveEffectivePolicy( + toolId, + ruleSet.workspaceRows, + ownerRankForRow, + defaultRequiresApproval, + ); + const providerPolicy = + ruleSet.kind === "prepared" + ? ruleSet.resolve({ toolId, defaultRequiresApproval }) + : ruleSet.provider.resolve + ? yield* ruleSet.provider.resolve({ toolId, defaultRequiresApproval }) + : resolveProviderPolicyFromRules(toolId, ruleSet.rules ?? []); + return combineEffectivePolicies(providerPolicy, workspacePolicy); + }); // ------------------------------------------------------------------ // Tools (read surface) @@ -5949,7 +5962,6 @@ export const createExecutor = => Effect.gen(function* () { const parsed = parseToolAddress(String(address)); - const policyRows = yield* core.findMany("tool_policy", {}); const toolId = parsed ? `${parsed.integration}.${parsed.owner}.${parsed.connection}.${parsed.tool}` : String(address); @@ -5970,7 +5982,8 @@ export const createExecutor = { }); }); +describe("combineEffectivePolicies", () => { + const user = (action: "approve" | "require_approval" | "block", pattern: string) => ({ + action, + source: "user" as const, + pattern, + }); + const pluginDefault = (action: "approve" | "require_approval") => ({ + action, + source: "plugin-default" as const, + }); + + it("keeps a provider capability-boundary block", () => { + expect(combineEffectivePolicies(user("block", "*"), user("approve", "sample.*"))).toEqual( + user("block", "*"), + ); + }); + + it("keeps a workspace block", () => { + expect( + combineEffectivePolicies(user("approve", "sample.ctl.read"), user("block", "sample.*")), + ).toEqual(user("block", "sample.*")); + }); + + it("keeps workspace approval when the toolkit approves", () => { + expect( + combineEffectivePolicies( + user("approve", "sample.ctl.read"), + user("require_approval", "sample.*"), + ), + ).toEqual(user("require_approval", "sample.*")); + }); + + it("uses an explicit rule over a plugin default", () => { + expect( + combineEffectivePolicies( + user("approve", "sample.ctl.read"), + pluginDefault("require_approval"), + ), + ).toEqual(user("approve", "sample.ctl.read")); + }); + + it("uses the more restrictive result when both are plugin defaults", () => { + expect( + combineEffectivePolicies(pluginDefault("approve"), pluginDefault("require_approval")), + ).toEqual(pluginDefault("require_approval")); + }); +}); + // --------------------------------------------------------------------------- // Executor integration — v2 surface. A test plugin produces per-connection // tools via `resolveTools`; policies are owner-scoped; tools are addressed by @@ -695,6 +744,100 @@ describe("active tool-policy provider", () => { expect(Predicate.isTagged("ToolBlockedError")(blocked.failure)).toBe(true); }), ); + + it.effect("enforces workspace require_approval over provider approve", () => + Effect.gen(function* () { + const executor = yield* makeTestExecutor({ + plugins: [staticPlugin, policyProviderPlugin] as const, + }); + yield* executor.policies.create({ + owner: "org", + pattern: "toolkit-fixture.ctl.allowed", + action: "require_approval", + }); + + const calls = { count: 0 }; + const result = yield* executor.execute( + ToolAddress.make("toolkit-fixture.ctl.allowed"), + {}, + { onElicitation: recordingHandler(calls) }, + ); + expect(result).toBe("allowed"); + expect(calls.count).toBe(1); + }), + ); + + it.effect("enforces workspace block over provider approve", () => + Effect.gen(function* () { + const executor = yield* makeTestExecutor({ + plugins: [staticPlugin, policyProviderPlugin] as const, + }); + yield* executor.policies.create({ + owner: "org", + pattern: "toolkit-fixture.ctl.allowed", + action: "block", + }); + + expect(yield* executor.tools.list()).toHaveLength(0); + const policy = yield* executor.policies.resolve( + ToolAddress.make("toolkit-fixture.ctl.allowed"), + ); + expect(policy.action).toBe("block"); + + const result = yield* Effect.result( + executor.execute(ToolAddress.make("toolkit-fixture.ctl.allowed"), {}), + ); + expect(Result.isFailure(result)).toBe(true); + if (!Result.isFailure(result)) return; + expect(Predicate.isTagged("ToolBlockedError")(result.failure)).toBe(true); + }), + ); + + it.effect("combines a prepared provider with workspace policies", () => + Effect.gen(function* () { + const preparedProviderPlugin = definePlugin(() => ({ + id: "prepared-policy-provider" as const, + storage: () => ({}), + toolPolicyProvider: () => ({ + list: () => Effect.succeed([]), + prepare: () => + Effect.succeed((input: { readonly toolId: string }) => + input.toolId === "toolkit-fixture.ctl.allowed" + ? { + action: "approve" as const, + source: "user" as const, + pattern: "toolkit-fixture.ctl.allowed", + } + : { action: "block" as const, source: "user" as const, pattern: "*" }, + ), + }), + }))(); + const executor = yield* makeTestExecutor({ + plugins: [staticPlugin, preparedProviderPlugin] as const, + }); + yield* executor.policies.create({ + owner: "org", + pattern: "toolkit-fixture.ctl.allowed", + action: "require_approval", + }); + + const calls = { count: 0 }; + yield* executor.execute( + ToolAddress.make("toolkit-fixture.ctl.allowed"), + {}, + { onElicitation: recordingHandler(calls) }, + ); + expect(calls.count).toBe(1); + + const hidden = yield* Effect.result( + executor.execute(ToolAddress.make("toolkit-fixture.ctl.hidden"), {}), + ); + expect(Result.isFailure(hidden)).toBe(true); + expect( + (yield* executor.policies.resolve(ToolAddress.make("toolkit-fixture.ctl.hidden"))).action, + ).toBe("block"); + }), + ); }); describe("approve / require_approval interaction with annotations", () => { diff --git a/packages/core/sdk/src/policies.ts b/packages/core/sdk/src/policies.ts index 8620d9c6d3..b2bd5c0e0d 100644 --- a/packages/core/sdk/src/policies.ts +++ b/packages/core/sdk/src/policies.ts @@ -198,6 +198,22 @@ const moreRestrictive = ( return candidateRank > currentRank ? candidate : current; }; +export const combineEffectivePolicies = ( + providerPolicy: EffectivePolicy, + workspacePolicy: EffectivePolicy, +): EffectivePolicy => { + if (providerPolicy.action === "block") return providerPolicy; + if (workspacePolicy.action === "block") return workspacePolicy; + + if (providerPolicy.source === "user" && workspacePolicy.source === "user") { + return moreRestrictive(providerPolicy, workspacePolicy); + } + + if (workspacePolicy.source === "user") return workspacePolicy; + if (providerPolicy.source === "user") return providerPolicy; + return moreRestrictive(providerPolicy, workspacePolicy); +}; + export const resolveToolPolicy = ( toolId: string, policies: readonly ToolPolicyRow[], diff --git a/packages/plugins/toolkits/src/server.test.ts b/packages/plugins/toolkits/src/server.test.ts index bab67eb0e9..03a8e704c6 100644 --- a/packages/plugins/toolkits/src/server.test.ts +++ b/packages/plugins/toolkits/src/server.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from "@effect/vitest"; -import { Effect, Predicate, Result } from "effect"; -import { makeTestExecutor } from "@executor-js/sdk/testing"; +import { Effect, Predicate, Result, Schema } from "effect"; +import { createExecutor, definePlugin, tool, ToolAddress } from "@executor-js/sdk"; +import { makeTestExecutor, makeTestWorkspaceHarness } from "@executor-js/sdk/testing"; import { toolkitsPlugin } from "./server"; @@ -199,4 +200,112 @@ describe("toolkitsPlugin", () => { ).toContain("google_docs.org.* approve"); }), ); + + it.effect("enforces workspace policies in a toolkit-scoped executor", () => + Effect.gen(function* () { + const samplePlugin = definePlugin(() => ({ + id: "sample" as const, + storage: () => ({}), + staticIntegrations: () => [ + { + kind: "control" as const, + id: "sample.ctl", + name: "Sample Control", + tools: [ + tool({ + name: "readTool", + description: "read tool", + inputSchema: Schema.toStandardSchemaV1( + Schema.toStandardJSONSchemaV1(Schema.Struct({})), + ), + execute: () => Effect.succeed("read-data"), + }), + tool({ + name: "deleteTool", + description: "delete tool", + inputSchema: Schema.toStandardSchemaV1( + Schema.toStandardJSONSchemaV1(Schema.Struct({})), + ), + execute: () => Effect.succeed("deleted"), + }), + tool({ + name: "outsideTool", + description: "outside toolkit", + inputSchema: Schema.toStandardSchemaV1( + Schema.toStandardJSONSchemaV1(Schema.Struct({})), + ), + execute: () => Effect.succeed("outside"), + }), + ], + }, + ], + }))(); + const harness = yield* makeTestWorkspaceHarness({ + plugins: [toolkitsPlugin(), samplePlugin] as const, + }); + const setup = harness.executor; + const toolkit = yield* setup.toolkits.create({ owner: "org", name: "Test Kit" }); + yield* setup.toolkits.createConnection(toolkit.id, { + pattern: "sample.ctl.readTool", + }); + yield* setup.toolkits.createConnection(toolkit.id, { + pattern: "sample.ctl.deleteTool", + }); + yield* setup.toolkits.createPolicy(toolkit.id, { + pattern: "sample.ctl.readTool", + action: "approve", + }); + yield* setup.policies.create({ + owner: "org", + pattern: "sample.ctl.readTool", + action: "require_approval", + }); + yield* setup.policies.create({ + owner: "org", + pattern: "sample.ctl.deleteTool", + action: "block", + }); + + const scoped = yield* Effect.acquireRelease( + createExecutor({ + ...harness.config, + plugins: [toolkitsPlugin({ activeToolkitSlug: toolkit.slug }), samplePlugin] as const, + }), + (executor) => executor.close().pipe(Effect.ignore), + ); + const tools = yield* scoped.tools.list(); + expect(tools.map((entry) => String(entry.address))).toEqual(["sample.ctl.readTool"]); + expect((yield* scoped.policies.resolve(ToolAddress.make("sample.ctl.readTool"))).action).toBe( + "require_approval", + ); + expect( + (yield* scoped.policies.resolve(ToolAddress.make("sample.ctl.deleteTool"))).action, + ).toBe("block"); + expect( + (yield* scoped.policies.resolve(ToolAddress.make("sample.ctl.outsideTool"))).action, + ).toBe("block"); + + let elicited = false; + expect( + yield* scoped.execute( + ToolAddress.make("sample.ctl.readTool"), + {}, + { + onElicitation: () => { + elicited = true; + return Effect.succeed({ action: "accept" as const }); + }, + }, + ), + ).toBe("read-data"); + expect(elicited).toBe(true); + + const blocked = yield* Effect.result( + scoped.execute(ToolAddress.make("sample.ctl.deleteTool"), {}), + ); + expect(Result.isFailure(blocked)).toBe(true); + if (!Result.isFailure(blocked)) return; + expect(Predicate.isTagged("ToolBlockedError")(blocked.failure)).toBe(true); + }), + ); });