Add session grant buttons to Telegram approval keyboard (fixes #230) by yablokolabs · Pull Request #231 · AtomicBot-ai/atomic-agent

yablokolabs · 2026-08-22T20:29:22Z

The TUI approval prompt supports granting a category (s) or shell command shape (a) for the remainder of the session, but the Telegram inline keyboard only offered Approve (y) and Deny (n). This forced Telegram users to re-approve every command individually even for repeated safe operations.

Changes

buildKeyboard() now accepts the full ApprovalRequest and conditionally adds:
- s row ("Grant category for session") when canGrantCategory() returns true
- a row ("Grant <shape> for session") when canGrantShape() returns true
handleCallback() now parses s/a callback kinds and passes the matching grant scope through ApprovalGate.resolve()
19 tests pass (3 new: grant category, grant shape, keyboard layout with buttons)

Impact

Uses existing gate infrastructure (recordGrant, canGrantCategory, canGrantShape) — no new logic, just new buttons reaching existing code
Callback data stays well within Telegram's 64-byte limit (appr:<id>:s / appr:<id>:a)
Zero new dependencies

Closes #230

The TUI approval prompt supports granting a category (s) or shell command shape (a) for the remainder of the session, but the Telegram inline keyboard only offered Approve (y) and Deny (n). This forced Telegram users to re-approve every command individually even for repeated safe operations. Add two new inline keyboard rows when applicable: - "Grant category for session" (s) — uses existing canGrantCategory() - "Grant <shape> for session" (a) — uses existing canGrantShape() The ApprovalGate already supports grant scopes via recordGrant(); this change simply wires the bridge callbacks to pass them through resolve(). Fixes AtomicBot-ai#230

sosidudku1 · 2026-08-27T12:41:05Z

Reviewed at f3abe05. Typecheck clean, 19/19 bridge tests pass, and the full channels/approval/tui/cli sweep is green (179 files; the one failure is an unrelated llama-server EACCES sandbox flake in a local-models test).

I verified the security posture against the real ApprovalGate rather than the test mock, and it holds. A forged appr:<id>:s on a trust_config request grants nothing — recordGrant drops it because isGrantableCategory("trust_config") is false, and sessionGrants() stays empty. The feature itself works end to end: granting git auto-approves the next git while rm still prompts. Reusing canGrantCategory/canGrantShape/recordGrant instead of inventing new logic is the right call, and the 64-byte callback_data claim checks out (43 bytes with a UUID).

Four things before merge — one of them worth fixing:

1. The toast claims a grant the gate refused (approval-bridge.ts:238)

grantLabel is derived purely from the callback kind, with no check that the gate actually recorded anything:

const grantLabel = grant === "category" ? " (category granted for session)" : grant === "shape" ? " (shape granted for session)" : "";

Two reproducible cases where the operator is told a standing exception exists when it does not:

trust_config request + appr:<id>:s → gate drops the grant, sessionGrants() is {categories:[],shapes:[]}, toast reads Approved (category granted for session).
shell request with no commandShape (opaque bash -c …, where the shape button is deliberately withheld) + appr:<id>:a → grants stay empty, toast reads Approved (shape granted for session).

No privilege is gained, so this is not a security hole — but it is a false statement about the trust state, on the surface whose whole job is telling the operator what they just authorized. grantConfirmation in app-key-bindings.ts:740 and the CLI at run-agent.ts:156-163 both build the confirmation from the request's real category/shape; this is the one surface that guesses.

2. Grant kinds are not re-validated at decision time (approval-bridge.ts:207)

The TUI (app-key-bindings.ts:871,875) and CLI (run-agent.ts:156,161 — if (answer === "s" && grantCategory)) both re-check canGrantCategory/canGrantShape when handling the answer. The bridge does not, so the entire defence rests on the gate's own recordGrant filter. That filter is solid today, but this surface has no second check if it ever loosens.

The bridge can't call those helpers right now because it doesn't retain the request — PendingState holds only chatId/messageId/cancelTimer. Storing the ApprovalRequest there fixes both this and #1 in ~15 lines: re-validate the scope, and label the toast from request.category / request.commandShape.

3. The exact-match keyboard test never covers the shape row (approval-bridge.test.ts:135)

"sends an inline keyboard with approve, deny, and grant buttons" uses the base req() fixture, which sets no commandShape — so its toEqual over the full inline_keyboard only pins the approve/deny and category rows. The only test that sees a shape button (line 107) asserts button text via toContain and never inspects callback_data. A regression emitting :s instead of :a on the shape row, or the rows in the wrong order, would pass green. Given the file's documented "Locked invariants — pinned by the colocated test file" contract, the new row deserves the same exact-match treatment y/n gets.

4. Unbounded commandShape in the button label (approval-bridge.ts:351)

🔓 Grant "${request.commandShape}" for session has no length bound. An 80-char shape yields a 103-char label; long inline-button text truncates in narrow clients, and an oversize reply_markup rejection would be caught by dispatch's handler and auto-deny the approval. Real argv[0] basenames are short, so this is an edge case, not a routine break — worth a truncation guard if you're touching the file anyway. (Message text has no parse_mode, so there's no markdown-injection path from the quoted shape.)

Happy to see this land once #1 and #3 are addressed.

The grant rows shipped in AtomicBot-ai#231 built their confirmation toast from the button that was pressed, not from what the gate did with it. Two cases said "granted for session" when nothing was recorded: a `trust_config` request answered with `s` (recordGrant refuses that category outright), and a shell request with no commandShape answered with `a` (the shape button is deliberately withheld there, so the payload can only arrive hand-made). No privilege was gained either way — but the toast is the one surface whose whole job is telling the operator what they just authorised, so a false standing exception there is worse than a plain "Approved". PendingState now retains the ApprovalRequest the keyboard was built from, which fixes the label and the missing re-check together: - The scope is re-validated against that request before it reaches the gate, the way app-key-bindings.ts and run-agent.ts already do. The gate's own recordGrant filter is solid, but it was the only check on this path; a kind that no longer applies now degrades to a plain approval instead of riding on that filter. - The toast names the category or shape from the request itself, so "Approved (shell command granted for session)" is a statement about recorded state rather than about which button was pressed. Also bounds commandShape at 32 chars in the button label. Real shapes are argv[0] basenames, but nothing upstream enforces that, and an oversize reply_markup is rejected by Telegram — which dispatch turns into an auto-deny of an approval the operator never saw. Tests: the keyboard exact-match now covers a request that carries a commandShape, so the row order and every callback_data are pinned — previously a shape row emitting `:s` would have passed green. Three new cases drive the bridge against a real ApprovalGate rather than the recording stub, because only the real gate can show whether the toast and the trust state agree. AGENTS.md said "Telegram/ApprovalBridge never grants" and called it a threat-model decision: grants belonged to surfaces with physical machine access. AtomicBot-ai#230 reverses that and the doc had not caught up. It now records what changed (a remote chat can raise trust for one session) and what did not (no trust_config, no hardline bypass, no move of the standing level, still in-memory and session-scoped).

yablokolabs · 2026-08-27T18:48:15Z

Thanks @sosidudku1 — all four addressed in 8b72e7b, plus one thing the review did not cover that I think outranks the rest.

1 + 2 (toast honesty, missing re-check). Fixed together, as you suggested: PendingState now retains the ApprovalRequest. The scope is re-validated against it before it reaches the gate — same check app-key-bindings.ts and run-agent.ts do — and a kind that no longer applies degrades to a plain approval rather than leaning on recordGrant's filter as the only guard. The toast is now built from the request's own category/shape, so it reports recorded state instead of which button was pressed.

One deviation from the literal review text worth calling out: labelling from the request meant the wording changed rather than just becoming conditional. It now names the thing granted — Approved (shell command granted for session), Approved ("git" granted for session) — matching grantConfirmation. Both answerCallbackQuery expectations moved with it. Say the word if you would rather keep the old generic strings and only gate them.

3 (keyboard coverage). The exact-match test now runs on a request that carries a commandShape, so all three rows, their order, and every callback_data are pinned. I checked it actually bites: flipping the shape row to emit :s fails it. The old toContain variant is gone rather than kept alongside — it asserted a strict subset of the same thing.

4 (unbounded shape). SHAPE_LABEL_MAX = 32, applied to the button label and the toast.

Tests also gained three cases that drive the bridge against a real ApprovalGate instead of the recording stub: a trust_config request answered with s, a shapeless shell request answered with a, and the accepted git shape grant. Each asserts sessionGrants() and the toast text together — the stub accepts any decision it is handed, so it structurally cannot catch the class of bug in #1. That is the check that would have failed before this commit.

npm run lint clean; src/channels src/approval src/tui src/cli at 1867/1867. The llama-server EACCES failure you saw is a flake, not a constant: it failed once and passed on re-run, and the file passes 8/8 in isolation. Something in that sweep spawns the real binary out of a shared /tmp dir.

The thing I would not merge without a decision on: this PR reverses a documented threat-model carve-out, and #230 reads as though it were an oversight.

AGENTS.md said it twice, deliberately:

The grant is offered on the TUI ApprovalModal … and the CLI run stdin prompt; both surfaces have physical machine access, matching the level carve-out. Telegram/ApprovalBridge never grants: a remote channel approves per-request only, so a compromised bot token cannot raise session trust any more than it can raise the standing level.

and in locked invariant 9: "Grants are TUI/CLI-local (never from Telegram)."

So the bridge did not "just never wire the extra buttons" — the rule was that grants require physical access to the host, the same rule that keeps /privacy level out of the Telegram inbound-handler. This PR moves session-trust escalation onto the remote surface. The security review here checked that a forged s cannot exceed what the gate allows, which holds; it did not weigh whether a remote operator should be able to record a grant at all, which is the actual decision #230 makes.

For what it is worth I think the feature is defensible — the alternative escape was raising the standing level to 5, which is more permissive and durable, so the grant is the smaller hammer — and the hard limits all survive: no trust_config, no hardline bypass, in-memory, session-scoped, standing level untouched. But it is a posture change, and it should be recorded as one.

I have updated all three places in AGENTS.md to say what changed and what did not. Please read that wording as a proposal rather than a settled decision — it is your call, and if you would rather keep the old rule, the fix is to drop the feature, not the doc.

@sosidudku1

Addresses the four points @sosidudku1 raised reviewing AtomicBot-ai#231 at f3abe05. The grant rows shipped in AtomicBot-ai#231 built their confirmation toast from the button that was pressed, not from what the gate did with it. @sosidudku1 found two cases that said "granted for session" when nothing was recorded: a `trust_config` request answered with `s` (recordGrant refuses that category outright), and a shell request with no commandShape answered with `a` (the shape button is deliberately withheld there, so the payload can only arrive hand-made). No privilege was gained either way — but the toast is the one surface whose whole job is telling the operator what they just authorised, so a false standing exception there is worse than a plain "Approved". PendingState now retains the ApprovalRequest the keyboard was built from, which fixes the label and the missing re-check together, as @sosidudku1 suggested: - The scope is re-validated against that request before it reaches the gate, the way app-key-bindings.ts and run-agent.ts already do. The gate's own recordGrant filter is solid, but it was the only check on this path; a kind that no longer applies now degrades to a plain approval instead of riding on that filter. - The toast names the category or shape from the request itself, so "Approved (shell command granted for session)" is a statement about recorded state rather than about which button was pressed. This changes the wording rather than only gating it, so both answerCallbackQuery expectations move with it. Also bounds commandShape at 32 chars in the button label. Real shapes are argv[0] basenames, but nothing upstream enforces that, and an oversize reply_markup is rejected by Telegram — which dispatch turns into an auto-deny of an approval the operator never saw. Tests: the keyboard exact-match now covers a request that carries a commandShape, so the row order and every callback_data are pinned — previously a shape row emitting `:s` would have passed green, exactly as @sosidudku1 called out. Three new cases drive the bridge against a real ApprovalGate rather than the recording stub, because only the real gate can show whether the toast and the trust state agree. AGENTS.md said "Telegram/ApprovalBridge never grants" and called it a threat-model decision: grants belonged to surfaces with physical machine access. AtomicBot-ai#230 reverses that and the doc had not caught up. It now records what changed (a remote chat can raise trust for one session) and what did not (no trust_config, no hardline bypass, no move of the standing level, still in-memory and session-scoped). Reviewed-by: @sosidudku1

@sosidudku1

Thanks @sosidudku1 for the review at f3abe05 — all four points addressed. The grant rows shipped in AtomicBot-ai#231 built their confirmation toast from the button that was pressed, not from what the gate did with it. @sosidudku1 found two cases that said "granted for session" when nothing was recorded: a `trust_config` request answered with `s` (recordGrant refuses that category outright), and a shell request with no commandShape answered with `a` (the shape button is deliberately withheld there, so the payload can only arrive hand-made). No privilege was gained either way — but the toast is the one surface whose whole job is telling the operator what they just authorised, so a false standing exception there is worse than a plain "Approved". PendingState now retains the ApprovalRequest the keyboard was built from, which fixes the label and the missing re-check together, as @sosidudku1 suggested: - The scope is re-validated against that request before it reaches the gate, the way app-key-bindings.ts and run-agent.ts already do. The gate's own recordGrant filter is solid, but it was the only check on this path; a kind that no longer applies now degrades to a plain approval instead of riding on that filter. - The toast names the category or shape from the request itself, so "Approved (shell command granted for session)" is a statement about recorded state rather than about which button was pressed. This changes the wording rather than only gating it, so both answerCallbackQuery expectations move with it. Also bounds commandShape at 32 chars in the button label. Real shapes are argv[0] basenames, but nothing upstream enforces that, and an oversize reply_markup is rejected by Telegram — which dispatch turns into an auto-deny of an approval the operator never saw. Tests: the keyboard exact-match now covers a request that carries a commandShape, so the row order and every callback_data are pinned — previously a shape row emitting `:s` would have passed green, exactly as @sosidudku1 called out. Three new cases drive the bridge against a real ApprovalGate rather than the recording stub, because only the real gate can show whether the toast and the trust state agree. AGENTS.md said "Telegram/ApprovalBridge never grants" and called it a threat-model decision: grants belonged to surfaces with physical machine access. AtomicBot-ai#230 reverses that and the doc had not caught up. It now records what changed (a remote chat can raise trust for one session) and what did not (no trust_config, no hardline bypass, no move of the standing level, still in-memory and session-scoped). Reviewed-by: @sosidudku1

The grant rows shipped in AtomicBot-ai#231 built their confirmation toast from the button that was pressed, not from what the gate did with it. Two cases said "granted for session" when nothing was recorded: a `trust_config` request answered with `s` (recordGrant refuses that category outright), and a shell request with no commandShape answered with `a` (the shape button is deliberately withheld there, so the payload can only arrive hand-made). No privilege was gained either way — but the toast is the one surface whose whole job is telling the operator what they just authorised, so a false standing exception there is worse than a plain "Approved". PendingState now retains the ApprovalRequest the keyboard was built from, which fixes the label and the missing re-check together: - The scope is re-validated against that request before it reaches the gate, the way app-key-bindings.ts and run-agent.ts already do. The gate's own recordGrant filter is solid, but it was the only check on this path; a kind that no longer applies now degrades to a plain approval instead of riding on that filter. - The toast names the category or shape from the request itself, so "Approved (shell command granted for session)" is a statement about recorded state rather than about which button was pressed. Also bounds commandShape at 32 chars in the button label. Real shapes are argv[0] basenames, but nothing upstream enforces that, and an oversize reply_markup is rejected by Telegram — which dispatch turns into an auto-deny of an approval the operator never saw. Tests: the keyboard exact-match now covers a request that carries a commandShape, so the row order and every callback_data are pinned — previously a shape row emitting `:s` would have passed green. Three new cases drive the bridge against a real ApprovalGate rather than the recording stub, because only the real gate can show whether the toast and the trust state agree. AGENTS.md said "Telegram/ApprovalBridge never grants" and called it a threat-model decision: grants belonged to surfaces with physical machine access. AtomicBot-ai#230 reverses that and the doc had not caught up. It now records what changed (a remote chat can raise trust for one session) and what did not (no trust_config, no hardline bypass, no move of the standing level, still in-memory and session-scoped).

The approval toast was built from the button that was pressed, not from what the gate did with it, so two cases claimed "granted for session" when nothing was recorded: a trust_config request answered with `s`, which recordGrant refuses outright, and a shell request with no commandShape answered with `a`. No privilege was gained, but the toast is the surface whose whole job is telling the operator what they just authorised. PendingState now retains the ApprovalRequest the keyboard was built from. The scope is re-validated against it before it reaches the gate, as app-key-bindings.ts and run-agent.ts already do, and the toast names the category or shape from the request itself. commandShape is bounded at 32 chars in the button label. Real shapes are argv[0] basenames, but nothing upstream enforces that, and an oversize reply_markup is rejected by Telegram — which dispatch turns into an auto-deny of an approval nobody saw. The keyboard exact-match test now covers a request carrying a commandShape, pinning row order and every callback_data. Three new cases drive the bridge against a real ApprovalGate instead of the recording stub, which accepts any decision it is handed. AGENTS.md said Telegram never grants and called it a threat-model decision. It now records what changed — a remote chat can raise trust for one session — and what did not: no trust_config, no hardline bypass, no move of the standing level.

yablokolabs force-pushed the fix/telegram-approval-grant-buttons branch from db9cd95 to cbaca21 Compare August 22, 2026 20:32

yablokolabs force-pushed the fix/telegram-approval-grant-buttons branch from cbaca21 to f3abe05 Compare August 22, 2026 20:34

This was referenced Aug 27, 2026

Telegram approval keyboard missing grant category/shape buttons (s/a) #230

Open

Feature: User-configurable shell guard rules via config.json + /guard menu command #229

Open

yablokolabs force-pushed the fix/telegram-approval-grant-buttons branch from 8b72e7b to 446cdb2 Compare August 27, 2026 18:51

yablokolabs force-pushed the fix/telegram-approval-grant-buttons branch from 446cdb2 to c27698c Compare August 27, 2026 18:53

yablokolabs force-pushed the fix/telegram-approval-grant-buttons branch from c27698c to 60b9206 Compare August 27, 2026 18:54

yablokolabs force-pushed the fix/telegram-approval-grant-buttons branch from 60b9206 to ca2fa1f Compare August 27, 2026 18:57

Navigation Menu

Uh oh!

Add session grant buttons to Telegram approval keyboard (fixes #230) - #231

Add session grant buttons to Telegram approval keyboard (fixes #230)#231
yablokolabs wants to merge 2 commits into
AtomicBot-ai:mainfrom
yablokolabs:fix/telegram-approval-grant-buttons

yablokolabs commented Aug 22, 2026

Uh oh!

sosidudku1 commented Aug 27, 2026

Uh oh!

yablokolabs commented Aug 27, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

yablokolabs commented Aug 22, 2026

Changes

Impact

Uh oh!

sosidudku1 commented Aug 27, 2026

Uh oh!

yablokolabs commented Aug 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

yablokolabs commented Aug 27, 2026 •

edited

Loading