From 46b55cfc65c33abb9082cf630857829144351b98 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Fri, 9 Oct 2026 16:34:54 +1300 Subject: [PATCH] fix(integrations): escape telegram text so titles with < or & still deliver Telegram parses every message with parse_mode HTML, but issue titles, errors, URLs and trigger reasons went in raw. A title such as "Vec & co" made Telegram reject the message as malformed HTML, and a crafted title could inject markup into the chat. None of the message templates carry markup of their own, so the text is escaped once at the send boundary instead of per interpolated value: no future interpolation can forget it. Truncation to Telegram's 4096 character limit now happens on the plain text before escaping, since the limit counts parsed characters and cutting after escaping could split an entity. Also removes the file's section-header comments. Co-Authored-By: Claude Opus 5.5 --- .../src/notifier/html.rs | 40 ++++++++ .../claudear-integrations/src/notifier/mod.rs | 1 + .../src/notifier/telegram.rs | 99 ++++++++++++------- 3 files changed, 104 insertions(+), 36 deletions(-) create mode 100644 crates/claudear-integrations/src/notifier/html.rs diff --git a/crates/claudear-integrations/src/notifier/html.rs b/crates/claudear-integrations/src/notifier/html.rs new file mode 100644 index 00000000..dc98fa96 --- /dev/null +++ b/crates/claudear-integrations/src/notifier/html.rs @@ -0,0 +1,40 @@ +//! Escaping for notifier channels that render HTML. + +/// Escape `text` so an HTML renderer shows it verbatim, inside element text or a +/// double-quoted attribute value. +pub(super) fn escape(text: &str) -> String { + let mut escaped = String::with_capacity(text.len()); + for character in text.chars() { + match character { + '&' => escaped.push_str("&"), + '<' => escaped.push_str("<"), + '>' => escaped.push_str(">"), + '"' => escaped.push_str("""), + other => escaped.push(other), + } + } + escaped +} + +#[cfg(test)] +mod tests { + use super::escape; + + #[test] + fn escapes_markup_characters() { + assert_eq!( + escape("Tom & Jerry"), + "<a href="x">Tom & Jerry</a>" + ); + } + + #[test] + fn escapes_existing_entities_again() { + assert_eq!(escape("&"), "&amp;"); + } + + #[test] + fn leaves_plain_text_unchanged() { + assert_eq!(escape("PROJ-1 'quoted' ünïcode"), "PROJ-1 'quoted' ünïcode"); + } +} diff --git a/crates/claudear-integrations/src/notifier/mod.rs b/crates/claudear-integrations/src/notifier/mod.rs index 43826039..d910ea7a 100644 --- a/crates/claudear-integrations/src/notifier/mod.rs +++ b/crates/claudear-integrations/src/notifier/mod.rs @@ -49,6 +49,7 @@ pub mod ask_orchestrator; mod console; mod discord; mod email; +mod html; mod push; mod slack; mod sms; diff --git a/crates/claudear-integrations/src/notifier/telegram.rs b/crates/claudear-integrations/src/notifier/telegram.rs index 50da2292..64cbc4cc 100644 --- a/crates/claudear-integrations/src/notifier/telegram.rs +++ b/crates/claudear-integrations/src/notifier/telegram.rs @@ -1,5 +1,6 @@ //! Telegram notifier via Telegram Bot API. +use super::html; use super::Notifier; use crate::ask_reply_inbox; use abnegate_http::HttpResponse; @@ -19,6 +20,9 @@ use serde::Deserialize; use std::collections::HashSet; use std::sync::RwLock; +const MAXIMUM_TEXT_LENGTH: usize = 4096; +const TRUNCATION_MARKER: &str = "..."; + /// Trait for HTTP client used by Telegram notifier. #[async_trait] pub trait TelegramHttpClient: Send + Sync { @@ -387,12 +391,13 @@ impl TelegramNotifier { let url = format!("https://api.telegram.org/bot{}/sendMessage", bot_token); - // Truncate message to Telegram limit (4096 chars) - let truncated_text = if text.len() > 4096 { - format!("{}...", &text[..text.floor_char_boundary(4093)]) + let truncated_text = if text.len() > MAXIMUM_TEXT_LENGTH { + let kept = text.floor_char_boundary(MAXIMUM_TEXT_LENGTH - TRUNCATION_MARKER.len()); + format!("{}{TRUNCATION_MARKER}", &text[..kept]) } else { text.to_string() }; + let escaped_text = html::escape(&truncated_text); let recipients = self.resolve_recipients(issue); @@ -401,7 +406,7 @@ impl TelegramNotifier { for chat_id in &recipients { let body = serde_json::json!({ "chat_id": chat_id, - "text": truncated_text, + "text": escaped_text, "parse_mode": "HTML" }); @@ -758,8 +763,6 @@ mod tests { } } - // --- Basic trait tests --- - #[test] fn test_name() { let notifier = TelegramNotifier::new(disabled_config(), empty_registry()); @@ -788,8 +791,6 @@ mod tests { ); } - // --- Disabled config tests (no HTTP calls) --- - #[tokio::test] async fn test_notify_start_disabled() { let notifier = TelegramNotifier::new(disabled_config(), empty_registry()); @@ -891,8 +892,6 @@ mod tests { assert!(notifier.is_enabled()); } - // --- Mock-based tests for HTTP-dependent functionality --- - #[tokio::test] async fn test_send_message_success() { let mock = MockTelegramClient::success(); @@ -1525,6 +1524,60 @@ mod tests { assert!(text.contains("Memory leak in worker")); } + #[tokio::test] + async fn test_notify_start_escapes_html_in_title() { + let mock = MockTelegramClient::success(); + let notifier = TelegramNotifier::with_http_client(enabled_config(), mock); + let issue = Issue::new( + "1", + "SEN-42", + " & \"co\"", + "https://sentry.io/42", + "sentry", + ); + notifier.notify_start(&issue).await.unwrap(); + + let calls = notifier.http.get_last_calls(); + assert_eq!( + calls[0].1["text"], + "[Claudear] Processing SEN-42 from sentry - <script>alert(1)</script> & "co"" + ); + } + + #[tokio::test] + async fn test_notify_failed_escapes_html_in_error() { + let mock = MockTelegramClient::success(); + let notifier = TelegramNotifier::with_http_client(enabled_config(), mock); + let issue = Issue::new("1", "SEN-42", "Title", "https://sentry.io/42", "sentry"); + notifier + .notify_failed(&issue, "expected `Vec` & got ``") + .await + .unwrap(); + + let calls = notifier.http.get_last_calls(); + assert_eq!( + calls[0].1["text"], + "[Claudear] FAILED SEN-42: expected `Vec<u8>` & got `<none>`" + ); + } + + #[tokio::test] + async fn test_send_message_truncates_before_escaping() { + let mock = MockTelegramClient::success(); + let notifier = TelegramNotifier::with_http_client(enabled_config(), mock); + notifier.notify_status(&"&".repeat(5000)).await.unwrap(); + + let calls = notifier.http.get_last_calls(); + let text = calls[0].1["text"].as_str().unwrap(); + let body = text + .strip_prefix("[Claudear] ") + .and_then(|rest| rest.strip_suffix("...")) + .expect("truncated text keeps its prefix and marker"); + let kept = 4093 - "[Claudear] ".len(); + assert_eq!(body.matches("&").count(), kept); + assert_eq!(body.len(), kept * "&".len()); + } + #[tokio::test] async fn test_notify_failed_short_error_not_truncated() { let mock = MockTelegramClient::success(); @@ -1590,8 +1643,6 @@ mod tests { assert_eq!(calls[0].1["chat_id"], "999999999"); } - // --- Tests for cascade success message --- - #[tokio::test] async fn test_notify_success_cascade_message_format() { let mock = MockTelegramClient::success(); @@ -1612,8 +1663,6 @@ mod tests { assert!(text.contains("https://github.com/downstream/repo/pull/5")); } - // --- Tests for PR update success message --- - #[tokio::test] async fn test_notify_success_pr_update_message_format() { let mock = MockTelegramClient::success(); @@ -1633,8 +1682,6 @@ mod tests { assert!(text.contains("https://github.com/org/repo/pull/77")); } - // --- Tests for regression resolved completed message --- - #[tokio::test] async fn test_notify_completed_regression_resolved_message_format() { let mock = MockTelegramClient::success(); @@ -1651,8 +1698,6 @@ mod tests { assert!(text.contains("no regression")); } - // --- Tests for regression detected failed message --- - #[tokio::test] async fn test_notify_failed_regression_detected_message_format() { let mock = MockTelegramClient::success(); @@ -1672,8 +1717,6 @@ mod tests { assert!(text.contains("Tests failing again")); } - // --- Tests for cascade failed message --- - #[tokio::test] async fn test_notify_failed_cascade_message_format() { let mock = MockTelegramClient::success(); @@ -1691,8 +1734,6 @@ mod tests { assert!(text.contains("Build error")); } - // --- Tests for notify_merged and notify_closed --- - #[tokio::test] async fn test_notify_merged_message_format() { let mock = MockTelegramClient::success(); @@ -1729,8 +1770,6 @@ mod tests { assert!(text.contains("https://github.com/org/repo/pull/43")); } - // --- Test failed cascade with long error truncation --- - #[tokio::test] async fn test_notify_failed_cascade_truncates_long_error() { let mock = MockTelegramClient::success(); @@ -1747,8 +1786,6 @@ mod tests { assert!(text.contains("...")); } - // --- Test regression with long error truncation --- - #[tokio::test] async fn test_notify_failed_regression_truncates_long_error() { let mock = MockTelegramClient::success(); @@ -1765,8 +1802,6 @@ mod tests { assert!(text.contains("...")); } - // --- Test parse_mode is always HTML --- - #[tokio::test] async fn test_all_messages_use_html_parse_mode() { let mock = MockTelegramClient::success(); @@ -1792,8 +1827,6 @@ mod tests { } } - // --- Test multiple recipients get the same text --- - #[tokio::test] async fn test_multiple_recipients_receive_same_text() { let mock = MockTelegramClient::success(); @@ -1813,8 +1846,6 @@ mod tests { assert_eq!(calls[2].1["chat_id"], "333333333"); } - // --- Test config with only to_chat_ids (no primary chat_id) --- - #[tokio::test] async fn test_config_with_only_to_chat_ids() { let mock = MockTelegramClient::success(); @@ -1827,8 +1858,6 @@ mod tests { assert_eq!(calls[0].1["chat_id"], "444444444"); } - // --- Test http_response_fields --- - #[test] fn test_http_response_fields() { let response = HttpResponse::new(201, "Created"); @@ -1836,8 +1865,6 @@ mod tests { assert_eq!(response.body, "Created"); } - // --- Additional coverage tests --- - #[tokio::test] async fn test_notify_merged_disabled() { let notifier = TelegramNotifier::new(disabled_config(), empty_registry());