Skip to content

refactor(safety): delegate store::safety to the shared tinymemory-safety crate - #178

Merged
senamakel merged 3 commits into
mainfrom
w4-memory-rest
Sep 30, 2026
Merged

senamakel merged 3 commits into
mainfrom
w4-memory-rest

Conversation

@senamakel

@senamakel senamakel commented Sep 30, 2026 •

Copy link
Copy Markdown
Member

Summary

memory::store::safety (secret and PII scrubbing, ~1.5k lines with tests) moves to the new shared tinymemory-safety crate in tinyhumansai/tinymemory (tinyhumansai/tinymemory#174, branch w4-memory-rest). TinyCortex keeps its public safety::* paths and pins its one policy choice.

  • safety::{has_likely_pii, has_likely_secret, has_likely_email, SanitizationReport, Sanitized} are re-exports.
  • sanitize_text, sanitize_json and pii::redact_pii are thin wrappers that pass Policy::corroborated(), so behaviour is unchanged: a bare Luhn-valid digit run is only redacted as a card when it has a real network IIN or a nearby card keyword (opencompany#1201).
  • The 1.5k lines of pattern, checksum, prefilter and normalisation code and their tests moved with the crate.
  • New dependency: tinymemory-safety, by git rev like tinymemory-api ([patch."https://github.com/tinyhumansai/tinymemory"] in a host that vendors it).

Order

Stacks on tinymemory#174 (w4-memory-rest): the git rev in Cargo.toml names a commit that adds the crate. Merge the tinymemory PR first, then re-point the rev at the merged commit if the squash changes it.

Testing

cargo fmt --all -- --check, cargo clippy --all-targets -- -D warnings, cargo test: 1203 lib tests pass (3 new pin the engine policy and the re-exports).

Summary by CodeRabbit

  • Bug Fixes
    • Reduced false-positive credit-card redactions: Luhn-valid numbers are treated as cards only when supported by a recognized card prefix or nearby card-related wording.
    • Preserved 13-digit timestamps during PII, text, and JSON sanitization.
  • Improvements
    • Text and JSON sanitization detect and redact sensitive content, including real payment card numbers and API keys.
    • PII handling rejects formatted national IDs at the write boundary while scrubbing phone numbers and email addresses without rejecting every write that mentions them.

…ety crate

The scrubbers moved to tinymemory-safety (tinymemory w4-memory-rest), which
the OpenHuman host and tinymemory-core share. store::safety keeps its public
paths and pins the engine policy (corroborated bare-card gate, unchanged
behaviour).

Co-authored-by: Medulla <medulla@tinyhumans.ai>
@tinysweeper

tinysweeper Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Tiny Sweeper review

Tiny Sweeper reviewed this change across 6 lane(s) and found 0 active actionable finding(s). Detailed lane evidence and any incomplete work are listed below.

State: Incomplete
Priority: none
Reviewed head: 6958497ed313
Updated: 1790803552 (Unix time)

Review snapshot

Change surface Files Review signal Count
Production 5 Active findings 0
Tests 3 Noted findings 0
Documentation 0 Resolved findings 0
Configuration 1 Pending checks/questions 7

Completeness: Incomplete
Test assessment: No supported feature-to-test mapping was available; this does not mean tests are absent or passed.

What changed

The review could not produce a supported behavioral summary; inspect the cited changed surface and lane details below.

Features

None identified with supported citations.

Tests

No supported feature-to-test mapping was produced. Test execution is not inferred.

  • Unreviewed: tinysweeper/tests

Findings

No active actionable findings.

Could not review: src/memory/store/safety/mod.rs, src/memory/store/safety/safety_tests.rs, tinysweeper/description, tinysweeper/e2e, tinysweeper/tests

Before merge

  • Complete the critique review for src/memory/store/safety/mod.rs, src/memory/store/safety/safety_tests.rs.
  • Complete the security review for src/memory/store/safety/mod.rs, src/memory/store/safety/safety_tests.rs.
  • Complete the tests review for tinysweeper/tests.
  • Complete the description review for tinysweeper/description.
  • Complete the e2e review for tinysweeper/e2e.

How this fits together

flowchart LR
  n0["sanitize_text"]:::impacted
  n1["has_goal_pii"]:::impacted
  n2["set_global"]:::impacted
  n3["set_namespace"]:::impacted
  n4["pii"]:::impacted
  n5["redact_pii"]:::impacted
  n1 -->|calls| n0
  n2 -->|uses| n4
  n2 -->|calls| n5
  n3 -->|uses| n4
  n3 -->|calls| n5
  n5 -->|uses| n4
  classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
  classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
  classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
  classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff
Loading
Agent review details

critique

  • Conclusion: Neutral
  • Scope reviewed: incomplete; unanswered: src/memory/store/safety/mod.rs, src/memory/store/safety/safety_tests.rs
  • Lane summary: Reviewed 0 files; 0 findings. 2 files could not be reviewed: src/memory/store/safety/mod.rs, src/memory/store/safety/safety_tests.rs.

security

  • Conclusion: Neutral
  • Scope reviewed: incomplete; unanswered: src/memory/store/safety/mod.rs, src/memory/store/safety/safety_tests.rs
  • Lane summary: Reviewed 0 files; 0 findings. 2 files could not be reviewed: src/memory/store/safety/mod.rs, src/memory/store/safety/safety_tests.rs.

tests

  • Conclusion: Neutral
  • Scope reviewed: incomplete; unanswered: tinysweeper/tests
  • Lane summary: No reviewer could be consulted.

commits

  • Conclusion: Neutral
  • Scope reviewed: all assigned evidence
  • Lane summary: Nothing sensitive found in what this pull request commits.

description

  • Conclusion: Neutral
  • Scope reviewed: incomplete; unanswered: tinysweeper/description
  • Lane summary: No reviewer could be consulted.

e2e

  • Conclusion: Success
  • Scope reviewed: incomplete; unanswered: tinysweeper/e2e
  • Lane summary: No reviewer could be consulted; only the job states below are reported. _The code index is behind this pull request (indexed at `27183385d6eb`), so retrieved context may be out of date._ _3 memory call(s) failed (model: cortex: v1/answer answered 502 Bad Gateway), so this review saw part of what the engine holds._
Evidence and run details
  • Models: ladder/vectors
  • Spend: $0.000012
  • Tokens: 0 input · 0 output · 0 cached · 1184 embedding
Head State Pass summary
27183385d6eb incomplete 0 active finding(s), 0 resolved finding(s) (at 1790790129)
6958497ed313 incomplete 0 active finding(s), 0 resolved finding(s) (at 1790803552)

tinysweeper 0.1.0

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-30T17:45:26.916310Z 2718338 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@coderabbitai

coderabbitai Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 94ee1a4b-147a-4d53-8c23-ff05b26884b8

📥 Commits

Reviewing files that changed from the base of the PR and between 2718338 and 6958497.

📒 Files selected for processing (1)
  • src/memory/store/safety/mod.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/memory/store/safety/mod.rs

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.


📝 Walkthrough

Walkthrough

The engine adds the pinned tinymemory-safety dependency and delegates text sanitization, JSON sanitization, and PII redaction to it. The engine retains safety module paths through re-exports and wrappers, and uses a corroborated policy.

Changes

Shared safety integration

Layer / File(s) Summary
Shared safety dependency and policy
Cargo.toml, src/memory/store/safety/mod.rs
Adds the pinned tinymemory-safety Git dependency and documents vendored-host patch configuration. The safety module re-exports shared types and detection helpers and defines ENGINE_POLICY as Policy::corroborated().
Sanitization and PII delegation
src/memory/store/safety/mod.rs, src/memory/store/safety/pii*, src/memory/store/safety/pii/*, src/memory/store/safety/safety_tests.rs
Text and JSON sanitization and PII redaction delegate to the shared implementation. The local PII implementation and its tests are removed. Updated safety tests cover timestamp preservation, card and key redaction, and predicate paths.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Refactor

Merge Risk: ⚪ Minimal · up to 69584

No actionable merge-blocking defect was identified. The shared safety integration appears mergeable subject to normal checks, though dependency behavior was not independently verified.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 27183

The wrappers retain a fixed policy and sanitize before storage or evidence construction. No introduced security regression was established, but the shared library’s exact behavior could not be independently compared with the replaced implementation.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • observed — The evidenced downstream scope includes global KV records, namespace-scoped KV records and persona evidence excerpts and IDs. Caller-provided keys, namespaces, JSON values and raw excerpts reach these safety controls before persistence or evidence construction; deployment-wide and tenant-level reachability is not established by these excerpts.

Trust Boundaries and Controls

  • observed — Sanitization enforcement now depends on code from another repository, while policy selection remains engine-owned. The wrappers consistently supply the private corroborated policy and expose no caller-controlled policy parameter. The inspected host boundary adds no credential, network or tool-authority operation; upstream implementation behavior remains outside the hydrated source coverage.

Hardening Proposals

  • proposed — Compare the exact pinned implementation with the base contract for secret rejection, sensitive-key replacement, depth limits, PII replacement strings, predicate/redactor agreement and repeated sanitization. Include persisted KV keys and persona evidence IDs in compatibility checks, and repeat them for any effective vendored override.
🚥 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 clearly and concisely describes the primary change: delegating store safety logic to the shared tinymemory-safety crate.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 2 files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


A rabbit checks the digits in a row,
Then asks for proof before cards can show.
Shared safety keeps the text in line,
While timestamps pass without a sign.
Hop, hop—the wrappers guide the flow!

Comment @coderabbitai help to get the list of available commands.

@tinysweeper tinysweeper Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

tinysweeper found nothing blocking, but could not review everything, so this is not an approval: Cargo.toml, src/memory/store/safety/mod.rs, src/memory/store/safety/safety_tests.rs, tinysweeper/description, tinysweeper/e2e, tinysweeper/tests.

$0.0000 · 0 in / 0 out · 1,184 embedded · ladder/vectors

@tinysweeper tinysweeper Bot added the priority: p3 Whenever. Cosmetic, a nicety, or a cleanup with no user visible effect. label Sep 30, 2026
The documentation comment for the write-rejection boundary now uses the full module path `pii::has_likely_pii` instead of the bare function name, making the cross-reference unambiguous when the module is not imported at the call site.

Auto-committed-on: dragonfly
The doc comment for the safety module now links to `has_likely_pii` using its full module path instead of a relative reference, ensuring the link resolves correctly in generated documentation.

Auto-committed-on: dragonfly

@tinysweeper tinysweeper Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

tinysweeper found nothing blocking, but could not review everything, so this is not an approval: src/memory/store/safety/mod.rs, src/memory/store/safety/safety_tests.rs, tinysweeper/description, tinysweeper/e2e, tinysweeper/tests.

$0.0000 · 0 in / 0 out · 1,184 embedded · ladder/vectors

@senamakel
senamakel merged commit 1f309ab into main Sep 30, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority: p3 Whenever. Cosmetic, a nicety, or a cleanup with no user visible effect.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant