From bf269e7b675cc3dc57d4fda41bfdd97023e96fe0 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 6 Apr 2026 02:42:48 +0100 Subject: [PATCH] test(plugin-sdk): tighten ACP command dispatch guards --- .../reply/dispatch-acp-command-bypass.test.ts | 111 ++++++++++++++++++ .../reply/dispatch-acp-command-bypass.ts | 4 +- src/plugin-sdk/acp-runtime.test.ts | 19 +++ src/plugin-sdk/acp-runtime.ts | 19 ++- 4 files changed, 148 insertions(+), 5 deletions(-) create mode 100644 src/auto-reply/reply/dispatch-acp-command-bypass.test.ts diff --git a/src/auto-reply/reply/dispatch-acp-command-bypass.test.ts b/src/auto-reply/reply/dispatch-acp-command-bypass.test.ts new file mode 100644 index 00000000000..383b32720b7 --- /dev/null +++ b/src/auto-reply/reply/dispatch-acp-command-bypass.test.ts @@ -0,0 +1,111 @@ +import { describe, expect, it } from "vitest"; +import type { OpenClawConfig } from "../../config/config.js"; +import { shouldBypassAcpDispatchForCommand } from "./dispatch-acp-command-bypass.js"; +import { buildTestCtx } from "./test-ctx.js"; + +describe("shouldBypassAcpDispatchForCommand", () => { + it("returns false for plain-text ACP turns", () => { + const ctx = buildTestCtx({ + Provider: "discord", + Surface: "discord", + BodyForCommands: "write a test", + BodyForAgent: "write a test", + }); + + expect(shouldBypassAcpDispatchForCommand(ctx, {} as OpenClawConfig)).toBe(false); + }); + + it("returns false for ACP slash commands", () => { + const ctx = buildTestCtx({ + Provider: "discord", + Surface: "discord", + CommandBody: "/acp cancel", + BodyForCommands: "/acp cancel", + BodyForAgent: "/acp cancel", + }); + + expect(shouldBypassAcpDispatchForCommand(ctx, {} as OpenClawConfig)).toBe(false); + }); + + it("returns false for ACP reset-tail slash commands", () => { + const ctx = buildTestCtx({ + Provider: "discord", + Surface: "discord", + CommandSource: "native", + CommandBody: "/new continue with deployment", + BodyForCommands: "/new continue with deployment", + BodyForAgent: "/new continue with deployment", + }); + + expect(shouldBypassAcpDispatchForCommand(ctx, {} as OpenClawConfig)).toBe(false); + }); + + it("returns false for slash commands when text commands are disabled", () => { + const ctx = buildTestCtx({ + Provider: "discord", + Surface: "discord", + CommandBody: "/acp cancel", + BodyForCommands: "/acp cancel", + BodyForAgent: "/acp cancel", + CommandSource: "text", + }); + const cfg = { + commands: { + text: false, + }, + } as OpenClawConfig; + + expect(shouldBypassAcpDispatchForCommand(ctx, cfg)).toBe(false); + }); + + it("returns false for unauthorized bang-prefixed commands", () => { + const ctx = buildTestCtx({ + Provider: "discord", + Surface: "discord", + CommandBody: "!poll", + BodyForCommands: "!poll", + BodyForAgent: "!poll", + CommandAuthorized: false, + }); + + expect(shouldBypassAcpDispatchForCommand(ctx, {} as OpenClawConfig)).toBe(false); + }); + + it("returns false for bang-prefixed commands when text commands are disabled", () => { + const ctx = buildTestCtx({ + Provider: "discord", + Surface: "discord", + CommandBody: "!poll", + BodyForCommands: "!poll", + BodyForAgent: "!poll", + CommandAuthorized: true, + CommandSource: "text", + }); + const cfg = { + commands: { + text: false, + }, + } as OpenClawConfig; + + expect(shouldBypassAcpDispatchForCommand(ctx, cfg)).toBe(false); + }); + + it("returns true for authorized bang-prefixed commands when text commands are enabled", () => { + const ctx = buildTestCtx({ + Provider: "discord", + Surface: "discord", + CommandBody: "!poll", + BodyForCommands: "!poll", + BodyForAgent: "!poll", + CommandAuthorized: true, + CommandSource: "text", + }); + const cfg = { + commands: { + bash: true, + }, + } as OpenClawConfig; + + expect(shouldBypassAcpDispatchForCommand(ctx, cfg)).toBe(true); + }); +}); diff --git a/src/auto-reply/reply/dispatch-acp-command-bypass.ts b/src/auto-reply/reply/dispatch-acp-command-bypass.ts index 33a36dfe968..a2c97f5e89a 100644 --- a/src/auto-reply/reply/dispatch-acp-command-bypass.ts +++ b/src/auto-reply/reply/dispatch-acp-command-bypass.ts @@ -31,16 +31,16 @@ export function shouldBypassAcpDispatchForCommand( if (!candidate) { return false; } + const normalized = candidate.trim(); const allowTextCommands = shouldHandleTextCommands({ cfg, surface: ctx.Surface ?? ctx.Provider ?? "", commandSource: ctx.CommandSource, }); - if (maybeResolveTextAlias(candidate, cfg) != null) { + if (!normalized.startsWith("/") && maybeResolveTextAlias(candidate, cfg) != null) { return allowTextCommands; } - const normalized = candidate.trim(); if (!normalized.startsWith("!")) { return false; } diff --git a/src/plugin-sdk/acp-runtime.test.ts b/src/plugin-sdk/acp-runtime.test.ts index a9fde108cf3..78b117435a9 100644 --- a/src/plugin-sdk/acp-runtime.test.ts +++ b/src/plugin-sdk/acp-runtime.test.ts @@ -55,6 +55,25 @@ describe("tryDispatchAcpReplyHook", () => { vi.clearAllMocks(); }); + it("skips ACP runtime lookup for plain-text deny turns", async () => { + const result = await tryDispatchAcpReplyHook( + { + ...event, + sendPolicy: "deny", + ctx: buildTestCtx({ + SessionKey: "agent:test:session", + BodyForCommands: "write a test", + BodyForAgent: "write a test", + }), + }, + ctx, + ); + + expect(result).toBeUndefined(); + expect(bypassMock).not.toHaveBeenCalled(); + expect(dispatchMock).not.toHaveBeenCalled(); + }); + it("skips ACP dispatch when send policy denies delivery and no bypass applies", async () => { bypassMock.mockResolvedValue(false); diff --git a/src/plugin-sdk/acp-runtime.ts b/src/plugin-sdk/acp-runtime.ts index 2729476e1b4..423183fab89 100644 --- a/src/plugin-sdk/acp-runtime.ts +++ b/src/plugin-sdk/acp-runtime.ts @@ -42,9 +42,22 @@ function loadDispatchAcpRuntime() { } function hasExplicitCommandCandidate(ctx: PluginHookReplyDispatchEvent["ctx"]): boolean { - return [ctx.CommandBody, ctx.BodyForCommands].some( - (value) => typeof value === "string" && value.trim().length > 0, - ); + const commandBody = ctx.CommandBody; + if (typeof commandBody === "string" && commandBody.trim().length > 0) { + return true; + } + + const bodyForCommands = ctx.BodyForCommands; + if (typeof bodyForCommands !== "string") { + return false; + } + + const normalized = bodyForCommands.trim(); + if (!normalized) { + return false; + } + + return normalized.startsWith("!") || normalized.startsWith("/"); } export async function tryDispatchAcpReplyHook(