diff --git a/app/api/admin/version/route.ts b/app/api/admin/version/route.ts index 48655c049..2c63a7b33 100644 --- a/app/api/admin/version/route.ts +++ b/app/api/admin/version/route.ts @@ -5,6 +5,7 @@ import { loadState, checkOnce, effectiveEndpoint, + freshStatus, disabledByEnv, DEFAULT_VERSION_ENDPOINT, } from '@/lib/version-check'; @@ -29,7 +30,7 @@ export async function GET(request: NextRequest) { lastCheckedAt: state.lastCheckedAt, lastSuccessAt: state.lastSuccessAt, nextScheduledAt: state.nextScheduledAt, - status: state.status, + status: freshStatus(state), }, { headers: { 'Cache-Control': 'no-store' } }, ); diff --git a/app/api/system/update-status/route.ts b/app/api/system/update-status/route.ts index fd25da803..ca2de80e1 100644 --- a/app/api/system/update-status/route.ts +++ b/app/api/system/update-status/route.ts @@ -1,5 +1,5 @@ import { NextResponse } from 'next/server'; -import { checkOnce, loadState } from '@/lib/version-check'; +import { checkOnce, freshStatus, loadState } from '@/lib/version-check'; // Public endpoint that returns the latest cached update status. Fed by the // background scheduler started in instrumentation.node.ts; in production we @@ -16,9 +16,11 @@ export async function GET() { } const state = await loadState(); + // freshStatus drops a status persisted by an earlier build - after an + // upgrade it would still advertise the version we just upgraded from (#913). return NextResponse.json( { - status: state.status, + status: freshStatus(state), lastCheckedAt: state.lastCheckedAt, lastSuccessAt: state.lastSuccessAt, }, diff --git a/lib/__tests__/version-check-upgrade.test.ts b/lib/__tests__/version-check-upgrade.test.ts new file mode 100644 index 000000000..452b8c78b --- /dev/null +++ b/lib/__tests__/version-check-upgrade.test.ts @@ -0,0 +1,154 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { mkdtemp, readFile, rm, writeFile, mkdir } from 'node:fs/promises'; +import path from 'node:path'; +import os from 'node:os'; + +import type { UpdateStatus, VersionCheckStateFile } from '@/lib/version-check/types'; + +let dir: string; + +function statusFor(current: string, latest: string, updateAvailable: boolean): UpdateStatus { + return { + schema: 1, + current, + latest, + updateAvailable, + severity: updateAvailable ? 'normal' : 'none', + url: 'https://example.invalid/release', + advisory: null, + checkedAt: '2026-08-25T21:33:19.432Z', + }; +} + +async function writeStateFile(state: Partial): Promise { + await mkdir(dir, { recursive: true }); + await writeFile(path.join(dir, 'state.json'), JSON.stringify(state), 'utf8'); +} + +async function readStateFile(): Promise { + return JSON.parse(await readFile(path.join(dir, 'state.json'), 'utf8')) as VersionCheckStateFile; +} + +describe('version-check across an upgrade (#913)', () => { + beforeEach(async () => { + dir = await mkdtemp(path.join(os.tmpdir(), 'bulwark-version-check-')); + process.env.VERSION_CHECK_DATA_DIR = dir; + process.env.NEXT_PUBLIC_APP_VERSION = '1.9.0'; + delete process.env.BULWARK_UPDATE_CHECK; + process.env.BULWARK_UPDATE_CHECK_URL = 'https://version.invalid/'; + }); + + afterEach(async () => { + const { stopScheduler } = await import('@/lib/version-check'); + await stopScheduler(); + vi.useRealTimers(); + vi.resetModules(); + vi.unstubAllGlobals(); + delete process.env.VERSION_CHECK_DATA_DIR; + delete process.env.NEXT_PUBLIC_APP_VERSION; + delete process.env.BULWARK_UPDATE_CHECK_URL; + await rm(dir, { recursive: true, force: true }); + }); + + describe('freshStatus', () => { + it('hides a status persisted by the previous version', async () => { + const { freshStatus } = await import('@/lib/version-check'); + const state = { + endpoint: '', lastCheckedAt: null, lastSuccessAt: null, nextScheduledAt: null, + status: statusFor('1.7.8', '1.9.0', true), + } satisfies VersionCheckStateFile; + expect(freshStatus(state)).toBeNull(); + }); + + it('keeps a status written by the running version', async () => { + const { freshStatus } = await import('@/lib/version-check'); + const status = statusFor('1.9.0', '1.9.0', false); + const state = { + endpoint: '', lastCheckedAt: null, lastSuccessAt: null, nextScheduledAt: null, status, + } satisfies VersionCheckStateFile; + expect(freshStatus(state)).toBe(status); + }); + + it('keeps the status when the running version is unknown', async () => { + delete process.env.NEXT_PUBLIC_APP_VERSION; + const { freshStatus } = await import('@/lib/version-check'); + const status = statusFor('1.7.8', '1.9.0', true); + const state = { + endpoint: '', lastCheckedAt: null, lastSuccessAt: null, nextScheduledAt: null, status, + } satisfies VersionCheckStateFile; + expect(freshStatus(state)).toBe(status); + }); + }); + + describe('startScheduler', () => { + it('drops the old status and checks promptly instead of honoring the old schedule', async () => { + // State left behind by 1.7.8, with its next check still ~55 min out. + const future = new Date(Date.now() + 55 * 60 * 1000).toISOString(); + await writeStateFile({ + endpoint: 'https://version.invalid/', + lastCheckedAt: '2026-08-25T21:33:19.432Z', + lastSuccessAt: '2026-08-25T21:33:19.432Z', + nextScheduledAt: future, + status: statusFor('1.7.8', '1.9.0', true), + }); + + const { startScheduler } = await import('@/lib/version-check'); + await startScheduler(); + + const after = await readStateFile(); + expect(after.status).toBeNull(); + // Rescheduled to the ~30s startup delay, not the inherited 55 minutes. + const inMs = new Date(after.nextScheduledAt!).getTime() - Date.now(); + expect(inMs).toBeLessThan(60_000); + }); + + it('still honors a future schedule written by the running version', async () => { + const future = new Date(Date.now() + 55 * 60 * 1000).toISOString(); + await writeStateFile({ + endpoint: 'https://version.invalid/', + lastCheckedAt: '2026-08-25T21:33:19.432Z', + lastSuccessAt: '2026-08-25T21:33:19.432Z', + nextScheduledAt: future, + status: statusFor('1.9.0', '1.9.0', false), + }); + + const { startScheduler } = await import('@/lib/version-check'); + await startScheduler(); + + const after = await readStateFile(); + expect(after.status).not.toBeNull(); + const inMs = new Date(after.nextScheduledAt!).getTime() - Date.now(); + expect(inMs).toBeGreaterThan(50 * 60 * 1000); + }); + }); + + describe('selectBanner', () => { + it('does not show a banner for a status cached by the previous version', async () => { + const { selectBanner, selectHasUpdate } = await import('@/stores/update-store'); + const state = { + status: statusFor('1.7.8', '1.9.0', true), + loading: false, + lastFetchedAt: null, + fetchStatus: async () => {}, + startPolling: () => {}, + stopPolling: () => {}, + }; + expect(selectBanner(state)).toBeNull(); + expect(selectHasUpdate(state)).toBe(false); + }); + + it('still shows a banner for a status from the running version', async () => { + const { selectBanner, selectHasUpdate } = await import('@/stores/update-store'); + const state = { + status: statusFor('1.9.0', '1.9.1', true), + loading: false, + lastFetchedAt: null, + fetchStatus: async () => {}, + startPolling: () => {}, + stopPolling: () => {}, + }; + expect(selectBanner(state)?.severity).toBe('normal'); + expect(selectHasUpdate(state)).toBe(true); + }); + }); +}); diff --git a/lib/version-check/index.ts b/lib/version-check/index.ts index 9b7ec0844..efc5e213c 100644 --- a/lib/version-check/index.ts +++ b/lib/version-check/index.ts @@ -1,5 +1,8 @@ export { startScheduler, stopScheduler, checkOnce } from './sender'; -export { loadState, saveState, effectiveEndpoint, disabledByEnv } from './state'; +export { + loadState, saveState, effectiveEndpoint, disabledByEnv, + getCurrentVersion, isStatusStale, freshStatus, +} from './state'; export { fetchStatus, parseStatus } from './fetcher'; export type { UpdateStatus, UpdateSeverity, VersionCheckStateFile, diff --git a/lib/version-check/sender.ts b/lib/version-check/sender.ts index 9fa12a6a4..49d3fe767 100644 --- a/lib/version-check/sender.ts +++ b/lib/version-check/sender.ts @@ -1,5 +1,7 @@ import { logger } from '@/lib/logger'; -import { disabledByEnv, effectiveEndpoint, loadState, saveState } from './state'; +import { + disabledByEnv, effectiveEndpoint, getCurrentVersion, isStatusStale, loadState, saveState, +} from './state'; import { fetchStatus } from './fetcher'; import type { UpdateStatus } from './types'; @@ -15,10 +17,6 @@ function jitteredDelay(base: number): number { return Math.max(60_000, base + j); } -function getCurrentVersion(): string { - return (process.env.NEXT_PUBLIC_APP_VERSION || '').trim(); -} - export async function checkOnce(opts?: { reason?: string }): Promise<{ ok: boolean; status?: UpdateStatus; @@ -82,11 +80,26 @@ export async function startScheduler(): Promise { logger.info('version-check: scheduler not started (no endpoint)'); return; } + // State carried over from a previous build: the cached status is about a + // version we no longer run, and its schedule would keep that stale answer + // (e.g. "update available" for the version we just upgraded from) on screen + // for up to an hour. Drop it and check promptly instead (#913). + const stale = isStatusStale(state.status); + if (stale) { + logger.info('version-check: discarding status from a previous version', { + cached: state.status?.current ?? null, + running: getCurrentVersion(), + }); + state.status = null; + state.nextScheduledAt = null; + await saveState(state); + } + // If a previous schedule was still in the future, honor it (don't blast on // every restart). Cap at one hour so a wildly-in-the-future timestamp can't // permanently silence the check. let delay = FIRST_DELAY_MS; - if (state.nextScheduledAt) { + if (!stale && state.nextScheduledAt) { const remaining = new Date(state.nextScheduledAt).getTime() - Date.now(); if (remaining > 0) delay = Math.min(remaining, HOUR_MS + JITTER_MS); } diff --git a/lib/version-check/state.ts b/lib/version-check/state.ts index b6de00371..1797cb3e9 100644 --- a/lib/version-check/state.ts +++ b/lib/version-check/state.ts @@ -2,7 +2,7 @@ import { readFile, writeFile, mkdir, rename } from 'node:fs/promises'; import { existsSync } from 'node:fs'; import path from 'node:path'; import { logger } from '@/lib/logger'; -import type { VersionCheckStateFile } from './types'; +import type { UpdateStatus, VersionCheckStateFile } from './types'; import { DEFAULT_VERSION_ENDPOINT } from './types'; function getDir(): string { @@ -60,3 +60,24 @@ export function effectiveEndpoint(state: VersionCheckStateFile): string { if (envUrl !== undefined) return envUrl.trim(); return state.endpoint || DEFAULT_VERSION_ENDPOINT; } + +export function getCurrentVersion(): string { + return (process.env.NEXT_PUBLIC_APP_VERSION || '').trim(); +} + +// A persisted status describes the build that was running when it was written. +// After an upgrade it is stale by definition - it can keep claiming "update +// available" for a version we no longer run - so nothing may surface it until +// the next check refreshes it (#913). If the running version is unknown we +// can't tell, so we leave the status alone rather than blanking the banner. +export function isStatusStale( + status: UpdateStatus | null, + current: string = getCurrentVersion(), +): boolean { + if (!status || !current) return false; + return status.current !== current; +} + +export function freshStatus(state: VersionCheckStateFile): UpdateStatus | null { + return isStatusStale(state.status) ? null : state.status; +} diff --git a/stores/update-store.ts b/stores/update-store.ts index 7eeb87735..3db57e14c 100644 --- a/stores/update-store.ts +++ b/stores/update-store.ts @@ -79,9 +79,19 @@ export interface BannerInfo { advisory: string | null; } +// A status whose `current` isn't the build we're running was cached by a +// previous version and can't be trusted - after an upgrade it still reports +// the old version as outdated (#913). The server drops those too; this is the +// last line of defence against a response cached in the browser. +function isStale(st: UpdateStatus): boolean { + const running = (process.env.NEXT_PUBLIC_APP_VERSION || '').trim(); + return !!running && st.current !== running; +} + export function selectBanner(s: UpdateState): BannerInfo | null { const st = s.status; if (!st || !st.updateAvailable) return null; + if (isStale(st)) return null; if (st.severity === 'none' || st.severity === 'unknown') return null; if (st.severity === 'security') { @@ -114,5 +124,7 @@ export function selectBanner(s: UpdateState): BannerInfo | null { // Used by the admin shield + admin sidebar to show a dot when an update is // available. Mirrors selectBanner's "should we show something" logic. export function selectHasUpdate(s: UpdateState): boolean { - return !!s.status?.updateAvailable && s.status.severity !== 'unknown'; + const st = s.status; + if (!st || !st.updateAvailable || st.severity === 'unknown') return false; + return !isStale(st); }