fix(vc,ack-pay): bind credential verification to the verified proof (credential forgery)

.changeset/vc-ackpay-proof-binding.md

@@ -0,0 +1,18 @@
+---
+"@agentcommercekit/vc": patch
+"@agentcommercekit/ack-pay": patch
+---
+
+Security: bind credential trust decisions to the verified proof.
+
+`verifyParsedCredential` previously verified the JWT in `proof.jwt` but then made
+every trust decision (expiry, revocation, trusted-issuer, claim verifiers) from
+the caller-supplied credential object, which is not bound to the proof. An
+attacker could wrap a valid `proof.jwt` in an object with tampered `issuer` /
+`credentialSubject` fields and pass verification. `verifyPaymentReceipt` had the
+same gap on its object-input path, returning the tampered `paymentRequestToken`
+and `receipt`.
+
+`verifyProof` and `verifyParsedCredential` now return the credential decoded from
+the verified proof, and all trust decisions and returned values flow from that
+credential. The JWT-string input paths were already safe.

packages/ack-pay/src/verify-payment-receipt.test.ts

@@ -102,6 +102,31 @@ describe("verifyPaymentReceipt()", () => {
     expect(result.paymentRequest).toBeDefined()
   })
 
+  it("returns verified values when outer object fields are tampered", async () => {
+    // Reuse the valid proof from a legitimately signed receipt, but tamper the
+    // outer object's credentialSubject. The forgery must be ignored: all
+    // returned values must come from the credential decoded from the proof.
+    const tampered = {
+      ...signedReceipt,
+      credentialSubject: {
+        ...signedReceipt.credentialSubject,
+        paymentRequestToken: "forged.jwt.token",
+      },
+    } as Verifiable<W3CCredential>
+
+    const result = await verifyPaymentReceipt(tampered, {
+      resolver,
+      trustedReceiptIssuers: [receiptIssuerDid],
+      verifyPaymentRequestTokenJwt: false,
+    })
+
+    expect(result.paymentRequestToken).toBe(paymentRequestToken)
+    expect(
+      (result.receipt.credentialSubject as { paymentRequestToken: string })
+        .paymentRequestToken,
+    ).toBe(paymentRequestToken)
+  })
+
   it("preserves receipt metadata through JWT verification", async () => {
     const evidenceMetadata = {
       policyRef: "policy://merchant-spend-v3",

packages/ack-pay/src/verify-payment-receipt.ts

@@ -73,25 +73,37 @@ export async function verifyPaymentReceipt(
     throw new InvalidCredentialError("Receipt is not a JWT or Credential")
   }
 
+  // Cheap structural fast-reject only — NOT a trust boundary. The authoritative
+  // receipt check runs on the proof-verified credential below.
   if (!isPaymentReceiptCredential(parsedCredential)) {
     throw new InvalidCredentialError(
       "Credential is not a PaymentReceiptCredential",
     )
   }
 
-  await verifyParsedCredential(parsedCredential, {
+  // `verifyParsedCredential` returns the credential decoded from the verified
+  // proof. All reads below use that verified credential, never the
+  // caller-supplied `parsedCredential` object, whose fields are not bound to
+  // the proof and may have been tampered with on the object-input path.
+  const verifiedReceipt = await verifyParsedCredential(parsedCredential, {
     resolver,
     trustedIssuers: trustedReceiptIssuers,
     verifiers: [getReceiptClaimVerifier()],
   })
 
+  if (!isPaymentReceiptCredential(verifiedReceipt)) {
+    throw new InvalidCredentialError(
+      "Verified credential is not a PaymentReceiptCredential",
+    )
+  }
+
   // Verify the paymentRequestToken is a valid JWT
   const paymentRequestToken =
-    parsedCredential.credentialSubject.paymentRequestToken
+    verifiedReceipt.credentialSubject.paymentRequestToken
 
   if (!verifyPaymentRequestTokenJwt) {
     return {
-      receipt: parsedCredential,
+      receipt: verifiedReceipt,
       paymentRequestToken,
       paymentRequest: null,
     }
@@ -117,7 +129,7 @@ export async function verifyPaymentReceipt(
   )
 
   return {
-    receipt: parsedCredential,
+    receipt: verifiedReceipt,
     paymentRequestToken,
     paymentRequest,
   }

packages/vc/src/verification/verify-parsed-credential.test.ts

@@ -70,14 +70,18 @@ async function setup() {
     },
   } as unknown as Verifiable<W3CCredential>
 
+  // `verifyProof` returns the credential decoded from the verified proof. In
+  // the happy path that matches the object under test, so default the mock to
+  // return `vc` itself.
+  vi.mocked(verifyProof).mockResolvedValue(vc)
+
   return { vc, issuerDid, resolver }
 }
 
 describe("verifyParsedCredential", () => {
   beforeEach(() => {
     vi.mocked(isExpired).mockReturnValue(false)
     vi.mocked(isRevoked).mockResolvedValue(false)
-    vi.mocked(verifyProof).mockResolvedValue(undefined)
   })
 
   afterEach(() => {
@@ -217,6 +221,50 @@ describe("verifyParsedCredential", () => {
     ).resolves.not.toThrow()
   })
 
+  it("returns the proof-decoded credential and ignores tampered outer fields", async () => {
+    const { vc, issuerDid, resolver } = await setup()
+
+    // The authoritative credential decoded from the verified proof
+    const verifiedSubject = { id: "did:example:subject", role: "user" }
+    const verifiedCredential = {
+      ...vc,
+      issuer: { id: issuerDid },
+      credentialSubject: verifiedSubject,
+    } as unknown as Verifiable<W3CCredential>
+    vi.mocked(verifyProof).mockResolvedValue(verifiedCredential)
+
+    // The caller-supplied object carries tampered fields (untrusted issuer,
+    // escalated subject) while reusing the same valid proof.
+    const tampered = {
+      ...vc,
+      issuer: { id: "did:example:attacker" },
+      credentialSubject: { id: "did:example:subject", role: "admin" },
+    } as unknown as Verifiable<W3CCredential>
+
+    const received: unknown[] = []
+
+    const result = await verifyParsedCredential(tampered, {
+      // Only the real issuer is trusted; the tampered "attacker" issuer is not
+      trustedIssuers: [issuerDid],
+      resolver,
+      verifiers: [
+        {
+          accepts: () => true,
+          verify: (subject) => {
+            received.push(subject)
+            return Promise.resolve()
+          },
+        },
+      ],
+    })
+
+    // Returns the verified credential (the contract callers like
+    // verifyPaymentReceipt rely on), not the caller-supplied object
+    expect(result).toBe(verifiedCredential)
+    // Trust decisions used the verified payload, not the tampered outer object
+    expect(received).toEqual([verifiedSubject])
+  })
+
   it("verifies a valid credential without a list of trusted issuers", async () => {
     const { vc, resolver } = await setup()
 

packages/vc/src/verification/verify-parsed-credential.ts

@@ -48,51 +48,66 @@ function isVerifiable(
  *
  * @param credential - The credential to verify.
  * @param options - The {@link VerifyCredentialOptions} to use
+ * @returns The credential decoded from the verified proof. Callers MUST use
+ *   this returned value for any post-verification reads — the `credential`
+ *   argument is caller-supplied and is not bound to the proof.
  * @throws on error
  */
 export async function verifyParsedCredential(
   credential: W3CCredential,
   options: VerifyCredentialOptions,
-): Promise<void> {
+): Promise<Verifiable<W3CCredential>> {
   if (!isVerifiable(credential)) {
     throw new InvalidProofError("Credential does not contain a proof")
   }
 
-  await verifyProof(credential.proof, options.resolver)
+  // The proof (a JWT) is the only authoritative source. `verifyProof` returns
+  // the credential decoded from the verified proof; all trust decisions below
+  // run against that, never against the caller-supplied `credential` object,
+  // whose fields are not bound to the proof and may have been tampered with.
+  const verifiedCredential = await verifyProof(
+    credential.proof,
+    options.resolver,
+  )
 
-  if (isExpired(credential)) {
+  if (isExpired(verifiedCredential)) {
     throw new CredentialExpiredError()
   }
 
-  if (await isRevoked(credential)) {
+  if (await isRevoked(verifiedCredential)) {
     throw new CredentialRevokedError()
   }
 
   // If trustedIssuers is defined, we require the issuer is in the array (even
   // if the array is empty). If it is not defined, we skip the check.
   if (
     Array.isArray(options.trustedIssuers) &&
-    !options.trustedIssuers.includes(credential.issuer.id)
+    !options.trustedIssuers.includes(verifiedCredential.issuer.id)
   ) {
     throw new UntrustedIssuerError(
-      `Issuer is not trusted '${credential.issuer.id}'`,
+      `Issuer is not trusted '${verifiedCredential.issuer.id}'`,
     )
   }
 
   // If verifiers are provided, we verify the credential against them.
   if (options.verifiers?.length) {
     const verifiers = options.verifiers.filter((v) =>
-      v.accepts(credential.type),
+      v.accepts(verifiedCredential.type),
     )
 
     if (!verifiers.length) {
       throw new UnsupportedCredentialTypeError(
-        `Unsupported credential type: ${credential.type.join(", ")}`,
+        `Unsupported credential type: ${verifiedCredential.type.join(", ")}`,
       )
     }
 
     for (const verifier of verifiers) {
-      await verifier.verify(credential.credentialSubject, options.resolver)
+      await verifier.verify(
+        verifiedCredential.credentialSubject,
+        options.resolver,
+      )
     }
   }
+
+  return verifiedCredential
 }

packages/vc/src/verification/verify-proof.test.ts

@@ -56,13 +56,23 @@ describe("verifyProof", () => {
     ).rejects.toThrow(InvalidProofError)
   })
 
-  it("successfully verifies a valid JwtProof2020", async () => {
+  it("returns the credential decoded from the verified proof", async () => {
     const validProof = {
       type: "JwtProof2020",
       jwt: "valid.jwt.token",
     }
 
-    vi.mocked(verifyCredential).mockResolvedValueOnce({} as VerifiedCredential)
-    await expect(verifyProof(validProof, mockResolver)).resolves.not.toThrow()
+    const verifiableCredential = {
+      issuer: { id: "did:example:issuer" },
+      credentialSubject: { id: "did:example:subject" },
+    }
+
+    vi.mocked(verifyCredential).mockResolvedValueOnce({
+      verifiableCredential,
+    } as unknown as VerifiedCredential)
+
+    await expect(verifyProof(validProof, mockResolver)).resolves.toBe(
+      verifiableCredential,
+    )
   })
 })

packages/vc/src/verification/verify-proof.ts

@@ -1,8 +1,8 @@
 import type { Resolvable } from "@agentcommercekit/did"
-import { verifyCredential } from "did-jwt-vc"
 
 import type { Verifiable, W3CCredential } from "../types"
 import { InvalidProofError, UnsupportedProofTypeError } from "./errors"
+import { parseJwtCredential } from "./parse-jwt-credential"
 
 interface JwtProof {
   type: "JwtProof2020"
@@ -29,28 +29,35 @@ export function isJwtProof(proof: unknown): proof is JwtProof {
 async function verifyJwtProof(
   proof: Verifiable<W3CCredential>["proof"],
   resolver: Resolvable,
-): Promise<void> {
+): Promise<Verifiable<W3CCredential>> {
   if (!isJwtProof(proof)) {
     throw new InvalidProofError()
   }
 
   try {
-    await verifyCredential(proof.jwt, resolver)
+    return await parseJwtCredential(proof.jwt, resolver)
   } catch (_error) {
     throw new InvalidProofError()
   }
 }
 
 /**
- * Verify a proof
+ * Verify a proof and return the credential decoded from it.
+ *
+ * The returned credential is derived from the verified proof (the JWT payload),
+ * not from any caller-supplied object. Callers MUST treat the returned value as
+ * the authoritative credential: a `JwtProof2020` proof is only bound to the
+ * claims inside its own JWT, so any outer object wrapping the proof can carry
+ * tampered fields that the proof does not attest to.
  *
  * @param proof - The credential proof to verify
  * @param resolver - The resolver to use for did resolution
+ * @returns The {@link Verifiable<W3CCredential>} decoded from the verified proof
  */
 export async function verifyProof(
   proof: Verifiable<W3CCredential>["proof"],
   resolver: Resolvable,
-): Promise<void> {
+): Promise<Verifiable<W3CCredential>> {
   switch (proof.type) {
     case "JwtProof2020":
       return verifyJwtProof(proof, resolver)