From dc95c70ada16877d592ff6b538bc9e6a07280e76 Mon Sep 17 00:00:00 2001 From: ajianaz Date: Thu, 22 Oct 2026 07:44:42 +0700 Subject: [PATCH] refactor(core): redaction flag policy as its own module (#1520) --- CHANGELOG.md | 1 + crates/uteke-core/src/lib.rs | 1 + crates/uteke-core/src/operations.rs | 37 +--- crates/uteke-core/src/privacy.rs | 70 +------- crates/uteke-core/src/redaction_flags.rs | 210 +++++++++++++++++++++++ 5 files changed, 217 insertions(+), 102 deletions(-) create mode 100644 crates/uteke-core/src/redaction_flags.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 74fb187e..9a9543f9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ ### Changed +- Internal: the redaction flag policy is its own module, #1520 (round 2 of the architecture deepening, #1523): `redaction_flags::resolve` decides which `redacted` / `redactions` flags a memory carries after an update as one pure function with a test per rule, and `update_memory_detailed` loses its inline five-arm `match`. The three flag helpers moved with it and are still reachable as `uteke_core::privacy::{with,without,carry}_redaction_flags`. No behaviour change. - Internal: errors are classified once, part of #1517 (round 2 of the architecture deepening, #1523): `Error::NotFound` and `Error::class()` (`BadRequest`, `NotFound`, `SecretDetected`, `Internal`), and one `core_error` mapping in the server instead of seven hand-written `Validation => 400, else 500` arms. The two handlers that matched `contains("not found")` (`POST /doc/move`, `DELETE /room/delete`) now read the class. HTTP statuses do not change: a missing memory on update or a missing room on rename still answer 400 until the contract with uteke-cloud moves them. The one visible difference is the text of the `POST /doc/move` 404, which no longer starts with `Validation error:`. - Internal: `ort` 2.0.0-rc.12/rc.13 to rc.14 (replaces the dependabot PR, which cannot build): rc.14 builds sessions from an explicit `Environment`, so the one process-wide environment is now created in `embed::ort_init` and handed to `Session::builder`. The bundled ONNX Runtime stays at 1.24.4 (`api-18`). Checked with the real model: `remember` writes the embedding and `recall` finds it. - Internal: privacy filter modes and flag helpers, part 2b-1a of #1464 (`uteke_core::privacy`): `PrivacyMode` (`redact`, `strict`, `off`), `PrivacyConfig::prepare`, `Error::SecretDetected` and the `metadata.redacted`/`redactions` helpers. Not wired to a write path yet. diff --git a/crates/uteke-core/src/lib.rs b/crates/uteke-core/src/lib.rs index cdb08c16..0d2421d1 100644 --- a/crates/uteke-core/src/lib.rs +++ b/crates/uteke-core/src/lib.rs @@ -42,6 +42,7 @@ pub mod privacy; mod privacy_field_tests; #[cfg(test)] mod privacy_write_tests; +mod redaction_flags; pub use forget_request::{ ForgetErrorKind, ForgetInput, ForgetPolicy, ForgetRequest, ForgetRequestError, FullIdForm, diff --git a/crates/uteke-core/src/operations.rs b/crates/uteke-core/src/operations.rs index 212e9761..4f533c88 100644 --- a/crates/uteke-core/src/operations.rs +++ b/crates/uteke-core/src/operations.rs @@ -2006,41 +2006,8 @@ impl crate::Uteke { .collect(); let masked_content: Option = prepared.map(|p| p.content); let content: Option<&str> = masked_content.as_deref(); - let flagged_metadata: Option = if redactions.is_empty() { - match (content, metadata) { - // Clean new content and no metadata from the caller: the flags - // a previous version left no longer describe the content, - // unless the edit kept a mask marker (a read-edit-write round - // trip of masked text). - (Some(c), None) if !c.contains("[REDACTED:") => { - crate::privacy::without_redaction_flags(&existing.metadata) - } - // A metadata-only update keeps the masked content, so it keeps - // the flags that describe it. - (None, Some(m)) => crate::privacy::carry_redaction_flags(m, &existing.metadata), - // New content and new metadata: the flags follow the content, - // never what the caller's metadata says about itself. - (Some(c), Some(m)) if c.contains("[REDACTED:") => { - let own = crate::privacy::without_redaction_flags(m); - crate::privacy::carry_redaction_flags( - own.as_ref().unwrap_or(m), - &existing.metadata, - ) - .or(own) - } - (Some(_), Some(m)) => crate::privacy::without_redaction_flags(m), - _ => None, - } - } else { - crate::privacy::with_redaction_flags( - Some( - metadata - .cloned() - .unwrap_or_else(|| existing.metadata.clone()), - ), - &redactions, - ) - }; + let flagged_metadata = + crate::redaction_flags::resolve(content, metadata, &existing.metadata, &redactions); let metadata: Option<&serde_json::Value> = flagged_metadata.as_ref().or(metadata); let now = chrono::Utc::now(); diff --git a/crates/uteke-core/src/privacy.rs b/crates/uteke-core/src/privacy.rs index 270a9178..848d556d 100644 --- a/crates/uteke-core/src/privacy.rs +++ b/crates/uteke-core/src/privacy.rs @@ -324,73 +324,9 @@ fn merge_counts(into: &mut BTreeMap<&'static str, usize>, from: &[RedactionCount } } -/// Merge `redacted: true` and `redactions` into `metadata`. `None` becomes a new -/// object; a metadata value that is not an object is left alone (there is -/// nowhere to merge), and the caller still learns of the redaction from the -/// returned counts. -pub fn with_redaction_flags( - metadata: Option, - redactions: &[RedactionCount], -) -> Option { - if redactions.is_empty() { - return metadata; - } - // A JSON `null` (an import row with `"metadata": null`) is no metadata. - let mut value = match metadata { - None | Some(serde_json::Value::Null) => serde_json::json!({}), - Some(v) => v, - }; - if let serde_json::Value::Object(map) = &mut value { - map.insert("redacted".into(), serde_json::json!(true)); - map.insert( - "redactions".into(), - serde_json::to_value(redactions).unwrap_or_default(), - ); - } - Some(value) -} - -/// `metadata` without the `redacted` and `redactions` keys, or `None` when it -/// has neither (nothing to change). The flags describe the CURRENT content, so -/// an update to clean content drops the ones an earlier version left; the -/// `redacted` timeline event keeps the history. -pub fn without_redaction_flags(metadata: &serde_json::Value) -> Option { - let serde_json::Value::Object(map) = metadata else { - return None; - }; - if !map.contains_key("redacted") && !map.contains_key("redactions") { - return None; - } - let mut map = map.clone(); - map.remove("redacted"); - map.remove("redactions"); - Some(serde_json::Value::Object(map)) -} - -/// `new` with the redaction flags of `old` carried over, for an update that -/// replaces the metadata but leaves the (still masked) content alone. `None` -/// when `old` has no flags, `new` is not an object, or `new` sets its own. -pub fn carry_redaction_flags( - new: &serde_json::Value, - old: &serde_json::Value, -) -> Option { - let (serde_json::Value::Object(new_map), serde_json::Value::Object(old_map)) = (new, old) - else { - return None; - }; - if new_map.contains_key("redacted") || new_map.contains_key("redactions") { - return None; - } - let mut merged = new_map.clone(); - let mut carried = false; - for key in ["redacted", "redactions"] { - if let Some(v) = old_map.get(key) { - merged.insert(key.to_string(), v.clone()); - carried = true; - } - } - carried.then_some(serde_json::Value::Object(merged)) -} +pub use crate::redaction_flags::{ + carry_redaction_flags, with_redaction_flags, without_redaction_flags, +}; /// A whole-secret rule: `mask` builds the replacement from the match, or /// returns `None` to leave this match alone (a placeholder, prose). diff --git a/crates/uteke-core/src/redaction_flags.rs b/crates/uteke-core/src/redaction_flags.rs new file mode 100644 index 00000000..0c63eea1 --- /dev/null +++ b/crates/uteke-core/src/redaction_flags.rs @@ -0,0 +1,210 @@ +//! Which `redacted` / `redactions` flags a memory carries after a write +//! (#1464). The flags describe the CURRENT content, so the rules depend on what +//! the edit touched: [`resolve`] is that policy as one pure function, testable +//! without a store. + +use crate::privacy::RedactionCount; + +/// What a masked piece of text looks like. An edit that keeps one of these +/// markers (a read-edit-write round trip of masked text) still holds masked +/// content. +const MASK_MARKER: &str = "[REDACTED:"; + +/// Merge `redacted: true` and `redactions` into `metadata`. `None` becomes a new +/// object; a metadata value that is not an object is left alone (there is +/// nowhere to merge), and the caller still learns of the redaction from the +/// returned counts. +pub fn with_redaction_flags( + metadata: Option, + redactions: &[RedactionCount], +) -> Option { + if redactions.is_empty() { + return metadata; + } + // A JSON `null` (an import row with `"metadata": null`) is no metadata. + let mut value = match metadata { + None | Some(serde_json::Value::Null) => serde_json::json!({}), + Some(v) => v, + }; + if let serde_json::Value::Object(map) = &mut value { + map.insert("redacted".into(), serde_json::json!(true)); + map.insert( + "redactions".into(), + serde_json::to_value(redactions).unwrap_or_default(), + ); + } + Some(value) +} + +/// `metadata` without the `redacted` and `redactions` keys, or `None` when it +/// has neither (nothing to change). The flags describe the CURRENT content, so +/// an update to clean content drops the ones an earlier version left; the +/// `redacted` timeline event keeps the history. +pub fn without_redaction_flags(metadata: &serde_json::Value) -> Option { + let serde_json::Value::Object(map) = metadata else { + return None; + }; + if !map.contains_key("redacted") && !map.contains_key("redactions") { + return None; + } + let mut map = map.clone(); + map.remove("redacted"); + map.remove("redactions"); + Some(serde_json::Value::Object(map)) +} + +/// `new` with the redaction flags of `old` carried over, for an update that +/// replaces the metadata but leaves the (still masked) content alone. `None` +/// when `old` has no flags, `new` is not an object, or `new` sets its own. +pub fn carry_redaction_flags( + new: &serde_json::Value, + old: &serde_json::Value, +) -> Option { + let (serde_json::Value::Object(new_map), serde_json::Value::Object(old_map)) = (new, old) + else { + return None; + }; + if new_map.contains_key("redacted") || new_map.contains_key("redactions") { + return None; + } + let mut merged = new_map.clone(); + let mut carried = false; + for key in ["redacted", "redactions"] { + if let Some(v) = old_map.get(key) { + merged.insert(key.to_string(), v.clone()); + carried = true; + } + } + carried.then_some(serde_json::Value::Object(merged)) +} + +/// The metadata to write for an update, when the flag policy changes what the +/// caller sent (`None`: write the caller's metadata, if any, as it is). +/// +/// - `content`: the new content after masking, when the update has one. +/// - `caller_metadata`: the metadata the caller supplied, after masking. +/// - `existing`: the metadata the memory has now. +/// - `redactions`: what the filter masked in this update (content, tags and +/// metadata together). +/// +/// With something masked, the flags are set on the caller's metadata (or the +/// current one). Otherwise: clean new content drops the old flags unless it +/// still holds a mask marker; a metadata-only update keeps the flags that +/// describe the unchanged content; new content with new metadata follows the +/// content, never what the caller's metadata says about itself. +pub(crate) fn resolve( + content: Option<&str>, + caller_metadata: Option<&serde_json::Value>, + existing: &serde_json::Value, + redactions: &[RedactionCount], +) -> Option { + if !redactions.is_empty() { + return with_redaction_flags( + Some(caller_metadata.cloned().unwrap_or_else(|| existing.clone())), + redactions, + ); + } + match (content, caller_metadata) { + // Clean new content and no metadata from the caller: the flags a + // previous version left no longer describe the content, unless the + // edit kept a mask marker. + (Some(c), None) if !c.contains(MASK_MARKER) => without_redaction_flags(existing), + // A metadata-only update keeps the masked content, so it keeps the + // flags that describe it. + (None, Some(m)) => carry_redaction_flags(m, existing), + // New content that still holds a marker, with new metadata: carry the + // existing flags, ignoring any the caller wrote. + (Some(c), Some(m)) if c.contains(MASK_MARKER) => { + let own = without_redaction_flags(m); + carry_redaction_flags(own.as_ref().unwrap_or(m), existing).or(own) + } + // New content and new metadata: the flags follow the content. + (Some(_), Some(m)) => without_redaction_flags(m), + _ => None, + } +} + +#[cfg(test)] +mod tests { + use super::*; + use serde_json::json; + + fn flagged() -> serde_json::Value { + json!({"k": "v", "redacted": true, "redactions": [{"kind": "jwt", "count": 1}]}) + } + + fn counts() -> Vec { + vec![RedactionCount { + kind: "github-token", + count: 2, + }] + } + + #[test] + fn something_masked_sets_the_flags_on_the_callers_metadata_or_the_current_one() { + let existing = json!({"old": 1}); + let out = resolve(Some("x"), Some(&json!({"new": 2})), &existing, &counts()).unwrap(); + assert_eq!(out["new"], 2); + assert!( + out.get("old").is_none(), + "the caller's metadata replaces the current one" + ); + assert_eq!(out["redacted"], true); + assert_eq!(out["redactions"][0]["count"], 2); + + let out = resolve(Some("x"), None, &existing, &counts()).unwrap(); + assert_eq!( + out["old"], 1, + "no caller metadata: the current one is flagged" + ); + assert_eq!(out["redacted"], true); + } + + #[test] + fn clean_new_content_drops_the_old_flags_but_keeps_the_rest() { + let out = resolve(Some("plain"), None, &flagged(), &[]).unwrap(); + assert_eq!(out, json!({"k": "v"})); + // Nothing to drop: leave the metadata alone. + assert_eq!(resolve(Some("plain"), None, &json!({"k": "v"}), &[]), None); + } + + #[test] + fn content_that_still_holds_a_mask_marker_keeps_the_flags() { + let c = "edited, still ghp_[REDACTED:github-token]"; + assert_eq!(resolve(Some(c), None, &flagged(), &[]), None); + } + + #[test] + fn a_metadata_only_update_carries_the_flags_over() { + let out = resolve(None, Some(&json!({"n": 1})), &flagged(), &[]).unwrap(); + assert_eq!(out["n"], 1); + assert_eq!(out["redacted"], true); + // The caller's own flags win: nothing is carried. + assert_eq!( + resolve(None, Some(&json!({"redacted": false})), &flagged(), &[]), + None + ); + // The memory has no flags: nothing to carry. + assert_eq!(resolve(None, Some(&json!({"n": 1})), &json!({}), &[]), None); + } + + #[test] + fn new_content_with_new_metadata_follows_the_content_not_the_callers_flags() { + let own = json!({"k": "v", "redacted": true}); + // Clean content: the caller's flag is dropped. + assert_eq!( + resolve(Some("plain"), Some(&own), &flagged(), &[]).unwrap(), + json!({"k": "v"}) + ); + // Masked content marker: the caller's flag is replaced by the existing ones. + let c = "still [REDACTED:jwt]"; + let out = resolve(Some(c), Some(&own), &flagged(), &[]).unwrap(); + assert_eq!(out["k"], "v"); + assert_eq!(out["redactions"][0]["kind"], "jwt"); + } + + #[test] + fn no_content_and_no_metadata_changes_nothing() { + assert_eq!(resolve(None, None, &flagged(), &[]), None); + } +}