From 4537ebc43a0fe57d83bc188ce0fed2358de1d1c0 Mon Sep 17 00:00:00 2001 From: Shadow Date: Tue, 10 Feb 2026 00:25:23 -0600 Subject: [PATCH] fix: enforce Discord agent component DM auth (#11254) (thanks @thedudeabidesai) --- CHANGELOG.md | 1 + src/config/types.discord.ts | 7 + src/discord/monitor/agent-components.test.ts | 99 ++++ src/discord/monitor/agent-components.ts | 525 +++++++++++++++++++ src/discord/monitor/provider.ts | 29 +- 5 files changed, 659 insertions(+), 2 deletions(-) create mode 100644 src/discord/monitor/agent-components.test.ts create mode 100644 src/discord/monitor/agent-components.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 6e26753ed9..5a57d4e4c1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -138,6 +138,7 @@ Docs: https://docs.openclaw.ai - Cron: deliver announce runs directly, honor delivery mode, and respect wakeMode for summaries. (#8540) Thanks @tyler6204. - Telegram: include forward_from_chat metadata in forwarded messages and harden cron delivery target checks. (#8392) Thanks @Glucksberg. - macOS: fix cron payload summary rendering and ISO 8601 formatter concurrency safety. +- Discord: enforce DM allowlists for agent components (buttons/select menus), honoring pairing store approvals and tag matches. (#11254) Thanks @thedudeabidesai. ## 2026.2.2-3 diff --git a/src/config/types.discord.ts b/src/config/types.discord.ts index a54d7d1d00..8aaf1bca00 100644 --- a/src/config/types.discord.ts +++ b/src/config/types.discord.ts @@ -97,6 +97,11 @@ export type DiscordExecApprovalConfig = { sessionFilter?: string[]; }; +export type DiscordAgentComponentsConfig = { + /** Enable agent-controlled interactive components (buttons, select menus). Default: true. */ + enabled?: boolean; +}; + export type DiscordAccountConfig = { /** Optional display name for this account (used in CLI/UI lists). */ name?: string; @@ -153,6 +158,8 @@ export type DiscordAccountConfig = { heartbeat?: ChannelHeartbeatVisibilityConfig; /** Exec approval forwarding configuration. */ execApprovals?: DiscordExecApprovalConfig; + /** Agent-controlled interactive components (buttons, select menus). */ + agentComponents?: DiscordAgentComponentsConfig; /** Privileged Gateway Intents (must also be enabled in Discord Developer Portal). */ intents?: DiscordIntentsConfig; /** PluralKit identity resolution for proxied messages. */ diff --git a/src/discord/monitor/agent-components.test.ts b/src/discord/monitor/agent-components.test.ts new file mode 100644 index 0000000000..f3b1e98c22 --- /dev/null +++ b/src/discord/monitor/agent-components.test.ts @@ -0,0 +1,99 @@ +import type { ButtonInteraction, ComponentData, StringSelectMenuInteraction } from "@buape/carbon"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import type { OpenClawConfig } from "../../config/config.js"; +import { createAgentComponentButton, createAgentSelectMenu } from "./agent-components.js"; + +const readAllowFromStoreMock = vi.hoisted(() => vi.fn()); +const upsertPairingRequestMock = vi.hoisted(() => vi.fn()); +const enqueueSystemEventMock = vi.hoisted(() => vi.fn()); + +vi.mock("../../pairing/pairing-store.js", () => ({ + readChannelAllowFromStore: (...args: unknown[]) => readAllowFromStoreMock(...args), + upsertChannelPairingRequest: (...args: unknown[]) => upsertPairingRequestMock(...args), +})); + +vi.mock("../../infra/system-events.js", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + enqueueSystemEvent: (...args: unknown[]) => enqueueSystemEventMock(...args), + }; +}); + +const createCfg = (): OpenClawConfig => ({}) as OpenClawConfig; + +const createDmButtonInteraction = (overrides: Partial = {}) => { + const reply = vi.fn().mockResolvedValue(undefined); + const interaction = { + rawData: { channel_id: "dm-channel" }, + user: { id: "123456789", username: "Alice", discriminator: "1234" }, + reply, + ...overrides, + } as unknown as ButtonInteraction; + return { interaction, reply }; +}; + +const createDmSelectInteraction = (overrides: Partial = {}) => { + const reply = vi.fn().mockResolvedValue(undefined); + const interaction = { + rawData: { channel_id: "dm-channel" }, + user: { id: "123456789", username: "Alice", discriminator: "1234" }, + values: ["alpha"], + reply, + ...overrides, + } as unknown as StringSelectMenuInteraction; + return { interaction, reply }; +}; + +beforeEach(() => { + readAllowFromStoreMock.mockReset().mockResolvedValue([]); + upsertPairingRequestMock.mockReset().mockResolvedValue({ code: "PAIRCODE", created: true }); + enqueueSystemEventMock.mockReset(); +}); + +describe("agent components", () => { + it("sends pairing reply when DM sender is not allowlisted", async () => { + const button = createAgentComponentButton({ + cfg: createCfg(), + accountId: "default", + dmPolicy: "pairing", + }); + const { interaction, reply } = createDmButtonInteraction(); + + await button.run(interaction, { componentId: "hello" } as ComponentData); + + expect(reply).toHaveBeenCalledTimes(1); + expect(reply.mock.calls[0]?.[0]?.content).toContain("Pairing code: PAIRCODE"); + expect(enqueueSystemEventMock).not.toHaveBeenCalled(); + }); + + it("allows DM interactions when pairing store allowlist matches", async () => { + readAllowFromStoreMock.mockResolvedValue(["123456789"]); + const button = createAgentComponentButton({ + cfg: createCfg(), + accountId: "default", + dmPolicy: "allowlist", + }); + const { interaction, reply } = createDmButtonInteraction(); + + await button.run(interaction, { componentId: "hello" } as ComponentData); + + expect(reply).toHaveBeenCalledWith({ content: "✓", ephemeral: true }); + expect(enqueueSystemEventMock).toHaveBeenCalled(); + }); + + it("matches tag-based allowlist entries for DM select menus", async () => { + const select = createAgentSelectMenu({ + cfg: createCfg(), + accountId: "default", + dmPolicy: "allowlist", + allowFrom: ["Alice#1234"], + }); + const { interaction, reply } = createDmSelectInteraction(); + + await select.run(interaction, { componentId: "hello" } as ComponentData); + + expect(reply).toHaveBeenCalledWith({ content: "✓", ephemeral: true }); + expect(enqueueSystemEventMock).toHaveBeenCalled(); + }); +}); diff --git a/src/discord/monitor/agent-components.ts b/src/discord/monitor/agent-components.ts new file mode 100644 index 0000000000..6d1939e3f9 --- /dev/null +++ b/src/discord/monitor/agent-components.ts @@ -0,0 +1,525 @@ +import type { APIStringSelectComponent } from "discord-api-types/v10"; +import { + Button, + type ButtonInteraction, + type ComponentData, + StringSelectMenu, + type StringSelectMenuInteraction, +} from "@buape/carbon"; +import { ButtonStyle, ChannelType } from "discord-api-types/v10"; +import type { OpenClawConfig } from "../../config/config.js"; +import { logVerbose } from "../../globals.js"; +import { enqueueSystemEvent } from "../../infra/system-events.js"; +import { logDebug, logError } from "../../logger.js"; +import { buildPairingReply } from "../../pairing/pairing-messages.js"; +import { + readChannelAllowFromStore, + upsertChannelPairingRequest, +} from "../../pairing/pairing-store.js"; +import { resolveAgentRoute } from "../../routing/resolve-route.js"; +import { + type DiscordGuildEntryResolved, + normalizeDiscordAllowList, + normalizeDiscordSlug, + resolveDiscordAllowListMatch, + resolveDiscordChannelConfigWithFallback, + resolveDiscordGuildEntry, + resolveDiscordUserAllowed, +} from "./allow-list.js"; +import { formatDiscordUserTag } from "./format.js"; + +const AGENT_BUTTON_KEY = "agent"; +const AGENT_SELECT_KEY = "agentsel"; + +type DiscordUser = Parameters[0]; + +type AgentComponentInteraction = ButtonInteraction | StringSelectMenuInteraction; + +export type AgentComponentContext = { + cfg: OpenClawConfig; + accountId: string; + guildEntries?: Record; + /** DM allowlist (from dm.allowFrom config) */ + allowFrom?: Array; + /** DM policy (default: "pairing") */ + dmPolicy?: "open" | "pairing" | "allowlist" | "disabled"; +}; + +/** + * Build agent button custom ID: agent:componentId= + * The channelId is NOT embedded in customId - we use interaction.rawData.channel_id instead + * to prevent channel spoofing attacks. + * + * Carbon's customIdParser parses "key:arg1=value1;arg2=value2" into { arg1: value1, arg2: value2 } + */ +export function buildAgentButtonCustomId(componentId: string): string { + return `${AGENT_BUTTON_KEY}:componentId=${encodeURIComponent(componentId)}`; +} + +/** + * Build agent select menu custom ID: agentsel:componentId= + */ +export function buildAgentSelectCustomId(componentId: string): string { + return `${AGENT_SELECT_KEY}:componentId=${encodeURIComponent(componentId)}`; +} + +/** + * Parse agent component data from Carbon's parsed ComponentData + * Carbon parses "key:componentId=xxx" into { componentId: "xxx" } + */ +function parseAgentComponentData(data: ComponentData): { + componentId: string; +} | null { + if (!data || typeof data !== "object") { + return null; + } + const componentId = + typeof data.componentId === "string" + ? decodeURIComponent(data.componentId) + : typeof data.componentId === "number" + ? String(data.componentId) + : null; + if (!componentId) { + return null; + } + return { componentId }; +} + +function formatUsername(user: { username: string; discriminator?: string | null }): string { + if (user.discriminator && user.discriminator !== "0") { + return `${user.username}#${user.discriminator}`; + } + return user.username; +} + +/** + * Check if a channel type is a thread type + */ +function isThreadChannelType(channelType: number | undefined): boolean { + return ( + channelType === ChannelType.PublicThread || + channelType === ChannelType.PrivateThread || + channelType === ChannelType.AnnouncementThread + ); +} + +async function ensureDmComponentAuthorized(params: { + ctx: AgentComponentContext; + interaction: AgentComponentInteraction; + user: DiscordUser; + componentLabel: string; +}): Promise { + const { ctx, interaction, user, componentLabel } = params; + const dmPolicy = ctx.dmPolicy ?? "pairing"; + if (dmPolicy === "disabled") { + logVerbose(`agent ${componentLabel}: blocked (DM policy disabled)`); + try { + await interaction.reply({ + content: "DM interactions are disabled.", + ephemeral: true, + }); + } catch { + // Interaction may have expired + } + return false; + } + if (dmPolicy === "open") { + return true; + } + + const storeAllowFrom = await readChannelAllowFromStore("discord").catch(() => []); + const effectiveAllowFrom = [...(ctx.allowFrom ?? []), ...storeAllowFrom]; + const allowList = normalizeDiscordAllowList(effectiveAllowFrom, ["discord:", "user:", "pk:"]); + const allowMatch = allowList + ? resolveDiscordAllowListMatch({ + allowList, + candidate: { + id: user.id, + name: user.username, + tag: formatDiscordUserTag(user), + }, + }) + : { allowed: false }; + if (allowMatch.allowed) { + return true; + } + + if (dmPolicy === "pairing") { + const { code, created } = await upsertChannelPairingRequest({ + channel: "discord", + id: user.id, + meta: { + tag: formatDiscordUserTag(user), + name: user.username, + }, + }); + try { + await interaction.reply({ + content: created + ? buildPairingReply({ + channel: "discord", + idLine: `Your Discord user id: ${user.id}`, + code, + }) + : "Pairing already requested. Ask the bot owner to approve your code.", + ephemeral: true, + }); + } catch { + // Interaction may have expired + } + return false; + } + + logVerbose(`agent ${componentLabel}: blocked DM user ${user.id} (not in allowFrom)`); + try { + await interaction.reply({ + content: `You are not authorized to use this ${componentLabel}.`, + ephemeral: true, + }); + } catch { + // Interaction may have expired + } + return false; +} + +export class AgentComponentButton extends Button { + label = AGENT_BUTTON_KEY; + customId = `${AGENT_BUTTON_KEY}:seed=1`; + style = ButtonStyle.Primary; + private ctx: AgentComponentContext; + + constructor(ctx: AgentComponentContext) { + super(); + this.ctx = ctx; + } + + async run(interaction: ButtonInteraction, data: ComponentData): Promise { + // Parse componentId from Carbon's parsed ComponentData + const parsed = parseAgentComponentData(data); + if (!parsed) { + logError("agent button: failed to parse component data"); + try { + await interaction.reply({ + content: "This button is no longer valid.", + ephemeral: true, + }); + } catch { + // Interaction may have expired + } + return; + } + + const { componentId } = parsed; + + // P1 FIX: Use interaction's actual channel_id instead of trusting customId + // This prevents channel ID spoofing attacks where an attacker crafts a button + // with a different channelId to inject events into other sessions + const channelId = interaction.rawData.channel_id; + if (!channelId) { + logError("agent button: missing channel_id in interaction"); + return; + } + + const user = interaction.user; + if (!user) { + logError("agent button: missing user in interaction"); + return; + } + + const username = formatUsername(user); + const userId = user.id; + + // P1 FIX: Use rawData.guild_id as source of truth - interaction.guild can be null + // when guild is not cached even though guild_id is present in rawData + const rawGuildId = interaction.rawData.guild_id; + const isDirectMessage = !rawGuildId; + + if (isDirectMessage) { + const authorized = await ensureDmComponentAuthorized({ + ctx: this.ctx, + interaction, + user, + componentLabel: "button", + }); + if (!authorized) { + return; + } + } + + // P2 FIX: Check user allowlist before processing component interaction + // This prevents unauthorized users from injecting system events + const guild = interaction.guild; + const guildInfo = resolveDiscordGuildEntry({ + guild: guild ?? undefined, + guildEntries: this.ctx.guildEntries, + }); + + // Resolve channel info for thread detection and allowlist inheritance + const channel = interaction.channel; + const channelName = channel && "name" in channel ? (channel.name as string) : undefined; + const channelSlug = channelName ? normalizeDiscordSlug(channelName) : ""; + const channelType = channel && "type" in channel ? (channel.type as number) : undefined; + const isThread = isThreadChannelType(channelType); + + // Resolve thread parent for allowlist inheritance + // Note: We can get parentId from channel but cannot fetch parent name without a client. + // The parentId alone enables ID-based parent config matching. Name-based matching + // requires the channel cache to have parent info available. + let parentId: string | undefined; + let parentName: string | undefined; + let parentSlug = ""; + if (isThread && channel && "parentId" in channel) { + parentId = (channel.parentId as string) ?? undefined; + // Try to get parent name from channel's parent if available + if ("parent" in channel) { + const parent = (channel as { parent?: { name?: string } }).parent; + if (parent?.name) { + parentName = parent.name; + parentSlug = normalizeDiscordSlug(parentName); + } + } + } + + // Only check guild allowlists if this is a guild interaction + if (rawGuildId) { + const channelConfig = resolveDiscordChannelConfigWithFallback({ + guildInfo, + channelId, + channelName, + channelSlug, + parentId, + parentName, + parentSlug, + scope: isThread ? "thread" : "channel", + }); + + const channelUsers = channelConfig?.users ?? guildInfo?.users; + if (Array.isArray(channelUsers) && channelUsers.length > 0) { + const userOk = resolveDiscordUserAllowed({ + allowList: channelUsers, + userId, + userName: user.username, + userTag: user.discriminator ? `${user.username}#${user.discriminator}` : undefined, + }); + if (!userOk) { + logVerbose(`agent button: blocked user ${userId} (not in allowlist)`); + try { + await interaction.reply({ + content: "You are not authorized to use this button.", + ephemeral: true, + }); + } catch { + // Interaction may have expired + } + return; + } + } + } + + // Resolve route with full context (guildId, proper peer kind, parentPeer) + const route = resolveAgentRoute({ + cfg: this.ctx.cfg, + channel: "discord", + accountId: this.ctx.accountId, + guildId: rawGuildId, + peer: { + kind: isDirectMessage ? "dm" : "channel", + id: isDirectMessage ? userId : channelId, + }, + parentPeer: parentId ? { kind: "channel", id: parentId } : undefined, + }); + + const eventText = `[Discord component: ${componentId} clicked by ${username} (${userId})]`; + + logDebug(`agent button: enqueuing event for channel ${channelId}: ${eventText}`); + + enqueueSystemEvent(eventText, { + sessionKey: route.sessionKey, + contextKey: `discord:agent-button:${channelId}:${componentId}:${userId}`, + }); + + // Acknowledge the interaction + try { + await interaction.reply({ + content: "✓", + ephemeral: true, + }); + } catch (err) { + logError(`agent button: failed to acknowledge interaction: ${String(err)}`); + } + } +} + +export class AgentSelectMenu extends StringSelectMenu { + customId = `${AGENT_SELECT_KEY}:seed=1`; + options: APIStringSelectComponent["options"] = []; + private ctx: AgentComponentContext; + + constructor(ctx: AgentComponentContext) { + super(); + this.ctx = ctx; + } + + async run(interaction: StringSelectMenuInteraction, data: ComponentData): Promise { + // Parse componentId from Carbon's parsed ComponentData + const parsed = parseAgentComponentData(data); + if (!parsed) { + logError("agent select: failed to parse component data"); + try { + await interaction.reply({ + content: "This select menu is no longer valid.", + ephemeral: true, + }); + } catch { + // Interaction may have expired + } + return; + } + + const { componentId } = parsed; + + // Use interaction's actual channel_id (trusted source from Discord) + // This prevents channel spoofing attacks + const channelId = interaction.rawData.channel_id; + if (!channelId) { + logError("agent select: missing channel_id in interaction"); + return; + } + + const user = interaction.user; + if (!user) { + logError("agent select: missing user in interaction"); + return; + } + + const username = formatUsername(user); + const userId = user.id; + + // P1 FIX: Use rawData.guild_id as source of truth - interaction.guild can be null + // when guild is not cached even though guild_id is present in rawData + const rawGuildId = interaction.rawData.guild_id; + const isDirectMessage = !rawGuildId; + + if (isDirectMessage) { + const authorized = await ensureDmComponentAuthorized({ + ctx: this.ctx, + interaction, + user, + componentLabel: "select menu", + }); + if (!authorized) { + return; + } + } + + // Check user allowlist before processing component interaction + const guild = interaction.guild; + const guildInfo = resolveDiscordGuildEntry({ + guild: guild ?? undefined, + guildEntries: this.ctx.guildEntries, + }); + + // Resolve channel info for thread detection and allowlist inheritance + const channel = interaction.channel; + const channelName = channel && "name" in channel ? (channel.name as string) : undefined; + const channelSlug = channelName ? normalizeDiscordSlug(channelName) : ""; + const channelType = channel && "type" in channel ? (channel.type as number) : undefined; + const isThread = isThreadChannelType(channelType); + + // Resolve thread parent for allowlist inheritance + let parentId: string | undefined; + let parentName: string | undefined; + let parentSlug = ""; + if (isThread && channel && "parentId" in channel) { + parentId = (channel.parentId as string) ?? undefined; + // Try to get parent name from channel's parent if available + if ("parent" in channel) { + const parent = (channel as { parent?: { name?: string } }).parent; + if (parent?.name) { + parentName = parent.name; + parentSlug = normalizeDiscordSlug(parentName); + } + } + } + + // Only check guild allowlists if this is a guild interaction + if (rawGuildId) { + const channelConfig = resolveDiscordChannelConfigWithFallback({ + guildInfo, + channelId, + channelName, + channelSlug, + parentId, + parentName, + parentSlug, + scope: isThread ? "thread" : "channel", + }); + + const channelUsers = channelConfig?.users ?? guildInfo?.users; + if (Array.isArray(channelUsers) && channelUsers.length > 0) { + const userOk = resolveDiscordUserAllowed({ + allowList: channelUsers, + userId, + userName: user.username, + userTag: user.discriminator ? `${user.username}#${user.discriminator}` : undefined, + }); + if (!userOk) { + logVerbose(`agent select: blocked user ${userId} (not in allowlist)`); + try { + await interaction.reply({ + content: "You are not authorized to use this select menu.", + ephemeral: true, + }); + } catch { + // Interaction may have expired + } + return; + } + } + } + + // Extract selected values + const values = interaction.values ?? []; + const valuesText = values.length > 0 ? ` (selected: ${values.join(", ")})` : ""; + + // Resolve route with full context (guildId, proper peer kind, parentPeer) + const route = resolveAgentRoute({ + cfg: this.ctx.cfg, + channel: "discord", + accountId: this.ctx.accountId, + guildId: rawGuildId, + peer: { + kind: isDirectMessage ? "dm" : "channel", + id: isDirectMessage ? userId : channelId, + }, + parentPeer: parentId ? { kind: "channel", id: parentId } : undefined, + }); + + const eventText = `[Discord select menu: ${componentId} interacted by ${username} (${userId})${valuesText}]`; + + logDebug(`agent select: enqueuing event for channel ${channelId}: ${eventText}`); + + enqueueSystemEvent(eventText, { + sessionKey: route.sessionKey, + contextKey: `discord:agent-select:${channelId}:${componentId}:${userId}`, + }); + + // Acknowledge the interaction + try { + await interaction.reply({ + content: "✓", + ephemeral: true, + }); + } catch (err) { + logError(`agent select: failed to acknowledge interaction: ${String(err)}`); + } + } +} + +export function createAgentComponentButton(ctx: AgentComponentContext): Button { + return new AgentComponentButton(ctx); +} + +export function createAgentSelectMenu(ctx: AgentComponentContext): StringSelectMenu { + return new AgentSelectMenu(ctx); +} diff --git a/src/discord/monitor/provider.ts b/src/discord/monitor/provider.ts index 5d9a986c26..eba27f10a6 100644 --- a/src/discord/monitor/provider.ts +++ b/src/discord/monitor/provider.ts @@ -1,4 +1,4 @@ -import { Client } from "@buape/carbon"; +import { Client, type BaseMessageInteractiveComponent } from "@buape/carbon"; import { GatewayIntents, GatewayPlugin } from "@buape/carbon/gateway"; import { Routes } from "discord-api-types/v10"; import { inspect } from "node:util"; @@ -26,6 +26,7 @@ import { fetchDiscordApplicationId } from "../probe.js"; import { resolveDiscordChannelAllowlist } from "../resolve-channels.js"; import { resolveDiscordUserAllowlist } from "../resolve-users.js"; import { normalizeDiscordToken } from "../token.js"; +import { createAgentComponentButton, createAgentSelectMenu } from "./agent-components.js"; import { createExecApprovalButton, DiscordExecApprovalHandler } from "./exec-approvals.js"; import { registerGateway, unregisterGateway } from "./gateway-registry.js"; import { @@ -474,7 +475,10 @@ export async function monitorDiscordProvider(opts: MonitorDiscordOpts = {}) { }) : null; - const components = [ + const agentComponentsConfig = discordCfg.agentComponents ?? {}; + const agentComponentsEnabled = agentComponentsConfig.enabled ?? true; + + const components: BaseMessageInteractiveComponent[] = [ createDiscordCommandArgFallbackButton({ cfg, discordConfig: discordCfg, @@ -487,6 +491,27 @@ export async function monitorDiscordProvider(opts: MonitorDiscordOpts = {}) { components.push(createExecApprovalButton({ handler: execApprovalsHandler })); } + if (agentComponentsEnabled) { + components.push( + createAgentComponentButton({ + cfg, + accountId: account.accountId, + guildEntries, + allowFrom, + dmPolicy, + }), + ); + components.push( + createAgentSelectMenu({ + cfg, + accountId: account.accountId, + guildEntries, + allowFrom, + dmPolicy, + }), + ); + } + const client = new Client( { baseUrl: "http://localhost",