Repository navigation
fix: rename 'input' parameter to 'tool_input' in CacheHandler to avoi… - #9334
Raja Gopal (Rajagopalhertzian) wants to merge 1 commit into
Conversation
…d shadowing builtin
Strix Security ReviewTo fix before merging (1)
PR summary: Renames the `input` argument to `tool_input` in `CacheHandler.add()`/`read()` so it no longer shadows Python's builtin, and updates the callers in `tools_handler.py` and `agent_executor.py`. Several other call sites in the same codebase were not updated. Review summaryThis PR is a cosmetic rename of the Ruled out (1)
Fixed the findings? re-run the review, or tag Review 1 on this PR · Reviewed by Strix |
|
Thanks for the pull request. First-time contributors need an associated open issue before we can review a PR.
See the contributing guide. |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 4ec4f5c. Configure here.
| ): | ||
| cached_result = self.tools_handler.cache.read( | ||
| tool=func_name, input=input_str | ||
| tool=func_name, tool_input=input_str |
There was a problem hiding this comment.
Incomplete CacheHandler parameter rename
High Severity
Renaming CacheHandler.add/read from input to tool_input left several callers on the old keyword. Native cache writes, ReAct cache lookups, execute_native_tool_call in agent_utils.py, and project @memoize all pass input= and raise TypeError at runtime.
Reviewed by Cursor Bugbot for commit 4ec4f5c. Configure here.
| _lock: RWLock = PrivateAttr(default_factory=RWLock) | ||
|
|
||
| def add(self, tool: str, input: str, output: Any) -> None: | ||
| def add(self, tool: str, tool_input: str, output: Any) -> None: |
There was a problem hiding this comment.
🔵 Incomplete CacheHandler parameter rename breaks every remaining input= caller with TypeError
Severity: MEDIUM · CWE-628
The pull request renames input to tool_input on CacheHandler.add()/read() but updates only two call sites. Five others still pass the removed input= keyword: agent_executor.py:2369 (cache.add), agent_utils.py:1653 (cache.read) and :1680 (cache.add), tool_usage.py:310/:567 (cache.read), and the memoization wrappers at project/utils.py:81/86/106/111. Since the new signatures accept no input keyword, each raises TypeError: got an unexpected keyword argument 'input' whenever caching is enabled (Crew(cache=True) or Agent(cache=True)). In agent_executor.py the broad tool-execution handler catches it, so the tool runs but its real result is replaced by an error message, the task tool-error counter is incremented, and the model may retry and duplicate a non-idempotent side effect. In agent_utils.py and project/utils.py the failing call sits outside the try and propagates, breaking the tool loop.
Prompt to fix with AI
This is a security vulnerability found during a code review.
Vulnerability: Incomplete `CacheHandler` parameter rename breaks every remaining `input=` caller with `TypeError`
Severity: MEDIUM
CWE: CWE-628
The pull request renames `input` to `tool_input` on `CacheHandler.add()`/`read()` but updates only two call sites. Five others still pass the removed `input=` keyword: `agent_executor.py:2369` (`cache.add`), `agent_utils.py:1653` (`cache.read`) and `:1680` (`cache.add`), `tool_usage.py:310`/`:567` (`cache.read`), and the memoization wrappers at `project/utils.py:81/86/106/111`. Since the new signatures accept no `input` keyword, each raises `TypeError: got an unexpected keyword argument 'input'` whenever caching is enabled (`Crew(cache=True)` or `Agent(cache=True)`). In `agent_executor.py` the broad tool-execution handler catches it, so the tool runs but its real result is replaced by an error message, the task tool-error counter is incremented, and the model may retry and duplicate a non-idempotent side effect. In `agent_utils.py` and `project/utils.py` the failing call sits outside the `try` and propagates, breaking the tool loop.
Location: lib/crewai/src/crewai/agents/cache/cache_handler.py:20-20
Context: renamed `input` to `tool_input` in `add` (root cause)
```
def add(self, tool: str, tool_input: str, output: Any) -> None:
```
Location: lib/crewai/src/crewai/agents/cache/cache_handler.py:40-40
Context: renamed `input` to `tool_input` in `read` (root cause)
```
def read(self, tool: str, tool_input: str) -> Any | None:
```
Location: lib/crewai/src/crewai/agents/agent_executor.py:2369-2369
Context: cache write still passes the removed `input` keyword
```
// Before:
tool=func_name, input=input_str, output=raw_result
// After:
tool=func_name, tool_input=input_str, output=raw_result
```
How to fix:
Update every remaining `CacheHandler` call site to pass the renamed `tool_input` keyword, and land the signature change and the caller updates in the same change. Add a regression test that exercises the cached native-tool path end to end so an incomplete rename is caught. If a staged migration is required, temporarily accept the legacy `input` keyword as an alias so caching cannot fail at runtime.
Please fix this vulnerability. If you propose a fix, make it concise and minimal.React 👍 / 👎 to tune Strix for this repo. A repo collaborator (or the PR author) can resolve this thread to dismiss the finding.
| self._cache[f"{tool}-{tool_input}"] = output | ||
|
|
||
| def read(self, tool: str, input: str) -> Any | None: | ||
| def read(self, tool: str, tool_input: str) -> Any | None: |
There was a problem hiding this comment.
renamed input to tool_input in read (root cause)


Summary
Fixes TODO in
cache_handler.pyto avoid shadowing Python's builtininput()function.Changes
inputparameter →tool_inputinCacheHandler.add()andCacheHandler.read()tools_handler.pyandagent_executor.pyTesting
test_tool_cache_default.py)Addresses TODO comments in
lib/crewai/src/crewai/agents/cache/cache_handler.pyNote
Medium Risk
The public CacheHandler signature changes, but other callers (e.g. native cache writes in agent_executor and paths in agent_utils/project utils) still pass
input=, which would raise TypeError until aligned.Overview
Renames the
CacheHandler.add()andCacheHandler.read()keyword argument frominputtotool_inputso it no longer shadows Python’s builtininput(), and drops the related TODO notes incache_handler.py.Call sites in
tools_handler.pyand oneagent_executor.pycache lookup are updated to passtool_input=; behavior (cache keys stillf"{tool}-{tool_input}") is unchanged.Reviewed by Cursor Bugbot for commit 4ec4f5c. Bugbot is set up for automated code reviews on this repo. Configure here.