diff --git a/crates/forge_domain/src/auth/new_types.rs b/crates/forge_domain/src/auth/new_types.rs index 2d1e10ffee..579bf2b0ba 100644 --- a/crates/forge_domain/src/auth/new_types.rs +++ b/crates/forge_domain/src/auth/new_types.rs @@ -30,10 +30,13 @@ impl AsRef 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); + } +} diff --git a/crates/forge_json_repair/src/schema_coercion.rs b/crates/forge_json_repair/src/schema_coercion.rs index 6a88da28ca..9e8a4bd838 100644 --- a/crates/forge_json_repair/src/schema_coercion.rs +++ b/crates/forge_json_repair/src/schema_coercion.rs @@ -451,9 +451,13 @@ fn extract_array_from_string(s: &str) -> Option { } // 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 { } } + // 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::(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()); + } } diff --git a/crates/forge_main/src/tools_display.rs b/crates/forge_main/src/tools_display.rs index 703fe95dd0..960bcbb35e 100644 --- a/crates/forge_main/src/tools_display.rs +++ b/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 ' 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); + } +} diff --git a/crates/forge_repo/src/provider/provider_repo.rs b/crates/forge_repo/src/provider/provider_repo.rs index 74c71e9c08..d17c1c25c5 100644 --- a/crates/forge_repo/src/provider/provider_repo.rs +++ b/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::() ); // Create new credential with fresh token, preserving url_params and provider ID