Skip to content

fix(smtp): hot-reload renewed TLS certificates without restart - #524

Merged
driaug merged 2 commits into
useplunk:nextfrom
technopahadi:fix/smtp-cert-hot-reload
Oct 17, 2026
Merged

driaug merged 2 commits into
useplunk:nextfrom
technopahadi:fix/smtp-cert-hot-reload

Conversation

@technopahadi

@technopahadi technopahadi commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Description

The SMTP relay reads its TLS certificate once at startup, so renewals by Traefik/certbot are ignored until the container restarts, and ports 465/587 end up serving an expired certificate (details in #523).

This watches the certificate files and swaps a changed certificate into both servers with smtp-server's updateSecureContext(). Implicit TLS (465) and STARTTLS (587) both read the server's secure context per connection, so new connections get the new certificate right away. Open connections are unaffected and the process doesn't restart.

How it works:

  • Certificates issued after startup. If no certificate is found at startup (e.g. the container starts before Traefik finishes the ACME challenge for SMTP_DOMAIN), every source is watched. The first valid pair starts the 465 server and adds TLS to 587.
  • One source. Startup and reloads share one resolver. Once a source (acme.json or PEM files) is active, only its files are watched and reloads read only from it, so a half-written acme.json never falls back to the PEM files.
  • Stable files only. A change event records the size, mtime and inode of the watched files. A read is used only if none of them changed from the event until the read finished, at least 5s later. So a half-written fullchain.pem (e.g. leaf only) is never served. This also merges the events of several files into one reload.
  • Rollback. updateSecureContext() drops a server's live context before it builds the new one. If an update throws (e.g. a key/cert mismatch mid-renewal), the previous pair goes back on every server. No error can escape the watcher and crash the relay.
  • Quiet. Reloads log only when the certificate actually changes, so other domains' renewals in a shared acme.json are silent.
  • Polling, not inotify. fs.watchFile stats the files every 60s. That works on bind mounts and Docker Desktop and needs no new dependency.
  • Docs. PEM certificates are now mounted as a directory, since a single-file bind mount keeps the old file when a renewal replaces it. For certbot, the docs use a deploy hook that copies the files into that directory: certbot's live/ and archive/ are root-only, and the container runs as UID 1001. Traefik rewrites acme.json in place, so its single-file mount still works.

Type of Change

  • feat: New feature (MINOR version bump)
  • fix: Bug fix (PATCH version bump)
  • feat!: Breaking change - new feature (MAJOR version bump)
  • fix!: Breaking change - bug fix (MAJOR version bump)
  • docs: Documentation update (no version bump)
  • chore: Maintenance/dependencies (no version bump)
  • refactor: Code refactoring (no version bump)
  • test: Adding tests (no version bump)
  • perf: Performance improvement (PATCH version bump)

Testing

  • tsc --noEmit, eslint and prettier --check pass for apps/smtp.
  • Ran apps/smtp locally with the watch interval set to 2s and the settle delay to 3s, wrote certificate files while it ran, and checked what each port served with openssl s_client (-starttls smtp for 587). The test leaf certs are signed by a test intermediate, and fullchain.pem holds leaf + intermediate.

PEM files, starting with an empty /certs:

                                                   465                     587
0. started, no certificates                        not listening           CN=localhost
1. certA + keyB written (mismatched pair)          not listening           CN=localhost   warning, nothing changed
2. key rewritten to keyA                           CN=certA                CN=certA       465 started
3. renewal: both files rewritten to certB          CN=certB                CN=certB       one reload logged
4. only the cert rewritten to certC (key still B)  CN=certB                CN=certB       warning, rolled back
5. key rewritten to keyC                           CN=certC                CN=certC
6. certD written leaf-only, intermediate
   appended 2.5s later (inside the settle window)  CN=certC                CN=certC       partial file never served
   after settling                                  CN=certD (2 certs)      CN=certD (2 certs)
same listening PID throughout

acme.json with SMTP_DOMAIN=smtp.test:

                                                   465                     587
0. started, acme.json holds only other.test        not listening           CN=localhost
1. Traefik adds the smtp.test cert (certA)         CN=certA                CN=certA       465 started
2. PEM files (certD) added, then acme.json
   left truncated mid-write                        CN=certA                CN=certA       parse error logged, no switch to PEM
3. acme.json valid again, only other.test renewed  CN=certA                CN=certA       nothing logged
4. smtp.test renewed (certB)                       CN=certB                CN=certB       one reload logged

CN=localhost is smtp-server's built-in certificate, which 587 offers before any certificate is mounted (unchanged by this PR). The same overwrite on unpatched next (76b4efb) keeps serving certA indefinitely (output in #523).

Checklist

  • PR title follows conventional commits format
  • Code builds successfully
  • Tests pass locally (there are no existing tests for apps/smtp; verified manually as above)
  • Documentation updated (if needed)

Related Issues

Closes #523

🤖 Generated with Claude Code

The relay read its certificate once at startup, so renewals by Traefik/certbot
were ignored and ports 465/587 kept serving the old certificate until it
expired. Watch the mounted certificate files and swap renewed certificates into
both servers via SMTPServer#updateSecureContext().

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@driaug driaug left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for picking this up, hot-reloading certs is a nice improvement. A few issues before it can go in. The first three are the important ones: certs issued after startup are never picked up, the certbot mount advice doesn't work because of relative symlinks, and an uncaught error during the swap can crash the relay. Details are inline.

Comment thread apps/smtp/src/server.ts Outdated
});

// Pick up certificate renewals without a restart
if (certs) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Certificates issued after startup are never picked up. The watcher only starts when certs exists at boot. On a fresh Dokploy/Coolify deploy the container often starts before Traefik has finished the ACME challenge for SMTP_DOMAIN. getCertificates() then returns null, no watcher is started, the 465 server is never created and 587 runs without TLS. When Traefik writes the cert a minute later, nothing happens until someone restarts the container, which is the case this PR sets out to fix.

Suggestion: always start the watcher, and create (or start listening on) the 465 server the first time a valid key/cert pair appears.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 873849c. The watcher now always starts. If no certificate is found at startup, it watches every source, and the first valid pair creates the 465 server and adds TLS to 587. 587 needs no restart, because STARTTLS reads the server's secure context per connection.

Tested the Dokploy case: started with an acme.json that held only another domain, then added the SMTP_DOMAIN cert. 465 came up and both ports served the new cert.


If no certificates are mounted, SMTP runs without TLS.

Renewed certificates are picked up automatically within a minute of the mounted files changing, without restarting the container. Mount files that your ACME client updates in place, or mount the containing directory: a single-file bind mount of a file that gets replaced (such as certbot's `live/` symlinks) keeps pointing at the old certificate.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This doesn't work for certbot as written. The files in certbot's live/<domain>/ are relative symlinks (../../archive/<domain>/...). If you mount only live/<domain> as /certs, those links point at nothing inside the container, existsSync fails and SMTP silently runs without TLS. The PEM example just above still shows single-file mounts, which this paragraph says go stale.

Suggestion: tell certbot users to mount the whole /etc/letsencrypt directory and set CERT_PATH to /etc/letsencrypt/live/<domain>, and update the example to match.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch on the relative symlinks. While checking this I found two more problems for certbot users:

  • certbot creates live/ and archive/ as 0700 root (privkey*.pem is 0600), and the image runs as USER plunk (UID 1001). So even with /etc/letsencrypt mounted, the relay can't read the files.
  • The pm2 config in docker-entrypoint-nginx.sh hard-codes CERT_PATH: '/certs', so setting CERT_PATH has no effect in the image.

So instead of mounting /etc/letsencrypt, the docs now say: mount a directory at /certs (not single files), and for certbot, copy the two files into it from a --deploy-hook (the key with -o 1001). That works with the default CERT_PATH and fixes both the renewal and the permissions. I updated the PEM example to the directory mount, and the matching examples in docker-compose.yml and apps/smtp/README.md.

If you'd rather go the CERT_PATH route, the entrypoint can pass it through as '${CERT_PATH:-/certs}', but the permissions would still need a fix on the host.

Comment thread apps/smtp/src/server.ts Outdated
}

for (const server of servers) {
server.updateSecureContext({key: certs.key, cert: certs.cert});

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

A failed swap here can crash the relay. updateSecureContext runs with no try/catch inside a fs.watchFile listener. In smtp-server 3.18.3 it clears the server's secure context before building the new one from tlsOptions(this.options), which can include more options than the bare key/cert checked above. If that build throws, the server is left with no context (get('*') returns undefined) and the error escapes the watcher callback, so the process exits. With two servers, the ones updated before the error keep the new cert and the rest keep the old one.

Suggestion: wrap the loop in try/catch and put the previous key/cert back on all servers if any update fails.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 873849c. The update loop is now in a try/catch. If any updateSecureContext() throws, the previous pair goes back on every server, so no server is left without a context or on a different cert. The whole reload callback is also wrapped, so no error can escape the timer and exit the process.

For the first certificate there is no previous pair, so the 465 server is created first. Its constructor builds the context from the pair, so a bad pair throws before 587 is touched.

Tested with a cert/key mismatch while running: both ports kept the previous cert, and the process stayed up.

Comment thread apps/smtp/src/server.ts Outdated
let currentCert = initialCert;

const reload = () => {
const certs = loadFromTraefikAcme() ?? loadFromPemFiles();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

A reload can switch to a different certificate source. If the relay started on acme.json and a poll catches Traefik rewriting it in place, JSON.parse fails and this falls back to loadFromPemFiles(). If PEM files are also mounted, an older or different cert passes validation and is hot-swapped in until the next acme.json change. The reverse can happen in PEM mode.

This line also repeats the source order from getCertificates(). Suggestion: have one shared resolver that both startup and reload use, which remembers which source was active and reloads only from that source.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed. Startup and reloads now share one resolver (resolveCertificates) over one CERTIFICATE_SOURCES list. The watcher remembers which source was active and reloads only from it.

Tested: acme.json active, PEM files with a different cert present, then acme.json left truncated. Both ports kept the acme.json cert and logged the parse error, with no switch to the PEM cert.

Comment thread apps/smtp/src/server.ts Outdated
try {
// updateSecureContext() drops the live context before building the new one, so an invalid pair
// (e.g. key rewritten but cert not yet) would leave the servers without TLS - validate it first
tls.createSecureContext({key: certs.key, cert: certs.cert});

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

A half-written fullchain.pem can pass this check. If an ACME client rewrites fullchain.pem in place and the 60s poll fires partway through, the file may hold only the leaf certificate. That still parses and matches the key, so it goes live on both servers without the intermediate, and strict clients fail verification until the file changes again.

Suggestion: before swapping, check the files are stable (e.g. re-stat them and confirm size and mtime haven't changed), and/or check that the chain includes more than the leaf when the original did.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed with the stability check. A change event records the size, mtime and inode of the watched files, then waits 5s and reads. The read is used only if none of the files changed from the event until the read finished. If one did, it waits another 5s and tries again.

Tested with a poll that caught fullchain.pem holding only the leaf, then appended the intermediate 2.5s later: the leaf-only file was never served, and the full chain went live after the files settled.

I didn't add the chain-length check. It would also block a legitimate switch to a cert without intermediates, and the stability check already covers this case.

Comment thread apps/smtp/src/server.ts Outdated
signale.success('Reloaded renewed TLS certificate');
};

for (const file of [ACME_JSON_PATH, path.join(CERT_PATH, 'privkey.pem'), path.join(CERT_PATH, 'fullchain.pem')]) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Too many reloads and misleading logs. All three paths are watched no matter which source is in use, and each trigger reruns the loaders with their info/success logging. On Dokploy/Coolify, acme.json holds certs for every app on the host, so any other domain's renewal causes a synchronous read and JSON.parse of the whole file on the event loop and logs "Loaded certificate for ..." even though nothing was swapped. In PEM mode one renewal can trigger up to three reloads, one per file.

The PEM paths are also rebuilt here instead of reusing the ones loadFromPemFiles uses, so they can drift apart if the filenames ever change.

Suggestion: watch only the active source's files, share path constants with the loader, and use quiet loader variants on reload.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed:

  • Only the active source's files are watched. All sources are watched only until the first cert is found.
  • Reloads use a silent logger. "Loaded certificate for …" is now printed only at startup, and "Reloaded renewed TLS certificate" only when the cert actually changes.
  • Events from several files are merged into one reload, because each event restarts the settle timer. A PEM renewal now gives one reload.
  • The PEM paths are now PEM_KEY_PATH and PEM_CERT_PATH, shared by the loader, the startup message and the watcher.

One part remains: on Dokploy/Coolify, another domain's renewal still causes one read and JSON.parse of acme.json, because that's the only way to tell whether our cert changed. It now happens once per change, after the file settles, and logs nothing.

Comment thread apps/smtp/src/server.ts Outdated
try {
// updateSecureContext() drops the live context before building the new one, so an invalid pair
// (e.g. key rewritten but cert not yet) would leave the servers without TLS - validate it first
tls.createSecureContext({key: certs.key, cert: certs.cert});

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Minor: each successful reload builds the TLS context N+1 times (this throwaway check plus one per server inside updateSecureContext). It's cheap, but if you add the try/catch-and-restore around the update loop (see the comment on line 171), this pre-check is no longer needed.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Removed. The rollback covers this case now.

Review follow-up for the certificate hot-reload:

- Watch for certificates even when none were found at startup, and start
  the port 465 server when the first valid pair appears.
- Reload only from the source that was active at startup (no fallback from
  a half-written acme.json to PEM files), watch only that source's files,
  and load quietly on reload.
- Use a read only if no watched file changed from the change event until
  the read finished, at least 5s later, so a half-written file is never
  served. Coalesces the per-file change events into one reload.
- If updateSecureContext() throws, put the previous pair back on every
  server instead of pre-validating the pair, and never let an error escape
  the watcher.
- Docs: mount the PEM directory instead of single files; for certbot, copy
  the files from a deploy hook, since certbot's live/ and archive/ are
  root-only and the container runs as UID 1001.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@technopahadi

Copy link
Copy Markdown
Contributor Author

Thanks for the thorough review! Pushed 873849c, which addresses all seven comments (replies inline). I also updated the PR description with the new behaviour and the re-test results, including the cases from your comments.

@driaug
driaug merged commit bc84b4a into useplunk:next Oct 17, 2026
2 checks passed
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.

SMTP relay never reloads TLS certificates after renewal (serves expired cert until restart)

2 participants