From 985533efbc61343fc828b2d996444359055b4cd3 Mon Sep 17 00:00:00 2001 From: saram ali <140950904+SARAMALI15792@users.noreply.github.com> Date: Fri, 3 Apr 2026 08:11:41 +0500 Subject: [PATCH] fix: cover buffered Telegram apiRoot downloads (#59544) (thanks @SARAMALI15792) * test(telegram): add URL construction tests for custom apiRoot Add comprehensive test cases to verify that file download URLs are correctly constructed when using a custom apiRoot configuration for local Bot API servers. Tests validate: - Document downloads use the custom apiRoot in the constructed URL - Sticker downloads use the custom apiRoot in the constructed URL - SSRF policy correctly includes the custom hostname This ensures issue #59512 (Telegram file downloads with local Bot API) is properly covered by regression tests. * refactor(telegram): improve media resolution code quality Apply KISS and YAGNI principles to reduce code duplication and improve maintainability: 1. Extract media metadata resolution - Consolidate resolveMediaFileRef(), resolveTelegramFileName(), and resolveTelegramMimeType() into single resolveMediaMetadata() function - Returns typed MediaMetadata object with fileRef, fileName, mimeType - Reduces duplication and improves readability 2. Add logging for apiRoot parsing failures - Log when custom apiRoot URL parsing fails in buildTelegramMediaSsrfPolicy() - Helps debug configuration issues with local Bot API servers 3. Fix missing apiRoot in buffered messages - Add telegramCfg.apiRoot parameter to resolveMedia() calls in bot-handlers.buffers.ts (lines 150-159, 189) - Ensures reply media in buffered contexts respects custom apiRoot config - Fixes inconsistency where runtime handler passed apiRoot but buffers didn't These changes improve code quality while maintaining backward compatibility and ensuring issue #59512 (Telegram file downloads with local Bot API) works correctly in all contexts. * fix(telegram): resolve bot review issues Address critical issues identified by Greptile code review: 1. Define telegramCfg in bot-handlers.buffers.ts - Extract telegramCfg from cfg.channels?.telegram - Fixes ReferenceError when accessing telegramCfg.apiRoot - Ensures buffered message handlers can access apiRoot configuration 2. Restore type safety for MediaMetadata.fileRef - Change from 'unknown' to proper union type - Preserves type information for downstream file_id access - Prevents TypeScript strict mode compilation errors These fixes ensure the PR compiles correctly and handles buffered media downloads with custom apiRoot configuration. * fix(telegram): use optional chaining for telegramCfg.apiRoot TypeScript strict mode requires optional chaining when accessing properties on potentially undefined objects. Changed telegramCfg.apiRoot to telegramCfg?.apiRoot to handle cases where telegramCfg is undefined. Fixes TypeScript errors: - TS18048: 'telegramCfg' is possibly 'undefined' (line 160) - TS18048: 'telegramCfg' is possibly 'undefined' (line 191) * fix(telegram): add missing optional chaining on line 191 Complete the fix for telegramCfg optional chaining. Previous commit only fixed line 160, but line 191 also needs the same fix to prevent TS18048 error. * fix: cover buffered Telegram apiRoot downloads (#59544) (thanks @SARAMALI15792) --------- Co-authored-by: Ayaan Zaidi --- CHANGELOG.md | 1 + .../telegram/src/bot-handlers.buffers.ts | 4 +- ...dia-file-path-no-file-download.e2e.test.ts | 76 +++++++++++++++++++ .../bot/delivery.resolve-media-retry.test.ts | 54 +++++++++++++ .../src/bot/delivery.resolve-media.ts | 69 +++++++++-------- 5 files changed, 171 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d7fc5f0c283..9771d4c8710 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,7 @@ Docs: https://docs.openclaw.ai - Matrix: allow secret-storage recreation during automatic repair bootstrap so clients that lose their recovery key can recover and persist new cross-signing keys. (#59846) Thanks @al3mart. - Matrix/crypto persistence: capture and write the IndexedDB snapshot while holding the snapshot file lock so concurrent gateway and CLI persists cannot overwrite newer crypto state. (#59851) Thanks @al3mart. - Telegram/media: keep inbound image attachments readable on upgraded installs where legacy state roots still differ from the managed config-dir media cache. (#59971) Thanks @neeravmakwana. +- Telegram/local Bot API: thread `channels.telegram.apiRoot` through buffered reply-media and album downloads so self-hosted Bot API file paths stop falling back to `api.telegram.org` and 404ing. (#59544) Thanks @SARAMALI15792. ## 2026.4.2 diff --git a/extensions/telegram/src/bot-handlers.buffers.ts b/extensions/telegram/src/bot-handlers.buffers.ts index 532fef78928..455edc60b72 100644 --- a/extensions/telegram/src/bot-handlers.buffers.ts +++ b/extensions/telegram/src/bot-handlers.buffers.ts @@ -84,6 +84,7 @@ export function createTelegramInboundBufferRuntime(params: { runtime, telegramTransport, } = params; + const telegramCfg = cfg.channels?.telegram; const TELEGRAM_TEXT_FRAGMENT_START_THRESHOLD_CHARS = 4000; const TELEGRAM_TEXT_FRAGMENT_MAX_GAP_MS = typeof opts.testTimings?.textFragmentGapMs === "number" && @@ -156,6 +157,7 @@ export function createTelegramInboundBufferRuntime(params: { mediaMaxBytes, opts.token, telegramTransport, + telegramCfg?.apiRoot, ); if (!media) { return []; @@ -186,7 +188,7 @@ export function createTelegramInboundBufferRuntime(params: { for (const { ctx } of entry.messages) { let media; try { - media = await resolveMedia(ctx, mediaMaxBytes, opts.token, telegramTransport); + media = await resolveMedia(ctx, mediaMaxBytes, opts.token, telegramTransport, telegramCfg?.apiRoot); } catch (mediaErr) { if (!isRecoverableMediaGroupError(mediaErr)) { throw mediaErr; diff --git a/extensions/telegram/src/bot.media.downloads-media-file-path-no-file-download.e2e.test.ts b/extensions/telegram/src/bot.media.downloads-media-file-path-no-file-download.e2e.test.ts index e385c102681..39888ecf9bc 100644 --- a/extensions/telegram/src/bot.media.downloads-media-file-path-no-file-download.e2e.test.ts +++ b/extensions/telegram/src/bot.media.downloads-media-file-path-no-file-download.e2e.test.ts @@ -1,4 +1,5 @@ import { afterEach, describe, expect, it, vi } from "vitest"; +import { telegramBotDepsForTest } from "./bot.media.e2e-harness.js"; import { setNextSavedMediaPath } from "./bot.media.e2e-harness.js"; import { TELEGRAM_TEST_TIMINGS, @@ -237,6 +238,81 @@ describe("telegram media groups", () => { const MEDIA_GROUP_TEST_TIMEOUT_MS = process.platform === "win32" ? 45_000 : 20_000; const MEDIA_GROUP_FLUSH_MS = TELEGRAM_TEST_TIMINGS.mediaGroupFlushMs + 40; + it( + "uses custom apiRoot for buffered media-group downloads", + async () => { + const originalLoadConfig = telegramBotDepsForTest.loadConfig; + telegramBotDepsForTest.loadConfig = (() => ({ + channels: { + telegram: { + dmPolicy: "open", + allowFrom: ["*"], + apiRoot: "http://127.0.0.1:8081/custom-bot-api", + }, + }, + })) as typeof telegramBotDepsForTest.loadConfig; + + const runtimeError = vi.fn(); + const { handler, replySpy } = await createBotHandlerWithOptions({ runtimeError }); + const fetchSpy = mockTelegramPngDownload(); + + try { + await Promise.all([ + handler({ + message: { + chat: { id: 42, type: "private" as const }, + from: { id: 777, is_bot: false, first_name: "Ada" }, + message_id: 1, + caption: "Album", + date: 1736380800, + media_group_id: "album-custom-api-root", + photo: [{ file_id: "photo1" }], + }, + me: { username: "openclaw_bot" }, + getFile: async () => ({ file_path: "photos/photo1.jpg" }), + }), + handler({ + message: { + chat: { id: 42, type: "private" as const }, + from: { id: 777, is_bot: false, first_name: "Ada" }, + message_id: 2, + date: 1736380801, + media_group_id: "album-custom-api-root", + photo: [{ file_id: "photo2" }], + }, + me: { username: "openclaw_bot" }, + getFile: async () => ({ file_path: "photos/photo2.jpg" }), + }), + ]); + + await vi.waitFor( + () => { + expect(replySpy).toHaveBeenCalledTimes(1); + }, + { timeout: MEDIA_GROUP_FLUSH_MS * 4, interval: 2 }, + ); + + expect(runtimeError).not.toHaveBeenCalled(); + expect(fetchSpy).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ + url: "http://127.0.0.1:8081/custom-bot-api/file/bottok/photos/photo1.jpg", + }), + ); + expect(fetchSpy).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + url: "http://127.0.0.1:8081/custom-bot-api/file/bottok/photos/photo2.jpg", + }), + ); + } finally { + telegramBotDepsForTest.loadConfig = originalLoadConfig; + fetchSpy.mockRestore(); + } + }, + MEDIA_GROUP_TEST_TIMEOUT_MS, + ); + it( "handles same-group buffering and separate-group independence", async () => { diff --git a/extensions/telegram/src/bot/delivery.resolve-media-retry.test.ts b/extensions/telegram/src/bot/delivery.resolve-media-retry.test.ts index 49f396e3f7b..aa145983ed8 100644 --- a/extensions/telegram/src/bot/delivery.resolve-media-retry.test.ts +++ b/extensions/telegram/src/bot/delivery.resolve-media-retry.test.ts @@ -545,4 +545,58 @@ describe("resolveMedia original filename preservation", () => { ); expect(result).not.toBeNull(); }); + + it("constructs correct download URL with custom apiRoot for documents", async () => { + const getFile = vi.fn().mockResolvedValue({ file_path: "documents/file_42.pdf" }); + mockPdfFetchAndSave("file_42.pdf"); + + const customApiRoot = "http://192.168.1.50:8081/custom-bot-api"; + const ctx = makeCtx("document", getFile); + const result = await resolveMedia( + ctx, + MAX_MEDIA_BYTES, + BOT_TOKEN, + undefined, + customApiRoot, + ); + + // Verify the URL uses the custom apiRoot, not the default Telegram API + expect(fetchRemoteMedia).toHaveBeenCalledWith( + expect.objectContaining({ + url: `${customApiRoot}/file/bot${BOT_TOKEN}/documents/file_42.pdf`, + }), + ); + expect(result).not.toBeNull(); + }); + + it("constructs correct download URL with custom apiRoot for stickers", async () => { + const getFile = vi.fn().mockResolvedValue({ file_path: "stickers/file_0.webp" }); + fetchRemoteMedia.mockResolvedValueOnce({ + buffer: Buffer.from("sticker-data"), + contentType: "image/webp", + fileName: "file_0.webp", + }); + saveMediaBuffer.mockResolvedValueOnce({ + path: "/tmp/file_0.webp", + contentType: "image/webp", + }); + + const customApiRoot = "http://localhost:8081/bot"; + const ctx = makeCtx("sticker", getFile); + const result = await resolveMedia( + ctx, + MAX_MEDIA_BYTES, + BOT_TOKEN, + undefined, + customApiRoot, + ); + + // Verify the URL uses the custom apiRoot for sticker downloads + expect(fetchRemoteMedia).toHaveBeenCalledWith( + expect.objectContaining({ + url: `${customApiRoot}/file/bot${BOT_TOKEN}/stickers/file_0.webp`, + }), + ); + expect(result).not.toBeNull(); + }); }); diff --git a/extensions/telegram/src/bot/delivery.resolve-media.ts b/extensions/telegram/src/bot/delivery.resolve-media.ts index f0a256cf404..e5ed40e6eb2 100644 --- a/extensions/telegram/src/bot/delivery.resolve-media.ts +++ b/extensions/telegram/src/bot/delivery.resolve-media.ts @@ -32,8 +32,8 @@ function buildTelegramMediaSsrfPolicy(apiRoot?: string) { // still enforcing resolved-IP checks for the default public host. allowedHostnames = [customHost]; } - } catch { - // invalid URL; fall through to default + } catch (err) { + logVerbose(`telegram: invalid apiRoot URL "${apiRoot}": ${String(err)}`); } } return { @@ -70,35 +70,39 @@ function isRetryableGetFileError(err: unknown): boolean { return true; } -function resolveMediaFileRef(msg: TelegramContext["message"]) { - return ( - msg.photo?.[msg.photo.length - 1] ?? - msg.video ?? - msg.video_note ?? - msg.document ?? - msg.audio ?? - msg.voice - ); +interface MediaMetadata { + fileRef?: + | NonNullable[number] + | TelegramContext["message"]["video"] + | TelegramContext["message"]["video_note"] + | TelegramContext["message"]["document"] + | TelegramContext["message"]["audio"] + | TelegramContext["message"]["voice"]; + fileName?: string; + mimeType?: string; } -function resolveTelegramFileName(msg: TelegramContext["message"]): string | undefined { - return ( - msg.document?.file_name ?? - msg.audio?.file_name ?? - msg.video?.file_name ?? - msg.animation?.file_name - ); -} - -function resolveTelegramMimeType(msg: TelegramContext["message"]): string | undefined { - return ( - msg.audio?.mime_type ?? - msg.voice?.mime_type ?? - msg.video?.mime_type ?? - msg.document?.mime_type ?? - msg.animation?.mime_type ?? - undefined - ); +function resolveMediaMetadata(msg: TelegramContext["message"]): MediaMetadata { + return { + fileRef: + msg.photo?.[msg.photo.length - 1] ?? + msg.video ?? + msg.video_note ?? + msg.document ?? + msg.audio ?? + msg.voice, + fileName: + msg.document?.file_name ?? + msg.audio?.file_name ?? + msg.video?.file_name ?? + msg.animation?.file_name, + mimeType: + msg.audio?.mime_type ?? + msg.voice?.mime_type ?? + msg.video?.mime_type ?? + msg.document?.mime_type ?? + msg.animation?.mime_type, + }; } async function resolveTelegramFileWithRetry( @@ -314,7 +318,8 @@ export async function resolveMedia( return stickerResolved; } - const m = resolveMediaFileRef(msg); + const metadata = resolveMediaMetadata(msg); + const m = metadata.fileRef; if (!m?.file_id) { return null; } @@ -331,8 +336,8 @@ export async function resolveMedia( token, transport: resolveRequiredTelegramTransport(transport), maxBytes, - telegramFileName: resolveTelegramFileName(msg), - mimeType: resolveTelegramMimeType(msg), + telegramFileName: metadata.fileName, + mimeType: metadata.mimeType, apiRoot, }); const placeholder = resolveTelegramMediaPlaceholder(msg) ?? "";