diff --git a/hermes_cli/gateway.py b/hermes_cli/gateway.py index 6e298ea1bd40..ce60bfb2ccdf 100644 --- a/hermes_cli/gateway.py +++ b/hermes_cli/gateway.py @@ -5470,6 +5470,9 @@ def _set_platform_unauthorized_dm_behavior(platform_key: str, behavior: str) -> def _setup_standard_platform(platform: dict): """Interactive setup for Telegram, Discord, or Slack.""" + # Same hidden-knob list the dashboard/Desktop channel cards use. + from hermes_cli.setup_hidden_env import is_setup_hidden_env as _is_setup_hidden_env + emoji = platform["emoji"] label = platform["label"] token_var = platform["token_var"] @@ -5522,7 +5525,22 @@ def _setup_standard_platform(platform: dict): allowed_val_set = None # Track if user set an allowlist (for home channel offer) - for var in platform["vars"]: + # Skip the knobs the setup forms hide (home channel, reply mode, proxy, + # mention behavior). They're self-configuring or already correct by + # default — /sethome sets the home channel on the first chat — so asking + # about each one turned a 2-question setup into a 5-question one. Same + # exclusion the dashboard/Desktop cards use, so the surfaces agree. + # Required credentials are never skipped. + required_names = {token_var} + setup_vars = [ + v + for v in platform["vars"] + if v["name"] in required_names + or v.get("is_allowlist") + or not _is_setup_hidden_env(v["name"]) + ] + + for var in setup_vars: print() print_info(f" {var['help']}") existing = get_env_value(var["name"]) diff --git a/hermes_cli/setup_hidden_env.py b/hermes_cli/setup_hidden_env.py new file mode 100644 index 000000000000..cea85b5987e7 --- /dev/null +++ b/hermes_cli/setup_hidden_env.py @@ -0,0 +1,56 @@ +"""Which platform env vars the setup surfaces hide. + +Every messaging platform ships the same handful of knobs that are either set +for the user later or already correct by default. Listing them on a setup form +turns "paste your bot token" into a five-field interrogation where none of the +answers are discoverable — the Discord card asked for a home channel ID you +need Developer Mode to copy, next to a reply-threading preference. + +Hiding them is a *presentation* decision only. The env vars keep working +through ``hermes config set``, ``.env``, and ``config.yaml``; the gateway reads +them exactly as before. This module just says what a new user is asked during +setup. + +Lives here rather than in ``web_server`` so the CLI wizard can share it without +importing the dashboard's FastAPI surface. +""" + +# Suffix match, so plugin adapters nobody enumerated (IRC, SimpleX, LINE, ntfy) +# get the same treatment without a code change here. +# +# *_HOME_CHANNEL* the bot offers /sethome on the first chat +# *_ALLOW_ALL_USERS defaults off; enabling it is a security decision +# *_REPLY_TO_MODE cosmetic threading preference +# *_REPLY_MODE same, Mattermost's spelling +# *_REQUIRE_MENTION behavior toggle with a sane default +# *_AUTO_THREAD same +# *_FREE_RESPONSE_* per-channel tuning, done once the bot is in a server +# *_ALLOWED_CHANNELS same +# *_PROXY only for networks that block the platform +# +# Allowlists (*_ALLOWED_USERS) deliberately stay visible: that IS the decision +# a new user has to make, and the gateway denies everyone until it's set. +SETUP_HIDDEN_ENV_SUFFIXES = ( + "_HOME_CHANNEL", + "_HOME_CHANNEL_NAME", + "_HOME_CHANNEL_THREAD_ID", + "_HOME_ADDRESS", + "_ALLOW_ALL_USERS", + "_REPLY_TO_MODE", + "_REPLY_MODE", + "_REQUIRE_MENTION", + "_AUTO_THREAD", + "_FREE_RESPONSE_CHANNELS", + "_FREE_RESPONSE_ROOMS", + "_ALLOWED_CHANNELS", + "_PROXY", +) + + +def is_setup_hidden_env(name: str) -> bool: + """True when a var is self-configuring and shouldn't appear in setup forms. + + Callers must still keep any var a platform lists as *required* — hiding a + required credential would make that platform unconfigurable from the UI. + """ + return name.endswith(SETUP_HIDDEN_ENV_SUFFIXES) diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index 64136ce1a6ca..d9b7a1a56bf8 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -8046,7 +8046,6 @@ _PLATFORM_OVERRIDES: dict[str, dict[str, Any]] = { "env_vars": ( "DISCORD_BOT_TOKEN", "DISCORD_ALLOWED_USERS", - "DISCORD_REPLY_TO_MODE", ), "required_env": ("DISCORD_BOT_TOKEN",), }, @@ -8485,6 +8484,14 @@ def _platform_env_prefixes(platform_id: str) -> tuple[str, ...]: return (platform_id.upper().replace("-", "_") + "_",) +# Which per-platform knobs the setup UI hides, and why: see +# hermes_cli/setup_hidden_env.py. Shared with the `hermes setup gateway` +# wizard so the surfaces ask for the same things. +from hermes_cli.setup_hidden_env import ( # noqa: E402 + is_setup_hidden_env as _is_setup_hidden_env, +) + + def _discover_platform_env_vars(platform_id: str) -> tuple[str, ...]: """All messaging-category env vars for a platform (override + plugin + prefix).""" prefixes = _platform_env_prefixes(platform_id) @@ -8494,6 +8501,8 @@ def _discover_platform_env_vars(platform_id: str) -> tuple[str, ...]: continue if name in _MESSAGING_KEYS_PAGE_KEYS: continue + if _is_setup_hidden_env(name): + continue if not any(name.startswith(prefix) for prefix in prefixes): continue keys.append(name) @@ -8505,10 +8514,18 @@ def _merge_platform_env_vars( override: dict[str, Any], plugin_entry: Any | None, ) -> tuple[str, ...]: - """Canonical env-var list for a messaging platform card.""" + """Canonical env-var list for a messaging platform card. + + Required credentials always survive: a platform that genuinely needs one of + the hidden-suffix vars to connect keeps it, since hiding a required field + would make the platform unconfigurable. + """ discovered = _discover_platform_env_vars(platform_id) if "env_vars" in override: - return tuple(dict.fromkeys((*override["env_vars"], *discovered))) + explicit = tuple( + key for key in override["env_vars"] if not _is_setup_hidden_env(key) + ) + return tuple(dict.fromkeys((*explicit, *discovered))) if plugin_entry is not None and plugin_entry.required_env: return tuple(dict.fromkeys((*tuple(plugin_entry.required_env), *discovered))) return discovered diff --git a/tests/hermes_cli/test_setup_hidden_env.py b/tests/hermes_cli/test_setup_hidden_env.py new file mode 100644 index 000000000000..bbb40a6ce93c --- /dev/null +++ b/tests/hermes_cli/test_setup_hidden_env.py @@ -0,0 +1,163 @@ +"""Setup surfaces ask only for what a platform can't start without. + +The knobs in SETUP_HIDDEN_ENV_SUFFIXES are self-configuring (/sethome) or +already correct by default, so they don't belong on a setup form. They must +still be reachable everywhere else — this is a presentation change, not a +feature removal. +""" + +import pytest + +from hermes_cli.setup_hidden_env import is_setup_hidden_env + + +class TestIsSetupHiddenEnv: + @pytest.mark.parametrize( + "key", + [ + "DISCORD_HOME_CHANNEL", + "DISCORD_HOME_CHANNEL_NAME", + "DISCORD_ALLOW_ALL_USERS", + "DISCORD_REPLY_TO_MODE", + "MATTERMOST_REPLY_MODE", + "TELEGRAM_PROXY", + ], + ) + def test_self_configuring_knobs_are_hidden(self, key): + assert is_setup_hidden_env(key) + + @pytest.mark.parametrize( + "key", + [ + "DISCORD_BOT_TOKEN", + "TELEGRAM_BOT_TOKEN", + "SLACK_BOT_TOKEN", + "MATTERMOST_URL", + # Allowlists stay: the gateway denies everyone until one is set, + # so this is the decision a new user actually has to make. + "DISCORD_ALLOWED_USERS", + "SLACK_ALLOWED_USERS", + ], + ) + def test_credentials_and_allowlists_stay_visible(self, key): + assert not is_setup_hidden_env(key) + + def test_applies_to_plugin_platforms_nobody_enumerated(self): + """Suffix matching is the point — IRC/SimpleX/ntfy get this for free.""" + for key in ( + "IRC_ALLOW_ALL_USERS", + "SIMPLEX_HOME_CHANNEL", + "NTFY_HOME_CHANNEL_NAME", + "LINE_ALLOW_ALL_USERS", + ): + assert is_setup_hidden_env(key), key + + +class TestChannelCards: + def test_discord_card_asks_for_token_and_allowlist_only(self): + """The reported bug: the Discord card asked five questions for a + one-credential platform.""" + from hermes_cli.web_server import _build_catalog_entry + + assert set(_build_catalog_entry("discord")["env_vars"]) == { + "DISCORD_BOT_TOKEN", + "DISCORD_ALLOWED_USERS", + } + + def test_no_card_shows_a_hidden_knob(self): + from hermes_cli.web_server import _messaging_platform_catalog + + for entry in _messaging_platform_catalog(): + for key in entry["env_vars"]: + assert not is_setup_hidden_env(key), f"{entry['id']}: {key}" + + def test_required_credentials_are_never_hidden(self): + """Hiding a required var would make its platform unconfigurable.""" + from hermes_cli.web_server import _messaging_platform_catalog + + for entry in _messaging_platform_catalog(): + for key in entry["required_env"]: + assert key in entry["env_vars"], f"{entry['id']}: {key}" + + def test_hidden_knobs_move_to_the_keys_page_not_into_a_void(self): + """Keys hides what a Channels card owns. Dropping these from the card + must hand them back to Keys, not orphan them from every surface.""" + from hermes_cli.web_server import _channel_managed_env_keys + + managed = _channel_managed_env_keys() + for key in ( + "DISCORD_HOME_CHANNEL", + "DISCORD_ALLOW_ALL_USERS", + "DISCORD_REPLY_TO_MODE", + ): + assert key not in managed, f"{key} is hidden from both surfaces" + + +class TestCliWizard: + """`hermes setup gateway` drops the same knobs. Real wizard, scripted + stdin, real .env writes under a temp HERMES_HOME.""" + + @pytest.fixture + def home(self, tmp_path, monkeypatch): + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + return tmp_path + + def _run(self, answers, monkeypatch): + import io + + from hermes_cli import gateway as gw + + platform = next(p for p in gw._PLATFORMS if p["key"] == "mattermost") + monkeypatch.setattr("sys.stdin", io.StringIO("\n".join(answers) + "\n")) + gw._setup_standard_platform(dict(platform)) + + def _env(self, home): + path = home / ".env" + if not path.exists(): + return {} + out = {} + for line in path.read_text().splitlines(): + line = line.strip() + if line and not line.startswith("#") and "=" in line: + k, v = line.split("=", 1) + out[k] = v + return out + + def test_wizard_stops_asking_about_hidden_knobs(self, home, monkeypatch, capsys): + # Only the three real answers. If the wizard still prompted for the + # home channel it would read past them. + self._run( + ["https://mm.example.com", "tok-abc", "user26charid"], monkeypatch + ) + + written = self._env(home) + assert written.get("MATTERMOST_URL") == "https://mm.example.com" + assert written.get("MATTERMOST_TOKEN") == "tok-abc" + assert "MATTERMOST_HOME_CHANNEL" not in written + assert "MATTERMOST_REPLY_MODE" not in written + + out = capsys.readouterr().out + assert "Home channel" not in out + assert "Reply mode" not in out + + def test_required_token_still_gates_setup(self, home, monkeypatch): + """Proves the token wasn't swept out along with the knobs.""" + self._run(["https://mm.example.com", ""], monkeypatch) + + assert "MATTERMOST_TOKEN" not in self._env(home) + + +class TestStillConfigurable: + def test_gateway_still_honors_the_env_vars(self, tmp_path, monkeypatch): + """Nothing was removed from the product — only from the setup form.""" + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + monkeypatch.setenv("DISCORD_BOT_TOKEN", "t") + monkeypatch.setenv("DISCORD_HOME_CHANNEL", "999") + monkeypatch.setenv("DISCORD_REPLY_TO_MODE", "off") + + from gateway.config import Platform, load_gateway_config + + discord = load_gateway_config().platforms[Platform.DISCORD] + assert discord.home_channel is not None + assert discord.home_channel.chat_id == "999" + assert discord.reply_to_mode == "off" diff --git a/tests/hermes_cli/test_web_server.py b/tests/hermes_cli/test_web_server.py index e76c24a5fd61..895b0ddf7944 100644 --- a/tests/hermes_cli/test_web_server.py +++ b/tests/hermes_cli/test_web_server.py @@ -3153,6 +3153,14 @@ class TestWebServerEndpoints: assert data["AWS_PROFILE"]["provider"] == "bedrock" def test_platform_scoped_messaging_env_vars_are_channel_managed(self): + """Platform-scoped vars belong to a Channels card; cross-cutting + gateway vars belong to the Keys page. + + Uses credentials as the example: the self-configuring knobs + (*_HOME_CHANNEL, *_ALLOW_ALL_USERS, …) were deliberately dropped from + the setup cards and handed back to Keys — see + tests/hermes_cli/test_setup_hidden_env.py. + """ from hermes_cli.web_server import ( _MESSAGING_KEYS_PAGE_KEYS, _build_catalog_entry, @@ -3160,13 +3168,12 @@ class TestWebServerEndpoints: ) discord = _build_catalog_entry("discord") - assert "DISCORD_HOME_CHANNEL" in discord["env_vars"] - assert "DISCORD_ALLOW_ALL_USERS" in discord["env_vars"] + assert "DISCORD_BOT_TOKEN" in discord["env_vars"] + assert "DISCORD_ALLOWED_USERS" in discord["env_vars"] managed = _channel_managed_env_keys() - assert "DISCORD_HOME_CHANNEL" in managed - assert "BLUEBUBBLES_ALLOW_ALL_USERS" in managed - assert "MATTERMOST_ALLOW_ALL_USERS" in managed + assert "DISCORD_BOT_TOKEN" in managed + assert "MATTERMOST_TOKEN" in managed assert "GATEWAY_PROXY_URL" not in managed assert "GATEWAY_PROXY_URL" in _MESSAGING_KEYS_PAGE_KEYS