Skip to content

fix(slack): authorize app/webhook messages via SLACK_ALLOW_BOTS - #40883

Closed
DidneyWhorl wants to merge 1 commit into
NousResearch:mainfrom
DidneyWhorl:fix/slack-allow-bots-authorization
Closed

fix(slack): authorize app/webhook messages via SLACK_ALLOW_BOTS#40883
DidneyWhorl wants to merge 1 commit into
NousResearch:mainfrom
DidneyWhorl:fix/slack-allow-bots-authorization

Conversation

@DidneyWhorl

Copy link
Copy Markdown

Problem

Slack messages posted by apps / incoming webhooks (e.g. a voicemail-transcription
app) were silently dropped even when SLACK_ALLOW_BOTS=all (or
slack.allow_bots: all) was configured. No inbound message log line, no error —
the event simply vanished, making it look like Slack never delivered it.

Root cause

SLACK_ALLOW_BOTS only clears the adapter-level filter in
gateway/platforms/slack.py. After that, the message reaches the gateway auth
layer _is_user_authorized() in gateway/run.py. App/webhook posts carry a
bot_id but no user field, so they fail the if not user_id: return False
guard and are denied.

There is already a bot-admission bypass (platform_allow_bots_map) that lets
{PLATFORM}_ALLOW_BOTS admit bot-authored messages — but it only listed
Discord and Feishu. Slack was missing, and the Slack adapter never set
is_bot on the source, so even adding Slack to the map alone wouldn't have helped.

Fix

Mirror the existing Discord/Feishu bot-admission path for Slack:

  • gateway/run.py: add Platform.SLACK: "SLACK_ALLOW_BOTS" to
    platform_allow_bots_map.
  • gateway/platforms/slack.py: derive a synthetic user_id of
    bot:<bot_id> for bot/app posts (clears the not user_id guard), and pass
    is_bot=True into build_source(...) so the bypass authorizes them.

SLACK_ALLOW_BOTS semantics are unchanged: none (default) still drops bot
messages, mentions/all admit them.

Verification

Tested end-to-end on Windows with a real Slack voicemail app (Socket Mode):

  • Before: app post → no log, no response.
  • After: inbound message: platform=slack user=bot:B0B88SG9P6Y chat=... msg='...New Voicemail...'
    followed by a normal agent response in-thread.

Security posture preserved (unit-checked against the real _is_user_authorized):

  • Unknown humans (not in SLACK_ALLOWED_USERS) → still denied.
  • The configured human → authorized.
  • Bot message with SLACK_ALLOW_BOTS=none → still denied.

Notes

17 insertions across 2 files; no behavior change for existing setups (default
allow_bots=none is preserved).

Slack app/incoming-webhook posts carry a bot_id but no user field, so they
were silently denied by _is_user_authorized()'s 'if not user_id: return False'
guard even when SLACK_ALLOW_BOTS=all was set — the setting only cleared the
adapter-level filter, not the gateway auth layer.

Mirror the existing Discord/Feishu bot-admission path for Slack:
- run.py: add Platform.SLACK to platform_allow_bots_map so SLACK_ALLOW_BOTS
  (mentions|all) bypasses the human allowlist for bot-authored messages.
- slack.py: derive a synthetic user_id ('bot:<bot_id>') for bot/app posts so
  they clear the not-user_id guard, and pass is_bot=True into build_source so
  the bypass authorizes them.

Verified end-to-end on Windows: a Slack voicemail-transcription app's posts now
reach the agent (log shows user=bot:B... inbound + response) while the human
allowlist stays enforced (unknown users denied; allow_bots=none still blocks).
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists platform/slack Slack app adapter comp/gateway Gateway runner, session dispatch, delivery labels Jun 7, 2026
@liuhao1024

Copy link
Copy Markdown
Contributor

Verified: Slack bot authorization via SLACK_ALLOW_BOTS

Reviewed the full diff. The implementation correctly follows the established pattern used by Discord (DISCORD_ALLOW_BOTS) and Feishu (FEISHU_ALLOW_BOTS):

  1. Synthetic user_id: f"bot:{_bot_id}" from event.get("bot_id") provides a stable identifier that clears the gateway's if not user_id: return False guard without conflicting with real Slack user IDs.

  2. is_bot flag: build_source(is_bot=is_bot_source) is valid — build_source() in gateway/platforms/base.py:4640 already accepts is_bot: bool = False.

  3. Bot detection: event.get("bot_id") OR subtype == "bot_message" covers both incoming webhooks (which carry bot_id) and Slack app posts (which have subtype="bot_message"). This is the correct detection surface.

  4. Authorization map: Adding Platform.SLACK: "SLACK_ALLOW_BOTS" to platform_allow_bots_map in _is_user_authorized mirrors the Discord/Feishu pattern exactly.

The fix is consistent, well-scoped, and correctly prevents app/webhook messages from being silently denied when SLACK_ALLOW_BOTS is set.

@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.

Code Review Summary

Verdict: Approved

Mirrors the Discord bot allowlist pattern for Slack: bot-authored messages (no user field) are now given a synthetic user_id = bot:<bot_id> so they pass the authorization guard, and is_bot is surfaced in source metadata. Consistent with the existing DISCORD_ALLOW_BOTS design. No regression risk.

No issues found.

Reviewed by Hermes Agent

@kshitijk4poor

Copy link
Copy Markdown
Collaborator

Closing as superseded by #56316 (salvage of #52403). This PR patches gateway/platforms/slack.py and gateway/run.py helpers (_is_user_authorized / platform_allow_bots_map in run.py) that no longer exist on current main — that logic moved to plugins/platforms/slack/adapter.py and gateway/authz_mixin.py, so the hunks target vanished code. #56316 implements the same SLACK_ALLOW_BOTS intent on the current code paths (and fixes the user=None ordering). Credit for flagging the app/webhook auth gap early. Feel free to reopen/rebase if I've misread.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists platform/slack Slack app adapter type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants