Skip to content

fix(slack): warn when configured token is a user token, not a bot token - #55332

Closed
benbarclay wants to merge 1 commit into
mainfrom
fix/slack-user-token-warning
Closed

fix(slack): warn when configured token is a user token, not a bot token#55332
benbarclay wants to merge 1 commit into
mainfrom
fix/slack-user-token-warning

Conversation

@benbarclay

Copy link
Copy Markdown
Collaborator

Infographic

slack-bot-token-warning

Problem

A Slack user/legacy token (xoxp-…) configured in SLACK_BOT_TOKEN makes auth.test resolve to the installing human's member ID — and the response carries no bot_id. The adapter trusts that user_id blindly:

# plugins/platforms/slack/adapter.py — connect()
auth_response = await client.auth_test()
bot_user_id = auth_response.get("user_id", "")   # ← a human's member ID for a user token
self._team_bot_user_ids[team_id] = bot_user_id
if self._bot_user_id is None:
    self._bot_user_id = bot_user_id

From then on, self._bot_user_id is bound to a human's member ID, so every "is this the bot?" check misfires. The channel mention gate fires on that person's mentions:

bot_uid = self._team_bot_user_ids.get(team_id, self._bot_user_id)
is_mentioned = bool((bot_uid and f"<@{bot_uid}>" in routing_text) or ...)
...
if is_mentioned:
    text = text.replace(f"<@{bot_uid}>", "").strip()   # stripped as the bot's own mention

So when someone writes @thatperson …, the adapter classifies it as a bot @mention, strips the token, and the agent is genuinely told it was mentioned — it then replies to a message that was only addressed to that human, and (correctly, given the bad input) insists it was "explicitly mentioned." Other users' <@…> tokens are untouched and non-triggering; the misbehaviour is isolated to whichever human the token authenticates as.

There is no runtime API error to catch — a user token still sends and receives — so the only point this is observable is connect time, by noticing bot_id is absent from auth.test.

Fix (warning-only)

Add _warn_if_not_bot_token(auth_response, team_name), called in the per-token auth loop right next to the existing _warn_if_missing_group_dm_scopes nudge. When auth.test resolves a user_id but no bot_id, it logs an actionable warning naming the misbound member ID and pointing at the xoxb-… Bot User OAuth Token. It is warning-only — a misconfigured-but-working install is not hard-failed on connect — and fires once per workspace per process (mirrors the group-DM nudge's warned set). Diagnostics are wrapped so they can never break connect().

Tests

tests/gateway/test_slack_user_token_warning.py (mirrors test_slack_group_dm_scope_warning.py):

  • warns when bot_id is absent (user token) and names the member ID
  • no warning when bot_id is present (real bot token)
  • no warning when nothing resolves (empty response — don't guess)
  • warns only once per workspace
  • handles an attribute-only response shape (values on .data, not dict .get())

Prove-fail verified: with the adapter change stashed, all 5 fail with AttributeError: 'SlackAdapter' object has no attribute '_warn_if_not_bot_token'; with the fix applied, all 5 pass (and the 5 sibling group-DM-scope tests stay green).

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.
@benbarclay
benbarclay requested a review from teknium1 June 30, 2026 01:19
@github-actions

Copy link
Copy Markdown
Contributor

🔎 Lint report: fix/slack-user-token-warning vs origin/main

ruff

Total: 0 on HEAD, 0 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 0 pre-existing issues carried over.

ty (type checker)

Total: 11724 on HEAD, 11724 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 6174 pre-existing issues carried over.

Diagnostics are surfaced as warnings — this check never fails the build.

@alt-glitch alt-glitch added type/feature New feature or request comp/plugins Plugin system and bundled plugins platform/slack Slack app adapter P3 Low — cosmetic, nice to have labels Jun 30, 2026

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Connect-time detection of user tokens vs bot tokens with actionable warning. Well-documented, tests included.

@tonydwb

tonydwb commented Jun 30, 2026

Copy link
Copy Markdown

Code Review Summary

Verdict: Approved

Detects when a configured Slack token authenticates as a human user (no bot_id in auth.test response) and warns the operator. This prevents the misrouting bug where the bot mistakes human mentions for self-mentions.

✅ Looks Good

  • Clear detection: missing bot_id in auth.test response means user token
  • Warning-only (doesn't break working-but-misconfigured installs)
  • Once-per-workspace deduplication
  • Good test coverage: bot present, absent, unresolved, attribute-only response shape
  • Actionable warning message explaining the fix

Reviewed by Hermes Agent

@teknium1

Copy link
Copy Markdown
Contributor

Merged via PR #55395. Your commit was cherry-picked onto current main with your authorship preserved in git log (commit 184c10c). Thanks for the clean diagnostic — warning-only, once per workspace, didn't touch the trusted-input path. CI green, all 10 tests (5 new + 5 sibling group-DM) pass.

@teknium1 teknium1 closed this Jun 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have platform/slack Slack app adapter type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants