Skip to content

fix(telegram): admit allowed bot senders early - #92844

Closed
Ahmett101 wants to merge 1 commit into
NousResearch:mainfrom
Ahmett101:fix/92840-telegram-bot-auth-prefilter
Closed

fix(telegram): admit allowed bot senders early#92844
Ahmett101 wants to merge 1 commit into
NousResearch:mainfrom
Ahmett101:fix/92840-telegram-bot-auth-prefilter

Conversation

@Ahmett101

Copy link
Copy Markdown
Contributor

Summary

Telegram early intake authorization built a SessionSource without preserving from_user.is_bot, then fell back to the human allowlist when the multiplex profile handler was a closure with no __self__. That meant bot-authored messages were rejected before the normal gateway bot policy could admit them, even when TELEGRAM_ALLOW_BOTS was mentions or all. This PR carries the Telegram sender's bot bit into the auth source and mirrors AuthzMixin's bot-policy branch at the early prefilter before applying the human allowlist.

Changes

plugins/platforms/telegram/adapter.py: propagates message.from_user.is_bot into _source_from_message_for_auth() and admits bot-authored messages early when scoped TELEGRAM_ALLOW_BOTS resolves to mentions or all.
tests/gateway/test_telegram_auth_check.py: extends the Telegram message fixture with is_bot, adds a multiplex-closure regression for allowed bot-authored messages, and adds a negative human-sender case proving bot policy does not bypass the human allowlist.

How to Test

python -m pytest tests/gateway/test_telegram_auth_check.py tests/gateway/test_telegram_bot_auth_bypass.py -q

✅ 17/17 passed

ruff check plugins/platforms/telegram/adapter.py tests/gateway/test_telegram_auth_check.py tests/gateway/test_telegram_bot_auth_bypass.py

✅ passed

git diff --check

✅ passed

python scripts/check-windows-footguns.py --diff upstream/main

✅ passed

scripts/run_tests.sh

⚠️ Termux runner/environment failure: 3197 PermissionError/permission-denied patterns in the full-suite log

Notes

An open PR (#91483) also mentions TELEGRAM_ALLOW_BOTS, but it changes gateway/authz_mixin.py to add bot-loop protection for #91481. This PR is separate and scoped to Telegram adapter early-auth intake for #92840.

Checklist

  • Targeted tests pass — 17/17
  • Follows Conventional Commits
  • Changes scoped to this fix only
  • Cross-platform impact assessed (Linux / macOS / WSL2 / Windows / Termux)
  • profile-safe paths used — no hardcoded ~/.hermes
  • .env not used for non-credential settings (behavioral settings → config.yaml)

Risk & Impact

Low. The new bypass only applies when Telegram marks the sender as a bot and the existing bot policy explicitly allows bots; human senders still go through the existing allowlist path.
Type: Bug fix
Closes: #92840

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/plugins Plugin system and bundled plugins platform/telegram Telegram bot adapter area/auth Authentication, OAuth, credential pools sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages labels Aug 23, 2026
@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference, author can ignore or act on any point.

Correct fix with the right shape: I verified against gateway/authz_mixin.py:495-511 that the canonical bot-policy branch admits senders whenever {PLATFORM}_ALLOW_BOTS ∈ {mentions, all} without further conditions, so the adapter's early return faithfully mirrors the contract (including placement before the human allowlist), and it's needed because the multiplex closure handler genuinely can't reach the mixin via __self__. The negative-control test (bot policy must not admit humans) is exactly the right companion.

Two suggestions:

  1. Policy logic is now duplicated across two modules — the env name, accepted value set, and ordering live in both gateway/authz_mixin.py:501-511 and this adapter (plugins/platforms/telegram/adapter.py:~1431-1437). Drift between them would make authorization depend on which handler shape received the message. Extracting a tiny shared helper (e.g. bot_policy_admits(env_var, is_bot) -> bool next to the gate helpers, used by both call sites) keeps one source of truth while preserving the closure workaround.

  2. Missing negative case for the policy itself — tests cover bot+mentions → True and human+all → False, but not bot with TELEGRAM_ALLOW_BOTS=none/unset → False. That's the regression most likely to be introduced by a future refactor of the early-return block; one more test closes it.

Minor:

  • Confirm _scoped_gate_env resolves to the same value the mixin's _platform_gate_env would see on non-multiplex runs, so both entry paths can't disagree when a profile-scoped override exists.
  • Consider threading is_bot through _source_from_reaction_for_auth as well if bot reactions ever reach an auth gate.

@teknium1

teknium1 commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Thanks @Ahmett101 for this PR — Merged via #101250 (11f932c) on current main.

#101250 won because it was built on the earlier #65589 fix and covers the same behavior across all affected call paths in one change. You're credited via Co-authored-by on the merge and in the PR body of #101250.

Closing this PR as superseded by the merged work.

@teknium1 teknium1 closed this Sep 2, 2026
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/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have platform/telegram Telegram bot adapter sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(telegram): allow bot-authored messages through early auth in multiplex mode

4 participants