From 9b2151a3a892664743699146d43feb76a2351f0e Mon Sep 17 00:00:00 2001 From: "Grapette.L" <90967079+Lesereingrape@users.noreply.github.com> Date: Tue, 13 Oct 2026 14:55:49 +0800 Subject: [PATCH] fix(buzz): a single mention_free_channels ID was split into characters, a numeric one stopped the connector (#6776) * fix(buzz): accept a single mention-free channel id `_mention_free` iterated the configured value directly, so one unquoted UUID became a set of its characters and a numeric or boolean scalar raised TypeError while the channel was being constructed. Both shapes are documented as valid for the sibling Discord connector. Parse the key through a dedicated helper that follows the Discord convention: a scalar is a single entry, list entries are kept verbatim, and an unusable entry never matches so `require_mention` stays in force. * fix(channels): share scalar-or-list channel ID parsing * docs(channels): keep guidance within CI size budget --------- Co-authored-by: Willem Jiang --- backend/app/channels/AGENTS.md | 2 +- backend/app/channels/allowed_users.py | 65 ++++++++++++++++++++++----- backend/app/channels/buzz.py | 14 +++++- backend/app/channels/discord.py | 6 +-- backend/tests/test_allowed_users.py | 47 ++++++++++++++++++- backend/tests/test_buzz_channel.py | 35 +++++++++++++++ backend/tests/test_discord_channel.py | 14 ++++++ config.example.yaml | 3 +- 8 files changed, 168 insertions(+), 18 deletions(-) diff --git a/backend/app/channels/AGENTS.md b/backend/app/channels/AGENTS.md index c70653f15f..7c0764066d 100644 --- a/backend/app/channels/AGENTS.md +++ b/backend/app/channels/AGENTS.md @@ -68,7 +68,7 @@ The cached value is reused for both the blocking (`runs.wait`) and streaming (`_ - No public IP, OAuth callback URL, or provider webhook route is required by the current implementation. - Telegram binds via deep-link `/start ` over long polling; Slack, Discord, Feishu/Lark, DingTalk, WeChat, and WeCom use `/connect ` through their outbound workers. - WeChat `polling_timeout`, `polling_retry_delay`, `qrcode_poll_interval`, and `qrcode_poll_timeout` require positive finite seconds; invalid values use defaults to prevent hot loops or endless sleeps. -- `allowed_users.py` shares QQ/WeChat/Telegram parsing: unset/null/empty lists/blank strings allow all; invalid entries warn; all-invalid denies all with an error. QQ/WeChat opt into `warn_on_scalar_separators` without splitting IDs. WeChat converts integral numbers to text and gates before media downloads. +- `allowed_users.py`: QQ/WeChat/Telegram empty allows all; invalid warns; all-invalid denies all. WeChat coerces integral IDs and gates downloads. Buzz/Discord `parse_channel_ids`: trims IDs; empty = no exemption; no splitting/mapping keys. Buzz/QQ/WeChat warn on scalar separators. - WeCom serializes `start()` and `stop()` for each channel instance. The SDK `connect()` task covers connection setup only; after the handshake, the SDK owns a separate receive task. Shutdown cancels an in-progress connection attempt and awaits the SDK's actual asynchronous receive-task/socket cleanup before releasing lifecycle state or allowing a restart. Cancellation of `stop()` still propagates, but only after owned cleanup finishes and lifecycle references are cleared; real connection failures remain reported by `_on_ws_task_done`. - WeCom outbound content is capped at the protocol's 20480 UTF-8 bytes: stream replies clip on a character boundary with a truncation marker (one stream carries the whole reply and cannot split mid-way), while proactive pushes split into at most 10 sequential markdown messages per push, with the remaining tail clipped and marker-terminated. Both paths measure bytes, not characters. A per-chat send lock serializes each split batch end to end, because manager workers run concurrently and two long pushes to the same chat would otherwise interleave chunks; locks are reference-counted and reclaimed once no sender holds or waits on them, so the registry does not grow for the life of the Gateway. - Frontend APIs: `GET /api/channels/providers`, `GET /api/channels/connections`, `POST /api/channels/{provider}/connect`, and `DELETE /api/channels/connections/{connection_id}`. diff --git a/backend/app/channels/allowed_users.py b/backend/app/channels/allowed_users.py index 12982191be..6d7655a6da 100644 --- a/backend/app/channels/allowed_users.py +++ b/backend/app/channels/allowed_users.py @@ -1,4 +1,4 @@ -"""Shared fail-closed parsing for channel user allowlists.""" +"""Shared scalar-or-list parsing for channel ID configuration.""" from __future__ import annotations @@ -7,6 +7,52 @@ from collections.abc import Callable, Hashable from typing import Any +def _parse_id_entries( + value: Any, + *, + logger: logging.Logger, + channel_name: str, + key_name: str, + valid_id_name: str, + warn_on_scalar_separators: bool, +) -> list[Any]: + """Normalize config shape without splitting scalar IDs or iterating mappings.""" + if value is None or (isinstance(value, str) and not value.strip()): + return [] + if warn_on_scalar_separators and isinstance(value, str) and ("," in value or any(char.isspace() for char in value.strip())): + logger.warning( + "[%s] %s is a scalar string containing separators; treating it as one literal %s. Use a YAML list for multiple IDs", + channel_name, + key_name, + valid_id_name, + ) + return list(value) if isinstance(value, (list, tuple, set, frozenset)) else [value] + + +def parse_channel_ids( + value: Any, + *, + logger: logging.Logger, + channel_name: str, + key_name: str, + warn_on_scalar_separators: bool = False, +) -> frozenset[str]: + """Return trimmed literal channel IDs; empty config grants no exemptions. + + These IDs are compared as strings by the connector. An unusable literal + never matches, so it leaves the connector's mention requirement in force. + """ + entries = _parse_id_entries( + value, + logger=logger, + channel_name=channel_name, + key_name=key_name, + valid_id_name="channel id", + warn_on_scalar_separators=warn_on_scalar_separators, + ) + return frozenset(text for text in (str(entry).strip() for entry in entries) if text) + + def parse_allowed_users[UserId: Hashable]( allowed_users: Any, *, @@ -26,15 +72,14 @@ def parse_allowed_users[UserId: Hashable]( String-ID channels can opt into a scalar-separator warning without splitting the ID; surrounding whitespace alone does not trigger it. """ - if allowed_users is None or (isinstance(allowed_users, str) and not allowed_users.strip()): - return None - if warn_on_scalar_separators and isinstance(allowed_users, str) and ("," in allowed_users or any(char.isspace() for char in allowed_users.strip())): - logger.warning( - "[%s] allowed_users is a scalar string containing separators; treating it as one literal %s. Use a YAML list for multiple IDs", - channel_name, - valid_id_name, - ) - entries = list(allowed_users) if isinstance(allowed_users, (list, tuple, set, frozenset)) else [allowed_users] + entries = _parse_id_entries( + allowed_users, + logger=logger, + channel_name=channel_name, + key_name="allowed_users", + valid_id_name=valid_id_name, + warn_on_scalar_separators=warn_on_scalar_separators, + ) if not entries: return None diff --git a/backend/app/channels/buzz.py b/backend/app/channels/buzz.py index 35f056f9d8..bc825f124b 100644 --- a/backend/app/channels/buzz.py +++ b/backend/app/channels/buzz.py @@ -49,6 +49,7 @@ from typing import Any from urllib.parse import urlparse from app.channels import buzz_nostr +from app.channels.allowed_users import parse_channel_ids from app.channels.base import Channel from app.channels.buzz_seen_events import BuzzSeenEventStore from app.channels.commands import is_known_channel_command @@ -249,6 +250,17 @@ def _is_transient_close(reason: str) -> bool: return not any(marker in text for marker in _PERMANENT_CLOSE_MARKERS) +def _parse_mention_free_channels(mention_free_channels: Any) -> frozenset[str]: + """Parse Buzz mention exemptions without splitting scalar channel IDs.""" + return parse_channel_ids( + mention_free_channels, + logger=logger, + channel_name="buzz", + key_name="mention_free_channels", + warn_on_scalar_separators=True, + ) + + class BuzzChannel(Channel): _connect: Any = None # test seam: async callable returning an async-context-manager transport @@ -267,7 +279,7 @@ class BuzzChannel(Channel): self._keys: buzz_nostr.NostrKeys | None = None # parsed in start() so coincurve stays lazy self._allowed_users = {buzz_nostr.parse_pubkey(v) for v in config.get("allowed_users", []) or []} self._require_mention = bool(config.get("require_mention", True)) - self._mention_free = {str(c) for c in config.get("mention_free_channels", []) or []} + self._mention_free: frozenset[str] = _parse_mention_free_channels(config.get("mention_free_channels")) self._channel_meta: dict[str, dict[str, Any]] = {} self._stream_targets: dict[tuple[str, str | None], str] = {} self._stream_tails: dict[tuple[str, str | None], list[str]] = {} # overflow chunk ids beyond chunk 0, per conversation diff --git a/backend/app/channels/discord.py b/backend/app/channels/discord.py index 5b800ef120..e85ea80033 100644 --- a/backend/app/channels/discord.py +++ b/backend/app/channels/discord.py @@ -11,6 +11,7 @@ from concurrent.futures import Future from pathlib import Path from typing import Any +from app.channels.allowed_users import parse_channel_ids from app.channels.base import Channel from app.channels.commands import is_known_channel_command from app.channels.connection_identity import attach_connection_identity @@ -81,10 +82,7 @@ def _parse_allowed_channels(allowed_channels: Any) -> frozenset[str]: 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) + return parse_channel_ids(allowed_channels, logger=logger, channel_name="Discord", key_name="allowed_channels") class DiscordChannel(Channel): diff --git a/backend/tests/test_allowed_users.py b/backend/tests/test_allowed_users.py index 503807f32f..6c51c625e9 100644 --- a/backend/tests/test_allowed_users.py +++ b/backend/tests/test_allowed_users.py @@ -5,7 +5,7 @@ from typing import Any import pytest -from app.channels.allowed_users import parse_allowed_users +from app.channels.allowed_users import parse_allowed_users, parse_channel_ids logger = logging.getLogger(__name__) @@ -81,3 +81,48 @@ def test_opted_in_warning_only_checks_scalar_separators(allowed_users, expected, assert result == expected assert caplog.records == [] + + +@pytest.mark.parametrize( + "value, expected", + [ + (None, frozenset()), + (" \t\n ", frozenset()), + ([], frozenset()), + ((), frozenset()), + (set(), frozenset()), + (frozenset(), frozenset()), + (" 123 ", frozenset({"123"})), + (123, frozenset({"123"})), + (["123", " 456 ", "", "123"], frozenset({"123", "456"})), + (("123", "456"), frozenset({"123", "456"})), + ({"123", "456"}, frozenset({"123", "456"})), + (frozenset({"123", "456"}), frozenset({"123", "456"})), + ({"123": True}, frozenset({"{'123': True}"})), + ([None, True], frozenset({"None", "True"})), + (["123,456", "789 012"], frozenset({"123,456", "789 012"})), + ], +) +def test_channel_ids_preserve_literal_entries_and_empty_policy(value, expected, caplog): + result = parse_channel_ids(value, logger=logger, channel_name="buzz", key_name="mention_free_channels", warn_on_scalar_separators=True) + + assert result == expected + assert caplog.records == [] + + +@pytest.mark.parametrize("separator", [",", ", ", " ", "\t", "\n", "\u2003"]) +@pytest.mark.parametrize("channel_name, key_name, warn", [("buzz", "mention_free_channels", True), ("Discord", "allowed_channels", False)]) +def test_channel_id_warning_names_config_key_and_preserves_scalar(channel_name, key_name, warn, separator, caplog): + literal_id = f"123{separator}456" + with caplog.at_level(logging.WARNING, logger=logger.name): + result = parse_channel_ids(literal_id, logger=logger, channel_name=channel_name, key_name=key_name, warn_on_scalar_separators=warn) + + assert result == frozenset({literal_id}) + assert len(caplog.records) == int(warn) + if warn: + warning = caplog.records[0] + assert warning.levelno == logging.WARNING + assert f"[{channel_name}] {key_name}" in warning.message + assert "one literal channel id" in warning.message + assert "YAML list" in warning.message + assert "123" not in warning.message and "456" not in warning.message diff --git a/backend/tests/test_buzz_channel.py b/backend/tests/test_buzz_channel.py index d0a69c5528..b22df0b7ba 100644 --- a/backend/tests/test_buzz_channel.py +++ b/backend/tests/test_buzz_channel.py @@ -63,6 +63,41 @@ def test_config_parsing_normalizes_allowlist_and_defaults(): assert ch._relay_url == "wss://buzz.example.com" +def test_config_mention_free_channel_accepts_a_single_id(): + """One UUID, quoted or unquoted, is one entry -- not the characters of it. + + A scalar was iterated as if it were a list, so the exemption matched nothing: + the channel the operator just made mention-free kept requiring a mention, which + looks exactly like a deaf bot. Discord documents and parses the same shape. + """ + assert _channel(mention_free_channels=CHANNEL)._mention_free == frozenset({CHANNEL}) + assert _channel(mention_free_channels=f" {CHANNEL} ")._mention_free == frozenset({CHANNEL}) + assert CHANNEL in _channel(mention_free_channels=CHANNEL)._mention_free + + +def test_config_mention_free_channel_accepts_a_non_string_scalar(): + """An unquoted value YAML typed as a number must not abort channel setup.""" + assert _channel(mention_free_channels=136852)._mention_free == frozenset({"136852"}) + + +def test_config_mention_free_channel_empty_shapes_mean_no_exemption(): + for value in (None, "", " ", [], frozenset()): + assert _channel(mention_free_channels=value)._mention_free == frozenset() + + +def test_config_mention_free_channel_keeps_list_entries_verbatim(): + ch = _channel(mention_free_channels=[CHANNEL, f" {CHANNEL_B} ", ""]) + assert ch._mention_free == frozenset({CHANNEL, CHANNEL_B}) + + +def test_config_mention_free_channel_scalar_with_separators_warns(caplog): + """A comma/space-joined scalar is one literal id, and says so rather than going quiet.""" + with caplog.at_level(logging.WARNING, logger="app.channels.buzz"): + ch = _channel(mention_free_channels=f"{CHANNEL}, {CHANNEL_B}") + assert ch._mention_free == frozenset({f"{CHANNEL}, {CHANNEL_B}"}) + assert "treating it as one literal channel id" in caplog.text + + def test_config_rejects_non_websocket_relay_url(): with pytest.raises(ValueError): _channel(relay_url="https://buzz.example.com") diff --git a/backend/tests/test_discord_channel.py b/backend/tests/test_discord_channel.py index d75f91dd6f..124b83257f 100644 --- a/backend/tests/test_discord_channel.py +++ b/backend/tests/test_discord_channel.py @@ -36,6 +36,20 @@ def test_discord_channel_init() -> None: assert channel.name == "discord" +@pytest.mark.parametrize("collection", [list, tuple, set, frozenset]) +def test_allowed_channels_accepts_all_id_collections(collection): + channel = DiscordChannel(bus=MessageBus(), config={"allowed_channels": collection([" 456 ", "789", ""])}) + + assert channel._allowed_channels == frozenset({"456", "789"}) + + +@pytest.mark.parametrize("value", [None, "", " \t ", [], (), set(), frozenset()]) +def test_empty_allowed_channels_gives_no_mention_exemption(value): + channel = DiscordChannel(bus=MessageBus(), config={"allowed_channels": value}) + + assert channel._allowed_channels == frozenset() + + @pytest.mark.asyncio async def test_start_without_discord_module_points_at_the_extra(caplog) -> None: """discord.py ships in the optional ``discord`` extra, so the missing-dependency diff --git a/config.example.yaml b/config.example.yaml index 40e231405c..9f33104771 100644 --- a/config.example.yaml +++ b/config.example.yaml @@ -3211,7 +3211,8 @@ run_ownership: # # DENY-BY-DEFAULT (unlike other channels): empty # # means nobody, and DeerFlow logs a startup warning. # require_mention: true # @mention needed in channels -# mention_free_channels: [] # channel UUIDs that respond to every message +# mention_free_channels: [] # Channel UUIDs that respond to every message: +# # one UUID (quoted or unquoted) or a YAML list. # ============================================================================ # Guardrails Configuration