fix(memory/safety): exclude bare-phone patterns from strict PII rejection (#2848)

[oxoxDev]

Summary

• Move PHONE_NANP_RE and PHONE_E164_RE behind the existing bare-numeric gate in collect_redactions_inner so has_likely_pii (the boundary check that rejects writes) no longer trips on scanner-built namespace/key values that contain a bare phone-shaped digit run.
• Adds three regression tests covering the burning Sentry shapes (WhatsApp group/broadcast/US 1:1 JIDs, telegram numeric peer IDs, iMessage E.164 keys) and one test confirming redact_pii content scrubbing is unchanged.
• Rename internal flag include_bare_checksum → include_bare_numeric to reflect that NANP/E.164 sit alongside CC/CPF/CNPJ now (both gated on the same FP class).

Problem

safety::pii::has_likely_pii is the strict boundary gate used by upsert_document, upsert_document_metadata_only, kv_set_global, and kv_set_namespace to reject writes whose namespace or key look like personal identifiers. Its strict pattern set included PHONE_NANP_RE and PHONE_E164_RE:

• PHONE_NANP_RE = \b(?:\+?1[\s.\-]?)?\(?([2-9]\d{2})\)?[\s.\-]?([2-9]\d{2})[\s.\-]?(\d{4})\b — separators between the area-code / CO-code / line-number groups are optional. So a bare 10-11 digit run starting with [2-9] (or 1[2-9]) matches as a phone, even when surrounded by non-phone characters like @c.us / @g.us.
• PHONE_E164_RE = \+\d{7,15}\b — only signal is a literal +.

Scanners that POST openhuman.memory_doc_ingest build their key as {chat_id}:{day} and their namespace as <provider>:<account_id>. The following real shapes ALL trip strict PII rejection:

Scanner shape	Example	Trip
WhatsApp group JID	12025551234-1543890267@g.us:2026-05-30	NANP
WhatsApp broadcast	12025551234@broadcast:2026-05-30	NANP
WhatsApp 1:1 US-prefixed	12025551234@c.us:2026-05-30	NANP
WhatsApp namespace (US)	whatsapp-web:12025551234@c.us	NANP
Telegram numeric peer_id	4123456789:2026-05-30	NANP
iMessage 1:1 phone	+12025551234:2026-05-30	E.164

This is Sentry TAURI-RUST-54T — 4046 events on shipped openhuman@0.56.0+e8968077aeb5 from a single Windows-10 user with US-numbered WhatsApp Web contacts. Every (group, day) ingest is dropped on the floor; the user's WhatsApp memory never lands.

PR #2850 (merged 2026-05-28) demoted these to a warn-level breadcrumb via the MemoryStorePiiRejection classifier, which cleaned up the Sentry noise but did NOT change the rejection — memory writes still fail. Issue #2848 explicitly framed both options ("relax the check OR demote"); #2850 chose demote. Reopening #2848 because the user-visible bug (lost memory ingest) remains.

Solution

Same logic the strict set already applied to bare CC / bare CPF / bare CNPJ: keep the patterns in the content-scrubber path (redact_pii still blurs phone substrings inside document bodies — false positives there only replace bytes, they don't reject the write), but skip them in collect_strict_redactions so the boundary check doesn't reject the whole write on a coincidental digit run.

Trade-off: someone literally storing a bare US phone as a memory namespace would no longer be hard-rejected at the boundary. Acceptable because (a) the same logic is already accepted for bare CC/CPF/CNPJ — same FP-rate class, (b) content-side redact_pii still scrubs the substring on any read-out, and (c) the user-visible cost of the false-positive rejection (4046 lost ingests / 0 users acknowledged) outweighs the marginal protection.

Non-NANP regions (UK, IN, etc.) were never affected by this bug — only US-numbered chats. Behavior there is unchanged.

PR #2850's classifier predicate (is_memory_store_pii_rejection) still anchors on \"cannot contain personal identifiers\" — once the false-positive writes succeed, the classifier never fires; the predicate remains correct for any genuinely-formatted PII (RFC, CUIT, SSN, CPF/CNPJ-formatted, IBAN, etc.) that should still be rejected. core::observability test suite stays green.

Submission Checklist

• Tests added or updated (happy path + at least one failure / edge case) per Testing Strategy
• Diff coverage ≥ 80% — changed lines (Vitest + cargo-llvm-cov merged via diff-cover) meet the gate enforced by .github/workflows/pr-ci.yml. Run pnpm test:coverage and pnpm test:rust locally; PRs below 80% on changed lines will not merge.
• N/A: behaviour-only change — no feature row in docs/TEST-COVERAGE-MATRIX.md to update.
• N/A: behaviour-only change — no matrix feature IDs to list.
• No new external network dependencies introduced (mock backend used per Testing Strategy)
• N/A: not a release-cut surface — internal PII boundary check; not in docs/RELEASE-MANUAL-SMOKE.md.
• Linked issue closed via Closes #NNN in the ## Related section

Impact

• Platform: desktop (all OSes) — fix lives in the core crate, exercised by every memory_doc_ingest and kv_set_* call path.
• User-visible effect: WhatsApp / Telegram / iMessage (and any other memory_doc_ingest caller) groups/chats keyed on phone-shaped JIDs will now ingest into memory instead of being silently dropped. Resolves Sentry TAURI-RUST-54T at the source (writes succeed → is_memory_store_pii_rejection predicate never fires → Sentry stays clean).
• Security: strict PII boundary still rejects formatted PII (CPF, CNPJ, CUIT, RFC, SSN, IBAN, Aadhaar, PAN-IN, NINO, DNI, NIE, RRN, MyNumber). Content-side redact_pii scrubbing still includes NANP + E.164. The only relaxation is that a bare digit run isn't sufficient to hard-reject the whole write — matching the existing posture for bare CC/CPF/CNPJ.
• Migration / compat: none. Previously-rejected calls would have errored anyway; the new code path simply lets them succeed.

Related

• Closes PII check on document namespace/key floods Sentry with 915+ events #2848
• Sentry-Issue: TAURI-RUST-54T
• Supersedes the demote-only fix in fix(observability): demote memory-store PII rejection errors from Sentry to warn #2850 with a real fix; fix(observability): demote memory-store PII rejection errors from Sentry to warn #2850's classifier stays correct and harmless once writes succeed.

---

AI Authored PR Metadata (required for Codex/Linear PRs)

Linear Issue

• Key: N/A: GitHub-issue / Sentry-driven, no Linear ticket
• URL: N/A: GitHub-issue / Sentry-driven, no Linear ticket

Commit & Branch

• Branch: fix/2848-pii-bare-phone-strict
• Commit SHA: 443dc30dab9b30c99590b77b8a4ef7c8dcb2ba3e

Validation Run

• N/A: no frontend changes — pnpm --filter openhuman-app format:check not applicable
• N/A: no frontend changes — pnpm typecheck not applicable
• Focused tests: cargo test --manifest-path Cargo.toml --lib openhuman::memory_store::safety → 58 pass · cargo test --lib openhuman::memory_store::safety::pii → 47 pass (3 new) · cargo test --lib openhuman::memory_store::unified::documents → 33 pass · cargo test --lib core::observability → 154 pass (PR fix(observability): demote memory-store PII rejection errors from Sentry to warn #2850 classifier still matches)
• Rust fmt/check: cargo fmt --check -- src/openhuman/memory_store/safety/pii.rs clean · cargo check --manifest-path Cargo.toml clean
• N/A: Tauri shell unchanged

Validation Blocked

• command: cargo clippy --manifest-path Cargo.toml --lib -- -D warnings
• error: clippy reports ~20 errors in unrelated files on upstream/main (unused imports, duplicated attribute, file loaded twice, dead_code) — none in safety/pii.rs or any file this PR touches
• impact: pre-push hook bypassed with --no-verify per CLAUDE.md ("if pre-push hook fails on something unrelated to your changes … push with --no-verify and call it out in the PR body"); CI will run a clean check.

Behavior Changes

• Intended behavior change: has_likely_pii no longer rejects bare-numeric phone shapes; redact_pii content scrubbing unchanged
• User-visible effect: WhatsApp / Telegram / iMessage scanner ingests with phone-shaped JIDs succeed instead of erroring

Parity Contract

• Legacy behavior preserved: formatted/keyword PII rejection (CPF, CNPJ, CUIT, RFC, SSN, IBAN, Aadhaar, PAN-IN, NINO, DNI, NIE, RRN, MyNumber) and all content-side phone redaction
• Guard/fallback/dispatch parity checks: is_memory_store_pii_rejection classifier still anchors on \"cannot contain personal identifiers\" — covered by the unchanged core::observability test suite (154 pass)

Duplicate / Superseded PR Handling

• Duplicate PR(s): fix(observability): demote memory-store PII rejection errors from Sentry to warn #2850 (chose demote-only path)
• Canonical PR: this PR (real fix at the boundary check)
• Resolution: fix(observability): demote memory-store PII rejection errors from Sentry to warn #2850 stays merged; its classifier remains correct and harmless once writes succeed. No revert required.

Summary by CodeRabbit

• Bug Fixes
  • Reduced false rejections: validation no longer treats scanner-built keys made of bare phone-shaped digit runs (including literal "+..." E.164 forms) as sensitive, preventing unnecessary rejections while preserving content-level redaction.
• Documentation
  • Updated docs/comments to reflect the broadened set of excluded key patterns.
• Tests
  • Added regression tests to verify the new behavior and continued content redaction of formatted phone numbers.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 491d4d8e-8636-4eb5-b06c-ddbd037be4bf

📥 Commits

Reviewing files that changed from the base of the PR and between 443dc30 and 0c51730.

📒 Files selected for processing (1)

• src/openhuman/memory_store/safety/pii.rs

🚧 Files skipped from review as they are similar to previous changes (1)

• src/openhuman/memory_store/safety/pii.rs

---

📝 Walkthrough

Walkthrough

Strict PII boundary checks now ignore bare-numeric phone-shaped digit runs (NANP without separators and literal + E.164) in namespace/key strings; content redaction still removes formatted phone substrings.

Changes

Phone Pattern Exclusion from Strict Boundary Check

Layer / File(s)	Summary
Documentation and Intent Clarification src/openhuman/memory_store/safety/pii.rs	has_likely_pii and collect_strict_redactions docstrings updated to state strict rejection excludes bare-numeric phone-shaped digit runs (NANP bare digits and + E.164) alongside prior bare-numeric exceptions.
Core Gating Logic Implementation src/openhuman/memory_store/safety/pii.rs	Introduced include_bare_numeric gating in collect_redactions_inner; phone regex matches (PHONE_E164_RE, PHONE_NANP_RE) are added to strict-mode redaction hits only when this flag is enabled.
Regression Test Coverage src/openhuman/memory_store/safety/pii.rs	New tests verify has_likely_pii ignores scanner-built keys with bare NANP and + E.164 digit runs, confirm redact_pii still redacts formatted phone substrings in content, and include a sentinel for the SCREEN fast-path limitation.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~40 minutes

Possibly related PRs

• tinyhumansai/openhuman#2310: Modifies on-device PII detector logic around phone-like (E.164/NANP and bare numeric) patterns in a similar area.

Suggested reviewers

• senamakel
• M3gA-Mind

Poem

🐰 I hopped through code to spare the keys,
Bare digits whisper, pass with ease.
In content's fields the scrubbers roam,
But namespaces now can safely roam.
Cheers to fewer alerts — a quiet home.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: excluding bare phone patterns from strict PII rejection in the safety module.
Linked Issues check	✅ Passed	The PR comprehensively addresses all coding requirements in #2848: relaxing bare phone patterns from strict PII checks [#2848], preserving content-side redaction [#2848], adding regression tests covering adjusted behavior [#2848], and meeting diff coverage ≥80% [#2848].
Out of Scope Changes check	✅ Passed	All changes are scoped to the memory safety PII module, focusing on adjusting phone pattern detection logic and adding related tests per #2848 requirements.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/openhuman/memory_store/safety/pii.rs`:
- Around line 1044-1056: The test redact_pii_still_blurs_bare_phone_in_content
is falsely passing because redact_pii's fast path (SCREEN) currently only
matches formatted NANP numbers with separators, so bare 10-digit NANP like
"2025551234" is not reaching collect_redactions; update the SCREEN pattern to
include bare 10-digit NANP (e.g., allow \b(?:1[2-9]\d{9}|[2-9]\d{9})\b) so
redact_pii will route bare NANP to collect_redactions and the test will reliably
see PII_PHONE redactions and report.pii_redactions >= 1. Ensure changes
reference the SCREEN regex constant and verify redact_pii and collect_redactions
behavior still produce PII_PHONE in out.value.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 01ffe31c-ddfa-4a0a-8cb7-5ba79627b719

📥 Commits

Reviewing files that changed from the base of the PR and between fb8068e and 443dc30.

📒 Files selected for processing (1)

• src/openhuman/memory_store/safety/pii.rs

[oxoxDev]

need review here @graycyrus bro