From e47a6a8a95acf970934d00f59537d65face2d2a0 Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Thu, 11 Jun 2026 17:09:45 -0700 Subject: [PATCH] Fix logrotate.set dropping endscript on second script block logrotate stanzas may contain more than one shell-script directive (``prerotate``/``postrotate``/``firstaction``/``lastaction``/``preremove``), each terminated by its own ``endscript``. ``_parse_conf`` was parsing every line inside those blocks as an ordinary ``key value`` setting, so the embedded shell commands became bogus dict keys and the two ``endscript`` terminators collapsed into a single dict entry. When ``logrotate.set`` rewrote the stanza via ``_dict_to_stanza`` only one ``endscript`` was emitted, leaving the second script block unterminated and the config invalid. Recognize the script directives explicitly: collect their body lines as opaque shell content until ``endscript``, store them under the directive key as a list, and emit them back as a proper script block (with their own ``endscript``) on rewrite. Fixes #68293 --- changelog/68293.fixed.md | 5 ++ salt/modules/logrotate.py | 62 ++++++++++++++++- tests/pytests/unit/modules/test_logrotate.py | 70 ++++++++++++++++++++ 3 files changed, 134 insertions(+), 3 deletions(-) create mode 100644 changelog/68293.fixed.md diff --git a/changelog/68293.fixed.md b/changelog/68293.fixed.md new file mode 100644 index 000000000000..732e804ed243 --- /dev/null +++ b/changelog/68293.fixed.md @@ -0,0 +1,5 @@ +Fixed ``logrotate.set`` dropping the second ``endscript`` (and turning +embedded shell commands into bogus setting keys) when a stanza contained +multiple script blocks such as both ``prerotate`` and ``postrotate``. Script +directives are now parsed as opaque multi-line bodies and round-trip with +their own ``endscript`` terminator each. diff --git a/salt/modules/logrotate.py b/salt/modules/logrotate.py index 2e29876da815..d34303e05405 100644 --- a/salt/modules/logrotate.py +++ b/salt/modules/logrotate.py @@ -13,6 +13,19 @@ _LOG = logging.getLogger(__name__) _DEFAULT_CONF = "/etc/logrotate.conf" +# Directives that introduce a multi-line shell script body terminated by +# ``endscript``. The body must be kept opaque (not parsed as key/value +# settings) so that lines like ``invoke-rc.d syslog-ng reload`` or repeated +# ``endscript`` terminators do not collapse into bogus dict keys. See +# the logrotate(8) manpage for the full list of script directives. +_SCRIPT_DIRECTIVES = ( + "firstaction", + "lastaction", + "prerotate", + "postrotate", + "preremove", +) + # Define a function alias in order not to shadow built-in's __func_alias__ = {"set_": "set"} @@ -61,10 +74,31 @@ def _parse_conf(conf_file=_DEFAULT_CONF): multi_names = [] multi = {} prev_comps = None + # When inside a ``prerotate``/``postrotate``/... block, collect the raw + # script body lines here and stash them on the enclosing dict under the + # script directive name once ``endscript`` is seen. + script_directive = None + script_body = [] with salt.utils.files.fopen(conf_file, "r") as ifile: for line in ifile: - line = salt.utils.stringutils.to_unicode(line).strip() + line = salt.utils.stringutils.to_unicode(line).rstrip("\r\n") + stripped = line.strip() + + if script_directive is not None: + # Inside a script block: lines are opaque shell content until + # the ``endscript`` terminator. Do not strip comments or skip + # blank lines — they are part of the script. + if stripped == "endscript": + target = multi if mode == "multi" else ret + target[script_directive] = list(script_body) + script_directive = None + script_body = [] + continue + script_body.append(stripped) + continue + + line = stripped if not line: continue if line.startswith("#"): @@ -106,6 +140,15 @@ def _parse_conf(conf_file=_DEFAULT_CONF): ret[file_key] = include_conf[file_key] ret["include files"][include].append(file_key) + # Recognize the start of a script block. The body that follows + # (up to ``endscript``) is kept opaque so embedded shell commands + # don't get parsed as settings and so a second script block's + # ``endscript`` is not lost to a duplicate-key collision. + if comps[0] in _SCRIPT_DIRECTIVES and len(comps) == 1: + script_directive = comps[0] + script_body = [] + continue + prev_comps = comps if len(comps) > 2: key[comps[0]] = " ".join(comps[1:]) @@ -277,7 +320,20 @@ def _dict_to_stanza(key, stanza): """ ret = "" for skey in stanza: - if stanza[skey] is True: + value = stanza[skey] + # Script directives (``prerotate``/``postrotate``/...) are stored as + # a list of opaque body lines; emit them as a script block followed + # by an ``endscript`` terminator. Without this, a stanza containing + # both ``prerotate`` and ``postrotate`` would lose the second + # ``endscript`` on round-trip — see issue #68293. + if skey in _SCRIPT_DIRECTIVES and isinstance(value, list): + ret += f" {skey}\n" + for body_line in value: + ret += f" {body_line}\n" + ret += " endscript\n" + continue + if value is True: + value = "" stanza[skey] = "" - ret += f" {skey} {stanza[skey]}\n" + ret += f" {skey} {value}\n" return f"{key} {{\n{ret}}}" diff --git a/tests/pytests/unit/modules/test_logrotate.py b/tests/pytests/unit/modules/test_logrotate.py index 03872c4cf064..c8e2717ce277 100644 --- a/tests/pytests/unit/modules/test_logrotate.py +++ b/tests/pytests/unit/modules/test_logrotate.py @@ -4,6 +4,8 @@ Test cases for salt.modules.logrotate """ +import textwrap + import pytest import salt.modules.logrotate as logrotate @@ -81,6 +83,74 @@ def test_set_setting_failed(PARSE_CONF): pytest.raises(SaltInvocationError, logrotate.set_, **kwargs) +def test_parse_conf_preserves_script_blocks(tmp_path): + """ + Regression test for #68293. + + ``_parse_conf`` must treat the body of ``prerotate``/``postrotate``/ + ``firstaction``/``lastaction``/``preremove`` blocks as opaque script + content terminated by ``endscript``. Previously every line inside the + block was treated as a key/value setting, so the two ``endscript`` lines + collapsed into a single dict key, the embedded shell commands became + bogus setting keys, and rewriting the stanza dropped the second + ``endscript`` — breaking the rendered logrotate configuration. + """ + conf = textwrap.dedent( + """\ + /parsec/log/system/events { + rotate 10 + size 18M + missingok + notifempty + compress + delaycompress + sharedscripts + prerotate + chattr -a /parsec/log/system/events > /dev/null + endscript + postrotate + system-protect-event-log > /dev/null + invoke-rc.d syslog-ng reload > /dev/null + endscript + } + """ + ) + conf_file = tmp_path / "logrotate.conf" + conf_file.write_text(conf) + + parsed = logrotate._parse_conf(str(conf_file)) + stanza = parsed["/parsec/log/system/events"] + + # The script bodies are preserved verbatim, keyed by the script directive. + assert stanza["prerotate"] == [ + "chattr -a /parsec/log/system/events > /dev/null", + ] + assert stanza["postrotate"] == [ + "system-protect-event-log > /dev/null", + "invoke-rc.d syslog-ng reload > /dev/null", + ] + + # The shell commands inside the script blocks must not leak into the + # stanza as bogus setting keys. + for leaked in ("chattr", "system-protect-event-log", "invoke-rc.d"): + assert leaked not in stanza, ( + f"{leaked!r} was parsed as a setting key — script block was not" + " kept opaque" + ) + + # ``endscript`` is a block terminator, not a setting in its own right. + assert "endscript" not in stanza + + # ``_dict_to_stanza`` must round-trip both script blocks with their own + # ``endscript`` terminator each. + rendered = logrotate._dict_to_stanza("/parsec/log/system/events", stanza) + assert rendered.count("endscript") == 2, rendered + assert "prerotate\n" in rendered, rendered + assert "postrotate\n" in rendered, rendered + assert "chattr -a /parsec/log/system/events > /dev/null" in rendered + assert "invoke-rc.d syslog-ng reload > /dev/null" in rendered + + def test_get(PARSE_CONF): """ Test if get a value for a specific configuration line