Skip to content

fix(logrotate): cap host syslog and mail.log growth - #1958

Open
mrrobot47 wants to merge 5 commits into
EasyEngine:developfrom
mrrobot47:fix/logrotate-host-syslog-cap
Open

mrrobot47 wants to merge 5 commits into
EasyEngine:developfrom
mrrobot47:fix/logrotate-host-syslog-cap

Conversation

@mrrobot47

@mrrobot47 mrrobot47 commented Nov 2, 2026 •

Copy link
Copy Markdown
Member

Caps how large host syslog and mail logs can grow, so a runaway logger can't fill the disk and take the global database down with it. Targets v4.14.0.

Problem

EasyEngine bind-mounts the host's /dev/log into every postfix container. On systemd hosts that is journald's socket, which forwards to rsyslog, which writes both /var/log/mail.log and /var/log/syslog. A postfix process stuck in a logging loop (tlsmgr repeating one line ~100k times a second) wrote ~16 MB/s into each file. The distro rule in /etc/logrotate.d/rsyslog rotates weekly with no size limit and logrotate runs once a day, so the two files grew to ~900 GB each, the disk filled, and the global MariaDB could no longer start. Docker's own container log limits would not have helped, since these lines never go through Docker's logging driver. EasyEngine caps no log size anywhere today.

What changes

  • New migration 20261102100000_easyengine_cap_host_syslog calls EE\Logrotate\Utils::setup_host_log_cap() on Linux (fresh installs run it with all migrations, existing installs on update):
    • rsyslog rule: adds maxsize 2G and dateformat -%Y%m%d-%s (with a one-line comment) to the top of every stanza in /etc/logrotate.d/rsyslog (or the older RHEL name /etc/logrotate.d/syslog) that has no maxsize / size yet (dateformat is skipped when the stanza or file sets its own). The dateformat only matters with dateext: a day-only suffix makes a second rotation on the same day fail with "destination already exists", which would stop the cap after the first rotation of the day. The rest of the file is kept byte for byte, so distro and admin edits survive. A stanza or file that already sets a size limit is left alone, a missing file is skipped, and a symlinked or unparseable file is left alone with a warning that says what to add by hand. The edited copy must pass logrotate -d with no errors beyond the ones the original already reports (line numbers ignored), the original is saved to /opt/easyengine/.backup/logrotate/ (dir 0700, file 0600), and the file is replaced atomically with its mode and owner kept.
    • Hourly rotation (maxsize is only checked when logrotate runs): on systemd hosts with logrotate.timer, a drop-in /etc/systemd/system/logrotate.timer.d/hourly.conf (OnCalendar= then OnCalendar=hourly), followed by daemon-reload and try-restart, so a timer the admin stopped stays stopped. Hosts that run logrotate from /etc/cron.daily/logrotate (no systemd, or a logrotate without the timer) get /etc/cron.hourly/ee-logrotate, which runs that same daily script, so its own checks and alerting still apply. On a systemd host whose timer is masked or missing and whose daily script defers to systemd, nothing is scheduled and a warning says so. An existing drop-in or cron job is never overwritten.
  • ee_cli, ee_sites and ee_nginx_proxy get maxsize 1G and the same dateformat, so a runaway CLI, site or proxy log is rotated within the hour too. These files are rewritten by the existing logrotate setup step on every update, as before.

Worst case afterwards: about 2 GB plus one hour of flood per file, with the distro's rotate count bounding the rotated copies.

On hosts that already use dateext, archives made before the update keep their -YYYYMMDD names, which the new -YYYYMMDD-<epoch> pattern doesn't match, so rotate no longer prunes them. That is a one-time set (at most the rotate count per log, never growing) that can be removed by hand. Nothing changes on hosts without dateext, the Debian/Ubuntu default.

Why a migration and not the setup step

The setup step rewrites EasyEngine's own files on every update. The rsyslog rule and the timer belong to the host, so they are changed once: if an admin later changes the limit, the schedule, or removes either, an update doesn't put them back. Every part is idempotent, so a retried migration changes nothing.

Testing

Throwaway containers only:

  • Parser on PHP 7.4 and 8.5 (php -l clean on both): the stock rules of Ubuntu 20.04 (two stanzas), 22.04, 24.04, Debian 12 and RHEL 9, a rule that already has maxsize, a mixed file (one stanza with size, braces inside a postrotate script and in a comment), a file-level maxsize, a file without a trailing newline, and unparseable files (unclosed stanza, missing endscript, one-line stanza). Each result is idempotent (applying it twice gives the same file).
  • Ubuntu 24.04 without systemd: rule edited, second run changes nothing, backup equals the original, mode and owner kept, no temporary file left in /etc/logrotate.d, cron job installed and runs the daily script, logrotate -d /etc/logrotate.conf has no errors, and a 2.1 GB mail.log is rotated on the next run while the other logs (weekly, not due) are not. An admin's maxsize 500M, a symlinked rule, an unparseable rule, a failed write and an existing cron job are all left untouched.
  • Ubuntu 24.04 without rsyslog: nothing to edit, no warning, cron job installed.
  • Debian 12 and Ubuntu 24.04 with systemd as PID 1: rule edited, drop-in installed, logrotate.timer shows OnCalendar=*-*-* *:00:00, second run changes nothing, no cron job, logrotate -d clean, a 2.1 GB mail.log is rotated by logrotate.service, a stopped timer stays stopped, and an existing drop-in is left alone.
  • Rocky Linux 9 (non-Debian layout): rule edited and still accepted by logrotate -d; no timer or cron.daily in the container, so no schedule change.
  • dateext enabled in /etc/logrotate.conf (Rocky 9 default, and Ubuntu 24.04 with it added): three size rotations on the same day through /etc/logrotate.conf give three archives each for mail.log / maillog and a site log, with no "already exists" error. Without dateext the archive names stay numeric as before.
  • A rule with an unrelated pre-existing error below the insertion point is still capped; a systemd host with logrotate.timer masked gets the warning and no cron job.
  • EasyEngine's own rules pass the setup step's validation and logrotate -d.

