-
Notifications
You must be signed in to change notification settings - Fork 88
M4: Webhook lifecycle reconciliation #6211
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,191 @@ | ||
| import { describe, test, expect, mock, afterEach } from "bun:test"; | ||
| import { reconcileTelegramWebhook } from "../telegram/webhook-manager.js"; | ||
| import type { GatewayConfig } from "../config.js"; | ||
|
|
||
| function makeConfig(overrides: Partial<GatewayConfig> = {}): GatewayConfig { | ||
| return { | ||
| telegramBotToken: "test-bot-token", | ||
| telegramWebhookSecret: "test-webhook-secret", | ||
| telegramApiBaseUrl: "https://api.telegram.org", | ||
| assistantRuntimeBaseUrl: "http://localhost:7821", | ||
| routingEntries: [], | ||
| defaultAssistantId: undefined, | ||
| unmappedPolicy: "reject", | ||
| port: 7830, | ||
| runtimeBearerToken: undefined, | ||
| runtimeProxyEnabled: false, | ||
| runtimeProxyRequireAuth: false, | ||
| runtimeProxyBearerToken: undefined, | ||
| shutdownDrainMs: 5000, | ||
| runtimeTimeoutMs: 30000, | ||
| runtimeMaxRetries: 2, | ||
| runtimeInitialBackoffMs: 500, | ||
| telegramInitialBackoffMs: 1000, | ||
| telegramMaxRetries: 0, | ||
| telegramTimeoutMs: 15000, | ||
| maxWebhookPayloadBytes: 1048576, | ||
| logFile: { dir: undefined, retentionDays: 30 }, | ||
| maxAttachmentBytes: 20971520, | ||
| maxAttachmentConcurrency: 3, | ||
| twilioAuthToken: undefined, | ||
| ingressPublicBaseUrl: "https://example.ngrok.io", | ||
| publicUrl: undefined, | ||
| ...overrides, | ||
| }; | ||
| } | ||
|
|
||
| const originalFetch = globalThis.fetch; | ||
|
|
||
| afterEach(() => { | ||
| globalThis.fetch = originalFetch; | ||
| }); | ||
|
|
||
| function makeTelegramResponse(result: unknown) { | ||
| return new Response(JSON.stringify({ ok: true, result }), { | ||
| status: 200, | ||
| headers: { "content-type": "application/json" }, | ||
| }); | ||
| } | ||
|
|
||
| describe("reconcileTelegramWebhook", () => { | ||
| test("calls setWebhook when URL does not match", 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({ method: "getWebhookInfo", body: null }); | ||
| return makeTelegramResponse({ | ||
| url: "https://old-url.example.com/webhooks/telegram", | ||
| has_custom_certificate: false, | ||
| pending_update_count: 0, | ||
| }); | ||
| } | ||
| if (url.includes("/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); | ||
|
|
||
| expect(calls).toHaveLength(2); | ||
| expect(calls[0].method).toBe("getWebhookInfo"); | ||
| expect(calls[1].method).toBe("setWebhook"); | ||
| expect((calls[1].body as any).url).toBe("https://example.ngrok.io/webhooks/telegram"); | ||
| expect((calls[1].body as any).secret_token).toBe("test-webhook-secret"); | ||
| expect((calls[1].body as any).allowed_updates).toEqual(["message", "edited_message"]); | ||
| }); | ||
|
|
||
| test("does not call setWebhook when URL already matches", async () => { | ||
| const calls: string[] = []; | ||
|
|
||
| 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"); | ||
| return makeTelegramResponse({ | ||
| url: "https://example.ngrok.io/webhooks/telegram", | ||
| has_custom_certificate: false, | ||
| pending_update_count: 0, | ||
| }); | ||
| } | ||
| if (url.includes("/setWebhook")) { | ||
| calls.push("setWebhook"); | ||
| return makeTelegramResponse(true); | ||
| } | ||
| return new Response("Not found", { status: 404 }); | ||
| }) as any; | ||
|
|
||
| const config = makeConfig(); | ||
| await reconcileTelegramWebhook(config); | ||
|
|
||
| expect(calls).toEqual(["getWebhookInfo"]); | ||
| }); | ||
|
|
||
| test("calls setWebhook when secret may have changed (forceUpdate)", async () => { | ||
| const calls: string[] = []; | ||
|
|
||
| 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"); | ||
| return makeTelegramResponse({ | ||
| url: "https://example.ngrok.io/webhooks/telegram", | ||
| has_custom_certificate: false, | ||
| pending_update_count: 0, | ||
| }); | ||
| } | ||
| if (url.includes("/setWebhook")) { | ||
| calls.push("setWebhook"); | ||
| return makeTelegramResponse(true); | ||
| } | ||
| return new Response("Not found", { status: 404 }); | ||
| }) as any; | ||
|
|
||
| const config = makeConfig(); | ||
| await reconcileTelegramWebhook(config, { forceUpdate: true }); | ||
|
|
||
| expect(calls).toEqual(["getWebhookInfo", "setWebhook"]); | ||
| }); | ||
|
|
||
| test("skips reconciliation when bot token is not configured", async () => { | ||
| const fetchMock = mock(async () => new Response("", { status: 200 })); | ||
| globalThis.fetch = fetchMock as any; | ||
|
|
||
| const config = makeConfig({ telegramBotToken: undefined }); | ||
| await reconcileTelegramWebhook(config); | ||
|
|
||
| expect(fetchMock).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| test("skips reconciliation when webhook secret is not configured", async () => { | ||
| const fetchMock = mock(async () => new Response("", { status: 200 })); | ||
| globalThis.fetch = fetchMock as any; | ||
|
|
||
| const config = makeConfig({ telegramWebhookSecret: undefined }); | ||
| await reconcileTelegramWebhook(config); | ||
|
|
||
| expect(fetchMock).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| test("skips reconciliation when ingress URL is not configured", async () => { | ||
| const fetchMock = mock(async () => new Response("", { status: 200 })); | ||
| globalThis.fetch = fetchMock as any; | ||
|
|
||
| const config = makeConfig({ ingressPublicBaseUrl: undefined }); | ||
| await reconcileTelegramWebhook(config); | ||
|
|
||
| expect(fetchMock).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| test("calls setWebhook when current URL is empty", async () => { | ||
| const calls: string[] = []; | ||
|
|
||
| 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"); | ||
| return makeTelegramResponse({ | ||
| url: "", | ||
| has_custom_certificate: false, | ||
| pending_update_count: 0, | ||
| }); | ||
| } | ||
| if (url.includes("/setWebhook")) { | ||
| calls.push("setWebhook"); | ||
| return makeTelegramResponse(true); | ||
| } | ||
| return new Response("Not found", { status: 404 }); | ||
| }) as any; | ||
|
|
||
| const config = makeConfig(); | ||
| await reconcileTelegramWebhook(config); | ||
|
|
||
| expect(calls).toEqual(["getWebhookInfo", "setWebhook"]); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,75 @@ | ||
| import type { GatewayConfig } from "../config.js"; | ||
| import { callTelegramApi } from "./api.js"; | ||
| import { getLogger } from "../logger.js"; | ||
|
|
||
| const log = getLogger("webhook-manager"); | ||
|
|
||
| interface WebhookInfo { | ||
| url: string; | ||
| has_custom_certificate: boolean; | ||
| pending_update_count: number; | ||
| /** Telegram does not return the secret itself, but we can detect a mismatch by re-setting. */ | ||
| } | ||
|
|
||
| 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. | ||
| */ | ||
| export async function reconcileTelegramWebhook( | ||
| config: GatewayConfig, | ||
| options?: { forceUpdate?: boolean }, | ||
| ): Promise<void> { | ||
| if (!config.telegramBotToken || !config.telegramWebhookSecret) { | ||
| log.debug("Skipping webhook reconciliation: Telegram credentials not configured"); | ||
| return; | ||
| } | ||
|
|
||
| if (!config.ingressPublicBaseUrl) { | ||
| log.debug("Skipping webhook reconciliation: INGRESS_PUBLIC_BASE_URL not set"); | ||
| return; | ||
| } | ||
|
|
||
| const expectedUrl = `${config.ingressPublicBaseUrl}/webhooks/telegram`; | ||
|
|
||
| const info = await callTelegramApi<WebhookInfo>(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) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This early return skips Useful? React with 👍 / 👎. |
||
| 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, | ||
| }, | ||
| "Telegram webhook state differs from expected, updating", | ||
| ); | ||
|
|
||
| await callTelegramApi(config, "setWebhook", { | ||
| url: expectedUrl, | ||
| secret_token: config.telegramWebhookSecret, | ||
| allowed_updates: ALLOWED_UPDATES, | ||
| }); | ||
|
|
||
| log.info({ url: expectedUrl }, "Telegram webhook registered successfully"); | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Building
expectedUrlvia raw string concatenation means a configuredINGRESS_PUBLIC_BASE_URLending with/produces...//webhooks/telegram. Telegram will then post to a different path than the gateway route check (/webhooks/telegram), so webhook delivery fails even though reconciliation reports success. Because this commit automates registration, this malformed URL can now be introduced automatically from config instead of only via manual setup mistakes.Useful? React with 👍 / 👎.