From 10d876e319b8ba45e0712dfa041a448f230d753f Mon Sep 17 00:00:00 2001 From: Colin Date: Mon, 16 Feb 2026 12:30:33 -0500 Subject: [PATCH] Slack: validate blocks input shape centrally --- src/agents/tools/slack-actions.e2e.test.ts | 14 +++++++ src/agents/tools/slack-actions.ts | 21 +--------- src/channels/plugins/actions/actions.test.ts | 19 +++++++++ src/plugin-sdk/slack-message-actions.ts | 20 +--------- src/slack/blocks-input.test.ts | 41 ++++++++++++++++++++ src/slack/blocks-input.ts | 41 ++++++++++++++++++++ 6 files changed, 119 insertions(+), 37 deletions(-) create mode 100644 src/slack/blocks-input.test.ts create mode 100644 src/slack/blocks-input.ts diff --git a/src/agents/tools/slack-actions.e2e.test.ts b/src/agents/tools/slack-actions.e2e.test.ts index b9e807329d4..fbba80d8f64 100644 --- a/src/agents/tools/slack-actions.e2e.test.ts +++ b/src/agents/tools/slack-actions.e2e.test.ts @@ -193,6 +193,20 @@ describe("handleSlackAction", () => { ).rejects.toThrow(/blocks must be valid JSON/i); }); + it("rejects empty blocks arrays", async () => { + const cfg = { channels: { slack: { botToken: "tok" } } } as OpenClawConfig; + await expect( + handleSlackAction( + { + action: "sendMessage", + to: "channel:C123", + blocks: "[]", + }, + cfg, + ), + ).rejects.toThrow(/at least one block/i); + }); + it("requires at least one of content, blocks, or mediaUrl", async () => { const cfg = { channels: { slack: { botToken: "tok" } } } as OpenClawConfig; await expect( diff --git a/src/agents/tools/slack-actions.ts b/src/agents/tools/slack-actions.ts index 7f6b16c1d03..eff7ba45ec8 100644 --- a/src/agents/tools/slack-actions.ts +++ b/src/agents/tools/slack-actions.ts @@ -1,5 +1,4 @@ import type { AgentToolResult } from "@mariozechner/pi-agent-core"; -import type { Block, KnownBlock } from "@slack/web-api"; import type { OpenClawConfig } from "../../config/config.js"; import { resolveSlackAccount } from "../../slack/accounts.js"; import { @@ -17,6 +16,7 @@ import { sendSlackMessage, unpinSlackMessage, } from "../../slack/actions.js"; +import { parseSlackBlocksInput } from "../../slack/blocks-input.js"; import { parseSlackTarget, resolveSlackChannelId } from "../../slack/targets.js"; import { withNormalizedTimestamp } from "../date-time.js"; import { @@ -86,24 +86,7 @@ function resolveThreadTsFromContext( } function readSlackBlocksParam(params: Record) { - const raw = params.blocks; - if (raw == null) { - return undefined; - } - const parsed = - typeof raw === "string" - ? (() => { - try { - return JSON.parse(raw); - } catch { - throw new Error("blocks must be valid JSON"); - } - })() - : raw; - if (!Array.isArray(parsed)) { - throw new Error("blocks must be an array"); - } - return parsed as (Block | KnownBlock)[]; + return parseSlackBlocksInput(params.blocks); } export async function handleSlackAction( diff --git a/src/channels/plugins/actions/actions.test.ts b/src/channels/plugins/actions/actions.test.ts index ee134f650f9..86cd5b12365 100644 --- a/src/channels/plugins/actions/actions.test.ts +++ b/src/channels/plugins/actions/actions.test.ts @@ -594,6 +594,25 @@ describe("slack actions adapter", () => { expect(handleSlackAction).not.toHaveBeenCalled(); }); + it("rejects empty blocks arrays for send", async () => { + const cfg = { channels: { slack: { botToken: "tok" } } } as OpenClawConfig; + const actions = createSlackActions("slack"); + + await expect( + actions.handleAction?.({ + channel: "slack", + action: "send", + cfg, + params: { + to: "channel:C1", + message: "", + blocks: "[]", + }, + }), + ).rejects.toThrow(/at least one block/i); + expect(handleSlackAction).not.toHaveBeenCalled(); + }); + it("forwards blocks JSON for edit", async () => { const cfg = { channels: { slack: { botToken: "tok" } } } as OpenClawConfig; const actions = createSlackActions("slack"); diff --git a/src/plugin-sdk/slack-message-actions.ts b/src/plugin-sdk/slack-message-actions.ts index 0672c8d6d0d..8cff504708f 100644 --- a/src/plugin-sdk/slack-message-actions.ts +++ b/src/plugin-sdk/slack-message-actions.ts @@ -1,6 +1,7 @@ import type { AgentToolResult } from "@mariozechner/pi-agent-core"; import type { ChannelMessageActionContext } from "../channels/plugins/types.js"; import { readNumberParam, readStringParam } from "../agents/tools/common.js"; +import { parseSlackBlocksInput } from "../slack/blocks-input.js"; type SlackActionInvoke = ( action: Record, @@ -9,24 +10,7 @@ type SlackActionInvoke = ( ) => Promise>; function readSlackBlocksParam(actionParams: Record) { - const raw = actionParams.blocks; - if (raw == null) { - return undefined; - } - const parsed = - typeof raw === "string" - ? (() => { - try { - return JSON.parse(raw); - } catch { - throw new Error("blocks must be valid JSON"); - } - })() - : raw; - if (!Array.isArray(parsed)) { - throw new Error("blocks must be an array"); - } - return parsed as Record[]; + return parseSlackBlocksInput(actionParams.blocks) as Record[] | undefined; } export async function handleSlackMessageAction(params: { diff --git a/src/slack/blocks-input.test.ts b/src/slack/blocks-input.test.ts new file mode 100644 index 00000000000..72b851ce27f --- /dev/null +++ b/src/slack/blocks-input.test.ts @@ -0,0 +1,41 @@ +import { describe, expect, it } from "vitest"; +import { parseSlackBlocksInput } from "./blocks-input.js"; + +describe("parseSlackBlocksInput", () => { + it("returns undefined when blocks are missing", () => { + expect(parseSlackBlocksInput(undefined)).toBeUndefined(); + expect(parseSlackBlocksInput(null)).toBeUndefined(); + }); + + it("accepts blocks arrays", () => { + const parsed = parseSlackBlocksInput([{ type: "divider" }]); + expect(parsed).toEqual([{ type: "divider" }]); + }); + + it("accepts JSON blocks strings", () => { + const parsed = parseSlackBlocksInput( + '[{"type":"section","text":{"type":"mrkdwn","text":"hi"}}]', + ); + expect(parsed).toEqual([{ type: "section", text: { type: "mrkdwn", text: "hi" } }]); + }); + + it("rejects invalid JSON", () => { + expect(() => parseSlackBlocksInput("{bad-json")).toThrow(/valid JSON/i); + }); + + it("rejects non-array payloads", () => { + expect(() => parseSlackBlocksInput({ type: "divider" })).toThrow(/must be an array/i); + }); + + it("rejects empty arrays", () => { + expect(() => parseSlackBlocksInput([])).toThrow(/at least one block/i); + }); + + it("rejects non-object blocks", () => { + expect(() => parseSlackBlocksInput(["not-a-block"])).toThrow(/must be an object/i); + }); + + it("rejects blocks without type", () => { + expect(() => parseSlackBlocksInput([{}])).toThrow(/non-empty string type/i); + }); +}); diff --git a/src/slack/blocks-input.ts b/src/slack/blocks-input.ts new file mode 100644 index 00000000000..1d8b61c2bd3 --- /dev/null +++ b/src/slack/blocks-input.ts @@ -0,0 +1,41 @@ +import type { Block, KnownBlock } from "@slack/web-api"; + +const SLACK_MAX_BLOCKS = 50; + +function parseBlocksJson(raw: string) { + try { + return JSON.parse(raw); + } catch { + throw new Error("blocks must be valid JSON"); + } +} + +function assertBlocksArray(raw: unknown) { + if (!Array.isArray(raw)) { + throw new Error("blocks must be an array"); + } + if (raw.length === 0) { + throw new Error("blocks must contain at least one block"); + } + if (raw.length > SLACK_MAX_BLOCKS) { + throw new Error(`blocks cannot exceed ${SLACK_MAX_BLOCKS} items`); + } + for (const block of raw) { + if (!block || typeof block !== "object" || Array.isArray(block)) { + throw new Error("each block must be an object"); + } + const type = (block as { type?: unknown }).type; + if (typeof type !== "string" || type.trim().length === 0) { + throw new Error("each block must include a non-empty string type"); + } + } +} + +export function parseSlackBlocksInput(raw: unknown): (Block | KnownBlock)[] | undefined { + if (raw == null) { + return undefined; + } + const parsed = typeof raw === "string" ? parseBlocksJson(raw) : raw; + assertBlocksArray(parsed); + return parsed as (Block | KnownBlock)[]; +}