Slack: harden interactive reply trust and validation

This commit is contained in:
Vincent Koc
2026-03-13 14:06:08 -07:00
parent d477373656
commit aa6574d6c8
5 changed files with 77 additions and 22 deletions

View File

@@ -627,7 +627,7 @@ describe("parseSlackDirectives", () => {
text: "Approve",
emoji: true,
},
value: "approve",
value: "reply_1_approve",
},
{
type: "button",
@@ -637,7 +637,7 @@ describe("parseSlackDirectives", () => {
text: "Reject",
emoji: true,
},
value: "reject",
value: "reply_2_reject",
},
],
},
@@ -670,7 +670,7 @@ describe("parseSlackDirectives", () => {
text: "Alpha",
emoji: true,
},
value: "alpha",
value: "reply_1_alpha",
},
{
text: {
@@ -678,7 +678,7 @@ describe("parseSlackDirectives", () => {
text: "Beta",
emoji: true,
},
value: "beta",
value: "reply_2_beta",
},
],
},
@@ -719,7 +719,7 @@ describe("parseSlackDirectives", () => {
text: "Retry",
emoji: true,
},
value: "retry",
value: "reply_1_retry",
},
],
},
@@ -751,7 +751,7 @@ describe("parseSlackDirectives", () => {
text: "Alpha",
emoji: true,
},
value: "alpha",
value: "reply_1_alpha",
},
],
},
@@ -776,7 +776,7 @@ describe("parseSlackDirectives", () => {
text: "Retry",
emoji: true,
},
value: "retry",
value: "reply_1_retry",
},
],
},
@@ -858,6 +858,19 @@ describe("parseSlackDirectives", () => {
},
});
});
it("ignores malformed existing Slack blocks during directive compilation", () => {
expect(() =>
parseSlackDirectives({
text: "Choose [[slack_buttons: Retry:retry]]",
channelData: {
slack: {
blocks: "{not json}",
},
},
}),
).not.toThrow();
});
});
function createDeferred<T>() {

View File

@@ -193,7 +193,7 @@ describe("normalizeReplyPayload", () => {
text: "Retry",
emoji: true,
},
value: "retry",
value: "reply_1_retry",
},
{
type: "button",
@@ -203,7 +203,7 @@ describe("normalizeReplyPayload", () => {
text: "Ignore",
emoji: true,
},
value: "ignore",
value: "reply_2_ignore",
},
],
},

View File

@@ -230,6 +230,26 @@ describe("routeReply", () => {
);
});
it("does not bypass the empty-reply guard for invalid Slack blocks", async () => {
mocks.sendMessageSlack.mockClear();
const res = await routeReply({
payload: {
text: " ",
channelData: {
slack: {
blocks: " ",
},
},
},
channel: "slack",
to: "channel:C123",
cfg: {} as never,
});
expect(res.ok).toBe(true);
expect(mocks.sendMessageSlack).not.toHaveBeenCalled();
});
it("does not derive responsePrefix from agent identity when routing", async () => {
mocks.sendMessageSlack.mockClear();
const cfg = {

View File

@@ -12,6 +12,7 @@ import { resolveEffectiveMessagesConfig } from "../../agents/identity.js";
import { normalizeChannelId } from "../../channels/plugins/index.js";
import type { OpenClawConfig } from "../../config/config.js";
import { buildOutboundSessionContext } from "../../infra/outbound/session-context.js";
import { parseSlackBlocksInput } from "../../slack/blocks-input.js";
import { isSlackInteractiveRepliesEnabled } from "../../slack/interactive-replies.js";
import { INTERNAL_MESSAGE_CHANNEL, normalizeMessageChannel } from "../../utils/message-channel.js";
import type { OriginatingChannelType } from "../templating.js";
@@ -109,14 +110,22 @@ export async function routeReply(params: RouteReplyParams): Promise<RouteReplyRe
? [normalized.mediaUrl]
: [];
const replyToId = normalized.replyToId;
const hasSlackBlocks =
let hasSlackBlocks = false;
if (
channel === "slack" &&
Boolean(
normalized.channelData?.slack &&
typeof normalized.channelData.slack === "object" &&
!Array.isArray(normalized.channelData.slack) &&
(normalized.channelData.slack as { blocks?: unknown }).blocks,
);
normalized.channelData?.slack &&
typeof normalized.channelData.slack === "object" &&
!Array.isArray(normalized.channelData.slack)
) {
try {
hasSlackBlocks = Boolean(
parseSlackBlocksInput((normalized.channelData.slack as { blocks?: unknown }).blocks)
?.length,
);
} catch {
hasSlackBlocks = false;
}
}
// Skip empty replies.
if (!text.trim() && mediaUrls.length === 0 && !hasSlackBlocks) {

View File

@@ -60,6 +60,15 @@ function parseChoices(raw: string, maxItems: number): SlackChoice[] {
.slice(0, maxItems);
}
function buildSlackReplyChoiceToken(value: string, index: number): string {
const slug = value
.trim()
.toLowerCase()
.replace(/[^a-z0-9]+/g, "_")
.replace(/^_+|_+$/g, "");
return truncateSlackText(`reply_${index}_${slug || "choice"}`, SLACK_OPTION_VALUE_MAX);
}
function buildSectionBlock(text: string): SlackBlock | null {
const trimmed = text.trim();
if (!trimmed) {
@@ -82,7 +91,7 @@ function buildButtonsBlock(raw: string, index: number): SlackBlock | null {
return {
type: "actions",
block_id: `openclaw_reply_buttons_${index}`,
elements: choices.map((choice) => ({
elements: choices.map((choice, choiceIndex) => ({
type: "button",
action_id: SLACK_REPLY_BUTTON_ACTION_ID,
text: {
@@ -90,7 +99,7 @@ function buildButtonsBlock(raw: string, index: number): SlackBlock | null {
text: truncateSlackText(choice.label, SLACK_PLAIN_TEXT_MAX),
emoji: true,
},
value: truncateSlackText(choice.value, SLACK_OPTION_VALUE_MAX),
value: buildSlackReplyChoiceToken(choice.value, choiceIndex + 1),
})),
};
}
@@ -121,13 +130,13 @@ function buildSelectBlock(raw: string, index: number): SlackBlock | null {
text: truncateSlackText(placeholder, SLACK_PLAIN_TEXT_MAX),
emoji: true,
},
options: choices.map((choice) => ({
options: choices.map((choice, choiceIndex) => ({
text: {
type: "plain_text",
text: truncateSlackText(choice.label, SLACK_PLAIN_TEXT_MAX),
emoji: true,
},
value: truncateSlackText(choice.value, SLACK_OPTION_VALUE_MAX),
value: buildSlackReplyChoiceToken(choice.value, choiceIndex + 1),
})),
},
],
@@ -136,8 +145,12 @@ function buildSelectBlock(raw: string, index: number): SlackBlock | null {
function readExistingSlackBlocks(payload: ReplyPayload): SlackBlock[] {
const slackData = payload.channelData?.slack as SlackChannelData | undefined;
const blocks = parseSlackBlocksInput(slackData?.blocks) as SlackBlock[] | undefined;
return blocks ?? [];
try {
const blocks = parseSlackBlocksInput(slackData?.blocks) as SlackBlock[] | undefined;
return blocks ?? [];
} catch {
return [];
}
}
export function hasSlackDirectives(text: string): boolean {