Unify DefaultPolicyEngine decision and explanation rule chain - #286
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors DefaultPolicyEngine so both authorization decisions (evaluate()) and denial explanations (explain()) are driven by a single canonical ordered rule chain, eliminating the prior duplicated condition trees. It also adds a read-only rate-limit path (RateLimiter.peek()) so explanations can predict rate-limit denials without mutating limiter state, and adds regression tests to enforce evaluate/explain agreement and strict no-mutation behavior.
Changes:
- Introduce a shared
_DEFAULT_RULESrule chain (DefaultPolicyRuleChain) and routeDefaultPolicyEngine.evaluate()/.explain()through it (short-circuit vs collect-all). - Add
RateLimiter.peek()and use it for read-only rate-limit evaluation during explanation. - Add targeted tests for decision/explanation agreement and read-only invariants; update docs and contributor guidance accordingly.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_policy_rule_chain.py | Adds agreement + read-only invariant tests to prevent drift and limiter mutation in explain(). |
| src/weaver_kernel/rate_limit.py | Adds RateLimiter.peek() for non-mutating rate-limit prediction. |
| src/weaver_kernel/policy.py | Replaces duplicated evaluate/explain logic with shared chain traversal; preserves trace + reason-code behavior. |
| src/weaver_kernel/default_policy_rules.py | Defines canonical ordered _DEFAULT_RULES and DefaultPolicyRuleChain.run() traversal. |
| src/weaver_kernel/default_policy_rule_types.py | Adds shared context/result/failure datatypes and shared constants for the rule chain. |
| src/weaver_kernel/default_policy_limit_rules.py | Implements row-cap and rate-limit rules, including read-only limiter behavior. |
| src/weaver_kernel/default_policy_access_rules.py | Implements safety/sensitivity/secrets/memory access rules as chain components. |
| docs/architecture.md | Documents the unified chain and read-only rate-limit explanation behavior. |
| CHANGELOG.md | Notes the behavior-preserving refactor and new agreement/no-mutation coverage. |
| AGENTS.md | Updates “Adding a policy rule” guidance to register rules once in _DEFAULT_RULES + add agreement coverage. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
dgenio
pushed a commit
that referenced
this pull request
Aug 17, 2026
Red-team follow-up (PR #259 review by dgenio): - enforce_arg_constraints now validates inner rule values, not just the outer containers: a non-string `prefix` value (e.g. {"prefix":{"path":123}}) and a non-string / unhashable `allowed_keys` element (e.g. a nested list) previously reached value.startswith()/set(allowed) and raised an untyped TypeError that escaped the audited denial path. Both now fail closed with TokenScopeError(arg_constraint_violation). Regression cases added. Rebase integration onto current main (was 6 commits behind): - Reconciled with the #286 shared policy rule-chain refactor: DefaultPolicyEngine keeps max_ttl_s (#203) on top of the new DefaultPolicyRuleChain structure. - Merged #284 (actionable secret warning) with the #185 rotation keyring in _secrets.py; merged docs/security.md and CHANGELOG. - coding_agent_demo.py (#253) now imports HMACTokenProvider from _hmac_provider, since tokens.py no longer re-exports it (cycle fix). - CHANGELOG lifecycle entries moved under [Unreleased]; dropped the stale "removed rate-limit aliases" note (main keeps them) and the stale re-export claim. make ci green: 912 passed, 1 skipped. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019WRxQL8t2Uusa6845jWtVV
dgenio
added a commit
that referenced
this pull request
Aug 19, 2026
* feat: harden the capability-token lifecycle — rotation, TTL, invoke-time enforcement Groups six issues that share one implementation path (a CapabilityToken from issuance through invoke-time enforcement) and one code area. - #185 Signing-key rotation: HMACTokenProvider(secrets={key_id: secret}, active_key_id=...) verifies previous-key tokens during an overlap window; key_id is signed into the payload; unknown key id fails closed. New WEAVER_KERNEL_SECRETS / WEAVER_KERNEL_ACTIVE_KEY env resolution. - #200 CapabilityToken.from_dict raises typed TokenInvalid on malformed input instead of leaking KeyError/ValueError. - #203 Per-grant TTL: grant_capability(ttl_s=...) with DefaultPolicyEngine( max_ttl_s=...); non-positive or over-max is denied (not clamped), audited. - #183 Signed argument constraints (constraints["args"]: allowed_keys/pinned/ prefix) enforced at invoke() and invoke_stream() before the driver runs, raising TokenScopeError with a failure trace; dry-run predicts the same. - #170 Opt-in per-invocation rate limiting (Kernel(invoke_rate_limits=...)), default off, race-free check-then-record; dry-run never consumes. - #224 ADR docs/adr/0001-token-signing-evolution.md (HMAC vs macaroon vs Biscuit, measured); recommends HMAC + re-issuance now. No code/deps change. To stay within the AGENTS.md 300-line module budget, HMACTokenProvider moved to _hmac_provider.py (re-exported; logger name unchanged) and helpers were split into _token_signing.py, policy_ttl.py, kernel/_grant.py, kernel/_constraints.py. Removed the private policy._DEFAULT_RATE_LIMITS/_SERVICE_RATE_MULTIPLIER aliases (part of #196). Breaking: the signed payload now includes key_id, so pre-upgrade tokens no longer verify (accepted pre-1.0; legacy config unchanged). make ci: fmt-check, lint, mypy --strict (67 files), 856 passed / 1 skipped, 94% branch coverage, all 13 examples. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019WRxQL8t2Uusa6845jWtVV * fix: break token-module import cycle (CodeQL) and harden fail-closed inputs Addresses PR #259 review: - CodeQL cyclic-import errors: the tokens <-> _hmac_provider re-export and the tokens <-> _token_signing TYPE_CHECKING import formed cycles. Made the graph acyclic — _token_signing no longer references tokens (build_signable_payload inlined into CapabilityToken._signable_payload), and tokens no longer re-exports HMACTokenProvider. Importers (__init__, kernel, cli/_doctor) now import HMACTokenProvider from _hmac_provider; the public `from weaver_kernel import HMACTokenProvider` is unchanged. - Copilot: from_dict now coerces naive ISO timestamps to UTC, so a malformed token can't cause a naive-vs-aware TypeError at verify() time. - Copilot: DefaultPolicyEngine(max_ttl_s=...) now rejects non-SafetyClass dict keys at construction (fail closed instead of silently ignoring the cap). - Copilot: enforce_arg_constraints fails closed (TokenScopeError, arg_constraint_violation) on malformed args.allowed_keys/pinned/prefix types instead of raising an untyped TypeError or silently ignoring them. Tests added for each. make ci green: 861 passed, 1 skipped, 94% branch coverage. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019WRxQL8t2Uusa6845jWtVV * fix: close remaining arg-constraint type gaps; rebase onto main Red-team follow-up (PR #259 review by dgenio): - enforce_arg_constraints now validates inner rule values, not just the outer containers: a non-string `prefix` value (e.g. {"prefix":{"path":123}}) and a non-string / unhashable `allowed_keys` element (e.g. a nested list) previously reached value.startswith()/set(allowed) and raised an untyped TypeError that escaped the audited denial path. Both now fail closed with TokenScopeError(arg_constraint_violation). Regression cases added. Rebase integration onto current main (was 6 commits behind): - Reconciled with the #286 shared policy rule-chain refactor: DefaultPolicyEngine keeps max_ttl_s (#203) on top of the new DefaultPolicyRuleChain structure. - Merged #284 (actionable secret warning) with the #185 rotation keyring in _secrets.py; merged docs/security.md and CHANGELOG. - coding_agent_demo.py (#253) now imports HMACTokenProvider from _hmac_provider, since tokens.py no longer re-exports it (cycle fix). - CHANGELOG lifecycle entries moved under [Unreleased]; dropped the stale "removed rate-limit aliases" note (main keeps them) and the stale re-export claim. make ci green: 912 passed, 1 skipped. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019WRxQL8t2Uusa6845jWtVV --------- Co-authored-by: Claude <noreply@anthropic.com>
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
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.
What changed
DefaultPolicyEngine.evaluate()/.explain()condition trees with one ordered_DEFAULT_RULESchainRateLimiter.peek()so explanation can predict rate-limit denials without creating, pruning, or consuming limiter stateexplain()Verification
Authoritative
make cipassed on the final architecture:The new policy modules are all below the mandatory 300-line ceiling.
policy.pyitself is reduced substantially by removing the two hand-maintained rule copies.Closes #219.