From 1791c7c3049ffb2e07400710b6192f3c37804be8 Mon Sep 17 00:00:00 2001 From: Ayaan Zaidi Date: Sun, 29 Mar 2026 10:26:31 +0530 Subject: [PATCH] fix: unify telegram exec approval auth --- .../telegram/src/bot-handlers.runtime.ts | 12 +--- .../telegram/src/exec-approvals.test.ts | 48 +++++++++++++++ extensions/telegram/src/exec-approvals.ts | 35 ++++++++--- scripts/lib/plugin-sdk-facades.mjs | 2 + src/auto-reply/reply/commands-approve.ts | 25 +++----- src/auto-reply/reply/commands.test.ts | 59 ++++++++++++++++++- src/plugin-sdk/telegram-runtime.ts | 1 + src/plugin-sdk/telegram-surface.ts | 2 + 8 files changed, 147 insertions(+), 37 deletions(-) diff --git a/extensions/telegram/src/bot-handlers.runtime.ts b/extensions/telegram/src/bot-handlers.runtime.ts index da697caca4d..83a3cf3df3b 100644 --- a/extensions/telegram/src/bot-handlers.runtime.ts +++ b/extensions/telegram/src/bot-handlers.runtime.ts @@ -78,9 +78,7 @@ import { } from "./conversation-route.js"; import { enforceTelegramDmAccess } from "./dm-access.js"; import { - isTelegramExecApprovalApprover, - isTelegramExecApprovalClientEnabled, - isTelegramExecApprovalTargetRecipient, + isTelegramExecApprovalAuthorizedSender, shouldEnableTelegramExecApprovalButtons, } from "./exec-approvals.js"; import { @@ -1290,13 +1288,7 @@ export const registerTelegramHandlers = ({ const runtimeCfg = telegramDeps.loadConfig(); if (isApprovalCallback) { - const isExplicitApprover = - isTelegramExecApprovalClientEnabled({ cfg: runtimeCfg, accountId }) && - isTelegramExecApprovalApprover({ cfg: runtimeCfg, accountId, senderId }); - if ( - !isExplicitApprover && - !isTelegramExecApprovalTargetRecipient({ cfg: runtimeCfg, senderId, accountId }) - ) { + if (!isTelegramExecApprovalAuthorizedSender({ cfg: runtimeCfg, accountId, senderId })) { logVerbose( `Blocked telegram exec approval callback from ${senderId || "unknown"} (not an approver)`, ); diff --git a/extensions/telegram/src/exec-approvals.test.ts b/extensions/telegram/src/exec-approvals.test.ts index ef57186202d..6bb38b8f8f4 100644 --- a/extensions/telegram/src/exec-approvals.test.ts +++ b/extensions/telegram/src/exec-approvals.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from "vitest"; import type { OpenClawConfig } from "../../../src/config/config.js"; import { + isTelegramExecApprovalAuthorizedSender, isTelegramExecApprovalApprover, isTelegramExecApprovalClientEnabled, isTelegramExecApprovalTargetRecipient, @@ -164,5 +165,52 @@ describe("telegram exec approvals", () => { isTelegramExecApprovalTargetRecipient({ cfg, senderId: "12345", accountId: "any-account" }), ).toBe(true); }); + + it("requires active target forwarding mode", () => { + const cfg = { + channels: { telegram: { botToken: "tok" } }, + approvals: { + exec: { + enabled: true, + mode: "session", + targets: [{ channel: "telegram", to: "12345" }], + }, + }, + } as OpenClawConfig; + expect(isTelegramExecApprovalTargetRecipient({ cfg, senderId: "12345" })).toBe(false); + }); + + it("normalizes prefixed Telegram DM targets", () => { + const cfg = buildTargetConfig([{ channel: "telegram", to: "tg:12345" }]); + expect(isTelegramExecApprovalTargetRecipient({ cfg, senderId: "12345" })).toBe(true); + }); + + it("normalizes accountId matching", () => { + const cfg = buildTargetConfig([{ channel: "telegram", to: "12345", accountId: "Work Bot" }]); + expect( + isTelegramExecApprovalTargetRecipient({ cfg, senderId: "12345", accountId: "work-bot" }), + ).toBe(true); + }); + }); + + describe("isTelegramExecApprovalAuthorizedSender", () => { + it("accepts explicit approvers", () => { + const cfg = buildConfig({ enabled: true, approvers: ["123"] }); + expect(isTelegramExecApprovalAuthorizedSender({ cfg, senderId: "123" })).toBe(true); + }); + + it("accepts active forwarded DM targets", () => { + const cfg = { + channels: { telegram: { botToken: "tok" } }, + approvals: { + exec: { + enabled: true, + mode: "targets", + targets: [{ channel: "telegram", to: "12345" }], + }, + }, + } as OpenClawConfig; + expect(isTelegramExecApprovalAuthorizedSender({ cfg, senderId: "12345" })).toBe(true); + }); }); }); diff --git a/extensions/telegram/src/exec-approvals.ts b/extensions/telegram/src/exec-approvals.ts index 811270f5574..a9daac8a6df 100644 --- a/extensions/telegram/src/exec-approvals.ts +++ b/extensions/telegram/src/exec-approvals.ts @@ -2,9 +2,10 @@ import { getExecApprovalReplyMetadata } from "openclaw/plugin-sdk/approval-runti import type { OpenClawConfig } from "openclaw/plugin-sdk/config-runtime"; import type { TelegramExecApprovalConfig } from "openclaw/plugin-sdk/config-runtime"; import type { ReplyPayload } from "openclaw/plugin-sdk/reply-runtime"; +import { normalizeAccountId } from "openclaw/plugin-sdk/routing"; import { resolveTelegramAccount } from "./accounts.js"; import { resolveTelegramInlineButtonsConfigScope } from "./inline-buttons.js"; -import { isNumericTelegramChatId, resolveTelegramTargetChatType } from "./targets.js"; +import { normalizeTelegramChatId, resolveTelegramTargetChatType } from "./targets.js"; function normalizeApproverId(value: string | number): string { return String(value).trim(); @@ -47,37 +48,55 @@ export function isTelegramExecApprovalApprover(params: { return approvers.includes(senderId); } -/** Check if sender is an implicit approver via exec approval forwarding targets. */ +function isTelegramExecApprovalTargetsMode(cfg: OpenClawConfig): boolean { + const execApprovals = cfg.approvals?.exec; + if (!execApprovals?.enabled) { + return false; + } + return execApprovals.mode === "targets" || execApprovals.mode === "both"; +} + export function isTelegramExecApprovalTargetRecipient(params: { cfg: OpenClawConfig; senderId?: string | null; accountId?: string | null; }): boolean { const senderId = params.senderId?.trim(); - if (!senderId) { + if (!senderId || !isTelegramExecApprovalTargetsMode(params.cfg)) { return false; } const targets = params.cfg.approvals?.exec?.targets; - if (!targets || !Array.isArray(targets)) { + if (!targets) { return false; } - const accountId = params.accountId?.trim() || undefined; + const accountId = params.accountId ? normalizeAccountId(params.accountId) : undefined; return targets.some((target) => { const channel = target.channel?.trim().toLowerCase(); if (channel !== "telegram") { return false; } - if (accountId && target.accountId && target.accountId !== accountId) { + if (accountId && target.accountId && normalizeAccountId(target.accountId) !== accountId) { return false; } - const to = target.to?.trim(); - if (!to || !isNumericTelegramChatId(to) || to.startsWith("-")) { + const to = target.to ? normalizeTelegramChatId(target.to) : undefined; + if (!to || to.startsWith("-")) { return false; } return to === senderId; }); } +export function isTelegramExecApprovalAuthorizedSender(params: { + cfg: OpenClawConfig; + accountId?: string | null; + senderId?: string | null; +}): boolean { + return ( + (isTelegramExecApprovalClientEnabled(params) && isTelegramExecApprovalApprover(params)) || + isTelegramExecApprovalTargetRecipient(params) + ); +} + export function resolveTelegramExecApprovalTarget(params: { cfg: OpenClawConfig; accountId?: string | null; diff --git a/scripts/lib/plugin-sdk-facades.mjs b/scripts/lib/plugin-sdk-facades.mjs index d4c9aa519f5..5ec7f953564 100644 --- a/scripts/lib/plugin-sdk-facades.mjs +++ b/scripts/lib/plugin-sdk-facades.mjs @@ -897,7 +897,9 @@ export const GENERATED_PLUGIN_SDK_FACADES = [ "inspectTelegramAccount", "InspectedTelegramAccount", "isTelegramExecApprovalApprover", + "isTelegramExecApprovalAuthorizedSender", "isTelegramExecApprovalClientEnabled", + "isTelegramExecApprovalTargetRecipient", "listTelegramAccountIds", "listTelegramDirectoryGroupsFromConfig", "listTelegramDirectoryPeersFromConfig", diff --git a/src/auto-reply/reply/commands-approve.ts b/src/auto-reply/reply/commands-approve.ts index 6ff94e17ec2..7395d5ae102 100644 --- a/src/auto-reply/reply/commands-approve.ts +++ b/src/auto-reply/reply/commands-approve.ts @@ -6,9 +6,8 @@ import { isDiscordExecApprovalClientEnabled, } from "../../plugin-sdk/discord-surface.js"; import { + isTelegramExecApprovalAuthorizedSender, isTelegramExecApprovalApprover, - isTelegramExecApprovalClientEnabled, - isTelegramExecApprovalTargetRecipient, } from "../../plugin-sdk/telegram-runtime.js"; import { GATEWAY_CLIENT_MODES, GATEWAY_CLIENT_NAMES } from "../../utils/message-channel.js"; import { requireGatewayClientScopeForInternalChannel } from "./command-gates.js"; @@ -141,24 +140,14 @@ export const handleApproveCommand: CommandHandler = async (params, allowTextComm senderId: params.command.senderId, }; - const isImplicitTargetApprover = isTelegramExecApprovalTargetRecipient(telegramApproverContext); - - if (!isPluginId) { - const isExplicitApprover = - isTelegramExecApprovalClientEnabled({ cfg: params.cfg, accountId: params.ctx.AccountId }) && - isTelegramExecApprovalApprover(telegramApproverContext); - if (!isExplicitApprover && !isImplicitTargetApprover) { - return { - shouldContinue: false, - reply: { text: "❌ You are not authorized to approve exec requests on Telegram." }, - }; - } + if (!isPluginId && !isTelegramExecApprovalAuthorizedSender(telegramApproverContext)) { + return { + shouldContinue: false, + reply: { text: "❌ You are not authorized to approve exec requests on Telegram." }, + }; } - // Keep plugin-ID routing independent from exec approval client enablement so - // forwarded plugin approvals remain resolvable, but still require explicit - // Telegram approver membership for security parity. - if (isPluginId && !isTelegramExecApprovalApprover(telegramApproverContext) && !isImplicitTargetApprover) { + if (isPluginId && !isTelegramExecApprovalApprover(telegramApproverContext)) { return { shouldContinue: false, reply: { text: "❌ You are not authorized to approve plugin requests on Telegram." }, diff --git a/src/auto-reply/reply/commands.test.ts b/src/auto-reply/reply/commands.test.ts index c18e727c543..cf3aa8e19f0 100644 --- a/src/auto-reply/reply/commands.test.ts +++ b/src/auto-reply/reply/commands.test.ts @@ -361,6 +361,28 @@ describe("/approve command", () => { } as OpenClawConfig; } + function createTelegramTargetApproveCfg( + targets: Array<{ channel: string; to: string; accountId?: string }> = [ + { channel: "telegram", to: "123" }, + ], + ): OpenClawConfig { + return { + commands: { text: true }, + channels: { + telegram: { + allowFrom: ["*"], + }, + }, + approvals: { + exec: { + enabled: true, + mode: "targets", + targets, + }, + }, + } as OpenClawConfig; + } + function createDiscordApproveCfg( execApprovals: { enabled: boolean; @@ -607,7 +629,7 @@ describe("/approve command", () => { SenderId: "123", }, setup: undefined, - expectedText: "Telegram exec approvals are not enabled", + expectedText: "not authorized to approve", expectGatewayCalls: 0, }, { @@ -635,6 +657,27 @@ describe("/approve command", () => { } }); + it("accepts Telegram /approve from active exec forwarding targets", async () => { + const cfg = createTelegramTargetApproveCfg([{ channel: "telegram", to: "tg:123" }]); + const params = buildParams("/approve abc12345 allow-once", cfg, { + Provider: "telegram", + Surface: "telegram", + SenderId: "123", + }); + + callGatewayMock.mockResolvedValue({ ok: true }); + + const result = await handleCommands(params); + expect(result.shouldContinue).toBe(false); + expect(result.reply?.text).toContain("Approval allow-once submitted"); + expect(callGatewayMock).toHaveBeenCalledWith( + expect.objectContaining({ + method: "exec.approval.resolve", + params: { id: "abc12345", decision: "allow-once" }, + }), + ); + }); + it("rejects Telegram plugin-prefixed IDs when no approver policy is configured", async () => { const cfg = createTelegramApproveCfg(null); const params = buildParams("/approve plugin:abc123 allow-once", cfg, { @@ -685,6 +728,20 @@ describe("/approve command", () => { ); }); + it("keeps Telegram plugin-prefixed IDs explicit-only for exec forwarding targets", async () => { + const cfg = createTelegramTargetApproveCfg(); + const params = buildParams("/approve plugin:abc123 allow-once", cfg, { + Provider: "telegram", + Surface: "telegram", + SenderId: "123", + }); + + const result = await handleCommands(params); + expect(result.shouldContinue).toBe(false); + expect(result.reply?.text).toContain("not authorized to approve plugin requests"); + expect(callGatewayMock).toHaveBeenCalledTimes(0); + }); + it("enforces gateway approval scopes", async () => { const cfg = { commands: { text: true }, diff --git a/src/plugin-sdk/telegram-runtime.ts b/src/plugin-sdk/telegram-runtime.ts index 3b1fdffb552..77cc733a4ba 100644 --- a/src/plugin-sdk/telegram-runtime.ts +++ b/src/plugin-sdk/telegram-runtime.ts @@ -15,6 +15,7 @@ export { getModelsPageSize, inspectTelegramAccount, isTelegramExecApprovalApprover, + isTelegramExecApprovalAuthorizedSender, isTelegramExecApprovalClientEnabled, isTelegramExecApprovalTargetRecipient, listTelegramAccountIds, diff --git a/src/plugin-sdk/telegram-surface.ts b/src/plugin-sdk/telegram-surface.ts index 9b4f05c1557..9228f3a227e 100644 --- a/src/plugin-sdk/telegram-surface.ts +++ b/src/plugin-sdk/telegram-surface.ts @@ -11,7 +11,9 @@ export { getModelsPageSize, inspectTelegramAccount, isTelegramExecApprovalApprover, + isTelegramExecApprovalAuthorizedSender, isTelegramExecApprovalClientEnabled, + isTelegramExecApprovalTargetRecipient, listTelegramAccountIds, listTelegramDirectoryGroupsFromConfig, listTelegramDirectoryPeersFromConfig,