fix(slack): treat allowlisted users' Web-API (user-token) posts as human
Posts made with a user token (xoxp-) arrive with app_id and no client_msg_id, so _event_declares_bot_sender dropped them as app traffic; the only workaround was allow_bots: all. Adds platforms.slack.extra.api_human_users (SLACK_API_HUMAN_USERS fallback), a users-only allowlist consulted inside the predicate. Salvaged from #100964 (users only: an app-id allowlist would also admit the app's own xoxb bot posts, which share the user+app_id shape).
This commit is contained in:
xoy8n
@@ -1320,6 +1320,10 @@ platform_toolsets:
|
||||
# # Render live tool calls as Slack-native plan/task cards. This explicit
|
||||
# # opt-in works even though Slack text tool_progress defaults to off.
|
||||
# native_task_cards: false
|
||||
# # Slack user IDs whose Web-API posts (user token, e.g. your own
|
||||
# # dashboard/mobile front-end) count as human instead of being dropped
|
||||
# # as app traffic. Narrower than allow_bots: all. Users only — never apps.
|
||||
# api_human_users: ["U0AAAAAAA", "U0BBBBBBB"]
|
||||
# # Suppress automatic link-preview cards without removing clickable links.
|
||||
# # Omit either key to preserve Slack's default for that preview type.
|
||||
# unfurl_links: false
|
||||
|
||||
@@ -3839,6 +3839,30 @@ class SlackAdapter(BasePlatformAdapter):
|
||||
return "none"
|
||||
return value
|
||||
|
||||
def _slack_api_human_users(self) -> frozenset:
|
||||
"""Slack user IDs whose Web-API posts count as human-authored.
|
||||
|
||||
A message posted with a *user* token (``xoxp-``) is authored by a real
|
||||
person, but Slack still stamps it with the posting ``app_id`` and it
|
||||
carries no ``client_msg_id`` — exactly the #35777 app/bot signature in
|
||||
``_event_declares_bot_sender``. Operators running their own front-end
|
||||
(dashboard, mobile shell) allowlist those *users* via
|
||||
``platforms.slack.extra.api_human_users`` (``SLACK_API_HUMAN_USERS``
|
||||
fallback) instead of ``allow_bots: all``. Users only — an app-id
|
||||
allowlist would also admit the app's own ``xoxb`` bot posts, which
|
||||
carry the same user+app_id shape.
|
||||
"""
|
||||
cached = getattr(self, "_api_human_users_cache", None)
|
||||
if cached is None:
|
||||
raw = self.config.extra.get("api_human_users")
|
||||
if raw is None:
|
||||
raw = os.getenv("SLACK_API_HUMAN_USERS", "")
|
||||
parts = raw if isinstance(raw, (list, tuple, set)) else str(raw).split(",")
|
||||
cached = self._api_human_users_cache = frozenset(
|
||||
str(p).strip() for p in parts if str(p).strip()
|
||||
)
|
||||
return cached
|
||||
|
||||
def _event_declares_bot_sender(self, event: dict) -> bool:
|
||||
"""Return True when the Slack event itself identifies a bot sender."""
|
||||
if event.get("bot_id") or event.get("bot_profile"):
|
||||
@@ -3853,7 +3877,11 @@ class SlackAdapter(BasePlatformAdapter):
|
||||
# human-authored messages normally carry client_msg_id, so treat the
|
||||
# combination as app/bot-authored (#35777).
|
||||
if event.get("app_id") and not event.get("client_msg_id"):
|
||||
return True
|
||||
# ...unless the operator allowlisted this user's API posts
|
||||
# (_slack_api_human_users). ``user`` is required so classic bot
|
||||
# posts (no ``user``) never match; bot_message/bot_id already
|
||||
# returned True above.
|
||||
return event.get("user") not in self._slack_api_human_users()
|
||||
return False
|
||||
|
||||
def _resolve_thread_ts(
|
||||
|
||||
@@ -0,0 +1,94 @@
|
||||
"""Tests for the Slack ``api_human_users`` allowlist.
|
||||
|
||||
A message posted through the Web API with a *user* token (``xoxp-``) is
|
||||
authored by a real person, but it arrives with the posting ``app_id`` and no
|
||||
``client_msg_id`` — the #35777 app/bot signature — so
|
||||
``_event_declares_bot_sender`` drops it. ``platforms.slack.extra.api_human_users``
|
||||
allowlists those *users* (never apps: an app's own ``xoxb`` bot posts carry
|
||||
the same user+app_id shape).
|
||||
"""
|
||||
|
||||
import sys
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
# Mock slack-bolt / slack-sdk the same way test_slack_mention.py does.
|
||||
def _ensure_slack_mock():
|
||||
if "slack_bolt" in sys.modules and hasattr(sys.modules["slack_bolt"], "__file__"):
|
||||
return
|
||||
slack_bolt = MagicMock()
|
||||
slack_bolt.async_app.AsyncApp = MagicMock
|
||||
slack_bolt.adapter.socket_mode.async_handler.AsyncSocketModeHandler = MagicMock
|
||||
slack_sdk = MagicMock()
|
||||
slack_sdk.web.async_client.AsyncWebClient = MagicMock
|
||||
for name, mod in [
|
||||
("slack_bolt", slack_bolt),
|
||||
("slack_bolt.async_app", slack_bolt.async_app),
|
||||
("slack_bolt.adapter", slack_bolt.adapter),
|
||||
("slack_bolt.adapter.socket_mode", slack_bolt.adapter.socket_mode),
|
||||
(
|
||||
"slack_bolt.adapter.socket_mode.async_handler",
|
||||
slack_bolt.adapter.socket_mode.async_handler,
|
||||
),
|
||||
("slack_sdk", slack_sdk),
|
||||
("slack_sdk.web", slack_sdk.web),
|
||||
("slack_sdk.web.async_client", slack_sdk.web.async_client),
|
||||
]:
|
||||
sys.modules.setdefault(name, mod)
|
||||
sys.modules.setdefault("aiohttp", MagicMock())
|
||||
|
||||
|
||||
_ensure_slack_mock()
|
||||
|
||||
import plugins.platforms.slack.adapter as _slack_mod # noqa: E402
|
||||
|
||||
_slack_mod.SLACK_AVAILABLE = True
|
||||
|
||||
from plugins.platforms.slack.adapter import SlackAdapter # noqa: E402
|
||||
|
||||
from gateway.config import Platform, PlatformConfig # noqa: E402
|
||||
|
||||
|
||||
HUMAN_ID = "U_human"
|
||||
|
||||
|
||||
def _make_adapter(extra=None):
|
||||
adapter = object.__new__(SlackAdapter)
|
||||
adapter.platform = Platform.SLACK
|
||||
adapter.config = PlatformConfig(enabled=True, extra=dict(extra or {}))
|
||||
return adapter
|
||||
|
||||
|
||||
def _api_post(**overrides):
|
||||
"""A user-token chat.postMessage as delivered over Socket Mode:
|
||||
real ``user``, app_id stamp, no ``client_msg_id``."""
|
||||
event = {"type": "message", "user": HUMAN_ID, "app_id": "A_frontend", "text": "hi"}
|
||||
event.update(overrides)
|
||||
return event
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _clean_env(monkeypatch):
|
||||
monkeypatch.delenv("SLACK_API_HUMAN_USERS", raising=False)
|
||||
|
||||
|
||||
def test_api_post_is_bot_by_default():
|
||||
assert _make_adapter()._event_declares_bot_sender(_api_post()) is True
|
||||
|
||||
|
||||
def test_allowlisted_user_api_post_is_human():
|
||||
adapter = _make_adapter({"api_human_users": ["U_other", HUMAN_ID]})
|
||||
assert adapter._event_declares_bot_sender(_api_post()) is False
|
||||
# Same predicate everywhere: no other user, and no user-less app post, rides it.
|
||||
assert adapter._event_declares_bot_sender(_api_post(user="U_stranger")) is True
|
||||
assert adapter._event_declares_bot_sender({"app_id": "A_frontend", "text": "hi"}) is True
|
||||
|
||||
|
||||
def test_bot_markers_win_over_allowlist():
|
||||
"""Allowlisting a user never admits genuine bot posts, so the app's own
|
||||
``xoxb`` traffic (bot_id / subtype=bot_message) cannot loop back in."""
|
||||
adapter = _make_adapter({"api_human_users": HUMAN_ID})
|
||||
assert adapter._event_declares_bot_sender(_api_post(subtype="bot_message")) is True
|
||||
assert adapter._event_declares_bot_sender(_api_post(bot_id="B_stamp")) is True
|
||||
@@ -472,6 +472,7 @@ platforms:
|
||||
| `platforms.slack.extra.suggested_prompts` | `[]` | Up to four `{title, message}` prompts for Agent/Assistant DM entry points; accepts either a list or `{title, prompts}`. |
|
||||
| `platforms.slack.extra.assistant_thread_titles` | `true` | When `true`, names Agent/Assistant DM threads from the first user message. |
|
||||
| `platforms.slack.extra.allow_bots` | `"none"` | Controls messages from other Slack bots: `"none"` ignores them, `"mentions"` accepts a bot message only when **that message itself** @mentions Hermes, and `"all"` accepts all of them. Use `"mentions"` for the safest bot-to-bot collaboration mode. See [Accepting messages from other bots](#accepting-messages-from-other-bots-allow_bots). |
|
||||
| `platforms.slack.extra.api_human_users` | `[]` | Slack user IDs whose **Web-API (user-token) posts count as human**. Such posts carry the posting `app_id` and no `client_msg_id`, so by default they are dropped as app traffic; allowlist your own front-end's users here instead of `allow_bots: all`. See [Treating your own app's user-token posts as human](#treating-your-own-apps-user-token-posts-as-human-api_human_users). |
|
||||
| `platforms.slack.extra.cron_continuable_surface` | `"thread"` | Delivery surface for [continuable cron jobs](../features/cron.md#flat-in-channel-continuation-slack). `"thread"` opens a dedicated thread per delivery (default); `"in_channel"` delivers flat into the channel timeline. Pair `in_channel` with `reply_in_thread: false` (and `require_mention: false`) so a plain channel reply continues the job. |
|
||||
|
||||
The equivalent environment variable is `SLACK_ALLOW_BOTS=none|mentions|all`.
|
||||
@@ -701,6 +702,38 @@ How `mentions` mode gates:
|
||||
|
||||
For strict multi-bot deployments, pair with `require_mention: true` and `strict_mention: true` — see the smoke-check profile below.
|
||||
|
||||
### Treating your own app's user-token posts as human (`api_human_users`)
|
||||
|
||||
A message posted through the Web API with a **user token** (`xoxp-`) is
|
||||
authored by a real person, but it arrives with the posting `app_id` and no
|
||||
`client_msg_id` — the same signature Hermes uses to recognise app posts — so it
|
||||
is dropped as bot traffic. This blocks a common pattern: a custom front-end (an
|
||||
internal dashboard, a mobile shell, a kiosk) that sends messages to Hermes *as*
|
||||
the logged-in user.
|
||||
|
||||
`allow_bots: all` would let those posts through, but it opens the door to every
|
||||
bot in the channel and weakens the loop protections. Instead, allowlist just
|
||||
the people who use your front-end:
|
||||
|
||||
```yaml
|
||||
platforms:
|
||||
slack:
|
||||
extra:
|
||||
api_human_users: ["U0AAAAAAA", "U0BBBBBBB"]
|
||||
```
|
||||
|
||||
The equivalent environment variable is `SLACK_API_HUMAN_USERS` (comma-separated).
|
||||
|
||||
Scope and safety:
|
||||
|
||||
- The allowlist is **users only**. There is deliberately no app-ID variant: a
|
||||
modern bot token (`xoxb-`) posts with the same `user` + `app_id` shape, so
|
||||
trusting an app would also admit its own bot posts and defeat the loop guard.
|
||||
- Events carrying `bot_id` or `subtype: bot_message`, or no `user` at all, are
|
||||
always treated as bot posts regardless of the allowlist.
|
||||
- The rest of the pipeline is unchanged: mention gating, `allowed_channels`,
|
||||
and `SLACK_ALLOWED_USERS` still apply to the (now human) sender.
|
||||
|
||||
### Reaction Triggers (`reaction_triggers`)
|
||||
|
||||
By default, emoji reactions are acknowledged and dropped — a 👍 on a bot
|
||||
|
||||
Reference in New Issue
Block a user