From 7acb78852fdc96d32aee60a6fff9eb656766fbc8 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Wed, 29 Apr 2026 15:44:13 +0100 Subject: [PATCH] fix: keep Discord DM wildcard out of owner checks --- extensions/discord/src/monitor.test.ts | 17 ++++++++++ extensions/discord/src/monitor/allow-list.ts | 5 ++- .../native-command.commands-allowfrom.test.ts | 33 +++++++++++++++++++ .../discord/src/monitor/provider.test.ts | 2 ++ 4 files changed, 56 insertions(+), 1 deletion(-) diff --git a/extensions/discord/src/monitor.test.ts b/extensions/discord/src/monitor.test.ts index fc5eb15506c..540c1bea257 100644 --- a/extensions/discord/src/monitor.test.ts +++ b/extensions/discord/src/monitor.test.ts @@ -10,6 +10,7 @@ import { resolveDiscordChannelConfig, resolveDiscordChannelConfigWithFallback, resolveDiscordGuildEntry, + resolveDiscordOwnerAccess, resolveDiscordShouldRequireMention, resolveGroupDmAllow, shouldEmitDiscordReactionNotification, @@ -252,6 +253,22 @@ describe("discord allowlist helpers", () => { expect(allowListMatches(allow, { id: "member-123" })).toBe(true); expect(allowListMatches(allow, { id: "member-999" })).toBe(false); }); + + it("does not treat DM wildcard access as owner access", () => { + const wildcardOnly = resolveDiscordOwnerAccess({ + allowFrom: ["*"], + sender: { id: "123" }, + }); + expect(wildcardOnly.ownerAllowList).toBeNull(); + expect(wildcardOnly.ownerAllowed).toBe(false); + + const explicitOwner = resolveDiscordOwnerAccess({ + allowFrom: ["*", "user:123"], + sender: { id: "123" }, + }); + expect(explicitOwner.ownerAllowList).not.toBeNull(); + expect(explicitOwner.ownerAllowed).toBe(true); + }); }); describe("discord guild/channel resolution", () => { diff --git a/extensions/discord/src/monitor/allow-list.ts b/extensions/discord/src/monitor/allow-list.ts index bc5e558574e..692e9bb82ef 100644 --- a/extensions/discord/src/monitor/allow-list.ts +++ b/extensions/discord/src/monitor/allow-list.ts @@ -279,8 +279,11 @@ export function resolveDiscordOwnerAccess(params: { ownerAllowList: DiscordAllowList | null; ownerAllowed: boolean; } { + const ownerAllowFrom = params.allowFrom?.filter( + (entry) => (normalizeOptionalString(entry) ?? "") !== "*", + ); const ownerAllowList = normalizeDiscordAllowList( - params.allowFrom, + ownerAllowFrom && ownerAllowFrom.length > 0 ? ownerAllowFrom : undefined, DISCORD_OWNER_ALLOWLIST_PREFIXES, ); const ownerAllowed = ownerAllowList diff --git a/extensions/discord/src/monitor/native-command.commands-allowfrom.test.ts b/extensions/discord/src/monitor/native-command.commands-allowfrom.test.ts index 2ccca68faa8..ce2cda3415a 100644 --- a/extensions/discord/src/monitor/native-command.commands-allowfrom.test.ts +++ b/extensions/discord/src/monitor/native-command.commands-allowfrom.test.ts @@ -342,6 +342,39 @@ describe("Discord native slash commands with commands.allowFrom", () => { expectUnauthorizedReply(interaction); }); + it("does not treat open-DM wildcard access as guild command owner authorization", async () => { + const { dispatchSpy, interaction } = await runGuildSlashCommand({ + userId: "999999999999999999", + mutateConfig: (cfg) => { + cfg.commands = { + ...cfg.commands, + useAccessGroups: false, + allowFrom: undefined, + }; + cfg.channels = { + ...cfg.channels, + discord: { + ...cfg.channels?.discord, + dmPolicy: "open", + allowFrom: ["*"], + guilds: { + "000000000000000000": { + channels: { + "111111111111111111": { + enabled: true, + requireMention: false, + }, + }, + }, + }, + }, + }; + }, + }); + expect(dispatchSpy).not.toHaveBeenCalled(); + expectUnauthorizedReply(interaction); + }); + it("rejects guild slash commands when commands.allowFrom.discord does not match the sender", async () => { const { dispatchSpy, interaction } = await runGuildSlashCommand({ userId: "999999999999999999", diff --git a/extensions/discord/src/monitor/provider.test.ts b/extensions/discord/src/monitor/provider.test.ts index 8bcb1759db6..a5262e44bb9 100644 --- a/extensions/discord/src/monitor/provider.test.ts +++ b/extensions/discord/src/monitor/provider.test.ts @@ -174,6 +174,8 @@ describe("monitorDiscordProvider", () => { vi.doMock("../accounts.js", () => ({ resolveDiscordAccount: (...args: Parameters) => resolveDiscordAccountMock(...args), + resolveDiscordAccountAllowFrom: () => undefined, + resolveDiscordAccountDmPolicy: () => undefined, })); vi.doMock("../probe.js", () => ({ fetchDiscordApplicationId: async () => "app-1",