From 3e7ade418d030d241bda4c6352ff52409f4582ec Mon Sep 17 00:00:00 2001 From: yungchentang <46495124+yungchentang@users.noreply.github.com> Date: Tue, 7 Jul 2026 16:42:47 -0700 Subject: [PATCH] fix(discord): explain fail-closed allowlist default Log a one-shot structured warning when Discord denies traffic because no allowlist/policy is configured, and correct the setup wizard's inverted warning text. The fail-closed default itself is unchanged. Fixes #58682. --- plugins/platforms/discord/adapter.py | 36 +++++++- .../test_discord_fail_closed_feedback.py | 92 +++++++++++++++++++ 2 files changed, 126 insertions(+), 2 deletions(-) create mode 100644 tests/gateway/test_discord_fail_closed_feedback.py diff --git a/plugins/platforms/discord/adapter.py b/plugins/platforms/discord/adapter.py index d9f03b6fbba..ddaac7ab074 100644 --- a/plugins/platforms/discord/adapter.py +++ b/plugins/platforms/discord/adapter.py @@ -900,6 +900,7 @@ class DiscordAdapter(BasePlatformAdapter): # rate limit (~1 edit per stream tick for the rest of a long reply). # Mirrors the Telegram #58563 fix. Entries are dropped on finalize. self._last_overflow_preview: Dict[tuple, str] = {} + self._warned_fail_closed_default = False def _handle_bot_task_done(self, task: asyncio.Task) -> None: """Surface post-startup discord.py task exits to the gateway supervisor. @@ -1160,6 +1161,7 @@ class DiscordAdapter(BasePlatformAdapter): is_dm=_is_dm, channel_ids=_msg_channel_ids, ): + self._warn_if_fail_closed_default() return _role_authorized = bool(getattr(self, "_allowed_role_ids", set())) @@ -3355,6 +3357,28 @@ class DiscordAdapter(BasePlatformAdapter): m_roles = getattr(m, "roles", None) or [] return any(getattr(r, "id", None) in allowed_roles for r in m_roles) + def _warn_if_fail_closed_default(self) -> None: + """Log once when Discord is rejecting traffic with no allowlist set.""" + if getattr(self, "_warned_fail_closed_default", False): + return + allowed_users = getattr(self, "_allowed_user_ids", set()) or set() + allowed_roles = getattr(self, "_allowed_role_ids", set()) or set() + if allowed_users or allowed_roles: + return + if os.getenv("DISCORD_ALLOWED_CHANNELS", "").strip(): + return + if os.getenv("DISCORD_ALLOW_ALL_USERS", "").strip().lower() in {"true", "1", "yes"}: + return + if os.getenv("GATEWAY_ALLOW_ALL_USERS", "").strip().lower() in {"true", "1", "yes"}: + return + self._warned_fail_closed_default = True + logger.warning( + "[%s] Discord messages are being denied because no allowlist is configured. " + "Set DISCORD_ALLOWED_USERS, DISCORD_ALLOWED_ROLES, or " + "DISCORD_ALLOWED_CHANNELS, or set DISCORD_ALLOW_ALL_USERS=true for open access.", + self.name, + ) + # ── Slash command authorization ───────────────────────────────────── # Slash commands (``_run_simple_slash`` and ``_handle_thread_create_slash``) # are a separate Discord interaction surface from regular messages and @@ -8121,7 +8145,11 @@ def interactive_setup() -> None: print_info("Discord: already configured") if not prompt_yes_no("Reconfigure Discord?", False): if not get_env_value("DISCORD_ALLOWED_USERS"): - print_info("⚠️ Discord has no user allowlist - anyone can use your bot!") + print_info( + "⚠️ Discord has no user allowlist. With the fail-closed default, " + "messages are denied unless you configure allowed users, roles, " + "or channels, or set DISCORD_ALLOW_ALL_USERS=true." + ) if prompt_yes_no("Add allowed users now?", True): print_info(" To find Discord ID: Enable Developer Mode, right-click name → Copy ID") allowed_users = prompt("Allowed user IDs (comma-separated)") @@ -8154,7 +8182,11 @@ def interactive_setup() -> None: save_env_value("DISCORD_ALLOWED_USERS", ",".join(cleaned_ids)) print_success("Discord allowlist configured") else: - print_info("⚠️ No allowlist set - anyone in servers with your bot can use it!") + print_info( + "⚠️ No allowlist set. Discord will deny messages until you set " + "DISCORD_ALLOWED_USERS, DISCORD_ALLOWED_ROLES, DISCORD_ALLOWED_CHANNELS, " + "or DISCORD_ALLOW_ALL_USERS=true for open access." + ) print() print_info("📬 Home Channel: where Hermes delivers cron job results,") diff --git a/tests/gateway/test_discord_fail_closed_feedback.py b/tests/gateway/test_discord_fail_closed_feedback.py new file mode 100644 index 00000000000..5a45e4c4423 --- /dev/null +++ b/tests/gateway/test_discord_fail_closed_feedback.py @@ -0,0 +1,92 @@ +import logging + +from gateway.config import PlatformConfig +from plugins.platforms.discord.adapter import DiscordAdapter, interactive_setup + + +def _make_adapter() -> DiscordAdapter: + return DiscordAdapter(PlatformConfig(enabled=True, token="***")) + + +def test_discord_fail_closed_default_logs_once(monkeypatch, caplog): + adapter = _make_adapter() + adapter._allowed_user_ids = set() + adapter._allowed_role_ids = set() + + for var in ( + "DISCORD_ALLOWED_CHANNELS", + "DISCORD_ALLOW_ALL_USERS", + "GATEWAY_ALLOW_ALL_USERS", + ): + monkeypatch.delenv(var, raising=False) + + with caplog.at_level(logging.WARNING): + adapter._warn_if_fail_closed_default() + adapter._warn_if_fail_closed_default() + + messages = [record.message for record in caplog.records] + matches = [ + msg for msg in messages + if "Discord messages are being denied because no allowlist is configured" in msg + ] + assert len(matches) == 1 + assert "DISCORD_ALLOWED_USERS" in matches[0] + assert "DISCORD_ALLOW_ALL_USERS=true" in matches[0] + + +def test_discord_fail_closed_default_warning_skips_explicit_channel_gate(monkeypatch, caplog): + adapter = _make_adapter() + adapter._allowed_user_ids = set() + adapter._allowed_role_ids = set() + monkeypatch.setenv("DISCORD_ALLOWED_CHANNELS", "12345") + monkeypatch.delenv("DISCORD_ALLOW_ALL_USERS", raising=False) + monkeypatch.delenv("GATEWAY_ALLOW_ALL_USERS", raising=False) + + with caplog.at_level(logging.WARNING): + adapter._warn_if_fail_closed_default() + + assert "no allowlist is configured" not in caplog.text + + +def test_discord_setup_existing_token_warns_fail_closed_not_fail_open(monkeypatch): + info_lines: list[str] = [] + yes_no_answers = iter([False, False]) + + def fake_get_env_value(key: str): + return "token" if key == "DISCORD_BOT_TOKEN" else "" + + monkeypatch.setattr("hermes_cli.config.get_env_value", fake_get_env_value) + monkeypatch.setattr("hermes_cli.config.save_env_value", lambda *_args, **_kwargs: None) + monkeypatch.setattr("hermes_cli.cli_output.print_header", lambda *_args, **_kwargs: None) + monkeypatch.setattr("hermes_cli.cli_output.print_success", lambda *_args, **_kwargs: None) + monkeypatch.setattr("hermes_cli.cli_output.print_info", lambda msg="", **_kwargs: info_lines.append(str(msg))) + monkeypatch.setattr("hermes_cli.cli_output.prompt", lambda *_args, **_kwargs: "") + monkeypatch.setattr("hermes_cli.cli_output.prompt_yes_no", lambda *_args, **_kwargs: next(yes_no_answers)) + + interactive_setup() + + joined = "\n".join(info_lines) + assert "anyone can use your bot" not in joined + assert "fail-closed default" in joined + assert "DISCORD_ALLOW_ALL_USERS=true" in joined + + +def test_discord_setup_new_token_empty_allowlist_warns_denied_until_configured(monkeypatch): + info_lines: list[str] = [] + prompts = iter(["token", "", ""]) + + monkeypatch.setattr("hermes_cli.config.get_env_value", lambda _key: "") + monkeypatch.setattr("hermes_cli.config.save_env_value", lambda *_args, **_kwargs: None) + monkeypatch.setattr("hermes_cli.cli_output.print_header", lambda *_args, **_kwargs: None) + monkeypatch.setattr("hermes_cli.cli_output.print_success", lambda *_args, **_kwargs: None) + monkeypatch.setattr("hermes_cli.cli_output.print_info", lambda msg="", **_kwargs: info_lines.append(str(msg))) + monkeypatch.setattr("hermes_cli.cli_output.prompt", lambda *_args, **_kwargs: next(prompts)) + monkeypatch.setattr("hermes_cli.cli_output.prompt_yes_no", lambda *_args, **_kwargs: False) + + interactive_setup() + + joined = "\n".join(info_lines) + assert "anyone in servers with your bot can use it" not in joined + assert "Discord will deny messages" in joined + assert "DISCORD_ALLOWED_ROLES" in joined + assert "DISCORD_ALLOW_ALL_USERS=true" in joined