fix(discord): honor commands.allowFrom in guild slash auth#38794
Conversation
There was a problem hiding this comment.
Pull request overview
Updates Discord guild slash-command authorization to honor commands.allowFrom.discord (and commands.allowFrom["*"]) by reusing the shared command authorization resolver, plus a regression test ensuring an allowlisted sender can run native commands in an allowlisted guild channel.
Changes:
- Add
resolveDiscordNativeCommandAllowlistAccess()and integrate it into guild slash-command gating as an additional authorizer. - Ensure guild slash-command auth uses the shared
resolveCommandAuthorization()behavior forcommands.allowFrom. - Add a regression test for
commands.allowFrom.discordin a guild channel.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/discord/monitor/native-command.ts | Adds a commands.allowFrom-backed authorizer to the guild slash-command authorization path via shared command-auth logic. |
| src/discord/monitor/native-command.commands-allowfrom.test.ts | Adds a vitest regression test covering an allowlisted Discord user invoking a native slash command in a guild channel. |
Greptile SummaryThis PR fixes a genuine authorization gap where The fix introduces Key observations:
Confidence Score: 4/5
Last reviewed commit: aaba93e |
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| } as OpenClawConfig; | ||
| } | ||
|
|
||
| describe("Discord native slash commands with commands.allowFrom", () => { | ||
| it("authorizes guild slash commands when commands.allowFrom.discord matches the sender", async () => { | ||
| const cfg = createConfig(); | ||
| const commandSpec: NativeCommandSpec = { | ||
| name: "status", | ||
| description: "Status", | ||
| acceptsArgs: false, | ||
| }; | ||
| const command = createDiscordNativeCommand({ | ||
| command: commandSpec, | ||
| cfg, | ||
| discordConfig: cfg.channels?.discord ?? {}, | ||
| accountId: "default", | ||
| sessionPrefix: "discord:slash", | ||
| ephemeralDefault: true, | ||
| threadBindings: createNoopThreadBindingManager("default"), | ||
| }); | ||
| 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<void> }).run(interaction as unknown); | ||
|
|
||
| expect(dispatchSpy).toHaveBeenCalledTimes(1); | ||
| expect(interaction.reply).not.toHaveBeenCalledWith( | ||
| expect.objectContaining({ content: "You are not authorized to use this command." }), | ||
| ); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Missing negative test case for rejection
The test only verifies the happy path: that an allowlisted sender is authorized. There is no assertion that a sender who is not in commands.allowFrom.discord still receives "You are not authorized to use this command." in the guild slash command path.
Without this, the test suite can't catch a regression that accidentally grants access to everyone (e.g., if the configured/allowed logic in resolveCommandAuthorizedFromAuthorizers breaks). Consider adding a second it block that creates a sender with a different userId (not in the allowlist), confirms dispatchSpy is not called, and confirms interaction.reply is called with the authorization-denied message.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/discord/monitor/native-command.commands-allowfrom.test.ts
Line: 76-122
Comment:
Missing negative test case for rejection
The test only verifies the happy path: that an allowlisted sender is authorized. There is no assertion that a sender who is *not* in `commands.allowFrom.discord` still receives `"You are not authorized to use this command."` in the guild slash command path.
Without this, the test suite can't catch a regression that accidentally grants access to everyone (e.g., if the `configured`/`allowed` logic in `resolveCommandAuthorizedFromAuthorizers` breaks). Consider adding a second `it` block that creates a sender with a different `userId` (not in the allowlist), confirms `dispatchSpy` is *not* called, and confirms `interaction.reply` is called with the authorization-denied message.
How can I resolve this? If you propose a fix, please make it concise.|
Addressed the review feedback in Changes:
Retested:
|
|
@thewilloftheshadow tagging you because Discord appears to be your domain based on the maintainer/contributing notes, and this fix is specifically in the native Discord slash-command auth path. If someone else is a better reviewer for this auth behavior, happy to retarget. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
25d1599 to
2b2f8bd
Compare
* origin/main: (40 commits) fix(discord): honor commands.allowFrom in guild slash auth (#38794) test: narrow pairing setup helper token type refactor(agents): share skill plugin fixture writer in tests refactor(auto-reply): share discord auth registry test fixture refactor(test-utils): share direct channel plugin test fixture refactor(commands): dedupe config-only channel status fixtures refactor(commands): dedupe channel plugin test fixture builders refactor(tui): dedupe mode-specific exec secret fixtures refactor(tui): dedupe gateway token resolution path refactor(memory): dedupe local embedding init concurrency fixtures refactor(feishu): dedupe non-streaming reply dispatcher setup refactor(feishu): dedupe accounts env secret-ref checks refactor(feishu): dedupe client timeout assertion scaffolding refactor(feishu): dedupe onboarding status env setup tests refactor(web): dedupe self-chat response-prefix tests refactor(pairing): dedupe inferred auth token fixtures refactor(gateway): dedupe blocked chat reply mock setup refactor(gateway): dedupe maintenance timer test setup refactor(gateway): dedupe legacy migration validation assertions refactor(gateway): dedupe probe route assertion loops ... # Conflicts: # extensions/feishu/src/onboarding.test.ts # extensions/googlechat/src/channel.outbound.test.ts # src/agents/minimax-vlm.normalizes-api-key.test.ts # src/pairing/setup-code.test.ts
…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 (openclaw#38794) (thanks @jskoiz) --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Shadow <hi@shadowing.dev>
…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 (openclaw#38794) (thanks @jskoiz) --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Shadow <hi@shadowing.dev>
…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 (openclaw#38794) (thanks @jskoiz) --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Shadow <hi@shadowing.dev>
…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 (openclaw#38794) (thanks @jskoiz) --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Shadow <hi@shadowing.dev>
…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 (openclaw#38794) (thanks @jskoiz) --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Shadow <hi@shadowing.dev>
…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 (openclaw#38794) (thanks @jskoiz) --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Shadow <hi@shadowing.dev>
…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 (openclaw#38794) (thanks @jskoiz) --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Shadow <hi@shadowing.dev> (cherry picked from commit 262fef6)
…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 (openclaw#38794) (thanks @jskoiz) --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Shadow <hi@shadowing.dev> (cherry picked from commit 262fef6)
…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 (openclaw#38794) (thanks @jskoiz) --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Shadow <hi@shadowing.dev>
…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 (openclaw#38794) (thanks @jskoiz) --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Shadow <hi@shadowing.dev>
…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 (openclaw#38794) (thanks @jskoiz) --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Shadow <hi@shadowing.dev>
Summary
commands.allowFrom.discordfor native Discord slash commands in guild channelsRoot cause
Discord native slash commands in guild channels only considered Discord owner/channel member allowlists during the pre-dispatch auth check. If
commands.allowFrom.discordwas configured without matchingchannels.discord.allowFromor guildusers/roles,/newand other native commands returnedYou are not authorized to use this command.even though the shared command auth layer would allow that sender.Fix
commands.allowFrom.discordmatch as a valid guild slash-command authorizerTest plan
pnpm test src/discord/monitor/native-command.commands-allowfrom.test.tspnpm buildpnpm checkcurrently fails on upstreammainwith an unrelated TypeScript error inextensions/feishu/src/monitor.test-mocks.tsFixes #19310
AI disclosure: This PR is AI-assisted. It has been tested locally with the commands above, and I understand what the code does.