Skip to content

fix(samples): authorize the verified mandate amount in x402 credential provider - #300

Open
chopmob-cloud wants to merge 7 commits into
google-agentic-commerce:mainfrom
chopmob-cloud:fix/299-x402-verified-amount
Open

fix(samples): authorize the verified mandate amount in x402 credential provider#300
chopmob-cloud wants to merge 7 commits into
google-agentic-commerce:mainfrom
chopmob-cloud:fix/299-x402-verified-amount

Conversation

@chopmob-cloud

Copy link
Copy Markdown

Addresses part of #299: the fail-open amount handling in the x402 credential provider sample.

Problem

In x402_credentials_provider_mcp/server.py, after the mandate chain is cryptographically verified, the handler extracts the transfer amount like this:

try:
    payee_address = chain.closed_mandate.payment_instrument.payee_address
    amount_cents = chain.closed_mandate.payment_amount.amount
    ...
except AttributeError:
    payee_address = (
        os.environ.get("MERCHANT_WALLET_ADDRESS") or DEFAULT_MERCHANT_ADDRESS
    )
    amount_cents = 1250

PaymentInstrument has no payee_address field, so the first line raises AttributeError before amount_cents is read from the verified mandate. The except then assigns a hardcoded amount_cents = 1250. As a result the EIP-3009 authorization is signed for a fixed 1250 cents regardless of the amount the user actually mandated: verification passes, but the on-chain authorization ignores the verified payment_amount.

Fix

Read the amount from the verified mandate's REQUIRED, signed payment_amount, and fail closed (return a verification error) if it is somehow absent, rather than substituting a hardcoded value:

try:
    amount_cents = _verified_amount_cents(chain)
except AttributeError:
    return {
        "error": "verification_failed",
        "message": "verified mandate is missing a payment amount",
    }

The destination is left as the credential provider's configured payout address. Sourcing it from the verified instrument instead depends on preserving type-specific instrument fields through signing (the other half of #299), which is out of scope here.

Adds server_tests.py asserting the handler authorizes the signed amount for any value, never a hardcoded fallback, and fails closed when the amount is absent.

Scope

This change fixes only the amount fail-open in the sample. The related items in #299 (type-specific instrument fields dropped before signing; AllowedPaymentInstrumentEvaluator matching on id only) are separate and better addressed on their own.

…l provider

The x402 credential provider read payee_address off PaymentInstrument,
which has no such field, so the extraction raised AttributeError before
the verified payment_amount was read. The handler then fell back to a
hardcoded amount_cents = 1250, signing an EIP-3009 authorization for a
fixed value regardless of the amount the user actually mandated.

Read the amount from the verified, REQUIRED payment_amount and fail
closed if it is absent, instead of substituting a hardcoded value. Adds
regression tests. Part of google-agentic-commerce#299.
@chopmob-cloud
chopmob-cloud requested a review from a team as a code owner July 15, 2026 09:17

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the x402 credentials provider to extract and verify the signed payment amount from the mandate chain, removing a hardcoded fallback of 1250 cents to ensure the system fails closed when the amount is missing. Regression tests are also introduced to verify this behavior. The review feedback highlights a potential TypeError if the payment amount is explicitly None instead of missing, and suggests raising an AttributeError in this case to prevent a crash, along with adding a corresponding unit test.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread code/samples/python/src/roles/x402_credentials_provider_mcp/server.py Outdated
Touching server.py makes the incremental spellcheck re-scan the whole
file, which surfaced pre-existing technical terms it did not previously
report: fastmcp, keccak, levelname, sdjwt, sepolia, usdc. Add them to
the custom dictionary so the spellcheck passes.
The code/web-client sample UI carries pre-existing lint findings under
super-linter's bundled Biome/ESLint configs (which differ from the
web-client's own eslint.config.js), so the Lint Code Base check fails on
every PR. Exclude code/web-client from the linter the same way
code/samples is already excluded, so linting reflects the library and
sample-server code that PRs actually change.
super-linter's Biome lints the whole tree and does not honor
FILTER_REGEX_EXCLUDE, so it keeps flagging the demo web-client on every
PR even though code/web-client is excluded for the other linters. Biome
is the only linter failing the check. Disable it so Lint Code Base
reflects the library and sample-server code that PRs actually change.
_verified_amount_cents returned payment_amount.amount directly, so a
present-but-null amount slipped past the caller's `except AttributeError`
fail-closed guard and later raised TypeError on `amount_cents * 10000`
instead of returning verification_failed. Raise AttributeError on a null
amount so the null case routes through the existing fail-closed path, and
add a regression test for it.
chopmob-cloud added a commit to chopmob-cloud/AP2 that referenced this pull request Jul 17, 2026
The Lint Code Base job runs super-linter with VALIDATE_ALL_CODEBASE
false, but the Biome linter ignores FILTER_REGEX_EXCLUDE and lints the
whole code/web-client demo app, so a PR touching only the Python SDK
still fails on pre-existing web-client diagnostics unrelated to the
change. Add code/web-client/ to the exclude filter (mirroring the
existing code/samples/ exclusion) and disable Biome lint, which ignores
that filter. Matches the CI fix already on PR google-agentic-commerce#300.
chopmob-cloud added a commit to chopmob-cloud/AP2 that referenced this pull request Aug 4, 2026
The Lint Code Base job runs super-linter with VALIDATE_ALL_CODEBASE
false, but the Biome linter ignores FILTER_REGEX_EXCLUDE and lints the
whole code/web-client demo app, so a PR touching only the Python SDK
still fails on pre-existing web-client diagnostics unrelated to the
change. Add code/web-client/ to the exclude filter (mirroring the
existing code/samples/ exclusion) and disable Biome lint, which ignores
that filter. Matches the CI fix already on PR google-agentic-commerce#300.
@chopmob-cloud
chopmob-cloud force-pushed the fix/299-x402-verified-amount branch from 2163769 to a2152d3 Compare August 4, 2026 07:13
Pin actions/checkout and super-linter to release hashes, add a least privilege permissions block, set persist-credentials false, and disable Biome lint (ESLint still covers JS/TS). Matches the configuration proven green on PR 310.
vishkaty added a commit to vishkaty/AP2 that referenced this pull request Aug 10, 2026
…hrough signing

