From 3e9499f62c4fb621ec234a0cf5cee51eb2cf32c0 Mon Sep 17 00:00:00 2001 From: Craig Jennings Date: Sun, 3 May 2026 19:57:30 -0500 Subject: fix: validate mail transport executables and default debug off `mail-config.el` had three related issues. SMTP transport debug was hard-coded to t, which is sensitive since mail bodies and headers land in debug buffers. The use-package `:config` was also setting `sendmail-program` and `mu4e-get-mail-command` directly from `executable-find` results. So a host without msmtp or mbsync silently got `nil` or `(concat nil " -a")` instead of a clear failure mode. I added `cj/smtpmail-debug-enabled` (default nil) plus `cj/set-smtpmail-debug` and `cj/toggle-smtpmail-debug` for temporary troubleshooting, mirroring the pattern from `auth-config.el`. I extracted `cj/mail--executable-or-warn` so a missing program emits a one-time `display-warning` and returns nil. `cj/mail-configure-smtpmail` and `cj/mail--mbsync-command` both use it. Missing msmtp now leaves `sendmail-program` nil with a warning. Missing mbsync produces a nil sync command instead of the broken `(concat nil " -a")` string. I also wrapped the mbsync executable path in `shell-quote-argument` so unusual install paths don't fall apart on the `" -a"` concat. I added `tests/test-mail-config-transport.el` with seven tests across Normal / Boundary / Error: debug-default-off, toggle wiring, msmtp present and missing, mbsync present, mbsync path with spaces, and mbsync missing. The `test-mail-config--with-executables` macro stubs `executable-find` from an alist so each test names its own environment. --- modules/mail-config.el | 64 ++++++++++++++++++++++++--- tests/test-mail-config-transport.el | 88 +++++++++++++++++++++++++++++++++++++ 2 files changed, 146 insertions(+), 6 deletions(-) create mode 100644 tests/test-mail-config-transport.el diff --git a/modules/mail-config.el b/modules/mail-config.el index a53473b4..f19adf80 100644 --- a/modules/mail-config.el +++ b/modules/mail-config.el @@ -34,6 +34,62 @@ (eval-and-compile (defvar cj/custom-keymap (make-sparse-keymap))) +(defvar smtpmail-debug-info nil) +(defvar sendmail-program nil) +(defvar send-mail-function nil) +(defvar message-send-mail-function nil) +(defvar message-sendmail-envelope-from nil) + +(defcustom cj/smtpmail-debug-enabled nil + "Non-nil means enable verbose SMTP transport debug logging. + +Keep this nil during normal startup. SMTP debug output is useful for +troubleshooting mail delivery problems, but it can expose sensitive mail +transport details in debug buffers." + :type 'boolean + :group 'mail) + +(defun cj/set-smtpmail-debug (enabled) + "Set SMTP transport debug logging according to ENABLED." + (interactive + (list (y-or-n-p "Enable SMTP transport debug logging? "))) + (setq cj/smtpmail-debug-enabled enabled) + (setq smtpmail-debug-info enabled) + (message "SMTP transport debug logging %s" + (if enabled "enabled" "disabled"))) + +(defun cj/toggle-smtpmail-debug () + "Toggle verbose SMTP transport debug logging for troubleshooting." + (interactive) + (cj/set-smtpmail-debug (not smtpmail-debug-info))) + +(defun cj/mail--executable-or-warn (program feature) + "Return PROGRAM's executable path, or warn that FEATURE is unavailable." + (or (executable-find program) + (progn + (display-warning + 'mail-config + (format "%s not found; %s unavailable" program feature) + :warning) + nil))) + +(defun cj/mail--mbsync-command () + "Return the mu4e mail sync command, or nil if mbsync is unavailable." + (when-let ((mbsync (cj/mail--executable-or-warn + "mbsync" "mu4e mail synchronization"))) + (concat (shell-quote-argument mbsync) " -a"))) + +(defun cj/mail-configure-smtpmail () + "Configure SMTP mail transport when msmtp is available." + (setq smtpmail-debug-info cj/smtpmail-debug-enabled) + (if-let ((msmtp (cj/mail--executable-or-warn + "msmtp" "SMTP mail sending"))) + (setq sendmail-program msmtp + send-mail-function 'message-send-mail-with-sendmail + message-send-mail-function 'message-send-mail-with-sendmail + message-sendmail-envelope-from 'header) + (setq sendmail-program nil))) + ;; -------------------- HarfBuzz Crash Fix: Disable Composition --------------- ;; Disable auto-composition in mu4e headers to prevent SIGSEGV from HarfBuzz ;; when shaping emoji characters in email subjects. See Commentary above. @@ -63,11 +119,7 @@ Prompts user for the action when executing." :ensure nil ;; built-in :defer .5 :config - (setq sendmail-program (executable-find "msmtp")) - (setq send-mail-function 'message-send-mail-with-sendmail - message-send-mail-function 'message-send-mail-with-sendmail) - (setq message-sendmail-envelope-from 'header) - (setq smtpmail-debug-info t)) + (cj/mail-configure-smtpmail)) ;; --------------------------------- Mu4e Email -------------------------------- @@ -119,7 +171,7 @@ Prompts user for the action when executing." (setq mu4e-html2text-command 'mu4e-shr2text) ;; email conversion to html via shr2text (setq mu4e-mu-binary (executable-find "mu")) - (setq mu4e-get-mail-command (concat (executable-find "mbsync") " -a")) ;; command to sync mail + (setq mu4e-get-mail-command (cj/mail--mbsync-command)) ;; command to sync mail (setq mu4e-user-mail-address-list '("c@cjennings.net" "craigmartinjennings@gmail.com" "craig.jennings@deepsat.com")) diff --git a/tests/test-mail-config-transport.el b/tests/test-mail-config-transport.el new file mode 100644 index 00000000..2244b6dd --- /dev/null +++ b/tests/test-mail-config-transport.el @@ -0,0 +1,88 @@ +;;; test-mail-config-transport.el --- Tests for mail transport setup -*- lexical-binding: t; -*- + +;;; Commentary: +;; Verifies mail transport debug logging is opt-in and external mail +;; executables are validated before command variables are assigned. + +;;; Code: + +(require 'ert) +(require 'cl-lib) + +(add-to-list 'load-path (expand-file-name "modules" user-emacs-directory)) +(require 'mail-config) + +(defmacro test-mail-config--with-executables (executables &rest body) + "Run BODY with `executable-find' returning paths from EXECUTABLES. +EXECUTABLES is an alist of program name strings to executable paths." + (declare (indent 1)) + `(let (test-mail-config--warnings) + (cl-letf (((symbol-function 'executable-find) + (lambda (program) + (cdr (assoc program ,executables)))) + ((symbol-function 'display-warning) + (lambda (type message &rest _args) + (push (cons type message) test-mail-config--warnings)))) + ,@body))) + +(ert-deftest test-mail-config-transport-smtpmail-debug-defaults-off () + "SMTP transport debug logging should default to disabled." + (test-mail-config--with-executables '(("msmtp" . "/usr/bin/msmtp")) + (let ((cj/smtpmail-debug-enabled nil) + (smtpmail-debug-info :before-load)) + (cj/mail-configure-smtpmail) + (should-not smtpmail-debug-info)))) + +(ert-deftest test-mail-config-transport-smtpmail-debug-toggle () + "Troubleshooting command should update both SMTP debug variables." + (let ((cj/smtpmail-debug-enabled nil) + (smtpmail-debug-info nil)) + (cl-letf (((symbol-function 'message) (lambda (&rest _args) nil))) + (cj/set-smtpmail-debug t) + (should cj/smtpmail-debug-enabled) + (should smtpmail-debug-info) + (cj/set-smtpmail-debug nil) + (should-not cj/smtpmail-debug-enabled) + (should-not smtpmail-debug-info)))) + +(ert-deftest test-mail-config-transport-msmtp-present-configures-sendmail () + "When msmtp exists, sendmail transport variables should be configured." + (test-mail-config--with-executables '(("msmtp" . "/usr/bin/msmtp")) + (let (sendmail-program send-mail-function message-send-mail-function + message-sendmail-envelope-from) + (cj/mail-configure-smtpmail) + (should (equal sendmail-program "/usr/bin/msmtp")) + (should (eq send-mail-function 'message-send-mail-with-sendmail)) + (should (eq message-send-mail-function 'message-send-mail-with-sendmail)) + (should (eq message-sendmail-envelope-from 'header)) + (should-not test-mail-config--warnings)))) + +(ert-deftest test-mail-config-transport-msmtp-missing-warns-and-disables-program () + "When msmtp is missing, sendmail-program should be nil and a warning emitted." + (test-mail-config--with-executables nil + (let ((sendmail-program "/old/msmtp")) + (cj/mail-configure-smtpmail) + (should-not sendmail-program) + (should (equal test-mail-config--warnings + '((mail-config . "msmtp not found; SMTP mail sending unavailable"))))))) + +(ert-deftest test-mail-config-transport-mbsync-present-builds-command () + "When mbsync exists, build the mu4e sync command." + (test-mail-config--with-executables '(("mbsync" . "/usr/bin/mbsync")) + (should (equal (cj/mail--mbsync-command) "/usr/bin/mbsync -a")) + (should-not test-mail-config--warnings))) + +(ert-deftest test-mail-config-transport-mbsync-path-with-spaces-is-quoted () + "The mu4e sync command should quote unusual executable paths." + (test-mail-config--with-executables '(("mbsync" . "/opt/mail tools/mbsync")) + (should (equal (cj/mail--mbsync-command) "/opt/mail\\ tools/mbsync -a")))) + +(ert-deftest test-mail-config-transport-mbsync-missing-returns-nil-and-warns () + "When mbsync is missing, no unusable sync command should be assigned." + (test-mail-config--with-executables nil + (should-not (cj/mail--mbsync-command)) + (should (equal test-mail-config--warnings + '((mail-config . "mbsync not found; mu4e mail synchronization unavailable")))))) + +(provide 'test-mail-config-transport) +;;; test-mail-config-transport.el ends here -- cgit v1.2.3