Description
Follow-up from #6553 (fixed in PR #6615). That PR added a MAX_SEARCH_LIMIT clamp at the wrapper-method level — EmbeddingStore::search/search_collection, EmbeddingRegistry::search_raw, and ReasoningMemory::retrieve_by_embedding all now clamp limit/top_k before forwarding to the underlying store.
The impl-critic review of that PR flagged (as a non-blocking minor, explicitly deferred) that the clamp sits on these wrapper methods, not on the VectorStore trait's own search method — the actual choke point one layer down. Any caller that reaches a VectorStore implementor directly (bypassing the wrapper methods) still has no clamp.
Impact
Low — P3/P4. No currently known live instance of a caller bypassing the wrappers to call VectorStore::search directly with an unclamped, externally-influenced value. This is the same class of defense-in-depth gap as #6553's original findings: not exploitable today, but a footgun for a future call site.
Reproduction Steps
- Inspect the
VectorStore trait definition and its search method signature in zeph-memory.
- Inspect all implementors and all call sites that invoke
VectorStore::search directly (not through EmbeddingStore/EmbeddingRegistry/ReasoningMemory).
- Observe: no clamp exists at the trait-method level itself.
Expected Behavior
The MAX_SEARCH_LIMIT safety guarantee should hold regardless of which layer a caller enters through, not just the three wrapper types fixed in #6553.
Actual Behavior
Clamp only exists in the wrapper methods; the trait method itself is unclamped.
Remediation Considered
Moving the clamp into the trait's default method body (if using a default impl) or into a shared helper each implementor's search calls — was out of scope for #6553 since it requires touching the trait signature/contract and every implementor, which is a larger change than a defense-in-depth hardening PR should carry in one pass.
Environment
Logs / Evidence
See impl-critic finding M1 in PR #6615 review discussion.
Description
Follow-up from #6553 (fixed in PR #6615). That PR added a
MAX_SEARCH_LIMITclamp at the wrapper-method level —EmbeddingStore::search/search_collection,EmbeddingRegistry::search_raw, andReasoningMemory::retrieve_by_embeddingall now clamplimit/top_kbefore forwarding to the underlying store.The impl-critic review of that PR flagged (as a non-blocking minor, explicitly deferred) that the clamp sits on these wrapper methods, not on the
VectorStoretrait's ownsearchmethod — the actual choke point one layer down. Any caller that reaches aVectorStoreimplementor directly (bypassing the wrapper methods) still has no clamp.Impact
Low — P3/P4. No currently known live instance of a caller bypassing the wrappers to call
VectorStore::searchdirectly with an unclamped, externally-influenced value. This is the same class of defense-in-depth gap as #6553's original findings: not exploitable today, but a footgun for a future call site.Reproduction Steps
VectorStoretrait definition and itssearchmethod signature inzeph-memory.VectorStore::searchdirectly (not throughEmbeddingStore/EmbeddingRegistry/ReasoningMemory).Expected Behavior
The
MAX_SEARCH_LIMITsafety guarantee should hold regardless of which layer a caller enters through, not just the three wrapper types fixed in #6553.Actual Behavior
Clamp only exists in the wrapper methods; the trait method itself is unclamped.
Remediation Considered
Moving the clamp into the trait's default method body (if using a default impl) or into a shared helper each implementor's
searchcalls — was out of scope for #6553 since it requires touching the trait signature/contract and every implementor, which is a larger change than a defense-in-depth hardening PR should carry in one pass.Environment
Logs / Evidence
See impl-critic finding M1 in PR #6615 review discussion.