From 25a7ac48770674ceff7a27141782874cc8cdd635 Mon Sep 17 00:00:00 2001
From: gorkem-bwl <gorkem.cetin@bluewavelabs.ca>
Date: Tue, 18 Aug 2026 02:47:10 -0400
Subject: [PATCH] fix(monitors): reject certificate lookups for plain-HTTP
 monitors
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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.
---
 server/src/api/controllers/controllerUtils.ts |  9 +++++++--
 .../unit/utils/monitorCertificate.test.ts     | 19 +++++++++++++++++++
 2 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/server/src/api/controllers/controllerUtils.ts b/server/src/api/controllers/controllerUtils.ts
index 6b86d5d5b..15dfc4832 100755
--- a/server/src/api/controllers/controllerUtils.ts
+++ b/server/src/api/controllers/controllerUtils.ts
@@ -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) {
diff --git a/server/test/unit/utils/monitorCertificate.test.ts b/server/test/unit/utils/monitorCertificate.test.ts
index 3838d526d..16240beb2 100644
--- a/server/test/unit/utils/monitorCertificate.test.ts
+++ b/server/test/unit/utils/monitorCertificate.test.ts
@@ -62,6 +62,25 @@ describe("fetchMonitorCertificate", () => {
 		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();