diff --git a/src/channels/web/static/app.js b/src/channels/web/static/app.js index 538be296..1513c418 100644 --- a/src/channels/web/static/app.js +++ b/src/channels/web/static/app.js @@ -563,6 +563,13 @@ function sendApprovalAction(requestId, action) { function renderMarkdown(text) { if (typeof marked !== 'undefined') { + // Escape raw HTML error pages instead of rendering them as markup. + // Only triggers when the text *starts with* a doctype or tag + // (after optional whitespace), so normal messages that mention HTML + // tags in prose or code fences are not affected. See #263. + if (/^\s*]/i.test(text)) { + return escapeHtml(text); + } let html = marked.parse(text); // Sanitize HTML output to prevent XSS from tool output or LLM responses. html = sanitizeRenderedHtml(html); diff --git a/src/llm/session.rs b/src/llm/session.rs index 94b3f243..dd61e629 100644 --- a/src/llm/session.rs +++ b/src/llm/session.rs @@ -200,9 +200,10 @@ impl SessionManager { let status = response.status(); let body = response.text().await.unwrap_or_default(); + let preview = crate::agent::truncate_for_preview(&body, 200); Err(LlmError::SessionRenewalFailed { provider: "nearai".to_string(), - reason: format!("Validation failed: HTTP {}: {}", status, body), + reason: format!("Validation failed: HTTP {status}: {preview}"), }) } diff --git a/src/tools/mcp/client.rs b/src/tools/mcp/client.rs index 316851fc..e6af2d1c 100644 --- a/src/tools/mcp/client.rs +++ b/src/tools/mcp/client.rs @@ -261,9 +261,9 @@ impl McpClient { if !response.status().is_success() { let status = response.status(); let body = response.text().await.unwrap_or_default(); + let preview = sanitize_error_body(&body); return Err(ToolError::ExternalService(format!( - "MCP server returned status: {} - {}", - status, body + "MCP server returned status: {status} - {preview}", ))); } @@ -548,6 +548,58 @@ impl Tool for McpToolWrapper { } } +/// Sanitize an HTTP error response body for safe display. +/// +/// Detects full HTML error pages (containing ` String { + const MAX_CHARS: usize = 200; + + // Only strip tags when the body looks like a full HTML document. + // Plain text that happens to contain `<` / `>` (e.g. log lines, + // comparison expressions) is left untouched. + let lower = body.to_ascii_lowercase(); + let is_html_document = lower.contains("' { + (out, false) + } else if !in_tag { + out.push(c); + (out, false) + } else { + (out, true) + } + }) + .0; + stripped.split_whitespace().collect::>().join(" ") + } else { + body.to_string() + }; + + // Truncate at a char boundary (safe for multi-byte UTF-8). + if text.chars().count() > MAX_CHARS { + let byte_offset = text + .char_indices() + .nth(MAX_CHARS) + .map(|(i, _)| i) + .unwrap_or(text.len()); + format!("{}... ({} bytes total)", &text[..byte_offset], body.len()) + } else { + text + } +} + #[cfg(test)] mod tests { use super::*; @@ -740,4 +792,73 @@ mod tests { }; assert!(!tool.requires_approval()); } + + // Regression tests for #263: HTML error bodies must not propagate raw + // markup through the error chain into the web UI. + + #[test] + fn test_sanitize_error_body_strips_html_tags() { + let html = + r#"

422 Error

Invalid token

"#; + let result = sanitize_error_body(html); + assert!(!result.contains('<'), "HTML tags must be stripped"); + assert!(!result.contains('>'), "HTML tags must be stripped"); + assert!(result.contains("422 Error")); + assert!(result.contains("Invalid token")); + } + + #[test] + fn test_sanitize_error_body_truncates_large_html_page() { + let html = format!( + "

{}

", + "error detail ".repeat(50) + ); + let result = sanitize_error_body(&html); + assert!(result.contains("...")); + assert!(result.contains("bytes total)")); + assert!(!result.contains('<')); + } + + #[test] + fn test_sanitize_error_body_passes_short_plain_text() { + assert_eq!(sanitize_error_body("Not Found"), "Not Found"); + } + + #[test] + fn test_sanitize_error_body_truncates_long_plain_text() { + let long = "x".repeat(300); + let result = sanitize_error_body(&long); + assert!(result.contains("...")); + assert!(result.contains("300 bytes total)")); + } + + #[test] + fn test_sanitize_error_body_multibyte_no_panic() { + // 300 CJK characters = 900 bytes; truncation must land on a + // char boundary, not in the middle of a multi-byte sequence. + let cjk = "错误".repeat(150); + let result = sanitize_error_body(&cjk); + assert!(result.contains("...")); + // Must be valid UTF-8 (would have panicked otherwise). + assert!(result.is_char_boundary(result.len())); + } + + #[test] + fn test_sanitize_error_body_strips_uppercase_html() { + let html = "

500 Internal Server Error

"; + let result = sanitize_error_body(html); + assert!( + !result.contains('<'), + "uppercase HTML tags must be stripped" + ); + assert!(result.contains("500 Internal Server Error")); + } + + #[test] + fn test_sanitize_error_body_preserves_angle_brackets_in_non_html() { + // Text with < and > that is NOT an HTML document should be + // left untouched (e.g. log lines, comparison expressions). + let text = "value < 10 and value > 0"; + assert_eq!(sanitize_error_body(text), text); + } }