mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-09 08:21:50 +00:00
chore(gateway): drop plugin-migrated platforms from /update allowlist
`gateway/run.py::_UPDATE_ALLOWED_PLATFORMS` was a hardcoded frozenset listing every messaging platform allowed to invoke the `/update` slash command. Plugin-migrated platforms (currently Discord and Mattermost, soon also Home Assistant via #32500) declare `allow_update_command=True` on their `PlatformEntry`, and `_handle_update_command` already falls back to the registry when a platform isn't in the frozenset. The result was a silent redundancy: those entries said "allowed" twice, and the registry flag was a no-op for them in practice. - Removed `Platform.DISCORD` and `Platform.MATTERMOST` from the frozenset. - Updated the docstring to make the split explicit (built-ins live in the frozenset; plugins use `allow_update_command` on the registry entry). The remaining frozenset entries are all still built-in platforms living under `gateway/platforms/` today. Future plugin migrations should drop their entry from the frozenset as part of the migration PR (or in a sibling chore PR like this one). Added a `TestUpdateCommandPlatformGate` test class that pins down all three branches of the gate so future changes don't silently regress: - Programmatic interfaces (`Platform.WEBHOOK`, `Platform.API_SERVER`) must remain blocked. - Plugin-migrated platforms (Discord, Mattermost) must pass via the registry fallback. - Built-in platforms in the hardcoded frozenset (Telegram) must still pass without needing the registry. The gate previously had zero direct test coverage — its only existing coverage was `test_no_adapter_for_platform` which exercised a different code path.
This commit is contained in:
parent
c37c6eaf29
commit
ef7e5168b5
2 changed files with 129 additions and 4 deletions
|
|
@ -14967,11 +14967,15 @@ class GatewayRunner:
|
|||
return t("gateway.deny.denied_plural", count=count)
|
||||
return t("gateway.deny.denied_singular")
|
||||
|
||||
# Platforms where /update is allowed. ACP, API server, and webhooks are
|
||||
# programmatic interfaces that should not trigger system updates.
|
||||
# Built-in messaging platforms where the ``/update`` command is allowed.
|
||||
# ACP, API server, and webhooks are programmatic interfaces that should
|
||||
# not trigger system updates. Plugin-migrated platforms (discord,
|
||||
# mattermost, teams, irc, line, …) are NOT listed here — they declare
|
||||
# ``allow_update_command=True`` on their ``PlatformEntry`` and are
|
||||
# honored via the registry fallback at ``_handle_update_command`` below.
|
||||
_UPDATE_ALLOWED_PLATFORMS = frozenset({
|
||||
Platform.TELEGRAM, Platform.DISCORD, Platform.SLACK, Platform.WHATSAPP,
|
||||
Platform.SIGNAL, Platform.MATTERMOST, Platform.MATRIX,
|
||||
Platform.TELEGRAM, Platform.SLACK, Platform.WHATSAPP,
|
||||
Platform.SIGNAL, Platform.MATRIX,
|
||||
Platform.HOMEASSISTANT, Platform.EMAIL, Platform.SMS, Platform.DINGTALK,
|
||||
Platform.FEISHU, Platform.WECOM, Platform.WECOM_CALLBACK, Platform.WEIXIN, Platform.BLUEBUBBLES, Platform.QQBOT, Platform.LOCAL,
|
||||
})
|
||||
|
|
|
|||
|
|
@ -365,6 +365,127 @@ class TestHandleUpdateCommand:
|
|||
assert "stream progress" in result
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Platform allowlist gate
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestUpdateCommandPlatformGate:
|
||||
"""Tests for the platform-allowlist gate at the top of
|
||||
``_handle_update_command``. Built-in messaging platforms are listed in
|
||||
``_UPDATE_ALLOWED_PLATFORMS``; plugin-migrated platforms (discord,
|
||||
mattermost, teams, …) are NOT in the frozenset and rely on the
|
||||
registry's ``allow_update_command=True`` fallback. Programmatic
|
||||
interfaces (ACP, API server, webhooks) must be blocked.
|
||||
"""
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_blocks_programmatic_interface(self, monkeypatch):
|
||||
"""``Platform.WEBHOOK`` is not a messaging platform and must be
|
||||
blocked by the allowlist gate before any side effects fire."""
|
||||
runner = _make_runner()
|
||||
event = _make_event(platform=Platform.WEBHOOK)
|
||||
# Stop _handle_update_command from progressing further if the gate
|
||||
# somehow lets the event through — the assertion on the returned
|
||||
# string is the real test.
|
||||
monkeypatch.setenv("HERMES_MANAGED", "")
|
||||
|
||||
result = await runner._handle_update_command(event)
|
||||
|
||||
# The exact rejection message comes from
|
||||
# ``gateway.update.platform_not_messaging`` translation key.
|
||||
assert "only available from messaging platforms" in result
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_blocks_api_server_platform(self, monkeypatch):
|
||||
"""``Platform.API_SERVER`` (programmatic, not messaging) must be
|
||||
blocked by the allowlist gate.
|
||||
"""
|
||||
runner = _make_runner()
|
||||
event = _make_event(platform=Platform.API_SERVER)
|
||||
monkeypatch.setenv("HERMES_MANAGED", "")
|
||||
|
||||
result = await runner._handle_update_command(event)
|
||||
|
||||
assert "only available from messaging platforms" in result
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_allows_plugin_platform_via_registry_fallback(self, monkeypatch):
|
||||
"""A plugin-migrated platform (DISCORD) is no longer in
|
||||
``_UPDATE_ALLOWED_PLATFORMS`` but must still pass the gate via
|
||||
the registry's ``allow_update_command=True`` flag.
|
||||
|
||||
This test is the empirical guarantee that removing DISCORD from
|
||||
the hardcoded frozenset does not regress the /update command for
|
||||
Discord users.
|
||||
"""
|
||||
from gateway.run import GatewayRunner
|
||||
|
||||
# Precondition: DISCORD is NOT in the hardcoded set anymore.
|
||||
assert Platform.DISCORD not in GatewayRunner._UPDATE_ALLOWED_PLATFORMS
|
||||
|
||||
# Make sure the plugin registry is populated so the fallback fires.
|
||||
from hermes_cli.plugins import PluginManager
|
||||
PluginManager().discover_and_load(force=True)
|
||||
from gateway.platform_registry import platform_registry
|
||||
discord_entry = platform_registry.get("discord")
|
||||
assert discord_entry is not None
|
||||
assert discord_entry.allow_update_command is True
|
||||
|
||||
runner = _make_runner()
|
||||
event = _make_event(platform=Platform.DISCORD)
|
||||
monkeypatch.setenv("HERMES_MANAGED", "")
|
||||
|
||||
result = await runner._handle_update_command(event)
|
||||
|
||||
# The gate must NOT have rejected us — anything other than the
|
||||
# ``platform_not_messaging`` rejection string is acceptable here.
|
||||
# Later steps may legitimately return success ("Starting Hermes
|
||||
# update…") or fail for environment reasons.
|
||||
assert "only available from messaging platforms" not in result
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_allows_mattermost_via_registry_fallback(self, monkeypatch):
|
||||
"""Same as DISCORD: MATTERMOST is now plugin-migrated and not in
|
||||
the hardcoded frozenset; the registry must keep /update working.
|
||||
"""
|
||||
from gateway.run import GatewayRunner
|
||||
|
||||
assert Platform.MATTERMOST not in GatewayRunner._UPDATE_ALLOWED_PLATFORMS
|
||||
|
||||
from hermes_cli.plugins import PluginManager
|
||||
PluginManager().discover_and_load(force=True)
|
||||
from gateway.platform_registry import platform_registry
|
||||
mm_entry = platform_registry.get("mattermost")
|
||||
assert mm_entry is not None
|
||||
assert mm_entry.allow_update_command is True
|
||||
|
||||
runner = _make_runner()
|
||||
event = _make_event(platform=Platform.MATTERMOST)
|
||||
monkeypatch.setenv("HERMES_MANAGED", "")
|
||||
|
||||
result = await runner._handle_update_command(event)
|
||||
|
||||
assert "only available from messaging platforms" not in result
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_allows_builtin_platform_in_allowlist(self, monkeypatch):
|
||||
"""``Platform.TELEGRAM`` is in the hardcoded allowlist — gate
|
||||
must pass without consulting the registry.
|
||||
"""
|
||||
from gateway.run import GatewayRunner
|
||||
|
||||
assert Platform.TELEGRAM in GatewayRunner._UPDATE_ALLOWED_PLATFORMS
|
||||
|
||||
runner = _make_runner()
|
||||
event = _make_event(platform=Platform.TELEGRAM)
|
||||
monkeypatch.setenv("HERMES_MANAGED", "")
|
||||
|
||||
result = await runner._handle_update_command(event)
|
||||
|
||||
assert "only available from messaging platforms" not in result
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# _send_update_notification
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue