mirror of https://github.com/openclaw/openclaw.git
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 <hi@obviy.us>
This commit is contained in:
parent
5f6e3499f3
commit
985533efbc
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
|
|
@ -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 () => {
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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<TelegramContext["message"]["photo"]>[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) ?? "<media:document>";
|
||||
|
|
|
|||
Loading…
Reference in New Issue