From 094455e9cecf91921d35590322dae08b6caea9ef Mon Sep 17 00:00:00 2001 From: "Constantine.mirin" Date: Sun, 18 Oct 2026 03:38:39 -0400 Subject: [PATCH 1/9] test(webhooks): pass operation_id to send_mcp in the webhook sender tests operation_id became a required keyword of WebhookSender.send_mcp in #1066. Twenty-five calls in test_webhook_sender_e2e.py, test_webhook_sender_alt_auth.py, and test_webhook_sender_from_pem.py still omitted it, so 24 tests raised TypeError before they reached the behavior they test. CI doesn't install fastapi, so the modules skip there and nothing reported it (#1524). Each call now passes operation_id. --- .../signing/test_webhook_sender_alt_auth.py | 7 +++++++ .../signing/test_webhook_sender_e2e.py | 15 +++++++++++++++ .../signing/test_webhook_sender_from_pem.py | 3 +++ 3 files changed, 25 insertions(+) diff --git a/tests/conformance/signing/test_webhook_sender_alt_auth.py b/tests/conformance/signing/test_webhook_sender_alt_auth.py index dc61c5f78..c89afe805 100644 --- a/tests/conformance/signing/test_webhook_sender_alt_auth.py +++ b/tests/conformance/signing/test_webhook_sender_alt_auth.py @@ -98,6 +98,7 @@ async def test_bearer_token_attaches_authorization_header() -> None: result = await sender.send_mcp( url="http://test/webhooks/adcp", task_id="task_bearer", + operation_id="op_test", task_type="create_media_buy", status="completed", ) @@ -124,6 +125,7 @@ async def test_bearer_extra_headers_cannot_override_authorization() -> None: await sender.send_mcp( url="http://test/webhooks/adcp", task_id="t", + operation_id="op_test", task_type="create_media_buy", status="completed", extra_headers={"Authorization": "Bearer attacker"}, @@ -148,6 +150,7 @@ async def test_adcp_legacy_hmac_round_trips_against_verifier() -> None: result = await sender.send_mcp( url="http://test/webhooks/adcp", task_id="task_legacy_hmac", + operation_id="op_test", task_type="create_media_buy", status="completed", ) @@ -184,6 +187,7 @@ async def test_adcp_legacy_hmac_resend_re_signs_with_fresh_timestamp() -> None: first = await sender.send_mcp( url="http://test/webhooks/adcp", task_id="task_resend", + operation_id="op_test", task_type="create_media_buy", status="completed", ) @@ -267,6 +271,7 @@ async def test_standard_webhooks_round_trips_against_verifier() -> None: result = await sender.send_mcp( url="http://test/webhooks/adcp", task_id="task_sw", + operation_id="op_test", task_type="create_media_buy", status="completed", ) @@ -300,6 +305,7 @@ async def test_standard_webhooks_resend_uses_fresh_msg_id_and_timestamp() -> Non first = await sender.send_mcp( url="http://test/webhooks/adcp", task_id="task_sw_resend", + operation_id="op_test", task_type="create_media_buy", status="completed", ) @@ -331,6 +337,7 @@ async def test_standard_webhooks_svix_verifier_interop() -> None: result = await sender.send_mcp( url="http://test/webhooks/adcp", task_id="task_sw_svix", + operation_id="op_test", task_type="create_media_buy", status="completed", ) diff --git a/tests/conformance/signing/test_webhook_sender_e2e.py b/tests/conformance/signing/test_webhook_sender_e2e.py index 9c58539e0..fe8670e26 100644 --- a/tests/conformance/signing/test_webhook_sender_e2e.py +++ b/tests/conformance/signing/test_webhook_sender_e2e.py @@ -109,6 +109,7 @@ async def test_sender_to_receiver_happy_path() -> None: result = await sender.send_mcp( url="http://test/webhooks/adcp", task_id="task_sender_e2e", + operation_id="op_test", task_type="create_media_buy", status="completed", result={"media_buy_id": "mb_1"}, @@ -132,6 +133,7 @@ async def test_retry_with_same_idempotency_key_dedupes() -> None: first = await sender.send_mcp( url="http://test/webhooks/adcp", task_id="task_retry", + operation_id="op_test", task_type="create_media_buy", status="completed", ) @@ -139,6 +141,7 @@ async def test_retry_with_same_idempotency_key_dedupes() -> None: retry = await sender.send_mcp( url=first.url, task_id="task_retry", + operation_id="op_test", task_type="create_media_buy", status="completed", idempotency_key=first.idempotency_key, @@ -163,6 +166,7 @@ async def test_extra_headers_cannot_override_signature_bindings() -> None: await sender.send_mcp( url="http://test/webhooks/adcp", task_id="t", + operation_id="op_test", task_type="create_media_buy", status="completed", extra_headers={"Signature": "attacker-controlled"}, @@ -171,6 +175,7 @@ async def test_extra_headers_cannot_override_signature_bindings() -> None: await sender.send_mcp( url="http://test/webhooks/adcp", task_id="t", + operation_id="op_test", task_type="create_media_buy", status="completed", extra_headers={"Content-Digest": "forged"}, @@ -318,6 +323,7 @@ async def capture(request: Request) -> JSONResponse: result = await sender.send_mcp( url="http://test/webhooks/adcp/capture", task_id="task_byte_identity", + operation_id="op_test", task_type="create_media_buy", status="completed", result={"media_buy_id": "mb_bytes"}, @@ -338,6 +344,7 @@ async def test_resend_replays_identical_bytes_with_fresh_signature() -> None: first = await sender.send_mcp( url="http://test/webhooks/adcp", task_id="task_resend", + operation_id="op_test", task_type="create_media_buy", status="completed", result={"media_buy_id": "mb_resend"}, @@ -374,6 +381,7 @@ async def test_extra_header_case_insensitive_rejection() -> None: await sender.send_mcp( url="http://test/webhooks/adcp", task_id="t", + operation_id="op_test", task_type="create_media_buy", status="completed", extra_headers={bad: "attacker"}, @@ -397,6 +405,7 @@ async def test_receiver_rejects_when_sender_key_wrong() -> None: result = await sender.send_mcp( url="http://test/webhooks/adcp", task_id="t", + operation_id="op_test", task_type="create_media_buy", status="completed", ) @@ -419,6 +428,7 @@ async def test_owned_client_rejects_loopback_destination() -> None: await sender.send_mcp( url="https://127.0.0.1/webhooks/adcp", task_id="task_ssrf", + operation_id="op_test", task_type="create_media_buy", status="completed", ) @@ -451,6 +461,7 @@ async def test_owned_client_rejects_disallowed_port_when_hardening_configured() await sender.send_mcp( url="https://example.com:6379/webhooks/adcp", task_id="task_port", + operation_id="op_test", task_type="create_media_buy", status="completed", ) @@ -538,6 +549,7 @@ def fake_build(uri: str, **_kwargs: Any) -> Any: result = await sender.send_mcp( url="http://test:9443/webhooks/adcp", task_id="task_nonstd", + operation_id="op_test", task_type="create_media_buy", status="completed", ) @@ -575,6 +587,7 @@ async def echo(_request: Request) -> JSONResponse: result = await sender.send_mcp( url="http://test/webhooks/adcp", task_id="task_op_trust", + operation_id="op_test", task_type="create_media_buy", status="completed", ) @@ -643,6 +656,7 @@ async def post(self, *_args: Any, **_kwargs: Any) -> Any: await sender.send_mcp( url="https://buyer.example.com/webhooks/adcp", task_id="task_proxy", + operation_id="op_test", task_type="create_media_buy", status="completed", ) @@ -698,6 +712,7 @@ def reserved_headers(self) -> frozenset[str]: await sender.send_mcp( url="https://127.0.0.1/webhooks/adcp", task_id="task_no_sign", + operation_id="op_test", task_type="create_media_buy", status="completed", ) diff --git a/tests/conformance/signing/test_webhook_sender_from_pem.py b/tests/conformance/signing/test_webhook_sender_from_pem.py index 7c37c0d7b..339f9d0f8 100644 --- a/tests/conformance/signing/test_webhook_sender_from_pem.py +++ b/tests/conformance/signing/test_webhook_sender_from_pem.py @@ -95,6 +95,7 @@ async def test_from_pem_signatures_verify_against_public_jwk( result = await sender.send_mcp( url="http://test/webhooks/adcp", task_id="task_from_pem", + operation_id="op_test", task_type="create_media_buy", status="completed", result={"media_buy_id": "mb_1"}, @@ -121,6 +122,7 @@ async def test_from_pem_accepts_raw_bytes() -> None: result = await sender.send_mcp( url="http://test/webhooks/adcp", task_id="task_bytes", + operation_id="op_test", task_type="create_media_buy", status="completed", ) @@ -148,6 +150,7 @@ async def test_from_pem_encrypted_pem_with_passphrase(tmp_path: Path) -> None: result = await sender.send_mcp( url="http://test/webhooks/adcp", task_id="task_enc", + operation_id="op_test", task_type="create_media_buy", status="completed", ) From 90e78c2e87a524721e9d8d4181bc1ea9d165cec1 Mon Sep 17 00:00:00 2001 From: "Constantine.mirin" Date: Sun, 18 Oct 2026 03:39:01 -0400 Subject: [PATCH 2/9] test(webhooks): give the proxy-env test's fake client the stream method the sender calls Since #1066 the sender posts through client.stream, which bounds the response body it keeps. The fake httpx.AsyncClient in test_owned_client_ignores_https_proxy_env still had only post, so the test failed with AttributeError once the operation_id fix let it run. The fake now yields its response from stream. --- tests/conformance/signing/test_webhook_sender_e2e.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/tests/conformance/signing/test_webhook_sender_e2e.py b/tests/conformance/signing/test_webhook_sender_e2e.py index fe8670e26..9254c62b9 100644 --- a/tests/conformance/signing/test_webhook_sender_e2e.py +++ b/tests/conformance/signing/test_webhook_sender_e2e.py @@ -14,8 +14,10 @@ from __future__ import annotations +import contextlib import copy import json +from collections.abc import AsyncIterator from pathlib import Path from typing import Any @@ -629,7 +631,9 @@ def __init__(self, **kwargs: Any) -> None: # delivery path. The last writer wins; the per-request call # is the one we care about. captured_kwargs.update(kwargs) - self._response = MagicMock(status_code=200, headers={}, content=b"{}") + self._response = MagicMock( + status_code=200, headers={}, content=b"{}", is_stream_consumed=True + ) async def __aenter__(self) -> _FakeAsyncClient: return self @@ -640,8 +644,10 @@ async def __aexit__(self, *_args: Any) -> None: async def aclose(self) -> None: return None - async def post(self, *_args: Any, **_kwargs: Any) -> Any: - return self._response + # The sender posts with ``client.stream`` to bound the response body. + @contextlib.asynccontextmanager + async def stream(self, *_args: Any, **_kwargs: Any) -> AsyncIterator[Any]: + yield self._response # Set HTTPS_PROXY pointing at an unreachable address. With # trust_env=True (the httpx default), this would override the From 1e66161904797527186213cd0011a56db2d9657f Mon Sep 17 00:00:00 2001 From: "Constantine.mirin" Date: Sun, 18 Oct 2026 03:39:09 -0400 Subject: [PATCH 3/9] test(webhooks): retry with the first delivery's timestamp in the dedupe test An idempotency key binds the payload. test_retry_with_same_idempotency_key_dedupes called send_mcp twice with the same key and no timestamp, so each call stamped its own time, the second body differed, and the receiver answered 409 instead of a duplicate 200. send_mcp's docstring names the conflict. Both calls now pass one timestamp, so the retry repeats the first body and the receiver dedupes it. --- tests/conformance/signing/test_webhook_sender_e2e.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/tests/conformance/signing/test_webhook_sender_e2e.py b/tests/conformance/signing/test_webhook_sender_e2e.py index 9254c62b9..a2687c8d2 100644 --- a/tests/conformance/signing/test_webhook_sender_e2e.py +++ b/tests/conformance/signing/test_webhook_sender_e2e.py @@ -129,7 +129,14 @@ async def test_sender_to_receiver_happy_path() -> None: async def test_retry_with_same_idempotency_key_dedupes() -> None: """The whole reason WebhookDeliveryResult exposes idempotency_key: so the retry call reuses it and the receiver dedupes correctly. Without this, - every retry processes as a fresh event.""" + every retry processes as a fresh event. + + The key binds the payload, so the retry repeats the first call's + ``timestamp``: a fresh one makes a different body under the same key, + which the receiver refuses with 409.""" + from datetime import datetime, timezone + + sent_at = datetime(2026, 10, 18, 12, 0, tzinfo=timezone.utc) app, _ = _build_receiver_app() async with _build_sender(app) as sender: first = await sender.send_mcp( @@ -138,6 +145,7 @@ async def test_retry_with_same_idempotency_key_dedupes() -> None: operation_id="op_test", task_type="create_media_buy", status="completed", + timestamp=sent_at, ) # Simulate caller deciding to retry (e.g., if first had been non-2xx). retry = await sender.send_mcp( @@ -146,6 +154,7 @@ async def test_retry_with_same_idempotency_key_dedupes() -> None: operation_id="op_test", task_type="create_media_buy", status="completed", + timestamp=sent_at, idempotency_key=first.idempotency_key, ) From 1c54ef9c72b6465d56ddc2867f9a24abb5567377 Mon Sep 17 00:00:00 2001 From: "Constantine.mirin" Date: Sun, 18 Oct 2026 03:40:18 -0400 Subject: [PATCH 4/9] test(signing): sign the digest-off FastAPI case with the 3.1 profile Since #1038 the 3.2 signer refuses a non-empty body without content-digest, as AdCP 3.2 request signing requires. The either-False case of test_signed_request_verifies_end_to_end asked the 3.2 signer for exactly that and raised ValueError before it reached the verifier. The case now signs with the 3.1 profile, where content-digest is optional, which is what the either policy accepts; the required case still signs with 3.2. --- tests/conformance/signing/test_e2e_fastapi.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/tests/conformance/signing/test_e2e_fastapi.py b/tests/conformance/signing/test_e2e_fastapi.py index e86b75b71..547ab554f 100644 --- a/tests/conformance/signing/test_e2e_fastapi.py +++ b/tests/conformance/signing/test_e2e_fastapi.py @@ -64,8 +64,14 @@ async def create_media_buy(request: Request) -> Any: return app -@pytest.mark.parametrize("policy,cover_digest", [("either", False), ("required", True)]) -async def test_signed_request_verifies_end_to_end(policy: str, cover_digest: bool) -> None: +# AdCP 3.2 signers must cover content-digest over a non-empty body, so the +# digest-off case signs with the 3.1 profile, the verifier's default. +@pytest.mark.parametrize( + "policy,cover_digest,profile", [("either", False, "3.1"), ("required", True, "3.2")] +) +async def test_signed_request_verifies_end_to_end( + policy: str, cover_digest: bool, profile: str +) -> None: app = _build_app(policy) private_key = private_key_from_jwk(ED25519_KEY, d_field="_private_d_for_test_only") @@ -82,7 +88,7 @@ async def test_signed_request_verifies_end_to_end(policy: str, cover_digest: boo key_id="test-ed25519-2026", alg="ed25519", cover_content_digest=cover_digest, - signing_profile_version="3.2", + signing_profile_version=profile, ) request_headers = {**headers, **signed.as_dict()} From de442d157dd430069a6e5aa10d4dd668c8f55690 Mon Sep 17 00:00:00 2001 From: "Constantine.mirin" Date: Sun, 18 Oct 2026 03:42:15 -0400 Subject: [PATCH 5/9] test(webhooks): re-sign the retry in the HTTP dedupe test test_duplicate_retry_dedupes_over_real_http posted the same body twice with the same Signature and nonce, so the receiver refused the second POST with 401 webhook_signature_replayed before it reached deduplication. A sender's retry repeats the body under a fresh signature, as WebhookSender.resend does. Each POST now carries its own signature, and the second one gets 200 with duplicate=true. --- .../conformance/signing/test_webhook_e2e_fastapi.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/tests/conformance/signing/test_webhook_e2e_fastapi.py b/tests/conformance/signing/test_webhook_e2e_fastapi.py index 80a050695..9ef6cb5bf 100644 --- a/tests/conformance/signing/test_webhook_e2e_fastapi.py +++ b/tests/conformance/signing/test_webhook_e2e_fastapi.py @@ -138,7 +138,11 @@ async def test_signed_webhook_verifies_end_to_end() -> None: @pytest.mark.asyncio async def test_duplicate_retry_dedupes_over_real_http() -> None: """The sender's retry MUST get a 200 with duplicate=True, and the - receiver MUST NOT reprocess. Real HTTP roundtrip twice.""" + receiver MUST NOT reprocess. Real HTTP roundtrip twice. + + A retry repeats the body under a fresh signature, as + ``WebhookSender.resend`` does: the receiver refuses a replayed nonce + before it reaches deduplication.""" app, _ = _build_app() payload = create_mcp_webhook_payload( @@ -149,12 +153,13 @@ async def test_duplicate_retry_dedupes_over_real_http() -> None: idempotency_key="whk_e2e_dupeaaaaaaaaaaaaaaa", ) body = json.dumps(to_wire_dict(payload)).encode("utf-8") - headers = _sign_and_send_body(body) transport = httpx.ASGITransport(app=app) async with httpx.AsyncClient(transport=transport, base_url="http://test") as client: - first = await client.post("/webhooks/adcp", content=body, headers=headers) - second = await client.post("/webhooks/adcp", content=body, headers=headers) + first = await client.post("/webhooks/adcp", content=body, headers=_sign_and_send_body(body)) + second = await client.post( + "/webhooks/adcp", content=body, headers=_sign_and_send_body(body) + ) assert first.status_code == 200 assert first.json()["duplicate"] is False From 4546605c02018aed8fab88656e520b9b7d9c7b31 Mon Sep 17 00:00:00 2001 From: "Constantine.mirin" Date: Sun, 18 Oct 2026 23:40:29 -0400 Subject: [PATCH 6/9] ci: install fastapi and svix with the dev extra Every CI job installs the package with .[dev] or .[dev,pg], and neither extra resolved fastapi or svix. Five signing test modules skip whole on pytest.importorskip("fastapi"), and test_standard_webhooks.py skips its two svix interop tests, so CI never ran them and their failures went unseen (#1524). The dev extra now carries both packages, so each job that runs the signing suite runs these files too. Closes #1524 --- pyproject.toml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pyproject.toml b/pyproject.toml index fdfbd956e..9af5e1913 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -158,6 +158,12 @@ dev = [ # tests/test_mcp_middleware_composition.py and future integration # tests that exercise the streamable-HTTP ASGI app in-process. "asgi-lifespan>=2.1.0", + # The signing suite's end-to-end tests serve a FastAPI app, and its + # Standard Webhooks interop tests verify with svix. Each skips its module + # (pytest.importorskip) when the package is missing, so without these + # CI never runs them (#1524). + "fastapi>=0.100", + "svix>=1.0", # mypy stubs for the protobuf runtime we use via a2a-sdk 1.0 # (``google.protobuf.json_format``, ``struct_pb2``, ``timestamp_pb2``). # Without these mypy flags every import as ``import-untyped``. From 67172aa1825bcabcb412d226ed25696127f46054 Mon Sep 17 00:00:00 2001 From: "Constantine.mirin" Date: Sun, 18 Oct 2026 23:43:37 -0400 Subject: [PATCH 7/9] fix(governance): require each critical marker, its crit entry, and its claim together security.mdx checklist step 4 (AdCP 3.2.1 to 3.2.5) requires, for authorized_commitment, authorized_task, and authorized_payload_hash, "each boolean protected-header marker, its crit entry, and its payload claim to appear together", and the unknown-extension rule says verifiers "MUST also reject when the critical marker and its corresponding claim do not appear together". verify_governance_authorization compared the claim only against a crit entry with a true marker, so it accepted a marker for a claim the payload doesn't carry: a crit entry with a false marker, a marker with no crit entry, or a false marker alone. The check now requires the three to be present together, with the marker true. test_critical_marker_crit_entry_and_claim_appear_together re-signs the accepted vector with each header, so only the crit check can refuse it; it failed before this change. --- src/adcp/governance.py | 7 ++- tests/test_governance_authorization.py | 87 ++++++++++++++++++++++++++ 2 files changed, 91 insertions(+), 3 deletions(-) diff --git a/src/adcp/governance.py b/src/adcp/governance.py index ba38f9b56..f4428521e 100644 --- a/src/adcp/governance.py +++ b/src/adcp/governance.py @@ -1156,9 +1156,10 @@ def reject( ): return reject("governance_token_invalid", "invalid governance JWS crit header") for name in GOVERNANCE_AUTHORIZATION_CRITICAL_CLAIMS: - has_claim = name in claims - has_marker = name in critical and header.get(name) is True - if has_claim != has_marker: + # security.mdx checklist step 4: the marker, its crit entry, and its + # claim appear together, and the marker is true. + together = (name in claims) == (name in critical) == (name in header) + if not together or header.get(name, True) is not True: return reject( "governance_token_invalid", f"{name} claim and critical marker must match" ) diff --git a/tests/test_governance_authorization.py b/tests/test_governance_authorization.py index 313a4f6d1..4dc3c57cb 100644 --- a/tests/test_governance_authorization.py +++ b/tests/test_governance_authorization.py @@ -37,6 +37,7 @@ ) from adcp.server.idempotency import IdempotencyStore, MemoryBackend from adcp.signing import InMemoryReplayStore, StaticJwksResolver, private_key_from_jwk +from adcp.signing.crypto import ALG_ED25519, b64url_decode, b64url_encode, sign_signature_base VECTOR_PATH = Path(__file__).parent / "conformance/vectors/governance-authorization.json" VECTORS = json.loads(VECTOR_PATH.read_text()) @@ -618,3 +619,89 @@ def test_stateless_governance_applicability( task: str, payload: dict[str, Any], expected: bool | None ) -> None: assert stateless_governance_applicability(task, payload) is expected + + +async def _verify_signed_vector( + header_updates: dict[str, Any], drop_claims: tuple[str, ...] = () +) -> Any: + """Re-sign the accepted vector with a changed header, then verify it. + + Re-signing keeps the signature valid, so a refusal comes from the header + or claims check, not from signature verification. + """ + signed = VECTORS["signed_jws"] + case = next(case for case in signed["cases"] if case["id"] == "valid-exact-authorization") + b64_header, b64_claims, _ = case["compact_jws"].split(".") + header = {**json.loads(b64url_decode(b64_header)), **header_updates} + claims = { + name: value + for name, value in json.loads(b64url_decode(b64_claims)).items() + if name not in drop_claims + } + test_key = dict(signed["test_key"]) + test_key["d"] = test_key.pop("_private_d_for_test_only") + signing_input = ".".join( + b64url_encode(json.dumps(segment, separators=(",", ":")).encode()) + for segment in (header, claims) + ) + signature = sign_signature_base( + alg=ALG_ED25519, + private_key=private_key_from_jwk(test_key), + signature_base=signing_input.encode("ascii"), + ) + defaults = signed["verification_defaults"] + public_jwk = { + key: value for key, value in test_key.items() if key != "d" and not key.startswith("$") + } + return await verify_governance_authorization( + token=f"{signing_input}.{b64url_encode(signature)}", + expected_issuer=defaults["expected_issuer"], + expected_audience=defaults["expected_audience"], + authenticated_caller=defaults["authenticated_caller"], + expected_task=defaults["expected_task"], + payload=defaults["payload"], + actual_commitment=defaults["actual_commitment"], + expected_phase=defaults["expected_phase"], + jwks=StaticJwksResolver({"keys": [public_jwk]}), + replay_store=InMemoryGovernanceReplayStore(), + now=lambda: defaults["now"], + clock_skew_seconds=defaults["clock_skew_seconds"], + ) + + +@pytest.mark.asyncio +async def test_a_re_signed_vector_is_accepted() -> None: + """``_verify_signed_vector`` re-signs a token that the verifier accepts.""" + result = await _verify_signed_vector({}) + + assert result.ok + + +@pytest.mark.parametrize( + "header", + [ + { + "crit": ["authorized_commitment", "authorized_task", "authorized_payload_hash"], + "authorized_commitment": False, + }, + {"crit": ["authorized_task", "authorized_payload_hash"]}, + {"crit": ["authorized_task", "authorized_payload_hash"], "authorized_commitment": False}, + ], + ids=["crit-entry-and-false-marker", "marker-alone", "false-marker-alone"], +) +@pytest.mark.asyncio +async def test_critical_marker_crit_entry_and_claim_appear_together( + header: dict[str, Any], +) -> None: + """security.mdx checklist step 4: marker, crit entry, and claim appear together. + + "Verifiers MUST also reject when the critical marker and its corresponding + claim do not appear together" (security.mdx, AdCP 3.2.1 to 3.2.4). Each + header here carries an ``authorized_commitment`` marker or ``crit`` entry + for a claim the payload doesn't have. + """ + result = await _verify_signed_vector(header, drop_claims=("authorized_commitment",)) + + assert not result.ok + assert result.error == "governance_token_invalid" + assert result.message == "authorized_commitment claim and critical marker must match" From 05c8bc54587ddb673712b86d3155836af0aae4fa Mon Sep 17 00:00:00 2001 From: "Constantine.mirin" Date: Sun, 18 Oct 2026 23:44:19 -0400 Subject: [PATCH 8/9] fix(governance): answer a non-string token alg with governance_token_invalid verify_governance_authorization tested the header's alg for membership in a set. A JSON list or object there is unhashable, so the verifier raised TypeError instead of refusing the token. It now refuses an alg that isn't a string as governance_token_invalid, the code the other malformed-header cases get. test_a_non_string_alg_is_an_invalid_token failed before this change. --- src/adcp/governance.py | 6 +++++- tests/test_governance_authorization.py | 10 ++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/adcp/governance.py b/src/adcp/governance.py index f4428521e..06e704575 100644 --- a/src/adcp/governance.py +++ b/src/adcp/governance.py @@ -1145,7 +1145,11 @@ def reject( except (TypeError, ValueError): return reject("governance_token_invalid", "governance_context contains invalid JSON") alg = header.get("alg") - if alg not in {"EdDSA", "ES256"} or header.get("typ") != GOVERNANCE_AUTHORIZATION_TYP: + if ( + type(alg) is not str + or alg not in {"EdDSA", "ES256"} + or header.get("typ") != GOVERNANCE_AUTHORIZATION_TYP + ): return reject("governance_token_invalid", "unsupported governance JWS alg or typ") critical = header.get("crit") if ( diff --git a/tests/test_governance_authorization.py b/tests/test_governance_authorization.py index 4dc3c57cb..b7b140dc4 100644 --- a/tests/test_governance_authorization.py +++ b/tests/test_governance_authorization.py @@ -705,3 +705,13 @@ async def test_critical_marker_crit_entry_and_claim_appear_together( assert not result.ok assert result.error == "governance_token_invalid" assert result.message == "authorized_commitment claim and critical marker must match" + + +@pytest.mark.parametrize("alg", [["EdDSA"], {"EdDSA": True}], ids=["list", "object"]) +@pytest.mark.asyncio +async def test_a_non_string_alg_is_an_invalid_token(alg: object) -> None: + """A header ``alg`` that isn't a string is ``governance_token_invalid``, not a TypeError.""" + result = await _verify_signed_vector({"alg": alg}) + + assert not result.ok + assert result.error == "governance_token_invalid" From 2c4fdb1c7ea58957a3fe1d411443d5ce3f4f09d7 Mon Sep 17 00:00:00 2001 From: "Constantine.mirin" Date: Mon, 19 Oct 2026 02:37:21 -0400 Subject: [PATCH 9/9] fix(signing): decode base64url strictly JWS and JWK members are base64url: the RFC 4648 section 5 alphabet with the padding omitted (RFC 7515 section 2). RFC 4648 section 3.3 says implementations MUST reject encoded data that contains characters outside the base alphabet. b64url_decode used the standard library's lenient decoder, which accepted padding and the standard alphabet's + and /, dropped other characters, and ignored trailing bits. Verifier step 8 decodes the signer's JWK through public_key_from_jwk, so the request and webhook verifiers accepted a JWK whose x member had "!!" inserted. b64url_decode now refuses padding, + and /, and decodes the rest with base64.b64decode(altchars=b"-_", validate=True), which refuses any other character. It also refuses non-zero trailing bits (RFC 4648 section 3.5), so each value has the one spelling b64url_encode produces. Every caller already handles the ValueError it raises. test_jwk_members_are_strict_base64url failed before this change. --- src/adcp/signing/crypto.py | 16 ++++++++++--- tests/conformance/signing/test_crypto.py | 30 ++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/src/adcp/signing/crypto.py b/src/adcp/signing/crypto.py index 437931ac2..e0ac250dd 100644 --- a/src/adcp/signing/crypto.py +++ b/src/adcp/signing/crypto.py @@ -40,9 +40,19 @@ def b64url_decode(s: str) -> bytes: - """Decode base64url, tolerating missing padding.""" - pad = "=" * (-len(s) % 4) - return base64.urlsafe_b64decode(s + pad) + """Decode RFC 7515 base64url: the URL-safe alphabet, no padding, one spelling. + + RFC 4648 section 3.3 requires refusing characters outside the alphabet, + so padding, ``+``, ``/``, and any other character raise ``ValueError``, + and so do non-zero trailing bits (section 3.5), which would give the same + bytes a second spelling. + """ + if "=" in s or "+" in s or "/" in s: + raise ValueError("not unpadded base64url") + raw = base64.b64decode(s + "=" * (-len(s) % 4), altchars=b"-_", validate=True) + if b64url_encode(raw) != s: + raise ValueError("non-canonical base64url") + return raw def b64url_encode(b: bytes) -> str: diff --git a/tests/conformance/signing/test_crypto.py b/tests/conformance/signing/test_crypto.py index bc0da8c04..64a9c7db3 100644 --- a/tests/conformance/signing/test_crypto.py +++ b/tests/conformance/signing/test_crypto.py @@ -11,6 +11,7 @@ from __future__ import annotations import json +import string from pathlib import Path import pytest @@ -161,3 +162,32 @@ def test_jwk_with_wrong_length_x_rejected() -> None: } with pytest.raises(ValueError, match="length"): public_key_from_jwk(bad) + + +_B64URL = string.ascii_uppercase + string.ascii_lowercase + string.digits + "-_" + + +@pytest.mark.parametrize( + "spoil", + [ + lambda x: x[:10] + "!!" + x[10:], # characters outside the alphabet + lambda x: x[:10] + "+/" + x[12:], # the standard alphabet + lambda x: x + "=", # padding + lambda x: x[:-1] + _B64URL[_B64URL.index(x[-1]) ^ 1], # same bytes, non-zero pad bits + ], + ids=["alphabet", "standard-alphabet", "padding", "trailing-bits"], +) +def test_jwk_members_are_strict_base64url(spoil) -> None: + """RFC 7515 section 2 base64url: no padding, no other characters, one spelling. + + Verifier step 8 decodes the signer's JWK with ``public_key_from_jwk``, so a + lenient decoder would accept a key whose members a strict verifier refuses. + """ + from adcp.signing.crypto import b64url_decode + + jwk = {k: v for k, v in KEYS["test-ed25519-2026"].items() if not k.startswith("_")} + assert public_key_from_jwk(jwk) + with pytest.raises(ValueError): + public_key_from_jwk({**jwk, "x": spoil(jwk["x"])}) + with pytest.raises(ValueError): + b64url_decode(spoil(jwk["x"]))