From ef7e5168b52e4d3b38028fb960b48af88b149154 Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Tue, 26 May 2026 14:57:15 +0530 Subject: [PATCH] chore(gateway): drop plugin-migrated platforms from /update allowlist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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. --- gateway/run.py | 12 ++- tests/gateway/test_update_command.py | 121 +++++++++++++++++++++++++++ 2 files changed, 129 insertions(+), 4 deletions(-) diff --git a/gateway/run.py b/gateway/run.py index 8db1a52a5b0..dc8e0f14cc0 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -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, }) diff --git a/tests/gateway/test_update_command.py b/tests/gateway/test_update_command.py index 64998771d80..39d805edb49 100644 --- a/tests/gateway/test_update_command.py +++ b/tests/gateway/test_update_command.py @@ -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 # ---------------------------------------------------------------------------