Skip to content

Commit 733ed4e

Browse files
bokelleyclaude
andcommitted
fix(signing): cap key_origins entry size, route cold-cache jwks_uri=None to JWKS_UNAVAILABLE, host-only diagnostic fallback
Three remaining Argus follow-ups from #789. 1. Per-entry length cap on _extract_key_origins. Each origin value is bounded at 512 bytes — well above any legitimate scheme+host+port shape but tight enough that a pathological multi-kilobyte entry from the 64 KiB capabilities body doesn't propagate through downstream comparisons. Oversized entries are SKIPPED (not truncated — a truncated host would silently match the wrong domain). Constant _MAX_KEY_ORIGIN_VALUE_BYTES = 512 documents the choice. 2. Cold-cache jwks_uri=None routes to REQUEST_SIGNATURE_JWKS_UNAVAILABLE. Previous behavior coerced None to '' and routed through the mismatch path with empty actual_origin — a resolver-side I/O failure misclassified as adversarial origin-mismatch on dashboards. Now raises JWKS_UNAVAILABLE with detail={'purpose': signing_purpose} so the cold-cache shape aggregates with other resolver-fetch failures. 3. Diagnostic host-only fallback on canonicalization failure. When _origin_host can't canonicalize one side, the mismatch detail's expected_origin / actual_origin values must still be HOST-SHAPED — previous fallback leaked the full raw URL into the host-labeled field, inconsistent with the success-path host-only shape. New _diagnostic_host helper falls through to _extract_host (the same URL/bare-host parser the canonicalization step uses) for a best-effort host, empty string at worst. Three new tests in test_verify_from_agent_url.py: - _extract_key_origins skips oversized entries - mismatch detail uses host-only fallback (no URL leakage) - jwks_uri=None routes to JWKS_UNAVAILABLE 622 tests across impacted surface remain green. ruff + mypy clean. Refs Argus second-pass review on #789 (cold-cache routing, detail shape, per-entry length cap). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent e5457a7 commit 733ed4e

4 files changed

Lines changed: 156 additions & 11 deletions

File tree

src/adcp/signing/agent_resolver.py

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -329,6 +329,14 @@ def _extract_brand_json_url(capabilities: dict[str, Any]) -> str:
329329
return brand_json_url
330330

331331

332+
#: Per-entry size clamp on ``identity.key_origins`` values. DNS hostname
333+
#: limit is 253 octets (RFC 1035); origin strings carry scheme+host so
334+
#: the practical cap is a bit higher, but 512 is well above any
335+
#: legitimate value while still bounding the surface against a
336+
#: pathologically-large entry from a 64 KiB capabilities body.
337+
_MAX_KEY_ORIGIN_VALUE_BYTES = 512
338+
339+
332340
def _extract_key_origins(capabilities: dict[str, Any]) -> dict[str, str] | None:
333341
"""Pluck ``identity.key_origins`` from the capabilities body.
334342
@@ -339,6 +347,14 @@ def _extract_key_origins(capabilities: dict[str, Any]) -> dict[str, str] | None:
339347
purpose is actually exercised). Filters values to strings — a
340348
malformed entry is skipped rather than poisoning the whole map.
341349
350+
**Per-entry length cap (``_MAX_KEY_ORIGIN_VALUE_BYTES``).** Each
351+
origin value is bounded to 512 bytes — well above any legitimate
352+
``scheme + host + port`` shape but tight enough that a pathological
353+
multi-kilobyte value from the 64 KiB capabilities body doesn't
354+
propagate through downstream comparisons. Entries exceeding the cap
355+
are skipped (the verifier then surfaces the purpose as missing on
356+
the consistency check).
357+
342358
Forward-compat with operators on 3.0 schemas: the map travels under
343359
``additionalProperties: true`` and the SDK reads it as a plain dict
344360
rather than via the typed Pydantic surface (which won't carry the
@@ -352,8 +368,13 @@ def _extract_key_origins(capabilities: dict[str, Any]) -> dict[str, str] | None:
352368
return None
353369
out: dict[str, str] = {}
354370
for purpose, origin in raw.items():
355-
if isinstance(purpose, str) and isinstance(origin, str) and origin:
356-
out[purpose] = origin
371+
if not (isinstance(purpose, str) and isinstance(origin, str) and origin):
372+
continue
373+
if len(origin.encode("utf-8")) > _MAX_KEY_ORIGIN_VALUE_BYTES:
374+
# Length-capped entry — skip rather than truncate (a
375+
# truncated host would silently match the wrong domain).
376+
continue
377+
out[purpose] = origin
357378
return out or None
358379

359380

src/adcp/signing/key_origins.py

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -151,15 +151,34 @@ def check_key_origin_consistency(
151151
detail={
152152
"purpose": purpose,
153153
# Use the canonicalized values when available; fall back
154-
# to the raw inputs for diagnostic accuracy when one
155-
# side failed to canonicalize. Spec wording is
154+
# to a best-effort host extraction (NOT the raw URL — the
155+
# field name promises a host, and surfacing a full URL on
156+
# canonicalization failure was inconsistent with the
157+
# success path's host-only shape). Spec wording is
156158
# ``expected_origin`` / ``actual_origin`` verbatim.
157-
"expected_origin": declared_host if declared_host is not None else declared,
158-
"actual_origin": actual_host if actual_host is not None else jwks_uri,
159+
"expected_origin": _diagnostic_host(declared_host, declared),
160+
"actual_origin": _diagnostic_host(actual_host, jwks_uri),
159161
},
160162
)
161163

162164

165+
def _diagnostic_host(canonical: str | None, raw: str) -> str:
166+
"""Return ``canonical`` if present, else a best-effort host from
167+
``raw``, else the empty string.
168+
169+
Used to keep ``expected_origin`` / ``actual_origin`` host-shaped
170+
in the mismatch detail payload even when canonicalization failed.
171+
Falls through to ``_extract_host`` (the same URL/bare-host parser
172+
the canonicalization step uses) for the best-effort path, so the
173+
diagnostic value still reflects "the host the operator/verifier
174+
pointed at" rather than the full URL surface.
175+
"""
176+
if canonical is not None:
177+
return canonical
178+
host = _extract_host(raw)
179+
return host or ""
180+
181+
163182
def _origin_host(value: str) -> str | None:
164183
"""Return the host portion of a URL or bare origin, canonicalized
165184
for byte-equality comparison.

