fix(monitors): keep ignoreTlsErrors effective for proxied checks

Review found that the proxy path silently dropped ignoreTlsErrors.
rejectUnauthorized was passed to the HttpsProxyAgent constructor, where it
governs the socket to the proxy rather than the tunnelled handshake with the
target. Verified directly: the value lands in connectOpts, while the target
handshake reads got's per-request https options, which were unset and so
defaulted to strict.

The consequence was worse than a no-op. A monitor with ignoreTlsErrors
against a self-signed endpoint works today; configuring a proxy would have
made it fail verification, flip to down and fire a false incident with
notifications. Stating the option per request fixes it, and the agent cache
keeps its TLS split so a relaxed agent is never shared with a strict monitor.

Also from review:

- A fully-invalid PROXY_MONITOR_TYPES disabled proxying entirely, which in a
  restricted-egress deployment would take every HTTP monitor down at once —
  the exact failure the parser was meant to prevent. It now falls back to the
  defaults and warns loudly. "none" remains the way to disable deliberately.
- The "none" sentinel only matched when it was the sole entry, so "none,http"
  half-enabled proxying. It is now honoured wherever it appears.
- "unknown" is a real MonitorType but never a deliberate choice, so it is
  reported as a typo rather than as an unproxyable type.
- Dropped the unused warning field from ProxyResolution.
- Documented that lowercase proxy variables are honoured and take precedence,
  which matters on base images that already set them.

Adds 10 tests, including the request-level TLS assertions the previous suite
lacked. Verified by reverting the fix: both TLS tests fail without it.
This commit is contained in:
gorkem-bwl committed 2026-08-20 10:46:25 -04:00
1 parent 705c19f7c8
commit 3b03850a3b
5 files changed
+144 -8

No files matched your search

