From 9f6b546ffe19736c72d4ccedeeee16cbe9180d8b Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Fri, 25 Sep 2026 19:32:35 +0200 Subject: [PATCH] fix: stop reporting a proxy's version as the Stalwart version in telemetry The heartbeat's `stalwart_version` came from the `Server` header of `/.well-known/jmap`. Stalwart never sends one (0.16.0 through 1.0), so the field was null, or the version of a reverse proxy in front of Stalwart. The probe is gone: the version is reported only when `STALWART_VERSION` is set. --- .env.example | 6 +-- lib/__tests__/telemetry-payload.test.ts | 61 +++++++++++++++++++++++++ lib/telemetry/payload.ts | 58 ++++------------------- 3 files changed, 72 insertions(+), 53 deletions(-) create mode 100644 lib/__tests__/telemetry-payload.test.ts diff --git a/.env.example b/.env.example index 143a50a0a..649e542de 100644 --- a/.env.example +++ b/.env.example @@ -180,9 +180,9 @@ JMAP_SERVER_URL=https://your-jmap-server.com # guard; only useful when running a collector locally during development. # BULWARK_TELEMETRY_ALLOW_PRIVATE=1 -# Report a fixed Stalwart version instead of probing the JMAP server's Server -# header. Useful when a proxy strips that header. -# STALWART_VERSION=0.16.0 +# Stalwart version to report in heartbeats. Stalwart does not publish its +# version, so the heartbeat leaves it empty unless it is set here. +# STALWART_VERSION=0.16.23 # ============================================================================= # Server Listen Address diff --git a/lib/__tests__/telemetry-payload.test.ts b/lib/__tests__/telemetry-payload.test.ts new file mode 100644 index 000000000..2cc9b138c --- /dev/null +++ b/lib/__tests__/telemetry-payload.test.ts @@ -0,0 +1,61 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +vi.mock('@/lib/admin/config-manager', () => ({ + configManager: { + ensureLoaded: async () => {}, + getPolicy: () => ({ features: {} }), + getAll: () => ({}), + get: (key: string, fallback: unknown) => (key === 'jmapServerUrl' ? 'https://mail.example.org' : fallback), + }, +})); +vi.mock('@/lib/telemetry/state', () => ({ getInstanceId: async () => 'instance-1' })); +vi.mock('@/lib/telemetry/login-tracker', () => ({ getLoginCounts: async () => ({ total: 3, active7d: 1 }) })); +vi.mock('@/lib/admin/plugin-registry', () => ({ + getPluginRegistry: async () => ({ plugins: [] }), + getThemeRegistry: async () => ({ themes: [] }), +})); +// A probe, if one were made, would get past the SSRF guard. +vi.mock('@/lib/telemetry/endpoint-guard', () => ({ resolveEndpointAllowed: async () => ({ ok: true }) })); + +import { buildPayload } from '@/lib/telemetry/payload'; + +// Stalwart (0.16.0 through 1.0) sends no Server header and publishes no +// version a client could read. The heartbeat used to read the Server header of +// /.well-known/jmap anyway: null against Stalwart itself, and the proxy's +// version (nginx/1.25.3 -> "1.25.3") behind a reverse proxy. +describe('telemetry stalwart_version', () => { + let fetchSpy: ReturnType; + + beforeEach(() => { + delete process.env.STALWART_VERSION; + fetchSpy = vi.spyOn(globalThis, 'fetch').mockResolvedValue( + new Response(null, { status: 307, headers: { server: 'nginx/1.25.3', location: '/jmap/session' } }), + ); + }); + + afterEach(() => { + delete process.env.STALWART_VERSION; + vi.restoreAllMocks(); + }); + + it('is empty without STALWART_VERSION, on 0.16 and 1.0 alike, and probes nothing', async () => { + const payload = await buildPayload(); + + expect(payload.stalwart_version).toBeNull(); + expect(fetchSpy).not.toHaveBeenCalled(); + }); + + it('reports the version the admin set', async () => { + process.env.STALWART_VERSION = ' 0.16.23 '; + + const payload = await buildPayload(); + + expect(payload.stalwart_version).toBe('0.16.23'); + expect(fetchSpy).not.toHaveBeenCalled(); + }); + + it('treats an empty STALWART_VERSION as unset', async () => { + process.env.STALWART_VERSION = ''; + expect((await buildPayload()).stalwart_version).toBeNull(); + }); +}); diff --git a/lib/telemetry/payload.ts b/lib/telemetry/payload.ts index e4d033b1f..5928d41cd 100644 --- a/lib/telemetry/payload.ts +++ b/lib/telemetry/payload.ts @@ -1,8 +1,6 @@ import { readFileSync } from 'node:fs'; import path from 'node:path'; import { configManager } from '@/lib/admin/config-manager'; -import { logger } from '@/lib/logger'; -import { resolveEndpointAllowed } from './endpoint-guard'; import { getInstanceId } from './state'; import { getLoginCounts } from './login-tracker'; import type { @@ -74,53 +72,13 @@ async function readFeatures(): Promise { }; } -const STALWART_VERSION_TTL_MS = 24 * 60 * 60 * 1000; -let stalwartVersionCache: { version: string | null; fetchedAt: number } | null = null; - -// Stalwart returns the version in the Server response header -// (e.g. "Stalwart Mail Server v0.16.0"). The /.well-known/jmap endpoint -// requires auth, but the header is on the 401 response too, so an -// unauthenticated GET is enough. Cached for a day to avoid hammering -// the JMAP server on every payload preview. -async function detectStalwartVersion(): Promise { - if (process.env.STALWART_VERSION) return process.env.STALWART_VERSION; - if (stalwartVersionCache && - Date.now() - stalwartVersionCache.fetchedAt < STALWART_VERSION_TTL_MS) { - return stalwartVersionCache.version; - } - await configManager.ensureLoaded(); - const serverUrl = configManager.get('jmapServerUrl', '').trim(); - if (!serverUrl) { - stalwartVersionCache = { version: null, fetchedAt: Date.now() }; - return null; - } - const wellKnown = `${serverUrl.replace(/\/+$/, '')}/.well-known/jmap`; - // Reuse the SSRF guard so a misconfigured JMAP_SERVER_URL pointing at an - // internal host doesn't get probed from telemetry context either. - const guard = await resolveEndpointAllowed(wellKnown); - if (!guard.ok) { - stalwartVersionCache = { version: null, fetchedAt: Date.now() }; - return null; - } - try { - const res = await fetch(wellKnown, { - method: 'GET', - signal: AbortSignal.timeout(3000), - // See sender.ts: the guard vetted this URL, not wherever it redirects. - redirect: 'manual', - }); - const server = res.headers.get('server') ?? ''; - const m = server.match(/(\d+\.\d+\.\d+(?:-[\w.]+)?)/); - const version = m?.[1] ?? null; - stalwartVersionCache = { version, fetchedAt: Date.now() }; - return version; - } catch (err) { - logger.debug?.('telemetry: stalwart version probe failed', { - error: err instanceof Error ? err.message : String(err), - }); - stalwartVersionCache = { version: null, fetchedAt: Date.now() }; - return null; - } +// Only what the admin sets in STALWART_VERSION. Stalwart publishes no version +// a client could read: it sends no Server header (0.16.0 through 1.0), and +// the version it names elsewhere is a fixed "1.0.0" even on 0.16. Reading the +// Server header of /.well-known/jmap reported null, or whatever version a +// reverse proxy in front of Stalwart put there. +function stalwartVersion(): string | null { + return process.env.STALWART_VERSION?.trim() || null; } // Account counts come from the local login tracker, which records a per- @@ -147,7 +105,7 @@ export async function buildPayload(): Promise { const features = await readFeatures(); const accounts = await getLoginCounts(); const exts = await countExtensions(); - const stalwart_version = await detectStalwartVersion(); + const stalwart_version = stalwartVersion(); const uptime_days = Math.min( 365, Math.floor((Date.now() - processStartedAt) / 86_400_000),