From 9b295ad274d7cd57f0f72c1e6190d79e553d1222 Mon Sep 17 00:00:00 2001 From: Vishal Katyal Date: Tue, 1 Sep 2026 10:27:18 -0400 Subject: [PATCH] fix(rest/python): read signing_keys[] where the schema places it and drop the ucp.keys[] mirror --- rest/python/client/flower_shop/signing.py | 9 ++- rest/python/server/README.md | 4 +- rest/python/server/dependencies.py | 2 +- rest/python/server/integration_test.py | 17 ++++- rest/python/server/routes/discovery.py | 15 ++-- .../server/signature_integration_test.py | 5 +- rest/python/server/ucp_signing.py | 23 ++++-- rest/python/server/ucp_signing_test.py | 75 +++++++++++++++---- 8 files changed, 113 insertions(+), 37 deletions(-) diff --git a/rest/python/client/flower_shop/signing.py b/rest/python/client/flower_shop/signing.py index e03314c4..c6efd7fb 100644 --- a/rest/python/client/flower_shop/signing.py +++ b/rest/python/client/flower_shop/signing.py @@ -159,7 +159,14 @@ class LocalProfileServer: def __init__(self, jwk: dict, version: str = "2026-01-23") -> None: """Prepare the profile document and HTTP server (not yet started).""" - document = json.dumps({"ucp": {"version": version, "keys": [jwk]}}).encode() + # The merchant server's ucp_signing._extract_keys reads signing_keys[] + # as a top-level sibling of ucp (source/discovery/profile_schema.json + # $defs/base at the 2026-04-08 pin it declares), never a field nested + # inside ucp -- publish this demo profile the same way so the + # sign-then-verify loop it demonstrates actually round-trips. + document = json.dumps( + {"ucp": {"version": version}, "signing_keys": [jwk]} + ).encode() class _Handler(http.server.BaseHTTPRequestHandler): def do_GET(self) -> None: # noqa: N802 (http.server API) diff --git a/rest/python/server/README.md b/rest/python/server/README.md index 967c5eab..1ff7380d 100644 --- a/rest/python/server/README.md +++ b/rest/python/server/README.md @@ -97,7 +97,7 @@ The server verifies UCP request signatures as defined in the specification's [RFC 9421](https://www.rfc-editor.org/rfc/rfc9421.html) HTTP Message Signatures with an [RFC 9530](https://www.rfc-editor.org/rfc/rfc9530.html) `Content-Digest` over the raw body. The signer's public key is discovered from the profile URL in -the `UCP-Agent` header (its `keys[]`). `ES256` (fixed-width raw `r||s`, not +the `UCP-Agent` header (its `signing_keys[]`). `ES256` (fixed-width raw `r||s`, not ASN.1/DER) is the baseline; `Ed25519` is also supported. Behaviour is controlled by two flags: @@ -138,7 +138,7 @@ signed components cover the full request-signing table (`@method`, `content-digest`, `content-type`, `idempotency-key`, `ucp-agent`) plus the Standard Webhooks event headers (`webhook-id`, `webhook-timestamp`). The matching public JWK is published in the served profile's `signing_keys[]` -(and mirrored into `ucp.keys[]`) so platforms can verify. +so platforms can verify. Failed deliveries — transport errors or a 5xx from the receiver — are retried with exponential backoff, as `order.md` requires; a 4xx is treated as a diff --git a/rest/python/server/dependencies.py b/rest/python/server/dependencies.py index 1434c7d3..0426e6fe 100644 --- a/rest/python/server/dependencies.py +++ b/rest/python/server/dependencies.py @@ -72,7 +72,7 @@ async def verify_signature(request: Request) -> None: """Verify an inbound request's RFC 9421 signature per the UCP spec. The signer's public keys are discovered from the ``UCP-Agent`` header's - profile URL (its ``keys[]``). Behaviour depends on + profile URL (its ``signing_keys[]``). Behaviour depends on ``--require_signatures``: * When set, a missing or invalid signature is rejected with the spec's diff --git a/rest/python/server/integration_test.py b/rest/python/server/integration_test.py index 55b0ba63..7954d3e4 100644 --- a/rest/python/server/integration_test.py +++ b/rest/python/server/integration_test.py @@ -1301,15 +1301,24 @@ def test_profile_publishes_the_webhook_signing_key(self) -> None: """The served profile publishes the webhook public key for verifiers. signatures.md, Key Discovery: public keys live in the profile's - signing_keys[] (a top-level sibling of `ucp` per the discovery profile - schema). It is also mirrored into ucp.keys[], the JWK Set this server's - own verifier resolves. + signing_keys[], a top-level sibling of `ucp` per the discovery profile + schema at this server's declared version (source/discovery/ + profile_schema.json $defs/base at the 2026-04-08 pin -- + config.get_server_version()). That schema defines no `keys` field + anywhere, nested or otherwise; this server must not publish one, so a + peer's strict 2026-04-08 verifier sees exactly the fields the schema + promises and nothing it does not. """ with self.client: profile = self.client.get("/.well-known/ucp").json() jwk = webhook_signer.public_jwk() self.assertIn(jwk, profile.get("signing_keys", [])) - self.assertIn(jwk, profile.get("ucp", {}).get("keys", [])) + self.assertNotIn( + "keys", + profile.get("ucp", {}), + "ucp.keys[] has no basis in the 2026-04-08 schema and must not be " + "published", + ) def test_version_invalid_format(self) -> None: """Tests that UCP-Agent with invalid version format is rejected.""" diff --git a/rest/python/server/routes/discovery.py b/rest/python/server/routes/discovery.py index e4f8f21a..207a04d3 100644 --- a/rest/python/server/routes/discovery.py +++ b/rest/python/server/routes/discovery.py @@ -63,13 +63,16 @@ async def get_merchant_profile(request: Request, response: Response): # Publish the webhook-signing public key so platforms can verify our # order-event deliveries (order.md, Webhook Signature Verification / - # signatures.md, Key Discovery). The discovery profile schema places - # signing_keys[] at the top level of the served document (a sibling of - # `ucp`); it is mirrored into ucp.keys[], the RFC 7517 JWK Set this - # server's own verifier (ucp_signing._extract_keys) resolves, so both - # discovery conventions find the key. + # signatures.md, Key Discovery). source/discovery/profile_schema.json + # $defs/base at the 2026-04-08 pin this server declares + # (config.get_server_version()) requires `ucp` and separately declares + # `signing_keys` as a top-level sibling of `ucp` -- that schema defines no + # `keys` field anywhere, nested or otherwise, so publish signing_keys[] + # only. ucp#566 renames this field to a top-level keys[] for 2026-08-25 + # and later; when this server's declared version moves to that pin, this + # field name must move with it in the same change, together with + # ucp_signing.py's _extract_keys(). jwk = webhook_signer.public_jwk() profile_data.setdefault("signing_keys", []).append(jwk) - ucp.setdefault("keys", []).append(jwk) return profile_data diff --git a/rest/python/server/signature_integration_test.py b/rest/python/server/signature_integration_test.py index 6873af09..b83508af 100644 --- a/rest/python/server/signature_integration_test.py +++ b/rest/python/server/signature_integration_test.py @@ -96,8 +96,11 @@ def setUp(self) -> None: rsa_jwk = {"kid": "rsa-key", "kty": "RSA", "n": "abc", "e": "AQAB"} version = config.get_server_version() + # source/discovery/profile_schema.json $defs/base at this server's + # declared 2026-04-08 pin: signing_keys is a top-level sibling of ucp, + # not a field nested inside it. good = json.dumps( - {"ucp": {"version": version, "keys": [agent_jwk, rsa_jwk]}} + {"ucp": {"version": version}, "signing_keys": [agent_jwk, rsa_jwk]} ).encode() _ProfileHandler.routes = { "/profile.json": (200, good), diff --git a/rest/python/server/ucp_signing.py b/rest/python/server/ucp_signing.py index 2db8d283..1bbaef31 100644 --- a/rest/python/server/ucp_signing.py +++ b/rest/python/server/ucp_signing.py @@ -21,7 +21,7 @@ * RFC 9530 `Content-Digest` over the raw body bytes (`sha-256`). * ECDSA P-256 (`ES256`) verification with fixed-width raw `r||s` signatures (RFC 9421 Section 3.3.1) -- never ASN.1/DER -- and Ed25519 (RFC 8032). -* Signer public-key discovery from the `UCP-Agent` profile's `keys[]`. +* Signer public-key discovery from the `UCP-Agent` profile's `signing_keys[]`. Only the encoding and canonicalization are implemented here; every cryptographic primitive comes from the `cryptography` package. The module is @@ -760,8 +760,8 @@ async def fetch_signing_keys( ) -> list[dict]: """Fetch and cache a signer's published signing keys from its UCP profile. - Reads the profile's ``keys[]`` (the canonical RFC 7517 JWK Set field as - of ucp#566, which removed ``signing_keys[]``). + Reads the top-level ``signing_keys[]`` of the document behind the + ``UCP-Agent`` profile URL. Args: profile_url: The signer's profile URL from the ``UCP-Agent`` header. @@ -812,11 +812,20 @@ async def fetch_signing_keys( def _extract_keys(document: dict) -> list[dict]: """Pull the signing keys out of a profile document. - ``keys[]`` is the canonical RFC 7517 JWK Set field per ucp#566, which removed - the earlier ``signing_keys[]``; this reference verifier reads only ``keys[]``. + At the UCP version this server declares (2026-04-08, + ``config.get_server_version()``), ``source/discovery/profile_schema.json`` + ``$defs/base`` requires ``ucp`` and separately declares ``signing_keys`` as + a top-level sibling of ``ucp`` -- not a field nested inside it. ``ucp`` is + required on every real profile document, so a reader that looks inside + ``ucp`` whenever it is present can never see a top-level sibling field on + any real document; this reads the top level directly instead. + + ucp#566 (merged upstream) renames this field to a top-level ``keys[]`` for + 2026-08-25 and later. When this server's declared version moves to that + pin, this field name must move with it, in the same change, together with + ``routes/discovery.py``'s publication side. """ if not isinstance(document, dict): return [] - ucp = document.get("ucp", document) - value = ucp.get("keys") if isinstance(ucp, dict) else None + value = document.get("signing_keys") return value if isinstance(value, list) and value else [] diff --git a/rest/python/server/ucp_signing_test.py b/rest/python/server/ucp_signing_test.py index 0a6ae7cc..2452e8ad 100644 --- a/rest/python/server/ucp_signing_test.py +++ b/rest/python/server/ucp_signing_test.py @@ -368,24 +368,33 @@ def factory(*args, **kwargs): finally: signing.httpx.AsyncClient = real_client - def test_reads_keys_from_ucp_envelope(self) -> None: - """keys[] (canonical per ucp#566) is read from the ucp envelope.""" + def test_reads_top_level_signing_keys(self) -> None: + """signing_keys[] (this server's declared 2026-04-08 pin) is read. + + Read from the top level, a sibling of ucp -- not nested inside it. A + real profile document always carries `ucp` (schema-required), so this + is the realistic shape. + """ keys = self._fetch( - lambda req: httpx.Response(200, json={"ucp": {"keys": [{"kid": "a"}]}}) + lambda req: httpx.Response( + 200, + json={"ucp": {"version": "2026-04-08"}, "signing_keys": [{"kid": "a"}]}, + ) ) self.assertEqual(keys[0]["kid"], "a") - def test_reads_top_level_keys(self) -> None: - """A top-level keys[] array (no ucp wrapper) is read.""" + def test_reads_top_level_signing_keys_without_ucp_wrapper(self) -> None: + """A top-level signing_keys[] array (no ucp wrapper) is read.""" keys = self._fetch( - lambda req: httpx.Response(200, json={"keys": [{"kid": "b"}]}) + lambda req: httpx.Response(200, json={"signing_keys": [{"kid": "b"}]}) ) self.assertEqual(keys[0]["kid"], "b") - def test_legacy_signing_keys_is_not_read(self) -> None: - """A profile with only the removed signing_keys[] resolves to no keys. + def test_signing_keys_nested_under_ucp_is_not_read(self) -> None: + """signing_keys nested under ucp has no basis in the schema. - The reference verifier models the merged spec (keys[] only, ucp#566). + This is not a legacy shape to tolerate -- nesting it under ucp was + never a valid location at any pin. """ with self.assertRaises(signing.SignatureError) as ctx: self._fetch( @@ -395,6 +404,19 @@ def test_legacy_signing_keys_is_not_read(self) -> None: ) self.assertEqual(ctx.exception.code, "profile_malformed") + def test_future_top_level_keys_is_not_yet_read(self) -> None: + """A top-level keys[] (2026-08-25 name, ucp#566) is not yet read. + + This server has not yet moved its declared version to 2026-08-25. + """ + with self.assertRaises(signing.SignatureError) as ctx: + self._fetch( + lambda req: httpx.Response( + 200, json={"ucp": {}, "keys": [{"kid": "future"}]} + ) + ) + self.assertEqual(ctx.exception.code, "profile_malformed") + def test_redirect_is_unreachable(self) -> None: """A 3xx response is treated as unreachable (no redirects allowed).""" with self.assertRaises(signing.SignatureError) as ctx: @@ -622,22 +644,45 @@ def resolve(name: str): class ExtractKeysTest(absltest.TestCase): - """_extract_keys reads keys[] (canonical per ucp#566) and tolerates junk.""" + """_extract_keys reads the top-level signing_keys[] and tolerates junk. + + This server's declared 2026-04-08 version defines signing_keys[] as a + sibling of ucp, never nested inside it. + """ def test_non_dict_document(self) -> None: """A non-object profile yields no keys, not an error.""" self.assertEqual(signing._extract_keys(["not", "a", "dict"]), []) - def test_reads_canonical_keys(self) -> None: - """keys[] under the ucp envelope is the canonical source.""" - doc = {"ucp": {"keys": [{"kid": "k"}]}} + def test_reads_top_level_signing_keys(self) -> None: + """Top-level signing_keys[], a sibling of ucp, is the canonical source.""" + doc = {"ucp": {"version": "2026-04-08"}, "signing_keys": [{"kid": "k"}]} self.assertEqual(signing._extract_keys(doc), [{"kid": "k"}]) - def test_removed_signing_keys_is_ignored(self) -> None: - """The removed signing_keys[] field is not read (ucp#566).""" + def test_signing_keys_nested_under_ucp_is_not_read(self) -> None: + """signing_keys[] nested under ucp is not read. + + It is a top-level field only. + """ doc = {"ucp": {"signing_keys": [{"kid": "old"}]}} self.assertEqual(signing._extract_keys(doc), []) + def test_keys_nested_under_ucp_is_not_read(self) -> None: + """keys[] nested under ucp is not read. + + No schema at any pin nests keys under ucp. + """ + doc = {"ucp": {"keys": [{"kid": "k"}]}} + self.assertEqual(signing._extract_keys(doc), []) + + def test_future_top_level_keys_is_not_read(self) -> None: + """A top-level keys[] (the 2026-08-25 name) is not read. + + This server declares 2026-04-08. + """ + doc = {"ucp": {"version": "2026-04-08"}, "keys": [{"kid": "future"}]} + self.assertEqual(signing._extract_keys(doc), []) + class SigCapableTest(absltest.TestCase): """Signature-capable key filtering (ucp#566).