Skip to content

fix(telegram): accept scalar allowlist configuration - #64473

Open
visualfox-ch wants to merge 2 commits into
NousResearch:mainfrom
visualfox-ch:fix/telegram-scalar-allow-from
Open

fix(telegram): accept scalar allowlist configuration#64473
visualfox-ch wants to merge 2 commits into
NousResearch:mainfrom
visualfox-ch:fix/telegram-scalar-allow-from

Conversation

@visualfox-ch

Copy link
Copy Markdown

Summary

  • accept scalar allow_from / allowlist values in Telegram configuration
  • normalize scalar and sequence forms consistently
  • isolate authorization tests from live environment allowlists

Test plan

  • python -m pytest tests/gateway/test_telegram_auth_check.py tests/gateway/test_telegram_rich_messages.py -q -o 'addopts='
  • ruff check plugins/platforms/telegram/adapter.py tests/gateway/test_telegram_auth_check.py

Split from and supersedes the Telegram portion of #50894.

@alt-glitch alt-glitch added type/bug Something isn't working comp/plugins Plugin system and bundled plugins platform/telegram Telegram bot adapter area/auth Authentication, OAuth, credential pools P3 Low — cosmetic, nice to have labels Jul 14, 2026

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

Verdict: Approved

Fix for Telegram allowlist configuration to accept scalar values. Small (35 additions, 2 deletions). No security concerns.


Reviewed by Hermes Agent

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for isolating the scalar Telegram allowlist failure. Current main still directly iterates allow_from at plugins/platforms/telegram/adapter.py:931, while the configuration bridge preserves the raw configured value at gateway/config.py:1242-1243, so the integer-scalar bug is real.

Problems

  • plugins/platforms/telegram/adapter.py:931 narrows the existing accepted input from arbitrary iterables to list. Before this change, tuple and set values were iterated into normalized IDs. With the proposed fallback, they are stringified and comma-split, yielding parenthesized/quoted tokens that no longer match. PlatformConfig.extra intentionally permits Any values (gateway/config.py:477).

Suggested changes

  • Keep non-string iterable normalization while handling scalar strings/numbers separately, and add a tuple or set regression case next to the scalar tests.

This is an automated hermes-sweeper review.

adapter_allow_from = self.config.extra.get("allow_from")
if adapter_allow_from is not None:
allowed = {str(u).strip() for u in adapter_allow_from if str(u).strip()}
if isinstance(adapter_allow_from, list):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This narrows the prior behavior from any iterable to only list: tuples and sets previously normalized correctly but now reach the scalar str(...).split(",") path and no longer match their IDs. Please preserve non-string iterable handling while adding scalar support.

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 16, 2026
@GottZ

GottZ commented Aug 3, 2026

Copy link
Copy Markdown

This was generated by AI during triage.

Summary

Thirteen PRs address or reference three related Telegram authorization problems: the original missing user/group controls, YAML-versus-environment intake short-circuits for group and DM users, and preservation of the documented global/group/chat authorization union. Their diffs range from the merged gateway implementation in #17748 and focused adapter fixes in #55496/#55529 to scalar parsing coverage in #63350/#64473 and the broader union/multiplex work in #72084.

Related pull requests

Duplicates

#7659 and #17686 are predecessors of merged #17748; #55624 and #57189 duplicate the group-side work led by #55496, while #55529 contains that path plus the complementary DM fix. #63350 overlaps #64473's comma-string regression coverage, and #68784/#69617 are superseded approaches to the authorization-union problem now addressed more comprehensively by #72084.

Suggested consolidation

Keep #55496 open with the explicit-empty fail-closed fix, keep recorded best fix #55529 open for alias documentation, and keep #63350 open only as salvageable focused regression coverage. Author action on #64473: preserve tuple/set iterable behavior and retain the scalar tests; author action on recorded best fix #72084: rebase onto current main or split out the current intake/runner authorization union and multiplex profile-scope pieces, while the already closed #68784 and #69617 remain historical/superseded and #55624/#57189 remain duplicates.

Complex graph

flowchart LR
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    subgraph Dup63350 ["PRs duplicating each other"]
        P63350["PR #63350 (open)"]
        P64473["PR #64473 (open)"]
    end
    class P63350 open
    class P64473 open
    class P64473 target
    click P63350 "https://github.com/NousResearch/hermes-agent/pull/63350"
    click P64473 "https://github.com/NousResearch/hermes-agent/pull/64473"
Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 13 pull requests and 3 issues in this complex. Each diff was read against this issue; Assessment working set: 189 kB of PR diffs, 30 kB of issue/PR text, 25 kB of discussion (42 comments), 16 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

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:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants