security: fail closed on tenant-scoped memory reads (bounty #1852)

[laurentketterle-hub]

Summary

Fixes a cross-tenant data isolation failure in the memanto core read service, submitted for the Memanto Security Challenge (#1852).

MemoryReadService exposes two entry points whose default behaviour is to drop tenant scope and fan out across every namespace on the server's Moorcheh account when an agent_id is missing or empty:

• _get_search_namespaces(agent_id=None) -> list_namespaces() -> searches all memanto_agent_* namespaces (used by search_memories / temporal recall / recent recall).
• generate_answer(query, agent_id=None) -> namespaces[0] -> answers from the first namespace in the account list.

Both agent_id parameters default to None, so a single omitted argument (the default value) silently escalates an agent-scoped read into a read of every other tenant's memories — the exact "cross-tenant data leak" class this challenge asks us to find.

Threat model fit

• Tenant Isolation & Cross-Account Leaks (in scope): a caller that omits agent_id can retrieve memories belonging to other agents/tenants sharing the same server account.

Reproduction (PoC)

from unittest.mock import MagicMock
from memanto.app.services.memory_read_service import MemoryReadService

client = MagicMock()
client.namespaces.list.return_value = {
    "namespaces": [
        {"namespace_name": "memanto_agent_alice"},
        {"namespace_name": "memanto_agent_bob"},
    ]
}
client.similarity_search.query.side_effect = (
    lambda query=None, namespaces=None, **kw:
        {"results": [{"id": f"mem_of_{ns}", "text": f"secret of {ns}"}
                      for ns in (namespaces or [])]}
)
client.answer.generate.side_effect = (
    lambda namespace=None, **kw: {"answer": f"from {namespace}", "sources": []}
)

svc = MemoryReadService(client)

# Before this patch: returns BOTH tenants' memories
print(svc.search_memories(query="anything", agent_id=None, limit=10))
# Before this patch: answers from "memanto_agent_alice" (first tenant)
print(svc.generate_answer(query="anything", agent_id=None))

Fix

Fail closed instead of fail open. Both paths now require an explicit, non-empty agent_id and raise MemoryError otherwise, so a read can never silently expand beyond the caller's own namespace.

Changes

• memanto/app/services/memory_read_service.py: _get_search_namespaces and generate_answer now require an agent_id (no list_namespaces() / namespaces[0] fallback).
• tests/test_memory_read_tenant_isolation.py: regression tests covering the all-namespaces fan-out and the first-namespace answer fallback.

Verification

• pytest tests/test_memory_read_tenant_isolation.py -> 4 passed
• pytest tests/ (excluding network e2e/api) -> all green, no regressions

Closes the data-isolation gap described in #1852.

Summary by CodeRabbit

• Bug Fixes

  • Memory searches and answer generation now require a valid agent identifier.
  • Requests are restricted to the requesting agent’s namespace, preventing cross-tenant data access.
  • Removed fallback behavior that could search unrelated namespaces when an agent identifier was missing.

• Tests

  • Added coverage for missing identifiers, namespace isolation, and prevention of cross-tenant searches.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: d9351639-c433-43d2-b5d1-23d6eb99e2f1

📥 Commits

Reviewing files that changed from the base of the PR and between 48da99c and cdd42db.

📒 Files selected for processing (2)

• memanto/app/services/memory_read_service.py
• tests/test_memory_read_tenant_isolation.py

Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review.

---

📝 Walkthrough

Walkthrough

Memory reads now require a non-empty agent_id. Namespace resolution returns only the requesting agent’s namespace. New regression tests verify rejection of missing identifiers and prevent cross-tenant fallback or fan-out.

Changes

Tenant isolation

Layer / File(s)	Summary
Enforce agent-scoped namespaces memanto/app/services/memory_read_service.py	generate_answer and _get_search_namespaces reject missing or empty agent_id values. Namespace resolution now returns only the specified agent’s namespace.
Validate tenant isolation tests/test_memory_read_tenant_isolation.py	Tests verify required agent identifiers, single-namespace resolution, and rejection of cross-tenant fallback or fan-out.

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

Merge Risk: ⚪ Minimal · up to cdd42

The change makes tenant-scoped reads fail closed when no agent ID is provided, preventing unintended cross-tenant lookup behavior; no actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: het0814

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 37.50% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 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 main change: enforcing fail-closed behavior for tenant-scoped memory reads.
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 unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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