You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Three related gaps surfaced while building the section API (#114) and its namespace round-trip fix (#115). All three were found, verified and deliberately left out of scope; this issue records them with evidence so they do not live only in resolved PR threads.
They are listed in dependency order: (1) is the root cause, and fixing it very likely dissolves (2) and a large amount of machinery added in #115.
1. sanitize_namespace is not injective
UnifiedMemory::sanitize_namespace (crates/tinymemory-core/src/store/namespace_store/init.rs:417) maps every character outside [A-Za-z0-9\-_/] to _. That mapping is lossy and not invertible, so distinct logical namespaces collapse onto one physical address:
a:b_c -> a_b_c
a_b:c -> a_b_c
Both become the same row address and the same on-disk directory. Every operation — get, list, forget, recall, clear_namespace, graph relations — treats them as one namespace. clear_namespace on either deletes both. Enumeration reports only one of them.
This is pre-existing and predates the section API.#115 did not introduce it and does not fix it; it only made it visible, because the section surface is the first consumer that depends on a namespace round-tripping through namespaces().
Why it matters more now
#115 added a logical_namespace shadow column so enumeration reports the delimiter-preserving name. During review, six successive rounds of findings were all the same shape: some access path does not filter by logical namespace, so aliasing namespaces leak into each other. Each fix was correct; the set never closed, because we were enumerating the store's access paths by review round rather than by design. That work was reverted wholesale (see the "Out of scope" section of docs/specs/memory-section-api.md), restoring the pre-existing merge behaviour rather than half-isolating it.
The fix worth considering
Make the physical address reversible rather than shadowing it — a percent-style escape or a reserved sequence, so decode(address) == logical name.
That would mean:
no logical_namespace column and no migration;
no per-path logical filtering, so the entire class of leak disappears rather than being patched path by path;
aliasing becomes impossible by construction, so clear_namespace cannot delete another namespace's rows.
Constraints any fix must respect — these are why the obvious "just allow :" is wrong:
The sanitized value becomes a real filesystem directory via namespace_dir(), and clear_namespace calls remove_dir_all on it. The narrow allow-list is a path-traversal defence; see its own comment about a leading /.
: is illegal in a Windows filename and denotes an NTFS alternate data stream.
The same funnel performs the #5164 PII redaction that keeps a national ID from becoming a storage address.
Cost: a new addressing scheme plus a migration that renames existing rows and namespace directories.
2. A PII-redacted custom section prefix does not round-trip
canonical_identifier (crates/tinymemory-core/src/store/safety/mod.rs:46) rewrites a PII-bearing identifier to a [REDACTED_PII_*] placeholder, and canonical_logical_namespace (safety/mod.rs:82) substitutes _ for the brackets. The result is uppercase.
is_valid_section (crates/tinymemory-bus/src/namespace.rs:413) accepts only is_ascii_lowercase(), digits, - and _:
fnis_valid_section(prefix:&str) -> bool{
!prefix.is_empty()
&& prefix
.chars().all(|c| c.is_ascii_lowercase() || c.is_ascii_digit() || c == '-' || c == '_')}
So when a custom section prefix trips the PII gate, the reported logical namespace parses back as unsectioned, and the section enumeration #115 exists to provide silently does not apply to it. Known-section prefixes (conversation, document, learning, …) are unaffected — they are lowercase literals and never trip the PII gate.
This was raised in review on #115 and declined for a good reason: every candidate fix either breaks address-equivalence between the logical name and the stored address, or modifies canonical_identifier / sanitize_namespace, which are used store-wide well beyond namespaces. It needs its own design rather than a patch.
Note this is a narrower case of (1): a reversible address would likely remove it too, since the logical name would no longer be reconstructed through a lossy redaction step.
3. namespace: None on MemoryRecall::recall is unspecified in practice
The contract documents it as falling back to GLOBAL_NAMESPACE (crates/tinymemory-bus/src/recall.rs:56 and :108), and the embedded engine implements exactly that. The reference driver does not — it treats None as all namespaces:
assert_recall_respects_limit_and_namespace (crates/tinymemory-conformance/src/suite/mod.rs:435) only ever exercises the Some(namespace) case, so nothing catches the divergence.
This is a genuine interchangeability hole — precisely the claim tinymemory-conformance exists to defend. Two drivers that both pass the suite answer the same call differently.
It also has a practical consequence already recorded in docs/specs/memory-section-api.md: it is why SectionRecall::across_section is a fan-out over namespaces() rather than one unfiltered call. An implementation built on namespace: None would pass its tests against the reference driver and return nothing in production.
Fix: decide the semantics, state them in the contract, and add a suite assertion that pins them. Small change, but it is a contract decision rather than a bug fix, which is why it was not folded into #115.
Three related gaps surfaced while building the section API (#114) and its namespace round-trip fix (#115). All three were found, verified and deliberately left out of scope; this issue records them with evidence so they do not live only in resolved PR threads.
They are listed in dependency order: (1) is the root cause, and fixing it very likely dissolves (2) and a large amount of machinery added in #115.
1.
sanitize_namespaceis not injectiveUnifiedMemory::sanitize_namespace(crates/tinymemory-core/src/store/namespace_store/init.rs:417) maps every character outside[A-Za-z0-9\-_/]to_. That mapping is lossy and not invertible, so distinct logical namespaces collapse onto one physical address:Both become the same row address and the same on-disk directory. Every operation —
get,list,forget,recall,clear_namespace, graph relations — treats them as one namespace.clear_namespaceon either deletes both. Enumeration reports only one of them.This is pre-existing and predates the section API. #115 did not introduce it and does not fix it; it only made it visible, because the section surface is the first consumer that depends on a namespace round-tripping through
namespaces().Why it matters more now
#115 added a
logical_namespaceshadow column so enumeration reports the delimiter-preserving name. During review, six successive rounds of findings were all the same shape: some access path does not filter by logical namespace, so aliasing namespaces leak into each other. Each fix was correct; the set never closed, because we were enumerating the store's access paths by review round rather than by design. That work was reverted wholesale (see the "Out of scope" section ofdocs/specs/memory-section-api.md), restoring the pre-existing merge behaviour rather than half-isolating it.The fix worth considering
Make the physical address reversible rather than shadowing it — a percent-style escape or a reserved sequence, so
decode(address) == logical name.That would mean:
logical_namespacecolumn and no migration;clear_namespacecannot delete another namespace's rows.Constraints any fix must respect — these are why the obvious "just allow
:" is wrong:namespace_dir(), andclear_namespacecallsremove_dir_allon it. The narrow allow-list is a path-traversal defence; see its own comment about a leading/.:is illegal in a Windows filename and denotes an NTFS alternate data stream.Cost: a new addressing scheme plus a migration that renames existing rows and namespace directories.
2. A PII-redacted custom section prefix does not round-trip
canonical_identifier(crates/tinymemory-core/src/store/safety/mod.rs:46) rewrites a PII-bearing identifier to a[REDACTED_PII_*]placeholder, andcanonical_logical_namespace(safety/mod.rs:82) substitutes_for the brackets. The result is uppercase.is_valid_section(crates/tinymemory-bus/src/namespace.rs:413) accepts onlyis_ascii_lowercase(), digits,-and_:So when a custom section prefix trips the PII gate, the reported logical namespace parses back as unsectioned, and the section enumeration #115 exists to provide silently does not apply to it. Known-section prefixes (
conversation,document,learning, …) are unaffected — they are lowercase literals and never trip the PII gate.This was raised in review on #115 and declined for a good reason: every candidate fix either breaks address-equivalence between the logical name and the stored address, or modifies
canonical_identifier/sanitize_namespace, which are used store-wide well beyond namespaces. It needs its own design rather than a patch.Note this is a narrower case of (1): a reversible address would likely remove it too, since the logical name would no longer be reconstructed through a lossy redaction step.
3.
namespace: NoneonMemoryRecall::recallis unspecified in practiceThe contract documents it as falling back to
GLOBAL_NAMESPACE(crates/tinymemory-bus/src/recall.rs:56and:108), and the embedded engine implements exactly that. The reference driver does not — it treatsNoneas all namespaces:assert_recall_respects_limit_and_namespace(crates/tinymemory-conformance/src/suite/mod.rs:435) only ever exercises theSome(namespace)case, so nothing catches the divergence.This is a genuine interchangeability hole — precisely the claim
tinymemory-conformanceexists to defend. Two drivers that both pass the suite answer the same call differently.It also has a practical consequence already recorded in
docs/specs/memory-section-api.md: it is whySectionRecall::across_sectionis a fan-out overnamespaces()rather than one unfiltered call. An implementation built onnamespace: Nonewould pass its tests against the reference driver and return nothing in production.Fix: decide the semantics, state them in the contract, and add a suite assertion that pins them. Small change, but it is a contract decision rather than a bug fix, which is why it was not folded into #115.
Suggested order
logical_namespacecolumn added in Preserve the section delimiter in namespace enumeration #115 can be removed entirely.References
docs/specs/memory-section-api.md— §4 and its "Out of scope" section record (1) and (3); (2) is recorded nowhere until this issue