From e3bcc819ab928a6db708329acc7f6043d3442fd5 Mon Sep 17 00:00:00 2001 From: 0xyg3n Date: Tue, 14 Apr 2026 21:27:20 +0000 Subject: [PATCH 1/3] feat(discord): add DISCORD_ALLOWED_ROLES env var for role-based access control Adds a new DISCORD_ALLOWED_ROLES environment variable that allows filtering bot interactions by Discord role ID. Uses OR semantics with the existing DISCORD_ALLOWED_USERS - if a user matches either allowlist, they're permitted. Changes: - Parse DISCORD_ALLOWED_ROLES comma-separated role IDs on connect - Enable members intent when roles are configured (needed for role lookup) - Update _is_allowed_user() to accept optional author param for direct role check - Fallback to scanning mutual guilds when author object lacks roles (DMs, voice) - Fully backwards compatible: no behavior change when env var is unset --- gateway/platforms/discord.py | 59 +++++++++++++++++++++++++++++++----- gateway/run.py | 12 ++++++++ 2 files changed, 64 insertions(+), 7 deletions(-) diff --git a/gateway/platforms/discord.py b/gateway/platforms/discord.py index bed3fa9c376a1..640da125f27e8 100644 --- a/gateway/platforms/discord.py +++ b/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,43 @@ 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. + """ + has_users = bool(self._allowed_user_ids) + has_roles = bool(self._allowed_role_ids) + if not has_users and not has_roles: + return True + # Check user ID allowlist + if has_users and user_id in self._allowed_user_ids: 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 self._allowed_role_ids 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 self._allowed_role_ids for r in m_roles): + return True + return False async def send_image_file( self, diff --git a/gateway/run.py b/gateway/run.py index 261fecbf85af2..9b97968665eb5 100644 --- a/gateway/run.py +++ b/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): From 9df4826a41f39d59d92e8cab313ae8318f00bb82 Mon Sep 17 00:00:00 2001 From: Teknium Date: Fri, 17 Apr 2026 05:47:39 -0700 Subject: [PATCH 2/3] fix(discord): harden DISCORD_ALLOWED_ROLES and cover gateway layer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two follow-ups to the cherry-picked PR #9873 (`e3bcc819`): 1. `_is_allowed_user` now uses `getattr(self, '_allowed_*_ids', set())` so test fixtures that build the adapter via `object.__new__` (skipping __init__) don't crash with AttributeError. See AGENTS.md pitfall #17 — same pattern as gateway.run. 2. New 3-case regression coverage in test_discord_bot_auth_bypass.py: - role-only config bypasses the gateway 'no allowlists' branch - roles + users combined still authorizes user-allowlist matches - the role bypass does NOT leak to other platforms (Telegram, etc.) 3. Autouse fixture in test_discord_bot_auth_bypass.py clears all Discord auth env vars before each test so DISCORD_ALLOWED_ROLES leakage from a previous test in the session can't flip later 'should-reject' tests into false-pass. Required because the bare cherry-pick of #9873 only added the adapter- level role check — it didn't cover the gateway-level _is_user_authorized, which still rejected role-only setups via the 'no allowlists configured' branch. --- gateway/platforms/discord.py | 15 ++-- tests/gateway/test_discord_bot_auth_bypass.py | 72 +++++++++++++++++++ 2 files changed, 82 insertions(+), 5 deletions(-) diff --git a/gateway/platforms/discord.py b/gateway/platforms/discord.py index 640da125f27e8..79e70592e26e4 100644 --- a/gateway/platforms/discord.py +++ b/gateway/platforms/discord.py @@ -1386,19 +1386,24 @@ def _is_allowed_user(self, user_id: str, author=None) -> bool: When author is a Member, checks .roles directly; otherwise falls back to scanning the bot's mutual guilds for a Member record. """ - has_users = bool(self._allowed_user_ids) - has_roles = bool(self._allowed_role_ids) + # ``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 self._allowed_user_ids: + if has_users and user_id in allowed_users: return True # 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 self._allowed_role_ids for r in 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: @@ -1412,7 +1417,7 @@ def _is_allowed_user(self, user_id: str, author=None) -> bool: if m is None: continue m_roles = getattr(m, "roles", None) or [] - if any(getattr(r, "id", None) in self._allowed_role_ids for r in m_roles): + if any(getattr(r, "id", None) in allowed_roles for r in m_roles): return True return False diff --git a/tests/gateway/test_discord_bot_auth_bypass.py b/tests/gateway/test_discord_bot_auth_bypass.py index 29e6a8899e3b5..8ff39a1bf4990 100644 --- a/tests/gateway/test_discord_bot_auth_bypass.py +++ b/tests/gateway/test_discord_bot_auth_bypass.py @@ -22,6 +22,23 @@ from gateway.session import Platform, SessionSource +@pytest.fixture(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 From 3354fa0c82288e4b3966986fa947d351114c1230 Mon Sep 17 00:00:00 2001 From: Teknium Date: Fri, 17 Apr 2026 05:47:53 -0700 Subject: [PATCH 3/3] chore(release): map jz.pentest@gmail.com to @0xyg3n --- scripts/release.py | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/release.py b/scripts/release.py index ccbb4f2d457ac..74063a635afce 100755 --- a/scripts/release.py +++ b/scripts/release.py @@ -237,6 +237,7 @@ "mbelleau@Michels-MacBook-Pro.local": "malaiwah", "michel.belleau@malaiwah.com": "malaiwah", "gnanasekaran.sekareee@gmail.com": "gnanam1990", + "jz.pentest@gmail.com": "0xyg3n", "dhandhalyabhavik@gmail.com": "v1k22", "rucchizhao@zhaochenfeideMacBook-Pro.local": "RucchiZ", "lehaolin98@outlook.com": "LehaoLin",