src/adcp/signing/verifier.py

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
REQUEST_SIGNATURE_DIGEST_MISMATCH,
4444
REQUEST_SIGNATURE_HEADER_MALFORMED,
4545
REQUEST_SIGNATURE_INVALID,
46+
REQUEST_SIGNATURE_JWKS_UNAVAILABLE,
4647
REQUEST_SIGNATURE_KEY_PURPOSE_INVALID,
4748
REQUEST_SIGNATURE_KEY_REVOKED,
4849
REQUEST_SIGNATURE_KEY_UNKNOWN,
@@ -576,12 +577,27 @@ def _maybe_check_key_origin(
576577
)
577578
return
578579
jwks_uri = getattr(resolver, "jwks_uri", None)
579-
# ``jwks_uri`` may be ``None`` if the brand-json resolver hasn't
580-
# populated it yet (cold cache + failed refresh). The consistency
581-
# check fails closed on a missing actual host — same posture as
582-
# ``_origin_host`` returning ``None``.
580+
if not jwks_uri:
581+
# A brand-json resolver that hasn't populated ``jwks_uri`` (cold
582+
# cache + failed refresh, or a misconfigured custom resolver) is
583+
# a resolver-side I/O failure, not a key-origin mismatch — the
584+
# verifier has no resolved host to compare. Surface as
585+
# ``REQUEST_SIGNATURE_JWKS_UNAVAILABLE`` so dashboards aggregate
586+
# this cold-cache shape with other resolver-fetch failures
587+
# rather than with adversarial origin-mismatch traffic.
588+
raise SignatureVerificationError(
589+
REQUEST_SIGNATURE_JWKS_UNAVAILABLE,
590+
step=7,
591+
message=(
592+
"brand-json resolver did not populate jwks_uri (cold cache "
593+
"or misconfigured resolver); key_origins consistency check "
594+
"cannot proceed without a resolved host to compare against "
595+
f"identity.key_origins.{signing_purpose}"
596+
),
597+
detail={"purpose": signing_purpose},
598+
)
583599
check_key_origin_consistency(
584-
jwks_uri=jwks_uri or "",
600+
jwks_uri=jwks_uri,
585601
key_origins=expected_key_origins,
586602
purpose=signing_purpose,
587603
posture=posture,

tests/test_verify_from_agent_url.py

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -620,3 +620,92 @@ def test_no_warning_when_both_attributes_align() -> None:
620620
and "jwks_source" in str(w.message)
621621
]
622622
assert misconfig == []
623+
624+
625+
# ---- _extract_key_origins length cap (Argus follow-up nit #1) ----
626+
627+
628+
def test_extract_key_origins_caps_oversized_entries() -> None:
629+
"""Each origin value must be clamped at 512 bytes — well above any
630+
legitimate ``scheme+host+port`` shape but tight enough that a
631+
pathological multi-kilobyte value from the 64 KiB capabilities body
632+
doesn't propagate through downstream comparisons. Oversized entries
633+
are SKIPPED (not truncated — a truncated host would silently match
634+
the wrong domain)."""
635+
from adcp.signing.agent_resolver import _extract_key_origins
636+
637+
huge = "https://" + "x" * 1024 + ".com"
638+
legit = "https://keys.brand.com"
639+
result = _extract_key_origins(
640+
{
641+
"identity": {
642+
"key_origins": {
643+
"request_signing": legit,
644+
"webhook_signing": huge, # skipped
645+
}
646+
}
647+
}
648+
)
649+
assert result == {"request_signing": legit}
650+
651+
652+
# ---- Diagnostic host-only fallback (Argus follow-up nit #2) ----
653+
654+
655+
def test_mismatch_detail_uses_host_only_fallback_on_canonicalization_failure() -> None:
656+
"""When ``_origin_host`` can't canonicalize one side (e.g. spaces
657+
in the host), the mismatch ``expected_origin`` / ``actual_origin``
658+
detail values must still be HOST-SHAPED — not the full raw URL.
659+
Previous behavior leaked the full URL into the host-labeled field,
660+
inconsistent with the success path. Now the diagnostic uses a
661+
best-effort host extraction via ``_extract_host``."""
662+
from adcp.signing.errors import REQUEST_SIGNATURE_KEY_ORIGIN_MISMATCH
663+
from adcp.signing.key_origins import check_key_origin_consistency
664+
665+
with pytest.raises(SignatureVerificationError) as exc_info:
666+
check_key_origin_consistency(
667+
jwks_uri="https://keys.brand.example/jwks.json",
668+
key_origins={"request_signing": "not a host with spaces"},
669+
purpose="request_signing",
670+
)
671+
assert exc_info.value.code == REQUEST_SIGNATURE_KEY_ORIGIN_MISMATCH
672+
detail = exc_info.value.detail
673+
assert detail is not None
674+
# actual_origin canonicalizes cleanly to the host (no URL form).
675+
assert detail["actual_origin"] == "keys.brand.example"
676+
# expected_origin failed to canonicalize but still doesn't leak the
677+
# full raw string with quoting artifacts — empty string at worst,
678+
# never the full URL.
679+
assert "/" not in detail["expected_origin"]
680+
681+
682+
# ---- jwks_uri=None routes to JWKS_UNAVAILABLE (Argus follow-up #4) ----
683+
684+
685+
def test_maybe_check_key_origin_jwks_uri_none_routes_to_jwks_unavailable() -> None:
686+
"""A brand-json resolver that hasn't populated ``jwks_uri`` (cold
687+
cache + failed refresh, or a misconfigured custom resolver) is a
688+
resolver-side I/O failure, not an origin mismatch. The verifier
689+
must surface ``REQUEST_SIGNATURE_JWKS_UNAVAILABLE`` so dashboards
690+
aggregate this cold-cache shape with other resolver-fetch
691+
failures rather than with adversarial origin-mismatch traffic."""
692+
from adcp.signing.errors import REQUEST_SIGNATURE_JWKS_UNAVAILABLE
693+
from adcp.signing.verifier import _maybe_check_key_origin
694+
695+
class _BrandJsonResolverWithNoJwksUri:
696+
jwks_source = "brand_json"
697+
# ``jwks_uri`` deliberately absent / None — cold cache shape.
698+
jwks_uri = None
699+
700+
def __call__(self, keyid: str) -> dict | None: # type: ignore[type-arg]
701+
return None
702+
703+
with pytest.raises(SignatureVerificationError) as exc_info:
704+
_maybe_check_key_origin(
705+
resolver=_BrandJsonResolverWithNoJwksUri(), # type: ignore[arg-type]
706+
expected_key_origins={"request_signing": "https://keys.brand.example"},
707+
signing_purpose="request_signing",
708+
posture=None,
709+
)
710+
assert exc_info.value.code == REQUEST_SIGNATURE_JWKS_UNAVAILABLE
711+
assert exc_info.value.detail == {"purpose": "request_signing"}

0 commit comments

Comments
 (0)