mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-30 11:52:04 +00:00
fix(slack): warn when configured token is a user token, not a bot token
A Slack user/legacy token (xoxp-...) makes auth.test resolve to the installing human's member ID with no bot_id, so the adapter binds its identity (_bot_user_id / _team_bot_user_ids) to that human. Every "is this the bot?" check then misfires: that person's <@...> mentions wake the bot and are stripped as the bot's own mention, so the agent is genuinely told it was @mentioned and replies to messages merely addressed to that human (symptom: bot responds to "@trevor ..." and insists it was explicitly mentioned). There is no runtime API error to catch — a user token still sends/receives — so the only detectable moment is connect time. Add a warning-only nudge (_warn_if_not_bot_token) alongside the existing group-DM scope nudge: when auth.test resolves a user_id but no bot_id, log that the token is a user token and to use the xoxb-... Bot User OAuth Token. Warning-only: does not block a working-but-misconfigured install. Fires once per workspace per process.
This commit is contained in:
parent
33d044c3af
commit
184c10cf97
2 changed files with 185 additions and 0 deletions
|
|
@ -874,6 +874,70 @@ class SlackAdapter(BasePlatformAdapter):
|
|||
except Exception: # pragma: no cover - diagnostics must never break connect
|
||||
pass
|
||||
|
||||
def _warn_if_not_bot_token(self, auth_response, team_name: str) -> None:
|
||||
"""Warn when the configured token authenticates as a human, not a bot.
|
||||
|
||||
``auth.test`` returns the ``user_id`` of *whatever principal owns the
|
||||
token*. For a real bot token (``xoxb-…``) that is the app's bot user
|
||||
and the response carries a ``bot_id``. For a **user** token
|
||||
(``xoxp-…`` / a legacy/personal OAuth token) it is the *installing
|
||||
human's* member ID and there is **no** ``bot_id``.
|
||||
|
||||
When that happens, ``self._bot_user_id`` becomes a human's member ID,
|
||||
and every "is this the bot?" check downstream misfires: that one
|
||||
person's ``<@…>`` mentions wake the bot (``is_mentioned`` in
|
||||
``_handle_slack_message``) and get stripped as if they were the bot's
|
||||
own mention — so the agent is genuinely told it was @mentioned and
|
||||
replies to messages merely *addressed to that human*. There is no
|
||||
runtime API error to catch; the only detectable moment is here at
|
||||
connect time, by noticing ``bot_id`` is absent from ``auth.test``.
|
||||
|
||||
Warning-only: a user token can still send/receive, and we don't want
|
||||
to hard-fail a working-but-misconfigured install on connect. We log
|
||||
exactly what is wrong and how to fix it, once per workspace per
|
||||
process.
|
||||
"""
|
||||
try:
|
||||
warned = getattr(self, "_user_token_warned", None)
|
||||
if warned is None:
|
||||
warned = set()
|
||||
self._user_token_warned = warned
|
||||
team_key = team_name or ""
|
||||
if team_key in warned:
|
||||
return
|
||||
# ``auth.test`` includes ``bot_id`` only for bot tokens. Its
|
||||
# absence (with a resolved user_id) means a user/legacy token.
|
||||
bot_id = ""
|
||||
user_id = ""
|
||||
try:
|
||||
bot_id = auth_response.get("bot_id", "") or ""
|
||||
user_id = auth_response.get("user_id", "") or ""
|
||||
except Exception:
|
||||
# Some response shapes are attribute-only; fall back to .data.
|
||||
data = getattr(auth_response, "data", None) or {}
|
||||
bot_id = data.get("bot_id", "") or ""
|
||||
user_id = data.get("user_id", "") or ""
|
||||
if not user_id:
|
||||
return # Nothing resolved — don't guess.
|
||||
if not bot_id:
|
||||
warned.add(team_key)
|
||||
logger.warning(
|
||||
"[Slack] The configured Slack token for workspace %s "
|
||||
"authenticated as a USER (member %s), not a bot — the "
|
||||
"auth.test response has no 'bot_id'. This is almost "
|
||||
"certainly a user token (xoxp-...) instead of a Bot User "
|
||||
"OAuth Token (xoxb-...). The bot's identity is now bound "
|
||||
"to that member's ID, so mentions OF THAT PERSON will be "
|
||||
"misrouted as mentions of the bot (the bot replies to "
|
||||
"messages merely addressed to them). Use the 'Bot User "
|
||||
"OAuth Token' (xoxb-...) from your Slack app's 'OAuth & "
|
||||
"Permissions' page in SLACK_BOT_TOKEN.",
|
||||
team_key or "this workspace",
|
||||
user_id,
|
||||
)
|
||||
except Exception: # pragma: no cover - diagnostics must never break connect
|
||||
pass
|
||||
|
||||
async def connect(self, *, is_reconnect: bool = False) -> bool:
|
||||
"""Connect to Slack via Socket Mode."""
|
||||
if not SLACK_AVAILABLE:
|
||||
|
|
@ -1003,6 +1067,7 @@ class SlackAdapter(BasePlatformAdapter):
|
|||
)
|
||||
|
||||
self._warn_if_missing_group_dm_scopes(auth_response, team_name)
|
||||
self._warn_if_not_bot_token(auth_response, team_name)
|
||||
|
||||
# Register message event handler
|
||||
@self._app.event("message")
|
||||
|
|
|
|||
120
tests/gateway/test_slack_user_token_warning.py
Normal file
120
tests/gateway/test_slack_user_token_warning.py
Normal file
|
|
@ -0,0 +1,120 @@
|
|||
"""
|
||||
Tests for the connect-time user-token (vs bot-token) nudge.
|
||||
|
||||
``auth.test`` returns the ``user_id`` of whatever principal owns the configured
|
||||
token. A real bot token (``xoxb-…``) resolves to the app's bot user and the
|
||||
response carries a ``bot_id``; a user/legacy token (``xoxp-…``) resolves to the
|
||||
installing *human's* member ID with **no** ``bot_id``. In the latter case the
|
||||
adapter binds its identity to a human's member ID, so that person's ``<@…>``
|
||||
mentions are misrouted as mentions of the bot. ``_warn_if_not_bot_token``
|
||||
detects the missing ``bot_id`` at connect time — the only point where this is
|
||||
observable, since a user token still sends/receives without any runtime error —
|
||||
and logs an actionable, warning-only nudge.
|
||||
"""
|
||||
|
||||
import logging
|
||||
import sys
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Mock slack-bolt if not installed (same pattern as test_slack_mention.py)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
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)
|
||||
|
||||
|
||||
_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
|
||||
|
||||
|
||||
class _DictAuthResponse(dict):
|
||||
"""Mimics slack_sdk's AsyncSlackResponse — dict-like with .get(), like the
|
||||
real object the adapter already calls ``.get()`` on in ``connect``."""
|
||||
|
||||
|
||||
class _AttrAuthResponse:
|
||||
"""A response shape that is NOT dict-like; values live on ``.data``."""
|
||||
|
||||
def __init__(self, data):
|
||||
self.data = data
|
||||
|
||||
|
||||
def _make_adapter():
|
||||
# object.__new__ skips __init__ (heavy setup) — established slack-test pattern.
|
||||
return object.__new__(SlackAdapter)
|
||||
|
||||
|
||||
def test_warns_when_bot_id_absent(caplog):
|
||||
# User token: auth.test resolves a human member but carries no bot_id.
|
||||
adapter = _make_adapter()
|
||||
resp = _DictAuthResponse(team_id="T1", user_id="U_HUMAN", user="trevor")
|
||||
with caplog.at_level(logging.WARNING):
|
||||
adapter._warn_if_not_bot_token(resp, "Acme")
|
||||
matched = [r for r in caplog.records
|
||||
if "authenticated as a USER" in r.message and "U_HUMAN" in r.message]
|
||||
assert matched
|
||||
|
||||
|
||||
def test_no_warning_when_bot_id_present(caplog):
|
||||
# Real bot token: auth.test carries a bot_id.
|
||||
adapter = _make_adapter()
|
||||
resp = _DictAuthResponse(team_id="T1", user_id="U_BOT", bot_id="B123", user="hermes")
|
||||
with caplog.at_level(logging.WARNING):
|
||||
adapter._warn_if_not_bot_token(resp, "Acme")
|
||||
assert not any("authenticated as a USER" in r.message for r in caplog.records)
|
||||
|
||||
|
||||
def test_no_warning_when_user_id_unresolved(caplog):
|
||||
# Nothing resolved (e.g. odd/empty response) — don't guess, stay silent.
|
||||
adapter = _make_adapter()
|
||||
resp = _DictAuthResponse(team_id="T1")
|
||||
with caplog.at_level(logging.WARNING):
|
||||
adapter._warn_if_not_bot_token(resp, "Acme")
|
||||
assert not any("authenticated as a USER" in r.message for r in caplog.records)
|
||||
|
||||
|
||||
def test_warns_only_once_per_workspace(caplog):
|
||||
adapter = _make_adapter()
|
||||
resp = _DictAuthResponse(user_id="U_HUMAN")
|
||||
with caplog.at_level(logging.WARNING):
|
||||
adapter._warn_if_not_bot_token(resp, "Acme")
|
||||
adapter._warn_if_not_bot_token(resp, "Acme")
|
||||
warnings = [r for r in caplog.records if "authenticated as a USER" in r.message]
|
||||
assert len(warnings) == 1
|
||||
|
||||
|
||||
def test_handles_attribute_only_response_shape(caplog):
|
||||
# Response without dict .get(): values must be read off .data.
|
||||
adapter = _make_adapter()
|
||||
resp = _AttrAuthResponse({"user_id": "U_HUMAN", "user": "trevor"})
|
||||
with caplog.at_level(logging.WARNING):
|
||||
adapter._warn_if_not_bot_token(resp, "Acme")
|
||||
assert any("authenticated as a USER" in r.message and "U_HUMAN" in r.message
|
||||
for r in caplog.records)
|
||||
Loading…
Add table
Add a link
Reference in a new issue