From a3b2fb814c968318147bba755fbf9ec03b450ce5 Mon Sep 17 00:00:00 2001 From: YellowSnnowmann <167776381+YellowSnnowmann@users.noreply.github.com> Date: Tue, 5 May 2026 23:38:25 +0530 Subject: [PATCH] feat(memory-security): prevent secret leakage into agent memory with redaction, validation, and diagnostics (#1224) --- src/openhuman/memory/mod.rs | 1 + src/openhuman/memory/safety/mod.rs | 458 ++++++++++++++++++ src/openhuman/memory/store/README.md | 1 + src/openhuman/memory/store/unified/README.md | 1 + .../memory/store/unified/documents.rs | 47 ++ .../memory/store/unified/documents_tests.rs | 185 +++++++ src/openhuman/memory/store/unified/fts5.rs | 112 ++++- src/openhuman/memory/store/unified/kv.rs | 54 ++- src/openhuman/tools/impl/memory/store.rs | 31 ++ 9 files changed, 883 insertions(+), 7 deletions(-) create mode 100644 src/openhuman/memory/safety/mod.rs diff --git a/src/openhuman/memory/mod.rs b/src/openhuman/memory/mod.rs index 872c78d4f..efaabaa3d 100644 --- a/src/openhuman/memory/mod.rs +++ b/src/openhuman/memory/mod.rs @@ -11,6 +11,7 @@ pub mod global; pub mod ingestion; pub mod ops; pub mod rpc_models; +pub mod safety; pub mod schemas; pub mod slack_ingestion; pub mod store; diff --git a/src/openhuman/memory/safety/mod.rs b/src/openhuman/memory/safety/mod.rs new file mode 100644 index 000000000..8620cba56 --- /dev/null +++ b/src/openhuman/memory/safety/mod.rs @@ -0,0 +1,458 @@ +//! Secret-detection and redaction helpers for memory writes. +//! +//! This module is intentionally conservative: it prefers false positives over +//! leaking credentials into long-lived memory stores. + +use once_cell::sync::Lazy; +use regex::Regex; +use serde_json::Value; + +use crate::openhuman::memory::store::types::NamespaceDocumentInput; + +const REDACTED_SECRET: &str = "[REDACTED_SECRET]"; +const REDACTED_PRIVATE_KEY: &str = "[REDACTED_PRIVATE_KEY]"; +const MAX_JSON_SANITIZE_DEPTH: usize = 128; + +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +pub struct SanitizationReport { + pub text_redactions: usize, + pub key_redactions: usize, + pub blocked_secret_hits: usize, + pub depth_redactions: usize, +} + +impl SanitizationReport { + pub fn changed(&self) -> bool { + self.text_redactions > 0 + || self.key_redactions > 0 + || self.blocked_secret_hits > 0 + || self.depth_redactions > 0 + } + + pub fn merge(self, rhs: Self) -> Self { + Self { + text_redactions: self.text_redactions + rhs.text_redactions, + key_redactions: self.key_redactions + rhs.key_redactions, + blocked_secret_hits: self.blocked_secret_hits + rhs.blocked_secret_hits, + depth_redactions: self.depth_redactions + rhs.depth_redactions, + } + } +} + +#[derive(Debug, Clone)] +pub struct Sanitized { + pub value: T, + pub report: SanitizationReport, +} + +static BLOCK_PATTERNS: Lazy> = Lazy::new(|| { + vec![ + // Generic PEM private key blocks, including multiline bodies. + Regex::new( + r"(?is)-----BEGIN(?: [A-Z]+)? PRIVATE KEY-----.*?-----END(?: [A-Z]+)? PRIVATE KEY-----", + ) + .expect("valid private key block"), + // SSH private key blocks. + Regex::new(r"(?is)-----BEGIN OPENSSH PRIVATE KEY-----.*?-----END OPENSSH PRIVATE KEY-----") + .expect("valid openssh private key block"), + // PGP private key blocks. + Regex::new( + r"(?is)-----BEGIN PGP PRIVATE KEY BLOCK-----.*?-----END PGP PRIVATE KEY BLOCK-----", + ) + .expect("valid pgp private key block"), + ] +}); + +static REDACTION_PATTERNS: Lazy> = Lazy::new(|| { + vec![ + ( + Regex::new(r"(?i)(bearer\s+)[A-Za-z0-9._~+/=-]{8,}") + .expect("valid bearer redaction"), + "${1}[REDACTED]", + ), + ( + Regex::new(r#"(?i)(api[_-]?key\s*[=:\s]\s*["']?)[^\s"']+"#) + .expect("valid api key redaction"), + "${1}[REDACTED]", + ), + ( + Regex::new( + r#"(?i)\b(token|access[_-]?token|refresh[_-]?token|client[_-]?secret|password|secret)\b\s*[=:\s]\s*["']?[^\s"'&]+"#, + ) + .expect("valid token redaction"), + "[REDACTED]", + ), + ( + Regex::new(r"\bsk-[A-Za-z0-9]{20,}\b").expect("valid openai key redaction"), + "[REDACTED]", + ), + ( + Regex::new(r"\bgh[pousr]_[A-Za-z0-9_]{20,}\b").expect("valid github token redaction"), + "[REDACTED]", + ), + ( + Regex::new(r"\bAKIA[0-9A-Z]{16}\b").expect("valid aws key redaction"), + "[REDACTED]", + ), + ( + Regex::new(r"\bASIA[0-9A-Z]{16}\b").expect("valid aws sts key redaction"), + "[REDACTED]", + ), + ( + Regex::new(r#"\b(?:aws_)?secret(?:_access)?_key\b\s*[=:\s]\s*["']?[A-Za-z0-9/+=]{16,}"#) + .expect("valid aws secret key redaction"), + "[REDACTED]", + ), + ( + Regex::new(r"\beyJ[A-Za-z0-9_-]{8,}\.[A-Za-z0-9._-]{8,}\.[A-Za-z0-9._-]{8,}\b") + .expect("valid jwt redaction"), + "[REDACTED]", + ), + ( + // Common OAuth token artifacts in URLs and payloads. + Regex::new( + r#"(?i)\b(access_token|refresh_token|id_token|authorization_code|code_verifier|code_challenge)\b\s*[=:\s]\s*["']?[^\s"'&]+"#, + ) + .expect("valid oauth token redaction"), + "[REDACTED]", + ), + ( + // Google API key pattern. + Regex::new(r"\bAIza[0-9A-Za-z\-_]{35}\b").expect("valid google api key redaction"), + "[REDACTED]", + ), + ( + // Anthropic key pattern. + Regex::new(r"\bsk-ant-[A-Za-z0-9\-_]{16,}\b").expect("valid anthropic key redaction"), + "[REDACTED]", + ), + ( + // OpenAI project/org scoped keys and legacy variants. + Regex::new(r"\bsk-(?:proj|org)-[A-Za-z0-9\-_]{12,}\b") + .expect("valid openai scoped key redaction"), + "[REDACTED]", + ), + ( + // Stripe secret/restricted keys. + Regex::new(r"\b(?:sk|rk)_(?:live|test)_[A-Za-z0-9]{16,}\b") + .expect("valid stripe key redaction"), + "[REDACTED]", + ), + ( + // Slack tokens (bot/user/app/config). + Regex::new(r"\bxox(?:a|b|p|s|r)-[A-Za-z0-9-]{10,}\b") + .expect("valid slack token redaction"), + "[REDACTED]", + ), + ( + // GitHub fine-grained/pat/user tokens beyond gh[pousr]_. + Regex::new(r"\bgithub_pat_[A-Za-z0-9_]{20,}\b") + .expect("valid github pat redaction"), + "[REDACTED]", + ), + ( + // GitLab personal access token. + Regex::new(r"\bglpat-[A-Za-z0-9\-_]{16,}\b") + .expect("valid gitlab pat redaction"), + "[REDACTED]", + ), + ( + // NPM auth token. + Regex::new(r"\bnpm_[A-Za-z0-9]{20,}\b").expect("valid npm token redaction"), + "[REDACTED]", + ), + ( + // SendGrid API key. + Regex::new(r"\bSG\.[A-Za-z0-9_\-]{16,}\.[A-Za-z0-9_\-]{16,}\b") + .expect("valid sendgrid key redaction"), + "[REDACTED]", + ), + ( + // Twilio API key SID. + Regex::new(r"\bSK[a-fA-F0-9]{32}\b").expect("valid twilio sid redaction"), + "[REDACTED]", + ), + ( + // Azure Storage account key in key=value style payloads. + Regex::new(r"(?i)\bAccountKey\b\s*=\s*[A-Za-z0-9+/=]{20,}") + .expect("valid azure account key redaction"), + "[REDACTED]", + ), + ( + // Generic Authorization header values beyond Bearer. + Regex::new(r"(?i)(authorization\s*[:=]\s*)(?:basic|bearer|token)\s+[A-Za-z0-9._~+/=-]{8,}") + .expect("valid authorization header redaction"), + "${1}[REDACTED]", + ), + ] +}); + +pub fn has_likely_secret(value: &str) -> bool { + if BLOCK_PATTERNS.iter().any(|pattern| pattern.is_match(value)) { + return true; + } + REDACTION_PATTERNS + .iter() + .any(|(pattern, _)| pattern.is_match(value)) +} + +pub fn sanitize_text(value: &str) -> Sanitized { + let mut out = value.to_string(); + let mut report = SanitizationReport::default(); + + for pattern in BLOCK_PATTERNS.iter() { + let hits = pattern.find_iter(&out).count(); + if hits > 0 { + report.blocked_secret_hits += hits; + out = pattern.replace_all(&out, REDACTED_PRIVATE_KEY).into_owned(); + } + } + + for (pattern, replacement) in REDACTION_PATTERNS.iter() { + let hits = pattern.find_iter(&out).count(); + if hits > 0 { + report.text_redactions += hits; + out = pattern.replace_all(&out, *replacement).into_owned(); + } + } + + Sanitized { value: out, report } +} + +pub fn sanitize_json(value: &Value) -> Sanitized { + sanitize_json_inner(value, 0) +} + +pub fn sanitize_document_input(input: NamespaceDocumentInput) -> Sanitized { + let mut report = SanitizationReport::default(); + + let title = sanitize_text(&input.title); + report = report.merge(title.report); + let content = sanitize_text(&input.content); + report = report.merge(content.report); + + let mut tags = Vec::with_capacity(input.tags.len()); + for tag in input.tags { + let sanitized = sanitize_text(&tag); + report = report.merge(sanitized.report); + tags.push(sanitized.value); + } + + let metadata = sanitize_json(&input.metadata); + report = report.merge(metadata.report); + + Sanitized { + value: NamespaceDocumentInput { + namespace: input.namespace, + key: input.key, + title: title.value, + content: content.value, + source_type: input.source_type, + priority: input.priority, + tags, + metadata: metadata.value, + category: input.category, + session_id: input.session_id, + document_id: input.document_id, + }, + report, + } +} + +fn sanitize_json_inner(value: &Value, depth: usize) -> Sanitized { + if depth >= MAX_JSON_SANITIZE_DEPTH { + return Sanitized { + value: Value::String(REDACTED_SECRET.to_string()), + report: SanitizationReport { + depth_redactions: 1, + ..SanitizationReport::default() + }, + }; + } + + match value { + Value::Object(map) => { + let mut out = serde_json::Map::new(); + let mut report = SanitizationReport::default(); + for (key, value) in map { + if is_sensitive_key(key) { + report.key_redactions += 1; + out.insert(key.clone(), Value::String(REDACTED_SECRET.to_string())); + continue; + } + let sanitized = sanitize_json_inner(value, depth + 1); + report = report.merge(sanitized.report); + out.insert(key.clone(), sanitized.value); + } + Sanitized { + value: Value::Object(out), + report, + } + } + Value::Array(items) => { + let mut out = Vec::with_capacity(items.len()); + let mut report = SanitizationReport::default(); + for item in items { + let sanitized = sanitize_json_inner(item, depth + 1); + report = report.merge(sanitized.report); + out.push(sanitized.value); + } + Sanitized { + value: Value::Array(out), + report, + } + } + Value::String(value) => { + let sanitized = sanitize_text(value); + Sanitized { + value: Value::String(sanitized.value), + report: sanitized.report, + } + } + _ => Sanitized { + value: value.clone(), + report: SanitizationReport::default(), + }, + } +} + +fn is_sensitive_key(key: &str) -> bool { + let normalized: String = key + .chars() + .filter(|c| c.is_ascii_alphanumeric()) + .map(|c| c.to_ascii_lowercase()) + .collect(); + + matches!( + normalized.as_str(), + "apikey" + | "token" + | "accesstoken" + | "refreshtoken" + | "authorization" + | "password" + | "secret" + | "clientsecret" + ) || normalized.ends_with("token") + || normalized.ends_with("apikey") + || normalized.ends_with("clientsecret") + || normalized.contains("password") + || normalized.contains("secret") + || normalized.ends_with("key") +} + +#[cfg(test)] +mod tests { + use super::*; + use serde_json::json; + + #[test] + fn sanitize_text_redacts_bearer_and_openai_key() { + let input = "Authorization: Bearer abcdefghijklmnop and sk-1234567890123456789012345"; + let sanitized = sanitize_text(input); + assert!(sanitized.value.contains("Bearer [REDACTED]")); + assert!(!sanitized.value.contains("sk-1234567890123456789012345")); + assert!(sanitized.report.text_redactions >= 2); + } + + #[test] + fn sanitize_text_blocks_private_key_blocks() { + let input = "-----BEGIN PRIVATE KEY-----\nabc\n-----END PRIVATE KEY-----"; + let sanitized = sanitize_text(input); + assert!(sanitized.value.contains(REDACTED_PRIVATE_KEY)); + assert!(sanitized.report.blocked_secret_hits >= 1); + } + + #[test] + fn sanitize_json_redacts_sensitive_keys_and_nested_strings() { + let input = json!({ + "token": "abc123", + "nested": { + "notes": "Bearer supersecretvalue", + "ok": "hello" + }, + "arr": ["sk-1234567890123456789012345", "safe"] + }); + + let sanitized = sanitize_json(&input); + assert_eq!(sanitized.value["token"], json!(REDACTED_SECRET)); + assert_eq!(sanitized.value["nested"]["ok"], json!("hello")); + assert!(sanitized.value["nested"]["notes"] + .as_str() + .unwrap_or_default() + .contains("[REDACTED]")); + assert!(sanitized.report.key_redactions >= 1); + assert!(sanitized.report.text_redactions >= 2); + } + + #[test] + fn sanitize_json_redacts_common_sensitive_key_variants() { + let input = json!({ + "db_password": "p@ss", + "secret_key": "abc123", + "api_secret": "def456", + "monkey": "banana" + }); + + let sanitized = sanitize_json(&input); + assert_eq!(sanitized.value["db_password"], json!(REDACTED_SECRET)); + assert_eq!(sanitized.value["secret_key"], json!(REDACTED_SECRET)); + assert_eq!(sanitized.value["api_secret"], json!(REDACTED_SECRET)); + assert_eq!(sanitized.value["monkey"], json!(REDACTED_SECRET)); + assert!(sanitized.report.key_redactions >= 4); + } + + #[test] + fn has_likely_secret_detects_common_patterns() { + assert!(has_likely_secret("api_key=abc123")); + assert!(has_likely_secret("Bearer abcdefghijklmnopqrstuvwxyz")); + assert!(has_likely_secret("xoxb-1234567890-abcdef-ghijklmnop")); + assert!(has_likely_secret("glpat-aaaaaaaaaaaaaaaaaaaa")); + assert!(has_likely_secret("SG.aaaaaaaaaaaaaaaa.bbbbbbbbbbbbbbbb")); + assert!(!has_likely_secret("I prefer rust")); + } + + #[test] + fn sanitize_text_redacts_more_provider_secrets() { + let input = "auth=Basic QWxhZGRpbjpvcGVuIHNlc2FtZQ== stripe=sk_live_12345678901234567890 npm=npm_abcdefghijklmnopqrstuvwxyz"; + let sanitized = sanitize_text(input); + assert!(!sanitized.value.contains("sk_live_12345678901234567890")); + assert!(!sanitized.value.contains("npm_abcdefghijklmnopqrstuvwxyz")); + assert!(sanitized.value.contains("[REDACTED]")); + assert!(sanitized.report.text_redactions >= 2); + } + + #[test] + fn sanitize_text_redacts_oauth_url_style_params() { + let input = "https://example.com/callback?access_token=abcd1234&refresh_token=efgh5678&id_token=jwt"; + let sanitized = sanitize_text(input); + assert!(!sanitized.value.contains("abcd1234")); + assert!(!sanitized.value.contains("efgh5678")); + assert!(!sanitized.value.contains("id_token=jwt")); + assert!(sanitized.report.text_redactions >= 3); + } + + #[test] + fn sanitize_text_redacts_multiline_private_key_blocks() { + let input = "BEGIN\n-----BEGIN OPENSSH PRIVATE KEY-----\nline1\nline2\n-----END OPENSSH PRIVATE KEY-----\nEND"; + let sanitized = sanitize_text(input); + assert!(!sanitized.value.contains("OPENSSH PRIVATE KEY")); + assert!(sanitized.value.contains(REDACTED_PRIVATE_KEY)); + assert!(sanitized.report.blocked_secret_hits >= 1); + } + + #[test] + fn sanitize_json_redacts_values_beyond_max_depth() { + let mut nested = json!("leaf"); + for _ in 0..(MAX_JSON_SANITIZE_DEPTH + 2) { + nested = json!({ "nested": nested }); + } + + let sanitized = sanitize_json(&nested); + assert!(sanitized.report.depth_redactions >= 1); + assert!(sanitized + .value + .to_string() + .contains(&format!("\"{REDACTED_SECRET}\""))); + } +} diff --git a/src/openhuman/memory/store/README.md b/src/openhuman/memory/store/README.md index 872fd3d3c..40c7b7a9c 100644 --- a/src/openhuman/memory/store/README.md +++ b/src/openhuman/memory/store/README.md @@ -10,6 +10,7 @@ Storage backend for the memory subsystem. Houses the SQLite + FTS5 + vector + gr - **`client_tests.rs`** — coverage for the client-facing storage and graph round-trips against a fresh temp workspace. - **`factories.rs`** — `create_memory*` constructors that select the embedding provider from `MemoryConfig` and instantiate `UnifiedMemory`. `effective_memory_backend_name` always reports `"namespace"`. - **`memory_trait.rs`** — `impl Memory for UnifiedMemory`, mapping the generic trait surface (`store`, `recall`, `get`, `list`, `forget`, `namespace_summaries`) onto the unified store. Includes namespace normalisation and episodic-session augmentation. +- **`../safety/`** — shared secret-detection + redaction helpers used by memory write paths (documents, KV, episodic) to prevent credentials/tokens from being persisted into long-lived memory. - **`unified/`** — the SQLite implementation, broken into per-table submodules. See `unified/README.md`. ## How it fits diff --git a/src/openhuman/memory/store/unified/README.md b/src/openhuman/memory/store/unified/README.md index 1a0a491ec..da4f5cad5 100644 --- a/src/openhuman/memory/store/unified/README.md +++ b/src/openhuman/memory/store/unified/README.md @@ -8,6 +8,7 @@ SQLite-backed implementation of the memory store. One `UnifiedMemory` struct own - **`init.rs`** — constructor, `CREATE TABLE` bootstrap (docs, kv, graph, vector chunks, episodic FTS5, segments, events, profile), idempotent legacy-namespace migrations, plus path / namespace helpers (`sanitize_namespace`, `now_ts`, `namespace_dir`). - **`documents.rs`** — `memory_docs` CRUD: `upsert_document` (chunks + embeds + writes markdown sidecar), `upsert_document_metadata_only` (light path), `list_documents`, `list_namespaces`, `delete_document`, `clear_namespace`. - **`kv.rs`** — global and namespace-scoped get/set/delete/list against `kv_global` / `kv_namespace`. +- **`../../safety/`** — secret redaction/validation helpers. Document, KV, and episodic writes sanitize credentials before persistence and emit `[memory:safety]` diagnostics when a payload is rewritten. - **`graph.rs`** — `graph_namespace` / `graph_global` upserts with attribute merging and evidence accumulation, plus namespace / global / cross-namespace queries and document-scoped relation removal. - **`query.rs`** — hybrid retrieval. Combines graph relevance, vector similarity, keyword overlap, episodic signal and freshness; exposes `query_namespace_*` (with query) and `recall_namespace_*` (query-less) entry points used by `MemoryClient`. - **`helpers.rs`** — shared utilities: f32-vector byte codecs, cosine similarity, markdown chunking, text/graph normalisation, JSON attribute merging, recency scoring. diff --git a/src/openhuman/memory/store/unified/documents.rs b/src/openhuman/memory/store/unified/documents.rs index fbbe01e70..d4ddad26e 100644 --- a/src/openhuman/memory/store/unified/documents.rs +++ b/src/openhuman/memory/store/unified/documents.rs @@ -9,6 +9,7 @@ use serde_json::{json, Value}; use std::collections::BTreeSet; use uuid::Uuid; +use crate::openhuman::memory::safety; use crate::openhuman::memory::store::types::{NamespaceDocumentInput, StoredMemoryDocument}; use super::UnifiedMemory; @@ -18,6 +19,29 @@ impl UnifiedMemory { /// sidecar, replaces vector chunks, and embeds them with the configured /// provider. pub async fn upsert_document(&self, input: NamespaceDocumentInput) -> Result { + if safety::has_likely_secret(&input.namespace) || safety::has_likely_secret(&input.key) { + log::warn!( + "[memory:safety] document write rejected due to secret-like namespace/key namespace_chars={} key_chars={}", + input.namespace.chars().count(), + input.key.chars().count() + ); + return Err("document namespace/key cannot contain secrets".to_string()); + } + + let sanitized = safety::sanitize_document_input(input); + let input = sanitized.value; + if sanitized.report.changed() { + log::warn!( + "[memory:safety] document write sanitized namespace_chars={} key_chars={} text_redactions={} key_redactions={} blocked_secret_hits={} depth_redactions={}", + input.namespace.chars().count(), + input.key.chars().count(), + sanitized.report.text_redactions, + sanitized.report.key_redactions, + sanitized.report.blocked_secret_hits, + sanitized.report.depth_redactions + ); + } + let namespace = Self::sanitize_namespace(&input.namespace); let key = input.key.trim().to_string(); if key.is_empty() { @@ -158,6 +182,29 @@ impl UnifiedMemory { &self, input: NamespaceDocumentInput, ) -> Result { + if safety::has_likely_secret(&input.namespace) || safety::has_likely_secret(&input.key) { + log::warn!( + "[memory:safety] metadata-only write rejected due to secret-like namespace/key namespace_chars={} key_chars={}", + input.namespace.chars().count(), + input.key.chars().count() + ); + return Err("document namespace/key cannot contain secrets".to_string()); + } + + let sanitized = safety::sanitize_document_input(input); + let input = sanitized.value; + if sanitized.report.changed() { + log::warn!( + "[memory:safety] metadata-only write sanitized namespace_chars={} key_chars={} text_redactions={} key_redactions={} blocked_secret_hits={} depth_redactions={}", + input.namespace.chars().count(), + input.key.chars().count(), + sanitized.report.text_redactions, + sanitized.report.key_redactions, + sanitized.report.blocked_secret_hits, + sanitized.report.depth_redactions + ); + } + let namespace = Self::sanitize_namespace(&input.namespace); let key = input.key.trim().to_string(); if key.is_empty() { diff --git a/src/openhuman/memory/store/unified/documents_tests.rs b/src/openhuman/memory/store/unified/documents_tests.rs index 6a6b8d630..73bc0ee68 100644 --- a/src/openhuman/memory/store/unified/documents_tests.rs +++ b/src/openhuman/memory/store/unified/documents_tests.rs @@ -256,3 +256,188 @@ async fn clear_namespace_removes_on_disk_markdown_files() { "docs directory should be removed after clear_namespace" ); } + +#[tokio::test] +async fn upsert_document_redacts_secret_like_content_before_persisting() { + let tmp = TempDir::new().unwrap(); + let memory = UnifiedMemory::new(tmp.path(), Arc::new(NoopEmbedding), None).unwrap(); + + memory + .upsert_document(NamespaceDocumentInput { + namespace: "safe".to_string(), + key: "secret-note".to_string(), + title: "Bearer abcdefghijklmnop".to_string(), + content: "token=abc123\n-----BEGIN PRIVATE KEY-----\nabc\n-----END PRIVATE KEY-----" + .to_string(), + source_type: "doc".to_string(), + priority: "medium".to_string(), + tags: vec!["sk-1234567890123456789012345".to_string()], + metadata: json!({ + "token": "raw", + "notes": "api_key=really-secret" + }), + category: "core".to_string(), + session_id: None, + document_id: None, + }) + .await + .unwrap(); + + let docs = memory.load_documents_for_scope("safe").await.unwrap(); + assert_eq!(docs.len(), 1); + let doc = &docs[0]; + assert!(!doc.title.contains("abcdefghijklmnop")); + assert!(doc.title.contains("[REDACTED]")); + assert!(!doc.content.contains("BEGIN PRIVATE KEY")); + assert!(doc.content.contains("[REDACTED_PRIVATE_KEY]")); + assert_eq!(doc.metadata["token"], json!("[REDACTED_SECRET]")); + assert_eq!(doc.metadata["notes"], json!("api_key=[REDACTED]")); + assert_eq!(doc.tags[0], "[REDACTED]"); + + let markdown = std::fs::read_to_string(tmp.path().join(&doc.markdown_rel_path)).unwrap(); + assert!(!markdown.contains("BEGIN PRIVATE KEY")); + assert!(markdown.contains("[REDACTED_PRIVATE_KEY]")); +} + +#[tokio::test] +async fn kv_set_namespace_redacts_secret_like_payloads() { + let tmp = TempDir::new().unwrap(); + let memory = UnifiedMemory::new(tmp.path(), Arc::new(NoopEmbedding), None).unwrap(); + + memory + .kv_set_namespace( + "safe", + "key-1", + &json!({ + "token": "super-secret", + "note": "Bearer abcdefghijklmnop" + }), + ) + .await + .unwrap(); + + let rows = memory.kv_list_namespace("safe").await.unwrap(); + assert_eq!(rows.len(), 1); + assert_eq!(rows[0]["key"], json!("key-1")); + assert_eq!(rows[0]["value"]["token"], json!("[REDACTED_SECRET]")); + assert_eq!(rows[0]["value"]["note"], json!("Bearer [REDACTED]")); +} + +#[tokio::test] +async fn kv_set_namespace_rejects_secret_like_key() { + let tmp = TempDir::new().unwrap(); + let memory = UnifiedMemory::new(tmp.path(), Arc::new(NoopEmbedding), None).unwrap(); + + let err = memory + .kv_set_namespace( + "safe", + "api_key=sk-1234567890123456789012345", + &json!({"value": "ok"}), + ) + .await + .expect_err("secret-like key should be rejected"); + assert!(err.contains("cannot contain secrets")); +} + +#[tokio::test] +async fn kv_set_namespace_rejects_secret_like_namespace() { + let tmp = TempDir::new().unwrap(); + let memory = UnifiedMemory::new(tmp.path(), Arc::new(NoopEmbedding), None).unwrap(); + + let err = memory + .kv_set_namespace( + "Bearer abcdefghijklmnop", + "safe-key", + &json!({"value": "ok"}), + ) + .await + .expect_err("secret-like namespace should be rejected"); + assert!(err.contains("cannot contain secrets")); +} + +#[tokio::test] +async fn kv_set_global_rejects_secret_like_key() { + let tmp = TempDir::new().unwrap(); + let memory = UnifiedMemory::new(tmp.path(), Arc::new(NoopEmbedding), None).unwrap(); + + let err = memory + .kv_set_global( + "authorization=Bearer abcdefghijklmnop", + &json!({"value": "ok"}), + ) + .await + .expect_err("secret-like global key should be rejected"); + assert!(err.contains("cannot contain secrets")); +} + +#[tokio::test] +async fn upsert_document_rejects_secret_like_key() { + let tmp = TempDir::new().unwrap(); + let memory = UnifiedMemory::new(tmp.path(), Arc::new(NoopEmbedding), None).unwrap(); + + let err = memory + .upsert_document(NamespaceDocumentInput { + namespace: "safe".to_string(), + key: "api_key=sk-1234567890123456789012345".to_string(), + title: "Title".to_string(), + content: "Body".to_string(), + source_type: "doc".to_string(), + priority: "medium".to_string(), + tags: vec![], + metadata: json!({}), + category: "core".to_string(), + session_id: None, + document_id: None, + }) + .await + .expect_err("secret-like key should be rejected"); + assert!(err.contains("cannot contain secrets")); +} + +#[tokio::test] +async fn upsert_document_rejects_secret_like_namespace() { + let tmp = TempDir::new().unwrap(); + let memory = UnifiedMemory::new(tmp.path(), Arc::new(NoopEmbedding), None).unwrap(); + + let err = memory + .upsert_document(NamespaceDocumentInput { + namespace: "Bearer abcdefghijklmnop".to_string(), + key: "k1".to_string(), + title: "Title".to_string(), + content: "Body".to_string(), + source_type: "doc".to_string(), + priority: "medium".to_string(), + tags: vec![], + metadata: json!({}), + category: "core".to_string(), + session_id: None, + document_id: None, + }) + .await + .expect_err("secret-like namespace should be rejected"); + assert!(err.contains("cannot contain secrets")); +} + +#[tokio::test] +async fn upsert_document_metadata_only_rejects_secret_like_key() { + let tmp = TempDir::new().unwrap(); + let memory = UnifiedMemory::new(tmp.path(), Arc::new(NoopEmbedding), None).unwrap(); + + let err = memory + .upsert_document_metadata_only(NamespaceDocumentInput { + namespace: "safe".to_string(), + key: "refresh_token=abcdef".to_string(), + title: "Title".to_string(), + content: "Body".to_string(), + source_type: "doc".to_string(), + priority: "medium".to_string(), + tags: vec![], + metadata: json!({}), + category: "core".to_string(), + session_id: None, + document_id: None, + }) + .await + .expect_err("secret-like key should be rejected"); + assert!(err.contains("cannot contain secrets")); +} diff --git a/src/openhuman/memory/store/unified/fts5.rs b/src/openhuman/memory/store/unified/fts5.rs index 5319da8cc..26c245653 100644 --- a/src/openhuman/memory/store/unified/fts5.rs +++ b/src/openhuman/memory/store/unified/fts5.rs @@ -10,6 +10,8 @@ use rusqlite::Connection; use serde::{Deserialize, Serialize}; use std::sync::Arc; +use crate::openhuman::memory::safety; + /// A single episodic record (one turn or event). #[derive(Debug, Clone, Serialize, Deserialize)] pub struct EpisodicEntry { @@ -70,17 +72,69 @@ END; /// Insert an episodic entry. pub fn episodic_insert(conn: &Arc>, entry: &EpisodicEntry) -> anyhow::Result<()> { + if safety::has_likely_secret(&entry.session_id) || safety::has_likely_secret(&entry.role) { + tracing::warn!( + "[memory:safety] episodic insert rejected secret-like session/role session_chars={} role_chars={}", + entry.session_id.chars().count(), + entry.role.chars().count() + ); + anyhow::bail!("episodic session_id/role cannot contain secrets"); + } + + let content = safety::sanitize_text(&entry.content); + let lesson = entry + .lesson + .as_ref() + .map(|value| safety::sanitize_text(value)); + let tool_calls_json = entry.tool_calls_json.as_ref().map(|value| { + if let Ok(parsed) = serde_json::from_str::(value) { + let sanitized = safety::sanitize_json(&parsed); + safety::Sanitized { + value: sanitized.value.to_string(), + report: sanitized.report, + } + } else { + safety::sanitize_text(value) + } + }); + + let report = content + .report + .merge( + lesson + .as_ref() + .map(|value| value.report) + .unwrap_or_default(), + ) + .merge( + tool_calls_json + .as_ref() + .map(|value| value.report) + .unwrap_or_default(), + ); + if report.changed() { + tracing::warn!( + "[memory:safety] episodic insert sanitized session_chars={} role_chars={} text_redactions={} key_redactions={} blocked_secret_hits={} depth_redactions={}", + entry.session_id.chars().count(), + entry.role.chars().count(), + report.text_redactions, + report.key_redactions, + report.blocked_secret_hits, + report.depth_redactions + ); + } + let conn = conn.lock(); conn.execute( "INSERT INTO episodic_log (session_id, timestamp, role, content, lesson, tool_calls_json, cost_microdollars) VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7)", rusqlite::params![ - entry.session_id, + &entry.session_id, entry.timestamp, - entry.role, - entry.content, - entry.lesson, - entry.tool_calls_json, + &entry.role, + content.value, + lesson.map(|value| value.value), + tool_calls_json.map(|value| value.value), entry.cost_microdollars as i64, ], )?; @@ -221,4 +275,52 @@ mod tests { let results = episodic_search(&conn, "nonexistent query", 10).unwrap(); assert!(results.is_empty()); } + + #[test] + fn insert_redacts_secret_like_content() { + let conn = setup_db(); + episodic_insert( + &conn, + &EpisodicEntry { + id: None, + session_id: "s1".into(), + timestamp: 1000.0, + role: "user".into(), + content: "Bearer abcdefghijklmnop".into(), + lesson: Some("token=abc123".into()), + tool_calls_json: Some("{\"api_key\":\"sk-1234567890123456789012345\"}".into()), + cost_microdollars: 0, + }, + ) + .unwrap(); + + let rows = episodic_session_entries(&conn, "s1").unwrap(); + assert_eq!(rows.len(), 1); + assert_eq!(rows[0].content, "Bearer [REDACTED]"); + assert_eq!(rows[0].lesson.as_deref(), Some("[REDACTED]")); + assert_eq!( + rows[0].tool_calls_json.as_deref(), + Some("{\"api_key\":\"[REDACTED_SECRET]\"}") + ); + } + + #[test] + fn insert_rejects_secret_like_session_id() { + let conn = setup_db(); + let err = episodic_insert( + &conn, + &EpisodicEntry { + id: None, + session_id: "Bearer abcdefghijklmnop".into(), + timestamp: 1000.0, + role: "user".into(), + content: "hello".into(), + lesson: None, + tool_calls_json: None, + cost_microdollars: 0, + }, + ) + .expect_err("secret-like session_id should be rejected"); + assert!(err.to_string().contains("cannot contain secrets")); + } } diff --git a/src/openhuman/memory/store/unified/kv.rs b/src/openhuman/memory/store/unified/kv.rs index 381f5359e..18fd45038 100644 --- a/src/openhuman/memory/store/unified/kv.rs +++ b/src/openhuman/memory/store/unified/kv.rs @@ -6,6 +6,7 @@ use rusqlite::{params, OptionalExtension}; use serde_json::json; +use crate::openhuman::memory::safety; use crate::openhuman::memory::store::types::MemoryKvRecord; use super::UnifiedMemory; @@ -13,12 +14,33 @@ use super::UnifiedMemory; impl UnifiedMemory { /// Insert or update a global key-value pair. pub async fn kv_set_global(&self, key: &str, value: &serde_json::Value) -> Result<(), String> { + if safety::has_likely_secret(key) { + log::warn!( + "[memory:safety] kv_set_global rejected secret-like key key_chars={}", + key.chars().count() + ); + return Err("kv key cannot contain secrets".to_string()); + } + + let sanitized_value = safety::sanitize_json(value); + let report = sanitized_value.report; + if report.changed() { + log::warn!( + "[memory:safety] kv_set_global sanitized key_chars={} text_redactions={} key_redactions={} blocked_secret_hits={} depth_redactions={}", + key.chars().count(), + report.text_redactions, + report.key_redactions, + report.blocked_secret_hits, + report.depth_redactions + ); + } + let conn = self.conn.lock(); conn.execute( "INSERT INTO kv_global (key, value_json, updated_at) VALUES (?1, ?2, ?3) ON CONFLICT(key) DO UPDATE SET value_json = excluded.value_json, updated_at = excluded.updated_at", - params![key, value.to_string(), Self::now_ts()], + params![key, sanitized_value.value.to_string(), Self::now_ts()], ) .map_err(|e| format!("kv_set_global: {e}"))?; Ok(()) @@ -45,12 +67,40 @@ impl UnifiedMemory { key: &str, value: &serde_json::Value, ) -> Result<(), String> { + if safety::has_likely_secret(namespace) || safety::has_likely_secret(key) { + log::warn!( + "[memory:safety] kv_set_namespace rejected secret-like namespace/key namespace_chars={} key_chars={}", + namespace.chars().count(), + key.chars().count() + ); + return Err("kv namespace/key cannot contain secrets".to_string()); + } + + let sanitized_value = safety::sanitize_json(value); + let report = sanitized_value.report; + if report.changed() { + log::warn!( + "[memory:safety] kv_set_namespace sanitized namespace_chars={} key_chars={} text_redactions={} key_redactions={} blocked_secret_hits={} depth_redactions={}", + namespace.chars().count(), + key.chars().count(), + report.text_redactions, + report.key_redactions, + report.blocked_secret_hits, + report.depth_redactions + ); + } + let conn = self.conn.lock(); conn.execute( "INSERT INTO kv_namespace (namespace, key, value_json, updated_at) VALUES (?1, ?2, ?3, ?4) ON CONFLICT(namespace, key) DO UPDATE SET value_json = excluded.value_json, updated_at = excluded.updated_at", - params![Self::sanitize_namespace(namespace), key, value.to_string(), Self::now_ts()], + params![ + Self::sanitize_namespace(namespace), + key, + sanitized_value.value.to_string(), + Self::now_ts() + ], ) .map_err(|e| format!("kv_set_namespace: {e}"))?; Ok(()) diff --git a/src/openhuman/tools/impl/memory/store.rs b/src/openhuman/tools/impl/memory/store.rs index a83028e44..22b5901b8 100644 --- a/src/openhuman/tools/impl/memory/store.rs +++ b/src/openhuman/tools/impl/memory/store.rs @@ -1,3 +1,4 @@ +use crate::openhuman::memory::safety; use crate::openhuman::memory::{Memory, MemoryCategory}; use crate::openhuman::security::policy::ToolOperation; use crate::openhuman::security::SecurityPolicy; @@ -90,6 +91,19 @@ impl Tool for MemoryStoreTool { if key.is_empty() { return Ok(ToolResult::error("key cannot be empty".to_string())); } + + if safety::has_likely_secret(content) { + log::warn!( + "[memory:safety] memory_store rejected secret-like content namespace_chars={} key_chars={} content_chars={}", + namespace.chars().count(), + key.chars().count(), + content.chars().count() + ); + return Ok(ToolResult::error( + "Refusing to store content that looks like a secret. Remove credentials or tokens and try again.".to_string(), + )); + } + let display_key = format!("{namespace}/{key}"); match self .memory @@ -176,6 +190,23 @@ mod tests { assert_eq!(entry.category, MemoryCategory::Custom("project".into())); } + #[tokio::test] + async fn store_rejects_secret_like_content() { + let (_tmp, mem) = test_mem(); + let tool = MemoryStoreTool::new(mem.clone(), test_security()); + let result = tool + .execute(json!({ + "namespace": "global", + "key": "api", + "content": "api_key=sk-123456789012345678901234567890" + })) + .await + .unwrap(); + assert!(result.is_error); + assert!(result.output().contains("looks like a secret")); + assert!(mem.get("global", "api").await.unwrap().is_none()); + } + #[tokio::test] async fn store_missing_key() { let (_tmp, mem) = test_mem();