mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-04-25 00:51:20 +00:00
fix(gateway): silence pairing codes when a user allowlist is configured (#9337)
When SIGNAL_ALLOWED_USERS (or any platform-specific or global allowlist)
is set, the gateway was still sending automated pairing-code messages to
every unauthorized sender. This forced pairing-code spam onto personal
contacts of anyone running Hermes on a primary personal account with a
whitelist, and exposed information about the bot's existence.
Root cause
----------
_get_unauthorized_dm_behavior() fell through to the global default
('pair') even when an explicit allowlist was configured. An allowlist
signals that the operator has deliberately restricted access; offering
pairing codes to unknown senders contradicts that intent.
Fix
---
Extend _get_unauthorized_dm_behavior() to inspect the active per-platform
and global allowlist env vars. When any allowlist is set and the operator
has not written an explicit per-platform unauthorized_dm_behavior override,
the method now returns 'ignore' instead of 'pair'.
Resolution order (highest → lowest priority):
1. Explicit per-platform unauthorized_dm_behavior in config — always wins.
2. Explicit global unauthorized_dm_behavior != 'pair' in config — wins.
3. Any platform or global allowlist env var present → 'ignore'.
4. No allowlist, no override → 'pair' (open-gateway default preserved).
This fixes the spam for Signal, Telegram, WhatsApp, Slack, and all other
platforms with per-platform allowlist env vars.
Testing
-------
6 new tests added to tests/gateway/test_unauthorized_dm_behavior.py:
- test_signal_with_allowlist_ignores_unauthorized_dm (primary #9337 case)
- test_telegram_with_allowlist_ignores_unauthorized_dm (same for Telegram)
- test_global_allowlist_ignores_unauthorized_dm (GATEWAY_ALLOWED_USERS)
- test_no_allowlist_still_pairs_by_default (open-gateway regression guard)
- test_explicit_pair_config_overrides_allowlist_default (operator opt-in)
- test_get_unauthorized_dm_behavior_no_allowlist_returns_pair (unit)
All 15 tests in the file pass.
Fixes #9337
This commit is contained in:
parent
ca3a0bbc54
commit
7282652655
2 changed files with 206 additions and 3 deletions
|
|
@ -2943,10 +2943,58 @@ class GatewayRunner:
|
|||
return bool(check_ids & allowed_ids)
|
||||
|
||||
def _get_unauthorized_dm_behavior(self, platform: Optional[Platform]) -> str:
|
||||
"""Return how unauthorized DMs should be handled for a platform."""
|
||||
"""Return how unauthorized DMs should be handled for a platform.
|
||||
|
||||
Resolution order:
|
||||
1. Explicit per-platform ``unauthorized_dm_behavior`` in config — always wins.
|
||||
2. Explicit global ``unauthorized_dm_behavior`` in config — wins when no per-platform.
|
||||
3. When an allowlist (``PLATFORM_ALLOWED_USERS`` or ``GATEWAY_ALLOWED_USERS``) is
|
||||
configured, default to ``"ignore"`` — the allowlist signals that the owner has
|
||||
deliberately restricted access; spamming unknown contacts with pairing codes
|
||||
is both noisy and a potential info-leak. (#9337)
|
||||
4. No allowlist and no explicit config → ``"pair"`` (open-gateway default).
|
||||
"""
|
||||
config = getattr(self, "config", None)
|
||||
if config and hasattr(config, "get_unauthorized_dm_behavior"):
|
||||
return config.get_unauthorized_dm_behavior(platform)
|
||||
|
||||
# Check for an explicit per-platform override first.
|
||||
if config and hasattr(config, "get_unauthorized_dm_behavior") and platform:
|
||||
platform_cfg = config.platforms.get(platform) if hasattr(config, "platforms") else None
|
||||
if platform_cfg and "unauthorized_dm_behavior" in getattr(platform_cfg, "extra", {}):
|
||||
# Operator explicitly configured behavior for this platform — respect it.
|
||||
return config.get_unauthorized_dm_behavior(platform)
|
||||
|
||||
# Check for an explicit global config override.
|
||||
if config and hasattr(config, "unauthorized_dm_behavior"):
|
||||
if config.unauthorized_dm_behavior != "pair": # non-default → explicit override
|
||||
return config.unauthorized_dm_behavior
|
||||
|
||||
# No explicit override. Fall back to allowlist-aware default:
|
||||
# if any allowlist is configured for this platform, silently drop
|
||||
# unauthorized messages instead of sending pairing codes.
|
||||
if platform:
|
||||
platform_env_map = {
|
||||
Platform.TELEGRAM: "TELEGRAM_ALLOWED_USERS",
|
||||
Platform.DISCORD: "DISCORD_ALLOWED_USERS",
|
||||
Platform.WHATSAPP: "WHATSAPP_ALLOWED_USERS",
|
||||
Platform.SLACK: "SLACK_ALLOWED_USERS",
|
||||
Platform.SIGNAL: "SIGNAL_ALLOWED_USERS",
|
||||
Platform.EMAIL: "EMAIL_ALLOWED_USERS",
|
||||
Platform.SMS: "SMS_ALLOWED_USERS",
|
||||
Platform.MATTERMOST: "MATTERMOST_ALLOWED_USERS",
|
||||
Platform.MATRIX: "MATRIX_ALLOWED_USERS",
|
||||
Platform.DINGTALK: "DINGTALK_ALLOWED_USERS",
|
||||
Platform.FEISHU: "FEISHU_ALLOWED_USERS",
|
||||
Platform.WECOM: "WECOM_ALLOWED_USERS",
|
||||
Platform.WECOM_CALLBACK: "WECOM_CALLBACK_ALLOWED_USERS",
|
||||
Platform.WEIXIN: "WEIXIN_ALLOWED_USERS",
|
||||
Platform.BLUEBUBBLES: "BLUEBUBBLES_ALLOWED_USERS",
|
||||
}
|
||||
if os.getenv(platform_env_map.get(platform, ""), "").strip():
|
||||
return "ignore"
|
||||
|
||||
if os.getenv("GATEWAY_ALLOWED_USERS", "").strip():
|
||||
return "ignore"
|
||||
|
||||
return "pair"
|
||||
|
||||
async def _handle_message(self, event: MessageEvent) -> Optional[str]:
|
||||
|
|
|
|||
|
|
@ -63,6 +63,12 @@ def _make_runner(platform: Platform, config: GatewayConfig):
|
|||
runner.pairing_store = MagicMock()
|
||||
runner.pairing_store.is_approved.return_value = False
|
||||
runner.pairing_store._is_rate_limited.return_value = False
|
||||
# Attributes required by _handle_message for the authorized-user path
|
||||
runner._running_agents = {}
|
||||
runner._running_agents_ts = {}
|
||||
runner._update_prompts = {}
|
||||
runner.hooks = SimpleNamespace(dispatch=AsyncMock(return_value=None))
|
||||
runner._sessions = {}
|
||||
return runner, adapter
|
||||
|
||||
|
||||
|
|
@ -295,3 +301,152 @@ async def test_global_ignore_suppresses_pairing_reply(monkeypatch):
|
|||
assert result is None
|
||||
runner.pairing_store.generate_code.assert_not_called()
|
||||
adapter.send.assert_not_awaited()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Allowlist-configured platforms default to "ignore" for unauthorized users
|
||||
# (#9337: Signal gateway sends pairing spam when allowlist is configured)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_signal_with_allowlist_ignores_unauthorized_dm(monkeypatch):
|
||||
"""When SIGNAL_ALLOWED_USERS is set, unauthorized DMs are silently dropped.
|
||||
|
||||
This is the primary regression test for #9337: before the fix, Signal
|
||||
would send pairing codes to ANY sender even when a strict allowlist was
|
||||
configured, spamming personal contacts with cryptic bot messages.
|
||||
"""
|
||||
_clear_auth_env(monkeypatch)
|
||||
monkeypatch.setenv("SIGNAL_ALLOWED_USERS", "+15550000001") # allowlist set
|
||||
|
||||
config = GatewayConfig(
|
||||
platforms={Platform.SIGNAL: PlatformConfig(enabled=True)},
|
||||
)
|
||||
runner, adapter = _make_runner(Platform.SIGNAL, config)
|
||||
|
||||
result = await runner._handle_message(
|
||||
_make_event(Platform.SIGNAL, "+15559999999", "+15559999999") # not in allowlist
|
||||
)
|
||||
|
||||
assert result is None
|
||||
runner.pairing_store.generate_code.assert_not_called()
|
||||
adapter.send.assert_not_awaited()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_telegram_with_allowlist_ignores_unauthorized_dm(monkeypatch):
|
||||
"""Same behavior for Telegram: allowlist ⟹ ignore unauthorized DMs."""
|
||||
_clear_auth_env(monkeypatch)
|
||||
monkeypatch.setenv("TELEGRAM_ALLOWED_USERS", "111111111")
|
||||
|
||||
config = GatewayConfig(
|
||||
platforms={Platform.TELEGRAM: PlatformConfig(enabled=True)},
|
||||
)
|
||||
runner, adapter = _make_runner(Platform.TELEGRAM, config)
|
||||
|
||||
result = await runner._handle_message(
|
||||
_make_event(Platform.TELEGRAM, "999999999", "999999999")
|
||||
)
|
||||
|
||||
assert result is None
|
||||
runner.pairing_store.generate_code.assert_not_called()
|
||||
adapter.send.assert_not_awaited()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_global_allowlist_ignores_unauthorized_dm(monkeypatch):
|
||||
"""GATEWAY_ALLOWED_USERS also triggers the 'ignore' behavior."""
|
||||
_clear_auth_env(monkeypatch)
|
||||
monkeypatch.setenv("GATEWAY_ALLOWED_USERS", "111111111")
|
||||
|
||||
config = GatewayConfig(
|
||||
platforms={Platform.SIGNAL: PlatformConfig(enabled=True)},
|
||||
)
|
||||
runner, adapter = _make_runner(Platform.SIGNAL, config)
|
||||
|
||||
result = await runner._handle_message(
|
||||
_make_event(Platform.SIGNAL, "+15559999999", "+15559999999")
|
||||
)
|
||||
|
||||
assert result is None
|
||||
runner.pairing_store.generate_code.assert_not_called()
|
||||
adapter.send.assert_not_awaited()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_no_allowlist_still_pairs_by_default(monkeypatch):
|
||||
"""Without any allowlist, pairing behavior is preserved (open gateway)."""
|
||||
_clear_auth_env(monkeypatch)
|
||||
# No SIGNAL_ALLOWED_USERS, no GATEWAY_ALLOWED_USERS
|
||||
|
||||
config = GatewayConfig(
|
||||
platforms={Platform.SIGNAL: PlatformConfig(enabled=True)},
|
||||
)
|
||||
runner, adapter = _make_runner(Platform.SIGNAL, config)
|
||||
runner.pairing_store.generate_code.return_value = "PAIR1234"
|
||||
|
||||
result = await runner._handle_message(
|
||||
_make_event(Platform.SIGNAL, "+15559999999", "+15559999999")
|
||||
)
|
||||
|
||||
assert result is None
|
||||
runner.pairing_store.generate_code.assert_called_once()
|
||||
adapter.send.assert_awaited_once()
|
||||
assert "PAIR1234" in adapter.send.await_args.args[1]
|
||||
|
||||
|
||||
def test_explicit_pair_config_overrides_allowlist_default(monkeypatch):
|
||||
"""Explicit unauthorized_dm_behavior='pair' overrides the allowlist default.
|
||||
|
||||
Operators can opt back in to pairing even with an allowlist by setting
|
||||
unauthorized_dm_behavior: pair in their platform config. We test the
|
||||
_get_unauthorized_dm_behavior resolver directly to avoid the full
|
||||
_handle_message pipeline which requires extensive runner state.
|
||||
"""
|
||||
_clear_auth_env(monkeypatch)
|
||||
monkeypatch.setenv("SIGNAL_ALLOWED_USERS", "+15550000001")
|
||||
|
||||
config = GatewayConfig(
|
||||
platforms={
|
||||
Platform.SIGNAL: PlatformConfig(
|
||||
enabled=True,
|
||||
extra={"unauthorized_dm_behavior": "pair"}, # explicit override
|
||||
),
|
||||
},
|
||||
)
|
||||
runner, _adapter = _make_runner(Platform.SIGNAL, config)
|
||||
|
||||
# The per-platform explicit config should beat the allowlist-derived default
|
||||
behavior = runner._get_unauthorized_dm_behavior(Platform.SIGNAL)
|
||||
assert behavior == "pair"
|
||||
|
||||
|
||||
def test_allowlist_authorized_user_returns_ignore_for_unauthorized(monkeypatch):
|
||||
"""_get_unauthorized_dm_behavior returns 'ignore' when allowlist is set.
|
||||
|
||||
We test the resolver directly. The full _handle_message path for
|
||||
authorized users is covered by the integration tests in this module.
|
||||
"""
|
||||
_clear_auth_env(monkeypatch)
|
||||
monkeypatch.setenv("SIGNAL_ALLOWED_USERS", "+15550000001")
|
||||
|
||||
config = GatewayConfig(
|
||||
platforms={Platform.SIGNAL: PlatformConfig(enabled=True)},
|
||||
)
|
||||
runner, _adapter = _make_runner(Platform.SIGNAL, config)
|
||||
|
||||
behavior = runner._get_unauthorized_dm_behavior(Platform.SIGNAL)
|
||||
assert behavior == "ignore"
|
||||
|
||||
|
||||
def test_get_unauthorized_dm_behavior_no_allowlist_returns_pair(monkeypatch):
|
||||
"""Without any allowlist, 'pair' is still the default."""
|
||||
_clear_auth_env(monkeypatch)
|
||||
|
||||
config = GatewayConfig(
|
||||
platforms={Platform.SIGNAL: PlatformConfig(enabled=True)},
|
||||
)
|
||||
runner, _adapter = _make_runner(Platform.SIGNAL, config)
|
||||
|
||||
behavior = runner._get_unauthorized_dm_behavior(Platform.SIGNAL)
|
||||
assert behavior == "pair"
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue