From 9c774c476dfdcdb83f5bd3599e8dcc7301813732 Mon Sep 17 00:00:00 2001 From: vvillait88 Date: Fri, 17 Jul 2026 14:15:21 -0700 Subject: [PATCH] fix(checkout): malformed-402 re-challenge must run pre_validate (was 500 for dynamic pricing) Parity with node-commerce 2.7.1. The 2.5.7 malformed-credential re-challenge skipped pre_validate, so a merchant whose compute_pricing reads state that pre_validate populates crashed with a 500 instead of a 402. Fix: strip the junk credential and re-enter handle() so the full discovery flow (pre_validate + pricing + minting + compose) runs. Regression test added. Co-Authored-By: Claude Opus 4.8 --- agentscore_commerce/checkout.py | 48 ++++++++++++++++--------------- pyproject.toml | 2 +- tests/test_credential_precheck.py | 35 ++++++++++++++++++---- uv.lock | 2 +- 4 files changed, 56 insertions(+), 31 deletions(-) diff --git a/agentscore_commerce/checkout.py b/agentscore_commerce/checkout.py index 7fbfb55..34e5e05 100644 --- a/agentscore_commerce/checkout.py +++ b/agentscore_commerce/checkout.py @@ -1076,16 +1076,16 @@ async def handle(self, request: CheckoutRequest) -> CheckoutResult: else self.compose_mppx is not None ) if enforced and malformed is not None: - # Protocol-correct recovery: a junk credential gets a FRESH 402 - # challenge (same shape as the discovery leg) so an x402/MPP - # client re-pays, instead of a dead-end 400 it cannot act on. - # Strip the malformed credential first (``_discovery_view``) so - # recipient minting + MPP compose take their fresh-mint / - # fresh-challenge path rather than binding the garbage and - # raising another 400. pre_validate and the gate/assess are - # skipped by construction here (a junk credential must never burn - # the merchant's paid probe or an identity API call). - result = await self._emit_fresh_challenge(self._discovery_view(ctx)) + # A junk credential is treated as a discovery request: strip it + # and re-enter handle() so pre_validate + pricing + recipient + # minting + compose all run their fresh path exactly as for a + # no-credential request. That yields a fresh 402 the agent + # re-pays against, not a dead-end 400, and not a 500 when + # compute_pricing reads state that pre_validate populates. The + # gate/assess and settle are skipped by construction: after + # stripping there is no payment header, so no re-trigger of this + # check (max one level of recursion) and no identity call. + result = await self.handle(self._strip_payment_headers(request)) return dataclasses.replace(result, settle_phase="credential_malformed") # Pre-validate (optional): resolve merchant-specific per-request state @@ -2658,11 +2658,11 @@ async def _handle_mppx(self, ctx: CheckoutContext) -> CheckoutResult: ) async def _emit_fresh_challenge(self, ctx: CheckoutContext) -> CheckoutResult: - """Emit a fresh 402 challenge (the discovery leg). + """Emit the discovery-leg 402. - Factored out so the malformed-credential path can reuse it. Idempotent on - already-computed pricing / resolved recipients (``_emit_402`` resolves - recipients), so the normal discovery leg pays nothing extra. + pre_validate + pricing already ran in the main flow before this is + reached; idempotent on already-computed pricing / resolved recipients + (``_emit_402`` resolves recipients), so it primes nothing twice. """ if ctx.pricing is None: ctx.pricing = await _maybe_await(self.compute_pricing(ctx)) @@ -2679,24 +2679,26 @@ async def _emit_fresh_challenge(self, ctx: CheckoutContext) -> CheckoutResult: pass return await self._emit_402(ctx, mppx_headers=mppx_headers) - def _discovery_view(self, ctx: CheckoutContext) -> CheckoutContext: - """Return a copy of ``ctx`` with payment-credential headers removed. + def _strip_payment_headers(self, request: CheckoutRequest) -> CheckoutRequest: + """Return a copy of the request with payment-credential headers removed. - Recipient minting and MPP compose then take their discovery (fresh-mint, - fresh-challenge) path instead of binding the inbound credential, turning a - malformed-credential request into a clean 402 re-challenge. Pricing and - recipients are reset so they mint fresh. + Re-entering handle() with it treats the request as a discovery + (no-credential) request: pre_validate + pricing + minting + compose run + their fresh path, and the gate/assess and settle are skipped. Turns a + malformed-credential request into a clean 402 re-challenge. The raw + request is left intact; compose_mppx reads it only best-effort under a + try/except, while the stripped headers are what the shape check, gate + dispatch, and recipient minting read. """ headers = { k: v - for k, v in ctx.request.headers.items() + for k, v in request.headers.items() if not ( k.lower() in ("payment-signature", "x-payment") or (k.lower() == "authorization" and v.startswith("Payment ")) ) } - request = dataclasses.replace(ctx.request, headers=headers) - return dataclasses.replace(ctx, request=request, pricing=None, recipients={}) + return dataclasses.replace(request, headers=headers) async def _emit_402( self, diff --git a/pyproject.toml b/pyproject.toml index 103ec78..34a7bf1 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "agentscore-commerce" -version = "2.5.7" +version = "2.5.8" description = "Agent commerce SDK for Python — identity middleware (FastAPI, Flask, Django, AIOHTTP, Sanic, ASGI) + payment helpers + 402 builders + discovery + Stripe multichain. The full merchant-side toolkit for AgentScore-powered agent commerce." readme = "README.md" license = "MIT" diff --git a/tests/test_credential_precheck.py b/tests/test_credential_precheck.py index 6dbbd6b..35484ee 100644 --- a/tests/test_credential_precheck.py +++ b/tests/test_credential_precheck.py @@ -47,7 +47,7 @@ def test_malformed_payment_credential_classifies_channels() -> None: @pytest.mark.asyncio -async def test_junk_mpp_header_rechallenges_with_fresh_402_no_pre_validate() -> None: +async def test_junk_mpp_header_rechallenges_with_fresh_402_discovery_flow() -> None: calls: list[str] = [] async def _pre_validate(_ctx: Any) -> dict[str, Any]: @@ -69,12 +69,12 @@ async def _compose(_ctx: Any) -> MppxComposeOutcome: assert result.status == 402 assert result.settle_phase == "credential_malformed" assert result.headers["www-authenticate"] == 'Payment realm="fresh"' - # The junk credential must not burn the merchant's paid probe. - assert "pre_validate" not in calls + # Treated as a discovery request: pre_validate runs (pricing depends on its state). + assert "pre_validate" in calls @pytest.mark.asyncio -async def test_junk_x402_header_rechallenges_with_fresh_402_no_pre_validate() -> None: +async def test_junk_x402_header_rechallenges_with_fresh_402_discovery_flow() -> None: calls: list[str] = [] async def _pre_validate(_ctx: Any) -> dict[str, Any]: @@ -93,8 +93,31 @@ async def _pre_validate(_ctx: Any) -> dict[str, Any]: # A fresh challenge the agent can re-pay against, not a bare error body. assert result.body["accepted_methods"] is not None assert result.settle_phase == "credential_malformed" - # Junk must not burn the merchant's paid probe. - assert calls == [] + # Discovery flow: pre_validate runs. + assert calls == ["pre_validate"] + + +@pytest.mark.asyncio +async def test_malformed_credential_runs_pre_validate_so_stateful_pricing_survives() -> None: + # Regression: compute_pricing reads state that pre_validate populates (the + # martin-estate shape). The malformed re-challenge must run pre_validate + # first, or pricing dereferences missing state and 500s. + async def _pre_validate(_ctx: Any) -> dict[str, Any]: + return {"product": {"price_cents": 4800}} + + def _compute_pricing(ctx: Any) -> PricingResult: + return PricingResult(amount_usd=ctx.state["product"]["price_cents"] / 100) + + checkout = Checkout( + rails={"x402_base": X402BaseRailSpec(recipient="0x" + "00" * 19 + "dEaD")}, + url="https://api.example/purchase", + pre_validate=_pre_validate, + compute_pricing=_compute_pricing, + x402_server=object(), + ) + result = await checkout.handle(_req({"x-payment": "not-decodable"})) + assert result.status == 402 + assert result.body["accepted_methods"] is not None @pytest.mark.asyncio diff --git a/uv.lock b/uv.lock index 67ff4a2..6707d7b 100644 --- a/uv.lock +++ b/uv.lock @@ -13,7 +13,7 @@ constraints = [{ name = "fastapi", specifier = "!=0.136.3" }] [[package]] name = "agentscore-commerce" -version = "2.5.7" +version = "2.5.8" source = { editable = "." } dependencies = [ { name = "agentscore-py" },