+2
View File
@@ -37,6 +37,8 @@ STATUS_PAGE_THEMES_ENABLED=true
# --------------
# Route monitor checks through an HTTP proxy, for deployments whose egress is
# restricted. Credentials may be embedded: http://user:pass@proxy:3128
# The lowercase forms (http_proxy, https_proxy, no_proxy) are also honoured and
# take precedence over these, which matters on base images that already set them.
# HTTP_PROXY=http://proxy.corp.example:3128
# HTTPS_PROXY=http://proxy.corp.example:3128
#
@@ -186,6 +186,10 @@ export class HttpProvider implements IStatusProvider<HttpStatusPayload> {
if (proxyUrl) {
const proxyAgent = this.proxyAgents.get(proxyUrl, !ignoreTlsErrors);
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 };
} else {
options.agent = {
http: this.httpAgent,
+16 -6
View File
@@ -25,8 +25,6 @@ const HTTP_BACKED_MONITOR_TYPES = ["http", "pagespeed", "hardware"] as const;
export interface ProxyResolution {
/** Proxy URL to dial through, or null to connect directly. */
proxyUrl: string | null;
/** Set when the configured type list contained unusable entries. */
warning?: string;
}
/**
@@ -48,8 +46,9 @@ export const parseProxyMonitorTypes = (raw: string | undefined): { types: Set<st
.filter((entry) => entry !== "");
// Explicit opt-out, so proxying can be disabled without unsetting the proxy
// environment variables the rest of the container may rely on.
if (requested.length === 1 && requested[0] === "none") {
// environment variables the rest of the container may rely on. Honoured
// wherever it appears, so "none,http" disables rather than half-enabling.
if (requested.includes("none")) {
return { types: new Set<string>() };
}
@@ -58,7 +57,8 @@ export const parseProxyMonitorTypes = (raw: string | undefined): { types: Set<st
const unproxyable: string[] = [];
for (const entry of requested) {
if (!(MonitorTypes as readonly string[]).includes(entry)) {
// "unknown" is a real MonitorType but never a deliberate choice here.
if (entry === "unknown" || !(MonitorTypes as readonly string[]).includes(entry)) {
unknown.push(entry);
} else if (!(HTTP_BACKED_MONITOR_TYPES as readonly string[]).includes(entry)) {
unproxyable.push(entry);
@@ -75,9 +75,19 @@ 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
// monitor down in a restricted-egress deployment, so fall back to the defaults
// and make the misconfiguration loud instead.
if (types.size === 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.`,
};
}
return {
types,
warning: problems.length > 0 ? `PROXY_MONITOR_TYPES ignored ${problems.join("; ")}. Proxying: ${[...types].join(", ") || "(none)"}` : undefined,
warning: problems.length > 0 ? `PROXY_MONITOR_TYPES ignored ${problems.join("; ")}. Proxying: ${[...types].join(", ")}` : undefined,
};
};
@@ -74,9 +74,9 @@ const makeGotResponse = (overrides?: Record<string, any>) => ({
...overrides,
});
const createProvider = (matcher?: any) => {
const createProvider = (matcher?: any, proxyMonitorTypes?: string) => {
const advancedMatcher = matcher ?? createMockMatcher();
const provider = new HttpProvider(mockGot as any, advancedMatcher);
const provider = new HttpProvider(mockGot as any, advancedMatcher, proxyMonitorTypes);
return { provider, advancedMatcher };
};
@@ -494,4 +494,104 @@ describe("HttpProvider", () => {
expect(advancedMatcher.validate).not.toHaveBeenCalled();
});
});
// ── Outbound proxy ───────────────────────────────────────────────────
describe("outbound proxy routing", () => {
const PROXY_KEYS = ["HTTP_PROXY", "HTTPS_PROXY", "NO_PROXY", "http_proxy", "https_proxy", "no_proxy"];
const clearProxyEnv = () => {
for (const key of PROXY_KEYS) delete process.env[key];
};
const agentOf = () => (mockGot.mock.calls[0]?.[1] as any)?.agent;
const httpsOptionOf = () => (mockGot.mock.calls[0]?.[1] as any)?.https;
const isProxyAgent = (agent: any) => Boolean(agent?.https?.constructor?.name?.includes("Proxy"));
it("routes an http monitor through the configured proxy", async () => {
clearProxyEnv();
process.env.HTTPS_PROXY = "http://corp:3128";
mockGot.mockResolvedValue(makeGotResponse());
const { provider } = createProvider();
await provider.handle(makeMonitor({ url: "https://example.com" }));
expect(isProxyAgent(agentOf())).toBe(true);
clearProxyEnv();
});
it("leaves infrastructure monitors direct by default", async () => {
clearProxyEnv();
process.env.HTTPS_PROXY = "http://corp:3128";
mockGot.mockResolvedValue(makeGotResponse());
const { provider } = createProvider();
// Capture agents live on the internal network; proxying them breaks them.
await provider.handle(makeMonitor({ type: "hardware", url: "https://10.0.0.5:59232" }));
expect(isProxyAgent(agentOf())).toBe(false);
clearProxyEnv();
});
it("routes infrastructure monitors once opted in", async () => {
clearProxyEnv();
process.env.HTTPS_PROXY = "http://corp:3128";
mockGot.mockResolvedValue(makeGotResponse());
const { provider } = createProvider(undefined, "http,hardware");
await provider.handle(makeMonitor({ type: "hardware", url: "https://10.0.0.5:59232" }));
expect(isProxyAgent(agentOf())).toBe(true);
clearProxyEnv();
});
it("keeps ignoreTlsErrors effective for proxied monitors", async () => {
clearProxyEnv();
process.env.HTTPS_PROXY = "http://corp:3128";
mockGot.mockResolvedValue(makeGotResponse());
const { provider } = createProvider();
// The proxy agent's rejectUnauthorized applies to the socket to the proxy,
// not the tunnelled handshake, so this must be stated per request or a
// self-signed target would start failing the moment a proxy is configured.
await provider.handle(makeMonitor({ url: "https://self-signed.example.com", ignoreTlsErrors: true }));
expect(httpsOptionOf()).toEqual({ rejectUnauthorized: false });
clearProxyEnv();
});
it("keeps TLS strict for proxied monitors that did not opt out", async () => {
clearProxyEnv();
process.env.HTTPS_PROXY = "http://corp:3128";
mockGot.mockResolvedValue(makeGotResponse());
const { provider } = createProvider();
await provider.handle(makeMonitor({ url: "https://example.com", ignoreTlsErrors: false }));
expect(httpsOptionOf()).toEqual({ rejectUnauthorized: true });
clearProxyEnv();
});
it("does not set a proxy agent when no proxy is configured", async () => {
clearProxyEnv();
mockGot.mockResolvedValue(makeGotResponse());
const { provider } = createProvider();
await provider.handle(makeMonitor({ url: "https://example.com" }));
expect(isProxyAgent(agentOf())).toBe(false);
});
it("honours NO_PROXY", async () => {
clearProxyEnv();
process.env.HTTPS_PROXY = "http://corp:3128";
process.env.NO_PROXY = "example.com";
mockGot.mockResolvedValue(makeGotResponse());
const { provider } = createProvider();
await provider.handle(makeMonitor({ url: "https://example.com" }));
expect(isProxyAgent(agentOf())).toBe(false);
clearProxyEnv();
});
});
});
@@ -55,6 +55,26 @@ describe("parseProxyMonitorTypes", () => {
expect(warning).toBeUndefined();
});
it("honours the none sentinel wherever it appears", () => {
expect(parseProxyMonitorTypes("none,http").types.size).toBe(0);
});
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.
const { types, warning } = parseProxyMonitorTypes("htpp");
expect([...types].sort()).toEqual([...PROXYABLE_MONITOR_TYPES].sort());
expect(warning).toContain("Falling back to");
});
it("treats the internal unknown type as a typo", () => {
const { types, warning } = parseProxyMonitorTypes("unknown");
expect([...types].sort()).toEqual([...PROXYABLE_MONITOR_TYPES].sort());
expect(warning).toContain("unknown monitor type");
});
it("drops socket-based types that a proxy cannot carry, and says so", () => {
const { types, warning } = parseProxyMonitorTypes("http,ping,port,dns");