From afe64e2db75b755a77128db79bbf2b1521fe3b91 Mon Sep 17 00:00:00 2001 From: k-oc-agent Date: Wed, 16 Sep 2026 05:00:45 +0200 Subject: [PATCH] fix(discord): route slash commands from raw channel ids (#148852) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What Problem This Solves Discord can reject an allowed slash command when its payload contains `channel_id` but no hydrated channel. Autocomplete also loses choices, and the model picker can read the wrong session. ## Why This Change Was Made Pass the typed raw ID to the existing channel/parent resolver in all three native paths. Remove the extra fallback parser. This adds no state writer or access-policy mechanism. ## User Impact Allowed channels and parent-allowed threads use their intended sessions. Configured access restrictions and current-policy checks still apply. ## Evidence Boundary proof for the missing-channel branch: 21 registered-interaction cases pass after the fix. Main reproduces rejection, empty autocomplete choices, and the wrong picker session when the payload has `channel_id` but no hydrated channel. Controlled REST responses and session consumers cover allowed channels, parent-allowed threads, denied senders and parents, missing identity, and policy replacement. Live proof for the hydrated path: a real `/model` command in an allowed QA thread reached the production handler and saved the selection in that thread’s session. The parent session and configured default stayed unchanged. Discord supplied a hydrated channel in this capture; the private reply was not captured. @obviyus accepted this evidence split on September 16, 2026: boundary proof covers the missing-channel branch, and the live interaction proves the surrounding hydrated path. Run: `node scripts/run-vitest.mjs run extensions/discord/src/monitor/native-command.interaction-boundary.test.ts` Eight owner/sibling files pass, 186 tests, on a merge with main. Test ownership selects the new file exactly once. ## Compatibility No configuration, schema, dependency, protocol or public API changes. Production change: three one-line replacements. ## Consumers Native commands, autocomplete and model-picker interactions share existing authorization and routing. ## Invalidation Existing channel metadata caching and current-policy checks remain unchanged. ## Tests Registered interaction tests replace helper-only cases; existing partial-channel tests remain intact. Co-authored-by: Ayaan Zaidi --- .../src/monitor/native-command-auth.ts | 2 +- .../monitor/native-command-model-picker-ui.ts | 2 +- ...ative-command.interaction-boundary.test.ts | 449 ++++++++++++++++++ .../discord/src/monitor/native-command.ts | 2 +- .../test/discord-api-types-v10-runtime.ts | 1 + 5 files changed, 453 insertions(+), 3 deletions(-) create mode 100644 extensions/discord/src/monitor/native-command.interaction-boundary.test.ts diff --git a/extensions/discord/src/monitor/native-command-auth.ts b/extensions/discord/src/monitor/native-command-auth.ts index 1f17bd6dad2e..cf60fb8e736d 100644 --- a/extensions/discord/src/monitor/native-command-auth.ts +++ b/extensions/discord/src/monitor/native-command-auth.ts @@ -232,7 +232,7 @@ export async function resolveDiscordNativeAutocompleteAuthorized(params: { channel: interaction.channel, client: interaction.client, hasGuild: Boolean(interaction.guild), - channelIdFallback: "", + channelIdFallback: interaction.rawData.channel_id ?? "", }); if (params.isPolicyCurrent?.() === false) { return false; diff --git a/extensions/discord/src/monitor/native-command-model-picker-ui.ts b/extensions/discord/src/monitor/native-command-model-picker-ui.ts index 97d326848a8a..56b3e76f61d0 100644 --- a/extensions/discord/src/monitor/native-command-model-picker-ui.ts +++ b/extensions/discord/src/monitor/native-command-model-picker-ui.ts @@ -132,7 +132,7 @@ async function resolveDiscordModelPickerRouteState(params: { channel: interaction.channel, client: interaction.client, hasGuild: Boolean(interaction.guild), - channelIdFallback: "unknown", + channelIdFallback: interaction.rawData.channel_id ?? "unknown", }); const memberRoleIds = Array.isArray(interaction.rawData.member?.roles) ? interaction.rawData.member.roles.map((roleId: string) => roleId) diff --git a/extensions/discord/src/monitor/native-command.interaction-boundary.test.ts b/extensions/discord/src/monitor/native-command.interaction-boundary.test.ts new file mode 100644 index 000000000000..e53ab31f6599 --- /dev/null +++ b/extensions/discord/src/monitor/native-command.interaction-boundary.test.ts @@ -0,0 +1,449 @@ +import { + ApplicationCommandOptionType, + ChannelType, + GuildMemberFlags, + InteractionResponseType, + InteractionType, +} from "discord-api-types/v10"; +import type { OpenClawConfig } from "openclaw/plugin-sdk/config-contracts"; +import { createDeferred } from "openclaw/plugin-sdk/extension-shared"; +import * as sessionStore from "openclaw/plugin-sdk/session-store-runtime"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { + attachRestMock, + createInternalInteractionPayload, + createInternalComponentInteractionPayload, + createInternalTestClient, +} from "../internal/test-builders.test-support.js"; +import { createDiscordLivePolicyReader } from "./live-policy.js"; +import { clearDiscordChannelInfoCacheForTest } from "./message-channel-info.test-support.js"; +import * as pickerPreferences from "./model-picker-preferences.js"; +import * as pickerState from "./model-picker.state.js"; +import { createModelsProviderData } from "./model-picker.test-utils.js"; +import { + createDiscordModelPickerFallbackButton, + createDiscordNativeCommand, +} from "./native-command.js"; +import { nativeCommandRuntime } from "./native-command.runtime.js"; +import { createNoopThreadBindingManager } from "./thread-bindings.js"; + +const GUILD = "100000000000000001"; +const CHANNEL = "100000000000000002"; +const THREAD = "100000000000000003"; +const USER = "100000000000000004"; + +function createConfig(): OpenClawConfig { + return { + commands: { allowFrom: { discord: [`user:${USER}`] } }, + agents: { defaults: { model: { primary: "test-provider/test-model" } } }, + channels: { + discord: { + groupPolicy: "allowlist", + guilds: { [GUILD]: { channels: { [CHANNEL]: { enabled: true } } } }, + }, + }, + }; +} + +function createHarness() { + const cfg = createConfig(); + let currentConfig = cfg; + const readPolicy = createDiscordLivePolicyReader({ + cfg, + accountId: "default", + token: "test-token", + readConfig: () => currentConfig, + resolvedAllowlist: { guildEntries: cfg.channels?.discord?.guilds, allowFrom: [] }, + }); + const commandContext = { + cfg, + discordConfig: cfg.channels?.discord ?? {}, + readPolicy, + accountId: "default", + sessionPrefix: "discord:slash", + postApplySettleMs: 0, + threadBindings: createNoopThreadBindingManager("default"), + }; + const client = createInternalTestClient([ + createDiscordNativeCommand({ + ...commandContext, + ephemeralDefault: true, + command: { name: "status", description: "Status", acceptsArgs: false }, + }), + createDiscordNativeCommand({ + ...commandContext, + ephemeralDefault: true, + command: { + name: "boundary-choice", + description: "Choose", + acceptsArgs: true, + args: [ + { + name: "choice", + description: "Choice", + type: "string", + preferAutocomplete: true, + choices: ({ model }) => [model ?? "unresolved"], + }, + ], + }, + }), + ]); + client.componentHandler.register(createDiscordModelPickerFallbackButton(commandContext)); + const post = vi.fn(async () => undefined); + const get = vi.fn(async (path: string) => { + if (path === `/channels/${THREAD}`) { + return { id: THREAD, type: ChannelType.PublicThread, parent_id: CHANNEL, name: "topic" }; + } + if (path === `/channels/${CHANNEL}`) { + return { id: CHANNEL, type: ChannelType.GuildText, name: "allowed" }; + } + throw new Error(`Unexpected Discord GET ${path}`); + }); + const patch = vi.fn(async () => undefined); + attachRestMock(client, { post, get, patch }); + const session = vi.spyOn(sessionStore, "getSessionEntry").mockReturnValue(undefined); + vi.spyOn(pickerState, "loadDiscordModelPickerData").mockResolvedValue( + createModelsProviderData({ "test-provider": ["test-model"] }), + ); + vi.spyOn(pickerPreferences, "readDiscordModelPickerRecentModels").mockResolvedValue([]); + const dispatch = vi + .spyOn(nativeCommandRuntime, "dispatchChannelInboundTurn") + .mockImplementation(async () => { + throw new Error("Unexpected agent turn"); + }); + const status = vi + .spyOn(nativeCommandRuntime, "resolveDirectStatusReplyForSession") + .mockImplementation(async ({ sessionKey }) => ({ text: `Status for ${sessionKey}` })); + return { + client, + post, + get, + status, + dispatch, + session, + patch, + replacePolicy: () => { + currentConfig = { + ...cfg, + channels: { discord: { groupPolicy: "disabled" } }, + }; + }, + }; +} + +function payload(channelId: string, hydrated = false, userId = USER) { + return createInternalInteractionPayload({ + id: "interaction1", + token: "test-token", + guild_id: GUILD, + channel_id: channelId, + member: { + user: { id: userId, username: "tester", discriminator: "0", avatar: null, global_name: null }, + roles: [], + joined_at: "2026-01-01T00:00:00.000Z", + deaf: false, + mute: false, + permissions: "0", + flags: GuildMemberFlags.CompletedOnboarding, + }, + ...(hydrated ? { channel: { id: channelId, type: ChannelType.GuildText } } : {}), + data: { id: "command1", name: "status", type: 1 }, + }); +} + +function autocompletePayload(channelId: string, hydrated = false, userId = USER) { + return createInternalInteractionPayload({ + ...payload(channelId, hydrated, userId), + type: InteractionType.ApplicationCommandAutocomplete, + data: { + id: "command2", + name: "boundary-choice", + type: 1, + options: [ + { name: "choice", type: ApplicationCommandOptionType.String, value: "", focused: true }, + ], + }, + }); +} + +function pickerPayload(channelId: string, action: "back" | "reset" = "back", userId = USER) { + return createInternalComponentInteractionPayload({ + ...payload(channelId, false, userId), + data: { + custom_id: pickerState.buildDiscordModelPickerCustomId({ + command: "model", + action, + view: "providers", + userId, + }), + }, + }); +} + +function expectVisibleStatus(harness: ReturnType, channelId: string) { + const sessionKey = `agent:main:discord:channel:${channelId}`; + expect(harness.status, JSON.stringify(harness.post.mock.calls)).toHaveBeenCalledExactlyOnceWith( + expect.objectContaining({ sessionKey, channel: "discord", senderId: USER, isGroup: true }), + ); + expect(harness.post).toHaveBeenCalledWith( + "/webhooks/app1/test-token", + { + body: { content: `Status for ${sessionKey}`, flags: 64 }, + }, + undefined, + ); +} + +// Isolated boundary proof: REST is controlled; interaction construction, dispatch, +// access policy, conversation routing and response serialization are production code. +describe("Client.handleInteraction native command channel identity", () => { + beforeEach(() => clearDiscordChannelInfoCacheForTest()); + afterEach(() => vi.restoreAllMocks()); + + it("delivers status for a hydrated allowed channel", async () => { + const harness = createHarness(); + await harness.client.handleInteraction(payload(CHANNEL, true)); + expectVisibleStatus(harness, CHANNEL); + }); + + it.each([CHANNEL, THREAD])( + "delivers status for raw channel %s without hydration", + async (channelId) => { + const harness = createHarness(); + await harness.client.handleInteraction(payload(channelId)); + expectVisibleStatus(harness, channelId); + }, + ); + + it.each([true, false])( + "rejects a sender outside commands.allowFrom (hydrated=%s)", + async (hydrated) => { + const harness = createHarness(); + await harness.client.handleInteraction(payload(CHANNEL, hydrated, "100000000000000099")); + expect(harness.status).not.toHaveBeenCalled(); + expect(harness.post).toHaveBeenCalledWith( + "/webhooks/app1/test-token", + { + body: { content: "You are not authorized to use this command.", flags: 64 }, + }, + undefined, + ); + }, + ); + + it("rejects a thread whose parent is outside the allowlist", async () => { + const harness = createHarness(); + harness.get.mockResolvedValue({ + id: THREAD, + type: ChannelType.PublicThread, + parent_id: "denied", + name: "topic", + }); + await harness.client.handleInteraction(payload(THREAD)); + expect(harness.status).not.toHaveBeenCalled(); + expect(harness.post).toHaveBeenCalledWith( + "/webhooks/app1/test-token", + { + body: { content: "This channel is not allowed.", flags: 64 }, + }, + undefined, + ); + }); + + it("rejects missing channel identity under an allowlist", async () => { + const harness = createHarness(); + const interaction = payload(CHANNEL); + Reflect.deleteProperty(interaction, "channel_id"); + await harness.client.handleInteraction(interaction); + expect(harness.status).not.toHaveBeenCalled(); + expect(harness.post).toHaveBeenCalledWith( + "/webhooks/app1/test-token", + { + body: { content: "This channel is not allowed.", flags: 64 }, + }, + undefined, + ); + }); + + it.each([CHANNEL, THREAD])( + "autocompletes for raw channel %s through the registered option", + async (channelId) => { + const harness = createHarness(); + await harness.client.handleInteraction(autocompletePayload(channelId)); + expect(harness.post).toHaveBeenCalledWith("/interactions/interaction1/test-token/callback", { + body: { + type: InteractionResponseType.ApplicationCommandAutocompleteResult, + data: { choices: [{ name: "test-model", value: "test-model" }] }, + }, + }); + expect(harness.session).toHaveBeenCalledWith( + expect.objectContaining({ + sessionKey: `agent:main:discord:channel:${channelId}`, + }), + ); + }, + ); + + it.each([CHANNEL, THREAD])( + "opens the registered picker for the raw channel %s session", + async (channelId) => { + const harness = createHarness(); + await harness.client.handleInteraction(pickerPayload(channelId)); + expect(harness.session).toHaveBeenCalledWith( + expect.objectContaining({ + sessionKey: `agent:main:discord:channel:${channelId}`, + }), + ); + expect(harness.patch).toHaveBeenCalledWith( + "/webhooks/app1/test-token/messages/%40original", + expect.objectContaining({ + body: expect.objectContaining({ components: expect.any(Array) }), + }), + expect.anything(), + ); + expect(JSON.stringify(harness.patch.mock.calls)).toContain("test-provider"); + }, + ); + + it("rejects a policy replaced while the channel fetch is pending", async () => { + const harness = createHarness(); + const entered = createDeferred(); + const release = createDeferred(); + harness.get.mockImplementationOnce(async () => { + entered.resolve(); + await release.promise; + return { id: CHANNEL, type: ChannelType.GuildText, name: "allowed" }; + }); + const pending = harness.client.handleInteraction(payload(CHANNEL, true)); + await entered.promise; + harness.replacePolicy(); + release.resolve(); + await pending; + expect(harness.status).not.toHaveBeenCalled(); + expect(harness.post).toHaveBeenCalledWith( + "/webhooks/app1/test-token", + { + body: { content: "Access policy changed. Try this interaction again.", flags: 64 }, + }, + undefined, + ); + }); + + it.each(["sender", "parent", "identity"] as const)( + "denies raw autocomplete with denied %s", + async (denial) => { + const harness = createHarness(); + const interaction = autocompletePayload( + denial === "parent" ? THREAD : CHANNEL, + false, + denial === "sender" ? "100000000000000099" : USER, + ); + if (denial === "parent") { + harness.get.mockResolvedValue({ + id: THREAD, + type: ChannelType.PublicThread, + parent_id: "denied", + name: "topic", + }); + } + if (denial === "identity") { + Reflect.deleteProperty(interaction, "channel_id"); + } + await harness.client.handleInteraction(interaction); + expect(harness.post).toHaveBeenCalledExactlyOnceWith( + "/interactions/interaction1/test-token/callback", + { + body: { + type: InteractionResponseType.ApplicationCommandAutocompleteResult, + data: { choices: [] }, + }, + }, + ); + expect(harness.session).not.toHaveBeenCalled(); + }, + ); + + it.each(["sender", "parent", "identity"] as const)( + "denies raw picker selection with denied %s", + async (denial) => { + const harness = createHarness(); + const interaction = pickerPayload( + denial === "parent" ? THREAD : CHANNEL, + "reset", + denial === "sender" ? "100000000000000099" : USER, + ); + if (denial === "parent") { + harness.get.mockResolvedValue({ + id: THREAD, + type: ChannelType.PublicThread, + parent_id: "denied", + name: "topic", + }); + } + if (denial === "identity") { + Reflect.deleteProperty(interaction, "channel_id"); + } + await harness.client.handleInteraction(interaction); + expect(harness.dispatch).not.toHaveBeenCalled(); + expect(JSON.stringify(harness.post.mock.calls)).toContain( + "Failed to apply test-provider/test-model", + ); + expect(JSON.stringify(harness.post.mock.calls)).toContain( + denial === "sender" ? "not authorized" : "not allowed", + ); + }, + ); + + it.each(["status", "autocomplete", "picker"] as const)( + "denies raw %s when policy changes during the channel fetch", + async (surface) => { + const harness = createHarness(); + const entered = createDeferred(); + const release = createDeferred(); + harness.get.mockImplementationOnce(async () => { + entered.resolve(); + await release.promise; + return { id: CHANNEL, type: ChannelType.GuildText, name: "allowed" }; + }); + const interaction = + surface === "status" + ? payload(CHANNEL) + : surface === "autocomplete" + ? autocompletePayload(CHANNEL) + : pickerPayload(CHANNEL, "reset"); + const pending = harness.client.handleInteraction(interaction); + try { + const fetched = await Promise.race([ + entered.promise.then(() => true), + pending.then(() => false), + ]); + expect(fetched).toBe(true); + harness.replacePolicy(); + } finally { + release.resolve(); + await pending; + } + expect(harness.status).not.toHaveBeenCalled(); + expect(harness.dispatch).not.toHaveBeenCalled(); + if (surface === "autocomplete") { + expect(harness.session).not.toHaveBeenCalled(); + expect(harness.post).toHaveBeenCalledExactlyOnceWith( + "/interactions/interaction1/test-token/callback", + { + body: { + type: InteractionResponseType.ApplicationCommandAutocompleteResult, + data: { choices: [] }, + }, + }, + ); + } else { + expect(JSON.stringify(harness.post.mock.calls)).toContain( + surface === "status" + ? "Access policy changed" + : "Failed to apply test-provider/test-model", + ); + } + }, + ); +}); diff --git a/extensions/discord/src/monitor/native-command.ts b/extensions/discord/src/monitor/native-command.ts index 8acbc7fbc07b..fbd0daaa9f48 100644 --- a/extensions/discord/src/monitor/native-command.ts +++ b/extensions/discord/src/monitor/native-command.ts @@ -320,7 +320,7 @@ async function dispatchDiscordCommandInteraction(params: { channel, client: interaction.client, hasGuild: Boolean(interaction.guild), - channelIdFallback: "", + channelIdFallback: interaction.rawData.channel_id ?? "", }); if (policy?.isCurrent() === false) { await respond("Access policy changed. Try this interaction again.", { ephemeral: true }); diff --git a/extensions/discord/test/discord-api-types-v10-runtime.ts b/extensions/discord/test/discord-api-types-v10-runtime.ts index b8f19ef78e68..b844fdcf7a09 100644 --- a/extensions/discord/test/discord-api-types-v10-runtime.ts +++ b/extensions/discord/test/discord-api-types-v10-runtime.ts @@ -16,6 +16,7 @@ export const { GatewayDispatchEvents, GatewayIntentBits, GatewayOpcodes, + GuildMemberFlags, InteractionContextType, InteractionResponseType, InteractionType,