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.
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.
Deployments with restricted egress could not run checks at all: got does not
read HTTP_PROXY/HTTPS_PROXY (unlike curl or axios), and NODE_USE_ENV_PROXY
only affects Node's built-in fetch, which got does not use. Even a correctly
built proxy agent was discarded, because HttpProvider overwrote
options.agent on every request.
Proxy selection is resolved per check and applied where the agent is chosen.
NO_PROXY handling is delegated to proxy-from-env rather than reimplemented,
so wildcards, host lists and port-qualified entries behave exactly as
operators expect from curl.
Infrastructure monitors are excluded by default. Hardware and pagespeed
checks both delegate to HttpProvider, so a naive global proxy would also
capture Capture agents — which normally sit on the internal network, where
proxying them fails or hairpins. PROXY_MONITOR_TYPES is the escape hatch for
deployments whose agents genuinely are behind the proxy, and accepts "none"
to disable proxying without unsetting the environment variables. Values that
are not real monitor types, or that open raw sockets and cannot be proxied
(ping, port, docker, grpc, websocket, dns, game), are dropped and logged so
a typo cannot silently disable proxying.
Agents are cached by proxy URL and TLS mode. Keying on TLS matters: sharing
one agent between a strict monitor and an ignoreTlsErrors monitor would leak
the relaxed setting across monitors. Caching also preserves pooling, so 50
monitors share tunnels instead of dialling the proxy per check.
Both new dependencies were already present transitively (76K total).
Notification providers still call out directly and are unaffected; that is a
known follow-up for the same class of deployment.
The monitor picker on the status page create/configure form fetched
candidates using a hardcoded list of monitor types that omitted "dns", so
DNS monitors were filtered out server-side and never appeared in the
dialog. Everything else was already in place: the DNS provider stores
checks like any other monitor type, and the status page renders them once
attached.
Rather than appending "dns" to the hardcoded list, derive the query from
the existing SelectableMonitorTypes tuple in @/Types/Monitor. The two lists
had silently drifted apart, and deriving prevents the next new monitor type
from hitting the same bug. "hardware" is appended explicitly because
infrastructure monitors are created on their own page and so are absent
from the selectable set.
Net effect on the requested types: gains "dns", loses none.
Fixes#3721
Adds a regression suite for #3843 so the leak cannot return silently. The
client had no test runner, so this introduces Vitest, which reuses the
existing Vite resolution and the `@/` alias rather than needing a separate
transform pipeline. jsdom is required because the logger reads
window.location and navigator.userAgent.
The tests drive the exported `logger` rather than the private redaction
helpers, so they cover the wiring as well as the logic. That distinction
matters here: the first attempt at this fix redacted the JSON branch but
left the debug branch printing raw objects, a gap that testing the helper
in isolation would not have caught. Each case runs at both error and debug
level for the same reason.
Covered: the reported login payload, credentials carried on an AxiosError
(config.data and the Authorization header), non-JSON request bodies,
passwords on class instances, key-matching variants, and circular payloads.
One asserts non-sensitive context survives, so redaction cannot regress
into blanket suppression.
Verified by reverting the fix: 16 of 23 tests fail against the original
behaviour, including a pre-existing crash on circular payloads.
Follow-up to the initial redaction pass, addressing four defects:
Class instances bypassed redaction entirely. The prototype guard was meant
to skip FormData/Blob but caught every non-literal object, returning it by
reference — so a credential on a DTO or form-state instance was logged
verbatim, and the deep-copy contract was violated. The check is now an
explicit deny-list of opaque types; everything else is walked.
The raw Error was still logged by the debug branch. An AxiosError carries
config.data (the serialized request body) and config.headers.Authorization
(set for every request in ApiClient), so the password redacted from the
context was printed straight back out via the error object. Errors now go
through toSafeError(). Because Axios serializes the body to a string, which
key-based redaction cannot see into, it is parsed before redaction; a body
that is not JSON is replaced wholesale rather than logged raw.
The Blob guard was gated on `typeof FormData !== "undefined"`, so in an
environment with FormData but no Blob the instanceof check threw a
ReferenceError — from inside a catch block, turning a handled API error
into an unhandled crash. Each constructor is now resolved defensively.
Also widened the key list (cookie, jwt, privateKey, sessionId, csrf, pwd)
and dropped a redundant type annotation.
Verified with 18 assertions covering the reported payload, class instances,
cycles, shared siblings, axios error shapes, and a missing Blob global.
Failed requests logged the full request body to the browser console, so a
failing POST to /auth/login wrote the user's plaintext password where it
could be captured by error-reporting tools, screen shares, or support
sessions.
Redaction is applied centrally in the logger rather than at the call sites,
so every current and future caller is covered:
- createLogData() redacts context before it is serialized, covering the
JSON branches of error() and warn().
- The debug-level branches of error() and warn() printed the raw context
object directly, bypassing createLogData(); they now print the redacted
copy.
- debug() and info() redact their free-form data argument.
Keys are matched case-insensitively on a substring after stripping dashes
and underscores, so password, newPassword, X-Api-Key and Authorization are
all caught. The walk copies rather than mutates the caller's object, cuts
genuine cycles without mislabelling repeated siblings, and renders
FormData/Blob as a type marker instead of walking them.
Affected paths were POST (/auth/login, /auth/register) and PATCH/PUT, which
carry monitor secrets. Error-level logging is on by default, so this was
not limited to debug builds.
Fixes#3843
Matrix notifications failed for anyone who entered a room alias. The send
endpoint only accepts a room ID, and an unresolved alias broke the request
outright: the leading "#" opens a URI fragment, so the PUT landed on
/_matrix/client/v3/rooms/ instead of the send endpoint. Element shows users
the alias, so this is the value most people paste in.
- Resolve "#alias:server" through GET /_matrix/client/v3/directory/room/{alias}
before sending, per the client-server spec
- Percent-encode the room ID in the send path; ":" was previously sent raw
- Strip trailing slashes from the homeserver URL to avoid "//_matrix/..."
Alias resolution failures return false and log, matching how the provider
already handles send failures.
Closes#3478
sendEmail caught transport failures internally and resolved to false or
undefined, so every caller's .catch() was dead code and send failures were
silently suppressed. Password recovery in particular reported "email sent
successfully" while the user received nothing.
sendEmail now throws an AppError carrying the underlying cause, and returns
Promise<string> instead of Promise<string | false | undefined>.
Call sites updated for the new contract:
- UserService.registerUser: the existing .catch() now actually fires
- UserService.requestRecovery: the failure reaches the client instead of a
misleading success response
- InviteService and the test-email endpoint: keep their user-facing messages
and attach the SMTP cause
- EmailProvider: catches and returns false, because NotificationsService fans
providers out through Promise.all and a rejection there would abort the
other channels' alerts
Closes#3757
ntfy servers that require auth rejected every Checkmate alert because the
provider posted without an Authorization header, so alerts to protected
topics failed silently.
Adds ntfyAuthType ("none" | "token" | "basic"):
- token: Authorization: Bearer <accessToken>
- basic: Authorization: Basic base64(ntfyUsername:accessToken)
Both types store their secret in the existing accessToken field rather than
introducing a new secret, so credential handling stays in one place.
Incomplete credentials fail the send instead of falling back to an anonymous
post, and validation rejects partial auth config up front so a channel can't
be saved looking authenticated while posting anonymously.
Backend only; frontend follows once this lands.
Closes#3717
Build title_link from the monitor type (hardware -> infrastructure,
pagespeed -> pagespeed, uptime types -> uptime) and omit it when
clientHost is not configured, instead of always linking to the
infrastructure page.
- Bare IP addresses and non-domain hosts (localhost, IPv6) now return a
null expiry instead of throwing, so they no longer spam error logs.
- Extract the registrable domain with tldts instead of walking up
hostname labels, so a subdomain can never accidentally return its
parent domain's expiry date.
- Cache WHOIS lookups per registrable domain (1h positive, 10min
negative) so visiting details pages does not re-query the registry
every time.
Addresses review feedback on #3834.
gorkem-bwl
Alexander Holliday
egeoztass
Marcos Paulo Diniz
colinjtsen2010