fix: sanitize HTML error bodies from MCP servers to prevent web UI white screen (#263) (#656)

* fix: sanitize HTML error bodies from MCP servers to prevent web UI white screen (#263) * style: fix cargo fmt formatting in sanitize_error_body tests
2026-08-25 14:53:34 +00:00 · 2026-03-08 02:53:02 +00:00
parent 3b57d5bec9
commit a20e19ab16
3 changed files with 132 additions and 3 deletions
@@ -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 <html> tag
+    // (after optional whitespace), so normal messages that mention HTML
+    // tags in prose or code fences are not affected.  See #263.
+    if (/^\s*<!doctype\s/i.test(text) || /^\s*<html[\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);
@@ -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}"),
        })
    }

@@ -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 `<html` or `<!DOCTYPE`) and
+/// strips all tags, collapsing whitespace.  Non-HTML bodies are left
+/// intact.  In both cases the result is truncated to 200 *characters*
+/// (char-boundary safe) so that large payloads don't bloat error messages.
+///
+/// See #263 — raw HTML error pages were propagating through the error
+/// chain into the web UI, causing a white screen.
+fn sanitize_error_body(body: &str) -> 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("<html") || lower.contains("<!doctype");
+
+    let text = if is_html_document {
+        let stripped = body
+            .chars()
+            .fold((String::new(), false), |(mut out, in_tag), c| {
+                if c == '<' {
+                    (out, true)
+                } else if c == '>' {
+                    (out, false)
+                } else if !in_tag {
+                    out.push(c);
+                    (out, false)
+                } else {
+                    (out, true)
+                }
+            })
+            .0;
+        stripped.split_whitespace().collect::<Vec<_>>().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#"<!DOCTYPE html><html><body><h1>422 Error</h1><p>Invalid token</p></body></html>"#;
+        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!(
+            "<html><body><p>{}</p></body></html>",
+            "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 = "<HTML><BODY><H1>500 Internal Server Error</H1></BODY></HTML>";
+        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);
+    }
 }