maddy: add new package - #30340
Conversation
1e8b575 to
5ee7c49
Compare
openwrt-ai
left a comment
There was a problem hiding this comment.
Reviewed 1 new commit (new package maddy 0.9.5 — tag v0.9.5 verified upstream, COPYING present, GPL-3.0-or-later matches the source headers, cmd/maddy is the right build target).
Two findings I'd consider merge-relevant: the conffiles entry doesn't actually protect /etc/maddy/maddy.conf across package upgrades, and the version isn't stamped into the binary (which also bears on the CI runtime version check). The USERID comment is a design question for the maintainer rather than a defect. The rest is marked nit: and optional.
The shipped files/maddy.conf is byte-identical to upstream's maddy.conf at v0.9.5, so I did not review its contents.
Generated by Claude Code
| PKG_BUILD_FLAGS:=no-mips16 | ||
|
|
||
| GO_PKG:=github.com/foxcpp/maddy | ||
| GO_PKG_BUILD_PKG:=github.com/foxcpp/maddy/cmd/maddy |
There was a problem hiding this comment.
The built binary won't know its own version. maddy carries Version = "go-build" in maddy.go:84 and upstream stamps it at link time (build.sh:126). Without the -X flag maddy version prints go-build, and so do the "server started" log line and the ACME/certmagic User-Agent (maddy/go-build).
| GO_PKG_BUILD_PKG:=github.com/foxcpp/maddy/cmd/maddy | |
| GO_PKG_BUILD_PKG:=github.com/foxcpp/maddy/cmd/maddy | |
| GO_PKG_LDFLAGS_X:=$(GO_PKG).Version=$(PKG_VERSION) |
Separately, this alone won't make the CI generic version check pass: maddy never sets app.Version, so urfave/cli hides --version, and the -v flag is registered on the run subcommand only — the version is reachable only via maddy version. A test-version.sh in mail/maddy/ is likely needed to override the generic check.
Generated by Claude Code
| endef | ||
|
|
||
| define Package/maddy/conffiles | ||
| /etc/maddy |
There was a problem hiding this comment.
This entry doesn't protect maddy.conf on upgrade. /etc/maddy is a directory (and a directory entry would need a trailing slash, cf. mail/dovecot/Makefile:61-63), but the packaging only records a conffile checksum for entries that are regular files — [ -f $(IDIR)/$file ] || continue in include/package-pack.mk:579-586. A non-file entry only ends up in /lib/upgrade/keep.d, so a user-edited /etc/maddy/maddy.conf gets overwritten by a package upgrade. List the file that is actually installed:
| /etc/maddy | |
| /etc/maddy/maddy.conf |
Generated by Claude Code
| CATEGORY:=Mail | ||
| TITLE:=maddy | ||
| URL:=https://github.com/foxcpp/maddy | ||
| USERID:=maddy:maddy |
There was a problem hiding this comment.
USERID creates a maddy user and group that nothing ever uses: the init script sets no procd_set_param user/group, and it creates the runtime and state directories as root (maddy.init:14-15), so the daemon — an internet-facing SMTP/IMAP server — runs as root while an unused account is added to /etc/passwd.
Either drop USERID, or actually drop privileges. If you go the second way, the runtime/state dirs and /etc/maddy need to be owned by maddy, and binding 25/143/465/587/993 as non-root needs CAP_NET_BIND_SERVICE (upstream's systemd unit grants exactly that).
Generated by Claude Code
| define Package/maddy | ||
| SECTION:=mail | ||
| CATEGORY:=Mail | ||
| TITLE:=maddy |
There was a problem hiding this comment.
nit: TITLE is the one-line description shown in menuconfig, so repeating the package name there tells a user nothing. Optional, non-blocking:
| TITLE:=maddy | |
| TITLE:=Composable all-in-one mail server |
Generated by Claude Code
| procd_append_param command run | ||
| procd_set_param stdout 1 | ||
| procd_set_param stderr 1 | ||
| procd_set_param respawn |
There was a problem hiding this comment.
nit: procd isn't told about the config file, so editing /etc/maddy/maddy.conf doesn't mark the service as needing a restart (procd_set_param file is used by ~90 init scripts in this feed):
| procd_set_param respawn | |
| procd_set_param file "$CONF_FILE" | |
| procd_set_param respawn |
Also NAME=maddy on line 6 is set but never referenced — either use it (e.g. for procd_open_instance) or drop it.
Generated by Claude Code
5ee7c49 to
b13caf7
Compare
| define Package/maddy/description | ||
| Composable all-in-one mail server | ||
| endef |
There was a problem hiding this comment.
Good point, I have replaced the description.
BKPepe
left a comment
There was a problem hiding this comment.
I am not convinced about the use case for running a full mail server on an OpenWrt router.
OpenWrt already provides lightweight SMTP clients such as msmtp, which are sufficient for the common router use case of sending system notifications through an external SMTP server.
Maddy is a fundamentally different class of software: it is a full MTA/MX and IMAP server, intended to receive and store mail and perform direct outbound delivery. This raises the question of what the intended OpenWrt use case is and what kind of devices are expected to run it.
I would also like to see the expected installed size and memory/storage requirements. Adding a Go-based full mail server is a relatively substantial addition compared to the existing SMTP clients, so it would be useful to document the resource footprint and target hardware.
For a typical public MX deployment, the router would also need stable public connectivity, DNS/MX configuration, port 25 reachability, etc., which seems like a fairly niche use case for an OpenWrt device.
openwrt-ai
left a comment
There was a problem hiding this comment.
Re-reviewed the force-pushed commit (5ee7c49 → b13caf7). The previous round's findings are addressed: conffiles now lists the file rather than the directory, GO_PKG_LDFLAGS_X stamps the version (the x86_64 runtime job passing confirms maddy version prints 0.9.5), TITLE is descriptive, the dead NAME= is gone, procd_set_param file is set, and USERID is now actually used — the daemon drops to maddy:maddy under a ujail with CAP_NET_BIND_SERVICE from /etc/capabilities/maddy.json, no_new_privs, and the config dir chowned so the 0600 conffile stays readable. That is a good hardening story and closely mirrors net/adguardhome.
Switching $(INSTALL_CONF) to $(PKG_BUILD_DIR)/maddy.conf is fine — golang-build.sh copies rather than moves the sources, so the tarball's maddy.conf is still there at install time.
One merge-relevant gap in the new hardening: nothing provides or exposes a CA trust store, so MTA-STS policy fetches (enabled in the shipped default config) cannot verify certificates. Two inline comments cover the DEPENDS and the jail mount. The test-version.sh indentation is marked nit: and optional.
Not repeating @BKPepe's note on the package description.
Generated by Claude Code
| TITLE:=Composable all-in-one mail server | ||
| URL:=https://github.com/foxcpp/maddy | ||
| USERID:=maddy:maddy | ||
| DEPENDS:=$(GO_ARCH_DEPENDS) |
There was a problem hiding this comment.
No system trust store is pulled in, so on a minimal image /etc/ssl/certs/ca-certificates.crt does not exist and Go's crypto/x509 ends up with an empty root pool. The default config now installed from the tarball enables MTA-STS in target.remote's mx_auth (mtasts { cache fs ... }), which fetches https://mta-sts.<domain>/.well-known/mta-sts.txt over HTTPS — those lookups will fail certificate verification. The same applies to any ACME setup a user switches the tls directive to later.
Both Go network daemons in this feed that do outbound TLS declare it: net/dnsproxy/Makefile:35 and net/adguardhome/Makefile:45.
| DEPENDS:=$(GO_ARCH_DEPENDS) | |
| DEPENDS:=$(GO_ARCH_DEPENDS) +ca-bundle |
Installing it is only half of it — the jail also has to mount it; see my comment on files/maddy.init.
Generated by Claude Code
| procd_set_param respawn | ||
| procd_set_param file "$CONF_FILE" | ||
| procd_add_jail maddy | ||
| procd_add_jail_mount $CONF_DIR |
There was a problem hiding this comment.
The jail mounts only /etc/maddy, /run/maddy and /var/lib/maddy, so even once ca-bundle is a dependency the CA store is invisible inside the jail and outbound HTTPS (MTA-STS policy fetches, ACME) still fails. The two jailed Go daemons in this feed both mount it explicitly — adguardhome.init:93 and dnsproxy.init:143.
| procd_add_jail_mount $CONF_DIR | |
| procd_add_jail_mount $CONF_DIR | |
| procd_add_jail_mount /etc/ssl/certs |
Related, and worth a thought before this lands: the mount list is fixed, so it only works as long as everything maddy touches lives under /etc/maddy. The shipped config's tls file /etc/maddy/certs/$(hostname)/... satisfies that, but a user pointing it at acme's /etc/ssl/acme/ (the usual way to get certs on OpenWrt) gets a "no such file" inside the jail with no way to fix it from config. adguardhome solves this with UCI jail_mount / jail_mount_rw lists (adguardhome.init:94-95); a /etc/config/maddy with the same escape hatch, or at minimum mounting /etc/ssl/acme, would avoid the dead end.
Generated by Claude Code
| maddy) | ||
| maddy version | grep -F "$PKG_VERSION" | ||
| ;; | ||
| *) | ||
| echo "Untested package: $PKG_NAME" >&2 | ||
| exit 1 | ||
| ;; |
There was a problem hiding this comment.
nit: indented with four spaces; the script this is modelled on (utils/dbus/test-version.sh, same # shellcheck shell=busybox header and same case/*) fallback) and the other test-version.sh scripts in the feed use tabs — as does the maddy.json added in this PR. Optional, non-blocking.
| maddy) | |
| maddy version | grep -F "$PKG_VERSION" | |
| ;; | |
| *) | |
| echo "Untested package: $PKG_NAME" >&2 | |
| exit 1 | |
| ;; | |
| maddy) | |
| maddy version | grep -F "$PKG_VERSION" | |
| ;; | |
| *) | |
| echo "Untested package: $PKG_NAME" >&2 | |
| exit 1 | |
| ;; |
Generated by Claude Code
I can easily conceive that a full mail server is a niche use case for OpenWrt deployments, and it is certainly not suited to all devices. I have this use case for a small number of users and I thought that this might be of use to others. OpenWrt already includes other tools (dovecot, postfix, opendkim, etc) for a complete MX/MTA/IMAP/POP3 deployment, and I think an alternative could be welcome.
As for the intended use case, I can only speak for myself. I have a Banana Pi R4 board that hosts a number of services (mainly email, git hosting and file sharing) for a bit more than 10 users. On such a board, RAM and flash memory are quite sufficient (4 GB of DDR4 RAM and 8 GB of eMMC + a M.2 slot for mass storage). I expect maddy to be useful on the same kind of devices, rather than on low end devices.
The size of the package itself should be around 10 MB (again, not suited for devices with small flash) :
Storage for the received emails can theoretically grow infinitely, so the requirements depend on the actual use case. Actual memory usage is harder to determine precisely since the Go runtime reserves memory plentifully. I should be able to do some profiling of the process in the next few days. There are already some Go based packages, such as Syncthing, in the repository, so I believe maddy would be targeting roughly the same range of devices than the ones running Syncthing (which I would expect to be way more hungry on memory). I would gladly write some documentation on the package, its requirements and capabilities. What form should it take? Would you see a page on the wiki for this? In any case, thanks for the feedback and taking time on this. Have a good one! |
9531b0c to
052aa9a
Compare
openwrt-ai
left a comment
There was a problem hiding this comment.
Re-reviewed the force-pushed commit (b13caf7 → 052aa9a). The previous round's findings are addressed: +ca-bundle is in DEPENDS, /etc/ssl/certs is mounted into the jail, test-version.sh is tab-indented, the description is expanded, and the UCI jail_mount / jail_mount_rw escape hatch I asked about is now there — that closes the acme//etc/ssl/acme dead end.
The new UCI plumbing has two wiring bugs that keep it from taking effect, both merge-relevant:
- The file is installed as
/etc/config/maddy.conf, butconfig_load 'maddy'reads/etc/config/maddy, so nothing in it is ever loaded — including the newjail_mountlists. - The three
procd_append_param envcalls run beforeprocd_open_instance, so they attach to theinstancesobject rather than to themaddyinstance.
A third, smaller one: GOMEMLIMIT is passed as a bare integer, which Go reads as bytes, not the MB the config comment promises.
The CMaddy typo in the description and the four-space line in files/maddy.conf are marked nit: and optional. Not repeating @BKPepe's open question about the use case.
Generated by Claude Code
| 'maxprocs:uinteger:0' \ | ||
| 'memlimit:uinteger:0' | ||
|
|
||
| [ "$gc" -le 0 ] || procd_append_param env GOGC="$gc" |
There was a problem hiding this comment.
These three procd_append_param env calls (lines 27-29) run before procd_open_instance on line 38, so they don't land in the instance.
rc.common calls start_service between procd_open_service and procd_close_service, and _procd_open_service at procd.sh:86-97 leaves the JSON cursor inside the instances object. _procd_append_param at procd.sh:475-483 falls back to _procd_set_param, which for env does json_add_object env — so a phantom instance named env gets added alongside maddy, procd rejects it for having no command, and the real instance never sees GOGC/GOMAXPROCS/GOMEMLIMIT.
Move these three lines to just after procd_set_param command "$PROG" (line 39), which is where adguardhome.init:62-68 — the script this is modelled on — puts them.
Generated by Claude Code
| $(INSTALL_DIR) $(1)/etc/config | ||
| $(INSTALL_CONF) $(CURDIR)/files/maddy.conf $(1)/etc/config/maddy.conf |
There was a problem hiding this comment.
The UCI file is installed as /etc/config/maddy.conf, but the init script asks for the package maddy — config_load 'maddy' on maddy.init:15 resolves to /etc/config/maddy via uci_load in uci.sh:21. The shipped file is therefore never read: uci_validate_section falls back to the :0 defaults, and jail_mount / jail_mount_rw are always empty, so the escape hatch this commit adds does nothing. uci show maddy will also report the package as missing.
UCI package files carry no extension — no other package in this feed installs one with a suffix, cf. net/adguardhome/Makefile:77.
| $(INSTALL_DIR) $(1)/etc/config | |
| $(INSTALL_CONF) $(CURDIR)/files/maddy.conf $(1)/etc/config/maddy.conf | |
| $(INSTALL_DIR) $(1)/etc/config | |
| $(INSTALL_CONF) $(CURDIR)/files/maddy.conf $(1)/etc/config/maddy |
(The conffiles entry on line 37 needs the same rename.) Separately, files/maddy.conf now collides by name with the upstream maddy.conf installed to /etc/maddy/ on line 56 — two unrelated files with the same basename in the same install block is easy to misread later; files/maddy.config or files/maddy.uci would read better.
Generated by Claude Code
|
|
||
| define Package/maddy/conffiles | ||
| /etc/maddy/maddy.conf | ||
| /etc/config/maddy.conf |
There was a problem hiding this comment.
This path doesn't match anything that gets installed once the install-block path is corrected (see my comment on lines 58-59), and a conffiles entry that names a non-existent file records no checksum at all — so the UCI config would be silently overwritten on upgrade. Same shape as net/adguardhome/Makefile:49-51.
| /etc/config/maddy.conf | |
| /etc/config/maddy |
Generated by Claude Code
| endef | ||
|
|
||
| define Package/maddy/description | ||
| CMaddy Mail Server implements all functionality required to run a e-mail server. |
There was a problem hiding this comment.
nit: stray C before Maddy — this string is user-visible in menuconfig and in the package index. Also a e-mail → an e-mail.
| CMaddy Mail Server implements all functionality required to run a e-mail server. | |
| Maddy Mail Server implements all functionality required to run an e-mail server. |
Generated by Claude Code
| # Files and directories that maddy has read-write access to | ||
| # list jail_mount_rw '/path/to/dir' | ||
|
|
||
| # Go lang runtime options |
There was a problem hiding this comment.
nit: indented with four spaces while every other line in this file uses a tab. Optional, non-blocking.
| # Go lang runtime options | |
| # Go lang runtime options |
Generated by Claude Code
052aa9a to
d3bfda9
Compare
openwrt-ai
left a comment
There was a problem hiding this comment.
Re-reviewed the force-pushed commit (052aa9a → d3bfda9). Every finding from the previous round is addressed: the UCI file is now installed as /etc/config/maddy (and renamed to files/maddy.uci, which also removes the basename collision with the upstream maddy.conf), the conffiles entry matches, the three procd_append_param env calls moved inside procd_open_instance, GOMEMLIMIT carries a MiB suffix, the CMaddy typo and a e-mail are fixed, and the stray four-space line in the UCI file is a tab.
On GOMEMLIMIT — the doc quote in #discussion_r3838076221 reads the same way I did: the suffix is optional and a bare value is bytes, so ${memlimit}MiB is the right call for a value documented as MB.
One new gap, and it is the flip side of the UCI plumbing that just landed: there is no service_triggers(), so committing /etc/config/maddy never restarts the service and the new options are inert until a manual restart. Inline comment on files/maddy.init. Not a build or packaging breakage — the maintainer's call whether it blocks.
I could not re-verify the upstream maddy.conf this round (outbound fetches to non-GitHub hosts are blocked in this environment); the file is unchanged since the round where it was checked byte-for-byte against v0.9.5.
Not repeating @BKPepe's open question about the use case and resource footprint.
Generated by Claude Code
| procd_set_param no_new_privs 1 | ||
|
|
||
| procd_close_instance | ||
| } |
There was a problem hiding this comment.
Now that /etc/config/maddy actually gets read, nothing reacts to it changing. There is no service_triggers(), so uci commit maddy (or LuCI's Save & Apply) leaves the running instance untouched — the new jail_mount / jail_mount_rw entries and the gc / maxprocs / memlimit values only take effect after a manual /etc/init.d/maddy restart. procd_set_param file "$CONF_FILE" on line 49 covers /etc/maddy/maddy.conf only, not the UCI file.
A reload trigger is enough here: with USE_PROCD=1 and no reload_service, reload() in rc.common:165-172 falls through to start, which re-runs start_service and lets procd apply the new instance definition.
| } | |
| } | |
| service_triggers() { | |
| procd_add_reload_trigger 'maddy' | |
| } |
Both scripts this one is modelled on do the same — adguardhome.init:100-101 and dnsproxy.init:152-154 — as do ~147 other init scripts in this feed.
Generated by Claude Code
Maddy is a composable all-in-one mail server. It implements all functionality required to run a e-mail server into a single daemon. Signed-off-by: Romain Isnel <[email protected]>
d3bfda9 to
8f6dfb6
Compare
|
@BKPepe running full MTA is a niche, but still possible. E.g. i run Mox on some of my internet gateways, just because it's easier to have small server there for notifications. @fromagium how Maddy compares to Mox (i found it's weak point - spam rejection...)? |
openwrt-ai
left a comment
There was a problem hiding this comment.
Reviewed 1 new commit; no new issues found.
The force-push (d3bfda9 → 8f6dfb6) adds exactly the four lines the previous round asked for — a tab-indented service_triggers() with procd_add_reload_trigger 'maddy' at the end of files/maddy.init. That closes the last open finding: with USE_PROCD=1 and no reload_service, reload() in rc.common:165-172 falls through to start, so uci commit maddy now re-runs start_service and procd picks up the new jail_mount / jail_mount_rw / gc / maxprocs / memlimit values. Nothing else changed in the tree.
CI on 8f6dfb6 is clean (all three FormalityCheck runs green).
Still open from the human side and not something I can settle: @BKPepe's question about the intended OpenWrt use case and resource footprint, and @vooon's Mox comparison.
Generated by Claude Code
Maddy is less feature complete than Mox overall (no embedded webmail, no web interface for configuration and management, no integrated web server, no out of the box reputation tracking...) but I find that it manages to hit the sweet spot between a lightweight service that comes with all the features for modern emails and good email acceptability (DMARC, DKIM, MTA-STS, DANE, etc). If my memory is correct, Mox goes a bit further in terms of compliance with modern standards (such as TLS reporting) whereas Maddy does not but would probably benefit from doing. I find the configuration format is quite powerful as well in that it enables a lot of customization without being too verbose (e.g. you can extensively rework the inbound and outbound queue pipelines. see https://maddy.email/reference/smtp-pipeline/). Spam filtering can be handled with custom rules, scripts or by plugging external services like rspamd. Its all quite flexible. In any case, both servers are quite good in what they are trying to accomplish and are low maintenance when configured well (at least for my small installation). |
📦 Package Details
Maintainer: @fromagium
Description:
Maddy is a composable all-in-one mail server. It implements all functionality required to run a e-mail server into a single daemon.
🧪 Run Testing Details
✅ Formalities