fix: add before_llm_call_hooks to async acall() paths in all native providers - #6741
fix: add before_llm_call_hooks to async acall() paths in all native providers#6741Vinayakdwivedi wants to merge 2 commits into
Conversation
Add hook invocation to async paths in all 5 native LLM providers to close security gap where guardrails were bypassed on async calls. This mirrors protection already in sync call() methods.
|
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)
📝 WalkthroughWalkthroughAsync completion methods for Azure, Bedrock, Gemini, and OpenAI now enforce before-LLM hooks before requests proceed. Anthropic’s existing guard is reformatted without behavior changes. ChangesAsync before-LLM hook gating
Sequence Diagram(s)sequenceDiagram
participant ProviderCompletion
participant _invoke_before_llm_call_hooks
participant LLMRequest
ProviderCompletion->>_invoke_before_llm_call_hooks: formatted messages and from_agent
_invoke_before_llm_call_hooks-->>ProviderCompletion: allow or block
ProviderCompletion->>LLMRequest: continue async completion when allowed
ProviderCompletion-->>ProviderCompletion: raise ValueError when blocked
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
There was a problem hiding this comment.
Pull request overview
This PR closes a security gap where before_llm_call_hooks were not enforced on async LLM invocation paths (acall() / kickoff_async) for all native providers, ensuring hook-based guardrails apply consistently across sync and async calls.
Changes:
- Added
before_llm_call_hooksinvocation (matching existingcall()logic) to each provider’sacall()method. - Ensured hook blocking behavior is consistent by raising
ValueErrorwhen a hook returnsFalse. - Normalized the hook input for Gemini’s async path to match the existing sync behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/crewai/src/crewai/llms/providers/openai/completion.py | Enforces before_llm_call_hooks for async OpenAI calls to prevent bypass. |
| lib/crewai/src/crewai/llms/providers/gemini/completion.py | Enforces before_llm_call_hooks for async Gemini calls using the same hook message shape as sync. |
| lib/crewai/src/crewai/llms/providers/bedrock/completion.py | Enforces before_llm_call_hooks for async Bedrock Converse calls. |
| lib/crewai/src/crewai/llms/providers/azure/completion.py | Enforces before_llm_call_hooks for async Azure calls to prevent bypass. |
| lib/crewai/src/crewai/llms/providers/anthropic/completion.py | Enforces before_llm_call_hooks for async Anthropic calls to prevent bypass. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ) | ||
|
|
||
| if not self._invoke_before_llm_call_hooks( |
| raise ValueError("LLM call blocked by before_llm_call hook") | ||
|
|
||
| completion_params = self._prepare_completion_params( |
| if not self._invoke_before_llm_call_hooks( | ||
| formatted_messages, from_agent | ||
| ): | ||
| raise ValueError("LLM call blocked by before_llm_call hook") |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (3)
lib/crewai/src/crewai/llms/providers/azure/completion.py:609
- There is trailing whitespace on this blank line, which is likely to trip linters/formatters and creates noisy diffs. Remove the whitespace-only line (or make it a truly empty line).
lib/crewai/src/crewai/llms/providers/anthropic/completion.py:451 - This blank line contains trailing whitespace. Removing it avoids unnecessary lint/format churn and keeps diffs clean.
lib/crewai/src/crewai/llms/providers/openai/completion.py:606 - This change fixes a security-critical bypass on the async path, but there doesn't appear to be an automated test asserting that a registered before_llm_call hook can block
acall()before any provider request is issued (e.g., by stubbing/patching the async client). Adding a unit test for this prevents regressions across future refactors.
if not self._invoke_before_llm_call_hooks(
formatted_messages, from_agent
):
raise ValueError("LLM call blocked by before_llm_call hook")
Description
This PR fixes a critical security vulnerability where
before_llm_call_hooksare bypassed when using asynchronous LLM calls (acall()orkickoff_async).The Problem
The
before_llm_call_hooksmechanism is designed to intercept and potentially block LLM requests based on custom logic before they reach the API provider. This is essential for:PII (Personally Identifiable Information) filtering — blocking prompts containing sensitive data
Spend caps and cost control — preventing runaway costs
Policy enforcement — ensuring compliance with organizational rules
Prompt injection detection — catching malicious input patterns
Custom guardrails — user-defined security checks
The vulnerability: These hooks are invoked correctly in the synchronous
call()method, but are completely missing from the asynchronousacall()method in all 5 native LLM providers. This means:Guardrails that work on sync paths silently fail on async paths
Sensitive data can leak to the provider when using async
Cost controls are bypassed for async calls
Policy checks don't apply to async operations
Impact
call()acall()The Solution
Add the same hook invocation logic from each provider's
call()method to itsacall()method. This ensures both sync and async paths have identical protection.Example pattern (Anthropic):
Changes Made
Files Modified
llms/providers/anthropic/completion.pyacall()method after message formattingcall()methodllms/providers/azure/completion.pyacall()method after message formattingcall()methodllms/providers/bedrock/completion.pyacall()method after message formattingcall()methodllms/providers/gemini/completion.pyacall()method after message formattingcall()methodllms/providers/openai/completion.pyacall()method after message formattingcall()methodWhat Changed in Each File
For each provider's
acall()method:call())call()No changes to:
Testing
Manual Test Case
The fix can be verified using the reproduction case from the original issue:
Expected output after fix:
Before this PR (broken behavior):
Existing Tests
All existing async tests pass without modification, confirming this is a transparent fix (no breaking changes).
Why This Is Safe
✅ Minimal change: One hook invocation block per provider (mirrors existing
call()logic)✅ No logic changes: Reuses the exact pattern already working in
call()methods✅ Backward compatible: Normal users see no change in behavior; only those using hooks benefit
✅ No performance impact: Hook invocation happens before async operations (synchronous check)
✅ Follows existing pattern: Consistent with LiteLLM fallback which already has this protection (see
llms/base_llm.py)✅ Type-safe: No changes to method signatures or type hints
Real-World Example
PII Filter Hook (Before Fix)
After This PR
Breaking Changes
None. This is a pure security fix that:
Related Issues
before_llm_call_hooksbypassllms/base_llm.py(LiteLLM fallback)Checklist
call()methodType of Change
How to Test
Reviewers Notes
This is a straightforward security fix with minimal risk:
call()methodThis protects all users who rely on hooks for security, guardrails, or compliance.