mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-03 18:50:21 +00:00
feat(hooks): add async requireApproval to before_tool_call (#55339)
* Plugins: add native ask dialog for before_tool_call hooks Extend the before_tool_call plugin hook with a requireApproval return field that pauses agent execution and waits for real user approval via channels (Telegram, Discord, /approve command) instead of relying on the agent to cooperate with a soft block. - Add requireApproval field to PluginHookBeforeToolCallResult with id, title, description, severity, timeout, and timeoutBehavior options - Extend runModifyingHook merge callback to receive hook registration so mergers can stamp pluginId; always invoke merger even for the first result - Make ExecApprovalManager generic so it can be reused for plugin approvals - Add plugin.approval.request/waitDecision/resolve gateway methods with schemas, scope guards, and broadcast events - Handle requireApproval in pi-tools via two-phase gateway RPC with fallback to soft block when the gateway is unavailable - Extend the exec approval forwarder with plugin approval message builders and forwarding methods - Update /approve command to fall back to plugin.approval.resolve when exec approval lookup fails - Document before_tool_call requireApproval in hooks docs and unified /approve behavior in exec-approvals docs * Plugins: simplify plugin approval code - Extract mergeParamsWithApprovalOverrides helper to deduplicate param merge logic in before_tool_call hook handling - Use idiomatic conditional spread syntax in toolContext construction - Extract callApprovalMethod helper in /approve command to eliminate duplicated callGateway calls - Simplify plugin approval schema by removing unnecessary Type.Union with Type.Null on optional fields - Extract normalizeTrimmedString helper for turn source field trimming * Tests: add plugin approval wiring and /approve fallback coverage Fix 3 broken assertions expecting old "Exec approval" message text. Add tests for the /approve command's exec→plugin fallback path, plugin approval method registration and scope authorization, and handler factory key verification. * UI: wire plugin approval events into the exec approval overlay Handle plugin.approval.requested and plugin.approval.resolved gateway events by extending the existing exec approval queue with a kind discriminator. Plugin approvals reuse the same overlay, queue management, and expiry timer, with branched rendering for plugin-specific content (title, description, severity). The decision handler routes resolve calls to the correct gateway method based on kind. * fix: read plugin approval fields from nested request payload The gateway broadcasts plugin approval payloads with title, description, severity, pluginId, agentId, and sessionKey nested inside the request object (PluginApprovalRequestPayload), not at the top level. Fix the parser to read from the correct location so the overlay actually appears. * feat: invoke plugin onResolution callback after approval decision Adds onResolution to the requireApproval type and invokes it after the user resolves the approval dialog, enabling plugins to react to allow-always vs allow-once decisions. * docs: add onResolution callback to requireApproval hook documentation * test: fix /approve assertion for unified approval response text * docs: regenerate plugin SDK API baseline * docs: add changelog entry for plugin approval hooks * fix: harden plugin approval hook reliability - Add APPROVAL_NOT_FOUND error code so /approve fallback uses structured matching instead of fragile string comparison - Check block before requireApproval so higher-priority plugin blocks cannot be overridden by a lower-priority approval - Race waitDecision against abort signal so users are not stuck waiting for the full approval timeout after cancelling a run - Use null consistently for missing pluginDescription instead of converting to undefined - Add comments explaining the +10s timeout buffer on gateway RPCs * docs: document block > requireApproval precedence in hooks * fix: address Phase 1 critical correctness issues for plugin approval hooks - Fix timeout-allow param bug: return merged hook params instead of original params when timeoutBehavior is "allow", preventing security plugins from having their parameter rewrites silently discarded. - Host-generate approval IDs: remove plugin-provided id field from the requireApproval type, gateway request, and protocol schema. Server always generates IDs via randomUUID() to prevent forged/predictable ID attacks. - Define onResolution semantics: add PluginApprovalResolutions constants and PluginApprovalResolution type. onResolution callback now fires on every exit path (allow, deny, timeout, abort, gateway error, no-ID). Decision branching uses constants instead of hard-coded strings. - Fix pre-existing test infrastructure issues: bypass CJS mock cache for getGlobalHookRunner global singleton, reset gateway mock between tests, fix hook merger priority ordering in block+requireApproval test. * fix: tighten plugin approval schema and add kind-prefixed IDs Harden the plugin approval request schema: restrict severity to enum (info|warning|critical), cap timeoutMs at 600s, limit title to 80 chars and description to 256 chars. Prefix plugin approval IDs with `plugin:` so /approve routing can distinguish them from exec approvals deterministically instead of relying on fallback. * fix: address remaining PR feedback (Phases 1-3 source changes) * chore: regenerate baselines and protocol artifacts * fix: exclude requesting connection from approval-client availability check hasExecApprovalClients() counted the backend connection that issued the plugin.approval.request RPC as an approval client, preventing the no-approval-route fast path from firing in headless setups and causing 120s stalls. Pass the caller's connId so it is skipped. Applied to both plugin and exec approval handlers. * Approvals: complete Discord parity and compatibility fallback * Hooks: make plugin approval onResolution non-blocking * Hooks: freeze params after approval owner is selected * Gateway: harden plugin approval request/decision flow * Discord/Telegram: fix plugin approval delivery parity * Approvals: fix Telegram plugin approval edge cases * Auto-reply: enforce Telegram plugin approval approvers * Approvals: harden Telegram and plugin resolve policies * Agents: static-import gateway approval call and fix e2e mock loading * Auto-reply: restore /approve Telegram import boundary * Approvals: fail closed on no-route and neutralize Discord mentions * docs: refresh generated config and plugin API baselines --------- Co-authored-by: Václav Belák <vaclav.belak@gendigital.com>
This commit is contained in:
Josh Avant
@@ -17,6 +17,9 @@ vi.mock("../plugins/hook-runner-global.js", async (importOriginal) => {
|
||||
getGlobalHookRunner: vi.fn(),
|
||||
};
|
||||
});
|
||||
vi.mock("./tools/gateway.js", () => ({
|
||||
callGatewayTool: vi.fn(),
|
||||
}));
|
||||
|
||||
const mockGetGlobalHookRunner = vi.mocked(getGlobalHookRunner);
|
||||
|
||||
@@ -325,3 +328,525 @@ describe("before_tool_call loop detection behavior", () => {
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("before_tool_call requireApproval handling", () => {
|
||||
let runBeforeToolCallHook: (typeof import("./pi-tools.before-tool-call.js"))["runBeforeToolCallHook"];
|
||||
let hookRunner: {
|
||||
hasHooks: ReturnType<typeof vi.fn>;
|
||||
runBeforeToolCall: ReturnType<typeof vi.fn>;
|
||||
};
|
||||
|
||||
beforeEach(async () => {
|
||||
vi.resetModules();
|
||||
({ runBeforeToolCallHook } = await import("./pi-tools.before-tool-call.js"));
|
||||
|
||||
resetDiagnosticSessionStateForTest();
|
||||
resetDiagnosticEventsForTest();
|
||||
hookRunner = {
|
||||
hasHooks: vi.fn().mockReturnValue(true),
|
||||
runBeforeToolCall: vi.fn(),
|
||||
};
|
||||
const { getGlobalHookRunner: currentGetGlobalHookRunner } =
|
||||
await import("../plugins/hook-runner-global.js");
|
||||
// oxlint-disable-next-line typescript/no-explicit-any
|
||||
vi.mocked(currentGetGlobalHookRunner).mockReturnValue(hookRunner as any);
|
||||
// Keep the global singleton aligned as a fallback in case another setup path
|
||||
// preloads hook-runner-global before this test's module reset/mocks take effect.
|
||||
const hookRunnerGlobalStateKey = Symbol.for("openclaw.plugins.hook-runner-global-state");
|
||||
const hookRunnerGlobalState = globalThis as Record<
|
||||
symbol,
|
||||
{ hookRunner: unknown; registry?: unknown } | undefined
|
||||
>;
|
||||
if (!hookRunnerGlobalState[hookRunnerGlobalStateKey]) {
|
||||
hookRunnerGlobalState[hookRunnerGlobalStateKey] = {
|
||||
hookRunner: null,
|
||||
registry: null,
|
||||
};
|
||||
}
|
||||
hookRunnerGlobalState[hookRunnerGlobalStateKey].hookRunner = hookRunner;
|
||||
// Clear gateway mock state between tests to prevent call-count leaks.
|
||||
const { callGatewayTool } = await import("./tools/gateway.js");
|
||||
vi.mocked(callGatewayTool).mockReset();
|
||||
});
|
||||
|
||||
it("blocks without triggering approval when both block and requireApproval are set", async () => {
|
||||
const { callGatewayTool } = await import("./tools/gateway.js");
|
||||
const mockCallGateway = vi.mocked(callGatewayTool);
|
||||
|
||||
hookRunner.runBeforeToolCall.mockResolvedValue({
|
||||
block: true,
|
||||
blockReason: "Blocked by security plugin",
|
||||
requireApproval: {
|
||||
title: "Should not reach gateway",
|
||||
description: "This approval should be skipped",
|
||||
pluginId: "lower-priority-plugin",
|
||||
},
|
||||
});
|
||||
|
||||
const result = await runBeforeToolCallHook({
|
||||
toolName: "bash",
|
||||
params: { command: "rm -rf" },
|
||||
ctx: { agentId: "main", sessionKey: "main" },
|
||||
});
|
||||
|
||||
expect(result.blocked).toBe(true);
|
||||
expect(result).toHaveProperty("reason", "Blocked by security plugin");
|
||||
expect(mockCallGateway).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("calls gateway RPC and unblocks on allow-once", async () => {
|
||||
const { callGatewayTool } = await import("./tools/gateway.js");
|
||||
const mockCallGateway = vi.mocked(callGatewayTool);
|
||||
|
||||
hookRunner.runBeforeToolCall.mockResolvedValue({
|
||||
requireApproval: {
|
||||
title: "Sensitive",
|
||||
description: "Sensitive op",
|
||||
pluginId: "sage",
|
||||
},
|
||||
});
|
||||
|
||||
// First call: plugin.approval.request → returns server-generated id
|
||||
mockCallGateway.mockResolvedValueOnce({ id: "server-id-1", status: "accepted" });
|
||||
// Second call: plugin.approval.waitDecision → returns allow-once
|
||||
mockCallGateway.mockResolvedValueOnce({ id: "server-id-1", decision: "allow-once" });
|
||||
|
||||
const result = await runBeforeToolCallHook({
|
||||
toolName: "bash",
|
||||
params: { command: "rm -rf" },
|
||||
ctx: { agentId: "main", sessionKey: "main" },
|
||||
});
|
||||
|
||||
expect(result.blocked).toBe(false);
|
||||
expect(mockCallGateway).toHaveBeenCalledTimes(2);
|
||||
expect(mockCallGateway).toHaveBeenCalledWith(
|
||||
"plugin.approval.request",
|
||||
expect.any(Object),
|
||||
expect.objectContaining({ twoPhase: true }),
|
||||
{ expectFinal: false },
|
||||
);
|
||||
expect(mockCallGateway).toHaveBeenCalledWith(
|
||||
"plugin.approval.waitDecision",
|
||||
expect.any(Object),
|
||||
{ id: "server-id-1" },
|
||||
);
|
||||
});
|
||||
|
||||
it("blocks on deny decision", async () => {
|
||||
const { callGatewayTool } = await import("./tools/gateway.js");
|
||||
const mockCallGateway = vi.mocked(callGatewayTool);
|
||||
|
||||
hookRunner.runBeforeToolCall.mockResolvedValue({
|
||||
requireApproval: {
|
||||
title: "Dangerous",
|
||||
description: "Dangerous op",
|
||||
},
|
||||
});
|
||||
|
||||
mockCallGateway.mockResolvedValueOnce({ id: "server-id-2", status: "accepted" });
|
||||
mockCallGateway.mockResolvedValueOnce({ id: "server-id-2", decision: "deny" });
|
||||
|
||||
const result = await runBeforeToolCallHook({
|
||||
toolName: "bash",
|
||||
params: {},
|
||||
ctx: { agentId: "main", sessionKey: "main" },
|
||||
});
|
||||
|
||||
expect(result.blocked).toBe(true);
|
||||
expect(result).toHaveProperty("reason", "Denied by user");
|
||||
});
|
||||
|
||||
it("blocks on timeout with default deny behavior", async () => {
|
||||
const { callGatewayTool } = await import("./tools/gateway.js");
|
||||
const mockCallGateway = vi.mocked(callGatewayTool);
|
||||
|
||||
hookRunner.runBeforeToolCall.mockResolvedValue({
|
||||
requireApproval: {
|
||||
title: "Timeout test",
|
||||
description: "Will time out",
|
||||
},
|
||||
});
|
||||
|
||||
mockCallGateway.mockResolvedValueOnce({ id: "server-id-3", status: "accepted" });
|
||||
mockCallGateway.mockResolvedValueOnce({ id: "server-id-3", decision: null });
|
||||
|
||||
const result = await runBeforeToolCallHook({
|
||||
toolName: "bash",
|
||||
params: {},
|
||||
ctx: { agentId: "main", sessionKey: "main" },
|
||||
});
|
||||
|
||||
expect(result.blocked).toBe(true);
|
||||
expect(result).toHaveProperty("reason", "Approval timed out");
|
||||
});
|
||||
|
||||
it("allows on timeout when timeoutBehavior is allow and preserves hook params", async () => {
|
||||
const { callGatewayTool } = await import("./tools/gateway.js");
|
||||
const mockCallGateway = vi.mocked(callGatewayTool);
|
||||
|
||||
hookRunner.runBeforeToolCall.mockResolvedValue({
|
||||
params: { command: "safe-command" },
|
||||
requireApproval: {
|
||||
title: "Lenient timeout",
|
||||
description: "Should allow on timeout",
|
||||
timeoutBehavior: "allow",
|
||||
},
|
||||
});
|
||||
|
||||
mockCallGateway.mockResolvedValueOnce({ id: "server-id-4", status: "accepted" });
|
||||
mockCallGateway.mockResolvedValueOnce({ id: "server-id-4", decision: null });
|
||||
|
||||
const result = await runBeforeToolCallHook({
|
||||
toolName: "bash",
|
||||
params: { command: "rm -rf /" },
|
||||
ctx: { agentId: "main", sessionKey: "main" },
|
||||
});
|
||||
|
||||
expect(result.blocked).toBe(false);
|
||||
if (!result.blocked) {
|
||||
expect(result.params).toEqual({ command: "safe-command" });
|
||||
}
|
||||
});
|
||||
|
||||
it("falls back to block on gateway error", async () => {
|
||||
const { callGatewayTool } = await import("./tools/gateway.js");
|
||||
const mockCallGateway = vi.mocked(callGatewayTool);
|
||||
|
||||
hookRunner.runBeforeToolCall.mockResolvedValue({
|
||||
requireApproval: {
|
||||
title: "Gateway down",
|
||||
description: "Gateway is unavailable",
|
||||
},
|
||||
});
|
||||
|
||||
mockCallGateway.mockRejectedValueOnce(new Error("unknown method plugin.approval.request"));
|
||||
|
||||
const result = await runBeforeToolCallHook({
|
||||
toolName: "bash",
|
||||
params: {},
|
||||
ctx: { agentId: "main", sessionKey: "main" },
|
||||
});
|
||||
|
||||
expect(result.blocked).toBe(true);
|
||||
expect(result).toHaveProperty("reason", "Plugin approval required (gateway unavailable)");
|
||||
});
|
||||
|
||||
it("blocks when gateway returns no id", async () => {
|
||||
const { callGatewayTool } = await import("./tools/gateway.js");
|
||||
const mockCallGateway = vi.mocked(callGatewayTool);
|
||||
|
||||
hookRunner.runBeforeToolCall.mockResolvedValue({
|
||||
requireApproval: {
|
||||
title: "No ID",
|
||||
description: "Registration returns no id",
|
||||
},
|
||||
});
|
||||
|
||||
mockCallGateway.mockResolvedValueOnce({ status: "error" });
|
||||
|
||||
const result = await runBeforeToolCallHook({
|
||||
toolName: "bash",
|
||||
params: {},
|
||||
ctx: { agentId: "main", sessionKey: "main" },
|
||||
});
|
||||
|
||||
expect(result.blocked).toBe(true);
|
||||
expect(result).toHaveProperty("reason", "Registration returns no id");
|
||||
});
|
||||
|
||||
it("blocks on immediate null decision without calling waitDecision even when timeoutBehavior is allow", async () => {
|
||||
const { callGatewayTool } = await import("./tools/gateway.js");
|
||||
const mockCallGateway = vi.mocked(callGatewayTool);
|
||||
const onResolution = vi.fn();
|
||||
|
||||
hookRunner.runBeforeToolCall.mockResolvedValue({
|
||||
requireApproval: {
|
||||
title: "No route",
|
||||
description: "No approval route available",
|
||||
timeoutBehavior: "allow",
|
||||
onResolution,
|
||||
},
|
||||
});
|
||||
|
||||
mockCallGateway.mockResolvedValueOnce({ id: "server-id-immediate", decision: null });
|
||||
|
||||
const result = await runBeforeToolCallHook({
|
||||
toolName: "bash",
|
||||
params: {},
|
||||
ctx: { agentId: "main", sessionKey: "main" },
|
||||
});
|
||||
|
||||
expect(result.blocked).toBe(true);
|
||||
expect(result).toHaveProperty("reason", "Plugin approval unavailable (no approval route)");
|
||||
expect(onResolution).toHaveBeenCalledWith("cancelled");
|
||||
expect(mockCallGateway.mock.calls.map(([method]) => method)).toEqual([
|
||||
"plugin.approval.request",
|
||||
]);
|
||||
});
|
||||
|
||||
it("unblocks immediately when abort signal fires during waitDecision", async () => {
|
||||
const { callGatewayTool } = await import("./tools/gateway.js");
|
||||
const mockCallGateway = vi.mocked(callGatewayTool);
|
||||
|
||||
hookRunner.runBeforeToolCall.mockResolvedValue({
|
||||
requireApproval: {
|
||||
title: "Abortable",
|
||||
description: "Will be aborted",
|
||||
},
|
||||
});
|
||||
|
||||
const controller = new AbortController();
|
||||
|
||||
// First call: plugin.approval.request → accepted
|
||||
mockCallGateway.mockResolvedValueOnce({ id: "server-id-abort", status: "accepted" });
|
||||
// Second call: plugin.approval.waitDecision → never resolves (simulates long wait)
|
||||
mockCallGateway.mockImplementationOnce(
|
||||
() => new Promise(() => {}), // hangs forever
|
||||
);
|
||||
|
||||
// Abort after a short delay
|
||||
setTimeout(() => controller.abort(new Error("run cancelled")), 10);
|
||||
|
||||
const result = await runBeforeToolCallHook({
|
||||
toolName: "bash",
|
||||
params: {},
|
||||
ctx: { agentId: "main", sessionKey: "main" },
|
||||
signal: controller.signal,
|
||||
});
|
||||
|
||||
expect(result.blocked).toBe(true);
|
||||
expect(result).toHaveProperty("reason", "Approval cancelled (run aborted)");
|
||||
expect(mockCallGateway).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it("removes abort listener after waitDecision resolves", async () => {
|
||||
const { callGatewayTool } = await import("./tools/gateway.js");
|
||||
const mockCallGateway = vi.mocked(callGatewayTool);
|
||||
|
||||
hookRunner.runBeforeToolCall.mockResolvedValue({
|
||||
requireApproval: {
|
||||
title: "Cleanup listener",
|
||||
description: "Wait resolves quickly",
|
||||
},
|
||||
});
|
||||
|
||||
const controller = new AbortController();
|
||||
const removeListenerSpy = vi.spyOn(controller.signal, "removeEventListener");
|
||||
|
||||
mockCallGateway.mockResolvedValueOnce({ id: "server-id-cleanup", status: "accepted" });
|
||||
mockCallGateway.mockResolvedValueOnce({ id: "server-id-cleanup", decision: "allow-once" });
|
||||
|
||||
const result = await runBeforeToolCallHook({
|
||||
toolName: "bash",
|
||||
params: {},
|
||||
ctx: { agentId: "main", sessionKey: "main" },
|
||||
signal: controller.signal,
|
||||
});
|
||||
|
||||
expect(result.blocked).toBe(false);
|
||||
expect(removeListenerSpy.mock.calls.some(([type]) => type === "abort")).toBe(true);
|
||||
});
|
||||
|
||||
it("calls onResolution with allow-once on approval", async () => {
|
||||
const { callGatewayTool } = await import("./tools/gateway.js");
|
||||
const mockCallGateway = vi.mocked(callGatewayTool);
|
||||
const onResolution = vi.fn();
|
||||
|
||||
hookRunner.runBeforeToolCall.mockResolvedValue({
|
||||
requireApproval: {
|
||||
title: "Needs approval",
|
||||
description: "Check this",
|
||||
onResolution,
|
||||
},
|
||||
});
|
||||
|
||||
mockCallGateway.mockResolvedValueOnce({ id: "server-id-r1", status: "accepted" });
|
||||
mockCallGateway.mockResolvedValueOnce({ id: "server-id-r1", decision: "allow-once" });
|
||||
|
||||
await runBeforeToolCallHook({
|
||||
toolName: "bash",
|
||||
params: {},
|
||||
ctx: { agentId: "main", sessionKey: "main" },
|
||||
});
|
||||
|
||||
expect(onResolution).toHaveBeenCalledWith("allow-once");
|
||||
});
|
||||
|
||||
it("does not await onResolution before returning approval outcome", async () => {
|
||||
const { callGatewayTool } = await import("./tools/gateway.js");
|
||||
const mockCallGateway = vi.mocked(callGatewayTool);
|
||||
const onResolution = vi.fn(() => new Promise<void>(() => {}));
|
||||
|
||||
hookRunner.runBeforeToolCall.mockResolvedValue({
|
||||
requireApproval: {
|
||||
title: "Non-blocking callback",
|
||||
description: "Should not block tool execution",
|
||||
onResolution,
|
||||
},
|
||||
});
|
||||
|
||||
mockCallGateway.mockResolvedValueOnce({ id: "server-id-r1-nonblocking", status: "accepted" });
|
||||
mockCallGateway.mockResolvedValueOnce({
|
||||
id: "server-id-r1-nonblocking",
|
||||
decision: "allow-once",
|
||||
});
|
||||
|
||||
let timeoutId: NodeJS.Timeout | undefined;
|
||||
try {
|
||||
const result = await Promise.race([
|
||||
runBeforeToolCallHook({
|
||||
toolName: "bash",
|
||||
params: {},
|
||||
ctx: { agentId: "main", sessionKey: "main" },
|
||||
}),
|
||||
new Promise<never>((_, reject) => {
|
||||
timeoutId = setTimeout(
|
||||
() => reject(new Error("runBeforeToolCallHook waited for onResolution")),
|
||||
250,
|
||||
);
|
||||
}),
|
||||
]);
|
||||
|
||||
expect(result).toEqual({ blocked: false, params: {} });
|
||||
expect(onResolution).toHaveBeenCalledWith("allow-once");
|
||||
} finally {
|
||||
if (timeoutId) {
|
||||
clearTimeout(timeoutId);
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
it("calls onResolution with deny on denial", async () => {
|
||||
const { callGatewayTool } = await import("./tools/gateway.js");
|
||||
const mockCallGateway = vi.mocked(callGatewayTool);
|
||||
const onResolution = vi.fn();
|
||||
|
||||
hookRunner.runBeforeToolCall.mockResolvedValue({
|
||||
requireApproval: {
|
||||
title: "Needs approval",
|
||||
description: "Check this",
|
||||
onResolution,
|
||||
},
|
||||
});
|
||||
|
||||
mockCallGateway.mockResolvedValueOnce({ id: "server-id-r2", status: "accepted" });
|
||||
mockCallGateway.mockResolvedValueOnce({ id: "server-id-r2", decision: "deny" });
|
||||
|
||||
await runBeforeToolCallHook({
|
||||
toolName: "bash",
|
||||
params: {},
|
||||
ctx: { agentId: "main", sessionKey: "main" },
|
||||
});
|
||||
|
||||
expect(onResolution).toHaveBeenCalledWith("deny");
|
||||
});
|
||||
|
||||
it("calls onResolution with timeout when decision is null", async () => {
|
||||
const { callGatewayTool } = await import("./tools/gateway.js");
|
||||
const mockCallGateway = vi.mocked(callGatewayTool);
|
||||
const onResolution = vi.fn();
|
||||
|
||||
hookRunner.runBeforeToolCall.mockResolvedValue({
|
||||
requireApproval: {
|
||||
title: "Timeout resolution",
|
||||
description: "Will time out",
|
||||
onResolution,
|
||||
},
|
||||
});
|
||||
|
||||
mockCallGateway.mockResolvedValueOnce({ id: "server-id-r3", status: "accepted" });
|
||||
mockCallGateway.mockResolvedValueOnce({ id: "server-id-r3", decision: null });
|
||||
|
||||
await runBeforeToolCallHook({
|
||||
toolName: "bash",
|
||||
params: {},
|
||||
ctx: { agentId: "main", sessionKey: "main" },
|
||||
});
|
||||
|
||||
expect(onResolution).toHaveBeenCalledWith("timeout");
|
||||
});
|
||||
|
||||
it("calls onResolution with cancelled on gateway error", async () => {
|
||||
const { callGatewayTool } = await import("./tools/gateway.js");
|
||||
const mockCallGateway = vi.mocked(callGatewayTool);
|
||||
const onResolution = vi.fn();
|
||||
|
||||
hookRunner.runBeforeToolCall.mockResolvedValue({
|
||||
requireApproval: {
|
||||
title: "Gateway error",
|
||||
description: "Gateway will fail",
|
||||
onResolution,
|
||||
},
|
||||
});
|
||||
|
||||
mockCallGateway.mockRejectedValueOnce(new Error("gateway down"));
|
||||
|
||||
const result = await runBeforeToolCallHook({
|
||||
toolName: "bash",
|
||||
params: {},
|
||||
ctx: { agentId: "main", sessionKey: "main" },
|
||||
});
|
||||
|
||||
expect(result.blocked).toBe(true);
|
||||
expect(result).toHaveProperty("reason", "Plugin approval required (gateway unavailable)");
|
||||
expect(onResolution).toHaveBeenCalledWith("cancelled");
|
||||
});
|
||||
|
||||
it("calls onResolution with cancelled when abort signal fires", async () => {
|
||||
const { callGatewayTool } = await import("./tools/gateway.js");
|
||||
const mockCallGateway = vi.mocked(callGatewayTool);
|
||||
const onResolution = vi.fn();
|
||||
|
||||
hookRunner.runBeforeToolCall.mockResolvedValue({
|
||||
requireApproval: {
|
||||
title: "Abortable with callback",
|
||||
description: "Will be aborted",
|
||||
onResolution,
|
||||
},
|
||||
});
|
||||
|
||||
const controller = new AbortController();
|
||||
|
||||
mockCallGateway.mockResolvedValueOnce({ id: "server-id-r5", status: "accepted" });
|
||||
mockCallGateway.mockImplementationOnce(
|
||||
() => new Promise(() => {}), // hangs forever
|
||||
);
|
||||
|
||||
setTimeout(() => controller.abort(new Error("run cancelled")), 10);
|
||||
|
||||
const result = await runBeforeToolCallHook({
|
||||
toolName: "bash",
|
||||
params: {},
|
||||
ctx: { agentId: "main", sessionKey: "main" },
|
||||
signal: controller.signal,
|
||||
});
|
||||
|
||||
expect(result.blocked).toBe(true);
|
||||
expect(result).toHaveProperty("reason", "Approval cancelled (run aborted)");
|
||||
expect(onResolution).toHaveBeenCalledWith("cancelled");
|
||||
});
|
||||
|
||||
it("calls onResolution with cancelled when gateway returns no id", async () => {
|
||||
const { callGatewayTool } = await import("./tools/gateway.js");
|
||||
const mockCallGateway = vi.mocked(callGatewayTool);
|
||||
const onResolution = vi.fn();
|
||||
|
||||
hookRunner.runBeforeToolCall.mockResolvedValue({
|
||||
requireApproval: {
|
||||
title: "No ID",
|
||||
description: "Registration returns no id",
|
||||
onResolution,
|
||||
},
|
||||
});
|
||||
|
||||
mockCallGateway.mockResolvedValueOnce({ status: "error" });
|
||||
|
||||
await runBeforeToolCallHook({
|
||||
toolName: "bash",
|
||||
params: {},
|
||||
ctx: { agentId: "main", sessionKey: "main" },
|
||||
});
|
||||
|
||||
expect(onResolution).toHaveBeenCalledWith("cancelled");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -3,11 +3,13 @@ import type { SessionState } from "../logging/diagnostic-session-state.js";
|
||||
import { createSubsystemLogger } from "../logging/subsystem.js";
|
||||
import { getGlobalHookRunner } from "../plugins/hook-runner-global.js";
|
||||
import { copyPluginToolMeta } from "../plugins/tools.js";
|
||||
import { PluginApprovalResolutions, type PluginApprovalResolution } from "../plugins/types.js";
|
||||
import { createLazyRuntimeSurface } from "../shared/lazy-runtime.js";
|
||||
import { isPlainObject } from "../utils.js";
|
||||
import { copyChannelAgentToolMeta } from "./channel-tools.js";
|
||||
import { normalizeToolName } from "./tool-policy.js";
|
||||
import type { AnyAgentTool } from "./tools/common.js";
|
||||
import { callGatewayTool } from "./tools/gateway.js";
|
||||
|
||||
export type HookContext = {
|
||||
agentId?: string;
|
||||
@@ -39,6 +41,32 @@ function buildAdjustedParamsKey(params: { runId?: string; toolCallId: string }):
|
||||
return params.toolCallId;
|
||||
}
|
||||
|
||||
function mergeParamsWithApprovalOverrides(
|
||||
originalParams: unknown,
|
||||
approvalParams?: unknown,
|
||||
): unknown {
|
||||
if (approvalParams && isPlainObject(approvalParams)) {
|
||||
if (isPlainObject(originalParams)) {
|
||||
return { ...originalParams, ...approvalParams };
|
||||
}
|
||||
return approvalParams;
|
||||
}
|
||||
return originalParams;
|
||||
}
|
||||
|
||||
function isAbortSignalCancellation(err: unknown, signal?: AbortSignal): boolean {
|
||||
if (!signal?.aborted) {
|
||||
return false;
|
||||
}
|
||||
if (err === signal.reason) {
|
||||
return true;
|
||||
}
|
||||
if (err instanceof Error && err.name === "AbortError") {
|
||||
return true;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
function shouldEmitLoopWarning(state: SessionState, warningKey: string, count: number): boolean {
|
||||
if (!state.toolLoopWarningBuckets) {
|
||||
state.toolLoopWarningBuckets = new Map();
|
||||
@@ -93,6 +121,7 @@ export async function runBeforeToolCallHook(args: {
|
||||
params: unknown;
|
||||
toolCallId?: string;
|
||||
ctx?: HookContext;
|
||||
signal?: AbortSignal;
|
||||
}): Promise<HookOutcome> {
|
||||
const toolName = normalizeToolName(args.toolName || "tool");
|
||||
const params = args.params;
|
||||
@@ -156,18 +185,18 @@ export async function runBeforeToolCallHook(args: {
|
||||
const normalizedParams = isPlainObject(params) ? params : {};
|
||||
const toolContext = {
|
||||
toolName,
|
||||
...(args.ctx?.agentId ? { agentId: args.ctx.agentId } : {}),
|
||||
...(args.ctx?.sessionKey ? { sessionKey: args.ctx.sessionKey } : {}),
|
||||
...(args.ctx?.sessionId ? { sessionId: args.ctx.sessionId } : {}),
|
||||
...(args.ctx?.runId ? { runId: args.ctx.runId } : {}),
|
||||
...(args.toolCallId ? { toolCallId: args.toolCallId } : {}),
|
||||
...(args.ctx?.agentId && { agentId: args.ctx.agentId }),
|
||||
...(args.ctx?.sessionKey && { sessionKey: args.ctx.sessionKey }),
|
||||
...(args.ctx?.sessionId && { sessionId: args.ctx.sessionId }),
|
||||
...(args.ctx?.runId && { runId: args.ctx.runId }),
|
||||
...(args.toolCallId && { toolCallId: args.toolCallId }),
|
||||
};
|
||||
const hookResult = await hookRunner.runBeforeToolCall(
|
||||
{
|
||||
toolName,
|
||||
params: normalizedParams,
|
||||
...(args.ctx?.runId ? { runId: args.ctx.runId } : {}),
|
||||
...(args.toolCallId ? { toolCallId: args.toolCallId } : {}),
|
||||
...(args.ctx?.runId && { runId: args.ctx.runId }),
|
||||
...(args.toolCallId && { toolCallId: args.toolCallId }),
|
||||
},
|
||||
toolContext,
|
||||
);
|
||||
@@ -179,11 +208,152 @@ export async function runBeforeToolCallHook(args: {
|
||||
};
|
||||
}
|
||||
|
||||
if (hookResult?.params && isPlainObject(hookResult.params)) {
|
||||
if (isPlainObject(params)) {
|
||||
return { blocked: false, params: { ...params, ...hookResult.params } };
|
||||
if (hookResult?.requireApproval) {
|
||||
const approval = hookResult.requireApproval;
|
||||
const safeOnResolution = (resolution: PluginApprovalResolution): void => {
|
||||
const onResolution = approval.onResolution;
|
||||
if (typeof onResolution !== "function") {
|
||||
return;
|
||||
}
|
||||
try {
|
||||
void Promise.resolve(onResolution(resolution)).catch((err) => {
|
||||
log.warn(`plugin onResolution callback failed: ${String(err)}`);
|
||||
});
|
||||
} catch (err) {
|
||||
log.warn(`plugin onResolution callback failed: ${String(err)}`);
|
||||
}
|
||||
};
|
||||
try {
|
||||
const requestResult = await callGatewayTool<{
|
||||
id?: string;
|
||||
status?: string;
|
||||
decision?: string | null;
|
||||
}>(
|
||||
"plugin.approval.request",
|
||||
// Buffer beyond the approval timeout so the gateway can clean up
|
||||
// and respond before the client-side RPC timeout fires.
|
||||
{ timeoutMs: (approval.timeoutMs ?? 120_000) + 10_000 },
|
||||
{
|
||||
pluginId: approval.pluginId,
|
||||
title: approval.title,
|
||||
description: approval.description,
|
||||
severity: approval.severity,
|
||||
toolName,
|
||||
toolCallId: args.toolCallId,
|
||||
agentId: args.ctx?.agentId,
|
||||
sessionKey: args.ctx?.sessionKey,
|
||||
timeoutMs: approval.timeoutMs ?? 120_000,
|
||||
twoPhase: true,
|
||||
},
|
||||
{ expectFinal: false },
|
||||
);
|
||||
const id = requestResult?.id;
|
||||
if (!id) {
|
||||
safeOnResolution(PluginApprovalResolutions.CANCELLED);
|
||||
return {
|
||||
blocked: true,
|
||||
reason: approval.description || "Plugin approval request failed",
|
||||
};
|
||||
}
|
||||
const hasImmediateDecision = Object.prototype.hasOwnProperty.call(
|
||||
requestResult ?? {},
|
||||
"decision",
|
||||
);
|
||||
let decision: string | null | undefined;
|
||||
if (hasImmediateDecision) {
|
||||
decision = requestResult?.decision;
|
||||
if (decision === null) {
|
||||
safeOnResolution(PluginApprovalResolutions.CANCELLED);
|
||||
return {
|
||||
blocked: true,
|
||||
reason: "Plugin approval unavailable (no approval route)",
|
||||
};
|
||||
}
|
||||
} else {
|
||||
// Wait for the decision, but abort early if the agent run is cancelled
|
||||
// so the user isn't blocked for the full approval timeout.
|
||||
const waitPromise = callGatewayTool<{
|
||||
id?: string;
|
||||
decision?: string | null;
|
||||
}>(
|
||||
"plugin.approval.waitDecision",
|
||||
// Buffer beyond the approval timeout so the gateway can clean up
|
||||
// and respond before the client-side RPC timeout fires.
|
||||
{ timeoutMs: (approval.timeoutMs ?? 120_000) + 10_000 },
|
||||
{ id },
|
||||
);
|
||||
let waitResult: { id?: string; decision?: string | null } | undefined;
|
||||
if (args.signal) {
|
||||
let onAbort: (() => void) | undefined;
|
||||
const abortPromise = new Promise<never>((_, reject) => {
|
||||
if (args.signal!.aborted) {
|
||||
reject(args.signal!.reason);
|
||||
return;
|
||||
}
|
||||
onAbort = () => reject(args.signal!.reason);
|
||||
args.signal!.addEventListener("abort", onAbort, { once: true });
|
||||
});
|
||||
try {
|
||||
waitResult = await Promise.race([waitPromise, abortPromise]);
|
||||
} finally {
|
||||
if (onAbort) {
|
||||
args.signal.removeEventListener("abort", onAbort);
|
||||
}
|
||||
}
|
||||
} else {
|
||||
waitResult = await waitPromise;
|
||||
}
|
||||
decision = waitResult?.decision;
|
||||
}
|
||||
const resolution: PluginApprovalResolution =
|
||||
decision === PluginApprovalResolutions.ALLOW_ONCE ||
|
||||
decision === PluginApprovalResolutions.ALLOW_ALWAYS ||
|
||||
decision === PluginApprovalResolutions.DENY
|
||||
? decision
|
||||
: PluginApprovalResolutions.TIMEOUT;
|
||||
safeOnResolution(resolution);
|
||||
if (
|
||||
decision === PluginApprovalResolutions.ALLOW_ONCE ||
|
||||
decision === PluginApprovalResolutions.ALLOW_ALWAYS
|
||||
) {
|
||||
return {
|
||||
blocked: false,
|
||||
params: mergeParamsWithApprovalOverrides(params, hookResult.params),
|
||||
};
|
||||
}
|
||||
if (decision === PluginApprovalResolutions.DENY) {
|
||||
return { blocked: true, reason: "Denied by user" };
|
||||
}
|
||||
const timeoutBehavior = approval.timeoutBehavior ?? "deny";
|
||||
if (timeoutBehavior === "allow") {
|
||||
return {
|
||||
blocked: false,
|
||||
params: mergeParamsWithApprovalOverrides(params, hookResult.params),
|
||||
};
|
||||
}
|
||||
return { blocked: true, reason: "Approval timed out" };
|
||||
} catch (err) {
|
||||
safeOnResolution(PluginApprovalResolutions.CANCELLED);
|
||||
if (isAbortSignalCancellation(err, args.signal)) {
|
||||
log.warn(`plugin approval wait cancelled by run abort: ${String(err)}`);
|
||||
return {
|
||||
blocked: true,
|
||||
reason: "Approval cancelled (run aborted)",
|
||||
};
|
||||
}
|
||||
log.warn(`plugin approval gateway request failed, falling back to block: ${String(err)}`);
|
||||
return {
|
||||
blocked: true,
|
||||
reason: "Plugin approval required (gateway unavailable)",
|
||||
};
|
||||
}
|
||||
return { blocked: false, params: hookResult.params };
|
||||
}
|
||||
|
||||
if (hookResult?.params) {
|
||||
return {
|
||||
blocked: false,
|
||||
params: mergeParamsWithApprovalOverrides(params, hookResult.params),
|
||||
};
|
||||
}
|
||||
} catch (err) {
|
||||
const toolCallId = args.toolCallId ? ` toolCallId=${args.toolCallId}` : "";
|
||||
@@ -210,6 +380,7 @@ export function wrapToolWithBeforeToolCallHook(
|
||||
params,
|
||||
toolCallId,
|
||||
ctx,
|
||||
signal,
|
||||
});
|
||||
if (outcome.blocked) {
|
||||
throw new Error(outcome.reason);
|
||||
@@ -273,5 +444,6 @@ export const __testing = {
|
||||
buildAdjustedParamsKey,
|
||||
adjustedParamsByToolCallId,
|
||||
runBeforeToolCallHook,
|
||||
mergeParamsWithApprovalOverrides,
|
||||
isPlainObject,
|
||||
};
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { callGateway } from "../../gateway/call.js";
|
||||
import { ErrorCodes } from "../../gateway/protocol/index.js";
|
||||
import { logVerbose } from "../../globals.js";
|
||||
import {
|
||||
isTelegramExecApprovalApprover,
|
||||
@@ -72,6 +73,39 @@ function buildResolvedByLabel(params: Parameters<CommandHandler>[0]): string {
|
||||
return `${channel}:${sender}`;
|
||||
}
|
||||
|
||||
function readErrorCode(value: unknown): string | null {
|
||||
return typeof value === "string" && value.trim() ? value : null;
|
||||
}
|
||||
|
||||
function readApprovalNotFoundDetailsReason(value: unknown): string | null {
|
||||
if (!value || typeof value !== "object" || Array.isArray(value)) {
|
||||
return null;
|
||||
}
|
||||
const reason = (value as { reason?: unknown }).reason;
|
||||
return typeof reason === "string" && reason.trim() ? reason : null;
|
||||
}
|
||||
|
||||
function isApprovalNotFoundError(err: unknown): boolean {
|
||||
if (!(err instanceof Error)) {
|
||||
return false;
|
||||
}
|
||||
const gatewayCode = readErrorCode((err as { gatewayCode?: unknown }).gatewayCode);
|
||||
if (gatewayCode === ErrorCodes.APPROVAL_NOT_FOUND) {
|
||||
return true;
|
||||
}
|
||||
|
||||
const detailsReason = readApprovalNotFoundDetailsReason((err as { details?: unknown }).details);
|
||||
if (
|
||||
gatewayCode === ErrorCodes.INVALID_REQUEST &&
|
||||
detailsReason === ErrorCodes.APPROVAL_NOT_FOUND
|
||||
) {
|
||||
return true;
|
||||
}
|
||||
|
||||
// Legacy server/client combinations may only include the message text.
|
||||
return /unknown or expired approval id/i.test(err.message);
|
||||
}
|
||||
|
||||
export const handleApproveCommand: CommandHandler = async (params, allowTextCommands) => {
|
||||
if (!allowTextCommands) {
|
||||
return null;
|
||||
@@ -91,26 +125,39 @@ export const handleApproveCommand: CommandHandler = async (params, allowTextComm
|
||||
if (!parsed.ok) {
|
||||
return { shouldContinue: false, reply: { text: parsed.error } };
|
||||
}
|
||||
const isPluginId = parsed.id.startsWith("plugin:");
|
||||
|
||||
if (params.command.channel === "telegram") {
|
||||
if (
|
||||
!isTelegramExecApprovalClientEnabled({ cfg: params.cfg, accountId: params.ctx.AccountId })
|
||||
) {
|
||||
return {
|
||||
shouldContinue: false,
|
||||
reply: { text: "❌ Telegram exec approvals are not enabled for this bot account." },
|
||||
};
|
||||
const telegramApproverContext = {
|
||||
cfg: params.cfg,
|
||||
accountId: params.ctx.AccountId,
|
||||
senderId: params.command.senderId,
|
||||
};
|
||||
|
||||
if (!isPluginId) {
|
||||
if (
|
||||
!isTelegramExecApprovalClientEnabled({ cfg: params.cfg, accountId: params.ctx.AccountId })
|
||||
) {
|
||||
return {
|
||||
shouldContinue: false,
|
||||
reply: { text: "❌ Telegram exec approvals are not enabled for this bot account." },
|
||||
};
|
||||
}
|
||||
if (!isTelegramExecApprovalApprover(telegramApproverContext)) {
|
||||
return {
|
||||
shouldContinue: false,
|
||||
reply: { text: "❌ You are not authorized to approve exec requests on Telegram." },
|
||||
};
|
||||
}
|
||||
}
|
||||
if (
|
||||
!isTelegramExecApprovalApprover({
|
||||
cfg: params.cfg,
|
||||
accountId: params.ctx.AccountId,
|
||||
senderId: params.command.senderId,
|
||||
})
|
||||
) {
|
||||
|
||||
// Keep plugin-ID routing independent from exec approval client enablement so
|
||||
// forwarded plugin approvals remain resolvable, but still require explicit
|
||||
// Telegram approver membership for security parity.
|
||||
if (isPluginId && !isTelegramExecApprovalApprover(telegramApproverContext)) {
|
||||
return {
|
||||
shouldContinue: false,
|
||||
reply: { text: "❌ You are not authorized to approve exec requests on Telegram." },
|
||||
reply: { text: "❌ You are not authorized to approve plugin requests on Telegram." },
|
||||
};
|
||||
}
|
||||
}
|
||||
@@ -125,25 +172,51 @@ export const handleApproveCommand: CommandHandler = async (params, allowTextComm
|
||||
}
|
||||
|
||||
const resolvedBy = buildResolvedByLabel(params);
|
||||
try {
|
||||
const callApprovalMethod = async (method: string): Promise<void> => {
|
||||
await callGateway({
|
||||
method: "exec.approval.resolve",
|
||||
method,
|
||||
params: { id: parsed.id, decision: parsed.decision },
|
||||
clientName: GATEWAY_CLIENT_NAMES.GATEWAY_CLIENT,
|
||||
clientDisplayName: `Chat approval (${resolvedBy})`,
|
||||
mode: GATEWAY_CLIENT_MODES.BACKEND,
|
||||
});
|
||||
} catch (err) {
|
||||
return {
|
||||
shouldContinue: false,
|
||||
reply: {
|
||||
text: `❌ Failed to submit approval: ${String(err)}`,
|
||||
},
|
||||
};
|
||||
};
|
||||
|
||||
// Plugin approval IDs are kind-prefixed (`plugin:<uuid>`); route directly when detected.
|
||||
// Unprefixed IDs try exec first, then fall back to plugin for backward compat.
|
||||
if (isPluginId) {
|
||||
try {
|
||||
await callApprovalMethod("plugin.approval.resolve");
|
||||
} catch (err) {
|
||||
return {
|
||||
shouldContinue: false,
|
||||
reply: { text: `❌ Failed to submit approval: ${String(err)}` },
|
||||
};
|
||||
}
|
||||
} else {
|
||||
try {
|
||||
await callApprovalMethod("exec.approval.resolve");
|
||||
} catch (err) {
|
||||
if (isApprovalNotFoundError(err)) {
|
||||
try {
|
||||
await callApprovalMethod("plugin.approval.resolve");
|
||||
} catch (pluginErr) {
|
||||
return {
|
||||
shouldContinue: false,
|
||||
reply: { text: `❌ Failed to submit approval: ${String(pluginErr)}` },
|
||||
};
|
||||
}
|
||||
} else {
|
||||
return {
|
||||
shouldContinue: false,
|
||||
reply: { text: `❌ Failed to submit approval: ${String(err)}` },
|
||||
};
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return {
|
||||
shouldContinue: false,
|
||||
reply: { text: `✅ Exec approval ${parsed.decision} submitted for ${parsed.id}.` },
|
||||
reply: { text: `✅ Approval ${parsed.decision} submitted for ${parsed.id}.` },
|
||||
};
|
||||
};
|
||||
|
||||
@@ -345,7 +345,7 @@ describe("/approve command", () => {
|
||||
|
||||
function createTelegramApproveCfg(
|
||||
execApprovals: {
|
||||
enabled: true;
|
||||
enabled: boolean;
|
||||
approvers: string[];
|
||||
target: "dm";
|
||||
} | null = { enabled: true, approvers: ["123"], target: "dm" },
|
||||
@@ -383,7 +383,7 @@ describe("/approve command", () => {
|
||||
|
||||
const result = await handleCommands(params);
|
||||
expect(result.shouldContinue).toBe(false);
|
||||
expect(result.reply?.text).toContain("Exec approval allow-once submitted");
|
||||
expect(result.reply?.text).toContain("Approval allow-once submitted");
|
||||
expect(callGatewayMock).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
method: "exec.approval.resolve",
|
||||
@@ -405,7 +405,7 @@ describe("/approve command", () => {
|
||||
|
||||
const result = await handleCommands(params);
|
||||
expect(result.shouldContinue).toBe(false);
|
||||
expect(result.reply?.text).toContain("Exec approval allow-once submitted");
|
||||
expect(result.reply?.text).toContain("Approval allow-once submitted");
|
||||
expect(callGatewayMock).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
method: "exec.approval.resolve",
|
||||
@@ -439,9 +439,12 @@ describe("/approve command", () => {
|
||||
Surface: "telegram",
|
||||
SenderId: "123",
|
||||
},
|
||||
setup: () => callGatewayMock.mockRejectedValue(new Error("unknown or expired approval id")),
|
||||
setup: () =>
|
||||
callGatewayMock.mockRejectedValue(
|
||||
gatewayError("unknown or expired approval id", "APPROVAL_NOT_FOUND"),
|
||||
),
|
||||
expectedText: "unknown or expired approval id",
|
||||
expectGatewayCalls: 1,
|
||||
expectGatewayCalls: 2,
|
||||
},
|
||||
{
|
||||
name: "telegram approvals disabled",
|
||||
@@ -481,6 +484,56 @@ describe("/approve command", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("rejects Telegram plugin-prefixed IDs when no approver policy is configured", async () => {
|
||||
const cfg = createTelegramApproveCfg(null);
|
||||
const params = buildParams("/approve plugin:abc123 allow-once", cfg, {
|
||||
Provider: "telegram",
|
||||
Surface: "telegram",
|
||||
SenderId: "123",
|
||||
});
|
||||
|
||||
const result = await handleCommands(params);
|
||||
expect(result.shouldContinue).toBe(false);
|
||||
expect(result.reply?.text).toContain("not authorized to approve plugin requests");
|
||||
expect(callGatewayMock).toHaveBeenCalledTimes(0);
|
||||
});
|
||||
|
||||
it("enforces Telegram approver policy for plugin-prefixed IDs when configured", async () => {
|
||||
const cfg = createTelegramApproveCfg({ enabled: false, approvers: ["999"], target: "dm" });
|
||||
const params = buildParams("/approve plugin:abc123 allow-once", cfg, {
|
||||
Provider: "telegram",
|
||||
Surface: "telegram",
|
||||
SenderId: "123",
|
||||
});
|
||||
|
||||
const result = await handleCommands(params);
|
||||
expect(result.shouldContinue).toBe(false);
|
||||
expect(result.reply?.text).toContain("not authorized to approve plugin requests");
|
||||
expect(callGatewayMock).toHaveBeenCalledTimes(0);
|
||||
});
|
||||
|
||||
it("allows Telegram plugin-prefixed IDs for configured approvers even when exec approvals are disabled", async () => {
|
||||
const cfg = createTelegramApproveCfg({ enabled: false, approvers: ["123"], target: "dm" });
|
||||
const params = buildParams("/approve plugin:abc123 allow-once", cfg, {
|
||||
Provider: "telegram",
|
||||
Surface: "telegram",
|
||||
SenderId: "123",
|
||||
});
|
||||
|
||||
callGatewayMock.mockResolvedValueOnce({ ok: true });
|
||||
|
||||
const result = await handleCommands(params);
|
||||
expect(result.shouldContinue).toBe(false);
|
||||
expect(result.reply?.text).toContain("Approval allow-once submitted");
|
||||
expect(callGatewayMock).toHaveBeenCalledTimes(1);
|
||||
expect(callGatewayMock).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
method: "plugin.approval.resolve",
|
||||
params: { id: "plugin:abc123", decision: "allow-once" },
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("enforces gateway approval scopes", async () => {
|
||||
const cfg = {
|
||||
commands: { text: true },
|
||||
@@ -493,12 +546,12 @@ describe("/approve command", () => {
|
||||
},
|
||||
{
|
||||
scopes: ["operator.approvals"],
|
||||
expectedText: "Exec approval allow-once submitted",
|
||||
expectedText: "Approval allow-once submitted",
|
||||
expectedGatewayCalls: 1,
|
||||
},
|
||||
{
|
||||
scopes: ["operator.admin"],
|
||||
expectedText: "Exec approval allow-once submitted",
|
||||
expectedText: "Approval allow-once submitted",
|
||||
expectedGatewayCalls: 1,
|
||||
},
|
||||
] as const;
|
||||
@@ -527,6 +580,205 @@ describe("/approve command", () => {
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
function gatewayError(message: string, gatewayCode: string, opts?: { details?: unknown }): Error {
|
||||
const err = new Error(message) as Error & { gatewayCode?: string; details?: unknown };
|
||||
err.name = "GatewayClientRequestError";
|
||||
err.gatewayCode = gatewayCode;
|
||||
if (opts && "details" in opts) {
|
||||
err.details = opts.details;
|
||||
}
|
||||
return err;
|
||||
}
|
||||
|
||||
it("falls back to plugin.approval.resolve when exec approval id is unknown", async () => {
|
||||
const cfg = {
|
||||
commands: { text: true },
|
||||
channels: { whatsapp: { allowFrom: ["*"] } },
|
||||
} as OpenClawConfig;
|
||||
const params = buildParams("/approve plugin-123 allow-once", cfg, { SenderId: "123" });
|
||||
|
||||
callGatewayMock
|
||||
.mockRejectedValueOnce(gatewayError("unknown or expired approval id", "APPROVAL_NOT_FOUND"))
|
||||
.mockResolvedValueOnce({ ok: true });
|
||||
|
||||
const result = await handleCommands(params);
|
||||
expect(result.shouldContinue).toBe(false);
|
||||
expect(result.reply?.text).toContain("Approval allow-once submitted");
|
||||
expect(callGatewayMock).toHaveBeenCalledTimes(2);
|
||||
expect(callGatewayMock).toHaveBeenNthCalledWith(
|
||||
1,
|
||||
expect.objectContaining({ method: "exec.approval.resolve" }),
|
||||
);
|
||||
expect(callGatewayMock).toHaveBeenNthCalledWith(
|
||||
2,
|
||||
expect.objectContaining({
|
||||
method: "plugin.approval.resolve",
|
||||
params: { id: "plugin-123", decision: "allow-once" },
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("falls back to plugin.approval.resolve for INVALID_REQUEST with approval-not-found details", async () => {
|
||||
const cfg = {
|
||||
commands: { text: true },
|
||||
channels: { whatsapp: { allowFrom: ["*"] } },
|
||||
} as OpenClawConfig;
|
||||
const params = buildParams("/approve plugin-123 allow-once", cfg, { SenderId: "123" });
|
||||
|
||||
callGatewayMock
|
||||
.mockRejectedValueOnce(
|
||||
gatewayError("unknown or expired approval id", "INVALID_REQUEST", {
|
||||
details: { reason: "APPROVAL_NOT_FOUND" },
|
||||
}),
|
||||
)
|
||||
.mockResolvedValueOnce({ ok: true });
|
||||
|
||||
const result = await handleCommands(params);
|
||||
expect(result.shouldContinue).toBe(false);
|
||||
expect(result.reply?.text).toContain("Approval allow-once submitted");
|
||||
expect(callGatewayMock).toHaveBeenCalledTimes(2);
|
||||
expect(callGatewayMock).toHaveBeenNthCalledWith(
|
||||
1,
|
||||
expect.objectContaining({ method: "exec.approval.resolve" }),
|
||||
);
|
||||
expect(callGatewayMock).toHaveBeenNthCalledWith(
|
||||
2,
|
||||
expect.objectContaining({
|
||||
method: "plugin.approval.resolve",
|
||||
params: { id: "plugin-123", decision: "allow-once" },
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("falls back to plugin.approval.resolve for legacy message-only not-found errors", async () => {
|
||||
const cfg = {
|
||||
commands: { text: true },
|
||||
channels: { whatsapp: { allowFrom: ["*"] } },
|
||||
} as OpenClawConfig;
|
||||
const params = buildParams("/approve plugin-123 allow-once", cfg, { SenderId: "123" });
|
||||
|
||||
callGatewayMock
|
||||
.mockRejectedValueOnce(
|
||||
gatewayError("unknown or expired approval id", "INVALID_REQUEST", {
|
||||
details: { reason: "SOMETHING_ELSE" },
|
||||
}),
|
||||
)
|
||||
.mockResolvedValueOnce({ ok: true });
|
||||
|
||||
const result = await handleCommands(params);
|
||||
expect(result.shouldContinue).toBe(false);
|
||||
expect(result.reply?.text).toContain("Approval allow-once submitted");
|
||||
expect(callGatewayMock).toHaveBeenCalledTimes(2);
|
||||
expect(callGatewayMock).toHaveBeenNthCalledWith(
|
||||
1,
|
||||
expect.objectContaining({ method: "exec.approval.resolve" }),
|
||||
);
|
||||
expect(callGatewayMock).toHaveBeenNthCalledWith(
|
||||
2,
|
||||
expect.objectContaining({
|
||||
method: "plugin.approval.resolve",
|
||||
params: { id: "plugin-123", decision: "allow-once" },
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("supports old and new unknown-id gateway envelopes across sequential approvals", async () => {
|
||||
const cfg = {
|
||||
commands: { text: true },
|
||||
channels: { whatsapp: { allowFrom: ["*"] } },
|
||||
} as OpenClawConfig;
|
||||
const cases = [
|
||||
{
|
||||
id: "plugin-old-1",
|
||||
err: gatewayError("unknown or expired approval id", "APPROVAL_NOT_FOUND"),
|
||||
},
|
||||
{
|
||||
id: "plugin-new-2",
|
||||
err: gatewayError("unknown or expired approval id", "INVALID_REQUEST", {
|
||||
details: { reason: "APPROVAL_NOT_FOUND" },
|
||||
}),
|
||||
},
|
||||
] as const;
|
||||
|
||||
for (const testCase of cases) {
|
||||
callGatewayMock.mockReset();
|
||||
callGatewayMock.mockRejectedValueOnce(testCase.err).mockResolvedValueOnce({ ok: true });
|
||||
|
||||
const params = buildParams(`/approve ${testCase.id} allow-once`, cfg, { SenderId: "123" });
|
||||
const result = await handleCommands(params);
|
||||
|
||||
expect(result.shouldContinue, testCase.id).toBe(false);
|
||||
expect(result.reply?.text, testCase.id).toContain("Approval allow-once submitted");
|
||||
expect(callGatewayMock, testCase.id).toHaveBeenNthCalledWith(
|
||||
1,
|
||||
expect.objectContaining({
|
||||
method: "exec.approval.resolve",
|
||||
params: { id: testCase.id, decision: "allow-once" },
|
||||
}),
|
||||
);
|
||||
expect(callGatewayMock, testCase.id).toHaveBeenNthCalledWith(
|
||||
2,
|
||||
expect.objectContaining({
|
||||
method: "plugin.approval.resolve",
|
||||
params: { id: testCase.id, decision: "allow-once" },
|
||||
}),
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
it("surfaces plugin approval error when both exec and plugin resolve fail", async () => {
|
||||
const cfg = {
|
||||
commands: { text: true },
|
||||
channels: { whatsapp: { allowFrom: ["*"] } },
|
||||
} as OpenClawConfig;
|
||||
const params = buildParams("/approve bad-id deny", cfg, { SenderId: "123" });
|
||||
|
||||
callGatewayMock
|
||||
.mockRejectedValueOnce(gatewayError("unknown or expired approval id", "APPROVAL_NOT_FOUND"))
|
||||
.mockRejectedValueOnce(gatewayError("unknown or expired approval id", "APPROVAL_NOT_FOUND"));
|
||||
|
||||
const result = await handleCommands(params);
|
||||
expect(result.shouldContinue).toBe(false);
|
||||
expect(result.reply?.text).toContain("Failed to submit approval");
|
||||
expect(callGatewayMock).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it("routes plugin-prefixed IDs directly to plugin.approval.resolve", async () => {
|
||||
const cfg = {
|
||||
commands: { text: true },
|
||||
channels: { whatsapp: { allowFrom: ["*"] } },
|
||||
} as OpenClawConfig;
|
||||
const params = buildParams("/approve plugin:abc-123 allow-once", cfg, { SenderId: "123" });
|
||||
|
||||
callGatewayMock.mockResolvedValueOnce({ ok: true });
|
||||
|
||||
const result = await handleCommands(params);
|
||||
expect(result.shouldContinue).toBe(false);
|
||||
expect(result.reply?.text).toContain("Approval allow-once submitted");
|
||||
expect(callGatewayMock).toHaveBeenCalledTimes(1);
|
||||
expect(callGatewayMock).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
method: "plugin.approval.resolve",
|
||||
params: { id: "plugin:abc-123", decision: "allow-once" },
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("does not fall back to plugin resolve for non-id errors", async () => {
|
||||
const cfg = {
|
||||
commands: { text: true },
|
||||
channels: { whatsapp: { allowFrom: ["*"] } },
|
||||
} as OpenClawConfig;
|
||||
const params = buildParams("/approve abc allow-once", cfg, { SenderId: "123" });
|
||||
|
||||
callGatewayMock.mockRejectedValueOnce(new Error("gateway connection refused"));
|
||||
|
||||
const result = await handleCommands(params);
|
||||
expect(result.shouldContinue).toBe(false);
|
||||
expect(result.reply?.text).toContain("gateway connection refused");
|
||||
expect(callGatewayMock).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
|
||||
describe("/compact command", () => {
|
||||
|
||||
@@ -5,6 +5,10 @@ import type { GroupToolPolicyConfig } from "../../config/types.tools.js";
|
||||
import type { ExecApprovalRequest, ExecApprovalResolved } from "../../infra/exec-approvals.js";
|
||||
import type { OutboundDeliveryResult, OutboundSendDeps } from "../../infra/outbound/deliver.js";
|
||||
import type { OutboundIdentity } from "../../infra/outbound/identity.js";
|
||||
import type {
|
||||
PluginApprovalRequest,
|
||||
PluginApprovalResolved,
|
||||
} from "../../infra/plugin-approvals.js";
|
||||
import type { PluginRuntime } from "../../plugins/runtime/types.js";
|
||||
import type { RuntimeEnv } from "../../runtime.js";
|
||||
import type { ConfigWriteTarget } from "./config-writes.js";
|
||||
@@ -493,6 +497,17 @@ export type ChannelExecApprovalAdapter = {
|
||||
target: ChannelExecApprovalForwardTarget;
|
||||
payload: ReplyPayload;
|
||||
}) => Promise<void> | void;
|
||||
buildPluginPendingPayload?: (params: {
|
||||
cfg: OpenClawConfig;
|
||||
request: PluginApprovalRequest;
|
||||
target: ChannelExecApprovalForwardTarget;
|
||||
nowMs: number;
|
||||
}) => ReplyPayload | null;
|
||||
buildPluginResolvedPayload?: (params: {
|
||||
cfg: OpenClawConfig;
|
||||
resolved: PluginApprovalResolved;
|
||||
target: ChannelExecApprovalForwardTarget;
|
||||
}) => ReplyPayload | null;
|
||||
};
|
||||
|
||||
export type ChannelAllowlistAdapter = {
|
||||
|
||||
@@ -8388,6 +8388,74 @@ export const GENERATED_BASE_CONFIG_SCHEMA = {
|
||||
},
|
||||
additionalProperties: false,
|
||||
},
|
||||
plugin: {
|
||||
type: "object",
|
||||
properties: {
|
||||
enabled: {
|
||||
type: "boolean",
|
||||
},
|
||||
mode: {
|
||||
anyOf: [
|
||||
{
|
||||
type: "string",
|
||||
const: "session",
|
||||
},
|
||||
{
|
||||
type: "string",
|
||||
const: "targets",
|
||||
},
|
||||
{
|
||||
type: "string",
|
||||
const: "both",
|
||||
},
|
||||
],
|
||||
},
|
||||
agentFilter: {
|
||||
type: "array",
|
||||
items: {
|
||||
type: "string",
|
||||
},
|
||||
},
|
||||
sessionFilter: {
|
||||
type: "array",
|
||||
items: {
|
||||
type: "string",
|
||||
},
|
||||
},
|
||||
targets: {
|
||||
type: "array",
|
||||
items: {
|
||||
type: "object",
|
||||
properties: {
|
||||
channel: {
|
||||
type: "string",
|
||||
minLength: 1,
|
||||
},
|
||||
to: {
|
||||
type: "string",
|
||||
minLength: 1,
|
||||
},
|
||||
accountId: {
|
||||
type: "string",
|
||||
},
|
||||
threadId: {
|
||||
anyOf: [
|
||||
{
|
||||
type: "string",
|
||||
},
|
||||
{
|
||||
type: "number",
|
||||
},
|
||||
],
|
||||
},
|
||||
},
|
||||
required: ["channel", "to"],
|
||||
additionalProperties: false,
|
||||
},
|
||||
},
|
||||
},
|
||||
additionalProperties: false,
|
||||
},
|
||||
},
|
||||
additionalProperties: false,
|
||||
},
|
||||
@@ -12532,7 +12600,7 @@ export const GENERATED_BASE_CONFIG_SCHEMA = {
|
||||
},
|
||||
approvals: {
|
||||
label: "Approvals",
|
||||
help: "Approval routing controls for forwarding exec approval requests to chat destinations outside the originating session. Keep this disabled unless operators need explicit out-of-band approval visibility.",
|
||||
help: "Approval routing controls for forwarding exec and plugin approval requests to chat destinations outside the originating session. Keep these disabled unless operators need explicit out-of-band approval visibility.",
|
||||
tags: ["advanced"],
|
||||
},
|
||||
"approvals.exec": {
|
||||
@@ -12585,6 +12653,56 @@ export const GENERATED_BASE_CONFIG_SCHEMA = {
|
||||
help: "Optional thread/topic target for channels that support threaded delivery of forwarded approvals. Use this to keep approval traffic contained in operational threads instead of main channels.",
|
||||
tags: ["advanced"],
|
||||
},
|
||||
"approvals.plugin": {
|
||||
label: "Plugin Approval Forwarding",
|
||||
help: "Groups plugin-approval forwarding behavior including enablement, routing mode, filters, and explicit targets. Independent of exec approval forwarding. Configure here when plugin approval prompts must reach operational channels.",
|
||||
tags: ["advanced"],
|
||||
},
|
||||
"approvals.plugin.enabled": {
|
||||
label: "Forward Plugin Approvals",
|
||||
help: "Enables forwarding of plugin approval requests to configured delivery destinations (default: false). Independent of approvals.exec.enabled.",
|
||||
tags: ["advanced"],
|
||||
},
|
||||
"approvals.plugin.mode": {
|
||||
label: "Plugin Approval Forwarding Mode",
|
||||
help: 'Controls where plugin approval prompts are sent: "session" uses origin chat, "targets" uses configured targets, and "both" sends to both paths.',
|
||||
tags: ["advanced"],
|
||||
},
|
||||
"approvals.plugin.agentFilter": {
|
||||
label: "Plugin Approval Agent Filter",
|
||||
help: 'Optional allowlist of agent IDs eligible for forwarded plugin approvals, for example `["primary", "ops-agent"]`. Use this to limit forwarding blast radius.',
|
||||
tags: ["advanced"],
|
||||
},
|
||||
"approvals.plugin.sessionFilter": {
|
||||
label: "Plugin Approval Session Filter",
|
||||
help: 'Optional session-key filters matched as substring or regex-style patterns, for example `["discord:", "^agent:ops:"]`. Use narrow patterns so only intended approval contexts are forwarded.',
|
||||
tags: ["storage"],
|
||||
},
|
||||
"approvals.plugin.targets": {
|
||||
label: "Plugin Approval Forwarding Targets",
|
||||
help: "Explicit delivery targets used when plugin approval forwarding mode includes targets, each with channel and destination details.",
|
||||
tags: ["advanced"],
|
||||
},
|
||||
"approvals.plugin.targets[].channel": {
|
||||
label: "Plugin Approval Target Channel",
|
||||
help: "Channel/provider ID used for forwarded plugin approval delivery, such as discord, slack, or a plugin channel id.",
|
||||
tags: ["advanced"],
|
||||
},
|
||||
"approvals.plugin.targets[].to": {
|
||||
label: "Plugin Approval Target Destination",
|
||||
help: "Destination identifier inside the target channel (channel ID, user ID, or thread root depending on provider).",
|
||||
tags: ["advanced"],
|
||||
},
|
||||
"approvals.plugin.targets[].accountId": {
|
||||
label: "Plugin Approval Target Account ID",
|
||||
help: "Optional account selector for multi-account channel setups when plugin approvals must route through a specific account context.",
|
||||
tags: ["advanced"],
|
||||
},
|
||||
"approvals.plugin.targets[].threadId": {
|
||||
label: "Plugin Approval Target Thread ID",
|
||||
help: "Optional thread/topic target for channels that support threaded delivery of forwarded plugin approvals.",
|
||||
tags: ["advanced"],
|
||||
},
|
||||
"tools.message.allowCrossContextSend": {
|
||||
label: "Allow Cross-Context Messaging",
|
||||
help: "Legacy override: allow cross-context sends across all providers.",
|
||||
|
||||
@@ -630,7 +630,7 @@ export const FIELD_HELP: Record<string, string> = {
|
||||
"skills.load.watchDebounceMs":
|
||||
"Debounce window in milliseconds for coalescing rapid skill file changes before reload logic runs. Increase to reduce reload churn on frequent writes, or lower for faster edit feedback.",
|
||||
approvals:
|
||||
"Approval routing controls for forwarding exec approval requests to chat destinations outside the originating session. Keep this disabled unless operators need explicit out-of-band approval visibility.",
|
||||
"Approval routing controls for forwarding exec and plugin approval requests to chat destinations outside the originating session. Keep these disabled unless operators need explicit out-of-band approval visibility.",
|
||||
"approvals.exec":
|
||||
"Groups exec-approval forwarding behavior including enablement, routing mode, filters, and explicit targets. Configure here when approval prompts must reach operational channels instead of only the origin thread.",
|
||||
"approvals.exec.enabled":
|
||||
@@ -651,6 +651,26 @@ export const FIELD_HELP: Record<string, string> = {
|
||||
"Optional account selector for multi-account channel setups when approvals must route through a specific account context. Use this only when the target channel has multiple configured identities.",
|
||||
"approvals.exec.targets[].threadId":
|
||||
"Optional thread/topic target for channels that support threaded delivery of forwarded approvals. Use this to keep approval traffic contained in operational threads instead of main channels.",
|
||||
"approvals.plugin":
|
||||
"Groups plugin-approval forwarding behavior including enablement, routing mode, filters, and explicit targets. Independent of exec approval forwarding. Configure here when plugin approval prompts must reach operational channels.",
|
||||
"approvals.plugin.enabled":
|
||||
"Enables forwarding of plugin approval requests to configured delivery destinations (default: false). Independent of approvals.exec.enabled.",
|
||||
"approvals.plugin.mode":
|
||||
'Controls where plugin approval prompts are sent: "session" uses origin chat, "targets" uses configured targets, and "both" sends to both paths.',
|
||||
"approvals.plugin.agentFilter":
|
||||
'Optional allowlist of agent IDs eligible for forwarded plugin approvals, for example `["primary", "ops-agent"]`. Use this to limit forwarding blast radius.',
|
||||
"approvals.plugin.sessionFilter":
|
||||
'Optional session-key filters matched as substring or regex-style patterns, for example `["discord:", "^agent:ops:"]`. Use narrow patterns so only intended approval contexts are forwarded.',
|
||||
"approvals.plugin.targets":
|
||||
"Explicit delivery targets used when plugin approval forwarding mode includes targets, each with channel and destination details.",
|
||||
"approvals.plugin.targets[].channel":
|
||||
"Channel/provider ID used for forwarded plugin approval delivery, such as discord, slack, or a plugin channel id.",
|
||||
"approvals.plugin.targets[].to":
|
||||
"Destination identifier inside the target channel (channel ID, user ID, or thread root depending on provider).",
|
||||
"approvals.plugin.targets[].accountId":
|
||||
"Optional account selector for multi-account channel setups when plugin approvals must route through a specific account context.",
|
||||
"approvals.plugin.targets[].threadId":
|
||||
"Optional thread/topic target for channels that support threaded delivery of forwarded plugin approvals.",
|
||||
"tools.fs.workspaceOnly":
|
||||
"Restrict filesystem tools (read/write/edit/apply_patch) to the workspace directory (default: false).",
|
||||
"tools.sessions.visibility":
|
||||
|
||||
@@ -210,6 +210,16 @@ export const FIELD_LABELS: Record<string, string> = {
|
||||
"approvals.exec.targets[].to": "Approval Target Destination",
|
||||
"approvals.exec.targets[].accountId": "Approval Target Account ID",
|
||||
"approvals.exec.targets[].threadId": "Approval Target Thread ID",
|
||||
"approvals.plugin": "Plugin Approval Forwarding",
|
||||
"approvals.plugin.enabled": "Forward Plugin Approvals",
|
||||
"approvals.plugin.mode": "Plugin Approval Forwarding Mode",
|
||||
"approvals.plugin.agentFilter": "Plugin Approval Agent Filter",
|
||||
"approvals.plugin.sessionFilter": "Plugin Approval Session Filter",
|
||||
"approvals.plugin.targets": "Plugin Approval Forwarding Targets",
|
||||
"approvals.plugin.targets[].channel": "Plugin Approval Target Channel",
|
||||
"approvals.plugin.targets[].to": "Plugin Approval Target Destination",
|
||||
"approvals.plugin.targets[].accountId": "Plugin Approval Target Account ID",
|
||||
"approvals.plugin.targets[].threadId": "Plugin Approval Target Thread ID",
|
||||
"tools.message.allowCrossContextSend": "Allow Cross-Context Messaging",
|
||||
"tools.message.crossContext.allowWithinProvider": "Allow Cross-Context (Same Provider)",
|
||||
"tools.message.crossContext.allowAcrossProviders": "Allow Cross-Context (Across Providers)",
|
||||
|
||||
@@ -26,4 +26,5 @@ export type ExecApprovalForwardingConfig = {
|
||||
|
||||
export type ApprovalsConfig = {
|
||||
exec?: ExecApprovalForwardingConfig;
|
||||
plugin?: ExecApprovalForwardingConfig;
|
||||
};
|
||||
|
||||
@@ -23,6 +23,7 @@ const ExecApprovalForwardingSchema = z
|
||||
export const ApprovalsSchema = z
|
||||
.object({
|
||||
exec: ExecApprovalForwardingSchema,
|
||||
plugin: ExecApprovalForwardingSchema,
|
||||
})
|
||||
.strict()
|
||||
.optional();
|
||||
|
||||
@@ -9,9 +9,9 @@ const RESOLVED_ENTRY_GRACE_MS = 15_000;
|
||||
|
||||
export type ExecApprovalRequestPayload = InfraExecApprovalRequestPayload;
|
||||
|
||||
export type ExecApprovalRecord = {
|
||||
export type ExecApprovalRecord<TPayload = ExecApprovalRequestPayload> = {
|
||||
id: string;
|
||||
request: ExecApprovalRequestPayload;
|
||||
request: TPayload;
|
||||
createdAtMs: number;
|
||||
expiresAtMs: number;
|
||||
// Caller metadata (best-effort). Used to prevent other clients from replaying an approval id.
|
||||
@@ -23,8 +23,8 @@ export type ExecApprovalRecord = {
|
||||
resolvedBy?: string | null;
|
||||
};
|
||||
|
||||
type PendingEntry = {
|
||||
record: ExecApprovalRecord;
|
||||
type PendingEntry<TPayload = ExecApprovalRequestPayload> = {
|
||||
record: ExecApprovalRecord<TPayload>;
|
||||
resolve: (decision: ExecApprovalDecision | null) => void;
|
||||
reject: (err: Error) => void;
|
||||
timer: ReturnType<typeof setTimeout>;
|
||||
@@ -36,17 +36,13 @@ export type ExecApprovalIdLookupResult =
|
||||
| { kind: "ambiguous"; ids: string[] }
|
||||
| { kind: "none" };
|
||||
|
||||
export class ExecApprovalManager {
|
||||
private pending = new Map<string, PendingEntry>();
|
||||
export class ExecApprovalManager<TPayload = ExecApprovalRequestPayload> {
|
||||
private pending = new Map<string, PendingEntry<TPayload>>();
|
||||
|
||||
create(
|
||||
request: ExecApprovalRequestPayload,
|
||||
timeoutMs: number,
|
||||
id?: string | null,
|
||||
): ExecApprovalRecord {
|
||||
create(request: TPayload, timeoutMs: number, id?: string | null): ExecApprovalRecord<TPayload> {
|
||||
const now = Date.now();
|
||||
const resolvedId = id && id.trim().length > 0 ? id.trim() : randomUUID();
|
||||
const record: ExecApprovalRecord = {
|
||||
const record: ExecApprovalRecord<TPayload> = {
|
||||
id: resolvedId,
|
||||
request,
|
||||
createdAtMs: now,
|
||||
@@ -60,7 +56,10 @@ export class ExecApprovalManager {
|
||||
* This separates registration (synchronous) from waiting (async), allowing callers to
|
||||
* confirm registration before the decision is made.
|
||||
*/
|
||||
register(record: ExecApprovalRecord, timeoutMs: number): Promise<ExecApprovalDecision | null> {
|
||||
register(
|
||||
record: ExecApprovalRecord<TPayload>,
|
||||
timeoutMs: number,
|
||||
): Promise<ExecApprovalDecision | null> {
|
||||
const existing = this.pending.get(record.id);
|
||||
if (existing) {
|
||||
// Idempotent: return existing promise if still pending
|
||||
@@ -77,7 +76,7 @@ export class ExecApprovalManager {
|
||||
rejectPromise = reject;
|
||||
});
|
||||
// Create entry first so we can capture it in the closure (not re-fetch from map)
|
||||
const entry: PendingEntry = {
|
||||
const entry: PendingEntry<TPayload> = {
|
||||
record,
|
||||
resolve: resolvePromise!,
|
||||
reject: rejectPromise!,
|
||||
@@ -95,7 +94,7 @@ export class ExecApprovalManager {
|
||||
* @deprecated Use register() instead for explicit separation of registration and waiting.
|
||||
*/
|
||||
async waitForDecision(
|
||||
record: ExecApprovalRecord,
|
||||
record: ExecApprovalRecord<TPayload>,
|
||||
timeoutMs: number,
|
||||
): Promise<ExecApprovalDecision | null> {
|
||||
return this.register(record, timeoutMs);
|
||||
@@ -147,7 +146,7 @@ export class ExecApprovalManager {
|
||||
return true;
|
||||
}
|
||||
|
||||
getSnapshot(recordId: string): ExecApprovalRecord | null {
|
||||
getSnapshot(recordId: string): ExecApprovalRecord<TPayload> | null {
|
||||
const entry = this.pending.get(recordId);
|
||||
return entry?.record ?? null;
|
||||
}
|
||||
|
||||
@@ -75,6 +75,19 @@ describe("operator scope authorization", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it.each(["plugin.approval.request", "plugin.approval.waitDecision", "plugin.approval.resolve"])(
|
||||
"requires approvals scope for %s",
|
||||
(method) => {
|
||||
expect(authorizeOperatorScopesForMethod(method, ["operator.write"])).toEqual({
|
||||
allowed: false,
|
||||
missingScope: "operator.approvals",
|
||||
});
|
||||
expect(authorizeOperatorScopesForMethod(method, ["operator.approvals"])).toEqual({
|
||||
allowed: true,
|
||||
});
|
||||
},
|
||||
);
|
||||
|
||||
it("requires admin for unknown methods", () => {
|
||||
expect(authorizeOperatorScopesForMethod("unknown.method", ["operator.read"])).toEqual({
|
||||
allowed: false,
|
||||
@@ -83,6 +96,21 @@ describe("operator scope authorization", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("plugin approval method registration", () => {
|
||||
it("lists all plugin approval methods", () => {
|
||||
const methods = listGatewayMethods();
|
||||
expect(methods).toContain("plugin.approval.request");
|
||||
expect(methods).toContain("plugin.approval.waitDecision");
|
||||
expect(methods).toContain("plugin.approval.resolve");
|
||||
});
|
||||
|
||||
it("classifies plugin approval methods", () => {
|
||||
expect(isGatewayMethodClassified("plugin.approval.request")).toBe(true);
|
||||
expect(isGatewayMethodClassified("plugin.approval.waitDecision")).toBe(true);
|
||||
expect(isGatewayMethodClassified("plugin.approval.resolve")).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe("core gateway method classification", () => {
|
||||
it("treats node-role methods as classified even without operator scopes", () => {
|
||||
expect(isGatewayMethodClassified("node.pending.drain")).toBe(true);
|
||||
|
||||
@@ -36,6 +36,9 @@ const METHOD_SCOPE_GROUPS: Record<OperatorScope, readonly string[]> = {
|
||||
"exec.approval.request",
|
||||
"exec.approval.waitDecision",
|
||||
"exec.approval.resolve",
|
||||
"plugin.approval.request",
|
||||
"plugin.approval.waitDecision",
|
||||
"plugin.approval.resolve",
|
||||
],
|
||||
[PAIRING_SCOPE]: [
|
||||
"node.pair.request",
|
||||
|
||||
@@ -124,6 +124,10 @@ import {
|
||||
ExecApprovalRequestParamsSchema,
|
||||
type ExecApprovalResolveParams,
|
||||
ExecApprovalResolveParamsSchema,
|
||||
type PluginApprovalRequestParams,
|
||||
PluginApprovalRequestParamsSchema,
|
||||
type PluginApprovalResolveParams,
|
||||
PluginApprovalResolveParamsSchema,
|
||||
ErrorCodes,
|
||||
type ErrorShape,
|
||||
ErrorShapeSchema,
|
||||
@@ -437,6 +441,12 @@ export const validateExecApprovalRequestParams = ajv.compile<ExecApprovalRequest
|
||||
export const validateExecApprovalResolveParams = ajv.compile<ExecApprovalResolveParams>(
|
||||
ExecApprovalResolveParamsSchema,
|
||||
);
|
||||
export const validatePluginApprovalRequestParams = ajv.compile<PluginApprovalRequestParams>(
|
||||
PluginApprovalRequestParamsSchema,
|
||||
);
|
||||
export const validatePluginApprovalResolveParams = ajv.compile<PluginApprovalResolveParams>(
|
||||
PluginApprovalResolveParamsSchema,
|
||||
);
|
||||
export const validateExecApprovalsNodeGetParams = ajv.compile<ExecApprovalsNodeGetParams>(
|
||||
ExecApprovalsNodeGetParamsSchema,
|
||||
);
|
||||
|
||||
@@ -15,4 +15,5 @@ export * from "./schema/secrets.js";
|
||||
export * from "./schema/sessions.js";
|
||||
export * from "./schema/snapshot.js";
|
||||
export * from "./schema/types.js";
|
||||
export * from "./schema/plugin-approvals.js";
|
||||
export * from "./schema/wizard.js";
|
||||
|
||||
@@ -5,6 +5,7 @@ export const ErrorCodes = {
|
||||
NOT_PAIRED: "NOT_PAIRED",
|
||||
AGENT_TIMEOUT: "AGENT_TIMEOUT",
|
||||
INVALID_REQUEST: "INVALID_REQUEST",
|
||||
APPROVAL_NOT_FOUND: "APPROVAL_NOT_FOUND",
|
||||
UNAVAILABLE: "UNAVAILABLE",
|
||||
} as const;
|
||||
|
||||
|
||||
35
src/gateway/protocol/schema/plugin-approvals.ts
Normal file
35
src/gateway/protocol/schema/plugin-approvals.ts
Normal file
@@ -0,0 +1,35 @@
|
||||
import { Type } from "@sinclair/typebox";
|
||||
import {
|
||||
MAX_PLUGIN_APPROVAL_TIMEOUT_MS,
|
||||
PLUGIN_APPROVAL_DESCRIPTION_MAX_LENGTH,
|
||||
PLUGIN_APPROVAL_TITLE_MAX_LENGTH,
|
||||
} from "../../../infra/plugin-approvals.js";
|
||||
import { NonEmptyString } from "./primitives.js";
|
||||
|
||||
export const PluginApprovalRequestParamsSchema = Type.Object(
|
||||
{
|
||||
pluginId: Type.Optional(NonEmptyString),
|
||||
title: Type.String({ minLength: 1, maxLength: PLUGIN_APPROVAL_TITLE_MAX_LENGTH }),
|
||||
description: Type.String({ minLength: 1, maxLength: PLUGIN_APPROVAL_DESCRIPTION_MAX_LENGTH }),
|
||||
severity: Type.Optional(Type.String({ enum: ["info", "warning", "critical"] })),
|
||||
toolName: Type.Optional(Type.String()),
|
||||
toolCallId: Type.Optional(Type.String()),
|
||||
agentId: Type.Optional(Type.String()),
|
||||
sessionKey: Type.Optional(Type.String()),
|
||||
turnSourceChannel: Type.Optional(Type.String()),
|
||||
turnSourceTo: Type.Optional(Type.String()),
|
||||
turnSourceAccountId: Type.Optional(Type.String()),
|
||||
turnSourceThreadId: Type.Optional(Type.Union([Type.String(), Type.Number()])),
|
||||
timeoutMs: Type.Optional(Type.Integer({ minimum: 1, maximum: MAX_PLUGIN_APPROVAL_TIMEOUT_MS })),
|
||||
twoPhase: Type.Optional(Type.Boolean()),
|
||||
},
|
||||
{ additionalProperties: false },
|
||||
);
|
||||
|
||||
export const PluginApprovalResolveParamsSchema = Type.Object(
|
||||
{
|
||||
id: NonEmptyString,
|
||||
decision: NonEmptyString,
|
||||
},
|
||||
{ additionalProperties: false },
|
||||
);
|
||||
@@ -136,6 +136,10 @@ import {
|
||||
NodePairVerifyParamsSchema,
|
||||
NodeRenameParamsSchema,
|
||||
} from "./nodes.js";
|
||||
import {
|
||||
PluginApprovalRequestParamsSchema,
|
||||
PluginApprovalResolveParamsSchema,
|
||||
} from "./plugin-approvals.js";
|
||||
import { PushTestParamsSchema, PushTestResultSchema } from "./push.js";
|
||||
import {
|
||||
SecretsReloadParamsSchema,
|
||||
@@ -302,6 +306,8 @@ export const ProtocolSchemas = {
|
||||
ExecApprovalsSnapshot: ExecApprovalsSnapshotSchema,
|
||||
ExecApprovalRequestParams: ExecApprovalRequestParamsSchema,
|
||||
ExecApprovalResolveParams: ExecApprovalResolveParamsSchema,
|
||||
PluginApprovalRequestParams: PluginApprovalRequestParamsSchema,
|
||||
PluginApprovalResolveParams: PluginApprovalResolveParamsSchema,
|
||||
DevicePairListParams: DevicePairListParamsSchema,
|
||||
DevicePairApproveParams: DevicePairApproveParamsSchema,
|
||||
DevicePairRejectParams: DevicePairRejectParamsSchema,
|
||||
|
||||
@@ -128,6 +128,8 @@ export type ExecApprovalsNodeSetParams = SchemaType<"ExecApprovalsNodeSetParams"
|
||||
export type ExecApprovalsSnapshot = SchemaType<"ExecApprovalsSnapshot">;
|
||||
export type ExecApprovalRequestParams = SchemaType<"ExecApprovalRequestParams">;
|
||||
export type ExecApprovalResolveParams = SchemaType<"ExecApprovalResolveParams">;
|
||||
export type PluginApprovalRequestParams = SchemaType<"PluginApprovalRequestParams">;
|
||||
export type PluginApprovalResolveParams = SchemaType<"PluginApprovalResolveParams">;
|
||||
export type DevicePairListParams = SchemaType<"DevicePairListParams">;
|
||||
export type DevicePairApproveParams = SchemaType<"DevicePairApproveParams">;
|
||||
export type DevicePairRejectParams = SchemaType<"DevicePairRejectParams">;
|
||||
|
||||
@@ -12,6 +12,8 @@ import { logWs, shouldLogWs, summarizeAgentEventForWsLog } from "./ws-log.js";
|
||||
const EVENT_SCOPE_GUARDS: Record<string, string[]> = {
|
||||
"exec.approval.requested": [APPROVALS_SCOPE],
|
||||
"exec.approval.resolved": [APPROVALS_SCOPE],
|
||||
"plugin.approval.requested": [APPROVALS_SCOPE],
|
||||
"plugin.approval.resolved": [APPROVALS_SCOPE],
|
||||
"device.pair.requested": [PAIRING_SCOPE],
|
||||
"device.pair.resolved": [PAIRING_SCOPE],
|
||||
"node.pair.requested": [PAIRING_SCOPE],
|
||||
|
||||
@@ -29,6 +29,9 @@ const BASE_METHODS = [
|
||||
"exec.approval.request",
|
||||
"exec.approval.waitDecision",
|
||||
"exec.approval.resolve",
|
||||
"plugin.approval.request",
|
||||
"plugin.approval.waitDecision",
|
||||
"plugin.approval.resolve",
|
||||
"wizard.start",
|
||||
"wizard.next",
|
||||
"wizard.cancel",
|
||||
@@ -140,5 +143,7 @@ export const GATEWAY_EVENTS = [
|
||||
"voicewake.changed",
|
||||
"exec.approval.requested",
|
||||
"exec.approval.resolved",
|
||||
"plugin.approval.requested",
|
||||
"plugin.approval.resolved",
|
||||
GATEWAY_EVENT_UPDATE_AVAILABLE,
|
||||
];
|
||||
|
||||
@@ -19,6 +19,10 @@ import {
|
||||
} from "../protocol/index.js";
|
||||
import type { GatewayRequestHandlers } from "./types.js";
|
||||
|
||||
const APPROVAL_NOT_FOUND_DETAILS = {
|
||||
reason: ErrorCodes.APPROVAL_NOT_FOUND,
|
||||
} as const;
|
||||
|
||||
export function createExecApprovalHandlers(
|
||||
manager: ExecApprovalManager,
|
||||
opts?: { forwarder?: ExecApprovalForwarder },
|
||||
@@ -183,7 +187,7 @@ export function createExecApprovalHandlers(
|
||||
},
|
||||
{ dropIfSlow: true },
|
||||
);
|
||||
const hasExecApprovalClients = context.hasExecApprovalClients?.() ?? false;
|
||||
const hasExecApprovalClients = context.hasExecApprovalClients?.(client?.connId) ?? false;
|
||||
let forwarded = false;
|
||||
if (opts?.forwarder) {
|
||||
try {
|
||||
@@ -297,7 +301,9 @@ export function createExecApprovalHandlers(
|
||||
respond(
|
||||
false,
|
||||
undefined,
|
||||
errorShape(ErrorCodes.INVALID_REQUEST, "unknown or expired approval id"),
|
||||
errorShape(ErrorCodes.INVALID_REQUEST, "unknown or expired approval id", {
|
||||
details: APPROVAL_NOT_FOUND_DETAILS,
|
||||
}),
|
||||
);
|
||||
return;
|
||||
}
|
||||
@@ -322,7 +328,9 @@ export function createExecApprovalHandlers(
|
||||
respond(
|
||||
false,
|
||||
undefined,
|
||||
errorShape(ErrorCodes.INVALID_REQUEST, "unknown or expired approval id"),
|
||||
errorShape(ErrorCodes.INVALID_REQUEST, "unknown or expired approval id", {
|
||||
details: APPROVAL_NOT_FOUND_DETAILS,
|
||||
}),
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
431
src/gateway/server-methods/plugin-approval.test.ts
Normal file
431
src/gateway/server-methods/plugin-approval.test.ts
Normal file
@@ -0,0 +1,431 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import type { PluginApprovalRequestPayload } from "../../infra/plugin-approvals.js";
|
||||
import { ExecApprovalManager } from "../exec-approval-manager.js";
|
||||
import { createPluginApprovalHandlers } from "./plugin-approval.js";
|
||||
import type { GatewayRequestHandlerOptions } from "./types.js";
|
||||
|
||||
function createManager() {
|
||||
return new ExecApprovalManager<PluginApprovalRequestPayload>();
|
||||
}
|
||||
|
||||
function createMockOptions(
|
||||
method: string,
|
||||
params: Record<string, unknown>,
|
||||
overrides?: Partial<GatewayRequestHandlerOptions>,
|
||||
): GatewayRequestHandlerOptions {
|
||||
return {
|
||||
req: { method, params, id: "req-1" },
|
||||
params,
|
||||
client: {
|
||||
connect: {
|
||||
client: { id: "test-client", displayName: "Test Client" },
|
||||
},
|
||||
},
|
||||
isWebchatConnect: () => false,
|
||||
respond: vi.fn(),
|
||||
context: {
|
||||
broadcast: vi.fn(),
|
||||
logGateway: { error: vi.fn(), warn: vi.fn(), info: vi.fn(), debug: vi.fn() },
|
||||
hasExecApprovalClients: () => true,
|
||||
},
|
||||
...overrides,
|
||||
} as unknown as GatewayRequestHandlerOptions;
|
||||
}
|
||||
|
||||
describe("createPluginApprovalHandlers", () => {
|
||||
let manager: ExecApprovalManager<PluginApprovalRequestPayload>;
|
||||
|
||||
beforeEach(() => {
|
||||
manager = createManager();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
it("returns handlers for all three plugin approval methods", () => {
|
||||
const handlers = createPluginApprovalHandlers(manager);
|
||||
expect(handlers).toHaveProperty("plugin.approval.request");
|
||||
expect(handlers).toHaveProperty("plugin.approval.waitDecision");
|
||||
expect(handlers).toHaveProperty("plugin.approval.resolve");
|
||||
expect(typeof handlers["plugin.approval.request"]).toBe("function");
|
||||
expect(typeof handlers["plugin.approval.waitDecision"]).toBe("function");
|
||||
expect(typeof handlers["plugin.approval.resolve"]).toBe("function");
|
||||
});
|
||||
|
||||
describe("plugin.approval.request", () => {
|
||||
it("rejects invalid params", async () => {
|
||||
const handlers = createPluginApprovalHandlers(manager);
|
||||
const opts = createMockOptions("plugin.approval.request", {});
|
||||
await handlers["plugin.approval.request"](opts);
|
||||
expect(opts.respond).toHaveBeenCalledWith(
|
||||
false,
|
||||
undefined,
|
||||
expect.objectContaining({
|
||||
code: expect.any(String),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("creates and registers approval with twoPhase", async () => {
|
||||
const handlers = createPluginApprovalHandlers(manager);
|
||||
const respond = vi.fn();
|
||||
const opts = createMockOptions(
|
||||
"plugin.approval.request",
|
||||
{
|
||||
title: "Sensitive action",
|
||||
description: "This tool modifies production data",
|
||||
severity: "warning",
|
||||
twoPhase: true,
|
||||
},
|
||||
{ respond },
|
||||
);
|
||||
|
||||
// Don't await — the handler blocks waiting for the decision.
|
||||
// Instead, let it run and resolve the approval after the accepted response.
|
||||
const handlerPromise = handlers["plugin.approval.request"](opts);
|
||||
|
||||
// Wait for the twoPhase "accepted" response
|
||||
await vi.waitFor(() => {
|
||||
expect(respond).toHaveBeenCalledWith(
|
||||
true,
|
||||
expect.objectContaining({ status: "accepted", id: expect.any(String) }),
|
||||
undefined,
|
||||
);
|
||||
});
|
||||
|
||||
expect(opts.context.broadcast).toHaveBeenCalledWith(
|
||||
"plugin.approval.requested",
|
||||
expect.objectContaining({ id: expect.any(String) }),
|
||||
{ dropIfSlow: true },
|
||||
);
|
||||
|
||||
// Resolve the approval so the handler can complete
|
||||
const acceptedCall = respond.mock.calls.find(
|
||||
(c) => (c[1] as Record<string, unknown>)?.status === "accepted",
|
||||
);
|
||||
const approvalId = (acceptedCall?.[1] as Record<string, unknown>)?.id as string;
|
||||
manager.resolve(approvalId, "allow-once");
|
||||
|
||||
await handlerPromise;
|
||||
|
||||
// Final response with decision
|
||||
expect(respond).toHaveBeenCalledWith(
|
||||
true,
|
||||
expect.objectContaining({ id: approvalId, decision: "allow-once" }),
|
||||
undefined,
|
||||
);
|
||||
});
|
||||
|
||||
it("expires immediately when no approval route", async () => {
|
||||
const handlers = createPluginApprovalHandlers(manager);
|
||||
const opts = createMockOptions(
|
||||
"plugin.approval.request",
|
||||
{
|
||||
title: "Sensitive action",
|
||||
description: "Desc",
|
||||
},
|
||||
{
|
||||
context: {
|
||||
broadcast: vi.fn(),
|
||||
logGateway: { error: vi.fn(), warn: vi.fn(), info: vi.fn(), debug: vi.fn() },
|
||||
hasExecApprovalClients: () => false,
|
||||
} as unknown as GatewayRequestHandlerOptions["context"],
|
||||
},
|
||||
);
|
||||
await handlers["plugin.approval.request"](opts);
|
||||
expect(opts.respond).toHaveBeenCalledWith(
|
||||
true,
|
||||
expect.objectContaining({ decision: null }),
|
||||
undefined,
|
||||
);
|
||||
});
|
||||
|
||||
it("passes caller connId to hasExecApprovalClients to exclude self", async () => {
|
||||
const handlers = createPluginApprovalHandlers(manager);
|
||||
const hasExecApprovalClients = vi.fn().mockReturnValue(false);
|
||||
const opts = createMockOptions(
|
||||
"plugin.approval.request",
|
||||
{ title: "T", description: "D" },
|
||||
{
|
||||
client: {
|
||||
connId: "backend-conn-42",
|
||||
connect: { client: { id: "test", displayName: "Test" } },
|
||||
} as unknown as GatewayRequestHandlerOptions["client"],
|
||||
context: {
|
||||
broadcast: vi.fn(),
|
||||
logGateway: { error: vi.fn(), warn: vi.fn(), info: vi.fn(), debug: vi.fn() },
|
||||
hasExecApprovalClients,
|
||||
} as unknown as GatewayRequestHandlerOptions["context"],
|
||||
},
|
||||
);
|
||||
await handlers["plugin.approval.request"](opts);
|
||||
expect(hasExecApprovalClients).toHaveBeenCalledWith("backend-conn-42");
|
||||
});
|
||||
|
||||
it("rejects invalid severity value", async () => {
|
||||
const handlers = createPluginApprovalHandlers(manager);
|
||||
const opts = createMockOptions("plugin.approval.request", {
|
||||
title: "T",
|
||||
description: "D",
|
||||
severity: "extreme",
|
||||
});
|
||||
await handlers["plugin.approval.request"](opts);
|
||||
expect(opts.respond).toHaveBeenCalledWith(
|
||||
false,
|
||||
undefined,
|
||||
expect.objectContaining({ code: expect.any(String) }),
|
||||
);
|
||||
});
|
||||
|
||||
it("rejects title exceeding max length", async () => {
|
||||
const handlers = createPluginApprovalHandlers(manager);
|
||||
const opts = createMockOptions("plugin.approval.request", {
|
||||
title: "x".repeat(81),
|
||||
description: "D",
|
||||
});
|
||||
await handlers["plugin.approval.request"](opts);
|
||||
expect(opts.respond).toHaveBeenCalledWith(
|
||||
false,
|
||||
undefined,
|
||||
expect.objectContaining({ code: expect.any(String) }),
|
||||
);
|
||||
});
|
||||
|
||||
it("rejects description exceeding max length", async () => {
|
||||
const handlers = createPluginApprovalHandlers(manager);
|
||||
const opts = createMockOptions("plugin.approval.request", {
|
||||
title: "T",
|
||||
description: "x".repeat(257),
|
||||
});
|
||||
await handlers["plugin.approval.request"](opts);
|
||||
expect(opts.respond).toHaveBeenCalledWith(
|
||||
false,
|
||||
undefined,
|
||||
expect.objectContaining({ code: expect.any(String) }),
|
||||
);
|
||||
});
|
||||
|
||||
it("rejects timeoutMs exceeding max", async () => {
|
||||
const handlers = createPluginApprovalHandlers(manager);
|
||||
const opts = createMockOptions("plugin.approval.request", {
|
||||
title: "T",
|
||||
description: "D",
|
||||
timeoutMs: 700_000,
|
||||
});
|
||||
await handlers["plugin.approval.request"](opts);
|
||||
expect(opts.respond).toHaveBeenCalledWith(
|
||||
false,
|
||||
undefined,
|
||||
expect.objectContaining({ code: expect.any(String) }),
|
||||
);
|
||||
});
|
||||
|
||||
it("generates plugin-prefixed IDs", async () => {
|
||||
const handlers = createPluginApprovalHandlers(manager);
|
||||
const respond = vi.fn();
|
||||
const opts = createMockOptions(
|
||||
"plugin.approval.request",
|
||||
{ title: "T", description: "D" },
|
||||
{
|
||||
respond,
|
||||
context: {
|
||||
broadcast: vi.fn(),
|
||||
logGateway: { error: vi.fn(), warn: vi.fn(), info: vi.fn(), debug: vi.fn() },
|
||||
hasExecApprovalClients: () => false,
|
||||
} as unknown as GatewayRequestHandlerOptions["context"],
|
||||
},
|
||||
);
|
||||
await handlers["plugin.approval.request"](opts);
|
||||
const result = respond.mock.calls[0]?.[1] as Record<string, unknown> | undefined;
|
||||
expect(result?.id).toEqual(expect.stringMatching(/^plugin:/));
|
||||
});
|
||||
|
||||
it("passes plugin-prefixed IDs directly to manager.create", async () => {
|
||||
const handlers = createPluginApprovalHandlers(manager);
|
||||
const createSpy = vi.spyOn(manager, "create");
|
||||
const opts = createMockOptions(
|
||||
"plugin.approval.request",
|
||||
{ title: "T", description: "D" },
|
||||
{
|
||||
context: {
|
||||
broadcast: vi.fn(),
|
||||
logGateway: { error: vi.fn(), warn: vi.fn(), info: vi.fn(), debug: vi.fn() },
|
||||
hasExecApprovalClients: () => false,
|
||||
} as unknown as GatewayRequestHandlerOptions["context"],
|
||||
},
|
||||
);
|
||||
|
||||
await handlers["plugin.approval.request"](opts);
|
||||
|
||||
expect(createSpy).toHaveBeenCalledTimes(1);
|
||||
expect(createSpy.mock.calls[0]?.[2]).toEqual(expect.stringMatching(/^plugin:/));
|
||||
});
|
||||
|
||||
it("rejects plugin-provided id field", async () => {
|
||||
const handlers = createPluginApprovalHandlers(manager);
|
||||
const opts = createMockOptions("plugin.approval.request", {
|
||||
id: "plugin-provided-id",
|
||||
title: "T",
|
||||
description: "D",
|
||||
});
|
||||
await handlers["plugin.approval.request"](opts);
|
||||
expect(opts.respond).toHaveBeenCalledWith(
|
||||
false,
|
||||
undefined,
|
||||
expect.objectContaining({ message: expect.stringContaining("unexpected property") }),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("plugin.approval.waitDecision", () => {
|
||||
it("rejects missing id", async () => {
|
||||
const handlers = createPluginApprovalHandlers(manager);
|
||||
const opts = createMockOptions("plugin.approval.waitDecision", {});
|
||||
await handlers["plugin.approval.waitDecision"](opts);
|
||||
expect(opts.respond).toHaveBeenCalledWith(
|
||||
false,
|
||||
undefined,
|
||||
expect.objectContaining({ message: expect.stringContaining("id is required") }),
|
||||
);
|
||||
});
|
||||
|
||||
it("returns not found for unknown id", async () => {
|
||||
const handlers = createPluginApprovalHandlers(manager);
|
||||
const opts = createMockOptions("plugin.approval.waitDecision", { id: "unknown" });
|
||||
await handlers["plugin.approval.waitDecision"](opts);
|
||||
expect(opts.respond).toHaveBeenCalledWith(
|
||||
false,
|
||||
undefined,
|
||||
expect.objectContaining({ message: expect.stringContaining("expired or not found") }),
|
||||
);
|
||||
});
|
||||
|
||||
it("returns decision when resolved", async () => {
|
||||
const handlers = createPluginApprovalHandlers(manager);
|
||||
const record = manager.create({ title: "T", description: "D" }, 60_000);
|
||||
void manager.register(record, 60_000);
|
||||
|
||||
// Resolve before waiting
|
||||
manager.resolve(record.id, "allow-once");
|
||||
|
||||
const opts = createMockOptions("plugin.approval.waitDecision", { id: record.id });
|
||||
await handlers["plugin.approval.waitDecision"](opts);
|
||||
expect(opts.respond).toHaveBeenCalledWith(
|
||||
true,
|
||||
expect.objectContaining({ id: record.id, decision: "allow-once" }),
|
||||
undefined,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("plugin.approval.resolve", () => {
|
||||
it("rejects invalid params", async () => {
|
||||
const handlers = createPluginApprovalHandlers(manager);
|
||||
const opts = createMockOptions("plugin.approval.resolve", {});
|
||||
await handlers["plugin.approval.resolve"](opts);
|
||||
expect(opts.respond).toHaveBeenCalledWith(
|
||||
false,
|
||||
undefined,
|
||||
expect.objectContaining({
|
||||
code: expect.any(String),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("rejects invalid decision", async () => {
|
||||
const handlers = createPluginApprovalHandlers(manager);
|
||||
const record = manager.create({ title: "T", description: "D" }, 60_000);
|
||||
void manager.register(record, 60_000);
|
||||
const opts = createMockOptions("plugin.approval.resolve", {
|
||||
id: record.id,
|
||||
decision: "invalid",
|
||||
});
|
||||
await handlers["plugin.approval.resolve"](opts);
|
||||
expect(opts.respond).toHaveBeenCalledWith(
|
||||
false,
|
||||
undefined,
|
||||
expect.objectContaining({ message: "invalid decision" }),
|
||||
);
|
||||
});
|
||||
|
||||
it("resolves a pending approval", async () => {
|
||||
const handlers = createPluginApprovalHandlers(manager);
|
||||
const record = manager.create({ title: "T", description: "D" }, 60_000);
|
||||
void manager.register(record, 60_000);
|
||||
|
||||
const opts = createMockOptions("plugin.approval.resolve", {
|
||||
id: record.id,
|
||||
decision: "deny",
|
||||
});
|
||||
await handlers["plugin.approval.resolve"](opts);
|
||||
expect(opts.respond).toHaveBeenCalledWith(true, { ok: true }, undefined);
|
||||
expect(opts.context.broadcast).toHaveBeenCalledWith(
|
||||
"plugin.approval.resolved",
|
||||
expect.objectContaining({ id: record.id, decision: "deny" }),
|
||||
{ dropIfSlow: true },
|
||||
);
|
||||
});
|
||||
|
||||
it("rejects unknown approval id", async () => {
|
||||
const handlers = createPluginApprovalHandlers(manager);
|
||||
const opts = createMockOptions("plugin.approval.resolve", {
|
||||
id: "nonexistent",
|
||||
decision: "allow-once",
|
||||
});
|
||||
await handlers["plugin.approval.resolve"](opts);
|
||||
expect(opts.respond).toHaveBeenCalledWith(
|
||||
false,
|
||||
undefined,
|
||||
expect.objectContaining({
|
||||
code: "INVALID_REQUEST",
|
||||
message: expect.stringContaining("unknown or expired"),
|
||||
details: expect.objectContaining({ reason: "APPROVAL_NOT_FOUND" }),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("requires exact id and rejects prefixes", async () => {
|
||||
const handlers = createPluginApprovalHandlers(manager);
|
||||
const record = manager.create({ title: "T", description: "D" }, 60_000, "abcdef-1234");
|
||||
void manager.register(record, 60_000);
|
||||
|
||||
const opts = createMockOptions("plugin.approval.resolve", {
|
||||
id: "abcdef",
|
||||
decision: "allow-always",
|
||||
});
|
||||
await handlers["plugin.approval.resolve"](opts);
|
||||
expect(opts.respond).toHaveBeenCalledWith(
|
||||
false,
|
||||
undefined,
|
||||
expect.objectContaining({
|
||||
code: "INVALID_REQUEST",
|
||||
message: expect.stringContaining("unknown or expired"),
|
||||
details: expect.objectContaining({ reason: "APPROVAL_NOT_FOUND" }),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("does not leak candidate ids when prefixes are ambiguous", async () => {
|
||||
const handlers = createPluginApprovalHandlers(manager);
|
||||
const recordA = manager.create({ title: "A", description: "D" }, 60_000, "plugin:abc-1111");
|
||||
const recordB = manager.create({ title: "B", description: "D" }, 60_000, "plugin:abc-2222");
|
||||
void manager.register(recordA, 60_000);
|
||||
void manager.register(recordB, 60_000);
|
||||
|
||||
const opts = createMockOptions("plugin.approval.resolve", {
|
||||
id: "plugin:abc",
|
||||
decision: "deny",
|
||||
});
|
||||
await handlers["plugin.approval.resolve"](opts);
|
||||
expect(opts.respond).toHaveBeenCalledWith(
|
||||
false,
|
||||
undefined,
|
||||
expect.objectContaining({
|
||||
code: "INVALID_REQUEST",
|
||||
message: "unknown or expired approval id",
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
258
src/gateway/server-methods/plugin-approval.ts
Normal file
258
src/gateway/server-methods/plugin-approval.ts
Normal file
@@ -0,0 +1,258 @@
|
||||
import { randomUUID } from "node:crypto";
|
||||
import type { ExecApprovalForwarder } from "../../infra/exec-approval-forwarder.js";
|
||||
import type { ExecApprovalDecision } from "../../infra/exec-approvals.js";
|
||||
import type { PluginApprovalRequestPayload } from "../../infra/plugin-approvals.js";
|
||||
import {
|
||||
DEFAULT_PLUGIN_APPROVAL_TIMEOUT_MS,
|
||||
MAX_PLUGIN_APPROVAL_TIMEOUT_MS,
|
||||
} from "../../infra/plugin-approvals.js";
|
||||
import type { ExecApprovalManager } from "../exec-approval-manager.js";
|
||||
import {
|
||||
ErrorCodes,
|
||||
errorShape,
|
||||
formatValidationErrors,
|
||||
validatePluginApprovalRequestParams,
|
||||
validatePluginApprovalResolveParams,
|
||||
} from "../protocol/index.js";
|
||||
import type { GatewayRequestHandlers } from "./types.js";
|
||||
|
||||
const APPROVAL_NOT_FOUND_DETAILS = {
|
||||
reason: ErrorCodes.APPROVAL_NOT_FOUND,
|
||||
} as const;
|
||||
|
||||
export function createPluginApprovalHandlers(
|
||||
manager: ExecApprovalManager<PluginApprovalRequestPayload>,
|
||||
opts?: { forwarder?: ExecApprovalForwarder },
|
||||
): GatewayRequestHandlers {
|
||||
return {
|
||||
"plugin.approval.request": async ({ params, client, respond, context }) => {
|
||||
if (!validatePluginApprovalRequestParams(params)) {
|
||||
respond(
|
||||
false,
|
||||
undefined,
|
||||
errorShape(
|
||||
ErrorCodes.INVALID_REQUEST,
|
||||
`invalid plugin.approval.request params: ${formatValidationErrors(
|
||||
validatePluginApprovalRequestParams.errors,
|
||||
)}`,
|
||||
),
|
||||
);
|
||||
return;
|
||||
}
|
||||
const p = params as {
|
||||
pluginId?: string | null;
|
||||
title: string;
|
||||
description: string;
|
||||
severity?: string | null;
|
||||
toolName?: string | null;
|
||||
toolCallId?: string | null;
|
||||
agentId?: string | null;
|
||||
sessionKey?: string | null;
|
||||
turnSourceChannel?: string | null;
|
||||
turnSourceTo?: string | null;
|
||||
turnSourceAccountId?: string | null;
|
||||
turnSourceThreadId?: string | number | null;
|
||||
timeoutMs?: number;
|
||||
twoPhase?: boolean;
|
||||
};
|
||||
const twoPhase = p.twoPhase === true;
|
||||
const timeoutMs = Math.min(
|
||||
typeof p.timeoutMs === "number" ? p.timeoutMs : DEFAULT_PLUGIN_APPROVAL_TIMEOUT_MS,
|
||||
MAX_PLUGIN_APPROVAL_TIMEOUT_MS,
|
||||
);
|
||||
|
||||
const normalizeTrimmedString = (value?: string | null): string | null =>
|
||||
value?.trim() || null;
|
||||
|
||||
const request: PluginApprovalRequestPayload = {
|
||||
pluginId: p.pluginId ?? null,
|
||||
title: p.title,
|
||||
description: p.description,
|
||||
severity: (p.severity as PluginApprovalRequestPayload["severity"]) ?? null,
|
||||
toolName: p.toolName ?? null,
|
||||
toolCallId: p.toolCallId ?? null,
|
||||
agentId: p.agentId ?? null,
|
||||
sessionKey: p.sessionKey ?? null,
|
||||
turnSourceChannel: normalizeTrimmedString(p.turnSourceChannel),
|
||||
turnSourceTo: normalizeTrimmedString(p.turnSourceTo),
|
||||
turnSourceAccountId: normalizeTrimmedString(p.turnSourceAccountId),
|
||||
turnSourceThreadId: p.turnSourceThreadId ?? null,
|
||||
};
|
||||
|
||||
// Always server-generate the ID — never accept plugin-provided IDs.
|
||||
// Kind-prefix so /approve routing can distinguish plugin vs exec IDs deterministically.
|
||||
const record = manager.create(request, timeoutMs, `plugin:${randomUUID()}`);
|
||||
|
||||
let decisionPromise: Promise<ExecApprovalDecision | null>;
|
||||
try {
|
||||
decisionPromise = manager.register(record, timeoutMs);
|
||||
} catch (err) {
|
||||
respond(
|
||||
false,
|
||||
undefined,
|
||||
errorShape(ErrorCodes.INVALID_REQUEST, `registration failed: ${String(err)}`),
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
context.broadcast(
|
||||
"plugin.approval.requested",
|
||||
{
|
||||
id: record.id,
|
||||
request: record.request,
|
||||
createdAtMs: record.createdAtMs,
|
||||
expiresAtMs: record.expiresAtMs,
|
||||
},
|
||||
{ dropIfSlow: true },
|
||||
);
|
||||
|
||||
let forwarded = false;
|
||||
if (opts?.forwarder?.handlePluginApprovalRequested) {
|
||||
try {
|
||||
forwarded = await opts.forwarder.handlePluginApprovalRequested({
|
||||
id: record.id,
|
||||
request: record.request,
|
||||
createdAtMs: record.createdAtMs,
|
||||
expiresAtMs: record.expiresAtMs,
|
||||
});
|
||||
} catch (err) {
|
||||
context.logGateway?.error?.(`plugin approvals: forward request failed: ${String(err)}`);
|
||||
}
|
||||
}
|
||||
|
||||
const hasApprovalClients = context.hasExecApprovalClients?.(client?.connId) ?? false;
|
||||
if (!hasApprovalClients && !forwarded) {
|
||||
manager.expire(record.id, "no-approval-route");
|
||||
respond(
|
||||
true,
|
||||
{
|
||||
id: record.id,
|
||||
decision: null,
|
||||
createdAtMs: record.createdAtMs,
|
||||
expiresAtMs: record.expiresAtMs,
|
||||
},
|
||||
undefined,
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
if (twoPhase) {
|
||||
respond(
|
||||
true,
|
||||
{
|
||||
status: "accepted",
|
||||
id: record.id,
|
||||
createdAtMs: record.createdAtMs,
|
||||
expiresAtMs: record.expiresAtMs,
|
||||
},
|
||||
undefined,
|
||||
);
|
||||
}
|
||||
|
||||
const decision = await decisionPromise;
|
||||
respond(
|
||||
true,
|
||||
{
|
||||
id: record.id,
|
||||
decision,
|
||||
createdAtMs: record.createdAtMs,
|
||||
expiresAtMs: record.expiresAtMs,
|
||||
},
|
||||
undefined,
|
||||
);
|
||||
},
|
||||
|
||||
"plugin.approval.waitDecision": async ({ params, respond }) => {
|
||||
const p = params as { id?: string };
|
||||
const id = typeof p.id === "string" ? p.id.trim() : "";
|
||||
if (!id) {
|
||||
respond(false, undefined, errorShape(ErrorCodes.INVALID_REQUEST, "id is required"));
|
||||
return;
|
||||
}
|
||||
const decisionPromise = manager.awaitDecision(id);
|
||||
if (!decisionPromise) {
|
||||
respond(
|
||||
false,
|
||||
undefined,
|
||||
errorShape(ErrorCodes.INVALID_REQUEST, "approval expired or not found"),
|
||||
);
|
||||
return;
|
||||
}
|
||||
const snapshot = manager.getSnapshot(id);
|
||||
const decision = await decisionPromise;
|
||||
respond(
|
||||
true,
|
||||
{
|
||||
id,
|
||||
decision,
|
||||
createdAtMs: snapshot?.createdAtMs,
|
||||
expiresAtMs: snapshot?.expiresAtMs,
|
||||
},
|
||||
undefined,
|
||||
);
|
||||
},
|
||||
|
||||
"plugin.approval.resolve": async ({ params, respond, client, context }) => {
|
||||
if (!validatePluginApprovalResolveParams(params)) {
|
||||
respond(
|
||||
false,
|
||||
undefined,
|
||||
errorShape(
|
||||
ErrorCodes.INVALID_REQUEST,
|
||||
`invalid plugin.approval.resolve params: ${formatValidationErrors(
|
||||
validatePluginApprovalResolveParams.errors,
|
||||
)}`,
|
||||
),
|
||||
);
|
||||
return;
|
||||
}
|
||||
const p = params as { id: string; decision: string };
|
||||
const decision = p.decision as ExecApprovalDecision;
|
||||
if (decision !== "allow-once" && decision !== "allow-always" && decision !== "deny") {
|
||||
respond(false, undefined, errorShape(ErrorCodes.INVALID_REQUEST, "invalid decision"));
|
||||
return;
|
||||
}
|
||||
const approvalId = p.id.trim();
|
||||
const snapshot = manager.getSnapshot(approvalId);
|
||||
if (!snapshot || snapshot.resolvedAtMs !== undefined) {
|
||||
respond(
|
||||
false,
|
||||
undefined,
|
||||
errorShape(ErrorCodes.INVALID_REQUEST, "unknown or expired approval id", {
|
||||
details: APPROVAL_NOT_FOUND_DETAILS,
|
||||
}),
|
||||
);
|
||||
return;
|
||||
}
|
||||
const resolvedBy = client?.connect?.client?.displayName ?? client?.connect?.client?.id;
|
||||
const ok = manager.resolve(approvalId, decision, resolvedBy ?? null);
|
||||
if (!ok) {
|
||||
respond(
|
||||
false,
|
||||
undefined,
|
||||
errorShape(ErrorCodes.INVALID_REQUEST, "unknown or expired approval id", {
|
||||
details: APPROVAL_NOT_FOUND_DETAILS,
|
||||
}),
|
||||
);
|
||||
return;
|
||||
}
|
||||
context.broadcast(
|
||||
"plugin.approval.resolved",
|
||||
{ id: approvalId, decision, resolvedBy, ts: Date.now(), request: snapshot?.request },
|
||||
{ dropIfSlow: true },
|
||||
);
|
||||
void opts?.forwarder
|
||||
?.handlePluginApprovalResolved?.({
|
||||
id: approvalId,
|
||||
decision,
|
||||
resolvedBy,
|
||||
ts: Date.now(),
|
||||
request: snapshot?.request,
|
||||
})
|
||||
.catch((err) => {
|
||||
context.logGateway?.error?.(`plugin approvals: forward resolve failed: ${String(err)}`);
|
||||
});
|
||||
respond(true, { ok: true }, undefined);
|
||||
},
|
||||
};
|
||||
}
|
||||
@@ -868,7 +868,9 @@ describe("exec approval handlers", () => {
|
||||
false,
|
||||
undefined,
|
||||
expect.objectContaining({
|
||||
code: "INVALID_REQUEST",
|
||||
message: "unknown or expired approval id",
|
||||
details: expect.objectContaining({ reason: "APPROVAL_NOT_FOUND" }),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
@@ -2,6 +2,7 @@ import type { ModelCatalogEntry } from "../../agents/model-catalog.js";
|
||||
import type { createDefaultDeps } from "../../cli/deps.js";
|
||||
import type { HealthSummary } from "../../commands/health.js";
|
||||
import type { CronService } from "../../cron/service.js";
|
||||
import type { PluginApprovalRequestPayload } from "../../infra/plugin-approvals.js";
|
||||
import type { createSubsystemLogger } from "../../logging/subsystem.js";
|
||||
import type { WizardSession } from "../../wizard/session.js";
|
||||
import type { ChatAbortControllerEntry } from "../chat-abort.js";
|
||||
@@ -39,6 +40,7 @@ export type GatewayRequestContext = {
|
||||
cron: CronService;
|
||||
cronStorePath: string;
|
||||
execApprovalManager?: ExecApprovalManager;
|
||||
pluginApprovalManager?: ExecApprovalManager<PluginApprovalRequestPayload>;
|
||||
loadGatewayModelCatalog: () => Promise<ModelCatalogEntry[]>;
|
||||
getHealthCache: () => HealthSummary | null;
|
||||
refreshHealthSnapshot: (opts?: { probe?: boolean }) => Promise<HealthSummary>;
|
||||
@@ -54,7 +56,7 @@ export type GatewayRequestContext = {
|
||||
nodeUnsubscribe: (nodeId: string, sessionKey: string) => void;
|
||||
nodeUnsubscribeAll: (nodeId: string) => void;
|
||||
hasConnectedMobileNode: () => boolean;
|
||||
hasExecApprovalClients?: () => boolean;
|
||||
hasExecApprovalClients?: (excludeConnId?: string) => boolean;
|
||||
nodeRegistry: NodeRegistry;
|
||||
agentRunSeq: Map<string, number>;
|
||||
chatAbortControllers: Map<string, ChatAbortControllerEntry>;
|
||||
|
||||
@@ -99,6 +99,7 @@ import { GATEWAY_EVENTS, listGatewayMethods } from "./server-methods-list.js";
|
||||
import { coreGatewayHandlers } from "./server-methods.js";
|
||||
import { createExecApprovalHandlers } from "./server-methods/exec-approval.js";
|
||||
import { safeParseJson } from "./server-methods/nodes.helpers.js";
|
||||
import { createPluginApprovalHandlers } from "./server-methods/plugin-approval.js";
|
||||
import { createSecretsHandlers } from "./server-methods/secrets.js";
|
||||
import { hasConnectedMobileNode } from "./server-mobile-nodes.js";
|
||||
import { loadGatewayModelCatalog } from "./server-model-catalog.js";
|
||||
@@ -1127,6 +1128,12 @@ export async function startGatewayServer(
|
||||
const execApprovalHandlers = createExecApprovalHandlers(execApprovalManager, {
|
||||
forwarder: execApprovalForwarder,
|
||||
});
|
||||
const pluginApprovalManager = new ExecApprovalManager<
|
||||
import("../infra/plugin-approvals.js").PluginApprovalRequestPayload
|
||||
>();
|
||||
const pluginApprovalHandlers = createPluginApprovalHandlers(pluginApprovalManager, {
|
||||
forwarder: execApprovalForwarder,
|
||||
});
|
||||
const secretsHandlers = createSecretsHandlers({
|
||||
reloadSecrets: async () => {
|
||||
const active = getActiveSecretsRuntimeSnapshot();
|
||||
@@ -1159,6 +1166,7 @@ export async function startGatewayServer(
|
||||
cron,
|
||||
cronStorePath,
|
||||
execApprovalManager,
|
||||
pluginApprovalManager,
|
||||
loadGatewayModelCatalog,
|
||||
getHealthCache,
|
||||
refreshHealthSnapshot: refreshGatewayHealthSnapshot,
|
||||
@@ -1174,8 +1182,11 @@ export async function startGatewayServer(
|
||||
nodeUnsubscribe,
|
||||
nodeUnsubscribeAll,
|
||||
hasConnectedMobileNode: hasMobileNodeConnected,
|
||||
hasExecApprovalClients: () => {
|
||||
hasExecApprovalClients: (excludeConnId?: string) => {
|
||||
for (const gatewayClient of clients) {
|
||||
if (excludeConnId && gatewayClient.connId === excludeConnId) {
|
||||
continue;
|
||||
}
|
||||
const scopes = Array.isArray(gatewayClient.connect.scopes)
|
||||
? gatewayClient.connect.scopes
|
||||
: [];
|
||||
@@ -1240,6 +1251,7 @@ export async function startGatewayServer(
|
||||
extraHandlers: {
|
||||
...pluginRegistry.gatewayHandlers,
|
||||
...execApprovalHandlers,
|
||||
...pluginApprovalHandlers,
|
||||
...secretsHandlers,
|
||||
},
|
||||
broadcast,
|
||||
|
||||
@@ -17,12 +17,16 @@ import {
|
||||
} from "../utils/message-channel.js";
|
||||
import { resolveExecApprovalCommandDisplay } from "./exec-approval-command-display.js";
|
||||
import { resolveExecApprovalSessionTarget } from "./exec-approval-session-target.js";
|
||||
import type {
|
||||
ExecApprovalDecision,
|
||||
ExecApprovalRequest,
|
||||
ExecApprovalResolved,
|
||||
} from "./exec-approvals.js";
|
||||
import type { ExecApprovalRequest, ExecApprovalResolved } from "./exec-approvals.js";
|
||||
import { deliverOutboundPayloads } from "./outbound/deliver.js";
|
||||
import {
|
||||
approvalDecisionLabel,
|
||||
buildPluginApprovalExpiredMessage,
|
||||
buildPluginApprovalRequestMessage,
|
||||
buildPluginApprovalResolvedMessage,
|
||||
type PluginApprovalRequest,
|
||||
type PluginApprovalResolved,
|
||||
} from "./plugin-approvals.js";
|
||||
|
||||
const log = createSubsystemLogger("gateway/exec-approvals");
|
||||
export type { ExecApprovalRequest, ExecApprovalResolved };
|
||||
@@ -38,6 +42,8 @@ type PendingApproval = {
|
||||
export type ExecApprovalForwarder = {
|
||||
handleRequested: (request: ExecApprovalRequest) => Promise<boolean>;
|
||||
handleResolved: (resolved: ExecApprovalResolved) => Promise<void>;
|
||||
handlePluginApprovalRequested?: (request: PluginApprovalRequest) => Promise<boolean>;
|
||||
handlePluginApprovalResolved?: (resolved: PluginApprovalResolved) => Promise<void>;
|
||||
stop: () => void;
|
||||
};
|
||||
|
||||
@@ -182,15 +188,7 @@ function buildRequestMessage(request: ExecApprovalRequest, nowMs: number) {
|
||||
return lines.join("\n");
|
||||
}
|
||||
|
||||
function decisionLabel(decision: ExecApprovalDecision): string {
|
||||
if (decision === "allow-once") {
|
||||
return "allowed once";
|
||||
}
|
||||
if (decision === "allow-always") {
|
||||
return "allowed always";
|
||||
}
|
||||
return "denied";
|
||||
}
|
||||
const decisionLabel = approvalDecisionLabel;
|
||||
|
||||
function buildResolvedMessage(resolved: ExecApprovalResolved) {
|
||||
const base = `✅ Exec approval ${decisionLabel(resolved.decision)}.`;
|
||||
@@ -475,7 +473,186 @@ export function createExecApprovalForwarder(
|
||||
pending.clear();
|
||||
};
|
||||
|
||||
return { handleRequested, handleResolved, stop };
|
||||
const toSyntheticExecRequestFromPlugin = (params: {
|
||||
id: string;
|
||||
request: PluginApprovalRequest["request"];
|
||||
createdAtMs: number;
|
||||
expiresAtMs: number;
|
||||
}): ExecApprovalRequest => ({
|
||||
id: params.id,
|
||||
request: {
|
||||
command: params.request.title,
|
||||
agentId: params.request.agentId ?? null,
|
||||
sessionKey: params.request.sessionKey ?? null,
|
||||
turnSourceChannel: params.request.turnSourceChannel ?? null,
|
||||
turnSourceTo: params.request.turnSourceTo ?? null,
|
||||
turnSourceAccountId: params.request.turnSourceAccountId ?? null,
|
||||
turnSourceThreadId: params.request.turnSourceThreadId ?? null,
|
||||
},
|
||||
createdAtMs: params.createdAtMs,
|
||||
expiresAtMs: params.expiresAtMs,
|
||||
});
|
||||
|
||||
const pluginPending = new Map<string, PendingApproval>();
|
||||
|
||||
const handlePluginApprovalRequested = async (
|
||||
request: PluginApprovalRequest,
|
||||
): Promise<boolean> => {
|
||||
const cfg = getConfig();
|
||||
const config = cfg.approvals?.plugin;
|
||||
const syntheticExecRequest = toSyntheticExecRequestFromPlugin({
|
||||
id: request.id,
|
||||
request: request.request,
|
||||
createdAtMs: request.createdAtMs,
|
||||
expiresAtMs: request.expiresAtMs,
|
||||
});
|
||||
|
||||
const filteredTargets = [
|
||||
...(shouldForward({ config, request: syntheticExecRequest })
|
||||
? resolveForwardTargets({
|
||||
cfg,
|
||||
config,
|
||||
request: syntheticExecRequest,
|
||||
resolveSessionTarget,
|
||||
})
|
||||
: []),
|
||||
].filter(
|
||||
(target) => !shouldSkipForwardingFallback({ target, cfg, request: syntheticExecRequest }),
|
||||
);
|
||||
|
||||
if (filteredTargets.length === 0) {
|
||||
return false;
|
||||
}
|
||||
|
||||
const expiresInMs = Math.max(0, request.expiresAtMs - nowMs());
|
||||
const timeoutId = setTimeout(() => {
|
||||
void (async () => {
|
||||
const entry = pluginPending.get(request.id);
|
||||
if (!entry) {
|
||||
return;
|
||||
}
|
||||
pluginPending.delete(request.id);
|
||||
const expiredText = buildPluginApprovalExpiredMessage(request);
|
||||
await deliverToTargets({
|
||||
cfg,
|
||||
targets: entry.targets,
|
||||
buildPayload: () => ({ text: expiredText }),
|
||||
deliver,
|
||||
});
|
||||
})();
|
||||
}, expiresInMs);
|
||||
timeoutId.unref?.();
|
||||
|
||||
const pendingEntry: PendingApproval = {
|
||||
request: syntheticExecRequest,
|
||||
targets: filteredTargets,
|
||||
timeoutId,
|
||||
};
|
||||
pluginPending.set(request.id, pendingEntry);
|
||||
|
||||
void deliverToTargets({
|
||||
cfg,
|
||||
targets: filteredTargets,
|
||||
buildPayload: (target) => {
|
||||
const channel = normalizeMessageChannel(target.channel) ?? target.channel;
|
||||
const adapterPayload = channel
|
||||
? getChannelPlugin(channel)?.execApprovals?.buildPluginPendingPayload?.({
|
||||
cfg,
|
||||
request,
|
||||
target,
|
||||
nowMs: nowMs(),
|
||||
})
|
||||
: null;
|
||||
return adapterPayload ?? { text: buildPluginApprovalRequestMessage(request, nowMs()) };
|
||||
},
|
||||
beforeDeliver: async (target, payload) => {
|
||||
const channel = normalizeMessageChannel(target.channel) ?? target.channel;
|
||||
if (!channel) {
|
||||
return;
|
||||
}
|
||||
await getChannelPlugin(channel)?.execApprovals?.beforeDeliverPending?.({
|
||||
cfg,
|
||||
target,
|
||||
payload,
|
||||
});
|
||||
},
|
||||
deliver,
|
||||
shouldSend: () => pluginPending.get(request.id) === pendingEntry,
|
||||
}).catch((err) => {
|
||||
log.error(`plugin approvals: failed to deliver request ${request.id}: ${String(err)}`);
|
||||
});
|
||||
return true;
|
||||
};
|
||||
|
||||
const handlePluginApprovalResolved = async (resolved: PluginApprovalResolved) => {
|
||||
const cfg = getConfig();
|
||||
const entry = pluginPending.get(resolved.id);
|
||||
if (entry) {
|
||||
if (entry.timeoutId) {
|
||||
clearTimeout(entry.timeoutId);
|
||||
}
|
||||
pluginPending.delete(resolved.id);
|
||||
}
|
||||
let targets = entry?.targets;
|
||||
if (!targets && resolved.request) {
|
||||
const syntheticExecRequest = toSyntheticExecRequestFromPlugin({
|
||||
id: resolved.id,
|
||||
request: resolved.request,
|
||||
createdAtMs: resolved.ts,
|
||||
expiresAtMs: resolved.ts,
|
||||
});
|
||||
const config = cfg.approvals?.plugin;
|
||||
targets = [
|
||||
...(shouldForward({ config, request: syntheticExecRequest })
|
||||
? resolveForwardTargets({
|
||||
cfg,
|
||||
config,
|
||||
request: syntheticExecRequest,
|
||||
resolveSessionTarget,
|
||||
})
|
||||
: []),
|
||||
].filter(
|
||||
(target) => !shouldSkipForwardingFallback({ target, cfg, request: syntheticExecRequest }),
|
||||
);
|
||||
}
|
||||
if (!targets || targets.length === 0) {
|
||||
return;
|
||||
}
|
||||
await deliverToTargets({
|
||||
cfg,
|
||||
targets,
|
||||
buildPayload: (target) => {
|
||||
const channel = normalizeMessageChannel(target.channel) ?? target.channel;
|
||||
const adapterPayload = channel
|
||||
? getChannelPlugin(channel)?.execApprovals?.buildPluginResolvedPayload?.({
|
||||
cfg,
|
||||
resolved,
|
||||
target,
|
||||
})
|
||||
: null;
|
||||
return adapterPayload ?? { text: buildPluginApprovalResolvedMessage(resolved) };
|
||||
},
|
||||
deliver,
|
||||
});
|
||||
};
|
||||
|
||||
const stopAll = () => {
|
||||
stop();
|
||||
for (const entry of pluginPending.values()) {
|
||||
if (entry.timeoutId) {
|
||||
clearTimeout(entry.timeoutId);
|
||||
}
|
||||
}
|
||||
pluginPending.clear();
|
||||
};
|
||||
|
||||
return {
|
||||
handleRequested,
|
||||
handleResolved,
|
||||
handlePluginApprovalRequested,
|
||||
handlePluginApprovalResolved,
|
||||
stop: stopAll,
|
||||
};
|
||||
}
|
||||
|
||||
export function shouldForwardExecApproval(params: {
|
||||
|
||||
334
src/infra/plugin-approval-forwarder.test.ts
Normal file
334
src/infra/plugin-approval-forwarder.test.ts
Normal file
@@ -0,0 +1,334 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import type { ChannelPlugin } from "../channels/plugins/types.js";
|
||||
import type { OpenClawConfig } from "../config/config.js";
|
||||
import { setActivePluginRegistry } from "../plugins/runtime.js";
|
||||
import { createChannelTestPluginBase, createTestRegistry } from "../test-utils/channel-plugins.js";
|
||||
import { createExecApprovalForwarder } from "./exec-approval-forwarder.js";
|
||||
import type { PluginApprovalRequest, PluginApprovalResolved } from "./plugin-approvals.js";
|
||||
|
||||
afterEach(() => {
|
||||
vi.useRealTimers();
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
const emptyRegistry = createTestRegistry([]);
|
||||
|
||||
const PLUGIN_TARGETS_CFG = {
|
||||
approvals: {
|
||||
plugin: {
|
||||
enabled: true,
|
||||
mode: "targets",
|
||||
targets: [{ channel: "slack", to: "U123" }],
|
||||
},
|
||||
},
|
||||
} as OpenClawConfig;
|
||||
|
||||
const PLUGIN_DISABLED_CFG = {
|
||||
approvals: {
|
||||
plugin: {
|
||||
enabled: false,
|
||||
},
|
||||
},
|
||||
} as OpenClawConfig;
|
||||
|
||||
function createForwarder(params: { cfg: OpenClawConfig; deliver?: ReturnType<typeof vi.fn> }) {
|
||||
const deliver = params.deliver ?? vi.fn().mockResolvedValue([]);
|
||||
const forwarder = createExecApprovalForwarder({
|
||||
getConfig: () => params.cfg,
|
||||
deliver: deliver as unknown as NonNullable<
|
||||
NonNullable<Parameters<typeof createExecApprovalForwarder>[0]>["deliver"]
|
||||
>,
|
||||
nowMs: () => 1000,
|
||||
});
|
||||
return { deliver, forwarder };
|
||||
}
|
||||
|
||||
function makePluginRequest(overrides?: Partial<PluginApprovalRequest>): PluginApprovalRequest {
|
||||
return {
|
||||
id: "plugin-req-1",
|
||||
request: {
|
||||
pluginId: "sage",
|
||||
title: "Sensitive tool call",
|
||||
description: "The agent wants to call a sensitive tool",
|
||||
severity: "warning",
|
||||
toolName: "bash",
|
||||
agentId: "main",
|
||||
sessionKey: "agent:main:main",
|
||||
},
|
||||
createdAtMs: 1000,
|
||||
expiresAtMs: 6000,
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
describe("plugin approval forwarding", () => {
|
||||
beforeEach(() => {
|
||||
setActivePluginRegistry(emptyRegistry);
|
||||
});
|
||||
|
||||
describe("handlePluginApprovalRequested", () => {
|
||||
it("returns false when forwarding is disabled", async () => {
|
||||
const { forwarder } = createForwarder({ cfg: PLUGIN_DISABLED_CFG });
|
||||
const result = await forwarder.handlePluginApprovalRequested!(makePluginRequest());
|
||||
expect(result).toBe(false);
|
||||
});
|
||||
|
||||
it("forwards to configured targets", async () => {
|
||||
const deliver = vi.fn().mockResolvedValue([]);
|
||||
const { forwarder } = createForwarder({ cfg: PLUGIN_TARGETS_CFG, deliver });
|
||||
const result = await forwarder.handlePluginApprovalRequested!(makePluginRequest());
|
||||
expect(result).toBe(true);
|
||||
// Allow delivery to be async
|
||||
await vi.waitFor(() => {
|
||||
expect(deliver).toHaveBeenCalled();
|
||||
});
|
||||
const deliveryArgs = deliver.mock.calls[0]?.[0] as
|
||||
| { payloads?: Array<{ text?: string }> }
|
||||
| undefined;
|
||||
const text = deliveryArgs?.payloads?.[0]?.text ?? "";
|
||||
expect(text).toContain("Plugin approval required");
|
||||
expect(text).toContain("Sensitive tool call");
|
||||
expect(text).toContain("plugin-req-1");
|
||||
expect(text).toContain("/approve");
|
||||
});
|
||||
|
||||
it("includes severity icon for critical", async () => {
|
||||
const deliver = vi.fn().mockResolvedValue([]);
|
||||
const { forwarder } = createForwarder({ cfg: PLUGIN_TARGETS_CFG, deliver });
|
||||
const request = makePluginRequest();
|
||||
request.request.severity = "critical";
|
||||
await forwarder.handlePluginApprovalRequested!(request);
|
||||
await vi.waitFor(() => {
|
||||
expect(deliver).toHaveBeenCalled();
|
||||
});
|
||||
const text =
|
||||
(deliver.mock.calls[0]?.[0] as { payloads?: Array<{ text?: string }> })?.payloads?.[0]
|
||||
?.text ?? "";
|
||||
expect(text).toMatch(/🚨/);
|
||||
});
|
||||
|
||||
it("returns false when exec enabled but plugin disabled", async () => {
|
||||
const cfg = {
|
||||
approvals: {
|
||||
exec: { enabled: true, mode: "targets", targets: [{ channel: "slack", to: "U123" }] },
|
||||
plugin: { enabled: false },
|
||||
},
|
||||
} as OpenClawConfig;
|
||||
const { forwarder } = createForwarder({ cfg });
|
||||
const result = await forwarder.handlePluginApprovalRequested!(makePluginRequest());
|
||||
expect(result).toBe(false);
|
||||
});
|
||||
|
||||
it("forwards when plugin enabled but exec disabled", async () => {
|
||||
const cfg = {
|
||||
approvals: {
|
||||
exec: { enabled: false },
|
||||
plugin: {
|
||||
enabled: true,
|
||||
mode: "targets",
|
||||
targets: [{ channel: "slack", to: "U123" }],
|
||||
},
|
||||
},
|
||||
} as OpenClawConfig;
|
||||
const deliver = vi.fn().mockResolvedValue([]);
|
||||
const { forwarder } = createForwarder({ cfg, deliver });
|
||||
const result = await forwarder.handlePluginApprovalRequested!(makePluginRequest());
|
||||
expect(result).toBe(true);
|
||||
await vi.waitFor(() => {
|
||||
expect(deliver).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
it("returns false when no approvals config at all", async () => {
|
||||
const cfg = {} as OpenClawConfig;
|
||||
const { forwarder } = createForwarder({ cfg });
|
||||
const result = await forwarder.handlePluginApprovalRequested!(makePluginRequest());
|
||||
expect(result).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("channel adapter hooks", () => {
|
||||
it("uses buildPluginPendingPayload from channel adapter when available", async () => {
|
||||
const mockPayload = { text: "custom adapter payload" };
|
||||
const adapterPlugin: Pick<
|
||||
ChannelPlugin,
|
||||
"id" | "meta" | "capabilities" | "config" | "execApprovals"
|
||||
> = {
|
||||
...createChannelTestPluginBase({ id: "slack" as ChannelPlugin["id"] }),
|
||||
execApprovals: {
|
||||
buildPluginPendingPayload: vi.fn().mockReturnValue(mockPayload),
|
||||
},
|
||||
};
|
||||
const registry = createTestRegistry([
|
||||
{ pluginId: "slack", plugin: adapterPlugin, source: "test" },
|
||||
]);
|
||||
setActivePluginRegistry(registry);
|
||||
|
||||
const deliver = vi.fn().mockResolvedValue([]);
|
||||
const { forwarder } = createForwarder({ cfg: PLUGIN_TARGETS_CFG, deliver });
|
||||
await forwarder.handlePluginApprovalRequested!(makePluginRequest());
|
||||
await vi.waitFor(() => {
|
||||
expect(deliver).toHaveBeenCalled();
|
||||
});
|
||||
const deliveryArgs = deliver.mock.calls[0]?.[0] as
|
||||
| { payloads?: Array<{ text?: string }> }
|
||||
| undefined;
|
||||
expect(deliveryArgs?.payloads?.[0]?.text).toBe("custom adapter payload");
|
||||
});
|
||||
|
||||
it("falls back to plugin text when no adapter exists", async () => {
|
||||
const deliver = vi.fn().mockResolvedValue([]);
|
||||
const { forwarder } = createForwarder({ cfg: PLUGIN_TARGETS_CFG, deliver });
|
||||
await forwarder.handlePluginApprovalRequested!(makePluginRequest());
|
||||
await vi.waitFor(() => {
|
||||
expect(deliver).toHaveBeenCalled();
|
||||
});
|
||||
const text =
|
||||
(deliver.mock.calls[0]?.[0] as { payloads?: Array<{ text?: string }> })?.payloads?.[0]
|
||||
?.text ?? "";
|
||||
expect(text).toContain("Plugin approval required");
|
||||
});
|
||||
|
||||
it("calls beforeDeliverPending before plugin approval delivery", async () => {
|
||||
const beforeDeliverPending = vi.fn();
|
||||
const adapterPlugin: Pick<
|
||||
ChannelPlugin,
|
||||
"id" | "meta" | "capabilities" | "config" | "execApprovals"
|
||||
> = {
|
||||
...createChannelTestPluginBase({ id: "slack" as ChannelPlugin["id"] }),
|
||||
execApprovals: {
|
||||
beforeDeliverPending,
|
||||
},
|
||||
};
|
||||
const registry = createTestRegistry([
|
||||
{ pluginId: "slack", plugin: adapterPlugin, source: "test" },
|
||||
]);
|
||||
setActivePluginRegistry(registry);
|
||||
|
||||
const deliver = vi.fn().mockResolvedValue([]);
|
||||
const { forwarder } = createForwarder({ cfg: PLUGIN_TARGETS_CFG, deliver });
|
||||
await forwarder.handlePluginApprovalRequested!(makePluginRequest());
|
||||
await vi.waitFor(() => {
|
||||
expect(deliver).toHaveBeenCalled();
|
||||
});
|
||||
expect(beforeDeliverPending).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("uses buildPluginResolvedPayload from channel adapter for resolved messages", async () => {
|
||||
const mockPayload = { text: "custom resolved payload" };
|
||||
const adapterPlugin: Pick<
|
||||
ChannelPlugin,
|
||||
"id" | "meta" | "capabilities" | "config" | "execApprovals"
|
||||
> = {
|
||||
...createChannelTestPluginBase({ id: "slack" as ChannelPlugin["id"] }),
|
||||
execApprovals: {
|
||||
buildPluginResolvedPayload: vi.fn().mockReturnValue(mockPayload),
|
||||
},
|
||||
};
|
||||
const registry = createTestRegistry([
|
||||
{ pluginId: "slack", plugin: adapterPlugin, source: "test" },
|
||||
]);
|
||||
setActivePluginRegistry(registry);
|
||||
|
||||
const deliver = vi.fn().mockResolvedValue([]);
|
||||
const { forwarder } = createForwarder({ cfg: PLUGIN_TARGETS_CFG, deliver });
|
||||
|
||||
// First register request so targets are tracked
|
||||
await forwarder.handlePluginApprovalRequested!(makePluginRequest());
|
||||
await vi.waitFor(() => {
|
||||
expect(deliver).toHaveBeenCalled();
|
||||
});
|
||||
deliver.mockClear();
|
||||
|
||||
const resolved: PluginApprovalResolved = {
|
||||
id: "plugin-req-1",
|
||||
decision: "allow-once",
|
||||
resolvedBy: "telegram:user123",
|
||||
ts: 2000,
|
||||
};
|
||||
await forwarder.handlePluginApprovalResolved!(resolved);
|
||||
expect(deliver).toHaveBeenCalled();
|
||||
const deliveryArgs = deliver.mock.calls[0]?.[0] as
|
||||
| { payloads?: Array<{ text?: string }> }
|
||||
| undefined;
|
||||
expect(deliveryArgs?.payloads?.[0]?.text).toBe("custom resolved payload");
|
||||
});
|
||||
});
|
||||
|
||||
describe("handlePluginApprovalResolved", () => {
|
||||
it("delivers resolved message to targets", async () => {
|
||||
const deliver = vi.fn().mockResolvedValue([]);
|
||||
const { forwarder } = createForwarder({ cfg: PLUGIN_TARGETS_CFG, deliver });
|
||||
|
||||
// First register request so targets are tracked
|
||||
await forwarder.handlePluginApprovalRequested!(makePluginRequest());
|
||||
await vi.waitFor(() => {
|
||||
expect(deliver).toHaveBeenCalled();
|
||||
});
|
||||
deliver.mockClear();
|
||||
|
||||
const resolved: PluginApprovalResolved = {
|
||||
id: "plugin-req-1",
|
||||
decision: "allow-once",
|
||||
resolvedBy: "telegram:user123",
|
||||
ts: 2000,
|
||||
};
|
||||
await forwarder.handlePluginApprovalResolved!(resolved);
|
||||
expect(deliver).toHaveBeenCalled();
|
||||
const text =
|
||||
(deliver.mock.calls[0]?.[0] as { payloads?: Array<{ text?: string }> })?.payloads?.[0]
|
||||
?.text ?? "";
|
||||
expect(text).toContain("Plugin approval");
|
||||
expect(text).toContain("allowed once");
|
||||
});
|
||||
|
||||
it("reconstructs targets from resolved request snapshot when pending cache is missing", async () => {
|
||||
const deliver = vi.fn().mockResolvedValue([]);
|
||||
const { forwarder } = createForwarder({ cfg: PLUGIN_TARGETS_CFG, deliver });
|
||||
|
||||
await forwarder.handlePluginApprovalResolved!({
|
||||
id: "plugin-req-late",
|
||||
decision: "deny",
|
||||
resolvedBy: "telegram:user123",
|
||||
ts: 2_000,
|
||||
request: {
|
||||
pluginId: "sage",
|
||||
title: "Sensitive tool call",
|
||||
description: "The agent wants to call a sensitive tool",
|
||||
severity: "warning",
|
||||
toolName: "bash",
|
||||
agentId: "main",
|
||||
sessionKey: "agent:main:main",
|
||||
},
|
||||
});
|
||||
|
||||
expect(deliver).toHaveBeenCalled();
|
||||
const text =
|
||||
(deliver.mock.calls[0]?.[0] as { payloads?: Array<{ text?: string }> })?.payloads?.[0]
|
||||
?.text ?? "";
|
||||
expect(text).toContain("Plugin approval");
|
||||
expect(text).toContain("denied");
|
||||
});
|
||||
});
|
||||
|
||||
describe("stop", () => {
|
||||
it("clears pending plugin approvals", async () => {
|
||||
const deliver = vi.fn().mockResolvedValue([]);
|
||||
const { forwarder } = createForwarder({ cfg: PLUGIN_TARGETS_CFG, deliver });
|
||||
await forwarder.handlePluginApprovalRequested!(makePluginRequest());
|
||||
// Wait for the async delivery to flush before stopping
|
||||
await vi.waitFor(() => {
|
||||
expect(deliver).toHaveBeenCalled();
|
||||
});
|
||||
forwarder.stop();
|
||||
deliver.mockClear();
|
||||
// After stop, resolved should not deliver
|
||||
await forwarder.handlePluginApprovalResolved!({
|
||||
id: "plugin-req-1",
|
||||
decision: "deny",
|
||||
ts: 2000,
|
||||
});
|
||||
expect(deliver).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
});
|
||||
82
src/infra/plugin-approvals.ts
Normal file
82
src/infra/plugin-approvals.ts
Normal file
@@ -0,0 +1,82 @@
|
||||
import type { ExecApprovalDecision } from "./exec-approvals.js";
|
||||
|
||||
export type PluginApprovalRequestPayload = {
|
||||
pluginId?: string | null;
|
||||
title: string;
|
||||
description: string;
|
||||
severity?: "info" | "warning" | "critical" | null;
|
||||
toolName?: string | null;
|
||||
toolCallId?: string | null;
|
||||
agentId?: string | null;
|
||||
sessionKey?: string | null;
|
||||
turnSourceChannel?: string | null;
|
||||
turnSourceTo?: string | null;
|
||||
turnSourceAccountId?: string | null;
|
||||
turnSourceThreadId?: string | number | null;
|
||||
};
|
||||
|
||||
export type PluginApprovalRequest = {
|
||||
id: string;
|
||||
request: PluginApprovalRequestPayload;
|
||||
createdAtMs: number;
|
||||
expiresAtMs: number;
|
||||
};
|
||||
|
||||
export type PluginApprovalResolved = {
|
||||
id: string;
|
||||
decision: ExecApprovalDecision;
|
||||
resolvedBy?: string | null;
|
||||
ts: number;
|
||||
request?: PluginApprovalRequestPayload;
|
||||
};
|
||||
|
||||
export const DEFAULT_PLUGIN_APPROVAL_TIMEOUT_MS = 120_000;
|
||||
export const MAX_PLUGIN_APPROVAL_TIMEOUT_MS = 600_000;
|
||||
export const PLUGIN_APPROVAL_TITLE_MAX_LENGTH = 80;
|
||||
export const PLUGIN_APPROVAL_DESCRIPTION_MAX_LENGTH = 256;
|
||||
|
||||
export function approvalDecisionLabel(decision: ExecApprovalDecision): string {
|
||||
if (decision === "allow-once") {
|
||||
return "allowed once";
|
||||
}
|
||||
if (decision === "allow-always") {
|
||||
return "allowed always";
|
||||
}
|
||||
return "denied";
|
||||
}
|
||||
|
||||
export function buildPluginApprovalRequestMessage(
|
||||
request: PluginApprovalRequest,
|
||||
nowMsValue: number,
|
||||
): string {
|
||||
const lines: string[] = [];
|
||||
const severity = request.request.severity ?? "warning";
|
||||
const icon = severity === "critical" ? "🚨" : severity === "info" ? "ℹ️" : "🛡️";
|
||||
lines.push(`${icon} Plugin approval required`);
|
||||
lines.push(`Title: ${request.request.title}`);
|
||||
lines.push(`Description: ${request.request.description}`);
|
||||
if (request.request.toolName) {
|
||||
lines.push(`Tool: ${request.request.toolName}`);
|
||||
}
|
||||
if (request.request.pluginId) {
|
||||
lines.push(`Plugin: ${request.request.pluginId}`);
|
||||
}
|
||||
if (request.request.agentId) {
|
||||
lines.push(`Agent: ${request.request.agentId}`);
|
||||
}
|
||||
lines.push(`ID: ${request.id}`);
|
||||
const expiresIn = Math.max(0, Math.round((request.expiresAtMs - nowMsValue) / 1000));
|
||||
lines.push(`Expires in: ${expiresIn}s`);
|
||||
lines.push("Reply with: /approve <id> allow-once|allow-always|deny");
|
||||
return lines.join("\n");
|
||||
}
|
||||
|
||||
export function buildPluginApprovalResolvedMessage(resolved: PluginApprovalResolved): string {
|
||||
const base = `✅ Plugin approval ${approvalDecisionLabel(resolved.decision)}.`;
|
||||
const by = resolved.resolvedBy ? ` Resolved by ${resolved.resolvedBy}.` : "";
|
||||
return `${base}${by} ID: ${resolved.id}`;
|
||||
}
|
||||
|
||||
export function buildPluginApprovalExpiredMessage(request: PluginApprovalRequest): string {
|
||||
return `⏱️ Plugin approval expired. ID: ${request.id}`;
|
||||
}
|
||||
@@ -11,6 +11,7 @@ export * from "../infra/exec-approval-command-display.ts";
|
||||
export * from "../infra/exec-approval-reply.ts";
|
||||
export * from "../infra/exec-approval-session-target.ts";
|
||||
export * from "../infra/exec-approvals.ts";
|
||||
export * from "../infra/plugin-approvals.ts";
|
||||
export * from "../infra/fetch.js";
|
||||
export * from "../infra/file-lock.js";
|
||||
export * from "../infra/format-time/format-duration.ts";
|
||||
|
||||
209
src/plugins/hooks.before-tool-call.test.ts
Normal file
209
src/plugins/hooks.before-tool-call.test.ts
Normal file
@@ -0,0 +1,209 @@
|
||||
import { beforeEach, describe, expect, it } from "vitest";
|
||||
import { createHookRunner } from "./hooks.js";
|
||||
import { addTestHook } from "./hooks.test-helpers.js";
|
||||
import { createEmptyPluginRegistry, type PluginRegistry } from "./registry.js";
|
||||
import type { PluginHookToolContext } from "./types.js";
|
||||
import type { PluginHookBeforeToolCallResult, PluginHookRegistration } from "./types.js";
|
||||
|
||||
function addBeforeToolCallHook(
|
||||
registry: PluginRegistry,
|
||||
pluginId: string,
|
||||
handler: () => PluginHookBeforeToolCallResult | Promise<PluginHookBeforeToolCallResult>,
|
||||
priority?: number,
|
||||
) {
|
||||
addTestHook({
|
||||
registry,
|
||||
pluginId,
|
||||
hookName: "before_tool_call",
|
||||
handler: handler as PluginHookRegistration["handler"],
|
||||
priority,
|
||||
});
|
||||
}
|
||||
|
||||
const stubCtx: PluginHookToolContext = {
|
||||
toolName: "bash",
|
||||
agentId: "main",
|
||||
sessionKey: "agent:main:main",
|
||||
};
|
||||
|
||||
describe("before_tool_call hook merger — requireApproval", () => {
|
||||
let registry: PluginRegistry;
|
||||
|
||||
beforeEach(() => {
|
||||
registry = createEmptyPluginRegistry();
|
||||
});
|
||||
|
||||
it("propagates requireApproval from a single plugin", async () => {
|
||||
addBeforeToolCallHook(registry, "sage", () => ({
|
||||
requireApproval: {
|
||||
id: "approval-1",
|
||||
title: "Sensitive tool",
|
||||
description: "This tool does something sensitive",
|
||||
severity: "warning",
|
||||
},
|
||||
}));
|
||||
const runner = createHookRunner(registry);
|
||||
const result = await runner.runBeforeToolCall({ toolName: "bash", params: {} }, stubCtx);
|
||||
expect(result?.requireApproval).toEqual({
|
||||
id: "approval-1",
|
||||
title: "Sensitive tool",
|
||||
description: "This tool does something sensitive",
|
||||
severity: "warning",
|
||||
pluginId: "sage",
|
||||
});
|
||||
});
|
||||
|
||||
it("stamps pluginId from the registration", async () => {
|
||||
addBeforeToolCallHook(registry, "my-plugin", () => ({
|
||||
requireApproval: {
|
||||
id: "a1",
|
||||
title: "T",
|
||||
description: "D",
|
||||
},
|
||||
}));
|
||||
const runner = createHookRunner(registry);
|
||||
const result = await runner.runBeforeToolCall({ toolName: "bash", params: {} }, stubCtx);
|
||||
expect(result?.requireApproval?.pluginId).toBe("my-plugin");
|
||||
});
|
||||
|
||||
it("first hook with requireApproval wins when multiple plugins set it", async () => {
|
||||
addBeforeToolCallHook(
|
||||
registry,
|
||||
"plugin-a",
|
||||
() => ({
|
||||
requireApproval: {
|
||||
title: "First",
|
||||
description: "First plugin",
|
||||
},
|
||||
}),
|
||||
100,
|
||||
);
|
||||
addBeforeToolCallHook(
|
||||
registry,
|
||||
"plugin-b",
|
||||
() => ({
|
||||
requireApproval: {
|
||||
title: "Second",
|
||||
description: "Second plugin",
|
||||
},
|
||||
}),
|
||||
50,
|
||||
);
|
||||
const runner = createHookRunner(registry);
|
||||
const result = await runner.runBeforeToolCall({ toolName: "bash", params: {} }, stubCtx);
|
||||
expect(result?.requireApproval?.title).toBe("First");
|
||||
expect(result?.requireApproval?.pluginId).toBe("plugin-a");
|
||||
});
|
||||
|
||||
it("does not overwrite pluginId if plugin sets it (stamped by merger)", async () => {
|
||||
addBeforeToolCallHook(registry, "actual-plugin", () => ({
|
||||
requireApproval: {
|
||||
title: "T",
|
||||
description: "D",
|
||||
pluginId: "should-be-overwritten",
|
||||
},
|
||||
}));
|
||||
const runner = createHookRunner(registry);
|
||||
const result = await runner.runBeforeToolCall({ toolName: "bash", params: {} }, stubCtx);
|
||||
// The merger spreads the requireApproval then overwrites pluginId from registration
|
||||
expect(result?.requireApproval?.pluginId).toBe("actual-plugin");
|
||||
});
|
||||
|
||||
it("merges block and requireApproval from different plugins", async () => {
|
||||
addBeforeToolCallHook(
|
||||
registry,
|
||||
"approver",
|
||||
() => ({
|
||||
requireApproval: {
|
||||
title: "Needs approval",
|
||||
description: "Approval needed",
|
||||
},
|
||||
}),
|
||||
100,
|
||||
);
|
||||
addBeforeToolCallHook(
|
||||
registry,
|
||||
"blocker",
|
||||
() => ({
|
||||
block: true,
|
||||
blockReason: "blocked",
|
||||
}),
|
||||
50,
|
||||
);
|
||||
const runner = createHookRunner(registry);
|
||||
const result = await runner.runBeforeToolCall({ toolName: "bash", params: {} }, stubCtx);
|
||||
expect(result?.block).toBe(true);
|
||||
expect(result?.requireApproval?.title).toBe("Needs approval");
|
||||
});
|
||||
|
||||
it("returns undefined requireApproval when no plugin sets it", async () => {
|
||||
addBeforeToolCallHook(registry, "plain", () => ({
|
||||
params: { extra: true },
|
||||
}));
|
||||
const runner = createHookRunner(registry);
|
||||
const result = await runner.runBeforeToolCall({ toolName: "bash", params: {} }, stubCtx);
|
||||
expect(result?.requireApproval).toBeUndefined();
|
||||
});
|
||||
|
||||
it("freezes params after requireApproval when a lower-priority plugin tries to override them", async () => {
|
||||
addBeforeToolCallHook(
|
||||
registry,
|
||||
"approver",
|
||||
() => ({
|
||||
params: { source: "approver", safe: true },
|
||||
requireApproval: {
|
||||
title: "Needs approval",
|
||||
description: "Approval needed",
|
||||
},
|
||||
}),
|
||||
100,
|
||||
);
|
||||
addBeforeToolCallHook(
|
||||
registry,
|
||||
"mutator",
|
||||
() => ({
|
||||
params: { source: "mutator", safe: false },
|
||||
}),
|
||||
50,
|
||||
);
|
||||
|
||||
const runner = createHookRunner(registry);
|
||||
const result = await runner.runBeforeToolCall({ toolName: "bash", params: {} }, stubCtx);
|
||||
|
||||
expect(result?.requireApproval?.pluginId).toBe("approver");
|
||||
expect(result?.params).toEqual({ source: "approver", safe: true });
|
||||
});
|
||||
|
||||
it("still allows block=true from a lower-priority plugin after requireApproval", async () => {
|
||||
addBeforeToolCallHook(
|
||||
registry,
|
||||
"approver",
|
||||
() => ({
|
||||
params: { source: "approver", safe: true },
|
||||
requireApproval: {
|
||||
title: "Needs approval",
|
||||
description: "Approval needed",
|
||||
},
|
||||
}),
|
||||
100,
|
||||
);
|
||||
addBeforeToolCallHook(
|
||||
registry,
|
||||
"blocker",
|
||||
() => ({
|
||||
block: true,
|
||||
blockReason: "blocked",
|
||||
params: { source: "blocker", safe: false },
|
||||
}),
|
||||
50,
|
||||
);
|
||||
|
||||
const runner = createHookRunner(registry);
|
||||
const result = await runner.runBeforeToolCall({ toolName: "bash", params: {} }, stubCtx);
|
||||
|
||||
expect(result?.block).toBe(true);
|
||||
expect(result?.blockReason).toBe("blocked");
|
||||
expect(result?.requireApproval?.pluginId).toBe("approver");
|
||||
expect(result?.params).toEqual({ source: "approver", safe: true });
|
||||
});
|
||||
});
|
||||
@@ -121,7 +121,11 @@ export type HookRunnerOptions = {
|
||||
};
|
||||
|
||||
type ModifyingHookPolicy<K extends PluginHookName, TResult> = {
|
||||
mergeResults?: (accumulated: TResult | undefined, next: TResult) => TResult;
|
||||
mergeResults?: (
|
||||
accumulated: TResult | undefined,
|
||||
next: TResult,
|
||||
registration: PluginHookRegistration<K>,
|
||||
) => TResult;
|
||||
shouldStop?: (result: TResult) => boolean;
|
||||
terminalLabel?: string;
|
||||
onTerminal?: (params: { hookName: K; pluginId: string; result: TResult }) => void;
|
||||
@@ -307,7 +311,7 @@ export function createHookRunner(registry: PluginRegistry, options: HookRunnerOp
|
||||
|
||||
if (handlerResult !== undefined && handlerResult !== null) {
|
||||
if (policy.mergeResults) {
|
||||
result = policy.mergeResults(result, handlerResult);
|
||||
result = policy.mergeResults(result, handlerResult, hook);
|
||||
} else {
|
||||
result = handlerResult;
|
||||
}
|
||||
@@ -694,14 +698,24 @@ export function createHookRunner(registry: PluginRegistry, options: HookRunnerOp
|
||||
event,
|
||||
ctx,
|
||||
{
|
||||
mergeResults: (acc, next) => {
|
||||
mergeResults: (acc, next, reg) => {
|
||||
if (acc?.block === true) {
|
||||
return acc;
|
||||
}
|
||||
const approvalPluginId = acc?.requireApproval?.pluginId;
|
||||
const freezeParamsForDifferentPlugin =
|
||||
Boolean(approvalPluginId) && approvalPluginId !== reg.pluginId;
|
||||
return {
|
||||
params: lastDefined(acc?.params, next.params),
|
||||
params: freezeParamsForDifferentPlugin
|
||||
? acc?.params
|
||||
: lastDefined(acc?.params, next.params),
|
||||
block: stickyTrue(acc?.block, next.block),
|
||||
blockReason: lastDefined(acc?.blockReason, next.blockReason),
|
||||
requireApproval:
|
||||
acc?.requireApproval ??
|
||||
(next.requireApproval
|
||||
? { ...next.requireApproval, pluginId: reg.pluginId }
|
||||
: undefined),
|
||||
};
|
||||
},
|
||||
shouldStop: (result) => result.block === true,
|
||||
|
||||
@@ -1966,10 +1966,35 @@ export type PluginHookBeforeToolCallEvent = {
|
||||
toolCallId?: string;
|
||||
};
|
||||
|
||||
export const PluginApprovalResolutions = {
|
||||
ALLOW_ONCE: "allow-once",
|
||||
ALLOW_ALWAYS: "allow-always",
|
||||
DENY: "deny",
|
||||
TIMEOUT: "timeout",
|
||||
CANCELLED: "cancelled",
|
||||
} as const;
|
||||
|
||||
export type PluginApprovalResolution =
|
||||
(typeof PluginApprovalResolutions)[keyof typeof PluginApprovalResolutions];
|
||||
|
||||
export type PluginHookBeforeToolCallResult = {
|
||||
params?: Record<string, unknown>;
|
||||
block?: boolean;
|
||||
blockReason?: string;
|
||||
requireApproval?: {
|
||||
title: string;
|
||||
description: string;
|
||||
severity?: "info" | "warning" | "critical";
|
||||
timeoutMs?: number;
|
||||
timeoutBehavior?: "allow" | "deny";
|
||||
/** Set automatically by the hook runner — plugins should not set this. */
|
||||
pluginId?: string;
|
||||
/**
|
||||
* Best-effort callback invoked with the final outcome after approval resolves, times out, or is cancelled.
|
||||
* OpenClaw does not await this callback before allowing or denying the tool call.
|
||||
*/
|
||||
onResolution?: (decision: PluginApprovalResolution) => Promise<void> | void;
|
||||
};
|
||||
};
|
||||
|
||||
// after_tool_call hook
|
||||
|
||||
Reference in New Issue
Block a user