Skip to content

Commit faa3651

Browse files
committed
fix: harden input bounds, clamp confidence, deduplicate helpers
- Clamp confidence_bar to prevent negative float → usize wrap (security) - Add store-level limit bounds: search_hybrid/search_by_keywords cap at 1000 - Cap keywords array to 50 in search_by_keywords to prevent SQL explosion - Replace 4 inline embed_text format strings with Memory::embed_text() - Extract parse_keywords, topic_matches, keyword_matches helpers in MCP
1 parent 161900b commit faa3651

4 files changed

Lines changed: 47 additions & 29 deletions

File tree

crates/icm-cli/src/main.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -899,8 +899,7 @@ fn cmd_store(
899899

900900
// Auto-embed if embedder is available
901901
if let Some(emb) = embedder {
902-
let text = format!("{topic} {content}");
903-
match emb.embed(&text) {
902+
match emb.embed(&memory.embed_text()) {
904903
Ok(vec) => memory.embedding = Some(vec),
905904
Err(e) => eprintln!("warning: embedding failed: {e}"),
906905
}
@@ -1041,8 +1040,7 @@ fn cmd_update(
10411040

10421041
// Re-embed if embedder available
10431042
if let Some(emb) = embedder {
1044-
let text = format!("{} {}", memory.topic, content);
1045-
match emb.embed(&text) {
1043+
match emb.embed(&memory.embed_text()) {
10461044
Ok(vec) => memory.embedding = Some(vec),
10471045
Err(e) => eprintln!("warning: re-embedding failed: {e}"),
10481046
}

crates/icm-core/src/memoir.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,8 +113,9 @@ impl Concept {
113113
}
114114

115115
pub fn confidence_bar(&self) -> String {
116-
let filled = (self.confidence * 5.0).round() as usize;
117-
let empty = 5 - filled.min(5);
116+
let clamped = self.confidence.clamp(0.0, 1.0);
117+
let filled = (clamped * 5.0).round() as usize;
118+
let empty = 5 - filled;
118119
format!("{}{}", "#".repeat(filled), ".".repeat(empty))
119120
}
120121

crates/icm-mcp/src/tools.rs

Lines changed: 37 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,28 @@ use crate::protocol::ToolResult;
1212
/// Default threshold for auto-consolidation (can be overridden by config).
1313
const AUTO_CONSOLIDATE_THRESHOLD: usize = 10;
1414

15+
/// Parse a JSON keywords array from tool arguments.
16+
fn parse_keywords(args: &Value) -> Vec<String> {
17+
args.get("keywords")
18+
.and_then(|v| v.as_array())
19+
.map(|arr| {
20+
arr.iter()
21+
.filter_map(|v| v.as_str().map(String::from))
22+
.collect()
23+
})
24+
.unwrap_or_default()
25+
}
26+
27+
/// Check if a memory topic matches a filter (exact or prefix match).
28+
fn topic_matches(memory_topic: &str, filter: &str) -> bool {
29+
memory_topic == filter || memory_topic.starts_with(&format!("{filter}:"))
30+
}
31+
32+
/// Check if memory keywords contain a substring match.
33+
fn keyword_matches(keywords: &[String], filter: &str) -> bool {
34+
keywords.iter().any(|k| k.contains(filter))
35+
}
36+
1537
/// Try to auto-consolidate a topic if it exceeds the threshold.
1638
/// Returns a human-readable message if consolidation happened, or empty string.
1739
fn try_auto_consolidate(store: &SqliteStore, topic: &str, threshold: usize) -> String {
@@ -587,19 +609,17 @@ fn tool_store(
587609

588610
let mut memory = Memory::new(topic.into(), content.into(), importance);
589611

590-
if let Some(keywords) = args.get("keywords").and_then(|v| v.as_array()) {
591-
memory.keywords = keywords
592-
.iter()
593-
.filter_map(|v| v.as_str().map(String::from))
594-
.collect();
612+
let kw = parse_keywords(args);
613+
if !kw.is_empty() {
614+
memory.keywords = kw;
595615
}
596616

597617
if let Some(raw) = get_str(args, "raw_excerpt") {
598618
memory.raw_excerpt = Some(raw.into());
599619
}
600620

601621
// Auto-embed if embedder is available
602-
let embed_text = format!("{topic} {content}");
622+
let embed_text = memory.embed_text();
603623
let embed_vec = if let Some(emb) = embedder {
604624
match emb.embed(&embed_text) {
605625
Ok(vec) => Some(vec),
@@ -629,11 +649,9 @@ fn tool_store(
629649
if let Some(raw) = get_str(args, "raw_excerpt") {
630650
updated.raw_excerpt = Some(raw.into());
631651
}
632-
if let Some(keywords_arr) = args.get("keywords").and_then(|v| v.as_array()) {
633-
updated.keywords = keywords_arr
634-
.iter()
635-
.filter_map(|v| v.as_str().map(String::from))
636-
.collect();
652+
let kw = parse_keywords(args);
653+
if !kw.is_empty() {
654+
updated.keywords = kw;
637655
}
638656
updated.importance = importance;
639657
updated.embedding = Some(query_emb.clone());
@@ -744,11 +762,10 @@ fn tool_recall(
744762
if let Ok(results) = store.search_hybrid(query, &query_emb, limit) {
745763
let mut scored_results = results;
746764
if let Some(t) = topic {
747-
scored_results
748-
.retain(|(m, _)| m.topic == t || m.topic.starts_with(&format!("{t}:")));
765+
scored_results.retain(|(m, _)| topic_matches(&m.topic, t));
749766
}
750767
if let Some(kw) = keyword {
751-
scored_results.retain(|(m, _)| m.keywords.iter().any(|k| k.contains(kw)));
768+
scored_results.retain(|(m, _)| keyword_matches(&m.keywords, kw));
752769
}
753770

754771
// Batch update access counts
@@ -779,10 +796,10 @@ fn tool_recall(
779796
}
780797

781798
if let Some(t) = topic {
782-
results.retain(|m| m.topic == t || m.topic.starts_with(&format!("{t}:")));
799+
results.retain(|m| topic_matches(&m.topic, t));
783800
}
784801
if let Some(kw) = keyword {
785-
results.retain(|m| m.keywords.iter().any(|k| k.contains(kw)));
802+
results.retain(|m| keyword_matches(&m.keywords, kw));
786803
}
787804

788805
// Batch update access counts
@@ -918,17 +935,14 @@ fn tool_update(store: &SqliteStore, embedder: Option<&dyn Embedder>, args: &Valu
918935
}
919936
}
920937

921-
if let Some(keywords_arr) = args.get("keywords").and_then(|v| v.as_array()) {
922-
memory.keywords = keywords_arr
923-
.iter()
924-
.filter_map(|v| v.as_str().map(String::from))
925-
.collect();
938+
let kw = parse_keywords(args);
939+
if !kw.is_empty() {
940+
memory.keywords = kw;
926941
}
927942

928943
// Re-embed if embedder available
929944
if let Some(emb) = embedder {
930-
let text = format!("{} {}", memory.topic, content);
931-
if let Ok(vec) = emb.embed(&text) {
945+
if let Ok(vec) = emb.embed(&memory.embed_text()) {
932946
memory.embedding = Some(vec);
933947
}
934948
}

crates/icm-store/src/store.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -378,6 +378,10 @@ impl MemoryStore for SqliteStore {
378378
return Ok(Vec::new());
379379
}
380380

381+
// Cap keywords to avoid massive SQL generation
382+
let keywords = &keywords[..keywords.len().min(50)];
383+
let limit = limit.min(1000);
384+
381385
let where_parts: Vec<String> = (0..keywords.len())
382386
.map(|i| {
383387
let p = i + 1;
@@ -500,6 +504,7 @@ impl MemoryStore for SqliteStore {
500504
embedding: &[f32],
501505
limit: usize,
502506
) -> IcmResult<Vec<(Memory, f32)>> {
507+
let limit = limit.min(1000);
503508
let pool_size = limit * 4;
504509
let sanitized = sanitize_fts_query(query);
505510

0 commit comments

Comments
 (0)