From 1f33df1ea612769558103dd3622627dee18c4ea8 Mon Sep 17 00:00:00 2001 From: Xubin Ren Date: Tue, 14 Apr 2026 17:24:00 +0000 Subject: [PATCH] fix: preserve empty dict allow_from handling Keep dict-backed channel configs compatible with both allow_from and allowFrom without losing empty-list semantics, and add focused regression coverage for the allow-list boundary. Made-with: Cursor --- nanobot/channels/base.py | 5 ++++- nanobot/channels/manager.py | 5 ++++- tests/channels/test_base_channel.py | 12 ++++++++++++ tests/channels/test_channel_plugins.py | 24 +++++++++++++++++++++++- 4 files changed, 43 insertions(+), 3 deletions(-) diff --git a/nanobot/channels/base.py b/nanobot/channels/base.py index b6b50681c..35aac3e42 100644 --- a/nanobot/channels/base.py +++ b/nanobot/channels/base.py @@ -117,7 +117,10 @@ class BaseChannel(ABC): def is_allowed(self, sender_id: str) -> bool: """Check if *sender_id* is permitted. Empty list → deny all; ``"*"`` → allow all.""" if isinstance(self.config, dict): - allow_list = self.config.get("allow_from") or self.config.get("allowFrom") or [] + if "allow_from" in self.config: + allow_list = self.config.get("allow_from") + else: + allow_list = self.config.get("allowFrom", []) else: allow_list = getattr(self.config, "allow_from", []) if not allow_list: diff --git a/nanobot/channels/manager.py b/nanobot/channels/manager.py index 58531c412..634e04fe7 100644 --- a/nanobot/channels/manager.py +++ b/nanobot/channels/manager.py @@ -77,7 +77,10 @@ class ChannelManager: for name, ch in self.channels.items(): cfg = ch.config if isinstance(cfg, dict): - allow = cfg.get("allow_from") or cfg.get("allowFrom") + if "allow_from" in cfg: + allow = cfg.get("allow_from") + else: + allow = cfg.get("allowFrom") else: allow = getattr(cfg, "allow_from", None) if allow == []: diff --git a/tests/channels/test_base_channel.py b/tests/channels/test_base_channel.py index 5d10d4e15..660aff60e 100644 --- a/tests/channels/test_base_channel.py +++ b/tests/channels/test_base_channel.py @@ -23,3 +23,15 @@ def test_is_allowed_requires_exact_match() -> None: assert channel.is_allowed("allow@email.com") is True assert channel.is_allowed("attacker|allow@email.com") is False + + +def test_is_allowed_supports_dict_allow_from_alias() -> None: + channel = _DummyChannel({"allowFrom": ["alice"]}, MessageBus()) + + assert channel.is_allowed("alice") is True + + +def test_is_allowed_denies_empty_dict_allow_from() -> None: + channel = _DummyChannel({"allow_from": []}, MessageBus()) + + assert channel.is_allowed("alice") is False diff --git a/tests/channels/test_channel_plugins.py b/tests/channels/test_channel_plugins.py index 8bb95b532..584b5864f 100644 --- a/tests/channels/test_channel_plugins.py +++ b/tests/channels/test_channel_plugins.py @@ -646,7 +646,10 @@ class _ChannelWithAllowFrom(BaseChannel): def __init__(self, config, bus, allow_from): super().__init__(config, bus) - self.config.allow_from = allow_from + if isinstance(self.config, dict): + self.config["allow_from"] = allow_from + else: + self.config.allow_from = allow_from async def start(self) -> None: pass @@ -714,6 +717,25 @@ async def test_validate_allow_from_passes_with_asterisk(): mgr._validate_allow_from() +@pytest.mark.asyncio +async def test_validate_allow_from_raises_on_empty_dict_allow_from(): + """_validate_allow_from should reject empty dict-backed allow_from lists.""" + fake_config = SimpleNamespace( + channels=ChannelsConfig(), + providers=SimpleNamespace(groq=SimpleNamespace(api_key="")), + ) + + mgr = ChannelManager.__new__(ChannelManager) + mgr.config = fake_config + mgr.channels = {"test": _ChannelWithAllowFrom({"enabled": True}, None, [])} + mgr._dispatch_task = None + + with pytest.raises(SystemExit) as exc_info: + mgr._validate_allow_from() + + assert "empty allowFrom" in str(exc_info.value) + + @pytest.mark.asyncio async def test_get_channel_returns_channel_if_exists(): """get_channel should return the channel if it exists."""