feat: harden the capability-token lifecycle — rotation, TTL, invoke-time enforcement#259
Open
dgenio wants to merge 2 commits into
Open
feat: harden the capability-token lifecycle — rotation, TTL, invoke-time enforcement#259dgenio wants to merge 2 commits into
dgenio wants to merge 2 commits into
Conversation
…ime 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
There was a problem hiding this comment.
Pull request overview
This PR hardens the CapabilityToken lifecycle across issuance, transport/deserialization, rotation, and invoke-time enforcement in weaver_kernel, while splitting previously ratcheted modules to stay within the repo’s ≤300-line guidance.
Changes:
- Add signing-key rotation support with signed
key_id, plus env-based multi-key configuration and logging for non-active key verification. - Add per-grant TTL (
ttl_s) withDefaultPolicyEngine(max_ttl_s=...)bounds and audited denials for invalid/excessive TTL requests. - Enforce signed argument constraints and optional invoke-time rate limiting for both
invoke()andinvoke_stream(), with docs + tests.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_tokens.py | Adds coverage for key rotation, env config precedence/fail-closed behavior, and typed from_dict validation errors. |
| tests/test_policy.py | Adds tests for max_ttl_s validation and resolution semantics. |
| tests/test_kernel.py | Adds tests for per-grant TTL, arg-constraint enforcement parity/auditing, and invoke-time rate limiting (including concurrency). |
| tests/test_architecture.py | Updates module size ratchets to reflect tokens.py split and minor kernel budget change. |
| src/weaver_kernel/tokens.py | Refactors to keep token dataclass + Protocol, re-exports HMACTokenProvider, and routes signing/parsing to helpers. |
| src/weaver_kernel/policy.py | Adds max_ttl_s to DefaultPolicyEngine configuration and removes private rate-limit aliases. |
| src/weaver_kernel/policy_ttl.py | Introduces validation + resolution helpers for per-grant TTL maximum configuration. |
| src/weaver_kernel/policy_reasons.py | Adds stable denial reason codes for TTL exceeded and arg constraint violations. |
| src/weaver_kernel/otel.py | Threads ttl_s through the grant instrumentation wrapper. |
| src/weaver_kernel/kernel/_grant.py | Extracts grant orchestration and implements per-grant TTL pre-validation + audited denial handling. |
| src/weaver_kernel/kernel/_constraints.py | Adds invoke-time arg constraints enforcement and optional invoke-time rate limiting with audited failure traces. |
| src/weaver_kernel/kernel/init.py | Wires in ttl_s, invoke-time enforcement, and kernel-level rate-limit configuration + clock injection. |
| src/weaver_kernel/errors.py | Extends TokenScopeError with an optional stable reason_code. |
| src/weaver_kernel/_token_signing.py | Extracts canonical signable payload, HMAC signing helper, and hardened token parsing for untrusted inputs. |
| src/weaver_kernel/_secrets.py | Adds keyring + active-key env resolution with validation and precedence rules for rotation. |
| src/weaver_kernel/_hmac_provider.py | Moves HMACTokenProvider implementation; adds rotation-aware verify and keyring-backed issuance. |
| docs/security.md | Documents TTL, arg constraints, rotation runbook, invoke-time rate limiting, and typed deserialization behavior. |
| docs/agent-context/invariants.md | Documents placement requirements for invoke-time enforcement to avoid silent bypass. |
| docs/adr/0001-token-signing-evolution.md | Adds ADR investigating token format evolution (no production code change). |
| CHANGELOG.md | Records new lifecycle-hardening features and the deliberate breaking change to signed payload shape. |
Comment on lines
+32
to
+39
| if max_ttl_s is None: | ||
| return | ||
| values = max_ttl_s.values() if isinstance(max_ttl_s, dict) else [max_ttl_s] | ||
| for value in values: | ||
| if not isinstance(value, int) or isinstance(value, bool) or value <= 0: | ||
| raise AgentKernelError( | ||
| f"Invalid max_ttl_s: values must be positive integers, got {value!r}." | ||
| ) |
Comment on lines
+83
to
+102
| allowed = spec.get("allowed_keys") | ||
| if allowed is not None: | ||
| extra = sorted(set(args) - set(allowed)) | ||
| if extra: | ||
| raise _violation(f"arguments {extra} are not permitted by the token's allowed_keys.") | ||
|
|
||
| pinned = spec.get("pinned") | ||
| if isinstance(pinned, dict): | ||
| for key, expected in pinned.items(): | ||
| if key not in args or args[key] != expected: | ||
| raise _violation(f"argument '{key}' must equal the token's pinned value.") | ||
|
|
||
| prefix = spec.get("prefix") | ||
| if isinstance(prefix, dict): | ||
| for key, required_prefix in prefix.items(): | ||
| value = args.get(key) | ||
| if not isinstance(value, str) or not value.startswith(required_prefix): | ||
| raise _violation( | ||
| f"argument '{key}' must be a string starting with '{required_prefix}'." | ||
| ) |
…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
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.
Closes #170, closes #183, closes #185, closes #200, closes #203. Investigates #224 (ADR — no code).
These six issues share one implementation path — the lifecycle of a
CapabilityToken, from issuance through invoke-time enforcement — and one code area (tokens.py,policy.py,kernel/), so they land as one coherent, reviewable PR.What changed
#185 — Signing-key rotation
HMACTokenProvider(secrets={key_id: secret}, active_key_id=...)signs new tokens under one key while still verifying tokens signed under others during an overlap window, soWEAVER_KERNEL_SECRETcan rotate without invalidating every outstanding token.key_idis inside the signed payload (tamper-evident); an unknown key id fails closed asTokenInvalid.WEAVER_KERNEL_SECRETS(JSON{key_id: secret}) /WEAVER_KERNEL_ACTIVE_KEYenv resolution. A non-active-key verification logstoken_verified_non_active_key(key id only, never the secret).#200 — Typed
CapabilityToken.from_dicterrorsconstraints) now raisesTokenInvalidwith a descriptive message instead of a bareKeyError/ValueError. Valid round-trips unchanged.#203 — Per-grant TTL
Kernel.grant_capability(..., ttl_s=...)sets a token's lifetime per grant;DefaultPolicyEngine(max_ttl_s=...)(a single cap or per-safety-class map) bounds it.invalid_constraint/ttl_exceeded) and audited — never silently clamped.#183 — Signed argument-level constraints
constraints["args"](allowed_keys,pinned,prefix) is enforced byinvoke()andinvoke_stream()before the driver runs and budget is reserved, raisingTokenScopeError(arg_constraint_violation) with an audited failure trace. Dry-run predicts the identical outcome. The declarative engine needed no changes —constraintsalready flows into the issued token.#170 — Opt-in per-invocation rate limiting
Kernel(invoke_rate_limits={SafetyClass: (limit, window_s)})adds an invoke-time sliding-window limit, independent of and additional to the grant-time limit. Default off. The check-then-record pair has noawaitbetween them (no over-admission race); dry-run never consumes it.#224 — Token-format evolution (investigation only)
docs/adr/0001-token-signing-evolution.mdevaluates HMAC (status quo), macaroon-style caveat chaining, and Biscuit against the kernel's invariants with measured numbers, and recommends staying HMAC + re-issuance now (macaroon-chaining as the documented future path, Biscuit deferred). No code or dependency change.Why
The audit flagged that PR #247 once attempted this exact scope but closed unmerged — none of its code is in
main(verified). This is fresh work covering the same coherent group, plus the stream-path enforcement and ratchet compliance that attempt lacked.Architecture / module budget
To stay within the AGENTS.md ≤300-line module budget (
tokens.py,policy.py,kernel/__init__.pywere all at their ratchet ceilings), implementation was split into new sibling modules mirroring the existing_invoke/_dry_runpattern:_token_signing.py(signing, payload, hardened parse),_hmac_provider.py(the provider, re-exported fromtokens.py; logger name unchanged),policy_ttl.py,kernel/_grant.py,kernel/_constraints.py.policy._DEFAULT_RATE_LIMITS/_SERVICE_RATE_MULTIPLIERaliases (part of Consolidate legacy aliases and finish theweaver_kernelnaming migration #196) as line-budget offset.tokens.pydropped out of the size ratchet;kernel/__init__.pyceiling tightened.Compatibility
Deliberate breaking change: the signed payload now includes
key_id, so a token issued by pre-upgrade code no longer verifies (the signed shape differs even under the same secret). Accepted given pre-1.0 alpha status and the default 1-hour TTL; legacy single-secret configuration (secret=/WEAVER_KERNEL_SECRET) keeps working unchanged. No Protocol (TokenProvider/PolicyEngine/Driver) changed.How verified
make ci→ exit 0:ruff format --check+ruff check— cleanmypy src/(strict) — no issues, 67 filespytest— 856 passed, 1 skipped, branch coverage 94% (floor 90%)Scope notes / follow-ups
Deliberately out of scope: declarative (TOML/YAML) per-class TTL vocabulary,
max_usestoken counter (#170 "optionally"), and the publicAgentKernelErrorrename half of #196. The module splits partially advance #188 but do not close it.🤖 Generated with Claude Code
https://claude.ai/code/session_019WRxQL8t2Uusa6845jWtVV
Generated by Claude Code