Compare commits

...
Author SHA1 Message Date
gorkem-bwl a672f7604d 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.
2026-08-25 18:14:25 -04:00
gorkem-bwl 25a7ac4877 fix(monitors): reject certificate lookups for plain-HTTP monitors
Passing the monitor's own port to ssl-checker exposed an adjacent gap found
in review: a plain-HTTP monitor on a non-default port, such as
http://host:8080, now had a TLS handshake attempted against a port that
serves no TLS. That surfaced to the client as a 500 carrying a raw OpenSSL
error ("packet length too long") instead of the previous silent "N/A".

The endpoint never checked the scheme — it was already wrong for http://
monitors, just quietly so, since every such lookup went to port 443. Guard
on the protocol and fail with a 400 before opening a socket.

Adds tests for both plain-HTTP cases (default and custom port), each
asserting the checker is never invoked. Verified by removing the guard:
both fail without it.
2026-08-25 18:14:25 -04:00
gorkem-bwl f74d1d53f6 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
2026-08-25 18:14:25 -04:00
4 changed files with 134 additions and 4 deletions

No files matched your search

+16 -3
View File
@@ -34,6 +34,17 @@ import { Typography } from "@mui/material";
const certificateDateFormat = "MMM D, YYYY h A"; 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 { interface CertificateResponse {
certificateDate: string; certificateDate: string;
} }
@@ -80,13 +91,15 @@ const UptimeDetailsPage = () => {
const monitor = monitorData?.monitor; const monitor = monitorData?.monitor;
const monitorStats = monitorDetailsData?.monitorStats ?? null; 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(() => { const certificateUrl = useMemo(() => {
if (!monitorId || monitor?.type !== "http") { if (!monitorId || monitor?.type !== "http" || !isHttpsUrl(monitor?.url)) {
return null; return null;
} }
return `/monitors/certificate/${monitorId}`; return `/monitors/certificate/${monitorId}`;
}, [monitorId, monitor?.type]); }, [monitorId, monitor?.type, monitor?.url]);
const { data: certificateData } = useGet<CertificateResponse>( const { data: certificateData } = useGet<CertificateResponse>(
certificateUrl, certificateUrl,
+14 -1
View File
@@ -7,10 +7,23 @@ import { parse as parseDomain } from "tldts";
type SSLCheckerType = typeof sslChecker; type SSLCheckerType = typeof sslChecker;
type WhoisModule = typeof whoiser; 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<SSLDetails> => { export const fetchMonitorCertificate = async (checker: SSLCheckerType, monitor: Monitor): Promise<SSLDetails> => {
const monitorUrl = new URL(monitor.url); const monitorUrl = new URL(monitor.url);
// A plain-HTTP monitor serves no certificate. Reject before dialing, so the
// caller gets this instead of a raw OpenSSL handshake error from the socket.
if (monitorUrl.protocol !== "https:") {
throw new AppError({ message: "Certificate is only available for HTTPS monitors", status: 400 });
}
const hostname = monitorUrl.hostname; const hostname = monitorUrl.hostname;
const cert = await checker(hostname); // 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, timeout: CERTIFICATE_TIMEOUT_MS });
if (cert?.validTo === null || cert?.validTo === undefined) { if (cert?.validTo === null || cert?.validTo === undefined) {
throw new Error("Certificate not found"); throw new Error("Certificate not found");
} }
+3
View File
@@ -12,6 +12,9 @@ declare module "ssl-checker" {
export interface SSLOptions { export interface SSLOptions {
method?: "GET" | "HEAD"; method?: "GET" | "HEAD";
port?: number; port?: number;
// Inherited from https.RequestOptions; ssl-checker rejects with "Timed Out"
// when the socket exceeds it.
timeout?: number;
agent?: Agent; agent?: Agent;
rejectUnauthorized?: boolean; rejectUnauthorized?: boolean;
validateSubjectAltName?: boolean; validateSubjectAltName?: boolean;
@@ -0,0 +1,101 @@
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";
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("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();
await fetchMonitorCertificate(checker, monitorWithUrl("https://example.com"));
expect(Number.isNaN(calls[0].options?.port)).toBe(false);
});
it("rejects a plain-HTTP monitor without opening a TLS connection", async () => {
const { checker, calls } = spyChecker();
// Probing a non-TLS port would surface a raw OpenSSL handshake error.
await expect(fetchMonitorCertificate(checker, monitorWithUrl("http://example.com:8080"))).rejects.toThrow(
"Certificate is only available for HTTPS monitors"
);
expect(calls).toHaveLength(0);
});
it("rejects a plain-HTTP monitor on the default port", async () => {
const { checker, calls } = spyChecker();
await expect(fetchMonitorCertificate(checker, monitorWithUrl("http://example.com"))).rejects.toThrow(
"Certificate is only available for HTTPS monitors"
);
expect(calls).toHaveLength(0);
});
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");
});
});