diff --git a/gateway/run.py b/gateway/run.py index cee6256cefd6..9eeb04546b5c 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -941,6 +941,48 @@ def _uses_telegram_observed_group_context(channel_prompt: Optional[str]) -> bool return bool(channel_prompt and _TELEGRAM_OBSERVED_CONTEXT_PROMPT_MARKER in channel_prompt) +def _csv_or_list_to_set(raw: Any) -> set[str]: + """Normalize a config list or comma-separated scalar into a string set.""" + if raw is None: + return set() + if isinstance(raw, list): + return {str(part).strip() for part in raw if str(part).strip()} + s = str(raw).strip() + if not s: + return set() + return {part.strip() for part in s.split(",") if part.strip()} + + +def _slack_ignored_channels_from_gateway_config(config: Any) -> set[str]: + """Return Slack channels that the generic gateway must never dispatch. + + The Slack adapter has the first-line drop, but this runner-level guard is + intentionally duplicated as a fail-safe. If a future Slack code path, test + hook, malformed event, or stale adapter instance bypasses the Slack plugin + adapter, ignored channels still cannot reach auth, pairing, sessions, or + the agent/home-channel prompt pipeline. + """ + platform_cfg = getattr(config, "platforms", {}).get(Platform.SLACK) + raw = None + if platform_cfg is not None: + raw = getattr(platform_cfg, "extra", {}).get("ignored_channels") + return _csv_or_list_to_set(raw) + + +def _slack_parent_channel_id(chat_id: Any) -> str: + """Return the parent Slack channel from a possibly thread-scoped chat ID.""" + if not chat_id: + return "" + return str(chat_id).split(":", 1)[0] + + +def _is_slack_ignored_channel(config: Any, chat_id: Any) -> bool: + """Check the generic Slack gateway blacklist for channel or thread IDs.""" + channel_id = _slack_parent_channel_id(chat_id) + ignored = _slack_ignored_channels_from_gateway_config(config) + return bool(channel_id and ("*" in ignored or channel_id in ignored)) + + def _message_timestamps_enabled(user_config: Optional[dict]) -> bool: """True when gateway.message_timestamps.enabled is opted in. @@ -10191,6 +10233,17 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew return config = getattr(self, "config", None) + if ( + config + and getattr(source, "platform", None) == Platform.SLACK + and _is_slack_ignored_channel(config, getattr(source, "chat_id", None)) + ): + logger.info( + "Skipping Slack platform notice for configured ignored channel %s", + getattr(source, "chat_id", None), + ) + return + notice_delivery = "public" if config and hasattr(config, "get_notice_delivery"): notice_delivery = config.get_notice_delivery(source.platform) @@ -10397,18 +10450,32 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew except Exception: logger.debug("reset_session_vars failed at handler entry", exc_info=True) + # Internal events (e.g. background-process completion notifications) + # are system-generated and must skip user authorization. + is_internal = bool(getattr(event, "internal", False)) + + # Ignored-channel guard runs FIRST — before startup-restore queueing, + # plugin hooks, auth, and session setup — so a configured ignored + # channel can never reach pairing/auth/session state (#51899). + if ( + not is_internal + and getattr(source, "platform", None) == Platform.SLACK + and _is_slack_ignored_channel(self.config, getattr(source, "chat_id", None)) + ): + logger.info( + "Dropping Slack message from configured ignored channel %s", + getattr(source, "chat_id", None), + ) + return None + if ( getattr(self, "_startup_restore_in_progress", False) - and not getattr(event, "internal", False) + and not is_internal and not getattr(event, "_hermes_startup_restore_replay", False) ): self._queue_startup_restore_event(event) return None - # Internal events (e.g. background-process completion notifications) - # are system-generated and must skip user authorization. - is_internal = bool(getattr(event, "internal", False)) - # scale-to-zero (Phase 0, 0.B/F13): stamp the gateway-scoped last-inbound # clock for real (user-originated) inbound only. Internal/system events # (background-process completions, startup-restore replays) are NOT diff --git a/plugins/platforms/slack/adapter.py b/plugins/platforms/slack/adapter.py index 8bc0f1594912..02c3ae27cf81 100644 --- a/plugins/platforms/slack/adapter.py +++ b/plugins/platforms/slack/adapter.py @@ -2389,6 +2389,31 @@ class SlackAdapter(BasePlatformAdapter): except Exception as e: # pragma: no cover - defensive cleanup logger.debug("[Slack] status cleanup failed: %s", e) + def _slack_ignored_channels(self) -> set[str]: + """Configured Slack channels the generic gateway must never touch.""" + raw = self.config.extra.get("ignored_channels") + if raw is None: + raw = os.getenv("SLACK_IGNORED_CHANNELS") + if raw is None: + return set() + if isinstance(raw, list): + return {str(part).strip() for part in raw if str(part).strip()} + return {part.strip() for part in str(raw).split(",") if part.strip()} + + def _is_ignored_channel(self, channel_id: str) -> bool: + """Return True when generic Slack gateway must stay silent here. + + Most Slack call sites pass the parent channel ID directly, but some + gateway/session paths carry a thread-scoped identifier like + ``C123:1712345678.000001``. Ignored-channel matching is channel-level, + so normalize defensively before checking the configured blacklist. + """ + if not channel_id: + return False + parent_channel_id = str(channel_id).split(":", 1)[0] + ignored = self._slack_ignored_channels() + return "*" in ignored or parent_channel_id in ignored + async def send( self, chat_id: str, @@ -2397,6 +2422,12 @@ class SlackAdapter(BasePlatformAdapter): metadata: Optional[Dict[str, Any]] = None, ) -> SendResult: """Send a message to a Slack channel or DM.""" + if self._is_ignored_channel(chat_id): + logger.warning( + "[Slack] Suppressed outbound generic send to configured ignored channel %s", + chat_id, + ) + return SendResult(success=False, error="ignored_channel") if not self._app: return SendResult(success=False, error="Not connected") @@ -2576,6 +2607,12 @@ class SlackAdapter(BasePlatformAdapter): metadata: Optional[Dict[str, Any]] = None, ) -> SendResult: """Send a Slack ephemeral message visible only to one user.""" + if self._is_ignored_channel(chat_id): + logger.warning( + "[Slack] Suppressed outbound generic ephemeral notice to configured ignored channel %s", + chat_id, + ) + return SendResult(success=False, error="ignored_channel") if not self._app: return SendResult(success=False, error="Not connected") if not chat_id or not user_id: @@ -2658,6 +2695,12 @@ class SlackAdapter(BasePlatformAdapter): metadata: Optional[Dict[str, Any]] = None, ) -> SendResult: """Edit a previously sent Slack message.""" + if self._is_ignored_channel(chat_id): + logger.warning( + "[Slack] Suppressed message edit in configured ignored channel %s", + chat_id, + ) + return SendResult(success=False, error="ignored_channel") if not self._app: return SendResult(success=False, error="Not connected") try: @@ -2789,6 +2832,9 @@ class SlackAdapter(BasePlatformAdapter): Requires the assistant:write or chat:write scope. Auto-clears when the bot sends a reply to the thread. """ + if self._is_ignored_channel(chat_id): + logger.debug("[Slack] Suppressed typing/status in configured ignored channel %s", chat_id) + return if not self._app: return @@ -2877,6 +2923,10 @@ class SlackAdapter(BasePlatformAdapter): async def stop_typing(self, chat_id: str, metadata=None) -> None: """Clear the assistant thread status indicator.""" + if self._is_ignored_channel(chat_id): + logger.debug("[Slack] Suppressed status clear in configured ignored channel %s", chat_id) + self._active_status_threads.pop(chat_id, None) + return if not self._app: return requested_thread_ts = "" @@ -3095,6 +3145,12 @@ class SlackAdapter(BasePlatformAdapter): metadata: Optional[Dict[str, Any]] = None, ) -> SendResult: """Upload a local file to Slack.""" + if self._is_ignored_channel(chat_id): + logger.warning( + "[Slack] Suppressed file upload in configured ignored channel %s", + chat_id, + ) + return SendResult(success=False, error="ignored_channel") if not self._app: return SendResult(success=False, error="Not connected") @@ -3149,6 +3205,12 @@ class SlackAdapter(BasePlatformAdapter): The batch limit is 10 file uploads per call (Slack server-side cap). """ + if self._is_ignored_channel(chat_id): + logger.warning( + "[Slack] Suppressed multi-image upload in configured ignored channel %s", + chat_id, + ) + return if not self._app: return if not images: @@ -4871,6 +4933,9 @@ class SlackAdapter(BasePlatformAdapter): this lifecycle event. """ channel_id = event.get("channel_id") or event.get("channel") or "" + if self._is_ignored_channel(channel_id): + logger.info("[Slack] Ignoring file_shared event in configured ignored channel %s", channel_id) + return file_id = event.get("file_id") or (event.get("file") or {}).get("id") or "" if not channel_id or not file_id: return @@ -5155,6 +5220,11 @@ class SlackAdapter(BasePlatformAdapter): ): return + channel_id = event.get("channel", "") + if self._is_ignored_channel(channel_id): + logger.info("[Slack] Ignoring message in configured ignored channel %s", channel_id) + return + # Bot/app-authored message filtering (SLACK_ALLOW_BOTS / config # allow_bots): # "none" — ignore all bot/app-authored messages (default, diff --git a/tests/gateway/test_slack.py b/tests/gateway/test_slack.py index c196783da6a9..54f56c32a4ec 100644 --- a/tests/gateway/test_slack.py +++ b/tests/gateway/test_slack.py @@ -29,6 +29,7 @@ from gateway.platforms.base import ( MessageType, SendResult, SUPPORTED_VIDEO_TYPES, + SendResult, is_host_excluded_by_no_proxy, ) @@ -107,6 +108,85 @@ def test_slack_mock_bootstrap_preserves_installed_packages(): if PathFinder.find_spec("slack_sdk") is not None: assert isinstance(importlib.import_module("slack_sdk.errors"), ModuleType) +# --------------------------------------------------------------------------- +# TestIgnoredChannelOutboundSuppression +# --------------------------------------------------------------------------- + + +class TestIgnoredChannelOutboundSuppression: + """Ignored Slack channels must be a hard generic-gateway kill switch.""" + + def _ignored_adapter(self): + config = PlatformConfig( + enabled=True, + token="***", + extra={"ignored_channels": ["C_PRD"]}, + ) + adapter = SlackAdapter(config) + adapter._app = MagicMock() + adapter._app.client = AsyncMock() + adapter._bot_user_id = "U_BOT" + adapter._running = True + return adapter + + @pytest.mark.asyncio + async def test_send_suppressed_for_ignored_channel(self): + adapter = self._ignored_adapter() + + result = await adapter.send("C_PRD", "Acknowledged", reply_to="123.456") + + assert result.success is False + assert result.error == "ignored_channel" + adapter._app.client.chat_postMessage.assert_not_awaited() + + @pytest.mark.asyncio + async def test_private_notice_suppressed_for_ignored_channel(self): + adapter = self._ignored_adapter() + + result = await adapter.send_private_notice( + "C_PRD", "U_USER", "No home channel is set", reply_to="123.456" + ) + + assert result.success is False + assert result.error == "ignored_channel" + adapter._app.client.chat_postEphemeral.assert_not_awaited() + + @pytest.mark.asyncio + async def test_edit_status_and_media_paths_suppressed(self, tmp_path): + adapter = self._ignored_adapter() + adapter._active_status_threads["C_PRD"] = "123.456" + file_path = tmp_path / "note.txt" + file_path.write_text("secret") + + edit = await adapter.edit_message("C_PRD", "123.999", "updated", finalize=True) + await adapter.send_typing("C_PRD", {"thread_ts": "123.456"}) + await adapter.stop_typing("C_PRD") + upload = await adapter._upload_file("C_PRD", str(file_path)) + await adapter.send_multiple_images("C_PRD", [("https://example.com/image.png", "alt")]) + + assert edit.success is False + assert upload.success is False + assert edit.error == "ignored_channel" + assert upload.error == "ignored_channel" + adapter._app.client.chat_update.assert_not_awaited() + adapter._app.client.assistant_threads_setStatus.assert_not_awaited() + adapter._app.client.files_upload_v2.assert_not_awaited() + + @pytest.mark.asyncio + async def test_inbound_message_suppressed_for_ignored_channel(self): + adapter = self._ignored_adapter() + adapter.handle_message = AsyncMock() + + await adapter._handle_slack_message({ + "text": "<@U_BOT> review this", + "user": "U_USER", + "channel": "C_PRD", + "channel_type": "channel", + "ts": "123.456", + }) + + adapter.handle_message.assert_not_awaited() + async def _pending_for_fake_task(): # Stay pending so done-callbacks attached by the adapter (which would diff --git a/tests/gateway/test_slack_runner_ignored_channels.py b/tests/gateway/test_slack_runner_ignored_channels.py new file mode 100644 index 000000000000..c32d9bad1783 --- /dev/null +++ b/tests/gateway/test_slack_runner_ignored_channels.py @@ -0,0 +1,116 @@ +import pytest +from unittest.mock import AsyncMock + +from gateway.config import GatewayConfig, Platform, PlatformConfig +from gateway.platforms.base import MessageEvent, MessageType, SendResult +from gateway.run import ( + GatewayRunner, + _is_slack_ignored_channel, + _slack_ignored_channels_from_gateway_config, +) +from gateway.session import SessionSource + + +def _config_with_slack_extra(extra=None): + return GatewayConfig( + platforms={ + Platform.SLACK: PlatformConfig(enabled=True, extra=extra or {}), + } + ) + + +def test_slack_ignored_channels_from_config_list(): + config = _config_with_slack_extra({"ignored_channels": ["C_PRD", " C_OTHER ", ""]}) + + assert _slack_ignored_channels_from_gateway_config(config) == {"C_PRD", "C_OTHER"} + + +def test_slack_ignored_channel_matches_thread_scoped_chat_id(): + config = _config_with_slack_extra({"ignored_channels": "C_PRD"}) + + assert _is_slack_ignored_channel(config, "C_PRD") + assert _is_slack_ignored_channel(config, "C_PRD:1782283787.899249") + + +def test_slack_ignored_channel_supports_wildcard(): + config = _config_with_slack_extra({"ignored_channels": "*"}) + + assert _is_slack_ignored_channel(config, "C_ANY") + + +@pytest.mark.asyncio +async def test_runner_drops_slack_ignored_channel_before_auth_hooks_and_sessions(monkeypatch): + runner = object.__new__(GatewayRunner) + runner.config = _config_with_slack_extra({"ignored_channels": "C_PRD"}) + runner._startup_restore_in_progress = False + + # If the guard regresses, _handle_message will proceed into hooks/auth/session + # setup and one of these sentinels will fail the test. + runner.session_store = object() + monkeypatch.setattr( + "hermes_cli.plugins.invoke_hook", + lambda *args, **kwargs: (_ for _ in ()).throw(AssertionError("hook should not run")), + ) + runner._is_user_authorized = lambda source: (_ for _ in ()).throw(AssertionError("auth should not run")) + + event = MessageEvent( + text="<@U_BOT> review this PRD", + message_type=MessageType.TEXT, + source=SessionSource( + platform=Platform.SLACK, + user_id="U_USER", + user_name="shubham", + chat_id="C_PRD", + chat_type="group", + ), + ) + + assert await runner._handle_message(event) is None + + +@pytest.mark.asyncio +async def test_runner_drops_thread_scoped_slack_ignored_channel(): + runner = object.__new__(GatewayRunner) + runner.config = _config_with_slack_extra({"ignored_channels": "C_PRD"}) + runner._startup_restore_in_progress = False + runner._is_user_authorized = lambda source: (_ for _ in ()).throw(AssertionError("auth should not run")) + runner.session_store = None + + event = MessageEvent( + text="<@U_BOT> review this PRD", + message_type=MessageType.TEXT, + source=SessionSource( + platform=Platform.SLACK, + user_id="U_USER", + user_name="shubham", + chat_id="C_PRD:1782283787.899249", + chat_type="group", + thread_id="1782283787.899249", + ), + ) + + assert await runner._handle_message(event) is None + + +@pytest.mark.asyncio +async def test_platform_notice_suppressed_for_slack_ignored_channel(): + runner = object.__new__(GatewayRunner) + runner.config = _config_with_slack_extra({"ignored_channels": "C_PRD"}) + adapter = type("Adapter", (), {})() + adapter.send = AsyncMock(return_value=SendResult(success=True)) + adapter.send_private_notice = AsyncMock(return_value=SendResult(success=True)) + runner.adapters = {Platform.SLACK: adapter} + runner._thread_metadata_for_source = lambda source: {"thread_id": source.thread_id} + + source = SessionSource( + platform=Platform.SLACK, + user_id="U_USER", + chat_id="C_PRD", + chat_type="group", + thread_id="1782283787.899249", + ) + + await runner._deliver_platform_notice(source, "No home channel is set for Slack") + + adapter.send.assert_not_called() + adapter.send_private_notice.assert_not_called()