fix(telegram): admin @username resolution, store convergence, group-invite policy enforcement, privacy-mode warning#181
Merged
Conversation
…esolvers resolveTelegramAdmins (connect path, strict): numeric IDs pass through, @usernames resolve to numeric IDs via getChat — an unresolvable handle is an error, never stored raw. The gateway matches TELEGRAM_ALLOWED_USERS against numeric sender IDs verbatim, so a raw @handle silently locks the admin out. Takes an explicit bot token because at connect time TELEGRAM_BOT_TOKEN may not be in the agent .env yet. expandTelegramAllowlist (settings path, best-effort): mirrors expandSignalAllowlist — keeps each entry and appends its resolved numeric ID; failures pass through unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
syncFromAllowlist: after an operator write to a surface's ALLOWED_USERS env var, replace the explicit admin overlay (when one exists) with the valid identities from the new allowlist, so the policy plane the swarm_map_policy plugin queries never diverges from the env. Invalid entries (unresolved @Handles, wildcards) are dropped, never stored. With no explicit overlay the bootstrap default already reads the env live, so sync is a no-op there. approveGroupInvite: enforcement for the group-invite policy — allow-all approves anyone; approved-only (or unset, the secure default) approves only when the inviting user is an admin (fail-closed isAdmin). Approval appends the structurally-validated groupId to the platform's group allowlist env var (wildcard/already-listed short-circuit as a no-write approval). Audit-logged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…up-approval endpoint Connect route: config.adminUser now supports multiple comma-separated entries; telegram entries are resolved to numeric IDs (strict — an unresolvable @handle 400s with an inline-surfaceable error instead of being stored raw and never matching). Display names are persisted to resolved-identities.json like the settings path, and the policy-plane admin overlay is synced with the allowlist just written. Settings PUT: telegram allowlists are expanded with resolved numeric IDs (expandTelegramAllowlist, same pattern as the Signal UUID expansion) and the admin overlay is synced after the env write. GROUP_INVITE_VARS gains telegram: TELEGRAM_GROUP_INVITE_POLICY — the existing Group Invite Policy toggle previously wrote only signal + slack vars, so it was inert for Telegram. New POST /api/harnesses/:id/surfaces/:platform/groups/:groupId with body { addedByUserId } for the swarm_map_policy plugin: approves/rejects a group invite per the invite policy + admin check, appends approved groups to the group allowlist, and recreates the container so the env takes effect (contract documented in the route). Exempted from the operator-cookie gate via AGENT_CALLABLE_POST_PATHS (agents cannot obtain the cookie); trust model documented in middleware.ts. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Connect dialog (telegram): the getMe verification now also reads can_read_all_group_messages; when false, a non-blocking warning explains that Group Privacy is ON, the bot will not see regular group messages, how to disable it (BotFather -> /setprivacy -> Disable), and that the bot must be removed/re-added to existing groups after changing it. Admin field accepts multiple comma-separated entries and both fields (connect dialog + settings Admins TagInput) now say entries may be numeric Telegram user IDs or @usernames, with @usernames resolved to IDs automatically. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nbounded actor id Audit findings on this PR (independent adversarial pass): - F1 BLOCKING: groupId containing $/backtick passed structural validation and hit String.replace replacement-pattern expansion, splicing preceding .env content (incl. TELEGRAM_BOT_TOKEN line) into the allowlist line. Fixed both ways: $/backtick added to the reject class AND replacer functions so the value is always inserted literally (also applied to the settings-route env write, same class). Regression test reproduces the exact exploit input. - F2: settings PUT with an allow-all/empty telegram allowlist called syncFromAllowlist([]), wiping an explicitly-configured surfaceAdmins overlay on an unrelated DM-policy toggle. Overlay now only converges against a concrete non-empty allowlist. - F4: addedByUserId capped at 128 chars (audit-log spam guard). Full suite: 832 passed / 0 failed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
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.
Fixes four Telegram connector gaps in HSM.
1. Connect-path admins: @usernames resolved, multiple admins supported
The connect route previously wrote
config.adminUserverbatim intoTELEGRAM_ALLOWED_USERS— a typed@handlewas stored raw and never matched the numeric sender IDs the gateway compares against, silently locking the admin out.surfaces/connect): entries are split,@handlesresolved to numeric IDs viaresolveTelegramAdmins(strict — an unresolvable handle returns 400, surfaced inline in the dialog; nothing raw is ever stored). The bot token from the request payload is used, sinceTELEGRAM_BOT_TOKENmay not be in the agent.envyet at connect time.resolved-identities.json(merged, same shape as the settings path).expandTelegramAllowlist, mirroring the Signal phone→UUID expansion).2. Admin-store convergence
TELEGRAM_ALLOWED_USERS(env bootstrap) and theSurfaceAdminServiceoverlay (harnesses.json → surfaceAdmins, served live to the policy plugin) could diverge. Both the connect route and settings PUT now callSurfaceAdminService.syncFromAllowlist, which replaces the explicit overlay (when one exists) with the valid numeric identities from the freshly-written allowlist. Invalid entries (unresolved@handles, wildcards) are dropped by validation — never stored. With no explicit overlay, the bootstrap default already reads the env live, so no sync is needed andsetAdmins's bootstrap-actor semantics are preserved.3. Telegram group-invite policy is no longer inert
GROUP_INVITE_VARSgainstelegram: 'TELEGRAM_GROUP_INVITE_POLICY'— the existing Group Invite Policy toggle now actually writes it (approved-only/allow-all, same values as signal/slack).swarm_map_policyagent plugin (contract below).AGENT_CALLABLE_POST_PATHS(agents cannot obtain the cookie — same rationale as the existing agent-read allowlist). Trust model documented inmiddleware.ts: the caller self-reportsaddedByUserId, so blast radius is bounded — the handler can only append one structurally-validated groupId to that surface's group allowlist, still gated by policy + admin check, and audit-logged.4. Privacy-mode warning in the connect flow
The dialog's
getMeverification now readscan_read_all_group_messages; whenfalsea non-blocking warning explains Group Privacy is ON, the bot won't see regular group messages, how to disable it (BotFather →/setprivacy→ Disable), and that the bot must be removed/re-added to existing groups after changing it.Plus format hints on both the connect-dialog admin field and the settings Admins TagInput (numeric IDs or @usernames; @usernames auto-resolved).
New plugin contract (consumed by a hermes-agent-mt follow-up)
Responses (200 unless the request is malformed):
{ "approved": true, "restarted": <bool> }(restarted:false= env written but container recreate failed, e.g. no compose file yet)*wildcard or already listed){ "approved": true, "already_allowed": true }{ "approved": false, "reason": "<why>" }— plugin should leave the groupaddedByUserId, no agent.env)400 { "error": "<what>" }Policy semantics:
allow-all→ approve for anyone;approved-onlyor unset (secure default) → approve only whenaddedByUserIdis an admin (SurfaceAdminService.isAdmin, fail-closed). No auth cookie required on this one path (seeAGENT_CALLABLE_POST_PATHS).Test results
vitest run: 85 files, 829 tests passed (43 new: resolver strict/best-effort matrices, connect-route resolution + multi-admin normalization + 400-on-unresolvable, store convergence,approveGroupInvitepolicy × admin matrix, POST route responses, middleware gate exemptions, privacy-mode flag).eslint .: identical to baseline (72 pre-existing errors, none introduced).next build: passes.Noted but deliberately not fixed
addedByUserIdon the new POST is self-reported by the agent — the server cannot verify who performed the platform-side invite. Documented inmiddleware.ts; consistent with the trust already extended to the plugin's is_admin/is_group_allowed reads.@handlesin the env alongside the appended numeric IDs (display parity with the settings GET merge; harmless — they just never match).🤖 Generated with Claude Code