Not in this PR

  • Sending postfix logs to the container's stdout instead of host syslog (drops the /dev/log mount; needs a site-type migration).
  • journald / rsyslog rate limiting.

Postfix containers log through the host's /dev/log, so a logging loop lands in /var/log/syslog and /var/log/mail.log. Distros rotate those weekly with no size limit, and logrotate runs once a day, so a flood can fill the disk between runs and take the global DB down with it.

A one-time migration now adds `maxsize 2G` to every stanza of the rsyslog logrotate rule that has no size limit yet, keeping the rest of the file as it is (a stanza or file that already sets maxsize or size is left alone, an unparseable or symlinked file is left alone with a warning, and the result must pass `logrotate -d` with no new errors). The original is saved under the EasyEngine backup dir.

It also makes logrotate run hourly so the limit is checked in time: a logrotate.timer drop-in on systemd hosts (a stopped timer stays stopped), or an hourly cron job that runs the daily logrotate script on hosts that use cron. An existing drop-in or job is left alone.
The ee_cli, ee_sites and ee_nginx_proxy rules rotate daily with no size limit. With logrotate now running hourly, `maxsize 1G` rotates a runaway CLI, site or proxy log within the hour instead of letting it grow until the next day.
With dateext and the default day-only suffix, a second rotation on the same day fails with "destination already exists", so maxsize stopped capping a flood after the first rotation of the day (RHEL-family hosts enable dateext in logrotate.conf by default).

The host stanzas that get maxsize also get `dateformat -%Y%m%d-%s` unless the stanza or file sets its own dateformat, and EasyEngine's own rules get the same. It changes nothing when dateext is off, and keeps dated archive names when it is on.
…p cron job

The inserted lines shift the line numbers of errors the original rule already has, so an unrelated pre-existing error made the edit look like it added a new one and the cap was refused. Line numbers are now masked along with the path.

On a systemd host without a usable logrotate.timer (e.g. masked), the packaged cron.daily script exits early because systemd runs, so the hourly job would never rotate anything. EasyEngine now warns that maxsize is only checked when logrotate runs instead of installing that job.
copy() created the backup with the default umask, so a rule restricted to root (which may hold credentials in its scripts) was saved world-readable. The backup dir is now 0700 and the copy 0600.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Critical archive-name collisions under inherited day-only dateext settings block repeated same-day rotations, allowing flooded logs to keep growing despite configured size limits.

2 open issues
What changed in this PR

This PR reduces disk-exhaustion risk from runaway logging by adding size-triggered rotation for host syslog and EasyEngine logs.

Changes:

  • Adds a one-time migration to update host logrotate rules while preserving administrator settings.
  • Configures hourly logrotate checks through a systemd timer drop-in or cron fallback.
  • Adds maxsize 1G to EasyEngine’s CLI, site, and nginx-proxy log rules.
File Description
php/​EE/​Logrotate/​Utils.php Adds host-rule parsing, validation, backups, hourly scheduling, and EasyEngine log-size thresholds.
migrations/​db/​20261102100000_easyengine_cap_host_syslog.php Runs host log-cap setup once during migration.

Review effort: Balanced

Comment thread php/EE/Logrotate/Utils.php Resolved
Comment thread php/EE/Logrotate/Utils.php Resolved

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The hourly fallback can silently skip rotation, shifted diagnostic line numbers can block cap installation, and the new date suffix excludes existing archives from retention.

1 open issue
2 previously missed issues

In code that hasn't changed since last review

Medium severity Normalize diagnostic line numbers before comparing runs

php/​EE/​Logrotate/​Utils.php:234

Masking only the filename does not make existing diagnostics compare equal: adding directives shifts subsequent line numbers, so an unchanged error: <config>:7 ... can become error: <config>:10 .... array_diff() then treats that pre-existing error as new and refuses to install the cap. Normalize diagnostic line numbers as well, or map them back to the original positions before comparing the two runs.

Medium severity Cron fallback skips rotation when systemd is active

php/​EE/​Logrotate/​Utils.php:263

On a systemd host without a loadable logrotate.timer, this fallback installs a job that does nothing: Ubuntu's packaged /etc/cron.daily/logrotate exits immediately when /run/systemd/system exists, regardless of whether the timer is available. Consequently, the hourly job never checks maxsize, despite reporting successful scheduling. Handle this case with an invocation that actually runs rotation and preserves error reporting, or warn that no working hourly schedule could be installed.

2 resolved since last review

Review effort: Balanced

Comment thread php/EE/Logrotate/Utils.php Resolved

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

The backup directory is not made private, allowing a 0600 logrotate rule to be copied as 0644 and potentially exposing credentials to other local users.

No open issues 🎉

1 previously missed issue

In code that hasn't changed since last review

Medium severity Backups expose restricted host rules to local users

php/​EE/​Logrotate/​Utils.php:111

With the usual umask of 0022, backing up a 0600 host rule creates a 0644 copy inside this 0755 directory, even though the installed rule retains its original permissions. This makes an admin-restricted configuration (including any credentials in rotation scripts) readable by other local users when the backup's parent directories are traversable. Make the backup directory private, including when it already exists, before copying the file.

1 resolved since last review

Review effort: Balanced

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.

2 participants