feat: harden the capability-token lifecycle — rotation, TTL, invoke-time enforcement#247
Closed
dgenio wants to merge 1 commit into
Closed
Conversation
…ime enforcement Closes #170, closes #183, closes #185, closes #200, closes #203. Investigates #224 (ADR, no code). These five issues were selected as a coherent group: they share one implementation path — the lifecycle of a CapabilityToken, from issuance through invoke-time enforcement — and one code area (tokens.py, kernel/_grant.py, kernel/_invoke.py). #170 was explicitly flagged as a deliberate follow-up in PR #238's own description ("a distinct rate-limiting/policy concern... recommend it as a separate follow-up"), which is what this PR is. What changed: - #185 — HMACTokenProvider accepts a {key_id: secret} KeyRing + active_key_id, so WEAVER_KERNEL_SECRET can rotate without invalidating every outstanding token. An unknown key_id fails closed as TokenInvalid. New optional WEAVER_KERNEL_SECRETS env var (JSON key_id->secret map). - #200 — CapabilityToken.from_dict raises typed TokenInvalid on malformed input (missing field, wrong type, bad timestamp) instead of a bare KeyError/ValueError. - #203 — Kernel.grant_capability(..., ttl_s=...) sets a per-grant token TTL. DefaultPolicyEngine(max_ttl_s={...}) validates it via a new, Protocol-optional resolve_ttl() method (denies, never clamps) — the same non-breaking pattern already used for ExplainingPolicyEngine, so no third-party PolicyEngine breaks. - #183 — Signed constraints["args"] (allowed_keys/pinned/prefix) enforced by Kernel.invoke() before the driver runs and before budget is reserved; violations raise TokenScopeError with a stable reason code and an audited "deny" trace. Dry-run predicts the identical outcome. DeclarativePolicyEngine needed zero code changes — constraints is already a free-form mapping. - #170 — Opt-in, default-off per-invocation rate limiting (Kernel(invoke_rate_limits=...)), independent of the existing grant-time limit. Raises the new RateLimitExceeded; dry-run never consumes it. - #224 — docs/adr/0001-token-signing-evolution.md evaluates HMAC (status quo), macaroon-style chaining, and Biscuit against this kernel's actual invariants; recommends status quo + re-issuance now, defers Biscuit. No code/deps added. Architecture: tokens.py, kernel/__init__.py, and kernel/_invoke.py were all already at their line-budget ratchet ceilings before this change. Rather than raise every ceiling, grant_capability's body was extracted into a new kernel/_grant.py, trace-recording helpers were extracted from kernel/_invoke.py into kernel/_trace_record.py (shared with _stream.py/_audit.py, which already imported them from _invoke.py), and token signing/verification/deserialization was extracted into a new _token_signing.py and policy_ttl.py — mirroring the same split pattern already used for rate_limit.py, kernel/_dry_run.py, and kernel/_driver_exec.py. Net effect: kernel/__init__.py and tokens.py both end up smaller than before despite the new features; policy.py's ceiling needed a modest, justified raise (+47 lines) since further splitting would mean refactoring evaluate()/explain() — issue #219's job, out of scope here. Deliberate compatibility break: CapabilityToken's signable payload now includes key_id, so a token issued by pre-upgrade code fails verification after this deploys (documented in CHANGELOG under Changed). Given tokens default to a 1-hour TTL and this project is pre-1.0 alpha (see the 0.10.0 import-rename precedent), this is treated as an accepted break, not a compatibility shim. Out of scope (deliberately): #170's optional max_uses token-use-counter (explicitly marked "Optionally" in the issue — a good follow-up), and RateLimiter's lack of internal locking under concurrent invoke() (covered by existing issue #142's asyncio-stress-test scope, not expanded here — this PR's invoke-time check/record is written to run without an intervening await, and a concurrency test proves 10 concurrent calls against a limit of 3 admit exactly 3). How verified: Full repo gate (make ci -> fmt-check -> lint -> type -> test -> example) in a clean venv — exit 0: - ruff format --check — 124 files already formatted - ruff check — All checks passed - mypy src/ (strict) — no issues in 67 source files - pytest — 839 passed, 1 skipped, branch coverage 94.10% (floor 90%) - make example — all 13 examples complete New tests: tests/test_tokens.py (key rotation, unknown-key fail-closed, legacy single-secret compat, malformed from_dict parametrized over every failure class), tests/test_kernel.py (invoke-time rate limit on/off/window-reset/ per-principal-scoping/concurrency-race, arg-constraint violation+dry-run-parity +tamper-invalidates-signature+never-reaches-driver), tests/test_policy.py (resolve_ttl denial/pass-through/no-op-without-implementation, grant_capability ttl_s end-to-end, declarative args-constraint round-trip). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DsfDG5GmfqmnNbUsYUa8MA
|
Bugbot is not enabled for this team, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
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).
Note on base branch: this PR is stacked on #238 (still open) because both touch
kernel/_invoke.py'sperform_invoke. Basing here avoids a guaranteed merge conflict; once #238 merges tomain, please retarget this PR's base tomain(GitHub usually offers to do this automatically).Why this group
These five issues were selected as a coherent group: they share one implementation path — the lifecycle of a
CapabilityToken, from issuance through invoke-time enforcement — and one code area (tokens.py,kernel/_grant.py,kernel/_invoke.py). #170 was explicitly flagged as a deliberate follow-up in #238's own description ("a distinct rate-limiting/policy concern... recommend it as a separate follow-up"), which is what this PR is.What changed
#185 — Signing-key rotation
HMACTokenProvideraccepts a{key_id: secret}KeyRing+active_key_id, soWEAVER_KERNEL_SECRETcan rotate without invalidating every outstanding token at once.key_idfails closed asTokenInvalid.WEAVER_KERNEL_SECRETSenv var (JSON{key_id: secret}), taking precedence over the legacyWEAVER_KERNEL_SECRET.token_verified_non_active_key(key id only, never the secret) so operators can tell when it's safe to retire a key.#200 — Typed errors from
CapabilityToken.from_dictconstraints) 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={SafetyClass: seconds})validates it via a new, Protocol-optionalresolve_ttl()method — an excessive or non-positivettl_sis denied, never silently clamped.resolve_ttlis deliberately not added to thePolicyEngineProtocol — the same non-breaking pattern already used to split offExplainingPolicyEngine(seeCHANGELOG.md's 0.7.0 entry) — so no third-partyPolicyEngineimplementation breaks. A test (test_grant_capability_ttl_s_passthrough_without_resolve_ttl) proves the graceful degradation.#183 — Signed argument-level constraints
constraints["args"](allowed_keys,pinned,prefix) is enforced byKernel.invoke()before the driver runs and before budget is reserved, raisingTokenScopeErrorwith a stable reason code and an audited"deny"trace on violation.DeclarativePolicyEngineneeded zero code changes —constraintsis already a free-form mapping that flows straight into the issued token. New example rule inexamples/policies/default.{yaml,toml}.#170 — Opt-in per-invocation rate limiting
Kernel(invoke_rate_limits={SafetyClass: (limit, window_s)})adds a second, invoke-time sliding-window limit, independent of and in addition to the existing grant-time limit. Default off — behavior is unchanged unless configured.RateLimitExceededwith a stable reason code;dry_run=Truenever consumes it.test_invoke_rate_limit_concurrent_calls_do_not_exceed_limit) proves 10 concurrentinvoke()calls against a limit of 3 admit exactly 3 — the check-then-record pair runs with noawaitin between, so there's no over-admission race.#224 — Token signing format evolution (investigation only)
docs/adr/0001-token-signing-evolution.mdevaluates HMAC (status quo), macaroon-style caveat chaining, and Biscuit against this kernel's actual invariants (I-06, minimal-dependency policy, determinism, revocation model,explain()transparency) — including real measured numbers for the HMAC baseline (401-byte token, ~12.5µs verify).Architecture note
tokens.py,kernel/__init__.py, andkernel/_invoke.pywere all already at their line-budget ratchet ceilings before this change. Rather than raise every ceiling, this PR:grant_capability's body into a newkernel/_grant.py(mirrors the existing_invoke.py/_dry_run.pysplit).kernel/_invoke.pyintokernel/_trace_record.py(shared with_stream.py/_audit.py, which already imported them from_invoke.py)._token_signing.py, and TTL validation intopolicy_ttl.py— mirroring the existingrate_limit.py/kernel/_driver_exec.pysplit pattern.Net effect:
kernel/__init__.pyandtokens.pyboth end up smaller than before despite the new features.policy.py's ceiling needed a modest, justified raise (+47 lines, documented inline) since further splitting would mean refactoringevaluate()/explain()— that's issue #219's job, out of scope here.Compatibility
Deliberate breaking change:
CapabilityToken's signable payload now includeskey_id, so a token issued by pre-upgrade code fails verification after this deploys — even under the same secret, since the signed payload shape differs. Documented inCHANGELOG.mdunder### Changed. Given tokens default to a 1-hour TTL and this project is pre-1.0 alpha (see the 0.10.0 import-rename precedent already in the CHANGELOG), this is treated as an accepted break, not a compatibility shim.No
PolicyEngine/ExplainingPolicyEngineProtocol changes —resolve_ttlis additive and optional, so no existing engine implementation is broken.Scope notes
Deliberately out of scope:
max_usestoken-use-counter from Add per-invocation enforcement of rate limits (not only at grant time) #170 — the issue itself marks it "Optionally," and deferring it kepttokens.pycomfortably under its line budget. Good follow-up issue.RateLimiterthread-safety hardening — covered by existing issue [Testing] Document the concurrency model and add asyncio stress tests #142 (asyncio concurrency stress tests). This PR's own invoke-time check/record is written to avoid the specific race (noawaitbetween check and record), proven by the concurrency test above, but broader hardening belongs in [Testing] Document the concurrency model and add asyncio stress tests #142.DefaultPolicyEngine-level auto-attachment ofargsconstraints per safety class — the declarative engine already supports this today with zero code changes (see Support signed argument-level constraints enforced at invoke time #183 above); a code-level equivalent onDefaultPolicyEngineis a reasonable, separate follow-up if wanted.How verified
Full repo gate (
make ci→fmt-check → lint → type → test → example) in a clean venv — exit 0:ruff format --check— 124 files already formattedruff check— All checks passedmypy src/(strict) — no issues in 67 source filespytest— 839 passed, 1 skipped, branch coverage 94.10% (floor 90%)make example— all 13 examples completeNew/updated tests:
tests/test_tokens.py— key rotation + overlap window, unknown-key-id fails closed, legacy single-secret compat unaffected,active_key_idmisconfiguration errors, non-active-key verification logging (never the secret), malformedfrom_dictparametrized over every failure class, valid round-trip unchanged.tests/test_kernel.py— invoke-time rate limit on/off/window-reset/per-principal-scoping/concurrency-race/dry-run-non-consumption, arg-constraint violation + dry-run parity + tamper-invalidates-signature + never-reaches-driver + deny-trace-recorded.tests/test_policy.py—resolve_ttldenial (non-positive, over-max)/pass-through/no-op-without-implementation,grant_capability(ttl_s=...)end-to-end, declarativeargs-constraint round-trip.🤖 Generated with Claude Code
Generated by Claude Code