From ef4e66659cbaf9c7a9faaa5c8f7cf0462684febd Mon Sep 17 00:00:00 2001 From: Wali Lambert <98448225+YatoVoid@users.noreply.github.com> Date: Wed, 26 Aug 2026 17:40:24 -0500 Subject: [PATCH] fix: stop SSH-authenticating hosts during routine status polling (#1347) --- src/backend/hosts/metrics/host-status.test.ts | 8 ----- src/backend/hosts/metrics/host-status.ts | 15 -------- src/backend/hosts/metrics/index.ts | 35 ++++++------------ .../hosts/metrics/status-poll-auth.test.ts | 36 +++++++++++++++++++ 4 files changed, 47 insertions(+), 47 deletions(-) create mode 100644 src/backend/tests/hosts/metrics/status-poll-auth.test.ts diff --git a/src/backend/hosts/metrics/host-status.test.ts b/src/backend/hosts/metrics/host-status.test.ts index 55ab03c8..f41c4544 100644 --- a/src/backend/hosts/metrics/host-status.test.ts +++ b/src/backend/hosts/metrics/host-status.test.ts @@ -1,6 +1,5 @@ import { describe, expect, it } from "vitest"; import { - needsStatusPollAuthentication, statusAfterAuthentication, statusAfterReachabilityCheck, } from "./host-status.js"; @@ -22,11 +21,4 @@ describe("host availability status", () => { expect(statusAfterAuthentication(false, "online")).toBe("reachable"); expect(statusAfterAuthentication(false, "offline")).toBe("offline"); }); - - it("authenticates in the status probe unless a metrics poll will do it", () => { - expect(needsStatusPollAuthentication(false, false)).toBe(true); - expect(needsStatusPollAuthentication(false, true)).toBe(true); - expect(needsStatusPollAuthentication(true, false)).toBe(true); - expect(needsStatusPollAuthentication(true, true)).toBe(false); - }); }); diff --git a/src/backend/hosts/metrics/host-status.ts b/src/backend/hosts/metrics/host-status.ts index 7c1a39e5..3c5b3914 100644 --- a/src/backend/hosts/metrics/host-status.ts +++ b/src/backend/hosts/metrics/host-status.ts @@ -15,18 +15,3 @@ export function statusAfterAuthentication( if (authenticated) return "online"; return current === "offline" ? "offline" : "reachable"; } - -/** - * Whether the cheap status probe must also authenticate over SSH. - * - * "online" means authenticated, and normally the metrics poll proves that. - * That poll only runs while someone is viewing the host, so an unwatched host - * with metrics enabled would otherwise never leave "reachable" - while a host - * with metrics disabled, whose probe always authenticates, shows online. - */ -export function needsStatusPollAuthentication( - metricsEnabled: boolean, - hasViewers: boolean, -): boolean { - return !metricsEnabled || !hasViewers; -} diff --git a/src/backend/hosts/metrics/index.ts b/src/backend/hosts/metrics/index.ts index 9368c9ca..a5d49a47 100644 --- a/src/backend/hosts/metrics/index.ts +++ b/src/backend/hosts/metrics/index.ts @@ -72,7 +72,6 @@ import { } from "./helpers.js"; import { type HostStatus, - needsStatusPollAuthentication, statusAfterAuthentication, statusAfterReachabilityCheck, } from "./host-status.js"; @@ -579,30 +578,18 @@ class PollingManager { isOnline = await tcpPing(refreshedHost.ip, pingPort, 5000); } const config = this.pollingConfigs.get(refreshedHost.id); - let authenticated: boolean | undefined; - if ( - isOnline && - supportsMetrics(refreshedHost) && - needsStatusPollAuthentication( - !!config?.statsConfig.metricsEnabled, - this.activeViewers.has(refreshedHost.id), - ) - ) { - try { - await withSshConnection(refreshedHost, async () => undefined); - authenticated = true; - } catch { - authenticated = false; - } - } + // Routine status polling is TCP-only. Hosts with metrics disabled + // never get a full SSH auth attempt here, since on RADIUS/Duo-backed + // devices that fires a live 2FA push every interval with no session + // to show for it; those hosts settle at "reachable" rather than + // "online", since metrics collection (the only path that + // authenticates and can promote a host to "online") never runs for + // them. const statusEntry: StatusEntry = { - status: - authenticated === undefined - ? statusAfterReachabilityCheck( - isOnline, - this.statusStore.get(refreshedHost.id)?.status, - ) - : statusAfterAuthentication(authenticated), + status: statusAfterReachabilityCheck( + isOnline, + this.statusStore.get(refreshedHost.id)?.status, + ), lastChecked: new Date().toISOString(), }; this.statusStore.set(refreshedHost.id, statusEntry); diff --git a/src/backend/tests/hosts/metrics/status-poll-auth.test.ts b/src/backend/tests/hosts/metrics/status-poll-auth.test.ts new file mode 100644 index 00000000..a6c9fdcc --- /dev/null +++ b/src/backend/tests/hosts/metrics/status-poll-auth.test.ts @@ -0,0 +1,36 @@ +import { describe, it, expect } from "vitest"; +import fs from "fs"; +import path from "path"; + +// Regression guard for Support#1216: routine status polling on a host with +// metrics disabled ran a full SSH authentication attempt on every interval, +// which fires a live 2FA push each cycle on RADIUS/Duo-backed devices with +// no active session. Spinning up the full metrics-service Express app (DB, +// SSH clients, timers, polling managers, etc.) just to drive pollHostStatus +// through two intervals is out of scope, so this asserts directly against +// the source that the status-poll path never calls into SSH auth, the same +// way login-alert-route-order.test.ts guards route registration order. +describe("metrics service status polling", () => { + it("never authenticates over SSH from pollHostStatus, on the first poll or any later one", () => { + const source = fs.readFileSync( + path.resolve(__dirname, "../../../hosts/metrics/index.ts"), + "utf8", + ); + + const pollHostStatusStart = source.indexOf("private async pollHostStatus("); + const pollHostMetricsStart = source.indexOf( + "private async pollHostMetrics(", + ); + + expect(pollHostStatusStart).toBeGreaterThan(-1); + expect(pollHostMetricsStart).toBeGreaterThan(pollHostStatusStart); + + const pollHostStatusBody = source.slice( + pollHostStatusStart, + pollHostMetricsStart, + ); + + expect(pollHostStatusBody).not.toContain("withSshConnection"); + expect(pollHostStatusBody).not.toContain("statusAfterAuthentication"); + }); +});