fix: pin logrotate config mode in image and fail loud on bad permissions
test / test (push) Has been cancelled
test / test (push) Has been cancelled
COPY --chmod makes /etc/logrotate.d/mail 0644 regardless of build context file modes (Windows tar sync). logrotate-loop preflight exits non-zero when logrotate would ignore the config. E2e covers mode, forced rotation, and a group-writable context build. Co-Authored-By: Composer 2.5 <noreply@cursor.com> Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
@@ -5,6 +5,15 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version
|
|||||||
|
|
||||||
## [Unreleased]
|
## [Unreleased]
|
||||||
|
|
||||||
|
### Fixed
|
||||||
|
|
||||||
|
- image: `mail.log` rotation no longer silently stops when the build context
|
||||||
|
ships `logrotate-mail.conf` with group/other write bits (common after a
|
||||||
|
Windows checkout sync). Runtime `COPY --chmod` pins config and script modes
|
||||||
|
in the Dockerfile; `logrotate-loop.sh` refuses a config logrotate would
|
||||||
|
ignore. E2e checks mode `644`, forced rotation, and a group-writable context
|
||||||
|
build.
|
||||||
|
|
||||||
## [1.2.2] - 2026-08-12
|
## [1.2.2] - 2026-08-12
|
||||||
|
|
||||||
Status page layout after 1.2.1: paired cards in a wide column, denser machine
|
Status page layout after 1.2.1: paired cards in a wide column, denser machine
|
||||||
|
|||||||
+13
-13
@@ -88,20 +88,20 @@ COPY --from=build /out/selfpost-backup /usr/local/bin/selfpost-backup
|
|||||||
|
|
||||||
# Licence text shipped with the image (AGPL-3.0 conveyance). The panel also
|
# Licence text shipped with the image (AGPL-3.0 conveyance). The panel also
|
||||||
# serves the same text at /license from an embedded copy.
|
# serves the same text at /license from an embedded copy.
|
||||||
COPY LICENSE NOTICE /usr/share/doc/selfpost/
|
COPY --chmod=0644 LICENSE NOTICE /usr/share/doc/selfpost/
|
||||||
|
|
||||||
COPY build/opendkim.conf /etc/opendkim.conf
|
# File modes are pinned here so a build context copied from a checkout without
|
||||||
COPY build/logrotate-mail.conf /etc/logrotate.d/mail
|
# POSIX permissions (e.g. Windows tar sync) cannot land group-writable config
|
||||||
COPY build/postfix-wrapper.sh /usr/local/bin/postfix-wrapper.sh
|
# that logrotate would silently ignore — see docs/plans/logrotate-mode.md.
|
||||||
COPY build/postfix-config.sh /usr/local/bin/postfix-config.sh
|
COPY --chmod=0644 build/opendkim.conf /etc/opendkim.conf
|
||||||
COPY build/postfix-cert-reload.sh /usr/local/bin/postfix-cert-reload.sh
|
COPY --chmod=0644 build/logrotate-mail.conf /etc/logrotate.d/mail
|
||||||
COPY build/logrotate-loop.sh /usr/local/bin/logrotate-loop.sh
|
COPY --chmod=0755 build/postfix-wrapper.sh /usr/local/bin/postfix-wrapper.sh
|
||||||
COPY build/crashexit.py /usr/local/bin/crashexit.py
|
COPY --chmod=0755 build/postfix-config.sh /usr/local/bin/postfix-config.sh
|
||||||
COPY build/entrypoint.sh /usr/local/bin/entrypoint.sh
|
COPY --chmod=0755 build/postfix-cert-reload.sh /usr/local/bin/postfix-cert-reload.sh
|
||||||
COPY build/supervisord.conf /etc/supervisor/supervisord.conf
|
COPY --chmod=0755 build/logrotate-loop.sh /usr/local/bin/logrotate-loop.sh
|
||||||
RUN chmod +x /usr/local/bin/postfix-wrapper.sh /usr/local/bin/postfix-config.sh \
|
COPY --chmod=0755 build/crashexit.py /usr/local/bin/crashexit.py
|
||||||
/usr/local/bin/postfix-cert-reload.sh /usr/local/bin/logrotate-loop.sh \
|
COPY --chmod=0755 build/entrypoint.sh /usr/local/bin/entrypoint.sh
|
||||||
/usr/local/bin/crashexit.py /usr/local/bin/entrypoint.sh
|
COPY --chmod=0644 build/supervisord.conf /etc/supervisor/supervisord.conf
|
||||||
|
|
||||||
# Published submission ports: 465 (smtps, primary) and 587 (submission, optional)
|
# Published submission ports: 465 (smtps, primary) and 587 (submission, optional)
|
||||||
# plus the panel on 8080. Outbound delivery dials remote MXs on 25 as a client,
|
# plus the panel on 8080. Outbound delivery dials remote MXs on 25 as a client,
|
||||||
|
|||||||
+40
-1
@@ -14,10 +14,49 @@
|
|||||||
# often than daily — polling merely bounds how late a legitimate rotation runs.
|
# often than daily — polling merely bounds how late a legitimate rotation runs.
|
||||||
set -eu
|
set -eu
|
||||||
|
|
||||||
|
CONFIG=/etc/logrotate.d/mail
|
||||||
INTERVAL="${LOGROTATE_INTERVAL_SECONDS:-21600}"
|
INTERVAL="${LOGROTATE_INTERVAL_SECONDS:-21600}"
|
||||||
|
|
||||||
|
# logrotate refuses configs writable by group or others and exits 0 while
|
||||||
|
# ignoring them — fail here so supervisord reports the fault.
|
||||||
|
logrotate_config_ok() {
|
||||||
|
mode=$(stat -c '%a' "$CONFIG")
|
||||||
|
mode=${mode#0}
|
||||||
|
grp=$(( (mode / 10) % 10 ))
|
||||||
|
oth=$(( mode % 10 ))
|
||||||
|
case $grp in 2|3|6|7) return 1 ;; esac
|
||||||
|
case $oth in 2|3|6|7) return 1 ;; esac
|
||||||
|
return 0
|
||||||
|
}
|
||||||
|
|
||||||
|
logrotate_config_fatal() {
|
||||||
|
echo "logrotate-loop: refusing to run: $CONFIG mode $(stat -c '%a' "$CONFIG") is writable by group or others" >&2
|
||||||
|
exit 1
|
||||||
|
}
|
||||||
|
|
||||||
|
if ! logrotate_config_ok; then
|
||||||
|
logrotate_config_fatal
|
||||||
|
fi
|
||||||
|
|
||||||
|
run_logrotate() {
|
||||||
|
out=$(logrotate "$CONFIG" 2>&1) || {
|
||||||
|
echo "$out" >&2
|
||||||
|
return 1
|
||||||
|
}
|
||||||
|
case "$out" in
|
||||||
|
*Ignoring*|*Potentially\ dangerous\ mode*)
|
||||||
|
echo "$out" >&2
|
||||||
|
logrotate_config_fatal
|
||||||
|
;;
|
||||||
|
esac
|
||||||
|
return 0
|
||||||
|
}
|
||||||
|
|
||||||
while true; do
|
while true; do
|
||||||
if logrotate /etc/logrotate.d/mail; then
|
if ! logrotate_config_ok; then
|
||||||
|
logrotate_config_fatal
|
||||||
|
fi
|
||||||
|
if run_logrotate; then
|
||||||
:
|
:
|
||||||
else
|
else
|
||||||
echo "logrotate-loop: logrotate failed, will retry after ${INTERVAL}s" >&2
|
echo "logrotate-loop: logrotate failed, will retry after ${INTERVAL}s" >&2
|
||||||
|
|||||||
@@ -104,8 +104,9 @@ stderr_logfile=/dev/stderr
|
|||||||
stderr_logfile_maxbytes=0
|
stderr_logfile_maxbytes=0
|
||||||
|
|
||||||
; Periodic logrotate for /data/log/mail.log (spec 9, 10: daily, 7-14 files kept
|
; Periodic logrotate for /data/log/mail.log (spec 9, 10: daily, 7-14 files kept
|
||||||
; in the image). Runs as root so logrotate can read/rotate the log; never exits
|
; in the image). Runs as root so logrotate can read/rotate the log. Exits
|
||||||
; non-zero, so it neither trips the crashexit listener nor needs restarting.
|
; non-zero when the config is group/other-writable (logrotate would ignore it
|
||||||
|
; silently); autorestart surfaces BACKOFF on the Status page.
|
||||||
[program:logrotate]
|
[program:logrotate]
|
||||||
command=/usr/local/bin/logrotate-loop.sh
|
command=/usr/local/bin/logrotate-loop.sh
|
||||||
priority=400
|
priority=400
|
||||||
|
|||||||
+4
-2
@@ -162,8 +162,10 @@ docker build -f build/Dockerfile -t selfpost:dev --build-arg VERSION=dev .
|
|||||||
```
|
```
|
||||||
|
|
||||||
The Dockerfile has a build stage (`go vet`, `go build` with `VERSION`) and a
|
The Dockerfile has a build stage (`go vet`, `go build` with `VERSION`) and a
|
||||||
runtime stage (Debian + mail stack). See [architecture.md](architecture.md) §
|
runtime stage (Debian + mail stack). Runtime config and scripts use `COPY
|
||||||
Image and processes.
|
--chmod` so file modes in the image do not depend on how the build context was
|
||||||
|
synced (e.g. a Windows checkout widening permissions on `logrotate-mail.conf`).
|
||||||
|
See [architecture.md](architecture.md) § Image and processes.
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
|
|||||||
@@ -1,89 +1,28 @@
|
|||||||
# Plan: logrotate-mode (mail.log stops rotating in some images)
|
# Plan: logrotate-mode (mail.log stops rotating in some images)
|
||||||
|
|
||||||
**Status:** candidate
|
**Status:** done
|
||||||
**Version:** patch; no schema, no configuration surface.
|
**Version:** patch; no schema, no configuration surface.
|
||||||
**Order:** independent. Worth doing before anything that lets an instance run
|
|
||||||
unattended for months.
|
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
## What was observed
|
## Summary
|
||||||
|
|
||||||
The container log carries, on every start:
|
`mail.log` stopped rotating when `/etc/logrotate.d/mail` landed in the image
|
||||||
|
with group/other write permission (e.g. build context from a Windows tar sync).
|
||||||
|
logrotate ignores such configs but exits 0, so the loop looked healthy while
|
||||||
|
the log grew without bound.
|
||||||
|
|
||||||
```
|
## What shipped
|
||||||
warning: Potentially dangerous mode on /etc/logrotate.d/mail: 0664
|
|
||||||
error: Ignoring /etc/logrotate.d/mail because it is writable by group or others.
|
|
||||||
```
|
|
||||||
|
|
||||||
logrotate refuses a configuration file that group or others may write, so
|
- [`build/Dockerfile`](../../build/Dockerfile): `COPY --chmod` pins config
|
||||||
`mail.log` is never rotated in an image with that mode. It grows until the
|
(`0644`) and script (`0755`) modes so the image no longer depends on checkout
|
||||||
volume does.
|
file modes.
|
||||||
|
- [`build/logrotate-loop.sh`](../../build/logrotate-loop.sh): preflight and
|
||||||
|
per-iteration checks refuse group/other-writable configs; logrotate stderr
|
||||||
|
mentioning `Ignoring` is fatal.
|
||||||
|
- [`test/e2e/logrotate_check.go`](../../test/e2e/logrotate_check.go): asserts
|
||||||
|
mode `644`, forced `logrotate -f`, and that a group-writable context file
|
||||||
|
still produces `644` in the image.
|
||||||
|
|
||||||
Measured across three images built on the same host from the same commit range:
|
`create 0640 postfix selfpost` in [`build/logrotate-mail.conf`](../../build/logrotate-mail.conf)
|
||||||
|
is unchanged — required for panel readability after rotation.
|
||||||
| Image built from | Mode of `/etc/logrotate.d/mail` |
|
|
||||||
|---|---|
|
|
||||||
| a sync made two days earlier | `0644` — works |
|
|
||||||
| a later sync (`tar -czf -` pipe from a Windows checkout) | `0666` |
|
|
||||||
| a later sync (`git archive` from the same checkout) | `0664` |
|
|
||||||
|
|
||||||
So the file is fine in the repository (git records `100644`) and is spoiled on
|
|
||||||
the way into the build context. `COPY build/logrotate-mail.conf
|
|
||||||
/etc/logrotate.d/mail` ([build/Dockerfile](../../build/Dockerfile)) takes the
|
|
||||||
mode from the context as it finds it, and an archive produced from a checkout
|
|
||||||
without POSIX permissions carries the umask-widened mode instead of the one git
|
|
||||||
recorded.
|
|
||||||
|
|
||||||
**Not established:** whether images built by the release workflow are affected.
|
|
||||||
They are built from a checkout on Linux, where the mode should survive as
|
|
||||||
`0644`, but the published image could not be pulled to check. Confirm before
|
|
||||||
concluding that only locally built images have this.
|
|
||||||
|
|
||||||
## Why it deserves a plan rather than a one-line fix
|
|
||||||
|
|
||||||
Three separate things are wrong, and fixing only the visible one leaves the
|
|
||||||
other two.
|
|
||||||
|
|
||||||
1. **The image trusts the build context's file modes.** Every `COPY` in the
|
|
||||||
Dockerfile has this property, not just this one; the scripts happen to be
|
|
||||||
`chmod +x`-ed afterwards, which is why they were never noticed.
|
|
||||||
2. **The failure is silent.** `logrotate-loop.sh` runs
|
|
||||||
`logrotate /etc/logrotate.d/mail` and only reports a failure on a non-zero
|
|
||||||
exit — but logrotate *ignores* the file and exits 0, so the loop reports
|
|
||||||
nothing and the operator's only clue is a warning printed once at start.
|
|
||||||
3. **Nothing checks the outcome.** No test or health check notices that
|
|
||||||
`mail.log` has not rotated, and the panel's Status page has no view of it.
|
|
||||||
|
|
||||||
## Directions to weigh
|
|
||||||
|
|
||||||
- `COPY --chmod=0644` on the configuration files (and an explicit mode on the
|
|
||||||
scripts instead of the later `chmod +x`), which makes the image's file modes a
|
|
||||||
property of the Dockerfile rather than of whoever built it. Needs a check of
|
|
||||||
the minimum BuildKit version the project is willing to require.
|
|
||||||
- Or an explicit `chmod` in the same `RUN` that already fixes the scripts —
|
|
||||||
cruder, no build-time requirement.
|
|
||||||
- Make `logrotate-loop.sh` fail loudly: `logrotate` has `--debug`-free ways to
|
|
||||||
be told to care, but the simplest reliable check is that the loop verifies
|
|
||||||
the configuration is readable-and-not-writable before entering the loop, and
|
|
||||||
exits non-zero so supervisord reports it.
|
|
||||||
- Consider whether the e2e stack should assert that a rotation actually happens
|
|
||||||
(it can run with a short `LOGROTATE_INTERVAL_SECONDS`).
|
|
||||||
|
|
||||||
## Done when
|
|
||||||
|
|
||||||
- An image built from a Windows checkout and one built by the release workflow
|
|
||||||
both carry `0644`, and rotation runs in both.
|
|
||||||
- A configuration logrotate would ignore makes the container say so in a way an
|
|
||||||
operator will see, rather than exiting 0.
|
|
||||||
- The dev loop's sync step cannot silently widen file modes again, or the image
|
|
||||||
no longer cares if it does.
|
|
||||||
|
|
||||||
## Risks
|
|
||||||
|
|
||||||
- Low blast radius, but it touches the image's startup path — a mistake here is
|
|
||||||
a container that will not start rather than a log that does not rotate.
|
|
||||||
- The `create 0640 postfix selfpost` line in the rotate configuration is load
|
|
||||||
bearing (see the comment in `logrotate-loop.sh`: a postlogd-triggered recreate
|
|
||||||
lands the file unreadable by the unprivileged panel). Any rework of the
|
|
||||||
configuration must keep it.
|
|
||||||
|
|||||||
@@ -29,7 +29,6 @@ in `git log` and [CHANGELOG.md](../CHANGELOG.md).
|
|||||||
| inbound-relay | Inbound relay (backup-MX / forwarding) | **agreed** | [plans/inbound-relay.md](plans/inbound-relay.md) |
|
| inbound-relay | Inbound relay (backup-MX / forwarding) | **agreed** | [plans/inbound-relay.md](plans/inbound-relay.md) |
|
||||||
| contributing | `CONTRIBUTING.md` | candidate | — |
|
| contributing | `CONTRIBUTING.md` | candidate | — |
|
||||||
| dmarc-reports | DMARC aggregate report ingestion and panel UI | candidate | [plans/dmarc-reports.md](plans/dmarc-reports.md) |
|
| dmarc-reports | DMARC aggregate report ingestion and panel UI | candidate | [plans/dmarc-reports.md](plans/dmarc-reports.md) |
|
||||||
| logrotate-mode | `mail.log` stops rotating in some builds | candidate | [plans/logrotate-mode.md](plans/logrotate-mode.md) |
|
|
||||||
| panel-docs | In-panel operator documentation | candidate | — |
|
| panel-docs | In-panel operator documentation | candidate | — |
|
||||||
|
|
||||||
**Recommended order** (not binding): **inbound-relay** first among agreed
|
**Recommended order** (not binding): **inbound-relay** first among agreed
|
||||||
|
|||||||
@@ -0,0 +1,97 @@
|
|||||||
|
package e2e
|
||||||
|
|
||||||
|
import (
|
||||||
|
"fmt"
|
||||||
|
"os"
|
||||||
|
"os/exec"
|
||||||
|
"path/filepath"
|
||||||
|
"strings"
|
||||||
|
"testing"
|
||||||
|
)
|
||||||
|
|
||||||
|
// checkLogrotateConfigMode verifies the image pins /etc/logrotate.d/mail at 0644
|
||||||
|
// so logrotate will not silently ignore it (docs/plans/logrotate-mode.md).
|
||||||
|
func checkLogrotateConfigMode(s *stack) error {
|
||||||
|
mode, err := s.execIn("selfpost", "stat", "-c", "%a", "/etc/logrotate.d/mail")
|
||||||
|
if err != nil {
|
||||||
|
return fmt.Errorf("stat logrotate config: %w", err)
|
||||||
|
}
|
||||||
|
mode = strings.TrimSpace(mode)
|
||||||
|
if mode != "644" {
|
||||||
|
return fmt.Errorf("/etc/logrotate.d/mail mode is %q, want 644", mode)
|
||||||
|
}
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
|
// checkLogrotateRotation forces a rotation and checks the recreated mail.log is
|
||||||
|
// panel-readable (0640 postfix:selfpost per build/logrotate-mail.conf).
|
||||||
|
func checkLogrotateRotation(s *stack) error {
|
||||||
|
const logPath = "/data/log/mail.log"
|
||||||
|
marker := "e2e-logrotate-marker\n"
|
||||||
|
if _, err := s.execIn("selfpost", "sh", "-c",
|
||||||
|
fmt.Sprintf("printf %q >> %s", marker, logPath)); err != nil {
|
||||||
|
return fmt.Errorf("write mail.log: %w", err)
|
||||||
|
}
|
||||||
|
if _, err := s.execIn("selfpost", "logrotate", "-f", "/etc/logrotate.d/mail"); err != nil {
|
||||||
|
return fmt.Errorf("logrotate -f: %w", err)
|
||||||
|
}
|
||||||
|
rotated, err := s.execIn("selfpost", "sh", "-c", "test -f /data/log/mail.log.1 && echo yes")
|
||||||
|
if err != nil || strings.TrimSpace(rotated) != "yes" {
|
||||||
|
return fmt.Errorf("expected /data/log/mail.log.1 after forced rotation (out=%q err=%v)", rotated, err)
|
||||||
|
}
|
||||||
|
mode, err := s.execIn("selfpost", "stat", "-c", "%a", logPath)
|
||||||
|
if err != nil {
|
||||||
|
return fmt.Errorf("stat rotated mail.log: %w", err)
|
||||||
|
}
|
||||||
|
if strings.TrimSpace(mode) != "640" {
|
||||||
|
return fmt.Errorf("new %s mode is %q, want 640", logPath, strings.TrimSpace(mode))
|
||||||
|
}
|
||||||
|
body, err := s.execIn("selfpost", "grep", "-F", strings.TrimSpace(marker), "/data/log/mail.log.1")
|
||||||
|
if err != nil || !strings.Contains(body, strings.TrimSpace(marker)) {
|
||||||
|
return fmt.Errorf("rotated file does not contain marker (out=%q err=%v)", body, err)
|
||||||
|
}
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestImageBuildPreservesLogrotateMode builds from a context where
|
||||||
|
// logrotate-mail.conf is group-writable and checks the image still ships 0644.
|
||||||
|
func TestImageBuildPreservesLogrotateMode(t *testing.T) {
|
||||||
|
conf := filepath.Join(h.repoRoot, "build", "logrotate-mail.conf")
|
||||||
|
info, err := os.Stat(conf)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("stat source config: %v", err)
|
||||||
|
}
|
||||||
|
origMode := info.Mode().Perm()
|
||||||
|
|
||||||
|
if err := os.Chmod(conf, origMode|0o020); err != nil {
|
||||||
|
t.Fatalf("chmod g+w source config: %v", err)
|
||||||
|
}
|
||||||
|
t.Cleanup(func() {
|
||||||
|
_ = os.Chmod(conf, origMode)
|
||||||
|
})
|
||||||
|
|
||||||
|
tag := "selfpost:e2e-logrotate-mode"
|
||||||
|
build := exec.Command("docker", "build",
|
||||||
|
"-f", filepath.Join(h.repoRoot, "build", "Dockerfile"),
|
||||||
|
"-t", tag,
|
||||||
|
"--build-arg", "VERSION=e2e",
|
||||||
|
h.repoRoot,
|
||||||
|
)
|
||||||
|
build.Env = os.Environ()
|
||||||
|
if out, err := build.CombinedOutput(); err != nil {
|
||||||
|
t.Fatalf("docker build with group-writable context config: %v\n%s", err, out)
|
||||||
|
}
|
||||||
|
t.Cleanup(func() {
|
||||||
|
_, _ = exec.Command("docker", "rmi", "-f", tag).CombinedOutput()
|
||||||
|
})
|
||||||
|
|
||||||
|
run := exec.Command("docker", "run", "--rm", tag, "stat", "-c", "%a", "/etc/logrotate.d/mail")
|
||||||
|
run.Env = os.Environ()
|
||||||
|
out, err := run.CombinedOutput()
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("stat in image: %v\n%s", err, out)
|
||||||
|
}
|
||||||
|
if strings.TrimSpace(string(out)) != "644" {
|
||||||
|
t.Fatalf("image logrotate config mode is %q, want 644", strings.TrimSpace(string(out)))
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -105,6 +105,12 @@ func TestE2E(t *testing.T) {
|
|||||||
if err := checkSupervisorProcesses(h); err != nil {
|
if err := checkSupervisorProcesses(h); err != nil {
|
||||||
t.Fatal(err)
|
t.Fatal(err)
|
||||||
}
|
}
|
||||||
|
if err := checkLogrotateConfigMode(h); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
if err := checkLogrotateRotation(h); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
if err := waitForPanelReady(); err != nil {
|
if err := waitForPanelReady(); err != nil {
|
||||||
t.Fatal(err)
|
t.Fatal(err)
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user