fix(gateway): centralize plugin approval timeout bounds

This commit is contained in:
Peter Steinberger
2026-05-30 18:12:40 -04:00
parent 3d0dc15904
commit cae98c1daf
4 changed files with 80 additions and 12 deletions

View File

@@ -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<PluginApprovalRequestPayload>();
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: [],

View File

@@ -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),

View File

@@ -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;

View File

@@ -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";