diff --git a/bot/src/config.ts b/bot/src/config.ts index 595e3c3..9bbe25d 100644 --- a/bot/src/config.ts +++ b/bot/src/config.ts @@ -3,6 +3,10 @@ export interface Env { BOT_TOKEN: string; // wrangler secret (D-15) — Telegram Bot API token from @BotFather + // WEBHOOK_SECRET MUST be a non-empty secret (D-15) — /telegram/webhook + // fails closed at request time when missing/empty (WR-02), refusing all + // deliveries rather than letting grammY's secret check degrade to + // accept-everything. WEBHOOK_SECRET: string; // wrangler secret (D-15) — value Telegram echoes in X-Telegram-Bot-Api-Secret-Token chat_sessions: KVNamespace; // TTL-only namespace — first durable KV namespace (PRIV-02) } diff --git a/bot/src/webhook.ts b/bot/src/webhook.ts index e75b721..1385a24 100644 --- a/bot/src/webhook.ts +++ b/bot/src/webhook.ts @@ -6,8 +6,12 @@ * the configured secret token — Telegram's updates must carry * X-Telegram-Bot-Api-Secret-Token = env.WEBHOOK_SECRET or they are 401'd * by a constant-time compare BEFORE any bot processing or KV touch - * (T-02-03). The bot is instantiated per request because the token only - * arrives with `env` (research-verified Workers pattern). + * (T-02-03). FAIL-CLOSED (WR-02): if the WEBHOOK_SECRET binding is + * missing/empty at request time, the route rejects outright — grammY's + * compareSecretToken silently accepts EVERY request when its secret is + * undefined, so an unarmed binding must never reach it. The bot is + * instantiated per request because the token only arrives with `env` + * (research-verified Workers pattern). * POST /api/session → the phase's security gate (T-02-01): the caller * posts a verbatim Telegram.WebApp.initData string — capped at 4 KB * (WR-01: over-cap bodies are rejected 413 before any parsing; real @@ -18,7 +22,8 @@ * this endpoint adds no extra branch and no extra write path (T-02-06). * Anything else → 404. * - * D-14: the ONLY log statement is category-only (malformed/tampered/stale) — + * D-14: the ONLY log statements are category-only — malformed/tampered/stale + * on the initData gate, misconfigured on the fail-closed webhook gate — * never the raw payload, never user ids, never timestamps. Rejection * responses carry the category only. * D-17: no secret is ever logged or echoed; the bot token / webhook secret @@ -34,6 +39,17 @@ export default { async fetch(req: Request, env: Env, _ctx: ExecutionContext): Promise { const url = new URL(req.url); if (url.pathname === "/telegram/webhook") { + // WR-02 fail-closed: grammY's compareSecretToken accepts EVERY request + // when its secret is undefined (research-confirmed runtime behavior, + // convenience/webhook.js). A WEBHOOK_SECRET binding left unarmed after + // deploy would therefore silently disable webhook authentication and + // admit unsigned POSTs that inject fabricated updates. This guard runs + // BEFORE createBot/webhookCallback; the rejection is a constant 500 + // and the log carries the category only. + if (!env.WEBHOOK_SECRET) { + console.log("[webhook] rejected: misconfigured"); + return new Response("Server misconfigured", { status: 500 }); + } const bot = createBot(env); // env-bound token → per-request instantiation const cb = webhookCallback(bot, "cloudflare-mod", undefined, undefined, env.WEBHOOK_SECRET); return cb(req); diff --git a/bot/test/webhook.test.mjs b/bot/test/webhook.test.mjs index 7276bdc..86efe2b 100644 --- a/bot/test/webhook.test.mjs +++ b/bot/test/webhook.test.mjs @@ -13,7 +13,7 @@ * live Bot API. Armed-secret behavior is verified by the live smokes in * plan 02-03 instead. * - * Cases (WR-01 body cap, T-02-01 gate): + * Cases (WR-01 body cap, WR-02 fail-closed gate, T-02-01 gate): * WK1 POST valid initData under the cap → 200 {token: uuid}; * exactly two TTL-carrying KV writes (900 s) * WK2 POST an over-cap (4 KB+) but VALIDLY SIGNED initData → 413 @@ -24,6 +24,9 @@ * WK3 declared-length 999999 + unreadable body stream → 413 resolved * WITHOUT pulling the stream (the pre-read check refuses to buffer) * WK4 GET /api/session → 405 (the cap logic never runs on GET) + * WK5 unarmed WEBHOOK_SECRET → /telegram/webhook rejects CLOSED with a + * constant 500 BEFORE grammY's compareSecretToken (which would + * accept every request once the secret is undefined — WR-02) */ import { strict as assert } from "node:assert"; import { createHmac } from "node:crypto"; @@ -185,3 +188,17 @@ test("WK4 GET /api/session → 405", async () => { const res = await handler(req, makeEnv(makeKV()), {}); assert.equal(res.status, 405); }); + +/* ————— WK5 — WR-02 fail-closed webhook gate ————— */ + +test("WK5 unarmed WEBHOOK_SECRET ('') → webhook rejects CLOSED with a constant 500, no bot run, no KV touch", async () => { + const env = { BOT_TOKEN: TEST_TOKEN, WEBHOOK_SECRET: "", chat_sessions: undefined }; + const req = () => new Request("https://host/telegram/webhook", { method: "POST", body: '{\"update_id\":1}' }); + const res1 = await handler(req(), env, {}); + const res2 = await handler(req(), env, {}); + assert.equal(res1.status, 500, "missing binding must fail closed, not fall through to grammY's accept-everything compare"); + assert.equal(res2.status, 500, "the rejection is constant across calls"); + assert.equal(await res1.text(), "Server misconfigured"); + assert.equal(await res2.text(), "Server misconfigured"); + assert.ok(env.chat_sessions === undefined, "the guard fired before any storage or bot wiring was needed"); +});