Repository navigation
Clear the web-TLS-cert snapshot on stop so the expiry alert resolves (follow-up to #3514) - #3516
Merged
Merged
Conversation
…the expiry alert resolves (follow-up to #3514) Adversarial review of #3514 caught a reachable bug: WebTlsCertificateState was write-once and StopServerAsync disposed the certificate without clearing it, so disabling the web dashboard at runtime (a no-restart op) left the worker firing the expiry alert forever - escalating to a Critical that claims the LAN dashboard is unreachable, about a dashboard the operator deliberately turned off - with no resolution short of a full service restart. Add WebTlsCertificateState.Clear() and call it wherever the host stops serving TLS: StopServerAsync (runtime disable), DisposeFailedStartAsync (failed start), and the SAN-catch loopback degrade. The lifetime-refusal path deliberately keeps the published facts so an expired-at-start certificate still fires its Critical. Rebind-safe: Stop->Start run back-to-back in one supervisor tick, re-publishing before the worker's hourly sweep observes the null. Also makes the resolve path reachable (review finding #2): the worker's report is now built by a pure static BuildWebTlsCertReport(snapshot) mapping a null/cleared snapshot to Configured=false, so a stop resolves the alert. Dropped the misleading "renewed past the window" test (it fed a report the seam cannot produce in-process - in-place renewal is restart-only), kept the now-reachable disable->resolve test, and added seam tests for Clear() and the report builder. CHANGELOG wording corrected to match. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RzwoqcD62jKyp6tzsWpZLz
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #3514, closing the two findings from its post-merge adversarial review.
The bug (both reviewers, confirmed)
WebTlsCertificateStatewas write-once andStopServerAsyncdisposed the certificate without clearing it, so disabling the web dashboard at runtime (a no-restart op) left the worker firing the expiry alert forever — escalating to a Critical that claims "the LAN dashboard is unreachable now" about a dashboard the operator deliberately turned off — with no resolution short of a full service restart.Fix
WebTlsCertificateState.Clear(), called wherever the host stops serving TLS:StopServerAsync(runtime disable),DisposeFailedStartAsync(failed start), and the SAN-catch loopback degrade. The lifetime-refusal path deliberately keeps the published facts so an expired-at-start cert still fires its Critical. Rebind-safe: Stop→Start run back-to-back in one supervisor tick, re-publishing before the worker's hourly sweep observes the null.BuildWebTlsCertReport(snapshot)mapping a null/cleared snapshot →Configured=false→ the evaluator's resolve arm.Clear()and the report builder. CHANGELOG resolution wording corrected to match.Known gap left as-is (out of the feature's expiry scope, covered by a Critical startup log): a not-yet-valid cert (future NotBefore) publishes a future NotAfter as healthy while the host is loopback-only — filing separately.
Full
Darling.Testsgreen — only the known machine-localNpgsqlRootCertificateValidationTestsTLS-trust flake fails (passes in CI on Linux), unrelated.🤖 Generated with Claude Code
https://claude.ai/code/session_01RzwoqcD62jKyp6tzsWpZLz