From 03b9837002ad7663a1fc28bd3e287b36e475b611 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Fri, 23 Oct 2026 12:07:49 +0000 Subject: [PATCH] fix(cli): Refresh tokens with a corrupt issued_at A stored issued_at at or after expires_at made the remaining-lifetime ratio Infinity (or flipped its sign), so proactive refresh was skipped until the token actually expired. Fall back to the default lifetime when the stored lifetime is not positive, and share the ratio between refreshToken and the API client so they cannot drift. Ports the remaining fix from getsentry/cli#1200. Co-Authored-By: BYK --- packages/cli/src/lib/db/auth.ts | 28 ++++++++++++-- packages/cli/src/lib/sentry-client.ts | 17 +++------ packages/cli/test/lib/db/auth.test.ts | 54 +++++++++++++++++++++++++++ 3 files changed, 85 insertions(+), 14 deletions(-) diff --git a/packages/cli/src/lib/db/auth.ts b/packages/cli/src/lib/db/auth.ts index da16edf19..1e391a80e 100644 --- a/packages/cli/src/lib/db/auth.ts +++ b/packages/cli/src/lib/db/auth.ts @@ -74,6 +74,26 @@ export const REFRESH_THRESHOLD = 0.1; /** Default token lifetime (1 hour) for tokens without issuedAt */ export const DEFAULT_TOKEN_LIFETIME_MS = 3600 * 1000; +/** + * Fraction of a token's lifetime that remains at `now`. + * + * Uses {@link DEFAULT_TOKEN_LIFETIME_MS} when `issuedAt` is missing or not + * before `expiresAt`. A corrupt row with `issued_at >= expires_at` would + * otherwise divide by zero or flip the sign, and an `Infinity` ratio skips + * proactive refresh. + */ +export function getRemainingLifetimeRatio( + expiresAt: number, + issuedAt: number | null | undefined, + now: number, +): number { + const storedLifetime = + issuedAt === null || issuedAt === undefined ? 0 : expiresAt - issuedAt; + const totalLifetime = + storedLifetime > 0 ? storedLifetime : DEFAULT_TOKEN_LIFETIME_MS; + return (expiresAt - now) / totalLifetime; +} + /** * The active account row. `host` is NULL only for rows migrated from the * pre-v16 `auth` table; `migrateNullHost` backfills it on access. @@ -1679,10 +1699,12 @@ export async function refreshToken( return { ...baseResult, token: row.token, refreshed: false }; } - const issuedAt = row.issued_at ?? expiresAt - DEFAULT_TOKEN_LIFETIME_MS; - const totalLifetime = expiresAt - issuedAt; const remainingLifetime = expiresAt - now; - const remainingRatio = remainingLifetime / totalLifetime; + const remainingRatio = getRemainingLifetimeRatio( + expiresAt, + row.issued_at, + now, + ); const expiresIn = Math.max(0, Math.floor(remainingLifetime / 1000)); if (!force && remainingRatio > REFRESH_THRESHOLD && now < expiresAt) { diff --git a/packages/cli/src/lib/sentry-client.ts b/packages/cli/src/lib/sentry-client.ts index ac25222c3..fae7b82ae 100644 --- a/packages/cli/src/lib/sentry-client.ts +++ b/packages/cli/src/lib/sentry-client.ts @@ -32,8 +32,8 @@ import { import { applyCustomHeaders } from "./custom-headers.js"; import { type CredentialContext, - DEFAULT_TOKEN_LIFETIME_MS, getCredentialContext, + getRemainingLifetimeRatio, REFRESH_THRESHOLD, refreshToken, } from "./db/auth.js"; @@ -631,19 +631,14 @@ async function fetchWithRetry({ validatedRedirects, responseBodyMode, }: RetryContext): Promise { - const issuedAt = - credential.issuedAt ?? - (credential.expiresAt - ? credential.expiresAt - DEFAULT_TOKEN_LIFETIME_MS - : undefined); const shouldRefresh = Boolean( credential.refreshable && credential.expiresAt && - (Date.now() >= credential.expiresAt || - (issuedAt && - (credential.expiresAt - Date.now()) / - (credential.expiresAt - issuedAt) <= - REFRESH_THRESHOLD)), + getRemainingLifetimeRatio( + credential.expiresAt, + credential.issuedAt, + Date.now(), + ) <= REFRESH_THRESHOLD, ); const refreshed = shouldRefresh ? await refreshToken({ expectedCredential: credential }) diff --git a/packages/cli/test/lib/db/auth.test.ts b/packages/cli/test/lib/db/auth.test.ts index 392be4283..6fb5b3a03 100644 --- a/packages/cli/test/lib/db/auth.test.ts +++ b/packages/cli/test/lib/db/auth.test.ts @@ -11,14 +11,17 @@ import { describe, expect, test } from "vitest"; import { ANON_IDENTITY, clearAuth, + DEFAULT_TOKEN_LIFETIME_MS, getActiveEnvVarName, getAuthConfig, getAuthToken, getIdentityFingerprint, getRawEnvToken, + getRemainingLifetimeRatio, hasStoredAuthCredentials, isAuthenticated, isEnvTokenActive, + REFRESH_THRESHOLD, refreshToken, resetAuthTokenCache, resetHasStoredCredsCache, @@ -437,6 +440,19 @@ describe("refreshToken row-read memoization", () => { }); }); +describe("refreshToken: corrupt issued_at", () => { + test("proactively refreshes when issued_at equals expires_at", async () => { + // 2 minutes left is under 10% of the default 1h lifetime. A zero stored + // lifetime used to read as an Infinity ratio and skip the refresh path. + setAuthToken("corrupt_access", 120); + getDatabase().query("UPDATE accounts SET issued_at = expires_at").run(); + process.env.SENTRY_AUTH_TOKEN = "env_token"; + + const result = await refreshToken(); + expect(result.token).toBe("env_token"); + }); +}); + describe("hasStoredAuthCredentials memoization", () => { test("returns false and caches when no stored token", () => { expect(hasStoredAuthCredentials()).toBe(false); @@ -479,3 +495,41 @@ describe("hasStoredAuthCredentials memoization", () => { expect(hasStoredAuthCredentials()).toBe(true); }); }); + +describe("getRemainingLifetimeRatio", () => { + const now = 1_000_000_000; + + test("uses the stored lifetime when issuedAt precedes expiresAt", () => { + expect(getRemainingLifetimeRatio(now + 500, now - 500, now)).toBe(0.5); + }); + + test("falls back to the default lifetime when issuedAt is missing", () => { + const expiresAt = now + DEFAULT_TOKEN_LIFETIME_MS / 4; + expect(getRemainingLifetimeRatio(expiresAt, null, now)).toBe(0.25); + expect(getRemainingLifetimeRatio(expiresAt, undefined, now)).toBe(0.25); + }); + + test.each([ + ["issued_at equals expires_at", 0], + ["issued_at after expires_at", 1000], + ])( + "falls back to the default lifetime when %s", + (_label, issuedAfterExpiry) => { + // 5% of the default lifetime remains, so a sane lifetime must trigger + // proactive refresh instead of reading as Infinity or a huge ratio. + const expiresAt = now + DEFAULT_TOKEN_LIFETIME_MS * 0.05; + const ratio = getRemainingLifetimeRatio( + expiresAt, + expiresAt + issuedAfterExpiry, + now, + ); + expect(ratio).toBeCloseTo(0.05); + expect(ratio).toBeLessThanOrEqual(REFRESH_THRESHOLD); + }, + ); + + test("is non-positive once the token has expired", () => { + expect(getRemainingLifetimeRatio(now, now, now)).toBe(0); + expect(getRemainingLifetimeRatio(now - 1, now - 1, now)).toBeLessThan(0); + }); +});