Conversation
CheckoutMandateChain.verify() evaluated the supplied checkout_jwt against the open mandate's constraints but never derived its hash. The closed mandate's checkout_hash was only compared against an optional caller-provided expected_checkout_hash, so omitting that argument skipped the closed-mandate binding check entirely: a different but otherwise valid Checkout JWT that satisfied the open constraints verified cleanly. Derive the hash from the checkout_jwt being verified and compare it with the closed Checkout Mandate's checkout_hash, making the spec-required binding part of normal chain verification. expected_checkout_hash is retained as a secondary consistency assertion only. Per docs/ap2/specification.md (Verification -> Merchant), the Merchant MUST verify that the hash of the Checkout JWT sent for approval matches the checkout_hash claim. PR google-agentic-commerce#330 addresses the analogous Payment Mandate binding and identifies this as the same class of issue.
cspell rescans the whole of checkout_mandate_chain_tests.py because the file is in this PR's changed set, which surfaces the pre-existing ec.SECP256R1() curve name on line 22. The term is untouched by this change and already present on main.
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.
fix(sdk): verify checkout mandate binding from presented checkout
Closes #358.
Observed vs expected
CheckoutMandateChain.verify()evaluates the suppliedcheckout_jwtagainst theopen mandate's constraints but never derives its hash. The closed mandate's
checkout_hashis only compared against the optionalexpected_checkout_hashargument, so omitting that argument skips the closed-binding check entirely.
The defect is the binding never being derived, not the comparison itself.
Exact location
code/sdk/python/ap2/sdk/checkout_mandate_chain.py:44-110(verify): thecheckout_hashcomparison was gated onexpected_checkout_hash is not None,and the method never hashed the
checkout_jwtit parsed a few lines earlier.Spec grounding
docs/ap2/specification.md:311-312, Verification → Merchant:docs/ap2/specification.md:352-353, Verification → Dispute, independentlyrequires computing the hash from the included
checkout_jwt.docs/ap2/checkout_mandate.md:26-28definescheckout_hashas thebase64url-encoded hash of the value of
checkout_jwt.The fix
verify()now computes the hash of the exact serializedcheckout_jwtargumentit was asked to verify and compares it with
closed_mandate.checkout_hash. Thecheck runs unconditionally, so it cannot be skipped by omitting an argument.
The hashed value is the
checkout_jwtparameter — the checkout presented forexecution — not
self.closed_mandate.checkout_jwt. Hashing the embedded copywould only prove the mandate is internally consistent and would bind nothing.
Design alternatives considered and rejected:
expected_checkout_hashmandatory (the fail-closed shape fix(sdk): enforce Payment Mandate closed checkout-binding by default in chain verify #330 usedfor
PaymentMandateChain). Rejected here because the two cases genuinelydiffer:
PaymentMandateChaindoes not receive the external Checkout JWT, soits caller must supply the expected binding.
CheckoutMandateChain.verify()already receives the actual
checkout_jwt, so it can and should derive thehash itself rather than requiring callers to hash it correctly.
closed_mandate.checkout_jwt. Rejected: AP2specifies the binding through
checkout_hash, so the implementation shouldverify the protocol binding rather than a parallel invariant.
Open design question:
_sd_algI would appreciate maintainer direction here.
This patch calls the existing
compute_sha256_b64url()helper. That is correctfor the SDK's current SHA-256 issuance path, but it is not fully correct for
every AP2-valid mandate, and I would rather surface that than paper over it.
docs/ap2/checkout_mandate.md:26-28requirescheckout_hashto use theSD-JWT's
_sd_alg, defaulting tosha-256only when absent, and the SDK's ownSD-JWT layer already supports
sha-256,sha-384andsha-512(
_HASH_BY_SD_ALGinsdk/sdjwt/common.py).The obstacle is an abstraction boundary:
CheckoutMandateChain.parse()receivesonly the effective verified payloads, by which point
_sd_alghas been lost. Itlives on
ParsedToken.payload, and the generatedCheckoutMandatemodel has nosuch field with pydantic extras disabled — confirmed by passing
_sd_alg: 'sha-512'intoCheckoutMandate.model_validate()and observingmodel_extra is None.Supporting other algorithms correctly would mean retaining the verified closed
token's hash algorithm through the chain API. I deliberately kept that
API/data-flow change out of this patch to keep it narrow, and did not invent a
value or claim generic support the code does not have. Two options if you'd like
it addressed:
ParsedToken) intoparse()/verify(), inthis PR or a follow-up.
_sd_algas a known gap.Happy to follow your preference. Every
checkout_hashproducer in this repotoday uses SHA-256 (e.g.
merchant_agent/tools.py:207), so this patch matchescurrent issuance behavior.
Treatment of
expected_checkout_hashRetained, and redefined as a secondary consistency assertion documented as such.
The security invariant no longer depends on it.
I kept it rather than removing it because it is a positional-first public
parameter and the only
verify()caller in the repo(
merchant_agent_mcp/server.py:881) never passes it, so removal would be abreaking change for no security benefit. If you'd prefer it deprecated or
removed, I'm glad to do that — I saw no existing deprecation pattern here, so I
left it for a separate change.
test_checkout_binding_not_skipped_when_expected_hash_agreesproves acaller-supplied hash cannot mask a mismatched presented checkout.
Caller impact
None.
merchant_agent_mcp/server.py:881passeschain.closed_mandate.checkout_jwt; well-formed mandates verify unchanged, andthe sample now additionally detects an internally inconsistent mandate. The
SDK owns the invariant, so no hashing is duplicated into the sample.
Tests
code/sdk/python/ap2/tests/checkout_mandate_chain_tests.py(6 → 13):test_checkout_binding_rejects_other_checkout_without_expected_hash— binds A,presents B, omits
expected_checkout_hash, expects rejection. Red-to-greenanchor.
test_checkout_binding_not_skipped_when_expected_hash_agrees— a satisfiedoptional assertion cannot mask a mismatched checkout.
test_checkout_binding_reported_with_constraints_satisfied— binding failurereported even when all open constraints pass.
test_checkout_binding_matching_checkout_passes— the bound checkout stillverifies cleanly (no legitimate caller trapped).
test_checkout_binding_does_not_leak_checkout_jwt— the violation message doesnot embed JWT contents.
test_checkout_binding_missing_checkout_jwt_still_fails/..._malformed_checkout_jwt_still_fails— existing fail-closed behaviorpreserved.
Four pre-existing tests used placeholder hashes (
checkout_hash='hash','actual_hash') and passed only becauseverify()never derived the hash. Theynow carry the real hash of their checkout JWT.
test_checkout_fields_parsedkeepsits placeholder deliberately: it only calls
extract_parsed_checkout_object()and never
verify().Reverting only the SDK file and rerunning gives 4 failed / 9 passed, the anchor
failing on
assert []— the reported behavior. With the fix, 13 passed.Verification
pytest code/sdk/python/ap2/tests/checkout_mandate_chain_tests.py— 13 passed.pytest code/sdk/python— 193 passed, 2 failed. Both failures(
kb_sd_jwt_intermediate_tests.py::test_verify_rejects_aud_mismatch,::test_verify_rejects_nonce_mismatch) are pre-existing onmain@ e1ea56dand unrelated (tracked in [Bug]: Python terminal KB-SD-JWT verifier accepts tokens missing aud or nonce #319, PRs fix(sdjwt): honor expected aud/nonce on every KB hop #313/fix(sdjwt): require expected aud/nonce when a terminal KB hop carries them #326).
ruff checkandruff format --checkclean on both changed files under therepo
.ruff.toml.git diff --checkclean.Scope
Two files: the SDK fix and its tests. No changes to
constraints.py, schemas,generated models, samples, or the normative docs. No new checkout constraint
types, and no default verdict introduced for the unresolved open-mandate omission
semantics — this patch is strictly the closed → execution integrity binding.