security: address all issues from automated PR review
All checks were successful
Test / rust-fmt-check (pull_request) Successful in 10s
Test / frontend-typecheck (pull_request) Successful in 1m9s
Test / frontend-tests (pull_request) Successful in 1m13s
PR Review Automation / review (pull_request) Successful in 2m58s
Test / rust-clippy (pull_request) Successful in 3m50s
Test / rust-tests (pull_request) Successful in 5m12s

- Add missing CQL escaping for &, |, +, - characters
- Improve escape_wiql() to escape more dangerous characters: ", \, (, ), ~, *, ?, ;, =
- Sanitize HTML in excerpts using strip_html_tags() to prevent XSS
- Add unit tests for escape_wiql, escape_cql, canonicalize_url functions
- Document expand_query() behavior (always returns at least original query)
- All tests pass (158/158), cargo fmt and clippy pass
This commit is contained in:
Shaun Arman 2026-04-14 20:26:05 -05:00
parent 708e1e9c18
commit e6d1965342
3 changed files with 138 additions and 10 deletions

View File

@ -16,6 +16,29 @@ fn escape_wiql(s: &str) -> String {
.replace('=', "\\=") .replace('=', "\\=")
} }
/// Basic HTML tag stripping to prevent XSS in excerpts
fn strip_html_tags(html: &str) -> String {
let mut result = String::new();
let mut in_tag = false;
for ch in html.chars() {
match ch {
'<' => in_tag = true,
'>' => in_tag = false,
_ if !in_tag => result.push(ch),
_ => {}
}
}
// Clean up whitespace
result
.split_whitespace()
.collect::<Vec<_>>()
.join(" ")
.trim()
.to_string()
}
/// Search Azure DevOps Wiki for content matching the query /// Search Azure DevOps Wiki for content matching the query
pub async fn search_wiki( pub async fn search_wiki(
org_url: &str, org_url: &str,
@ -81,9 +104,7 @@ pub async fn search_wiki(
path path
); );
let excerpt = item["content"] let excerpt = strip_html_tags(item["content"].as_str().unwrap_or(""))
.as_str()
.unwrap_or("")
.chars() .chars()
.take(300) .take(300)
.collect::<String>(); .collect::<String>();
@ -298,3 +319,68 @@ async fn fetch_work_item_details(
source: "Azure DevOps".to_string(), source: "Azure DevOps".to_string(),
}) })
} }
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn test_escape_wiql_escapes_single_quotes() {
assert_eq!(escape_wiql("test'single"), "test''single");
}
#[test]
fn test_escape_wiql_escapes_double_quotes() {
assert_eq!(escape_wiql("test\"double"), "test\\\\\"double");
}
#[test]
fn test_escape_wiql_escapes_backslash() {
assert_eq!(escape_wiql("test\\backslash"), r#"test\\backslash"#);
}
#[test]
fn test_escape_wiql_escapes_parens() {
assert_eq!(escape_wiql("test(paren"), r#"test\(paren"#);
assert_eq!(escape_wiql("test)paren"), r#"test\)paren"#);
}
#[test]
fn test_escape_wiql_escapes_tilde() {
assert_eq!(escape_wiql("test~tilde"), r#"test\~tilde"#);
}
#[test]
fn test_escape_wiql_escapes_asterisk() {
assert_eq!(escape_wiql("test*star"), r#"test\*star"#);
}
#[test]
fn test_escape_wiql_escapes_question() {
assert_eq!(escape_wiql("test?quest"), r#"test\?quest"#);
}
#[test]
fn test_escape_wiql_escapes_semicolon() {
assert_eq!(escape_wiql("test;semi"), r#"test\;semi"#);
}
#[test]
fn test_escape_wiql_escapes_equals() {
assert_eq!(escape_wiql("test=equal"), r#"test\=equal"#);
}
#[test]
fn test_escape_wiql_no_special_chars() {
assert_eq!(escape_wiql("simple query"), "simple query");
}
#[test]
fn test_strip_html_tags() {
let html = "<p>Hello <strong>world</strong>!</p>";
assert_eq!(strip_html_tags(html), "Hello world!");
let html2 = "<div><h1>Title</h1><p>Content</p></div>";
assert_eq!(strip_html_tags(html2), "TitleContent");
}
}

View File

@ -31,6 +31,10 @@ fn escape_cql(s: &str) -> String {
.replace(')', "\\)") .replace(')', "\\)")
.replace('(', "\\(") .replace('(', "\\(")
.replace('~', "\\~") .replace('~', "\\~")
.replace('&', "\\&")
.replace('|', "\\|")
.replace('+', "\\+")
.replace('-', "\\-")
} }
/// Search Confluence for content matching the query /// Search Confluence for content matching the query
@ -100,12 +104,10 @@ pub async fn search_confluence(
base_url.to_string() base_url.to_string()
}; };
let excerpt = item["excerpt"] let excerpt = strip_html_tags(item["excerpt"].as_str().unwrap_or(""))
.as_str() .chars()
.unwrap_or("") .take(300)
.to_string() .collect::<String>();
.replace("<span class=\"highlight\">", "")
.replace("</span>", "");
let content = if let Some(content_id) = id { let content = if let Some(content_id) = id {
fetch_page_content(base_url, content_id, &cookie_header) fetch_page_content(base_url, content_id, &cookie_header)
@ -217,4 +219,43 @@ mod tests {
let html2 = "<div><h1>Title</h1><p>Content</p></div>"; let html2 = "<div><h1>Title</h1><p>Content</p></div>";
assert_eq!(strip_html_tags(html2), "TitleContent"); assert_eq!(strip_html_tags(html2), "TitleContent");
} }
#[test]
fn test_escape_cql_escapes_special_chars() {
assert_eq!(escape_cql("test\"quote"), r#"test\"quote"#);
assert_eq!(escape_cql("test(paren"), r#"test\(paren"#);
assert_eq!(escape_cql("test)paren"), r#"test\)paren"#);
assert_eq!(escape_cql("test~tilde"), r#"test\~tilde"#);
assert_eq!(escape_cql("test&and"), r#"test\&and"#);
assert_eq!(escape_cql("test|or"), r#"test\|or"#);
assert_eq!(escape_cql("test+plus"), r#"test\+plus"#);
assert_eq!(escape_cql("test-minus"), r#"test\-minus"#);
}
#[test]
fn test_escape_cql_no_special_chars() {
assert_eq!(escape_cql("simple query"), "simple query");
}
#[test]
fn test_canonicalize_url_removes_fragment() {
assert_eq!(
canonicalize_url("https://example.com/page#section"),
"https://example.com/page"
);
}
#[test]
fn test_canonicalize_url_removes_query() {
assert_eq!(
canonicalize_url("https://example.com/page?param=value"),
"https://example.com/page"
);
}
#[test]
fn test_canonicalize_url_handles_malformed() {
// Malformed URLs fall back to original
assert_eq!(canonicalize_url("not a url"), "not a url");
}
} }

View File

@ -76,7 +76,8 @@ fn get_product_synonyms(query: &str) -> Vec<String> {
/// ///
/// # Returns /// # Returns
/// A vector of query strings to search, with the original query first /// A vector of query strings to search, with the original query first
/// followed by expanded variations /// followed by expanded variations. Returns empty only if input is empty or
/// whitespace-only. Otherwise, always returns at least the original query.
pub fn expand_query(query: &str) -> Vec<String> { pub fn expand_query(query: &str) -> Vec<String> {
if query.trim().is_empty() { if query.trim().is_empty() {
return Vec::new(); return Vec::new();