Skip to content

test(memory): avoid PII-like tool rule names #2680

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sanil-23 merged 2 commits into from
May 27, 2026
+5 −2
Merged

test(memory): avoid PII-like tool rule names #2680

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f2f398f
test(memory): avoid pii-like tool rule names
YOMXXX May 26, 2026
2a74c24
chore(ci): rerun after Rust Core Tests disk I/O flake
YOMXXX May 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions src/openhuman/memory/ops/tool_memory.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,8 @@ pub async fn tool_rules_json(params: ToolRuleListParams) -> Result<RpcOutcome<Va

#[cfg(test)]
mod tests {
use std::sync::atomic::{AtomicUsize, Ordering};

use super::*;
use crate::openhuman::memory_tools::ToolMemoryPriority;

Expand All @@ -183,8 +185,9 @@ mod tests {
}

fn unique_tool_name() -> String {
let short = &uuid::Uuid::new_v4().as_simple().to_string()[..12];
format!("toolmem{short}")
static NEXT_TOOL_ID: AtomicUsize = AtomicUsize::new(1);
let id = NEXT_TOOL_ID.fetch_add(1, Ordering::Relaxed);
format!("toolmem_test_{id}")
Copy link
Copy Markdown
Contributor

@sanil-23 sanil-23 May 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question (non-blocking): confirm the rejection path this fix targets.

The PR says the old toolmem<12-hex> names "look like a UUID to the PII detection rules" and get rejected before storage. There's no UUID detector in safety/pii.rs — the only boundary rejection on a tool name is has_likely_pii(namespace), where namespace = tool-{tool_name} (memory_tools/types.rs:144, enforced in memory_store/kv.rs:89 and unified/documents.rs:206).

has_likely_pii only fires when collect_strict_redactions matches, and every strict pattern is word-boundary-anchored (\b…). For tool-toolmem<hex>, the toolmem segment is 7 contiguous letters with no internal word boundary, so no strict pattern (RFC, PAN-IN, NINO, DNI, SSN, IBAN, NANP…) can anchor inside the hex run — I couldn't reproduce a namespace-level rejection of the old names.

If the real trigger was content-level redact_pii intermittently redacting a rare all-digit hex run inside the serialized tool_name (bare CPF/CC checksums apply there, unlike strict has_likely_pii), that would break prompt.rendered.contains(&primary_tool) rather than reject the write — so the "rejected before storage" framing would be slightly off.

Either way this is non-blocking: toolmem_test_<n> is unambiguously free of digit-run / hex / formatted-ID patterns and is strictly safer. Just want to confirm the actual mechanism so we know it covers the Windows path too.

}

#[tokio::test]
Expand Down
Loading