fix(monitors): correct proxy transport and error attribution

Second review round found two blocking defects, both verified against a stub
proxy rather than inferred.

HttpsProxyAgent was used for every target, and it always issues CONNECT:

    PROXY RECEIVED: "CONNECT example.com:80 HTTP/1.1"

Squid's stock ACL permits CONNECT only to 443, so every http:// monitor would
have broken the moment HTTP_PROXY was set — in exactly the hardened networks
this feature targets. Plain-http targets now use http-proxy-agent, which
sends an absolute-URI request. The agent cache keys on target scheme.

A refused CONNECT is replayed onto the socket, so the proxy's own status was
parsed as the target's: a proxy ACL denial reported "the site returned 403",
and a monitor whose customUpCodes included 403 or 407 was marked UP while the
target was never reached. Proxy-tunnel rejections are now detected via the
agent's proxyConnect event and reported as a network error naming the proxy.
The listener is per request and removed in a finally block, so a failure
cannot be attributed to a concurrent check.

Also from review:

- The fallback for an invalid PROXY_MONITOR_TYPES no longer fires when every
  entry was a deliberate but unproxyable type ("ping,port"). Falling back
  there would have proxied http and pagespeed, which the operator never
  nominated. Typos still fall back; unproxyable lists stay empty and warn.
- Dropped the now-redundant strict/insecure agent split: TLS is applied per
  request, and Node already separates socket pools by TLS options, so the
  split only doubled tunnels to the proxy.
- Renamed to ProxyPolicy.ts to match the PascalCase peers in the directory.
- The warning no longer echoes the raw config value.

Adds 9 tests covering scheme selection, tunnel rejection, and the narrowed
fallback. Verified by reverting: the tunnel-failure test fails without the fix.

server/package-lock.json

@@ -26,6 +26,7 @@
 				"got": "14.4.7",
 				"handlebars": "^4.7.8",
 				"helmet": "^8.0.0",
+				"http-proxy-agent": "^9.1.0",
 				"https-proxy-agent": "^9.1.0",
 				"isomorphic-dompurify": "^2.26.0",
 				"jmespath": "^0.16.0",
@@ -7904,16 +7905,26 @@
 			}
 		},
 		"node_modules/http-proxy-agent": {
-			"version": "7.0.2",
-			"resolved": "https://registry.npmjs.org/http-proxy-agent/-/http-proxy-agent-7.0.2.tgz",
-			"integrity": "sha512-T1gkAiYYDWYx3V5Bmyu7HcfcvL7mUrTWiM6yOfa3PIphViJ/gFPbvidQ+veqSOHci/PxBcDabeUNCzpOODJZig==",
+			"version": "9.1.0",
+			"resolved": "https://registry.npmjs.org/http-proxy-agent/-/http-proxy-agent-9.1.0.tgz",
+			"integrity": "sha512-2NxoveTT58mjYT4n3RPTEfCZGLMbidoO8XEieXfpSYxu+PQJ1qpx4ypwH6N+uF9twBPIvRRgvkvW5HUTYWENig==",
 			"license": "MIT",
 			"dependencies": {
-				"agent-base": "^7.1.0",
-				"debug": "^4.3.4"
+				"agent-base": "9.0.0",
+				"debug": "^4.3.4",
+				"proxy-agent-negotiate": "1.1.0"
 			},
 			"engines": {
-				"node": ">= 14"
+				"node": ">= 20"
+			}
+		},
+		"node_modules/http-proxy-agent/node_modules/agent-base": {
+			"version": "9.0.0",
+			"resolved": "https://registry.npmjs.org/agent-base/-/agent-base-9.0.0.tgz",
+			"integrity": "sha512-TQf59BsZnytt8GdJKLPfUZ54g/iaUL2OWDSFCCvMOhsHduDQxO8xC4PNeyIkVcA5KwL2phPSv0douC0fgWzmnA==",
+			"license": "MIT",
+			"engines": {
+				"node": ">= 20"
 			}
 		},
 		"node_modules/http2-wrapper": {
@@ -8289,6 +8300,19 @@
 				"url": "https://github.com/fb55/entities?sponsor=1"
 			}
 		},
