fix(slack): fail closed on slash command channel type lookup
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -32,6 +32,9 @@ function createHarness(overrides?: {
|
||||
channelsConfig?: Record<string, { allow?: boolean; requireMention?: boolean }>;
|
||||
channelId?: string;
|
||||
channelName?: string;
|
||||
allowFrom?: string[];
|
||||
useAccessGroups?: boolean;
|
||||
resolveChannelName?: () => Promise<{ name?: string; type?: string }>;
|
||||
}) {
|
||||
const commands = new Map<unknown, (args: unknown) => Promise<void>>();
|
||||
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",
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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";
|
||||
|
||||
Reference in New Issue
Block a user