From e56b0cf1a04f992ac6ebc775899f48ea31687640 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Thu, 26 Feb 2026 01:02:36 +0100 Subject: [PATCH] fix: enforce telegram reaction authorization --- CHANGELOG.md | 1 + docs/channels/telegram.md | 1 + src/telegram/bot-handlers.ts | 187 +++++++++++++++++++++++++---------- src/telegram/bot.test.ts | 125 +++++++++++++++++++++++ 4 files changed, 260 insertions(+), 54 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fb4c5ec1afc..6d5bd81477d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ Docs: https://docs.openclaw.ai - Slack/Session threads: prevent oversized parent-session inheritance from silently bricking new thread sessions, surface embedded context-overflow empty-result failures to users, and add configurable `session.parentForkMaxTokens` (default `100000`, `0` disables). (#26912) Thanks @markshields-tl. - Security/Signal: enforce DM/group authorization before reaction-only notification enqueue so unauthorized senders can no longer inject Signal reaction system events under `dmPolicy`/`groupPolicy`; reaction notifications now require channel access checks first. This ships in the next npm release (`2026.2.25`). Thanks @tdjackey for reporting. +- Security/Telegram reactions: enforce `dmPolicy`/`allowFrom` and group allowlist authorization on `message_reaction` events before enqueueing reaction system events, preventing unauthorized reaction-triggered input in DMs and groups; ships in the next npm release (`2026.2.25`). Thanks @tdjackey for reporting. - Security/macOS beta onboarding: remove Anthropic OAuth sign-in and the legacy `oauth.json` onboarding path that exposed the PKCE verifier via OAuth `state`; this impacted the macOS beta onboarding path only. Anthropic subscription auth is now setup-token-only and will ship in the next npm release (`2026.2.25`). Thanks @zdi-disclosures for reporting. - Security/Nextcloud Talk: drop replayed signed webhook events with persistent per-account replay dedupe across restarts, and reject unexpected webhook backend origins when account base URL is configured. Thanks @aristorechina for reporting. - Security/Gateway: harden `agents.files` path handling to block out-of-workspace symlink targets for `agents.files.get`/`agents.files.set`, keep in-workspace symlink targets supported, and add gateway regression coverage for both blocked escapes and allowed in-workspace symlinks. Thanks @tdjackey for reporting. diff --git a/docs/channels/telegram.md b/docs/channels/telegram.md index 6a454bd8dcf..46db95202b4 100644 --- a/docs/channels/telegram.md +++ b/docs/channels/telegram.md @@ -553,6 +553,7 @@ curl "https://api.telegram.org/bot/getUpdates" Notes: - `own` means user reactions to bot-sent messages only (best-effort via sent-message cache). + - Reaction events still respect Telegram access controls (`dmPolicy`, `allowFrom`, `groupPolicy`, `groupAllowFrom`); unauthorized senders are dropped. - Telegram does not provide thread IDs in reaction updates. - non-forum groups route to group chat session - forum groups route to the group general-topic session (`:topic:1`), not the exact originating topic diff --git a/src/telegram/bot-handlers.ts b/src/telegram/bot-handlers.ts index e4d42cd889e..e7660717293 100644 --- a/src/telegram/bot-handlers.ts +++ b/src/telegram/bot-handlers.ts @@ -507,6 +507,99 @@ export const registerTelegramHandlers = ({ return false; }; + const isTelegramEventSenderAuthorized = async (params: { + chatId: number; + chatTitle?: string; + isGroup: boolean; + isForum: boolean; + messageThreadId?: number; + senderId: string; + senderUsername: string; + enforceDirectAuthorization: boolean; + enforceGroupAllowlistAuthorization: boolean; + deniedDmReason: string; + deniedGroupReason: string; + groupAllowContext?: Awaited>; + }) => { + const { + chatId, + chatTitle, + isGroup, + isForum, + messageThreadId, + senderId, + senderUsername, + enforceDirectAuthorization, + enforceGroupAllowlistAuthorization, + deniedDmReason, + deniedGroupReason, + groupAllowContext: preResolvedGroupAllowContext, + } = params; + const dmPolicy = telegramCfg.dmPolicy ?? "pairing"; + const groupAllowContext = + preResolvedGroupAllowContext ?? + (await resolveTelegramGroupAllowFromContext({ + chatId, + accountId, + dmPolicy, + isForum, + messageThreadId, + groupAllowFrom, + resolveTelegramGroupConfig, + })); + const { + resolvedThreadId, + storeAllowFrom, + groupConfig, + topicConfig, + effectiveGroupAllow, + hasGroupAllowOverride, + } = groupAllowContext; + if ( + shouldSkipGroupMessage({ + isGroup, + chatId, + chatTitle, + resolvedThreadId, + senderId, + senderUsername, + effectiveGroupAllow, + hasGroupAllowOverride, + groupConfig, + topicConfig, + }) + ) { + return false; + } + + if (!isGroup && enforceDirectAuthorization) { + if (dmPolicy === "disabled") { + logVerbose( + `Blocked telegram direct event from ${senderId || "unknown"} (${deniedDmReason})`, + ); + return false; + } + if (dmPolicy !== "open") { + const effectiveDmAllow = normalizeAllowFromWithStore({ + allowFrom, + storeAllowFrom, + dmPolicy, + }); + if (!isAllowlistAuthorized(effectiveDmAllow, senderId, senderUsername)) { + logVerbose(`Blocked telegram direct sender ${senderId || "unknown"} (${deniedDmReason})`); + return false; + } + } + } + if (isGroup && enforceGroupAllowlistAuthorization) { + if (!isAllowlistAuthorized(effectiveGroupAllow, senderId, senderUsername)) { + logVerbose(`Blocked telegram group sender ${senderId || "unknown"} (${deniedGroupReason})`); + return false; + } + } + return true; + }; + // Handle emoji reactions to messages. bot.on("message_reaction", async (ctx) => { try { @@ -521,6 +614,10 @@ export const registerTelegramHandlers = ({ const chatId = reaction.chat.id; const messageId = reaction.message_id; const user = reaction.user; + const senderId = user?.id != null ? String(user.id) : ""; + const senderUsername = user?.username ?? ""; + const isGroup = reaction.chat.type === "group" || reaction.chat.type === "supergroup"; + const isForum = reaction.chat.is_forum === true; // Resolve reaction notification mode (default: "own"). const reactionMode = telegramCfg.reactionNotifications ?? "own"; @@ -533,6 +630,21 @@ export const registerTelegramHandlers = ({ if (reactionMode === "own" && !wasSentByBot(chatId, messageId)) { return; } + const senderAuthorized = await isTelegramEventSenderAuthorized({ + chatId, + chatTitle: reaction.chat.title, + isGroup, + isForum, + senderId, + senderUsername, + enforceDirectAuthorization: true, + enforceGroupAllowlistAuthorization: false, + deniedDmReason: "reaction unauthorized by dm policy/allowlist", + deniedGroupReason: "reaction unauthorized by group allowlist", + }); + if (!senderAuthorized) { + return; + } // Detect added reactions. const oldEmojis = new Set( @@ -552,12 +664,12 @@ export const registerTelegramHandlers = ({ const senderName = user ? [user.first_name, user.last_name].filter(Boolean).join(" ").trim() || user.username : undefined; - const senderUsername = user?.username ? `@${user.username}` : undefined; + const senderUsernameLabel = user?.username ? `@${user.username}` : undefined; let senderLabel = senderName; - if (senderName && senderUsername) { - senderLabel = `${senderName} (${senderUsername})`; - } else if (!senderName && senderUsername) { - senderLabel = senderUsername; + if (senderName && senderUsernameLabel) { + senderLabel = `${senderName} (${senderUsernameLabel})`; + } else if (!senderName && senderUsernameLabel) { + senderLabel = senderUsernameLabel; } if (!senderLabel && user?.id) { senderLabel = `id:${user.id}`; @@ -567,8 +679,6 @@ export const registerTelegramHandlers = ({ // Reactions target a specific message_id; the Telegram Bot API does not include // message_thread_id on MessageReactionUpdated, so we route to the chat-level // session (forum topic routing is not available for reactions). - const isGroup = reaction.chat.type === "group" || reaction.chat.type === "supergroup"; - const isForum = reaction.chat.is_forum === true; const resolvedThreadId = isForum ? resolveTelegramForumThreadId({ isForum, messageThreadId: undefined }) : undefined; @@ -864,58 +974,27 @@ export const registerTelegramHandlers = ({ groupAllowFrom, resolveTelegramGroupConfig, }); - const { - resolvedThreadId, - storeAllowFrom, - groupConfig, - topicConfig, - effectiveGroupAllow, - hasGroupAllowOverride, - } = groupAllowContext; - const dmPolicy = telegramCfg.dmPolicy ?? "pairing"; - const effectiveDmAllow = normalizeAllowFromWithStore({ - allowFrom: telegramCfg.allowFrom, - storeAllowFrom, - dmPolicy, - }); + const { resolvedThreadId, storeAllowFrom } = groupAllowContext; const senderId = callback.from?.id ? String(callback.from.id) : ""; const senderUsername = callback.from?.username ?? ""; - if ( - shouldSkipGroupMessage({ - isGroup, - chatId, - chatTitle: callbackMessage.chat.title, - resolvedThreadId, - senderId, - senderUsername, - effectiveGroupAllow, - hasGroupAllowOverride, - groupConfig, - topicConfig, - }) - ) { + const senderAuthorized = await isTelegramEventSenderAuthorized({ + chatId, + chatTitle: callbackMessage.chat.title, + isGroup, + isForum, + messageThreadId, + senderId, + senderUsername, + enforceDirectAuthorization: inlineButtonsScope === "allowlist", + enforceGroupAllowlistAuthorization: inlineButtonsScope === "allowlist", + deniedDmReason: "callback unauthorized by inlineButtonsScope allowlist", + deniedGroupReason: "callback unauthorized by inlineButtonsScope allowlist", + groupAllowContext, + }); + if (!senderAuthorized) { return; } - if (inlineButtonsScope === "allowlist") { - if (!isGroup) { - if (dmPolicy === "disabled") { - return; - } - if (dmPolicy !== "open") { - const allowed = isAllowlistAuthorized(effectiveDmAllow, senderId, senderUsername); - if (!allowed) { - return; - } - } - } else { - const allowed = isAllowlistAuthorized(effectiveGroupAllow, senderId, senderUsername); - if (!allowed) { - return; - } - } - } - const paginationMatch = data.match(/^commands_page_(\d+|noop)(?::(.+))?$/); if (paginationMatch) { const pageValue = paginationMatch[1]; diff --git a/src/telegram/bot.test.ts b/src/telegram/bot.test.ts index 03380dbbf62..4a605abb170 100644 --- a/src/telegram/bot.test.ts +++ b/src/telegram/bot.test.ts @@ -832,6 +832,131 @@ describe("createTelegramBot", () => { ); }); + it("blocks reaction when dmPolicy is disabled", async () => { + onSpy.mockClear(); + enqueueSystemEventSpy.mockClear(); + + loadConfig.mockReturnValue({ + channels: { + telegram: { dmPolicy: "disabled", reactionNotifications: "all" }, + }, + }); + + createTelegramBot({ token: "tok" }); + const handler = getOnHandler("message_reaction") as ( + ctx: Record, + ) => Promise; + + await handler({ + update: { update_id: 510 }, + messageReaction: { + chat: { id: 1234, type: "private" }, + message_id: 42, + user: { id: 9, first_name: "Ada" }, + date: 1736380800, + old_reaction: [], + new_reaction: [{ type: "emoji", emoji: "👍" }], + }, + }); + + expect(enqueueSystemEventSpy).not.toHaveBeenCalled(); + }); + + it("blocks reaction in allowlist mode for unauthorized direct sender", async () => { + onSpy.mockClear(); + enqueueSystemEventSpy.mockClear(); + + loadConfig.mockReturnValue({ + channels: { + telegram: { dmPolicy: "allowlist", allowFrom: ["12345"], reactionNotifications: "all" }, + }, + }); + + createTelegramBot({ token: "tok" }); + const handler = getOnHandler("message_reaction") as ( + ctx: Record, + ) => Promise; + + await handler({ + update: { update_id: 511 }, + messageReaction: { + chat: { id: 1234, type: "private" }, + message_id: 42, + user: { id: 9, first_name: "Ada" }, + date: 1736380800, + old_reaction: [], + new_reaction: [{ type: "emoji", emoji: "👍" }], + }, + }); + + expect(enqueueSystemEventSpy).not.toHaveBeenCalled(); + }); + + it("allows reaction in allowlist mode for authorized direct sender", async () => { + onSpy.mockClear(); + enqueueSystemEventSpy.mockClear(); + + loadConfig.mockReturnValue({ + channels: { + telegram: { dmPolicy: "allowlist", allowFrom: ["9"], reactionNotifications: "all" }, + }, + }); + + createTelegramBot({ token: "tok" }); + const handler = getOnHandler("message_reaction") as ( + ctx: Record, + ) => Promise; + + await handler({ + update: { update_id: 512 }, + messageReaction: { + chat: { id: 1234, type: "private" }, + message_id: 42, + user: { id: 9, first_name: "Ada" }, + date: 1736380800, + old_reaction: [], + new_reaction: [{ type: "emoji", emoji: "👍" }], + }, + }); + + expect(enqueueSystemEventSpy).toHaveBeenCalledTimes(1); + }); + + it("blocks reaction in group allowlist mode for unauthorized sender", async () => { + onSpy.mockClear(); + enqueueSystemEventSpy.mockClear(); + + loadConfig.mockReturnValue({ + channels: { + telegram: { + dmPolicy: "open", + groupPolicy: "allowlist", + groupAllowFrom: ["12345"], + reactionNotifications: "all", + }, + }, + }); + + createTelegramBot({ token: "tok" }); + const handler = getOnHandler("message_reaction") as ( + ctx: Record, + ) => Promise; + + await handler({ + update: { update_id: 513 }, + messageReaction: { + chat: { id: 9999, type: "supergroup" }, + message_id: 77, + user: { id: 9, first_name: "Ada" }, + date: 1736380800, + old_reaction: [], + new_reaction: [{ type: "emoji", emoji: "🔥" }], + }, + }); + + expect(enqueueSystemEventSpy).not.toHaveBeenCalled(); + }); + it("skips reaction when reactionNotifications is off", async () => { onSpy.mockClear(); enqueueSystemEventSpy.mockClear();