fix: check telemetry targets at connect time and fail closed

The telemetry sender and the Stalwart version probe checked the target
with their own DNS lookup and then fetched with a plain `fetch`, which
resolved the name again: a rebinding answer between the two reached an
internal host, and a lookup that failed at check time passed the check
outright. Both now connect through `fetchPublicUrl`, which validates
the address inside the socket's own lookup and fails when the lookup
does. The `BULWARK_TELEMETRY_ALLOW_PRIVATE` dev bypass keeps the plain
fetch.

lib/__tests__/telemetry-fetch-target.test.ts

@@ -0,0 +1,42 @@
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
+
+// The telemetry sender checked the endpoint with its own DNS lookup, then
+// fetched with a plain fetch that resolved again (a rebinding window), and
+// the check passed outright when that first lookup failed.
+
+const guardedFetch = vi.fn();
+vi.mock('@/lib/security/url-guard', async (importOriginal) => {
+  const actual = await importOriginal<typeof import('@/lib/security/url-guard')>();
+  return { ...actual, fetchPublicUrl: (...args: unknown[]) => guardedFetch(...args) };
+});
+
+const plainFetch = vi.fn();
+
+beforeEach(() => {
+  guardedFetch.mockReset().mockResolvedValue(new Response('ok'));
+  plainFetch.mockReset().mockResolvedValue(new Response('ok'));
+  vi.stubGlobal('fetch', plainFetch);
+  delete process.env.BULWARK_TELEMETRY_ALLOW_PRIVATE;
+});
+
+afterEach(() => {
+  vi.unstubAllGlobals();
+  vi.resetModules();
+});
+
+describe('fetchTelemetryTarget', () => {
+  it('connects through the rebinding-safe fetch', async () => {
+    const { fetchTelemetryTarget } = await import('@/lib/telemetry/endpoint-guard');
+    await fetchTelemetryTarget('https://telemetry.example.org/', { method: 'POST', body: '{}' });
+    expect(guardedFetch).toHaveBeenCalledWith('https://telemetry.example.org/', { method: 'POST', body: '{}' });
+    expect(plainFetch).not.toHaveBeenCalled();
+  });
+
+  it('uses the plain fetch only with the dev bypass, still without redirects', async () => {
+    process.env.BULWARK_TELEMETRY_ALLOW_PRIVATE = '1';
+    const { fetchTelemetryTarget } = await import('@/lib/telemetry/endpoint-guard');
+    await fetchTelemetryTarget('http://localhost:4000/', { method: 'GET' });
+    expect(plainFetch).toHaveBeenCalledWith('http://localhost:4000/', { method: 'GET', redirect: 'manual' });
+    expect(guardedFetch).not.toHaveBeenCalled();
+  });
+});

lib/telemetry/endpoint-guard.ts

@@ -1,6 +1,7 @@
 import { lookup } from 'node:dns/promises';
 import { isIP } from 'node:net';
 import { isPrivateAddress } from '@/lib/security/ip-ranges';
+import { fetchPublicUrl } from '@/lib/security/url-guard';
 
 export { isPrivateAddress, parseV6Groups } from '@/lib/security/ip-ranges';
 
@@ -76,7 +77,31 @@ export async function resolveEndpointAllowed(raw: string): Promise<EndpointCheck
     // Don't block on transient DNS failures - fetch will fail loudly anyway,
     // and we don't want to lock admins out of their config when the resolver
     // is flaky. The literal-IP check above already covers the direct-attack
-    // case.
+    // case, and fetchTelemetryTarget re-checks at connect time, failing
+    // closed.
     return { ok: true };
   }
 }
+
+export interface TelemetryFetchInit {
+  method: 'GET' | 'POST';
+  headers?: Record<string, string>;
+  body?: string;
+  signal?: AbortSignal;
+}
+
+/**
+ * Fetch a telemetry target (the collector, the JMAP server's version
+ * probe). resolveEndpointAllowed runs its own DNS lookup, so on its own it
+ * is check-then-fetch: the socket resolves again and a rebinding answer, or
+ * a lookup that failed at check time, lands wherever it points. Unless the
+ * dev bypass is on, the address is checked inside the socket's own lookup
+ * instead, and a lookup failure fails the request. Redirects are never
+ * followed.
+ */
+export async function fetchTelemetryTarget(url: string, init: TelemetryFetchInit): Promise<Response> {
+  if (bypassEnabled()) {
+    return fetch(url, { ...init, redirect: 'manual' });
+  }
+  return (await fetchPublicUrl(url, init)) as unknown as Response;
+}

lib/telemetry/payload.ts

@@ -2,7 +2,7 @@ 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 { fetchTelemetryTarget, resolveEndpointAllowed } from './endpoint-guard';
 import { getInstanceId } from './state';
 import { getLoginCounts } from './login-tracker';
 import type {
@@ -103,11 +103,9 @@ async function detectStalwartVersion(): Promise<string | null> {
     return null;
   }
   try {
-    const res = await fetch(wellKnown, {
+    const res = await fetchTelemetryTarget(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.]+)?)/);

lib/telemetry/sender.ts

@@ -1,7 +1,7 @@
 import { logger } from '@/lib/logger';
 import { effectiveConsent, endpointEnabled, loadState, saveState } from './state';
 import { buildPayload } from './payload';
-import { resolveEndpointAllowed } from './endpoint-guard';
+import { fetchTelemetryTarget, resolveEndpointAllowed } from './endpoint-guard';
 import { DEFAULT_ENDPOINT } from './types';
 
 const DAY_MS = 24 * 60 * 60 * 1000;
@@ -35,14 +35,13 @@ export async function sendOnce(opts?: { reason?: string }): Promise<{
 
   const payload = await buildPayload();
   try {
-    const res = await fetch(endpoint, {
+    // Never follows redirects: the endpoint guard only vetted the URL we
+    // were given, so a 3xx to an internal host would bypass it.
+    const res = await fetchTelemetryTarget(endpoint, {
       method: 'POST',
       headers: { 'content-type': 'application/json' },
       body: JSON.stringify(payload),
       signal: AbortSignal.timeout(5000),
-      // Never follow redirects: the endpoint guard only vetted the URL we
-      // were given, so a 3xx to an internal host would bypass it.
-      redirect: 'manual',
     });
     const ok = res.ok;
     if (ok) {