Skip to content

feat: harden the capability-token lifecycle — rotation, TTL, invoke-time enforcement - #259

Open
dgenio wants to merge 2 commits into
mainfrom
claude/github-issue-triage-mhjohz
Open

feat: harden the capability-token lifecycle — rotation, TTL, invoke-time enforcement#259
dgenio wants to merge 2 commits into
mainfrom
claude/github-issue-triage-mhjohz

Conversation

@dgenio

@dgenio dgenio commented Jul 18, 2026

Copy link
Copy Markdown
Owner

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, so WEAVER_KERNEL_SECRET can rotate without invalidating every outstanding token.
  • The signing key_id is inside the signed payload (tamper-evident); an unknown key id fails closed as TokenInvalid.
  • New WEAVER_KERNEL_SECRETS (JSON {key_id: secret}) / WEAVER_KERNEL_ACTIVE_KEY env resolution. A non-active-key verification logs token_verified_non_active_key (key id only, never the secret).

#200 — Typed CapabilityToken.from_dict errors

  • A malformed serialized token (missing field, wrong type, invalid timestamp, non-object constraints) now raises TokenInvalid with a descriptive message instead of a bare KeyError/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.
  • A non-positive or over-maximum request is denied (invalid_constraint / ttl_exceeded) and audited — never silently clamped.

#183 — Signed argument-level constraints

  • A token's constraints["args"] (allowed_keys, pinned, prefix) is enforced by invoke() and invoke_stream() before the driver runs and budget is reserved, raising TokenScopeError (arg_constraint_violation) with an audited failure trace. Dry-run predicts the identical outcome. The declarative engine needed no changes — constraints already 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 no await between them (no over-admission race); dry-run never consumes it.

#224 — Token-format evolution (investigation only)

  • docs/adr/0001-token-signing-evolution.md evaluates 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__.py were all at their ratchet ceilings), implementation was split into new sibling modules mirroring the existing _invoke/_dry_run pattern:

  • _token_signing.py (signing, payload, hardened parse), _hmac_provider.py (the provider, re-exported from tokens.py; logger name unchanged), policy_ttl.py, kernel/_grant.py, kernel/_constraints.py.
  • Removed the private policy._DEFAULT_RATE_LIMITS/_SERVICE_RATE_MULTIPLIER aliases (part of Consolidate legacy aliases and finish the weaver_kernel naming migration #196) as line-budget offset. tokens.py dropped out of the size ratchet; kernel/__init__.py ceiling 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 — clean
  • mypy src/ (strict) — no issues, 67 files
  • pytest856 passed, 1 skipped, branch coverage 94% (floor 90%)
  • all 13 examples run
  • Plus a combined end-to-end smoke exercising rotation + TTL + arg-constraint + invoke-rate together.

Scope notes / follow-ups

Deliberately out of scope: declarative (TOML/YAML) per-class TTL vocabulary, max_uses token counter (#170 "optionally"), and the public AgentKernelError rename 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

…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
Copilot AI review requested due to automatic review settings July 18, 2026 07:34
Comment thread src/weaver_kernel/_hmac_provider.py Fixed
Comment thread src/weaver_kernel/_hmac_provider.py Fixed
Comment thread src/weaver_kernel/_token_signing.py Fixed
Comment thread src/weaver_kernel/tokens.py Fixed
Comment thread src/weaver_kernel/tokens.py Fixed
Comment thread src/weaver_kernel/tokens.py Fixed
Comment thread src/weaver_kernel/_hmac_provider.py Fixed

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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) with DefaultPolicyEngine(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() and invoke_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 thread src/weaver_kernel/_token_signing.py
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

dgenio commented Aug 11, 2026

Copy link
Copy Markdown
Owner Author

Red-team follow-up before this security PR is merged:

The current head has fixed two earlier review concerns: max_ttl_s mapping keys are now validated as SafetyClass, and the argument-constraint containers (allowed_keys, pinned, prefix) are type-checked.

One fail-closed gap remains in enforce_arg_constraints(): prefix validates the outer mapping but not that each required_prefix value is a string. A malformed signed constraint such as {"args":{"prefix":{"path":123}}} reaches value.startswith(required_prefix) and can raise an untyped TypeError instead of the intended audited TokenScopeError(arg_constraint_violation). Please validate inner rule values (and malformed/unhashable allowed_keys elements) and add regression tests showing malformed security-scoping specs fail closed with the stable reason code.

Also, this branch is currently 6 commits behind main. Before merge, bring it onto current main and rerun the current CI + CodeQL matrix; the historical green checks are useful evidence but not sufficient for a large security-sensitive PR after subsequent coding-agent/policy/CI changes.

After those points are addressed, this remains a high-value hardening PR and should close #170 plus the related token lifecycle items it targets.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

4 participants