From f74d1d53f69d4cc19d3592b9a4a5fa39e9bcdc13 Mon Sep 17 00:00:00 2001 From: gorkem-bwl Date: Tue, 18 Aug 2026 02:41:06 -0400 Subject: [PATCH] fix(monitors): check TLS certificate on the monitor's own port fetchMonitorCertificate parsed the monitor URL but passed only the hostname to ssl-checker, which then defaulted to 443. A monitor on a non-standard port such as https://host:54321 therefore had the certificate of whatever answered on 443 inspected instead of its own, surfacing as "N/A" in the certificate expiry box when nothing was listening there. URL.port is an empty string for default ports, so it is mapped to 443 explicitly rather than passed through Number(), which would have yielded NaN. ssl-checker already accepts the port via SSLOptions, so no dependency or signature change is needed. Adds a regression suite covering the reported case (non-standard port), the default-port fallback, an explicit :443, and the NaN guard. Verified by reverting the fix: the three port assertions fail without it. Fixes #3646 --- server/src/api/controllers/controllerUtils.ts | 7 +- .../unit/utils/monitorCertificate.test.ts | 78 +++++++++++++++++++ 2 files changed, 84 insertions(+), 1 deletion(-) create mode 100644 server/test/unit/utils/monitorCertificate.test.ts diff --git a/server/src/api/controllers/controllerUtils.ts b/server/src/api/controllers/controllerUtils.ts index 608c39d57..6b86d5d5b 100755 --- a/server/src/api/controllers/controllerUtils.ts +++ b/server/src/api/controllers/controllerUtils.ts @@ -7,10 +7,15 @@ import { parse as parseDomain } from "tldts"; type SSLCheckerType = typeof sslChecker; type WhoisModule = typeof whoiser; +const DEFAULT_HTTPS_PORT = 443; + export const fetchMonitorCertificate = async (checker: SSLCheckerType, monitor: Monitor): Promise => { const monitorUrl = new URL(monitor.url); const hostname = monitorUrl.hostname; - const cert = await checker(hostname); + // URL.port is "" for default ports, 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 }); if (cert?.validTo === null || cert?.validTo === undefined) { throw new Error("Certificate not found"); } diff --git a/server/test/unit/utils/monitorCertificate.test.ts b/server/test/unit/utils/monitorCertificate.test.ts new file mode 100644 index 000000000..3838d526d --- /dev/null +++ b/server/test/unit/utils/monitorCertificate.test.ts @@ -0,0 +1,78 @@ +import { describe, expect, it } from "@jest/globals"; +import { fetchMonitorCertificate } from "../../../src/api/controllers/controllerUtils.ts"; +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, + validFrom: "2026-01-01T00:00:00.000Z", + validTo: "2026-12-31T00:00:00.000Z", + validFor: ["example.com"], +}; + +/** Records the arguments the checker was called with. */ +const spyChecker = (cert: SSLDetails = VALID_CERT) => { + const calls: Array<{ hostname: string; options?: SSLOptions }> = []; + const checker = (async (hostname: string, options?: SSLOptions) => { + calls.push({ hostname, options }); + return cert; + }) as unknown as typeof import("ssl-checker").default; + return { checker, calls }; +}; + +const monitorWithUrl = (url: string) => ({ url }) as Monitor; + +describe("fetchMonitorCertificate", () => { + it("probes the port named in the monitor URL", async () => { + const { checker, calls } = spyChecker(); + + await fetchMonitorCertificate(checker, monitorWithUrl("https://example.com:54321")); + + expect(calls).toHaveLength(1); + expect(calls[0].hostname).toBe("example.com"); + expect(calls[0].options?.port).toBe(54321); + }); + + it("falls back to 443 when the URL names no port", async () => { + const { checker, calls } = spyChecker(); + + await fetchMonitorCertificate(checker, monitorWithUrl("https://example.com")); + + expect(calls[0].options?.port).toBe(443); + }); + + it("passes an explicit 443 through unchanged", async () => { + const { checker, calls } = spyChecker(); + + await fetchMonitorCertificate(checker, monitorWithUrl("https://example.com:443")); + + expect(calls[0].options?.port).toBe(443); + }); + + it("never passes NaN as the port", async () => { + const { checker, calls } = spyChecker(); + + await fetchMonitorCertificate(checker, monitorWithUrl("https://example.com")); + + expect(Number.isNaN(calls[0].options?.port)).toBe(false); + }); + + it("returns the certificate details it received", async () => { + const { checker } = spyChecker(); + + const cert = await fetchMonitorCertificate(checker, monitorWithUrl("https://example.com:8443")); + + expect(cert.validTo).toBe(VALID_CERT.validTo); + }); + + it("throws when the certificate has no expiry", async () => { + const { checker } = spyChecker({ ...VALID_CERT, validTo: undefined as unknown as string }); + + await expect(fetchMonitorCertificate(checker, monitorWithUrl("https://example.com:8443"))).rejects.toThrow("Certificate not found"); + }); +});