From 9514201fb9b51de5d0b23151110d0ff5d9c8bd67 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 24 Feb 2026 23:43:37 +0000 Subject: [PATCH] fix(telegram): block unauthorized DM media downloads --- CHANGELOG.md | 1 + src/telegram/bot-handlers.ts | 34 +++++ src/telegram/bot-message-context.ts | 113 +++------------- src/telegram/bot-native-commands.ts | 1 + src/telegram/bot.create-telegram-bot.test.ts | 127 ++++++++++++++++++ ...s-media-file-path-no-file-download.test.ts | 2 +- src/telegram/bot.ts | 1 + src/telegram/dm-access.ts | 109 +++++++++++++++ 8 files changed, 295 insertions(+), 93 deletions(-) create mode 100644 src/telegram/dm-access.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 354b008b5..ffbb3531c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ Docs: https://docs.openclaw.ai - Config/Plugins: treat stale removed `google-antigravity-auth` plugin references as compatibility warnings (not hard validation errors) across `plugins.entries`, `plugins.allow`, `plugins.deny`, and `plugins.slots.memory`, so startup no longer fails after antigravity removal. (#25538, #25862) Thanks @chilu18. - Security/Message actions: enforce local media root checks for `sendAttachment` and `setGroupIcon` when `sandboxRoot` is unset, preventing attachment hydration from reading arbitrary host files via local absolute paths. This ships in the next npm release. Thanks @GCXWLP for reporting. - Zalo/Group policy: enforce sender authorization for group messages with `groupPolicy` + `groupAllowFrom` (fallback to `allowFrom`), default runtime group behavior to fail-closed allowlist, and block unauthorized non-command group messages before dispatch. This ships in the next npm release. Thanks @tdjackey for reporting. +- Security/Telegram: enforce DM authorization before media download/write (including media groups) and move telegram inbound activity tracking after DM authorization, preventing unauthorized sender-triggered inbound media disk writes. This ships in the next npm release. Thanks @v8hid for reporting. - Security/Workspace FS: normalize `@`-prefixed paths before workspace-boundary checks (including workspace-only read/write/edit and sandbox mount path guards), preventing absolute-path escape attempts from bypassing guard validation. This ships in the next npm release. Thanks @tdjackey for reporting. - Security/Synology Chat: enforce fail-closed allowlist behavior for DM ingress so `dmPolicy: "allowlist"` with empty `allowedUserIds` rejects all senders instead of allowing unauthorized dispatch. This ships in the next npm release. Thanks @tdjackey for reporting. - Security/Native images: enforce `tools.fs.workspaceOnly` for native prompt image auto-load (including history refs), preventing out-of-workspace sandbox mounts from being implicitly ingested as vision input. This ships in the next npm release. Thanks @tdjackey for reporting. diff --git a/src/telegram/bot-handlers.ts b/src/telegram/bot-handlers.ts index 5d3cfc30b..d0817c1f9 100644 --- a/src/telegram/bot-handlers.ts +++ b/src/telegram/bot-handlers.ts @@ -45,6 +45,7 @@ import { resolveTelegramGroupAllowFromContext, } from "./bot/helpers.js"; import type { TelegramContext } from "./bot/types.js"; +import { enforceTelegramDmAccess } from "./dm-access.js"; import { evaluateTelegramGroupBaseAccess, evaluateTelegramGroupPolicyAccess, @@ -79,6 +80,7 @@ export const registerTelegramHandlers = ({ runtime, mediaMaxBytes, telegramCfg, + allowFrom, groupAllowFrom, resolveGroupPolicy, resolveTelegramGroupConfig, @@ -1182,6 +1184,38 @@ export const registerTelegramHandlers = ({ return; } + const hasInboundMedia = + Boolean(event.msg.media_group_id) || + (Array.isArray(event.msg.photo) && event.msg.photo.length > 0) || + Boolean( + event.msg.video ?? + event.msg.video_note ?? + event.msg.document ?? + event.msg.audio ?? + event.msg.voice ?? + event.msg.sticker, + ); + if (!event.isGroup && hasInboundMedia) { + const effectiveDmAllow = normalizeAllowFromWithStore({ + allowFrom, + storeAllowFrom, + dmPolicy: telegramCfg.dmPolicy ?? "pairing", + }); + const dmAuthorized = await enforceTelegramDmAccess({ + isGroup: event.isGroup, + dmPolicy: telegramCfg.dmPolicy ?? "pairing", + msg: event.msg, + chatId: event.chatId, + effectiveDmAllow, + accountId, + bot, + logger, + }); + if (!dmAuthorized) { + return; + } + } + await processInboundMessage({ ctx: event.ctx, msg: event.msg, diff --git a/src/telegram/bot-message-context.ts b/src/telegram/bot-message-context.ts index b3fa5b9f6..3ea805c94 100644 --- a/src/telegram/bot-message-context.ts +++ b/src/telegram/bot-message-context.ts @@ -33,17 +33,10 @@ import { readSessionUpdatedAt, resolveStorePath } from "../config/sessions.js"; import type { DmPolicy, TelegramGroupConfig, TelegramTopicConfig } from "../config/types.js"; import { logVerbose, shouldLogVerbose } from "../globals.js"; import { recordChannelActivity } from "../infra/channel-activity.js"; -import { buildPairingReply } from "../pairing/pairing-messages.js"; -import { upsertChannelPairingRequest } from "../pairing/pairing-store.js"; import { resolveAgentRoute } from "../routing/resolve-route.js"; import { resolveThreadSessionKeys } from "../routing/session-key.js"; import { withTelegramApiErrorLogging } from "./api-logging.js"; -import { - firstDefined, - isSenderAllowed, - normalizeAllowFromWithStore, - resolveSenderAllowMatch, -} from "./bot-access.js"; +import { firstDefined, isSenderAllowed, normalizeAllowFromWithStore } from "./bot-access.js"; import { buildGroupLabel, buildSenderLabel, @@ -61,6 +54,7 @@ import { resolveTelegramThreadSpec, } from "./bot/helpers.js"; import type { StickerMetadata, TelegramContext } from "./bot/types.js"; +import { enforceTelegramDmAccess } from "./dm-access.js"; import { evaluateTelegramGroupBaseAccess } from "./group-access.js"; import { resolveTelegramGroupPromptSettings } from "./group-config-helpers.js"; import { @@ -159,11 +153,6 @@ export const buildTelegramMessageContext = async ({ resolveTelegramGroupConfig, }: BuildTelegramMessageContextParams) => { const msg = primaryCtx.message; - recordChannelActivity({ - channel: "telegram", - accountId: account.accountId, - direction: "inbound", - }); const chatId = msg.chat.id; const isGroup = msg.chat.type === "group" || msg.chat.type === "supergroup"; const messageThreadId = (msg as { message_thread_id?: number }).message_thread_id; @@ -268,87 +257,27 @@ export const buildTelegramMessageContext = async ({ } }; - // DM access control (secure defaults): "pairing" (default) / "allowlist" / "open" / "disabled" - if (!isGroup) { - if (dmPolicy === "disabled") { - return null; - } - - if (dmPolicy !== "open") { - const senderUsername = msg.from?.username ?? ""; - const senderUserId = msg.from?.id != null ? String(msg.from.id) : null; - const candidate = senderUserId ?? String(chatId); - const allowMatch = resolveSenderAllowMatch({ - allow: effectiveDmAllow, - senderId: candidate, - senderUsername, - }); - const allowMatchMeta = `matchKey=${allowMatch.matchKey ?? "none"} matchSource=${ - allowMatch.matchSource ?? "none" - }`; - const allowed = - effectiveDmAllow.hasWildcard || (effectiveDmAllow.hasEntries && allowMatch.allowed); - if (!allowed) { - if (dmPolicy === "pairing") { - try { - const from = msg.from as - | { - first_name?: string; - last_name?: string; - username?: string; - id?: number; - } - | undefined; - const telegramUserId = from?.id ? String(from.id) : candidate; - const { code, created } = await upsertChannelPairingRequest({ - channel: "telegram", - id: telegramUserId, - accountId: account.accountId, - meta: { - username: from?.username, - firstName: from?.first_name, - lastName: from?.last_name, - }, - }); - if (created) { - logger.info( - { - chatId: String(chatId), - senderUserId: senderUserId ?? undefined, - username: from?.username, - firstName: from?.first_name, - lastName: from?.last_name, - matchKey: allowMatch.matchKey ?? "none", - matchSource: allowMatch.matchSource ?? "none", - }, - "telegram pairing request", - ); - await withTelegramApiErrorLogging({ - operation: "sendMessage", - fn: () => - bot.api.sendMessage( - chatId, - buildPairingReply({ - channel: "telegram", - idLine: `Your Telegram user id: ${telegramUserId}`, - code, - }), - ), - }); - } - } catch (err) { - logVerbose(`telegram pairing reply failed for chat ${chatId}: ${String(err)}`); - } - } else { - logVerbose( - `Blocked unauthorized telegram sender ${candidate} (dmPolicy=${dmPolicy}, ${allowMatchMeta})`, - ); - } - return null; - } - } + if ( + !(await enforceTelegramDmAccess({ + isGroup, + dmPolicy, + msg, + chatId, + effectiveDmAllow, + accountId: account.accountId, + bot, + logger, + })) + ) { + return null; } + recordChannelActivity({ + channel: "telegram", + accountId: account.accountId, + direction: "inbound", + }); + const botUsername = primaryCtx.me?.username?.toLowerCase(); const allowForCommands = isGroup ? effectiveGroupAllow : effectiveDmAllow; const senderAllowedForCommands = isSenderAllowed({ diff --git a/src/telegram/bot-native-commands.ts b/src/telegram/bot-native-commands.ts index adf413fbf..88316cbeb 100644 --- a/src/telegram/bot-native-commands.ts +++ b/src/telegram/bot-native-commands.ts @@ -91,6 +91,7 @@ export type RegisterTelegramHandlerParams = { opts: TelegramBotOptions; runtime: RuntimeEnv; telegramCfg: TelegramAccountConfig; + allowFrom?: Array; groupAllowFrom?: Array; resolveGroupPolicy: (chatId: string | number) => ChannelGroupPolicy; resolveTelegramGroupConfig: ( diff --git a/src/telegram/bot.create-telegram-bot.test.ts b/src/telegram/bot.create-telegram-bot.test.ts index 816cf224d..5bf422506 100644 --- a/src/telegram/bot.create-telegram-bot.test.ts +++ b/src/telegram/bot.create-telegram-bot.test.ts @@ -329,6 +329,133 @@ describe("createTelegramBot", () => { } } }); + it("blocks unauthorized DM media before download and sends pairing reply", async () => { + loadConfig.mockReturnValue({ + channels: { telegram: { dmPolicy: "pairing" } }, + }); + readChannelAllowFromStore.mockResolvedValue([]); + upsertChannelPairingRequest.mockResolvedValue({ code: "PAIRME12", created: true }); + sendMessageSpy.mockClear(); + replySpy.mockClear(); + + const fetchSpy = vi.spyOn(globalThis, "fetch").mockImplementation( + async () => + new Response(new Uint8Array([0xff, 0xd8, 0xff, 0x00]), { + status: 200, + headers: { "content-type": "image/jpeg" }, + }), + ); + const getFileSpy = vi.fn(async () => ({ file_path: "photos/p1.jpg" })); + + try { + createTelegramBot({ token: "tok" }); + const handler = getOnHandler("message") as (ctx: Record) => Promise; + + await handler({ + message: { + chat: { id: 1234, type: "private" }, + message_id: 410, + date: 1736380800, + photo: [{ file_id: "p1" }], + from: { id: 999, username: "random" }, + }, + me: { username: "openclaw_bot" }, + getFile: getFileSpy, + }); + + expect(getFileSpy).not.toHaveBeenCalled(); + expect(fetchSpy).not.toHaveBeenCalled(); + expect(sendMessageSpy).toHaveBeenCalledTimes(1); + expect(String(sendMessageSpy.mock.calls[0]?.[1])).toContain("Pairing code:"); + expect(replySpy).not.toHaveBeenCalled(); + } finally { + fetchSpy.mockRestore(); + } + }); + it("blocks DM media downloads completely when dmPolicy is disabled", async () => { + loadConfig.mockReturnValue({ + channels: { telegram: { dmPolicy: "disabled" } }, + }); + sendMessageSpy.mockClear(); + replySpy.mockClear(); + + const fetchSpy = vi.spyOn(globalThis, "fetch").mockImplementation( + async () => + new Response(new Uint8Array([0xff, 0xd8, 0xff, 0x00]), { + status: 200, + headers: { "content-type": "image/jpeg" }, + }), + ); + const getFileSpy = vi.fn(async () => ({ file_path: "photos/p1.jpg" })); + + try { + createTelegramBot({ token: "tok" }); + const handler = getOnHandler("message") as (ctx: Record) => Promise; + + await handler({ + message: { + chat: { id: 1234, type: "private" }, + message_id: 411, + date: 1736380800, + photo: [{ file_id: "p1" }], + from: { id: 999, username: "random" }, + }, + me: { username: "openclaw_bot" }, + getFile: getFileSpy, + }); + + expect(getFileSpy).not.toHaveBeenCalled(); + expect(fetchSpy).not.toHaveBeenCalled(); + expect(sendMessageSpy).not.toHaveBeenCalled(); + expect(replySpy).not.toHaveBeenCalled(); + } finally { + fetchSpy.mockRestore(); + } + }); + it("blocks unauthorized DM media groups before any photo download", async () => { + loadConfig.mockReturnValue({ + channels: { telegram: { dmPolicy: "pairing" } }, + }); + readChannelAllowFromStore.mockResolvedValue([]); + upsertChannelPairingRequest.mockResolvedValue({ code: "PAIRME12", created: true }); + sendMessageSpy.mockClear(); + replySpy.mockClear(); + + const fetchSpy = vi.spyOn(globalThis, "fetch").mockImplementation( + async () => + new Response(new Uint8Array([0xff, 0xd8, 0xff, 0x00]), { + status: 200, + headers: { "content-type": "image/jpeg" }, + }), + ); + const getFileSpy = vi.fn(async () => ({ file_path: "photos/p1.jpg" })); + + try { + createTelegramBot({ token: "tok", testTimings: TELEGRAM_TEST_TIMINGS }); + const handler = getOnHandler("message") as (ctx: Record) => Promise; + + await handler({ + message: { + chat: { id: 1234, type: "private" }, + message_id: 412, + media_group_id: "dm-album-1", + date: 1736380800, + photo: [{ file_id: "p1" }], + from: { id: 999, username: "random" }, + }, + me: { username: "openclaw_bot" }, + getFile: getFileSpy, + }); + + expect(getFileSpy).not.toHaveBeenCalled(); + expect(fetchSpy).not.toHaveBeenCalled(); + expect(sendMessageSpy).toHaveBeenCalledTimes(1); + expect(String(sendMessageSpy.mock.calls[0]?.[1])).toContain("Pairing code:"); + expect(replySpy).not.toHaveBeenCalled(); + } finally { + fetchSpy.mockRestore(); + } + }); it("triggers typing cue via onReplyStart", async () => { createTelegramBot({ token: "tok" }); const handler = getOnHandler("message") as (ctx: Record) => Promise; diff --git a/src/telegram/bot.media.downloads-media-file-path-no-file-download.test.ts b/src/telegram/bot.media.downloads-media-file-path-no-file-download.test.ts index 8f34fcdeb..8b2089a69 100644 --- a/src/telegram/bot.media.downloads-media-file-path-no-file-download.test.ts +++ b/src/telegram/bot.media.downloads-media-file-path-no-file-download.test.ts @@ -370,7 +370,7 @@ describe("telegram media groups", () => { () => { expect(replySpy).toHaveBeenCalledTimes(scenario.expectedReplyCount); }, - { timeout: MEDIA_GROUP_FLUSH_MS * 2, interval: 2 }, + { timeout: MEDIA_GROUP_FLUSH_MS * 4, interval: 2 }, ); expect(runtimeError).not.toHaveBeenCalled(); diff --git a/src/telegram/bot.ts b/src/telegram/bot.ts index 438ed1c9b..409815fa3 100644 --- a/src/telegram/bot.ts +++ b/src/telegram/bot.ts @@ -398,6 +398,7 @@ export function createTelegramBot(opts: TelegramBotOptions) { runtime, mediaMaxBytes, telegramCfg, + allowFrom, groupAllowFrom, resolveGroupPolicy, resolveTelegramGroupConfig, diff --git a/src/telegram/dm-access.ts b/src/telegram/dm-access.ts new file mode 100644 index 000000000..9d7e1d463 --- /dev/null +++ b/src/telegram/dm-access.ts @@ -0,0 +1,109 @@ +import type { Message } from "@grammyjs/types"; +import type { Bot } from "grammy"; +import type { DmPolicy } from "../config/types.js"; +import { logVerbose } from "../globals.js"; +import { buildPairingReply } from "../pairing/pairing-messages.js"; +import { upsertChannelPairingRequest } from "../pairing/pairing-store.js"; +import { withTelegramApiErrorLogging } from "./api-logging.js"; +import { resolveSenderAllowMatch, type NormalizedAllowFrom } from "./bot-access.js"; + +type TelegramDmAccessLogger = { + info: (obj: Record, msg: string) => void; +}; + +export async function enforceTelegramDmAccess(params: { + isGroup: boolean; + dmPolicy: DmPolicy; + msg: Message; + chatId: number; + effectiveDmAllow: NormalizedAllowFrom; + accountId: string; + bot: Bot; + logger: TelegramDmAccessLogger; +}): Promise { + const { isGroup, dmPolicy, msg, chatId, effectiveDmAllow, accountId, bot, logger } = params; + if (isGroup) { + return true; + } + if (dmPolicy === "disabled") { + return false; + } + if (dmPolicy === "open") { + return true; + } + + const senderUsername = msg.from?.username ?? ""; + const senderUserId = msg.from?.id != null ? String(msg.from.id) : null; + const candidate = senderUserId ?? String(chatId); + const allowMatch = resolveSenderAllowMatch({ + allow: effectiveDmAllow, + senderId: candidate, + senderUsername, + }); + const allowMatchMeta = `matchKey=${allowMatch.matchKey ?? "none"} matchSource=${ + allowMatch.matchSource ?? "none" + }`; + const allowed = + effectiveDmAllow.hasWildcard || (effectiveDmAllow.hasEntries && allowMatch.allowed); + if (allowed) { + return true; + } + + if (dmPolicy === "pairing") { + try { + const from = msg.from as + | { + first_name?: string; + last_name?: string; + username?: string; + id?: number; + } + | undefined; + const telegramUserId = from?.id ? String(from.id) : candidate; + const { code, created } = await upsertChannelPairingRequest({ + channel: "telegram", + id: telegramUserId, + accountId, + meta: { + username: from?.username, + firstName: from?.first_name, + lastName: from?.last_name, + }, + }); + if (created) { + logger.info( + { + chatId: String(chatId), + senderUserId: senderUserId ?? undefined, + username: from?.username, + firstName: from?.first_name, + lastName: from?.last_name, + matchKey: allowMatch.matchKey ?? "none", + matchSource: allowMatch.matchSource ?? "none", + }, + "telegram pairing request", + ); + await withTelegramApiErrorLogging({ + operation: "sendMessage", + fn: () => + bot.api.sendMessage( + chatId, + buildPairingReply({ + channel: "telegram", + idLine: `Your Telegram user id: ${telegramUserId}`, + code, + }), + ), + }); + } + } catch (err) { + logVerbose(`telegram pairing reply failed for chat ${chatId}: ${String(err)}`); + } + return false; + } + + logVerbose( + `Blocked unauthorized telegram sender ${candidate} (dmPolicy=${dmPolicy}, ${allowMatchMeta})`, + ); + return false; +}