feat(aios-protocol): add tier-specific PolicySet constructors#2
feat(aios-protocol): add tier-specific PolicySet constructors#2
Conversation
…ous/free/pro/enterprise) Adds anonymous(), free(), pro(), and enterprise() constructors to PolicySet for BRO-211. Each tier sets appropriate allow/gate capability lists and per-turn event/runtime limits. Includes unit tests for all four tiers. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdded four public constructor functions to Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/aios-protocol/src/policy.rs`:
- Around line 186-189: The assertions involving ps.allow_capabilities and
ps.gate_capabilities
(assert!(ps.allow_capabilities.contains(&Capability::new("net:egress:*"))); and
assert!(ps.gate_capabilities.contains(&Capability::new("exec:cmd:*")));) are
failing cargo fmt checks; run rustfmt by executing cargo fmt --all and update
the commit with the formatted changes so the file aios-protocol/src/policy.rs
(including the assert! lines referencing Capability::new) matches rustfmt style
and CI passes.
- Around line 63-65: PolicySet defines tier limits (max_tool_runtime_secs,
max_events_per_turn) but they are dropped in StaticPolicyEngine::from_policy_set
and never propagated to ToolExecutionRequest or enforced by SandboxRunner which
uses hardcoded defaults; update StaticPolicyEngine::from_policy_set to store
these limit fields, extend the policy-to-execution path to populate
ToolExecutionRequest (or an execution-limits struct) with max_tool_runtime_secs
and max_events_per_turn, modify SandboxRunner to read and enforce those limits
instead of hardcoded values, and add unit/integration tests that assert the
runner respects policy-specified time and event limits (and/or remove the fields
if you decide not to support them).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d557b05f-b796-4602-9ccf-a191da3a3ae5
📒 Files selected for processing (1)
crates/aios-protocol/src/policy.rs
| max_tool_runtime_secs: 30, | ||
| max_events_per_turn: 5, | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether tier limit fields are consumed beyond struct initialization.
# Expected: usages in enforcement/runtime code paths, not only in constructors/tests.
rg -n -C3 '\bmax_tool_runtime_secs\b|\bmax_events_per_turn\b' --type=rust
rg -n -C3 'from_policy_set\s*\(' --type=rustRepository: broomva/aiOS
Length of output: 12726
🏁 Script executed:
# Search for any enforcement logic that might use these tier limits
rg -n 'max_tool_runtime_secs|max_events_per_turn' --type=rust -A 2 -B 2 | head -100
# Also search for where runtime constraints are actually enforced
rg -n 'timeout|duration|runtime.*limit|event.*count' --type=rust -i | grep -i 'enforce\|check\|validate' | head -20
# Check if kernel or runtime code enforces these limits
rg -n 'StaticPolicyEngine|SessionPolicyEngine' --type=rust -A 5 | head -50Repository: broomva/aiOS
Length of output: 9393
🏁 Script executed:
# Search kernel for timeout/duration enforcement
rg -n 'timeout|duration|max_tool|max_events' crates/aios-kernel/src/ --type=rust
# Search sandbox for runtime constraint enforcement
rg -n 'timeout|duration|max_tool|max_events' crates/aios-sandbox/src/ --type=rust 2>/dev/null || echo "No sandbox enforcement found"
# Check if there's a separate timeout/constraint mechanism in runtime
fd -e rs . crates/ -x grep -l 'timeout\|duration' {} \; | head -10Repository: broomva/aiOS
Length of output: 1301
🏁 Script executed:
# Check sandbox module to see what timeout limit is used
sed -n '100,110p' crates/aios-sandbox/src/lib.rs
# Search for where execute() is called and what limit is passed
rg -n '\.execute\(' --type=rust -B 3 -A 3 | head -50
# Check if sandbox takes policy as parameter
rg -n 'impl.*Sandbox|pub.*execute|fn execute' crates/aios-sandbox/src/lib.rs -A 5 | head -40Repository: broomva/aiOS
Length of output: 3023
🏁 Script executed:
# Find ToolExecutionRequest struct definition
rg -n 'struct ToolExecutionRequest' --type=rust -A 10
# Find where ToolExecutionRequest is created in runtime
rg -n 'ToolExecutionRequest {' crates/aios-runtime/src/lib.rs -A 10 | head -30
# Check if limits field references PolicySet anywhere
rg -n 'limits.*policy|policy.*limits' --type=rustRepository: broomva/aiOS
Length of output: 876
🏁 Script executed:
# Final check: see if SandboxLimits is ever set from policy
rg -n 'SandboxLimits' crates/ --type=rust -B 2 -A 2 | grep -E 'policy|PolicySet|max_tool|max_events' || echo "No connection found"
# Verify ToolExecutionRequest definitely doesn't have limits
cat crates/aios-protocol/src/ports.rs | sed -n '83,88p'Repository: broomva/aiOS
Length of output: 273
Tier limit fields are not enforced anywhere downstream.
PolicySet configures max_tool_runtime_secs and max_events_per_turn, but StaticPolicyEngine::from_policy_set() (crates/aios-policy/src/lib.rs:61–66) discards them, storing only allow/gate capabilities. ToolExecutionRequest has no limits field, and SandboxRunner uses hardcoded defaults (30 seconds), completely bypassing policy-driven limits.
Either remove these unused fields or implement the full enforcement path with tests proving the limits are applied. Per coding guidelines, "Add or extend tests with every behavioral change; do not silently widen capabilities or safety boundaries."
Also applies to: 81-83, 92-94, 103-105
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/aios-protocol/src/policy.rs` around lines 63 - 65, PolicySet defines
tier limits (max_tool_runtime_secs, max_events_per_turn) but they are dropped in
StaticPolicyEngine::from_policy_set and never propagated to ToolExecutionRequest
or enforced by SandboxRunner which uses hardcoded defaults; update
StaticPolicyEngine::from_policy_set to store these limit fields, extend the
policy-to-execution path to populate ToolExecutionRequest (or an
execution-limits struct) with max_tool_runtime_secs and max_events_per_turn,
modify SandboxRunner to read and enforce those limits instead of hardcoded
values, and add unit/integration tests that assert the runner respects
policy-specified time and event limits (and/or remove the fields if you decide
not to support them).
Adds anonymous/free/pro/enterprise constructors to PolicySet. Part of BRO-211.
Changes
PolicySet::anonymous()— fs:read only, shell/write/net gated, 5 events/turnPolicySet::free()— fs:read + net allowed, write/exec/secrets gated, 15 events/turnPolicySet::pro()— allow all (*), 50 events/turnPolicySet::enterprise()— allow all (*), 200 events/turnTests
4 new unit tests verify correct allow/gate counts, that anonymous cannot exec, that pro/enterprise allow all via wildcard. All 90 tests pass, zero clippy warnings.
Co-Authored-By: Claude Sonnet 4.6 noreply@anthropic.com
Summary by CodeRabbit
New Features
Tests