From 5f31153d665c7255fbbd0a33cf65ac317be0cb83 Mon Sep 17 00:00:00 2001 From: EazyHood Date: Sun, 2 Aug 2026 00:46:04 -0500 Subject: [PATCH 1/2] fix(core): stop an unreadable token expiry meaning "never expires" tokenIsUsable treated any expires_at it could not parse as no expiry at all: const expiresAt = parseIsoDate(cacheDocument.expires_at); if (!expiresAt) { return true; } parseIsoDate returns null for anything it cannot read, so six different shapes all resolved to "usable forever": absent, null, "", "not-a-date", 1700000000000, and {}. The numeric case is the one that makes this more than theoretical. Epoch milliseconds is a standard way to send an expiry, and broker-client stringifies whatever the broker sends -- so 1700000000000 arrives here as "1700000000000", which new Date() reads as Invalid Date. If the broker ever emits a numeric expiry, the CLI caches that token and never refreshes it again. The failure surfaces much later as auth errors with no re-login, because the client is certain the token is fine. Two changes: - parseIsoDate accepts epoch seconds and milliseconds as well as ISO strings. It also matches the sign, so a negative value is rejected rather than handed to new Date(), which does not fail on it -- V8 reads "-1" as a date and returns 2001-01-01, a nonsense expiry rather than an error. - tokenIsUsable distinguishes absent from unreadable. Absent still keeps the token: the broker never committed to an expiry, and that is the existing behaviour. Present-but-unreadable now forces a refresh, because one extra login costs far less than a client that is certain about a token it cannot reason about. Worth noting the neighbouring pendingIsExpired assumes the opposite for the same unparseable input, via Boolean(expiresAt && ...). The two now agree that an unreadable date is not a reason for confidence. Tests: 8 cases covering both epoch forms, past and future, unreadable values, absent values, the minTtlSeconds window, and malformed tokens. core 20/20; type check clean. The codex-plugin failures in the full suite are pre-existing on main (they are the CRLF issue in #70) and appear with and without this change. --- packages/core/lib/cache.js | 43 ++++++++- packages/core/test/token-expiry.test.js | 112 ++++++++++++++++++++++++ 2 files changed, 151 insertions(+), 4 deletions(-) create mode 100644 packages/core/test/token-expiry.test.js diff --git a/packages/core/lib/cache.js b/packages/core/lib/cache.js index 4eaa15f..bb75a62 100644 --- a/packages/core/lib/cache.js +++ b/packages/core/lib/cache.js @@ -57,11 +57,35 @@ export function removeTokenCache(config) { removeFile(tokenCachePath(config.cacheRoot, config.serverUrl)); } +// Epoch seconds and milliseconds below this are indistinguishable from each +// other only for dates before 1973; anything a token expiry could plausibly +// carry is far above it in ms and far below it in seconds. +const EPOCH_MILLISECONDS_THRESHOLD = 1e11; + export function parseIsoDate(value) { - if (!value) { + if (value === null || value === undefined || value === "") { return null; } - const parsed = new Date(String(value)); + + // A numeric expiry is a standard representation, and broker-client stringifies + // whatever the broker sends — so an epoch arrives here as "1700000000000", + // which `new Date()` reads as Invalid Date. Without this branch a perfectly + // valid expiry is indistinguishable from no expiry at all. + // The sign is matched so a negative value lands in the numeric branch and is + // rejected below. Left to `new Date()` it does not fail — V8 reads "-1" as a + // date and yields 2001-01-01, which is a nonsense expiry rather than an error. + const raw = typeof value === "number" ? String(value) : String(value).trim(); + if (/^-?\d+$/.test(raw)) { + const numeric = Number(raw); + if (!Number.isFinite(numeric) || numeric <= 0) { + return null; + } + const milliseconds = numeric < EPOCH_MILLISECONDS_THRESHOLD ? numeric * 1000 : numeric; + const parsedEpoch = new Date(milliseconds); + return Number.isNaN(parsedEpoch.getTime()) ? null : parsedEpoch; + } + + const parsed = new Date(raw); if (Number.isNaN(parsed.getTime())) { return null; } @@ -76,9 +100,20 @@ export function tokenIsUsable(cacheDocument, minTtlSeconds) { if (!token || typeof token !== "object" || typeof token.access_token !== "string" || !token.access_token) { return false; } - const expiresAt = parseIsoDate(cacheDocument.expires_at); + const rawExpiry = cacheDocument.expires_at; + const expiryWasProvided = rawExpiry !== null && rawExpiry !== undefined && rawExpiry !== ""; + const expiresAt = parseIsoDate(rawExpiry); if (!expiresAt) { - return true; + // No expiry at all: the broker never committed to one, so keep using the + // token — that is the existing behaviour and it is reasonable. + // + // An expiry that is present but unreadable is a different situation, and + // reading it as "never expires" is the worst of the available choices: the + // token is then cached forever and never refreshed, and the failure only + // surfaces much later as auth errors with no re-login. Force a refresh + // instead; the cost of one extra login is far below the cost of a client + // that is certain about a token it cannot actually reason about. + return !expiryWasProvided; } return expiresAt.getTime() - Date.now() > Number(minTtlSeconds || 0) * 1000; } diff --git a/packages/core/test/token-expiry.test.js b/packages/core/test/token-expiry.test.js new file mode 100644 index 0000000..fbc37b3 --- /dev/null +++ b/packages/core/test/token-expiry.test.js @@ -0,0 +1,112 @@ +import test from "node:test"; +import assert from "node:assert/strict"; + +import { parseIsoDate, tokenIsUsable } from "@call-e/core/cache"; + +// Regression: an expiry that could not be parsed was read as "no expiry", so the +// token was treated as usable forever and never refreshed. +// +// The case that makes it more than theoretical is a numeric expiry. Epoch +// milliseconds is a standard representation, and broker-client stringifies +// whatever the broker sends — so 1700000000000 reaches the cache as the string +// "1700000000000", which `new Date()` reads as Invalid Date. Six different +// shapes of expires_at all resolved to "usable forever": +// +// absent, null, "", "not-a-date", 1700000000000, {} +// +// Absent still means usable — the broker never committed to an expiry, and +// keeping the token is the reasonable reading. Present-but-unreadable now forces +// a refresh: one extra login costs far less than a client that is certain about +// a token it cannot reason about. + +const TOKEN = { access_token: "abc" }; +const MIN_TTL = 60; + +function withExpiry(expires_at) { + return { token: TOKEN, expires_at }; +} + +test("a numeric expiry is understood instead of ignored", () => { + const inTenMinutes = Date.now() + 10 * 60 * 1000; + + // Milliseconds, as a number and as the string broker-client produces. + assert.ok(tokenIsUsable(withExpiry(inTenMinutes), MIN_TTL)); + assert.ok(tokenIsUsable(withExpiry(String(inTenMinutes)), MIN_TTL)); + + // Seconds, the other common epoch form. + assert.ok(tokenIsUsable(withExpiry(Math.floor(inTenMinutes / 1000)), MIN_TTL)); +}); + +test("a numeric expiry in the past is honoured, not treated as no expiry", () => { + const tenMinutesAgo = Date.now() - 10 * 60 * 1000; + + assert.equal(tokenIsUsable(withExpiry(tenMinutesAgo), MIN_TTL), false); + assert.equal(tokenIsUsable(withExpiry(String(tenMinutesAgo)), MIN_TTL), false); + assert.equal( + tokenIsUsable(withExpiry(Math.floor(tenMinutesAgo / 1000)), MIN_TTL), + false, + ); +}); + +test("an unreadable expiry forces a refresh rather than lasting forever", () => { + for (const bad of ["not-a-date", "2026-13-45T99:99:99Z", {}, [], true]) { + assert.equal( + tokenIsUsable(withExpiry(bad), MIN_TTL), + false, + `expires_at ${JSON.stringify(bad)} must not read as "never expires"`, + ); + } +}); + +test("an absent expiry still keeps the token", () => { + // Unchanged behaviour: no expiry was ever promised, so there is nothing to + // distrust. Only a value that is present and broken is suspicious. + assert.ok(tokenIsUsable({ token: TOKEN }, MIN_TTL)); + assert.ok(tokenIsUsable(withExpiry(null), MIN_TTL)); + assert.ok(tokenIsUsable(withExpiry(""), MIN_TTL)); +}); + +test("an ISO expiry behaves exactly as before", () => { + const future = new Date(Date.now() + 3600 * 1000).toISOString(); + const past = new Date(Date.now() - 3600 * 1000).toISOString(); + + assert.ok(tokenIsUsable(withExpiry(future), MIN_TTL)); + assert.equal(tokenIsUsable(withExpiry(past), MIN_TTL), false); +}); + +test("minTtlSeconds still shortens the usable window", () => { + const inThirtySeconds = new Date(Date.now() + 30 * 1000).toISOString(); + + assert.ok(tokenIsUsable(withExpiry(inThirtySeconds), 0)); + assert.equal( + tokenIsUsable(withExpiry(inThirtySeconds), 120), + false, + "a token expiring inside the minimum TTL is not usable", + ); +}); + +test("a malformed token is rejected regardless of expiry", () => { + const future = new Date(Date.now() + 3600 * 1000).toISOString(); + + assert.equal(tokenIsUsable(null, MIN_TTL), false); + assert.equal(tokenIsUsable({ expires_at: future }, MIN_TTL), false); + assert.equal(tokenIsUsable({ token: {}, expires_at: future }, MIN_TTL), false); + assert.equal( + tokenIsUsable({ token: { access_token: "" }, expires_at: future }, MIN_TTL), + false, + ); +}); + +test("parseIsoDate reads both epoch forms and rejects junk", () => { + const ms = 1_700_000_000_000; + + assert.equal(parseIsoDate(ms).getTime(), ms); + assert.equal(parseIsoDate(String(ms)).getTime(), ms); + assert.equal(parseIsoDate(ms / 1000).getTime(), ms, "epoch seconds scale up"); + + assert.equal(parseIsoDate("2026-01-15T10:00:00Z").toISOString(), "2026-01-15T10:00:00.000Z"); + + for (const junk of [null, undefined, "", "not-a-date", 0, -1, {}]) { + assert.equal(parseIsoDate(junk), null, `parseIsoDate(${JSON.stringify(junk)})`); + } +}); From b2de872b407e80f71e633d05dfde54280574d8e6 Mon Sep 17 00:00:00 2001 From: EazyHood <209367218+EazyHood@users.noreply.github.com> Date: Mon, 3 Aug 2026 00:50:32 -0500 Subject: [PATCH 2/2] chore(core): add the patch changeset for the token expiry fix Addresses the review on #72. Describes the epoch-expiry parsing and the unreadable-expiry refresh behaviour, as asked. Verified: pnpm run check:versions is in sync, pnpm --filter @call-e/core pack:dry-run builds the tarball, and the core suite is 20/20. --- .changeset/olive-donkeys-smile.md | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 .changeset/olive-donkeys-smile.md diff --git a/.changeset/olive-donkeys-smile.md b/.changeset/olive-donkeys-smile.md new file mode 100644 index 0000000..51ce159 --- /dev/null +++ b/.changeset/olive-donkeys-smile.md @@ -0,0 +1,9 @@ +--- +"@call-e/core": patch +--- + +Read numeric token expiries, and refresh instead of caching forever when one cannot be read. + +A broker that sends the expiry as epoch seconds or milliseconds arrived here as a string such as `"1700000000000"`, which `new Date()` reads as `Invalid Date`. A valid expiry was therefore indistinguishable from no expiry at all, and the token was cached as if it never expired. Numeric expiries are now parsed explicitly, with values below 1e11 taken as seconds and the rest as milliseconds; a negative or non-finite value is rejected rather than being turned into a nonsense date by `new Date("-1")`. + +An expiry that still cannot be read no longer means "never expires": the entry is treated as expired so the next call refreshes it.