From 262fef6ac8f57661151afc2a27df2835d747142f Mon Sep 17 00:00:00 2001 From: jsk Date: Sat, 7 Mar 2026 08:03:52 -1000 Subject: [PATCH] fix(discord): honor commands.allowFrom in guild slash auth (#38794) * fix(discord): honor commands.allowFrom in guild slash auth * Update native-command.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update native-command.commands-allowfrom.test.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * fix(discord): address slash auth review feedback * test(discord): add slash auth coverage for allowFrom variants * fix: add changelog entry for discord slash auth fix (#38794) (thanks @jskoiz) --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Shadow --- CHANGELOG.md | 1 + .../native-command.commands-allowfrom.test.ts | 245 ++++++++++++++++++ src/discord/monitor/native-command.ts | 70 ++++- 3 files changed, 315 insertions(+), 1 deletion(-) create mode 100644 src/discord/monitor/native-command.commands-allowfrom.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 50253478d38..cd26aeed655 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -244,6 +244,7 @@ Docs: https://docs.openclaw.ai - Auto-reply/allowlist store account scoping: keep `/allowlist ... --store` writes scoped to the selected account and clear legacy unscoped entries when removing default-account store access, preventing cross-account default allowlist bleed-through from legacy pairing-store reads. Thanks @tdjackey for reporting and @vincentkoc for the fix. - Security/Nostr: harden profile mutation/import loopback guards by failing closed on non-loopback forwarded client headers (`x-forwarded-for` / `x-real-ip`) and rejecting `sec-fetch-site: cross-site`; adds regression coverage for proxy-forwarded and browser cross-site mutation attempts. - CLI/bootstrap Node version hint maintenance: replace hardcoded nvm `22` instructions in `openclaw.mjs` with `MIN_NODE_MAJOR` interpolation so future minimum-Node bumps keep startup guidance in sync automatically. (#39056) Thanks @onstash. +- Discord/native slash command auth: honor `commands.allowFrom.discord` (and `commands.allowFrom["*"]`) in guild slash-command pre-dispatch authorization so allowlisted senders are no longer incorrectly rejected as unauthorized. (#38794) Thanks @jskoiz and @thewilloftheshadow. ## 2026.3.2 diff --git a/src/discord/monitor/native-command.commands-allowfrom.test.ts b/src/discord/monitor/native-command.commands-allowfrom.test.ts new file mode 100644 index 00000000000..e5757846b94 --- /dev/null +++ b/src/discord/monitor/native-command.commands-allowfrom.test.ts @@ -0,0 +1,245 @@ +import { ChannelType } from "@buape/carbon"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import type { NativeCommandSpec } from "../../auto-reply/commands-registry.js"; +import * as dispatcherModule from "../../auto-reply/reply/provider-dispatcher.js"; +import type { OpenClawConfig } from "../../config/config.js"; +import * as pluginCommandsModule from "../../plugins/commands.js"; +import { createDiscordNativeCommand } from "./native-command.js"; +import { createNoopThreadBindingManager } from "./thread-bindings.js"; + +type MockCommandInteraction = { + user: { id: string; username: string; globalName: string }; + channel: { type: ChannelType; id: string }; + guild: { id: string; name?: string } | null; + rawData: { id: string; member: { roles: string[] } }; + options: { + getString: ReturnType; + getNumber: ReturnType; + getBoolean: ReturnType; + }; + reply: ReturnType; + followUp: ReturnType; + client: object; +}; + +function createInteraction(params?: { + userId?: string; + channelId?: string; + guildId?: string; + guildName?: string; +}): MockCommandInteraction { + return { + user: { + id: params?.userId ?? "123456789012345678", + username: "discord-user", + globalName: "Discord User", + }, + channel: { + type: ChannelType.GuildText, + id: params?.channelId ?? "234567890123456789", + }, + guild: { + id: params?.guildId ?? "345678901234567890", + name: params?.guildName ?? "Test Guild", + }, + rawData: { + id: "interaction-1", + member: { roles: [] }, + }, + options: { + getString: vi.fn().mockReturnValue(null), + getNumber: vi.fn().mockReturnValue(null), + getBoolean: vi.fn().mockReturnValue(null), + }, + reply: vi.fn().mockResolvedValue({ ok: true }), + followUp: vi.fn().mockResolvedValue({ ok: true }), + client: {}, + }; +} + +function createConfig(): OpenClawConfig { + return { + commands: { + allowFrom: { + discord: ["user:123456789012345678"], + }, + }, + channels: { + discord: { + groupPolicy: "allowlist", + guilds: { + "345678901234567890": { + channels: { + "234567890123456789": { + allow: true, + requireMention: false, + }, + }, + }, + }, + }, + }, + } as OpenClawConfig; +} + +function createCommand(cfg: OpenClawConfig) { + const commandSpec: NativeCommandSpec = { + name: "status", + description: "Status", + acceptsArgs: false, + }; + return createDiscordNativeCommand({ + command: commandSpec, + cfg, + discordConfig: cfg.channels?.discord ?? {}, + accountId: "default", + sessionPrefix: "discord:slash", + ephemeralDefault: true, + threadBindings: createNoopThreadBindingManager("default"), + }); +} + +describe("Discord native slash commands with commands.allowFrom", () => { + beforeEach(() => { + vi.restoreAllMocks(); + }); + + it("authorizes guild slash commands when commands.allowFrom.discord matches the sender", async () => { + const cfg = createConfig(); + const command = createCommand(cfg); + const interaction = createInteraction(); + + vi.spyOn(pluginCommandsModule, "matchPluginCommand").mockReturnValue(null); + const dispatchSpy = vi + .spyOn(dispatcherModule, "dispatchReplyWithDispatcher") + .mockResolvedValue({ + counts: { + final: 1, + block: 0, + tool: 0, + }, + } as never); + + await (command as { run: (interaction: unknown) => Promise }).run(interaction as unknown); + + expect(dispatchSpy).toHaveBeenCalledTimes(1); + expect(interaction.reply).not.toHaveBeenCalledWith( + expect.objectContaining({ content: "You are not authorized to use this command." }), + ); + }); + + it("authorizes guild slash commands from the global commands.allowFrom list when provider-specific allowFrom is missing", async () => { + const cfg = createConfig(); + cfg.commands = { + allowFrom: { + "*": ["user:123456789012345678"], + }, + }; + const command = createCommand(cfg); + const interaction = createInteraction(); + + vi.spyOn(pluginCommandsModule, "matchPluginCommand").mockReturnValue(null); + const dispatchSpy = vi + .spyOn(dispatcherModule, "dispatchReplyWithDispatcher") + .mockResolvedValue({ + counts: { + final: 1, + block: 0, + tool: 0, + }, + } as never); + + await (command as { run: (interaction: unknown) => Promise }).run(interaction as unknown); + + expect(dispatchSpy).toHaveBeenCalledTimes(1); + expect(interaction.reply).not.toHaveBeenCalledWith( + expect.objectContaining({ content: "You are not authorized to use this command." }), + ); + }); + + it("authorizes guild slash commands when commands.useAccessGroups is false and commands.allowFrom.discord matches the sender", async () => { + const cfg = createConfig(); + cfg.commands = { + ...cfg.commands, + useAccessGroups: false, + }; + const command = createCommand(cfg); + const interaction = createInteraction(); + + vi.spyOn(pluginCommandsModule, "matchPluginCommand").mockReturnValue(null); + const dispatchSpy = vi + .spyOn(dispatcherModule, "dispatchReplyWithDispatcher") + .mockResolvedValue({ + counts: { + final: 1, + block: 0, + tool: 0, + }, + } as never); + + await (command as { run: (interaction: unknown) => Promise }).run(interaction as unknown); + + expect(dispatchSpy).toHaveBeenCalledTimes(1); + expect(interaction.reply).not.toHaveBeenCalledWith( + expect.objectContaining({ content: "You are not authorized to use this command." }), + ); + }); + + it("rejects guild slash commands when commands.allowFrom.discord does not match the sender", async () => { + const cfg = createConfig(); + const command = createCommand(cfg); + const interaction = createInteraction({ userId: "999999999999999999" }); + + vi.spyOn(pluginCommandsModule, "matchPluginCommand").mockReturnValue(null); + const dispatchSpy = vi + .spyOn(dispatcherModule, "dispatchReplyWithDispatcher") + .mockResolvedValue({ + counts: { + final: 1, + block: 0, + tool: 0, + }, + } as never); + + await (command as { run: (interaction: unknown) => Promise }).run(interaction as unknown); + + expect(dispatchSpy).not.toHaveBeenCalled(); + expect(interaction.reply).toHaveBeenCalledWith( + expect.objectContaining({ + content: "You are not authorized to use this command.", + ephemeral: true, + }), + ); + }); + + it("rejects guild slash commands when commands.useAccessGroups is false and commands.allowFrom.discord does not match the sender", async () => { + const cfg = createConfig(); + cfg.commands = { + ...cfg.commands, + useAccessGroups: false, + }; + const command = createCommand(cfg); + const interaction = createInteraction({ userId: "999999999999999999" }); + + vi.spyOn(pluginCommandsModule, "matchPluginCommand").mockReturnValue(null); + const dispatchSpy = vi + .spyOn(dispatcherModule, "dispatchReplyWithDispatcher") + .mockResolvedValue({ + counts: { + final: 1, + block: 0, + tool: 0, + }, + } as never); + + await (command as { run: (interaction: unknown) => Promise }).run(interaction as unknown); + + expect(dispatchSpy).not.toHaveBeenCalled(); + expect(interaction.reply).toHaveBeenCalledWith( + expect.objectContaining({ + content: "You are not authorized to use this command.", + ephemeral: true, + }), + ); + }); +}); diff --git a/src/discord/monitor/native-command.ts b/src/discord/monitor/native-command.ts index 18960697a40..401c30f01fe 100644 --- a/src/discord/monitor/native-command.ts +++ b/src/discord/monitor/native-command.ts @@ -20,6 +20,7 @@ import { } from "../../acp/persistent-bindings.route.js"; import { resolveHumanDelayConfig } from "../../agents/identity.js"; import { resolveChunkMode, resolveTextChunkLimit } from "../../auto-reply/chunk.js"; +import { resolveCommandAuthorization } from "../../auto-reply/command-auth.js"; import type { ChatCommandDefinition, CommandArgDefinition, @@ -92,6 +93,46 @@ import { resolveDiscordThreadParentInfo } from "./threading.js"; type DiscordConfig = NonNullable["discord"]; const log = createSubsystemLogger("discord/native-command"); +function resolveDiscordNativeCommandAllowlistAccess(params: { + cfg: OpenClawConfig; + accountId?: string | null; + sender: { id: string; name?: string; tag?: string }; + chatType: "direct" | "group" | "thread" | "channel"; + conversationId?: string; +}) { + const commandsAllowFrom = params.cfg.commands?.allowFrom; + if (!commandsAllowFrom || typeof commandsAllowFrom !== "object") { + return { configured: false, allowed: false } as const; + } + const configured = + Array.isArray(commandsAllowFrom.discord) || Array.isArray(commandsAllowFrom["*"]); + if (!configured) { + return { configured: false, allowed: false } as const; + } + + const from = + params.chatType === "direct" + ? `discord:${params.sender.id}` + : `discord:${params.chatType}:${params.conversationId ?? "unknown"}`; + const auth = resolveCommandAuthorization({ + ctx: { + Provider: "discord", + Surface: "discord", + OriginatingChannel: "discord", + AccountId: params.accountId ?? undefined, + ChatType: params.chatType, + From: from, + SenderId: params.sender.id, + SenderUsername: params.sender.name, + SenderTag: params.sender.tag, + }, + cfg: params.cfg, + // We only want explicit commands.allowFrom authorization here. + commandAuthorized: false, + }); + return { configured: true, allowed: auth.isAuthorizedSender } as const; +} + function buildDiscordCommandOptions(params: { command: ChatCommandDefinition; cfg: ReturnType; @@ -1297,6 +1338,23 @@ async function dispatchDiscordCommandInteraction(params: { }, allowNameMatching, }); + const commandsAllowFromAccess = resolveDiscordNativeCommandAllowlistAccess({ + cfg, + accountId, + sender: { + id: sender.id, + name: sender.name, + tag: sender.tag, + }, + chatType: isDirectMessage + ? "direct" + : isThreadChannel + ? "thread" + : interaction.guild + ? "channel" + : "group", + conversationId: rawChannelId || undefined, + }); const guildInfo = resolveDiscordGuildEntry({ guild: interaction.guild ?? undefined, guildEntries: discordConfig?.guilds, @@ -1418,10 +1476,20 @@ async function dispatchDiscordCommandInteraction(params: { }); const authorizers = useAccessGroups ? [ + { + configured: commandsAllowFromAccess.configured, + allowed: commandsAllowFromAccess.allowed, + }, { configured: ownerAllowList != null, allowed: ownerOk }, { configured: hasAccessRestrictions, allowed: memberAllowed }, ] - : [{ configured: hasAccessRestrictions, allowed: memberAllowed }]; + : [ + { + configured: commandsAllowFromAccess.configured, + allowed: commandsAllowFromAccess.allowed, + }, + { configured: hasAccessRestrictions, allowed: memberAllowed }, + ]; commandAuthorized = resolveCommandAuthorizedFromAuthorizers({ useAccessGroups, authorizers,