mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-23 16:36:23 +00:00
fix(slack): filter unlabeled app and bot authored events
This commit is contained in:
Hafiz Ahmad Ashfaq
parent
f9665d0475
commit
e2e11dab51
2 changed files with 113 additions and 15 deletions
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue