fix: stop the old telemetry timer when an admin changes consent, so one heartbeat runs #1066

lib/__tests__/scheduler-timer-module-copies.test.ts

@@ -0,0 +1,62 @@
+// @vitest-environment node
+import { mkdtempSync, rmSync } from 'node:fs';
+import { tmpdir } from 'node:os';
+import path from 'node:path';
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
+
+// Next loads lib/telemetry and lib/version-check once per bundle. The timer
+// sat in a module-level variable, so the admin route's reschedule() acted on
+// its own copy: the timer instrumentation started kept running next to the
+// new one (two heartbeats), and turning consent off didn't stop it.
+
+vi.mock('@/lib/admin/third-party-mode', () => ({ isNoThirdPartyTrafficOn: async () => false }));
+
+let dir: string;
+
+beforeEach(() => {
+  dir = mkdtempSync(path.join(tmpdir(), 'bw-scheduler-copies-'));
+  vi.stubEnv('TELEMETRY_DATA_DIR', path.join(dir, 'telemetry'));
+  vi.stubEnv('VERSION_CHECK_DATA_DIR', path.join(dir, 'version-check'));
+  vi.stubEnv('BULWARK_TELEMETRY', 'on');
+  vi.stubEnv('BULWARK_UPDATE_CHECK', '');
+  vi.stubEnv('BULWARK_UPDATE_CHECK_URL', 'https://version.example/');
+  vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] });
+});
+
+afterEach(() => {
+  vi.useRealTimers();
+  vi.unstubAllEnvs();
+  vi.resetModules();
+  rmSync(dir, { recursive: true, force: true });
+});
+
+async function copy<T>(load: () => Promise<T>): Promise<T> {
+  vi.resetModules();
+  return load();
+}
+
+describe('scheduler timers across module copies', () => {
+  it('telemetry: a stop from another copy clears the running timer, a restart leaves one', async () => {
+    const instrumentation = await copy(() => import('@/lib/telemetry/sender'));
+    const route = await copy(() => import('@/lib/telemetry/sender'));
+
+    await instrumentation.startScheduler();
+    expect(vi.getTimerCount()).toBe(1);
+    await route.startScheduler();
+    expect(vi.getTimerCount()).toBe(1);
+    await route.stopScheduler();
+    expect(vi.getTimerCount()).toBe(0);
+  });
+
+  it('version check: the same', async () => {
+    const instrumentation = await copy(() => import('@/lib/version-check/sender'));
+    const route = await copy(() => import('@/lib/version-check/sender'));
+
+    await instrumentation.startScheduler();
+    expect(vi.getTimerCount()).toBe(1);
+    await route.startScheduler();
+    expect(vi.getTimerCount()).toBe(1);
+    await route.stopScheduler();
+    expect(vi.getTimerCount()).toBe(0);
+  });
+});

lib/telemetry/sender.ts

@@ -8,7 +8,14 @@ const DAY_MS = 24 * 60 * 60 * 1000;
 const JITTER_MS = 2 * 60 * 60 * 1000; // ± 2 hours
 const FIRST_DELAY_MS = 60 * 60 * 1000;  // 1 hour after consent
 
-let currentTimer: NodeJS.Timeout | null = null;
+// On globalThis: Next loads this module once per bundle (instrumentation,
+// each route), and a reschedule from a route must clear the timer the
+// instrumentation copy started, not a copy of its own.
+const TIMER_KEY = Symbol.for('bulwark.telemetry.timer');
+function timer(): { current: NodeJS.Timeout | null } {
+  const g = globalThis as typeof globalThis & { [TIMER_KEY]?: { current: NodeJS.Timeout | null } };
+  return (g[TIMER_KEY] ??= { current: null });
+}
 
 function jitteredDelay(base: number): number {
   const j = (Math.random() * 2 - 1) * JITTER_MS;
@@ -64,12 +71,14 @@ export async function sendOnce(opts?: { reason?: string }): Promise<{
 }
 
 async function scheduleNext(delayMs: number): Promise<void> {
-  if (currentTimer) clearTimeout(currentTimer);
+  const t = timer();
+  if (t.current) clearTimeout(t.current);
   const at = new Date(Date.now() + delayMs).toISOString();
   await updateState((state) => { state.nextScheduledAt = at; });
-  currentTimer = setTimeout(() => { void tick(); }, delayMs);
+  if (t.current) clearTimeout(t.current);
+  t.current = setTimeout(() => { void tick(); }, delayMs);
   // Don't keep the process alive just for this.
-  currentTimer.unref?.();
+  t.current.unref?.();
 }
 
 async function tick(): Promise<void> {
@@ -104,8 +113,9 @@ export async function startScheduler(): Promise<void> {
 }
 
 export async function stopScheduler(): Promise<void> {
-  if (currentTimer) clearTimeout(currentTimer);
-  currentTimer = null;
+  const t = timer();
+  if (t.current) clearTimeout(t.current);
+  t.current = null;
 }
 
 // Called when consent flips on/off or the collector URL changes via the UI.

lib/version-check/sender.ts

@@ -13,7 +13,14 @@ const JITTER_MS = 5 * 60 * 1000;       // ± 5 min, keeps containers from syncin
 const FIRST_DELAY_MS = 30 * 1000;       // first check ~30s after boot
 const FAILURE_BACKOFF_MS = 15 * 60 * 1000; // after a failed fetch, retry in 15 min
 
-let currentTimer: NodeJS.Timeout | null = null;
+// On globalThis: Next loads this module once per bundle (instrumentation,
+// each route), and a reschedule from a route must clear the timer the
+// instrumentation copy started, not a copy of its own.
+const TIMER_KEY = Symbol.for('bulwark.versionCheck.timer');
+function timer(): { current: NodeJS.Timeout | null } {
+  const g = globalThis as typeof globalThis & { [TIMER_KEY]?: { current: NodeJS.Timeout | null } };
+  return (g[TIMER_KEY] ??= { current: null });
+}
 
 /** What checkOnce reports while NO_THIRD_PARTY_TRAFFIC is on. */
 export const NO_THIRD_PARTY_ERROR = 'off: no third-party traffic';
@@ -62,12 +69,14 @@ export async function checkOnce(opts?: { reason?: string }): Promise<{
 }
 
 async function scheduleNext(delayMs: number): Promise<void> {
-  if (currentTimer) clearTimeout(currentTimer);
+  const t = timer();
+  if (t.current) clearTimeout(t.current);
   const at = new Date(Date.now() + delayMs).toISOString();
   await updateState((state) => { state.nextScheduledAt = at; });
-  currentTimer = setTimeout(() => { void tick(); }, delayMs);
+  if (t.current) clearTimeout(t.current);
+  t.current = setTimeout(() => { void tick(); }, delayMs);
   // Don't keep the process alive just for this.
-  currentTimer.unref?.();
+  t.current.unref?.();
 }
 
 async function tick(): Promise<void> {
@@ -122,6 +131,7 @@ export async function startScheduler(): Promise<void> {
 }
 
 export async function stopScheduler(): Promise<void> {
-  if (currentTimer) clearTimeout(currentTimer);
-  currentTimer = null;
+  const t = timer();
+  if (t.current) clearTimeout(t.current);
+  t.current = null;
 }