fix: replace byte-index string slicing with char-safe truncation (#2903)

Co-authored-by: ForgeCode <noreply@forgecode.dev>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: Tushar Mathur <tusharmath@gmail.com>

Author: 4 people

crates/forge_domain/src/auth/new_types.rs

@@ -30,10 +30,13 @@ impl AsRef<str> for ApiKey {
 /// # Returns
 /// * A truncated version of the key for safe display
 pub fn truncate_key(key: &str) -> String {
-    if key.len() <= 20 {
+    let char_count = key.chars().count();
+    if char_count <= 20 {
         key.to_string()
     } else {
-        format!("{}...{}", &key[..=12], &key[key.len() - 4..])
+        let prefix: String = key.chars().take(13).collect();
+        let suffix: String = key.chars().skip(char_count - 4).collect();
+        format!("{prefix}...{suffix}")
     }
 }
 
@@ -153,3 +156,61 @@ pub struct RefreshToken(String);
 )]
 #[serde(transparent)]
 pub struct AccessToken(String);
+
+#[cfg(test)]
+mod tests {
+    use pretty_assertions::assert_eq;
+
+    use super::*;
+
+    #[test]
+    fn test_truncate_key_short_key() {
+        let fixture = "sk-abc123";
+        let actual = truncate_key(fixture);
+        let expected = "sk-abc123";
+        assert_eq!(actual, expected);
+    }
+
+    #[test]
+    fn test_truncate_key_long_ascii_key() {
+        let fixture = "sk-1234567890abcdefghijklmnop";
+        let actual = truncate_key(fixture);
+        let expected = "sk-1234567890...mnop";
+        assert_eq!(actual, expected);
+    }
+
+    #[test]
+    fn test_truncate_key_multibyte_chars_no_panic() {
+        // Keys with multi-byte UTF-8 characters should not panic
+        let fixture = "sk-12345678→→→→→→→→→→abcd";
+        let actual = truncate_key(fixture);
+        let expected = "sk-12345678→→...abcd";
+        assert_eq!(actual, expected);
+    }
+
+    #[test]
+    fn test_truncate_key_emoji_chars_no_panic() {
+        // Keys with 4-byte emoji characters should not panic
+        // 25 chars: a(13) + 🔑(8) + b(4) = 25
+        let fixture = "aaaaaaaaaaaaa🔑🔑🔑🔑🔑🔑🔑🔑bbbb";
+        let actual = truncate_key(fixture);
+        let expected = "aaaaaaaaaaaaa...bbbb";
+        assert_eq!(actual, expected);
+    }
+
+    #[test]
+    fn test_truncate_key_exactly_20_chars() {
+        let fixture = "12345678901234567890";
+        let actual = truncate_key(fixture);
+        let expected = "12345678901234567890";
+        assert_eq!(actual, expected);
+    }
+
+    #[test]
+    fn test_truncate_key_21_chars() {
+        let fixture = "123456789012345678901";
+        let actual = truncate_key(fixture);
+        let expected = "1234567890123...8901";
+        assert_eq!(actual, expected);
+    }
+}

crates/forge_json_repair/src/schema_coercion.rs

@@ -451,9 +451,13 @@ fn extract_array_from_string(s: &str) -> Option<Value> {
     }
 
     // Try to find matching closing bracket by parsing incrementally
-    // Start from the opening bracket and try to parse increasingly longer
-    // substrings We'll try the json_repair on the extracted portion
-    for end_idx in (start_idx + 1..=s.len()).rev() {
+    // Start from the opening bracket and try increasingly shorter substrings.
+    // We iterate over valid char boundaries to avoid panicking on multi-byte
+    // UTF-8 characters where byte offsets can land inside a character.
+    for (end_idx, _) in s.char_indices().rev() {
+        if end_idx <= start_idx {
+            break;
+        }
         let candidate = &s[start_idx..end_idx];
 
         // Try to repair and parse this candidate
@@ -464,6 +468,15 @@ fn extract_array_from_string(s: &str) -> Option<Value> {
         }
     }
 
+    // Also try the full string as a last resort (end at s.len() which is
+    // always a valid boundary)
+    let candidate = &s[start_idx..];
+    if let Ok(parsed) = crate::json_repair::<Value>(candidate)
+        && parsed.is_array()
+    {
+        return Some(parsed);
+    }
+
     None
 }
 
@@ -1344,4 +1357,39 @@ mod tests {
         let expected = json!({"count": null});
         assert_eq!(actual, expected);
     }
+
+    #[test]
+    fn test_extract_array_from_string_with_multibyte_chars() {
+        // Multi-byte UTF-8 characters (like arrows and emojis) should not
+        // cause panics when extract_array_from_string iterates over byte
+        // positions. The function must only slice at valid char boundaries.
+        let input = "prefix → [1, 2, 3] suffix";
+        let result = extract_array_from_string(input);
+        assert!(result.is_some());
+        let arr = result.unwrap();
+        assert!(arr.is_array());
+        assert_eq!(arr.as_array().unwrap().len(), 3);
+    }
+
+    #[test]
+    fn test_extract_array_from_string_with_emoji_prefix() {
+        // Emoji characters are 4 bytes each, many byte positions inside them
+        // are invalid char boundaries.
+        let input = "🔑🔒 [4, 5, 6]";
+        let result = extract_array_from_string(input);
+        assert!(result.is_some());
+        let arr = result.unwrap();
+        assert!(arr.is_array());
+        assert_eq!(arr.as_array().unwrap().len(), 3);
+    }
+
+    #[test]
+    fn test_extract_array_from_string_with_multibyte_inside_array() {
+        // Multi-byte chars inside the array value itself
+        let input = r#"["αβγ", "δεζ"]"#;
+        let result = extract_array_from_string(input);
+        assert!(result.is_some());
+        let arr = result.unwrap();
+        assert!(arr.is_array());
+    }
 }

