Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 12 additions & 8 deletions hermes_cli/gateway.py
Original file line number Diff line number Diff line change
Expand Up @@ -5814,17 +5814,21 @@ def _builtin_setup_fn(key: str):
from hermes_cli import setup as _s

return {
# telegram moved into the plugin: setup_fn registered by
# plugins/platforms/telegram/adapter.py::register(). #41112.
# discord moved into the plugin: setup_fn is registered by
# plugins/platforms/discord/adapter.py::register() and dispatched
# via the plugin path in _configure_platform().
# slack moved into the plugin: setup_fn is registered by
# plugins/platforms/slack/adapter.py::register() and dispatched
# via the plugin path in _configure_platform(). #41112.
# matrix moved into the plugin: setup_fn registered by
# plugins/platforms/matrix/adapter.py::register() and dispatched via
# the plugin path in _configure_platform(). #41112.
#
# telegram, slack, and matrix are built-in (not plugin-migrated —
# see the _PLATFORMS NOTE above). They keep their bespoke setup
# flows here because the generic _setup_standard_platform fallback
# treats the first `vars` entry (token_var) as mandatory and aborts
# the whole wizard if it's left empty. That breaks Matrix's
# documented "leave the access token empty for password login"
# path outright, and loses Telegram's token-format validation and
# Slack's manifest-regeneration prompt.
"telegram": _s._setup_telegram,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Restore open-access handling in Telegram setup

When a first-time Telegram user follows _setup_telegram() and leaves the allowlist empty for the advertised open-access mode, this dispatch bypasses _setup_standard_platform(), the only flow that asks for an access policy and writes GATEWAY_ALLOW_ALL_USERS=true. The bespoke helper merely claims anyone can use the bot without saving that flag or a wildcard, while Telegram authorization fails closed without either, so the newly configured bot does not provide the access mode the user selected.

Useful? React with 👍 / 👎.

"slack": _s._setup_slack,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Clear the Slack allowlist when the user selects deny-by-default

When reconfiguring Slack with an existing SLACK_ALLOWED_USERS value, leaving the bespoke allowlist prompt empty is described as denying everyone except paired users, but _setup_slack() never removes the existing value. The gateway therefore continues authorizing every previously listed user despite the access policy selected in the restored wizard, which can unintentionally retain access that the operator intended to revoke.

Useful? React with 👍 / 👎.

"matrix": _s._setup_matrix,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Clear stale Matrix tokens when selecting password login

When MATRIX_ACCESS_TOKEN already exists and a user chooses Reconfigure, leaves the token prompt empty, and supplies a password, _setup_matrix() saves MATRIX_PASSWORD but never removes the old access token. MatrixAdapter prioritizes the access token whenever it is present, so it continues using the old token—and may keep failing if that token was revoked—instead of switching to the newly entered password.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Persist the Matrix E2EE answer during reconfiguration

When an existing Matrix setup has MATRIX_ENCRYPTION=true, accepting the bespoke flow's default “No” answer to “Enable end-to-end encryption?” neither removes the variable nor writes false; the adapter therefore still enables E2EE even though this flow proceeds as if it were disabled and only selects the base mautrix package. This prevents users from disabling encryption through the restored wizard and can leave startup failing when the E2EE dependencies are unavailable.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Restore open-access handling in Matrix setup

When a Matrix user leaves MATRIX_ALLOWED_USERS empty as the bespoke prompt explicitly permits for open access, _setup_matrix() only prints that anyone can message the bot and saves no MATRIX_ALLOW_ALL_USERS or GATEWAY_ALLOW_ALL_USERS flag. Gateway authorization defaults to deny when no allowlist or allow-all flag exists, so this newly restored path produces a bot whose actual access policy contradicts the wizard and prevents unpaired users from using it.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Require a user ID for Matrix password authentication

For a fresh password-login setup, _setup_matrix() accepts an empty user ID, then saves the password and reports that Matrix credentials were saved. The platform is enabled from the presence of MATRIX_PASSWORD, but MatrixAdapter.connect() requires both a user ID and password and immediately fails, so the restored password path can finish successfully with credentials that cannot authenticate; reject or reprompt for an empty user ID before saving this mode.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Install Matrix E2EE extras when enabling encryption

When base mautrix is already installed without its encryption dependencies and the user enables E2EE, _setup_matrix() tests only __import__("mautrix"), so it skips installing mautrix[encryption] and still reports E2EE enabled. The gateway's E2EE requirements check then refuses to connect because dependencies such as python-olm are absent; the restored flow needs to check the encryption imports or install the selected extra even when the base package imports successfully.

Useful? React with 👍 / 👎.

# mattermost moved into the plugin: setup_fn is registered by
# plugins/platforms/mattermost/adapter.py::register() and dispatched
# via the plugin path in _configure_platform().
Expand Down
61 changes: 61 additions & 0 deletions tests/hermes_cli/test_gateway_platform_gating.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,67 @@
import sys


# Every built-in adapter under gateway/platforms/ that is expected to have
# a picker entry today. This intentionally excludes dingtalk/feishu/wecom/
# wecom_callback — those were dropped by the same e39b468 refactor but are
# tracked and restored separately (see .plans/missing-platform-plugin-shims.md
# and the PR that follows it); asserting them here would make this test
# depend on merge order between unrelated PRs.
BUILTIN_ADAPTER_KEYS = frozenset(
{
"telegram",
"slack",
"matrix",
"whatsapp",
"email",
"sms",
"mattermost",
"signal",
"weixin",
"bluebubbles",
"qqbot",
"yuanbao",
}
)


class TestBuiltinAdaptersStayInPicker:
def test_every_builtin_adapter_key_is_in_all_platforms(self, monkeypatch):
"""Regression guard for e39b468-style silent deletions.

A future refactor that deletes a _PLATFORMS entry (or claims a
built-in "moved to plugins" without actually creating the plugin)
should fail CI here instead of shipping a picker that's silently
missing a platform.
"""
import hermes_cli.gateway as gateway_mod

monkeypatch.setattr(gateway_mod.sys, "platform", "linux")
keys = {p["key"] for p in gateway_mod._all_platforms()}
missing = BUILTIN_ADAPTER_KEYS - keys
assert not missing, (
f"Built-in adapter(s) {sorted(missing)} have no _all_platforms() "
"entry — they exist in gateway/platforms/ but are invisible to "
"`hermes setup gateway`."
)

def test_telegram_slack_matrix_use_bespoke_setup_fn(self, monkeypatch):
"""Matrix's "leave the token empty for password login" path is real.

The generic _setup_standard_platform() fallback treats the first
`vars` entry (token_var) as mandatory and aborts the whole wizard
if left empty — which would silently break Matrix's documented
password-login path. These three must resolve to their bespoke
hermes_cli.setup functions, not fall through to the generic flow.
"""
import hermes_cli.gateway as gateway_mod
from hermes_cli import setup as _s

assert gateway_mod._builtin_setup_fn("telegram") is _s._setup_telegram
assert gateway_mod._builtin_setup_fn("slack") is _s._setup_slack
assert gateway_mod._builtin_setup_fn("matrix") is _s._setup_matrix


class TestMatrixHiddenOnWindows:
def test_matrix_present_on_linux(self, monkeypatch):
"""Sanity: matrix is still in the picker on Linux/macOS."""
Expand Down
Loading