diff --git a/gateway/platforms/discord.py b/gateway/platforms/discord.py index c5b12e09c1..ae107cdfb2 100644 --- a/gateway/platforms/discord.py +++ b/gateway/platforms/discord.py @@ -477,6 +477,34 @@ class VoiceReceiver: pass +def _read_dm_role_auth_guild() -> Optional[int]: + """Return the guild ID opted-in for DM role-based auth, or None. + + Reads ``discord.dm_role_auth_guild`` from config.yaml. This is + deliberately a config.yaml-only setting (not an env var): per repo + policy, ``~/.hermes/.env`` is for secrets only, and this is a + behavioral setting. Guild IDs aren't secrets. + + Accepts ints or numeric strings in the config. Anything else + (empty, malformed, None) returns None, which keeps the secure + default (DM role-auth disabled). + """ + try: + from hermes_cli.config import read_raw_config + cfg = read_raw_config() or {} + discord_cfg = cfg.get("discord", {}) or {} + raw = discord_cfg.get("dm_role_auth_guild") + except Exception: + return None + if raw is None or raw == "": + return None + try: + guild_id = int(raw) + except (TypeError, ValueError): + return None + return guild_id if guild_id > 0 else None + + class DiscordAdapter(BasePlatformAdapter): """ Discord bot adapter. @@ -2140,11 +2168,11 @@ class DiscordAdapter(BasePlatformAdapter): Role checks are **scoped to the guild the message originated from**. For DMs (no guild context), role-based auth is disabled by default and - only user-ID allowlist applies. Set ``DISCORD_DM_ROLE_AUTH_GUILD`` - to a specific guild ID to opt-in: role membership in that one guild - will authorize DMs. This prevents cross-guild privilege escalation - where a user with the configured role in any shared public server - could DM the bot and pass the allowlist. + only user-ID allowlist applies. Set ``discord.dm_role_auth_guild`` + in config.yaml to a specific guild ID to opt-in: role membership in + that one guild will authorize DMs. This prevents cross-guild + privilege escalation where a user with the configured role in any + shared public server could DM the bot and pass the allowlist. Args: user_id: Author ID as a string. @@ -2168,14 +2196,14 @@ class DiscordAdapter(BasePlatformAdapter): if not has_roles: return False - # DM path: roles require explicit opt-in via DISCORD_DM_ROLE_AUTH_GUILD. - # Without this, a user with the configured role in ANY mutual guild - # could DM the bot and bypass the allowlist (cross-guild leakage). + # DM path: roles require explicit opt-in via + # ``discord.dm_role_auth_guild`` in config.yaml. Without this, a + # user with the configured role in ANY mutual guild could DM the + # bot and bypass the allowlist (cross-guild leakage). if is_dm or guild is None: - dm_guild_env = os.getenv("DISCORD_DM_ROLE_AUTH_GUILD", "").strip() - if not dm_guild_env.isdigit(): + dm_guild_id = _read_dm_role_auth_guild() + if dm_guild_id is None: return False - dm_guild_id = int(dm_guild_env) if self._client is None: return False dm_guild = self._client.get_guild(dm_guild_id) diff --git a/hermes_cli/config.py b/hermes_cli/config.py index 6753ae3de0..9db661a27e 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -1108,6 +1108,12 @@ DEFAULT_CONFIG = { "auto_thread": True, # Auto-create threads on @mention in channels (like Slack) "reactions": True, # Add 👀/✅/❌ reactions to messages during processing "channel_prompts": {}, # Per-channel ephemeral system prompts (forum parents apply to child threads) + # Opt-in DM role-based auth (#12136). By default, DISCORD_ALLOWED_ROLES + # authorizes only guild messages in the role's own guild — DMs require + # DISCORD_ALLOWED_USERS. Set dm_role_auth_guild to a guild ID to also + # authorize DMs from members of that one trusted guild holding the + # allowed role. Unset / empty / 0 = secure default (DM role-auth off). + "dm_role_auth_guild": "", # discord / discord_admin tools: restrict which actions the agent may call. # Default (empty) = all actions allowed (subject to bot privileged intents). # Accepts comma-separated string ("list_guilds,list_channels,fetch_messages") diff --git a/tests/gateway/test_discord_roles_dm_scope.py b/tests/gateway/test_discord_roles_dm_scope.py index 604b4e0aab..0f10ba79ae 100644 --- a/tests/gateway/test_discord_roles_dm_scope.py +++ b/tests/gateway/test_discord_roles_dm_scope.py @@ -11,8 +11,8 @@ allowed a cross-guild DM bypass: in public server A, and authorizes the DM. The fix scopes role checks to the originating guild and disables role-based -auth on DMs unless ``DISCORD_DM_ROLE_AUTH_GUILD`` explicitly opts into a -single trusted guild. +auth on DMs unless ``discord.dm_role_auth_guild`` in config.yaml explicitly +opts into a single trusted guild. """ from types import SimpleNamespace @@ -23,6 +23,17 @@ import pytest from gateway.platforms.discord import DiscordAdapter +def _set_dm_role_auth_guild(monkeypatch, guild_id=None): + """Stub ``hermes_cli.config.read_raw_config`` so ``_read_dm_role_auth_guild`` + resolves to ``guild_id`` (or None for the opt-out default). + """ + cfg = {"discord": {"dm_role_auth_guild": guild_id if guild_id is not None else ""}} + # Patch the attribute ``hermes_cli.config.read_raw_config`` — that's + # what ``_read_dm_role_auth_guild`` imports at call time. + import hermes_cli.config as _cfg_mod + monkeypatch.setattr(_cfg_mod, "read_raw_config", lambda: cfg, raising=True) + + def _make_adapter(allowed_users=None, allowed_roles=None, guilds=None): """Build a minimal DiscordAdapter without running __init__.""" adapter = object.__new__(DiscordAdapter) @@ -69,7 +80,7 @@ def test_dm_rejects_role_held_in_other_guild(monkeypatch): Regression guard for the cross-guild DM bypass in the initial DISCORD_ALLOWED_ROLES implementation. """ - monkeypatch.delenv("DISCORD_DM_ROLE_AUTH_GUILD", raising=False) + _set_dm_role_auth_guild(monkeypatch) public_guild, _ = _guild_with_member( guild_id=111111, @@ -91,7 +102,7 @@ def test_dm_rejects_role_held_in_other_guild(monkeypatch): def test_dm_role_auth_requires_explicit_guild_optin(monkeypatch): - """With DISCORD_DM_ROLE_AUTH_GUILD set, only that specific guild counts. + """With dm_role_auth_guild set, only that specific guild counts. The user has the role in the opted-in guild — allowed. """ @@ -106,7 +117,7 @@ def test_dm_role_auth_requires_explicit_guild_optin(monkeypatch): allowed_roles=[5555], guilds=[other_guild, trusted_guild], ) - monkeypatch.setenv("DISCORD_DM_ROLE_AUTH_GUILD", "222222") + _set_dm_role_auth_guild(monkeypatch, 222222) assert ( adapter._is_allowed_user("42", author=None, guild=None, is_dm=True) @@ -115,7 +126,7 @@ def test_dm_role_auth_requires_explicit_guild_optin(monkeypatch): def test_dm_role_auth_optin_rejects_when_not_member(monkeypatch): - """DISCORD_DM_ROLE_AUTH_GUILD set but user isn't a member → reject.""" + """dm_role_auth_guild set but user isn't a member → reject.""" trusted_guild = SimpleNamespace( id=222222, get_member=lambda uid: None, # user not in trusted guild @@ -129,7 +140,7 @@ def test_dm_role_auth_optin_rejects_when_not_member(monkeypatch): allowed_roles=[5555], guilds=[public_guild, trusted_guild], ) - monkeypatch.setenv("DISCORD_DM_ROLE_AUTH_GUILD", "222222") + _set_dm_role_auth_guild(monkeypatch, 222222) assert ( adapter._is_allowed_user("42", author=None, guild=None, is_dm=True) @@ -146,7 +157,7 @@ def test_guild_message_role_check_scoped_to_originating_guild(monkeypatch): """A user with the role in a DIFFERENT guild than the message origin must NOT be authorized, even when both guilds are mutual. """ - monkeypatch.delenv("DISCORD_DM_ROLE_AUTH_GUILD", raising=False) + _set_dm_role_auth_guild(monkeypatch) public_guild, _ = _guild_with_member( guild_id=111111, @@ -172,7 +183,7 @@ def test_guild_message_role_check_scoped_to_originating_guild(monkeypatch): def test_guild_message_role_check_allows_when_role_in_same_guild(monkeypatch): """Positive path: user has the role IN the message's guild → allowed.""" - monkeypatch.delenv("DISCORD_DM_ROLE_AUTH_GUILD", raising=False) + _set_dm_role_auth_guild(monkeypatch) trusted_guild, _ = _guild_with_member( guild_id=222222, @@ -197,7 +208,7 @@ def test_guild_message_rejects_author_roles_from_different_guild(monkeypatch): message, the cached .roles on it must NOT be trusted — rely on the current guild's Member lookup instead. """ - monkeypatch.delenv("DISCORD_DM_ROLE_AUTH_GUILD", raising=False) + _set_dm_role_auth_guild(monkeypatch) # Author is a Member of a DIFFERENT guild with the allowed role foreign_guild = SimpleNamespace(id=999, get_member=lambda uid: None) @@ -264,7 +275,7 @@ 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) + _set_dm_role_auth_guild(monkeypatch) public_guild, _ = _guild_with_member( guild_id=111111, @@ -292,7 +303,7 @@ def test_slash_authorization_rejects_cross_guild_role_dm(monkeypatch): 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) + _set_dm_role_auth_guild(monkeypatch) public_guild, _ = _guild_with_member( guild_id=111111, @@ -320,7 +331,7 @@ def test_slash_authorization_rejects_cross_guild_role_in_guild(monkeypatch): 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) + _set_dm_role_auth_guild(monkeypatch) trusted_guild, _ = _guild_with_member( guild_id=222222,