test(telegram): pin comma-string allow_from / group_allow_from in adapter auth

[Rival]

Rebased onto current main — scope reduced to tests only. See the rebase note in the comments: main fixed the underlying parsing bug independently via the _coerce_allow_set extraction, so this PR's original one-line production change is now redundant and has been dropped. What remains is the regression coverage, which main does not have.

What

Three tests in tests/gateway/test_telegram_auth_check.py pinning that a comma-string allow_from / group_allow_from behaves exactly like the equivalent YAML list in TelegramAdapter._is_user_authorized_from_message.

No production change.

Why

_is_user_authorized_from_message picks group_allow_from for group/forum/channel and allow_from for DMs, then parses whichever it chose with _coerce_allow_set. Both keys accept either form:

• a list allow_from is ",".join(...)-ed onto TELEGRAM_ALLOWED_USERS, and the env branch of this same method splits it;
• scalar YAML — allow_from: "111,222" — reaches the adapter as one string.

If such a value were ever iterated directly instead of split, the allow-set would become single characters: every real user id rejected, locking the owner out of their own bot. That is a lockout-class failure, and it is the failure this branch originally shipped a fix for.

_coerce_allow_set handles it correctly today. But the existing adapter-auth tests (..._allow_from, ..._group_allow_from) exercise the list form only — nothing pins the string form through either branch, so the behavior could regress silently.

Ships

• ..._comma_string_allow_from — comma-string via allow_from on a DM.
• ..._comma_string_group_allow_from — comma-string via group_allow_from in a group.
• ..._comma_string_with_spaces — " 111 , 222 " tolerated (same .strip() as the env branch).
• ..._comma_string_wildcard — a bare * scalar still wildcards.

Verified adversarially: reverting _coerce_allow_set to the pre-extraction {str(u).strip() for u in raw} comprehension makes three of these four fail. (The wildcard case passes either way — iterating "*" char-wise still yields {"*"} — so it is coverage, not a discriminator.)

[Rival]

Rebased onto current main, and the rebase turned up something worth flagging: main has already fixed this bug independently, by a different route.

When this branch was cut, the adapter-level branch built its allow-set inline:

allowed = {str(u).strip() for u in adapter_allow_from if str(u).strip()}

Iterating a comma-string char-wise there is the lockout bug. main has since extracted that into gateway/authz_mixin.py::_coerce_allow_set, which splits a scalar on commas — its docstring even cites this exact case. So the one-line production change this PR shipped is now redundant, and I have dropped it rather than leave a no-op hunk in the diff for a reviewer to puzzle over.

Two other things the rebase surfaced:

1. main also made allow_from DM-only, introducing group_allow_from for group/forum/channel. The original tests here set allow_from and used the _make_message default chat_type="group", so on current main they exercised neither branch and passed vacuously. They now pass explicit chat types, matching the style of the adjacent ..._allow_from / ..._group_allow_from tests.
2. Neither of those adjacent tests covers the string form of either key — only lists. That gap is what is left worth merging, so this is now a tests-only PR (retitled accordingly).

Verified adversarially: reverting _coerce_allow_set to the old comprehension fails three of the four added tests. The wildcard case passes either way ("*" iterated char-wise is still {"*"}), so it is coverage rather than a discriminator — noting that explicitly rather than claiming four independent guards.

Happy to close this instead if you would rather not carry the extra coverage.

[GottZ]

This was generated by AI during triage.

Summary

Thirteen PRs address or reference three connected Telegram authorization problems: the original missing user/group controls, intake short-circuits that mask YAML or environment grants, and scalar parsing plus preservation of the documented global/group/chat authorization union. The diffs include the merged gateway implementation in #17748, overlapping group/DM fixes in #55496 and #55529, focused scalar regressions in #63350 and #64473, and broader union work culminating in #72084.

Related pull requests

• fix(telegram): add user-level access control for groups and DMs #7659 [closed] related — (+0/-0) — n/a: Its overwritten head leaves no diff, but it remains relevant as the original Telegram: Missing user-level access control — any user can trigger bot via @mention in any group #7651 proposal restored on a dedicated branch in fix(telegram): support group user allowlist #17686.
• feat: allow Telegram chat allowlists for groups and forums #15027 [merged] partial — (+30/-2) — n/a: This merged reference added group/forum chat-ID authorization but mapped group_allowed_chats through TELEGRAM_GROUP_ALLOWED_USERS, the compatibility case later handled by fix(telegram): support group user allowlist (salvages #17686) #17748.
• fix(telegram): support group user allowlist #17686 [closed] fixes — (+222/-20) — n/a: The diff separates global-user, group-user, and group-chat authorization and remains relevant because its implementation was cherry-picked with preserved authorship into fix(telegram): support group user allowlist (salvages #17686) #17748.
• fix(telegram): support group user allowlist (salvages #17686) #17748 [merged] best fix — (+369/-29) — n/a: The recorded best fix for Telegram: Missing user-level access control — any user can trigger bot via @mention in any group #7651 is the merged implementation that salvaged fix(telegram): support group user allowlist #17686, documented the three authorization scopes, and retained compatibility with feat: allow Telegram chat allowlists for groups and forums #15027-era chat-ID values.
• fix(telegram): honor group user allowlists #55496 partial — (+148/-6) — n/a: The diff fixes the group/forum allow_from short-circuit and bridges aliases, but the contributor keep_open review requires preserving fail-closed behavior for an explicitly empty allow_from.
• fix(telegram): honor DM env allowlist when yaml allow_from is set #55529 best fix — (+176/-6) — n/a: The recorded best fix for [Bug]: TELEGRAM_ALLOWED_USERS and TELEGRAM_GROUP_ALLOWED_USERS not authorizing additional users in supergroups (even after disabling Privacy Mode) #55462 contains the group-side work plus the reported DM fallback and alias support; consistent with its maintainer-bot keep_open verdict, the aliases and canonical-key precedence still need user documentation.
• fix(telegram): authorize TELEGRAM_GROUP_ALLOWED_USERS in group/forum chats (#55462) #55624 [closed] partial — (+605/-49) — n/a: This closed group-side variant remains relevant as a duplicate of fix(telegram): honor group user allowlists #55496, but its diff also bundles generated artifacts and incorrectly nests the callback authorization method.
• fix(gateway): honor group_allow_from for Telegram group/forum chats #57189 [closed] partial — (+17/-3) — n/a: This closed PR unions allow_from with group_allow_from at intake and remains relevant as a narrow duplicate explicitly superseded by the more complete fix(telegram): honor group user allowlists #55496/fix(telegram): honor DM env allowlist when yaml allow_from is set #55529 path.
• test(telegram): pin comma-string allow_from / group_allow_from in adapter auth #63350 related — (+56/-0) — n/a: After main independently gained scalar coercion, this test-only diff retains focused comma-string, whitespace, group-scope, and wildcard regression coverage rather than a production fix.
• fix(telegram): accept scalar allowlist configuration #64473 duplicate — (+35/-2) — n/a: The diff adds integer and CSV scalar handling, but the contributor keep_open review identifies a tuple/set regression caused by narrowing existing iterable handling to lists.
• fix(telegram): don't let allow_from short-circuit group authorization (#68716) #68784 [closed] best fix — (+381/-38) — n/a: Although recorded as a best fix for [Bug] Telegram adapter allow_from short-circuits group-scoped authorization #68716, this closed deferral-based implementation remains relevant as a verified predecessor superseded when current main redesigned intake to select allowlists by chat type.
• fix(telegram): preserve group authorization union #69617 [closed] partial — (+133/-37) — n/a: This closed union implementation remains relevant as the direct predecessor of fix(telegram): preserve authorization union across group sources #72084, but its contributor close rationale records that explicit empty config lists rejected existing group-gating cases.
• fix(telegram): preserve authorization union across group sources #72084 best fix — (+738/-67) — n/a: The recorded best fix for [Bug] Telegram adapter allow_from short-circuits group-scoped authorization #68716 extends the authorization union across config, environment, pairing, channels, and multiplex profiles; consistent with its maintainer-bot keep_open verdict, the DIRTY branch needs focused salvage onto the moved intake and runner paths.

Duplicates

#7659 and #17686 are predecessors of merged #17748; #55624 and #57189 duplicate the group-side work contained in #55496/#55529, with #55529 adding the complementary DM path. #63350 overlaps #64473 on CSV scalar coverage, while #68784 and #69617 are superseded approaches to the union problem carried forward more comprehensively by #72084.

Suggested consolidation

Keep #55529 open with a salvage path: preserve its combined group/DM fix, repair the explicit-empty fail-closed case identified on #55496, and add the alias and precedence documentation required by its maintainer-bot review. Close #55496 as duplicate of #55529 despite the keep_open review on #55496, because #55529's visible diff contains the same group/forum ordering and alias changes plus the missing DM fallback, while retaining the review's empty-list requirement for the consolidated patch; keep #63350 open for its focused regression tests, require author action on #64473 to preserve tuple/set iterable normalization, and require author action on recorded best fix #72084 to rebase onto main or split out the current intake/runner union and multiplex profile-scope work.

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 P63350 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.

[Rival]

Rebased onto main (87fd0ed25). main restructured this test file substantially — 6b81590c5 pruned most of the _is_user_authorized_from_message family (_wildcard, _no_from_user, _callback) plus test_unknown_dm_with_no_allowlist_passes_to_pairing, leaving test_is_user_authorized_from_message_allow_from as the only survivor.

The textual conflict would have re-added those. Rather than resolve it hunk by hunk, I rebuilt the commit on top of the current file so it adds only the four comma-string cases, immediately after the surviving list-form test:

• allow_from comma-string through the DM branch
• group_allow_from comma-string through the group branch
• surrounding whitespace
• bare "*" scalar wildcard

Diff is now +56/-0 — purely additive, no lines of yours touched.

Negative-checked: making _coerce_allow_set iterate a scalar char-wise fails the first three (the wildcard case correctly survives — a single "*" character iterated char-wise is still {"*"}, so it guards a different property). Full file green at 15 passed.

Still tests-only, as described in the earlier comment — the production fix this PR originally carried became a no-op once _coerce_allow_set was extracted, and remains dropped.