fix(rest/python): read signing_keys[] where the schema places it and drop the ucp.keys[] mirror - #228
Open
vishkaty wants to merge 1 commit into
Conversation
…drop the ucp.keys[] mirror
carolinerg1
requested review from
carolinerg1,
damaz91 and
nicholasjameshall
September 3, 2026 20:35
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
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.
Fixes #227
This is the Python twin of #226, the Node fix for the identical defect
named in #225. Both servers reached it by the same two steps, called out
in that issue: here #122 introduced the wrong reader and #169 added the
matching mirror, as #162 and #179 did on the Node side.
Observed vs expected
This server declares UCP_VERSION 2026-04-08
(routes/discovery_profile.json). At that pin,
source/discovery/profile_schema.json $defs/base requires
ucpandseparately declares
signing_keysas a top level sibling ofucp; thatschema defines no
keysfield anywhere. discovery.py publishedsigning_keys[]correctly at the top level but also mirrored it into anested
ucp.keys[]with no schema basis at any pin, and_extract_keys()in ucp_signing.py read only that nested field. Since
ucpis required onevery real profile document, a reader that looks inside
ucpwhenever itis present can never see a top level sibling field on any real document,
so the verifier could not resolve the schema correct field the same
server publishes.
Fix
_extract_keys()now reads the top levelsigning_keysfield directly instead of descending intoucpfirst.Comment states the schema citation and flags that when this server
moves UCP_VERSION to 2026-08-25, the field name must move to
keysinthe same change, per ucp#566.
ucpobject no longer carries akeysmember. Top level
signing_keys[]is unchanged.keys[]tosigning_keys[].keys[] to signing_keys[] in the request signature section, and the
webhook section no longer claims the ucp.keys[] mirror.
ProfileFetchTestandExtractKeysTestrewrittenaround the schema correct top level shape; new tests prove nested
ucp.keys/ucp.signing_keysare not read, and that a top levelkeys[](the 2026-08-25 name) is not read yet either, since thisserver has not moved its declared version.
the request-signature verification tests moved from
{"ucp": {"keys": [...]}}to{"ucp": {}, "signing_keys": [...]},matching the schema correct shape. This fixture was missed by an
initial grep for
.keyssince it is a bare dict key, not an attributeaccess.
test_profile_publishes_the_webhook_signing_keynow asserts
ucp.keysis absent instead of asserting it is present.LocalProfileServer, the counterparty profile owned bythe demo happy path client, moved from the same nested
ucp.keysshape to top levelsigning_keys, so the sign then verifyloop it exists to demonstrate actually round-trips against the fixed
reader.
Verification
confirmed both the read side and write side defect first (mirroring
the linked issue), then every new assertion was confirmed red for the
stated reason before the fix and green after.
(test_profile_publishes_the_webhook_signing_key): passing before and
after, with its assertion strengthened from requiring the mirror to
forbidding it.
in isolation with the new tests kept: reverting
_extract_keysto theold nested
ucp.keysread fails 4 tests in ucp_signing_test.py; reintroducing theucp.keysmirror in discovery.py fails the discovery test. Both restored to green
after.
/.well-known/ucp document, confirmed
ucp.keysis absent and top levelsigning_keys[]is present, confirmed the fixed_extract_keyson this same server resolves the served document, and validated the
document with Python jsonschema (Draft202012Validator, real $ref
resolution across the full vendored release/2026-04-08 schema tree)
against source/discovery/profile_schema.json: VALID.
publishes its own JWK through a local profile server, against the
fixed server with --allow_insecure_profile_urls: the server logs
"RFC 9421 signature verified" for every signed request, proving the
full sign then verify loop this demo exists to show now round-trips
end to end.
clean.
Notes for reviewers
A fresh
uv syncon unpinned main today resolves ucp-sdk 0.5.0 and theserver fails to import (ModuleNotFoundError), independent of this
change; this reproduces Fresh sync resolves ucp-sdk 0.5.0 and the Python server fails to import #221, already filed with a fix in fix(rest/python): bound ucp-sdk below 0.5 until the server targets v2026-08-25 #222. All
verification above pins ucp-sdk to 0.4.6 locally to get a working
environment, matching what fix(rest/python): bound ucp-sdk below 0.5 until the server targets v2026-08-25 #222 proposes; this PR does not touch
pyproject.toml, since that bound belongs to fix(rest/python): bound ucp-sdk below 0.5 until the server targets v2026-08-25 #222.
this branch, with the FAILED test set byte-identical (confirmed with a
diff of the sorted FAILED lines); passing goes 140 to 143, exactly the
3 new tests. The 117 are a separate,
pre-existing pydantic validation error in checkout payload
construction (FulfillmentMethodCreateRequest and ShippingDestination),
unrelated to key discovery and not touched by this PR. Flagging for
visibility rather than folding an unrelated fix into this change.
no backing schema and widened a TypeScript field type. Neither applies
here: the Python discovery_profile.json never declared those
capabilities, and there is no static type system to narrow.