Skip to content

XS✔ ◾ Restore TLS certificate validation for health-check clients - #903

Closed
Marc Hayek (marc-hayek) wants to merge 1 commit into
mainfrom
fix/health-check-certificate-validation
Closed

Marc Hayek (marc-hayek) wants to merge 1 commit into
mainfrom
fix/health-check-certificate-validation

Conversation

@marc-hayek

Copy link
Copy Markdown

Summary

  • Remove unconditional TLS server-certificate acceptance from AddHealthCheckHttpClient and the legacy publisher health-check client registration.
  • Restore platform certificate validation in every environment, including for loopback and IP-addressed HTTPS endpoints.
  • Preserve the named-client API, explicit HttpClientHandler, and the legacy client's redirect and default-credential settings.
  • Correct the XML documentation and add regression tests against the handlers created by IHttpMessageHandlerFactory.

Compatibility and rollout

HTTP health checks are unaffected. HTTPS health checks must present a trusted, valid certificate matching the configured hostname or IP address. Probes relying on untrusted/self-signed, expired, or hostname-mismatched certificates will fail after this change; their certificate, trust, or endpoint-address configuration must be corrected before rollout. Downstream service configurations have not been validated by this PR.

Validation

  • Both new regression tests were confirmed to fail on the existing accept-all callbacks before applying the fix.
  • Release Diagnostics.HealthChecks.UnitTests: 91 passed.
  • Release Diagnostics.HealthChecks.AspNetCore.UnitTests: 26 passed.
  • git diff --check passed.
  • New tests verify default certificate-validation configuration; they do not perform live TLS handshakes.

Relationship to #902

This is a stricter alternative to #902, not an additional change intended to merge unchanged alongside it. It uses default certificate validation rather than introducing local/IP-address or Development exceptions, and also fixes the independent legacy registration. Reviewers should coordinate the two approaches before merging.

Remove unconditional certificate acceptance from named and legacy health-check clients while preserving existing handler settings. Update documentation and add regression coverage for the registered handlers.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 97fd4f9b-6689-4de9-bea3-3cbe15794b28
@marc-hayek
Marc Hayek (marc-hayek) requested a review from a team as a code owner October 16, 2026 14:37
Copilot AI balanced review requested due to automatic review settings October 16, 2026 14:37
@microsoft-omex

Copy link
Copy Markdown
Contributor

PR Metrics

✔ Thanks for keeping your pull request small.
✔ Thanks for adding tests.

Lines
Product Code 4
Test Code 46
Subtotal 50
Ignored Code -
Total 50

Metrics computed by PR Metrics. Add it to your Azure DevOps and GitHub PRs!

@microsoft-omex microsoft-omex Bot changed the title Restore TLS certificate validation for health-check clients XS✔ ◾ Restore TLS certificate validation for health-check clients Oct 16, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The focused changes restore certificate validation, preserve client settings, and include regression coverage without identified blocking issues.

0 open findings

What changed in this PR

Restores platform TLS certificate validation for both health-check client registrations, taking the stricter approach described as an alternative to #902.

Changes:

  • Removes unconditional certificate acceptance while preserving existing client settings.
  • Corrects XML documentation and adds handler-configuration regression tests.
File Description
tests/​Diagnostics.HealthChecks.UnitTests/​ServiceCollectionExtensions.cs Tests default validation and preserved legacy settings.
src/​Diagnostics.HealthChecks/​ServiceCollectionExtensions.cs Removes the legacy client's certificate bypass.
src/​Diagnostics.HealthChecks/​HealthCheckHttpClientServiceCollectionExtensions.cs Removes the named-client bypass and updates documentation.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

This branch was successfully deployed

1 active deployment
github-app-main — 35c07da1 Deployed Oct 16, 2026 by marc-hayek via PR Metrics #2490
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants