Add tenant isolation for Crew storage (Redis, Manager, Handlers)#549
Add tenant isolation for Crew storage (Redis, Manager, Handlers)#549phenobarbital merged 11 commits intoui-implementationfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request introduces tenant isolation for AgentCrew storage across Redis, BotManager, and HTTP handlers. The changes enable multi-tenant segmentation while preserving backward compatibility with existing global crews through legacy key fallbacks.
Changes:
- Added
tenantfield toCrewDefinitionwith default value "global" for backward compatibility - Implemented tenant-aware Redis key namespacing (
crew:{tenant}:{name}) with legacy fallback support for existing keys - Extended BotManager cache to use tenant-prefixed keys and made all crew operations tenant-aware
- Updated CRUD and execution endpoints to accept and propagate tenant parameters through the request/response cycle
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 20 comments.
Show a summary per file
| File | Description |
|---|---|
| parrot/handlers/crew/models.py | Adds tenant field to CrewDefinition with "global" default |
| parrot/handlers/crew/redis_persistence.py | Implements tenant-aware key namespacing, legacy fallbacks, and multi-tenant listing operations |
| parrot/manager/manager.py | Updates in-memory cache to use tenant-prefixed keys and makes all crew operations tenant-aware |
| parrot/handlers/crew/handler.py | Threads tenant parameter through GET, PUT, DELETE endpoints and includes in responses |
| parrot/handlers/crew/execution_handler.py | Extracts tenant from execution requests and persists in job metadata |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| qs = self.get_arguments(self.request) | ||
| crew_id = match_params.get('id') or qs.get('crew_id') | ||
| crew_name = qs.get('name') | ||
| tenant = qs.get('tenant') or "global" |
There was a problem hiding this comment.
The tenant parameter defaults to "global" when not provided, which mirrors the same issue in the GET handler. In a multi-tenant system, silently defaulting to "global" could lead to accidental deletion of global tenant crews when the client intended to delete from a specific tenant but forgot the parameter. Consider making tenant a required parameter or at least validating/logging when the default is used.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
| qs = self.get_arguments(self.request) | ||
| crew_id = match_params.get('id') or qs.get('crew_id') | ||
| crew_name = qs.get('name') | ||
| tenant = qs.get('tenant') or "global" |
There was a problem hiding this comment.
Similar to the GET handler, the tenant parameter from the query string is not validated or sanitized. This creates a security risk where malicious tenant values could be used to construct invalid Redis keys or potentially access data from other tenants if the key construction is vulnerable to injection. Implement tenant validation to ensure only safe, expected tenant identifiers are accepted.
|
@phenobarbital I've opened a new pull request, #552, to work on those changes. Once the pull request is ready, I'll request review from you. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@phenobarbital I've opened a new pull request, #553, to work on those changes. Once the pull request is ready, I'll request review from you. |
|
@phenobarbital I've opened a new pull request, #554, to work on those changes. Once the pull request is ready, I'll request review from you. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…-one [WIP] Address feedback on tenant isolation for Crew storage implementation
[WIP] WIP Address feedback on tenant isolation for Crew storage
[WIP] WIP Address feedback on tenant isolation for Crew storage implementation
Motivation
tenantattribute and tenant-aware storage.Description
tenant: str = "global"toCrewDefinitionso every crew carries a tenant scope. (parrot/handlers/crew/models.py)crew:{tenant}:{name}andcrew:{tenant}:id:{crew_id}, tracking tenants increw:tenants, and keeping legacy global fallbacks for old keys. (parrot/handlers/crew/redis_persistence.py)load_crew(name, tenant),load_crew_by_id(crew_id, tenant),delete_crew(name, tenant),list_crews(tenant),list_all_crews(),get_all_crews(tenant),crew_exists(name, tenant), and metadata methods that includetenant. (parrot/handlers/crew/redis_persistence.py)tenantand responses includetenant. (parrot/handlers/crew/handler.py, parrot/handlers/crew/execution_handler.py)add_crew,get_crew,list_crews,remove_crew,update_crew,load_crews,sync_crews) tenant-aware. (parrot/manager/manager.py)Testing
Codex Task