Fix all 42 red team findings — full security hardening pass

CRITICAL fixes:
- Request body size limit (50MB) on HTTP server
- Sync re-validates all memories from remote DBs before insert
- Panic-safe ID display (id_short helper, UTF-8 boundary safe)
- MCP validates arguments field exists and is object
- MCP validates JSON-RPC version == "2.0"
- MCP validates tool name is present and non-empty
- Touch errors logged in recall instead of silently swallowed
- Hook script hardened (set -euo pipefail, input validation)

HIGH fixes:
- FTS sanitizer now strips (), :, -, AND/OR/NOT/NEAR operators
- ID validation added to promote HTTP endpoint
- Consolidate verifies all IDs exist before proceeding
- Consolidate rejects duplicate IDs
- Export handler propagates DB errors instead of unwrap_or_default
- MCP consolidate validates all array elements are strings
- MCP checkpoints WAL on shutdown

MEDIUM fixes:
- CORS layer (permissive for localhost service)
- ROLLBACK failures logged in touch() and consolidate()
- Migration uses PRAGMA table_info instead of error string matching
- access_count capped at 1,000,000
- UTF-8 safe tag error message truncation
- Release profile opt-level raised to 3

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: alphaonedev

Cargo.toml

@@ -36,6 +36,6 @@ name = "recall"
 harness = false
 
 [profile.release]
-opt-level = 2
+opt-level = 3
 strip = true
 lto = "thin"

hooks/session-start.sh

@@ -1,16 +1,32 @@
 #!/bin/bash
-# Claude Code hook: auto-recall relevant memories on session start
-# Add to .claude/settings.json under hooks.PreToolUse or run manually
+# AI Memory hook: auto-recall relevant memories on session start
+# Works with any MCP-compatible AI client
+set -euo pipefail
 
 DB="${AI_MEMORY_DB:-ai-memory.db}"
 BINARY="${AI_MEMORY_BIN:-ai-memory}"
 
+# Validate binary exists
+if ! command -v "$BINARY" &>/dev/null; then
+    exit 0
+fi
+
+# Validate DB path doesn't contain dangerous characters
+if [[ "$DB" == *".."* ]] || [[ "$DB" == /* && "$DB" != "$HOME"* && "$DB" != /tmp/* ]]; then
+    echo "warning: suspicious AI_MEMORY_DB path, skipping" >&2
+    exit 0
+fi
+
 # Auto-detect namespace from git
-NS=$($BINARY --db "$DB" --json store --tier short -T "_ns_probe" --content "probe" --source hook 2>/dev/null | grep -o '"namespace":"[^"]*"' | head -1 | cut -d'"' -f4)
+NS=$("$BINARY" --db "$DB" --json store --tier short -T "_ns_probe" --content "probe" --source hook 2>/dev/null | grep -o '"namespace":"[^"]*"' | head -1 | cut -d'"' -f4) || true
+# Validate namespace contains only safe characters
+if [[ -n "$NS" && ! "$NS" =~ ^[a-zA-Z0-9._-]+$ ]]; then
+    NS="global"
+fi
 [ -z "$NS" ] && NS="global"
 
 # Clean up probe
-$BINARY --db "$DB" forget --pattern "_ns_probe" 2>/dev/null
+"$BINARY" --db "$DB" forget --pattern "_ns_probe" 2>/dev/null || true
 
 # Recall recent context for this namespace
-$BINARY --db "$DB" recall "session context project overview" --namespace "$NS" --limit 5 --json 2>/dev/null
+"$BINARY" --db "$DB" recall "session context project overview" --namespace "$NS" --limit 5 --json 2>/dev/null || true

src/db.rs

@@ -93,18 +93,31 @@ fn migrate(conn: &Connection) -> Result<()> {
         )
         .unwrap_or(0);
     if version < 2 {
-        // Add confidence and source columns if missing (v1 -> v2)
-        // Ignore "duplicate column" errors but propagate real failures
-        for col_sql in &[
-            "ALTER TABLE memories ADD COLUMN confidence REAL NOT NULL DEFAULT 1.0",
-            "ALTER TABLE memories ADD COLUMN source TEXT NOT NULL DEFAULT 'api'",
-        ] {
-            match conn.execute(col_sql, []) {
-                Ok(_) => {}
-                Err(e) if e.to_string().contains("duplicate column") => {}
-                Err(e) => return Err(e.into()),
+        // Check which columns exist using PRAGMA
+        let mut has_confidence = false;
+        let mut has_source = false;
+        let mut stmt = conn.prepare("PRAGMA table_info(memories)")?;
+        let cols = stmt.query_map([], |row| row.get::<_, String>(1))?;
+        for col in cols {
+            match col?.as_str() {
+                "confidence" => has_confidence = true,
+                "source" => has_source = true,
+                _ => {}
             }
         }
+        drop(stmt);
+        if !has_confidence {
+            conn.execute(
+                "ALTER TABLE memories ADD COLUMN confidence REAL NOT NULL DEFAULT 1.0",
+                [],
+            )?;
+        }
+        if !has_source {
+            conn.execute(
+                "ALTER TABLE memories ADD COLUMN source TEXT NOT NULL DEFAULT 'api'",
+                [],
+            )?;
+        }
     }
     if version < CURRENT_SCHEMA_VERSION {
         conn.execute("DELETE FROM schema_version", [])?;
@@ -195,7 +208,7 @@ pub fn touch(conn: &Connection, id: &str) -> Result<()> {
     let result = (|| -> Result<()> {
         conn.execute(
             "UPDATE memories SET
-                access_count = access_count + 1,
+                access_count = MIN(access_count + 1, 1000000),
                 last_accessed_at = ?1,
                 expires_at = CASE
                     WHEN tier = 'long' THEN expires_at
@@ -224,7 +237,12 @@ pub fn touch(conn: &Connection, id: &str) -> Result<()> {
 
     match result {
         Ok(()) => { conn.execute_batch("COMMIT")?; Ok(()) }
-        Err(e) => { let _ = conn.execute_batch("ROLLBACK"); Err(e) }
+        Err(e) => {
+            if let Err(rb) = conn.execute_batch("ROLLBACK") {
+                tracing::error!("ROLLBACK failed in touch: {}", rb);
+            }
+            Err(e)
+        }
     }
 }
 
@@ -453,7 +471,9 @@ pub fn recall(
 
     // Touch all recalled memories (bumps access, extends TTL, auto-promotes)
     for mem in &results {
-        let _ = touch(conn, &mem.id);
+        if let Err(e) = touch(conn, &mem.id) {
+            tracing::warn!("touch failed for memory {}: {}", &mem.id, e);
+        }
     }
     Ok(results)
 }
@@ -537,6 +557,13 @@ pub fn consolidate(
     conn.execute_batch("BEGIN IMMEDIATE")?;
 
     let result = (|| -> Result<String> {
+        // Verify all IDs exist before proceeding
+        for id in ids {
+            if get(conn, id)?.is_none() {
+                anyhow::bail!("memory not found: {}", id);
+            }
+        }
+
         // Collect max priority and all tags from source memories
         let mut max_priority = 5i32;
         let mut all_tags: Vec<String> = Vec::new();
@@ -571,7 +598,12 @@ pub fn consolidate(
 
     match result {
         Ok(id) => { conn.execute_batch("COMMIT")?; Ok(id) }
-        Err(e) => { let _ = conn.execute_batch("ROLLBACK"); Err(e) }
+        Err(e) => {
+            if let Err(rb) = conn.execute_batch("ROLLBACK") {
+                tracing::error!("ROLLBACK failed in consolidate: {}", rb);
+            }
+            Err(e)
+        }
     }
 }
 
@@ -580,9 +612,22 @@ fn sanitize_fts_query(input: &str, use_or: bool) -> String {
     let tokens: Vec<String> = input
         .split_whitespace()
         .filter(|t| !t.is_empty())
+        .filter(|t| {
+            // Filter out FTS5 boolean operators as standalone tokens
+            let upper = t.to_uppercase();
+            upper != "AND" && upper != "OR" && upper != "NOT" && upper != "NEAR"
+        })
         .map(|token| {
-            // Strip all FTS5 special characters to prevent injection
-            let clean: String = token.chars().filter(|c| *c != '"' && *c != '*' && *c != '^' && *c != '{' && *c != '}').collect();
+            // Strip ALL FTS5 special characters to prevent injection
+            let clean: String = token
+                .chars()
+                .filter(|c| {
+                    *c != '"' && *c != '*' && *c != '^'
+                        && *c != '{' && *c != '}'
+                        && *c != '(' && *c != ')'
+                        && *c != ':' && *c != '-'
+                })
+                .collect();
             if clean.is_empty() {
                 return String::new();
             }
@@ -591,7 +636,7 @@ fn sanitize_fts_query(input: &str, use_or: bool) -> String {
         .filter(|t| !t.is_empty())
         .collect();
     if tokens.is_empty() {
-        return "\"\"".to_string();
+        return "\"_empty_\"".to_string();
     }
     tokens.join(joiner)
 }

src/handlers.rs

@@ -200,6 +200,13 @@ pub async fn promote_memory(
     State(state): State<Db>,
     Path(id): Path<String>,
 ) -> impl IntoResponse {
+    if let Err(e) = validate::validate_id(&id) {
+        return (
+            StatusCode::BAD_REQUEST,
+            Json(json!({"error": e.to_string()})),
+        )
+            .into_response();
+    }
     let lock = state.lock().await;
     match db::update(
         &lock.0, &id, None, None, Some(&Tier::Long), None, None, None, None, None,
@@ -212,11 +219,14 @@ pub async fn promote_memory(
             Json(json!({"promoted": true, "id": id, "tier": "long"})).into_response()
         }
         Ok(false) => (StatusCode::NOT_FOUND, Json(json!({"error": "not found"}))).into_response(),
-        Err(_e) => (
-            StatusCode::INTERNAL_SERVER_ERROR,
-            Json(json!({"error": "database error"})),
-        )
-            .into_response(),
+        Err(e) => {
+            tracing::error!("handler error: {e}");
+            (
+                StatusCode::INTERNAL_SERVER_ERROR,
+                Json(json!({"error": "internal server error"})),
+            )
+                .into_response()
+        }
     }
 }
 
@@ -465,9 +475,20 @@ pub async fn run_gc(State(state): State<Db>) -> impl IntoResponse {
 
 pub async fn export_memories(State(state): State<Db>) -> impl IntoResponse {
     let lock = state.lock().await;
-    let memories = db::export_all(&lock.0).unwrap_or_default();
-    let links = db::export_links(&lock.0).unwrap_or_default();
-    Json(json!({"memories": memories, "links": links, "count": memories.len(), "exported_at": Utc::now().to_rfc3339()})).into_response()
+    match (db::export_all(&lock.0), db::export_links(&lock.0)) {
+        (Ok(memories), Ok(links)) => {
+            let count = memories.len();
+            Json(json!({"memories": memories, "links": links, "count": count, "exported_at": Utc::now().to_rfc3339()})).into_response()
+        }
+        (Err(e), _) | (_, Err(e)) => {
+            tracing::error!("export error: {e}");
+            (
+                StatusCode::INTERNAL_SERVER_ERROR,
+                Json(json!({"error": "internal server error"})),
+            )
+                .into_response()
+        }
+    }
 }
 
 pub async fn import_memories(

src/main.rs

@@ -11,6 +11,7 @@ mod validate;
 
 use anyhow::Result;
 use axum::{
+    extract::DefaultBodyLimit,
     routing::{delete, get, post, put},
     Router,
 };
@@ -20,6 +21,7 @@ use clap_complete::{generate, Shell};
 use std::path::PathBuf;
 use std::sync::Arc;
 use tokio::sync::Mutex;
+use tower_http::cors::CorsLayer;
 use tower_http::trace::TraceLayer;
 use tracing_subscriber::EnvFilter;
 
@@ -29,6 +31,16 @@ const DEFAULT_DB: &str = "ai-memory.db";
 const DEFAULT_PORT: u16 = 9077;
 const GC_INTERVAL_SECS: u64 = 1800;
 
+fn id_short(id: &str) -> &str {
+    let end = id.len().min(8);
+    // Find a valid UTF-8 boundary
+    let mut end = end;
+    while end > 0 && !id.is_char_boundary(end) {
+        end -= 1;
+    }
+    &id[..end]
+}
+
 #[derive(Parser)]
 #[command(
     name = "ai-memory",
@@ -438,6 +450,8 @@ async fn serve(db_path: PathBuf, args: ServeArgs) -> Result<()> {
         .route("/api/v1/export", get(handlers::export_memories))
         .route("/api/v1/import", post(handlers::import_memories))
         .layer(TraceLayer::new_for_http())
+        .layer(DefaultBodyLimit::max(50 * 1024 * 1024)) // 50MB max request body
+        .layer(CorsLayer::permissive())
         .with_state(state);
 
     let addr = format!("{}:{}", args.host, args.port);
@@ -626,7 +640,7 @@ fn cmd_recall(db_path: PathBuf, args: RecallArgs, json_out: bool) -> Result<()>
         };
         println!(
             "[{}] {} {} (ns={}, {}x, {}{})",
-            color::tier_color(mem.tier.as_str(), &format!("{}/{}", mem.tier, &mem.id[..8])),
+            color::tier_color(mem.tier.as_str(), &format!("{}/{}", mem.tier, id_short(&mem.id))),
             color::bold(&mem.title),
             color::priority_bar(mem.priority),
             color::cyan(&mem.namespace),
@@ -674,7 +688,7 @@ fn cmd_search(db_path: PathBuf, args: SearchArgs, json_out: bool) -> Result<()>
         println!(
             "[{}/{}] {} (p={}, ns={}, {})",
             mem.tier,
-            &mem.id[..8],
+            id_short(&mem.id),
             mem.title,
             mem.priority,
             mem.namespace,
@@ -746,7 +760,7 @@ fn cmd_list(db_path: PathBuf, args: ListArgs, json_out: bool) -> Result<()> {
         println!(
             "[{}/{}] {} (p={}, ns={}, {})",
             mem.tier,
-            &mem.id[..8],
+            id_short(&mem.id),
             mem.title,
             mem.priority,
             mem.namespace,
@@ -1168,6 +1182,10 @@ fn cmd_sync(db_path: PathBuf, args: SyncArgs, json_out: bool) -> Result<()> {
             let links = db::export_links(&remote_conn)?;
             let mut n = 0;
             for mem in &mems {
+                if let Err(e) = validate::validate_memory(mem) {
+                    tracing::warn!("sync: skipping invalid memory {}: {}", mem.id, e);
+                    continue;
+                }
                 if db::insert(&local_conn, mem).is_ok() {
                     n += 1;
                 }
@@ -1194,6 +1212,10 @@ fn cmd_sync(db_path: PathBuf, args: SyncArgs, json_out: bool) -> Result<()> {
             let links = db::export_links(&local_conn)?;
             let mut n = 0;
             for mem in &mems {
+                if let Err(e) = validate::validate_memory(mem) {
+                    tracing::warn!("sync: skipping invalid memory {}: {}", mem.id, e);
+                    continue;
+                }
                 if db::insert(&remote_conn, mem).is_ok() {
                     n += 1;
                 }
@@ -1223,6 +1245,9 @@ fn cmd_sync(db_path: PathBuf, args: SyncArgs, json_out: bool) -> Result<()> {
             let (mut pulled, mut pushed) = (0, 0);
             // Use timestamp-aware insert so newer version wins on conflict
             for mem in &r_mems {
+                if validate::validate_memory(mem).is_err() {
+                    continue;
+                }
                 if db::insert_if_newer(&local_conn, mem).is_ok() {
                     pulled += 1;
                 }
@@ -1236,6 +1261,9 @@ fn cmd_sync(db_path: PathBuf, args: SyncArgs, json_out: bool) -> Result<()> {
                 );
             }
             for mem in &l_mems {
+                if validate::validate_memory(mem).is_err() {
+                    continue;
+                }
                 if db::insert_if_newer(&remote_conn, mem).is_ok() {
                     pushed += 1;
                 }