FIX(client): Explain self-signed certificates in SSL verification dialog

[64johnlee]

Summary

Closes #7140

• When the certificate error list includes QSslError::SelfSignedCertificate or QSslError::SelfSignedCertificateInChain, an extra explanatory paragraph is appended to the Path A (certificate verification failed) dialog
• The note explains what a self-signed certificate is, reassures the user they can accept it if they trust the server, and points server admins to Let's Encrypt
• When neither self-signed error code is present the dialog is byte-for-byte identical to before

Changes

• src/mumble/MainWindow.cpp — detect self-signed errors inside the existing qlErrors loop (single pass, no second iteration), build selfSignedNote, inject it as %4 into the dialog template

Test plan

• Connect to a server with a self-signed certificate → verify the new explanatory paragraph appears in the warning dialog
• Connect to a server with an untrusted CA-signed certificate (non-self-signed) → verify the dialog is unchanged (no extra paragraph)
• Accept the certificate on a self-signed server → verify Mumble stores the digest and reconnects normally
• Deny the certificate → verify disconnect behaves as before
• Verify the Let's Encrypt URL appears as plain copyable text (not a dead hyperlink)

🤖 Generated with Claude Code

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 48bb97e3-0781-4065-9a09-e5959cd84ebf

📥 Commits

Reviewing files that changed from the base of the PR and between ea64b12 and f333b57.

📒 Files selected for processing (1)

• src/mumble/MainWindow.cpp

---

Walkthrough

The PR modifies the SSL certificate verification failure dialog shown during disconnect/reconnect scenarios in MainWindow.cpp. The change adds detection logic to identify when encountered QSslError values indicate a self-signed certificate (including chains). When detected, the code builds an additional HTML note and includes it as an extra message argument to the warning dialog, providing users with context about the self-signed certificate situation.

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Linked Issues check	❓ Inconclusive	The PR addresses issue #7140 by detecting self-signed certificate errors and displaying explanatory text to users; however, it implements client-side changes rather than the server-side detection of domain-based access mentioned in the issue.	Clarify whether the client-side certificate explanation adequately addresses the issue's objectives, or if server-side domain detection functionality is still pending in a separate implementation.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main change: explaining self-signed certificates in the SSL verification dialog, which directly relates to the code modifications.
Description check	✅ Passed	The pull request description is comprehensive, covering the summary, changes made, test plan, and includes a reference to the closed issue (#7140), though the commit guidelines checklist is not explicitly confirmed.
Out of Scope Changes check	✅ Passed	All changes are focused on enhancing the SSL verification dialog for self-signed certificates on the client side, which is directly related to the primary objective of explaining certificate issues to users.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

• Generate code and open pull requests
• Plan features and break down work
• Investigate incidents and troubleshoot customer tickets together
• Automate recurring tasks and respond to alerts with triggers
• Summarize progress and report instantly

Built for teams:

• Shared memory across your entire org—no repeating context
• Per-thread sandboxes to safely plan and execute work
• Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[Krzmbrzl]

@64johnlee have you manually reviewed/verified the code that Claude generated?

[64johnlee]

Yes — I used Claude as a coding assistant but reviewed the diff carefully before committing. Here's what the change does:

• Lines 3690–3698: The original single-purpose loop that built the error list (qsl) now also sets bSelfSigned = true if any error is QSslError::SelfSignedCertificate or SelfSignedCertificateInChain — single pass, no double iteration.
• Lines 3699–3708: selfSignedNote is empty by default. If bSelfSigned is true, it becomes the explanatory paragraph with Let's Encrypt as plain text (not a hyperlink — the audit caught that QMessageBox suppresses link clicks by default since setOpenExternalLinks defaults to false).
• Lines 3710–3719: %4 is added to the template between the error list and the accept question. When selfSignedNote is empty, %4 expands to an empty string and the dialog is identical to before for non-self-signed errors.
• Lines 3721–3739: Default button (No), escape button, View Certificate button, and the digest-pin logic are all untouched.

I also noted the CodeRabbit flag about server-side domain detection — your own comment in the issue suggested focusing on a clearer client-side message for non-experts first, which is what this addresses.

[Hartmnt]

@64johnlee Using an LLM to write that response does not really spark confidence tbh

[Hartmnt]

This is the u2014 incident all over again. Slightly less funny this time though.

[64johnlee]

Fixed. — is gone — the sentence now ends with a full stop instead, so no HTML entity or control character appears inside any tr() call.

[64johnlee]

Hi @Hartmnt, fair point. I did use Claude Code as an assistance tool, but I reviewed everything carefully before committing. The mutable removal, the Let's Encrypt link issue, and the HTML-in-tr() pattern were all things I understood and verified. Happy to walk through any specific part of the code if that would help build confidence. Thanks for the honest feedback.

[64johnlee]

Fixed the translation CI failure — the two new source strings (self-signed cert explanation paragraph + modified accept-cert dialog with %4 argument) weren't reflected in the .ts files. Ran scripts/updatetranslations.py to regenerate all 45 locale files. Pushed as 5f535a8.

[64johnlee]

Thanks for the detailed feedback. Pushed an update:

• Moved the HTML out of the tr() string — it now uses QString("<p>%1</p>").arg(tr("plain text")) per the pattern you described.
• Replaced — with a literal em dash kept outside the translatable portion.
• Removed the Let's Encrypt name and URL entirely to avoid the sponsored-feel and dead-link risk.
• Updated all 45 .ts translation source entries to match the new string.

Let me know if anything else needs adjusting.

[64johnlee]

Friendly ping on this one — it's a small, self-contained client UX fix (clarifies the SSL verification dialog wording for self-signed certificates). All CI is green (Azure PR/Docs/Translations + FreeBSD), no conflicts. It's just waiting on a maintainer review whenever someone has a moment. Happy to adjust the wording or rebase if needed. Thanks!