diff --git a/src/agent-memory/src/consolidation/episode.rs b/src/agent-memory/src/consolidation/episode.rs index d35e08754..1d9e55b8d 100644 --- a/src/agent-memory/src/consolidation/episode.rs +++ b/src/agent-memory/src/consolidation/episode.rs @@ -161,8 +161,18 @@ impl Episode { ) } + /// Sanitize a value for safe frontmatter inclusion. + /// Replaces newlines and ASCII control chars with spaces. + /// Does NOT apply YAML double-quoting or backslash escaping: + /// the hand-rolled frontmatter readers do not interpret those. fn sanitize(&self, s: &str) -> String { - s.replace('\n', " ").replace('"', "'") + s.chars() + .map(|c| match c { + '\n' | '\r' => ' ', + c if c.is_ascii_control() => ' ', + other => other, + }) + .collect() } } diff --git a/src/agent-memory/src/consolidation/fact.rs b/src/agent-memory/src/consolidation/fact.rs index 9817a4127..5d62035ca 100644 --- a/src/agent-memory/src/consolidation/fact.rs +++ b/src/agent-memory/src/consolidation/fact.rs @@ -106,12 +106,12 @@ impl ConsolidatedFact { out.push_str(&format!("id: {}\n", self.id)); out.push_str(&format!("session_id: {}\n", self.session_id)); out.push_str(&format!("category: {}\n", self.category)); - out.push_str(&format!("title: {}\n", yaml_quote(&self.title))); + out.push_str(&format!("title: {}\n", sanitize_hint(&self.title))); out.push_str(&format!("source_tool: {}\n", self.source_tool)); if !self.related_paths.is_empty() { out.push_str("related_paths:\n"); for p in &self.related_paths { - out.push_str(&format!(" - {}\n", yaml_quote(p))); + out.push_str(&format!(" - {}\n", sanitize_hint(p))); } } out.push_str(&format!("created_at: {}\n", self.created_at)); @@ -131,15 +131,27 @@ impl ConsolidatedFact { } } -/// Emit a YAML double-quoted scalar. Escapes `\`, `"`, and newlines so -/// user-controlled values (file paths, search queries, error messages) -/// cannot break frontmatter parsing. -fn yaml_quote(s: &str) -> String { - let escaped: String = s - .replace('\\', "\\\\") - .replace('"', "\\\"") - .replace('\n', " "); - format!("\"{escaped}\"") +/// Sanitize a value for safe frontmatter inclusion. +/// +/// The hand-rolled frontmatter readers (parse_frontmatter_flat / +/// extract_title_and_description) use split_once(": ") + +/// trim_matches('"') and do NOT interpret YAML escapes or double-quoting. +/// YAML-style escaping would be asymmetric and cause round-trip +/// regression on Windows paths containing backslashes. +/// +/// Strategy: +/// - Replace newlines (\n, \r) with spaces to prevent premature +/// termination of the "---" frontmatter block marker. +/// - Replace other ASCII control characters (\x00-\x1F) with spaces. +/// - Keep all other characters unchanged, including #, :, ", and \. +fn sanitize_hint(s: &str) -> String { + s.chars() + .map(|c| match c { + '\n' | '\r' => ' ', + c if c.is_ascii_control() => ' ', + other => other, + }) + .collect() } #[cfg(test)] @@ -202,4 +214,130 @@ mod tests { assert_eq!(FactCategory::WorkingContext.to_string(), "working-context"); assert_eq!(FactCategory::Lesson.to_string(), "lesson"); } + + #[test] + fn sanitize_hint_preserves_normal_text() { + let result = sanitize_hint("a simple title"); + assert_eq!(result, "a simple title"); + } + + #[test] + fn sanitize_hint_preserves_hashes() { + let result = sanitize_hint("fix #123 and #456"); + assert_eq!(result, "fix #123 and #456"); + } + + #[test] + fn sanitize_hint_preserves_colons() { + let result = sanitize_hint("note: has a colon"); + assert_eq!(result, "note: has a colon"); + } + + #[test] + fn sanitize_hint_preserves_quotes_and_backslashes() { + let result = sanitize_hint("she said \"hello\""); + assert_eq!(result, "she said \"hello\""); + let result2 = sanitize_hint("C:\\Users\\admin"); + assert_eq!(result2, "C:\\Users\\admin"); + } + + #[test] + fn sanitize_hint_replaces_newlines() { + let result = sanitize_hint("line1\nline2"); + assert_eq!(result, "line1 line2"); + } + + #[test] + fn sanitize_hint_crlf() { + let result = sanitize_hint("line1\r\nline2"); + assert_eq!(result, "line1 line2"); + } + + #[test] + fn sanitize_hint_empty() { + let result = sanitize_hint(""); + assert_eq!(result, ""); + } + + #[test] + fn sanitize_hint_control_chars() { + let result = sanitize_hint("pre\x00mid\x1Fpost"); + assert_eq!(result, "pre mid post"); + } + + #[test] + fn fact_title_with_special_chars() { + let f = ConsolidatedFact::new( + "sid", + FactCategory::Lesson, + "Use const\nnot var".into(), + "Details".into(), + "mem_edit".into(), + vec![], + 0.7, + ); + let md = f.to_markdown(); + // Verify the title is encoded without YAML quoting (round-trip compatible) + assert!(md.contains("title: Use const not var")); + // Round-trip: extract title from frontmatter and confirm it matches sanitized input + let title_line = md + .lines() + .skip_while(|l| *l != "---") + .skip(1) // skip opening --- + .find(|l| l.starts_with("title: ")) + .expect("title field missing in frontmatter"); + let extracted_title = title_line + .strip_prefix("title: ") + .expect("title prefix missing"); + assert_eq!( + extracted_title, "Use const not var", + "round-trip: special chars survive write->read" + ); + // Verify no YAML double-quoting artifact + assert!( + !md.contains("title: \""), + "must not use YAML double-quoting" + ); + } + + #[test] + fn fact_path_with_backslashes() { + let f = ConsolidatedFact::new( + "sid", + FactCategory::WorkingContext, + "Work".into(), + "Details".into(), + "mem_write".into(), + vec!["C:\\Users\\admin\\file.txt".into()], + 0.5, + ); + let md = f.to_markdown(); + // Verify paths are kept verbatim (no YAML double-quoting) + assert!(md.contains("C:\\Users\\admin\\file.txt")); + // Round-trip: extract related_paths from frontmatter and confirm + let mut paths = Vec::new(); + let mut in_frontmatter = false; + for line in md.lines() { + if line == "---" { + if in_frontmatter { + break; + } + in_frontmatter = true; + continue; + } + if in_frontmatter && line.starts_with(" - ") { + paths.push(line.strip_prefix(" - ").unwrap().to_string()); + } + } + assert_eq!( + paths, + vec!["C:\\Users\\admin\\file.txt"], + "round-trip: Windows paths survive write->read without escaping artifacts" + ); + // Verify no YAML double-quoting artifact + assert!( + !md.contains("C:\\\"Users"), + "must not use YAML double-quoting on paths" + ); + } }