diff --git a/src/gateway/node-invoke-plugin-policy.test.ts b/src/gateway/node-invoke-plugin-policy.test.ts index 0b8e0341495..7c438844bec 100644 --- a/src/gateway/node-invoke-plugin-policy.test.ts +++ b/src/gateway/node-invoke-plugin-policy.test.ts @@ -1,5 +1,8 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; -import type { PluginApprovalRequestPayload } from "../infra/plugin-approvals.js"; +import { + MAX_PLUGIN_APPROVAL_TIMEOUT_MS, + type PluginApprovalRequestPayload, +} from "../infra/plugin-approvals.js"; import type { PluginRegistry } from "../plugins/registry-types.js"; import type { OpenClawPluginNodeInvokePolicyContext } from "../plugins/types.js"; import { ExecApprovalManager } from "./exec-approval-manager.js"; @@ -256,6 +259,70 @@ describe("applyPluginNodeInvokePolicy", () => { }); }); + it("caps plugin policy approval timeouts through the shared approval policy", async () => { + const manager = new ExecApprovalManager(); + registryState.current = { + nodeHostCommands: [ + { + pluginId: "demo", + command: { + command: "demo.read", + dangerous: true, + handle: async () => "{}", + }, + source: "test", + }, + ], + nodeInvokePolicies: [ + { + pluginId: "demo", + policy: { + commands: ["demo.read"], + handle: async (ctx: OpenClawPluginNodeInvokePolicyContext) => { + const approval = await ctx.approvals?.request({ + title: "Sensitive action", + description: "Needs approval", + timeoutMs: Number.MAX_SAFE_INTEGER, + }); + return { ok: true, payload: approval ?? null }; + }, + }, + pluginConfig: { enabled: true }, + source: "test", + }, + ], + } as unknown as PluginRegistry; + const { context } = createContext({ + pluginApprovalManager: manager, + getApprovalClientConnIds: createApprovalClientLookup([ + createApprovalClient({ + connId: "conn-owner-approval", + clientId: "client-owner", + deviceId: "device-owner", + }), + ]), + }); + const resultPromise = applyPluginNodeInvokePolicy({ + context, + client: createOperatorClient(), + nodeSession: createNodeSession(), + command: "demo.read", + params: { path: "/tmp/x" }, + }); + + await vi.waitFor(() => { + expect(manager.listPendingRecords()).toHaveLength(1); + }); + const [record] = manager.listPendingRecords(); + expect(record.expiresAtMs - record.createdAtMs).toBe(MAX_PLUGIN_APPROVAL_TIMEOUT_MS); + + expect(manager.resolve(record.id, "allow-once")).toBe(true); + await expect(resultPromise).resolves.toStrictEqual({ + ok: true, + payload: { id: record.id, decision: "allow-once" }, + }); + }); + it("leaves commands without a dangerous plugin registration to normal allowlist handling", async () => { registryState.current = { nodeHostCommands: [], diff --git a/src/gateway/node-invoke-plugin-policy.ts b/src/gateway/node-invoke-plugin-policy.ts index 2f6342ed066..c2578bfff3f 100644 --- a/src/gateway/node-invoke-plugin-policy.ts +++ b/src/gateway/node-invoke-plugin-policy.ts @@ -1,7 +1,7 @@ import { randomUUID } from "node:crypto"; import { normalizeOptionalString } from "@openclaw/normalization-core/string-coerce"; import type { PluginApprovalRequestPayload } from "../infra/plugin-approvals.js"; -import { DEFAULT_PLUGIN_APPROVAL_TIMEOUT_MS } from "../infra/plugin-approvals.js"; +import { resolvePluginApprovalTimeoutMs } from "../infra/plugin-approvals.js"; import { getActiveRuntimePluginRegistry } from "../plugins/active-runtime-registry.js"; import type { PluginRegistry } from "../plugins/registry-types.js"; import type { @@ -54,10 +54,7 @@ function createApprovalRuntime(params: { } return { async request(input) { - const timeoutMs = - typeof input.timeoutMs === "number" && Number.isFinite(input.timeoutMs) - ? input.timeoutMs - : DEFAULT_PLUGIN_APPROVAL_TIMEOUT_MS; + const timeoutMs = resolvePluginApprovalTimeoutMs(input.timeoutMs); const request: PluginApprovalRequestPayload = { pluginId: params.pluginId, title: input.title.slice(0, 80), diff --git a/src/gateway/server-methods/plugin-approval.ts b/src/gateway/server-methods/plugin-approval.ts index 29fde8eb8e4..a85355c33da 100644 --- a/src/gateway/server-methods/plugin-approval.ts +++ b/src/gateway/server-methods/plugin-approval.ts @@ -10,9 +10,8 @@ import { import type { ExecApprovalForwarder } from "../../infra/exec-approval-forwarder.js"; import type { PluginApprovalRequestPayload } from "../../infra/plugin-approvals.js"; import { - DEFAULT_PLUGIN_APPROVAL_TIMEOUT_MS, - MAX_PLUGIN_APPROVAL_TIMEOUT_MS, resolvePluginApprovalRequestAllowedDecisions, + resolvePluginApprovalTimeoutMs, } from "../../infra/plugin-approvals.js"; import type { ExecApprovalManager } from "../exec-approval-manager.js"; import { @@ -67,10 +66,7 @@ export function createPluginApprovalHandlers( 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 timeoutMs = resolvePluginApprovalTimeoutMs(p.timeoutMs); const normalizeTrimmedString = (value?: string | null): string | null => normalizeOptionalString(value) || null; diff --git a/src/infra/plugin-approvals.ts b/src/infra/plugin-approvals.ts index 52e2a735856..ca1e6b5d31f 100644 --- a/src/infra/plugin-approvals.ts +++ b/src/infra/plugin-approvals.ts @@ -50,6 +50,14 @@ export const DEFAULT_PLUGIN_APPROVAL_DECISIONS = [ "deny", ] as const satisfies readonly ExecApprovalDecision[]; +export function resolvePluginApprovalTimeoutMs(value: unknown): number { + const candidate = + typeof value === "number" && Number.isFinite(value) + ? value + : DEFAULT_PLUGIN_APPROVAL_TIMEOUT_MS; + return Math.min(MAX_PLUGIN_APPROVAL_TIMEOUT_MS, Math.max(1, Math.floor(candidate))); +} + export function approvalDecisionLabel(decision: ExecApprovalDecision): string { if (decision === "allow-once") { return "allowed once";