-
-
Notifications
You must be signed in to change notification settings - Fork 3k
fix(notifications): escape and truncate Telegram error messages and check API response (#5392) #5440
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
fix(notifications): escape and truncate Telegram error messages and check API response (#5392) #5440
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,121 @@ | ||
| import { describe, expect, it, vi } from "vitest"; | ||
|
|
||
| describe("Telegram Error Notifications (Fixes Issue #5392)", () => { | ||
| // Utility implementations matching packages/server/src/utils/notifications/utils.ts | ||
| const escapeHtml = (text: string): string => { | ||
| return text | ||
| .replace(/&/g, "&") | ||
| .replace(/</g, "<") | ||
| .replace(/>/g, ">"); | ||
| }; | ||
|
|
||
| const formatTelegramErrorMessage = ( | ||
| errorMessage: string, | ||
| maxLen = 3000, | ||
| ): string => { | ||
| const truncated = | ||
| errorMessage.length > maxLen | ||
| ? `${errorMessage.substring(0, maxLen)}…` | ||
| : errorMessage; | ||
| return escapeHtml(truncated); | ||
| }; | ||
|
|
||
| const sendTelegramNotification = async ( | ||
| connection: { | ||
| botToken: string; | ||
| chatId: string; | ||
| messageThreadId?: string; | ||
| }, | ||
| messageText: string, | ||
| inlineButton?: { text: string; url: string }[][], | ||
| fetchMock = fetch, | ||
| ) => { | ||
| try { | ||
| const url = `https://api.telegram.org/bot${connection.botToken}/sendMessage`; | ||
| const response = await fetchMock(url, { | ||
| method: "POST", | ||
| headers: { "Content-Type": "application/json" }, | ||
| body: JSON.stringify({ | ||
| chat_id: connection.chatId, | ||
| message_thread_id: connection.messageThreadId, | ||
| text: messageText, | ||
| parse_mode: "HTML", | ||
| disable_web_page_preview: true, | ||
| reply_markup: { | ||
| inline_keyboard: inlineButton, | ||
| }, | ||
| }), | ||
| }); | ||
| if (!response.ok) { | ||
| const errorText = await response.text(); | ||
| throw new Error( | ||
| `Failed to send telegram notification: ${response.status} ${errorText}`, | ||
| ); | ||
| } | ||
| } catch (err) { | ||
| throw new Error( | ||
| `Failed to send telegram notification ${err instanceof Error ? err.message : "Unknown error"}`, | ||
| ); | ||
| } | ||
| }; | ||
|
|
||
| it("escapes raw HTML tags in error message to prevent Telegram parse_mode rejection", () => { | ||
| const rawError = 'process "/bin/sh -c npm ci" did not complete: exit 1 <-- see log & check <stdin>'; | ||
| const formatted = formatTelegramErrorMessage(rawError); | ||
|
|
||
| expect(formatted).not.toContain("<--"); | ||
| expect(formatted).not.toContain("<stdin>"); | ||
| expect(formatted).toContain("<-- see log & check <stdin>"); | ||
| }); | ||
|
|
||
| it("truncates excessively long error message to fit within Telegram 4096-char payload limit", () => { | ||
| // Simulating 22 KB compose build failure log | ||
| const hugeError = "x".repeat(22000) + "<failure>"; | ||
| const formatted = formatTelegramErrorMessage(hugeError, 3000); | ||
|
|
||
| expect(formatted.length).toBeLessThanOrEqual(3002 + 10); // 3000 + ellipsis + HTML escaping | ||
| expect(formatted.endsWith("…")).toBe(true); | ||
| expect(formatted).not.toContain("<failure>"); | ||
| }); | ||
|
|
||
| it("preserves short clean error message without unwanted truncation", () => { | ||
| const shortError = "Connection refused on port 5432"; | ||
| const formatted = formatTelegramErrorMessage(shortError); | ||
|
|
||
| expect(formatted).toBe(shortError); | ||
| }); | ||
|
|
||
| it("throws error when Telegram API returns non-2xx response instead of silently swallowing", async () => { | ||
| const fakeFetch = vi.fn(async () => ({ | ||
| ok: false, | ||
| status: 400, | ||
| text: async () => '{"ok":false,"error_code":400,"description":"Bad Request: message is too long"}', | ||
| })) as unknown as typeof fetch; | ||
|
|
||
| await expect( | ||
| sendTelegramNotification( | ||
| { botToken: "test-token", chatId: "test-chat" }, | ||
| "<b>Build Failed</b>", | ||
| undefined, | ||
| fakeFetch, | ||
| ), | ||
| ).rejects.toThrow("Bad Request: message is too long"); | ||
| }); | ||
|
|
||
| it("succeeds when Telegram API returns 200 OK", async () => { | ||
| const fakeFetch = vi.fn(async () => ({ | ||
| ok: true, | ||
| status: 200, | ||
| text: async () => '{"ok":true,"result":{}}', | ||
| })) as unknown as typeof fetch; | ||
|
|
||
| await expect( | ||
| sendTelegramNotification( | ||
| { botToken: "test-token", chatId: "test-chat" }, | ||
| "<b>Build Failed</b>", | ||
| undefined, | ||
| fakeFetch, | ||
| ), | ||
| ).resolves.toBeUndefined(); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -101,6 +101,24 @@ export const sendDiscordNotification = async ( | |
| } | ||
| }; | ||
|
|
||
| export const escapeHtml = (text: string): string => { | ||
| return text | ||
| .replace(/&/g, "&") | ||
| .replace(/</g, "<") | ||
| .replace(/>/g, ">"); | ||
| }; | ||
|
|
||
| export const formatTelegramErrorMessage = ( | ||
| errorMessage: string, | ||
| maxLen = 3000, | ||
| ): string => { | ||
| const truncated = | ||
| errorMessage.length > maxLen | ||
| ? `${errorMessage.substring(0, maxLen)}…` | ||
| : errorMessage; | ||
| return escapeHtml(truncated); | ||
|
Comment on lines
+115
to
+119
Contributor
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. Truncating before HTML escaping does not enforce Telegram's length limit. For example, 3,000 ampersands expand to 15,000 characters after escaping, so Telegram still rejects the notification as too long. Bound the escaped output without cutting an HTML entity, or calculate the output within the final payload budget. |
||
| }; | ||
|
|
||
| export const sendTelegramNotification = async ( | ||
| connection: typeof telegram.$inferInsert, | ||
| messageText: string, | ||
|
|
@@ -111,7 +129,7 @@ export const sendTelegramNotification = async ( | |
| ) => { | ||
| try { | ||
| const url = `https://api.telegram.org/bot${connection.botToken}/sendMessage`; | ||
| await fetch(url, { | ||
| const response = await fetch(url, { | ||
| method: "POST", | ||
| headers: { "Content-Type": "application/json" }, | ||
| body: JSON.stringify({ | ||
|
|
@@ -125,8 +143,17 @@ export const sendTelegramNotification = async ( | |
| }, | ||
| }), | ||
| }); | ||
| if (!response.ok) { | ||
| const errorText = await response.text(); | ||
| throw new Error( | ||
| `Failed to send telegram notification: ${response.status} ${errorText}`, | ||
| ); | ||
| } | ||
| } catch (err) { | ||
| console.log(err); | ||
| console.log("error", err); | ||
| throw new Error( | ||
| `Failed to send telegram notification ${err instanceof Error ? err.message : "Unknown error"}`, | ||
| ); | ||
|
Comment on lines
152
to
+156
Contributor
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 now rethrows Telegram API and network failures, but each notification handler wraps the entire channel sequence in one try/catch. Because Telegram runs before Slack, Mattermost, custom, Lark, Pushover, and Teams, a Telegram failure prevents those later configured channels from receiving the same alert. |
||
| } | ||
| }; | ||
|
|
||
|
|
||
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.
These tests reimplement all three production functions and therefore execute no production notification code. They will keep passing if the real escaping, truncation, exports, fetch handling, or handler wiring regresses. Import the exported utilities and mock global
fetchinstead so CI covers the actual implementation.