Conversation
The bot never registered its commands, so Telegram's in-app "/" menu button
had nothing to show and the commands were only discoverable by reading
/mp_help. Register the list with setMyCommands so commands are tappable.
Lists are scoped to mirror the role model in _process_cmd rather than
exposing the whole admin surface to every user:
- default scope -> 5 public self-service commands
- admin/superadmin chats via admins.conf -> 18 command control plane
- root chat and superadmin admins -> those 18 plus the four commands
gated on the superadmin role (/mp_remove, /mp_restart, /mp_update,
/mp_lockdown)
The command tables live once, in the manager. The generated bot daemon
re-runs `mtproxymax telegram sync-commands` on boot instead of carrying its
own copy, so the menu self-heals and picks up newly added admins. Syncing is
best-effort throughout: a Telegram outage must never break setup or the poll
loop, and revoking an admin calls deleteMyCommands so a stale admin menu is
not left behind.
Also add an explicit `telegram sync-commands` subcommand for manual re-sync.
Foundation for interactive inline-keyboard menus. No user-visible change:
nothing attaches a keyboard yet, and the callback dispatcher is a stub.
Additions (all inside the generated bot daemon, which is self-contained
because the heredoc is quoted and inherits nothing from the manager):
- Bot API primitives: _tg_post_method as a single curl chokepoint, plus
tg_send_kb/tg_send_to_kb/tg_edit/tg_edit_markup/tg_answer_cb. reply_markup
travels as an ordinary urlencoded form field, so no Content-Type header and
no temp file, and the token stays out of the process list.
- Message chunking (_tg_chunk_text/_tg_send_pieces). Telegram caps a message
at 4096 units and bot messages are built by appending one line per secret,
so a large enough fleet produced a message that failed to send outright. A
400 from a malformed Markdown entity now retries without parse_mode,
costing the formatting instead of the whole message.
- callback_data codec (_cb_enc/_cb_dec/_cb_label_ok). The cap is 64 bytes and
one over-long payload makes Telegram reject the entire reply_markup, so the
encoder refuses rather than truncating — a truncated payload would decode
into a different, still-valid target.
Fixes two latent bugs in the getUpdates path:
- The no-python3 fallback extracted text and chat id in two independent
grep|tail passes and paired them by position, so a batch lost every update
but the last and could pair one update's text with another's chat id.
- callback_query updates were never parsed. The extractor read
r.get('message',{}), which is empty for a callback, so _process_cmd ran on
empty input, wrote the offset, and the callback was confirmed and never
redelivered.
The replacement awk extractor is a character scanner rather than a regex
pass, since a regex cannot tell whether a brace or quote sits inside a string
literal — a command's text can contain both. Both extractors now emit
identical records, and the tests assert that byte-for-byte, because the awk
path is the only one available without python3 (notably on Alpine, which this
project supports via OpenRC).
Also makes _tg_have_python probe by executing rather than by `command -v`:
the Windows Store ships a python3.exe alias that is on PATH but fails when
run. And drops the offset write from _process_cmd into _consume_updates, so
it advances per consumed record and a parser failure mid-batch redelivers the
tail instead of losing it.
Tapping now navigates: a hub, a paginated user list, a per-user detail card with its own actions, and a confirmation step before anything destructive. All navigation state travels in callback_data, so there is no server-side session store and an old message's buttons keep working. Security model: callback_data is entirely attacker-controlled — a user can send any payload they like — so every tap is re-authorised against the same rules _process_cmd applies. TG_CB_CAPS is the single source of truth for "what capability does this action need", and BOTH the renderer (which buttons to show) and the enforcer (whether to act) read it, so the two cannot drift. A table-driven test fails if it stops matching _process_cmd's gates. The four refusals mirror _process_cmd exactly: public actions run before any role check, an unauthenticated chatter is answered but ignored silently, a recognised-but-underprivileged role is refused loudly and audited, and an unrecognised role string fails closed to public-only (admins.conf is a plain file an operator can hand-edit). Answering the callback is structural rather than a discipline: _cb_dispatch never answers and _process_callback always does, exactly once, on every path including denial, a stale menu and an unknown namespace. Without that the client spins forever. Also attaches a role-filtered button bar to /start, /mp_status, /mp_secrets, /mp_traffic and /mp_help. Inline buttons carry their message implicitly via callback_query.message.message_id, so a reply becomes a live dashboard with no stored message id. Fixes a pagination bug found while testing: `local _i _start=$(( page * per )) _end=$(( _start + per ))` expands _start against the OUTER scope, so _end came out as per rather than start+per and every page after the first rendered empty. tests/test_telegram_reseller_rbac.sh gains stubs for the two new senders and exposes the menu block, since /start now routes through tg_send_to_kb and builds its button bar via _tg_button_bar.
Two pieces of reply plumbing, both in service of making the bot usable from
buttons and readable on a phone.
Pending input
-------------
Inline buttons cannot collect typed text, so a flow that needs a value arms a
prompt for that chat and takes the next plain message from it as the answer.
State is one line per chat in relay_stats/.tg_pending; the daemon is its only
reader and only writer, so the rewrite needs no lock.
The consumption rule is the part that has to be right, because a stale or
misfiled entry would swallow a user's next real message:
- a slash command always escapes, and clears the prompt on the way out;
- re-arming replaces rather than appends;
- an expired row is dropped on read;
- the answer runs as the sender, after the role lookup.
The first consumer is the "add user" flow, which validates the label before it
reaches the CLI and hands back a connect link built from the secret it created.
No code box
-----------
Every fenced site wrapped CLI output in ```…```, which Telegram draws as a
monospace box with a copy button — the wrong shape for a status reply.
The replacement is one line per fact. Padding is not available as an alignment
mechanism: printf "%-14s" pads with letters, and letters do not have a uniform
advance width in a proportional font, so padded columns collapse outside a
fence. Block-element glyphs are a different case and stay inline.
Two fixes fall out of reading vouchers.conf instead of the CLI's padded table:
- /mp_voucher create showed the WRONG rows. It read the active list back and
did `tail -n +3`, which skipped the first voucher and re-announced an
existing code instead of the one just generated. It now snapshots the
active count and reports only the rows past it.
- /mp_fleet rendered one block per node rather than a padded table, so the
columns survive without a fence.
Adds the manage card behind a user (u:m) with a picker per limit field —
quota, connections, IPs, expiry and the monthly quota-reset day — and a
template picker that applies a saved template to that secret.
The write path is the part that matters. The obvious verb for "set the quota"
is `secret setlimits <label> <conns> <ips> <quota> <expires>`, but
secret_set_limits reads "0" as UNLIMITED rather than "leave alone", so tapping
"quota: 10G" through it would silently clear the connection and IP caps too.
Every commit goes through the per-field `secret setlimit <label> <field>
<value>` form instead, and the tests pin that a quota change never calls
setlimits at all.
Values arrive through callback_data, which is attacker-controlled, and the
label can also come back out of secrets.conf, which is hand-editable, so both
are re-validated before they reach the CLI.
Two smaller things worth naming:
- Relative expiry ("+30d") and "never" mean different CLI verbs — `secret
extend` does the calendar arithmetic, `secret setlimit expires 0` clears
the date — so one picker drives both.
- A custom value the presets do not cover arms a pending prompt whose verb is
the same one the preset commits through, so the typed value lands on
exactly the same validated path rather than a second, laxer one.
_kb_spec drops a button whose payload would exceed 64 bytes: one oversized
payload makes Telegram reject the whole keyboard, so a missing button beats a
card that will not render. The template picker names those templates in the
body instead of letting them vanish silently.
Completes the parity goal: an operator should never have to remember a command.
The hub gains Server, Templates and Tools sections, and the views behind them
cover what /mp_digest, /mp_upstreams, /mp_fleet, /mp_voucher, /mp_update,
/mp_add, /mp_broadcast, /mp_restart, /mp_update and /mp_lockdown used to be the
only route to.
Global verbs. Rotate-all, restart, update and lockdown act on the server rather
than on one secret, so they have no label. They carry "_" as a placeholder, and
that placeholder is matched ONLY for these four — a secret genuinely called "_"
is legal and keeps its enable/disable/rotate/remove verbs. All four go through a
confirmation; the confirm tap is the only thing that performs the write, and the
test pins that the first tap runs nothing.
Templates are now editable, not just applicable. templates.conf rows are
"name|conns|ips|quota|expires|notes" and the CLI's `template save` overwrites by
name, so an edit is a read-modify-write that must carry the fields it did not
touch — notes and expiry being the easiest two to lose, since they are the last
columns. The rebuild reads and writes in one call for that reason.
/help is now rendered from `telegram commands`, which prints the same TG_CMDS_*
lists that are registered with Telegram. The old view was a hand-maintained
string that had already drifted: it advertised commands that no longer existed
and missed ones that did.
Two bugs found while wiring this up, both silent:
- /mp_digest called load_ssl_config, load_speed_limits and
load_cloud_backup_config, which are manager-only. Inside the daemon they are
command-not-found under 2>/dev/null, so the digest advertised an SSL Shield
and a Cloud Backup status it could never read. The new digest view shows
only the settings the daemon genuinely loads.
- _kb_can was written where _cb_can was meant in the template editor, so the
Apply and Delete buttons never rendered at all.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
WIP, I will provide screenshots after I finish all the tests and improvements
Fixes #138
Related PR list:
feat(telegram): interactive inline-keyboard menus
Every action currently requires typing a command with hand-supplied arguments —
/mp_remove alice,/mp_setlimit alice 5 2 10G. There are no buttons anywherein the codebase. This adds them.
What it looks like
Tapping 👥 Users opens a paginated list; tapping a user opens a card with that
user's live state and its own actions attached:
Tapping 📈 Traffic opens a view with 24h / 7d / 30d windows, and the ordinary
command replies (
/start,/mp_status,/mp_secrets,/mp_traffic,/mp_help) gain a button bar so the same views are reachable without typing.Security model
callback_datais entirely attacker-controlled — a user can send any payloadthey like, from any client. So every tap is re-authorised against exactly the
rules
_process_cmdapplies, and a button must never grant more than typingthe equivalent command would.
TG_CB_CAPSis the single source of truth for "what capability does this actionneed", and both the renderer (which buttons to show) and the enforcer
(whether to act) read it, so the two cannot drift apart. A table-driven test
fails if it ever stops matching
_process_cmd's gates.The four refusals mirror
_process_cmdexactly:none(unauthenticated)SECURITYline inaudit.logThat last one matters:
admins.confis a plain file an operator can hand-edit,so anything that is not exactly
superadminorresellermust never exceedpublic.
a:remove/c:removerequire superadmin because/mp_removere-checkssuperadmin;
a:rotateonly requires admin because/mp_rotatehas no extragate.
Additional properties:
callback_data. Identity comes fromcallback_query.message.chat.id, which Telegram authenticates. A chat id inthe payload would be an attacker-controlled privilege token.
the
secrets.confcharset and checked to still exist before anything runs,so a stale menu cannot act on a renamed or deleted secret.
_cb_dispatchnever answers and_process_callbackalways does, exactly once, on every path — includingdenial, a stale menu and an unknown namespace. Without that the client spins
forever. A test asserts exactly one
answerCallbackQueryper dispatch acrossnine different payloads.
Destructive actions
The
a:namespace only renders a confirmation; onlyc:executes. Tapping⏸ Disable asks, and a second tap on ✅ Yes, disable performs it — nothing
destructive is one mis-tap away. The confirmation states the consequence
("This immediately disconnects 3 active session(s)").
Statelessness
All navigation state lives in
callback_data; there is no server-side sessionstore. Back-navigation carries the origin page forward into the detail payload
(
u:s:alice:2), which costs 2 bytes and means Back returns to the page you camefrom. The practical payoff: inline buttons carry their message implicitly via
callback_query.message.message_id, so a periodic report sent once with akeyboard immediately becomes a live dashboard with no stored message id.
Testing
198 new assertions, 0 failures.
The dispatcher test extracts
_process_callbackand_cb_dispatchfrom thegenerated daemon (via a marker-delimited block) so it exercises what ships,
not a copy. It covers: reseller denial + audit, silent handling of
none,fail-closed handling of five unrecognised role strings, the confirmation flow,
stale labels, injected payloads, and the capability-parity table.
The render test checks every view for every role: valid markup shape, balanced
brackets, every
callback_data≤64 bytes and decodable, no row exceeding eightbuttons, no body exceeding 4096, pagination boundaries, and that a role never
sees a button it cannot use.
I also verified the guards actually bite rather than passing vacuously: weakening
a:removetoadminfails the parity test, and breaking the silent-deny forrole nonefails the audit test.A bug this caught
Pagination rendered empty beyond the first page:
local _i _start=$(( page * per )) _end=$(( _start + per ))expands_startagainst the outer scope, so
_endcame out asperrather thanstart + per. Fixed by declaring the variables separately. Pinned by the rendertest.
Files
mtproxymax.sh— daemon heredoc:TG_CB_CAPS, dispatcher, menu views, button bars on five existing repliestests/test_telegram_callback_dispatch.sh— newtests/test_telegram_menu_render.sh— newtests/test_telegram_reseller_rbac.sh— stubs the two new senders and exposes the menu block, since/startnow routes throughtg_send_to_kb