From 7751f6c9f7d497c7ee776cab24d1bcd31ee09960 Mon Sep 17 00:00:00 2001 From: Stefan Obradovic Date: Thu, 12 Mar 2026 00:35:17 +1000 Subject: [PATCH 1/4] fix: use floor_char_boundary for UTF-8 safe string truncation Fixes unsafe byte-level string slicing that panics on multi-byte UTF-8 content (Cyrillic, CJK, emoji) in browser tool display truncation and OAuth token debug logging. Closes #391 --- src/main.rs | 10 ++++++++-- src/tools/browser.rs | 6 +++--- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/main.rs b/src/main.rs index 776673a0f..ad3ed6f4e 100644 --- a/src/main.rs +++ b/src/main.rs @@ -521,8 +521,14 @@ fn cmd_auth(config_path: Option, auth_cmd: AuthCommand) -> a } else { eprintln!("Anthropic OAuth: valid (expires in {}m)", expires_min); } - eprintln!(" access token: {}...", &creds.access_token[..20]); - eprintln!(" refresh token: {}...", &creds.refresh_token[..20]); + eprintln!( + " access token: {}...", + &creds.access_token[..creds.access_token.floor_char_boundary(20)] + ); + eprintln!( + " refresh token: {}...", + &creds.refresh_token[..creds.refresh_token.floor_char_boundary(20)] + ); eprintln!( " credentials file: {}", spacebot::auth::credentials_path(&instance_dir).display() diff --git a/src/tools/browser.rs b/src/tools/browser.rs index d0590846c..6bfac29d0 100644 --- a/src/tools/browser.rs +++ b/src/tools/browser.rs @@ -530,7 +530,7 @@ fn render_snapshot_node(node: &SnapshotNode, depth: usize, output: &mut String) output.push_str(" \""); // Truncate very long names for context efficiency. let display_name = if node.name.len() > 200 { - format!("{}...", &node.name[..200]) + format!("{}...", &node.name[..node.name.floor_char_boundary(200)]) } else { node.name.clone() }; @@ -575,7 +575,7 @@ fn render_snapshot_node(node: &SnapshotNode, depth: usize, output: &mut String) // Value (e.g., text input current value) if let Some(ref value) = node.value { let display_value = if value.len() > 100 { - format!("{}...", &value[..100]) + format!("{}...", &value[..value.floor_char_boundary(100)]) } else { value.clone() }; @@ -1589,7 +1589,7 @@ impl Tool for BrowserTypeTool { ) } else { let display_text = if text_value.len() > 50 { - format!("{}...", &text_value[..50]) + format!("{}...", &text_value[..text_value.floor_char_boundary(50)]) } else { text_value }; From 78eec0ab9dec52aa75ce60068467583a31ef1fc2 Mon Sep 17 00:00:00 2001 From: Stefan Obradovic Date: Thu, 12 Mar 2026 00:41:16 +1000 Subject: [PATCH 2/4] fix: collapse nested if statements to satisfy clippy collapsible_if --- src/tools/send_message_to_another_channel.rs | 54 +++++++++----------- 1 file changed, 25 insertions(+), 29 deletions(-) diff --git a/src/tools/send_message_to_another_channel.rs b/src/tools/send_message_to_another_channel.rs index abf72e239..bb0681fab 100644 --- a/src/tools/send_message_to_another_channel.rs +++ b/src/tools/send_message_to_another_channel.rs @@ -150,14 +150,13 @@ impl Tool for SendMessageTool { // If explicit prefix returned default "signal" adapter but we're in a named // Signal adapter conversation (e.g., signal:gvoice1), use the current adapter // to ensure the message goes through the correct account. - if target.adapter == "signal" { - if let Some(current_adapter) = self + if target.adapter == "signal" + && let Some(current_adapter) = self .current_adapter .as_ref() .filter(|adapter| adapter.starts_with("signal:")) - { - target.adapter = current_adapter.clone(); - } + { + target.adapter = current_adapter.clone(); } self.messaging_manager @@ -189,31 +188,28 @@ impl Tool for SendMessageTool { .current_adapter .as_ref() .filter(|adapter| adapter.starts_with("signal")) + && let Some(target) = parse_implicit_signal_shorthand(&args.target, current_adapter) { - if let Some(target) = parse_implicit_signal_shorthand(&args.target, current_adapter) { - self.messaging_manager - .broadcast( - &target.adapter, - &target.target, - crate::OutboundResponse::Text(args.message), - ) - .await - .map_err(|error| { - SendMessageError(format!("failed to send message: {error}")) - })?; - - tracing::info!( - adapter = %target.adapter, - broadcast_target = %"[REDACTED]", - "message sent via implicit Signal shorthand" - ); - - return Ok(SendMessageOutput { - success: true, - target: target.target, - platform: target.adapter, - }); - } + self.messaging_manager + .broadcast( + &target.adapter, + &target.target, + crate::OutboundResponse::Text(args.message), + ) + .await + .map_err(|error| SendMessageError(format!("failed to send message: {error}")))?; + + tracing::info!( + adapter = %target.adapter, + broadcast_target = %"[REDACTED]", + "message sent via implicit Signal shorthand" + ); + + return Ok(SendMessageOutput { + success: true, + target: target.target, + platform: target.adapter, + }); } // Check for explicit email target From 47c26569b626bc042ebf54a04120b78292c2c424 Mon Sep 17 00:00:00 2001 From: Stefan Obradovic Date: Thu, 12 Mar 2026 00:43:55 +1000 Subject: [PATCH 3/4] fix: address review feedback - redact tokens, use truncate_utf8_ellipsis helper --- src/main.rs | 8 ++++---- src/tools/browser.rs | 18 +++--------------- 2 files changed, 7 insertions(+), 19 deletions(-) diff --git a/src/main.rs b/src/main.rs index ad3ed6f4e..120608375 100644 --- a/src/main.rs +++ b/src/main.rs @@ -522,12 +522,12 @@ fn cmd_auth(config_path: Option, auth_cmd: AuthCommand) -> a eprintln!("Anthropic OAuth: valid (expires in {}m)", expires_min); } eprintln!( - " access token: {}...", - &creds.access_token[..creds.access_token.floor_char_boundary(20)] + " access token: ({} bytes)", + creds.access_token.len() ); eprintln!( - " refresh token: {}...", - &creds.refresh_token[..creds.refresh_token.floor_char_boundary(20)] + " refresh token: ({} bytes)", + creds.refresh_token.len() ); eprintln!( " credentials file: {}", diff --git a/src/tools/browser.rs b/src/tools/browser.rs index 6bfac29d0..f5affe75f 100644 --- a/src/tools/browser.rs +++ b/src/tools/browser.rs @@ -529,11 +529,7 @@ fn render_snapshot_node(node: &SnapshotNode, depth: usize, output: &mut String) if !node.name.is_empty() { output.push_str(" \""); // Truncate very long names for context efficiency. - let display_name = if node.name.len() > 200 { - format!("{}...", &node.name[..node.name.floor_char_boundary(200)]) - } else { - node.name.clone() - }; + let display_name = super::truncate_utf8_ellipsis(&node.name, 200); output.push_str(&display_name.replace('"', "\\\"")); output.push('"'); } @@ -574,11 +570,7 @@ fn render_snapshot_node(node: &SnapshotNode, depth: usize, output: &mut String) // Value (e.g., text input current value) if let Some(ref value) = node.value { - let display_value = if value.len() > 100 { - format!("{}...", &value[..value.floor_char_boundary(100)]) - } else { - value.clone() - }; + let display_value = super::truncate_utf8_ellipsis(value, 100); output.push_str(&format!( " value=\"{}\"", display_value.replace('"', "\\\"") @@ -1588,11 +1580,7 @@ impl Tool for BrowserTypeTool { args.secret.as_deref().unwrap_or("unknown") ) } else { - let display_text = if text_value.len() > 50 { - format!("{}...", &text_value[..text_value.floor_char_boundary(50)]) - } else { - text_value - }; + let display_text = super::truncate_utf8_ellipsis(&text_value, 50); format!("Typed '{display_text}' into element at {label}") }; From 81c1bb882bdf53d7dba527a95e5a43ee37d846b5 Mon Sep 17 00:00:00 2001 From: Stefan Obradovic Date: Thu, 12 Mar 2026 00:49:12 +1000 Subject: [PATCH 4/4] test: add UTF-8 multibyte truncation tests for Cyrillic, CJK, and emoji --- src/tools.rs | 83 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 83 insertions(+) diff --git a/src/tools.rs b/src/tools.rs index eeaecd3ac..b30f46244 100644 --- a/src/tools.rs +++ b/src/tools.rs @@ -979,4 +979,87 @@ mod tests { assert_eq!(truncate_utf8_ellipsis(text, 10), "🙂..."); assert!(truncate_output(text, 5).starts_with("🙂")); } + + #[test] + fn truncate_cyrillic_does_not_panic() { + // Cyrillic chars are 2 bytes each in UTF-8 + let text = "Привет, мир!"; // "Hello, world!" in Russian + // Cutting at byte 5 would land inside 'и' (bytes 4..5) - this must not panic + let result = truncate_utf8_ellipsis(text, 5); + assert!(!result.is_empty()); + // "П" (2 bytes) + "..." (3 bytes) = 5, fits exactly + assert_eq!(result, "П..."); + + let result = truncate_utf8_ellipsis(text, 10); + assert!(!result.is_empty()); + // Should truncate to a valid char boundary and append "..." + assert!(result.ends_with("...")); + } + + #[test] + fn truncate_cjk_does_not_panic() { + // CJK chars are 3 bytes each in UTF-8 + let text = "你好世界测试"; // "Hello world test" in Chinese + // Cutting at byte 4 would land inside '好' (bytes 3..5) + let result = truncate_utf8_ellipsis(text, 4); + assert_eq!(result, "你"); // only 3 bytes fit, no room for "..." + + let result = truncate_utf8_ellipsis(text, 10); + assert!(!result.is_empty()); + // 2 CJK chars (6 bytes) + "..." (3 bytes) = 9 bytes fits in 10 + assert_eq!(result, "你好..."); + } + + #[test] + fn truncate_emoji_does_not_panic() { + // Emoji are 4 bytes each in UTF-8 + let text = "Hello 😀🎉🚀 World"; + let result = truncate_utf8_ellipsis(text, 10); + assert!(!result.is_empty()); + // "Hello " (6 bytes) + "😀" won't fit with "..." in 10 bytes + // "Hello " (6 bytes) + "..." (3 bytes) = 9 bytes fits + assert_eq!(result, "Hello ..."); + + // Larger budget + let result = truncate_utf8_ellipsis(text, 15); + assert!(!result.is_empty()); + assert!(result.ends_with("...")); + } + + #[test] + fn truncate_mixed_multibyte_content() { + // Mix of ASCII, Cyrillic (2-byte), CJK (3-byte), and emoji (4-byte) + let text = "Hi Привет 你好 😀"; + let result = truncate_utf8_ellipsis(text, 20); + assert!(!result.is_empty()); + assert!(result.ends_with("...")); + // Verify it's valid UTF-8 by iterating chars + assert!(result.chars().count() > 0); + } + + #[test] + fn truncate_output_multibyte_does_not_panic() { + let cyrillic = "Привет, мир! Это тестовая строка для проверки."; + let result = truncate_output(cyrillic, 15); + assert!(!result.is_empty()); + + let cjk = "你好世界，这是一个测试字符串。"; + let result = truncate_output(cjk, 10); + assert!(!result.is_empty()); + + let emoji = "🎉🚀😀🌍💻🔥"; + let result = truncate_output(emoji, 6); + assert!(!result.is_empty()); + } + + #[test] + fn truncate_at_exact_char_boundary_works() { + let text = "абв"; // 3 Cyrillic chars, 6 bytes total + // Exactly at a char boundary (4 bytes = 2 chars) + let result = truncate_utf8_ellipsis(text, 6); + assert_eq!(result, "абв"); // fits entirely + + let result = truncate_utf8_ellipsis(text, 7); + assert_eq!(result, "абв"); // also fits, no truncation needed + } }