Deferred from PR #18. This is the mechanism that made that review's findings possible, and it stays armed for all future code until it's removed.
The problem
Roughly 50 Core APIs declare string tenantId = "" — across CognitiveIndex, KnowledgeGraph, ClusterManager, LifecycleEngine, AccretionScanner, MemoryDiffusionKernel, NamespaceRegistry, SpreadingActivationService.
"" is not a sentinel. It is the legacy partition: a real, readable, writable dataset.
So "I forgot to pass the tenant" is well-typed, compiles without a warning, and degrades to cross-tenant legacy scope rather than to an error. The compiler cannot help, and neither can review — the call site looks fine.
Evidence this actually fires
Two clean instances, both found in the PR #18 review:
SynthesisEngine — commit 4eb5386 added string tenantId = "" to the signature and threaded it into GetAllInNamespace, then dropped it one frame down into ChunkMemories(entries, ns), where ListClusters(ns) / GetCluster(id) bound silently to the legacy tenant. Result: another tenant's cluster label reached the model prompt.
DiffusionKernelWarmupService — identical shape. GetBasis(ns) defaults tenantId to "", so no tenant partition is ever warmed even though the sweep enumerates tenant-only namespace names.
Same failure, twice, independently. That's a defaulting problem, not a discipline problem.
Approach
Drop the = "" default from the retrieval-side Core APIs and make every call site name its tenant explicitly. Large but mechanical, and the compiler finds every site.
PR #18 already applied this rule to everything it added: ChunkMemories, GetClusterMembershipsForEntry, and all six DebateSessionManager session members take a required tenantId. This issue extends it to the existing surface.
Fold in: converge the bare-id resolvers
Four sites still open-code the "resolve a bare id and check access" predicate with three different resolution semantics:
GraphTools.DenyIfCannotWriteLifecycleTools.ResolveAdminTools.CanReadEndpointCoreMemoryTools.CanReadEntryById
All four are ACL-correct today, which is why PR #18 left them alone. Converting them to EntryAccessResolver changes legacy resolution semantics from "first match wins per the global id→ns map" to "unique match among visible namespaces" — a behaviour delta on the most common deployment, taken for consistency rather than for a live bug. It belongs here, behind its own tests, not in a security PR.
Worth noting while doing it: the comment at CoreMemoryTools.cs:78-79 justifying its duplicate helper is factually wrong — CognitiveEntryInfo does carry Namespace (SearchResults.cs:33). That incorrect belief is what spawned the duplicates. Delete the comment along with the helper.
Acceptance criteria
Deferred from PR #18. This is the mechanism that made that review's findings possible, and it stays armed for all future code until it's removed.
The problem
Roughly 50 Core APIs declare
string tenantId = ""— acrossCognitiveIndex,KnowledgeGraph,ClusterManager,LifecycleEngine,AccretionScanner,MemoryDiffusionKernel,NamespaceRegistry,SpreadingActivationService.""is not a sentinel. It is the legacy partition: a real, readable, writable dataset.So "I forgot to pass the tenant" is well-typed, compiles without a warning, and degrades to cross-tenant legacy scope rather than to an error. The compiler cannot help, and neither can review — the call site looks fine.
Evidence this actually fires
Two clean instances, both found in the PR #18 review:
SynthesisEngine— commit 4eb5386 addedstring tenantId = ""to the signature and threaded it intoGetAllInNamespace, then dropped it one frame down intoChunkMemories(entries, ns), whereListClusters(ns)/GetCluster(id)bound silently to the legacy tenant. Result: another tenant's cluster label reached the model prompt.DiffusionKernelWarmupService— identical shape.GetBasis(ns)defaultstenantIdto"", so no tenant partition is ever warmed even though the sweep enumerates tenant-only namespace names.Same failure, twice, independently. That's a defaulting problem, not a discipline problem.
Approach
Drop the
= ""default from the retrieval-side Core APIs and make every call site name its tenant explicitly. Large but mechanical, and the compiler finds every site.PR #18 already applied this rule to everything it added:
ChunkMemories,GetClusterMembershipsForEntry, and all sixDebateSessionManagersession members take a requiredtenantId. This issue extends it to the existing surface.Fold in: converge the bare-id resolvers
Four sites still open-code the "resolve a bare id and check access" predicate with three different resolution semantics:
GraphTools.DenyIfCannotWriteLifecycleTools.ResolveAdminTools.CanReadEndpointCoreMemoryTools.CanReadEntryByIdAll four are ACL-correct today, which is why PR #18 left them alone. Converting them to
EntryAccessResolverchanges legacy resolution semantics from "first match wins per the global id→ns map" to "unique match among visible namespaces" — a behaviour delta on the most common deployment, taken for consistency rather than for a live bug. It belongs here, behind its own tests, not in a security PR.Worth noting while doing it: the comment at
CoreMemoryTools.cs:78-79justifying its duplicate helper is factually wrong —CognitiveEntryInfodoes carryNamespace(SearchResults.cs:33). That incorrect belief is what spawned the duplicates. Delete the comment along with the helper.Acceptance criteria
= ""removed from retrieval-side Core APIs; call sites name their tenant.EntryAccessResolver, with a test pinning the legacy resolution-semantics change.CoreMemoryTools.cs:78-79comment removed.