Skip to content

telegram: enforce TELEGRAM_ALLOWED_USERS on DMs (mirror DingTalk allowlist)#8

Open
zenprocess wants to merge 1 commit into
AtomicBot-ai:mainfrom
zenprocess:fix/telegram-allowed-users-dm-enforcement
Open

telegram: enforce TELEGRAM_ALLOWED_USERS on DMs (mirror DingTalk allowlist)#8
zenprocess wants to merge 1 commit into
AtomicBot-ai:mainfrom
zenprocess:fix/telegram-allowed-users-dm-enforcement

Conversation

@zenprocess
Copy link
Copy Markdown

Summary

TELEGRAM_ALLOWED_USERS currently gates only callback buttons via _is_callback_user_authorized (gateway/platforms/telegram.py lines 1632, 1680). DMs are not gated — _should_process_message short-circuits non-group chats unconditionally at line 2367:

if not self._is_group_chat(message):
    return True

This means an operator who sets TELEGRAM_ALLOWED_USERS=<id> expecting it to lock the bot's DM surface is silently exposed — any Telegram user who knows the bot's @-handle can DM it freely.

Fix

Move the user-id check to the top of _should_process_message, reusing the existing static method _is_callback_user_authorized so the allowlist gates both DMs and groups. 9 insertions, 1 deletion.

def _should_process_message(self, message: Message, *, is_command: bool = False) -> bool:
    sender_id = str(getattr(getattr(message, "from_user", None), "id", ""))
    if not self._is_callback_user_authorized(sender_id):
        return False
    if not self._is_group_chat(message):
        return True
    ...

Backwards compatibility

_is_callback_user_authorized already returns True when TELEGRAM_ALLOWED_USERS is empty or unset — the legacy permissive default is preserved for every existing deployment that hasn't opted into the allowlist.

Prior art

This mirrors the pattern dingtalk.py already uses:

  • _load_allowed_users + _is_user_allowed (lines 402-422)
  • Gate call at the top of the message handler (line 546)

DingTalk gates both DM and group; Telegram now does the same.

Verification

Unit-tested locally with mocked Message objects:

Sender Channel TELEGRAM_ALLOWED_USERS Expected Got
Allowed user DM set accept accept
Disallowed user DM set drop drop
Allowed user group set accept accept
Disallowed user group set drop drop
Disallowed user DM unset/empty accept (legacy) accept

Also live-tested end-to-end via PYTHONPATH-shadow against hermes-agent 0.11.0 (Atomic Hermes 0.1.36): patched gateway started cleanly, polling worked, allowlist enforced on inbound messages.

Notes

  • The static method _is_callback_user_authorized already accepts user_id: str and parses the CSV env var; calling it from _should_process_message requires no new helper.
  • No new env var or config key introduced — operators who already set TELEGRAM_ALLOWED_USERS for callbacks now get DM enforcement for free.

…wlist)

The existing TELEGRAM_ALLOWED_USERS env var gated only callback buttons
(_is_callback_user_authorized at lines 1632, 1680). _should_process_message
short-circuited DMs unconditionally with 'if not self._is_group_chat(message):
return True' — bypassing any allowlist check.

This change moves the user-id check to the top of _should_process_message,
reusing the existing _is_callback_user_authorized static method, so the
allowlist gates both DM and group messages. Empty env preserves the legacy
permissive default; backwards compatible.

Mirrors the dingtalk.py:402-422 + 546 pattern that already does this for
DingTalk.

Verified: 5/5 unit-test cases (allowed/disallowed × DM/group × backcompat).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant