diff --git a/CHANGELOG.md b/CHANGELOG.md index 3aa5fabe825..64a83a76789 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- fix(integrations): enforce channel read target allowlists [AI]. (#84982) Thanks @pgondhi987. - Gateway CLI: surface local post-challenge connect assembly failures immediately instead of waiting for the wrapper timeout. Fixes #68944. (#85253) Thanks @samzong. - Agents/exec: treat denied exec approvals as terminal instead of feeding them back into agent follow-up work, and recognize Chinese stop phrases in abort handling. Fixes #69386. (#85194) Thanks @samzong. - CLI/agents: abort accepted Gateway-backed `openclaw agent` runs on SIGINT/SIGTERM so cron and supervisor timeouts do not leave remote agent work alive. Fixes #71710. (#84381) Thanks @Kaspre. diff --git a/extensions/discord/src/actions/runtime.messaging.messages.ts b/extensions/discord/src/actions/runtime.messaging.messages.ts index 35d995f4189..6dfa8e884bb 100644 --- a/extensions/discord/src/actions/runtime.messaging.messages.ts +++ b/extensions/discord/src/actions/runtime.messaging.messages.ts @@ -54,6 +54,7 @@ export async function handleDiscordMessageManagementAction(ctx: DiscordMessaging throw new Error("Discord permissions are disabled."); } const channelId = ctx.resolveChannelId(); + await ctx.assertReadTargetAllowed({ channelId }); const permissions = await discordMessagingActionRuntime.fetchChannelPermissionsDiscord( channelId, ctx.withOpts(), @@ -79,6 +80,7 @@ export async function handleDiscordMessageManagementAction(ctx: DiscordMessaging "Discord message fetch requires guildId, channelId, and messageId (or a valid messageLink).", ); } + await ctx.assertReadTargetAllowed({ guildId, channelId }); const message = await discordMessagingActionRuntime.fetchMessageDiscord( channelId, messageId, @@ -97,6 +99,7 @@ export async function handleDiscordMessageManagementAction(ctx: DiscordMessaging throw new Error("Discord message reads are disabled."); } const channelId = ctx.resolveChannelId(); + await ctx.assertReadTargetAllowed({ channelId }); const query = { limit: readNumberParam(ctx.params, "limit"), before: readStringParam(ctx.params, "before"), @@ -172,6 +175,7 @@ export async function handleDiscordMessageManagementAction(ctx: DiscordMessaging throw new Error("Discord pins are disabled."); } const channelId = ctx.resolveChannelId(); + await ctx.assertReadTargetAllowed({ channelId }); const pins = await discordMessagingActionRuntime.listPinsDiscord(channelId, ctx.withOpts()); return jsonResult({ ok: true, pins: pins.map((pin) => ctx.normalizeMessage(pin)) }); } @@ -191,6 +195,13 @@ export async function handleDiscordMessageManagementAction(ctx: DiscordMessaging const authorIds = readStringArrayParam(ctx.params, "authorIds"); const limit = readNumberParam(ctx.params, "limit"); const channelIdList = [...(channelIds ?? []), ...(channelId ? [channelId] : [])]; + if (channelIdList.length > 0) { + for (const targetChannelId of channelIdList) { + await ctx.assertReadTargetAllowed({ guildId, channelId: targetChannelId }); + } + } else { + await ctx.assertGuildReadTargetAllowed({ guildId }); + } const authorIdList = [...(authorIds ?? []), ...(authorId ? [authorId] : [])]; const results = await discordMessagingActionRuntime.searchMessagesDiscord( { diff --git a/extensions/discord/src/actions/runtime.messaging.reactions.ts b/extensions/discord/src/actions/runtime.messaging.reactions.ts index c64a2873eca..dde70a7fb3d 100644 --- a/extensions/discord/src/actions/runtime.messaging.reactions.ts +++ b/extensions/discord/src/actions/runtime.messaging.reactions.ts @@ -54,6 +54,7 @@ export async function handleDiscordReactionMessagingAction(ctx: DiscordMessaging required: true, }); const limit = readNumberParam(ctx.params, "limit"); + await ctx.assertReadTargetAllowed({ channelId }); const reactions = await discordMessagingActionRuntime.fetchReactionsDiscord( channelId, messageId, diff --git a/extensions/discord/src/actions/runtime.messaging.runtime.ts b/extensions/discord/src/actions/runtime.messaging.runtime.ts index 81a22c4387a..c13135ca28a 100644 --- a/extensions/discord/src/actions/runtime.messaging.runtime.ts +++ b/extensions/discord/src/actions/runtime.messaging.runtime.ts @@ -7,6 +7,7 @@ import { editMessageDiscord, editChannelDiscord, fetchChannelInfoDiscord, + fetchGuildInfoDiscord, fetchChannelPermissionsDiscord, fetchMessageDiscord, fetchReactionsDiscord, @@ -33,6 +34,7 @@ export const discordMessagingActionRuntime = { editChannelDiscord, editMessageDiscord, fetchChannelInfoDiscord, + fetchGuildInfoDiscord, fetchChannelPermissionsDiscord, fetchMessageDiscord, fetchReactionsDiscord, diff --git a/extensions/discord/src/actions/runtime.messaging.shared.ts b/extensions/discord/src/actions/runtime.messaging.shared.ts index b38bda26b1f..f7854ff4cd0 100644 --- a/extensions/discord/src/actions/runtime.messaging.shared.ts +++ b/extensions/discord/src/actions/runtime.messaging.shared.ts @@ -1,5 +1,12 @@ -import { resolveDefaultDiscordAccountId } from "../accounts.js"; +import { resolveOpenProviderRuntimeGroupPolicy } from "openclaw/plugin-sdk/runtime-group-policy"; +import { mergeDiscordAccountConfig, resolveDefaultDiscordAccountId } from "../accounts.js"; import { createDiscordRuntimeAccountContext } from "../client.js"; +import { + isDiscordGroupAllowedByPolicy, + normalizeDiscordSlug, + resolveDiscordChannelConfigWithFallback, + type DiscordGuildEntryResolved, +} from "../monitor/allow-list.js"; import { type ActionGate, readStringParam, @@ -29,6 +36,8 @@ export type DiscordMessagingActionContext = { options?: DiscordMessagingActionOptions; accountId?: string; resolveChannelId: () => string; + assertReadTargetAllowed: (params: { guildId?: string; channelId: string }) => Promise; + assertGuildReadTargetAllowed: (params: { guildId: string }) => Promise; resolveReactionChannelId: () => Promise; withOpts: (extra?: Record) => { cfg: OpenClawConfig; accountId?: string }; withReactionRuntimeOptions: = Record>( @@ -37,6 +46,138 @@ export type DiscordMessagingActionContext = { normalizeMessage: (message: unknown) => unknown; }; +function hasDiscordGuildEntries( + guilds: DiscordGuildEntryResolved["channels"] | undefined, +): guilds is NonNullable { + return Boolean(guilds && Object.keys(guilds).length > 0); +} + +function allowsAllDiscordGuildChannels( + channels: DiscordGuildEntryResolved["channels"] | undefined, +): boolean { + const wildcard = channels?.["*"]; + if (!wildcard || wildcard.enabled === false) { + return false; + } + return Object.values(channels ?? {}).every((entry) => entry?.enabled !== false); +} + +function resolveDiscordActionGuildEntry(params: { + guilds?: Record; + guildId?: string; + guildName?: string; + includeWildcard?: boolean; +}): DiscordGuildEntryResolved | null { + const guildId = params.guildId?.trim(); + if (!params.guilds) { + return null; + } + if (guildId && params.guilds[guildId]) { + return { ...params.guilds[guildId], id: guildId }; + } + if (guildId) { + const byConfiguredId = Object.values(params.guilds).find((guild) => guild?.id === guildId); + if (byConfiguredId) { + return { ...byConfiguredId, id: guildId }; + } + } + const guildSlug = params.guildName ? normalizeDiscordSlug(params.guildName) : ""; + if (guildSlug) { + const bySlug = + params.guilds[guildSlug] ?? + Object.values(params.guilds).find((guild) => guild?.slug === guildSlug); + if (bySlug) { + return { ...bySlug, id: guildId, slug: guildSlug || bySlug.slug }; + } + } + if (params.includeWildcard === false) { + return null; + } + const wildcard = params.guilds["*"]; + return wildcard ? { ...wildcard, id: guildId } : null; +} + +type DiscordReadTargetContext = { + channelId: string; + guildId?: string; + channelName?: string; + channelSlug: string; + parentId?: string; + parentName?: string; + parentSlug?: string; + scope?: "channel" | "thread"; +}; + +function readDiscordChannelStringField(value: unknown, ...keys: string[]): string | undefined { + if (!value || typeof value !== "object") { + return undefined; + } + const record = value as Record; + for (const key of keys) { + const candidate = record[key]; + if (typeof candidate === "string" && candidate.trim()) { + return candidate.trim(); + } + } + return undefined; +} + +function readDiscordChannelType(value: unknown): number | undefined { + if (!value || typeof value !== "object") { + return undefined; + } + const type = (value as Record).type; + return typeof type === "number" ? type : undefined; +} + +function isDiscordThreadChannel(value: unknown): boolean { + const type = readDiscordChannelType(value); + return type === 10 || type === 11 || type === 12; +} + +function isDiscordReadTargetAllowedInGuild(params: { + groupPolicy: "open" | "disabled" | "allowlist"; + guildInfo: DiscordGuildEntryResolved | null; + target: DiscordReadTargetContext; +}): boolean { + const channelConfig = resolveDiscordChannelConfigWithFallback({ + guildInfo: params.guildInfo, + channelId: params.target.channelId, + channelName: params.target.channelName, + channelSlug: params.target.channelSlug, + parentId: params.target.parentId, + parentName: params.target.parentName, + parentSlug: params.target.parentSlug, + scope: params.target.scope, + }); + if (channelConfig?.allowed === false) { + return false; + } + return isDiscordGroupAllowedByPolicy({ + groupPolicy: params.groupPolicy, + guildAllowlisted: Boolean(params.guildInfo), + channelAllowlistConfigured: hasDiscordGuildEntries(params.guildInfo?.channels), + channelAllowed: true, + }); +} + +function isDiscordReadTargetExplicitlyAllowedById(params: { + groupPolicy: "open" | "disabled" | "allowlist"; + guildInfo: DiscordGuildEntryResolved | null; + target: DiscordReadTargetContext; +}): boolean { + const channelEntry = params.guildInfo?.channels?.[params.target.channelId]; + if (!channelEntry || channelEntry.enabled === false) { + return false; + } + return isDiscordGroupAllowedByPolicy({ + groupPolicy: params.groupPolicy, + guildAllowlisted: Boolean(params.guildInfo), + channelAllowlistConfigured: true, + channelAllowed: true, + }); +} + export function createDiscordMessagingActionContext(params: { action: string; input: Record; @@ -46,6 +187,17 @@ export function createDiscordMessagingActionContext(params: { }): DiscordMessagingActionContext { const accountId = readStringParam(params.input, "accountId"); const cfgOptions = { cfg: params.cfg }; + const accountConfig = mergeDiscordAccountConfig( + params.cfg, + accountId ?? resolveDefaultDiscordAccountId(params.cfg), + ); + const guilds = accountConfig.guilds as Record; + const hasGuildEntries = Object.keys(guilds ?? {}).length > 0; + const { groupPolicy } = resolveOpenProviderRuntimeGroupPolicy({ + providerConfigPresent: params.cfg.channels?.discord !== undefined, + groupPolicy: accountConfig.groupPolicy, + defaultGroupPolicy: params.cfg.channels?.defaults?.groupPolicy, + }); const withOpts = (extra?: Record) => createDiscordActionOptions({ cfg: params.cfg, accountId, extra }); const resolvedReactionAccountId = accountId ?? resolveDefaultDiscordAccountId(params.cfg); @@ -55,6 +207,96 @@ export function createDiscordMessagingActionContext(params: { accountId: resolvedReactionAccountId, }) : cfgOptions; + const guildNameById = new Map(); + const resolveGuildName = async (guildId: string): Promise => { + if (guildNameById.has(guildId)) { + return guildNameById.get(guildId) ?? null; + } + try { + const guildInfo = await discordMessagingActionRuntime.fetchGuildInfoDiscord( + guildId, + withOpts(), + ); + const guildName = readDiscordChannelStringField(guildInfo, "name") ?? null; + guildNameById.set(guildId, guildName); + return guildName; + } catch { + guildNameById.set(guildId, null); + return null; + } + }; + const resolveReadGuildEntry = async ( + guildId?: string, + ): Promise => { + const direct = resolveDiscordActionGuildEntry({ + guilds, + guildId, + includeWildcard: false, + }); + if (direct || !guildId) { + return direct; + } + const guildName = await resolveGuildName(guildId); + const named = resolveDiscordActionGuildEntry({ + guilds, + guildId, + guildName: guildName ?? undefined, + includeWildcard: false, + }); + if (named) { + return named; + } + return resolveDiscordActionGuildEntry({ guilds, guildId }); + }; + const resolveReadTargetContext = async (channelId: string): Promise => { + const fallback: DiscordReadTargetContext = { + channelId, + channelSlug: normalizeDiscordSlug(channelId) || channelId, + }; + let channelInfo: unknown; + try { + channelInfo = await discordMessagingActionRuntime.fetchChannelInfoDiscord( + channelId, + withOpts(), + ); + } catch { + return fallback; + } + const channelName = readDiscordChannelStringField(channelInfo, "name"); + const target: DiscordReadTargetContext = { + channelId, + channelSlug: channelName ? normalizeDiscordSlug(channelName) : fallback.channelSlug, + }; + const targetGuildId = readDiscordChannelStringField(channelInfo, "guild_id", "guildId"); + if (targetGuildId) { + target.guildId = targetGuildId; + } + if (channelName) { + target.channelName = channelName; + } + if (!isDiscordThreadChannel(channelInfo)) { + return target; + } + target.scope = "thread"; + target.parentId = readDiscordChannelStringField(channelInfo, "parent_id", "parentId"); + if (!target.parentId) { + return target; + } + try { + const parentInfo = await discordMessagingActionRuntime.fetchChannelInfoDiscord( + target.parentId, + withOpts(), + ); + const parentName = readDiscordChannelStringField(parentInfo, "name"); + if (parentName) { + target.parentName = parentName; + target.parentSlug = normalizeDiscordSlug(parentName); + } + } catch { + // Parent id fallback is enough for allowlist checks when the parent fetch is unavailable. + } + return target; + }; return { action: params.action, params: params.input, @@ -68,6 +310,73 @@ export function createDiscordMessagingActionContext(params: { required: true, }), ), + assertReadTargetAllowed: async ({ guildId, channelId }) => { + const targetChannelId = discordMessagingActionRuntime.resolveDiscordChannelId(channelId); + if (!hasGuildEntries && groupPolicy !== "disabled" && groupPolicy !== "allowlist") { + return; + } + const target = await resolveReadTargetContext(targetChannelId); + if (guildId) { + if (target.guildId && target.guildId !== guildId) { + throw new Error("Discord read target channel is not allowed."); + } + const guildInfo = await resolveReadGuildEntry(guildId); + if ( + !isDiscordReadTargetAllowedInGuild({ + groupPolicy, + guildInfo, + target, + }) + ) { + throw new Error("Discord read target channel is not allowed."); + } + return; + } + if (target.guildId) { + const guildInfo = await resolveReadGuildEntry(target.guildId); + if ( + !isDiscordReadTargetAllowedInGuild({ + groupPolicy, + guildInfo, + target, + }) + ) { + throw new Error("Discord read target channel is not allowed."); + } + return; + } + const allowed = Object.values(guilds ?? {}).some((guildInfo) => + isDiscordReadTargetExplicitlyAllowedById({ + groupPolicy, + guildInfo: guildInfo ?? null, + target, + }), + ); + if (!allowed) { + throw new Error("Discord read target channel is not allowed."); + } + }, + assertGuildReadTargetAllowed: async ({ guildId }) => { + const guildInfo = await resolveReadGuildEntry(guildId); + if ( + !isDiscordGroupAllowedByPolicy({ + groupPolicy, + guildAllowlisted: Boolean(guildInfo), + channelAllowlistConfigured: false, + channelAllowed: true, + }) + ) { + throw new Error("Discord read target channel is not allowed."); + } + if ( + hasDiscordGuildEntries(guildInfo?.channels) && + !allowsAllDiscordGuildChannels(guildInfo.channels) + ) { + throw new Error( + "Discord message search requires channelId or channelIds so each read target can be authorized.", + ); + } + }, resolveReactionChannelId: async () => { const target = readStringParam(params.input, "channelId") ?? diff --git a/extensions/discord/src/actions/runtime.test.ts b/extensions/discord/src/actions/runtime.test.ts index e90e6f245b8..308958db3cf 100644 --- a/extensions/discord/src/actions/runtime.test.ts +++ b/extensions/discord/src/actions/runtime.test.ts @@ -3,7 +3,6 @@ import type { DiscordActionConfig } from "openclaw/plugin-sdk/config-contracts"; import { beforeEach, describe, expect, it, vi } from "vitest"; import { clearPresences, setPresence } from "../monitor/presence-cache.js"; import { DiscordThreadInitialMessageError } from "../send.js"; -import { EMPTY_DISCORD_TEST_CONFIG } from "../test-support/config.js"; import { discordGuildActionRuntime, handleDiscordGuildAction } from "./runtime.guild.js"; import { handleDiscordAction } from "./runtime.js"; import { @@ -19,6 +18,14 @@ const originalDiscordMessagingActionRuntime = { ...discordMessagingActionRuntime const originalDiscordGuildActionRuntime = { ...discordGuildActionRuntime }; const originalDiscordModerationActionRuntime = { ...discordModerationActionRuntime }; +type DiscordChannelInfoTest = { + id: string; + type: number; + guild_id?: string; + name?: string; + parent_id?: string; +}; + const discordSendMocks = { banMemberDiscord: vi.fn(async () => ({})), createChannelDiscord: vi.fn(async () => ({ @@ -34,8 +41,14 @@ const discordSendMocks = { name: "edited", })), editMessageDiscord: vi.fn(async () => ({})), - fetchChannelInfoDiscord: vi.fn(async () => ({ id: "C1", type: 0 })), + fetchChannelInfoDiscord: vi.fn( + async (channelId: string): Promise => ({ id: channelId, type: 0 }), + ), fetchChannelPermissionsDiscord: vi.fn(async () => ({})), + fetchGuildInfoDiscord: vi.fn(async (guildId: string) => ({ + id: guildId, + name: "Guild", + })), fetchMessageDiscord: vi.fn(async () => ({})), fetchReactionsDiscord: vi.fn(async () => ({})), kickMemberDiscord: vi.fn(async () => ({})), @@ -66,6 +79,8 @@ const { deleteChannelDiscord, editChannelDiscord, fetchChannelInfoDiscord, + fetchChannelPermissionsDiscord, + fetchGuildInfoDiscord, fetchReactionsDiscord, fetchMessageDiscord, kickMemberDiscord, @@ -87,7 +102,14 @@ const { } = discordSendMocks; const enableAllActions = () => true; -const DISCORD_TEST_CFG = EMPTY_DISCORD_TEST_CONFIG; +const DISCORD_TEST_CFG = { + channels: { + discord: { + token: "token", + groupPolicy: "open", + }, + }, +} as OpenClawConfig; type MockCallSource = { mock: { calls: Array> } }; @@ -283,6 +305,34 @@ describe("handleDiscordMessagingAction", () => { }); }); + it("rejects Discord reaction reads for non-allowlisted target channels", async () => { + const cfg = { + channels: { + discord: { + token: "token", + groupPolicy: "allowlist", + guilds: { + "111": { + channels: { + "222": { enabled: true }, + }, + }, + }, + }, + }, + } as OpenClawConfig; + + await expect( + handleMessagingAction( + "reactions", + { channelId: "444", messageId: "M1" }, + enableAllActions, + cfg, + ), + ).rejects.toThrow("Discord read target channel is not allowed."); + expect(fetchReactionsDiscord).not.toHaveBeenCalled(); + }); + it("removes reactions on empty emoji", async () => { await handleMessagingAction( "react", @@ -370,6 +420,29 @@ describe("handleDiscordMessagingAction", () => { ); }); + it("rejects Discord permission reads for non-allowlisted target channels", async () => { + const cfg = { + channels: { + discord: { + token: "token", + groupPolicy: "allowlist", + guilds: { + "111": { + channels: { + "222": { enabled: true }, + }, + }, + }, + }, + }, + } as OpenClawConfig; + + await expect( + handleMessagingAction("permissions", { channelId: "444" }, enableAllActions, cfg), + ).rejects.toThrow("Discord read target channel is not allowed."); + expect(fetchChannelPermissionsDiscord).not.toHaveBeenCalled(); + }); + it("adds normalized timestamps to readMessages payloads", async () => { readMessagesDiscord.mockResolvedValueOnce([ { id: "1", timestamp: "2026-01-15T10:00:00.000Z" }, @@ -413,6 +486,110 @@ describe("handleDiscordMessagingAction", () => { ); }); + it("reads from allowlisted Discord target channels", async () => { + const cfg = { + channels: { + discord: { + token: "token", + groupPolicy: "allowlist", + guilds: { + "111": { + channels: { + "222": { enabled: true }, + }, + }, + }, + }, + }, + } as OpenClawConfig; + + await handleMessagingAction("readMessages", { channelId: "222" }, enableAllActions, cfg); + + expect(readMessagesDiscord).toHaveBeenCalledWith( + "222", + { limit: undefined, before: undefined, after: undefined, around: undefined }, + { cfg }, + ); + }); + + it("reads from Discord target channels allowlisted under a guild slug", async () => { + fetchChannelInfoDiscord.mockResolvedValueOnce({ + id: "222", + guild_id: "111", + type: 0, + }); + fetchGuildInfoDiscord.mockResolvedValueOnce({ + id: "111", + name: "Friends of OpenClaw", + }); + const cfg = { + channels: { + discord: { + token: "token", + groupPolicy: "allowlist", + guilds: { + "friends-of-openclaw": { + channels: { + "222": { enabled: true }, + }, + }, + }, + }, + }, + } as OpenClawConfig; + + await handleMessagingAction("readMessages", { channelId: "222" }, enableAllActions, cfg); + + expect(fetchGuildInfoDiscord).toHaveBeenCalledWith("111", { cfg }); + expect(readMessagesDiscord).toHaveBeenCalledWith( + "222", + { limit: undefined, before: undefined, after: undefined, around: undefined }, + { cfg }, + ); + }); + + it("rejects Discord reads for non-allowlisted target channels", async () => { + const cfg = { + channels: { + discord: { + token: "token", + groupPolicy: "allowlist", + guilds: { + "111": { + channels: { + "222": { enabled: true }, + }, + }, + }, + }, + }, + } as OpenClawConfig; + + await expect( + handleMessagingAction("readMessages", { channelId: "333" }, enableAllActions, cfg), + ).rejects.toThrow("Discord read target channel is not allowed."); + expect(readMessagesDiscord).not.toHaveBeenCalled(); + }); + + it("fails closed for Discord message reads when provider config is missing", async () => { + const cfg = {} as OpenClawConfig; + + await expect( + handleMessagingAction("readMessages", { channelId: "C1" }, enableAllActions, cfg), + ).rejects.toThrow("Discord read target channel is not allowed."); + expect(readMessagesDiscord).not.toHaveBeenCalled(); + + await expect( + handleMessagingAction( + "fetchMessage", + { messageLink: "https://discord.com/channels/111/222/333" }, + enableAllActions, + cfg, + ), + ).rejects.toThrow("Discord read target channel is not allowed."); + expect(fetchMessageDiscord).not.toHaveBeenCalled(); + }); + it("adds normalized timestamps to fetchMessage payloads", async () => { fetchMessageDiscord.mockResolvedValueOnce({ id: "1", @@ -448,6 +625,176 @@ describe("handleDiscordMessagingAction", () => { expect(fetchMessageDiscord).toHaveBeenCalledWith("C1", "M1", { cfg }); }); + it("fetches Discord messages from channels allowlisted under a guild slug", async () => { + fetchChannelInfoDiscord.mockResolvedValueOnce({ + id: "222", + guild_id: "111", + type: 0, + }); + fetchGuildInfoDiscord.mockResolvedValueOnce({ + id: "111", + name: "Friends of OpenClaw", + }); + const cfg = { + channels: { + discord: { + token: "token", + groupPolicy: "allowlist", + guilds: { + "friends-of-openclaw": { + channels: { + "222": { enabled: true }, + }, + }, + }, + }, + }, + } as OpenClawConfig; + + await handleMessagingAction( + "fetchMessage", + { messageLink: "https://discord.com/channels/111/222/333" }, + enableAllActions, + cfg, + ); + + expect(fetchGuildInfoDiscord).toHaveBeenCalledWith("111", { cfg }); + expect(fetchMessageDiscord).toHaveBeenCalledWith("222", "333", { cfg }); + }); + + it("rejects Discord message links for non-allowlisted target channels", async () => { + const cfg = { + channels: { + discord: { + token: "token", + groupPolicy: "allowlist", + guilds: { + "111": { + channels: { + "222": { enabled: true }, + }, + }, + }, + }, + }, + } as OpenClawConfig; + + await expect( + handleMessagingAction( + "fetchMessage", + { messageLink: "https://discord.com/channels/111/333/444" }, + enableAllActions, + cfg, + ), + ).rejects.toThrow("Discord read target channel is not allowed."); + expect(fetchMessageDiscord).not.toHaveBeenCalled(); + }); + + it("allows Discord message links in threads under allowlisted parent channels", async () => { + fetchChannelInfoDiscord.mockImplementation(async (channelId: string) => { + if (channelId === "333") { + return { id: "333", name: "incident-thread", parent_id: "222", type: 11 }; + } + if (channelId === "222") { + return { id: "222", name: "team-updates", type: 0 }; + } + return { id: channelId, type: 0 }; + }); + const cfg = { + channels: { + discord: { + token: "token", + groupPolicy: "allowlist", + guilds: { + "111": { + channels: { + "222": { enabled: true }, + }, + }, + }, + }, + }, + } as OpenClawConfig; + + await handleMessagingAction( + "fetchMessage", + { messageLink: "https://discord.com/channels/111/333/444" }, + enableAllActions, + cfg, + ); + + expect(fetchMessageDiscord).toHaveBeenCalledWith("333", "444", { cfg }); + }); + + it("rejects Discord message links when the fetched channel belongs to a different guild", async () => { + fetchChannelInfoDiscord.mockImplementation(async (channelId: string) => ({ + id: channelId, + guild_id: "222", + name: "allowed-channel", + type: 0, + })); + const cfg = { + channels: { + discord: { + token: "token", + groupPolicy: "allowlist", + guilds: { + "111": { + channels: { + "allowed-channel": { enabled: true }, + }, + }, + }, + }, + }, + } as OpenClawConfig; + + await expect( + handleMessagingAction( + "fetchMessage", + { messageLink: "https://discord.com/channels/111/333/444" }, + enableAllActions, + cfg, + ), + ).rejects.toThrow("Discord read target channel is not allowed."); + expect(fetchMessageDiscord).not.toHaveBeenCalled(); + }); + + it.each([ + { action: "readMessages", runtimeCall: readMessagesDiscord }, + { action: "listPins", runtimeCall: listPinsDiscord }, + ])( + "rejects Discord $action when the fetched guild is not allowlisted by a matching channel name", + async ({ action, runtimeCall }) => { + fetchChannelInfoDiscord.mockImplementation(async (channelId: string) => ({ + id: channelId, + guild_id: "222", + name: "allowed-channel", + type: 0, + })); + const cfg = { + channels: { + discord: { + token: "token", + groupPolicy: "allowlist", + guilds: { + "111": { + channels: { + "allowed-channel": { enabled: true }, + }, + }, + }, + }, + }, + } as OpenClawConfig; + + await expect( + handleMessagingAction(action, { channelId: "333" }, enableAllActions, cfg), + ).rejects.toThrow("Discord read target channel is not allowed."); + expect(runtimeCall).not.toHaveBeenCalled(); + }, + ); + it("adds normalized timestamps to listPins payloads", async () => { listPinsDiscord.mockResolvedValueOnce([{ id: "1", timestamp: "2026-01-15T12:00:00.000Z" }]); @@ -461,6 +808,29 @@ describe("handleDiscordMessagingAction", () => { expect(payload.pins[0].timestampUtc).toBe(new Date(expectedMs).toISOString()); }); + it("rejects Discord pin reads for non-allowlisted target channels", async () => { + const cfg = { + channels: { + discord: { + token: "token", + groupPolicy: "allowlist", + guilds: { + "111": { + channels: { + "222": { enabled: true }, + }, + }, + }, + }, + }, + } as OpenClawConfig; + + await expect( + handleMessagingAction("listPins", { channelId: "444" }, enableAllActions, cfg), + ).rejects.toThrow("Discord read target channel is not allowed."); + expect(listPinsDiscord).not.toHaveBeenCalled(); + }); + it("adds normalized timestamps to searchMessages payloads", async () => { searchMessagesDiscord.mockResolvedValueOnce({ total_results: 1, @@ -483,6 +853,114 @@ describe("handleDiscordMessagingAction", () => { ); }); + it("rejects Discord searches for non-allowlisted target channels", async () => { + const cfg = { + channels: { + discord: { + token: "token", + groupPolicy: "allowlist", + guilds: { + "111": { + channels: { + "222": { enabled: true }, + }, + }, + }, + }, + }, + } as OpenClawConfig; + + await expect( + handleMessagingAction( + "searchMessages", + { guildId: "111", channelId: "444", content: "canary" }, + enableAllActions, + cfg, + ), + ).rejects.toThrow("Discord read target channel is not allowed."); + expect(searchMessagesDiscord).not.toHaveBeenCalled(); + }); + + it("requires explicit Discord search targets when channels are allowlisted", async () => { + const cfg = { + channels: { + discord: { + token: "token", + groupPolicy: "allowlist", + guilds: { + "111": { + channels: { + "222": { enabled: true }, + }, + }, + }, + }, + }, + } as OpenClawConfig; + + await expect( + handleMessagingAction( + "searchMessages", + { guildId: "111", content: "canary" }, + enableAllActions, + cfg, + ), + ).rejects.toThrow( + "Discord message search requires channelId or channelIds so each read target can be authorized.", + ); + expect(searchMessagesDiscord).not.toHaveBeenCalled(); + }); + + it("fails closed for Discord guild-wide searches when provider config is missing", async () => { + const cfg = {} as OpenClawConfig; + + await expect( + handleMessagingAction( + "searchMessages", + { guildId: "111", content: "canary" }, + enableAllActions, + cfg, + ), + ).rejects.toThrow("Discord read target channel is not allowed."); + expect(searchMessagesDiscord).not.toHaveBeenCalled(); + }); + + it("allows guild-wide Discord searches when the guild has a wildcard channel allowlist", async () => { + const cfg = { + channels: { + discord: { + token: "token", + groupPolicy: "allowlist", + guilds: { + "111": { + channels: { + "*": { enabled: true }, + }, + }, + }, + }, + }, + } as OpenClawConfig; + + await handleMessagingAction( + "searchMessages", + { guildId: "111", content: "canary" }, + enableAllActions, + cfg, + ); + + expect(searchMessagesDiscord).toHaveBeenCalledWith( + { + guildId: "111", + content: "canary", + channelIds: undefined, + authorIds: undefined, + limit: undefined, + }, + { cfg }, + ); + }); + it("sends voice messages from a local file path", async () => { sendVoiceMessageDiscord.mockClear(); sendMessageDiscord.mockClear(); diff --git a/extensions/discord/src/send.guild.ts b/extensions/discord/src/send.guild.ts index 9472596d3aa..69be3a620dd 100644 --- a/extensions/discord/src/send.guild.ts +++ b/extensions/discord/src/send.guild.ts @@ -1,4 +1,5 @@ import type { + APIGuild, APIGuildMember, APIGuildScheduledEvent, APIRole, @@ -12,6 +13,7 @@ import { createGuildBan, createGuildScheduledEvent, getChannel, + getGuild, getGuildMember, getGuildVoiceState, listGuildChannels, @@ -68,6 +70,14 @@ export async function fetchChannelInfoDiscord( return await getChannel(rest, channelId); } +export async function fetchGuildInfoDiscord( + guildId: string, + opts: DiscordReactOpts, +): Promise { + const rest = resolveDiscordRest(opts); + return await getGuild(rest, guildId); +} + export async function listGuildChannelsDiscord( guildId: string, opts: DiscordReactOpts, diff --git a/extensions/discord/src/send.ts b/extensions/discord/src/send.ts index e9844b0880d..fbaacdd8058 100644 --- a/extensions/discord/src/send.ts +++ b/extensions/discord/src/send.ts @@ -17,6 +17,7 @@ export { createScheduledEventDiscord, resolveEventCoverImage, fetchChannelInfoDiscord, + fetchGuildInfoDiscord, fetchMemberInfoDiscord, fetchRoleInfoDiscord, fetchVoiceStatusDiscord, diff --git a/extensions/slack/src/action-runtime.test.ts b/extensions/slack/src/action-runtime.test.ts index a1e413d9282..e2352d08105 100644 --- a/extensions/slack/src/action-runtime.test.ts +++ b/extensions/slack/src/action-runtime.test.ts @@ -298,6 +298,20 @@ describe("handleSlackAction", () => { ).rejects.toThrow(/Slack reactions are disabled/); }); + it("rejects Slack reaction reads for non-allowlisted target channels", async () => { + const cfg = slackConfig({ + groupPolicy: "allowlist", + channels: { + C_ALLOWED: { enabled: true }, + }, + }); + + await expect( + handleSlackAction({ action: "reactions", channelId: "C_OTHER", messageId: "123.456" }, cfg), + ).rejects.toThrow("Slack read target channel is not allowed."); + expect(listSlackReactions).not.toHaveBeenCalled(); + }); + it("passes threadTs to sendSlackMessage for thread replies", async () => { const cfg = slackConfig(); await handleSlackAction( @@ -344,6 +358,7 @@ describe("handleSlackAction", () => { { action: "downloadFile", fileId: "F123", + channelId: "C1", }, slackConfig(), ); @@ -354,6 +369,34 @@ describe("handleSlackAction", () => { expect(requireDetails(result).ok).toBe(false); }); + it("fails closed for downloadFile when no channel target can be authorized", async () => { + await expect( + handleSlackAction({ action: "downloadFile", fileId: "F123" }, slackConfig()), + ).rejects.toThrow( + "Slack file download requires channelId or to so the read target can be authorized.", + ); + expect(downloadSlackFile).not.toHaveBeenCalled(); + }); + + it("uses current channel context to authorize downloadFile", async () => { + downloadSlackFile.mockResolvedValueOnce(null); + const cfg = slackConfig({ + groupPolicy: "allowlist", + channels: { + C1: { enabled: true }, + }, + }); + + const result = await handleSlackAction({ action: "downloadFile", fileId: "F123" }, cfg, { + currentChannelId: "C1", + }); + + expectRecordFields(requireRecordArg(downloadSlackFile, "downloadSlackFile", 0, 1), { + channelId: "C1", + }); + expect(requireDetails(result).ok).toBe(false); + }); + it("passes download scope (channel/thread) to downloadSlackFile", async () => { downloadSlackFile.mockResolvedValueOnce(null); @@ -386,6 +429,7 @@ describe("handleSlackAction", () => { { action: "downloadFile", fileId: "F123", + channelId: "C1", }, slackConfig(), ); @@ -410,14 +454,17 @@ describe("handleSlackAction", () => { it("forwards resolved botToken to action functions instead of relying on config re-read", async () => { downloadSlackFile.mockResolvedValueOnce(null); - await handleSlackAction({ action: "downloadFile", fileId: "F123" }, slackConfig()); + await handleSlackAction( + { action: "downloadFile", fileId: "F123", channelId: "C1" }, + slackConfig(), + ); expect(requireRecordArg(downloadSlackFile, "downloadSlackFile", 0, 1).token).toBe("tok"); }); it("keeps resolved userToken for downloadFile reads when configured", async () => { downloadSlackFile.mockResolvedValueOnce(null); await handleSlackAction( - { action: "downloadFile", fileId: "F123" }, + { action: "downloadFile", fileId: "F123", channelId: "C1" }, slackConfig({ accounts: { default: { @@ -828,6 +875,114 @@ describe("handleSlackAction", () => { }); }); + it("reads from allowlisted Slack target channels", async () => { + readSlackMessages.mockResolvedValueOnce({ messages: [], hasMore: false }); + + const cfg = slackConfig({ + groupPolicy: "allowlist", + channels: { + C_ALLOWED: { enabled: true }, + }, + }); + await handleSlackAction({ action: "readMessages", channelId: "C_ALLOWED" }, cfg); + + expect(requireMockArg(readSlackMessages, "readSlackMessages", 0, 0)).toBe("C_ALLOWED"); + }); + + it("rejects Slack reads for non-allowlisted target channels", async () => { + const cfg = slackConfig({ + groupPolicy: "allowlist", + channels: { + C_ALLOWED: { enabled: true }, + }, + }); + + await expect( + handleSlackAction({ action: "readMessages", channelId: "C_OTHER" }, cfg), + ).rejects.toThrow("Slack read target channel is not allowed."); + expect(readSlackMessages).not.toHaveBeenCalled(); + }); + + it("allows Slack reads from unlisted targets when group policy is open", async () => { + readSlackMessages.mockResolvedValueOnce({ messages: [], hasMore: false }); + + const cfg = slackConfig({ + groupPolicy: "open", + channels: { + C_CONFIGURED: { enabled: true }, + }, + }); + await handleSlackAction({ action: "readMessages", channelId: "C_OTHER" }, cfg); + + expect(requireMockArg(readSlackMessages, "readSlackMessages", 0, 0)).toBe("C_OTHER"); + }); + + it("rejects Slack reads from disabled targets when group policy is open", async () => { + const cfg = slackConfig({ + groupPolicy: "open", + channels: { + C_DISABLED: { enabled: false }, + }, + }); + + await expect( + handleSlackAction({ action: "readMessages", channelId: "C_DISABLED" }, cfg), + ).rejects.toThrow("Slack read target channel is not allowed."); + expect(readSlackMessages).not.toHaveBeenCalled(); + }); + + it("fails closed for read-like Slack actions when provider config is missing", async () => { + const cfg = {} as OpenClawConfig; + + await expect( + handleSlackAction({ action: "readMessages", channelId: "C1" }, cfg), + ).rejects.toThrow("Slack read target channel is not allowed."); + expect(readSlackMessages).not.toHaveBeenCalled(); + + await expect( + handleSlackAction({ action: "reactions", channelId: "C1", messageId: "123.456" }, cfg), + ).rejects.toThrow("Slack read target channel is not allowed."); + expect(listSlackReactions).not.toHaveBeenCalled(); + + await expect( + handleSlackAction({ action: "downloadFile", fileId: "F123", channelId: "C1" }, cfg), + ).rejects.toThrow("Slack read target channel is not allowed."); + expect(downloadSlackFile).not.toHaveBeenCalled(); + + await expect(handleSlackAction({ action: "listPins", channelId: "C1" }, cfg)).rejects.toThrow( + "Slack read target channel is not allowed.", + ); + expect(listSlackPins).not.toHaveBeenCalled(); + }); + + it("rejects Slack file downloads for non-allowlisted target channels", async () => { + const cfg = slackConfig({ + groupPolicy: "allowlist", + channels: { + C_ALLOWED: { enabled: true }, + }, + }); + + await expect( + handleSlackAction({ action: "downloadFile", fileId: "F123", channelId: "C_OTHER" }, cfg), + ).rejects.toThrow("Slack read target channel is not allowed."); + expect(downloadSlackFile).not.toHaveBeenCalled(); + }); + + it("rejects Slack pin reads for non-allowlisted target channels", async () => { + const cfg = slackConfig({ + groupPolicy: "allowlist", + channels: { + C_ALLOWED: { enabled: true }, + }, + }); + + await expect( + handleSlackAction({ action: "listPins", channelId: "C_OTHER" }, cfg), + ).rejects.toThrow("Slack read target channel is not allowed."); + expect(listSlackPins).not.toHaveBeenCalled(); + }); + it("passes messageId through to readSlackMessages", async () => { readSlackMessages.mockResolvedValueOnce({ messages: [], hasMore: false }); diff --git a/extensions/slack/src/action-runtime.ts b/extensions/slack/src/action-runtime.ts index ccf62ff7380..b648b5f3f57 100644 --- a/extensions/slack/src/action-runtime.ts +++ b/extensions/slack/src/action-runtime.ts @@ -1,8 +1,12 @@ import type { AgentToolResult } from "@earendil-works/pi-agent-core"; import { readBooleanParam } from "openclaw/plugin-sdk/boolean-param"; import { isSingleUseReplyToMode } from "openclaw/plugin-sdk/reply-reference"; +import { resolveOpenProviderRuntimeGroupPolicy } from "openclaw/plugin-sdk/runtime-group-policy"; import { normalizeLowercaseStringOrEmpty } from "openclaw/plugin-sdk/string-coerce-runtime"; +import type { ResolvedSlackAccount } from "./accounts.js"; import { parseSlackBlocksInput } from "./blocks-input.js"; +import { resolveSlackChannelConfig } from "./monitor/channel-config.js"; +import { isSlackChannelAllowedByPolicy } from "./monitor/policy.js"; import { createActionGate, imageResultFromFile, @@ -159,6 +163,42 @@ function isImageContentType(value: string | undefined): boolean { return value?.trim().toLowerCase().startsWith("image/") === true; } +function assertSlackReadTargetAllowed(params: { + account: ResolvedSlackAccount; + cfg: OpenClawConfig; + channelId: string; +}) { + const channels = params.account.config.channels; + const channelKeys = Object.keys(channels ?? {}); + const channelConfig = resolveSlackChannelConfig({ + channelId: params.channelId, + channels, + channelKeys, + allowNameMatching: params.account.config.dangerouslyAllowNameMatching, + defaultRequireMention: params.account.config.requireMention, + }); + const channelAllowed = channelConfig?.allowed !== false; + const { groupPolicy } = resolveOpenProviderRuntimeGroupPolicy({ + providerConfigPresent: params.cfg.channels?.slack !== undefined, + groupPolicy: params.account.config.groupPolicy, + defaultGroupPolicy: params.cfg.channels?.defaults?.groupPolicy, + }); + if ( + groupPolicy === "disabled" || + (groupPolicy === "allowlist" && + !isSlackChannelAllowedByPolicy({ + groupPolicy, + channelAllowlistConfigured: channelKeys.length > 0, + channelAllowed, + })) + ) { + throw new Error("Slack read target channel is not allowed."); + } + if (!channelAllowed && (groupPolicy !== "open" || channelConfig?.matchSource)) { + throw new Error("Slack read target channel is not allowed."); + } +} + export async function handleSlackAction( params: Record, cfg: OpenClawConfig, @@ -235,6 +275,7 @@ export async function handleSlackAction( } return jsonResult({ ok: true, added: emoji }); } + assertSlackReadTargetAllowed({ account, cfg, channelId }); const reactions = readOpts ? await slackActionRuntime.listSlackReactions(channelId, messageId, readOpts) : await slackActionRuntime.listSlackReactions(channelId, messageId); @@ -387,6 +428,7 @@ export async function handleSlackAction( } case "readMessages": { const channelId = resolveChannelId(); + assertSlackReadTargetAllowed({ account, cfg, channelId }); const limitRaw = params.limit; const limit = typeof limitRaw === "number" && Number.isFinite(limitRaw) ? limitRaw : undefined; @@ -412,8 +454,17 @@ export async function handleSlackAction( } case "downloadFile": { const fileId = readStringParam(params, "fileId", { required: true }); - const channelTarget = readStringParam(params, "channelId") ?? readStringParam(params, "to"); - const channelId = channelTarget ? resolveSlackChannelId(channelTarget) : undefined; + const channelTarget = + readStringParam(params, "channelId") ?? + readStringParam(params, "to") ?? + context?.currentChannelId; + if (!channelTarget) { + throw new Error( + "Slack file download requires channelId or to so the read target can be authorized.", + ); + } + const channelId = resolveSlackChannelId(channelTarget); + assertSlackReadTargetAllowed({ account, cfg, channelId }); const threadId = readStringParam(params, "threadId") ?? readStringParam(params, "replyTo"); const maxBytes = account.config?.mediaMaxMb ? account.config.mediaMaxMb * 1024 * 1024 @@ -488,6 +539,7 @@ export async function handleSlackAction( } return jsonResult({ ok: true }); } + assertSlackReadTargetAllowed({ account, cfg, channelId }); const pins = writeOpts ? await slackActionRuntime.listSlackPins(channelId, readOpts) : await slackActionRuntime.listSlackPins(channelId); diff --git a/extensions/slack/src/message-action-dispatch.test.ts b/extensions/slack/src/message-action-dispatch.test.ts index 2023080347c..0ece136b9cb 100644 --- a/extensions/slack/src/message-action-dispatch.test.ts +++ b/extensions/slack/src/message-action-dispatch.test.ts @@ -425,6 +425,32 @@ describe("handleSlackMessageAction", () => { expectForwardedCfg(invoke, cfg); }); + it("forwards tool context for current-channel download-file actions", async () => { + const invoke = createInvokeSpy(); + const cfg = slackConfig(); + const toolContext = { currentChannelId: "C1" }; + + await handleSlackMessageAction({ + providerId: "slack", + ctx: { + action: "download-file", + cfg, + toolContext, + params: { + fileId: "F123", + }, + } as never, + invoke: invoke as never, + }); + + const action = firstAction(invoke); + expect(action.action).toBe("downloadFile"); + expect(action.fileId).toBe("F123"); + expect(action.channelId).toBeUndefined(); + expectForwardedCfg(invoke, cfg); + expect(firstInvokeCall(invoke)[2]).toBe(toolContext); + }); + it("maps download-file target aliases to scope fields", async () => { const invoke = createInvokeSpy(); const cfg = slackConfig(); diff --git a/extensions/slack/src/message-action-dispatch.ts b/extensions/slack/src/message-action-dispatch.ts index 4e3c46aca1b..cfa9416bb4c 100644 --- a/extensions/slack/src/message-action-dispatch.ts +++ b/extensions/slack/src/message-action-dispatch.ts @@ -224,6 +224,7 @@ export async function handleSlackMessageAction(params: { accountId, }, cfg, + ctx.toolContext, ); }