fix(telegram): authenticate webhook deliveries with the Telegram secret token - #13175
Conversation
|
Flagging for review priority: of the 14 open PRs in this audit (queue overview), this is the only one with a security impact rather than a resource-management one. An unauthenticated caller could POST a synthetic Telegram update with any Note this is intentionally breaking for existing webhook deployments: operators must set |
a1b9b02
into
diegosouzapw:release/v3.8.51
…et token (diegosouzapw#13175) Real abuse vector: the bot-webhook branch reached `proxyChat()` — which mints an API key and spends upstream quota — with nothing proving the caller was Telegram. Fail-closed 503 when the secret is unset is the right default. --- Validated in one consolidated worktree cut from `release/v3.8.51`, boarded together with the other 13 PRs of this batch — zero merge conflicts between them. - `typecheck:core` clean - complexity 2799 / baseline 3218 and cognitive-complexity 1265 / baseline 1437 — both under baseline - 71 focused assertions green across the 13 test files this batch adds or touches⚠️ base-red inherited: diegosouzapw#12732 — `Docs Gates (fast-path)`, `Merge integrity`, `No new ESLint warnings`, `Unit Tests fast-path` and `Fast Quality Gates` all reproduce on the pure `release/v3.8.51` tip (provider count 356 vs the 358 the modules define, SKILL.md drift, and `open-sse/utils/stream.ts` at 3115 > frozen 3098). None of them touch this diff. Thanks @anhtahaylove — the root-cause write-up, the measured before/after numbers and the red-before-green proof on every one of these made the batch reviewable as a unit.
Fixes #13172.
Problem
POST /api/telegram/updateserves two callers, and only one of them was authenticated:{ initData, message }) — verifies the TelegraminitDataHMAC. Safe.message.chat.idstraight from the body. The only gate wasisTelegramEnabled(), which merely checks that a bot token is configured on our side. Nothing proved the request came from Telegram.chat.idflows intoproxyChat(), which mints a real API key for that id and spends upstream model quota. A caller who knows the URL can therefore create unbounded keys and burn quota. They don't get the replies back (those go to the spoofed chat id), so this is resource abuse and key-table growth, not data exfiltration.setWebhookalso never registered asecret_token, so the header needed for verification was never being sent by Telegram in the first place.Fix
Use Telegram's own mechanism: the
secret_tokengiven tosetWebhookis echoed back on every delivery asX-Telegram-Bot-Api-Secret-Token.TELEGRAM_WEBHOOK_SECRETconfig getter (src/lib/telegram/config.ts).setWebhookregisterssecret_tokenwhen configured (src/lib/telegram/botApi.ts).proxyChat():Comparison uses
timingSafeEqualwith an up-front length check, mirroringwebhookSecretMatches's counterpart insrc/app/api/a2a/tasks/route.ts, so the shared-prefix length doesn't leak through response timing.The Mini App path is untouched and does not require the secret.
Incidental schema fix
The zod body schema typed
messageas an optional string (the Mini App shape), but a real Telegram update sendsmessageas an object..passthrough()preserves unknown keys but still type-checks known ones, so realistic webhook deliveries were rejected with 400 before reaching any routing.messagenow accepts either shape; each branch already narrows the shape it needs (typeof body.message === "string"for Mini App,extractChatMessage()for webhook). This surfaced while writing the test — a body that mirrors a genuine Telegram update.Verification
tests/unit/telegram-webhook-secret-13172.test.ts— 4/4 pass with the fix, 4/4 fail without it (verified by stashing the source changes):npx tsc --noEmitclean.Operator note — breaking for existing webhook deployments
Anyone already running the webhook must set
TELEGRAM_WEBHOOK_SECRETand re-runsetWebhookso Telegram starts sending the header; until then deliveries return 503. This is deliberate: silently serving an unauthenticated webhook is the vulnerability. Documented in.env.exampleanddocs/reference/ENVIRONMENT.md.Note
check:docs-countsreports 3 pre-existing strict drifts (171 → 172 migrations) on this base; unrelated to this change and fixed by #13160.