Add hard ceilings to max_iter and flow router hops to prevent unbounded loops#6370
Open
camgrimsec wants to merge 1 commit into
Open
Add hard ceilings to max_iter and flow router hops to prevent unbounded loops#6370camgrimsec wants to merge 1 commit into
camgrimsec wants to merge 1 commit into
Conversation
Introduces two non-breaking guardrails identified in a security audit: * max_iter ceiling: enforces a hard upper bound of 500 via Pydantic gt=0, le=500 on the max_iter field across BaseAgent, BaseAgentExecutor, and the experimental AgentExecutor. The default remains 25. Prevents denial-of-wallet attacks where a misconfigured or malicious config (e.g. max_iter=10_000_000) would burn unbounded API credits. * Flow router hop limit: adds MAX_ROUTER_HOPS = 100 counter to the router loop in FlowRuntime._route(). Raises RuntimeError on cycle instead of hanging the event loop indefinitely. Mitigates CWE-770 (unbounded resource consumption) and CWE-835 (loop with unreachable exit). Affects no well-formed workflows.
📝 WalkthroughWalkthroughAdds upper-bound caps to prevent unbounded execution: ChangesIteration and Router Hop Caps
Suggested Reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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: 1
🧹 Nitpick comments (1)
lib/crewai/src/crewai/agents/agent_builder/base_agent_executor.py (1)
27-27: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse
MAX_ITER_CEILINGinstead of the literal500.The ceiling is now a named constant in
base_agent.py, but this field (andAgentExecutor.max_iter) hard-code500. For a security guardrail, three copies of the bound risk silent drift if it is ever changed in one place only. Consider importing and reusing the shared constant.🤖 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/agents/agent_builder/base_agent_executor.py` at line 27, The max iteration ceiling is hard-coded here instead of using the shared guardrail constant, which can drift from the value used elsewhere. Update the max_iter Field definition in base_agent_executor and the AgentExecutor.max_iter validation to import and reuse MAX_ITER_CEILING from base_agent.py so the bound stays consistent in one place.
🤖 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/crewai/src/crewai/flow/runtime/__init__.py`:
- Around line 2745-2753: The router hop cap in the flow runtime is too low for
valid experimental agent loops, so update the MAX_ROUTER_HOPS check in the
router loop to allow longer runs. Either raise the static limit or derive it
from the existing max_iter/max_method_calls settings used by the flow runtime so
the cycle guard stays consistent with those limits. Keep the change localized
around the hop_count validation and the router cycle detection logic.
---
Nitpick comments:
In `@lib/crewai/src/crewai/agents/agent_builder/base_agent_executor.py`:
- Line 27: The max iteration ceiling is hard-coded here instead of using the
shared guardrail constant, which can drift from the value used elsewhere. Update
the max_iter Field definition in base_agent_executor and the
AgentExecutor.max_iter validation to import and reuse MAX_ITER_CEILING from
base_agent.py so the bound stays consistent in one place.
🪄 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: bf047ac8-7c78-41d5-bc5a-1c45b6315c53
📒 Files selected for processing (4)
lib/crewai/src/crewai/agents/agent_builder/base_agent.pylib/crewai/src/crewai/agents/agent_builder/base_agent_executor.pylib/crewai/src/crewai/experimental/agent_executor.pylib/crewai/src/crewai/flow/runtime/__init__.py
Comment on lines
+2745
to
+2753
| hop_count = 0 | ||
|
|
||
| while True: | ||
| hop_count += 1 | ||
| if hop_count > MAX_ROUTER_HOPS: | ||
| raise RuntimeError( | ||
| f"Flow router cycle detected: exceeded {MAX_ROUTER_HOPS} " | ||
| "router hops. Check for circular @router definitions." | ||
| ) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Map the AgentExecutor router graph and confirm which routers can chain
# back into one another within a single _execute_listeners pass.
fd -t f 'agent_executor.py' lib/crewai/src/crewai/experimental \
--exec rg -nP '`@router`\(|return "(initialized|continue_reasoning|todo_not_satisfied|check_iteration|parsed)"' {}
# Check for any existing test exercising many iterations through the flow router loop.
rg -nP --type=py -C2 'MAX_ROUTER_HOPS|max_iter\s*=\s*(1\d\d|[2-9]\d\d)' lib/crewaiRepository: crewAIInc/crewAI
Length of output: 2879
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the router functions involved in the AgentExecutor loop and the runtime hop counter.
sed -n '1320,1455p' lib/crewai/src/crewai/flow/runtime/__init__.py
printf '\n---\n'
sed -n '2050,2245p' lib/crewai/src/crewai/flow/runtime/__init__.py
printf '\n---\n'
sed -n '2660,2715p' lib/crewai/src/crewai/flow/runtime/__init__.py
printf '\n---\n'
sed -n '2735,2760p' lib/crewai/src/crewai/flow/runtime/__init__.pyRepository: crewAIInc/crewAI
Length of output: 18309
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the max-iteration configuration and the class that uses the router loop.
rg -n -C3 'max_iter|max_method_calls|MAX_ROUTER_HOPS|increment_and_continue|check_max_iterations|continue_reasoning|route_by_answer_type|check_todo_completion' lib/crewai/src/crewai/flow/runtime/__init__.pyRepository: crewAIInc/crewAI
Length of output: 1783
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the router bodies that form the suspected cycle.
sed -n '1338,1395p' lib/crewai/src/crewai/flow/runtime/__init__.py
printf '\n---\n'
sed -n '2088,2145p' lib/crewai/src/crewai/flow/runtime/__init__.py
printf '\n---\n'
sed -n '2234,2245p' lib/crewai/src/crewai/flow/runtime/__init__.py
printf '\n---\n'
sed -n '2648,2715p' lib/crewai/src/crewai/flow/runtime/__init__.pyRepository: crewAIInc/crewAI
Length of output: 8488
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for the agent-side max_iter default/configuration.
rg -n -C2 'max_iter\s*[:=]\s*500|max_iter\s*[:=]\s*\d+|max_iter' lib/crewai/src/crewai/agents lib/crewai/src/crewai/flow lib/crewai/src/crewaiRepository: crewAIInc/crewAI
Length of output: 25918
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the agent loop routers and the surrounding methods in the experimental executor.
sed -n '2080,2255p' lib/crewai/src/crewai/experimental/agent_executor.py
printf '\n---\n'
sed -n '2230,2305p' lib/crewai/src/crewai/experimental/agent_executor.pyRepository: crewAIInc/crewAI
Length of output: 10204
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show where the experimental executor re-enters listener execution so we can tell whether
# the router chain is bounded per method/result or across the whole agent run.
rg -n -C3 '_execute_listeners\(' lib/crewai/src/crewai/experimental/agent_executor.pyRepository: crewAIInc/crewAI
Length of output: 154
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the missing router methods around the iteration/reasoning loop.
sed -n '1360,1465p' lib/crewai/src/crewai/experimental/agent_executor.py
printf '\n---\n'
sed -n '1465,1615p' lib/crewai/src/crewai/experimental/agent_executor.py
printf '\n---\n'
sed -n '1615,1775p' lib/crewai/src/crewai/experimental/agent_executor.py
printf '\n---\n'
sed -n '1775,1915p' lib/crewai/src/crewai/experimental/agent_executor.pyRepository: crewAIInc/crewAI
Length of output: 21862
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find every router/listener in the experimental executor that can feed back into the loop.
rg -n -C2 '`@router`\(|`@listen`\(' lib/crewai/src/crewai/experimental/agent_executor.pyRepository: crewAIInc/crewAI
Length of output: 5730
MAX_ROUTER_HOPS is too low for the experimental agent loop Consider raising it or deriving it from max_iter/max_method_calls; the current 100-hop cap can trip valid runs before they exhaust the 500-iteration limit.
🤖 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/flow/runtime/__init__.py` around lines 2745 - 2753, The
router hop cap in the flow runtime is too low for valid experimental agent
loops, so update the MAX_ROUTER_HOPS check in the router loop to allow longer
runs. Either raise the static limit or derive it from the existing
max_iter/max_method_calls settings used by the flow runtime so the cycle guard
stays consistent with those limits. Keep the change localized around the
hop_count validation and the router cycle detection logic.
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.
Introduces two non-breaking guardrails identified in a security audit:
max_iter ceiling: enforces a hard upper bound of 500 via Pydantic gt=0, le=500 on the max_iter field across BaseAgent, BaseAgentExecutor, and the experimental AgentExecutor. The default remains 25. Prevents denial-of-wallet attacks where a misconfigured or malicious config (e.g. max_iter=10_000_000) would burn unbounded API credits.
Flow router hop limit: adds MAX_ROUTER_HOPS = 100 counter to the router loop in FlowRuntime._route(). Raises RuntimeError on cycle instead of hanging the event loop indefinitely.
Mitigates CWE-770 (unbounded resource consumption) and CWE-835 (loop with unreachable exit). Affects no well-formed workflows.
Summary by CodeRabbit