diff --git a/CHANGELOG.md b/CHANGELOG.md index 2814824b3..75b9e38a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ Docs: https://docs.openclaw.ai - fix(webchat): respect user scroll position during streaming and refresh (#7226) (thanks @marcomarandiz) - Telegram: recover from grammY long-poll timed out errors. (#7466) Thanks @macmimi23. - Media understanding: skip binary media from file text extraction. (#7475) Thanks @AlexZhangji. +- Security: enforce access-group gating for Slack slash commands when channel type lookup fails. - Security: guard skill installer downloads with SSRF checks (block private/localhost URLs). - Media understanding: apply SSRF guardrails to provider fetches; allow private baseUrl overrides explicitly. - Tests: stub SSRF DNS pinning in web auto-reply + Gemini video coverage. (#6619) Thanks @joshp123. diff --git a/src/slack/monitor/slash.policy.test.ts b/src/slack/monitor/slash.policy.test.ts index 72606e755..3b03b27ce 100644 --- a/src/slack/monitor/slash.policy.test.ts +++ b/src/slack/monitor/slash.policy.test.ts @@ -32,6 +32,9 @@ function createHarness(overrides?: { channelsConfig?: Record; channelId?: string; channelName?: string; + allowFrom?: string[]; + useAccessGroups?: boolean; + resolveChannelName?: () => Promise<{ name?: string; type?: string }>; }) { const commands = new Map Promise>(); const postEphemeral = vi.fn().mockResolvedValue({ ok: true }); @@ -51,14 +54,14 @@ function createHarness(overrides?: { botToken: "bot-token", botUserId: "bot", teamId: "T1", - allowFrom: ["*"], + allowFrom: overrides?.allowFrom ?? ["*"], dmEnabled: true, dmPolicy: "open", groupDmEnabled: false, groupDmChannels: [], defaultRequireMention: true, groupPolicy: overrides?.groupPolicy ?? "open", - useAccessGroups: true, + useAccessGroups: overrides?.useAccessGroups ?? true, channelsConfig: overrides?.channelsConfig, slashCommand: { enabled: true, @@ -69,7 +72,8 @@ function createHarness(overrides?: { textLimit: 4000, app, isChannelAllowed: () => true, - resolveChannelName: async () => ({ name: channelName, type: "channel" }), + resolveChannelName: + overrides?.resolveChannelName ?? (async () => ({ name: channelName, type: "channel" })), resolveUserName: async () => ({ name: "Ada" }), } as unknown; @@ -194,3 +198,109 @@ describe("slack slash commands channel policy", () => { }); }); }); + +describe("slack slash commands access groups", () => { + it("fails closed when channel type lookup returns empty for channels", async () => { + const { commands, ctx, account, channelId, channelName } = createHarness({ + allowFrom: [], + channelId: "C_UNKNOWN", + channelName: "unknown", + resolveChannelName: async () => ({}), + }); + registerSlackMonitorSlashCommands({ ctx: ctx as never, account: account as never }); + + const handler = [...commands.values()][0]; + if (!handler) { + throw new Error("Missing slash handler"); + } + + const respond = vi.fn().mockResolvedValue(undefined); + await handler({ + command: { + user_id: "U1", + user_name: "Ada", + channel_id: channelId, + channel_name: channelName, + text: "hello", + trigger_id: "t1", + }, + ack: vi.fn().mockResolvedValue(undefined), + respond, + }); + + expect(dispatchMock).not.toHaveBeenCalled(); + expect(respond).toHaveBeenCalledWith({ + text: "You are not authorized to use this command.", + response_type: "ephemeral", + }); + }); + + it("still treats D-prefixed channel ids as DMs when lookup fails", async () => { + const { commands, ctx, account } = createHarness({ + allowFrom: [], + channelId: "D123", + channelName: "notdirectmessage", + resolveChannelName: async () => ({}), + }); + registerSlackMonitorSlashCommands({ ctx: ctx as never, account: account as never }); + + const handler = [...commands.values()][0]; + if (!handler) { + throw new Error("Missing slash handler"); + } + + const respond = vi.fn().mockResolvedValue(undefined); + await handler({ + command: { + user_id: "U1", + user_name: "Ada", + channel_id: "D123", + channel_name: "notdirectmessage", + text: "hello", + trigger_id: "t1", + }, + ack: vi.fn().mockResolvedValue(undefined), + respond, + }); + + expect(dispatchMock).toHaveBeenCalledTimes(1); + expect(respond).not.toHaveBeenCalledWith( + expect.objectContaining({ text: "You are not authorized to use this command." }), + ); + }); + + it("enforces access-group gating when lookup fails for private channels", async () => { + const { commands, ctx, account, channelId, channelName } = createHarness({ + allowFrom: [], + channelId: "G123", + channelName: "private", + resolveChannelName: async () => ({}), + }); + registerSlackMonitorSlashCommands({ ctx: ctx as never, account: account as never }); + + const handler = [...commands.values()][0]; + if (!handler) { + throw new Error("Missing slash handler"); + } + + const respond = vi.fn().mockResolvedValue(undefined); + await handler({ + command: { + user_id: "U1", + user_name: "Ada", + channel_id: channelId, + channel_name: channelName, + text: "hello", + trigger_id: "t1", + }, + ack: vi.fn().mockResolvedValue(undefined), + respond, + }); + + expect(dispatchMock).not.toHaveBeenCalled(); + expect(respond).toHaveBeenCalledWith({ + text: "You are not authorized to use this command.", + response_type: "ephemeral", + }); + }); +}); diff --git a/src/slack/monitor/slash.ts b/src/slack/monitor/slash.ts index 4519ce638..19c804643 100644 --- a/src/slack/monitor/slash.ts +++ b/src/slack/monitor/slash.ts @@ -34,6 +34,7 @@ import { } from "./allow-list.js"; import { resolveSlackChannelConfig, type SlackChannelConfigResolved } from "./channel-config.js"; import { buildSlackSlashCommandMatcher, resolveSlackSlashCommandConfig } from "./commands.js"; +import { normalizeSlackChannelType } from "./context.js"; import { isSlackChannelAllowedByPolicy } from "./policy.js"; import { deliverSlackSlashReplies } from "./replies.js"; @@ -176,8 +177,9 @@ export function registerSlackMonitorSlashCommands(params: { } const channelInfo = await ctx.resolveChannelName(command.channel_id); - const channelType = + const rawChannelType = channelInfo?.type ?? (command.channel_name === "directmessage" ? "im" : undefined); + const channelType = normalizeSlackChannelType(rawChannelType, command.channel_id); const isDirectMessage = channelType === "im"; const isGroupDm = channelType === "mpim"; const isRoom = channelType === "channel" || channelType === "group";