fix(certs): refuse a canonical-host change that would loop, and clean up the cert UI - #89
Merged
Merged
Conversation
… up the cert UI Five things the SSL surface was getting wrong, all found by using it. REDIRECT LOOP GUARD. Setting a canonical host is one half of a conversation: the application has an opinion about its own address too. When the two disagree they take turns bouncing the visitor and the site dies with ERR_TOO_MANY_REDIRECTS. This is not hypothetical — it is how ingratia.cz went down. nginx said www (our setting), WordPress's siteurl said apex, and each redirected at the other. `nginx -t` was perfectly happy, because the config parses; it just does not mean anything sane. Nothing can predict this from config alone — the other half lives in the site's own database. So test the real thing: apply, follow the chain over loopback with an explicit Host/SNI, and roll back if it cycles. The operator keeps the behaviour they had and gets a message naming what disagreed, instead of a working panel and a dead site. An inconclusive probe (no curl, refused, timeout) does NOT block the change — this catches the certain case and stays out of the way otherwise. DELETE BEFORE RE-ISSUE. Issuing over a healthy certificate is nearly always a mistake: it spends Let's Encrypt's per-hostname budget on something that already worked, and a failed attempt leaves the site worse off than before it was touched. When a real cert is in place, the SSL tab now offers Delete rather than Issue, so replacing one is deliberate rather than something you reach by clicking the obvious button twice. Deleting drops the site to the self-signed bootstrap — it keeps serving, with a browser warning, rather than taking nginx down over a missing ssl_certificate file. STAGING CHECKBOX REMOVED. It was checked by DEFAULT and labelled "recommended", so the most likely click minted an untrusted certificate and made the site show a security warning — the exact symptom operators then report as "the site is broken". Staging answers "will validation work" at the cost of "the site is trusted"; the diagnostics added in #84 answer the same question without breaking anything. The field stays on the API for scripted use. EXPIRES was a raw unix timestamp. `1794055205` is not a date to anyone. Added an askama `date`/`datetime` filter — the certs table now reads 2026-11-07, with the raw value kept in the title attribute. FINGERPRINT rendered an empty <code> block, because it is not a column and the detail page was displaying `String::new()`. Read it from the certificate on disk instead, which is also the more honest answer: it reports what is being SERVED, not what we last remembered writing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
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.
Five things the SSL surface was getting wrong, all found by using it.
1. Redirect-loop guard — this is what took ingratia.cz down
Setting a canonical host is one half of a conversation: the application has an opinion about its own address too. When the two disagree they take turns bouncing the visitor and the site dies with
ERR_TOO_MANY_REDIRECTS.Confirmed on the live box:
nginx -twas perfectly happy — the config parses, it just doesn't mean anything sane.Nothing can predict this from config alone, because the other half lives in the site's own database. So test the real thing: apply → follow the chain over loopback with an explicit Host/SNI → roll back if it cycles. The operator keeps the behaviour they had and gets a message naming what disagreed, instead of a working panel and a dead site.
An inconclusive probe (no curl, connection refused, timeout) does not block the change — this catches the certain case and stays out of the way otherwise.
2. Delete before re-issue
Issuing over a healthy certificate is nearly always a mistake: it spends Let's Encrypt's per-hostname budget on something that already worked, and a failed attempt leaves the site worse off than before it was touched. With a real cert in place the SSL tab now offers Delete, not Issue.
Deleting drops the site to the self-signed bootstrap — it keeps serving (with a browser warning) rather than taking nginx down over a missing
ssl_certificatefile. NewCertDeleteRPC, dispatched to the owning node.3. Staging checkbox removed
No. It was checked by default and labelled "recommended", so the most likely click minted an untrusted certificate and made the site show a security warning — the exact symptom then reported as "the site is broken". Staging answers "will validation work" at the cost of "the site is trusted"; the diagnostics added in #84 answer the same question without breaking anything. The field stays on the API for scripted use.
4. EXPIRES was a raw unix timestamp
1794055205is not a date to anyone. New askamadate/datetimefilter; the certs table now reads2026-11-07, raw value kept in thetitle.5. Fingerprint rendered an empty block
It isn't a column — the detail page was displaying
String::new(). Now read from the certificate on disk, which is also the more honest answer: it reports what is being served, not what we last remembered writing.Tests
cargo test --workspacegreen,cargo fmt --allclean. New unit tests on the date filter (including out-of-range → em dash rather than a fake date).🤖 Generated with Claude Code