Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 57 additions & 7 deletions gateway/platforms/discord.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
12 changes: 12 additions & 0 deletions gateway/run.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
1 change: 1 addition & 0 deletions scripts/release.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
72 changes: 72 additions & 0 deletions tests/gateway/test_discord_bot_auth_bypass.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
# -----------------------------------------------------------------------------
Expand Down Expand Up @@ -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
Loading