From 5c045b8f6ca5d6ca682ea9a7e56bad68fe0d6143 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Thu, 7 May 2026 05:43:55 -0700 Subject: [PATCH] fix(discord): extend role-scope fix to slash surface + fixture update MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sibling-site fix: _evaluate_slash_authorization was the fourth _is_allowed_user caller and didn't pass guild/is_dm through, so slash interactions would take the DM branch regardless of whether they came from a guild channel. Now reads interaction.guild + in_dm and forwards. Also updates test_discord_slash_auth fixture (_make_interaction) so the SimpleNamespace guild mock has a get_member(uid)->None method — required by the new guild-scoped fallback path in _is_allowed_user. Tests exercising positive role paths still work via user.roles. Three new regression tests in test_discord_roles_dm_scope: - Slash DM + role in mutual public guild → rejected - Slash in guild B + role only in guild A → rejected - Slash in guild B + role in guild B → allowed (positive control) 368 Discord tests pass. test_discord_free_channel_skips_auto_thread also fails on clean main (pre-existing, unrelated to this fix). --- gateway/platforms/discord.py | 11 ++- tests/gateway/test_discord_roles_dm_scope.py | 90 ++++++++++++++++++++ tests/gateway/test_discord_slash_auth.py | 6 +- 3 files changed, 105 insertions(+), 2 deletions(-) diff --git a/gateway/platforms/discord.py b/gateway/platforms/discord.py index 0f2b0bbad6..c5b12e09c1 100644 --- a/gateway/platforms/discord.py +++ b/gateway/platforms/discord.py @@ -2305,7 +2305,16 @@ class DiscordAdapter(BasePlatformAdapter): return (True, None) user_id = str(user.id) - if not self._is_allowed_user(user_id, author=user): + # Pass guild + is_dm so role check is scoped to the originating + # guild and cross-guild DM bypass (#12136) can't land via the + # slash surface either. + interaction_guild = getattr(interaction, "guild", None) + if not self._is_allowed_user( + user_id, + author=user, + guild=interaction_guild, + is_dm=in_dm, + ): return ( False, "user not in DISCORD_ALLOWED_USERS / DISCORD_ALLOWED_ROLES", diff --git a/tests/gateway/test_discord_roles_dm_scope.py b/tests/gateway/test_discord_roles_dm_scope.py index a8c8561164..604b4e0aab 100644 --- a/tests/gateway/test_discord_roles_dm_scope.py +++ b/tests/gateway/test_discord_roles_dm_scope.py @@ -252,3 +252,93 @@ def test_empty_allowlists_allow_everyone(): adapter._is_allowed_user("42", author=None, guild=None, is_dm=True) is True ) + + +# --------------------------------------------------------------------------- +# Slash-surface sibling site: _evaluate_slash_authorization must pass +# guild/is_dm through so the cross-guild bypass can't land via slash either. +# --------------------------------------------------------------------------- + + +def test_slash_authorization_rejects_cross_guild_role_dm(monkeypatch): + """Slash interaction in a DM must not be authorized by a role held in + any mutual guild (parallel to the on_message cross-guild bypass).""" + import discord as _discord # type: ignore + monkeypatch.delenv("DISCORD_DM_ROLE_AUTH_GUILD", raising=False) + + public_guild, _ = _guild_with_member( + guild_id=111111, + member_id=42, + role_ids=[5555], + ) + adapter = _make_adapter( + allowed_roles=[5555], + guilds=[public_guild], + ) + + # Fake a DM interaction: user is Member-like, channel is DMChannel, + # interaction.guild is None. + interaction = SimpleNamespace( + user=SimpleNamespace(id=42), + channel=MagicMock(spec=_discord.DMChannel), + channel_id=None, + guild=None, + ) + + allowed, reason = adapter._evaluate_slash_authorization(interaction) + assert allowed is False + assert "ALLOWED" in (reason or "") + + +def test_slash_authorization_rejects_cross_guild_role_in_guild(monkeypatch): + """Slash in guild B must not be authorized by a role held in guild A.""" + monkeypatch.delenv("DISCORD_DM_ROLE_AUTH_GUILD", raising=False) + + public_guild, _ = _guild_with_member( + guild_id=111111, + member_id=42, + role_ids=[5555], + ) + # Interaction arrives in trusted_guild where user 42 has no role + trusted_guild = SimpleNamespace(id=222222, get_member=lambda uid: None) + adapter = _make_adapter( + allowed_roles=[5555], + guilds=[public_guild, trusted_guild], + ) + + interaction = SimpleNamespace( + user=SimpleNamespace(id=42), + channel=SimpleNamespace(id=9999), # not a DMChannel instance + channel_id=9999, + guild=trusted_guild, + ) + + allowed, reason = adapter._evaluate_slash_authorization(interaction) + assert allowed is False + assert "ALLOWED" in (reason or "") + + +def test_slash_authorization_allows_in_scope_guild_role(monkeypatch): + """Positive control: slash in guild B, user has role in guild B → allowed.""" + monkeypatch.delenv("DISCORD_DM_ROLE_AUTH_GUILD", raising=False) + + trusted_guild, _ = _guild_with_member( + guild_id=222222, + member_id=42, + role_ids=[5555], + ) + adapter = _make_adapter( + allowed_roles=[5555], + guilds=[trusted_guild], + ) + + interaction = SimpleNamespace( + user=SimpleNamespace(id=42), + channel=SimpleNamespace(id=9999), + channel_id=9999, + guild=trusted_guild, + ) + + allowed, reason = adapter._evaluate_slash_authorization(interaction) + assert allowed is True + assert reason is None diff --git a/tests/gateway/test_discord_slash_auth.py b/tests/gateway/test_discord_slash_auth.py index a52ee1fd7e..e51f240e3a 100644 --- a/tests/gateway/test_discord_slash_auth.py +++ b/tests/gateway/test_discord_slash_auth.py @@ -158,7 +158,11 @@ def _make_interaction( return SimpleNamespace( user=user_obj, - guild=SimpleNamespace(owner_id=999), + # `get_member` needed for the guild-scoped role fallback path in + # _is_allowed_user after the #12136 cross-guild fix. Fixture guild + # has no members by default — tests exercising positive role paths + # assign their own Member via user.roles + matching allowed_role_ids. + guild=SimpleNamespace(owner_id=999, id=guild_id, get_member=lambda uid: None), guild_id=guild_id, channel_id=channel_id, channel=channel,