From bd4cf9fb7fc6d5f0979958c01825228f8db9b7d3 Mon Sep 17 00:00:00 2001 From: "K. Isom" Date: Mon, 28 Sep 2026 14:12:12 -0700 Subject: [PATCH] review: a key split by an invisible character is still a secret has_secret masked each character a character rule matches to a space, so one zero-width space inside a key cut it into two runs too short to match. The secret rules now also run on the text with those characters deleted, and the text is a secret if either matches. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/memoryd/src/honcho/decide.rs | 29 +++++++------ crates/memoryd/tests/honcho_decide_hidden.rs | 44 ++++++++++++++++++++ 2 files changed, 60 insertions(+), 13 deletions(-) create mode 100644 crates/memoryd/tests/honcho_decide_hidden.rs diff --git a/crates/memoryd/src/honcho/decide.rs b/crates/memoryd/src/honcho/decide.rs index 5c15b45..62e493c 100644 --- a/crates/memoryd/src/honcho/decide.rs +++ b/crates/memoryd/src/honcho/decide.rs @@ -78,21 +78,24 @@ pub fn peer_name(speaker: &str) -> String { /// Whether `text` holds a known-prefix secret, by `refuse`'s secret rules alone. `refuse` gives /// only the first rule a text breaks, and its character rules come first, so each character they -/// match is first replaced by a space: it is not a key character and not a letter, so a secret's -/// prefix and run are found as before, and a carriage return cannot hide a key. +/// match is taken out of the way, in two ways, and the text is a secret if either is: replaced by +/// a space, which is not a key character and not a letter, so a secret's prefix and run are found +/// as before and a carriage return cannot hide a key; and deleted, so that one inside a key cannot +/// split it into two runs too short to match. pub fn has_secret(text: &str) -> bool { let mut buf = [0u8; 4]; - let masked: String = text - .chars() - .map(|c| { - if refuse(c.encode_utf8(&mut buf)).is_some() { - ' ' - } else { - c - } - }) - .collect(); - refuse(&masked).is_some_and(|r| r.rule.starts_with("secret-")) + let mut masked = String::with_capacity(text.len()); + let mut deleted = String::with_capacity(text.len()); + for c in text.chars() { + if refuse(c.encode_utf8(&mut buf)).is_some() { + masked.push(' '); + } else { + masked.push(c); + deleted.push(c); + } + } + let secret = |t: &str| refuse(t).is_some_and(|r| r.rule.starts_with("secret-")); + secret(&masked) || secret(&deleted) } /// The turn's user text and its speaker, from its first record, the `User` one. diff --git a/crates/memoryd/tests/honcho_decide_hidden.rs b/crates/memoryd/tests/honcho_decide_hidden.rs new file mode 100644 index 0000000..7e2c28c --- /dev/null +++ b/crates/memoryd/tests/honcho_decide_hidden.rs @@ -0,0 +1,44 @@ +//! A key split by an invisible character is still a secret (review of M5c-1, finding 5): masking +//! the character to a space would cut the key into two short runs, so the text with it deleted is +//! checked too. Every key here is built from pieces so that no file holds one whole. + +use memoryd::honcho::decide::has_secret; + +fn mixed(n: usize) -> String { + "A1b2".chars().cycle().take(n).collect() +} + +#[test] +fn an_invisible_character_inside_a_key_does_not_hide_it() { + let github = ["gh", "p_"].concat(); + let sk = ["sk", "-proj-"].concat(); + let aws = ["AK", "IA"].concat(); + for mark in ["\u{200b}", "\u{7}", "\r", "\u{e000}", "\u{fdd0}", "\u{ad}"] { + let texts = [ + format!("{github}{}{mark}{}", mixed(18), mixed(18)), + format!("{github}{mark}{}", mixed(36)), + format!("{}{mark}{}{}", &github[..2], &github[2..], mixed(36)), + format!("{sk}{}{mark}{}", mixed(20), mixed(20)), + format!("{aws}IOSF{mark}ODNN7EXAMPLE"), + format!( + "my token is {github}{}{mark}{}, keep it", + mixed(30), + mixed(6) + ), + ]; + for text in &texts { + assert!(has_secret(text), "{text:?}"); + } + } +} + +#[test] +fn deleting_invisible_characters_finds_no_secret_in_plain_text() { + for text in [ + "a zero width\u{200b}space", + "gh\u{200b}p_ is how GitHub tokens start", + "soft\u{ad}hyphen and sk-learn", + ] { + assert!(!has_secret(text), "{text:?}"); + } +}