From 5cad44369b50dc0fb185f8cdafc19953e32f2720 Mon Sep 17 00:00:00 2001 From: ygd58 Date: Fri, 4 Sep 2026 07:04:51 +0200 Subject: [PATCH] fix(ack-pay): bind payment receipt issuer to the selected payment option's receiptService Fixes #192 verifyPaymentReceipt only checked that the receipt's issuer was somewhere in the caller's global trustedReceiptIssuers list, and that its paymentOptionId existed in the verified Payment Request. It did not check that the receipt issuer matched the specific receiptService the selected payment option named. In multi-rail deployments where a verifier trusts multiple receipt issuers globally (one per payment rail: card, USDC, Solana), this meant a receipt legitimately issued by one trusted service could be accepted for a payment option that named a different trusted receiptService, weakening per-option trust separation. Fix: changed the existence check (paymentOptions.some) into a lookup (paymentOptions.find) for the selected option, then - when its receiptService is a DID - require it to equal verifiedReceipt.issuer.id. URL-form receiptService values are left unenforced, since binding a DID-based issuer to a URL isn't well-defined without an application-level policy; matches the option laid out in the issue. Added three regression tests: rejects a receipt from a different trusted issuer than the option's receiptService (the exact reproduction from the issue), accepts a receipt from the option's own receiptService, and confirms the check is a no-op when the option doesn't specify a receiptService. Verified the rejection test fails when the binding check is disabled and passes with it enabled. pnpm --filter @agentcommercekit/ack-pay exec vitest run - 15/15 passing (full test file), 53/53 passing (full package). oxlint and oxfmt clean. AI usage disclosure: this fix was developed with Claude (Anthropic) assistance - identifying the exact code path, writing the fix and tests, and verifying locally including a manual before/after check that the tests catch the regression. I reviewed and understand the change: it's a targeted authorization check that binds a receipt's verified issuer to the specific receiptService the paid-for option named, closing a gap where a globally-trusted-but-wrong issuer could be accepted. --- .changeset/receipt-issuer-service-binding.md | 24 +++ .../src/verify-payment-receipt.test.ts | 165 ++++++++++++++++++ .../ack-pay/src/verify-payment-receipt.ts | 28 ++- 3 files changed, 215 insertions(+), 2 deletions(-) create mode 100644 .changeset/receipt-issuer-service-binding.md diff --git a/.changeset/receipt-issuer-service-binding.md b/.changeset/receipt-issuer-service-binding.md new file mode 100644 index 00000000..28fda9db --- /dev/null +++ b/.changeset/receipt-issuer-service-binding.md @@ -0,0 +1,24 @@ +--- +"@agentcommercekit/ack-pay": patch +--- + +Fixed `verifyPaymentReceipt` not binding the verified receipt's issuer to +the selected `PaymentOption.receiptService`. + +Previously, `verifyPaymentReceipt` only checked that the receipt's issuer was +somewhere in the caller's global `trustedReceiptIssuers` list, and that its +`paymentOptionId` existed in the verified Payment Request. It did not check +that the receipt issuer matched the *specific* `receiptService` the selected +payment option named. + +In multi-rail deployments where a verifier trusts multiple receipt issuers +globally (e.g. one per payment rail: card, USDC, Solana), this meant a +receipt legitimately issued by one trusted service could be accepted for a +payment option that named a *different* trusted `receiptService`, weakening +per-option trust separation. + +`verifyPaymentReceipt` now rejects a receipt whose verified issuer does not +match the selected payment option's `receiptService`, when that value is a +DID. URL-form `receiptService` values are left unenforced, since binding a +DID-based issuer to a URL isn't well-defined without an application-level +policy. diff --git a/packages/ack-pay/src/verify-payment-receipt.test.ts b/packages/ack-pay/src/verify-payment-receipt.test.ts index 30fadb9e..1e791830 100644 --- a/packages/ack-pay/src/verify-payment-receipt.test.ts +++ b/packages/ack-pay/src/verify-payment-receipt.test.ts @@ -265,3 +265,168 @@ describe("verifyPaymentReceipt()", () => { ).rejects.toThrow(UntrustedIssuerError) }) }) + +describe("verifyPaymentReceipt() - receiptService binding", () => { + let resolver: Resolvable + let legitReceiptIssuerDid: DidUri + let legitReceiptIssuerKeypair: Awaited> + let otherTrustedReceiptIssuerDid: DidUri + let otherTrustedReceiptIssuerKeypair: Awaited< + ReturnType + > + let paymentRequestToken: JwtString + + beforeEach(async () => { + legitReceiptIssuerKeypair = await generateKeypair("secp256k1") + legitReceiptIssuerDid = createDidKeyUri(legitReceiptIssuerKeypair) + + otherTrustedReceiptIssuerKeypair = await generateKeypair("secp256k1") + otherTrustedReceiptIssuerDid = createDidKeyUri( + otherTrustedReceiptIssuerKeypair, + ) + + const paymentRequestIssuerKeypair = await generateKeypair("secp256k1") + const paymentRequestIssuerDid = createDidKeyUri(paymentRequestIssuerKeypair) + + resolver = getDidResolver() + + // A payment option that names a specific receiptService (the legit + // issuer), per the issue's minimal reproduction. + const paymentRequestInit: PaymentRequestInit = { + id: "test-request-id", + paymentOptions: [ + { + id: "card-option", + amount: 100, + decimals: 2, + currency: "USD", + network: "eip155:84532", + recipient: "0x592D4858DE40BC81A77E5B373238B70D7C79D3C79", + receiptService: legitReceiptIssuerDid, + }, + ], + } + + const paymentRequiredBody = await createSignedPaymentRequest( + paymentRequestInit, + { + issuer: paymentRequestIssuerDid, + signer: createJwtSigner(paymentRequestIssuerKeypair), + algorithm: curveToJwtAlgorithm(paymentRequestIssuerKeypair.curve), + }, + ) + paymentRequestToken = paymentRequiredBody.paymentRequestToken + }) + + async function signReceiptAs( + issuerDid: DidUri, + issuerKeypair: Awaited>, + ): Promise { + const unsignedReceipt = createPaymentReceipt({ + paymentRequestToken, + paymentOptionId: "card-option", + issuer: issuerDid, + payerDid: createDidPkhUri( + "eip155:84532", + "0x7B3D8F2E1C9A4B5D6E7F8A9B0C1D2E3F4A5B6C", + ), + }) + + return signCredential(unsignedReceipt, { + did: issuerDid, + signer: createJwtSigner(issuerKeypair), + }) + } + + it("rejects a receipt from a different trusted issuer than the option's receiptService", async () => { + // The receipt is issued and signed by otherTrustedReceiptIssuerDid, not + // by legitReceiptIssuerDid (the DID the selected payment option names as + // its receiptService). Both are in trustedReceiptIssuers, so the old + // "is the issuer trusted at all, globally" check alone would accept + // this - the fix must bind to the option's own receiptService instead. + const receiptJwt = await signReceiptAs( + otherTrustedReceiptIssuerDid, + otherTrustedReceiptIssuerKeypair, + ) + + await expect( + verifyPaymentReceipt(receiptJwt, { + resolver, + trustedReceiptIssuers: [ + legitReceiptIssuerDid, + otherTrustedReceiptIssuerDid, + ], + }), + ).rejects.toThrow(InvalidPaymentReceiptError) + }) + + it("accepts a receipt from the option's own receiptService", async () => { + const receiptJwt = await signReceiptAs( + legitReceiptIssuerDid, + legitReceiptIssuerKeypair, + ) + + const result = await verifyPaymentReceipt(receiptJwt, { + resolver, + trustedReceiptIssuers: [ + legitReceiptIssuerDid, + otherTrustedReceiptIssuerDid, + ], + }) + + expect(result.receipt).toBeDefined() + }) + + it("does not enforce receiptService binding when the option doesn't specify one", async () => { + // Re-run createSignedPaymentRequest without receiptService on the option, + // to confirm the binding check is a no-op (opt-in) rather than requiring + // every payment option to declare a receiptService. + const paymentRequestIssuerKeypair = await generateKeypair("secp256k1") + const paymentRequestIssuerDid = createDidKeyUri(paymentRequestIssuerKeypair) + + const paymentRequestInit: PaymentRequestInit = { + id: "test-request-id-no-receipt-service", + paymentOptions: [ + { + id: "card-option", + amount: 100, + decimals: 2, + currency: "USD", + network: "eip155:84532", + recipient: "0x592D4858DE40BC81A77E5B373238B70D7C79D3C79", + // no receiptService + }, + ], + } + + const paymentRequiredBody = await createSignedPaymentRequest( + paymentRequestInit, + { + issuer: paymentRequestIssuerDid, + signer: createJwtSigner(paymentRequestIssuerKeypair), + algorithm: curveToJwtAlgorithm(paymentRequestIssuerKeypair.curve), + }, + ) + + const unsignedReceipt = createPaymentReceipt({ + paymentRequestToken: paymentRequiredBody.paymentRequestToken, + paymentOptionId: "card-option", + issuer: otherTrustedReceiptIssuerDid, + payerDid: createDidPkhUri( + "eip155:84532", + "0x7B3D8F2E1C9A4B5D6E7F8A9B0C1D2E3F4A5B6C", + ), + }) + const receiptJwt = await signCredential(unsignedReceipt, { + did: otherTrustedReceiptIssuerDid, + signer: createJwtSigner(otherTrustedReceiptIssuerKeypair), + }) + + const result = await verifyPaymentReceipt(receiptJwt, { + resolver, + trustedReceiptIssuers: [otherTrustedReceiptIssuerDid], + }) + + expect(result.receipt).toBeDefined() + }) +}) diff --git a/packages/ack-pay/src/verify-payment-receipt.ts b/packages/ack-pay/src/verify-payment-receipt.ts index 2b4eeab4..a84346da 100644 --- a/packages/ack-pay/src/verify-payment-receipt.ts +++ b/packages/ack-pay/src/verify-payment-receipt.ts @@ -1,4 +1,5 @@ import type { Resolvable } from "@agentcommercekit/did" +import { isDidUri } from "@agentcommercekit/did" import { isJwtString, type JwtString } from "@agentcommercekit/jwt" import { InvalidCredentialError, @@ -132,17 +133,40 @@ export async function verifyPaymentReceipt( // Bind the receipt's selected option back to an option actually offered by // the verified Payment Request. Reads from `verifiedReceipt` (proof-decoded), // so a mutated outer credential cannot smuggle in an unoffered option. - const paymentOptionExists = paymentRequest.paymentOptions.some( + const selectedPaymentOption = paymentRequest.paymentOptions.find( (paymentOption) => paymentOption.id === verifiedReceipt.credentialSubject.paymentOptionId, ) - if (!paymentOptionExists) { + if (!selectedPaymentOption) { throw new InvalidPaymentReceiptError( "Receipt paymentOptionId does not match any payment option in the Payment Request token", ) } + // A payment option can name a specific receiptService (typically to + // support multi-rail deployments where different payment options delegate + // receipt issuance to different services). When it's a DID, the verified + // receipt's issuer must be that exact DID - otherwise a receipt issued by + // *any* trusted issuer (from a global `trustedReceiptIssuers` list) would + // be accepted for an option that named a different, specific issuer, + // defeating the per-option trust separation the deployment relies on. + // + // URL-form receiptService values are left unenforced here, since binding a + // DID-based receipt issuer to a URL isn't well-defined without an + // application-level policy for what "matches" means; callers that need + // this can compare `selectedPaymentOption.receiptService` against + // `verifiedReceipt.issuer.id` themselves. + if ( + selectedPaymentOption.receiptService !== undefined && + isDidUri(selectedPaymentOption.receiptService) && + selectedPaymentOption.receiptService !== verifiedReceipt.issuer.id + ) { + throw new InvalidPaymentReceiptError( + "Receipt issuer does not match the selected payment option's receiptService", + ) + } + return { receipt: verifiedReceipt, paymentRequestToken,