crates/forge_main/src/tools_display.rs

@@ -48,14 +48,74 @@ pub fn format_tools(agent_tools: &[ToolName], overview: &ToolsOverview) -> Info
         for (server_name, error) in overview.mcp.get_failures().iter() {
             // Truncate error message for readability in list view
             // Use 'mcp show <name>' for full error details
-            let truncated_error = if error.len() > 80 {
-                format!("{}...", &error[..77])
-            } else {
-                error.clone()
-            };
+            let truncated_error = truncate_error(error);
             info = info.add_value(format!("[✗] {server_name} - {truncated_error}"));
         }
     }
 
     info
 }
+
+/// Truncates an error message to at most 80 characters for display.
+///
+/// If the message exceeds 80 characters, the first 77 characters are kept
+/// followed by "...". Uses character-based counting to avoid panicking on
+/// multi-byte UTF-8 strings.
+fn truncate_error(error: &str) -> String {
+    if error.chars().count() > 80 {
+        let truncated: String = error.chars().take(77).collect();
+        format!("{truncated}...")
+    } else {
+        error.to_string()
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use pretty_assertions::assert_eq;
+
+    use super::*;
+
+    #[test]
+    fn test_truncate_error_short_message() {
+        let fixture = "Connection refused";
+        let actual = truncate_error(fixture);
+        let expected = "Connection refused";
+        assert_eq!(actual, expected);
+    }
+
+    #[test]
+    fn test_truncate_error_long_ascii_message() {
+        let fixture = "A".repeat(100);
+        let actual = truncate_error(&fixture);
+        assert_eq!(actual.chars().count(), 80);
+        assert!(actual.ends_with("..."));
+    }
+
+    #[test]
+    fn test_truncate_error_multibyte_chars_no_panic() {
+        // Error containing multi-byte UTF-8 chars (→ is 3 bytes)
+        let fixture = "Error: struct → prioritizes struct definitions, trait → prioritizes traits, impl → prioritizes impls, and more details follow here";
+        let actual = truncate_error(fixture);
+        // Should not panic and should truncate correctly by char count
+        assert!(actual.chars().count() <= 80);
+        assert!(actual.ends_with("..."));
+    }
+
+    #[test]
+    fn test_truncate_error_emoji_no_panic() {
+        // Error containing 4-byte emojis - 90 emoji chars > 80 limit
+        let fixture = "🚀".repeat(90);
+        let actual = truncate_error(&fixture);
+        assert_eq!(actual.chars().count(), 80);
+        assert!(actual.ends_with("..."));
+    }
+
+    #[test]
+    fn test_truncate_error_exactly_80_chars() {
+        let fixture = "A".repeat(80);
+        let actual = truncate_error(&fixture);
+        let expected = "A".repeat(80);
+        assert_eq!(actual, expected);
+    }
+}

crates/forge_repo/src/provider/provider_repo.rs

@@ -460,7 +460,7 @@ impl<
         );
         tracing::debug!(
             "Token starts with: {}",
-            &access_token.token[..access_token.token.len().min(20)]
+            access_token.token.chars().take(20).collect::<String>()
         );
 
         // Create new credential with fresh token, preserving url_params and provider ID