feat(signing): live revocation-list fetcher with signed JWS verification#201
Merged
feat(signing): live revocation-list fetcher with signed JWS verification#201
Conversation
️✅ There are no secrets present in this pull request anymore.If these secrets were true positive and are still valid, we highly recommend you to revoke them. 🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request. |
Adds production-ready revocation checking per security.mdx §Revocation
(landed via adcp#2316). Callers wire a `CachingRevocationChecker` into
`VerifyOptions.revocation_checker`; it fetches
`{origin}/.well-known/governance-revocations.json`, verifies the
operator's JWS, caches the parsed list, refetches near `next_update`,
and fails closed past `next_update + grace * last_interval`.
New public surface (`adcp.signing`):
* `CachingRevocationChecker` — implements the existing
`RevocationChecker` Protocol. Convenience constructor
`.from_issuer_origin("https://gov.example.com")` pins the .well-known
path. `.prime()` fetches synchronously so integrators can fail-fast
at startup rather than at first verification. `.is_jti_revoked(jti)`
exposes the per-token revocation surface governance verifiers need.
* `RevocationListFetcher` Protocol + `default_revocation_list_fetcher`
with SSRF validation (reuses JWKS rules), `If-None-Match` and
`If-Modified-Since` conditional requests, `follow_redirects=False`.
* `verify_jws_document` + compact / general-JSON parsers. Hand-rolled
against the existing crypto primitives (EdDSA / ES256 whitelist,
`alg=none` rejection, byte-equal `typ` match, `crit` refused) —
auditable without pulling in pyjwt / authlib.
* Shared `JwksResolver` Protocol consolidated in `adcp.signing.jwks`
(was duplicated in `jws.py` + `verifier.py`).
Defense-in-depth (applied after expert review):
* JWS compact-form signing input uses the ORIGINAL base64url substring,
not decode-then-re-encode — guards against lenient-decode mismatches.
* Refresh rejects a list whose `updated` is older than the cached one.
Blocks replay of older lists after a kid has been revoked.
* Parse rejects declared cadences below the 60s spec floor.
* Issuer comparison is case-insensitive on scheme + host and strips
trailing slash / path (RFC 6454 origin semantics).
* Version check soft-accepts any positive integer so additive schema
changes don't force the library into fail-closed on old SDKs.
Tests (+74 across four files, 212 signing tests total):
* `test_jws.py` (22) — parse/verify round-trips, negative cases
(`alg=none`, wrong `typ`, missing `kid`, `crit`, tampered payload,
non-JSON / non-dict payload).
* `test_revocation_fetcher.py` (24) — happy path, cache hit, past-
`next_update` refresh, 304, signature tampering, wrong issuer,
cadence-floor rejection, clock skew, grace window (within + past),
general-JSON form, cooldown on failure, replay rejection,
`from_issuer_origin`, `prime()`, `is_jti_revoked`, case-variant
issuer acceptance, `If-Modified-Since` threading, alternate-encoding
rejection.
* `test_revocation_e2e.py` (5) — real Starlette app via
`httpx.ASGITransport`, plugs into `verify_request_signature`; covers
kid-revoked → 401, kid-clean → 200, past-grace → freshness error.
Closes #188.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bokelley
force-pushed
the
bokelley/signing-revocation
branch
from
2912d5b to
fbe3e90
Compare
…t_update on 304 Expert re-review of PR #201 surfaced three load-bearing issues beyond the original 13-item fix pass. All addressed here plus defensive tests. Security -------- * Issuer normalization rejects userinfo (``user:pass@host``) and IDNA-encodes the host so homoglyph attacks (``go\u1d20.example.com``) produce a byte-distinct punycode form that fails comparison against a legitimately-configured origin. * ``Last-Modified`` header values are now shape-validated (RFC 7231 HTTP-date regex + 64-byte cap) before being persisted for the next ``If-Modified-Since`` request. Blocks CRLF-injection-adjacent echoing of a malicious origin's header into our outbound requests. Behavior -------- * On 304, advance the cached ``next_update`` by the declared polling interval. Previously a steady-state 304 would leave ``next_update`` at the original value, causing every subsequent verification to retry the refresh logic (gated by 60s cooldown but still wasted work). Now the freshness window slides forward and the hot path short-circuits. Safety / DX ----------- * Document single-threaded contract on ``CachingRevocationChecker`` (defer lock until a threaded caller actually lands). * Constructor rejects ``time.time`` passed as ``clock`` (wall-clock jumps would break cooldown math) and rejects passing identical callables to ``clock`` and ``wall_clock``. * ``RevocationListSignatureError`` now documented as explicitly NOT re-exported from ``adcp.signing`` (still importable from the submodule for callers that want to distinguish signature from parse failures). * Cross-link ``__call__`` and ``is_jti_revoked`` docstrings so governance-token verifiers find the jti surface. * Stronger replay-rejection assertion pins the "cached list not replaced" invariant explicitly. Tests (+7): userinfo rejection, homoglyph distinct-punycode, legit IDN stability, Last-Modified sanitization including CRLF + length cap, 304 sliding next_update forward, time.time clock rejection, identical- callable clock rejection. Total signing suite now 219 tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bokelley
added a commit
that referenced
this pull request
Apr 19, 2026
Adds async counterparts to the sync types that landed in PR #201 so verifiers running on an asyncio event loop (FastAPI, Starlette, etc.) don't need to bridge through ``asyncio.run`` or block the loop on revocation/JWKS fetches. Addresses the dx-expert's top DX callout on PR #201. New public surface (``adcp.signing``) ------------------------------------- * ``AsyncJwksResolver`` / ``AsyncJwksFetcher`` Protocols * ``AsyncCachingJwksResolver`` with ``asyncio.Lock``-serialized refreshes * ``async_default_jwks_fetcher`` using ``httpx.AsyncClient`` * ``as_async_resolver(sync)`` adapter so ``StaticJwksResolver`` can plug into async pipelines * ``AsyncRevocationListFetcher`` Protocol * ``async_default_revocation_list_fetcher`` * ``AsyncCachingRevocationChecker`` — mirrors ``CachingRevocationChecker`` with awaitable ``__call__``, ``aprime()``, awaitable ``is_jti_revoked``, ``from_issuer_origin`` classmethod. ``asyncio.Lock`` serializes concurrent refreshes so N parallel first-miss tasks fire one fetch. * ``averify_detached_jws`` / ``averify_jws_document`` Refactor -------- Sync and async paths share: * ``_post_jws_validation`` — schema + clock-skew + replay check * ``_slide_next_update`` — 304 response handling * ``_polling_interval_seconds`` — clamped interval math * ``_build_fetch_headers`` / ``_fetch_result_from_response`` — HTTP glue * ``_validate_header`` / ``_verify_signature_and_decode_payload`` — JWS pipeline split at the resolver call * ``_CheckerState`` mixin — cache state + ``_handle_not_modified`` / ``_commit`` / ``_grace_seconds`` methods inherited by both checkers Preserves sync behavior byte-for-byte; sync test suite unchanged at 219. Defense-in-depth (from expert re-review) ---------------------------------------- * Eager ``asyncio.Lock()`` construction in ``__init__``. Lazy init was racy — two tasks could each see ``self._lock is None`` and construct separate locks. Python 3.10+ no longer requires a running loop for ``asyncio.Lock()``. * Cancellation-safe ``_last_refresh_attempt`` — rolls back on ``CancelledError`` so a cancelled task doesn't burn the 60s cooldown for the next legitimate caller (DoS-amplification defense). * Clock reads recomputed inside the lock — the pre-lock reads could be stale after a slow refresh. Tests (+20) ----------- ``test_async_revocation.py`` — 20 tests mirroring sync coverage plus: * ``test_concurrent_first_calls_share_one_refresh`` — 20 tasks → exactly 1 refresh (regression test for the lazy-lock race) * ``test_cancellation_rolls_back_cooldown_attempt`` — pins the cancellation-safety invariant * ``test_async_e2e_asgi_round_trip`` — native Starlette + ``httpx.AsyncClient`` e2e, NO ``asyncio.run`` bridge (the whole point of the async path) * ``test_async_caching_jwks_resolver_handles_concurrent_misses`` — async JWKS lock serialization 239 signing tests pass, ruff + mypy clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
4 tasks
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
1 participant
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.
Summary
Ships a production-ready
CachingRevocationCheckerthat fetches, verifies, and caches operator-signed revocation lists from{origin}/.well-known/governance-revocations.jsonper security.mdx §Revocation (landed via adcp#2316). Plugs directly into the existingVerifyOptions.revocation_checker.Closes #188.
What integrators get
Behavior
If-None-Match+If-Modified-Sinceconditional requests,follow_redirects=Falsealg=nonerejected,typ=adcp-gov-revocation+jwsbyte-equal match,critextensions refusedprime()); refetch whennow >= next_updatenext_update + 2× declared_interval, subsequent calls raiseRevocationListFreshnessError(→request_signature_revocation_stale)updatedis older than the cached onenext_update - updated < 60srejected at parse (spec floor)next_update(stops a high-QPS verifier from hammering a dead origin)Design notes
JwksResolverProtocol consolidated inadcp.signing.jwks(previously duplicated injws.py+verifier.py).Review
Reviewed by code-reviewer, security-reviewer, ad-tech-protocol-expert, and dx-expert subagents. 13 follow-up fixes applied:
JwksResolverProtocolallow_privateredundancy from Protocolnew.updated < current.updated(replay block)If-Modified-Since/Last-Modifiedsupportis_jti_revoked()for governance step 14version > 1from_issuer_origin()classmethodprime()for fail-fast at startupRevocationListSignatureError, polling constants, internal JWS names)Not in this PR (follow-ups)
AsyncRevocationListFetcher+aprime()) — the dx-expert's highest-impact suggestion. FastAPI verifiers are async; forcing sync means callers either block the loop or bridge throughasyncio.run. Deferred to its own PR because it also wants a consistent pass onCachingJwksResolver.TimeSourceprotocol consolidatingclock+wall_clock. TouchesCachingJwksResolvertoo.Test plan
pytest tests/conformance/signing/ -q— 212 passed, 1 skippedhttpx.ASGITransportagainst the liveverify_request_signaturepipelineruff check src/adcp/signing/ tests/conformance/signing/— cleanmypy src/adcp/signing/— clean (16 source files)🤖 Generated with Claude Code