Skip to content

fix(docker): probe /healthz over https when the web UI serves TLS - #2848

Merged
cliffhall merged 1 commit into
v2/mainfrom
v2/fix/2825-docker-healthcheck-https
Oct 29, 2026
Merged

cliffhall merged 1 commit into
v2/mainfrom
v2/fix/2825-docker-healthcheck-https

Conversation

@cliffhall

Copy link
Copy Markdown
Member

Closes #2825

The Docker HEALTHCHECK (scripts/docker-healthcheck.mjs) always built an http:// URL, so with both INSPECTOR_TLS_KEY_PATH and INSPECTOR_TLS_CERT_PATH set every probe failed and a correctly running TLS container was reported unhealthy forever.

  • Scheme follows the server's TLS rule. New tlsEnabled(env) mirrors clients/web/server/tls.ts: https only when both paths are set, a blank value counting as unset.
  • Certificate verification is skipped for this probe. It only ever reaches the container's own listener, where a self-signed or private-CA cert (often naming the public hostname, not the loopback address probed) is the normal case. The probe now uses node:http/node:https instead of global fetch, since fetch has no per-request way to skip verification without importing undici, which the copied-out script cannot resolve.
  • Probes /healthz (Web client's backend server has no health/readiness endpoint #2438) instead of /. Verified on v2/main: both backends serve it unauthenticated, it is exempt from the Host allow-list, and it avoids reading and token-injecting index.html on every probe.
  • Dockerfile comment and docs/docker.md updated.

Tests

scripts/docker-healthcheck.test.mjs: probeUrl expectations move to /healthz plus https cases; a tlsEnabled table; the http probe asserts the path it hit; a new probe over https block runs against a real TLS listener with a throwaway openssl-generated self-signed cert (healthy), and an http probe to that port (unhealthy).

Hand smoke (built launcher, self-signed cert)

  • TLS web on :17274, new probe → exit 0; the v2/main probe → exit 1 (the bug).
  • Plain-HTTP web on :17284, new probe → exit 0.

npm run local:gate green.

🤖 Generated with Claude Code

)

The image's HEALTHCHECK always built an http:// URL, so with both
INSPECTOR_TLS_KEY_PATH and INSPECTOR_TLS_CERT_PATH set the probe failed
every time and a correctly running TLS container was reported unhealthy
forever. The probe now follows the server's TLS rule (both paths set, a
blank value counting as unset) and uses https without verifying the
certificate, since it only reaches the container's own listener where a
self-signed or private-CA certificate is the normal case. It uses
node:http/https rather than fetch because fetch has no per-request way
to skip verification without undici, which the copied script cannot
resolve.

It also probes the unauthenticated /healthz route (#2438) instead of /,
which read and token-injected index.html on every probe.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: cliffhall <cliff@futurescale.com>
@cliffhall cliffhall added the v2 Issues and PRs for v2 label Oct 29, 2026
@cliffhall
cliffhall requested a balanced review from Copilot October 29, 2026 03:44
Comment thread scripts/docker-healthcheck.mjs Dismissed

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No active review issues remain for the TLS-aware Docker health-check changes.

No open issues 🎉

What changed in this PR

Fixes Docker health checks for TLS-enabled web deployments, addressing #2825 by matching the web server’s protocol and using the existing /healthz endpoint.

Changes:

  • Selects HTTPS when both TLS paths are set and skips certificate verification for the container-local probe.
  • Replaces fetch with Node’s HTTP/HTTPS clients and probes /healthz instead of /.
  • Adds TLS configuration and real HTTPS listener tests, and updates Docker documentation.
File Description
scripts/​docker-healthcheck.test.mjs Tests health-route URLs, TLS detection, and self-signed HTTPS probing.
scripts/​docker-healthcheck.mjs Adds protocol-aware /healthz probing using Node’s HTTP/HTTPS clients.
docs/​docker.md Documents HTTPS health checks and certificate-verification behavior.
Dockerfile Updates comments to describe /healthz and TLS-aware probing; configuration is unchanged.

Review effort: Balanced

@cliffhall

Copy link
Copy Markdown
Member Author

Copilot review loop closed: round 1 (review 5692602420) was clean — no inline comments, no headline finding, no suppressed comments — so per pr-flow 7c no further round is requested.

@cliffhall

Copy link
Copy Markdown
Member Author

CodeQL flags alert #84 (js/disabling-certificate-validation, scripts/docker-healthcheck.mjs:169). That is the fix itself, as #2825 proposes: the probe connects only to the container's own listener, where a self-signed or private-CA certificate is the normal case, and that certificate names the public hostname rather than the loopback address probed. Verifying it would fail on exactly the deployments the fix is for and prove nothing about liveness. Pinning the configured cert as ca doesn't work either: a private-CA leaf without its root fails chain building. The script's header records this reasoning.

Suggested dismissal: won't fix, comment: "Loopback-only Docker HEALTHCHECK probe of the container's own TLS listener (#2825); a self-signed/private-CA cert is the expected case and verification would report every TLS container unhealthy. No data is sent beyond GET /healthz." I've left the dismissal to a maintainer.

@cliffhall
cliffhall merged commit 8fc1a14 into v2/main Oct 29, 2026
11 checks passed
@cliffhall
cliffhall deleted the v2/fix/2825-docker-healthcheck-https branch October 29, 2026 06:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

v2 Issues and PRs for v2

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Docker: HEALTHCHECK always reports unhealthy when the web UI is served over HTTPS (INSPECTOR_TLS_*)

3 participants