From 396ee69032deeb186f39cb2d67ff3c720dc5e657 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Sun, 24 May 2026 15:44:26 -0700 Subject: [PATCH] fix(gateway): seed plugin extras before is_connected gate (#31703) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to 54e61f933. The plugin enablement gate calls ``entry.is_connected(probe_cfg)`` BEFORE ``env_enablement_fn`` runs, and the probe is built as ``existing_cfg or PlatformConfig()`` — empty extras, ``enabled=False``. For plugins whose ``is_connected`` reads ``config.extra`` instead of env vars directly, that probe is a misrepresentation of what the platform will look like after enablement. Google Chat's ``_is_connected`` short-circuits on ``config.enabled`` and inspects ``config.extra["project_id"]`` / ``config.extra["subscription_name"]`` — both False on the default probe even when the user has set ``GOOGLE_CHAT_PROJECT_ID`` and ``GOOGLE_CHAT_SUBSCRIPTION_NAME``. Result: Google Chat silently fails the gate on every env-var-only setup. Build a candidate probe that mirrors what the platform will look like post-enablement: - pre-call ``env_enablement_fn`` and layer its result into the probe's ``extra`` (without mutating any existing platform config) - pass ``enabled=True`` on the probe — we're asking "would this BE configured if we let it in?" not "is it currently enabled?" - reuse the same seeded extras when we commit the platform to ``config.platforms`` (avoids calling ``env_enablement_fn`` twice) Discord/IRC/Teams/LINE/ntfy/Simplex ``_is_connected`` hooks read env vars directly, so they are unaffected. This change only restores Google Chat on env-var-only setups while keeping the original #31116 Discord-no-token block intact. All 6 shipped ``env_enablement_fn`` implementations were audited and are pure reads (no ``os.environ`` writes), so running them earlier in the loop has no observable side effects. Tests: 2 new in tests/gateway/test_platform_registry.py covering extras-seeded-before-is_connected and don't-leak-extras-on-gate-fail. 693 tests across 11 adjacent suites pass (platform_registry, config, google_chat, matrix, discord_connect, ntfy_plugin, simplex_plugin, line_plugin, irc_adapter, teams, gateway_platform_gating). Refs #31116. --- gateway/config.py | 94 ++++++++++++++++++------- tests/gateway/test_platform_registry.py | 89 +++++++++++++++++++++++ 2 files changed, 157 insertions(+), 26 deletions(-) diff --git a/gateway/config.py b/gateway/config.py index 9d3c966b77a..ec98532a273 100644 --- a/gateway/config.py +++ b/gateway/config.py @@ -1837,6 +1837,25 @@ def _apply_env_overrides(config: GatewayConfig) -> None: continue platform = Platform(entry.name) existing_cfg = config.platforms.get(platform) + # Seed candidate extras from ``env_enablement_fn`` so plugins + # whose ``is_connected`` reads ``config.extra`` (e.g. Google + # Chat's ``_is_connected`` checks ``config.extra["project_id"]``) + # see the same state they will after enablement. Without this, + # Google-Chat-on-env-vars-only setups silently fail the gate + # below even though the user is configured. Plugins whose + # ``is_connected`` reads env vars directly (Discord, IRC, + # Teams, LINE, ntfy, Simplex) are unaffected; this only + # restores Google Chat. + seed_for_probe = None + if entry.env_enablement_fn is not None: + try: + seed_for_probe = entry.env_enablement_fn() + except Exception as e: + logger.debug( + "env_enablement_fn for %s raised: %s", entry.name, e + ) + seed_for_probe = None + # Only consult is_connected for platforms that are NOT already # explicitly configured in YAML / env (existing_cfg with # enabled=True means the user wrote it themselves or another @@ -1844,7 +1863,35 @@ def _apply_env_overrides(config: GatewayConfig) -> None: if existing_cfg is None or not existing_cfg.enabled: if entry.is_connected is not None: try: - probe_cfg = existing_cfg or PlatformConfig() + # Probe with ``enabled=True`` since we're asking + # "would this plugin BE configured if we enabled + # it?" not "is it currently enabled?". Google + # Chat's ``_is_connected`` short-circuits on + # ``config.enabled`` being False, which on the + # default ``PlatformConfig()`` would fail the + # gate even with proper env vars set. + if existing_cfg is not None: + probe_cfg = existing_cfg + if not probe_cfg.enabled: + probe_cfg = PlatformConfig( + enabled=True, + extra=dict(probe_cfg.extra or {}), + ) + else: + probe_cfg = PlatformConfig(enabled=True) + if isinstance(seed_for_probe, dict) and seed_for_probe: + # Don't mutate ``existing_cfg``; the probe gets + # a transient view with env-seeded extras layered + # on top of whatever's already there. + probe_extra = dict(getattr(probe_cfg, "extra", {}) or {}) + for k, v in seed_for_probe.items(): + if k == "home_channel": + continue + probe_extra.setdefault(k, v) + probe_cfg = PlatformConfig( + enabled=True, + extra=probe_extra, + ) configured = bool(entry.is_connected(probe_cfg)) except Exception as exc: logger.debug( @@ -1862,31 +1909,26 @@ def _apply_env_overrides(config: GatewayConfig) -> None: if platform not in config.platforms: config.platforms[platform] = PlatformConfig() config.platforms[platform].enabled = True - # Seed extras from env if the plugin opted in. - if entry.env_enablement_fn is not None: - try: - seed = entry.env_enablement_fn() - except Exception as e: - logger.debug( - "env_enablement_fn for %s raised: %s", entry.name, e + # Commit env-seeded extras onto the now-enabled platform. + # We've already called ``env_enablement_fn`` above (for the + # probe); reuse that result instead of calling it twice. + if isinstance(seed_for_probe, dict) and seed_for_probe: + seed = dict(seed_for_probe) + # Extract the home_channel dict (if provided) so we wire it + # up as a proper HomeChannel dataclass. Everything else is + # merged into ``extra``. + home = seed.pop("home_channel", None) + config.platforms[platform].extra.update(seed) + if isinstance(home, dict) and home.get("chat_id"): + config.platforms[platform].home_channel = HomeChannel( + platform=platform, + chat_id=str(home["chat_id"]), + name=str(home.get("name") or "Home"), + thread_id=( + str(home["thread_id"]) + if home.get("thread_id") + else None + ), ) - seed = None - if isinstance(seed, dict) and seed: - # Extract the home_channel dict (if provided) so we wire it - # up as a proper HomeChannel dataclass. Everything else is - # merged into ``extra``. - home = seed.pop("home_channel", None) - config.platforms[platform].extra.update(seed) - if isinstance(home, dict) and home.get("chat_id"): - config.platforms[platform].home_channel = HomeChannel( - platform=platform, - chat_id=str(home["chat_id"]), - name=str(home.get("name") or "Home"), - thread_id=( - str(home["thread_id"]) - if home.get("thread_id") - else None - ), - ) except Exception as e: logger.debug("Plugin platform enable pass failed: %s", e) diff --git a/tests/gateway/test_platform_registry.py b/tests/gateway/test_platform_registry.py index 88d607b1c39..9ca80fe8a1f 100644 --- a/tests/gateway/test_platform_registry.py +++ b/tests/gateway/test_platform_registry.py @@ -895,3 +895,92 @@ class TestPluginEnablementGate: ) finally: _reg.unregister("myexplicitplat") + + def test_is_connected_sees_env_seeded_extras(self, tmp_path, monkeypatch): + """``env_enablement_fn`` extras must be visible to ``is_connected``. + + Some plugins (e.g. Google Chat) implement ``is_connected`` by + inspecting ``config.extra`` (where ``env_enablement_fn`` deposits + env-var-derived state) rather than reading ``os.environ`` directly. + If the gate runs BEFORE the seeding step, those plugins fail the + gate even when the user is genuinely configured via env vars. + + Pin the contract: when both hooks are present, ``env_enablement_fn`` + feeds a candidate config to ``is_connected``. + """ + from gateway.platform_registry import platform_registry as _reg + + seen_extras: dict = {} + + def _is_connected(cfg): + seen_extras["snapshot"] = dict(getattr(cfg, "extra", {}) or {}) + extra = getattr(cfg, "extra", {}) or {} + return bool(extra.get("project_id") and extra.get("subscription_name")) + + def _env_enablement(): + return {"project_id": "p", "subscription_name": "s"} + + _reg.register(PlatformEntry( + name="myextrasplat", + label="MyExtras", + adapter_factory=lambda cfg: None, + check_fn=lambda: True, + is_connected=_is_connected, + env_enablement_fn=_env_enablement, + source="plugin", + )) + try: + home = self._write_config(tmp_path) + monkeypatch.setenv("HERMES_HOME", str(home)) + + from gateway.config import load_gateway_config, Platform + cfg = load_gateway_config() + + plat = Platform("myextrasplat") + assert plat in cfg.platforms, ( + "is_connected was called with empty extras — " + "env_enablement_fn must seed the probe BEFORE the gate" + ) + assert cfg.platforms[plat].enabled is True + # extras populated on the live config too + assert cfg.platforms[plat].extra.get("project_id") == "p" + assert cfg.platforms[plat].extra.get("subscription_name") == "s" + # and the probe saw them + assert seen_extras["snapshot"]["project_id"] == "p" + finally: + _reg.unregister("myextrasplat") + + def test_is_connected_failed_gate_does_not_leak_extras( + self, tmp_path, monkeypatch + ): + """When the gate rejects, env-seeded extras must NOT leak onto + ``config.platforms``. A rejected plugin should be invisible, not + present-but-partially-populated. + """ + from gateway.platform_registry import platform_registry as _reg + + _reg.register(PlatformEntry( + name="myrejectedplat", + label="MyRejected", + adapter_factory=lambda cfg: None, + check_fn=lambda: True, + is_connected=lambda cfg: False, + env_enablement_fn=lambda: {"some_key": "should-not-leak"}, + source="plugin", + )) + try: + home = self._write_config(tmp_path) + monkeypatch.setenv("HERMES_HOME", str(home)) + + from gateway.config import load_gateway_config, Platform + cfg = load_gateway_config() + + plat = Platform("myrejectedplat") + if plat in cfg.platforms: + assert cfg.platforms[plat].enabled is False + assert "some_key" not in cfg.platforms[plat].extra, ( + "Rejected plugin's env-seeded extras leaked onto " + "config.platforms" + ) + finally: + _reg.unregister("myrejectedplat")