admin/Moltbot
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Slack: harden slash and interactions ingress checks (openclaw#29091) thanks @Solvely-Colin

Browse Source 
Verified:
- pnpm build
- pnpm check
- pnpm test:macmini

Co-authored-by: Solvely-Colin <211764741+Solvely-Colin@users.noreply.github.com>
Co-authored-by: Tak Hoffman <781889+Takhoffman@users.noreply.github.com>
This commit is contained in:
Colin Johnson
2026-03-01 10:40:57 -05:00
committed by GitHub
parent 3aad6c8bdb
commit 0f36ee5a2e
7 changed files with 236 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@@ -81,6 +81,7 @@ Docs: https://docs.openclaw.ai
### Fixes
- Slack/Security ingress mismatch guard: drop slash-command and interaction payloads when app/team identifiers do not match the active Slack account context (including nested `team.id` interaction payloads), preventing cross-app or cross-workspace payload injection into system-event handling. (#29091) Thanks @Solvely-Colin.
- Cron/Failure alerts: add configurable repeated-failure alerting with per-job overrides and Web UI cron editor support (`inherit|disabled|custom` with threshold/cooldown/channel/target fields). (#24789) Thanks xbrak.
- Cron/Isolated model defaults: resolve isolated cron `subagents.model` (including object-form `primary`) through allowlist-aware model selection so isolated cron runs honor subagent model defaults unless explicitly overridden by job payload model. (#11474) Thanks @AnonO6.
- Cron/Isolated sessions list: persist the intended pre-run model/provider on isolated cron session entries so `sessions_list` reflects payload/session model overrides even when runs fail before post-run telemetry persistence. (#21279) Thanks @altaywtf.

82
src/slack/monitor/context.test.ts Normal file
View File

@@ -0,0 +1,82 @@
import type { App } from "@slack/bolt";
import { describe, expect, it } from "vitest";
import type { OpenClawConfig } from "../../config/config.js";
import type { RuntimeEnv } from "../../runtime.js";
import { createSlackMonitorContext } from "./context.js";
function createTestContext() {
return createSlackMonitorContext({
cfg: {
channels: { slack: { enabled: true } },
session: { dmScope: "main" },
} as OpenClawConfig,
accountId: "default",
botToken: "xoxb-test",
app: { client: {} } as App,
runtime: {} as RuntimeEnv,
botUserId: "U_BOT",
teamId: "T_EXPECTED",
apiAppId: "A_EXPECTED",
historyLimit: 0,
sessionScope: "per-sender",
mainKey: "main",
dmEnabled: true,
dmPolicy: "open",
allowFrom: [],
allowNameMatching: false,
groupDmEnabled: false,
groupDmChannels: [],
defaultRequireMention: true,
groupPolicy: "allowlist",
useAccessGroups: true,
reactionMode: "off",
reactionAllowlist: [],
replyToMode: "off",
threadHistoryScope: "thread",
threadInheritParent: false,
slashCommand: {
enabled: true,
name: "openclaw",
ephemeral: true,
sessionPrefix: "slack:slash",
},
textLimit: 4000,
ackReactionScope: "group-mentions",
mediaMaxBytes: 20 * 1024 * 1024,
removeAckAfterReply: false,
});
}
describe("createSlackMonitorContext shouldDropMismatchedSlackEvent", () => {
it("drops mismatched top-level app/team identifiers", () => {
const ctx = createTestContext();
expect(
ctx.shouldDropMismatchedSlackEvent({
api_app_id: "A_WRONG",
team_id: "T_EXPECTED",
}),
).toBe(true);
expect(
ctx.shouldDropMismatchedSlackEvent({
api_app_id: "A_EXPECTED",
team_id: "T_WRONG",
}),
).toBe(true);
});
it("drops mismatched nested team.id payloads used by interaction bodies", () => {
const ctx = createTestContext();
expect(
ctx.shouldDropMismatchedSlackEvent({
api_app_id: "A_EXPECTED",
team: { id: "T_WRONG" },
}),
).toBe(true);
expect(
ctx.shouldDropMismatchedSlackEvent({
api_app_id: "A_EXPECTED",
team: { id: "T_EXPECTED" },
}),
).toBe(false);
});
});

13
src/slack/monitor/context.ts
View File

@@ -365,9 +365,18 @@ export function createSlackMonitorContext(params: {
if (!body || typeof body !== "object") {
return false;
}
const raw = body as { api_app_id?: unknown; team_id?: unknown };
const raw = body as {
api_app_id?: unknown;
team_id?: unknown;
team?: { id?: unknown };
};
const incomingApiAppId = typeof raw.api_app_id === "string" ? raw.api_app_id : "";
const incomingTeamId = typeof raw.team_id === "string" ? raw.team_id : "";
const incomingTeamId =
typeof raw.team_id === "string"
? raw.team_id
: typeof raw.team?.id === "string"
? raw.team.id
: "";
if (params.apiAppId && incomingApiAppId && incomingApiAppId !== params.apiAppId) {
logVerbose(

85
src/slack/monitor/events/interactions.test.ts
View File

@@ -65,6 +65,7 @@ function createContext(overrides?: {
allowFrom?: string[];
allowNameMatching?: boolean;
channelsConfig?: Record<string, { users?: string[] }>;
shouldDropMismatchedSlackEvent?: (body: unknown) => boolean;
isChannelAllowed?: (params: {
channelId?: string;
channelName?: string;
@@ -128,6 +129,8 @@ function createContext(overrides?: {
allowNameMatching: overrides?.allowNameMatching ?? false,
channelsConfig: overrides?.channelsConfig ?? {},
defaultRequireMention: true,
shouldDropMismatchedSlackEvent: (body: unknown) =>
overrides?.shouldDropMismatchedSlackEvent?.(body) ?? false,
isChannelAllowed,
resolveUserName,
resolveChannelName,
@@ -224,6 +227,88 @@ describe("registerSlackInteractionEvents", () => {
expect(app.client.chat.update).toHaveBeenCalledTimes(1);
});
it("drops block actions when mismatch guard triggers", async () => {
enqueueSystemEventMock.mockClear();
const { ctx, app, getHandler } = createContext({
shouldDropMismatchedSlackEvent: () => true,
});
registerSlackInteractionEvents({ ctx: ctx as never });
const handler = getHandler();
expect(handler).toBeTruthy();
const ack = vi.fn().mockResolvedValue(undefined);
const respond = vi.fn().mockResolvedValue(undefined);
await handler!({
ack,
respond,
body: {
user: { id: "U123" },
team: { id: "T9" },
channel: { id: "C1" },
container: { channel_id: "C1", message_ts: "100.200" },
message: {
ts: "100.200",
text: "fallback",
blocks: [],
},
},
action: {
type: "button",
action_id: "openclaw:verify",
},
});
expect(ack).toHaveBeenCalledTimes(1);
expect(enqueueSystemEventMock).not.toHaveBeenCalled();
expect(app.client.chat.update).not.toHaveBeenCalled();
expect(respond).not.toHaveBeenCalled();
});
it("drops modal lifecycle payloads when mismatch guard triggers", async () => {
enqueueSystemEventMock.mockClear();
const { ctx, getViewHandler, getViewClosedHandler } = createContext({
shouldDropMismatchedSlackEvent: () => true,
});
registerSlackInteractionEvents({ ctx: ctx as never });
const viewHandler = getViewHandler();
const viewClosedHandler = getViewClosedHandler();
expect(viewHandler).toBeTruthy();
expect(viewClosedHandler).toBeTruthy();
const ackSubmit = vi.fn().mockResolvedValue(undefined);
await viewHandler!({
ack: ackSubmit,
body: {
user: { id: "U123" },
team: { id: "T9" },
view: {
id: "V123",
callback_id: "openclaw:deploy_form",
private_metadata: JSON.stringify({ userId: "U123" }),
},
},
});
expect(ackSubmit).toHaveBeenCalledTimes(1);
const ackClosed = vi.fn().mockResolvedValue(undefined);
await viewClosedHandler!({
ack: ackClosed,
body: {
user: { id: "U123" },
team: { id: "T9" },
view: {
id: "V123",
callback_id: "openclaw:deploy_form",
private_metadata: JSON.stringify({ userId: "U123" }),
},
},
});
expect(ackClosed).toHaveBeenCalledTimes(1);
expect(enqueueSystemEventMock).not.toHaveBeenCalled();
});
it("captures select values and updates action rows for non-button actions", async () => {
enqueueSystemEventMock.mockClear();
const { ctx, app, getHandler } = createContext();

10
src/slack/monitor/events/interactions.ts
View File

@@ -527,6 +527,12 @@ function registerModalLifecycleHandler(params: {
}) {
params.register(params.matcher, async ({ ack, body }: SlackModalEventHandlerArgs) => {
await ack();
if (params.ctx.shouldDropMismatchedSlackEvent?.(body)) {
params.ctx.runtime.log?.(
`slack:interaction drop ${params.interactionType} payload (mismatched app/team)`,
);
return;
}
await emitSlackModalLifecycleEvent({
ctx: params.ctx,
body: body as SlackModalBody,
@@ -561,6 +567,10 @@ export function registerSlackInteractionEvents(params: { ctx: SlackMonitorContex
// Acknowledge the action immediately to prevent the warning icon
await ack();
if (ctx.shouldDropMismatchedSlackEvent?.(body)) {
ctx.runtime.log?.("slack:interaction drop block action payload (mismatched app/team)");
return;
}
// Extract action details using proper Bolt types
const typedAction = readInteractionAction(action);

24
src/slack/monitor/slash.test.ts
View File

@@ -700,6 +700,7 @@ function createPolicyHarness(overrides?: {
channelName?: string;
allowFrom?: string[];
useAccessGroups?: boolean;
shouldDropMismatchedSlackEvent?: (body: unknown) => boolean;
resolveChannelName?: () => Promise<{ name?: string; type?: string }>;
}) {
const commands = new Map<unknown, (args: unknown) => Promise<void>>();
@@ -738,6 +739,8 @@ function createPolicyHarness(overrides?: {
textLimit: 4000,
app,
isChannelAllowed: () => true,
shouldDropMismatchedSlackEvent: (body: unknown) =>
overrides?.shouldDropMismatchedSlackEvent?.(body) ?? false,
resolveChannelName:
overrides?.resolveChannelName ?? (async () => ({ name: channelName, type: "channel" })),
resolveUserName: async () => ({ name: "Ada" }),
@@ -750,6 +753,7 @@ function createPolicyHarness(overrides?: {
async function runSlashHandler(params: {
commands: Map<unknown, (args: unknown) => Promise<void>>;
body?: unknown;
command: Partial<{
user_id: string;
user_name: string;
@@ -769,6 +773,7 @@ async function runSlashHandler(params: {
const ack = vi.fn().mockResolvedValue(undefined);
await handler({
body: params.body,
command: {
user_id: "U1",
user_name: "Ada",
@@ -785,6 +790,7 @@ async function runSlashHandler(params: {
async function registerAndRunPolicySlash(params: {
harness: ReturnType<typeof createPolicyHarness>;
body?: unknown;
command?: Partial<{
user_id: string;
user_name: string;
@@ -797,6 +803,7 @@ async function registerAndRunPolicySlash(params: {
await registerCommands(params.harness.ctx, params.harness.account);
return await runSlashHandler({
commands: params.harness.commands,
body: params.body,
command: {
channel_id: params.command?.channel_id ?? params.harness.channelId,
channel_name: params.command?.channel_name ?? params.harness.channelName,
@@ -822,6 +829,23 @@ function expectUnauthorizedResponse(respond: ReturnType<typeof vi.fn>) {
}
describe("slack slash commands channel policy", () => {
it("drops mismatched slash payloads before dispatch", async () => {
const harness = createPolicyHarness({
shouldDropMismatchedSlackEvent: () => true,
});
const { respond, ack } = await registerAndRunPolicySlash({
harness,
body: {
api_app_id: "A_MISMATCH",
team_id: "T_MISMATCH",
},
});
expect(ack).toHaveBeenCalledTimes(1);
expect(dispatchMock).not.toHaveBeenCalled();
expect(respond).not.toHaveBeenCalled();
});
it("allows unlisted channels when groupPolicy is open", async () => {
const harness = createPolicyHarness({
groupPolicy: "open",

26
src/slack/monitor/slash.ts
View File

@@ -284,12 +284,20 @@ export async function registerSlackMonitorSlashCommands(params: {
command: SlackCommandMiddlewareArgs["command"];
ack: SlackCommandMiddlewareArgs["ack"];
respond: SlackCommandMiddlewareArgs["respond"];
body?: unknown;
prompt: string;
commandArgs?: CommandArgs;
commandDefinition?: ChatCommandDefinition;
}) => {
const { command, ack, respond, prompt, commandArgs, commandDefinition } = p;
const { command, ack, respond, body, prompt, commandArgs, commandDefinition } = p;
try {
if (ctx.shouldDropMismatchedSlackEvent?.(body)) {
await ack();
runtime.log?.(
`slack: drop slash command from user=${command.user_id ?? "unknown"} channel=${command.channel_id ?? "unknown"} (mismatched app/team)`,
);
return;
}
if (!prompt.trim()) {
await ack({
text: "Message required.",
@@ -667,7 +675,7 @@ export async function registerSlackMonitorSlashCommands(params: {
for (const command of nativeCommands) {
ctx.app.command(
`/${command.name}`,
async ({ command: cmd, ack, respond }: SlackCommandMiddlewareArgs) => {
async ({ command: cmd, ack, respond, body }: SlackCommandMiddlewareArgs) => {
const commandDefinition = registry.findCommandByNativeName(command.name, "slack");
const rawText = cmd.text?.trim() ?? "";
const commandArgs = commandDefinition
@@ -684,6 +692,7 @@ export async function registerSlackMonitorSlashCommands(params: {
command: cmd,
ack,
respond,
body,
prompt,
commandArgs,
commandDefinition: commandDefinition ?? undefined,
@@ -694,11 +703,12 @@ export async function registerSlackMonitorSlashCommands(params: {
} else if (slashCommand.enabled) {
ctx.app.command(
buildSlackSlashCommandMatcher(slashCommand.name),
async ({ command, ack, respond }: SlackCommandMiddlewareArgs) => {
async ({ command, ack, respond, body }: SlackCommandMiddlewareArgs) => {
await handleSlashCommand({
command,
ack,
respond,
body,
prompt: command.text?.trim() ?? "",
});
},
@@ -725,6 +735,11 @@ export async function registerSlackMonitorSlashCommands(params: {
return;
}
appWithOptions.options(SLACK_COMMAND_ARG_ACTION_ID, async ({ ack, body }) => {
if (ctx.shouldDropMismatchedSlackEvent?.(body)) {
await ack({ options: [] });
runtime.log?.("slack: drop slash arg options payload (mismatched app/team)");
return;
}
const typedBody = body as {
value?: string;
user?: { id?: string };
@@ -779,6 +794,10 @@ export async function registerSlackMonitorSlashCommands(params: {
const { ack, body, respond } = args;
const action = args.action as { value?: string; selected_option?: { value?: string } };
await ack();
if (ctx.shouldDropMismatchedSlackEvent?.(body)) {
runtime.log?.("slack: drop slash arg action payload (mismatched app/team)");
return;
}
const respondFn =
respond ??
(async (payload: { text: string; blocks?: SlackBlock[]; response_type?: string }) => {
@@ -836,6 +855,7 @@ export async function registerSlackMonitorSlashCommands(params: {
command: commandPayload,
ack: async () => {},
respond: respondFn,
body,
prompt,
commandArgs,
commandDefinition: commandDefinition ?? undefined,