Gateway auth policy enforcement and security fixes

Fixes on record

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

• #21241 fix(discord): scope DISCORD_ALLOWED_ROLES to originating guild (#12136, CVSS 8.1)

  merged to main

  files: gateway/platforms/discord.py, hermes_cli/config.py, tests/gateway/test_discord_roles_dm_scope.py, tests/gateway/test_discord_slash_auth.py

  Show the diff

  • gateway/platforms/discord.py

    @@ -477,6 +477,34 @@ def pcm_to_wav(pcm_data: bytes, output_path: str,
                     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.
    @@ -701,7 +729,17 @@ async def on_message(message: DiscordMessage):
                         # human-user allowlist below (bots aren't in it).
                     else:
                         # Non-bot: enforce the configured user/role allowlists.
    -                    if not self._is_allowed_user(str(message.author.id), message.author):
    +                    # Pass guild + is_dm so role checks are scoped to the
    +                    # originating guild (prevents cross-guild DM bypass, see
    +                    # _is_allowed_user docstring).
    +                    _msg_guild = getattr(message, "guild", None)
    +                    _is_dm = isinstance(message.channel, discord.DMChannel) or _msg_guild is None
    +                    if not self._is_allowed_user(
    +                        str(message.author.id),
    +                        message.author,
    +                        guild=_msg_guild,
    +                        is_dm=_is_dm,
    +                    ):
                             return
                     
                     # Multi-agent filtering: if the message mentions specific bots
    @@ -2063,8 +2101,16 @@ async def _voice_listen_loop(self, guild_id: int):
                             pass
     
                     completed = receiver.check_silence()
    +                # Voice inputs always originate from a specific guild
    +                # (guild_id is in scope). Pass it so role checks are
    +                # guild-scoped and not cross-guild.
    +                _vc_guild = self._client.get_guild(guild_id) if self._client is not None else None
                     for user_id, pcm_data in completed:
    -                    if not self._is_allowed_user(str(user_id)):
    +                    if not self._is_allowed_user(
    +                        str(user_id),
    +                        guild=_vc_guild,
    +                        is_dm=False,
    +                    ):
                             continue
                         await self._process_voice_input(guild_id, user_id, pcm_data)
             except asyncio.CancelledError:
    @@ -2107,13 +2153,32 @@ 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, author=None) -> bool:
    +    def _is_allowed_user(
    +        self,
    +        user_id: str,
    +        author=None,
    +        *,
    +        guild=None,
    +        is_dm: bool = False,
    +    ) -> 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.
    +
    +        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``
    +        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.
    +            author: Optional Member/User object for in-guild role lookup.
    +            guild: The guild the message arrived in (None for DMs).
    +            is_dm: True if the message came from a DM channel.
             """
             # ``getattr`` fallbacks here guard against test fixtures that build
             # an adapter via ``object.__new__(DiscordAdapter)`` and skip __init__
    @@ -2124,31 +2189,54 @@ def _is_allowed_user(self, user_id: str, author=None) -> bool:
             has_roles = bool(allowed_roles)
             if not has_users and not has_roles:
                 return True
    -        # Check user ID allowlist
    +        # Check user ID allowlist (works for both DMs and guild messages)
             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 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
    +        # Role allowlist is only consulted when configured.
    +        if not has_roles:
    +            return False
    +
    +        # 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_id = _read_dm_role_auth_guild()
    +            if dm_guild_id is None:
    +                return False
    +            if self._client is None:
    +                return False
    +            dm_guild = self._client.get_guild(dm_guild_id)
    +            if dm_guild is None:
    +                return False
    +            try:
    +                uid_int = int(user_id)
    +            except (TypeError, ValueError):
    +                return False
    +            m = dm_guild.get_member(uid_int)
    +            if m is None:
    +                return False
    +            m_roles = getattr(m, "roles", None) or []
    +            return any(getattr(r, "id", None) in allowed_roles for r in m_roles)
    +
    +        # Guild path: role check is scoped to THIS guild only.
    +        # 1) Prefer the direct Member object passed in (correct guild by construction).
    +        direct_roles = getattr(author, "roles", None) if author is not None else None
    +        author_guild = getattr(author, "guild", None)
    +        if direct_roles and (author_guild is None or author_guild.id == guild.id):
    +            if any(getattr(r, "id", None) in allowed_roles for r in direct_roles):
    +                return True
    +        # 2) Fallback: resolve the Member in the message's guild only — NEVER
    +        #    scan other mutual guilds (that is the cross-guild bypass bug).
    +        try:
    +            uid_int = int(user_id)
    +        except (TypeError, ValueError):
    +            return False
    +        m = guild.get_member(uid_int)
    +        if m is None:
    +            return False
    +        m_roles = getattr(m, "roles", None) or []
    +        return any(getattr(r, "id", None) in allowed_roles for r in m_roles)
     
         # ── Slash command authorization ─────────────────────────────────────
         # Slash commands (``_run_simple_slash`` and ``_handle_thread_create_slash``)
    @@ -2245,7 +2333,16 @@ def _evaluate_slash_authorization(
                 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",

  • hermes_cli/config.py

    @@ -1108,6 +1108,12 @@ def _ensure_hermes_home_managed(home: Path):
             "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")

  • tests/gateway/test_discord_roles_dm_scope.py

    @@ -0,0 +1,355 @@
    +"""Regression guard: DISCORD_ALLOWED_ROLES must be guild-scoped, not global.
    +
    +Prior to this fix, ``_is_allowed_user`` iterated ``self._client.guilds`` and
    +returned True if the user held any allowed role in ANY mutual guild. This
    +allowed a cross-guild DM bypass:
    +
    +1. Bot is in both a large public server A and a private trusted server B.
    +2. User has role ``R`` in public server A. ``DISCORD_ALLOWED_ROLES`` is
    +   configured with ``R`` intending it to authorize server B members.
    +3. User DMs the bot. The role check scans every mutual guild, finds ``R``
    +   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`` in config.yaml explicitly
    +opts into a single trusted guild.
    +"""
    +
    +from types import SimpleNamespace
    +from unittest.mock import MagicMock
    +
    +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)
    +    adapter._allowed_user_ids = set(allowed_users or [])
    +    adapter._allowed_role_ids = set(allowed_roles or [])
    +
    +    client = MagicMock()
    +    client.guilds = guilds or []
    +    client.get_guild = lambda gid: next(
    +        (g for g in (guilds or []) if getattr(g, "id", None) == gid),
    +        None,
    +    )
    +    adapter._client = client
    +    return adapter
    +
    +
    +def _role(role_id):
    +    return SimpleNamespace(id=role_id)
    +
    +
    +def _guild_with_member(guild_id, member_id, role_ids):
    +    """Build a fake guild that holds one member with the given roles."""
    +    member = SimpleNamespace(
    +        id=member_id,
    +        roles=[_role(rid) for rid in role_ids],
    +        guild=None,  # filled below
    +    )
    +    guild = SimpleNamespace(
    +        id=guild_id,
    +        get_member=lambda uid: member if uid == member_id else None,
    +    )
    +    member.guild = guild
    +    return guild, member
    +
    +
    +# ---------------------------------------------------------------------------
    +# Cross-guild DM bypass — MUST be rejected
    +# ---------------------------------------------------------------------------
    +
    +
    +def test_dm_rejects_role_held_in_other_guild(monkeypatch):
    +    """A user with an allowed role in a DIFFERENT guild must NOT pass a DM.
    +
    +    Regression guard for the cross-guild DM bypass in the initial
    +    DISCORD_ALLOWED_ROLES implementation.
    +    """
    +    _set_dm_role_auth_guild(monkeypatch)
    +
    +    public_guild, _ = _guild_with_member(
    +        guild_id=111111,
    +        member_id=42,
    +        role_ids=[5555],  # allowed role, but in the wrong guild
    +    )
    +    trusted_guild = SimpleNamespace(id=222222, get_member=lambda uid: None)
    +
    +    adapter = _make_adapter(
    +        allowed_roles=[5555],
    +        guilds=[public_guild, trusted_guild],
    +    )
    +
    +    # DM from user 42: role check must NOT scan other guilds.
    +    assert (
    +        adapter._is_allowed_user("42", author=None, guild=None, is_dm=True)
    +        is False
    +    )
    +
    +
    +def test_dm_role_auth_requires_explicit_guild_optin(monkeypatch):
    +    """With dm_role_auth_guild set, only that specific guild counts.
    +
    +    The user has the role in the opted-in guild — allowed.
    +    """
    +    trusted_guild, _ = _guild_with_member(
    +        guild_id=222222,
    +        member_id=42,
    +        role_ids=[5555],
    +    )
    +    other_guild = SimpleNamespace(id=333333, get_member=lambda uid: None)
    +
    +    adapter = _make_adapter(
    +        allowed_roles=[5555],
    +        guilds=[other_guild, trusted_guild],
    +    )
    +    _set_dm_role_auth_guild(monkeypatch, 222222)
    +
    +    assert (
    +        adapter._is_allowed_user("42", author=None, guild=None, is_dm=True)
    +        is True
    +    )
    +
    +
    +def test_dm_role_auth_optin_rejects_when_not_member(monkeypatch):
    +    """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
    +    )
    +    public_guild, _ = _guild_with_member(
    +        guild_id=111111,
    +        member_id=42,
    +        role_ids=[5555],
    +    )
    +    adapter = _make_adapter(
    +        allowed_roles=[5555],
    +        guilds=[public_guild, trusted_guild],
    +    )
    +    _set_dm_role_auth_guild(monkeypatch, 222222)
    +
    +    assert (
    +        adapter._is_allowed_user("42", author=None, guild=None, is_dm=True)
    +        is False
    +    )
    +
    +
    +# ---------------------------------------------------------------------------
    +# Guild messages — role check must be scoped to THIS guild only
    +# ---------------------------------------------------------------------------
    +
    +
    +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.
    +    """
    +    _set_dm_role_auth_guild(monkeypatch)
    +
    +    public_guild, _ = _guild_with_member(
    +        guild_id=111111,
    +        member_id=42,
    +        role_ids=[5555],  # allowed role in public guild only
    +    )
    +    # Message 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],
    +    )
    +
    +    # No author object passed → falls through to guild.get_member path
    +    assert (
    +        adapter._is_allowed_user(
    +            "42", author=None, guild=trusted_guild, is_dm=False
    +        )
    +        is False
    +    )
    +
    +
    +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."""
    +    _set_dm_role_auth_guild(monkeypatch)
    +
    +    trusted_guild, _ = _guild_with_member(
    +        guild_id=222222,
    +        member_id=42,
    +        role_ids=[5555],
    +    )
    +    adapter = _make_adapter(
    +        allowed_roles=[5555],
    +        guilds=[trusted_guild],
    +    )
    +
    +    assert (
    +        adapter._is_allowed_user(
    +            "42", author=None, guild=trusted_guild, is_dm=False
    +        )
    +        is True
    +    )
    +
    +
    +def test_guild_message_rejects_author_roles_from_different_guild(monkeypatch):
    +    """If an author Member object comes from a different guild than the
    +    message, the cached .roles on it must NOT be trusted — rely on the
    +    current guild's Member lookup instead.
    +    """
    +    _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)
    +    foreign_author = SimpleNamespace(
    +        id=42,
    +        roles=[_role(5555)],
    +        guild=foreign_guild,
    +    )
    +    # Message arrives in this_guild where user 42 has NO role
    +    this_guild = SimpleNamespace(id=222222, get_member=lambda uid: None)
    +
    +    adapter = _make_adapter(
    +        allowed_roles=[5555],
    +        guilds=[foreign_guild, this_guild],
    +    )
    +
    +    assert (
    +        adapter._is_allowed_user(
    +            "42", author=foreign_author, guild=this_guild, is_dm=False
    +        )
    +        is False
    +    )
    +
    +
    +# ---------------------------------------------------------------------------
    +# Backwards-compatibility — user-ID allowlist still works in both contexts
    +# ---------------------------------------------------------------------------
    +
    +
    +def test_user_id_allowlist_works_in_dm():
    +    adapter = _make_adapter(allowed_users=["42"])
    +    assert (
    +        adapter._is_allowed_user("42", author=None, guild=None, is_dm=True)
    +        is True
    +    )
    +
    +
    +def test_user_id_allowlist_works_in_guild():
    +    adapter = _make_adapter(allowed_users=["42"])
    +    some_guild = SimpleNamespace(id=111, get_member=lambda uid: None)
    +    assert (
    +        adapter._is_allowed_user(
    +            "42", author=None, guild=some_guild, is_dm=False
    +        )
    +        is True
    +    )
    +
    +
    +def test_empty_allowlists_allow_everyone():
    +    adapter = _make_adapter()
    +    assert (
    +        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
    +    _set_dm_role_auth_guild(monkeypatch)
    +
    +    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."""
    +    _set_dm_role_auth_guild(monkeypatch)
    +
    +    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."""
    +    _set_dm_role_auth_guild(monkeypatch)
    +
    +    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

  • 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,

• #45634 fix(security): fail closed own-policy gateway adapters

  merged to main

  files: gateway/authz_mixin.py, gateway/platforms/base.py, tests/gateway/test_config_driven_access_policy.py