Discord role-based access control bugs and fixes

Fixes on record

Merged to main. A stamp counts the issues a fix is linked to.

• #11608 feat(discord): DISCORD_ALLOWED_ROLES for role-based access control

  merged to main

  files: gateway/platforms/discord.py, gateway/run.py, scripts/release.py, tests/gateway/test_discord_bot_auth_bypass.py

  Show the diff

  • gateway/platforms/discord.py

    @@ -495,6 +495,7 @@ def __init__(self, config: PlatformConfig):
             self._client: Optional[commands.Bot] = None
             self._ready_event = asyncio.Event()
             self._allowed_user_ids: set = set()  # For button approval authorization
    +        self._allowed_role_ids: set = set()  # For DISCORD_ALLOWED_ROLES filtering
             # Voice channel state (per-guild)
             self._voice_clients: Dict[int, Any] = {}  # guild_id -> VoiceClient
             # Text batching: merge rapid successive messages (Telegram-style)
    @@ -573,6 +574,15 @@ async def connect(self) -> bool:
                         if uid.strip()
                     }
     
    +            # Parse DISCORD_ALLOWED_ROLES — comma-separated role IDs.
    +            # Users with ANY of these roles can interact with the bot.
    +            roles_env = os.getenv("DISCORD_ALLOWED_ROLES", "")
    +            if roles_env:
    +                self._allowed_role_ids = {
    +                    int(rid.strip()) for rid in roles_env.split(",")
    +                    if rid.strip().isdigit()
    +                }
    +
                 # Set up intents.
                 # Message Content is required for normal text replies.
                 # Server Members is only needed when the allowlist contains usernames
    @@ -584,7 +594,10 @@ async def connect(self) -> bool:
                 intents.message_content = True
                 intents.dm_messages = True
                 intents.guild_messages = True
    -            intents.members = any(not entry.isdigit() for entry in self._allowed_user_ids)
    +            intents.members = (
    +                any(not entry.isdigit() for entry in self._allowed_user_ids)
    +                or bool(self._allowed_role_ids)  # Need members intent for role lookup
    +            )
                 intents.voice_states = True
     
                 # Resolve proxy (DISCORD_PROXY > generic env vars > macOS system proxy)
    @@ -653,8 +666,8 @@ async def on_message(message: DiscordMessage):
                         # "all" falls through; bot is permitted — skip the
                         # human-user allowlist below (bots aren't in it).
                     else:
    -                    # Non-bot: enforce the configured user allowlist.
    -                    if not self._is_allowed_user(str(message.author.id)):
    +                    # Non-bot: enforce the configured user/role allowlists.
    +                    if not self._is_allowed_user(str(message.author.id), message.author):
                             return
                     
                     # Multi-agent filtering: if the message mentions specific bots
    @@ -1365,11 +1378,48 @@ async def _process_voice_input(self, guild_id: int, user_id: int, pcm_data: byte
                 except OSError:
                     pass
     
    -    def _is_allowed_user(self, user_id: str) -> bool:
    -        """Check if user is in DISCORD_ALLOWED_USERS."""
    -        if not self._allowed_user_ids:
    +    def _is_allowed_user(self, user_id: str, author=None) -> bool:
    +        """Check if user is allowed via DISCORD_ALLOWED_USERS or DISCORD_ALLOWED_ROLES.
    +
    +        Uses OR semantics: if the user matches EITHER allowlist, they're allowed.
    +        If both allowlists are empty, everyone is allowed (backwards compatible).
    +        When author is a Member, checks .roles directly; otherwise falls back
    +        to scanning the bot's mutual guilds for a Member record.
    +        """
    +        # ``getattr`` fallbacks here guard against test fixtures that build
    +        # an adapter via ``object.__new__(DiscordAdapter)`` and skip __init__
    +        # (see AGENTS.md pitfall #17 — same pattern as gateway.run).
    +        allowed_users = getattr(self, "_allowed_user_ids", set())
    +        allowed_roles = getattr(self, "_allowed_role_ids", set())
    +        has_users = bool(allowed_users)
    +        has_roles = bool(allowed_roles)
    +        if not has_users and not has_roles:
    +            return True
    +        # Check user ID allowlist
    +        if has_users and user_id in allowed_users:
                 return True
    -        return user_id in self._allowed_user_ids
    +        # Check role allowlist
    +        if has_roles:
    +            # Try direct role check from Member object
    +            direct_roles = getattr(author, "roles", None) if author is not None else None
    +            if direct_roles:
    +                if any(getattr(r, "id", None) in allowed_roles for r in direct_roles):
    +                    return True
    +            # Fallback: scan mutual guilds for member's roles
    +            if self._client is not None:
    +                try:
    +                    uid_int = int(user_id)
    +                except (TypeError, ValueError):
    +                    uid_int = None
    +                if uid_int is not None:
    +                    for guild in self._client.guilds:
    +                        m = guild.get_member(uid_int)
    +                        if m is None:
    +                            continue
    +                        m_roles = getattr(m, "roles", None) or []
    +                        if any(getattr(r, "id", None) in allowed_roles for r in m_roles):
    +                            return True
    +        return False
     
         async def send_image_file(
             self,

  • gateway/run.py

    @@ -2655,6 +2655,18 @@ def _is_user_authorized(self, source: SessionSource) -> bool:
                 if allow_bots in ("mentions", "all"):
                     return True
     
    +        # Discord role-based access (DISCORD_ALLOWED_ROLES): the adapter's
    +        # on_message pre-filter already verified role membership — if the
    +        # message reached here, the user passed that check. Authorize
    +        # directly to avoid the "no allowlists configured" branch below
    +        # rejecting role-only setups where DISCORD_ALLOWED_USERS is empty
    +        # (issue #7871).
    +        if (
    +            source.platform == Platform.DISCORD
    +            and os.getenv("DISCORD_ALLOWED_ROLES", "").strip()
    +        ):
    +            return True
    +
             # Check pairing store (always checked, regardless of allowlists)
             platform_name = source.platform.value if source.platform else ""
             if self.pairing_store.is_approved(platform_name, user_id):

  • scripts/release.py

    @@ -237,6 +237,7 @@
         "[redacted:email]": "malaiwah",
         "[redacted:email]": "malaiwah",
         "[redacted:email]": "gnanam1990",
    +    "[redacted:email]": "0xyg3n",
         "[redacted:email]": "v1k22",
         "[redacted:email]": "RucchiZ",
         "[redacted:email]": "LehaoLin",

  • tests/gateway/test_discord_bot_auth_bypass.py

    @@ -22,6 +22,23 @@
     from gateway.session import Platform, SessionSource
     
     
    [redacted:email](autouse=True)
    +def _isolate_discord_env(monkeypatch):
    +    """Make every test start with a clean Discord env so prior tests in the
    +    session (or CI setups) can't leak DISCORD_ALLOWED_ROLES / DISCORD_ALLOWED_USERS
    +    / DISCORD_ALLOW_BOTS and silently flip the auth result.
    +    """
    +    for var in (
    +        "DISCORD_ALLOW_BOTS",
    +        "DISCORD_ALLOWED_USERS",
    +        "DISCORD_ALLOWED_ROLES",
    +        "DISCORD_ALLOW_ALL_USERS",
    +        "GATEWAY_ALLOW_ALL_USERS",
    +        "GATEWAY_ALLOWED_USERS",
    +    ):
    +        monkeypatch.delenv(var, raising=False)
    +
    +
     # -----------------------------------------------------------------------------
     # Gate 2: _is_user_authorized bypasses allowlist for permitted bots
     # -----------------------------------------------------------------------------
    @@ -152,3 +169,58 @@ def test_bot_bypass_does_not_leak_to_other_platforms(monkeypatch):
             is_bot=True,
         )
         assert runner._is_user_authorized(telegram_bot) is False
    +
    +
    +# -----------------------------------------------------------------------------
    +# DISCORD_ALLOWED_ROLES gateway-layer bypass (#7871)
    +# -----------------------------------------------------------------------------
    +
    +
    +def test_discord_role_config_bypasses_gateway_allowlist(monkeypatch):
    +    """When DISCORD_ALLOWED_ROLES is set, _is_user_authorized must trust
    +    the adapter's pre-filter and authorize. Without this, role-only setups
    +    (DISCORD_ALLOWED_ROLES populated, DISCORD_ALLOWED_USERS empty) would
    +    hit the 'no allowlists configured' branch and get rejected.
    +    """
    +    runner = _make_bare_runner()
    +
    +    monkeypatch.setenv("DISCORD_ALLOWED_ROLES", "1493705176387948674")
    +    # Note: DISCORD_ALLOWED_USERS is NOT set — the entire point.
    +
    +    source = _make_discord_human_source(user_id="999888777")
    +    assert runner._is_user_authorized(source) is True
    +
    +
    +def test_discord_role_config_still_authorizes_alongside_users(monkeypatch):
    +    """Sanity: setting both DISCORD_ALLOWED_ROLES and DISCORD_ALLOWED_USERS
    +    doesn't break the user-id path. Users in the allowlist should still be
    +    authorized even if they don't have a role. (OR semantics.)
    +    """
    +    runner = _make_bare_runner()
    +
    +    monkeypatch.setenv("DISCORD_ALLOWED_ROLES", "1493705176387948674")
    +    monkeypatch.setenv("DISCORD_ALLOWED_USERS", "100200300")
    +
    +    # User on the user allowlist, no role → still authorized at gateway
    +    # level via the role bypass (adapter already approved them).
    +    source = _make_discord_human_source(user_id="100200300")
    +    assert runner._is_user_authorized(source) is True
    +
    +
    +def test_discord_role_bypass_does_not_leak_to_other_platforms(monkeypatch):
    +    """DISCORD_ALLOWED_ROLES must only affect Discord. Setting it should
    +    not suddenly start authorizing Telegram users whose platform has its
    +    own empty allowlist.
    +    """
    +    runner = _make_bare_runner()
    +
    +    monkeypatch.setenv("DISCORD_ALLOWED_ROLES", "1493705176387948674")
    +    # Telegram has its own empty allowlist and no allow-all flag.
    +
    +    telegram_user = SessionSource(
    +        platform=Platform.TELEGRAM,
    +        chat_id="123",
    +        chat_type="channel",
    +        user_id="999888777",
    +    )
    +    assert runner._is_user_authorized(telegram_user) is False

• #43327 fix(discord): propagate role_authorized flag so DISCORD_ALLOWED_ROLES works end-to-end

  merged to main

  files: gateway/authz_mixin.py, gateway/platforms/base.py, gateway/session.py, plugins/platforms/discord/adapter.py, scripts/release.py

  Show the diff

  • gateway/authz_mixin.py

    @@ -207,6 +207,14 @@ def _is_user_authorized(self, source: SessionSource) -> bool:
             if platform_allow_all_var and os.getenv(platform_allow_all_var, "").lower() in {"true", "1", "yes"}:
                 return True
     
    +        # Adapter-verified role auth: the Discord adapter already confirmed the
    +        # user holds a role in DISCORD_ALLOWED_ROLES before dispatching the message.
    +        # Compare with ``is True`` so the real bool field authorizes while a
    +        # MagicMock source (test fixtures using ``object.__new__`` runners with
    +        # mock sources) does not auto-truthy through this gate (see pitfall #13).
    +        if getattr(source, "role_authorized", False) is True:
    +            return True
    +
             if getattr(source, "is_bot", False):
                 allow_bots_var = platform_allow_bots_map.get(source.platform)
                 if allow_bots_var and os.getenv(allow_bots_var, "none").lower().strip() in {"mentions", "all"}:

  • gateway/platforms/base.py

    @@ -4651,6 +4651,7 @@ def build_source(
             guild_id: Optional[str] = None,
             parent_chat_id: Optional[str] = None,
             message_id: Optional[str] = None,
    +        role_authorized: bool = False,
         ) -> SessionSource:
             """Helper to build a SessionSource for this platform."""
             # Normalize empty topic to None
    @@ -4671,6 +4672,7 @@ def build_source(
                 guild_id=str(guild_id) if guild_id else None,
                 parent_chat_id=str(parent_chat_id) if parent_chat_id else None,
                 message_id=str(message_id) if message_id else None,
    +            role_authorized=role_authorized,
             )
         
         @abstractmethod

  • gateway/session.py

    @@ -91,6 +91,7 @@ class SessionSource:
         guild_id: Optional[str] = None  # Discord guild / Slack workspace / Matrix server scope
         parent_chat_id: Optional[str] = None  # Parent channel when chat_id refers to a thread
         message_id: Optional[str] = None  # ID of the triggering message (for pin/reply/react)
    +    role_authorized: bool = False  # True when adapter granted access via role (not user ID)
         
         @property
         def description(self) -> str:

  • plugins/platforms/discord/adapter.py

    @@ -789,6 +789,7 @@ async def on_message(message: DiscordMessage):
                     # Must run BEFORE the user allowlist check so that bots
                     # permitted by DISCORD_ALLOW_BOTS are not rejected for
                     # not being in DISCORD_ALLOWED_USERS (fixes #4466).
    +                _role_authorized = False
                     if getattr(message.author, "bot", False):
                         allow_bots = os.getenv("DISCORD_ALLOW_BOTS", "none").lower().strip()
                         if allow_bots == "none":
    @@ -812,6 +813,7 @@ async def on_message(message: DiscordMessage):
                             is_dm=_is_dm,
                         ):
                             return
    +                    _role_authorized = bool(getattr(self, "_allowed_role_ids", set()))
                     
                     # Multi-agent filtering: if the message mentions specific bots
                     # but NOT this bot, the sender is talking to another agent —
    @@ -853,7 +855,7 @@ async def on_message(message: DiscordMessage):
                             if "*" not in _free_channels and not (_channel_ids & _free_channels):
                                 return
     
    -                await self._handle_message(message)
    +                await self._handle_message(message, role_authorized=_role_authorized)
     
                 @self._client.event
                 async def on_voice_state_update(member, before, after):
    @@ -4702,7 +4704,7 @@ async def _cache_discord_document(self, att, ext: str) -> bytes:
                         raise Exception(f"HTTP {resp.status}")
                     return await resp.read()
     
    -    async def _handle_message(self, message: DiscordMessage) -> None:
    +    async def _handle_message(self, message: DiscordMessage, role_authorized: bool = False) -> None:
             """Handle incoming Discord messages."""
             # In server channels (not DMs), require the bot to be @mentioned
             # UNLESS the channel is in the free-response list or the message is
    @@ -4886,6 +4888,7 @@ async def _handle_message(self, message: DiscordMessage) -> None:
                 guild_id=str(guild.id) if guild else None,
                 parent_chat_id=parent_channel_id,
                 message_id=str(message.id),
    +            role_authorized=role_authorized,
             )
     
             # Build media URLs -- download image attachments to local cache so the

  • scripts/release.py

    @@ -1503,6 +1503,7 @@
         "[redacted:email]": "sanidhyasin",  # PR #40403 salvage (model.default_headers for custom OpenAI-compatible providers, #40033)
         "[redacted:email]": "JoelJJohnson",  # PR #39913 salvage (Windows ConPTY dashboard chat bridge)
         "[redacted:email]": "Nea74",  # PR #40022 co-author credit (same Windows ConPTY bridge design)
    +    "[redacted:email]": "joel611",  # PR #33958 salvage (DISCORD_ALLOWED_ROLES role_authorized gateway flag)
     }