diff --git a/plugins/fusion-plugin-acp-runtime/src/__tests__/control-handler.test.ts b/plugins/fusion-plugin-acp-runtime/src/__tests__/control-handler.test.ts new file mode 100644 index 0000000000..22724e6590 --- /dev/null +++ b/plugins/fusion-plugin-acp-runtime/src/__tests__/control-handler.test.ts @@ -0,0 +1,285 @@ +// U5 security-floor tests for the PURE permission resolver. +// +// Each `it` is a security assertion. Do NOT weaken these to go green — if one +// fails, the implementation is wrong, not the test. + +import { describe, it, expect, vi } from "vitest"; +import type { + PermissionOption, + RequestPermissionResponse, + ToolCallUpdate, + ToolKind, +} from "@agentclientprotocol/sdk"; +import { + classifyToolKind, + selectOption, + resolvePermission, + DENY, +} from "../control-handler.js"; +import type { GateDisposition, PermissionGate } from "../types.js"; + +// A full option set the agent might offer (includes the dangerous *_always). +const ALL_OPTIONS: PermissionOption[] = [ + { optionId: "allow_once_id", name: "Allow once", kind: "allow_once" }, + { optionId: "allow_always_id", name: "Allow always", kind: "allow_always" }, + { optionId: "reject_once_id", name: "Reject once", kind: "reject_once" }, + { optionId: "reject_always_id", name: "Reject always", kind: "reject_always" }, +]; + +function toolCall(kind: ToolKind | null | undefined, extra: Partial = {}): ToolCallUpdate { + return { toolCallId: "tc-1", kind, ...extra } as ToolCallUpdate; +} + +function gateWithRules(rules: Record, extra: Partial = {}): PermissionGate { + return { permissionPolicy: { rules }, ...extra }; +} + +/** The shipped `unrestricted` default: every category → allow. */ +const UNRESTRICTED: Record = { + git_write: "allow", + file_write_delete: "allow", + command_execution: "allow", + network_api: "allow", + task_agent_mutation: "allow", +}; + +function selectedId(res: RequestPermissionResponse): string | undefined { + return res.outcome.outcome === "selected" ? res.outcome.optionId : undefined; +} + +describe("classifyToolKind", () => { + it("maps execute → command_execution", () => { + expect(classifyToolKind("execute")).toBe("command_execution"); + }); + it("maps edit/delete/move → file_write_delete", () => { + expect(classifyToolKind("edit")).toBe("file_write_delete"); + expect(classifyToolKind("delete")).toBe("file_write_delete"); + expect(classifyToolKind("move")).toBe("file_write_delete"); + }); + it("maps fetch → network_api", () => { + expect(classifyToolKind("fetch")).toBe("network_api"); + }); + it("maps read/search/think/switch_mode → exempt", () => { + expect(classifyToolKind("read")).toBe("exempt"); + expect(classifyToolKind("search")).toBe("exempt"); + expect(classifyToolKind("think")).toBe("exempt"); + expect(classifyToolKind("switch_mode")).toBe("exempt"); + }); + it("maps other/undefined/null/unknown → DENY sentinel", () => { + expect(classifyToolKind("other")).toBe(DENY); + expect(classifyToolKind(undefined)).toBe(DENY); + expect(classifyToolKind(null)).toBe(DENY); + expect(classifyToolKind("totally_made_up" as ToolKind)).toBe(DENY); + }); +}); + +describe("selectOption — allow_once ONLY (S2)", () => { + it("allow selects allow_once, never allow_always", () => { + const sel = selectOption("allow", ALL_OPTIONS); + expect(sel).toEqual({ decision: "allow", optionId: "allow_once_id" }); + }); + it("allow with NO allow_once falls back to reject (never allow_always)", () => { + const noAllowOnce = ALL_OPTIONS.filter((o) => o.kind !== "allow_once"); + const sel = selectOption("allow", noAllowOnce); + expect(sel.decision).toBe("deny"); + expect(sel.optionId).not.toBe("allow_always_id"); + expect(sel.optionId).toBe("reject_once_id"); + }); + it("deny selects reject_once, never reject_always", () => { + const sel = selectOption("deny", ALL_OPTIONS); + expect(sel).toEqual({ decision: "deny", optionId: "reject_once_id" }); + }); + it("deny with no reject_once leaves optionId undefined (→ cancelled)", () => { + const onlyAllow: PermissionOption[] = [ + { optionId: "allow_once_id", name: "Allow once", kind: "allow_once" }, + { optionId: "allow_always_id", name: "Allow always", kind: "allow_always" }, + ]; + const sel = selectOption("deny", onlyAllow); + expect(sel.decision).toBe("deny"); + expect(sel.optionId).toBeUndefined(); + }); +}); + +describe("resolvePermission — the security floor", () => { + // [Risk S1] per-category honored, NOT preset-allowed. + it("blocks an execute call when command_execution is custom-blocked even under an otherwise-unrestricted policy", async () => { + const gate = gateWithRules({ ...UNRESTRICTED, command_execution: "block" }); + const res = await resolvePermission(toolCall("execute"), ALL_OPTIONS, gate); + expect(res.outcome.outcome).toBe("selected"); + expect(selectedId(res)).toBe("reject_once_id"); + }); + + // [Risk S2] allow → allow_once, allow_always NEVER selected. + it("selects allow_once for an allow category and never allow_always even when offered", async () => { + const gate = gateWithRules({ ...UNRESTRICTED, command_execution: "allow" }); + const res = await resolvePermission(toolCall("execute"), ALL_OPTIONS, gate); + expect(selectedId(res)).toBe("allow_once_id"); + expect(selectedId(res)).not.toBe("allow_always_id"); + }); + + it("exempt kinds (read) always allow via allow_once", async () => { + // Even with a block-everything policy, a read-only kind is exempt → allow. + const gate = gateWithRules({ + git_write: "block", + file_write_delete: "block", + command_execution: "block", + network_api: "block", + task_agent_mutation: "block", + }); + const res = await resolvePermission(toolCall("read"), ALL_OPTIONS, gate); + expect(selectedId(res)).toBe("allow_once_id"); + }); + + // [KTD3a] missing / other / unknown kind → denied even under unrestricted. + it("denies a missing kind even under the unrestricted default", async () => { + const gate = gateWithRules(UNRESTRICTED); + const res = await resolvePermission(toolCall(undefined), ALL_OPTIONS, gate); + expect(selectedId(res)).toBe("reject_once_id"); + }); + it("denies an `other` kind even under the unrestricted default", async () => { + const gate = gateWithRules(UNRESTRICTED); + const res = await resolvePermission(toolCall("other"), ALL_OPTIONS, gate); + expect(selectedId(res)).toBe("reject_once_id"); + }); + + // No gate / no policy → default-deny. + it("default-denies when no gate is supplied", async () => { + const res = await resolvePermission(toolCall("read"), ALL_OPTIONS, undefined); + expect(selectedId(res)).toBe("reject_once_id"); + }); + it("default-denies when permissionPolicy is absent", async () => { + const res = await resolvePermission(toolCall("read"), ALL_OPTIONS, {} as PermissionGate); + expect(selectedId(res)).toBe("reject_once_id"); + }); + + // Options missing the expected *_once kind → safe fallback, never *_always, no throw. + it("falls back to cancelled (never allow_always) when an allow category offers no allow_once", async () => { + const gate = gateWithRules({ ...UNRESTRICTED, command_execution: "allow" }); + const noAllowOnce: PermissionOption[] = [ + { optionId: "allow_always_id", name: "Allow always", kind: "allow_always" }, + { optionId: "reject_always_id", name: "Reject always", kind: "reject_always" }, + ]; + const res = await resolvePermission(toolCall("execute"), noAllowOnce, gate); + // No reject_once either → cancelled, and definitely not allow_always. + expect(res.outcome.outcome).toBe("cancelled"); + expect(selectedId(res)).toBeUndefined(); + }); + + describe("require-approval HITL", () => { + it("creates an approval request, blocks until decision, granted → allow_once", async () => { + let resolvePause: (() => void) | undefined; + const order: string[] = []; + const gate: PermissionGate = gateWithRules( + { ...UNRESTRICTED, command_execution: "require-approval" }, + { + createApprovalRequest: vi.fn(async () => { + order.push("create"); + return { id: "appr-1" }; + }), + findApprovalByDedupeKey: vi + .fn() + // first lookup (reuse check): nothing prior + .mockResolvedValueOnce(null) + // second lookup (after pause): approved + .mockResolvedValueOnce({ id: "appr-1", status: "approved" }), + pauseForApproval: vi.fn( + () => + new Promise((resolve) => { + order.push("pause"); + resolvePause = () => { + order.push("resume"); + resolve(); + }; + }), + ), + markApprovalCompleted: vi.fn(async () => { + order.push("complete"); + }), + }, + ); + + const promise = resolvePermission(toolCall("execute"), ALL_OPTIONS, gate); + + // It must be blocked on pauseForApproval — give the microtask queue a tick. + await Promise.resolve(); + await Promise.resolve(); + expect(order).toEqual(["create", "pause"]); + + resolvePause!(); + const res = await promise; + expect(selectedId(res)).toBe("allow_once_id"); + expect(gate.createApprovalRequest).toHaveBeenCalledTimes(1); + expect(gate.markApprovalCompleted).toHaveBeenCalledWith("appr-1"); + expect(order).toEqual(["create", "pause", "resume", "complete"]); + }); + + it("rejected decision → reject_once", async () => { + const gate: PermissionGate = gateWithRules( + { ...UNRESTRICTED, command_execution: "require-approval" }, + { + createApprovalRequest: vi.fn(async () => ({ id: "appr-2" })), + findApprovalByDedupeKey: vi + .fn() + .mockResolvedValueOnce(null) + .mockResolvedValueOnce({ id: "appr-2", status: "denied" }), + pauseForApproval: vi.fn(async () => undefined), + markApprovalCompleted: vi.fn(async () => undefined), + }, + ); + const res = await resolvePermission(toolCall("execute"), ALL_OPTIONS, gate); + expect(selectedId(res)).toBe("reject_once_id"); + }); + + it("timeout/error during pause → reject_once (no throw)", async () => { + const gate: PermissionGate = gateWithRules( + { ...UNRESTRICTED, command_execution: "require-approval" }, + { + createApprovalRequest: vi.fn(async () => ({ id: "appr-3" })), + findApprovalByDedupeKey: vi.fn().mockResolvedValueOnce(null), + pauseForApproval: vi.fn(async () => { + throw new Error("timed out"); + }), + }, + ); + const res = await resolvePermission(toolCall("execute"), ALL_OPTIONS, gate); + expect(selectedId(res)).toBe("reject_once_id"); + }); + + it("reuses a prior approved decision via the dedupe key (no new request)", async () => { + const createApprovalRequest = vi.fn(async () => ({ id: "appr-x" })); + const gate: PermissionGate = gateWithRules( + { ...UNRESTRICTED, command_execution: "require-approval" }, + { + createApprovalRequest, + findApprovalByDedupeKey: vi.fn(async () => ({ id: "prior", status: "approved" as const })), + }, + ); + const res = await resolvePermission(toolCall("execute"), ALL_OPTIONS, gate); + expect(selectedId(res)).toBe("allow_once_id"); + expect(createApprovalRequest).not.toHaveBeenCalled(); + }); + + it("require-approval with NO closures → default-deny, no throw", async () => { + const gate = gateWithRules({ ...UNRESTRICTED, command_execution: "require-approval" }); + const res = await resolvePermission(toolCall("execute"), ALL_OPTIONS, gate); + expect(selectedId(res)).toBe("reject_once_id"); + }); + + it("require-approval with createApprovalRequest but no pauseForApproval → default-deny", async () => { + const gate: PermissionGate = gateWithRules( + { ...UNRESTRICTED, command_execution: "require-approval" }, + { createApprovalRequest: vi.fn(async () => ({ id: "a" })) }, + ); + const res = await resolvePermission(toolCall("execute"), ALL_OPTIONS, gate); + expect(selectedId(res)).toBe("reject_once_id"); + }); + }); + + it("treats a category with no explicit rule as require-approval (not allow)", async () => { + // command_execution missing from rules entirely → require-approval → with no + // closures that default-denies (never silent allow). + const gate = gateWithRules({ git_write: "allow" }); + const res = await resolvePermission(toolCall("execute"), ALL_OPTIONS, gate); + expect(selectedId(res)).toBe("reject_once_id"); + }); +}); diff --git a/plugins/fusion-plugin-acp-runtime/src/__tests__/provider-permission.test.ts b/plugins/fusion-plugin-acp-runtime/src/__tests__/provider-permission.test.ts new file mode 100644 index 0000000000..7977d80763 --- /dev/null +++ b/plugins/fusion-plugin-acp-runtime/src/__tests__/provider-permission.test.ts @@ -0,0 +1,115 @@ +// U5 — provider integration of the permission floor + cancel-drain. +// +// Exercises `createBridgingClientHandler(callbacks, gate)`: its +// `requestPermission` delegates to the per-category resolver, and `cancelPending` +// drains in-flight requests so the agent never deadlocks on teardown (KTD4a). + +import { describe, it, expect } from "vitest"; +import type { + PermissionOption, + RequestPermissionRequest, + RequestPermissionResponse, + ToolKind, +} from "@agentclientprotocol/sdk"; +import { createBridgingClientHandler } from "../provider.js"; +import type { GateDisposition, PermissionGate } from "../types.js"; + +const ALL_OPTIONS: PermissionOption[] = [ + { optionId: "allow_once_id", name: "Allow once", kind: "allow_once" }, + { optionId: "allow_always_id", name: "Allow always", kind: "allow_always" }, + { optionId: "reject_once_id", name: "Reject once", kind: "reject_once" }, + { optionId: "reject_always_id", name: "Reject always", kind: "reject_always" }, +]; + +function req(kind: ToolKind | undefined, id = "tc-1"): RequestPermissionRequest { + return { + sessionId: "sess-1", + toolCall: { toolCallId: id, kind } as RequestPermissionRequest["toolCall"], + options: ALL_OPTIONS, + }; +} + +const UNRESTRICTED: Record = { + git_write: "allow", + file_write_delete: "allow", + command_execution: "allow", + network_api: "allow", + task_agent_mutation: "allow", +}; + +function gate(rules: Record): PermissionGate { + return { permissionPolicy: { rules } }; +} + +function selectedId(res: RequestPermissionResponse): string | undefined { + return res.outcome.outcome === "selected" ? res.outcome.optionId : undefined; +} + +describe("createBridgingClientHandler — requestPermission delegates to the gate", () => { + it("answers allow_once for an allow category", async () => { + const { handler } = createBridgingClientHandler({}, gate({ ...UNRESTRICTED, command_execution: "allow" })); + const res = await handler.requestPermission(req("execute")); + expect(selectedId(res)).toBe("allow_once_id"); + }); + + it("default-denies (reject_once) when no gate is supplied", async () => { + const { handler } = createBridgingClientHandler({}); + const res = await handler.requestPermission(req("read")); + expect(selectedId(res)).toBe("reject_once_id"); + }); + + it("honors a per-category block under an otherwise-unrestricted policy", async () => { + const { handler } = createBridgingClientHandler({}, gate({ ...UNRESTRICTED, command_execution: "block" })); + const res = await handler.requestPermission(req("execute")); + expect(selectedId(res)).toBe("reject_once_id"); + }); +}); + +describe("cancel drain (KTD4a — no permission deadlock)", () => { + it("resolves two in-flight permission requests as cancelled and answers later requests cancelled immediately", async () => { + // A require-approval category with a pause that NEVER resolves on its own — + // the only way these complete is the cancel drain. + let pauseCount = 0; + const blockingGate: PermissionGate = { + permissionPolicy: { rules: { ...UNRESTRICTED, command_execution: "require-approval" } }, + createApprovalRequest: async () => ({ id: "appr" }), + findApprovalByDedupeKey: async () => null, + pauseForApproval: () => + new Promise(() => { + pauseCount += 1; + /* never resolves */ + }), + }; + + const { handler, cancelPending } = createBridgingClientHandler({}, blockingGate); + + const p1 = handler.requestPermission(req("execute", "tc-1")); + const p2 = handler.requestPermission(req("execute", "tc-2")); + + // Let both reach the blocking pause. + await Promise.resolve(); + await Promise.resolve(); + expect(pauseCount).toBe(2); + + cancelPending(); + + const [r1, r2] = await Promise.all([p1, p2]); + expect(r1.outcome.outcome).toBe("cancelled"); + expect(r2.outcome.outcome).toBe("cancelled"); + + // A request arriving AFTER cancel is answered cancelled immediately. + const r3 = await handler.requestPermission(req("execute", "tc-3")); + expect(r3.outcome.outcome).toBe("cancelled"); + }); + + it("cancelPending is idempotent", async () => { + const { handler, cancelPending } = createBridgingClientHandler( + {}, + gate({ ...UNRESTRICTED, command_execution: "allow" }), + ); + cancelPending(); + cancelPending(); + const res = await handler.requestPermission(req("execute")); + expect(res.outcome.outcome).toBe("cancelled"); + }); +}); diff --git a/plugins/fusion-plugin-acp-runtime/src/__tests__/provider-session.test.ts b/plugins/fusion-plugin-acp-runtime/src/__tests__/provider-session.test.ts index a6d75d656b..b16498a9d2 100644 --- a/plugins/fusion-plugin-acp-runtime/src/__tests__/provider-session.test.ts +++ b/plugins/fusion-plugin-acp-runtime/src/__tests__/provider-session.test.ts @@ -105,7 +105,7 @@ describe("session driving helpers", () => { const conn = await connect({ ...baseOpts({ ACP_FIXTURE_RICH_PROMPT: "1" }), - clientHandler: createBridgingClientHandler({ onText, onThinking, onToolStart, onToolEnd }), + clientHandler: createBridgingClientHandler({ onText, onThinking, onToolStart, onToolEnd }).handler, }); try { const { sessionId } = await newAcpSession(conn, { cwd: process.cwd() }); diff --git a/plugins/fusion-plugin-acp-runtime/src/__tests__/runtime-adapter.test.ts b/plugins/fusion-plugin-acp-runtime/src/__tests__/runtime-adapter.test.ts index a7e3f99ed5..b62f8e534f 100644 --- a/plugins/fusion-plugin-acp-runtime/src/__tests__/runtime-adapter.test.ts +++ b/plugins/fusion-plugin-acp-runtime/src/__tests__/runtime-adapter.test.ts @@ -43,7 +43,7 @@ describe("AcpRuntimeAdapter (U3)", () => { it("createSession persists actionGateContext and cwd on the session", async () => { const adapter = makeAdapter(); - const gate = { permissionPolicy: { preset: "unrestricted" } }; + const gate = { permissionPolicy: { rules: { command_execution: "allow" as const } } }; // cwd must be a real, spawnable directory (it is the subprocess cwd too). const cwd = os.tmpdir(); const { session } = await adapter.createSession( diff --git a/plugins/fusion-plugin-acp-runtime/src/control-handler.ts b/plugins/fusion-plugin-acp-runtime/src/control-handler.ts new file mode 100644 index 0000000000..9715a57478 --- /dev/null +++ b/plugins/fusion-plugin-acp-runtime/src/control-handler.ts @@ -0,0 +1,229 @@ +// U5 — the SECURITY FLOOR for `session/request_permission`. +// +// The ACP agent is an UNTRUSTED subprocess. When it asks permission to run a +// tool call, this resolver classifies the call PER-CATEGORY against Fusion's +// live action gate and answers `allow_once` / `reject_once` / `cancelled`. +// +// Why per-category and not per-preset (S1 / KTD3a): Fusion's shipped default +// policy preset is `unrestricted` (every category → allow). Mapping a preset id +// straight to an outcome would auto-approve EVERY tool call of an untrusted +// agent the instant a user selects the ACP runtime. So we classify the call's +// `kind` into a Fusion category and read `gate.permissionPolicy.rules[category]`. +// +// Default-deny is the floor everywhere a decision can't be made safely: +// - no gate / no permissionPolicy → deny +// - an unmappable / missing / `other` kind → deny (most-restrictive) +// - `require-approval` with no HITL machinery → deny +// - the `allow_once` option isn't offered → reject (never `*_always`, S2) + +import type { + PermissionOption, + RequestPermissionResponse, + ToolCallUpdate, + ToolKind, +} from "@agentclientprotocol/sdk"; +import type { + ApprovalStatus, + FusionCategory, + GateDisposition, + PermissionGate, +} from "./types.js"; + +/** Sentinel returned by `classifyToolKind` for an unmappable kind → force deny. */ +export const DENY = "deny" as const; + +/** + * Map an ACP `toolCall.kind` to a Fusion action-gate category (KTD3a). + * + * Read-only / benign kinds map to the implicit `exempt` category (always allow). + * `other`, `undefined`, and any unknown kind map to the `DENY` sentinel — the + * most-restrictive outcome — and MUST NOT fall through to allow. + */ +export function classifyToolKind(kind: ToolKind | null | undefined): FusionCategory | "exempt" | typeof DENY { + switch (kind) { + case "execute": + return "command_execution"; + case "edit": + case "delete": + case "move": + return "file_write_delete"; + case "fetch": + return "network_api"; + case "read": + case "search": + case "think": + case "switch_mode": + return "exempt"; + // "other", undefined, null, or anything unknown → most-restrictive deny. + default: + return DENY; + } +} + +/** + * Select the ACP option to answer with, honoring the allow_once-ONLY rule (S2). + * + * - `allow` → an option whose `kind === "allow_once"`. Never `allow_always` + * (delegating a blanket grant to untrusted code loses Fusion's per-call + * interception). If no `allow_once` option is offered → fall back to deny. + * - `deny` → an option whose `kind === "reject_once"`. If none is offered the + * caller answers `{ outcome: "cancelled" }`. Never `reject_always`. + */ +export function selectOption( + decision: "allow" | "deny", + options: PermissionOption[], +): { decision: "allow" | "deny"; optionId?: string } { + const list = Array.isArray(options) ? options : []; + if (decision === "allow") { + const allowOnce = list.find((o) => o?.kind === "allow_once"); + if (allowOnce?.optionId) return { decision: "allow", optionId: allowOnce.optionId }; + // No allow_once offered: do NOT up-grade to allow_always. Fall back to deny. + const rejectOnce = list.find((o) => o?.kind === "reject_once"); + return { decision: "deny", optionId: rejectOnce?.optionId }; + } + const rejectOnce = list.find((o) => o?.kind === "reject_once"); + return { decision: "deny", optionId: rejectOnce?.optionId }; +} + +/** Build the ACP response for a resolved {decision, optionId}. */ +function buildResponse(sel: { + decision: "allow" | "deny"; + optionId?: string; +}): RequestPermissionResponse { + if (sel.optionId) { + return { outcome: { outcome: "selected", optionId: sel.optionId } }; + } + // No usable option (e.g. deny with no reject_once offered) → cancelled. + return { outcome: { outcome: "cancelled" } }; +} + +/** Read the per-category disposition from the live policy (exempt → allow). */ +function dispositionFor( + category: FusionCategory | "exempt", + gate: PermissionGate, +): GateDisposition { + if (category === "exempt") return "allow"; + const rules = gate.permissionPolicy?.rules; + const disposition = rules?.[category]; + // A category with no explicit rule is treated as require-approval (not allow): + // never silently allow an unmapped category for an untrusted agent. + return disposition ?? "require-approval"; +} + +/** A stable dedupe key for an identical tool call (decision reuse). */ +function dedupeKeyFor(toolCall: ToolCallUpdate, category: string): string { + return [toolCall.toolCallId ?? "", category, toolCall.title ?? ""].join("|"); +} + +/** + * Run the human-in-the-loop approval flow for a `require-approval` category. + * + * Requires `createApprovalRequest` (the one non-optional HITL closure). When it + * is absent there is no human channel → DEFAULT-DENY (never throw, never allow). + * + * Flow: reuse a prior decision via `findApprovalByDedupeKey` when present; + * otherwise register the request, block on `pauseForApproval`, re-read the final + * status, finalize via `markApprovalCompleted`. `approved` → allow; everything + * else (denied / pending / completed / lookup-failure) → deny. + */ +async function runApproval( + toolCall: ToolCallUpdate, + category: FusionCategory, + gate: PermissionGate, +): Promise<"allow" | "deny"> { + if (typeof gate.createApprovalRequest !== "function") { + // No human channel available → default-deny. + return "deny"; + } + + const dedupeKey = dedupeKeyFor(toolCall, category); + const decisionPayload = { + disposition: "require-approval" as const, + category, + toolName: toolCall.title ?? category, + approvalDedupeKey: dedupeKey, + }; + + const mapStatus = (status: ApprovalStatus | undefined): "allow" | "deny" => + status === "approved" ? "allow" : "deny"; + + try { + // Reuse a prior decision for an identical call when available. + if (typeof gate.findApprovalByDedupeKey === "function") { + const prior = await gate.findApprovalByDedupeKey(dedupeKey); + if (prior && (prior.status === "approved" || prior.status === "denied")) { + return mapStatus(prior.status); + } + } + + const created = (await gate.createApprovalRequest( + decisionPayload, + (toolCall.rawInput && typeof toolCall.rawInput === "object" + ? (toolCall.rawInput as Record) + : {}), + )) as { id?: string } | undefined; + const approvalRequestId = typeof created?.id === "string" ? created.id : dedupeKey; + + if (typeof gate.pauseForApproval === "function") { + await gate.pauseForApproval({ approvalRequestId, decision: decisionPayload }); + } else { + // No way to block for a human decision → default-deny. + return "deny"; + } + + // Re-read the final status after the pause resolves. + let finalStatus: ApprovalStatus | undefined; + if (typeof gate.findApprovalByDedupeKey === "function") { + const resolved = await gate.findApprovalByDedupeKey(dedupeKey); + finalStatus = resolved?.status; + } + + if (typeof gate.markApprovalCompleted === "function") { + await gate.markApprovalCompleted(approvalRequestId); + } + + return mapStatus(finalStatus); + } catch { + // Any HITL failure (timeout/dismiss/store error) → default-deny, no throw. + return "deny"; + } +} + +/** + * The full per-call security floor: classify → read the per-category + * disposition → run HITL for `require-approval` → select an `allow_once`-only + * option → build the ACP response. + * + * Default-deny on: missing gate, missing `permissionPolicy`, unmappable kind, + * `require-approval` without a resolvable approver, or a missing `allow_once` + * option. + */ +export async function resolvePermission( + toolCall: ToolCallUpdate, + options: PermissionOption[], + gate: PermissionGate | undefined, +): Promise { + // No gate / no policy → default-deny. + if (!gate || !gate.permissionPolicy) { + return buildResponse(selectOption("deny", options)); + } + + const category = classifyToolKind(toolCall?.kind); + // Unmappable / missing / `other` kind → most-restrictive deny. + if (category === DENY) { + return buildResponse(selectOption("deny", options)); + } + + const disposition = dispositionFor(category, gate); + + if (disposition === "allow") { + return buildResponse(selectOption("allow", options)); + } + if (disposition === "block") { + return buildResponse(selectOption("deny", options)); + } + + // require-approval → HITL (or default-deny when no human channel exists). + const decision = await runApproval(toolCall, category as FusionCategory, gate); + return buildResponse(selectOption(decision, options)); +} diff --git a/plugins/fusion-plugin-acp-runtime/src/provider.ts b/plugins/fusion-plugin-acp-runtime/src/provider.ts index ade90f09aa..7055d96454 100644 --- a/plugins/fusion-plugin-acp-runtime/src/provider.ts +++ b/plugins/fusion-plugin-acp-runtime/src/provider.ts @@ -20,11 +20,13 @@ import { type Agent, type Client, type ContentBlock, + type RequestPermissionResponse, type StopReason, } from "@agentclientprotocol/sdk"; import { spawnAgent, captureStderr, forceKill, unregisterProcess } from "./process-manager.js"; import { createEventBridge } from "./event-bridge.js"; -import type { AcpCallbacks } from "./types.js"; +import { resolvePermission } from "./control-handler.js"; +import type { AcpCallbacks, PermissionGate } from "./types.js"; /** Default bound for the `initialize` handshake. */ export const DEFAULT_INITIALIZE_TIMEOUT_MS = 30_000; @@ -69,23 +71,85 @@ export function createDefaultClientHandler(): Client { }; } +/** A bridging client handler plus a drain control for its in-flight permissions. */ +export interface BridgingClientHandler { + /** The ACP `Client` impl handed to `ClientSideConnection`. */ + handler: Client; + /** + * Resolve every in-flight `requestPermission` with `{ cancelled }` and mark the + * handler cancelled so any request arriving afterward is answered cancelled + * immediately (U5 cancel-drain — KTD4a). Idempotent. + */ + cancelPending(): void; +} + /** - * The real client handler (U4): bridges every `session/update` notification into - * the engine callbacks via an event bridge so streamed agent text/thinking/tool - * activity surfaces in Fusion. The permission floor is still the safe default — - * U5 replaces `requestPermission` with the per-category action gate. + * The real client handler (U4 + U5): bridges every `session/update` notification + * into the engine callbacks, AND answers `session/request_permission` through the + * per-category action gate (U5 — the SECURITY FLOOR). + * + * Permission requests are routed to `resolvePermission`, which classifies each + * call per-category against the live `gate` and selects `allow_once` only (never + * `*_always`). When no `gate` is supplied the resolver default-denies. + * + * Cancel-drain (KTD4a / Risk: in-flight permission deadlock): every pending + * `requestPermission` promise is tracked; `cancelPending()` resolves them all + * with `{ cancelled }`. A request that arrives AFTER cancel is answered + * `{ cancelled }` immediately so the agent never blocks on teardown. */ -export function createBridgingClientHandler(callbacks: AcpCallbacks): Client { +export function createBridgingClientHandler( + callbacks: AcpCallbacks, + gate?: PermissionGate, +): BridgingClientHandler { const bridge = createEventBridge(callbacks); - return { + + const cancelledResponse: RequestPermissionResponse = { + outcome: { outcome: "cancelled" }, + }; + + let cancelled = false; + // Each entry resolves its pending requestPermission with a cancelled outcome. + const pending = new Set<(response: RequestPermissionResponse) => void>(); + + function cancelPending(): void { + cancelled = true; + for (const resolveCancelled of [...pending]) { + resolveCancelled(cancelledResponse); + } + pending.clear(); + } + + const handler: Client = { async sessionUpdate(params) { bridge.handleSessionUpdate(params.update); }, - async requestPermission() { - // U5 replaces this with the per-category gate; default-cancel for now. - return { outcome: { outcome: "cancelled" } }; + async requestPermission(params): Promise { + // A request arriving after cancel is answered cancelled immediately. + if (cancelled) return cancelledResponse; + + // Race the real gate resolution against a cancel-drain so an in-flight + // request is answered the moment teardown drains it (never deadlocks). + return await new Promise((resolve) => { + let settled = false; + const finish = (response: RequestPermissionResponse) => { + if (settled) return; + settled = true; + pending.delete(drain); + resolve(response); + }; + const drain = (response: RequestPermissionResponse) => finish(response); + pending.add(drain); + + resolvePermission(params.toolCall, params.options, gate).then( + (response) => finish(response), + // resolvePermission never rejects, but stay safe: deny-by-cancel. + () => finish(cancelledResponse), + ); + }); }, }; + + return { handler, cancelPending }; } export interface AcpConnection { diff --git a/plugins/fusion-plugin-acp-runtime/src/runtime-adapter.ts b/plugins/fusion-plugin-acp-runtime/src/runtime-adapter.ts index 1348472463..401fdb98bd 100644 --- a/plugins/fusion-plugin-acp-runtime/src/runtime-adapter.ts +++ b/plugins/fusion-plugin-acp-runtime/src/runtime-adapter.ts @@ -53,6 +53,15 @@ export class AcpRuntimeAdapter implements AgentRuntime { onToolEnd: options.onToolEnd, }; + // Build the bridging client handler with the per-run permission gate (U5): + // its `requestPermission` classifies each call per-category against the live + // gate (KTD3a) and selects `allow_once` only (S2). `cancelPending` drains + // in-flight permission requests on teardown so the agent never deadlocks. + const { handler: clientHandler, cancelPending } = createBridgingClientHandler( + callbacks, + options.actionGateContext, + ); + // Spawn + initialize (U2). fs capabilities are advertised only where the // resolved settings enable them (KTD6); the subprocess env is built from the // allow-list, never inherited process.env (KTD6b). @@ -62,7 +71,7 @@ export class AcpRuntimeAdapter implements AgentRuntime { cwd: options.cwd, env: buildSpawnEnv(this.settings.envAllowList), advertiseFs: { read: this.settings.fsRead, write: this.settings.fsWrite }, - clientHandler: createBridgingClientHandler(callbacks), + clientHandler, }); // Open the ACP session over the task worktree (empty mcpServers — KTD5). @@ -90,6 +99,9 @@ export class AcpRuntimeAdapter implements AgentRuntime { dispose: () => { if (disposed) return; disposed = true; + // Drain in-flight permission requests BEFORE the registry kill so a + // blocked agent is released (KTD4a — the SIGKILL is still authoritative). + cancelPending(); connection.dispose(); }, }; diff --git a/plugins/fusion-plugin-acp-runtime/src/types.ts b/plugins/fusion-plugin-acp-runtime/src/types.ts index b84db53be6..19c1a92608 100644 --- a/plugins/fusion-plugin-acp-runtime/src/types.ts +++ b/plugins/fusion-plugin-acp-runtime/src/types.ts @@ -20,22 +20,54 @@ export interface AcpCallbacks { onToolEnd?: (toolName: string, isError: boolean, result?: unknown) => void; } +/** Per-category permission disposition (mirrors the engine policy shape). */ +export type GateDisposition = "allow" | "block" | "require-approval"; + +/** + * Fusion action-gate categories the ACP `toolCall.kind` is classified into + * (KTD3a). `"exempt"` is implicit (read-only / benign) and always allows. + */ +export type FusionCategory = + | "git_write" + | "file_write_delete" + | "command_execution" + | "network_api" + | "task_agent_mutation"; + +/** Approval lifecycle status as returned by the gate's lookup closure. */ +export type ApprovalStatus = "pending" | "approved" | "denied" | "completed"; + /** * Narrow structural view of the engine's `AgentActionGateContext` * (`packages/engine/src/agent-action-gate.ts`). The plugin reads only these * members; typing them locally avoids a hard dependency on `@fusion/engine`. * - * All HITL closures are optional: when absent, the permission floor (U5) - * default-denies `require-approval` categories rather than throwing. + * `permissionPolicy.rules` is the per-category disposition map the U5 floor + * consults — NEVER a preset id (S1/KTD3a). All HITL closures except + * `createApprovalRequest` are optional: when the HITL machinery is absent, the + * permission floor (U5) default-denies `require-approval` categories rather than + * throwing (Risk S1). */ export interface PermissionGate { - permissionPolicy?: unknown; - evaluate?: (toolName: string, args: unknown) => unknown; - resolveGateOutcome?: (evaluation: unknown) => unknown; - createApprovalRequest?: (...args: unknown[]) => Promise | unknown; - findApprovalByDedupeKey?: (...args: unknown[]) => Promise | unknown; - pauseForApproval?: (...args: unknown[]) => Promise | unknown; - markApprovalCompleted?: (...args: unknown[]) => Promise | unknown; + permissionPolicy?: { + rules?: Record; + }; + /** Register an approval request; returns the created record (with an `id`). */ + createApprovalRequest?: ( + decision: unknown, + args: Record, + ) => Promise | unknown; + /** Look up a prior decision by dedupe key (decision reuse). */ + findApprovalByDedupeKey?: ( + dedupeKey: string, + ) => Promise<{ id: string; status: ApprovalStatus } | null> | { id: string; status: ApprovalStatus } | null; + /** Block until the human resolves the referenced approval request. */ + pauseForApproval?: (info: { + approvalRequestId: string; + decision: unknown; + }) => Promise | void; + /** Mark an approval request finalized after the decision is consumed. */ + markApprovalCompleted?: (approvalRequestId: string) => Promise | void; } /** Plugin-local copy of the engine's AgentRuntimeOptions (subset this runtime reads). */