fix(gateway): enforce auth check in busy-session path to prevent unauthorized injection (#17775)#17920
Merged
Merged
Conversation
…thorized injection (#17775) The busy-session handler (_handle_active_session_busy_message) bypassed the authorization gate that the cold path enforces via _is_user_authorized(). In shared-thread contexts (Slack threads, Telegram forum topics, Discord threads) where thread_sessions_per_user=False (the default), all participants share one session_key. An unauthorized user posting in the same thread as an authorized user would hit the active-session branch, skip the auth check, and have their text merged into _pending_messages or injected via agent.interrupt(). This commit adds the same _is_user_authorized() check at the top of the busy handler, before any message queuing, steering, or interrupt logic. Unauthorized messages are silently dropped (return True) with a warning log — matching the cold-path behavior. Affected platforms: Slack, Telegram, Discord, any adapter with shared-session thread contexts. Closes #17775
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Salvage of #17816 by @Bartok9 onto current main.
Closes #17775.
Summary
Adds the missing
_is_user_authorized()gate at the top of_handle_active_session_busy_message(), closing a P0 authorization bypass in shared-thread contexts (Slack/Telegram/Discord withthread_sessions_per_user=False, the default).Root cause
The cold path (
_handle_message) correctly calls_is_user_authorized()before creating a session, but the busy path — reached when an active session already exists — skipped the check entirely. Non-allowlisted users in the same thread as an authorized user could queue text into_pending_messages, triggeragent.interrupt()with their content, receive a public⚡ Interrupting...ack, and end up addressed by name in the LLM reply.Bypass commands (
/stop,/new,/approve, etc.) were already safe — they route through_message_handlerwhich hits the cold-path auth gate. The gap was exactly the busy/interrupt fallthrough.Changes
gateway/run.py: 16-line auth gate at the top of_handle_active_session_busy_message— log warning, return True (handled = silently dropped).tests/gateway/test_busy_session_auth_bypass.py: 4 new cases — unauthorized dropped, authorized still processed, unauthorized blocked during drain, unauthorized can't steer.Validation
Also ran all adjacent gateway auth suites: 41/41 pass across
test_allowlist_startup_check,test_auth_fallback,test_discord_bot_auth_bypass,test_unauthorized_dm_behavior.E2E reproduced the bypass on main and confirmed the fix blocks it — intruder → no queue, no interrupt, no ack; authorized user → unchanged.
Credit to @Bartok9 for the report-to-fix workflow and test coverage.