diff --git a/plugins/platforms/slack/adapter.py b/plugins/platforms/slack/adapter.py index 50d215ea65e3..feba37f47c64 100644 --- a/plugins/platforms/slack/adapter.py +++ b/plugins/platforms/slack/adapter.py @@ -947,6 +947,11 @@ def __init__(self, config: PlatformConfig): # per-user). self._channel_name_cache: Dict[Tuple[str, str], str] = {} self._CHANNEL_NAME_CACHE_MAX = 5000 + # Channel metadata used by dynamic private-channel allowlisting. + # Keep this separate from the display-name cache because the policy + # also needs Slack's privacy and membership flags. + self._channel_info_cache: Dict[Tuple[str, str], Dict[str, Any]] = {} + self._CHANNEL_INFO_CACHE_MAX = 5000 # (team_id, user_id) → Slack bot identity, same workspace scoping as # the name cache. Used to catch peer-agent posts that arrive as plain # user messages without bot_id/subtype=bot_message markers. @@ -4386,6 +4391,38 @@ async def _resolve_channel_name( ) return name + async def _resolve_channel_info( + self, channel_id: str, team_id: str = "" + ) -> Optional[Dict[str, Any]]: + """Return cached Slack channel metadata, or ``None`` on failure.""" + if not channel_id or not self._app: + return None + team_id = str(team_id or self._channel_team.get(channel_id, "")) + cache_key = (team_id, str(channel_id)) + cache = getattr(self, "_channel_info_cache", None) + if cache is None: + cache = {} + self._channel_info_cache = cache + cached = cache.get(cache_key) + if cached is not None: + return cached + try: + resp = await self._get_client( + channel_id, team_id=team_id or None + ).conversations_info(channel=channel_id) + payload = _slack_response_payload(resp) + channel = payload.get("channel") if payload.get("ok") else None + if not isinstance(channel, dict): + return None + except Exception as e: + logger.debug("[Slack] conversations.info failed for %s: %s", channel_id, e) + return None + cache[cache_key] = channel + self._trim_oldest_dict_entries( + cache, getattr(self, "_CHANNEL_INFO_CACHE_MAX", 5000) + ) + return channel + async def _humanize_user_mentions( self, text: str, chat_id: str = "", team_id: str = "" ) -> str: @@ -6160,9 +6197,10 @@ async def _handle_slack_message( return if not is_one_to_one_dm and bot_uid: - # Check allowed channels — if set, only respond in these channels (whitelist) - allowed_channels = self._slack_allowed_channels() - if allowed_channels and channel_id not in allowed_channels: + # Exact channel IDs and joined private channel-name prefixes form + # one allowlist. Prefix lookup fails closed on missing metadata or + # Slack API errors. + if not await self._slack_channel_is_allowed(channel_id, team_id=team_id): logger.debug( "[Slack] Ignoring message in non-allowed channel: %s", channel_id ) @@ -8909,6 +8947,46 @@ def _slack_allowed_channels(self) -> set: return {part.strip() for part in raw.split(",") if part.strip()} return set() + def _slack_allowed_private_channel_prefixes(self) -> set: + """Return channel-name prefixes allowed through the Slack gate. + + Prefix matches are intentionally limited to private channels where + Slack reports this app as a member. This lets operators safely admit + newly created project channels without copying channel IDs into the + configuration first. + """ + raw = self.config.extra.get("allowed_private_channel_prefixes") + if raw is None: + raw = os.getenv("SLACK_ALLOWED_PRIVATE_CHANNEL_PREFIXES", "") + if isinstance(raw, (list, tuple, set)): + return {str(part).strip() for part in raw if str(part).strip()} + if isinstance(raw, str) and raw.strip(): + return {part.strip() for part in raw.split(",") if part.strip()} + return set() + + async def _slack_channel_is_allowed( + self, channel_id: str, team_id: str = "" + ) -> bool: + """Apply exact-ID and joined-private-prefix allowlisting.""" + allowed_channels = self._slack_allowed_channels() + prefixes = self._slack_allowed_private_channel_prefixes() + if not allowed_channels and not prefixes: + return True + if channel_id in allowed_channels: + return True + if not prefixes: + return False + + channel = await self._resolve_channel_info(channel_id, team_id=team_id) + if not channel: + return False + if channel.get("is_im") or channel.get("is_mpim"): + return False + if not channel.get("is_private") or not channel.get("is_member"): + return False + name = str(channel.get("name_normalized") or channel.get("name") or "") + return any(name.startswith(prefix) for prefix in prefixes) + def _slack_require_mention_channels(self) -> set: """Return channel IDs where a bot @mention is ALWAYS required. @@ -9544,6 +9622,11 @@ def _apply_yaml_config(yaml_cfg: dict, slack_cfg: dict) -> dict | None: if isinstance(ac, list): ac = ",".join(str(v) for v in ac) os.environ["SLACK_ALLOWED_CHANNELS"] = str(ac) + apcp = slack_cfg.get("allowed_private_channel_prefixes") + if apcp is not None and not os.getenv("SLACK_ALLOWED_PRIVATE_CHANNEL_PREFIXES"): + if isinstance(apcp, list): + apcp = ",".join(str(v) for v in apcp) + os.environ["SLACK_ALLOWED_PRIVATE_CHANNEL_PREFIXES"] = str(apcp) # ignored_channels: blacklist channels where Slack must never respond. ic = slack_cfg.get("ignored_channels") if ic is not None and not os.getenv("SLACK_IGNORED_CHANNELS"): diff --git a/tests/gateway/test_slack_mention.py b/tests/gateway/test_slack_mention.py index a3d3aed0b85c..a02a6b76f1e1 100644 --- a/tests/gateway/test_slack_mention.py +++ b/tests/gateway/test_slack_mention.py @@ -60,7 +60,8 @@ def _ensure_slack_mock(): def _make_adapter(require_mention=None, strict_mention=None, free_response_channels=None, - allowed_channels=None, mention_patterns=None): + allowed_channels=None, allowed_private_channel_prefixes=None, + mention_patterns=None): extra = {} if require_mention is not None: extra["require_mention"] = require_mention @@ -70,6 +71,8 @@ def _make_adapter(require_mention=None, strict_mention=None, free_response_chann extra["free_response_channels"] = free_response_channels if allowed_channels is not None: extra["allowed_channels"] = allowed_channels + if allowed_private_channel_prefixes is not None: + extra["allowed_private_channel_prefixes"] = allowed_private_channel_prefixes if mention_patterns is not None: extra["mention_patterns"] = mention_patterns @@ -485,6 +488,52 @@ def test_allowed_channels_env_var_fallback(monkeypatch): assert OTHER_CHANNEL_ID in result +def test_allowed_private_channel_prefixes_env_var_fallback(monkeypatch): + monkeypatch.setenv("SLACK_ALLOWED_PRIVATE_CHANNEL_PREFIXES", "b-,project-") + adapter = _make_adapter() + assert adapter._slack_allowed_private_channel_prefixes() == {"b-", "project-"} + + +@pytest.mark.asyncio +async def test_exact_channel_allowlist_wins_without_metadata_lookup(): + adapter = _make_adapter( + allowed_channels=[CHANNEL_ID], + allowed_private_channel_prefixes=["b-"], + ) + adapter._resolve_channel_info = AsyncMock() + + assert await adapter._slack_channel_is_allowed(CHANNEL_ID, team_id="T1") is True + adapter._resolve_channel_info.assert_not_awaited() + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + ("channel", "expected"), + [ + ({"name": "b-project-alpha", "is_private": True, "is_member": True}, True), + ({"name": "b-project-alpha", "is_private": False, "is_member": True}, False), + ({"name": "b-project-alpha", "is_private": True, "is_member": False}, False), + ({"name": "other-project", "is_private": True, "is_member": True}, False), + ({"name": "b-project-alpha", "is_private": True, "is_member": True, "is_mpim": True}, False), + (None, False), + ], +) +async def test_private_channel_prefix_allowlist_fails_closed(channel, expected): + adapter = _make_adapter(allowed_private_channel_prefixes=["b-"]) + adapter._resolve_channel_info = AsyncMock(return_value=channel) + + assert await adapter._slack_channel_is_allowed(OTHER_CHANNEL_ID, team_id="T1") is expected + + +@pytest.mark.asyncio +async def test_no_channel_allowlist_keeps_backward_compatible_open_behavior(): + adapter = _make_adapter() + adapter._resolve_channel_info = AsyncMock() + + assert await adapter._slack_channel_is_allowed(OTHER_CHANNEL_ID, team_id="T1") is True + adapter._resolve_channel_info.assert_not_awaited() + + # --------------------------------------------------------------------------- # Tests: allowed_channels gating integration # --------------------------------------------------------------------------- diff --git a/tests/gateway/test_slack_sdk_response.py b/tests/gateway/test_slack_sdk_response.py index 96b0dd9c9999..3a9ec23c8a04 100644 --- a/tests/gateway/test_slack_sdk_response.py +++ b/tests/gateway/test_slack_sdk_response.py @@ -113,9 +113,11 @@ def _make_adapter(): adapter._user_name_cache = {} adapter._user_is_bot_cache = {} adapter._channel_name_cache = {} + adapter._channel_info_cache = {} adapter._channel_team = {} adapter._USER_NAME_CACHE_MAX = 5000 adapter._CHANNEL_NAME_CACHE_MAX = 5000 + adapter._CHANNEL_INFO_CACHE_MAX = 5000 return adapter @@ -183,6 +185,20 @@ def test_channel_name_resolves(self, make_response): adapter._get_client = lambda *_a, **_kw: client assert asyncio.run(adapter._resolve_channel_name("C_GEN")) == "general" + @response_shape + def test_channel_info_resolves_and_is_cached(self, make_response): + adapter = _make_adapter() + client = MagicMock() + channel = {"name": "b-project-alpha", "is_private": True, "is_member": True} + client.conversations_info = AsyncMock( + return_value=make_response({"ok": True, "channel": channel}) + ) + adapter._get_client = lambda *_a, **_kw: client + + assert asyncio.run(adapter._resolve_channel_info("C_PROJECT")) == channel + assert asyncio.run(adapter._resolve_channel_info("C_PROJECT")) == channel + client.conversations_info.assert_awaited_once() + def test_unknown_shape_still_falls_back_to_the_id(self): """Degradation for genuinely unreadable responses must be preserved.""" adapter = _make_adapter() diff --git a/tests/test_slack_thread_require_mention.py b/tests/test_slack_thread_require_mention.py index 30bcd6a7d78c..31e3e1dec27d 100644 --- a/tests/test_slack_thread_require_mention.py +++ b/tests/test_slack_thread_require_mention.py @@ -1,5 +1,6 @@ import asyncio import os +from unittest.mock import AsyncMock from gateway.config import PlatformConfig from plugins.platforms.slack.adapter import SlackAdapter, _apply_yaml_config @@ -59,6 +60,48 @@ def test_thread_require_mention_env_bridge(monkeypatch): assert os.environ["SLACK_THREAD_REQUIRE_MENTION"] == "true" +def test_allowed_private_channel_prefixes_env_bridge(monkeypatch): + monkeypatch.delenv("SLACK_ALLOWED_PRIVATE_CHANNEL_PREFIXES", raising=False) + + _apply_yaml_config( + {}, + { + "allowed_private_channel_prefixes": ["b-", "project-"], + }, + ) + + assert os.environ["SLACK_ALLOWED_PRIVATE_CHANNEL_PREFIXES"] == "b-,project-" + + +def test_private_prefix_channel_still_requires_explicit_mention(): + adapter = make_adapter( + { + "allowed_private_channel_prefixes": ["b-"], + "require_mention": True, + "reply_in_thread": True, + } + ) + adapter._resolve_channel_info = AsyncMock( + return_value={"name": "b-project-alpha", "is_private": True, "is_member": True} + ) + handled = [] + + async def capture(event): + handled.append(event) + + adapter.handle_message = capture + + run(adapter._handle_slack_message(slack_event("unaddressed request"))) + assert handled == [] + + run( + adapter._handle_slack_message( + slack_event("<@UBOT> addressed request", ts="101.000") + ) + ) + assert len(handled) == 1 + + def test_thread_require_mention_parses_yaml_and_env(monkeypatch): monkeypatch.setenv("SLACK_THREAD_REQUIRE_MENTION", "true") diff --git a/website/docs/reference/environment-variables.md b/website/docs/reference/environment-variables.md index 8bdeb8b7369f..21bb9511bbcd 100644 --- a/website/docs/reference/environment-variables.md +++ b/website/docs/reference/environment-variables.md @@ -345,6 +345,8 @@ These are set automatically by the Docker terminal backend when `proxy.enabled: | `SLACK_ALLOWED_USERS` | Comma-separated Slack user IDs | | `SLACK_ALLOW_ALL_USERS` | Allow any Slack user to trigger the bot (dev only). | | `SLACK_ALLOW_BOTS` | Accept messages from other Slack bots: `none` (default), `mentions`, or `all`. The bot always ignores its own messages. | +| `SLACK_ALLOWED_CHANNELS` | Comma-separated Slack channel IDs. When set, a non-DM conversation must match an exact ID or an allowed private-channel prefix. | +| `SLACK_ALLOWED_PRIVATE_CHANNEL_PREFIXES` | Comma-separated channel-name prefixes. Matches only private channels where Slack confirms the bot is a member; failures deny access. | | `SLACK_THREAD_REQUIRE_MENTION` | Require an explicit @mention for Slack thread replies while preserving top-level free-response channels | | `SLACK_HOME_CHANNEL` | Default Slack channel for cron delivery | | `SLACK_HOME_CHANNEL_NAME` | Display name for the Slack home channel | diff --git a/website/docs/user-guide/messaging/slack.md b/website/docs/user-guide/messaging/slack.md index 544ed727e914..fb7e42dcbbea 100644 --- a/website/docs/user-guide/messaging/slack.md +++ b/website/docs/user-guide/messaging/slack.md @@ -766,7 +766,7 @@ Failure buckets: If this target passes but a live workspace still misroutes messages, investigate Slack token/workspace connectivity and runtime deployment state outside the routing logic itself. -### Channel allowlist (`allowed_channels`) +### Channel allowlist Restrict the bot to a fixed set of Slack channels — useful when the bot is invited to many channels but should only respond in a few. When set, messages from channels NOT in this list are **silently ignored**, even if the bot is `@mentioned`. @@ -785,10 +785,33 @@ Or via env var (comma-separated): SLACK_ALLOWED_CHANNELS="C0123456789,C0987654321" ``` +To admit newly created private project channels without registering each +channel ID, add one or more channel-name prefixes: + +```yaml +slack: + allowed_channels: + - "C0123456789" + allowed_private_channel_prefixes: + - "b-" +``` + +Or via env var: + +```bash +SLACK_ALLOWED_PRIVATE_CHANNEL_PREFIXES="b-" +``` + +A prefix match is accepted only when Slack confirms that the conversation is +a private channel and the bot is currently a member. Public channels, group +DMs, non-member channels, missing metadata, and Slack API failures are denied. +Exact channel IDs and private-channel prefixes form a union, so explicit IDs +remain useful for exceptions that do not follow the naming convention. + Behavior: -- Empty / unset → no restriction (fully backward compatible). -- Non-empty → channel ID must be on the list, or the message is dropped before any other gating (mention requirement, `free_response_channels`, etc.) runs. +- Both settings empty / unset → no restriction (fully backward compatible). +- Either setting non-empty → the channel must match an exact ID or a verified private-channel prefix, or the message is dropped before any other gating (mention requirement, `free_response_channels`, etc.) runs. - Slack channel IDs start with `C` (public), `G` (private), or `D` (DM). Look them up via the Slack UI's "Open channel details" → "About" panel, or via the API. See also: [admin/user slash command split](../../reference/slash-commands.md#permissions-and-adminuser-split).