From 5083d5f78e82c042fc68653c494ce59cd1f05d44 Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Mon, 14 Sep 2026 18:04:53 -0700 Subject: [PATCH] fix(gateway): honour allow_all_users from config.yaml by bridging it to GATEWAY_ALLOW_ALL_USERS `gateway.allow_all_users: true` (and the top-level spelling) in config.yaml was a silent no-op: `_TOPLEVEL_BRIDGE` never forwarded it, GatewayConfig has no field, and every allow-all reader (authz mixin default-deny branch, startup access check, own-policy adapters, Discord/Matrix/Email plugin gates) consults the GATEWAY_ALLOW_ALL_USERS env var only. Bridge the YAML key into that env var in `bridge_core_env_settings`, the one seam every reader already shares, instead of threading a new config attribute through ten readers: - first-writer-wins: an explicit env var beats YAML (matches every other {PLATFORM}_* gate); - skipped inside a multiplexed secondary profile's scope (#80099 class): the secondary's config.yaml must not become the default profile's policy, and the isolation test now asserts GATEWAY_ALLOW_ALL_USERS stays unset; - a startup warning names config.yaml as the grant source, because the key was inert until now and a forgotten `true` flips the posture to open. Tests: both spellings authorize a stranger through `_is_user_authorized`; `false`, absent key, and env=false over YAML=true all stay denied. Docs: security guide, env-var reference, gateway internals. Fixes #110690 --- gateway/config_loader.py | 27 +++++++++--- tests/gateway/test_config.py | 44 +++++++++++++++++++ ...est_multiplex_yaml_env_bridge_isolation.py | 4 +- .../docs/developer-guide/gateway-internals.md | 2 +- .../docs/reference/environment-variables.md | 2 +- website/docs/user-guide/security.md | 2 + 6 files changed, 73 insertions(+), 8 deletions(-) diff --git a/gateway/config_loader.py b/gateway/config_loader.py index 334d7024c5..f26c5d5ba3 100644 --- a/gateway/config_loader.py +++ b/gateway/config_loader.py @@ -297,20 +297,37 @@ def apply_plugin_yaml_hooks(yaml_cfg: dict, gateway_platforms: Any, platforms_da def bridge_core_env_settings(yaml_cfg: dict, platforms_data: dict) -> None: - """The two YAML→env bridges that stay in core (per-platform ones live in plugin hooks). + """The YAML→env bridges that stay in core (per-platform ones live in plugin hooks). Top-level ``require_mention`` → Telegram when the ``telegram:`` section has none: users write it alongside ``group_sessions_per_user`` expecting it to work, and the telegram plugin's hook only runs when a telegram block exists. Signal ``require_mention`` → ``SIGNAL_REQUIRE_MENTION`` (env wins). + ``allow_all_users`` (top-level or ``gateway.allow_all_users``) → ``GATEWAY_ALLOW_ALL_USERS``: every + allow-all reader (authz mixin, startup access check, own-policy adapters, plugin gates) consults + that env var, so the bridge is the one seam that makes the YAML key reach all of them (#110690). - Both values are ALWAYS seeded into the owning platform's ``extra`` (the adapters read extra first); - the process-env write is skipped while a multiplexed secondary profile's scope is active — the - loader runs inside ``_profile_runtime_scope`` for every secondary, and a first-writer-wins write - there would make the secondary's mention policy the DEFAULT profile's (#80099 class). + Platform values are ALWAYS seeded into the owning platform's ``extra`` (the adapters read extra + first); the process-env write is skipped while a multiplexed secondary profile's scope is active — + the loader runs inside ``_profile_runtime_scope`` for every secondary, and a first-writer-wins write + there would make the secondary's policy the DEFAULT profile's (#80099 class). A secondary profile + sets ``GATEWAY_ALLOW_ALL_USERS`` in its own ``.env`` like every other scoped authorization gate. """ from gateway.platforms._shared import profile_scoped skip_env_bridge = profile_scoped() + gateway_section = yaml_cfg.get("gateway") + allow_all = yaml_cfg.get("allow_all_users") + if allow_all is None and isinstance(gateway_section, dict): + allow_all = gateway_section.get("allow_all_users") + if allow_all is not None and not skip_env_bridge and not os.getenv("GATEWAY_ALLOW_ALL_USERS"): + os.environ["GATEWAY_ALLOW_ALL_USERS"] = str(allow_all).lower() + if os.environ["GATEWAY_ALLOW_ALL_USERS"] in {"true", "1", "yes"}: + # The key was inert before it was bridged, so a forgotten line silently flips the + # posture to open — name the grant source at startup. + logger.warning( + "config.yaml allow_all_users: true grants every sender on every platform access " + "(bridged to GATEWAY_ALLOW_ALL_USERS; an explicit env var wins)." + ) tl_require_mention = yaml_cfg.get("require_mention") if tl_require_mention is not None and "require_mention" not in (yaml_cfg.get("telegram") or {}): tg_plat = platforms_data.setdefault(Platform.TELEGRAM.value, {}) diff --git a/tests/gateway/test_config.py b/tests/gateway/test_config.py index 3301935b4a..f78a482a6f 100644 --- a/tests/gateway/test_config.py +++ b/tests/gateway/test_config.py @@ -844,6 +844,50 @@ class TestLoadGatewayConfig: ] assert os.environ.get("DINGTALK_ALLOWED_USERS") == "user-id-1,user-id-2" + @pytest.mark.parametrize("yaml_text", ["gateway:\n allow_all_users: true\n", "allow_all_users: true\n"]) + def test_allow_all_users_yaml_reaches_the_authz_gate(self, tmp_path, monkeypatch, yaml_text): + """Both spellings must open the gate for an unknown sender; before #110690 the key was inert + because every allow-all reader consults GATEWAY_ALLOW_ALL_USERS only.""" + from gateway.authz_mixin import GatewayAuthorizationMixin + from gateway.session import SessionSource + + hermes_home = tmp_path / ".hermes" + hermes_home.mkdir() + (hermes_home / "config.yaml").write_text(yaml_text, encoding="utf-8") + monkeypatch.setenv("HERMES_HOME", str(hermes_home)) + monkeypatch.delenv("GATEWAY_ALLOW_ALL_USERS", raising=False) + + runner = object.__new__(GatewayAuthorizationMixin) + runner.config = load_gateway_config() + runner.adapters = {} + stranger = SessionSource(platform=Platform.TELEGRAM, user_id="999", chat_id="999", chat_type="dm") + assert runner._is_user_authorized(stranger) is True + + @pytest.mark.parametrize("yaml_text, env, expected", [ + ("gateway:\n allow_all_users: false\n", None, "false"), + ("gateway:\n allow_all_users: true\n", "false", "false"), # explicit env wins over YAML + ("gateway: {}\n", None, None), + ]) + def test_allow_all_users_yaml_never_widens_past_env(self, tmp_path, monkeypatch, yaml_text, env, expected): + from gateway.authz_mixin import GatewayAuthorizationMixin + from gateway.session import SessionSource + + hermes_home = tmp_path / ".hermes" + hermes_home.mkdir() + (hermes_home / "config.yaml").write_text(yaml_text, encoding="utf-8") + monkeypatch.setenv("HERMES_HOME", str(hermes_home)) + if env is None: + monkeypatch.delenv("GATEWAY_ALLOW_ALL_USERS", raising=False) + else: + monkeypatch.setenv("GATEWAY_ALLOW_ALL_USERS", env) + + runner = object.__new__(GatewayAuthorizationMixin) + runner.config = load_gateway_config() + runner.adapters = {} + assert os.environ.get("GATEWAY_ALLOW_ALL_USERS") == expected + stranger = SessionSource(platform=Platform.TELEGRAM, user_id="999", chat_id="999", chat_type="dm") + assert runner._is_user_authorized(stranger) is False + def test_top_level_platforms_override_nested_gateway_platforms(self, tmp_path, monkeypatch): hermes_home = tmp_path / ".hermes" diff --git a/tests/gateway/test_multiplex_yaml_env_bridge_isolation.py b/tests/gateway/test_multiplex_yaml_env_bridge_isolation.py index 27dde12775..349b3f7e95 100644 --- a/tests/gateway/test_multiplex_yaml_env_bridge_isolation.py +++ b/tests/gateway/test_multiplex_yaml_env_bridge_isolation.py @@ -20,6 +20,8 @@ from hermes_constants import reset_hermes_home_override, set_hermes_home_overrid _SECONDARY_YAML = """\ require_mention: false +gateway: + allow_all_users: true telegram: mention_patterns: ['^bot2'] reactions: false @@ -49,7 +51,7 @@ _BRIDGED_ENV = ( "MATRIX_REQUIRE_MENTION", "MATRIX_ALLOWED_USERS", "MATRIX_SESSION_SCOPE", "WHATSAPP_DM_POLICY", "WHATSAPP_ALLOWED_USERS", "FEISHU_ALLOW_BOTS", "SLACK_ALLOW_BOTS", "SLACK_IGNORED_CHANNELS", "DINGTALK_ALLOWED_USERS", - "DISCORD_AUTO_THREAD", "DISCORD_REACTIONS", "SIGNAL_REQUIRE_MENTION", + "DISCORD_AUTO_THREAD", "DISCORD_REACTIONS", "SIGNAL_REQUIRE_MENTION", "GATEWAY_ALLOW_ALL_USERS", ) _EXPECTED_EXTRA = ( diff --git a/website/docs/developer-guide/gateway-internals.md b/website/docs/developer-guide/gateway-internals.md index 2e5361b1fb..d397d16402 100644 --- a/website/docs/developer-guide/gateway-internals.md +++ b/website/docs/developer-guide/gateway-internals.md @@ -96,7 +96,7 @@ The gateway uses a multi-layer authorization check, evaluated in order: 1. **Per-platform allow-all flag** (e.g., `TELEGRAM_ALLOW_ALL_USERS`) — if set, all users on that platform are authorized 2. **Platform allowlist** (e.g., `TELEGRAM_ALLOWED_USERS`) — comma-separated user IDs 3. **DM pairing** — authenticated users can pair new users via a pairing code -4. **Global allow-all** (`GATEWAY_ALLOW_ALL_USERS`) — if set, all users across all platforms are authorized +4. **Global allow-all** (`GATEWAY_ALLOW_ALL_USERS`, or `gateway.allow_all_users` in `config.yaml`, bridged to the env var by `gateway/config_loader.py::bridge_core_env_settings`) — if set, all users across all platforms are authorized 5. **Default: deny** — unauthorized users are rejected ### DM Pairing Flow diff --git a/website/docs/reference/environment-variables.md b/website/docs/reference/environment-variables.md index 17282aa408..01b16f3db5 100644 --- a/website/docs/reference/environment-variables.md +++ b/website/docs/reference/environment-variables.md @@ -546,7 +546,7 @@ These are set automatically by the Docker terminal backend when `proxy.enabled: | `GATEWAY_PROXY_KEY` | Bearer token for authenticating with the remote API server in proxy mode. Must match `API_SERVER_KEY` on the remote host. | | `MESSAGING_CWD` | Deprecated compatibility fallback for gateway working directory. Prefer `terminal.cwd` in `config.yaml`. | | `GATEWAY_ALLOWED_USERS` | Comma-separated user IDs allowed across all platforms | -| `GATEWAY_ALLOW_ALL_USERS` | Allow all users without allowlists (`true`/`false`, default: `false`) | +| `GATEWAY_ALLOW_ALL_USERS` | Allow all users without allowlists (`true`/`false`, default: `false`). Also configurable via `gateway.allow_all_users` in `config.yaml`; the env var wins when both are set. | ### Web Dashboard & Hermes Desktop diff --git a/website/docs/user-guide/security.md b/website/docs/user-guide/security.md index 4c6d950a0d..7078810079 100644 --- a/website/docs/user-guide/security.md +++ b/website/docs/user-guide/security.md @@ -410,6 +410,8 @@ DISCORD_ALLOW_ALL_USERS=true GATEWAY_ALLOW_ALL_USERS=true ``` +The global allow-all can also live in `config.yaml` as `gateway.allow_all_users: true` (or top-level `allow_all_users: true`); it is bridged to `GATEWAY_ALLOW_ALL_USERS` at gateway startup, an explicit env var wins, and the gateway logs a warning naming `config.yaml` as the grant source. In a multi-profile gateway a secondary profile sets `GATEWAY_ALLOW_ALL_USERS` in its own `.env` (its `config.yaml` is never bridged into the process environment). + :::warning If **no allowlists are configured** and `GATEWAY_ALLOW_ALL_USERS` is not set, **all users are denied**. The gateway logs a warning at startup: