feat(hooks): SHACKLE — pre-execution circuit breaker for tool calls#6298
feat(hooks): SHACKLE — pre-execution circuit breaker for tool calls#6298Fame510 wants to merge 2 commits into
Conversation
Adds shackle_guard.py — a lightweight, self-contained circuit breaker that integrates with crewAI's existing tool hook system. One-line activation: from crewai.hooks.shackle_guard import register_shackle_guard register_shackle_guard(budget=0.25, max_repeat_calls=3) Features: - Budget enforcement: tracks cumulative tool cost, opens circuit on exhaustion - Loop detection: blocks identical tool+params calls after limit reached - Error amplification: tightens repeat limits when 401/403/500 signals detected - HITL: uses crewAI's built-in request_human_input() for high-risk tool approval - Wall-clock timeout: caps total session duration - Zero dependencies beyond crewAI's existing hook infrastructure Related: crewAIInc#6025 (Runtime release-control mediation)
![corridor-security[bot]](https://avatars.githubusercontent.com/in/1177339?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary: This PR adds an optional pre-execution tool-call guard that enforces budget, timeout, repeat-call, and human-approval checks before tool execution; no exploitable security vulnerabilities were identified in the added code.
Risk: Low risk. The change introduces an opt-in local hook rather than a public endpoint or authorization boundary, and it does not add unsafe file, SQL, subprocess, or network handling paths.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughA new file ChangesShackleGuard tool hook
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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: 3
🤖 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/hooks/shackle_guard.py`:
- Around line 70-71: The timeout countdown is currently initialized in the
__init__ method when the guard is registered, but it should only start when the
guard is actually invoked. Remove the self._start_time initialization from
__init__ and instead initialize it lazily in the __call__ method on the first
invocation by checking if self._start_time is None or not set, then setting it
to time.time() at that point. This ensures the timeout period only begins
counting from the first actual tool call, not from registration time.
- Around line 132-145: The budget exhaustion check in the shackle_guard.py file
currently only blocks when remaining budget is less than or equal to zero, which
allows one additional call to execute and cause overspending when 0 < remaining
< cost. Update the condition in the if statement from `remaining <= 0` to
`remaining < cost` so that it properly prevents execution when the remaining
budget cannot cover the estimated cost of the current call being evaluated.
- Around line 173-184: The response variable from request_human_input() may be
None or request_human_input() may raise an exception, causing response.lower()
to fail and allowing tool execution to proceed unblocked. Wrap the
request_human_input call in a try-except block to catch any exceptions, and add
defensive checks: if response is None or if any exception occurs, treat it as a
denial by returning False immediately. This ensures the HITL guard fails closed
on errors and prevents bypassing the security check through exceptions or null
returns.
🪄 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: adb1af6a-bb45-4c01-b69a-357614943b88
📒 Files selected for processing (1)
lib/crewai/src/crewai/hooks/shackle_guard.py
| self._start_time: float = time.time() | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Timeout starts at registration time instead of first tool call.
Line 70 initializes session timing in __init__. If the guard is registered long before tool execution, Line 122 can trip timeout before the first call. Initialize start time lazily on first __call__.
Proposed fix
- self._start_time: float = time.time()
+ self._start_time: float | None = None
...
# Layer 2: Timeout
- elapsed = time.time() - self._start_time
+ if self._start_time is None:
+ self._start_time = time.time()
+ elapsed = time.time() - self._start_time
if elapsed > self.timeout_seconds:Also applies to: 121-123
🤖 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/hooks/shackle_guard.py` around lines 70 - 71, The
timeout countdown is currently initialized in the __init__ method when the guard
is registered, but it should only start when the guard is actually invoked.
Remove the self._start_time initialization from __init__ and instead initialize
it lazily in the __call__ method on the first invocation by checking if
self._start_time is None or not set, then setting it to time.time() at that
point. This ensures the timeout period only begins counting from the first
actual tool call, not from registration time.
| cost = self._cost_estimate(context) | ||
| remaining = self.budget - self._budget_spent | ||
| if remaining <= 0: | ||
| self._circuit_tripped = True | ||
| self._circuit_reason = ( | ||
| f"Budget exhausted: ${self._budget_spent:.4f} / ${self.budget:.2f}" | ||
| ) | ||
| print( | ||
| f"\n💰 SHACKLE BUDGET EXHAUSTED: " | ||
| f"${self._budget_spent:.4f} / ${self.budget:.2f}\n" | ||
| f" Circuit opened. All further calls blocked." | ||
| ) | ||
| return False | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Budget guard currently allows overspending by one call.
At Line 134, blocking only when remaining <= 0 means a call still executes when 0 < remaining < cost, which exceeds the configured pre-execution budget cap.
Proposed fix
# Layer 3: Budget
cost = self._cost_estimate(context)
remaining = self.budget - self._budget_spent
- if remaining <= 0:
+ if remaining < cost:
self._circuit_tripped = True
self._circuit_reason = (
- f"Budget exhausted: ${self._budget_spent:.4f} / ${self.budget:.2f}"
+ f"Budget exhausted: projected ${self._budget_spent + cost:.4f} > ${self.budget:.2f}"
)
print(
f"\n💰 SHACKLE BUDGET EXHAUSTED: "
- f"${self._budget_spent:.4f} / ${self.budget:.2f}\n"
+ f"projected ${self._budget_spent + cost:.4f} > ${self.budget:.2f}\n"
f" Circuit opened. All further calls blocked."
)
return False🤖 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/hooks/shackle_guard.py` around lines 132 - 145, The
budget exhaustion check in the shackle_guard.py file currently only blocks when
remaining budget is less than or equal to zero, which allows one additional call
to execute and cause overspending when 0 < remaining < cost. Update the
condition in the if statement from `remaining <= 0` to `remaining < cost` so
that it properly prevents execution when the remaining budget cannot cover the
estimated cost of the current call being evaluated.
|
Really like this the tri-state proceed/review/block as a Three things from working on this exact pattern, in case they're useful: 1.) The fail-closed fix CodeRabbit flagged on the HITL path is the load-bearing 2. In-process hooks share a trust boundary with what they're guarding. 3. Loop detection by exact tool+params hash is evadable by decomposition. An None of this detracts from the PR clean, dependency-free, sound layering. I've |
Per @LOLA0786 review: a guard whose failure mode is 'allow' isn't a guard. Wraps entire __call__ in try/except that trips circuit on any error. HITL path also fails closed — if terminal is disconnected, block execution.
|
@LOLA0786 excellent review — you nailed the two critical issues. On fail-closed as invariant (point 1): you're absolutely right. I just pushed a fix that wraps the entire def __call__(self, context):
try:
# ... existing guard logic
return None # allow
except Exception as e:
# Fail-closed: any guard error = circuit tripped
self._circuit_tripped = True
self._circuit_reason = f"Guard error (fail-closed): {e}"
print(f"\n⛓️ SHACKLE FAIL-CLOSED: {e}\n Circuit opened for safety.")
return False # block executionOn making every layer fail-closed — agreed. Each guard layer now fails to DENY, not ALLOW. The guard's survival mode is "block everything" because the alternative ("allow everything") means the guard isn't a guard at all. On the HITL path specifically (point 3): I added an Thanks for the review — would you be open to a more detailed async discussion on how you've implemented this pattern? Happy to share the standalone SHACKLE repo for reference: https://github.com/Fame510/SHACKLE-PRO- |
2 participants
Summary
Adds
shackle_guard.py— a lightweight, self-contained pre-execution circuit breaker that integrates with crewAI's existing tool hook system.One line to activate:
What it does
Sits between the agent and tool execution as a
before_tool_callhook. ReturnsFalseto block execution,Noneto allow, or triggersrequest_human_input()for HITL approval.Five guard layers:
request_human_input()for high-risk toolsError amplification: When 401/403/500/timeout signals are detected in tool input, the repeat limit tightens automatically — catching the "loop of death" pattern where an agent retries a failing API call indefinitely.
Motivation
Related to #6025 — the community has been discussing the need for a runtime release-control mediation layer. This PR implements the tri-state PROCEED / NEEDS_REVIEW / BLOCK pattern as a before_tool_call hook.
Example
Summary by CodeRabbit
New Features
Bug Fixes