GuardrailProvider — content-addressed decision audit chain for tool call authorization#6597
GuardrailProvider — content-addressed decision audit chain for tool call authorization#6597Correctover wants to merge 4 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
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)
📝 WalkthroughWalkthroughThe PR adds a guardrails package with immutable authorization decisions, content-addressed integrity checks, built-in providers, tool-call hook enforcement, in-memory auditing, configuration validation, public exports, and end-to-end tests. ChangesGuardrails subsystem
Sequence Diagram(s)sequenceDiagram
participant ToolCallHook
participant GuardrailContext
participant GuardrailProvider
participant AuditTrail
ToolCallHook->>GuardrailContext: before_tool_call(context)
GuardrailContext->>GuardrailProvider: authorize(context)
GuardrailProvider-->>GuardrailContext: GuardrailDecisionV1
GuardrailContext->>AuditTrail: record_decision(decision)
GuardrailContext-->>ToolCallHook: None or False
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
src/crewai/guardrails/guardrail_provider.py (2)
386-396: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winMisleading docstring: "Thread-safe for single-threaded crew execution."
This is self-contradictory and
AuditTrailhas no locking aroundself._decisions/self._envelopes. If tool-call hooks ever fire from more than one thread (parallel agent/tool execution), concurrentrecord_decision/record_envelopecalls on the plain dicts can race. As per path instructions to document public APIs and complex logic accurately, either clarify that this class is not safe for concurrent access, or add a lock.🤖 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 `@src/crewai/guardrails/guardrail_provider.py` around lines 386 - 396, Correct the misleading thread-safety documentation in AuditTrail and align it with the implementation. Since _decisions and _envelopes are plain dictionaries without locking, document that the class is not safe for concurrent access rather than claiming thread safety; leave the storage behavior unchanged.Source: Path instructions
352-374: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
param_matchesdocstring claims regex support that isn't implemented.The predicate table docstring says
param_matches : context.tool_input[key] matches regex/value(Line 356), but the implementation only does an equality check (==, Line 371). Consumers relying on the docs to write regex-based constraints will get silently wrong (never-matching) behavior. As per path instructions to document complex logic accurately, either fix the docstring or implement regex support.🤖 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 `@src/crewai/guardrails/guardrail_provider.py` around lines 352 - 374, Align the `param_matches` documentation and implementation: update the built-in predicate table and its `param_matches` callable so regex constraints are actually supported, while preserving equality matching for literal values. Ensure the behavior matches the documented regex/value contract without changing the other predicates.Source: Path instructions
🤖 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 `@src/crewai/guardrails/guardrail_provider.py`:
- Around line 58-65: Ensure every decision ID is computed from the same preimage
that verify_integrity() validates: pass each decision’s expires_at to
compute_decision_id in AllowAllGuardrailProvider, DenyAllGuardrailProvider,
ToolListGuardrailProvider, CKGGuardrailProvider, and the documented example,
while preserving the existing claims and expiry values on GuardrailDecisionV1.
- Around line 316-330: Update CKGGuardrailProvider.authorize so
constraint_results preserves every predicate evaluation, including duplicate
predicate names with different parameters. Key each result by the constraint’s
iteration index (or another guaranteed-unique constraint identifier) instead of
predicate_name, while leaving all_satisfied calculation and predicate evaluation
unchanged.
- Around line 504-526: Update make_guardrail_hook so the returned hook is a
plain wrapper function that delegates to ctx.before_tool_call, then attach ctx
through the wrapper’s context attribute; preserve the existing provider, trail,
and on_deny initialization and return the callable hook without assigning
attributes to the bound method.
In `@tests/guardrails/test_guardrail_provider.py`:
- Around line 490-499: Update test_decision_integrity_in_hook to obtain the
decision through AuditTrail’s public all_decisions() API instead of accessing
ctx.trail._decisions. Preserve the existing single-decision lookup and integrity
assertion.
---
Nitpick comments:
In `@src/crewai/guardrails/guardrail_provider.py`:
- Around line 386-396: Correct the misleading thread-safety documentation in
AuditTrail and align it with the implementation. Since _decisions and _envelopes
are plain dictionaries without locking, document that the class is not safe for
concurrent access rather than claiming thread safety; leave the storage behavior
unchanged.
- Around line 352-374: Align the `param_matches` documentation and
implementation: update the built-in predicate table and its `param_matches`
callable so regex constraints are actually supported, while preserving equality
matching for literal values. Ensure the behavior matches the documented
regex/value contract without changing the other predicates.
🪄 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: e9890c2d-c325-4799-a43a-a62eaa1b400d
📒 Files selected for processing (3)
src/crewai/guardrails/__init__.pysrc/crewai/guardrails/guardrail_provider.pytests/guardrails/test_guardrail_provider.py
| def authorize(self, context: ToolCallHookContext) -> GuardrailDecisionV1: | ||
| results: dict[str, bool] = {} | ||
| all_satisfied = True | ||
|
|
||
| for predicate_name, params in self._constraints: | ||
| ok = self._eval_predicate(predicate_name, context, **params) | ||
| results[predicate_name] = ok | ||
| if not ok: | ||
| all_satisfied = False | ||
|
|
||
| claims = { | ||
| "tool": context.tool_name, | ||
| "agent": getattr(context.agent, "role", "unknown"), | ||
| "constraint_results": results, | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Duplicate predicate names silently overwrite audit results in CKGGuardrailProvider.
results is keyed by predicate_name alone (Line 317-322). If the same predicate is used twice with different params (e.g. two has_param checks for different keys), the later call's boolean overwrites the earlier one in results, so the recorded claims["constraint_results"] loses fidelity even though all_satisfied itself is computed correctly. Since this is an audit trail whose whole purpose is faithful record-keeping, losing per-constraint results for duplicate predicate names undermines that guarantee.
♻️ Proposed fix — key by constraint index instead of predicate name
- results: dict[str, bool] = {}
+ results: dict[str, bool] = {}
all_satisfied = True
- for predicate_name, params in self._constraints:
+ for i, (predicate_name, params) in enumerate(self._constraints):
ok = self._eval_predicate(predicate_name, context, **params)
- results[predicate_name] = ok
+ results[f"{predicate_name}#{i}"] = ok
if not ok:
all_satisfied = False📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def authorize(self, context: ToolCallHookContext) -> GuardrailDecisionV1: | |
| results: dict[str, bool] = {} | |
| all_satisfied = True | |
| for predicate_name, params in self._constraints: | |
| ok = self._eval_predicate(predicate_name, context, **params) | |
| results[predicate_name] = ok | |
| if not ok: | |
| all_satisfied = False | |
| claims = { | |
| "tool": context.tool_name, | |
| "agent": getattr(context.agent, "role", "unknown"), | |
| "constraint_results": results, | |
| } | |
| def authorize(self, context: ToolCallHookContext) -> GuardrailDecisionV1: | |
| results: dict[str, bool] = {} | |
| all_satisfied = True | |
| for i, (predicate_name, params) in enumerate(self._constraints): | |
| ok = self._eval_predicate(predicate_name, context, **params) | |
| results[f"{predicate_name}#{i}"] = ok | |
| if not ok: | |
| all_satisfied = False | |
| claims = { | |
| "tool": context.tool_name, | |
| "agent": getattr(context.agent, "role", "unknown"), | |
| "constraint_results": results, | |
| } |
🤖 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 `@src/crewai/guardrails/guardrail_provider.py` around lines 316 - 330, Update
CKGGuardrailProvider.authorize so constraint_results preserves every predicate
evaluation, including duplicate predicate names with different parameters. Key
each result by the constraint’s iteration index (or another guaranteed-unique
constraint identifier) instead of predicate_name, while leaving all_satisfied
calculation and predicate evaluation unchanged.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/crewai/guardrails/__init__.py`:
- Around line 14-46: Remove canonical_json from the import list and __all__ in
the guardrails package initializer. Keep the remaining guardrail exports
unchanged so importing src.crewai.guardrails no longer references the undefined
symbol.
🪄 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: 470ec622-76f9-4fef-8c19-4390c295b67a
📒 Files selected for processing (1)
src/crewai/guardrails/__init__.py
Remove ToolCallHookContext and canonical_json from imports and __all__. These symbols are not defined in guardrail_provider.py and cause ImportError.
…results) - All 4 providers now pass expires_at to compute_decision_id (anti-tamper) - Docstring example fixed to match - CKGGuardrailProvider results: dict→list[tuple] to prevent overwrite - make_guardrail_hook: wrapper fn instead of bound method attr assignment - Brand attribution already present (CCS/Correctover)
…deRabbit) - Replace ctx.trail._decisions with ctx.trail.all_decisions()[0] - Tests should use public API, not implementation details
67baa2d to
9de0ddd
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/crewai/guardrails/guardrail_provider.py`:
- Around line 325-352: Update the denial-reason comprehension in authorize to
iterate over the list of (predicate_name, ok) tuples directly instead of calling
results.items(). Preserve duplicate entries and include every failed predicate
name while keeping the authorized decision behavior unchanged.
🪄 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: 04975142-3f9f-4863-8017-1e5948b87337
📒 Files selected for processing (3)
src/crewai/guardrails/__init__.pysrc/crewai/guardrails/guardrail_provider.pytests/guardrails/test_guardrail_provider.py
🚧 Files skipped from review as they are similar to previous changes (1)
- src/crewai/guardrails/init.py
1 participant
Summary
Implements
GuardrailProvider— a content-addressed decision audit chain for tool call authorization, integrating through crewAI's existingBeforeToolCallHookinfrastructure (register_before_tool_call_hook).This PR is aligned with the converged direction from #4877 (safal207's
GuardrailDecisionV1spec, babyblueviper1's independent recompute verification, Yarmoluk's CKG declarative authorization).1. Problem
crewAI agents can register arbitrary tools via
@tooland execute them throughCrewStructuredTool. Today there is no built-in mechanism to:Without these, a multi-agent crew has no way to express "agent A can read files, agent B cannot" at the framework layer, and no cryptographic guarantee that an authorization decision hasn't been tampered with after issuance.
2. Design
Core data types
decision_id algorithm (deterministic, content-addressed):
claims ∪ {"_expires_at": expires_at}(if expires_at is set)json.dumps(preimage, sort_keys=True, separators=(",", ":"))SHA-256(canonical_bytes).hexdigest()This means:
verify_integrity()returns FalseProvider protocol
Reference implementations included:
AllowAllGuardrailProvider— permissive defaultDenyAllGuardrailProvider— safety lockToolListGuardrailProvider— allowlist/blocklist by tool nameCKGGuardrailProvider— constraint-based declarative authorization (6 built-in predicates:tool_name_in,tool_name_not_in,agent_role_in,param_matches,has_param,no_param)Audit trail
An in-memory
AuditTrailrecords every decision and envelope bydecision_id. Each decision is self-verifying:decision.verify_integrity()recomputes the hash and asserts match.3. Integration
One line to register:
The hook:
provider.authorize(context)with the liveToolCallHookContextGuardrailDecisionV1in the audit trailFalseto block execution if not authorized,Noneto allowPost-execution evidence capture (via
register_after_tool_call_hook) is prepared but not wired by default — it's available asGuardrailContext.after_tool_callfor users who need the full decision + envelope chain.4. Verification
Tests
Engine-v4 independent validation
The Correctover engine-v4 scanner includes AS-GUARDRAIL-MISS-001, a detection seed that flags agents and tools operating without any registered GuardrailProvider — i.e., missing runtime authorization entirely.
The scanner ran against this PR's code and confirmed that every
GuardrailDecisionV1is self-verifying and thatverify_integrity()catches both tampered claims and tampered expiry.5. Discussion points
safal207's GuardrailDecisionV1 spec. The
decision_id = SHA-256(canonical_json(claims ∪ expires_at))matches the spec's content-addressed approach.expires_atis included in the hash preimage, so post-issuance modification of either claims or expiry is detectable. The separation betweenGuardrailDecisionV1(pre-execution authorization) andActionEnvelopeV1(post-execution evidence) follows the spec's distinction.babyblueviper1's independent recompute. The
verify_integrity()method provides the recompute path — any holder of a decision can independently verify that itsdecision_idmatches its claims.Yarmoluk's CKG declarative authorization. The
CKGGuardrailProvidermaps constraint predicates to authorization outcomes through a simple evaluation engine. Built-in predicates cover common patterns (tool name matching, role-based access, parameter presence/value matching), and the constraint list is extensible at runtime viaadd_constraint().design choice: stateless protocol.
GuardrailProvideris a pure protocol — it receives context and returns a decision. No mutable state in the provider itself. Stateful concern (audit trail) is handled byGuardrailContextandAuditTrail, which are composable separately. This keeps providers testable and allows swapping between allowlist, CKG, or custom logic without changing the audit chain.not in this PR (future):
register_after_tool_call_hookAuditTrailis adequate for single-crew runs; a durable backend (SQLite, Redis) can be swapped in without changing the data typesActionEnvelopeV1model already capturestool_input_snapshotandtool_result_digest, which are the preconditions for idempotency dedup; a follow-up can add the dedup logicChecklist
GuardrailProviderprotocol + 4 reference implementationsdecision_idper [FEATURE] GuardrailProvider interface for pre-tool-call authorization #4877 spec (SHA-256, canonical JSON, expires_at in preimage)GuardrailDecisionV1/ActionEnvelopeV1separationCKGGuardrailProviderfor declarative constraint-based authorizationverify_integrity()— independent recompute for tamper detectionAuditTrail— in-memory decision + envelope storemake_guardrail_hook()— one-line integration withregister_before_tool_call_hookdetect_missing_guardrail()— programmatic gap analysisAS-GUARDRAIL-MISS-001)