feat(signing): v3-identity Tier 1 — BrandJsonJwksResolver + CapabilityCache (port from JS)#345
Merged
Merged
Conversation
…ity priming from JS Tier 1 of the v3-identity bundle (RFC at docs/proposals/v3-identity-bundle-design.md). Direct port of three JS-side modules: * BrandJsonJwksResolver — implements adcp.signing.AsyncJwksResolver. Walks brand.json/agents[], follows authoritative_location/house redirects with bare-host validation, picks agent by (type, id?, brand_id?), falls back to origin-bound /.well-known/jwks.json (security: prevents cross-origin trust pivot), honors Cache-Control + ETag, cascade refresh on unknown kid. 10 typed error codes match JS for cross-language conformance. Composes with AsyncCachingJwksResolver for inner JWK caching — does NOT reinvent that layer. * CapabilityCache + build_capability_cache_key — TTL-based per-agent cache for request_signing capability blocks. Cache-key format byte-identical to JS so a future shared Redis cache works cross-language. * ensure_capability_loaded — async primer with fail-open negative- cache TTL (60s) on fetch failures, in-flight dedup via asyncio.Future, MCP/A2A transport unwrapping (structuredContent, content[].text, result.artifacts[].parts[].data, result.parts[].data). This is the JWKS-resolution layer of the v3 trust chain. The verifier machinery already exists in adcp.signing/ (verify_starlette_request, AsyncCachingJwksResolver, ReplayStore, etc.); this connects the brand-side discovery to the existing verifier. Composes with future Tier 2 (BuyerAgentRegistry per adcontextprotocol/adcp-client#1269) and Tier 3 (BrandAuthorizationResolver, gated on adcontextprotocol/adcp#3690 for eTLD+1 binding + authorized_operators[]). Tests: 61 new (34 brand_jwks, 27 capability_cache + priming), 2,975 total pass. ruff + mypy clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Issue #346 requests renaming the brand-operator authorization Protocol from (Triage-managed comment from issue #346.) Generated by Claude Code |
…canon Six findings from the four expert reviews on Tier 1 JWKS port: **Critical (security):** * SSRF on brand.json fetch — brand.json walker now uses ``build_async_ip_pinned_transport`` per hop with ``trust_env=False``, matching the JWKS fetcher's posture. Without this, an attacker-controlled ``authoritative_location`` could redirect the fetch chain to private IPs / cloud-metadata endpoints. The brand.json walker is the verifier's trust root — its SSRF posture must be at least as strict as JWKS. * No response-body size cap — counterparty serving a multi-megabyte brand.json would OOM the verifier. Added ``DEFAULT_MAX_BRAND_JSON_BYTES`` (256 KiB) cap, rejected before parse with ``invalid_body``. **Critical (correctness):** * URL canonicalization gap — ``urlsplit`` lowercases scheme but NOT host, and does not strip default ports. JS ``new URL()`` does both. Without these, the redirect-loop detector saw ``https://X.example/`` and ``https://x.example/`` as distinct entries; the well-known fallback origin check spuriously rejected ``https://x:443`` vs ``https://x``. Fixed by normalizing host to lowercase and stripping default ports (443 for https, 80 for http). **Real bugs (concurrency):** * Refresh single-flighting was serializing — replaced ``asyncio.Lock`` with a Future-based dedup, mirroring the JS pattern. N concurrent ``resolve()`` calls on a cold cache now share ONE brand.json fetch instead of doing N fetches in series. * ``Future`` lifecycle on ``BaseException`` in ``ensure_capability_loaded`` — the bare ``except Exception`` previously swallowed most errors but let ``BaseException`` (including ``CancelledError``) propagate without resolving the in-flight future, leaving joined waiters hung forever. Now narrowed to expected discovery errors; BaseException propagates with the future explicitly excepted. **Real bugs (DX):** * Capability-priming silent error swallow — bare ``except Exception`` meant operators had no signal when a counterparty's discovery endpoint was poisoning their negative cache. Added WARNING-level logging on every fail-open path. **Tests:** * Tests now use a ``client_factory`` injection seam instead of monkey-patching ``httpx.AsyncClient.__init__`` globally — only resolvers built in the test scope pick up the mock; concurrent AsyncClient construction for unrelated reasons is unaffected. * Added 7 new tests covering: host case lowering, default port stripping, scheme lowering, non-default port preservation, oversized-body rejection, case-aliasing redirect-loop detection, and N-concurrent-resolve fetch dedup. 68 tests pass (up from 61), 2960 total. ruff + mypy clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
Author
Expert review feedback addressed (commit fb65e12)Four reviews ran (code-reviewer, security-reviewer, ad-tech-protocol-expert, python-expert). Six findings folded in. Critical (security):
Critical (correctness):
Real bugs (concurrency):
Real bug (DX):
Tests:
Verification:
Findings deferred (low/info, not merge-blockers):
|
Contributor
Author
|
Thanks for the thorough write-up, @bokelley. The six findings look well-addressed — the SSRF hardening on the brand.json walker and the Future-based dedup for concurrent resolvers are the right calls. Good call deferring the No action needed from triage. This is in good shape for human review. Generated by Claude Code |
This was referenced May 2, 2026
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
Port of three JS-side modules that close Python's gap on the JWKS-resolution layer of the v3 trust chain. The verifier machinery already exists in `adcp.signing/` (verify_starlette_request, AsyncCachingJwksResolver, ReplayStore, etc.); this connects the brand-side discovery to the existing verifier.
10 typed error codes match JS for cross-language conformance.
Why this is a port, not new design
JS already has `brand-jwks.ts` (577 LOC), `capability-cache.ts` (119 LOC), and `capability-priming.ts` (159 LOC). Python's first-draft RFC initially proposed designing these from scratch — after a code audit of both sides, the actual gap is just porting the JS work. Cache key format and error code list are byte-equal so a future shared cache works cross-language.
Composes with
Full design: `docs/proposals/v3-identity-bundle-design.md`.
Test plan
Security checks
Files
Cross-links
🤖 Generated with Claude Code