From aacfec75a18e625a794620e45dcc88bb3a356b5b Mon Sep 17 00:00:00 2001 From: Noa Flaherty Date: Sat, 21 Feb 2026 16:33:54 -0500 Subject: [PATCH] fix: always reconcile webhook and normalize ingress URL Co-Authored-By: Claude --- .../telegram-webhook-manager.test.ts | 23 +++++++------ gateway/src/index.ts | 2 +- gateway/src/telegram/webhook-manager.ts | 34 ++++++------------- 3 files changed, 24 insertions(+), 35 deletions(-) diff --git a/gateway/src/__tests__/telegram-webhook-manager.test.ts b/gateway/src/__tests__/telegram-webhook-manager.test.ts index 8cf2839baa3..16198a79332 100644 --- a/gateway/src/__tests__/telegram-webhook-manager.test.ts +++ b/gateway/src/__tests__/telegram-webhook-manager.test.ts @@ -82,7 +82,7 @@ describe("reconcileTelegramWebhook", () => { expect((calls[1].body as any).allowed_updates).toEqual(["message", "edited_message"]); }); - test("does not call setWebhook when URL already matches", async () => { + test("always calls setWebhook even when URL already matches (secret may have rotated)", async () => { const calls: string[] = []; globalThis.fetch = mock(async (input: string | URL | Request) => { @@ -105,33 +105,36 @@ describe("reconcileTelegramWebhook", () => { const config = makeConfig(); await reconcileTelegramWebhook(config); - expect(calls).toEqual(["getWebhookInfo"]); + expect(calls).toEqual(["getWebhookInfo", "setWebhook"]); }); - test("calls setWebhook when secret may have changed (forceUpdate)", async () => { - const calls: string[] = []; + test("normalizes trailing slash on ingress base URL", async () => { + const calls: { method: string; body: unknown }[] = []; globalThis.fetch = mock(async (input: string | URL | Request) => { const url = typeof input === "string" ? input : input instanceof URL ? input.toString() : input.url; if (url.includes("/getWebhookInfo")) { - calls.push("getWebhookInfo"); + calls.push({ method: "getWebhookInfo", body: null }); return makeTelegramResponse({ - url: "https://example.ngrok.io/webhooks/telegram", + url: "", has_custom_certificate: false, pending_update_count: 0, }); } if (url.includes("/setWebhook")) { - calls.push("setWebhook"); + const req = typeof input === "object" && "json" in input ? input : null; + const body = req ? await (req as Request).json() : null; + calls.push({ method: "setWebhook", body }); return makeTelegramResponse(true); } return new Response("Not found", { status: 404 }); }) as any; - const config = makeConfig(); - await reconcileTelegramWebhook(config, { forceUpdate: true }); + const config = makeConfig({ ingressPublicBaseUrl: "https://example.ngrok.io/" }); + await reconcileTelegramWebhook(config); - expect(calls).toEqual(["getWebhookInfo", "setWebhook"]); + expect(calls).toHaveLength(2); + expect((calls[1].body as any).url).toBe("https://example.ngrok.io/webhooks/telegram"); }); test("skips reconciliation when bot token is not configured", async () => { diff --git a/gateway/src/index.ts b/gateway/src/index.ts index d3283fbaf46..7a22ba7ac19 100644 --- a/gateway/src/index.ts +++ b/gateway/src/index.ts @@ -143,7 +143,7 @@ function main() { config.telegramWebhookSecret = credentials.webhookSecret; log.info("Telegram credentials loaded from credential vault"); registerTelegramCommands(); - reconcileTelegramWebhook(config, { forceUpdate: true }).catch((err) => { + reconcileTelegramWebhook(config).catch((err) => { log.error({ err }, "Failed to reconcile Telegram webhook after credential change"); }); } else { diff --git a/gateway/src/telegram/webhook-manager.ts b/gateway/src/telegram/webhook-manager.ts index f97b772aea6..e158ce48643 100644 --- a/gateway/src/telegram/webhook-manager.ts +++ b/gateway/src/telegram/webhook-manager.ts @@ -17,16 +17,13 @@ const ALLOWED_UPDATES = ["message", "edited_message"]; * Reconciles the Telegram webhook registration against the expected state * derived from the gateway's ingress URL and current webhook secret. * - * If the currently registered webhook URL differs from the expected URL, - * or if the secret may have changed (we always re-set when the URL matches - * but we can't verify the secret from getWebhookInfo), the webhook is - * re-registered via setWebhook. - * - * This is safe to call repeatedly; Telegram treats setWebhook as idempotent. + * Always calls setWebhook because Telegram does not expose the current + * secret_token via getWebhookInfo — a secret rotation with an unchanged URL + * would be invisible to us, causing all deliveries to fail with 401. + * setWebhook is idempotent, so calling it unconditionally is safe. */ export async function reconcileTelegramWebhook( config: GatewayConfig, - options?: { forceUpdate?: boolean }, ): Promise { if (!config.telegramBotToken || !config.telegramWebhookSecret) { log.debug("Skipping webhook reconciliation: Telegram credentials not configured"); @@ -38,31 +35,20 @@ export async function reconcileTelegramWebhook( return; } - const expectedUrl = `${config.ingressPublicBaseUrl}/webhooks/telegram`; + // Strip trailing slashes to avoid double-slash in the path + // (e.g. "https://example.com/" + "/webhooks/telegram" => "https://example.com//webhooks/telegram") + const baseUrl = config.ingressPublicBaseUrl.replace(/\/+$/, ""); + const expectedUrl = `${baseUrl}/webhooks/telegram`; const info = await callTelegramApi(config, "getWebhookInfo", {}); - const urlMatches = info.url === expectedUrl; - - // Telegram does not expose the current secret_token via getWebhookInfo, - // so we cannot compare it directly. When credentials are refreshed - // (forceUpdate), we always re-set to ensure the secret is current. - if (urlMatches && !options?.forceUpdate) { - log.info( - { currentUrl: info.url, expectedUrl }, - "Telegram webhook URL matches expected state, no update needed", - ); - return; - } - log.info( { currentUrl: info.url || "(none)", expectedUrl, - forceUpdate: !!options?.forceUpdate, - urlMatches, + urlMatches: info.url === expectedUrl, }, - "Telegram webhook state differs from expected, updating", + "Reconciling Telegram webhook", ); await callTelegramApi(config, "setWebhook", {