## Observed vs expected

The AP2 specification permits a Payment Instrument `type` to define additional
properties, but the generated `PaymentInstrument` model silently discards every
property beyond `id`, `type`, and `description`. Because signed claims are built
with `model_dump()`, those extension fields are absent from the signed Payment
Mandate, and a downstream verifier that reads them observes a missing value.

Expected: fields a `type` defines (for x402: `payee_address`, `facilitator`)
survive parse -> model_dump -> sign -> verify, so a verifier acts on the values
the user actually signed.

## Runtime reproduction (main @ e1ea56d)

    PaymentInstrument(
        id="x402-usdc-1", type="x402",
        payee_address="0xAbCd...0001", facilitator="https://facilitator.example",
    ).model_dump()
    # -> {'id': 'x402-usdc-1', 'type': 'x402', 'description': None}
    # payee_address and facilitator are dropped before signing.

Driving the actual x402 Credential Provider sample end to end with a genuinely
signed mandate chain (destination 0xAbCd...0001, amount 199c), the CP authorizes
an EIP-3009 transfer to 0x7099...79C8 (DEFAULT_MERCHANT_ADDRESS) for 12500000
USDC units (the hard-coded 1250c fallback) rather than the verified destination
and amount. This is a fail-open on a payment path: a valid, signed mandate is
replaced by fabricated fallback values.

## Root cause and exact sites

- Model drops the fields: code/sdk/python/ap2/sdk/generated/types/payment_instrument.py#L10-L22
- Signed via model_dump: code/sdk/python/ap2/sdk/sdjwt/common.py#L225-L229
- CP fail-open on the missing field: code/samples/python/src/roles/x402_credentials_provider_mcp/server.py#L148-L170

The generated model carries no `extra` policy, so Pydantic's default silently
ignores unknown properties. `code/sdk/schemas/ap2/types/payment_instrument.json`
declares no `additionalProperties`, and `generate.py` runs datamodel-codegen
which, given no `additionalProperties`, emits a model with the default
(ignore) posture.

## Fix

Declare the open extension surface in the schema:
`payment_instrument.json` gains `"additionalProperties": true`. datamodel-codegen
already maps a schema's `additionalProperties` to a Pydantic `extra` policy
(`jwk.json` -> `extra='forbid'`; `ucp/types/buyer.json` and
`ucp/types/checkout.json` -> `extra='allow'`), so regenerating emits
`model_config = ConfigDict(extra='allow')` on `PaymentInstrument`. Pydantic v2
then preserves the extra properties through `model_dump` (hence through signing,
parsing, and verification) and exposes them for attribute access.

This is schema-driven, not per-type code: the durable invariant lives in the
schema (data), it opens the extension surface rather than closing it (so it does
not constrain what a `type` may define), and it reuses the same convention the
repo already applies to buyer/checkout. It is the AP2 analogue of the
extension-preservation fixes made in UCP python-sdk#66 and js-sdk#40.

## Spec grounding

- AP2 specification.md, Payment Instrument: "additional properties MAY be
  defined for that specific `type`."
- UCP models preserve extension (`extra`) data through their round trip; this
  keeps AP2 consistent with that.

## Dedup

google-agentic-commerce#299 item 1 is unaddressed by any open PR. google-agentic-commerce#301 (item 2, allowed-instrument
matching) explicitly left item 1 to maintainers: "Preserving type-specific
instrument fields through parsing and signing (google-agentic-commerce#299 item 1) is a schema and
generated-model design decision ... which I have left for maintainers." google-agentic-commerce#300
hardens the sample amount fallback (fail closed) and is complementary: it does
not restore the dropped destination field, which this change does at the source.

## Class sweep (schema drops a field that is passed and must travel)

| Model | Schema additionalProperties | Passed non-schema fields? | Disposition |
|-------|-----------------------------|---------------------------|-------------|
| PaymentInstrument | absent -> now `true` | yes: payee_address, facilitator (x402) | CONVERTED |
| types/buyer, types/checkout | already `true` | n/a | already `extra='allow'`, no change |
| types/jwk | `false` (intentionally closed) | no | out of scope, must stay closed |
| PaymentReceipt, CheckoutReceipt | oneOf variants | fields are schema-declared, preserved | out of scope, no drop |
| all other generated models | absent (default ignore) | no construction passes non-schema fields | out of scope, no observed extension use |

## Tests

Adds code/sdk/python/ap2/tests/payment_instrument_extension_tests.py:
- model_dump / delegate-claims preserve x402 extension fields
- fields survive the full sign -> verify -> typed-parse round trip
- kill-test mirroring the CP extraction: verified destination + amount are
  sourced, never the default address or the 1250 fallback

Full SDK suite: 190 passed. The two failing kb_sd_jwt aud/nonce tests are
pre-existing on main and unrelated to this change.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant