mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-05-08 03:01:47 +00:00
fix(discord): route DM role-auth opt-in through config.yaml (not env var)
Per repo policy, ~/.hermes/.env is for secrets only. Guild IDs are behavioral configuration, not secrets. Replacing the DISCORD_DM_ROLE_AUTH_GUILD env var from the original fix with discord.dm_role_auth_guild in config.yaml. - New module-level _read_dm_role_auth_guild() helper reads hermes_cli.config.read_raw_config()['discord']['dm_role_auth_guild']. Fails closed on any parse error (safe default = DM role-auth off). - DEFAULT_CONFIG['discord'] gains dm_role_auth_guild: '' with a comment documenting the opt-in. - Tests patch hermes_cli.config.read_raw_config directly (via the _set_dm_role_auth_guild helper) instead of setenv/delenv. 12 tests in test_discord_roles_dm_scope pass; no env var involvement. - Docstring + module docstring + comments updated to reference discord.dm_role_auth_guild. - E2E verified with real imports across 6 scenarios: unset, int, string, garbage, zero, and (crucially) env-var-only-no-config all return None except the valid int/string cases. Env var has zero effect — policy compliance confirmed.
This commit is contained in:
Teknium
parent
5c045b8f6c
commit
80717a157f
3 changed files with 69 additions and 24 deletions
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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")
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue