Sync with live vault: 28 new files, 7 imports, 16 updates
This commit is contained in:
@@ -0,0 +1,47 @@
|
||||
# Hermes Telegram Picker — Audit
|
||||
|
||||
> Deep-dive findings from a focused code audit of the Telegram inline-keyboard picker.
|
||||
> Component: `hermes-agent/plugins/platforms/telegram/adapter.py` (gateway run.py orchestrates).
|
||||
> Audit date: 2026-08-29.
|
||||
|
||||
## Scope
|
||||
- Model picker (`send_model_picker` → `_handle_model_picker_callback`; callbacks `mp:`/`mpg:`/`mpv:`/`mm:`/`mc:`/`mb`/`mx`/`mg:`)
|
||||
- Choice picker (`send_choice_picker` → `_handle_choice_picker_callback`; callback `cp:`)
|
||||
- Shared pagination core `_format_choice_page` in `gateway/platforms/base.py` (~line 4333)
|
||||
|
||||
## Finding (broken / inconsistent)
|
||||
- **[BUG — auth gap]** `_handle_model_picker_callback` performs **NO** `_is_callback_user_authorized` gate.
|
||||
Every other state-flipping inline-button path in the same adapter gates:
|
||||
- choice picker (`_handle_choice_picker_callback`, line ~6673) — gated
|
||||
- approval buttons (`ea:`, lines ~7238 / ~7418) — gated
|
||||
- slash-confirm (`sc:`, ~7318) — gated
|
||||
- clarify (offer_job / scan, ~7531 / ~7602) — gated
|
||||
- Only the model picker is ungated.
|
||||
- Consequence: in a shared group/supergroup/forum, **any** user who can reach the picker message
|
||||
can tap buttons and switch the session's active model (global-ish config flip) without being
|
||||
an allowed user. The choice-picker comment explicitly states the intent: "unauthorized users in
|
||||
a shared group must not flip session/config state via someone else's picker message" — the model
|
||||
picker violates exactly that rule, for the highest-impact state.
|
||||
- Severity: medium (needs a group context + picker message reachable; DMs are single-user so only
|
||||
group/forum/channel contexts are exposed). A security inconsistency, not an outage.
|
||||
|
||||
## Non-issues confirmed (looked, OK)
|
||||
- Callback prefix routing `("mp:","mpg:","mpv:","mm:","mc:","mb","mx","mg:")` — ordering safe;
|
||||
`mpg:` does not collide with `mp:` (different 2nd char), `mc:` present, no prefix collision.
|
||||
- `_format_choice_page` pagination math (clamp, slice, `(N–M of T)` suffix) is correct.
|
||||
- `_build_model_keyboard` uses global `abs_idx = start + i` so `mm:` maps correctly across pages.
|
||||
- `mx:noop` page-counter button handled by the catch-all branch.
|
||||
- Choice-picker button indices `cp:{i}` use the flat local enumerate — correct for a single level.
|
||||
|
||||
## Recommended fix
|
||||
Add the same `_is_callback_user_authorized` gate at the top of `_handle_model_picker_callback`
|
||||
(mirror the choice-picker gate at line ~6673). Gate before mutating `_model_picker_state`.
|
||||
|
||||
## Owner / status
|
||||
- **✅ FIXED (2026-08-29)** — committed `38622dc` to local hermes-agent checkout.
|
||||
Fix is **structural / regression-proof**: moved the auth gate to the single dispatch
|
||||
point in `_handle_callback_query` (telegram adapter.py ~7199), covering ALL picker
|
||||
callback prefixes (`mp:`/`mpg:`/`mpv:`/`mm:`/`mc:`/`mb`/`mx`/`mg:`/`cp:`) in one place.
|
||||
Any future picker callback prefix added there is auto-protected. `py_compile` OK.
|
||||
- **Activation pending**: running gateway PID 22864 loaded pre-patch code; will pick up
|
||||
the fix on its next (supervisor) restart. Do NOT force-kill mid-conversation.
|
||||
Reference in New Issue
Block a user