From 28234c07750ee6b168a62608b72550b98eb301dc Mon Sep 17 00:00:00 2001 From: Matt Venables Date: Sat, 20 Jun 2026 10:59:56 -0400 Subject: [PATCH 1/3] fix(vc): validate decoded credential shape in parseJwtCredential did-jwt-vc's `verifyCredential` returns its own credential shape, which `parseJwtCredential` cast to `Verifiable` unchecked. If that shape ever diverged, the cast would silently mask the mismatch. Validate the decoded credential against `isCredential` (our `credentialSchema`) before returning, throwing `InvalidCredentialError` on a non-conforming shape. Adds a regression test (delegating did-jwt-vc mock so the real-signing tests still run); legitimate did-jwt-vc output is unaffected. Follow-up to #113 (addresses the unchecked-cast review comment from #110). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../parse-jwt-credential-shape-guard.md | 9 +++++++ .../verification/parse-jwt-credential.test.ts | 24 ++++++++++++++++++- .../src/verification/parse-jwt-credential.ts | 11 +++++++++ .../vc/src/verification/verify-proof.test.ts | 4 ++++ 4 files changed, 47 insertions(+), 1 deletion(-) create mode 100644 .changeset/parse-jwt-credential-shape-guard.md diff --git a/.changeset/parse-jwt-credential-shape-guard.md b/.changeset/parse-jwt-credential-shape-guard.md new file mode 100644 index 0000000..3d0c5cf --- /dev/null +++ b/.changeset/parse-jwt-credential-shape-guard.md @@ -0,0 +1,9 @@ +--- +"@agentcommercekit/vc": patch +--- + +Validate the decoded credential shape in `parseJwtCredential` instead of relying +on an unchecked cast. `verifyCredential` (did-jwt-vc) returns its own credential +shape; `parseJwtCredential` now checks it conforms to `W3CCredential` via +`isCredential` and throws `InvalidCredentialError` on a divergent shape, rather +than silently casting it. diff --git a/packages/vc/src/verification/parse-jwt-credential.test.ts b/packages/vc/src/verification/parse-jwt-credential.test.ts index 6912a82..30a2abe 100644 --- a/packages/vc/src/verification/parse-jwt-credential.test.ts +++ b/packages/vc/src/verification/parse-jwt-credential.test.ts @@ -5,12 +5,21 @@ import { } from "@agentcommercekit/did" import { createJwtSigner } from "@agentcommercekit/jwt" import { generateKeypair } from "@agentcommercekit/keys" -import { expect, test } from "vitest" +import { verifyCredential } from "did-jwt-vc" +import { expect, test, vi } from "vitest" import { createCredential } from "../create-credential" import { signCredential } from "../signing/sign-credential" +import { InvalidCredentialError } from "./errors" import { parseJwtCredential } from "./parse-jwt-credential" +vi.mock("did-jwt-vc", async (importOriginal) => { + const actual = await importOriginal() + // Delegate to the real implementation by default; individual tests can + // override `verifyCredential` to exercise malformed decoder output. + return { ...actual, verifyCredential: vi.fn(actual.verifyCredential) } +}) + test("parseJwtCredential should parse a valid credential", async () => { const resolver = getDidResolver() @@ -59,3 +68,16 @@ test("verifyCredentialJwt should throw for invalid credential", async () => { parseJwtCredential(invalidCredential, resolver), ).rejects.toThrow() }) + +test("throws when the verified JWT does not decode to a valid credential", async () => { + const resolver = getDidResolver() + + // Simulate did-jwt-vc returning a shape that diverges from W3CCredential + vi.mocked(verifyCredential).mockResolvedValueOnce({ + verifiableCredential: { not: "a credential" }, + } as unknown as Awaited>) + + await expect(parseJwtCredential("a.b.c", resolver)).rejects.toThrow( + InvalidCredentialError, + ) +}) diff --git a/packages/vc/src/verification/parse-jwt-credential.ts b/packages/vc/src/verification/parse-jwt-credential.ts index 5b75d67..022c2ae 100644 --- a/packages/vc/src/verification/parse-jwt-credential.ts +++ b/packages/vc/src/verification/parse-jwt-credential.ts @@ -1,7 +1,9 @@ import type { Resolvable } from "@agentcommercekit/did" import { verifyCredential } from "did-jwt-vc" +import { isCredential } from "../is-credential" import type { Verifiable, W3CCredential } from "../types" +import { InvalidCredentialError } from "./errors" /** * Parse a JWT credential @@ -16,5 +18,14 @@ export async function parseJwtCredential( ): Promise> { const result = await verifyCredential(jwt, resolver) + // `verifyCredential` returns did-jwt-vc's own credential shape. Validate it + // conforms to our `W3CCredential` before returning it, rather than trusting an + // unchecked cast that would silently mask a divergent shape. + if (!isCredential(result.verifiableCredential)) { + throw new InvalidCredentialError( + "Verified JWT did not decode to a valid W3C credential", + ) + } + return result.verifiableCredential as Verifiable } diff --git a/packages/vc/src/verification/verify-proof.test.ts b/packages/vc/src/verification/verify-proof.test.ts index 50d87e6..d8bac7b 100644 --- a/packages/vc/src/verification/verify-proof.test.ts +++ b/packages/vc/src/verification/verify-proof.test.ts @@ -63,8 +63,12 @@ describe("verifyProof", () => { } const verifiableCredential = { + "@context": ["https://www.w3.org/2018/credentials/v1"], + type: ["VerifiableCredential"], issuer: { id: "did:example:issuer" }, + issuanceDate: "2024-01-01T00:00:00.000Z", credentialSubject: { id: "did:example:subject" }, + proof: { type: "JwtProof2020", jwt: "valid.jwt.token" }, } vi.mocked(verifyCredential).mockResolvedValueOnce({ From 463daacfdb6bc9eec2fe5e6171dedb723d8b4fde Mon Sep 17 00:00:00 2001 From: Matt Venables Date: Sat, 20 Jun 2026 12:57:50 -0400 Subject: [PATCH 2/3] fix(vc): target the parseJwtCredential guard to the consumed fields MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Panel review of #115 found the `isCredential` guard was the wrong validator: - it validates ACK's authoring schema's pre-transform shape, so it would PASS a top-level string `issuer` that downstream then reads as `issuer.id === undefined` (codex); - and it is simultaneously too strict — rejecting valid W3C VCs with an object `@context` entry or a VC 2.0 shape that the previous cast accepted (claude). Replace it with a focused `isDecodedCredential` guard that validates exactly what the verification chain consumes: `credentialSubject` (object), `issuer.id` (string), `type` (array), and `proof` — and nothing else, so valid third-party VCs are not false-rejected. Add tests for the string-issuer rejection and the object-`@context` acceptance. Also preserve `InvalidCredentialError` through `verifyJwtProof` instead of re-wrapping it as `InvalidProofError`, so a malformed decoded credential is distinguishable from a bad signature (opencode). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../verification/parse-jwt-credential.test.ts | 44 +++++++++++++++++++ .../src/verification/parse-jwt-credential.ts | 39 +++++++++++++--- packages/vc/src/verification/verify-proof.ts | 13 +++++- 3 files changed, 89 insertions(+), 7 deletions(-) diff --git a/packages/vc/src/verification/parse-jwt-credential.test.ts b/packages/vc/src/verification/parse-jwt-credential.test.ts index 30a2abe..52c553e 100644 --- a/packages/vc/src/verification/parse-jwt-credential.test.ts +++ b/packages/vc/src/verification/parse-jwt-credential.test.ts @@ -81,3 +81,47 @@ test("throws when the verified JWT does not decode to a valid credential", async InvalidCredentialError, ) }) + +test("throws when the decoded credential has a non-normalized string issuer", async () => { + const resolver = getDidResolver() + + // Downstream reads `issuer.id`, so a top-level string issuer must be rejected + vi.mocked(verifyCredential).mockResolvedValueOnce({ + verifiableCredential: { + "@context": ["https://www.w3.org/2018/credentials/v1"], + type: ["VerifiableCredential"], + issuer: "did:example:issuer", + issuanceDate: "2024-01-01T00:00:00.000Z", + credentialSubject: { id: "did:example:subject" }, + proof: { type: "JwtProof2020", jwt: "a.b.c" }, + }, + } as unknown as Awaited>) + + await expect(parseJwtCredential("a.b.c", resolver)).rejects.toThrow( + InvalidCredentialError, + ) +}) + +test("accepts a credential with a JSON-LD object context entry", async () => { + const resolver = getDidResolver() + + // A valid VC with an object `@context` entry must NOT be false-rejected + const verifiableCredential = { + "@context": [ + "https://www.w3.org/2018/credentials/v1", + { ex: "https://example.com/vocab#" }, + ], + type: ["VerifiableCredential"], + issuer: { id: "did:example:issuer" }, + issuanceDate: "2024-01-01T00:00:00.000Z", + credentialSubject: { id: "did:example:subject" }, + proof: { type: "JwtProof2020", jwt: "a.b.c" }, + } + vi.mocked(verifyCredential).mockResolvedValueOnce({ + verifiableCredential, + } as unknown as Awaited>) + + await expect(parseJwtCredential("a.b.c", resolver)).resolves.toBe( + verifiableCredential, + ) +}) diff --git a/packages/vc/src/verification/parse-jwt-credential.ts b/packages/vc/src/verification/parse-jwt-credential.ts index 022c2ae..d53fc46 100644 --- a/packages/vc/src/verification/parse-jwt-credential.ts +++ b/packages/vc/src/verification/parse-jwt-credential.ts @@ -1,10 +1,42 @@ import type { Resolvable } from "@agentcommercekit/did" import { verifyCredential } from "did-jwt-vc" -import { isCredential } from "../is-credential" import type { Verifiable, W3CCredential } from "../types" import { InvalidCredentialError } from "./errors" +/** + * Validate that a decoded credential has the shape the verification chain relies + * on. did-jwt-vc returns its own credential type; this guards exactly the fields + * downstream code reads — `issuer.id`, `type`, `credentialSubject` — plus the + * `proof` that makes it a {@link Verifiable}, rather than trusting an unchecked + * cast that would silently mask a divergent shape. + * + * It intentionally does NOT enforce ACK's authoring schema, so valid third-party + * VCs (e.g. an object `@context` entry, or a VC 2.0 `validFrom`) are not rejected + * here; conversely a top-level string `issuer` is rejected because downstream + * reads `issuer.id`. + */ +function isDecodedCredential( + value: unknown, +): value is Verifiable { + if (typeof value !== "object" || value === null) { + return false + } + + const credential = value as Record + const issuer = credential.issuer + + return ( + typeof credential.credentialSubject === "object" && + credential.credentialSubject !== null && + typeof issuer === "object" && + issuer !== null && + typeof (issuer as Record).id === "string" && + Array.isArray(credential.type) && + credential.proof != null + ) +} + /** * Parse a JWT credential * @@ -18,10 +50,7 @@ export async function parseJwtCredential( ): Promise> { const result = await verifyCredential(jwt, resolver) - // `verifyCredential` returns did-jwt-vc's own credential shape. Validate it - // conforms to our `W3CCredential` before returning it, rather than trusting an - // unchecked cast that would silently mask a divergent shape. - if (!isCredential(result.verifiableCredential)) { + if (!isDecodedCredential(result.verifiableCredential)) { throw new InvalidCredentialError( "Verified JWT did not decode to a valid W3C credential", ) diff --git a/packages/vc/src/verification/verify-proof.ts b/packages/vc/src/verification/verify-proof.ts index 1221ada..72939de 100644 --- a/packages/vc/src/verification/verify-proof.ts +++ b/packages/vc/src/verification/verify-proof.ts @@ -1,7 +1,11 @@ import type { Resolvable } from "@agentcommercekit/did" import type { Verifiable, W3CCredential } from "../types" -import { InvalidProofError, UnsupportedProofTypeError } from "./errors" +import { + InvalidCredentialError, + InvalidProofError, + UnsupportedProofTypeError, +} from "./errors" import { parseJwtCredential } from "./parse-jwt-credential" interface JwtProof { @@ -36,7 +40,12 @@ async function verifyJwtProof( try { return await parseJwtCredential(proof.jwt, resolver) - } catch (_error) { + } catch (error) { + // Preserve a malformed-credential error (the decoded credential is the + // problem, not the signature); wrap anything else as an invalid proof. + if (error instanceof InvalidCredentialError) { + throw error + } throw new InvalidProofError() } } From eaf5ccf9f2c063a3d8439b4fcd062553401611ee Mon Sep 17 00:00:00 2001 From: Matt Venables Date: Sat, 20 Jun 2026 13:12:32 -0400 Subject: [PATCH 3/3] test(vc): use it() for parseJwtCredential tests per repo convention Resolves the CodeRabbit comment on #115: the file used test() while the rest of the suite (and AGENTS.md) uses assertive it("...") names. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../vc/src/verification/parse-jwt-credential.test.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/vc/src/verification/parse-jwt-credential.test.ts b/packages/vc/src/verification/parse-jwt-credential.test.ts index 52c553e..280852c 100644 --- a/packages/vc/src/verification/parse-jwt-credential.test.ts +++ b/packages/vc/src/verification/parse-jwt-credential.test.ts @@ -6,7 +6,7 @@ import { import { createJwtSigner } from "@agentcommercekit/jwt" import { generateKeypair } from "@agentcommercekit/keys" import { verifyCredential } from "did-jwt-vc" -import { expect, test, vi } from "vitest" +import { expect, it, vi } from "vitest" import { createCredential } from "../create-credential" import { signCredential } from "../signing/sign-credential" @@ -20,7 +20,7 @@ vi.mock("did-jwt-vc", async (importOriginal) => { return { ...actual, verifyCredential: vi.fn(actual.verifyCredential) } }) -test("parseJwtCredential should parse a valid credential", async () => { +it("parseJwtCredential should parse a valid credential", async () => { const resolver = getDidResolver() // Generate keypair for the issuer @@ -60,7 +60,7 @@ test("parseJwtCredential should parse a valid credential", async () => { expect(vc.type).toContain("TestCredential") }) -test("verifyCredentialJwt should throw for invalid credential", async () => { +it("verifyCredentialJwt should throw for invalid credential", async () => { const resolver = getDidResolver() const invalidCredential = "invalid.jwt.token" @@ -69,7 +69,7 @@ test("verifyCredentialJwt should throw for invalid credential", async () => { ).rejects.toThrow() }) -test("throws when the verified JWT does not decode to a valid credential", async () => { +it("throws when the verified JWT does not decode to a valid credential", async () => { const resolver = getDidResolver() // Simulate did-jwt-vc returning a shape that diverges from W3CCredential @@ -82,7 +82,7 @@ test("throws when the verified JWT does not decode to a valid credential", async ) }) -test("throws when the decoded credential has a non-normalized string issuer", async () => { +it("throws when the decoded credential has a non-normalized string issuer", async () => { const resolver = getDidResolver() // Downstream reads `issuer.id`, so a top-level string issuer must be rejected @@ -102,7 +102,7 @@ test("throws when the decoded credential has a non-normalized string issuer", as ) }) -test("accepts a credential with a JSON-LD object context entry", async () => { +it("returns the decoded credential for a JSON-LD object context entry", async () => { const resolver = getDidResolver() // A valid VC with an object `@context` entry must NOT be false-rejected