diff --git a/CHANGELOG.md b/CHANGELOG.md index 8421eea4f86..b1f0a5d9500 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -132,6 +132,7 @@ Docs: https://docs.openclaw.ai - Models/chat commands: keep `/model ...@YYYYMMDD` version suffixes intact by default, but still honor matching stored numeric auth-profile overrides for the same provider. (#48896) Thanks @Alix-007. - Gateway/channels: serialize per-account channel startup so overlapping starts do not boot the same provider twice, preventing MS Teams `EADDRINUSE` crash loops during startup and restart. (#49583) Thanks @sudie-codes. - Tests/OpenAI Codex auth: align login expectations with the default `gpt-5.4` model so CI coverage stays consistent with the current OpenAI Codex default. (#44367) Thanks @jrrcdev. +- Discord: enforce strict DM component allowlist auth (#49997) Thanks @joshavant. ### Fixes diff --git a/extensions/discord/src/monitor/agent-components-helpers.ts b/extensions/discord/src/monitor/agent-components-helpers.ts index a954c626111..eecbe73c351 100644 --- a/extensions/discord/src/monitor/agent-components-helpers.ts +++ b/extensions/discord/src/monitor/agent-components-helpers.ts @@ -429,6 +429,21 @@ async function ensureDmComponentAuthorized(params: { replyOpts: { ephemeral?: boolean }; }) { const { ctx, interaction, user, componentLabel, replyOpts } = params; + const allowFromPrefixes = ["discord:", "user:", "pk:"]; + const resolveAllowMatch = (entries: string[]) => { + const allowList = normalizeDiscordAllowList(entries, allowFromPrefixes); + return allowList + ? resolveDiscordAllowListMatch({ + allowList, + candidate: { + id: user.id, + name: user.username, + tag: formatDiscordUserTag(user), + }, + allowNameMatching: isDangerousNameMatchingEnabled(ctx.discordConfig), + }) + : { allowed: false }; + }; const dmPolicy = ctx.dmPolicy ?? "pairing"; if (dmPolicy === "disabled") { logVerbose(`agent ${componentLabel}: blocked (DM policy disabled)`); @@ -444,24 +459,27 @@ async function ensureDmComponentAuthorized(params: { return true; } + if (dmPolicy === "allowlist") { + const allowMatch = resolveAllowMatch(ctx.allowFrom ?? []); + if (allowMatch.allowed) { + return true; + } + logVerbose(`agent ${componentLabel}: blocked DM user ${user.id} (not in allowFrom)`); + try { + await interaction.reply({ + content: `You are not authorized to use this ${componentLabel}.`, + ...replyOpts, + }); + } catch {} + return false; + } + const storeAllowFrom = await readStoreAllowFromForDmPolicy({ provider: "discord", accountId: ctx.accountId, dmPolicy, }); - 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), - }, - allowNameMatching: isDangerousNameMatchingEnabled(ctx.discordConfig), - }) - : { allowed: false }; + const allowMatch = resolveAllowMatch([...(ctx.allowFrom ?? []), ...storeAllowFrom]); if (allowMatch.allowed) { return true; } diff --git a/extensions/discord/src/monitor/monitor.test.ts b/extensions/discord/src/monitor/monitor.test.ts index 84b36d74ec6..7f0dae736d7 100644 --- a/extensions/discord/src/monitor/monitor.test.ts +++ b/extensions/discord/src/monitor/monitor.test.ts @@ -191,10 +191,14 @@ describe("agent components", () => { expect(reply).toHaveBeenCalledTimes(1); expect(reply.mock.calls[0]?.[0]?.content).toContain("Pairing code: PAIRCODE"); expect(enqueueSystemEventMock).not.toHaveBeenCalled(); + expect(readAllowFromStoreMock).toHaveBeenCalledWith({ + provider: "discord", + accountId: "default", + dmPolicy: "pairing", + }); }); - it("blocks DM interactions when only pairing store entries match in allowlist mode", async () => { - readAllowFromStoreMock.mockResolvedValue(["123456789"]); + it("blocks DM interactions in allowlist mode when sender is not in configured allowFrom", async () => { const button = createAgentComponentButton({ cfg: createCfg(), accountId: "default", @@ -210,6 +214,62 @@ describe("agent components", () => { expect(readAllowFromStoreMock).not.toHaveBeenCalled(); }); + it("authorizes DM interactions from pairing-store entries in pairing mode", async () => { + readAllowFromStoreMock.mockResolvedValue(["123456789"]); + const button = createAgentComponentButton({ + cfg: createCfg(), + accountId: "default", + dmPolicy: "pairing", + }); + const { interaction, defer, reply } = createDmButtonInteraction(); + + await button.run(interaction, { componentId: "hello" } as ComponentData); + + expect(defer).toHaveBeenCalledWith({ ephemeral: true }); + expect(reply).toHaveBeenCalledWith({ content: "✓" }); + expect(enqueueSystemEventMock).toHaveBeenCalled(); + expect(upsertPairingRequestMock).not.toHaveBeenCalled(); + expect(readAllowFromStoreMock).toHaveBeenCalledWith({ + provider: "discord", + accountId: "default", + dmPolicy: "pairing", + }); + }); + + it("allows DM component interactions in open mode without reading pairing store", async () => { + readAllowFromStoreMock.mockResolvedValue(["123456789"]); + const button = createAgentComponentButton({ + cfg: createCfg(), + accountId: "default", + dmPolicy: "open", + }); + const { interaction, defer, reply } = createDmButtonInteraction(); + + await button.run(interaction, { componentId: "hello" } as ComponentData); + + expect(defer).toHaveBeenCalledWith({ ephemeral: true }); + expect(reply).toHaveBeenCalledWith({ content: "✓" }); + expect(enqueueSystemEventMock).toHaveBeenCalled(); + expect(readAllowFromStoreMock).not.toHaveBeenCalled(); + }); + + it("blocks DM component interactions in disabled mode without reading pairing store", async () => { + readAllowFromStoreMock.mockResolvedValue(["123456789"]); + const button = createAgentComponentButton({ + cfg: createCfg(), + accountId: "default", + dmPolicy: "disabled", + }); + const { interaction, defer, reply } = createDmButtonInteraction(); + + await button.run(interaction, { componentId: "hello" } as ComponentData); + + expect(defer).toHaveBeenCalledWith({ ephemeral: true }); + expect(reply).toHaveBeenCalledWith({ content: "DM interactions are disabled." }); + expect(enqueueSystemEventMock).not.toHaveBeenCalled(); + expect(readAllowFromStoreMock).not.toHaveBeenCalled(); + }); + it("matches tag-based allowlist entries for DM select menus", async () => { const select = createAgentSelectMenu({ cfg: createCfg(), @@ -225,6 +285,7 @@ describe("agent components", () => { expect(defer).toHaveBeenCalledWith({ ephemeral: true }); expect(reply).toHaveBeenCalledWith({ content: "✓" }); expect(enqueueSystemEventMock).toHaveBeenCalled(); + expect(readAllowFromStoreMock).not.toHaveBeenCalled(); }); it("accepts cid payloads for agent button interactions", async () => { @@ -244,6 +305,7 @@ describe("agent components", () => { expect.stringContaining("hello_cid"), expect.any(Object), ); + expect(readAllowFromStoreMock).not.toHaveBeenCalled(); }); it("keeps malformed percent cid values without throwing", async () => { @@ -263,6 +325,7 @@ describe("agent components", () => { expect.stringContaining("hello%2G"), expect.any(Object), ); + expect(readAllowFromStoreMock).not.toHaveBeenCalled(); }); }); diff --git a/extensions/discord/src/monitor/native-command-ui.ts b/extensions/discord/src/monitor/native-command-ui.ts index 778d8decc06..5c31e81ed8f 100644 --- a/extensions/discord/src/monitor/native-command-ui.ts +++ b/extensions/discord/src/monitor/native-command-ui.ts @@ -38,6 +38,7 @@ import { type DiscordModelPickerPreferenceScope, } from "./model-picker-preferences.js"; import { + DISCORD_MODEL_PICKER_CUSTOM_ID_KEY, loadDiscordModelPickerData, parseDiscordModelPickerData, renderDiscordModelPickerModelsView, @@ -949,7 +950,7 @@ class DiscordCommandArgFallbackButton extends Button { class DiscordModelPickerFallbackButton extends Button { label = "modelpick"; - customId = "modelpick:seed=btn"; + customId = `${DISCORD_MODEL_PICKER_CUSTOM_ID_KEY}:seed=btn`; private ctx: DiscordModelPickerContext; private safeInteractionCall: SafeDiscordInteractionCall; private dispatchCommandInteraction: DispatchDiscordCommandInteraction; @@ -977,7 +978,7 @@ class DiscordModelPickerFallbackButton extends Button { } class DiscordModelPickerFallbackSelect extends StringSelectMenu { - customId = "modelpick:seed=sel"; + customId = `${DISCORD_MODEL_PICKER_CUSTOM_ID_KEY}:seed=sel`; options = []; private ctx: DiscordModelPickerContext; private safeInteractionCall: SafeDiscordInteractionCall; diff --git a/extensions/discord/src/monitor/native-command.model-picker.test.ts b/extensions/discord/src/monitor/native-command.model-picker.test.ts index 0faba40c2d3..23b20ee0591 100644 --- a/extensions/discord/src/monitor/native-command.model-picker.test.ts +++ b/extensions/discord/src/monitor/native-command.model-picker.test.ts @@ -246,7 +246,12 @@ describe("Discord model picker interactions", () => { const select = createDiscordModelPickerFallbackSelect(context); expect(button.customId).not.toBe(select.customId); - expect(button.customId.split(":")[0]).toBe(select.customId.split(":")[0]); + expect(button.customId.split(":")[0]).toBe( + modelPickerModule.DISCORD_MODEL_PICKER_CUSTOM_ID_KEY, + ); + expect(select.customId.split(":")[0]).toBe( + modelPickerModule.DISCORD_MODEL_PICKER_CUSTOM_ID_KEY, + ); }); it("ignores interactions from users other than the picker owner", async () => {