fix: stop SSH-authenticating hosts during routine status polling (#1347)

This commit is contained in:
Wali Lambert
2026-08-27 06:40:24 +08:00
committed by GitHub
parent 19d4d91eee
commit ef4e66659c
4 changed files with 47 additions and 47 deletions
@@ -1,6 +1,5 @@
import { describe, expect, it } from "vitest"; import { describe, expect, it } from "vitest";
import { import {
needsStatusPollAuthentication,
statusAfterAuthentication, statusAfterAuthentication,
statusAfterReachabilityCheck, statusAfterReachabilityCheck,
} from "./host-status.js"; } from "./host-status.js";
@@ -22,11 +21,4 @@ describe("host availability status", () => {
expect(statusAfterAuthentication(false, "online")).toBe("reachable"); expect(statusAfterAuthentication(false, "online")).toBe("reachable");
expect(statusAfterAuthentication(false, "offline")).toBe("offline"); 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);
});
}); });
-15
View File
@@ -15,18 +15,3 @@ export function statusAfterAuthentication(
if (authenticated) return "online"; if (authenticated) return "online";
return current === "offline" ? "offline" : "reachable"; 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;
}
+9 -22
View File
@@ -72,7 +72,6 @@ import {
} from "./helpers.js"; } from "./helpers.js";
import { import {
type HostStatus, type HostStatus,
needsStatusPollAuthentication,
statusAfterAuthentication, statusAfterAuthentication,
statusAfterReachabilityCheck, statusAfterReachabilityCheck,
} from "./host-status.js"; } from "./host-status.js";
@@ -579,30 +578,18 @@ class PollingManager {
isOnline = await tcpPing(refreshedHost.ip, pingPort, 5000); isOnline = await tcpPing(refreshedHost.ip, pingPort, 5000);
} }
const config = this.pollingConfigs.get(refreshedHost.id); const config = this.pollingConfigs.get(refreshedHost.id);
let authenticated: boolean | undefined; // Routine status polling is TCP-only. Hosts with metrics disabled
if ( // never get a full SSH auth attempt here, since on RADIUS/Duo-backed
isOnline && // devices that fires a live 2FA push every interval with no session
supportsMetrics(refreshedHost) && // to show for it; those hosts settle at "reachable" rather than
needsStatusPollAuthentication( // "online", since metrics collection (the only path that
!!config?.statsConfig.metricsEnabled, // authenticates and can promote a host to "online") never runs for
this.activeViewers.has(refreshedHost.id), // them.
)
) {
try {
await withSshConnection(refreshedHost, async () => undefined);
authenticated = true;
} catch {
authenticated = false;
}
}
const statusEntry: StatusEntry = { const statusEntry: StatusEntry = {
status: status: statusAfterReachabilityCheck(
authenticated === undefined
? statusAfterReachabilityCheck(
isOnline, isOnline,
this.statusStore.get(refreshedHost.id)?.status, this.statusStore.get(refreshedHost.id)?.status,
) ),
: statusAfterAuthentication(authenticated),
lastChecked: new Date().toISOString(), lastChecked: new Date().toISOString(),
}; };
this.statusStore.set(refreshedHost.id, statusEntry); this.statusStore.set(refreshedHost.id, statusEntry);
@@ -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");
});
});