fix(sdjwt): serialize pydantic models in json mode to support enum claims in mandates - #355
Open
harkanwalbedi wants to merge 2 commits into
Conversation
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.
Description
An
OpenPaymentMandatecarrying anAgentRecurrenceconstraint can't be signed.MandateClient.create()throws:What's happening
delegate_claims_from_model()was using a plainmodel_dump(), which leaves enums asEnummembers rather than their values. Nothing in our own code minds that, but the claims then get handed to thesd_jwtlibrary, which hashes every disclosure with stdlibjson.dumps()— and that has no idea what aFrequencyis:The fix
delegate_claims_from_model()is the one spot where Pydantic models cross into the SD-JWT layer — bothsd_jwt.pyandkb_sd_jwt.pyroute through it — so fixing it there covers both paths rather than patching each call site.A note on the scope of
mode='json'This flag does more than enums, so I checked what else it touches in the generated types. Two things:
AnyUrlandAwareDatetime. All three were already broken:json.dumps?EnumFrequency.DAILY'DAILY'AnyUrlAnyUrl('https://example.com/x')'https://example.com/x'AwareDatetimedatetime(..., tzinfo=...)'2026-09-11T20:00:00-07:00'So this can't change any claim that previously signed successfully — none of these ever made it through
json.dumps()in the first place. It just happens to fix the URL and timestamp cases along the way.One thing I specifically verified:
AnyUrlnormalization (Example.COM:443→example.com) happens at validation time, not at dump time, so signed values don't shift.Tests
Added
test_open_payment_mandate_agent_recurrence_can_be_signed. It signs a mandate containingAgentRecurrenceand verifies the round trip, asserting the enum comes back as a realFrequency.DAILYand not a bare string. Fails onmainwith the exact error from the issue, passes with this change.agent_recurrence_tests.py— 8/8 passingruff checkclean against the repo's.ruff.tomlOne heads-up:
kb_sd_jwt_intermediate_tests.py::test_verify_rejects_aud_mismatchand::test_verify_rejects_nonce_mismatchfail, but they fail onmainas well — I stashed this change and re-ran to confirm. They're key-binding checks, unrelated to serialization. Happy to file a separate issue if it's not already known.CONTRIBUTINGGuide.Fixes #354 🦕