diff --git a/.env.example b/.env.example index 67097b71045..f259d4d854a 100644 --- a/.env.example +++ b/.env.example @@ -3070,6 +3070,11 @@ QUOTA_STORE_DRIVER=sqlite # Telegram Mini App bridge. The update endpoint remains disabled while the bot # token is unset. Used by: src/lib/telegram/* and src/app/api/telegram/update/route.ts. # TELEGRAM_BOT_TOKEN= +# Shared secret registered with setWebhook and echoed back by Telegram as the +# X-Telegram-Bot-Api-Secret-Token header. REQUIRED for the webhook path: without +# it the webhook is rejected with 503, because an unauthenticated update lets any +# caller mint API keys and spend upstream quota. The Mini App path does not use it. +# TELEGRAM_WEBHOOK_SECRET= # TELEGRAM_DEFAULT_MODEL=auto/chat # TELEGRAM_BOT_API_BASE=https://api.telegram.org # TELEGRAM_WEBHOOK_TIMEOUT_MS=60000 diff --git a/changelog.d/fixes/13172-telegram-webhook-secret.md b/changelog.d/fixes/13172-telegram-webhook-secret.md new file mode 100644 index 00000000000..49f14b55fb7 --- /dev/null +++ b/changelog.d/fixes/13172-telegram-webhook-secret.md @@ -0,0 +1 @@ +- **fix(telegram):** authenticate webhook deliveries with Telegram's `secret_token` so an unauthenticated caller can no longer mint API keys or spend upstream quota ([#13172](https://github.com/diegosouzapw/OmniRoute/issues/13172)) diff --git a/docs/reference/ENVIRONMENT.md b/docs/reference/ENVIRONMENT.md index f357f735840..13b70dd4e3b 100644 --- a/docs/reference/ENVIRONMENT.md +++ b/docs/reference/ENVIRONMENT.md @@ -1623,6 +1623,7 @@ These settings were introduced after the previous environment-contract snapshot. | `ADOBE_FIREFLY_CHROME_HEADLESS` | `0` | `open-sse/services/adobeFireflyBrowserLogin.ts` | Debug-only true-headless mode; Adobe colligo normally rejects the resulting risk session. | | `CHROME_PATH` | auto-detect | `open-sse/executors/cloudflare-playground.ts`, `open-sse/executors/chatgpt-web-codex.ts` | Optional absolute Chrome executable used by the browser-driven executors when platform auto-detection is insufficient. | | `TELEGRAM_BOT_TOKEN` | _(unset)_ | `src/lib/telegram/config.ts` | BotFather token that enables the inbound webhook and signs Mini App `initData`. | +| `TELEGRAM_WEBHOOK_SECRET` | _(unset)_ | `src/lib/telegram/config.ts` | Shared secret registered via `setWebhook` and verified against the `X-Telegram-Bot-Api-Secret-Token` header on every webhook delivery. Required for the webhook path; unset means webhook deliveries are refused with 503. | | `TELEGRAM_DEFAULT_MODEL` | `auto/chat` | `src/lib/telegram/chatProxy.ts` | Model used for Telegram chat replies. | | `TELEGRAM_BOT_API_BASE` | `https://api.telegram.org` | `src/lib/telegram/config.ts` | Bot API base URL override for proxies or self-hosted Bot API servers. | | `TELEGRAM_WEBHOOK_TIMEOUT_MS` | `60000` | `src/lib/telegram/config.ts` | Timeout in milliseconds for outbound Bot API calls. | diff --git a/src/app/api/telegram/update/route.ts b/src/app/api/telegram/update/route.ts index 1fd3582da99..c2194a58955 100644 --- a/src/app/api/telegram/update/route.ts +++ b/src/app/api/telegram/update/route.ts @@ -13,12 +13,18 @@ * 3. Handles /start (returns the Mini App deep link) and everything else * as a chat prompt proxied through the OmniRoute pipeline. */ +import { timingSafeEqual } from "node:crypto"; import { NextResponse } from "next/server"; import { z } from "zod"; import { validateBody, isValidationFailure } from "@/shared/validation/helpers"; import type { TelegramUpdate } from "@/lib/telegram/botApi"; import { extractChatMessage, sendTelegramMessage } from "@/lib/telegram/botApi"; -import { getTelegramBotToken, isTelegramEnabled } from "@/lib/telegram/config"; +import { + getTelegramBotToken, + getTelegramWebhookSecret, + isTelegramEnabled, + isTelegramWebhookSecretConfigured, +} from "@/lib/telegram/config"; import { verifyInitData, parseInitData } from "@/lib/telegram/initData"; import { proxyChat } from "@/lib/telegram/chatProxy"; import { formatTelegramGatewayError } from "@/lib/telegram/errorMessage"; @@ -33,7 +39,12 @@ import { resolveOmniRouteBaseUrl } from "@/shared/utils/resolveOmniRouteBaseUrl" const telegramBodySchema = z .object({ initData: z.string().optional(), - message: z.string().optional(), + // `message` is a STRING on the Mini App path ({ initData, message }) and an + // OBJECT on the webhook path (a Telegram update). Constraining it to a + // string rejected every real webhook delivery with 400 before any auth or + // routing ran, so accept either shape here and let each branch validate the + // shape it actually needs. + message: z.union([z.string(), z.record(z.string(), z.unknown())]).optional(), update_id: z.number().optional(), // allow unknown update fields }) @@ -103,6 +114,21 @@ export async function POST(request: Request) { } // ── Bot webhook path: TelegramUpdate ───────────────────────────────────── + // Unlike the Mini App branch above (which verifies the initData HMAC), a + // webhook body carries no proof of origin: `chat.id` is attacker-chosen and + // reaches proxyChat(), which mints a real API key and spends upstream quota. + // Telegram's `secret_token` echo is the only authentication available here. + if (!isTelegramWebhookSecretConfigured()) { + return NextResponse.json( + { ok: false, error: "Telegram webhook secret not configured" }, + { status: 503 } + ); + } + const presentedSecret = request.headers.get("x-telegram-bot-api-secret-token") || ""; + if (!webhookSecretMatches(presentedSecret, getTelegramWebhookSecret())) { + return NextResponse.json({ ok: false, error: "Unauthorized" }, { status: 401 }); + } + const update = body as unknown as TelegramUpdate; const chat = extractChatMessage(update); if (!chat) { @@ -117,6 +143,22 @@ export async function POST(request: Request) { return NextResponse.json({ ok: true }); } +/** + * Constant-time comparison of the presented webhook secret against the + * configured one. A plain `===` short-circuits on the first differing byte and + * leaks the shared-prefix length through response timing; `timingSafeEqual` + * does not. It requires equal-length buffers, so a length mismatch is rejected + * up front (the length itself is not secret). + * + * Exported as a test seam only — not part of the route contract. + */ +export function webhookSecretMatches(presented: string, expected: string): boolean { + const a = Buffer.from(presented); + const b = Buffer.from(expected); + if (a.length !== b.length) return false; + return timingSafeEqual(a, b); +} + async function handleAndReply(chatId: number, text: string, messageId?: number): Promise { try { const trimmed = text.trim(); diff --git a/src/lib/telegram/botApi.ts b/src/lib/telegram/botApi.ts index 4bdc50071a5..c1a0e5e0489 100644 --- a/src/lib/telegram/botApi.ts +++ b/src/lib/telegram/botApi.ts @@ -5,7 +5,12 @@ * replies and setWebhook for webhook registration. Streaming is emulated * by the caller via progressive edits (sendMessage / editMessageText). */ -import { getTelegramBotApiBase, getTelegramBotToken, getTelegramWebhookTimeoutMs } from "./config"; +import { + getTelegramBotApiBase, + getTelegramBotToken, + getTelegramWebhookTimeoutMs, + getTelegramWebhookSecret, +} from "./config"; export interface TelegramSendMessageParams { chat_id: number | string; @@ -92,7 +97,15 @@ export async function setTelegramWebhook( opts: { dropPending?: boolean } = {} ): Promise<{ url: string; pending_update_count?: number }> { if (url) { - return botFetch("setWebhook", { url, drop_pending_updates: opts.dropPending ?? true }); + // Register the shared secret so Telegram echoes it back as + // X-Telegram-Bot-Api-Secret-Token on every delivery; the webhook route + // rejects deliveries that do not carry it (#13172). + const secret = getTelegramWebhookSecret(); + return botFetch("setWebhook", { + url, + drop_pending_updates: opts.dropPending ?? true, + ...(secret ? { secret_token: secret } : {}), + }); } return botFetch("deleteWebhook", { drop_pending_updates: opts.dropPending ?? true }); } diff --git a/src/lib/telegram/config.ts b/src/lib/telegram/config.ts index 421739ef5ed..817641e9be9 100644 --- a/src/lib/telegram/config.ts +++ b/src/lib/telegram/config.ts @@ -25,6 +25,30 @@ export function getTelegramWebhookTimeoutMs(): number { return Number.isInteger(parsed) && parsed > 0 ? parsed : DEFAULT_WEBHOOK_TIMEOUT_MS; } +/** + * Shared secret for authenticating Telegram webhook deliveries. + * + * Telegram echoes the `secret_token` passed to `setWebhook` back on every + * delivery in the `X-Telegram-Bot-Api-Secret-Token` header, which is the only + * way to prove a webhook POST actually came from Telegram. Kept in the + * environment alongside the bot token so it is never stored in the DB. + */ +export function getTelegramWebhookSecret(): string { + return process.env.TELEGRAM_WEBHOOK_SECRET || ""; +} + +/** + * Whether webhook deliveries are authenticated. + * + * When no secret is configured the webhook path is rejected outright rather + * than served unauthenticated: an open path mints API keys and spends upstream + * quota for any caller (see #13172). The Mini App path is unaffected — it + * authenticates with the initData HMAC and does not use this secret. + */ +export function isTelegramWebhookSecretConfigured(): boolean { + return getTelegramWebhookSecret().length > 0; +} + export function getTelegramBotApiBase(): string { return process.env.TELEGRAM_BOT_API_BASE || "https://api.telegram.org"; } diff --git a/tests/unit/telegram-webhook-secret-13172.test.ts b/tests/unit/telegram-webhook-secret-13172.test.ts new file mode 100644 index 00000000000..f525ee745a5 --- /dev/null +++ b/tests/unit/telegram-webhook-secret-13172.test.ts @@ -0,0 +1,72 @@ +/** + * Regression test for #13172: the Telegram webhook path must authenticate. + * + * Telegram echoes the `secret_token` given to `setWebhook` back on every + * delivery as `X-Telegram-Bot-Api-Secret-Token`. Without checking it, any + * caller can POST a synthetic update with an arbitrary `chat.id`, which reaches + * proxyChat() and mints a real API key plus upstream spend. + * + * The Mini App branch authenticates separately (initData HMAC) and must keep + * working without a webhook secret. + */ +import { describe, test, before, after } from "node:test"; +import assert from "node:assert/strict"; + +const BOT_TOKEN = "123456:AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; +const SECRET = "s3cret-webhook-token"; + +let POST: (req: Request) => Promise; +let webhookSecretMatches: (a: string, b: string) => boolean; +const proxied: number[] = []; + +before(async () => { + process.env.TELEGRAM_BOT_TOKEN = BOT_TOKEN; + process.env.TELEGRAM_WEBHOOK_SECRET = SECRET; + + const mod = await import("../../src/app/api/telegram/update/route.ts"); + POST = mod.POST as typeof POST; + webhookSecretMatches = mod.webhookSecretMatches as typeof webhookSecretMatches; +}); + +after(() => { + delete process.env.TELEGRAM_WEBHOOK_SECRET; +}); + +function webhookRequest(headers: Record = {}): Request { + return new Request("https://example.test/api/telegram/update", { + method: "POST", + headers: { "content-type": "application/json", ...headers }, + // A realistic Telegram update: `message` is an object here, whereas the + // Mini App path sends it as a string. Both shapes must reach their branch. + body: JSON.stringify({ + update_id: 1, + message: { chat: { id: 999 }, text: "hi", message_id: 5 }, + }), + }); +} + +describe("telegram webhook authentication (#13172)", () => { + test("rejects a delivery with no secret header", async () => { + const res = await POST(webhookRequest()); + assert.equal(res.status, 401, "unauthenticated webhook must be rejected"); + assert.deepEqual(proxied, [], "no chat should be proxied"); + }); + + test("rejects a delivery with a wrong secret", async () => { + const res = await POST( + webhookRequest({ "x-telegram-bot-api-secret-token": "wrong-token-value" }) + ); + assert.equal(res.status, 401, "a mismatched secret must be rejected"); + }); + + test("accepts a delivery carrying the configured secret", async () => { + const res = await POST(webhookRequest({ "x-telegram-bot-api-secret-token": SECRET })); + assert.equal(res.status, 200, "a correctly authenticated delivery must be accepted"); + }); + + test("comparison is length-safe and value-correct", () => { + assert.equal(webhookSecretMatches(SECRET, SECRET), true); + assert.equal(webhookSecretMatches("short", SECRET), false, "length mismatch must not throw"); + assert.equal(webhookSecretMatches("", ""), true, "equal empties compare equal"); + }); +});