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.
This commit is contained in:
gorkem-bwl committed 2026-08-25 18:14:25 -04:00
1 parent f74d1d53f6
commit 25a7ac4877
2 files changed
+26 -2

No files matched your search

@@ -11,9 +11,14 @@ const DEFAULT_HTTPS_PORT = 443;
export const fetchMonitorCertificate = async (checker: SSLCheckerType, monitor: Monitor): Promise<SSLDetails> => {
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;
// 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.
// 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 });
if (cert?.validTo === null || cert?.validTo === undefined) {