diff --git a/gateway/platforms/slack.py b/gateway/platforms/slack.py index 5accfdb41089..4a496bc5d44e 100644 --- a/gateway/platforms/slack.py +++ b/gateway/platforms/slack.py @@ -310,6 +310,7 @@ class SlackAdapter(BasePlatformAdapter): self._handler: Optional[Any] = None self._bot_user_id: Optional[str] = None self._user_name_cache: Dict[str, str] = {} # user_id → display name + self._user_is_bot_cache: Dict[str, bool] = {} # user_id → Slack bot user? self._socket_mode_task: Optional[asyncio.Task] = None # Multi-workspace support self._team_clients: Dict[str, Any] = {} # team_id → WebClient @@ -1377,12 +1378,23 @@ class SlackAdapter(BasePlatformAdapter): or user_id ) self._user_name_cache[user_id] = name + self._user_is_bot_cache[user_id] = bool(user.get("is_bot")) return name except Exception as e: logger.debug("[Slack] users.info failed for %s: %s", user_id, e) self._user_name_cache[user_id] = user_id return user_id + async def _is_known_bot_user(self, user_id: str, chat_id: str = "") -> bool: + if not user_id: + return False + if user_id == self._bot_user_id: + return True + if user_id in self._user_is_bot_cache: + return self._user_is_bot_cache[user_id] + await self._resolve_user_name(user_id, chat_id=chat_id) + return self._user_is_bot_cache.get(user_id, False) + async def send_image_file( self, chat_id: str, @@ -1771,11 +1783,28 @@ class SlackAdapter(BasePlatformAdapter): if event_ts and self._dedup.is_duplicate(event_ts): return - # Bot message filtering (SLACK_ALLOW_BOTS / config allow_bots): - # "none" — ignore all bot messages (default, backward-compatible) - # "mentions" — accept bot messages only when they @mention us - # "all" — accept all bot messages (except our own) - if event.get("bot_id") or event.get("subtype") == "bot_message": + # Bot/app-authored message filtering (SLACK_ALLOW_BOTS / config + # allow_bots): + # "none" — ignore all bot/app-authored messages (default, + # backward-compatible) + # "mentions" — accept bot/app-authored messages only when they + # @mention us + # "all" — accept all bot/app-authored messages (except our own) + # + # Some Slack app-originated events arrive without subtype=bot_message or + # bot_id, but they still carry app_id and no client_msg_id. Treat those + # as bot-authored too so sibling Hermes bots do not leak through to the + # generic unauthorized-user path. + client_msg_id = event.get("client_msg_id") + is_app_authored = bool(event.get("app_id")) and not bool(client_msg_id) + msg_user = event.get("user", "") + # Only probe users.info for suspicious unlabeled events. Real human-authored + # Slack messages normally carry client_msg_id; bot/app-originated events that + # slip past bot_id/subtype markers often do not. + is_known_bot_user = False + if msg_user and not client_msg_id: + is_known_bot_user = await self._is_known_bot_user(msg_user, chat_id=event.get("channel", "")) + if event.get("bot_id") or event.get("subtype") == "bot_message" or is_app_authored or is_known_bot_user: allow_bots = self.config.extra.get("allow_bots", "") if not allow_bots: allow_bots = os.getenv("SLACK_ALLOW_BOTS", "none") @@ -1788,7 +1817,6 @@ class SlackAdapter(BasePlatformAdapter): return # "all" falls through to process the message # Always ignore our own messages to prevent echo loops - msg_user = event.get("user", "") if msg_user and self._bot_user_id and msg_user == self._bot_user_id: return diff --git a/tests/gateway/test_slack.py b/tests/gateway/test_slack.py index bc09279eec4e..e8bfd24d367d 100644 --- a/tests/gateway/test_slack.py +++ b/tests/gateway/test_slack.py @@ -19,8 +19,6 @@ from gateway.config import Platform, PlatformConfig from gateway.platforms.base import ( MessageEvent, MessageType, - SendResult, - SUPPORTED_DOCUMENT_TYPES, is_host_excluded_by_no_proxy, ) @@ -72,11 +70,20 @@ from gateway.platforms.slack import SlackAdapter # noqa: E402 @pytest.fixture() def adapter(): - config = PlatformConfig(enabled=True, token="xoxb-fake-token") + config = PlatformConfig(enabled=True, token="***") a = SlackAdapter(config) # Mock the Slack app client a._app = MagicMock() a._app.client = AsyncMock() + a._app.client.users_info = AsyncMock( + return_value={ + "user": { + "is_bot": False, + "profile": {"display_name": "Test User"}, + "real_name": "Test User", + } + } + ) a._bot_user_id = "U_BOT" a._running = True # Capture events instead of processing them @@ -1233,6 +1240,54 @@ class TestMessageRouting: await adapter._handle_slack_message(event) adapter.handle_message.assert_not_called() + @pytest.mark.asyncio + async def test_app_authored_messages_without_client_msg_id_are_ignored(self, adapter): + """Slack app-authored events can arrive without bot_id/subtype markers.""" + adapter._app.client.users_info = AsyncMock( + return_value={ + "user": { + "is_bot": False, + "profile": {"display_name": "helper-app"}, + "real_name": "Helper App", + } + } + ) + event = { + "text": "workflow reply", + "app_id": "A_HELPER", + "user": "U_APP_HELPER", + "channel": "C123", + "channel_type": "im", + "ts": "1234567890.000002", + } + await adapter._handle_slack_message(event) + adapter._app.client.users_info.assert_awaited_once_with(user="U_APP_HELPER") + adapter.handle_message.assert_not_called() + + @pytest.mark.asyncio + async def test_known_bot_users_ignored_even_without_bot_markers(self, adapter): + """users.info bot identities should still route through bot filtering.""" + adapter._app.client.users_info = AsyncMock( + return_value={ + "user": { + "is_bot": True, + "profile": {"display_name": "helper-bot"}, + "real_name": "Helper Bot", + } + } + ) + event = { + "text": "helper response", + "user": "U_HELPER_BOT", + "channel": "C123", + "channel_type": "im", + "ts": "1234567890.000003", + } + await adapter._handle_slack_message(event) + adapter._app.client.users_info.assert_awaited_once_with(user="U_HELPER_BOT") + assert adapter._user_is_bot_cache["U_HELPER_BOT"] is True + adapter.handle_message.assert_not_called() + @pytest.mark.asyncio async def test_message_edits_ignored(self, adapter): """Message edits should be ignored.""" @@ -1834,8 +1889,7 @@ class TestReactions: assert "1234567890.000001" in adapter._reacting_message_ids # Simulate the base class calling on_processing_start - from gateway.platforms.base import MessageEvent, MessageType, SessionSource - from gateway.config import Platform + from gateway.platforms.base import MessageType, SessionSource source = SessionSource( platform=Platform.SLACK, chat_id="C123", @@ -1874,8 +1928,7 @@ class TestReactions: adapter._app.client.reactions_add = AsyncMock() adapter._app.client.reactions_remove = AsyncMock() - from gateway.platforms.base import MessageEvent, MessageType, SessionSource, ProcessingOutcome - from gateway.config import Platform + from gateway.platforms.base import MessageType, SessionSource, ProcessingOutcome source = SessionSource( platform=Platform.SLACK, chat_id="C123", @@ -1944,8 +1997,7 @@ class TestReactions: assert "1234567890.000004" not in adapter._reacting_message_ids # Hooks should also be no-ops when disabled - from gateway.platforms.base import MessageEvent, MessageType, SessionSource, ProcessingOutcome - from gateway.config import Platform + from gateway.platforms.base import MessageType, SessionSource, ProcessingOutcome source = SessionSource( platform=Platform.SLACK, chat_id="C123", @@ -1997,6 +2049,15 @@ class TestThreadReplyHandling: a = SlackAdapter(config) a._app = MagicMock() a._app.client = AsyncMock() + a._app.client.users_info = AsyncMock( + return_value={ + "user": { + "is_bot": False, + "profile": {"display_name": "Test User"}, + "real_name": "Test User", + } + } + ) a._bot_user_id = "U_BOT" a._team_bot_user_ids = {"T_TEAM": "U_BOT"} a._running = True @@ -2137,6 +2198,15 @@ class TestAssistantThreadLifecycle: a = SlackAdapter(config) a._app = MagicMock() a._app.client = AsyncMock() + a._app.client.users_info = AsyncMock( + return_value={ + "user": { + "is_bot": False, + "profile": {"display_name": "Test User"}, + "real_name": "Test User", + } + } + ) a._bot_user_id = "U_BOT" a._team_bot_user_ids = {"T_TEAM": "U_BOT"} a._running = True