feat(memory): per-tenant memory isolation#5967
Open
John-Jepsen wants to merge 8 commits into
Open
Conversation
DD-0001 captures the scope, invariant, propagation path, partitioning tradeoff, threat model, test contract, and merge order for closing the multi-tenant memory leak in the unified Memory subsystem. Includes the source-of-truth Mermaid diagram for the identity propagation path under design-docs/diagrams/. The diagram is embedded in the design doc for inline review.
Additive schema change. tenant_id defaults to "_default" (single-tenant deployments unchanged); user_id defaults to None. Updates the docstrings on the legacy source/private fields to point at tenant_id for callers who were using them as an isolation hint -- those fields were never an isolation boundary (the post-filter at unified_memory.py:704-709 ran after retrieval). Backends do not yet persist or filter on the new fields; that lands in the StorageBackend protocol change in the next PR. This commit is purely the Pydantic model + a hermetic round-trip test that pins the defaults and the "legacy row loads as _default" backward-compat path. Refs: design-docs/0001-per-tenant-memory-isolation.md
Add tenant_id as a required keyword-only parameter on every read method
of StorageBackend (search, delete, get_record, list_records,
get_scope_info, list_scopes, list_categories, count, reset + async
variants). The "required without default" form is deliberate: mypy
--strict turns any forgotten caller into a CI failure, which is the
static enforcement behind the isolation invariant.
save() and update() do not gain a tenant_id parameter -- the tenant
lives on the record, and a separate parameter would invite a "record
says A, param says B" mismatch.
Backend changes:
* LanceDBStorage: tenant_id/user_id columns added to placeholder schema;
auto-add via add_columns on opening pre-isolation tables; every WHERE
clause includes the tenant predicate via a new _tenant_where helper;
reset() can no longer drop the table -- it scopes to the tenant.
* QdrantEdgeStorage: tenant_id/user_id added to payload; new
_build_tenant_filter replaces _build_scope_filter so every FieldCondition
chain starts from the tenant; tenant_id and user_id added to payload
indexes; reset() now goes through delete() with the tenant predicate.
Memory and the Flows (encoding_flow, recall_flow) temporarily pass
tenant_id="_default" to every storage call so behavior is unchanged
in this PR. The proper per-call tenant resolution lands when
ScopedStorage is wired through Memory in the next PR.
Test fixtures updated to pass tenant_id="_default" on direct backend
calls. The synthetic orphan payload in
test_orphaned_shard_cleanup gains tenant_id so the tenant filter matches
the fixture (real pre-isolation orphan data is migrated by the upcoming
crewai memory migrate command).
Refs: design-docs/0001-per-tenant-memory-isolation.md
ScopedStorage wraps any StorageBackend and binds every operation to a
fixed (tenant_id, user_id) pair. It holds three contracts and they
live nowhere else:
1. Stamp on write -- every record's tenant_id is overwritten with the
wrapper's bound tenant before persisting; a record arriving with a
different non-default tenant_id raises PermissionError instead of
being silently relabeled.
2. Inject on read -- every read forwarded to the underlying backend
carries the tenant_id predicate; the wrapper exposes no API to omit
it.
3. Verify on return -- after the backend returns rows, the wrapper
re-checks each row's tenant_id and raises RuntimeError on a
foreign-tenant leak. Loud over silent, so a broken backend filter
surfaces in the next test run instead of shipping quietly.
The triple contract is defense in depth around the Protocol's required
keyword arg (type-check-time enforcement) and the backend's pushed-down
WHERE/FieldCondition (data-layer enforcement).
test_tenant_isolation.py adds the security contract from the design
doc. Nine ScopedStorage tests pass today (the wrapper is the
enforcement chokepoint and works without Memory's involvement). One
backcompat test pins the '_default' tenant single-tenant path. Two
Memory-level tests are XFAIL'd with strict=True, raises=TypeError --
they will pass when PR crewAIInc#4 wires Memory's remember/recall/forget to
take tenant_id kwargs and route through ScopedStorage.
Refs: design-docs/0001-per-tenant-memory-isolation.md
Adds tenant_id and user_id to Memory and threads them through every public method (remember, remember_many, recall, forget, update, list_records, list_scopes, list_categories, info, tree, reset, plus async variants). Resolution order is per-call kwarg > instance default > "_default", so single-tenant deployments keep working unchanged. The mechanism: a new _scoped(tenant_id, user_id) factory builds a ScopedStorage proxy bound to the resolved tenant. Every internal storage call routes through it; the temporary tenant_id="_default" arguments from PR crewAIInc#2 are removed. RecallFlow and EncodingFlow are constructed with the ScopedStorage instance, so once a Flow is running its leaf calls cannot escape the tenant filter -- the deep recall LLM exploration path inherits isolation by construction. Within-tenant policy: the existing source/private post-filter in recall() is kept (it operates AFTER ScopedStorage has already filtered to the tenant, so it cannot leak cross-tenant rows). It is demoted in the docstring to a within-tenant convenience; per-user isolation should use user_id, not source/private. Tests: the two previously-XFAIL'd Memory-level isolation tests now pass (cross-tenant recall and forget). A new test_instance_default_tenant_holds pins the "Memory(tenant_id='X')" construction pattern. All 13 isolation tests green; full memory suite 129 passed, 19 skipped (qdrant optional dep). Refs: design-docs/0001-per-tenant-memory-isolation.md
Converts 'crewai memory' from a single command into a click group
(invoke_without_command=True preserves the bare 'crewai memory' TUI
behavior). Adds a 'migrate' subcommand for stamping pre-isolation
LanceDB rows with a tenant_id.
Options:
--storage-dir Memory storage directory (default: $CREWAI_STORAGE_DIR/memory)
--default-tenant Tenant for unstamped rows (default: '_default')
--from-metadata-key Optional metadata key whose value becomes tenant_id;
rows missing the key fall back to --default-tenant.
Useful when existing data carries customer/user
identifiers in metadata.
--table-name LanceDB table name (default: 'memories')
--dry-run Print what would change without writing. Recommended
for the first run against production.
The command is idempotent. Schema migration (adding the tenant_id and
user_id columns) is applied via table.add_columns; per-row updates only
fire when the row's current tenant_id differs from the resolved target.
Dry-run simulates the column-add side effect so its reported row count
matches a real run.
Refs: design-docs/0001-per-tenant-memory-isolation.md
User-facing Mintlify page at docs/en/concepts/memory-isolation.mdx covers when to use tenant_id, the three concepts (tenant_id, user_id, scope) and what each is/isn't a security boundary for, common patterns (SaaS, Crew.kickoff, Flows, instance-bound), the migration command, the threat model, FAQ, and API reference. docs.json: adds en/concepts/memory-isolation right after en/concepts/memory in all 15 nav sections (one per language variant and primary nav). docs/en/concepts/memory.mdx: fixes the "Customer support (per-customer context)" example that previously implied scope provided isolation. It now uses tenant_id and adds a Warning callout pointing at the new isolation page. Translations to ar/ko/pt-BR for the new page can land in follow-up PRs. Refs: design-docs/0001-per-tenant-memory-isolation.md
📝 WalkthroughWalkthroughThis PR implements per-tenant memory isolation throughout CrewAI's memory system by requiring ChangesPer-Tenant Memory Isolation Implementation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/crewai/src/crewai/memory/storage/lancedb_storage.py (1)
506-540:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix
LanceDBStorage.delete()to honorrecord_idswhen combined witholder_than/categories/metadata_filter(match Qdrant semantics)
- With
record_ids+older_thanand nocategories/metadata_filter, LanceDB takes therecord_ids-only branch and ignores age (lines 508-514).- With
record_ids+ (categories/metadata_filter) it scans and deletes by those predicates, but never intersects withrecord_ids(lines 515-540).- Qdrant gates the
record_ids-only path withnot (categories or metadata_filter or older_than)and appliesallowed_idsinside the scan branch.🐛 Proposed fix to match Qdrant semantics
- if record_ids and not (categories or metadata_filter): + if record_ids and not (categories or metadata_filter or older_than): before = int(self._table.count_rows()) ids_expr = ", ".join(f"'{_sql_quote(rid)}'" for rid in record_ids) self._do_write( "delete", f"({tenant_clause}) AND id IN ({ids_expr})" ) return before - int(self._table.count_rows()) - if categories or metadata_filter: + if categories or metadata_filter or older_than: rows = self._scan_rows( scope_prefix, tenant_id=tenant_id, user_id=user_id ) + allowed_ids = set(record_ids) if record_ids else None to_delete: list[str] = [] for row in rows: record = self._row_to_record(row) + if allowed_ids is not None and record.id not in allowed_ids: + continue if categories and not any( c in record.categories for c in categories ): continue if metadata_filter and not all( record.metadata.get(k) == v for k, v in metadata_filter.items() ): continue if older_than and record.created_at >= older_than: continue to_delete.append(record.id)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/crewai/src/crewai/memory/storage/lancedb_storage.py` around lines 506 - 540, LanceDBStorage.delete currently skips `older_than` when `record_ids` are provided and, when combined with `categories`/`metadata_filter`, doesn’t intersect scan results with the provided `record_ids`; update the guard in the fast-path to `if record_ids and not (categories or metadata_filter or older_than)` so the ID-only delete only runs when no age/category/metadata constraints exist, and in the scanning branch (where `_scan_rows`, `_row_to_record`, and building `to_delete` are used) apply an `allowed_ids = set(record_ids)` check so you only append record.id to `to_delete` if it is in `allowed_ids` (i.e., intersect scan results with `record_ids`), leaving `_do_write("delete", ...)` and the count math (`_table.count_rows()`) unchanged.
🧹 Nitpick comments (3)
lib/crewai/src/crewai/memory/unified_memory.py (1)
256-257: 💤 Low valueNote:
user_id=Nonecannot override instance default.The resolution logic
user_id if user_id is not None else self.user_idmeans a caller with an instance-leveluser_idset cannot explicitly request "all users within this tenant" on a per-call basis by passinguser_id=None. They would need a separateMemoryinstance without a defaultuser_id.This is likely acceptable for the intended use case (instance defaults are sticky), but worth documenting if callers may need to query across all users within a tenant while also having a user-specific default.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/crewai/src/crewai/memory/unified_memory.py` around lines 256 - 257, The current resolution assigns resolved_user with "user_id if user_id is not None else self.user_id", which prevents callers from explicitly passing None to mean “all users” when an instance-level default exists; to fix, add a sentinel (e.g. UNSET) as the default for the user_id parameter and change the resolution logic to: if user_id is UNSET then use self.user_id, elif user_id is None then treat as explicit “no user”/all-users, else use the provided user_id; update the resolved_user assignment and any callers and docstrings to reference the new UNSET sentinel and the clarified behavior.design-docs/0001-per-tenant-memory-isolation.md (2)
355-356: ⚡ Quick winConsider limiting the startup warning row scan.
The startup warning that detects unstamped rows could become expensive on large deployments with millions of records. Consider adding a note about limiting the scan to a sample (e.g., first 1000 rows) or making it opt-in via environment variable.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@design-docs/0001-per-tenant-memory-isolation.md` around lines 355 - 356, The startup warning in Memory.model_post_init currently scans for unstamped rows and can be expensive at scale; change it to either limit the scan to a configurable sample (e.g., check only the first N rows or a random N rows) or make the full scan opt-in via an environment variable (e.g., MEMORY_STARTUP_SCAN=true) and expose a sample size config (e.g., MEMORY_STARTUP_SAMPLE_SIZE) so large deployments default to a cheap/sample check while operators can opt into or increase the scan via env vars; update Memory.model_post_init to read these settings and implement the sampled/opt-in logic and ensure the warning message reports when a sample was used.
304-309: 💤 Low valueConsider specifying the
_quoteescaping strategy.The
_quotehelper is referenced but the exact escaping mechanism isn't defined. Consider adding a note specifying whether this uses parameterized queries (preferred) or manual SQL escaping, and which characters must be escaped.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@design-docs/0001-per-tenant-memory-isolation.md` around lines 304 - 309, Clarify and document the escaping strategy used by the _quote helper referenced in the where clause (where = f"tenant_id = {_quote(tenant_id)}" and optional user_id) — update the design doc to state that DB parameterized queries are the preferred approach and either replace/annotate _quote to use parameter binding from the DB client (e.g., prepared statements) or explicitly list the manual-escaping rules and characters escaped (quotes, backslashes, NULL, control chars) if parameterization cannot be used; include a short note referencing the _quote helper name and the tenant_id/user_id usage so readers know which code to change or implement.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/cli/src/crewai_cli/cli.py`:
- Around line 415-418: The "without key" count uses summary['rows_to_stamp']
which can be smaller than summary['rows_with_metadata_key'] on reruns; change
the calculation to derive the count from scanned rows instead by replacing the
expression that prints summary['rows_to_stamp'] -
summary['rows_with_metadata_key'] with summary['rows_scanned'] -
summary['rows_with_metadata_key']; update the block guarded by from_metadata_key
(the click.echo lines) so it prints the corrected value and ensure summary
contains 'rows_scanned' when building this output.
In `@lib/cli/src/crewai_cli/memory_migrate.py`:
- Around line 125-126: The current code materializes up to 10M rows with
table.search().limit(10_000_000).to_list(), which loads all columns (including
large vectors) into memory and silently truncates scans; change this to a
batched scanner that requests only the needed columns ("id", "tenant_id",
"metadata_str") and iterates in pages (e.g., using
table.search().select(...).limit(batch_size) with an offset/cursor or streaming
API), processing and stamping each batch and incrementing
summary["rows_scanned"] per batch so you never load the full table into memory
and never fetch the vector column.
In `@lib/crewai/tests/memory/test_qdrant_edge_storage.py`:
- Around line 57-62: Extend test_save_search to exercise tenant isolation:
create two records with the same embedding (use _rec to build them) but
different tenant ids (e.g., "alice" and "bob"), call storage.save([...]) for
both tenants, then run storage.search(...) with tenant_id set to each tenant and
assert results only contain that tenant's record; also exercise
storage.delete(...) with a tenant-scoped delete and verify the other tenant's
data remains. Use the existing symbols test_save_search, _rec, storage.save,
storage.search, and storage.delete to implement these checks.
In `@lib/crewai/tests/memory/test_tenant_isolation.py`:
- Around line 258-333: Add a deep-recall isolation test to TestMemoryIsolation
that uses Memory and its recall(...) with depth="deep" so the
RecallFlow/LLM-driven branch is exercised; create two records for tenants
"alice" and "bob" with identical embeddings (use mock_embedder to return same
vectors), call recall("something", tenant_id="alice", depth="deep") and
recall(... tenant_id="bob", depth="deep"), and assert results are tenant-scoped
(all h.record.tenant_id matches expected and no cross-tenant secrets appear) to
cover the LLM-driven path alongside the existing shallow tests.
- Around line 46-58: The mock_embedder fixture (mock_embedder -> embed ->
m.side_effect) uses abs(hash(t)) % 1000 which buckets inputs into 1000
collisions; replace that with a deterministic digest-based mapping (e.g.,
compute a SHA256/MD5 digest of t.encode(), then convert bytes into normalized
floats) to produce the 4-element embedding vector so different texts map to
highly distinct embeddings while remaining deterministic for tests.
---
Outside diff comments:
In `@lib/crewai/src/crewai/memory/storage/lancedb_storage.py`:
- Around line 506-540: LanceDBStorage.delete currently skips `older_than` when
`record_ids` are provided and, when combined with
`categories`/`metadata_filter`, doesn’t intersect scan results with the provided
`record_ids`; update the guard in the fast-path to `if record_ids and not
(categories or metadata_filter or older_than)` so the ID-only delete only runs
when no age/category/metadata constraints exist, and in the scanning branch
(where `_scan_rows`, `_row_to_record`, and building `to_delete` are used) apply
an `allowed_ids = set(record_ids)` check so you only append record.id to
`to_delete` if it is in `allowed_ids` (i.e., intersect scan results with
`record_ids`), leaving `_do_write("delete", ...)` and the count math
(`_table.count_rows()`) unchanged.
---
Nitpick comments:
In `@design-docs/0001-per-tenant-memory-isolation.md`:
- Around line 355-356: The startup warning in Memory.model_post_init currently
scans for unstamped rows and can be expensive at scale; change it to either
limit the scan to a configurable sample (e.g., check only the first N rows or a
random N rows) or make the full scan opt-in via an environment variable (e.g.,
MEMORY_STARTUP_SCAN=true) and expose a sample size config (e.g.,
MEMORY_STARTUP_SAMPLE_SIZE) so large deployments default to a cheap/sample check
while operators can opt into or increase the scan via env vars; update
Memory.model_post_init to read these settings and implement the sampled/opt-in
logic and ensure the warning message reports when a sample was used.
- Around line 304-309: Clarify and document the escaping strategy used by the
_quote helper referenced in the where clause (where = f"tenant_id =
{_quote(tenant_id)}" and optional user_id) — update the design doc to state that
DB parameterized queries are the preferred approach and either replace/annotate
_quote to use parameter binding from the DB client (e.g., prepared statements)
or explicitly list the manual-escaping rules and characters escaped (quotes,
backslashes, NULL, control chars) if parameterization cannot be used; include a
short note referencing the _quote helper name and the tenant_id/user_id usage so
readers know which code to change or implement.
In `@lib/crewai/src/crewai/memory/unified_memory.py`:
- Around line 256-257: The current resolution assigns resolved_user with
"user_id if user_id is not None else self.user_id", which prevents callers from
explicitly passing None to mean “all users” when an instance-level default
exists; to fix, add a sentinel (e.g. UNSET) as the default for the user_id
parameter and change the resolution logic to: if user_id is UNSET then use
self.user_id, elif user_id is None then treat as explicit “no user”/all-users,
else use the provided user_id; update the resolved_user assignment and any
callers and docstrings to reference the new UNSET sentinel and the clarified
behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 5ea368e1-6a3f-4e8e-a42f-5c27e6e2c70e
📒 Files selected for processing (18)
design-docs/0001-per-tenant-memory-isolation.mddesign-docs/diagrams/identity-propagation.mmddocs/docs.jsondocs/en/concepts/memory-isolation.mdxdocs/en/concepts/memory.mdxlib/cli/src/crewai_cli/cli.pylib/cli/src/crewai_cli/memory_migrate.pylib/cli/tests/test_memory_migrate.pylib/crewai/src/crewai/memory/storage/backend.pylib/crewai/src/crewai/memory/storage/lancedb_storage.pylib/crewai/src/crewai/memory/storage/qdrant_edge_storage.pylib/crewai/src/crewai/memory/storage/scoped_storage.pylib/crewai/src/crewai/memory/types.pylib/crewai/src/crewai/memory/unified_memory.pylib/crewai/tests/memory/test_memory_record_tenant_fields.pylib/crewai/tests/memory/test_qdrant_edge_storage.pylib/crewai/tests/memory/test_tenant_isolation.pylib/crewai/tests/memory/test_unified_memory.py
Major:
- LanceDBStorage.delete(): record_ids + older_than took the fast path
and silently dropped older_than; record_ids + categories/metadata_filter
scanned by the predicates but never intersected with record_ids.
Now matches Qdrant semantics: fast path is gated on no other predicates,
scan branch intersects with allowed_ids = set(record_ids). Two new
regression tests pin both branches.
- crewai memory migrate now streams rows in paginated batches instead of
materializing up to 10M rows in one call. Only id, tenant_id, and
metadata_str are selected -- the heavy vector column is never read.
Pre-isolation tables (no tenant_id column) adapt the select set so
dry-run works against unmigrated schemas. New test fixes the page size
and confirms a multi-page table is scanned end-to-end.
- Adds tenant-isolation tests to test_qdrant_edge_storage so when the
optional qdrant_edge dep is installed the Qdrant backend's isolation
contract is verified, not just LanceDB's.
- Adds test_deep_recall_honors_tenant exercising the depth='deep'
RecallFlow path with two tenants and colliding embeddings.
Minor:
- CLI "without key" count now derives from rows_scanned instead of
rows_to_stamp, so it stays correct on reruns where rows_to_stamp shrinks
to zero as the data converges.
- mock_embedder in test_tenant_isolation now uses a SHA-256 digest instead
of abs(hash(t)) % 1000, removing the 1000-bucket collision risk that
could let two different texts produce identical vectors and make
isolation assertions pass vacuously.
Skipped (intentional):
- Design-doc nits about startup-warning scan limits and _quote escaping
strategy -- the warning is documented as a one-line log, not a load-
bearing scan, and the escaping helper is documented inline in the
LanceDB implementation.
- The user_id sentinel suggestion -- per-call user_id=None meaning
"instance default" is the intended SaaS pattern; callers who need
"all users within tenant" construct a Memory without a default user_id.
Test counts: 140 passed, 21 skipped (was 136/19; the deltas are the two
new Qdrant isolation tests, the new deep-recall test, and the two new
LanceDB delete-intersection tests).
Author
|
Thanks @coderabbitai. Addressed in 90073fb: Fixed (5 of 5 actionable):
Skipped (with reason):
Test counts: 140 passed, 21 skipped (was 136/19). Deltas: 2 LanceDB delete tests, 2 Qdrant isolation tests (skipped without the optional dep), 1 deep-recall test, 1 migrate-pagination test. |
|
Tip For best results, initiate chat on the files or code changes.
The rationale for skipping the The 🐇✨ If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #2584
This PR implements per-tenant memory isolation and contextual memory retrieval.
Summary
Closes the per-tenant memory leak in the unified
Memorysubsystem. Right now every user's memories pool into the same vector collection andrecall()mixes them across users — a data leak, not a UX nit. This PR threadstenant_id(and optionaluser_id) through the entire save → store → recall path and enforces isolation at the storage layer.Invariant: a
recall()scoped to tenant A can never return a memory written by tenant B — under any ranking, embedding collision, query depth, or backend.Backward compatible. Default
tenant_idis"_default", so single-tenant deployments keep working unchanged. Existing rows on disk read back as"_default"via row-level fallbacks and an auto-add_columnsstep when LanceDB tables are first opened.Design
Full design doc:
design-docs/0001-per-tenant-memory-isolation.md— covers the invariant, identity propagation, partitioning tradeoffs, threat model, test contract, and rollout order.The enforcement chokepoint is a new
ScopedStorageproxy that holds three contracts:tenant_idis overwritten with the bound tenant; cross-tenant records raisePermissionError.tenant_idpredicate; callers cannot omit it.RuntimeErrorloudly, never silently filtered.The triple is defense in depth: mypy
--strictcatches forgotten kwargs at type-check time (Protocol-level), backends pushWHERE tenant_id = ?into the vector query (data-layer), andScopedStorageis the runtime guard when either of the first two fails.Commits (intended as one stacked PR; happy to split if you prefer)
docs(memory): add design doc…feat(memory): add tenant_id and user_id fields to MemoryRecord_default. Hermetic round-trip tests.refactor(memory): require tenant_id keyword on StorageBackend protocoltenant_idon every read method; LanceDB + Qdrant push the predicate into the vector query. mypy--strictenforces.feat(memory): add ScopedStorage wrapper and tenant isolation testsfeat(memory): wire ScopedStorage through Memory and Flowstenant_id/user_idfields and per-call kwargs onremember/recall/forget/etc;RecallFlowandEncodingFlowhold aScopedStorageso deep recall cannot escape.feat(cli): add 'crewai memory migrate'--storage-dir,--default-tenant,--from-metadata-key,--dry-run. Idempotent.docs(memory): add per-tenant memory isolation guidememory.mdx.What this is not
Explicitly excluded (each its own ticket): mem0 replacement, per-tenant encryption, namespace-per-tenant partitioning (Option B in the design doc), multi-tenant rate limiting,
Crew.kickoff()integration (Memory API is in;Crew.kickoff(tenant_id=…)belongs to a follow-up that touches agent execution).Test plan
lib/crewai/tests/memory/test_tenant_isolation.py):recall()returns nothing (shallow and deep)PermissionErroron cross-tenant save through scoped handleRuntimeErroron backend leak detectionforget/reset/get_recordare tenant-scopeduser_idsub-partition within tenantlib/cli/tests/test_memory_migrate.py): missing dir/table no-op, schema migration, per-row--from-metadata-key, idempotence,--dry-run, validationruff checkclean across all changed filesmypy --strictclean (the 16 errors inqdrant_edge_storage.pyare pre-existingno-any-unimportedfrom the optionalqdrant_edgedep being uninstalled — present onmainbefore this branch)Security note
This PR fixes a data-isolation bug. Reviewers — please run the checklist on the design doc in this order:
tenant_idOpen questions (called out in the design doc)
"_default"the right sentinel? Alternatives bikeshed in DD-0001.Crew.kickoff(tenant_id=...)surface — proposed for a separate PR.tenant_idvalues must never appear in telemetry payloads. Owner ofcrewai/telemetry/should weigh in.BaseRAGStoragedeprecation timeline.AI-generated PR. Per
.github/CONTRIBUTING.md, thellm-generatedlabel is required. I attempted to apply it viagh pr create --labelbut lack permission on this repo. Could a maintainer please apply thellm-generatedlabel?Summary by CodeRabbit
New Features
crewai memory migrateto stamp existing memories with tenant identifiers and report dry-run vs applied changes.Documentation