fix(discord): read allowed_guilds as IDs and deny every guild on an unreadable allowlist (#6338)

* test(discord): expose allowlist parsing failures instead of opening the bot

config.example.yaml documents channels.discord.allowed_guilds as a value that
"can also be a single guild ID", but the constructor iterates it: a scalar
string becomes one entry per character, and an unquoted YAML int raises
TypeError. Whatever remains is gated by truthiness, so an allowlist with no
parseable entry leaves the set empty and admits every guild.

* fix(discord): read allowed_guilds as IDs, and fail closed when unreadable

The allowlist was built by iterating the configured value, so the documented
single-guild shorthand ("can also be a single guild ID" in
config.example.yaml) became one entry per character: allowed_guilds: "321"
admitted guilds 1, 2 and 3 while blocking 321, and an unquoted YAML int raised
TypeError in the constructor. The empty result then read as "no allowlist", so
a restriction the operator configured could open the bot to every guild the bot
joined.

Parse both ID lists through helpers that treat a scalar as a one-entry list and
drop unreadable entries with a warning, following the telegram channel: an
allowlist with no parseable entry now denies every guild and logs at ERROR.
Mention exemptions keep their IDs verbatim, where an unreadable entry only
leaves mention_only in force.

* docs(discord): explain allowlist parsing and deny-all behavior

---------

Co-authored-by: Willem Jiang <willem.jiang@gmail.com>

README.md

@@ -782,6 +782,14 @@ DeerFlow can also expose user-owned IM channel connections in the workspace UI.
 
 **Configuration in `config.yaml`:**
 
+Discord's `channels.discord.allowed_guilds` accepts one positive numeric guild
+ID (quoted or unquoted) or a YAML list. Unset, `null`, `[]`, or a blank string
+allows all guilds. Invalid entries are ignored with a warning; any other
+configured value yielding no valid ID denies every guild and logs an error.
+`allowed_channels` accepts one channel ID (quoted or unquoted) or a YAML list
+of IDs exempt from `mention_only`, within allowed guilds. An empty value gives
+no exemptions, so `mention_only` applies everywhere when enabled.
+
 ```yaml
 channels:
   # LangGraph-compatible Gateway API base URL (default: http://localhost:8001/api)

backend/app/channels/discord.py

@@ -34,16 +34,71 @@ DISCORD_OUTBOUND_TIMEOUT_SECONDS = 30.0
 DISCORD_UPLOAD_TIMEOUT_SECONDS = 120.0
 
 
+def _parse_discord_guild_id(entry: Any) -> int | None:
+    """Return a Discord guild (snowflake) ID, or ``None`` if ``entry`` is not one."""
+    if isinstance(entry, int) and not isinstance(entry, bool) and entry > 0:
+        return entry
+    if isinstance(entry, str):
+        text = entry.strip()
+        if text.isascii() and text.isdigit():
+            return int(text) or None
+    return None
+
+
+def _parse_allowed_guilds(allowed_guilds: Any) -> frozenset[int] | None:
+    """Parse ``channels.discord.allowed_guilds``; ``None`` means no allowlist.
+
+    A single ID is shorthand for a one-entry list, the form
+    ``config.example.yaml`` advertises. Iterating a scalar instead turns
+    ``"321"`` into guilds 1, 2 and 3 and raises ``TypeError`` on an unquoted
+    YAML int. Entries that are not positive numeric snowflakes are dropped with
+    a warning. If none remain, the result is an empty set that denies every
+    guild: the operator asked for a restriction, so an unreadable one must not
+    open the bot to every server it joined.
+    """
+    if allowed_guilds is None or (isinstance(allowed_guilds, str) and not allowed_guilds.strip()):
+        return None
+    entries = list(allowed_guilds) if isinstance(allowed_guilds, list | tuple | set) else [allowed_guilds]
+    if not entries:
+        return None
+    guild_ids: set[int] = set()
+    for entry in entries:
+        guild_id = _parse_discord_guild_id(entry)
+        if guild_id is None:
+            logger.warning("[Discord] Ignoring allowed_guilds entry %r: expected a positive numeric guild ID; list several IDs as a YAML list", entry)
+        else:
+            guild_ids.add(guild_id)
+    if not guild_ids:
+        logger.error("[Discord] allowed_guilds has no valid numeric guild ID; denying every guild until it is fixed")
+    return frozenset(guild_ids)
+
+
+def _parse_allowed_channels(allowed_channels: Any) -> frozenset[str]:
+    """Parse ``channels.discord.allowed_channels``; a single ID is one entry.
+
+    Channel IDs are snowflakes that only ever get compared as strings here, so
+    the entries are kept verbatim rather than validated — an unreadable ID just
+    never matches, which leaves ``mention_only`` in force (fail-closed).
+    """
+    if allowed_channels is None or (isinstance(allowed_channels, str) and not allowed_channels.strip()):
+        return frozenset()
+    entries = list(allowed_channels) if isinstance(allowed_channels, list | tuple | set) else [allowed_channels]
+    return frozenset(text for text in (str(entry).strip() for entry in entries) if text)
+
+
 class DiscordChannel(Channel):
     """Discord bot channel.
 
     Configuration keys (in ``config.yaml`` under ``channels.discord``):
         - ``bot_token``: Discord Bot token.
-        - ``allowed_guilds``: (optional) List of allowed Discord guild IDs. Empty = allow all.
+        - ``allowed_guilds``: (optional) List of allowed Discord guild IDs, or a
+          single ID. Empty = allow all; a non-empty list with no valid ID denies
+          every guild.
         - ``mention_only``: (optional) If true, only respond when the bot is mentioned.
         - ``allowed_channels``: (optional) List of channel IDs where messages are always accepted
-          (even when mention_only is true). Use for channels where you want the bot to respond
-          without mentions. Empty = mention_only applies everywhere.
+          (even when mention_only is true), or a single ID. Use for channels where
+          you want the bot to respond without mentions. Empty = mention_only applies
+          everywhere.
         - ``thread_mode``: (optional) If true, group a channel conversation into a thread.
           Default: same as ``mention_only``.
     """
@@ -51,17 +106,10 @@ class DiscordChannel(Channel):
     def __init__(self, bus: MessageBus, config: dict[str, Any]) -> None:
         super().__init__(name="discord", bus=bus, config=config)
         self._bot_token = str(config.get("bot_token", "")).strip()
-        self._allowed_guilds: set[int] = set()
-        for guild_id in config.get("allowed_guilds", []):
-            try:
-                self._allowed_guilds.add(int(guild_id))
-            except (TypeError, ValueError):
-                continue
+        self._allowed_guilds: frozenset[int] | None = _parse_allowed_guilds(config.get("allowed_guilds"))
         self._mention_only: bool = bool(config.get("mention_only", False))
         self._thread_mode: bool = config.get("thread_mode", self._mention_only)
-        self._allowed_channels: set[str] = set()
-        for channel_id in config.get("allowed_channels", []):
-            self._allowed_channels.add(str(channel_id))
+        self._allowed_channels: frozenset[str] = _parse_allowed_channels(config.get("allowed_channels"))
 
         # Session tracking: channel_id -> Discord thread_id (in-memory, persisted to JSON).
         # Uses a dedicated JSON file separate from ChannelStore, which maps IM
@@ -487,6 +535,11 @@ class DiscordChannel(Channel):
         task.add_done_callback(self._on_ack_reaction_task_done)
         return task
 
+    def _check_guild(self, guild) -> bool:
+        if self._allowed_guilds is None:
+            return True
+        return guild is not None and guild.id in self._allowed_guilds
+
     async def _on_message(self, message) -> None:
         if not self._running or not self._client:
             return
@@ -498,9 +551,8 @@ class DiscordChannel(Channel):
             return
 
         guild = message.guild
-        if self._allowed_guilds:
-            if guild is None or guild.id not in self._allowed_guilds:
-                return
+        if not self._check_guild(guild):
+            return
 
         text = (message.content or "").strip()
         if not text:

backend/tests/test_discord_channel.py

@@ -11,7 +11,7 @@ import sys
 import threading
 import weakref
 from types import SimpleNamespace
-from unittest.mock import AsyncMock, patch
+from unittest.mock import AsyncMock, MagicMock, patch
 
 import pytest
 
@@ -787,3 +787,115 @@ async def test_send_control_calls_keep_the_default_outbound_bound() -> None:
     assert len(run_mock.await_args_list) == 2  # stop_typing + one text chunk
     for call in run_mock.await_args_list:
         assert "timeout" not in call.kwargs
+
+
+# ---------------------------------------------------------------------------
+# allowed_guilds / allowed_channels parsing
+# ---------------------------------------------------------------------------
+
+
+class TestDiscordAllowedGuilds:
+    """A restriction the operator configured must never silently open the bot."""
+
+    @staticmethod
+    def _channel(config_extra: dict) -> DiscordChannel:
+        return DiscordChannel(bus=MessageBus(), config={"bot_token": "token", **config_extra})
+
+    @pytest.mark.parametrize("config_extra", [{}, {"allowed_guilds": None}, {"allowed_guilds": []}, {"allowed_guilds": " "}])
+    def test_unset_or_empty_allowlist_allows_every_guild_without_warning(self, config_extra, caplog):
+        with caplog.at_level(logging.WARNING, logger="app.channels.discord"):
+            channel = self._channel(config_extra)
+
+        assert channel._check_guild(SimpleNamespace(id=321))
+        assert channel._check_guild(None)
+        assert caplog.records == []
+
+    def test_listed_guild_ids_are_allowed_and_others_denied(self):
+        channel = self._channel({"allowed_guilds": [321, "654"]})
+
+        assert channel._check_guild(SimpleNamespace(id=321))
+        assert channel._check_guild(SimpleNamespace(id=654))
+        assert not channel._check_guild(SimpleNamespace(id=1))
+        assert not channel._check_guild(None)
+
+    @pytest.mark.parametrize("allowed_guilds", ["321", 321], ids=["str", "int"])
+    def test_scalar_guild_id_is_one_entry_not_its_digits(self, allowed_guilds):
+        # config.example.yaml sells this shorthand ("can also be a single guild
+        # ID"). Iterating the string "321" admitted guilds 1, 2 and 3 while
+        # blocking 321; an unquoted YAML int raised TypeError in the constructor.
+        channel = self._channel({"allowed_guilds": allowed_guilds})
+
+        assert channel._check_guild(SimpleNamespace(id=321))
+        assert not channel._check_guild(SimpleNamespace(id=3))
+
+    @pytest.mark.parametrize("bad_entry", ["@acme", "321,654", 0, -5, None, 4.2, True])
+    def test_unparseable_entry_is_dropped_with_warning(self, bad_entry, caplog):
+        with caplog.at_level(logging.WARNING, logger="app.channels.discord"):
+            channel = self._channel({"allowed_guilds": [321, bad_entry]})
+
+        assert channel._check_guild(SimpleNamespace(id=321))
+        assert not channel._check_guild(SimpleNamespace(id=1))
+        assert repr(bad_entry) in caplog.text
+        # 0, -5 and True are numeric, so the hint has to say what they lack.
+        assert "positive numeric guild ID" in caplog.text
+
+    @pytest.mark.parametrize(
+        "allowed_guilds",
+        # "321,654" is what a $ENV reference to a comma-separated value resolves
+        # to; the dict reaches the constructor from the runtime channel config.
+        [["@acme", "server"], "@acme", "321,654", [True], [4.2], {"id": 321}],
+    )
+    def test_allowlist_without_a_parseable_entry_denies_every_guild(self, allowed_guilds, caplog):
+        with caplog.at_level(logging.WARNING, logger="app.channels.discord"):
+            channel = self._channel({"allowed_guilds": allowed_guilds})
+
+        assert not channel._check_guild(SimpleNamespace(id=321))
+        assert not channel._check_guild(SimpleNamespace(id=3))
+        assert any(record.levelno == logging.ERROR for record in caplog.records)
+
+    @pytest.mark.asyncio
+    @pytest.mark.parametrize(("allowed_guilds", "admitted"), [("321", True), ("322", False)])
+    async def test_on_message_applies_the_allowlist(self, allowed_guilds, admitted):
+        # _make_discord_message reports guild ID 321 — the documented scalar.
+        channel = self._channel({"allowed_guilds": allowed_guilds})
+        channel._running = True
+        channel._client = SimpleNamespace(user=SimpleNamespace(id=999, mention="<@999>"))
+        channel._discord_module = SimpleNamespace(Thread=type("FakeThread", (), {}))
+        channel._main_loop = asyncio.get_running_loop()
+        # A refused capacity slot ends the hand-off here, so nothing is stubbed
+        # beyond it: the assertion is purely "did the gate let the message in".
+        channel._reserve_inbound = MagicMock(return_value=None)
+
+        await channel._on_message(_make_discord_message("hello"))
+
+        assert channel._reserve_inbound.called is admitted
+
+
+class TestDiscordAllowedChannels:
+    """``allowed_channels`` is parsed by the same lines, so it breaks the same way."""
+
+    @staticmethod
+    def _channel(config_extra: dict) -> DiscordChannel:
+        return DiscordChannel(bus=MessageBus(), config={"bot_token": "token", "mention_only": True, "thread_mode": False, **config_extra})
+
+    @pytest.mark.parametrize("allowed_channels", ["456", 456], ids=["str", "int"])
+    def test_scalar_channel_id_is_one_entry_not_its_digits(self, allowed_channels):
+        channel = self._channel({"allowed_channels": allowed_channels})
+
+        assert "456" in channel._allowed_channels
+        assert "4" not in channel._allowed_channels
+
+    @pytest.mark.asyncio
+    @pytest.mark.parametrize(("allowed_channels", "admitted"), [("456", True), ("457", False)])
+    async def test_exempt_channel_needs_no_mention(self, allowed_channels, admitted):
+        channel = self._channel({"allowed_channels": allowed_channels})
+        channel._running = True
+        channel._client = SimpleNamespace(user=SimpleNamespace(id=999, mention="<@999>"))
+        channel._discord_module = SimpleNamespace(Thread=type("FakeThread", (), {}))
+        channel._main_loop = asyncio.get_running_loop()
+        channel._reserve_inbound = MagicMock(return_value=MagicMock())
+        channel._publish_reserved = MagicMock(return_value=None)
+
+        await channel._on_message(_make_discord_message("hello"))
+
+        assert channel._publish_reserved.called is admitted

config.example.yaml

@@ -3039,9 +3039,15 @@ run_ownership:
 #   discord:
 #     enabled: false
 #     bot_token: $DISCORD_BOT_TOKEN
-#     allowed_guilds: []              # empty = allow all guilds; can also be a single guild ID
+#     # Allowed guild IDs: one positive numeric ID (quoted or unquoted) or a YAML list.
+#     # Unset, null, [], or a blank string allows all guilds.
+#     # Invalid entries are ignored with a warning. Any other configured value
+#     # with no valid ID denies every guild and logs an error.
+#     allowed_guilds: []
 #     mention_only: false             # If true, only respond when the bot is mentioned
-#     allowed_channels: []              # Optional: channel IDs exempt from mention_only (bot responds without mention)
+#     # Channel IDs exempt from mention_only: one ID (quoted or unquoted) or a YAML list.
+#     # Empty = no exemptions; mention_only applies to every channel when enabled.
+#     allowed_channels: []
 #     thread_mode: false               # If true, group a channel conversation into a thread
 #
 #   # Buzz (https://github.com/block/buzz) — Nostr-relay team workspace.