[shepherd] 02/02: log-rotation: Skip non-regular files.

[Ludovic Courtès]

civodul pushed a commit to branch main
in repository shepherd.

commit fb0f6fe5cef6772580d7d3f8505411342320069e
Author: Ludovic Courtès <ludo@gnu.org>
AuthorDate: Wed Dec 11 11:58:56 2024 +0100

    log-rotation: Skip non-regular files.
    
    Until now, /dev/console, /dev/tty12, etc. were skipped, but only because
    they were always below the size threshold.  The service could
    potentially attempt to rotate directories specified as external log
    files (reported size is usually 4 KiB), leading to an uncaught exception
    in the timer action when calling ‘truncate-file’.
    
    This change makes the non-regular-file check explicit.
    
    * modules/shepherd/service/log-rotation.scm (rotate-file): Explicitly
    skip non-regular files.
    * tests/services/log-rotation.sh: Test it.
    * NEWS: Update.
---
 NEWS                                      | 10 ++++++++++
 modules/shepherd/service/log-rotation.scm | 31 ++++++++++++++++++-------------
 tests/services/log-rotation.sh            | 10 ++++++++--
 3 files changed, 36 insertions(+), 15 deletions(-)

diff --git a/NEWS b/NEWS
index eba97b9..6b6bb98 100644
--- a/NEWS
+++ b/NEWS
@@ -11,6 +11,16 @@ Copyright © 2013-2014, 2016, 2018-2020, 2022-2024 Ludovic 
Courtès <ludo@gnu.or
 
 Please send Shepherd bug reports to bug-guix@gnu.org.
 
+* Changes in 1.0.1
+
+** ‘log-rotation’ service explicitly skips non-regular files
+
+Previously, the log rotation service would attempt to rotate non-regular files
+with a reported size greater than the threshold; in practice that could
+potentially happen when specifying a directory as an external log file, and
+only if the threshold is set below 4096 bytes.  Non-regular files are now
+explicitly skipped.
+
 * Changes in 1.0.0
 
 ** ‘herd status SERVICE’ shows high-level info about services
diff --git a/modules/shepherd/service/log-rotation.scm 
b/modules/shepherd/service/log-rotation.scm
index 64eb1cd..d4a1289 100644
--- a/modules/shepherd/service/log-rotation.scm
+++ b/modules/shepherd/service/log-rotation.scm
@@ -139,19 +139,24 @@ called with @var{file}."
      (local-output (l10n "Log file '~a' is inaccessible; not rotating.")
                    file))
     (stat
-     (if (> (stat:size stat) rotation-size-threshold)
-         (begin
-           ;; First rotate past logs, to free "FILE.1".
-           (rotate-past-logs file rotate)
-           ;; Then rename FILE to "FILE.1".
-           (let* ((next (string-append file ".1"))
-                  (rotated? (rotate-current file next)))
-             (when rotated?
-               (compress-file compression next)
-               (local-output (l10n "Rotated '~a'.") file))))
-         (local-output
-          (l10n "Not rotating '~a', which is below the ~a B threshold.")
-          file rotation-size-threshold)))))
+     (cond ((not (eq? 'regular (stat:type stat)))
+            ;; Do not rotate /dev/tty12, directories, or anything like that.
+            (local-output
+             (l10n "Not rotating '~a', which is not a regular file.")
+             file))
+           ((> (stat:size stat) rotation-size-threshold)
+            ;; First rotate past logs, to free "FILE.1".
+            (rotate-past-logs file rotate)
+            ;; Then rename FILE to "FILE.1".
+            (let* ((next (string-append file ".1"))
+                   (rotated? (rotate-current file next)))
+              (when rotated?
+                (compress-file compression next)
+                (local-output (l10n "Rotated '~a'.") file))))
+           (else
+            (local-output
+             (l10n "Not rotating '~a', which is below the ~a B threshold.")
+             file rotation-size-threshold))))))
 
 (define* (rotate-logs logger #:optional (rotate rename-file)
                       #:key
diff --git a/tests/services/log-rotation.sh b/tests/services/log-rotation.sh
index 854bd70..91f8748 100644
--- a/tests/services/log-rotation.sh
+++ b/tests/services/log-rotation.sh
@@ -26,11 +26,13 @@ pid="t-pid-$$"
 service_log1="$PWD/t-service-log1-$$"
 service_log2="$PWD/t-service-log2-$$"
 external_log="$PWD/t-service-extlog-$$"
+invalid_external_log="$PWD/t-service-invalid-extlog-$$"
 
 herd="herd -s $socket"
 
 trap "zcat $log* || true;
       rm -f $socket $conf $log* $service_log1* $service_log2* $external_log*;
+      rmdir $invalid_external_log;
       test -f $pid && kill \`cat $pid\` || true; rm -f $pid" EXIT
 
 cat > "$conf" <<EOF
@@ -65,7 +67,7 @@ cat > "$conf" <<EOF
         (log-rotation-service
           ;; Arrange so that it does not trigger automatically.
           (calendar-event #:months (list past-month))
-          #:external-log-files '("$external_log")
+          #:external-log-files '("$invalid_external_log" "$external_log")
           #:rotation-size-threshold 0)))
 
 (register-services services)
@@ -83,7 +85,7 @@ $herd start log-rotation
 
 sleep 0.5
 
-for file in "$service_log1" "$service_log2" "$external_log" "$log"
+for file in "$service_log1" "$service_log2" "$external_log" 
"$invalid_external_log" "$log"
 do
     $herd files log-rotation | grep "$file"
 done
@@ -92,6 +94,8 @@ test -f "$service_log1"
 test -f "$service_log2"
 echo "This is an external log file." > "$external_log"
 
+mkdir "$invalid_external_log"                  # not a regular file, so should 
not be rotated
+
 # First rotation.
 $herd trigger log-rotation
 
@@ -109,6 +113,8 @@ gunzip < "$external_log.1.gz" | grep "external log file"
 test -f "$external_log"
 guile -c "(exit (zero? (stat:size (stat \"$external_log\"))))"
 
+zgrep "Not rotating .*$invalid_external_log" "$log"* # not a regular file
+
 # Second rotation.
 $herd trigger log-rotation