fix(signing): serialize non-string HMAC bodies with json.dumps (#108) - #109
Open
erik-polymarket wants to merge 1 commit into
Open
fix(signing): serialize non-string HMAC bodies with json.dumps (#108)#109erik-polymarket wants to merge 1 commit into
erik-polymarket wants to merge 1 commit into
Conversation
build_hmac_signature rendered non-string bodies with
str(body).replace("'", '"'), which emits Python False/None instead of
JSON false/null. Signing a dict like {"deferExec": False} therefore
produced an HMAC over `{"deferExec": False}` while the server receives
`{"deferExec": false}`, yielding an invalid signature for external
callers that rely on the dict path (internal callers dodge this via
serialized_body).
Non-string bodies are now serialized with json.dumps(body,
ensure_ascii=False); pre-serialized string bodies are signed verbatim.
The canonical dict form is unchanged for string-only bodies (default
separators still match the existing spaced baseline), so previously
valid signatures are preserved.
Adds regression tests covering boolean, None, and nested bool/null
dictionary values.
Fixes #108
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.
Problem
build_hmac_signatureserialized non-string bodies withstr(body).replace("'", '"'). That matches JSON for string-only dicts, but PythonFalse/Nonerender asFalse/Noneinstead of JSONfalse/null. Signing{"deferExec": False}produces an HMAC over{"deferExec": False}while the payload actually sent to the server is{"deferExec": false}— so the signature is invalid.Internal callers avoid this by passing
serialized_body(compactjson.dumps), whichcreate_level_2_headersprefers. Butbuild_hmac_signatureis exported and the header builder falls back to the dictbodywhenserialized_bodyis omitted, so external/manual callers on the dict path get a broken signature.Fixes #108.
Fix
py_clob_client_v2/signing/hmac.py:json.dumps(body, ensure_ascii=False), soFalse/Nonebecomefalse/null..replace("'", '"')hack, which only existed to convert Python dict repr to JSON and could corrupt strings containing apostrophes).Backward compatibility: for string-only dicts,
json.dumpswith default separators produces the same bytes as the oldstr(dict).replace(...)form (e.g.{"hash": "0x123"}), so previously valid signatures are unchanged. The existing pinned baseline signature test still passes.Tests
Added regression tests in
tests/signing/test_hmac.pycovering boolean,None, and nested bool/null dict values — each asserts the signature equals thejson.dumpsform and the explicit JSON literal (e.g.{"deferExec": false}), and does not equal the old brokenstr()-based rendering.Full suite: 230 passed (22 in
test_hmac.py).hmac.pypassesblack --check.Note
Medium Risk
Changes request-signing payload canonicalization; wrong serialization would break authenticated API calls, though string-only dicts stay compatible and tests cover the regression.
Overview
Fixes invalid API HMAC signatures when callers pass a dict body containing
False,None, or other non-JSON-literal Python values.build_hmac_signatureno longer builds the signed message fromstr(body).replace("'", '"'). String bodies are appended verbatim; structured bodies usejson.dumps(..., ensure_ascii=False)so the signed bytes match JSON on the wire (false/null, notFalse/None). That aligns signing with the dict fallback path increate_level_2_headerswhenserialized_bodyis omitted.Regression tests assert dict signing matches explicit JSON literals and no longer matches the old
str()-based payload.Reviewed by Cursor Bugbot for commit 3a1d787. Bugbot is set up for automated code reviews on this repo. Configure here.