fix(gateway): delegate Telegram callback auth to shared user authorizer#18016
Closed
premie wants to merge 1 commit into
Closed
fix(gateway): delegate Telegram callback auth to shared user authorizer#18016premie wants to merge 1 commit into
premie wants to merge 1 commit into
Conversation
Isolate the WhatsApp bridge package-lock change since it's unrelated. The _is_callback_user_authorized method on TelegramAdapter was checking only TELEGRAM_ALLOWED_USERS env var, bypassing the gateway's full auth path (pairing store + global allowlists). This meant inline-button callbacks could succeed for users rejected by the normal message auth flow. Changes: - BasePlatformAdapter: add _user_authorizer field + set_user_authorizer() - TelegramAdapter: _is_callback_user_authorized now calls injected authorizer when available, falls back to env-only check in tests - GatewayRunner: wire _is_user_id_authorized into every adapter on connect - Add test: callback auth delegates to injected authorizer
Collaborator
|
Related to #17862 which addresses the same Telegram inline callback auth bypass. This PR adds a more general injector pattern via BasePlatformAdapter. |
Collaborator
|
Related to #17862 which addresses the same Telegram inline callback auth bypass. This PR takes a slightly different approach (injected authorizer vs delegating to runner). |
19 tasks
Contributor
|
Automated hermes-sweeper review: this appears to be implemented on current main via the merged #18180 salvage path, so this PR is now superseded. Evidence:
The implementation differs from this PR's exact injector pattern, but the bug this PR targets — inline-button callbacks bypassing pairing/global gateway authorization — is fixed on main. |
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
3 participants
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.
Summary
TelegramAdapter._is_callback_user_authorizedonly checked theTELEGRAM_ALLOWED_USERSenv var, bypassing the gateway's full auth path (pairing store + global allowlists). This meant inline-button callback events were authorized inconsistently with inbound messages — users paired through the gateway could be silently rejected on approval-button clicks even though their messages were accepted.Changes
BasePlatformAdapter: add_user_authorizerfield +set_user_authorizer()injectorTelegramAdapter._is_callback_user_authorized: call the injected authorizer when present; fall back to the env-only check for tests / unwired adaptersGatewayRunner: wire_is_user_id_authorizedinto every adapter on connect so callbacks go through the same auth path as inbound messagesTest plan
tests/gateway/test_telegram_approval_buttons.py— callback auth delegates to injected authorizerhermes updateaccidentally reverted it