Fix #5446: Prevent RCE via unvalidated dynamic module import in load_agent_from_repository#5447
Fix #5446: Prevent RCE via unvalidated dynamic module import in load_agent_from_repository#5447devin-ai-integration[bot] wants to merge 4 commits into
Conversation
…event RCE Add an allowlist of trusted module prefixes (crewai_tools., crewai.tools.) and a BaseTool subclass check before dynamically importing and instantiating tool classes from the repository API response. Previously, tool['module'] and tool['name'] from the API were passed directly to importlib.import_module() and getattr() without any validation, allowing a compromised or MITM'd API to import arbitrary Python modules (e.g. subprocess.Popen) and achieve Remote Code Execution. Fixes #5446 Co-Authored-By: João <joao@crewai.com>
|
Prompt hidden (unlisted session) |
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Co-Authored-By: João <joao@crewai.com>
- Re-raise AgentRepositoryError before the generic except handler so security error messages are not swallowed (Cursor Bugbot feedback) - Move module import to top-level to fix mixed import style warning - Replace unnecessary lambda with MagicMock class reference - Add noqa for PERF203 on the new except clause Co-Authored-By: João <joao@crewai.com>
| import pytest | ||
| from pydantic import BaseModel, Field | ||
|
|
||
| import crewai.utilities.agent_utils as _agent_utils_mod |
Co-Authored-By: João <joao@crewai.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 3c8d2d2. Configure here.
| ALLOWED_TOOL_MODULE_PREFIXES: Final[tuple[str, ...]] = ( | ||
| "crewai_tools.", | ||
| "crewai.tools.", | ||
| ) |
There was a problem hiding this comment.
Allowlist rejects the actual production module path format
High Severity
The ALLOWED_TOOL_MODULE_PREFIXES only contains "crewai_tools." and "crewai.tools." (with trailing dots), but the existing tests in test_agent.py (lines 2086, 2091, 2123, 2147) show the CrewAI Plus API actually returns "module": "crewai_tools" (without a trailing dot or submodule) for tools like SerperDevTool and FileReadTool. Since "crewai_tools".startswith("crewai_tools.") is False, all legitimate tool imports using the bare package name will be rejected as untrusted, breaking repository agent loading in production.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 3c8d2d2. Configure here.


Summary
Fixes a critical Remote Code Execution vulnerability (#5446) in
load_agent_from_repository. When loading agents from the CrewAI Plus API, thetool["module"]andtool["name"]fields were passed directly toimportlib.import_module()andgetattr()without any validation. An attacker who controls or compromises the API response could force the victim to import arbitrary Python modules (e.g.subprocess.Popen) and execute arbitrary code.Changes
lib/crewai/src/crewai/utilities/agent_utils.py:ALLOWED_TOOL_MODULE_PREFIXESconstant — a strict allowlist of trusted module prefixes (crewai_tools.,crewai.tools.)_is_trusted_tool_module()helper to validate module paths against the allowlistAgentRepositoryErrorBaseToolsubclass before instantiationimport importlibfrom local scope to module-level importlib/crewai/tests/utilities/test_agent_utils.py:TestIsTrustedToolModule(11 tests) — validates allowlist logic for trusted prefixes, dangerous modules, partial matches, and edge casesTestAllowedToolModulePrefixes(2 tests) — ensures the constant contains expected prefixes and no dangerous onesTestLoadAgentFromRepositoryModuleValidation(7 tests) — integration tests covering the exact attack vector from the issue (subprocess.Popen),osmodule, arbitrary modules, non-BaseTool class rejection, trusted module acceptance, non-tool attribute passthrough, and mixed malicious/legitimate tool listsReview & Testing Checklist for Human
crewai_tools.,crewai.tools.) cover all legitimate tool module paths used in production repository agent definitionsALLOWED_TOOL_MODULE_PREFIXESfrom_repositoryto confirm tools still load correctly end-to-endNotes
crewai_tools.andcrewai.tools.are allowed. This is a security-critical path and an allowlist is safer than a denylist.BaseToolsubclass check acts as a second layer of defense even if a module passes the prefix check.test_agent_utils.pypass, including the 20 new ones. Ruff lint passes cleanly.Link to Devin session: https://app.devin.ai/sessions/5d8f71d83cac40ae8805c1139726c27f
Note
High Risk
Adds strict validation to dynamic tool imports when loading agents from the repository API, which can impact any users relying on custom/non-allowlisted tool modules. Security-critical path change; mistakes in allowlist or type checks could break legitimate agents but reduces RCE risk.
Overview
Closes an RCE vector in
load_agent_from_repositoryby blocking untrusted dynamic imports from repository-supplied tool metadata.Tool loading now enforces an allowlist (
ALLOWED_TOOL_MODULE_PREFIXES) via_is_trusted_tool_module, and additionally verifies the resolved symbol is aBaseToolsubclass before instantiation; errors are surfaced asAgentRepositoryErrorwithout being masked by generic exception handling.Adds comprehensive tests covering allowlist edge cases and end-to-end rejection of malicious modules/classes (e.g.
subprocess.Popen,os.system), plus a positive-path load of an allowedcrewai_tools.*tool.Reviewed by Cursor Bugbot for commit 3c8d2d2. Bugbot is set up for automated code reviews on this repo. Configure here.