From 587bb10575016763cf151b152e5ba6a7ce21f193 Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Sat, 19 Sep 2026 23:35:59 -0700 Subject: [PATCH] fix(platforms): Discord/WhatsApp/DingTalk gates honour allowlists stored as a JSON-list string MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `hermes config set KEY '["-100","-200"]'` used to write the literal as one quoted YAML string; the writer now emits a real list (acaac9a18, #88163) and the Telegram gate decodes the legacy string shape (122ad719, #110213). The Discord, WhatsApp and DingTalk gate parsers still comma-split that string into `{'["-100"', '"-200"]'}`, so a config written before the writer fix silently locks every allowlisted chat, channel or user out — with no warning. Route every remaining comma-split gate through the shared `gateway/platforms/_shared.py::decode_json_list_literal`: - Discord `_gate_csv_set` (allowed/ignored/no-thread channels, allowed users/roles), `_discord_free_response_channels` and `_missed_message_backfill_channels` now share the one parser instead of three hand-rolled splits. - WhatsApp `_coerce_allow_list` (allow_from, group_allow_from, free_response_chats). - DingTalk `_csv_set` (allowed_users, allowed_chats, free_response_chats). Plain CSV strings, YAML lists and malformed JSON keep their previous meaning. --- gateway/platforms/whatsapp_common.py | 8 ++- plugins/platforms/dingtalk/adapter.py | 7 +-- plugins/platforms/discord/adapter.py | 25 ++++----- .../platforms/test_bracket_list_allowlists.py | 54 +++++++++++++++++++ 4 files changed, 73 insertions(+), 21 deletions(-) create mode 100644 tests/plugins/platforms/test_bracket_list_allowlists.py diff --git a/gateway/platforms/whatsapp_common.py b/gateway/platforms/whatsapp_common.py index 228cb530f8..1ac99c8839 100644 --- a/gateway/platforms/whatsapp_common.py +++ b/gateway/platforms/whatsapp_common.py @@ -18,7 +18,10 @@ import re from pathlib import Path from typing import Any, Dict, Optional -from gateway.platforms._shared import extra_or_secret as _extra_or_wsecret, get_scoped_secret as _get_wsecret +from gateway.platforms._shared import ( + decode_json_list_literal as _decode_json_list_literal, extra_or_secret as _extra_or_wsecret, + get_scoped_secret as _get_wsecret +) from gateway.platforms.access_policy_mixin import OwnAccessPolicyMixin @@ -99,9 +102,10 @@ class WhatsAppBehaviorMixin(OwnAccessPolicyMixin): @staticmethod def _coerce_allow_list(raw) -> set[str]: - """Parse allow_from / group_allow_from from config (list) or env var (CSV).""" + """Parse allow_from / group_allow_from from config (list or JSON-list string) or env var (CSV).""" if raw is None: return set() + raw = _decode_json_list_literal(raw) parts = raw if isinstance(raw, list) else str(raw).split(",") return {str(part).strip() for part in parts if str(part).strip()} diff --git a/plugins/platforms/dingtalk/adapter.py b/plugins/platforms/dingtalk/adapter.py index fabfe31f48..a4406fa16e 100644 --- a/plugins/platforms/dingtalk/adapter.py +++ b/plugins/platforms/dingtalk/adapter.py @@ -50,8 +50,8 @@ from gateway.platforms.helpers import MessageDeduplicator, compile_mention_patte from gateway.platforms.base import BasePlatformAdapter, SendResult from gateway.platforms.event import MessageEvent from gateway.platforms._shared import ( - apply_yaml_bridge as _apply_yaml_bridge, extra_or_secret as _extra_or_secret, - get_scoped_secret as _get_scoped_secret, send_error + apply_yaml_bridge as _apply_yaml_bridge, decode_json_list_literal as _decode_json_list_literal, + extra_or_secret as _extra_or_secret, get_scoped_secret as _get_scoped_secret, send_error ) from plugins.platforms.dingtalk.inbound import collect_download_codes, extract_media, extract_text @@ -73,7 +73,8 @@ _NO_LOCAL_UPLOAD = "DingTalk session webhook replies do not support local %s. On def _csv_set(raw: Any) -> Set[str]: - """Split a list or comma-separated string into a set of stripped, non-empty items.""" + """Split a list, JSON-list string or comma-separated string into a set of stripped, non-empty items.""" + raw = _decode_json_list_literal(raw) parts = raw if isinstance(raw, list) else str(raw).split(",") return {str(part).strip() for part in parts if str(part).strip()} diff --git a/plugins/platforms/discord/adapter.py b/plugins/platforms/discord/adapter.py index b36bb1ebcb..0355161850 100644 --- a/plugins/platforms/discord/adapter.py +++ b/plugins/platforms/discord/adapter.py @@ -272,8 +272,9 @@ from gateway.platforms.base import ( from gateway.platforms.event import MessageEvent, MessageType, ProcessingOutcome from tools.url_safety import is_safe_url from gateway.platforms._shared import ( - env_is_connected as _env_is_connected, extra_or_secret as _extra_or_secret, - platform_gate_env as _scoped_gate_env, send_error, yaml_env_setter as _yaml_env_setter + decode_json_list_literal as _decode_json_list_literal, env_is_connected as _env_is_connected, + extra_or_secret as _extra_or_secret, platform_gate_env as _scoped_gate_env, send_error, + yaml_env_setter as _yaml_env_setter ) # Every refusal (slash command, approval button, picker, prompt) says the same thing. @@ -2160,17 +2161,14 @@ class DiscordAdapter(DiscordMediaMixin, BasePlatformAdapter): free-response channels by default; ``channels: "*"`` scans every text channel.""" configured = self.config.extra.get("missed_message_backfill") if isinstance(configured, dict) and "channels" in configured: - raw = configured.get("channels") - if isinstance(raw, list): - return {str(item).strip() for item in raw if str(item).strip()} - raw = str(raw or "") - if raw.strip(): - return {item.strip() for item in raw.split(",") if item.strip()} + channels = self._gate_csv_set(configured.get("channels")) + if channels: + return channels raw = self._gate_env("DISCORD_MISSED_MESSAGE_BACKFILL_CHANNELS") if not raw.strip(): allowed = self._get_allowed_channels() return allowed | self._discord_free_response_channels() - return {item.strip() for item in raw.split(",") if item.strip()} + return self._gate_csv_set(raw) def _missed_message_backfill_number(self, key: str, env_key: str, default, cast, lo, hi=None): """Numeric ``missed_message_backfill.`` (dict extra wins over env), clamped to [lo, hi].""" @@ -4832,6 +4830,7 @@ class DiscordAdapter(DiscordMediaMixin, BasePlatformAdapter): def _gate_csv_set(raw) -> set: if raw is None: return set() + raw = _decode_json_list_literal(raw) if isinstance(raw, list): return {str(part).strip() for part in raw if str(part).strip()} return {part.strip() for part in str(raw).split(",") if part.strip()} @@ -4934,13 +4933,7 @@ class DiscordAdapter(DiscordMediaMixin, BasePlatformAdapter): raw = self.config.extra.get("free_response_channels") if raw is None: raw = self._gate_env("DISCORD_FREE_RESPONSE_CHANNELS") - if isinstance(raw, list): - return {str(part).strip() for part in raw if str(part).strip()} - # YAML parses a bare numeric value as int; str() any scalar before splitting. - s = str(raw).strip() if raw is not None else "" - if s: - return {part.strip() for part in s.split(",") if part.strip()} - return set() + return self._gate_csv_set(raw) def _raw_mentioned_user_ids(self, message: Any) -> set: """Extract user-mention IDs (``<@ID>`` and legacy ``<@!ID>``) from raw content, diff --git a/tests/plugins/platforms/test_bracket_list_allowlists.py b/tests/plugins/platforms/test_bracket_list_allowlists.py new file mode 100644 index 0000000000..d0194f522a --- /dev/null +++ b/tests/plugins/platforms/test_bracket_list_allowlists.py @@ -0,0 +1,54 @@ +"""Allowlists stored as a JSON-list *string* are honoured by every platform gate (issue #76457). + +Configs written by ``hermes config set KEY '["-100","-200"]'`` before the writer learned +to emit YAML lists hold the literal ``'["-100","-200"]'`` as a string. The Telegram gate +already decodes that shape (``gateway/platforms/_shared.py::decode_json_list_literal``); +the Discord / WhatsApp / DingTalk gates comma-split it into one bogus entry that matches +nothing, silently locking out every allowlisted chat or user. +""" + +from gateway.config import Platform, PlatformConfig + +BRACKET_LIST = '["-100", "-200"]' + + +def _discord(extra): + from plugins.platforms.discord.adapter import DiscordAdapter + + adapter = object.__new__(DiscordAdapter) + adapter.platform = Platform.DISCORD + adapter.config = PlatformConfig(enabled=True, token="x", extra=dict(extra)) + adapter._gate_env_snapshot = None + return adapter + + +def _whatsapp(extra): + from plugins.platforms.whatsapp.adapter import WhatsAppAdapter + + adapter = object.__new__(WhatsAppAdapter) + adapter.platform = Platform.WHATSAPP + adapter.config = PlatformConfig(enabled=True, extra=dict(extra)) + return adapter + + +def _dingtalk(extra): + from plugins.platforms.dingtalk.adapter import DingTalkAdapter + + return DingTalkAdapter(PlatformConfig(enabled=True, extra=dict(extra))) + + +def test_bracket_list_string_is_decoded_by_every_platform_gate(): + assert _discord({"allowed_channels": BRACKET_LIST})._get_allowed_channels() == {"-100", "-200"} + assert _discord({"free_response_channels": BRACKET_LIST})._discord_free_response_channels() == {"-100", "-200"} + assert _whatsapp({"free_response_chats": BRACKET_LIST})._whatsapp_free_response_chats() == {"-100", "-200"} + dingtalk = _dingtalk({"allowed_chats": BRACKET_LIST, "allowed_users": '["Alice"]'}) + assert dingtalk._dingtalk_allowed_chats() == {"-100", "-200"} + assert dingtalk._allowed_users == {"alice"} + + +def test_plain_csv_and_yaml_lists_keep_their_meaning(): + assert _discord({"allowed_channels": "-100, -200"})._get_allowed_channels() == {"-100", "-200"} + assert _discord({"allowed_channels": ["-100", "-200"]})._get_allowed_channels() == {"-100", "-200"} + assert _whatsapp({"free_response_chats": "a,b"})._whatsapp_free_response_chats() == {"a", "b"} + # Malformed JSON is not a list: it stays on the legacy comma-split path. + assert _dingtalk({"allowed_chats": "[not-json"})._dingtalk_allowed_chats() == {"[not-json"}