Skip to content

fix(slack): authorize bot/workflow senders before the no-user-id guard (salvage #52403) - #56316

Merged
kshitijk4poor merged 1 commit into
NousResearch:mainfrom
kshitijk4poor:salvage-52403
Jul 1, 2026
Merged

fix(slack): authorize bot/workflow senders before the no-user-id guard (salvage #52403)#56316
kshitijk4poor merged 1 commit into
NousResearch:mainfrom
kshitijk4poor:salvage-52403

Conversation

@kshitijk4poor

Copy link
Copy Markdown
Collaborator

Summary

Salvage of #52403 by @cypctlinux (rebased onto current main with a conflict resolution). Lets allowed Slack bot/workflow senders route by authorizing them before the no-user-id guard.

The bug

gateway/authz_mixin.py has a {PLATFORM}_ALLOW_BOTS bypass, but on main it (a) didn't list Slack, and (b) ran the bot bypass ~83 lines after the if not user_id: return False guard. Slack Workflow-Builder / app posts arrive as subtype=bot_message with no user field → user_id=None, so they hit that guard and were rejected before the bypass could ever run. Net effect: SLACK_ALLOW_BOTS=mentions|all had no effect for exactly the workflow/app posts it was meant to admit.

The fix

  • Relocate the platform_allow_bots_map + if getattr(source, "is_bot") bypass to run before the no-user-id guard (so a user=None bot event can still be admitted), and add Platform.SLACK: "SLACK_ALLOW_BOTS" to the map.
  • Flag Slack bot-authored events as bot sources (is_bot = bool(event.get("bot_id")) or subtype == "bot_message") so the bypass applies.
  • Regression tests: bot user_id=None authorized under mentions/all, denied when unset/none, humans unaffected.

The bypass still requires is_bot AND SLACK_ALLOW_BOTS ∈ {mentions, all}; unmapped platforms and non-bot traffic fall through to the guard exactly as before — moving it earlier changes when a would-be-authorized bot is admitted, never whether.

Rebase conflict resolution (this salvage)

main had drifted since the PR was cut: it added Platform.TELEGRAM: "TELEGRAM_ALLOW_BOTS" to the old (late-position) map. The PR's relocated map only had DISCORD/FEISHU/SLACK. Resolved by deleting the old block and re-adding Platform.TELEGRAM to the relocated map, so no platform is dropped — the map now has DISCORD/FEISHU/TELEGRAM/SLACK. Also added an AUTHOR_MAP entry (t.chen@aftership.com@cypctlinux).

Chosen over competing PRs (issue: Slack bot mentions don't route)

This was a 5-PR cluster. #52403 is the correct base because it relocates the bypass before the guard — the actual user=None failure mode. Two near-duplicates are superseded:

Distinct/complementary PRs left open: #52390 (Block-Kit-only mention detection) and #51627 (peer bot-routing loops).

Review

Ran hermes-agent-dev + hermes-pr-review Phase 2c — 0 Critical / 0 Warnings. Verified the relocation opens no new hole, Telegram preserved, comment now literally accurate, consistent double-gate with the adapter's own bot filter. Locally: 53 tests pass (31 Slack bot-auth + 22 Telegram/Discord/Feishu bot-auth — the relocation doesn't regress other platforms).

Tests

pytest tests/gateway/test_slack_bot_auth_bypass.py tests/gateway/test_slack_approval_buttons.py -q   # 31 passed (CI runs the full suite)

Supersedes #52403 (and #56286 / #40883). Full credit to @cypctlinux.

Slack Workflow Builder posts (and other app/bot messages) arrive as
subtype=bot_message with user=None. _is_user_authorized rejected them at
the `if not user_id: return False` guard, which runs *before* the NousResearch#4466
{PLATFORM}_ALLOW_BOTS bypass — so @mentioning the bot from a Slack
workflow silently did nothing, even with SLACK_ALLOW_BOTS (or
SLACK_ALLOW_ALL_USERS) set. The chat-scoped allowlist for Telegram/QQ
already runs before that guard for the same reason (channel broadcasts
with no from_user); Slack was both missing from the bot-bypass map and
had the bypass running too late.

- gateway/authz_mixin: move the {PLATFORM}_ALLOW_BOTS bypass ahead of the
  no-user-id guard and add Platform.SLACK -> SLACK_ALLOW_BOTS.
- plugins/platforms/slack/adapter: set is_bot=True on inbound
  bot_message events so the gateway can identify workflow/app senders
  (they carry no user_id to match against the allowlist).

Tested: new tests/gateway/test_slack_bot_auth_bypass.py plus the existing
Discord/Feishu bot-auth and gateway authz/gating suites all pass.
@kshitijk4poor
kshitijk4poor enabled auto-merge (rebase) July 1, 2026 10:58
@kshitijk4poor
kshitijk4poor merged commit d3c8667 into NousResearch:main Jul 1, 2026
29 checks passed
@alt-glitch alt-glitch added type/security Security vulnerability or hardening area/auth Authentication, OAuth, credential pools sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data comp/gateway Gateway runner, session dispatch, delivery platform/slack Slack app adapter P2 Medium — degraded but workaround exists labels Jul 1, 2026
@kshitijk4poor
kshitijk4poor deleted the salvage-52403 branch August 5, 2026 07:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/auth Authentication, OAuth, credential pools comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists platform/slack Slack app adapter sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/security Security vulnerability or hardening

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants