Automatic Letsencrypt certificate refresh by j-chmielewski · Pull Request #2730 · DefGuard/defguard

j-chmielewski · 2026-04-16T12:11:45Z

No description provided.

Copilot

Pull request overview

This PR adds an automated Let's Encrypt certificate renewal/refresh mechanism for the Edge (proxy) certificate, including admin email notifications (with logs attached) on ACME failures, and refactors shared ACME helper code into a dedicated core module.

Changes:

Add periodic (daily) LetsEncrypt certificate expiry checks in the utility thread and trigger ACME refresh when within a threshold.
Introduce a new defguard_core::letsencrypt module that encapsulates ACME trigger, timeout handling, cert persistence, and notification logic.
Add a new mail template + DB migration entries to notify admins when automatic certificate refresh fails (including logs attachment).

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
migrations/20260417073540_[2.0.0]_letsencrypt_cert_refresh.up.sql	Adds `mail_context` rows for the new failure notification template.
migrations/20260417073540_[2.0.0]_letsencrypt_cert_refresh.down.sql	Removes the inserted `mail_context` rows for rollback.
crates/defguard_mail/templates/letsencrypt-cert-refresh-failed.text	Adds plaintext template for refresh-failure notification.
crates/defguard_mail/templates/letsencrypt-cert-refresh-failed.mjml	Adds MJML template for refresh-failure notification.
crates/defguard_mail/src/templates.rs	Adds mail-sending function for refresh-failure notifications (with log attachment).
crates/defguard_mail/src/mail.rs	Adds new `MailMessage` variant and wires it to templates + subject.
crates/defguard_core/src/utility_thread.rs	Schedules daily LE expiry checks and triggers refresh.
crates/defguard_core/src/lib.rs	Exposes new `letsencrypt` module.
crates/defguard_core/src/letsencrypt.rs	Implements refresh logic, proxy ACME trigger, cert persistence, and admin notification (plus tests).
crates/defguard_core/src/handlers/component_setup.rs	Reuses the shared LE/ACME helpers from the new module for the SSE setup flow.
crates/defguard_common/src/db/models/settings.rs	Adds `Settings::proxy_hostname()` helper + new `SettingsUrlError` variants.
crates/defguard/src/main.rs	Updates utility thread startup to pass `proxy_control_tx`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-17T12:00:31Z

+    pub fn proxy_hostname(&self) -> Result<String, SettingsUrlError> {
+        let url = self
+            .proxy_public_url()
+            .map_err(|_err| SettingsUrlError::UnparsableEdgeUrl(self.public_proxy_url.clone()))?;
+        let hostname = url
+            .host_str()
+            .ok_or_else(|| SettingsUrlError::EdgeUrlMissingHostname(self.public_proxy_url.clone()))?
+            .to_string();


proxy_hostname() doesn’t special-case an empty/whitespace public_proxy_url. If it’s empty, the user-facing error becomes Unparsable Edge url: , which is much less actionable than the previous behavior in the ACME flow. Consider trimming + explicitly erroring on empty, with guidance (e.g., “Public Edge URL is not configured…”) before attempting Url::parse.

Copilot · 2026-04-17T12:00:31Z

+    for user in admin_users {
+        templates::letsencrypt_cert_refresh_failed_mail(&user.email, &mut conn, &logs.join("\n"))


logs.join("\n") creates a temporary String and you’re passing a reference to it into an async call that is awaited. This pattern typically fails to compile (borrowed temporary does not live long enough across .await). Assign the joined logs to a local variable (e.g., let joined = logs.join("\n");) and pass &joined into the mail function (or change the mail function to take an owned String).

Suggested change

for user in admin_users {

templates::letsencrypt_cert_refresh_failed_mail(&user.email, &mut conn, &logs.join("\n"))

let joined_logs = logs.join("\n");

for user in admin_users {

templates::letsencrypt_cert_refresh_failed_mail(&user.email, &mut conn, &joined_logs)

Copilot · 2026-04-17T12:00:31Z

+    tokio::spawn(async move {
+        let result = call_proxy_trigger_acme(
+            &pool_clone,
+            &proxy_host,
+            proxy_port,


tokio::spawn is used for the ACME RPC without keeping a JoinHandle. If the timeout branch triggers (see the sleep_until(deadline) arm below), do_letsencrypt_refresh returns but the spawned task keeps running in the background (potentially holding an open gRPC stream / keeping the Edge busy). Consider storing the JoinHandle and aborting it on timeout, or wrapping the entire call_proxy_trigger_acme future in tokio::time::timeout so it is actually cancelled.

Copilot · 2026-04-17T12:00:32Z

+use tokio::sync::mpsc::{self, UnboundedSender, unbounded_channel};
+use tonic::{
+    Request,
+    service::Interceptor,


tonic::service::Interceptor is imported but not used in this module (the client uses an inline closure interceptor). Removing the unused import will avoid warnings (and potential CI failures if warnings are denied).

Suggested change

service::Interceptor,

Copilot · 2026-04-17T12:00:32Z

+    let attachment = Attachment::new(
+        format!("defguard-letsencrypt-refresh-logs-{now}.txt"),


The attachment filename uses {now} via DateTime’s default Display formatting, which includes spaces and :. Those characters are commonly problematic in filenames/content-disposition handling across clients. Consider formatting the timestamp into a safe, filesystem/email-friendly form (e.g., now.format("%Y%m%dT%H%M%SZ")).

Suggested change

let attachment = Attachment::new(

format!("defguard-letsencrypt-refresh-logs-{now}.txt"),

let timestamp = now.format("%Y%m%dT%H%M%SZ");

let attachment = Attachment::new(

format!("defguard-letsencrypt-refresh-logs-{timestamp}.txt"),

Copilot · 2026-04-17T12:00:32Z

+    ('letsencrypt-cert-refresh-failed', 'title', 'en_US', 'Letsencrypt certificate refresh failed'),
+    ('letsencrypt-cert-refresh-failed', 'content', 'en_US', 'Automatic Letsencrypt certificate refresh has failed. Please verify your Edge setup.');


User-facing email copy uses "Letsencrypt"; elsewhere in the codebase docs/messages use "Let's Encrypt". Consider updating the inserted mail_context text to the standard branding/spelling for consistency.

Suggested change

('letsencrypt-cert-refresh-failed', 'title', 'en_US', 'Letsencrypt certificate refresh failed'),

('letsencrypt-cert-refresh-failed', 'content', 'en_US', 'Automatic Letsencrypt certificate refresh has failed. Please verify your Edge setup.');

('letsencrypt-cert-refresh-failed', 'title', 'en_US', 'Let''s Encrypt certificate refresh failed'),

('letsencrypt-cert-refresh-failed', 'content', 'en_US', 'Automatic Let''s Encrypt certificate refresh has failed. Please verify your Edge setup.');

Copilot · 2026-04-17T12:00:33Z

            Self::UserImportBlocked => "User import blocked".to_string(),
            Self::EnrollmentNotification => "Defguard: User enrollment completed".to_string(),
+            Self::LetsencryptCertRefreshFailed => {
+                "Defguard: automatic Letsencrypt certificate refresh failed".to_string()


Email subject uses "Letsencrypt". For consistency with other user-facing text in the repo, consider using the standard "Let's Encrypt" spelling/capitalization.

Suggested change

"Defguard: automatic Letsencrypt certificate refresh failed".to_string()

"Defguard: automatic Let's Encrypt certificate refresh failed".to_string()

Copilot · 2026-04-17T12:00:33Z

@@ -0,0 +1 @@
+DELETE FROM mail_context where "template" = 'letsencrypt-cert-refresh-failed';


In migrations, other mail_context deletions use unquoted column names and uppercase WHERE (e.g., migrations/20260331114131_[2.0.0]_fix_mjml_label.up.sql:1). For consistency, consider DELETE FROM mail_context WHERE template = ... here as well (quotes on "template" aren’t needed).

Suggested change

DELETE FROM mail_context where "template" = 'letsencrypt-cert-refresh-failed';

DELETE FROM mail_context WHERE template = 'letsencrypt-cert-refresh-failed';

j-chmielewski added 15 commits April 16, 2026 14:11

initial implementation of LE cert refresh utility thread

417d909

cleanup

55fbeb7

update proxy cert after successful refresh

6ff0c3f

send email on failure

96d1579

fix the template

e217436

letsencrypt module

8c93c99

move shared code to letsencrypt mod

d3a19c8

refactor proxy_hostname function

cbd74b7

fix imports

b57757d

dedicated LetsencryptError

85c4b98

tests

35c7f1a

docs

4ab7593

remove unnecessary args

2360063

Merge branch 'release/2.0' into refresh-le-certs

ad60b92

check le certs every 24 hours

4cee608

j-chmielewski changed the title ~~initial implementation of LE cert refresh utility thread~~ Automatic Letsencrypt certificate refresh Apr 17, 2026

j-chmielewski marked this pull request as ready for review April 17, 2026 11:52

j-chmielewski requested a review from Copilot April 17, 2026 11:53

Copilot started reviewing on behalf of j-chmielewski April 17, 2026 11:54 View session

Copilot AI reviewed Apr 17, 2026

View reviewed changes

terminate acme thread on timeout

e14c4f3

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Automatic Letsencrypt certificate refresh#2730

Automatic Letsencrypt certificate refresh#2730
j-chmielewski wants to merge 16 commits intorelease/2.0from
refresh-le-certs

j-chmielewski commented Apr 16, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		for user in admin_users {
		templates::letsencrypt_cert_refresh_failed_mail(&user.email, &mut conn, &logs.join("\n"))

		let attachment = Attachment::new(
		format!("defguard-letsencrypt-refresh-logs-{now}.txt"),

		('letsencrypt-cert-refresh-failed', 'title', 'en_US', 'Letsencrypt certificate refresh failed'),
		('letsencrypt-cert-refresh-failed', 'content', 'en_US', 'Automatic Letsencrypt certificate refresh has failed. Please verify your Edge setup.');

	"Defguard: automatic Letsencrypt certificate refresh failed".to_string()
	"Defguard: automatic Let's Encrypt certificate refresh failed".to_string()

		@@ -0,0 +1 @@
		DELETE FROM mail_context where "template" = 'letsencrypt-cert-refresh-failed';

	DELETE FROM mail_context where "template" = 'letsencrypt-cert-refresh-failed';
	DELETE FROM mail_context WHERE template = 'letsencrypt-cert-refresh-failed';

Uh oh!

Conversation

j-chmielewski commented Apr 16, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants