From 008caa88a21c3683a652ed54696ab23c8668ef34 Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Sat, 12 Sep 2026 23:46:49 -0700 Subject: [PATCH] fix(platforms): extra_or_secret keeps blank-string YAML values where the old readers did dingtalk _extra_get, mattermost _extra_or_env and slack _extra_or_env_flag/_channel_set fell through to env only on None, so `allowed_channels: ""` / `free_response_channels: ""` meant "no whitelist" rather than "use the env CSV". The shared reader treated blank as unset and silently widened those to the env value. New `blank_is_unset=False` knob restores the old semantics at those seven call sites; the default (blank = unset) stays for the readers whose old body was `extra.get(k) or env`. --- gateway/platforms/_shared.py | 11 +++++++---- plugins/platforms/dingtalk/adapter.py | 4 ++-- plugins/platforms/mattermost/adapter.py | 6 +++--- plugins/platforms/slack/adapter.py | 4 ++-- tests/gateway/test_shared_platform_boilerplate.py | 3 +++ 5 files changed, 17 insertions(+), 11 deletions(-) diff --git a/gateway/platforms/_shared.py b/gateway/platforms/_shared.py index 04b5d5147d..3dea6065e0 100644 --- a/gateway/platforms/_shared.py +++ b/gateway/platforms/_shared.py @@ -86,16 +86,19 @@ def platform_gate_env(name: str, default: str = "") -> str: return (os.getenv(name) or default).strip() -def extra_or_secret(extra: Optional[dict], key: str, env: str, default: Any = "") -> Any: +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``). ``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``. - "Unset" is ``None`` or a blank string — an explicit ``False``/``0`` in YAML is a real value - (``require_mention: false`` must not fall through to the env default). + 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 + ``blank_is_unset=False`` so only a missing/``None`` key falls through. """ value = (extra or {}).get(key) - if value is None or (isinstance(value, str) and not value.strip()): + if value is None or (blank_is_unset and isinstance(value, str) and not value.strip()): return get_scoped_secret(env, default) return value diff --git a/plugins/platforms/dingtalk/adapter.py b/plugins/platforms/dingtalk/adapter.py index 5474e83055..a1165ef9cc 100644 --- a/plugins/platforms/dingtalk/adapter.py +++ b/plugins/platforms/dingtalk/adapter.py @@ -254,11 +254,11 @@ class DingTalkAdapter(BasePlatformAdapter): def _csv_setting(self, key: str, env_name: str) -> Set[str]: """List/CSV setting from config.extra[key], falling back to the env var.""" - return _csv_set(_extra_or_secret(self.config.extra, key, env_name)) + return _csv_set(_extra_or_secret(self.config.extra, key, env_name, blank_is_unset=False)) def _dingtalk_require_mention(self) -> bool: """Whether group chats require an explicit bot trigger.""" - configured = _extra_or_secret(self.config.extra, "require_mention", "DINGTALK_REQUIRE_MENTION", "false") + configured = _extra_or_secret(self.config.extra, "require_mention", "DINGTALK_REQUIRE_MENTION", "false", blank_is_unset=False) return configured.lower() in _TRUTHY if isinstance(configured, str) else bool(configured) def _dingtalk_allowed_chats(self) -> Set[str]: diff --git a/plugins/platforms/mattermost/adapter.py b/plugins/platforms/mattermost/adapter.py index e1f195b270..a4f68c1640 100644 --- a/plugins/platforms/mattermost/adapter.py +++ b/plugins/platforms/mattermost/adapter.py @@ -495,14 +495,14 @@ class MattermostAdapter(BasePlatformAdapter): """Mention-gate a non-DM post; return the cleaned text, or None to ignore it. allowed_channels is a whitelist checked first (@mentions elsewhere are ignored); require_mention (default true) is bypassed in free_response_channels.""" - allowed_channels = _channel_id_set(_extra_or_secret(self.config.extra, "allowed_channels", "MATTERMOST_ALLOWED_CHANNELS")) + allowed_channels = _channel_id_set(_extra_or_secret(self.config.extra, "allowed_channels", "MATTERMOST_ALLOWED_CHANNELS", blank_is_unset=False)) if allowed_channels and channel_id not in allowed_channels: logger.debug("Mattermost: ignoring message in non-allowed channel: %s", channel_id) return None - require_mention = str(_extra_or_secret(self.config.extra, "require_mention", "MATTERMOST_REQUIRE_MENTION", "true") + require_mention = str(_extra_or_secret(self.config.extra, "require_mention", "MATTERMOST_REQUIRE_MENTION", "true", blank_is_unset=False) ).lower() not in {"false", "0", "no"} free_channels = _channel_id_set( - _extra_or_secret(self.config.extra, "free_response_channels", "MATTERMOST_FREE_RESPONSE_CHANNELS")) + _extra_or_secret(self.config.extra, "free_response_channels", "MATTERMOST_FREE_RESPONSE_CHANNELS", blank_is_unset=False)) mention_patterns = [f"@{self._bot_username}", f"@{self._bot_user_id}"] has_mention = any(pattern.lower() in message_text.lower() for pattern in mention_patterns) if require_mention and channel_id not in free_channels and not has_mention: diff --git a/plugins/platforms/slack/adapter.py b/plugins/platforms/slack/adapter.py index 7fd7c25149..56ea96491a 100644 --- a/plugins/platforms/slack/adapter.py +++ b/plugins/platforms/slack/adapter.py @@ -5963,7 +5963,7 @@ class SlackAdapter(BasePlatformAdapter): def _extra_or_env_flag(self, key: str, env_var: str, *, strip: bool = False) -> bool: """Opt-in boolean: ``config.extra[key]`` wins, else ``env_var`` (default false).""" - configured = _extra_or_secret(self.config.extra, key, env_var, "false") + configured = _extra_or_secret(self.config.extra, key, env_var, "false", blank_is_unset=False) if isinstance(configured, str): if strip: configured = configured.strip() @@ -5997,7 +5997,7 @@ class SlackAdapter(BasePlatformAdapter): self, key: str, env_var: str, *, coerce_scalar: bool = False) -> set: """Channel-ID set from ``config.extra[key]`` (list or CSV) else ``env_var`` CSV. ``coerce_scalar`` accepts non-str scalars (a bare numeric YAML value loads as int).""" - raw = _extra_or_secret(self.config.extra, key, env_var, "") + raw = _extra_or_secret(self.config.extra, key, env_var, "", blank_is_unset=False) if isinstance(raw, list): return {str(part).strip() for part in raw if str(part).strip()} if coerce_scalar: diff --git a/tests/gateway/test_shared_platform_boilerplate.py b/tests/gateway/test_shared_platform_boilerplate.py index 7e40b21377..33b2bad357 100644 --- a/tests/gateway/test_shared_platform_boilerplate.py +++ b/tests/gateway/test_shared_platform_boilerplate.py @@ -108,6 +108,9 @@ def test_extra_or_secret_honours_explicit_false_but_not_blank(monkeypatch): 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" + # 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:" def test_external_fallback_consults_profile_scope_only_when_unscoped(monkeypatch):