From e7bec04d29bd12dfc444f98c81d4f448f15b0f48 Mon Sep 17 00:00:00 2001 From: ygd58 Date: Thu, 13 Aug 2026 15:46:56 +0200 Subject: [PATCH 1/3] fix(vc): isExpired fails closed on an unparseable expirationDate Fixes #148 isExpired returned false (not expired) whenever credential.expirationDate was present but could not be parsed into a valid date. Since isExpired is the check verifyParsedCredential uses to reject expired credentials, a credential with a malformed or malicious expirationDate value was treated as never-expiring instead of being rejected. isExpired now fails closed: an unparseable expirationDate is treated as expired, matching the safer default for a security-relevant check. Updated the existing test that asserted the old fail-open behavior, and added a changeset (patch, @agentcommercekit/vc). AI usage disclosure: this fix was developed with Claude (Anthropic) assistance - identifying the bug, writing the fix, updating the test, and verifying locally (pnpm --filter @agentcommercekit/vc test, oxlint, oxfmt). I reviewed and understand the change: it flips a single boolean return value in one function so a credential with an unparseable expiration date is rejected instead of silently accepted, and updates the one test that covered that branch. --- .changeset/is-expired-fail-closed.md | 14 ++++++++++++++ packages/vc/src/verification/is-expired.test.ts | 4 ++-- packages/vc/src/verification/is-expired.ts | 14 +++++++++++--- 3 files changed, 27 insertions(+), 5 deletions(-) create mode 100644 .changeset/is-expired-fail-closed.md diff --git a/.changeset/is-expired-fail-closed.md b/.changeset/is-expired-fail-closed.md new file mode 100644 index 00000000..15d0b347 --- /dev/null +++ b/.changeset/is-expired-fail-closed.md @@ -0,0 +1,14 @@ +--- +"@agentcommercekit/vc": patch +--- + +Fix `isExpired` failing open on an unparseable `expirationDate` + +`isExpired` returned `false` (not expired) whenever `credential.expirationDate` +was present but could not be parsed into a valid date. Since `isExpired` is +the check `verifyParsedCredential` uses to reject expired credentials, a +credential with a malformed or malicious `expirationDate` value was treated +as never-expiring instead of being rejected. + +`isExpired` now fails closed: an unparseable `expirationDate` is treated as +expired, matching the safer default for a security-relevant check. diff --git a/packages/vc/src/verification/is-expired.test.ts b/packages/vc/src/verification/is-expired.test.ts index ac456e47..ee94c86b 100644 --- a/packages/vc/src/verification/is-expired.test.ts +++ b/packages/vc/src/verification/is-expired.test.ts @@ -47,9 +47,9 @@ describe("isExpired", () => { expect(isExpired(credential)).toBe(false) }) - it("handles invalid date strings gracefully", () => { + it("treats an unparseable expiration date as expired (fail closed)", () => { const credential = buildCredential("invalid-date") - expect(isExpired(credential)).toBe(false) + expect(isExpired(credential)).toBe(true) }) }) diff --git a/packages/vc/src/verification/is-expired.ts b/packages/vc/src/verification/is-expired.ts index 1d66ac8e..829dfccd 100644 --- a/packages/vc/src/verification/is-expired.ts +++ b/packages/vc/src/verification/is-expired.ts @@ -3,8 +3,13 @@ import type { W3CCredential } from "../types" /** * Check if a credential is expired * + * Fails closed: a credential with an `expirationDate` that is present but + * cannot be parsed as a valid date is treated as expired, not as + * non-expiring. + * * @param credential - The {@link W3CCredential} to check - * @returns `true` if the credential is expired, `false` otherwise + * @returns `true` if the credential is expired (or has an unparseable + * expiration date), `false` otherwise */ export function isExpired(credential: W3CCredential): boolean { if (!credential.expirationDate) { @@ -14,8 +19,11 @@ export function isExpired(credential: W3CCredential): boolean { const expirationDate = new Date(credential.expirationDate) if (isNaN(expirationDate.getTime())) { - // Expiration date is invalid, so we consider the credential not expired - return false + // Expiration date is present but unparseable. Fail closed: an + // unparseable expiration date must not be treated as "never expires", + // since that would let a malformed or malicious `expirationDate` value + // grant a credential unbounded validity. + return true } return expirationDate < new Date() From ab564b2d432c462477f08b14841e2abf6c48e816 Mon Sep 17 00:00:00 2001 From: ygd58 Date: Fri, 14 Aug 2026 14:21:06 +0200 Subject: [PATCH 2/3] fix(vc): isExpired also fails closed on a present-but-empty expirationDate Addresses review feedback from @qlxjcj on this PR. The initial fix's guard, if (!credential.expirationDate), treats an false before ever reaching the isNaN fail-closed branch. An empty string is present but unparseable (new Date("") -> NaN), so it should fail closed like any other unparseable value - the falsy check was silently leaving the same fail-open hole open for this one case. Changed the guard to credential.expirationDate === undefined, which only treats a genuinely absent field as absent, letting an empty string reach the isNaN check and correctly fail closed. Added a regression test for this case. pnpm --filter @agentcommercekit/vc exec vitest run src/verification/is-expired.test.ts - 6/6 passing. oxlint and oxfmt clean. --- packages/vc/src/verification/is-expired.test.ts | 9 +++++++++ packages/vc/src/verification/is-expired.ts | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/packages/vc/src/verification/is-expired.test.ts b/packages/vc/src/verification/is-expired.test.ts index ee94c86b..2fde1166 100644 --- a/packages/vc/src/verification/is-expired.test.ts +++ b/packages/vc/src/verification/is-expired.test.ts @@ -52,4 +52,13 @@ describe("isExpired", () => { expect(isExpired(credential)).toBe(true) }) + + it("treats a present but empty-string expiration date as expired (fail closed)", () => { + // An empty string is present (not `undefined`) but unparseable + // (`new Date("")` -> `NaN`). It must not be conflated with an absent + // `expirationDate` via a falsy check, or it silently fails open. + const credential = buildCredential("") + + expect(isExpired(credential)).toBe(true) + }) }) diff --git a/packages/vc/src/verification/is-expired.ts b/packages/vc/src/verification/is-expired.ts index 829dfccd..09a5cc98 100644 --- a/packages/vc/src/verification/is-expired.ts +++ b/packages/vc/src/verification/is-expired.ts @@ -12,7 +12,7 @@ import type { W3CCredential } from "../types" * expiration date), `false` otherwise */ export function isExpired(credential: W3CCredential): boolean { - if (!credential.expirationDate) { + if (credential.expirationDate === undefined) { return false } From 1d320f0e0823f79da1b83db4f026eb1272f82586 Mon Sep 17 00:00:00 2001 From: ygd58 Date: Thu, 3 Sep 2026 05:25:44 +0200 Subject: [PATCH 3/3] fix(vc): isExpired also rejects non-string expirationDate values Addresses review feedback from @venables on this PR. parseJwtCredential does not validate that expirationDate is a string, so a malformed or malicious credential could carry a number (which Date() accepts as epoch milliseconds) or another non-string JSON value. Without an explicit typeof check, a numeric value corresponding to a future date would incorrectly pass isExpired as 'not expired' via the normal date-comparison path, since new Date(epochMs) produces a perfectly valid Date object. Added a typeof credential.expirationDate !== 'string' check, per @venables' suggested fix. Added a regression test using a numeric epochMs value ten years in the future - chosen specifically because a small number (e.g. 1) would coincidentally 'pass' via the normal date-comparison path regardless of whether the guard exists, giving false confidence; a future-dated epoch number only fails without the explicit type check. Also updated the stale comment in is-revoked.ts's direct expiry check, per @venables' first point: it referenced isExpired's old fail-open behavior on unparseable dates, which this PR already made false. Reworded to the reasons that direct check still needs to exist (must throw undetermined() with a URL-specific message; must reject a list expiring at exactly 'now' via <=, whereas isExpired's own bound is strict <). Left @venables' second point (verify-parsed-credential.ts's generic 'Credential is expired' message vs. is-revoked.ts's more specific 'unreadable expirationDate' wording) as-is, per the reviewer's own suggested option, to keep this diff minimal and scoped to the type-confusion fix and the one comment it made inaccurate. pnpm --filter @agentcommercekit/vc exec vitest run - 111/111 passing (full package). oxlint and oxfmt (both scoped and pnpm run check:format repo-wide) clean. --- .../vc/src/verification/is-expired.test.ts | 20 +++++++++++++++++++ packages/vc/src/verification/is-expired.ts | 10 ++++++++++ packages/vc/src/verification/is-revoked.ts | 9 ++++++--- 3 files changed, 36 insertions(+), 3 deletions(-) diff --git a/packages/vc/src/verification/is-expired.test.ts b/packages/vc/src/verification/is-expired.test.ts index 2fde1166..1e6bc808 100644 --- a/packages/vc/src/verification/is-expired.test.ts +++ b/packages/vc/src/verification/is-expired.test.ts @@ -61,4 +61,24 @@ describe("isExpired", () => { expect(isExpired(credential)).toBe(true) }) + + it("treats a non-string expiration date as expired (fail closed)", () => { + // `parseJwtCredential` does not validate that `expirationDate` is a + // string, so a malformed or untrusted credential can carry a number + // (or another non-string JSON value) here at runtime, bypassing the + // type system. `new Date(epochMs)` parses to a valid date, so without + // an explicit typeof check, a numeric value corresponding to a *future* + // date would incorrectly pass as "not expired" via the normal + // date-comparison path below - this must be rejected before it gets + // that far, regardless of which date it happens to encode. + const tenYearsFromNowMs = Date.now() + 10 * 365 * 24 * 60 * 60 * 1000 + + // oxlint-disable-next-line typescript/no-unsafe-type-assertion -- models an untyped/malformed JWT payload + const credential = { + ...buildCredential(), + expirationDate: tenYearsFromNowMs, + } as unknown as W3CCredential + + expect(isExpired(credential)).toBe(true) + }) }) diff --git a/packages/vc/src/verification/is-expired.ts b/packages/vc/src/verification/is-expired.ts index 09a5cc98..b912f94f 100644 --- a/packages/vc/src/verification/is-expired.ts +++ b/packages/vc/src/verification/is-expired.ts @@ -16,6 +16,16 @@ export function isExpired(credential: W3CCredential): boolean { return false } + // `parseJwtCredential` does not validate that `expirationDate` is a + // string, so a malformed or malicious credential could carry a number + // (which `Date()` accepts as epoch milliseconds) or another non-string + // JSON value here. Reject anything that isn't a string outright, rather + // than letting it reach `new Date()`, which would silently accept types + // the {@link W3CCredential} type only documents as a string. + if (typeof credential.expirationDate !== "string") { + return true + } + const expirationDate = new Date(credential.expirationDate) if (isNaN(expirationDate.getTime())) { diff --git a/packages/vc/src/verification/is-revoked.ts b/packages/vc/src/verification/is-revoked.ts index 48255533..974938b9 100644 --- a/packages/vc/src/verification/is-revoked.ts +++ b/packages/vc/src/verification/is-revoked.ts @@ -411,9 +411,12 @@ async function resolveStatusListCredential( ) } - // Check the expiry directly rather than through `isExpired`, which reads an - // unparseable date as "not expired". The expiry is the main bound on status - // list replay, so a malformed one must not quietly remove it. + // Check the expiry directly rather than through `isExpired`: this path + // must throw `undetermined()` with a URL-specific message (isExpired only + // returns a boolean), and must reject a list that expires at exactly `now` + // (`<=`), whereas isExpired's own bound is strict (`<`). The expiry is the + // main bound on status list replay, so a malformed one must not quietly + // remove it. if (verified.expirationDate !== undefined) { const expiresAt = Date.parse(verified.expirationDate)