diff --git a/CHANGELOG.md b/CHANGELOG.md index 847bf215a..cc5b1b790 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -102,6 +102,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- Nodes/Screen recording guardrails: cap `nodes` tool `screen_record` `durationMs` to 5 minutes at both schema-validation and runtime invocation layers to prevent long-running blocking captures from unbounded durations. Landed from contributor PR #31106 by @BlueBirdBack. Thanks @BlueBirdBack. - Telegram/Empty final replies: skip outbound send for null/undefined final text payloads without media so Telegram typing indicators do not linger on `text must be non-empty` errors. Landed from contributor PR #30969 by @haosenwang1018. Thanks @haosenwang1018. - Routing/Binding peer-kind parity: treat `peer.kind` `group` and `channel` as equivalent for binding scope matching (while keeping `direct` separate) so Slack/public channel bindings do not silently fall through. Landed from contributor PR #31135 by @Sid-Qin. Thanks @Sid-Qin. - Agents/FS workspace default: honor documented host file-tool default `tools.fs.workspaceOnly=false` when unset so host `write`/`edit` calls are not incorrectly workspace-restricted unless explicitly enabled. Landed from contributor PR #31128 by @SaucePackets. Thanks @SaucePackets. diff --git a/src/agents/tools/nodes-tool.test.ts b/src/agents/tools/nodes-tool.test.ts new file mode 100644 index 000000000..b7f787ff0 --- /dev/null +++ b/src/agents/tools/nodes-tool.test.ts @@ -0,0 +1,88 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const gatewayMocks = vi.hoisted(() => ({ + callGatewayTool: vi.fn(), + readGatewayCallOptions: vi.fn(() => ({})), +})); + +const nodeUtilsMocks = vi.hoisted(() => ({ + resolveNodeId: vi.fn(async () => "node-1"), + listNodes: vi.fn(async () => []), + resolveNodeIdFromList: vi.fn(() => "node-1"), +})); + +const screenMocks = vi.hoisted(() => ({ + parseScreenRecordPayload: vi.fn(() => ({ + base64: "ZmFrZQ==", + format: "mp4", + durationMs: 300_000, + fps: 10, + screenIndex: 0, + hasAudio: true, + })), + screenRecordTempPath: vi.fn(() => "/tmp/screen-record.mp4"), + writeScreenRecordToFile: vi.fn(async () => ({ path: "/tmp/screen-record.mp4" })), +})); + +vi.mock("./gateway.js", () => ({ + callGatewayTool: (...args: unknown[]) => gatewayMocks.callGatewayTool(...args), + readGatewayCallOptions: (...args: unknown[]) => gatewayMocks.readGatewayCallOptions(...args), +})); + +vi.mock("./nodes-utils.js", () => ({ + resolveNodeId: (...args: unknown[]) => nodeUtilsMocks.resolveNodeId(...args), + listNodes: (...args: unknown[]) => nodeUtilsMocks.listNodes(...args), + resolveNodeIdFromList: (...args: unknown[]) => nodeUtilsMocks.resolveNodeIdFromList(...args), +})); + +vi.mock("../../cli/nodes-screen.js", () => ({ + parseScreenRecordPayload: (...args: unknown[]) => screenMocks.parseScreenRecordPayload(...args), + screenRecordTempPath: (...args: unknown[]) => screenMocks.screenRecordTempPath(...args), + writeScreenRecordToFile: (...args: unknown[]) => screenMocks.writeScreenRecordToFile(...args), +})); + +import { createNodesTool } from "./nodes-tool.js"; + +describe("createNodesTool screen_record duration guardrails", () => { + beforeEach(() => { + gatewayMocks.callGatewayTool.mockReset(); + gatewayMocks.readGatewayCallOptions.mockReset(); + gatewayMocks.readGatewayCallOptions.mockReturnValue({}); + nodeUtilsMocks.resolveNodeId.mockClear(); + screenMocks.parseScreenRecordPayload.mockClear(); + screenMocks.writeScreenRecordToFile.mockClear(); + }); + + it("caps durationMs schema at 300000", () => { + const tool = createNodesTool(); + const schema = tool.parameters as { + properties?: { + durationMs?: { + maximum?: number; + }; + }; + }; + expect(schema.properties?.durationMs?.maximum).toBe(300_000); + }); + + it("clamps screen_record durationMs argument to 300000 before gateway invoke", async () => { + gatewayMocks.callGatewayTool.mockResolvedValue({ payload: { ok: true } }); + const tool = createNodesTool(); + + await tool.execute("call-1", { + action: "screen_record", + node: "macbook", + durationMs: 900_000, + }); + + expect(gatewayMocks.callGatewayTool).toHaveBeenCalledWith( + "node.invoke", + {}, + expect.objectContaining({ + params: expect.objectContaining({ + durationMs: 300_000, + }), + }), + ); + }); +}); diff --git a/src/agents/tools/nodes-tool.ts b/src/agents/tools/nodes-tool.ts index 2ec726a12..9a867e356 100644 --- a/src/agents/tools/nodes-tool.ts +++ b/src/agents/tools/nodes-tool.ts @@ -120,7 +120,7 @@ const NodesToolSchema = Type.Object({ delayMs: Type.Optional(Type.Number()), deviceId: Type.Optional(Type.String()), duration: Type.Optional(Type.String()), - durationMs: Type.Optional(Type.Number()), + durationMs: Type.Optional(Type.Number({ maximum: 300_000 })), includeAudio: Type.Optional(Type.Boolean()), // screen_record fps: Type.Optional(Type.Number()), @@ -421,12 +421,14 @@ export function createNodesTool(options?: { case "screen_record": { const node = readStringParam(params, "node", { required: true }); const nodeId = await resolveNodeId(gatewayOpts, node); - const durationMs = + const durationMs = Math.min( typeof params.durationMs === "number" && Number.isFinite(params.durationMs) ? params.durationMs : typeof params.duration === "string" ? parseDurationMs(params.duration) - : 10_000; + : 10_000, + 300_000, + ); const fps = typeof params.fps === "number" && Number.isFinite(params.fps) ? params.fps : 10; const screenIndex =