diff --git a/gateway/platforms/_shared.py b/gateway/platforms/_shared.py index b3ab6a65b1..6c20976aa3 100644 --- a/gateway/platforms/_shared.py +++ b/gateway/platforms/_shared.py @@ -105,18 +105,27 @@ def decode_json_list_literal(raw): def extra_or_secret(extra: Optional[dict], key: str, env: str, default: Any = "", *, blank_is_unset: bool = True) -> Any: - """``config.extra[key]`` when set, else the scoped env var ``env`` (else ``default``). + """The ONE per-profile setting reader: explicit env ``env`` → the profile's YAML + ``config.extra[key]`` → ``default``. - ``extra`` is the per-profile truth under multiplexing (the YAML→env bridge is skipped for a - secondary profile), so it is consulted first; the env read goes through ``get_scoped_secret``. - An explicit ``False``/``0`` in YAML is always a real value (``require_mention: false`` must not - fall through to the env default). A blank string is unset by default; readers whose YAML key - means "clear it" (``allowed_channels: ""`` = no whitelist, not "use the env CSV") pass + The env rung is the owning profile's, read through ``get_scoped_secret``: a secondary + multiplex profile sees its own ``.env`` and a miss falls to ITS YAML, never to the launch + process's ``os.environ`` (which holds the default profile's bridged values); single-profile + and default-profile installs read ``os.environ`` there, keeping the documented env-over-YAML + contract (an explicit ``DISCORD_ALLOW_MENTION_EVERYONE=false`` beats ``everyone: true``, + #108440; ``TELEGRAM_REACTIONS=true`` beats the stock ``reactions: false``, #109032). A blank + env value is unset. An explicit ``False``/``0`` in YAML is a real value (``require_mention: + false`` must not fall to ``default``). A blank YAML string is unset by default; readers whose + YAML key means "clear it" (``allowed_channels: "" `` = no whitelist) pass ``blank_is_unset=False`` so only a missing/``None`` key falls through. """ + if env: + env_value = get_scoped_secret(env, None) + if env_value is not None and str(env_value).strip(): + return env_value value = (extra or {}).get(key) if value is None or (blank_is_unset and isinstance(value, str) and not value.strip()): - return get_scoped_secret(env, default) + return default return value diff --git a/plugins/platforms/discord/adapter.py b/plugins/platforms/discord/adapter.py index 6408c694a1..1a1b013ae9 100644 --- a/plugins/platforms/discord/adapter.py +++ b/plugins/platforms/discord/adapter.py @@ -271,8 +271,8 @@ from gateway.platforms.base import ( from gateway.platforms.event import MessageEvent, MessageType, ProcessingOutcome from tools.url_safety import is_safe_url from gateway.platforms._shared import ( - env_is_connected as _env_is_connected, platform_gate_env as _scoped_gate_env, send_error, - yaml_env_setter as _yaml_env_setter + env_is_connected as _env_is_connected, extra_or_secret as _extra_or_secret, + platform_gate_env as _scoped_gate_env, send_error, yaml_env_setter as _yaml_env_setter ) @@ -613,9 +613,12 @@ def _build_allowed_mentions(extra: Optional[dict] = None): configured = configured if isinstance(configured, dict) else {} def _b(name: str, key: str, default: bool) -> bool: - if (raw := configured.get(key)) is not None: - return str(raw).strip().lower() in {"true", "1", "yes", "on"} - return _env_bool(name, default) + # Explicit (scoped) env → this profile's YAML → safe default; a scoped miss never reads + # another profile's bridged env, and an explicit ``=false`` beats ``everyone: true``. + raw = _extra_or_secret(configured, key, name, None) + if raw is None: + return default + return raw if isinstance(raw, bool) else str(raw).strip().lower() in {"true", "1", "yes", "on"} return discord.AllowedMentions( everyone=_b("DISCORD_ALLOW_MENTION_EVERYONE", "everyone", False), @@ -4563,17 +4566,17 @@ class DiscordAdapter(DiscordMediaMixin, BasePlatformAdapter): return resolve_channel_prompt(self.config.extra, channel_id, parent_id) def _extra_or_env_flag(self, key: str, env_key: str, env_default: str, *, truthy: bool) -> bool: - """Boolean from ``config.extra[key]`` (str parsed permissively) else ``env_key``. - ``truthy=True`` env values must be in {true,1,yes,on}; ``truthy=False`` env values are on - unless in {false,0,no,off} — matching each flag's historical default shape.""" + """Boolean: explicit scoped ``env_key`` → ``config.extra[key]`` (str parsed permissively) → + ``env_default``. ``truthy=True`` values must be in {true,1,yes,on}; ``truthy=False`` values are + on unless in {false,0,no,off} — matching each flag's historical default shape.""" extra = getattr(self.config, "extra", None) - configured = extra.get(key) if isinstance(extra, dict) else None - if configured is not None: - if isinstance(configured, str): - return configured.lower() not in {"false", "0", "no", "off"} - return bool(configured) - env = _scoped_gate_env(env_key, env_default).lower() - return env in {"true", "1", "yes", "on"} if truthy else env not in {"false", "0", "no", "off"} + configured = _extra_or_secret(extra if isinstance(extra, dict) else None, key, env_key, None) + if configured is None: + configured = env_default + if isinstance(configured, bool): + return configured + text = str(configured).strip().lower() + return text in {"true", "1", "yes", "on"} if truthy else text not in {"false", "0", "no", "off"} def _discord_require_mention(self) -> bool: """Return whether Discord channel messages require a bot mention.""" diff --git a/plugins/platforms/feishu/adapter.py b/plugins/platforms/feishu/adapter.py index 75bed402f9..7a7de53936 100644 --- a/plugins/platforms/feishu/adapter.py +++ b/plugins/platforms/feishu/adapter.py @@ -1293,7 +1293,7 @@ class FeishuAdapter(BasePlatformAdapter): # Scoped read: under multiplex a secondary profile's .env must govern its own adapter; yaml # feishu.allow_bots reaches it via ``extra`` (the env bridge is skipped under its scope). # See #86905. - allow_bots = str(_get_scoped_secret("FEISHU_ALLOW_BOTS", "") or extra.get("allow_bots") or "none").strip().lower() + allow_bots = str(_extra_or_secret("allow_bots", "FEISHU_ALLOW_BOTS", "none") or "none").strip().lower() if allow_bots not in {"none", "mentions", "all"}: logger.warning( "[Feishu] Unknown allow_bots=%r, falling back to 'none'. Valid: none, mentions, all.", diff --git a/plugins/platforms/matrix/adapter.py b/plugins/platforms/matrix/adapter.py index 69225c8f50..ce95ef2249 100644 --- a/plugins/platforms/matrix/adapter.py +++ b/plugins/platforms/matrix/adapter.py @@ -476,9 +476,8 @@ def _csv_set(raw: Any) -> Set[str]: def _extra_csv_set(config, key: str, env_name: str) -> Set[str]: - """Resolve a room/user list from config.extra[key] (blank = unset), else the scoped env var — - under multiplex os.environ is the DEFAULT profile's room/user list.""" - return _csv_set(_extra_or_secret(config.extra, key, env_name)) + """Resolve a room/user list: scoped env var → config.extra[key] → empty.""" + return _csv_set(_extra_or_secret(config.extra, key, env_name, "", blank_is_unset=False)) def _recovery_key_output_path() -> Optional[Path]: @@ -819,10 +818,10 @@ class MatrixAdapter(BasePlatformAdapter): self._auto_thread: bool = self._extra_truthy(config, "auto_thread", "MATRIX_AUTO_THREAD", "true") self._dm_auto_thread: bool = _env_truthy("MATRIX_DM_AUTO_THREAD", "false") self._dm_mention_threads: bool = self._extra_truthy(config, "dm_mention_threads", "MATRIX_DM_MENTION_THREADS", "false") - raw_session_scope = str(config.extra.get("session_scope") or _get_scoped_secret("MATRIX_SESSION_SCOPE", "auto")).strip().lower() + raw_session_scope = str(_extra_or_secret(config.extra, "session_scope", "MATRIX_SESSION_SCOPE", "auto")).strip().lower() self._matrix_session_scope = raw_session_scope if raw_session_scope in {"auto", "room", "thread"} else "auto" self._process_notices: bool = self._extra_truthy(config, "process_notices", "MATRIX_PROCESS_NOTICES", "false") - self._reactions_enabled: bool = str(_get_scoped_secret("MATRIX_REACTIONS", "true")).lower() not in {"false", "0", "no"} + self._reactions_enabled: bool = str(_extra_or_secret(config.extra, "reactions", "MATRIX_REACTIONS", "true")).lower() not in {"false", "0", "no"} self._pending_reactions: dict[tuple[str, str], str] = {} # Let the final message land before redacting reactions ("missing event" in some # clients). 5s is empirically safe; if it must be tunable, use config.yaml not env. @@ -844,12 +843,13 @@ class MatrixAdapter(BasePlatformAdapter): self._approval_timeout_seconds = _env_number("MATRIX_APPROVAL_TIMEOUT_SECONDS", 300, int) self._model_picker_prompts_by_event: Dict[str, _MatrixPickerPrompt] = {} self._choice_picker_prompts_by_event: Dict[str, _MatrixPickerPrompt] = {} - # Authz lists via the scoped reader: under multiplex os.environ is the DEFAULT profile's - # allowlist, which must not decide who approves tool calls on a secondary bot. - self._allowed_user_ids: Set[str] = _csv_set(_get_scoped_secret("MATRIX_ALLOWED_USERS", "").strip()) + # Authz lists: scoped env → this profile's YAML (``allowed_users`` / ``ignore_user_patterns``, + # seeded by the bridge) → empty. Under multiplex os.environ is the DEFAULT profile's allowlist, + # which must not decide who approves tool calls on a secondary bot. + self._allowed_user_ids: Set[str] = _extra_csv_set(config, "allowed_users", "MATRIX_ALLOWED_USERS") self._allowed_room_ids: Set[str] = set(self._allowed_rooms) self._ignored_user_patterns: list[re.Pattern[str]] = [] - for pattern in (p.strip() for p in _get_scoped_secret("MATRIX_IGNORE_USER_PATTERNS", "").strip().split(",") if p.strip()): + for pattern in _csv_set(_extra_or_secret(config.extra, "ignore_user_patterns", "MATRIX_IGNORE_USER_PATTERNS", "")): try: self._ignored_user_patterns.append(re.compile(pattern)) except re.error as exc: @@ -869,7 +869,7 @@ class MatrixAdapter(BasePlatformAdapter): @staticmethod def _extra_truthy(config, key: str, env_name: str, default: str) -> bool: - """``config.extra[key]`` (YAML-bridged, per profile; blank = unset) else the env var, true/1/yes.""" + """Scoped env var → ``config.extra[key]`` (YAML, per profile) → ``default``; true/1/yes semantics.""" configured = _extra_or_secret(config.extra, key, env_name, default) return configured if isinstance(configured, bool) else str(configured).lower() in ("true", "1", "yes") @@ -887,19 +887,15 @@ class MatrixAdapter(BasePlatformAdapter): @staticmethod def _parse_require_mention(config) -> bool: - """require_mention from config.extra, else MATRIX_REQUIRE_MENTION (default true).""" - configured = MatrixAdapter._configured_bool(config, "require_mention") - if configured is not None: - return configured - return str(_get_scoped_secret("MATRIX_REQUIRE_MENTION", "true")).lower() not in {"false", "0", "no", "off"} + """MATRIX_REQUIRE_MENTION (scoped) → ``require_mention`` in config.extra → true.""" + configured = _extra_or_secret(config.extra, "require_mention", "MATRIX_REQUIRE_MENTION", True) + return configured if isinstance(configured, bool) else str(configured).lower() not in {"false", "0", "no", "off"} @staticmethod def _parse_thread_require_mention(config) -> bool: - """thread_require_mention from config.extra, else MATRIX_THREAD_REQUIRE_MENTION (default false).""" - configured = MatrixAdapter._configured_bool(config, "thread_require_mention") - if configured is not None: - return configured - return str(_get_scoped_secret("MATRIX_THREAD_REQUIRE_MENTION", "false")).lower() in {"true", "1", "yes", "on"} + """MATRIX_THREAD_REQUIRE_MENTION (scoped) → ``thread_require_mention`` in config.extra → false.""" + configured = _extra_or_secret(config.extra, "thread_require_mention", "MATRIX_THREAD_REQUIRE_MENTION", False) + return configured if isinstance(configured, bool) else str(configured).lower() not in {"false", "0", "no", "off"} @staticmethod def _extract_server_ed25519(device_keys_obj: Any) -> Optional[str]: diff --git a/plugins/platforms/slack/adapter.py b/plugins/platforms/slack/adapter.py index 46ae570309..414ac6b0ef 100644 --- a/plugins/platforms/slack/adapter.py +++ b/plugins/platforms/slack/adapter.py @@ -2567,9 +2567,8 @@ class SlackAdapter(BasePlatformAdapter): pass def _slack_allow_bots(self) -> str: - """Return normalized Slack bot-message policy.""" - # Scoped read: under multiplex os.environ is the DEFAULT profile's bot-admission policy. - raw = self.config.extra.get("allow_bots", "") or _get_scoped_secret("SLACK_ALLOW_BOTS", "none") + """Return normalized Slack bot-message policy (scoped ``SLACK_ALLOW_BOTS`` → YAML → none).""" + raw = _extra_or_secret(self.config.extra, "allow_bots", "SLACK_ALLOW_BOTS", "none") value = str(raw).lower().strip() if value not in {"none", "mentions", "all"}: logger.warning("[Slack] Unknown allow_bots=%r; treating as 'none'", raw) @@ -2974,10 +2973,8 @@ class SlackAdapter(BasePlatformAdapter): return await self._react(channel, timestamp, emoji, team_id, remove=True) def _reactions_enabled(self) -> bool: - """Whether message reactions are enabled (``extra.reactions`` / ``SLACK_REACTIONS``).""" - configured = self.config.extra.get("reactions") - if configured is None: - configured = _get_scoped_secret("SLACK_REACTIONS", "true") + """Whether message reactions are enabled (scoped ``SLACK_REACTIONS`` → ``extra.reactions`` → on).""" + configured = _extra_or_secret(self.config.extra, "reactions", "SLACK_REACTIONS", "true") return str(configured).lower() not in {"false", "0", "no"} def _reacting_target(self, event: MessageEvent) -> Optional[Tuple[str, str, Any]]: diff --git a/plugins/platforms/telegram/adapter.py b/plugins/platforms/telegram/adapter.py index f8f1f3774b..b1adc6f248 100644 --- a/plugins/platforms/telegram/adapter.py +++ b/plugins/platforms/telegram/adapter.py @@ -20,7 +20,7 @@ logger = logging.getLogger(__name__) from agent.deadline import run_bounded_async from gateway.platforms._shared import ( decode_json_list_literal as _decode_json_list_literal, - get_scoped_secret as _get_scoped_secret, + extra_or_secret as _extra_or_secret, get_scoped_secret as _get_scoped_secret, platform_gate_env as _scoped_gate_env, ) @@ -5046,22 +5046,20 @@ class TelegramAdapter(BasePlatformAdapter): # ── Group mention gating ────────────────────────────────────────────── def _extra_bool(self, key: str, env_name: str, default: str, *fallback_keys: str) -> bool: - """Boolean gate from ``config.extra[key]`` (then ``fallback_keys``), else env var.""" - configured = self.config.extra.get(key) + """Boolean gate: scoped ``env_name`` → ``config.extra[key]`` (then ``fallback_keys``) → ``default``.""" + configured = _extra_or_secret(self.config.extra, key, env_name, None) for alt in fallback_keys: if configured is None: configured = self.config.extra.get(alt) - if configured is not None: - if isinstance(configured, str): - return configured.lower() in {"true", "1", "yes", "on"} - return bool(configured) - return _scoped_gate_env(env_name, default).lower() in {"true", "1", "yes", "on"} + if configured is None: + configured = default + if isinstance(configured, bool): + return configured + return str(configured).strip().lower() in {"true", "1", "yes", "on"} def _extra_str_set(self, key: str, env_name: str) -> set[str]: - """Comma/list allowlist from ``config.extra[key]``, else the profile-scoped env var.""" - raw = self.config.extra.get(key) - if raw is None: - raw = _scoped_gate_env(env_name) + """Comma/list allowlist: scoped ``env_name`` → ``config.extra[key]`` → empty.""" + raw = _extra_or_secret(self.config.extra, key, env_name, "", blank_is_unset=False) raw = _decode_json_list_literal(raw) if isinstance(raw, list): return {str(part).strip() for part in raw if str(part).strip()} @@ -6392,17 +6390,13 @@ class TelegramAdapter(BasePlatformAdapter): # -- Message reactions (processing lifecycle) -- def _reactions_enabled(self) -> bool: - """Reactions enabled via TELEGRAM_REACTIONS or ``extra.reactions`` (YAML, per profile). + """Reactions: scoped ``TELEGRAM_REACTIONS`` → ``extra.reactions`` (YAML, per profile) → off. - An explicitly set env var wins over YAML — the same rule ``yaml_env_setter`` documents for - the YAML→env bridge — so the stock ``reactions: false`` every install materializes cannot - silently kill a documented ``TELEGRAM_REACTIONS=true`` (#109032). Under multiplex a scoped - miss returns the default instead of another profile's process-env value (#72348), so only - a scoped/env hit counts as explicit; otherwise the profile's own YAML decides. + An explicit env var wins over YAML, so the stock ``reactions: false`` every install + materializes cannot silently kill a documented ``TELEGRAM_REACTIONS=true`` (#109032). Under + multiplex a scoped miss falls to the profile's own YAML, never another profile's env (#72348). """ - configured = _scoped_gate_env("TELEGRAM_REACTIONS", "") - if not configured: - configured = self.config.extra.get("reactions") + configured = _extra_or_secret(self.config.extra, "reactions", "TELEGRAM_REACTIONS", None) if configured is None: return False return str(configured).lower() not in {"false", "0", "no"} diff --git a/tests/gateway/test_shared_platform_boilerplate.py b/tests/gateway/test_shared_platform_boilerplate.py index 33b2bad357..862e0fc290 100644 --- a/tests/gateway/test_shared_platform_boilerplate.py +++ b/tests/gateway/test_shared_platform_boilerplate.py @@ -103,14 +103,40 @@ def test_buzz_yaml_bridge_seeds_extra_for_a_secondary_profile(monkeypatch): monkeypatch.delenv(var, raising=False) -def test_extra_or_secret_honours_explicit_false_but_not_blank(monkeypatch): - monkeypatch.setattr(shared, "get_scoped_secret", lambda n, d=None, **k: f"env:{d}") +def test_extra_or_secret_precedence_env_then_yaml_then_default(monkeypatch): + """Explicit scoped env → the profile's YAML → default; a blank env value is unset (#108440, #109032).""" + env: dict = {} + monkeypatch.setattr(shared, "get_scoped_secret", lambda n, d=None, **k: env.get(n, d)) + # YAML alone: explicit False is a real value; blank/None fall to the default. assert shared.extra_or_secret({"require_mention": False}, "require_mention", "X", "true") is False - assert shared.extra_or_secret({"require_mention": ""}, "require_mention", "X", "true") == "env:true" - assert shared.extra_or_secret(None, "require_mention", "X", "true") == "env:true" + assert shared.extra_or_secret({"require_mention": ""}, "require_mention", "X", "true") == "true" + assert shared.extra_or_secret(None, "require_mention", "X", "true") == "true" # Readers where a blank YAML value means "cleared" (channel whitelists) keep it as a value. - assert shared.extra_or_secret({"allowed_channels": ""}, "allowed_channels", "X", "", blank_is_unset=False) == "" - assert shared.extra_or_secret({}, "allowed_channels", "X", "", blank_is_unset=False) == "env:" + assert shared.extra_or_secret({"allowed_channels": ""}, "allowed_channels", "X", "dflt", blank_is_unset=False) == "" + assert shared.extra_or_secret({}, "allowed_channels", "X", "dflt", blank_is_unset=False) == "dflt" + # An explicit env value beats YAML in either direction; a blank env value does not. + env["X"] = "false" + assert shared.extra_or_secret({"reactions": True}, "reactions", "X", "true") == "false" + env["X"] = "true" + assert shared.extra_or_secret({"reactions": False}, "reactions", "X", "false") == "true" + env["X"] = " " + assert shared.extra_or_secret({"reactions": False}, "reactions", "X", "true") is False + + +def test_extra_or_secret_scoped_miss_never_reads_launch_env(monkeypatch): + """Under a secondary's scope the launch process's os.environ is another profile's value: a miss + falls to the secondary's OWN YAML, then the default — never to os.environ.""" + monkeypatch.setenv("X_FLAG", "launch-value") + ss.set_multiplex_active(True) + token = ss.set_secret_scope({}) + try: + assert shared.extra_or_secret({"flag": "yaml-value"}, "flag", "X_FLAG", "dflt") == "yaml-value" + assert shared.extra_or_secret({}, "flag", "X_FLAG", "dflt") == "dflt" + finally: + ss.reset_secret_scope(token) + # Unscoped (single-profile / default profile): env-over-YAML exactly as documented. + ss.set_multiplex_active(False) + assert shared.extra_or_secret({"flag": "yaml-value"}, "flag", "X_FLAG", "dflt") == "launch-value" def test_external_fallback_consults_profile_scope_only_when_unscoped(monkeypatch): diff --git a/tests/gateway/test_slack_thread_require_mention.py b/tests/gateway/test_slack_thread_require_mention.py index 30bcd6a7d7..ea915b0500 100644 --- a/tests/gateway/test_slack_thread_require_mention.py +++ b/tests/gateway/test_slack_thread_require_mention.py @@ -60,13 +60,14 @@ def test_thread_require_mention_env_bridge(monkeypatch): def test_thread_require_mention_parses_yaml_and_env(monkeypatch): + """Explicit env beats YAML (documented env-over-YAML contract); YAML decides when env is unset.""" monkeypatch.setenv("SLACK_THREAD_REQUIRE_MENTION", "true") - assert make_adapter()._slack_thread_require_mention() is True - assert ( - make_adapter({"thread_require_mention": "false"})._slack_thread_require_mention() - is False - ) + assert make_adapter({"thread_require_mention": "false"})._slack_thread_require_mention() is True + monkeypatch.setenv("SLACK_THREAD_REQUIRE_MENTION", "false") + assert make_adapter({"thread_require_mention": True})._slack_thread_require_mention() is False + monkeypatch.delenv("SLACK_THREAD_REQUIRE_MENTION", raising=False) + assert make_adapter({"thread_require_mention": "false"})._slack_thread_require_mention() is False assert make_adapter({"thread_require_mention": True})._slack_thread_require_mention() is True diff --git a/tests/gateway/test_telegram_group_gating.py b/tests/gateway/test_telegram_group_gating.py index dd15c6d9ab..20daa20fe5 100644 --- a/tests/gateway/test_telegram_group_gating.py +++ b/tests/gateway/test_telegram_group_gating.py @@ -7,6 +7,23 @@ from gateway.config import Platform, PlatformConfig, load_gateway_config from gateway.platforms.event import MessageType from gateway.session import SessionSource +import os + +import pytest + + +@pytest.fixture(autouse=True) +def _restore_telegram_env(): + """The YAML→env bridge tests below write TELEGRAM_* into os.environ; an explicit env value now + beats ``config.extra`` for the owning profile, so a leaked bridge value would silently override the + ``extra`` the later adapter tests construct with.""" + saved = {k: v for k, v in os.environ.items() if k.startswith("TELEGRAM_")} + yield + for k in [k for k in os.environ if k.startswith("TELEGRAM_")]: + if k not in saved: + del os.environ[k] + os.environ.update(saved) + def _make_adapter( require_mention=None,