From a672f7604d6ca5f29983c52c46fa1bca1134b142 Mon Sep 17 00:00:00 2001 From: gorkem-bwl Date: Thu, 20 Aug 2026 12:37:37 -0400 Subject: [PATCH] fix(monitors): bound certificate probe and gate client on scheme Addresses review feedback on #3848. Timeout: the probe now dials a user-defined port, so it can reach a service that accepts the connection but never completes the TLS handshake, hanging the request indefinitely. ssl-checker already wires a "timeout" listener that rejects with "Timed Out", but its defaults set no timeout, so it never fired. Passing one activates it. The bundled type declaration is a hand-maintained subset of https.RequestOptions and omitted "timeout"; it is added there so the call typechecks against an option the library already supports. Client gating: "http" is a single monitor type covering both schemes, so an http:// monitor passed the existing type check and requested a certificate anyway. With the server now rejecting those, the details page would surface a 400 on every visit. The scheme is gated client side to match, and the stat box falls back to "N/A" as before. Test comment removed as requested; the timeout is covered by a new assertion in the same file. --- client/src/Pages/Uptime/Details/index.tsx | 19 ++++++++++++++++--- server/src/api/controllers/controllerUtils.ts | 5 ++++- server/src/types/ssl-checker.d.ts | 3 +++ .../unit/utils/monitorCertificate.test.ts | 12 ++++++++---- 4 files changed, 31 insertions(+), 8 deletions(-) diff --git a/client/src/Pages/Uptime/Details/index.tsx b/client/src/Pages/Uptime/Details/index.tsx index bcf7177ac..2c2309f1f 100644 --- a/client/src/Pages/Uptime/Details/index.tsx +++ b/client/src/Pages/Uptime/Details/index.tsx @@ -34,6 +34,17 @@ import { Typography } from "@mui/material"; const certificateDateFormat = "MMM D, YYYY h A"; +const isHttpsUrl = (url?: string): boolean => { + if (!url) { + return false; + } + try { + return new URL(url).protocol === "https:"; + } catch { + return false; + } +}; + interface CertificateResponse { certificateDate: string; } @@ -80,13 +91,15 @@ const UptimeDetailsPage = () => { const monitor = monitorData?.monitor; const monitorStats = monitorDetailsData?.monitorStats ?? null; - // Certificate fetch - only for HTTP monitors + // Certificate fetch - only for monitors served over HTTPS. A "http" type + // monitor may still have an http:// URL, which serves no certificate, so the + // scheme is gated here to match the server rather than requesting a 400. const certificateUrl = useMemo(() => { - if (!monitorId || monitor?.type !== "http") { + if (!monitorId || monitor?.type !== "http" || !isHttpsUrl(monitor?.url)) { return null; } return `/monitors/certificate/${monitorId}`; - }, [monitorId, monitor?.type]); + }, [monitorId, monitor?.type, monitor?.url]); const { data: certificateData } = useGet( certificateUrl, diff --git a/server/src/api/controllers/controllerUtils.ts b/server/src/api/controllers/controllerUtils.ts index 15dfc4832..fe057ea24 100755 --- a/server/src/api/controllers/controllerUtils.ts +++ b/server/src/api/controllers/controllerUtils.ts @@ -8,6 +8,9 @@ type SSLCheckerType = typeof sslChecker; type WhoisModule = typeof whoiser; const DEFAULT_HTTPS_PORT = 443; +// Ports are user-defined, so a probe can dial something that accepts the +// connection and never completes the handshake. Bound the wait explicitly. +const CERTIFICATE_TIMEOUT_MS = 10_000; export const fetchMonitorCertificate = async (checker: SSLCheckerType, monitor: Monitor): Promise => { const monitorUrl = new URL(monitor.url); @@ -20,7 +23,7 @@ export const fetchMonitorCertificate = async (checker: SSLCheckerType, monitor: // URL.port is "" for the scheme's default port, so fall back to 443; without // this the certificate of a monitor on a non-standard port was probed on 443. const port = monitorUrl.port === "" ? DEFAULT_HTTPS_PORT : Number(monitorUrl.port); - const cert = await checker(hostname, { port }); + const cert = await checker(hostname, { port, timeout: CERTIFICATE_TIMEOUT_MS }); if (cert?.validTo === null || cert?.validTo === undefined) { throw new Error("Certificate not found"); } diff --git a/server/src/types/ssl-checker.d.ts b/server/src/types/ssl-checker.d.ts index bcbc39858..829668737 100644 --- a/server/src/types/ssl-checker.d.ts +++ b/server/src/types/ssl-checker.d.ts @@ -12,6 +12,9 @@ declare module "ssl-checker" { export interface SSLOptions { method?: "GET" | "HEAD"; port?: number; + // Inherited from https.RequestOptions; ssl-checker rejects with "Timed Out" + // when the socket exceeds it. + timeout?: number; agent?: Agent; rejectUnauthorized?: boolean; validateSubjectAltName?: boolean; diff --git a/server/test/unit/utils/monitorCertificate.test.ts b/server/test/unit/utils/monitorCertificate.test.ts index 16240beb2..c73f6a3eb 100644 --- a/server/test/unit/utils/monitorCertificate.test.ts +++ b/server/test/unit/utils/monitorCertificate.test.ts @@ -3,10 +3,6 @@ import { fetchMonitorCertificate } from "../../../src/api/controllers/controller import type { Monitor } from "../../../src/domain/monitors/monitor.type.ts"; import type { SSLDetails, SSLOptions } from "ssl-checker"; -// Regression cover for #3646: the certificate probe parsed the monitor URL but -// dropped its port, so a monitor on a non-standard port had its certificate -// checked on 443 and surfaced as "N/A" in the UI. - const VALID_CERT: SSLDetails = { daysRemaining: 30, valid: true, @@ -54,6 +50,14 @@ describe("fetchMonitorCertificate", () => { expect(calls[0].options?.port).toBe(443); }); + it("bounds the probe with a timeout so a user-defined port cannot hang it", async () => { + const { checker, calls } = spyChecker(); + + await fetchMonitorCertificate(checker, monitorWithUrl("https://example.com:54321")); + + expect(calls[0].options?.timeout).toBe(10_000); + }); + it("never passes NaN as the port", async () => { const { checker, calls } = spyChecker();