fix(backup): rotate the legacy directory backups too, and stop warning about a normal state

app/Support/BackupManager.php

@@ -51,6 +51,16 @@ class BackupManager
      */
     private const GENERATED_NAME_PATTERN = '/^backup_\d{4}-\d{2}-\d{2}_\d{6}_[0-9a-f]{6}\.zip$/';
 
+    /**
+     * The pre-0.7.x layout: a directory holding a single database.sql, written
+     * by an updater that no longer exists. Nothing creates these any more, but
+     * listBackups() still surfaces them as backups (contents: 'db'), so an
+     * operator sees them in the same list — and until now nothing pruned them.
+     * Same discipline as the pattern above: match the EXACT generated shape, so
+     * a directory someone parked there by hand is never a rotation candidate.
+     */
+    private const LEGACY_DIR_PATTERN = '/^update_\d{4}-\d{2}-\d{2}_\d{6}$/';
+
     /**
      * Hard cap for the cumulative DECOMPRESSED size of a restore archive (4 GB).
      * Guards against a decompression-bomb ZIP whose compressed form passes
@@ -242,30 +252,57 @@ private function pruneOldBackups(string $justWritten): void
                 return;
             }
 
-            $files = [];
+            // One pool across BOTH formats, because listBackups() shows them as
+            // one list sorted by date: rotating them separately would let the
+            // operator watch a recent entry vanish while an older one survives.
+            $entries = [];
             foreach (glob($this->backupPath . '/backup_*.zip') ?: [] as $file) {
                 if (!is_file($file) || realpath($file) === realpath($justWritten)) {
                     continue;
                 }
                 if (preg_match(self::GENERATED_NAME_PATTERN, basename($file)) !== 1) {
                     continue; // hand-placed archive: never a rotation candidate
                 }
-                $files[$file] = (int) filemtime($file);
+                $entries[$file] = (int) filemtime($file);
+            }
+            foreach (glob($this->backupPath . '/update_*', GLOB_ONLYDIR) ?: [] as $dir) {
+                // A symlink must never be a rotation candidate: deleteDirectory()
+                // would unlink the link, but a link is not something this class
+                // wrote and not ours to reclaim.
+                if (is_link($dir) || !is_dir($dir)) {
+                    continue;
+                }
+                if (preg_match(self::LEGACY_DIR_PATTERN, basename($dir)) !== 1) {
+                    continue; // hand-placed directory: never a rotation candidate
+                }
+                $entries[$dir] = (int) filemtime($dir);
             }
-            arsort($files);
+            arsort($entries);
 
             // The freshly written file counts against the quota too.
             $slots = max(0, $keep - 1);
-            $stale = array_slice(array_keys($files), $slots);
+            $stale = array_slice(array_keys($entries), $slots);
             $removed = 0;
-            foreach ($stale as $file) {
+            $removedLegacy = 0;
+            foreach ($stale as $path) {
+                if (is_dir($path)) {
+                    if ($this->deleteDirectory($path)) {
+                        $removed++;
+                        $removedLegacy++;
+                    }
+                    continue;
+                }
                 // nosemgrep: php.lang.security.unlink-use.unlink-use -- glob-matched backup_*.zip under storage/backups, not user input
-                if (@unlink($file)) {
+                if (@unlink($path)) {
                     $removed++;
                 }
             }
             if ($removed > 0) {
-                SecureLogger::info('BackupManager: rotated old backups', ['removed' => $removed, 'kept' => $keep]);
+                SecureLogger::info('BackupManager: rotated old backups', [
+                    'removed' => $removed,
+                    'legacy_dirs' => $removedLegacy,
+                    'kept' => $keep,
+                ]);
             }
         } catch (\Throwable $e) {
             SecureLogger::warning('BackupManager: backup rotation failed', ['error' => $e->getMessage()]);
@@ -1420,22 +1457,21 @@ private function readManifestFromZip(string $zipPath): array
         return is_array($decoded) ? $decoded : [];
     }
 
-    private function deleteDirectory(string $dir): void
+    private function deleteDirectory(string $dir): bool
     {
         // A symlinked root must not be followed either — unlink the link
         // itself, never recurse into its target (symmetric with the per-child
         // is_link guard below; is_dir() returns true through a dir symlink). (#167 review)
         if (is_link($dir)) {
             // nosemgrep: php.lang.security.unlink-use.unlink-use -- removes the symlink, not its target
-            @unlink($dir);
-            return;
+            return @unlink($dir);
         }
         if (!is_dir($dir)) {
-            return;
+            return true; // already gone: the postcondition holds
         }
         $files = @scandir($dir);
         if ($files === false) {
-            return;
+            return false;
         }
         foreach (array_diff($files, ['.', '..']) as $file) {
             $path = $dir . '/' . $file;
@@ -1450,7 +1486,11 @@ private function deleteDirectory(string $dir): void
                 @unlink($path);
             }
         }
-        @rmdir($dir);
+        // The return value is what the rotation counts on. Re-checking the path
+        // afterwards would be the obvious alternative, but static analysis has
+        // already narrowed it to "a directory" and cannot see a filesystem side
+        // effect, so the helper reports its own outcome instead.
+        return @rmdir($dir);
     }
 
     private function getCurrentVersion(): string

storage/plugins/api-book-scraper/ApiBookScraperPlugin.php

@@ -139,9 +139,18 @@ private function registerHooks(): void
             return;
         }
 
-        // Se il plugin non è abilitato, non registrare gli hooks
+        // Se il plugin non è abilitato, non registrare gli hooks.
+        //
+        // DEBUG, not warning: a plugin that is present but not configured is the
+        // normal state of every plugin the operator has not set up, and this path
+        // is reached on a schedule. On one production install it produced 8.217 of
+        // the 15.607 lines in app.log over eight months — more than half the file,
+        // for a condition that is not a problem. A warning level that fires
+        // continuously trains the reader to skip warnings, which is how the real
+        // ones get missed. The genuinely abnormal case above (missing DB or plugin
+        // ID) keeps its warning.
         if (!$this->enabled || empty($this->apiEndpoint) || empty($this->apiKey)) {
-            \App\Support\SecureLogger::warning('[ApiBookScraper] Plugin not enabled or missing configuration');
+            \App\Support\SecureLogger::debug('[ApiBookScraper] Plugin not enabled or missing configuration: hooks not registered');
             return;
         }
 

tests/backup-retention.unit.php

@@ -98,6 +98,13 @@
 mkdir($tmp, 0777, true);
 $cleanup = static function () use ($tmp, &$origRetention, &$setRetention): void {
     foreach (glob($tmp . '/*') ?: [] as $f) {
+        if (is_dir($f)) {
+            foreach (glob($f . '/*') ?: [] as $inner) {
+                @unlink($inner);
+            }
+            @rmdir($f);
+            continue;
+        }
         @unlink($f);
     }
     @rmdir($tmp);
@@ -194,5 +201,72 @@
     'the default retention applies when the setting cannot be read (kept ' . $kept . ')');
 
 $cleanup();
+echo "E. the legacy directory format is rotated too\n";
+// Pre-0.7.x updates left a directory holding a single database.sql. Nothing
+// creates them any more, but listBackups() still shows them as backups — and the
+// rotation only ever globbed backup_*.zip, so they accumulated forever. One
+// production install had 60 of them, 17 MB, spanning six months.
+foreach (glob($tmp . '/*') ?: [] as $f) {
+    if (is_dir($f)) {
+        foreach (glob($f . '/*') ?: [] as $inner) { @unlink($inner); }
+        @rmdir($f);
+    } else {
+        @unlink($f);
+    }
+}
+
+/** Seed n legacy update_ directories, oldest first. */
+$seedLegacy = static function (int $n) use ($tmp): array {
+    $paths = [];
+    for ($i = 0; $i < $n; $i++) {
+        $d = $tmp . '/update_2025-12-' . str_pad((string) ($i + 1), 2, '0', STR_PAD_LEFT) . '_000000';
+        @mkdir($d, 0775, true);
+        file_put_contents($d . '/database.sql', 'x');
+        touch($d, time() - ((100 - $i) * 3600));
+        $paths[] = $d;
+    }
+    return $paths;
+};
+
+$legacy = $seedLegacy(8);
+$zips = $seed(4);
+$newest = end($zips);
+$manager = $makeManager($tmp, '5');
+$prune($manager, $newest);
+
+$survivingZips = glob($tmp . '/backup_*.zip') ?: [];
+$survivingDirs = glob($tmp . '/update_*', GLOB_ONLYDIR) ?: [];
+$check(count($survivingZips) + count($survivingDirs) === 5,
+    'both formats share one retention pool (got ' . count($survivingZips) . ' zip + ' . count($survivingDirs) . ' dir)');
+$check(in_array($newest, $survivingZips, true), 'the backup just written still survives');
+// The zips were seeded newer than every legacy directory, so with five slots the
+// four zips plus the single newest directory must be what remains.
+$check(count($survivingZips) === 4, 'the four recent archives all survive');
+$check($survivingDirs === [end($legacy)], 'only the newest legacy directory survives, by age');
+$check(!is_dir($legacy[0]), 'a rotated legacy directory is removed with its contents');
+
+echo "F. hand-placed directories are never touched\n";
+foreach (glob($tmp . '/*') ?: [] as $f) {
+    if (is_dir($f)) {
+        foreach (glob($f . '/*') ?: [] as $inner) { @unlink($inner); }
+        @rmdir($f);
+    } else {
+        @unlink($f);
+    }
+}
+$seedLegacy(6);
+// Same discipline as the archive pattern: only the EXACT generated shape is a
+// candidate. A directory an operator parked here by hand must survive whatever
+// the retention says.
+$manual = $tmp . '/update_migrazione_manuale';
+@mkdir($manual, 0775, true);
+file_put_contents($manual . '/database.sql', 'x');
+touch($manual, time() - (999 * 3600)); // older than every seeded one
+$zips = $seed(2);
+$manager = $makeManager($tmp, '2');
+$prune($manager, end($zips));
+$check(is_dir($manual), 'a hand-named directory is never a rotation candidate');
+$check(is_file($manual . '/database.sql'), 'and its contents are left alone');
+
 echo PHP_EOL . "Passed: {$passed}   Failed: {$failed}" . PHP_EOL;
 exit($failed === 0 ? 0 : 1);