From 851f75d4df0a1b5fb4d58c78087652c311add903 Mon Sep 17 00:00:00 2001 From: bykim0119 Date: Sat, 27 Jun 2026 18:58:31 -0700 Subject: [PATCH] fix(discord): honor "*" wildcard in DISCORD_ALLOWED_USERS (#22334) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DISCORD_ALLOWED_USERS="*" now means "allow everyone", matching the SIGNAL_ALLOWED_USERS / DISCORD_ALLOWED_CHANNELS wildcard convention and the value `claw migrate` emits. Previously _is_allowed_user did exact ID matching only, so "*" matched no user and blocked every non-self sender — a P1 with no workaround. Three sites, all required for the fix to hold at runtime: - _is_allowed_user: short-circuit when "*" is in the allowlist. - connect(): exclude "*" from the intents.members trigger so the wildcard does not request the privileged Server Members intent (which can block the bot from coming online). - _resolve_allowed_usernames: preserve "*" verbatim; otherwise it lands in the username-resolution bucket, matches no member, and is silently dropped from the set and env var on the first on_ready — quietly undoing the fix. Slash auth delegates to _is_allowed_user (auto-covered); component auth already honors "*" on main. --- plugins/platforms/discord/adapter.py | 24 +++++++++++-- scripts/release.py | 1 + tests/gateway/test_discord_connect.py | 52 +++++++++++++++++++++++++++ tests/gateway/test_voice_command.py | 26 ++++++++++++++ 4 files changed, 100 insertions(+), 3 deletions(-) diff --git a/plugins/platforms/discord/adapter.py b/plugins/platforms/discord/adapter.py index d4ae59843bc..4327ce59afb 100644 --- a/plugins/platforms/discord/adapter.py +++ b/plugins/platforms/discord/adapter.py @@ -934,7 +934,12 @@ class DiscordAdapter(BasePlatformAdapter): intents.dm_messages = True intents.guild_messages = True intents.members = ( - any(not entry.isdigit() for entry in self._allowed_user_ids) + # ``"*"`` is the open-mode wildcard (honored in _is_allowed_user), + # not a username to resolve, so it must not pull in the privileged + # Server Members intent — exactly the migrate-from-OpenClaw path + # the wildcard fix targets would otherwise silently fail to come + # online when Members Intent isn't enabled in the Developer Portal. + any(entry != "*" and not entry.isdigit() for entry in self._allowed_user_ids) or bool(self._allowed_role_ids) # Need members intent for role lookup ) intents.voice_states = True @@ -2766,8 +2771,12 @@ class DiscordAdapter(BasePlatformAdapter): has_roles = bool(allowed_roles) if not has_users and not has_roles: return True - # Check user ID allowlist (works for both DMs and guild messages) - if has_users and user_id in allowed_users: + # Check user ID allowlist (works for both DMs and guild messages). + # ``"*"`` is honored as an open-mode wildcard, mirroring + # ``SIGNAL_ALLOWED_USERS`` and the existing ``DISCORD_ALLOWED_CHANNELS`` / + # ``DISCORD_IGNORED_CHANNELS`` / ``DISCORD_FREE_RESPONSE_CHANNELS`` + # semantics. This is the convention ``claw migrate`` emits ("*"). + if has_users and ("*" in allowed_users or user_id in allowed_users): return True # Role allowlist is only consulted when configured. if not has_roles: @@ -3363,6 +3372,15 @@ class DiscordAdapter(BasePlatformAdapter): for entry in self._allowed_user_ids: if entry.isdigit(): numeric_ids.add(entry) + elif entry == "*": + # Preserve the open-mode wildcard verbatim. It is not a + # username to resolve; without this branch it would land in + # ``to_resolve``, fail to match any guild member, and then be + # silently dropped from both ``self._allowed_user_ids`` and + # ``DISCORD_ALLOWED_USERS`` by the rewrite below — quietly + # undoing the wildcard fix in ``_is_allowed_user`` after the + # first ``on_ready``. + numeric_ids.add(entry) else: to_resolve.add(entry.lower()) diff --git a/scripts/release.py b/scripts/release.py index c2c9ad556ff..662a70c99fd 100755 --- a/scripts/release.py +++ b/scripts/release.py @@ -50,6 +50,7 @@ AUTHOR_MAP = { "blaryx@gmail.com": "Blaryxoff", # PR #32602 salvage (deep-merge PUT /api/config to preserve unrelated sections; #13396) "diamondeyesfox@gmail.com": "DiamondEyesFox", # PR #53351 salvage (rebaseline in-place compression flushes to prevent duplicate compacted rows; #9096) "piyrw9754@gmail.com": "rlaope", # PR #35075 salvage (align cron invisible-unicode set with install-time scanner; #35075) + "bukim0119@gmail.com": "bykim0119", # PR #22335 salvage (honor "*" wildcard in DISCORD_ALLOWED_USERS; #22334) "rebel@rebels-Mac-Studio-2.local": "rebel0789", # PR #47308 salvage (redact browser_type typed text across display surfaces; #47197) "267614622+agt-user@users.noreply.github.com": "agt-user", # PR #48496 salvage (telegram CLOSE-WAIT polling heartbeat, #48495) "80915+DavidMetcalfe@users.noreply.github.com": "DavidMetcalfe", # PR #52272 salvage (route reasoning-model thinking-timeouts to timeout not context_overflow + reasoning-specific guidance; #52271) diff --git a/tests/gateway/test_discord_connect.py b/tests/gateway/test_discord_connect.py index 2f12138feb0..32dc3d27fe7 100644 --- a/tests/gateway/test_discord_connect.py +++ b/tests/gateway/test_discord_connect.py @@ -1,5 +1,6 @@ import asyncio import json +import os import sys from types import SimpleNamespace from unittest.mock import AsyncMock, MagicMock @@ -146,6 +147,13 @@ class SlowSyncBot(FakeBot): ("769524422783664158", False), ("abhey-gupta", True), ("769524422783664158,abhey-gupta", True), + # ``"*"`` is the open-mode wildcard, not a username to resolve, so it + # must not pull in the privileged Server Members intent. Requesting + # that intent without it being enabled in the Discord Developer Portal + # can prevent the bot from coming online at all — and that is exactly + # the migration-from-OpenClaw path the wildcard fix targets (#22334). + ("*", False), + ("769524422783664158,*", False), ], ) async def test_connect_only_requests_members_intent_when_needed(monkeypatch, allowed_users, expected_members_intent): @@ -182,6 +190,50 @@ async def test_connect_only_requests_members_intent_when_needed(monkeypatch, all await adapter.disconnect() +@pytest.mark.asyncio +@pytest.mark.parametrize( + "initial_allowed", + [ + {"*"}, + {"769524422783664158", "*"}, + ], +) +async def test_resolve_allowed_usernames_preserves_wildcard(monkeypatch, initial_allowed): + """Regression (#22334): ``_resolve_allowed_usernames`` must not strip ``"*"``. + + The method splits entries into numeric-IDs (kept) vs non-numeric (treated + as usernames to resolve), then rewrites ``self._allowed_user_ids`` and the + ``DISCORD_ALLOWED_USERS`` env var from the numeric set. Since ``"*"`` is + non-numeric, without the wildcard branch it ends up in the resolution + bucket, fails to match any guild member, and is silently dropped from both + the in-memory set and the env var on the first ``on_ready`` — quietly + undoing the wildcard fix in ``_is_allowed_user`` for every subsequent + message. + """ + adapter = DiscordAdapter(PlatformConfig(enabled=True, token="test-token")) + adapter._allowed_user_ids = set(initial_allowed) + adapter._client = SimpleNamespace(guilds=[]) # no guilds → no resolution work + + monkeypatch.setenv("DISCORD_ALLOWED_USERS", ",".join(sorted(initial_allowed))) + + await adapter._resolve_allowed_usernames() + + assert "*" in adapter._allowed_user_ids, ( + "wildcard must survive _resolve_allowed_usernames; it is the open-mode " + "marker, not a username to resolve" + ) + env_entries = { + e.strip() + for e in os.environ.get("DISCORD_ALLOWED_USERS", "").split(",") + if e.strip() + } + assert "*" in env_entries, ( + "DISCORD_ALLOWED_USERS env must still contain '*' after resolution; " + "downstream readers (and any restart-time re-parse) rely on it" + ) + + + @pytest.mark.asyncio async def test_reconnect_closes_previous_client_to_prevent_zombie_websocket(monkeypatch): """Regression for #18187: calling connect() twice without disconnect() in diff --git a/tests/gateway/test_voice_command.py b/tests/gateway/test_voice_command.py index 4d52591a230..6c408cb05e8 100644 --- a/tests/gateway/test_voice_command.py +++ b/tests/gateway/test_voice_command.py @@ -1223,6 +1223,32 @@ class TestDiscordVoiceChannelMethods: adapter._allowed_user_ids = {"99"} assert adapter._is_allowed_user("42") is False + def test_is_allowed_user_wildcard_only(self): + """``DISCORD_ALLOWED_USERS="*"`` opens access to all users. + + Mirrors ``SIGNAL_ALLOWED_USERS`` and the existing + ``DISCORD_ALLOWED_CHANNELS`` / ``_IGNORED_CHANNELS`` / + ``_FREE_RESPONSE_CHANNELS`` wildcard handling. This is the + convention ``claw migrate`` emits (#22334). + """ + adapter = self._make_adapter() + adapter._allowed_user_ids = {"*"} + assert adapter._is_allowed_user("42") is True + assert adapter._is_allowed_user("999999999999999999") is True + + def test_is_allowed_user_wildcard_mixed_with_ids(self): + """``DISCORD_ALLOWED_USERS="123,*"`` honors ``*`` for any user.""" + adapter = self._make_adapter() + adapter._allowed_user_ids = {"123456789012345678", "*"} + assert adapter._is_allowed_user("42") is True + assert adapter._is_allowed_user("123456789012345678") is True + + def test_is_allowed_user_wildcard_in_dm(self): + """Wildcard short-circuits before role-auth gating, so DMs honor it too.""" + adapter = self._make_adapter() + adapter._allowed_user_ids = {"*"} + assert adapter._is_allowed_user("42", is_dm=True) is True + @pytest.mark.asyncio async def test_process_voice_input_success(self): """Successful voice input: PCM->WAV->STT->callback."""