mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-01 12:02:05 +00:00
fix(discord): honor "*" wildcard in DISCORD_ALLOWED_USERS (#22334)
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.
This commit is contained in:
parent
1207d81eed
commit
851f75d4df
4 changed files with 100 additions and 3 deletions
|
|
@ -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())
|
||||
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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."""
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue