fix: comprehensive audit fixes — security, performance, tech debt, tests (#51)

Security:
- Credentials file permissions set to 0o600 on Unix
- Input validation for MCP tool_store (topic max 255, content max 100k)
- FTS query hardening (length limit, token cap, quote stripping)
- Embedding dims validated to 64-4096 range
- Search result limits capped to 100

Performance:
- CLI recall uses batch_update_access instead of N individual calls
- Reduced cloning in search_hybrid (owned keys iteration)
- Reduced cloning in tool_store dedup (struct literal vs clone)

Tech debt + Logging:
- Replaced eprintln! with tracing::warn! (6 locations)
- Fixed unwrap() panic in extract_patterns
- Replaced .expect() with proper error handling in embed command
- Added tracing on rollback paths and datetime parse failures
- Added DEFAULT_EMBEDDING_DIMS constant (replaces hardcoded 384)
- Added DEDUP_SIMILARITY_THRESHOLD constant (replaces magic 0.85)

Redundancy + Dead code:
- Removed unused delete_cloud_memory and sync_memory_background
- Extracted topic_matches/keyword_matches to icm-core (shared CLI+MCP)
- Added MSG_NO_MEMORIES constant for consistent messaging

Tests (+29 new):
- Store: empty queries, nonexistent CRUD, batch access, auto-consolidate,
  decay, prune, topics, stats, topic prefix
- MCP: empty recall, input validation (topic/content length)
- Security: FTS sanitization, embedding dims bounds, search limits,
  credentials permissions

Author: pszymkowiak

Cargo.lock

@@ -77,6 +77,14 @@ pub fn save_credentials(creds: &Credentials) -> Result<()> {
     }
     let json = serde_json::to_string_pretty(creds)?;
     std::fs::write(&path, json)?;
+
+    // Restrict file permissions to owner-only on Unix (0o600)
+    #[cfg(unix)]
+    {
+        use std::os::unix::fs::PermissionsExt;
+        std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o600))?;
+    }
+
     Ok(())
 }
 
@@ -459,51 +467,6 @@ pub fn pull_memories(
     Ok(memories)
 }
 
-/// Delete a memory from RTK Cloud.
-/// DELETE {endpoint}/api/icm/memories/{id}
-pub fn delete_cloud_memory(creds: &Credentials, memory_id: &str) -> Result<()> {
-    let url = format!(
-        "{}/api/icm/memories/{}",
-        creds.endpoint.trim_end_matches('/'),
-        memory_id
-    );
-
-    let resp = ureq::delete(&url)
-        .set("Authorization", &format!("Bearer {}", creds.token))
-        .set("X-Org-Id", &creds.org_id)
-        .timeout(std::time::Duration::from_secs(5))
-        .call()
-        .context("Failed to delete cloud memory")?;
-
-    let status = resp.status();
-    if status != 200 && status != 204 {
-        let body = resp.into_string().unwrap_or_default();
-        anyhow::bail!("Cloud delete failed ({}): {}", status, body);
-    }
-
-    Ok(())
-}
-
-/// Fire-and-forget sync: push memory in a background thread.
-/// Used after store/update operations to sync without blocking.
-pub fn sync_memory_background(memory: Memory) {
-    let creds = match load_credentials() {
-        Some(c) => c,
-        None => return,
-    };
-
-    // Only sync project/org scoped memories
-    if memory.scope == Scope::User {
-        return;
-    }
-
-    std::thread::spawn(move || {
-        if let Err(e) = sync_memory(&creds, &memory) {
-            tracing::warn!("Cloud sync failed: {}", e);
-        }
-    });
-}
-
 /// Check if cloud sync is available (credentials exist and scope requires it).
 pub fn requires_cloud(scope: Scope) -> bool {
     scope != Scope::User
@@ -541,6 +504,28 @@ mod tests {
         assert_eq!(url_decode("no_encoding"), "no_encoding");
     }
 
+    #[cfg(unix)]
+    #[test]
+    fn test_credentials_file_permissions_0600() {
+        use std::os::unix::fs::PermissionsExt;
+
+        // Create a temp file, apply the same permission logic as save_credentials
+        let dir = std::env::temp_dir().join(format!("icm-test-{}", std::process::id()));
+        std::fs::create_dir_all(&dir).unwrap();
+        let path = dir.join("test-credentials.json");
+
+        std::fs::write(&path, "{}").unwrap();
+        std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o600)).unwrap();
+
+        let metadata = std::fs::metadata(&path).unwrap();
+        let mode = metadata.permissions().mode() & 0o777;
+        assert_eq!(mode, 0o600, "credentials file should be owner-only (0o600)");
+
+        // Cleanup
+        let _ = std::fs::remove_file(&path);
+        let _ = std::fs::remove_dir(&dir);
+    }
+
     #[test]
     fn test_requires_cloud() {
         assert!(!requires_cloud(Scope::User));

crates/icm-cli/src/cloud.rs

@@ -14,8 +14,8 @@ use clap::{Parser, Subcommand, ValueEnum};
 use serde_json::Value;
 
 use icm_core::{
-    Concept, ConceptLink, Feedback, FeedbackStore, Importance, Label, Memoir, MemoirStore, Memory,
-    MemoryStore, Relation,
+    keyword_matches, topic_matches, Concept, ConceptLink, Feedback, FeedbackStore, Importance,
+    Label, Memoir, MemoirStore, Memory, MemoryStore, Relation, MSG_NO_MEMORIES,
 };
 use icm_store::SqliteStore;
 
@@ -678,7 +678,7 @@ fn main() -> Result<()> {
             use icm_core::Embedder;
             e.dimensions()
         })
-        .unwrap_or(384);
+        .unwrap_or(icm_core::DEFAULT_EMBEDDING_DIMS);
     let db_path = cli.db.clone().unwrap_or_else(default_db_path);
     let store = open_store(cli.db, embedding_dims)?;
 
@@ -774,7 +774,10 @@ fn main() -> Result<()> {
         } => {
             #[cfg(feature = "embeddings")]
             {
-                let emb = embedder.as_ref().expect("embeddings feature enabled");
+                let emb = match embedder.as_ref() {
+                    Some(e) => e,
+                    None => bail!("embeddings not available — check your configuration"),
+                };
                 cmd_embed(&store, emb, topic.as_deref(), force, batch_size)
             }
             #[cfg(not(feature = "embeddings"))]
@@ -919,27 +922,30 @@ fn cmd_recall(
     keyword: Option<&str>,
 ) -> Result<()> {
     // Auto-decay if >24h since last decay
-    let _ = store.maybe_auto_decay();
+    if let Err(e) = store.maybe_auto_decay() {
+        tracing::warn!(error = %e, "auto-decay failed during recall");
+    }
 
     // Try hybrid search if embedder is available
     if let Some(emb) = embedder {
         if let Ok(query_emb) = emb.embed(query) {
             if let Ok(results) = store.search_hybrid(query, &query_emb, limit) {
                 let mut scored = results;
                 if let Some(t) = topic {
-                    scored.retain(|(m, _)| m.topic == t);
+                    scored.retain(|(m, _)| topic_matches(&m.topic, t));
                 }
                 if let Some(kw) = keyword {
-                    scored.retain(|(m, _)| m.keywords.iter().any(|k| k.contains(kw)));
+                    scored.retain(|(m, _)| keyword_matches(&m.keywords, kw));
                 }
 
                 if scored.is_empty() {
-                    println!("No memories found.");
+                    println!("{MSG_NO_MEMORIES}");
                     return Ok(());
                 }
 
+                let ids: Vec<&str> = scored.iter().map(|(m, _)| m.id.as_str()).collect();
+                let _ = store.batch_update_access(&ids);
                 for (mem, score) in &scored {
-                    let _ = store.update_access(&mem.id);
                     print_memory_detail(mem, Some(*score));
                 }
                 return Ok(());
@@ -956,19 +962,20 @@ fn cmd_recall(
     }
 
     if let Some(t) = topic {
-        results.retain(|m| m.topic == t);
+        results.retain(|m| topic_matches(&m.topic, t));
     }
     if let Some(kw) = keyword {
-        results.retain(|m| m.keywords.iter().any(|k| k.contains(kw)));
+        results.retain(|m| keyword_matches(&m.keywords, kw));
     }
 
     if results.is_empty() {
-        println!("No memories found.");
+        println!("{MSG_NO_MEMORIES}");
         return Ok(());
     }
 
+    let ids: Vec<&str> = results.iter().map(|m| m.id.as_str()).collect();
+    let _ = store.batch_update_access(&ids);
     for mem in &results {
-        let _ = store.update_access(&mem.id);
         print_memory_detail(mem, None);
     }
 
@@ -992,7 +999,7 @@ fn cmd_list(store: &SqliteStore, topic: Option<&str>, all: bool, sort: SortField
     }
 
     if memories.is_empty() {
-        println!("No memories found.");
+        println!("{MSG_NO_MEMORIES}");
         return Ok(());
     }
 

crates/icm-cli/src/main.rs

@@ -9,6 +9,9 @@ pub mod memoir_store;
 pub mod memory;
 pub mod store;
 
+/// Default embedding vector dimensions (used when no embedder is configured).
+pub const DEFAULT_EMBEDDING_DIMS: usize = 384;
+
 pub use embedder::Embedder;
 pub use error::{IcmError, IcmResult};
 #[cfg(feature = "embeddings")]
@@ -21,3 +24,16 @@ pub use memory::{
     Importance, Memory, MemorySource, PatternCluster, Scope, StoreStats, TopicHealth,
 };
 pub use store::MemoryStore;
+
+/// Common message for empty search results.
+pub const MSG_NO_MEMORIES: &str = "No memories found.";
+
+/// Check if a memory's topic matches a filter (supports prefix with ':').
+pub fn topic_matches(memory_topic: &str, filter: &str) -> bool {
+    memory_topic == filter || memory_topic.starts_with(&format!("{filter}:"))
+}
+
+/// Check if any keyword contains the filter string.
+pub fn keyword_matches(keywords: &[String], filter: &str) -> bool {
+    keywords.iter().any(|k| k.contains(filter))
+}