+		"node_modules/isomorphic-dompurify/node_modules/http-proxy-agent": {
+			"version": "7.0.2",
+			"resolved": "https://registry.npmjs.org/http-proxy-agent/-/http-proxy-agent-7.0.2.tgz",
+			"integrity": "sha512-T1gkAiYYDWYx3V5Bmyu7HcfcvL7mUrTWiM6yOfa3PIphViJ/gFPbvidQ+veqSOHci/PxBcDabeUNCzpOODJZig==",
+			"license": "MIT",
+			"dependencies": {
+				"agent-base": "^7.1.0",
+				"debug": "^4.3.4"
+			},
+			"engines": {
+				"node": ">= 14"
+			}
+		},
 		"node_modules/isomorphic-dompurify/node_modules/https-proxy-agent": {
 			"version": "7.0.6",
 			"resolved": "https://registry.npmjs.org/https-proxy-agent/-/https-proxy-agent-7.0.6.tgz",
@@ -9240,6 +9264,19 @@
 				}
 			}
 		},
+		"node_modules/jsdom/node_modules/http-proxy-agent": {
+			"version": "7.0.2",
+			"resolved": "https://registry.npmjs.org/http-proxy-agent/-/http-proxy-agent-7.0.2.tgz",
+			"integrity": "sha512-T1gkAiYYDWYx3V5Bmyu7HcfcvL7mUrTWiM6yOfa3PIphViJ/gFPbvidQ+veqSOHci/PxBcDabeUNCzpOODJZig==",
+			"license": "MIT",
+			"dependencies": {
+				"agent-base": "^7.1.0",
+				"debug": "^4.3.4"
+			},
+			"engines": {
+				"node": ">= 14"
+			}
+		},
 		"node_modules/jsdom/node_modules/https-proxy-agent": {
 			"version": "7.0.6",
 			"resolved": "https://registry.npmjs.org/https-proxy-agent/-/https-proxy-agent-7.0.6.tgz",

server/package.json

@@ -49,6 +49,7 @@
 		"got": "14.4.7",
 		"handlebars": "^4.7.8",
 		"helmet": "^8.0.0",
+		"http-proxy-agent": "^9.1.0",
 		"https-proxy-agent": "^9.1.0",
 		"isomorphic-dompurify": "^2.26.0",
 		"jmespath": "^0.16.0",

server/src/service/network/HttpProvider.ts

@@ -9,7 +9,7 @@ import { Monitor, MonitorType } from "@/domain/monitors/monitor.type.js";
 import { isStatusUp } from "@/service/network/utils.js";
 import { NETWORK_ERROR } from "@/types/network.js";
 import CacheableLookup from "cacheable-lookup";
-import { ProxyAgentCache, parseProxyMonitorTypes, resolveProxyForCheck } from "@/service/network/proxyPolicy.js";
+import { ProxyAgentCache, parseProxyMonitorTypes, resolveProxyForCheck } from "@/service/network/ProxyPolicy.js";
 
 export class HttpProvider implements IStatusProvider<HttpStatusPayload> {
 	readonly type = "http";
@@ -179,17 +179,31 @@ export class HttpProvider implements IStatusProvider<HttpStatusPayload> {
 		const options: Record<string, unknown> = {
 			headers: monitor.secret ? { Authorization: `Bearer ${secret}` } : undefined,
 		};
+		let cleanupProxyListener: (() => void) | undefined;
 
 		// Route through an outbound proxy when the environment configures one and
 		// this monitor type is eligible; otherwise use the shared direct agents.
 		const { proxyUrl } = resolveProxyForCheck(monitor.type, url, this.proxyableTypes);
+		// Set when the proxy itself refuses the tunnel, so a proxy ACL denial is not
+		// reported as the target's own status code.
+		let tunnelFailure: { statusCode: number } | null = null;
 		if (proxyUrl) {
-			const proxyAgent = this.proxyAgents.get(proxyUrl, !ignoreTlsErrors);
+			const proxyAgent = this.proxyAgents.get(proxyUrl, url);
 			options.agent = { http: proxyAgent, https: proxyAgent };
 			// The agent's rejectUnauthorized governs the socket to the proxy, not the
 			// tunnelled handshake with the target, so ignoreTlsErrors has to be stated
 			// per request or it would be silently dropped for proxied monitors.
 			options.https = { rejectUnauthorized: !ignoreTlsErrors };
+
+			const emitter = proxyAgent as unknown as {
+				once?: (event: string, cb: (connect: { statusCode: number }) => void) => void;
+				off?: (event: string, cb: (connect: { statusCode: number }) => void) => void;
+			};
+			const onProxyConnect = (connect: { statusCode: number }) => {
+				if (connect.statusCode >= 400) tunnelFailure = { statusCode: connect.statusCode };
+			};
+			emitter.once?.("proxyConnect", onProxyConnect);
+			cleanupProxyListener = () => emitter.off?.("proxyConnect", onProxyConnect);
 		} else {
 			options.agent = {
 				http: this.httpAgent,
@@ -213,7 +227,24 @@ export class HttpProvider implements IStatusProvider<HttpStatusPayload> {
 				timings: response.timings,
 			});
 		} catch (error: unknown) {
+			if (tunnelFailure) {
+				// The proxy rejected the tunnel, so the target was never reached and
+				// its status is unknown. Reporting NETWORK_ERROR keeps customUpCodes
+				// from marking the monitor up on the proxy's 403/407.
+				return {
+					monitorId: monitor.id,
+					teamId: monitor.teamId,
+					type: monitor.type,
+					status: false,
+					code: NETWORK_ERROR,
+					message: `Proxy refused the connection (HTTP ${(tunnelFailure as { statusCode: number }).statusCode})`,
+					responseTime: 0,
+					payload: null as T,
+				};
+			}
 			return this.handleHttpError(error, monitor);
+		} finally {
+			cleanupProxyListener?.();
 		}
 	}
 }

server/src/service/network/ProxyPolicy.ts

@@ -1,3 +1,4 @@
+import { HttpProxyAgent } from "http-proxy-agent";
 import { HttpsProxyAgent } from "https-proxy-agent";
 import { getProxyForUrl } from "proxy-from-env";
 import type { Agent as HttpAgent } from "http";
@@ -75,13 +76,17 @@ export const parseProxyMonitorTypes = (raw: string | undefined): { types: Set<st
 		problems.push(`monitor type(s) that do not use HTTP and cannot be proxied: ${unproxyable.join(", ")}`);
 	}
 
-	// Every entry was rejected. Disabling all proxying here would take every HTTP
+	// Every entry was a typo. Disabling all proxying here would take every HTTP
 	// monitor down in a restricted-egress deployment, so fall back to the defaults
 	// and make the misconfiguration loud instead.
-	if (types.size === 0) {
+	//
+	// An all-unproxyable list (e.g. "ping,port") is left empty on purpose: those
+	// are deliberate choices, and silently proxying http instead would route
+	// traffic the operator never nominated.
+	if (types.size === 0 && unproxyable.length === 0) {
 		return {
 			types: new Set<string>(PROXYABLE_MONITOR_TYPES),
-			warning: `PROXY_MONITOR_TYPES ("${raw}") matched no proxyable monitor type — ${problems.join("; ")}. Falling back to: ${PROXYABLE_MONITOR_TYPES.join(", ")}. Use "none" to disable proxying.`,
+			warning: `PROXY_MONITOR_TYPES matched no proxyable monitor type — ${problems.join("; ")}. Falling back to: ${PROXYABLE_MONITOR_TYPES.join(", ")}. Use "none" to disable proxying.`,
 		};
 	}
 
@@ -114,25 +119,28 @@ export const resolveProxyForCheck = (monitorType: MonitorType, url: string, prox
 /**
  * Builds proxy agents on demand and reuses them.
  *
- * Keyed by proxy URL *and* TLS mode: a monitor with ignoreTlsErrors must not
- * share an agent with a strict one, or the relaxed setting would leak across
- * monitors. Caching also preserves connection pooling — without it every check
- * would open a fresh tunnel to the proxy.
+ * The agent must match the *target* scheme, not the proxy's. HttpsProxyAgent
+ * always issues CONNECT, which hardened proxies (Squid's stock ACL among them)
+ * permit only to 443 — so a plain-http target needs HttpProxyAgent, which sends
+ * an absolute-URI request instead.
+ *
+ * Caching preserves connection pooling: without it every check would open a
+ * fresh connection to the proxy. Node keys its own socket pools on the TLS
+ * options it receives per request, so agents are shared across monitors
+ * regardless of ignoreTlsErrors — that setting is applied per request in
+ * HttpProvider, not baked into the agent.
  */
 export class ProxyAgentCache {
 	private readonly agents = new Map<string, HttpAgent | HttpsAgent>();
 
-	get(proxyUrl: string, rejectUnauthorized: boolean): HttpAgent | HttpsAgent {
-		const key = `${proxyUrl}::${rejectUnauthorized ? "strict" : "insecure"}`;
+	get(proxyUrl: string, targetUrl: string): HttpAgent | HttpsAgent {
+		const targetIsSecure = safeProtocol(targetUrl) !== "http:";
+		const key = `${proxyUrl}::${targetIsSecure ? "https" : "http"}`;
 		const existing = this.agents.get(key);
 		if (existing) return existing;
 
-		const agent = new HttpsProxyAgent(proxyUrl, {
-			keepAlive: true,
-			maxSockets: 256,
-			maxFreeSockets: 256,
-			rejectUnauthorized,
-		}) as unknown as HttpsAgent;
+		const options = { keepAlive: true, maxSockets: 256, maxFreeSockets: 256 };
+		const agent = (targetIsSecure ? new HttpsProxyAgent(proxyUrl, options) : new HttpProxyAgent(proxyUrl, options)) as unknown as HttpsAgent;
 
 		this.agents.set(key, agent);
 		return agent;
@@ -143,3 +151,12 @@ export class ProxyAgentCache {
 		return this.agents.size;
 	}
 }
+
+/** Target URLs are validated upstream; fall back to https on anything odd. */
+const safeProtocol = (url: string): string => {
+	try {
+		return new URL(url).protocol;
+	} catch {
+		return "https:";
+	}
+};

server/test/unit/providers/network/httpProvider.test.ts

@@ -581,6 +581,30 @@ describe("HttpProvider", () => {
 			expect(isProxyAgent(agentOf())).toBe(false);
 		});
 
+		it("reports a refused proxy tunnel as a network error, not the target's status", async () => {
+			clearProxyEnv();
+			process.env.HTTPS_PROXY = "http://corp:3128";
+			const { provider } = createProvider();
+			// The agent replays a refused CONNECT onto the socket, so the proxy's own
+			// status would otherwise be parsed as the target's.
+			const agent: any = (provider as any).proxyAgents.get("http://corp:3128", "https://example.com");
+			const err = new HTTPError("Response code 403 (Forbidden)");
+			(err as any).response = { statusCode: 403, body: "", headers: {} };
+			(err as any).timings = { phases: { total: 5 } };
+			mockGot.mockImplementation(() => {
+				agent.emit("proxyConnect", { statusCode: 403 });
+				return Promise.reject(err);
+			});
+
+			const result = await provider.handle(makeMonitor({ url: "https://example.com", customUpCodes: [403] }));
+
+			// customUpCodes must not mark the monitor up on the proxy's status.
+			expect(result.status).toBe(false);
+			expect(result.code).toBe(NETWORK_ERROR);
+			expect(result.message).toContain("Proxy refused");
+			clearProxyEnv();
+		});
+
 		it("honours NO_PROXY", async () => {
 			clearProxyEnv();
 			process.env.HTTPS_PROXY = "http://corp:3128";

server/test/unit/providers/network/proxyPolicy.test.ts

@@ -4,7 +4,7 @@ import {
 	ProxyAgentCache,
 	parseProxyMonitorTypes,
 	resolveProxyForCheck,
-} from "../../../../src/service/network/proxyPolicy.ts";
+} from "../../../../src/service/network/ProxyPolicy.ts";
 
 // proxy-from-env reads process.env directly, so these tests set and restore it.
 const PROXY_KEYS = ["HTTP_PROXY", "HTTPS_PROXY", "NO_PROXY", "http_proxy", "https_proxy", "no_proxy"];
@@ -59,6 +59,15 @@ describe("parseProxyMonitorTypes", () => {
 		expect(parseProxyMonitorTypes("none,http").types.size).toBe(0);
 	});
 
+	it("leaves an all-unproxyable list empty rather than proxying types nobody named", () => {
+		// "ping,port" is a misunderstanding, but a deliberate one — silently
+		// proxying http instead would route traffic the operator never nominated.
+		const { types, warning } = parseProxyMonitorTypes("ping,port");
+
+		expect(types.size).toBe(0);
+		expect(warning).toContain("cannot be proxied");
+	});
+
 	it("falls back to defaults when every entry is invalid, rather than disabling proxying", () => {
 		// Silently proxying nothing would take every HTTP monitor down in a
 		// restricted-egress deployment, which is worse than ignoring the typo.
@@ -158,30 +167,52 @@ describe("ProxyAgentCache", () => {
 	it("reuses one agent per proxy so connections stay pooled", () => {
 		const cache = new ProxyAgentCache();
 
-		const first = cache.get("http://corp:3128", true);
-		const second = cache.get("http://corp:3128", true);
+		const first = cache.get("http://corp:3128", "https://example.com");
+		const second = cache.get("http://corp:3128", "https://other.example.com");
 
 		expect(second).toBe(first);
 		expect(cache.size).toBe(1);
 	});
 
-	it("keeps strict and TLS-ignoring agents separate", () => {
+	it("uses a CONNECT-based agent for https targets", () => {
 		const cache = new ProxyAgentCache();
 
-		const strict = cache.get("http://corp:3128", true);
-		const insecure = cache.get("http://corp:3128", false);
+		const agent = cache.get("http://corp:3128", "https://example.com");
+
+		expect(agent.constructor.name).toBe("HttpsProxyAgent");
+	});
+
+	it("uses an absolute-URI agent for plain http targets", () => {
+		const cache = new ProxyAgentCache();
+
+		// HttpsProxyAgent would issue CONNECT, which hardened proxies allow only
+		// to 443, breaking every http:// monitor.
+		const agent = cache.get("http://corp:3128", "http://example.com");
+
+		expect(agent.constructor.name).toBe("HttpProxyAgent");
+	});
+
+	it("keeps http and https target agents separate", () => {
+		const cache = new ProxyAgentCache();
+
+		cache.get("http://corp:3128", "https://example.com");
+		cache.get("http://corp:3128", "http://example.com");
 
-		// Sharing one agent would leak ignoreTlsErrors across monitors.
-		expect(insecure).not.toBe(strict);
 		expect(cache.size).toBe(2);
 	});
 
 	it("keeps a separate agent per proxy URL", () => {
 		const cache = new ProxyAgentCache();
 
-		cache.get("http://a:3128", true);
-		cache.get("http://b:3128", true);
+		cache.get("http://a:3128", "https://example.com");
+		cache.get("http://b:3128", "https://example.com");
 
 		expect(cache.size).toBe(2);
 	});
+
+	it("falls back to a CONNECT agent for an unparseable target", () => {
+		const cache = new ProxyAgentCache();
+
+		expect(cache.get("http://corp:3128", "not-a-url").constructor.name).toBe("HttpsProxyAgent");
+	});
 });