Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions src/channels/web/static/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -541,6 +541,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);
Expand Down
3 changes: 2 additions & 1 deletion src/llm/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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}"),
})
}

Expand Down
125 changes: 123 additions & 2 deletions src/tools/mcp/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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}",
)));
}

Expand Down Expand Up @@ -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::*;
Expand Down Expand Up @@ -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);
}
}
Loading