diff --git a/.gitleaks.toml b/.gitleaks.toml index 7ffbf11507..23e316a26d 100644 --- a/.gitleaks.toml +++ b/.gitleaks.toml @@ -39,6 +39,12 @@ condition = "AND" paths = ['''^crates/tui/src/tui/widgets/workflow_panel\.rs$'''] regexes = ['''^sk-receipt-secret-1234567890$'''] +[[rules.allowlists]] +description = "Synthetic memory-refusal test fixtures" +condition = "AND" +paths = ['''^crates/tui/src/native_memory/tests\.rs$'''] +regexes = ['''^sk-live-abc123$''', '''^ghp_0123456789abcdef$'''] + [[rules]] id = "jwt" diff --git a/crates/tui/src/native_memory.rs b/crates/tui/src/native_memory.rs index 38d299f3a8..b06c3e0a51 100644 --- a/crates/tui/src/native_memory.rs +++ b/crates/tui/src/native_memory.rs @@ -5,6 +5,7 @@ //! or network dependency: callers decide when a note is reviewed and written. use std::collections::{HashMap, HashSet}; +use std::fmt; use std::fs::{self, File, OpenOptions}; use std::io::{self, Write}; use std::path::{Path, PathBuf}; @@ -177,6 +178,15 @@ impl NativeMemoryStore { /// Import the pre-v0.9.2 single memory file without removing or mutating /// it. An existing native source wins so repeated startup is idempotent. + /// + /// Content is screened like any other store entry: a legacy file written + /// under the advisory-only regime may hold captured secrets, and import + /// must not bulk-inject them into every future prompt. The file is + /// screened as a whole — not line by line — so a secret split across + /// lines (`the password is\nhunter2`) cannot slip between per-line + /// checks. One tripping note refuses the whole import: nothing + /// partially migrates, and the legacy file is left untouched so it can + /// be cleaned and imported again. pub fn import_legacy(&self, legacy_path: &Path) -> Result { self.with_write_lock(|| { if !legacy_path.is_file() || self.global_path().exists() { @@ -187,6 +197,7 @@ impl NativeMemoryStore { if content.trim().is_empty() { return Ok(false); } + refuse_sensitive_note(&content)?; let target = self.global_path(); ensure_memory_file(&target)?; fs::write(&target, content)?; @@ -196,7 +207,10 @@ impl NativeMemoryStore { } /// Append a reviewed note to the selected Markdown source and refresh its - /// index. The note is treated as data, never as an instruction. + /// index. The note is treated as data, never as an instruction, and is + /// refused outright when it looks like a secret or other sensitive + /// personal data — the memory files are plain text on disk and are + /// injected verbatim into future prompts. pub fn remember( &self, scope: MemoryScope, @@ -204,6 +218,7 @@ impl NativeMemoryStore { note: &str, ) -> Result { let note = normalize_note(note)?; + refuse_sensitive_note(¬e)?; let path = match scope { MemoryScope::Global => self.global_path(), MemoryScope::Workspace => self.workspace_path( @@ -257,6 +272,13 @@ impl NativeMemoryStore { evidence: &str, ) -> Result { let to = normalize_note(to)?; + // The gate runs on the replacement only. `from` matches a note that + // is already in the store — possibly written before this gate + // existed — so refusing it would make an already-stored sensitive + // note impossible to revise away. `retire` is ungated for the same + // reason, and evidence is journal-only text that is never indexed + // or injected. + refuse_sensitive_note(&to)?; let evidence = normalize_evidence(evidence)?; let from_needle = normalize_note(from)?; let path = self.scope_path(scope, workspace_id)?; @@ -893,6 +915,277 @@ fn normalize_note(note: &str) -> Result { Ok(note.trim_start_matches('-').trim().to_string()) } +/// Stable, model-actionable refusal for note content that looks like a +/// secret or other sensitive personal data. The string is part of the +/// contract: the `remember` tool surfaces it verbatim so the model can act +/// on it (rephrase without the sensitive values, or store nothing). +#[derive(Debug, Clone)] +pub struct SensitiveNoteRefusal; + +const SENSITIVE_NOTE_REASON: &str = "memory note refused: it looks like it contains sensitive \ + data such as a credential, token, password, phone or ID number. Memory files are plain \ + text on disk and are injected verbatim into future prompts, so never store secrets \ + there. Restate the durable fact without the sensitive values, or keep the secret in a \ + dedicated credential store instead."; + +impl fmt::Display for SensitiveNoteRefusal { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(SENSITIVE_NOTE_REASON) + } +} + +impl std::error::Error for SensitiveNoteRefusal {} + +/// Fail-closed gate on the memory write path: a note that trips any +/// sensitive-content heuristic is refused with [`SensitiveNoteRefusal`] +/// instead of being written. Nothing is silently dropped. +fn refuse_sensitive_note(note: &str) -> Result<()> { + if looks_sensitive(note) { + return Err(SensitiveNoteRefusal.into()); + } + Ok(()) +} + +/// Sensitive-content heuristics for the memory write path, ported from the +/// app-side embedder's production gate (`features/memory/util.rs:: +/// looks_sensitive` and its helpers), where they have filtered captured +/// memory notes since before this store existed. This is a heuristic net, +/// not a parser, and it errs closed: false positives cost a rephrased note, +/// false negatives leak a secret into plain-text files that are injected +/// into every future prompt. Matching runs on a whitespace-collapsed +/// projection ([`collapse_whitespace`]): `normalize_note` joins lines but +/// preserves whitespace inside a line, and a tab or NBSP must not split a +/// needle or an assignment marker. One deviation from the source is +/// disclosed here: the embedder screens a 500-character `clean_text` +/// projection, while this gate screens the full note — strictly more, in +/// the same err-closed direction. +/// +/// Refused families: +/// - a run of 11 or more ASCII digits, bridging the separators grouped +/// numbers are conventionally written with (spaces, hyphen/dash variants, +/// and dots) — CN mobile numbers are 11 digits and resident ID cards 18, +/// while ordinary dates and version numbers stay under it (`2024-02-01` +/// bridges to 8) (adapted from the embedder's total-digit count, which +/// false-positives on date/version-heavy full-sentence notes) +/// - credential needles, including the Chinese markers 身份证 / 手机号 / +/// 密码 / 口令 / 密钥 / 私钥 — kept deliberately: they are the real-world +/// secret markers in the locales the source heuristics run in production; +/// `api-key` (the HTTP header spelling), the `private_key` underscore +/// spelling, and the compound key kinds `ssh key` / `deploy key` / +/// `access key` are additions on top of the port +/// - `token` (or another credential word) tied to an assignment marker +/// (`=`, `:`, the full-width `：`, the CJK copulas 是/为, and — +/// adaptations for English notes — the copulas " is " / " was " and the +/// arrow `->`; without them "the password is hunter2" walks through) +/// - AWS access-key IDs (`AKIA`/`ASIA` + 16 uppercase alphanumerics, +/// matched case-insensitively so a lowercased spelling cannot walk +/// through) — added on top of the port so a bare key is refused even with +/// no assignment or needle around it +/// - PEM private-key armor (`-----BEGIN ... PRIVATE KEY-----`) — +/// `private key` is likewise an addition to the ported needle list +/// - URLs with any scheme (`scheme://`, not just web) and email addresses — +/// a durable preference rarely needs either and credentialed URLs are the +/// classic `user:password@host` leak; the embedder matched only http(s), +/// which missed `postgres://user:pw@db/app` +/// - filesystem paths — unix roots `/home/ /root/ /tmp/ /users/ /var/ +/// /etc/ /opt/` and `~/` anywhere in the note, `\users\` / `\appdata\`, +/// and any Windows drive root (`c:\`, `d:/keys`); the embedder refused +/// only sentence-initial `~/` and the `c:` drive +/// +/// Known slack, accepted because the net errs closed and the refusal +/// message says how to restate: a bare token with no credential word or +/// assignment around it (a GitHub PAT, a JWT, a PEM body without its armor +/// line) passes, as do homoglyph, full-width digit, and invisible-format +/// character spellings, and so does a generic "… key is …" shape with no +/// compound kind (`the database key is …`). The net also over-refuses on +/// purpose: sharing a documentation link, mentioning `/tmp/` caches, or +/// naming a server trips the URL, path, and digit rules, as do +/// space-separated number lists (ports, backoffs), IPv4 addresses with 11+ +/// digits (`192.168.100.100`), and a credential word anywhere near an +/// unrelated colon (`rotate the github token every 90 days: set a +/// reminder`); long order or epoch-millisecond numbers trip the digit rule. +/// +/// The gate runs where content enters the store: append, the revise +/// replacement, and `import_legacy` (screened as a whole, refusing +/// wholesale). Retrieval (`search`, `get`) is read-only and never gated; +/// `retire` and the revise source are never gated so a sensitive note that +/// predates the gate can always be removed again — note that both still +/// record the original text in the plaintext journal (`JOURNAL.md`), which +/// is never indexed or injected but is not scrubbed. Writes through the +/// approval-gated file tools are out of scope: with the user approving +/// each write, they carry the same trust as hand-editing the file. +const SENSITIVE_NEEDLES: [&str; 19] = [ + "身份证", + "手机号", + "手机号码", + "电话号码", + "联系电话", + "密码", + "口令", + "密钥", + "私钥", + "api_key", + "apikey", + "api key", + "api-key", + "secret", + "private key", + "private_key", + "ssh key", + "deploy key", + "access key", +]; + +fn looks_sensitive(value: &str) -> bool { + // Matching runs on the whitespace-canonical projection: `normalize_note` + // joins lines but preserves whitespace inside a line, and a tab or NBSP + // in "the passwordis hunter2" must not split a needle or the + // " is " copula. Folding only removes whitespace variation, so nothing + // that matches on the raw note can match less on the projection. + let value = collapse_whitespace(value); + let lower = value.to_ascii_lowercase(); + if has_digit_run(&value, 11) { + return true; + } + if SENSITIVE_NEEDLES + .iter() + .any(|needle| lower.contains(needle)) + { + return true; + } + if looks_like_aws_access_key_id(&value) { + return true; + } + if lower.contains("token") && has_assignment_marker(&lower, &value) { + return true; + } + looks_like_url(&lower) + || looks_like_email(&value) + || looks_like_filesystem_path(&value) + || looks_like_credential_assignment(&value) +} + +/// Collapse every Unicode whitespace run to a single ASCII space. +fn collapse_whitespace(value: &str) -> String { + value.split_whitespace().collect::>().join(" ") +} + +/// Digits forming a run of `min_run`, bridging the separators grouped +/// numbers are conventionally written with (spaces, hyphen/dash variants, +/// dots). Any other character breaks the run, so punctuation between +/// unrelated numbers does not accumulate: `2024-02-01` bridges to 8 +/// digits, and a comma-separated list stays flat. +const DIGIT_RUN_SEPARATORS: [char; 6] = [' ', '-', '－', '–', '—', '.']; + +fn has_digit_run(value: &str, min_run: usize) -> bool { + let mut run = 0; + for ch in value.chars() { + if ch.is_ascii_digit() { + run += 1; + if run >= min_run { + return true; + } + } else if !DIGIT_RUN_SEPARATORS.contains(&ch) { + run = 0; + } + } + false +} + +/// AWS access-key IDs are a fixed, unambiguous credential shape: four +/// letters starting `AKIA` (long-lived) or `ASIA` (temporary), then 16 +/// uppercase letters or digits. Matched case-insensitively: a lowercased +/// spelling (`akia…`) is the same credential, and no prose word is +/// 20 alphanumeric characters long. +fn looks_like_aws_access_key_id(value: &str) -> bool { + let upper = value.to_ascii_uppercase(); + upper + .split(|ch: char| !ch.is_ascii_uppercase() && !ch.is_ascii_digit()) + .any(|token| token.len() == 20 && (token.starts_with("AKIA") || token.starts_with("ASIA"))) +} + +/// Any scheme, not just web URLs: `postgres://user:pw@db/app` and +/// `redis://:pass@host` are credentialed-URL shapes too, and a durable +/// preference rarely needs a scheme'd link at all. +fn looks_like_url(lower: &str) -> bool { + lower.contains("://") +} + +fn looks_like_email(value: &str) -> bool { + value + .split(|c: char| { + c.is_whitespace() + || matches!( + c, + '，' | '。' | '；' | ';' | ',' | '<' | '>' | '"' | '\'' | '(' | ')' | '[' | ']' + ) + }) + .any(|token| { + let Some((left, right)) = token.split_once('@') else { + return false; + }; + !left.is_empty() && right.contains('.') && right.len() >= 3 + }) +} + +fn looks_like_filesystem_path(value: &str) -> bool { + let lower = value.to_ascii_lowercase(); + let unix_roots = [ + "/home/", "/root/", "/tmp/", "/users/", "/var/", "/etc/", "/opt/", + ]; + if unix_roots.iter().any(|root| lower.contains(root)) { + return true; + } + lower.contains("~/") + || lower.contains("\\users\\") + || lower.contains("\\appdata\\") + || has_windows_drive(&lower) +} + +/// A Windows drive root with any ASCII drive letter (`c:\users`, +/// `d:/keys`), not just `c:`. Byte-window matching is UTF-8 safe: the +/// pattern is pure ASCII and ASCII bytes never occur inside a multi-byte +/// char. +fn has_windows_drive(lower: &str) -> bool { + lower + .as_bytes() + .windows(3) + .any(|w| w[0].is_ascii_alphabetic() && w[1] == b':' && (w[2] == b'\\' || w[2] == b'/')) +} + +/// An assignment-shaped join between a credential word and a value: symbol +/// assignment (`=`, `:`, the full-width `：`), the CJK copulas 是/为, or — +/// for English notes — the copulas " is " / " was " and the arrow `->`. +/// Shared by the token rule and the credential-assignment rule so the two +/// cannot drift. The substring markers are safe to match because matching +/// runs on the whitespace-collapsed projection (see +/// [`collapse_whitespace`]). +fn has_assignment_marker(lower: &str, value: &str) -> bool { + lower.contains('=') + || lower.contains(':') + || lower.contains('：') + || lower.contains("->") + || value.contains('是') + || value.contains('为') + || lower.contains(" is ") + || lower.contains(" was ") +} + +fn looks_like_credential_assignment(value: &str) -> bool { + let lower = value.to_ascii_lowercase(); + let has_assignment = has_assignment_marker(&lower, value); + // Words that are bare needles above (`api_key`, `secret`, 密钥, …) are + // repeated here on purpose: this must stay the full credential-word + // view so the rule stays correct even if either list changes alone. + has_assignment + && [ + "token", "api_key", "apikey", "api key", "secret", "password", "passwd", "pwd", + "passcode", "密钥", "私钥", "口令", "密码", + ] + .iter() + .any(|needle| lower.contains(needle)) +} + fn validate_query(query: &str) -> Result<&str> { let query = query.trim(); if query.is_empty() || query.chars().count() > MAX_QUERY_CHARS { diff --git a/crates/tui/src/native_memory/tests.rs b/crates/tui/src/native_memory/tests.rs index 3259175f3b..0d9daa7911 100644 --- a/crates/tui/src/native_memory/tests.rs +++ b/crates/tui/src/native_memory/tests.rs @@ -315,3 +315,243 @@ fn corrupt_or_old_cache_rebuilds_from_markdown() { .unwrap(); assert_eq!(store.search("recoverable", 10).unwrap().len(), 1); } + +/// Secrets must never reach the plain-text memory files: the write path +/// refuses sensitive-looking notes with the stable refusal reason instead +/// of writing or silently dropping them. +#[test] +fn sensitive_notes_are_refused_at_the_write_path() { + let tmp = TempDir::new().unwrap(); + let store = NativeMemoryStore::new(tmp.path()); + for note in [ + "aws key AKIAIOSFODNN7EXAMPLE belongs to the ci account", + "rotate it from -----BEGIN RSA PRIVATE KEY----- in the vault", + "her 身份证 is 110105194912310021", + "密码：hunter2", + "password：hunter2", + "the service expects api_key=sk-live-abc123", + "the X-API-Key header carries hunter2", + "token: ghp_0123456789abcdef", + "rotate the k8s secret monthly", + "call her at 13812345678", + "call her at 138-1234-5678", + "his number is 138 1234 5678 these days", + "order id 100000000001", + "the password\tis hunter2", + "postgres://user:hunter2@db:5432/app", + "docs at https://user:hunter2@example.test/internal", + "docs live at https://intranet.example.test/notes", + "my email is alice.smith@example.org", + "ssh keys sit in /root/.ssh/id_rsa", + "her dotfiles live in the ~/notes folder", + "backups land on D:/keys/archive", + "call her at 138.1234.5678", + "the card number is 4111.1111.1111.1111", + "lowercase akiaiosfodnn7example also leaks", + "the private_key field holds the signing material", + "the deploy key is hunter2horse", + "the ssh key is hunter2horse", + "her access key is hunter2horse", + "the old password was hunter2", + "password -> hunter2", + ] { + let result = store.remember(MemoryScope::Global, None, note); + let error = result.expect_err(&format!("must refuse: {note}")); + assert!( + error.to_string().contains("memory note refused"), + "unexpected refusal reason for {note}: {error}" + ); + } + // The gate is scope-independent: workspace capture is screened too. + let error = store + .remember( + MemoryScope::Workspace, + Some("origin-hash"), + "token: ghp_0123456789abcdef", + ) + .unwrap_err(); + assert!(error.to_string().contains("memory note refused"), "{error}"); + assert!( + fs::read_to_string(store.global_path()) + .unwrap_or_default() + .trim() + .is_empty(), + "a refused note must leave no trace in the store" + ); +} + +/// The net errs closed but must stay livable: ordinary notes with dates, +/// version numbers, and short or grouped numbers pass untouched. +#[test] +fn ordinary_notes_still_pass_the_sensitivity_gate() { + let tmp = TempDir::new().unwrap(); + let store = NativeMemoryStore::new(tmp.path()); + for note in [ + "Use 4-space indentation in this repo", + "Deploys run on Tuesdays", + "Release 2024.01.15 was reviewed on 2024-02-01 at 14:30", + "Use Node 20.11.1 and pnpm 9.4.2 for builds", + "The user prefers pytest over unittest", + "415-555-1234 is the front desk line", + "retry backoff seconds: 30 60 120 300", + // Bare "key" is not a needle (keybindings, key-value stores): only + // the compound kinds are refused on sight. + "the key insight is that builds are cached", + ] { + store + .remember(MemoryScope::Global, None, note) + .unwrap_or_else(|error| panic!("must accept {note}: {error}")); + } +} + +/// The import path is part of the store surface: a legacy file written +/// under the advisory-only regime must not bulk-inject a captured secret, +/// and a refused import leaves both files untouched. +#[test] +fn import_legacy_refuses_sensitive_content_wholesale() { + let tmp = TempDir::new().unwrap(); + let legacy = tmp.path().join("memory.md"); + fs::write( + &legacy, + "keep this legacy note\n- legacy api_key=sk-live-abc123\n", + ) + .unwrap(); + let store = NativeMemoryStore::new(tmp.path().join("native")); + let error = store.import_legacy(&legacy).unwrap_err(); + assert!(error.to_string().contains("memory note refused"), "{error}"); + assert!( + !store.global_path().exists(), + "a refused import must write nothing" + ); + assert_eq!( + fs::read_to_string(&legacy).unwrap(), + "keep this legacy note\n- legacy api_key=sk-live-abc123\n", + "the legacy file is never mutated" + ); +} + +/// The import screen runs on the whole file, not per line: a secret split +/// across lines must not walk through between the per-line checks. +#[test] +fn import_legacy_refuses_secrets_split_across_lines() { + let tmp = TempDir::new().unwrap(); + let legacy = tmp.path().join("memory.md"); + fs::write(&legacy, "keep this legacy note\nthe password is\nhunter2\n").unwrap(); + let store = NativeMemoryStore::new(tmp.path().join("native")); + let error = store.import_legacy(&legacy).unwrap_err(); + assert!(error.to_string().contains("memory note refused"), "{error}"); + assert!( + !store.global_path().exists(), + "a refused import must write nothing" + ); + assert_eq!( + fs::read_to_string(&legacy).unwrap(), + "keep this legacy note\nthe password is\nhunter2\n", + "the legacy file is never mutated" + ); +} + +/// The empty and oversize bounds predate the sensitivity gate and are +/// unchanged by it. +#[test] +fn empty_and_oversize_bounds_are_unchanged() { + let tmp = TempDir::new().unwrap(); + let store = NativeMemoryStore::new(tmp.path()); + let empty = store + .remember(MemoryScope::Global, None, " \r\n ") + .unwrap_err(); + assert!( + empty.to_string().contains("memory note is empty"), + "{empty}" + ); + let oversize = store + .remember(MemoryScope::Global, None, &"x".repeat(64 * 1024 + 1)) + .unwrap_err(); + assert!( + oversize.to_string().contains("exceeds 65536 bytes"), + "{oversize}" + ); + // Precedence: the bounds run before the sensitivity screen, so an + // oversize note reports the bound even when it also trips the gate. + let oversize_sensitive = store + .remember( + MemoryScope::Global, + None, + &format!("the secret is {}", "x".repeat(64 * 1024)), + ) + .unwrap_err(); + assert!( + oversize_sensitive + .to_string() + .contains("exceeds 65536 bytes"), + "{oversize_sensitive}" + ); +} + +/// The gate covers what enters the store, not what leaves it: a sensitive +/// replacement is refused, while a sensitive note that predates the gate +/// (written directly to the Markdown source here) can still be revised +/// away. +#[test] +fn revise_refuses_sensitive_replacements_but_can_still_remove_stored_ones() { + let tmp = TempDir::new().unwrap(); + let store = NativeMemoryStore::new(tmp.path()); + store + .remember(MemoryScope::Global, None, "Deploys run on Tuesdays") + .unwrap(); + + let error = store + .revise( + MemoryScope::Global, + None, + "Deploys run on Tuesdays", + "deploy password is hunter2", + "schedule changed", + ) + .unwrap_err(); + assert!(error.to_string().contains("memory note refused"), "{error}"); + assert_eq!( + store.search("Deploys", 10).unwrap()[0].text, + "Deploys run on Tuesdays", + "a refused replacement must leave the store untouched" + ); + + let path = store.global_path(); + ensure_memory_file(&path).unwrap(); + fs::write(&path, "- legacy api_key=sk-live-abc123\n").unwrap(); + store + .revise( + MemoryScope::Global, + None, + "legacy api_key=sk-live-abc123", + "legacy credentials were rotated out of memory", + "gate cleanup", + ) + .unwrap_or_else(|error| panic!("retiring stored sensitive notes must work: {error}")); + assert_eq!( + store.search("legacy", 10).unwrap()[0].text, + "legacy credentials were rotated out of memory" + ); +} + +/// Removal stays possible for content that predates the gate: `retire` is +/// never sensitivity-gated, or the gate would preserve the very leak it +/// exists to prevent. +#[test] +fn retire_is_never_gated_by_the_sensitivity_check() { + let tmp = TempDir::new().unwrap(); + let store = NativeMemoryStore::new(tmp.path()); + let path = store.global_path(); + ensure_memory_file(&path).unwrap(); + fs::write(&path, "- the old 密码 is 123456\n").unwrap(); + + store + .retire( + MemoryScope::Global, + None, + "the old 密码 is 123456", + "secret removed from the store", + ) + .unwrap_or_else(|error| panic!("retire must not be gated: {error}")); + assert!(store.search("密码", 10).unwrap().is_empty()); +} diff --git a/crates/tui/src/tools/native_memory.rs b/crates/tui/src/tools/native_memory.rs index f7433f482a..2b452f7cf3 100644 --- a/crates/tui/src/tools/native_memory.rs +++ b/crates/tui/src/tools/native_memory.rs @@ -284,11 +284,14 @@ mod tests { "first workspace note", ) .unwrap(); + // The fixture word must not trip the sensitive-content gate the + // write path enforces; the test is about scope boundaries, not + // content screening. let second_hit = store .remember( crate::native_memory::MemoryScope::Workspace, Some(&second_id), - "second workspace secret", + "second workspace exclusion", ) .unwrap(); @@ -298,7 +301,7 @@ mod tests { .await .unwrap(); assert!(result.content.contains("first workspace note")); - assert!(!result.content.contains("second workspace secret")); + assert!(!result.content.contains("second workspace exclusion")); let error = MemoryGetTool .execute(json!({"id": second_hit.id}), &context) diff --git a/crates/tui/src/tools/remember.rs b/crates/tui/src/tools/remember.rs index 91821557da..c0194e7d5f 100644 --- a/crates/tui/src/tools/remember.rs +++ b/crates/tui/src/tools/remember.rs @@ -17,6 +17,18 @@ use serde_json::{Value, json}; use super::spec::{ ApprovalRequirement, ToolCapability, ToolContext, ToolError, ToolResult, ToolSpec, required_str, }; +use crate::native_memory::SensitiveNoteRefusal; + +/// Map a native store failure to a tool error. A sensitive-content refusal +/// is input the model can fix, so its stable reason string is surfaced as +/// invalid input instead of a generic execution failure. +fn write_error(error: anyhow::Error, failure: &str) -> ToolError { + if error.downcast_ref::().is_some() { + ToolError::invalid_input(error.to_string()) + } else { + ToolError::execution_failed(format!("{failure}: {error}")) + } +} /// Tool that appends one bullet to the user memory file. pub struct RememberTool; @@ -33,7 +45,10 @@ impl ToolSpec for RememberTool { or a fact about themselves or their workflow that you should not \ have to relearn next time. Keep notes terse (one sentence). Don't \ store secrets, transient tasks, or reasoning scratch — those belong \ - in a checklist or in the conversation.\n\n\ + in a checklist or in the conversation. The no-secrets rule is \ + enforced: a note that looks like it carries a credential, token, \ + password, phone, or ID number is refused, so state the durable \ + fact without the sensitive values in the first place.\n\n\ Memory you only add to decays: corrections pile up behind the notes \ they contradict, and what gets injected next session drifts toward \ noise. So also maintain it. When something you were told before \ @@ -181,17 +196,11 @@ impl ToolSpec for RememberTool { let evidence = required_str(&input, "evidence")?; store .revise(scope, workspace_id.as_deref(), replaces, ¬e, evidence) - .map_err(|error| { - ToolError::execution_failed(format!("failed to revise memory: {error}")) - })? + .map_err(|error| write_error(error, "failed to revise memory"))? } else { store .remember(scope, workspace_id.as_deref(), ¬e) - .map_err(|error| { - ToolError::execution_failed(format!( - "failed to write native memory: {error}" - )) - })? + .map_err(|error| write_error(error, "failed to write native memory"))? }; let verb = if action == "revise" { "revised" @@ -445,4 +454,85 @@ mod tests { let err = tool.execute(json!({}), &ctx).await.unwrap_err(); assert!(err.to_string().to_lowercase().contains("note"), "{err}"); } + + /// The store refuses sensitive-looking notes fail-closed; the tool surfaces + /// that as invalid input carrying the stable refusal reason, not as a + /// generic execution failure, and writes nothing. + #[tokio::test] + async fn sensitive_notes_are_refused_with_a_stable_reason() { + let tmp = tempdir().unwrap(); + let root = tmp.path().join("memory"); + let mut ctx = ToolContext::new(tmp.path()); + ctx.memory_path = Some(root.join("global/MEMORY.md")); + + let err = RememberTool + .execute( + json!({"note": "aws key AKIAIOSFODNN7EXAMPLE belongs to the ci account"}), + &ctx, + ) + .await + .unwrap_err(); + let message = err.to_string(); + assert!( + message.starts_with("Failed to validate input"), + "a refusal is fixable input, not an execution failure: {message}" + ); + assert!( + message.contains("memory note refused"), + "refusal must carry the stable reason: {message}" + ); + assert!( + message.contains("never store secrets"), + "refusal must be model-actionable: {message}" + ); + assert!( + !root.join("global/MEMORY.md").exists(), + "a refused note must not be written" + ); + + let result = RememberTool + .execute(json!({"note": "Use 4-space indentation"}), &ctx) + .await + .expect("the store must stay usable after a refusal"); + assert!(result.success); + } + + /// The revise branch is mapped through the same helper: a sensitive + /// replacement surfaces as invalid input with the stable reason, and the + /// stored note is left untouched. + #[tokio::test] + async fn revise_refusal_surfaces_as_invalid_input_too() { + let tmp = tempdir().unwrap(); + let root = tmp.path().join("memory"); + let mut ctx = ToolContext::new(tmp.path()); + ctx.memory_path = Some(root.join("global/MEMORY.md")); + + RememberTool + .execute(json!({"note": "Deploys run on Fridays"}), &ctx) + .await + .expect("append"); + let err = RememberTool + .execute( + json!({ + "action": "revise", + "replaces": "Deploys run on Fridays", + "note": "deploy password is hunter2", + "evidence": "schedule changed" + }), + &ctx, + ) + .await + .unwrap_err(); + let message = err.to_string(); + assert!( + message.starts_with("Failed to validate input") + && message.contains("memory note refused"), + "a refused replacement is invalid input: {message}" + ); + + let store = crate::native_memory::NativeMemoryStore::new(&root); + let hits = store.search("Deploys", 10).expect("search"); + assert_eq!(hits.len(), 1, "the stored note must be untouched"); + assert_eq!(hits[0].text, "Deploys run on Fridays"); + } } diff --git a/docs/MEMORY.md b/docs/MEMORY.md index c2e806cc8a..e2a1cee620 100644 --- a/docs/MEMORY.md +++ b/docs/MEMORY.md @@ -156,7 +156,8 @@ repository with an `origin` remote (the scope id is a hash of it). Memory is for **durable** signal. Things that should NOT live there: - **Secrets** — no API keys, tokens, passwords. The files are plain - text on disk and entries are injected into the system prompt. + text on disk and entries are injected into the system prompt. This + one is enforced, not advisory — see below. - **Transient task state** — "I'm currently working on the parser" changes every session; it doesn't belong in cross-session memory. - **Conversation snippets** — quote-style notes belong in the notes @@ -164,6 +165,77 @@ Memory is for **durable** signal. Things that should NOT live there: - **Long instructions** — anything over a few sentences should live in `AGENTS.md` (project-level) or in a skill. +## Enforced sensitive-content refusal + +Since this gate landed, the write path does not rely on the model's +judgement about secrets. `NativeMemoryStore::remember`, the `to` half +of `revise`, and `import_legacy` (screened as a whole, so a secret +split across lines cannot slip through; one tripping note refuses the +whole import) run every note through heuristic screens +(`crates/tui/src/native_memory.rs`, `looks_sensitive` and its helpers, +ported from the app-side embedder where they filter captured memory in +production). Matching runs on a whitespace-collapsed projection of the +note, so tabs or full-width spaces cannot split a needle or a marker: + +- a run of 11+ digits, bridging the spaces, hyphens, dashes, and dots + grouped numbers are conventionally written with (CN mobile numbers + are 11, resident ID cards 18; dates and version numbers stay under + it — `2024-02-01` bridges to 8), +- credential needles — `api_key`, `apikey`, `api key`, `api-key`, + `secret`, `private key`, `private_key`, the compound key kinds + `ssh key` / `deploy key` / `access key`, and the Chinese markers + 身份证 / 手机号 / 手机号码 / 电话号码 / 联系电话 / 密码 / 口令 / 密钥 / + 私钥, which are the real-world secret markers in the locales the + source heuristics run in, +- `token` or another credential word (including `password`, `passwd`, + `pwd`, `passcode`) tied to an assignment (`=`, `:`, `：`, `->`, 是, + 为, or the English copulas " is " / " was "), +- AWS access-key IDs (`AKIA`/`ASIA` + 16 uppercase alphanumerics, + matched case-insensitively) and PEM private-key armor, +- URLs with any scheme and email addresses: a durable preference rarely + needs one, and credentialed URLs are the classic `user:password@host` + leak — `postgres://user:pw@db/app` is refused like `https://...`, +- filesystem paths — `/home/`, `/root/`, `/tmp/`, `/users/`, `/var/`, + `/etc/`, `/opt/` and `~/` anywhere in the note, plus `\users\`, + `\appdata\`, and any Windows drive root (`c:\`, `d:/keys`). + +A note that trips the screen is refused fail-closed — never written, +never silently dropped — with a stable, model-actionable error: + +```text +memory note refused: it looks like it contains sensitive data such as +a credential, token, password, phone or ID number. Memory files are +plain text on disk and are injected verbatim into future prompts, so +never store secrets there. Restate the durable fact without the +sensitive values, or keep the secret in a dedicated credential store +instead. +``` + +The screen covers what enters the store. Retrieval (`memory_search`, +`memory_get`) is read-only and untouched, and `retire` plus the revise +source are never screened, so a sensitive note that predates the gate +can always be revised away or retired — note that both still record +the original note text in the plaintext journal (`JOURNAL.md`), which +is never indexed or injected but is not scrubbed. Writes through the +approval-gated file tools are out of scope: with the user approving +each write, they carry the same trust as hand-editing the file. + +The heuristics are a net, not a parser: they err closed because a +false positive costs one rephrased note while a false negative leaks +a secret into plain-text files injected into every future prompt. +Known slack, accepted in that direction: a bare token with no +credential word or assignment around it (a GitHub PAT, a JWT, a PEM +body without its armor line) passes, as do homoglyph, full-width +digit, and invisible-character spellings, and so does a generic +"… key is …" shape with no compound kind (`the database key is …`). +The net also over-refuses on purpose: sharing a documentation link, +mentioning `/tmp/` caches, or naming a server trips the URL, path, +and digit rules, as do space-separated number lists (ports, +backoffs), IPv4 addresses with 11+ digits (`192.168.100.100`), and a +credential word anywhere near an unrelated colon (`rotate the github +token every 90 days: set a reminder`); long order or +epoch-millisecond numbers trip the digit rule. + ## Privacy and scope The store lives entirely on your machine. It is never uploaded to any