From ecd57f002c26b3bd79acfa0179ce808c4fd67844 Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Wed, 9 Sep 2026 23:40:23 +0200 Subject: [PATCH 01/10] feat(updater): refuse an update that will not fit, and say why a copy failed Both follow-ups from #422, where an update failed on a full account and the error named a file inside vendor/, which sent the diagnosis after a corrupt package that was in fact intact and verified. The update now checks for room before touching anything, right after the existing writability preflight and before the rollback copy that is what actually fills the disk. The requirement is measured from the very directories that copy duplicates: backup, restore and estimate now read one shared constant, so a directory added to the backup cannot silently leave the estimate behind. Two checks, because neither alone is enough on shared hosting: disk_free_space() sees the filesystem, which on a cPanel account happily reports tens of gigabytes while the account's own quota is exhausted, so a real write proves what the account can do right now. The probe is capped at 16 MB, since it has to prove the account can write at all, not reserve the full amount and double every update. When a copy does fail, the message now carries the reason: space or quota exhausted, destination file not writable, directory missing or not writable, otherwise whatever PHP reported. A file name on its own is close to useless, because the copy stops at whatever entry it had reached when the real problem occurred, and that entry is arbitrary. Eight strings, translated in all five catalogues. --- app/Support/Updater.php | 197 ++++++++++++++++++++++++-- locale/da_DK.json | 10 +- locale/de_DE.json | 10 +- locale/en_US.json | 10 +- locale/fr_FR.json | 10 +- locale/it_IT.json | 10 +- tests/update-space-preflight.unit.php | 134 ++++++++++++++++++ 7 files changed, 368 insertions(+), 13 deletions(-) create mode 100644 tests/update-space-preflight.unit.php diff --git a/app/Support/Updater.php b/app/Support/Updater.php index a6d4b87c1..f7c519594 100644 --- a/app/Support/Updater.php +++ b/app/Support/Updater.php @@ -42,6 +42,21 @@ class Updater */ private array $releaseByVersionCache = []; + /** + * Directories copied aside for rollback before an update — and therefore + * the directories whose size the space preflight has to account for. One + * list for both, so an addition here can never make the estimate lie. + */ + private const APP_BACKUP_DIRS = ['app', 'config', 'locale', 'public/assets', 'installer', 'vendor']; + + /** + * Upper bound for the write probe of the space preflight. Writing the full + * requirement would double the cost of every update; this only has to prove + * the account can still write at all, which is what an exhausted quota + * prevents. + */ + private const SPACE_PROBE_BYTES = 16 * 1024 * 1024; + /** @var array Files/directories to preserve during update */ private array $preservePaths = [ '.env', @@ -2303,6 +2318,18 @@ public function installUpdate(string $sourcePath, string $targetVersion): array )); } + // PRE-FLIGHT (issue #422): free space. The next step copies the whole + // application aside for rollback, so an account that is nearly full + // fails MID-COPY on whatever file the loop happens to reach — an + // error naming a file in vendor/ that says nothing about the real + // cause, which is exactly how a healthy release once looked broken. + // Refuse here, with the numbers, instead of failing halfway. + $spaceError = $this->checkFreeSpaceForUpdate(); + if ($spaceError !== null) { + $this->debugLog('ERROR', 'Preflight: spazio insufficiente', ['detail' => $spaceError]); + throw new Exception($spaceError); + } + // Log update start $logId = $this->logUpdateStart($currentVersion, $targetVersion, null); @@ -2419,6 +2446,157 @@ public function installUpdate(string $sourcePath, string $targetVersion): array /** * Backup application files for atomic rollback */ + /** + * Space needed by an update, in bytes: the rollback copy of the directories + * backupAppFiles() duplicates, plus a margin for the new files landing + * alongside the old ones during the copy. + */ + private function estimateUpdateSpace(): int + { + $total = 0; + foreach (self::APP_BACKUP_DIRS as $dir) { + $total += $this->directorySize($this->rootPath . '/' . $dir); + } + // 30% margin: the copy writes new files before the old ones are gone, + // and a partially-fitting update is the failure mode being prevented. + return (int) ($total * 1.3); + } + + /** Size of a directory in bytes; 0 when it does not exist or cannot be read. */ + private function directorySize(string $path): int + { + if (!is_dir($path)) { + return 0; + } + $bytes = 0; + try { + $it = new \RecursiveIteratorIterator( + new \RecursiveDirectoryIterator($path, \FilesystemIterator::SKIP_DOTS), + \RecursiveIteratorIterator::LEAVES_ONLY + ); + foreach ($it as $file) { + if ($file instanceof \SplFileInfo && $file->isFile()) { + $bytes += (int) $file->getSize(); + } + } + } catch (\Throwable $e) { + $this->debugLog('DEBUG', 'Impossibile misurare directory', ['path' => $path, 'error' => $e->getMessage()]); + } + return $bytes; + } + + /** + * Refuse the update when the space it needs is not there. Returns the + * message to show, or null when there is room. + * + * Two checks, because neither alone is enough on shared hosting: + * disk_free_space() sees the FILESYSTEM, which on a cPanel account can + * report tens of gigabytes free while the account's own quota is exhausted; + * and a real write proves what the account can actually do right now. The + * probe is capped — it is meant to detect "cannot write at all", not to + * reserve the full amount, which would double the cost of every update. + */ + private function checkFreeSpaceForUpdate(): ?string + { + $needed = $this->estimateUpdateSpace(); + if ($needed <= 0) { + return null; // nothing measurable to copy: let the update proceed + } + + $free = @disk_free_space($this->rootPath); + if (is_float($free) && $free > 0 && $free < $needed) { + return sprintf( + __('Spazio su disco insufficiente per aggiornare: servono circa %1$s, disponibili %2$s. Libera spazio (i backup automatici in storage/backups sono i primi candidati) e riprova.'), + $this->formatBytes($needed), + $this->formatBytes((int) $free) + ); + } + + $probeBytes = (int) min($needed, self::SPACE_PROBE_BYTES); + if (!$this->canWriteBytes($probeBytes)) { + return sprintf( + __('Impossibile scrivere %1$s di prova: lo spazio disponibile o la quota dell\'account sono esauriti. L\'aggiornamento richiede circa %2$s. Libera spazio (i backup automatici in storage/backups sono i primi candidati) e riprova.'), + $this->formatBytes($probeBytes), + $this->formatBytes($needed) + ); + } + + $this->debugLog('INFO', 'Preflight spazio superato', [ + 'needed_bytes' => $needed, + 'probe_bytes' => $probeBytes, + 'disk_free_bytes' => is_float($free) ? (int) $free : null, + ]); + return null; + } + + /** + * Write and remove a real file of the given size under storage/tmp. + * Sparse allocation would not do: a hole consumes no quota, and the quota + * is precisely what this is testing. + */ + private function canWriteBytes(int $bytes): bool + { + $dir = $this->rootPath . '/storage/tmp'; + if (!is_dir($dir) && !@mkdir($dir, 0775, true) && !is_dir($dir)) { + return false; + } + $probe = $dir . '/.space_probe_' . bin2hex(random_bytes(4)); + $handle = @fopen($probe, 'wb'); + if ($handle === false) { + return false; + } + $ok = true; + try { + $chunk = str_repeat('0', 1024 * 1024); + $written = 0; + while ($written < $bytes) { + $slice = $bytes - $written >= strlen($chunk) ? $chunk : substr($chunk, 0, $bytes - $written); + $result = @fwrite($handle, $slice); + if ($result === false || $result === 0) { + $ok = false; + break; + } + $written += $result; + } + if ($ok && !@fflush($handle)) { + $ok = false; + } + } catch (\Throwable) { + $ok = false; + } finally { + @fclose($handle); + // nosemgrep: php.lang.security.unlink-use.unlink-use -- own probe file under storage/tmp, name generated here + @unlink($probe); + } + return $ok; + } + + /** + * Why a write to this path failed, in words an operator can act on. + * Checked in order of likelihood, ending with whatever PHP reported. + */ + private function describeWriteFailure(string $targetPath): string + { + $dir = dirname($targetPath); + + if (!$this->canWriteBytes(1024 * 1024)) { + return __('spazio su disco o quota dell\'account esauriti'); + } + if (is_file($targetPath) && !is_writable($targetPath)) { + return __('il file di destinazione esiste e non è scrivibile'); + } + if (!is_dir($dir)) { + return __('la directory di destinazione non esiste'); + } + if (!is_writable($dir)) { + return __('la directory di destinazione non è scrivibile'); + } + + $last = error_get_last(); + $message = $last === null ? '' : trim($last['message']); + return $message !== '' ? $message : __('causa sconosciuta'); + } + private function backupAppFiles(): string { $timestamp = date('Y-m-d_His'); @@ -2441,9 +2619,7 @@ private function backupAppFiles(): string throw new Exception(__('Impossibile creare directory di backup applicazione')); } - $dirsToBackup = ['app', 'config', 'locale', 'public/assets', 'installer', 'vendor']; - - foreach ($dirsToBackup as $dir) { + foreach (self::APP_BACKUP_DIRS as $dir) { $sourcePath = $this->rootPath . '/' . $dir; $destPath = $backupPath . '/' . $dir; @@ -2473,9 +2649,7 @@ private function backupAppFiles(): string */ private function restoreAppFiles(string $backupPath): void { - $dirsToRestore = ['app', 'config', 'locale', 'public/assets', 'installer', 'vendor']; - - foreach ($dirsToRestore as $dir) { + foreach (self::APP_BACKUP_DIRS as $dir) { $sourcePath = $backupPath . '/' . $dir; $destPath = $this->rootPath . '/' . $dir; @@ -2568,8 +2742,15 @@ private function copyDirectoryRecursive(string $source, string $dest): void throw new Exception(sprintf(__('Impossibile creare directory: %s'), dirname($relativePath))); } } - if (!copy(str_replace('\\', '/', $item->getPathname()), $targetPath)) { - throw new Exception(sprintf(__('Errore nella copia del file: %s'), $relativePath)); + if (!@copy(str_replace('\\', '/', $item->getPathname()), $targetPath)) { + // The file name alone is misleading: the copy stops at + // whatever entry it had reached when the real problem (a + // full disk, a read-only target) occurred. Say why. + throw new Exception(sprintf( + __('Errore nella copia del file: %s — %s'), + $relativePath, + $this->describeWriteFailure($targetPath) + )); } } } diff --git a/locale/da_DK.json b/locale/da_DK.json index 1960d1135..e48f1b287 100644 --- a/locale/da_DK.json +++ b/locale/da_DK.json @@ -7505,5 +7505,13 @@ "Abbonamento eliminato": "Abonnement slettet", "Eventi plugin": "Pluginhændelser", "Annata %1$d conservata separatamente come volume «%2$s» per mantenere i dati descrittivi.": "Årgang %1$d bevaret separat som bind »%2$s« for at bevare de beskrivende oplysninger.", - "Le annate con dati descrittivi discordanti restano separate con un volume distinto.": "Årgange med modstridende beskrivende oplysninger bevares separat med en særskilt bindbetegnelse." + "Le annate con dati descrittivi discordanti restano separate con un volume distinto.": "Årgange med modstridende beskrivende oplysninger bevares separat med en særskilt bindbetegnelse.", + "Spazio su disco insufficiente per aggiornare: servono circa %1$s, disponibili %2$s. Libera spazio (i backup automatici in storage/backups sono i primi candidati) e riprova.": "Ikke nok diskplads til opdateringen: der kræves ca. %1$s, %2$s er tilgængelige. Frigør plads (de automatiske sikkerhedskopier i storage/backups er de første kandidater), og prøv igen.", + "Impossibile scrivere %1$s di prova: lo spazio disponibile o la quota dell'account sono esauriti. L'aggiornamento richiede circa %2$s. Libera spazio (i backup automatici in storage/backups sono i primi candidati) e riprova.": "Kunne ikke skrive en testfil på %1$s: den tilgængelige plads eller kontoens kvote er opbrugt. Opdateringen kræver ca. %2$s. Frigør plads (de automatiske sikkerhedskopier i storage/backups er de første kandidater), og prøv igen.", + "spazio su disco o quota dell'account esauriti": "diskplads eller kontokvote opbrugt", + "il file di destinazione esiste e non è scrivibile": "destinationsfilen findes og kan ikke skrives", + "la directory di destinazione non esiste": "destinationsmappen findes ikke", + "la directory di destinazione non è scrivibile": "destinationsmappen kan ikke skrives", + "causa sconosciuta": "ukendt årsag", + "Errore nella copia del file: %s — %s": "Fejl ved kopiering af filen: %s — %s" } diff --git a/locale/de_DE.json b/locale/de_DE.json index 93dc1b40e..190ad1715 100644 --- a/locale/de_DE.json +++ b/locale/de_DE.json @@ -7505,5 +7505,13 @@ "Abbonamento eliminato": "Abonnement gelöscht", "Eventi plugin": "Plugin-Ereignisse", "Annata %1$d conservata separatamente come volume «%2$s» per mantenere i dati descrittivi.": "Jahrgang %1$d separat als Band „%2$s“ beibehalten, um die beschreibenden Daten zu erhalten.", - "Le annate con dati descrittivi discordanti restano separate con un volume distinto.": "Jahrgänge mit widersprüchlichen Beschreibungsdaten bleiben unter einer eigenen Bandbezeichnung getrennt." + "Le annate con dati descrittivi discordanti restano separate con un volume distinto.": "Jahrgänge mit widersprüchlichen Beschreibungsdaten bleiben unter einer eigenen Bandbezeichnung getrennt.", + "Spazio su disco insufficiente per aggiornare: servono circa %1$s, disponibili %2$s. Libera spazio (i backup automatici in storage/backups sono i primi candidati) e riprova.": "Nicht genügend Speicherplatz für das Update: benötigt werden etwa %1$s, verfügbar sind %2$s. Geben Sie Speicherplatz frei (die automatischen Sicherungen in storage/backups sind die ersten Kandidaten) und versuchen Sie es erneut.", + "Impossibile scrivere %1$s di prova: lo spazio disponibile o la quota dell'account sono esauriti. L'aggiornamento richiede circa %2$s. Libera spazio (i backup automatici in storage/backups sono i primi candidati) e riprova.": "Eine Testdatei von %1$s konnte nicht geschrieben werden: der verfügbare Speicher oder das Kontingent des Kontos ist erschöpft. Das Update benötigt etwa %2$s. Geben Sie Speicherplatz frei (die automatischen Sicherungen in storage/backups sind die ersten Kandidaten) und versuchen Sie es erneut.", + "spazio su disco o quota dell'account esauriti": "Speicherplatz oder Kontingent des Kontos erschöpft", + "il file di destinazione esiste e non è scrivibile": "die Zieldatei existiert und ist nicht beschreibbar", + "la directory di destinazione non esiste": "das Zielverzeichnis existiert nicht", + "la directory di destinazione non è scrivibile": "das Zielverzeichnis ist nicht beschreibbar", + "causa sconosciuta": "unbekannte Ursache", + "Errore nella copia del file: %s — %s": "Fehler beim Kopieren der Datei: %s — %s" } diff --git a/locale/en_US.json b/locale/en_US.json index ead22168a..a09bcc147 100644 --- a/locale/en_US.json +++ b/locale/en_US.json @@ -7505,5 +7505,13 @@ "Abbonamento eliminato": "Subscription deleted", "Eventi plugin": "Plugin events", "Annata %1$d conservata separatamente come volume «%2$s» per mantenere i dati descrittivi.": "Year %1$d retained separately as volume “%2$s” to preserve its descriptive data.", - "Le annate con dati descrittivi discordanti restano separate con un volume distinto.": "Years with conflicting descriptive data remain separate under a distinct volume." + "Le annate con dati descrittivi discordanti restano separate con un volume distinto.": "Years with conflicting descriptive data remain separate under a distinct volume.", + "Spazio su disco insufficiente per aggiornare: servono circa %1$s, disponibili %2$s. Libera spazio (i backup automatici in storage/backups sono i primi candidati) e riprova.": "Not enough disk space to update: about %1$s is needed, %2$s available. Free some space (the automatic backups in storage/backups are the first candidates) and try again.", + "Impossibile scrivere %1$s di prova: lo spazio disponibile o la quota dell'account sono esauriti. L'aggiornamento richiede circa %2$s. Libera spazio (i backup automatici in storage/backups sono i primi candidati) e riprova.": "Could not write a %1$s test file: the available space or the account quota is exhausted. The update needs about %2$s. Free some space (the automatic backups in storage/backups are the first candidates) and try again.", + "spazio su disco o quota dell'account esauriti": "disk space or account quota exhausted", + "il file di destinazione esiste e non è scrivibile": "the destination file exists and is not writable", + "la directory di destinazione non esiste": "the destination directory does not exist", + "la directory di destinazione non è scrivibile": "the destination directory is not writable", + "causa sconosciuta": "unknown cause", + "Errore nella copia del file: %s — %s": "Error copying file: %s — %s" } diff --git a/locale/fr_FR.json b/locale/fr_FR.json index 206da771f..918d3c9b2 100644 --- a/locale/fr_FR.json +++ b/locale/fr_FR.json @@ -7505,5 +7505,13 @@ "Abbonamento eliminato": "Abonnement supprimé", "Eventi plugin": "Événements des extensions", "Annata %1$d conservata separatamente come volume «%2$s» per mantenere i dati descrittivi.": "Année %1$d conservée séparément comme volume « %2$s » pour préserver les données descriptives.", - "Le annate con dati descrittivi discordanti restano separate con un volume distinto.": "Les années dont les données descriptives sont contradictoires restent séparées sous un volume distinct." + "Le annate con dati descrittivi discordanti restano separate con un volume distinto.": "Les années dont les données descriptives sont contradictoires restent séparées sous un volume distinct.", + "Spazio su disco insufficiente per aggiornare: servono circa %1$s, disponibili %2$s. Libera spazio (i backup automatici in storage/backups sono i primi candidati) e riprova.": "Espace disque insuffisant pour la mise à jour : environ %1$s sont nécessaires, %2$s disponibles. Libérez de l'espace (les sauvegardes automatiques dans storage/backups sont les premières candidates) et réessayez.", + "Impossibile scrivere %1$s di prova: lo spazio disponibile o la quota dell'account sono esauriti. L'aggiornamento richiede circa %2$s. Libera spazio (i backup automatici in storage/backups sono i primi candidati) e riprova.": "Impossible d'écrire un fichier de test de %1$s : l'espace disponible ou le quota du compte est épuisé. La mise à jour nécessite environ %2$s. Libérez de l'espace (les sauvegardes automatiques dans storage/backups sont les premières candidates) et réessayez.", + "spazio su disco o quota dell'account esauriti": "espace disque ou quota du compte épuisé", + "il file di destinazione esiste e non è scrivibile": "le fichier de destination existe et n'est pas accessible en écriture", + "la directory di destinazione non esiste": "le répertoire de destination n'existe pas", + "la directory di destinazione non è scrivibile": "le répertoire de destination n'est pas accessible en écriture", + "causa sconosciuta": "cause inconnue", + "Errore nella copia del file: %s — %s": "Erreur lors de la copie du fichier : %s — %s" } diff --git a/locale/it_IT.json b/locale/it_IT.json index ae3aa8f02..9e17714e4 100644 --- a/locale/it_IT.json +++ b/locale/it_IT.json @@ -7505,5 +7505,13 @@ "Abbonamento eliminato": "Abbonamento eliminato", "Eventi plugin": "Eventi plugin", "Annata %1$d conservata separatamente come volume «%2$s» per mantenere i dati descrittivi.": "Annata %1$d conservata separatamente come volume «%2$s» per mantenere i dati descrittivi.", - "Le annate con dati descrittivi discordanti restano separate con un volume distinto.": "Le annate con dati descrittivi discordanti restano separate con un volume distinto." + "Le annate con dati descrittivi discordanti restano separate con un volume distinto.": "Le annate con dati descrittivi discordanti restano separate con un volume distinto.", + "Spazio su disco insufficiente per aggiornare: servono circa %1$s, disponibili %2$s. Libera spazio (i backup automatici in storage/backups sono i primi candidati) e riprova.": "Spazio su disco insufficiente per aggiornare: servono circa %1$s, disponibili %2$s. Libera spazio (i backup automatici in storage/backups sono i primi candidati) e riprova.", + "Impossibile scrivere %1$s di prova: lo spazio disponibile o la quota dell'account sono esauriti. L'aggiornamento richiede circa %2$s. Libera spazio (i backup automatici in storage/backups sono i primi candidati) e riprova.": "Impossibile scrivere %1$s di prova: lo spazio disponibile o la quota dell'account sono esauriti. L'aggiornamento richiede circa %2$s. Libera spazio (i backup automatici in storage/backups sono i primi candidati) e riprova.", + "spazio su disco o quota dell'account esauriti": "spazio su disco o quota dell'account esauriti", + "il file di destinazione esiste e non è scrivibile": "il file di destinazione esiste e non è scrivibile", + "la directory di destinazione non esiste": "la directory di destinazione non esiste", + "la directory di destinazione non è scrivibile": "la directory di destinazione non è scrivibile", + "causa sconosciuta": "causa sconosciuta", + "Errore nella copia del file: %s — %s": "Errore nella copia del file: %s — %s" } diff --git a/tests/update-space-preflight.unit.php b/tests/update-space-preflight.unit.php new file mode 100644 index 000000000..f86349576 --- /dev/null +++ b/tests/update-space-preflight.unit.php @@ -0,0 +1,134 @@ +setAccessible(true); + return $ref->invoke($u, ...$args); +}; + +$env = []; +foreach (preg_split('/\r?\n/', (string) @file_get_contents($root . '/.env')) as $line) { + if (!str_contains($line, '=') || str_starts_with(trim($line), '#')) { + continue; + } + [$k, $v] = explode('=', $line, 2); + $env[trim($k)] = trim(trim($v), "\"'"); +} +$socket = getenv('E2E_DB_SOCKET') ?: ($env['DB_SOCKET'] ?? ''); +try { + $db = $socket !== '' && file_exists($socket) + ? new mysqli(null, getenv('E2E_DB_USER') ?: ($env['DB_USER'] ?? ''), getenv('E2E_DB_PASS') ?: ($env['DB_PASS'] ?? ($env['DB_PASSWORD'] ?? '')), getenv('E2E_DB_NAME') ?: ($env['DB_NAME'] ?? ''), 0, $socket) + : new mysqli(getenv('E2E_DB_HOST') ?: ($env['DB_HOST'] ?? '127.0.0.1'), getenv('E2E_DB_USER') ?: ($env['DB_USER'] ?? ''), getenv('E2E_DB_PASS') ?: ($env['DB_PASS'] ?? ($env['DB_PASSWORD'] ?? '')), getenv('E2E_DB_NAME') ?: ($env['DB_NAME'] ?? ''), (int) (getenv('E2E_DB_PORT') ?: ($env['DB_PORT'] ?? 3306))); + $db->set_charset('utf8mb4'); +} catch (Throwable $e) { + fwrite(STDERR, "FAIL: database unreachable — Updater needs a connection: {$e->getMessage()}\n"); + exit(1); +} + +$updater = new Updater($db, $root); + +echo "A. the estimate follows the directories the rollback backup copies\n"; +$dirsRef = new ReflectionClassConstant(Updater::class, 'APP_BACKUP_DIRS'); +$dirs = $dirsRef->getValue(); +$check(is_array($dirs) && $dirs !== [], 'the backup directory list is a shared constant'); +$sum = 0; +foreach ($dirs as $dir) { + $sum += (int) $call($updater, 'directorySize', [$root . '/' . $dir]); +} +$estimate = (int) $call($updater, 'estimateUpdateSpace'); +$check($sum > 0, 'the measured directories are not empty on this checkout'); +$check($estimate > $sum, 'the estimate adds a margin over the raw size'); +$check($estimate < $sum * 2, 'the margin stays proportionate (no runaway over-estimate)'); + +// The guarantee that matters: a directory added to the backup must show up in +// the estimate by construction. Reading the same constant in both places is +// what provides it, so assert the backup really uses it. +$src = (string) file_get_contents($root . '/app/Support/Updater.php'); +$check(substr_count($src, 'self::APP_BACKUP_DIRS') >= 3, + 'backup, restore and estimate all read the same constant'); + +echo "B. the write probe reflects what can actually be written\n"; +$check($call($updater, 'canWriteBytes', [1024]) === true, 'a small write succeeds on a healthy install'); +// An absurd request must be refused rather than filling the disk trying. +$huge = PHP_INT_MAX; +$freeBytes = @disk_free_space($root); +if (is_float($freeBytes) && $freeBytes > 0) { + $check($call($updater, 'canWriteBytes', [(int) $freeBytes + (1024 * 1024 * 1024)]) === false, + 'a request larger than the free space is refused'); +} else { + echo " SKIP disk_free_space() unavailable on this filesystem\n"; +} + +echo "C. a copy failure is described by cause, not just by file name\n"; +$tmp = sys_get_temp_dir() . '/zz_preflight_' . bin2hex(random_bytes(4)); +mkdir($tmp, 0777, true); + +$missingDir = $tmp . '/nope/deeper/file.txt'; +$reason = (string) $call($updater, 'describeWriteFailure', [$missingDir]); +$check(str_contains($reason, 'directory') && str_contains($reason, 'esiste'), + 'a missing destination directory is named as such (' . $reason . ')'); + +$roFile = $tmp . '/readonly.txt'; +file_put_contents($roFile, 'x'); +chmod($roFile, 0444); +$reason = (string) $call($updater, 'describeWriteFailure', [$roFile]); +$check(str_contains($reason, 'non è scrivibile'), 'an unwritable target file is named as such (' . $reason . ')'); +chmod($roFile, 0644); + +$roDir = $tmp . '/rodir'; +mkdir($roDir, 0555, true); +$reason = (string) $call($updater, 'describeWriteFailure', [$roDir . '/new.txt']); +$check(str_contains($reason, 'directory') && str_contains($reason, 'scrivibile'), + 'an unwritable destination directory is named as such (' . $reason . ')'); +chmod($roDir, 0755); + +// The message must never be empty: an empty reason is the original defect. +$reason = (string) $call($updater, 'describeWriteFailure', [$tmp . '/plain.txt']); +$check(trim($reason) !== '', 'a cause is always reported, even when nothing obvious is wrong'); + +foreach (glob($tmp . '/*') ?: [] as $f) { + is_dir($f) ? @rmdir($f) : @unlink($f); +} +@rmdir($tmp); + +echo "D. the preflight does not block a healthy installation\n"; +$verdict = $call($updater, 'checkFreeSpaceForUpdate'); +$check($verdict === null, 'this checkout passes the preflight (' . var_export($verdict, true) . ')'); +$check(count(glob($root . '/storage/tmp/.space_probe_*') ?: []) === 0, 'the probe file is removed after the check'); + +echo PHP_EOL . "Passed: {$passed} Failed: {$failed}" . PHP_EOL; +exit($failed === 0 ? 0 : 1); From c07747b8740f75ce2b835490745395bed184427a Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Thu, 10 Sep 2026 09:06:47 +0200 Subject: [PATCH 02/10] fix(updater): prove the whole space requirement, and name the cause on every write MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The preflight added in the previous commit covered about a sixth of the failure it was written for. Its only comparison against the requirement lived in the disk_free_space() branch, which its own docblock calls unreliable under a cPanel account quota, while the fallback probe was capped at 16MB — so it could tell "cannot write at all" from "can write", but never "cannot write enough". An account with headroom between 16MB and the ~95MB the update needs passed both checks and still died mid-copy. The probe is now one self-bounding tri-state primitive. It refuses up front when the filesystem provably cannot hold the request, which is what lets an impossible request be answered in microseconds instead of by filling the volume; above the dense threshold it charges the full range to the quota by touching one byte per 1KiB block, so the real requirement is proven at a thousandth of the I/O; and it distinguishes "the probe could not be created" from "the write ran out of room". That last distinction matters twice: an unwritable storage/tmp used to be reported as an exhausted quota, and in the gate it used to refuse a legitimate update with a message about disk space when the actual problem was a permission. The diagnosis was also only on one of the two copy engines. copyDirectory() — which lays the new release over the live installation immediately after the backup has duplicated the tree, i.e. at the fullest moment of the whole update — still threw the bare "Errore nella copia del file: " that made a healthy 0.7.82 release look corrupt. It, both mkdir branches, the symlink-replacement branch, the download save, both extraction failures and BackupManager's three write failures now all name their cause. So does scripts/manual-upgrade.php, which the updater's own error message points operators to and which had neither the probe nor a cause; it keeps its own dependency-free copies of both helpers. The gate also ran too late to matter. It sat inside installUpdate(), after createBackup() had written a dump and downloadUpdate() had written a ZIP and extracted it — together as much as the rollback copy it was modelling. Both entry points now check before createBackup(), sized for the whole update via an optional extra-bytes argument. The gate inside installUpdate() is kept, not moved: it re-measures after those steps have consumed disk, and that is the only measurement valid for the copy. Two defects in the test shipped with the original commit. It asked the probe for free space plus 1GB, and since the 16MB bound lived only in the caller, that wrote real 1MB chunks until ENOSPC — the assertion passed by filling the disk, which it did once during review on a 106GB volume. And two assertions manufactured failures with chmod, which uid 0 does not honour, so a root run failed for reasons unrelated to the code. Both are fixed: the file now runs in 0.8s with a negligible space delta, and the permission-dependent cases assert what the function should say in whichever state actually obtains rather than skipping — an indented skip notice is invisible to ci-run-unit-tests.sh and would have reported as a pass. Fix group FG-1 — F003, F004, F008, F015, F016, F024, F026. Post-fix review: 6 findings verified, 1 partial, 0 regressions. F026 is partial: describeWriteFailure() correctly captures error_get_last() as its first statement, and two of the four new call sites hoist the cause above debugLog() for the same reason — SecureLogger ends in a suppressed file_put_contents that would replace the error being reported. The other two call sites (downloadUpdate's save failure, extractPackage's teardown) still log first. The window is narrow, since ENOSPC is caught earlier by the probe, but the convention is applied inconsistently within one file. Left open for a follow-up rather than fixed blind at commit time. Verified before committing: php -l clean on all four PHP files, locale keys and placeholders aligned across the five locales, PHPStan level 5 clean over 540 files, and the unit suite at 42 passed / 0 failed in 0.80s. The strided allocation was checked empirically rather than assumed: a 1KiB stride against 4096-byte blocks allocates 100% of the range, so the quota is genuinely charged and the probe is not silently sparse. --- app/Support/BackupManager.php | 91 ++++++- app/Support/Updater.php | 354 +++++++++++++++++++++----- locale/da_DK.json | 6 +- locale/de_DE.json | 6 +- locale/en_US.json | 6 +- locale/fr_FR.json | 6 +- locale/it_IT.json | 6 +- scripts/manual-upgrade.php | 172 ++++++++++++- tests/update-space-preflight.unit.php | 252 ++++++++++++++++-- 9 files changed, 808 insertions(+), 91 deletions(-) diff --git a/app/Support/BackupManager.php b/app/Support/BackupManager.php index 4db01da3e..8c7e9c453 100644 --- a/app/Support/BackupManager.php +++ b/app/Support/BackupManager.php @@ -65,6 +65,77 @@ public function __construct(mysqli $db, string $rootPath) $this->backupPath = $this->rootPath . '/storage/backups'; } + /** + * Why a write to this path failed, in words an operator can act on. + * + * A deliberate local mirror of Updater::describeWriteFailure(): this class + * is used by the admin Backup UI as well as by the updater and must not + * depend on it. The message strings are the SAME msgids, so the five locale + * files already carry them. + * + * Cheapest and most specific first, and error_get_last() is captured before + * any I/O of our own — it is process-global and the diagnosis would + * otherwise overwrite the very error it explains. The probe writes a real + * 1 MB file: a sparse hole consumes no quota, and the quota is what an + * exhausted cPanel account runs out of while the filesystem still reports + * gigabytes free. + */ + private function describeWriteFailure(string $targetPath): string + { + $last = error_get_last(); + $dir = dirname($targetPath); + + if (is_file($targetPath) && !is_writable($targetPath)) { + return __('il file di destinazione esiste e non è scrivibile'); + } + if (!is_dir($dir)) { + return __('la directory di destinazione non esiste'); + } + if (!is_writable($dir)) { + return __('la directory di destinazione non è scrivibile'); + } + if ($this->outOfSpace($dir)) { + return __('spazio su disco o quota dell\'account esauriti'); + } + + $message = $last === null ? '' : trim($last['message']); + return $message !== '' ? $message : __('causa sconosciuta'); + } + + /** True when a real, bounded 1 MB write into $dir cannot be completed. */ + private function outOfSpace(string $dir): bool + { + $bytes = 1024 * 1024; + $free = @disk_free_space($dir); + if (is_float($free) && $free > 0 && $free < $bytes) { + return true; + } + $probe = $dir . '/.backup_probe_' . bin2hex(random_bytes(4)); + $handle = @fopen($probe, 'wb'); + if ($handle === false) { + // Could not create the probe at all: that is a permission problem, + // not a space one, and the caller's stat checks already covered it. + return false; + } + $ok = true; + try { + $written = @fwrite($handle, str_repeat('0', $bytes)); + if ($written === false || $written < $bytes) { + $ok = false; + } + if ($ok && !@fflush($handle)) { + $ok = false; + } + } catch (\Throwable) { + $ok = false; + } finally { + @fclose($handle); + // nosemgrep: php.lang.security.unlink-use.unlink-use -- own probe file, name generated here + @unlink($probe); + } + return !$ok; + } + // --------------------------------------------------------------------- // Create // --------------------------------------------------------------------- @@ -85,7 +156,10 @@ public function createBackup(string $scope = 'full'): array throw new \RuntimeException(__('Estensione ZipArchive non disponibile')); } if (!is_dir($this->backupPath) && !@mkdir($this->backupPath, 0755, true) && !is_dir($this->backupPath)) { - throw new \RuntimeException(__('Impossibile creare directory di backup')); + throw new \RuntimeException( + __('Impossibile creare directory di backup') + . ' — ' . $this->describeWriteFailure($this->backupPath) + ); } // Serialize the whole write-then-rotate sequence. Two concurrent @@ -108,7 +182,13 @@ public function createBackup(string $scope = 'full'): array // 2. Open the ZIP. $zip = new ZipArchive(); if ($zip->open($zipPath, ZipArchive::CREATE | ZipArchive::OVERWRITE) !== true) { - throw new \RuntimeException(__('Impossibile creare il file di backup')); + // This is the FIRST writer of the whole update flow: on an + // account whose quota is exhausted it is the most likely place + // to fail, and "Backup fallito" alone says nothing about why. + throw new \RuntimeException( + __('Impossibile creare il file di backup') + . ' — ' . $this->describeWriteFailure($zipPath) + ); } $zip->addFile($sqlTmp, 'database.sql'); @@ -137,7 +217,12 @@ public function createBackup(string $scope = 'full'): array $zip->addFromString('manifest.json', (string) json_encode($manifest, JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES)); if (!$zip->close()) { - throw new \RuntimeException(__('Errore nella scrittura del backup')); + // ZipArchive flushes on close: a quota that ran out while the + // entries were being written surfaces HERE, not at open(). + throw new \RuntimeException( + __('Errore nella scrittura del backup') + . ' — ' . $this->describeWriteFailure($zipPath) + ); } // nosemgrep: php.lang.security.unlink-use.unlink-use -- tempnam()-generated temp dump path, not user input diff --git a/app/Support/Updater.php b/app/Support/Updater.php index f7c519594..fac85244d 100644 --- a/app/Support/Updater.php +++ b/app/Support/Updater.php @@ -50,13 +50,31 @@ class Updater private const APP_BACKUP_DIRS = ['app', 'config', 'locale', 'public/assets', 'installer', 'vendor']; /** - * Upper bound for the write probe of the space preflight. Writing the full - * requirement would double the cost of every update; this only has to prove - * the account can still write at all, which is what an exhausted quota - * prevents. + * Size above which the write probe switches from streaming every byte to + * touching one byte per filesystem block. Below it the payload is small + * enough that a dense write costs nothing; above it, streaming the whole + * requirement through PHP is what would risk a timeout on shared hosting. + * It is NOT a cap on what the probe may prove — see probeWrite(). */ private const SPACE_PROBE_BYTES = 16 * 1024 * 1024; + /** + * Stride of the large-size probe: one byte written every N bytes. A hole + * consumes no quota but a touched block does, so a stride no larger than + * the filesystem block size allocates the WHOLE range while writing a + * thousandth of it. 1 KiB is at or below every block size in practice. + */ + private const SPACE_PROBE_STRIDE = 1024; + + /** + * Memoised write-probe verdicts for this update run, keyed by the number of + * bytes asked for. describeWriteFailure() is called once per failing file — + * inside a bundled-plugin loop that does not re-throw — and must not write a + * probe onto the very disk it is diagnosing twenty times over. + * @var array + */ + private array $probeVerdicts = []; + /** @var array Files/directories to preserve during update */ private array $preservePaths = [ '.env', @@ -1218,12 +1236,15 @@ public function downloadUpdate(string $version): array // Create temp directory if (!is_dir($this->tempPath)) { $this->debugLog('DEBUG', 'Creazione directory temporanea', ['path' => $this->tempPath]); - if (!mkdir($this->tempPath, 0755, true) && !is_dir($this->tempPath)) { + if (!@mkdir($this->tempPath, 0755, true) && !is_dir($this->tempPath)) { + // Describe first: debugLog() writes to disk and would replace + // the mkdir error that error_get_last() has to report. + $cause = $this->describeWriteFailure($this->tempPath); $this->debugLog('ERROR', 'Impossibile creare directory temporanea', [ 'path' => $this->tempPath, - 'error' => error_get_last() + 'error' => $cause ]); - throw new Exception(__('Impossibile creare directory temporanea')); + throw new Exception(__('Impossibile creare directory temporanea') . ' — ' . $cause); } } @@ -1428,7 +1449,13 @@ public function downloadUpdate(string $version): array 'path' => $zipPath, 'error' => error_get_last() ]); - throw new Exception(__('Impossibile salvare il file di aggiornamento')); + // Name the cause: on a full account this is the FIRST write to + // fail, and "Download fallito" alone sends the operator looking + // for a corrupt release instead of for free space. + throw new Exception( + __('Impossibile salvare il file di aggiornamento') + . ' — ' . $this->describeWriteFailure($zipPath) + ); } $this->debugLog('INFO', 'File salvato', [ @@ -1570,13 +1597,17 @@ public function downloadUpdate(string $version): array 'zip_status' => $zip->status, 'last_error' => error_get_last() ]); + // Describe BEFORE the cleanup below deletes the extraction path + // and the ZIP: once they are gone the probe can no longer see + // the state that produced the failure. + $cause = $this->describeWriteFailure($extractPath); // Clean up if (is_dir($extractPath)) { $this->deleteDirectory($extractPath); } // nosemgrep: php.lang.security.unlink-use.unlink-use -- internal updater-controlled path (constant or constructed under storage), not user input @unlink($zipPath); - throw new Exception(__('Estrazione del pacchetto fallita')); + throw new Exception(__('Estrazione del pacchetto fallita') . ' — ' . $cause); } $zip->close(); @@ -1677,6 +1708,30 @@ protected function getReleaseByVersion(string $version): ?array } } + /** + * Size in bytes of the installable pinakes-*.zip asset of a release, or 0 + * when it cannot be determined. Used by the early space gate to account for + * the download and its extraction; getReleaseByVersion() is memoised, so + * asking here costs no extra GitHub round trip. + */ + private function releaseAssetBytes(string $version): int + { + $release = $this->getReleaseByVersion($version); + if (!is_array($release) || !isset($release['assets']) || !is_array($release['assets'])) { + return 0; + } + foreach ($release['assets'] as $asset) { + if (!is_array($asset) || !isset($asset['name']) || !is_string($asset['name'])) { + continue; + } + if (preg_match('/pinakes.*\.zip$/i', $asset['name']) && isset($asset['size'])) { + $size = (int) $asset['size']; + return $size > 0 ? $size : 0; + } + } + return 0; + } + /** * Save uploaded update package to temp directory * @return array{success: bool, path: string|null, error: string|null} @@ -1689,8 +1744,11 @@ public function saveUploadedPackage(\Psr\Http\Message\UploadedFileInterface $upl // Create temp directory for uploaded package $uploadTempPath = $this->rootPath . '/storage/tmp/manual_update_' . bin2hex(random_bytes(16)); - if (!mkdir($uploadTempPath, 0755, true)) { - throw new Exception(__('Impossibile creare directory temporanea per upload')); + if (!@mkdir($uploadTempPath, 0755, true) && !is_dir($uploadTempPath)) { + throw new Exception( + __('Impossibile creare directory temporanea per upload') + . ' — ' . $this->describeWriteFailure($uploadTempPath) + ); } $this->debugLog('DEBUG', 'Directory temporanea creata', ['path' => $uploadTempPath]); @@ -1864,6 +1922,19 @@ public function performUpdateFromFile(string $uploadTempPath): array $result = null; try { + // PRE-FLIGHT (issue #422): same early gate as the automatic route. + // This is the path an operator reaches for AFTER the automatic + // update failed, so it is the one most likely to run on an account + // that is already short of room; it had no space check at all + // before the backup and the extraction. + $uploadedZip = $uploadTempPath . '/update.zip'; + $uploadedBytes = is_file($uploadedZip) ? (int) filesize($uploadedZip) : 0; + $spaceError = $this->checkFreeSpaceForUpdate($uploadedBytes > 0 ? $uploadedBytes * 4 : 0); + if ($spaceError !== null) { + $this->debugLog('ERROR', 'Preflight iniziale: spazio insufficiente', ['detail' => $spaceError]); + throw new Exception($spaceError); + } + // Step 1: Backup $this->debugLog('INFO', '>>> STEP 1: Creazione backup <<<'); $backupResult = $this->createBackup(); @@ -1874,15 +1945,18 @@ public function performUpdateFromFile(string $uploadTempPath): array // Step 2: Extract uploaded ZIP $this->debugLog('INFO', '>>> STEP 2: Estrazione pacchetto caricato <<<'); - $zipPath = $uploadTempPath . '/update.zip'; + $zipPath = $uploadedZip; if (!file_exists($zipPath)) { throw new Exception(__('Pacchetto caricato non trovato')); } $extractPath = $uploadTempPath . '/extracted'; - if (!mkdir($extractPath, 0755, true)) { - throw new Exception(__('Impossibile creare directory di estrazione')); + if (!@mkdir($extractPath, 0755, true) && !is_dir($extractPath)) { + throw new Exception( + __('Impossibile creare directory di estrazione') + . ' — ' . $this->describeWriteFailure($extractPath) + ); } $zip = new ZipArchive(); @@ -1905,7 +1979,12 @@ public function performUpdateFromFile(string $uploadTempPath): array if (!$zip->extractTo($extractPath)) { $zip->close(); - throw new Exception(__('Estrazione del pacchetto fallita')); + // An extraction that runs out of room says nothing about the + // package; say which of the two it was. + throw new Exception( + __('Estrazione del pacchetto fallita') + . ' — ' . $this->describeWriteFailure($extractPath) + ); } $zip->close(); @@ -2492,16 +2571,24 @@ private function directorySize(string $path): int * Two checks, because neither alone is enough on shared hosting: * disk_free_space() sees the FILESYSTEM, which on a cPanel account can * report tens of gigabytes free while the account's own quota is exhausted; - * and a real write proves what the account can actually do right now. The - * probe is capped — it is meant to detect "cannot write at all", not to - * reserve the full amount, which would double the cost of every update. + * and a real allocation proves what the account can actually do right now. + * The allocation is for the WHOLE requirement — proving that 16 MB can be + * written says nothing about an account with 50 MB of headroom facing a + * 120 MB update, which is precisely the failure this gate exists to stop. + * + * @param int $extraBytes Space the caller knows it is about to consume on + * top of the rollback copy (the release ZIP and its extracted tree, + * the pre-update backup archive). 0 when there is nothing to add. */ - private function checkFreeSpaceForUpdate(): ?string + private function checkFreeSpaceForUpdate(int $extraBytes = 0): ?string { $needed = $this->estimateUpdateSpace(); if ($needed <= 0) { return null; // nothing measurable to copy: let the update proceed } + if ($extraBytes > 0) { + $needed += $extraBytes; + } $free = @disk_free_space($this->rootPath); if (is_float($free) && $free > 0 && $free < $needed) { @@ -2512,51 +2599,136 @@ private function checkFreeSpaceForUpdate(): ?string ); } - $probeBytes = (int) min($needed, self::SPACE_PROBE_BYTES); - if (!$this->canWriteBytes($probeBytes)) { + // Fresh probe: this gate is called again after the backup and the + // download have consumed disk, and a memoised verdict from before them + // would answer for a state that no longer exists. + $verdict = $this->probeWrite($needed, true); + if ($verdict === 'unavailable') { + // NOT a space problem: the working directory itself is missing or + // read-only. Saying "free up space" here would reproduce the exact + // misdiagnosis this preflight was added to remove. return sprintf( - __('Impossibile scrivere %1$s di prova: lo spazio disponibile o la quota dell\'account sono esauriti. L\'aggiornamento richiede circa %2$s. Libera spazio (i backup automatici in storage/backups sono i primi candidati) e riprova.'), - $this->formatBytes($probeBytes), + __('Impossibile usare la directory di lavoro dell\'aggiornamento (%s): non esiste o non è scrivibile dall\'utente del web server. Correggi i permessi e riprova.'), + $this->rootPath . '/storage/tmp' + ); + } + if ($verdict === 'nospace') { + return sprintf( + __('Impossibile riservare i %1$s richiesti dall\'aggiornamento: lo spazio disponibile o la quota dell\'account sono esauriti. Libera spazio (i backup automatici in storage/backups sono i primi candidati) e riprova.'), $this->formatBytes($needed) ); } $this->debugLog('INFO', 'Preflight spazio superato', [ 'needed_bytes' => $needed, - 'probe_bytes' => $probeBytes, + 'extra_bytes' => $extraBytes, 'disk_free_bytes' => is_float($free) ? (int) $free : null, ]); return null; } /** - * Write and remove a real file of the given size under storage/tmp. - * Sparse allocation would not do: a hole consumes no quota, and the quota - * is precisely what this is testing. + * Can this account still write $bytes under storage/tmp, and if not, why. + * + * Returns '' when it can, 'nospace' when the write itself ran out of room + * (a full filesystem or an exhausted account quota), and 'unavailable' when + * the probe could never be created at all (storage/tmp missing or not + * writable). Those are three different failures and must not share a + * message — collapsing them into a bool is what let a permission problem be + * reported as an exhausted quota. + * + * Bounded BY CONSTRUCTION, not by the discipline of its callers: + * - a request the filesystem provably cannot hold is refused from + * disk_free_space() without writing a byte, so asking for more than the + * volume holds answers in microseconds instead of filling it; + * - above SPACE_PROBE_BYTES the range is allocated by touching one byte per + * block instead of streaming every byte — the same blocks are consumed + * (a hole consumes no quota, a touched block does) at a fraction of the + * I/O, so the full requirement can be proven without risking a timeout; + * - when free space cannot be read at all, the write falls back to the + * dense bounded form: an unreadable disk_free_space() must never turn + * into a refusal, it is common on locked-down shared hosting. + * + * @param bool $fresh Re-measure instead of answering from the memo. + * @return string '' | 'nospace' | 'unavailable' */ - private function canWriteBytes(int $bytes): bool + private function probeWrite(int $bytes, bool $fresh = false): string + { + if ($bytes <= 0) { + return ''; + } + if ($fresh) { + $this->probeVerdicts = []; + } elseif (isset($this->probeVerdicts[$bytes])) { + return $this->probeVerdicts[$bytes]; + } + + $verdict = $this->runWriteProbe($bytes); + $this->probeVerdicts[$bytes] = $verdict; + return $verdict; + } + + /** + * The probe itself. Sparse allocation would not do: a hole consumes no + * quota, and the quota is precisely what this is testing. + * + * @return string '' | 'nospace' | 'unavailable' + */ + private function runWriteProbe(int $bytes): string { $dir = $this->rootPath . '/storage/tmp'; if (!is_dir($dir) && !@mkdir($dir, 0775, true) && !is_dir($dir)) { - return false; + return 'unavailable'; + } + + // Self-bounding: when the filesystem itself cannot hold the request + // there is nothing to prove by writing, and trying would fill the + // volume before it could answer. + $free = @disk_free_space($dir); + if (is_float($free) && $free > 0 && $free < $bytes) { + return 'nospace'; + } + // Free space unreadable (open_basedir, disabled function): fall back to + // the bounded dense write rather than refusing a legitimate update. + $strided = $bytes > self::SPACE_PROBE_BYTES; + if ($strided && !is_float($free)) { + $bytes = self::SPACE_PROBE_BYTES; + $strided = false; } + $probe = $dir . '/.space_probe_' . bin2hex(random_bytes(4)); $handle = @fopen($probe, 'wb'); if ($handle === false) { - return false; + return 'unavailable'; } $ok = true; try { - $chunk = str_repeat('0', 1024 * 1024); - $written = 0; - while ($written < $bytes) { - $slice = $bytes - $written >= strlen($chunk) ? $chunk : substr($chunk, 0, $bytes - $written); - $result = @fwrite($handle, $slice); - if ($result === false || $result === 0) { - $ok = false; - break; + if ($strided) { + // One byte per block: every block in [0, $bytes) is allocated, + // so the account is charged the whole amount. + for ($offset = 0; $offset < $bytes; $offset += self::SPACE_PROBE_STRIDE) { + if (@fseek($handle, $offset, SEEK_SET) !== 0) { + $ok = false; + break; + } + $result = @fwrite($handle, '0'); + if ($result === false || $result === 0) { + $ok = false; + break; + } + } + } else { + $chunk = str_repeat('0', 1024 * 1024); + $written = 0; + while ($written < $bytes) { + $slice = $bytes - $written >= strlen($chunk) ? $chunk : substr($chunk, 0, $bytes - $written); + $result = @fwrite($handle, $slice); + if ($result === false || $result === 0) { + $ok = false; + break; + } + $written += $result; } - $written += $result; } if ($ok && !@fflush($handle)) { $ok = false; @@ -2568,20 +2740,23 @@ private function canWriteBytes(int $bytes): bool // nosemgrep: php.lang.security.unlink-use.unlink-use -- own probe file under storage/tmp, name generated here @unlink($probe); } - return $ok; + return $ok ? '' : 'nospace'; } /** * Why a write to this path failed, in words an operator can act on. - * Checked in order of likelihood, ending with whatever PHP reported. + * Cheapest and most specific first: a broad, side-effecting probe placed + * first becomes the default answer for everything below it, and on a nearly + * full volume it would report "no space" for a missing directory. */ private function describeWriteFailure(string $targetPath): string { + // error_get_last() is process-global: capture it BEFORE this function + // performs any I/O of its own, or the diagnosis overwrites the very + // error it was called to explain. + $last = error_get_last(); $dir = dirname($targetPath); - if (!$this->canWriteBytes(1024 * 1024)) { - return __('spazio su disco o quota dell\'account esauriti'); - } if (is_file($targetPath) && !is_writable($targetPath)) { return __('il file di destinazione esiste e non è scrivibile'); } @@ -2591,8 +2766,13 @@ private function describeWriteFailure(string $targetPath): string if (!is_writable($dir)) { return __('la directory di destinazione non è scrivibile'); } + // Only a probe that actually ran out of room may claim a space problem; + // 'unavailable' means the probe could not be created, which says + // nothing about the failure being described. + if ($this->probeWrite(1024 * 1024) === 'nospace') { + return __('spazio su disco o quota dell\'account esauriti'); + } - $last = error_get_last(); $message = $last === null ? '' : trim($last['message']); return $message !== '' ? $message : __('causa sconosciuta'); } @@ -2611,12 +2791,15 @@ private function backupAppFiles(): string $this->debugLog('DEBUG', 'Creazione backup app files', ['path' => $backupPath]); - if (!mkdir($backupPath, 0755, true) && !is_dir($backupPath)) { + if (!@mkdir($backupPath, 0755, true) && !is_dir($backupPath)) { + // Describe first: debugLog() writes to disk and would replace the + // mkdir error that error_get_last() has to report. + $cause = $this->describeWriteFailure($backupPath); $this->debugLog('ERROR', 'Impossibile creare directory backup app', [ 'path' => $backupPath, - 'error' => error_get_last() + 'error' => $cause ]); - throw new Exception(__('Impossibile creare directory di backup applicazione')); + throw new Exception(__('Impossibile creare directory di backup applicazione') . ' — ' . $cause); } foreach (self::APP_BACKUP_DIRS as $dir) { @@ -2688,7 +2871,11 @@ private function copyDirectoryRecursive(string $source, string $dest): void if (!is_dir($dest)) { if (!@mkdir($dest, 0755, true) && !is_dir($dest)) { - throw new Exception(sprintf(__('Impossibile creare directory: %s'), $dest)); + throw new Exception(sprintf( + __('Impossibile creare directory: %s — %s'), + $dest, + $this->describeWriteFailure($dest) + )); } } @@ -2732,14 +2919,24 @@ private function copyDirectoryRecursive(string $source, string $dest): void if ($item->isDir()) { if (!is_dir($targetPath)) { if (!@mkdir($targetPath, 0755, true) && !is_dir($targetPath)) { - throw new Exception(sprintf(__('Impossibile creare directory: %s'), $relativePath)); + // A directory is the FIRST write of any new subtree: on an + // exhausted quota it fails before any copy() is reached. + throw new Exception(sprintf( + __('Impossibile creare directory: %s — %s'), + $relativePath, + $this->describeWriteFailure($targetPath) + )); } } } else { $parentDir = dirname($targetPath); if (!is_dir($parentDir)) { if (!@mkdir($parentDir, 0755, true) && !is_dir($parentDir)) { - throw new Exception(sprintf(__('Impossibile creare directory: %s'), dirname($relativePath))); + throw new Exception(sprintf( + __('Impossibile creare directory: %s — %s'), + dirname($relativePath), + $this->describeWriteFailure($parentDir) + )); } } if (!@copy(str_replace('\\', '/', $item->getPathname()), $targetPath)) { @@ -3302,15 +3499,26 @@ private function copyDirectory(string $source, string $dest): void if ($item->isDir()) { if (!is_dir($targetPath)) { - if (!mkdir($targetPath, 0755, true) && !is_dir($targetPath)) { - throw new Exception(sprintf(__('Impossibile creare directory: %s'), $relativePath)); + // Suppressed: an unsuppressed warning prints absolute server + // paths into the update response; the throw below is what + // stops the update, and it now says why. + if (!@mkdir($targetPath, 0755, true) && !is_dir($targetPath)) { + throw new Exception(sprintf( + __('Impossibile creare directory: %s — %s'), + $relativePath, + $this->describeWriteFailure($targetPath) + )); } } } else { $parentDir = dirname($targetPath); if (!is_dir($parentDir)) { - if (!mkdir($parentDir, 0755, true) && !is_dir($parentDir)) { - throw new Exception(sprintf(__('Impossibile creare directory: %s'), dirname($relativePath))); + if (!@mkdir($parentDir, 0755, true) && !is_dir($parentDir)) { + throw new Exception(sprintf( + __('Impossibile creare directory: %s — %s'), + dirname($relativePath), + $this->describeWriteFailure($parentDir) + )); } } // Un file di destinazione che è un SYMLINK va sostituito, mai @@ -3322,11 +3530,26 @@ private function copyDirectory(string $source, string $dest): void // altrove sono setup legittimi.) if (is_link($targetPath)) { if (!@unlink($targetPath)) { - throw new Exception(sprintf(__('Errore nella copia del file: %s'), $relativePath)); + // No copy was attempted here: reusing the copy-failure + // message misattributed the failure entirely. + throw new Exception(sprintf( + __('Impossibile sostituire il collegamento simbolico nella destinazione: %s — %s'), + $relativePath, + $this->describeWriteFailure($targetPath) + )); } } - if (!copy(str_replace('\\', '/', $item->getPathname()), $targetPath)) { - throw new Exception(sprintf(__('Errore nella copia del file: %s'), $relativePath)); + // This copy installs the new release, immediately after + // backupAppFiles() duplicated the whole tree — the fullest the + // account gets during an update, and the likeliest moment for a + // quota to run out. The file name alone is misleading: the loop + // stops at whatever entry it had reached. Say why. + if (!@copy(str_replace('\\', '/', $item->getPathname()), $targetPath)) { + throw new Exception(sprintf( + __('Errore nella copia del file: %s — %s'), + $relativePath, + $this->describeWriteFailure($targetPath) + )); } } } @@ -4320,6 +4543,21 @@ public function performUpdate(string $targetVersion): array ]); } + // PRE-FLIGHT (issue #422): refuse BEFORE the first byte is written. + // Steps 1 and 2 write the backup archive, the release ZIP and its + // extracted tree — together as much as the rollback copy the gate in + // installUpdate() protects. On an account whose quota is exhausted + // the update used to die inside one of them with "Backup fallito" or + // "Download fallito" and no mention of space at all. The gate in + // installUpdate() stays: it re-measures once these steps have + // consumed their share, which is the only measurement valid there. + $packageBytes = $this->releaseAssetBytes($targetVersion); + $spaceError = $this->checkFreeSpaceForUpdate($packageBytes > 0 ? $packageBytes * 4 : 0); + if ($spaceError !== null) { + $this->debugLog('ERROR', 'Preflight iniziale: spazio insufficiente', ['detail' => $spaceError]); + throw new Exception($spaceError); + } + // Step 1: Backup $this->debugLog('INFO', '>>> STEP 1: Creazione backup <<<'); $backupResult = $this->createBackup(); diff --git a/locale/da_DK.json b/locale/da_DK.json index e48f1b287..8f6b3631d 100644 --- a/locale/da_DK.json +++ b/locale/da_DK.json @@ -7513,5 +7513,9 @@ "la directory di destinazione non esiste": "destinationsmappen findes ikke", "la directory di destinazione non è scrivibile": "destinationsmappen kan ikke skrives", "causa sconosciuta": "ukendt årsag", - "Errore nella copia del file: %s — %s": "Fejl ved kopiering af filen: %s — %s" + "Errore nella copia del file: %s — %s": "Fejl ved kopiering af filen: %s — %s", + "Impossibile riservare i %1$s richiesti dall'aggiornamento: lo spazio disponibile o la quota dell'account sono esauriti. Libera spazio (i backup automatici in storage/backups sono i primi candidati) e riprova.": "Kunne ikke reservere de %1$s, som opdateringen kræver: den tilgængelige plads eller kontokvoten er opbrugt. Frigør plads (de automatiske sikkerhedskopier i storage/backups er de første kandidater), og prøv igen.", + "Impossibile usare la directory di lavoro dell'aggiornamento (%s): non esiste o non è scrivibile dall'utente del web server. Correggi i permessi e riprova.": "Opdateringens arbejdsmappe (%s) kan ikke bruges: den findes ikke eller kan ikke skrives af webserverens bruger. Ret rettighederne, og prøv igen.", + "Impossibile creare directory: %s — %s": "Kunne ikke oprette mappe: %s — %s", + "Impossibile sostituire il collegamento simbolico nella destinazione: %s — %s": "Kunne ikke erstatte det symbolske link i destinationen: %s — %s" } diff --git a/locale/de_DE.json b/locale/de_DE.json index 190ad1715..2caee86bd 100644 --- a/locale/de_DE.json +++ b/locale/de_DE.json @@ -7513,5 +7513,9 @@ "la directory di destinazione non esiste": "das Zielverzeichnis existiert nicht", "la directory di destinazione non è scrivibile": "das Zielverzeichnis ist nicht beschreibbar", "causa sconosciuta": "unbekannte Ursache", - "Errore nella copia del file: %s — %s": "Fehler beim Kopieren der Datei: %s — %s" + "Errore nella copia del file: %s — %s": "Fehler beim Kopieren der Datei: %s — %s", + "Impossibile riservare i %1$s richiesti dall'aggiornamento: lo spazio disponibile o la quota dell'account sono esauriti. Libera spazio (i backup automatici in storage/backups sono i primi candidati) e riprova.": "Die für das Update benötigten %1$s konnten nicht reserviert werden: Der verfügbare Speicherplatz oder das Kontingent des Kontos ist erschöpft. Geben Sie Speicherplatz frei (die automatischen Sicherungen in storage/backups sind die ersten Kandidaten) und versuchen Sie es erneut.", + "Impossibile usare la directory di lavoro dell'aggiornamento (%s): non esiste o non è scrivibile dall'utente del web server. Correggi i permessi e riprova.": "Das Arbeitsverzeichnis des Updates (%s) kann nicht verwendet werden: Es existiert nicht oder ist für den Benutzer des Webservers nicht beschreibbar. Korrigieren Sie die Berechtigungen und versuchen Sie es erneut.", + "Impossibile creare directory: %s — %s": "Verzeichnis konnte nicht erstellt werden: %s — %s", + "Impossibile sostituire il collegamento simbolico nella destinazione: %s — %s": "Der symbolische Link am Ziel konnte nicht ersetzt werden: %s — %s" } diff --git a/locale/en_US.json b/locale/en_US.json index a09bcc147..4064b153f 100644 --- a/locale/en_US.json +++ b/locale/en_US.json @@ -7513,5 +7513,9 @@ "la directory di destinazione non esiste": "the destination directory does not exist", "la directory di destinazione non è scrivibile": "the destination directory is not writable", "causa sconosciuta": "unknown cause", - "Errore nella copia del file: %s — %s": "Error copying file: %s — %s" + "Errore nella copia del file: %s — %s": "Error copying file: %s — %s", + "Impossibile riservare i %1$s richiesti dall'aggiornamento: lo spazio disponibile o la quota dell'account sono esauriti. Libera spazio (i backup automatici in storage/backups sono i primi candidati) e riprova.": "Could not reserve the %1$s the update needs: the available space or the account quota is exhausted. Free some space (the automatic backups in storage/backups are the first candidates) and try again.", + "Impossibile usare la directory di lavoro dell'aggiornamento (%s): non esiste o non è scrivibile dall'utente del web server. Correggi i permessi e riprova.": "Cannot use the update working directory (%s): it does not exist or is not writable by the web server user. Fix the permissions and try again.", + "Impossibile creare directory: %s — %s": "Unable to create directory: %s — %s", + "Impossibile sostituire il collegamento simbolico nella destinazione: %s — %s": "Unable to replace the symbolic link at the destination: %s — %s" } diff --git a/locale/fr_FR.json b/locale/fr_FR.json index 918d3c9b2..c22d4ad60 100644 --- a/locale/fr_FR.json +++ b/locale/fr_FR.json @@ -7513,5 +7513,9 @@ "la directory di destinazione non esiste": "le répertoire de destination n'existe pas", "la directory di destinazione non è scrivibile": "le répertoire de destination n'est pas accessible en écriture", "causa sconosciuta": "cause inconnue", - "Errore nella copia del file: %s — %s": "Erreur lors de la copie du fichier : %s — %s" + "Errore nella copia del file: %s — %s": "Erreur lors de la copie du fichier : %s — %s", + "Impossibile riservare i %1$s richiesti dall'aggiornamento: lo spazio disponibile o la quota dell'account sono esauriti. Libera spazio (i backup automatici in storage/backups sono i primi candidati) e riprova.": "Impossible de réserver les %1$s requis par la mise à jour : l'espace disponible ou le quota du compte est épuisé. Libérez de l'espace (les sauvegardes automatiques dans storage/backups sont les premières candidates) et réessayez.", + "Impossibile usare la directory di lavoro dell'aggiornamento (%s): non esiste o non è scrivibile dall'utente del web server. Correggi i permessi e riprova.": "Impossible d'utiliser le répertoire de travail de la mise à jour (%s) : il n'existe pas ou n'est pas accessible en écriture par l'utilisateur du serveur web. Corrigez les permissions et réessayez.", + "Impossibile creare directory: %s — %s": "Impossible de créer le répertoire : %s — %s", + "Impossibile sostituire il collegamento simbolico nella destinazione: %s — %s": "Impossible de remplacer le lien symbolique de destination : %s — %s" } diff --git a/locale/it_IT.json b/locale/it_IT.json index 9e17714e4..00b25a3b2 100644 --- a/locale/it_IT.json +++ b/locale/it_IT.json @@ -7513,5 +7513,9 @@ "la directory di destinazione non esiste": "la directory di destinazione non esiste", "la directory di destinazione non è scrivibile": "la directory di destinazione non è scrivibile", "causa sconosciuta": "causa sconosciuta", - "Errore nella copia del file: %s — %s": "Errore nella copia del file: %s — %s" + "Errore nella copia del file: %s — %s": "Errore nella copia del file: %s — %s", + "Impossibile riservare i %1$s richiesti dall'aggiornamento: lo spazio disponibile o la quota dell'account sono esauriti. Libera spazio (i backup automatici in storage/backups sono i primi candidati) e riprova.": "Impossibile riservare i %1$s richiesti dall'aggiornamento: lo spazio disponibile o la quota dell'account sono esauriti. Libera spazio (i backup automatici in storage/backups sono i primi candidati) e riprova.", + "Impossibile usare la directory di lavoro dell'aggiornamento (%s): non esiste o non è scrivibile dall'utente del web server. Correggi i permessi e riprova.": "Impossibile usare la directory di lavoro dell'aggiornamento (%s): non esiste o non è scrivibile dall'utente del web server. Correggi i permessi e riprova.", + "Impossibile creare directory: %s — %s": "Impossibile creare directory: %s — %s", + "Impossibile sostituire il collegamento simbolico nella destinazione: %s — %s": "Impossibile sostituire il collegamento simbolico nella destinazione: %s — %s" } diff --git a/scripts/manual-upgrade.php b/scripts/manual-upgrade.php index 35fd89653..d534f82e2 100644 --- a/scripts/manual-upgrade.php +++ b/scripts/manual-upgrade.php @@ -179,6 +179,126 @@ function formatBytes(float $bytes): string return round($bytes, 2) . ' ' . $units[$i]; } +/** + * Prove that $bytes can really be written by THIS account under storage/tmp. + * + * disk_free_space() sees the FILESYSTEM: on a cPanel account it happily reports + * tens of gigabytes free while the account's own quota is exhausted, which is + * how a healthy release came to look corrupt (issue #422). Only a real + * allocation answers the question, so this writes real bytes — a sparse hole + * consumes no quota, and the quota is precisely what is being measured. + * + * Bounded by construction: a request the filesystem provably cannot hold is + * refused without writing anything, and above 16 MB the range is allocated by + * touching one byte per 1 KiB block instead of streaming every byte (the same + * blocks are charged, at a thousandth of the I/O). When free space cannot be + * read at all the write falls back to a bounded 16 MB — an unreadable + * disk_free_space() must never become a refusal. + * + * Deliberately duplicated from app/Support/Updater.php: this script must keep + * working with no autoloader and no application classes, because it is the tool + * used when the application itself will not boot. + * + * @return string '' when the space is there, 'nospace' when the write ran out + * of room, 'unavailable' when the probe could not be created + */ +function probeWriteBytes(string $rootPath, int $bytes): string +{ + if ($bytes <= 0) { + return ''; + } + $denseLimit = 16 * 1024 * 1024; + $stride = 1024; + + $dir = rtrim(str_replace('\\', '/', $rootPath), '/') . '/storage/tmp'; + if (!is_dir($dir) && !@mkdir($dir, 0775, true) && !is_dir($dir)) { + return 'unavailable'; + } + + $free = @disk_free_space($dir); + if (is_float($free) && $free > 0 && $free < $bytes) { + return 'nospace'; + } + $strided = $bytes > $denseLimit; + if ($strided && !is_float($free)) { + $bytes = $denseLimit; + $strided = false; + } + + $probe = $dir . '/.upgrade_probe_' . bin2hex(random_bytes(4)); + $handle = @fopen($probe, 'wb'); + if ($handle === false) { + return 'unavailable'; + } + $ok = true; + try { + if ($strided) { + for ($offset = 0; $offset < $bytes; $offset += $stride) { + if (@fseek($handle, $offset, SEEK_SET) !== 0) { + $ok = false; + break; + } + $result = @fwrite($handle, '0'); + if ($result === false || $result === 0) { + $ok = false; + break; + } + } + } else { + $chunk = str_repeat('0', 1024 * 1024); + $written = 0; + while ($written < $bytes) { + $slice = $bytes - $written >= strlen($chunk) ? $chunk : substr($chunk, 0, $bytes - $written); + $result = @fwrite($handle, $slice); + if ($result === false || $result === 0) { + $ok = false; + break; + } + $written += $result; + } + } + if ($ok && !@fflush($handle)) { + $ok = false; + } + } catch (Throwable $e) { + $ok = false; + } finally { + @fclose($handle); + @unlink($probe); + } + return $ok ? '' : 'nospace'; +} + +/** + * Why a write to this path failed, in words an operator can act on. + * + * Cheapest and most specific first, with error_get_last() captured before this + * function performs any I/O of its own — it is process-global, so a diagnosis + * that writes first ends up reporting its own error. Same wording as the + * in-app updater, hardcoded in Italian like the rest of this script. + */ +function describeWriteFailure(string $rootPath, string $targetPath): string +{ + $last = error_get_last(); + $dir = dirname($targetPath); + + if (is_file($targetPath) && !is_writable($targetPath)) { + return 'il file di destinazione esiste e non è scrivibile'; + } + if (!is_dir($dir)) { + return 'la directory di destinazione non esiste'; + } + if (!is_writable($dir)) { + return 'la directory di destinazione non è scrivibile'; + } + if (probeWriteBytes($rootPath, 1024 * 1024) === 'nospace') { + return 'spazio su disco o quota dell\'account esauriti'; + } + + $message = $last === null ? '' : trim($last['message']); + return $message !== '' ? $message : 'causa sconosciuta'; +} + function deleteDirectory(string $dir): void { if (!is_dir($dir)) { @@ -356,8 +476,11 @@ function copyTree(string $src, string $dst, string $rootDst, array $skipRelative if (is_link($dstPath) || is_link($dstDir)) { throw new RuntimeException('Percorso destinazione simbolico non consentito: ' . $dstPath); } - if (!is_dir($dstDir) && !mkdir($dstDir, 0755, true) && !is_dir($dstDir)) { - throw new RuntimeException('Impossibile creare directory destinazione: ' . $dstDir); + if (!is_dir($dstDir) && !@mkdir($dstDir, 0755, true) && !is_dir($dstDir)) { + throw new RuntimeException( + 'Impossibile creare directory destinazione: ' . $dstDir + . ' — ' . describeWriteFailure($rootDst, $dstDir) + ); } $rootReal = realpath($rootDst); $dstDirReal = realpath($dstDir); @@ -368,8 +491,16 @@ function copyTree(string $src, string $dst, string $rootDst, array $skipRelative ) { throw new RuntimeException('Percorso destinazione non valido: ' . $dstPath); } - if (!copy($srcPath, $dstPath)) { - throw new RuntimeException('Copia file fallita: ' . $srcPath . ' -> ' . $dstPath); + // The file name alone is misleading: the loop stops at whatever + // entry it had reached when the real problem (an exhausted quota, a + // read-only target) occurred — that is how a healthy release came to + // look corrupt. Say why, and here it matters more than in the app: + // this script overwrites the live tree with no file rollback. + if (!@copy($srcPath, $dstPath)) { + throw new RuntimeException( + 'Copia file fallita: ' . $srcPath . ' -> ' . $dstPath + . ' — ' . describeWriteFailure($rootDst, $dstPath) + ); } $count++; } @@ -470,6 +601,18 @@ function copyTree(string $src, string $dst, string $rootDst, array $skipRelative if ($freeSpace !== false && $freeSpace < 200 * 1024 * 1024) { throw new RuntimeException('Spazio disco insufficiente: ' . formatBytes((int)$freeSpace) . ' disponibili, servono almeno 200 MB.'); } + // disk_free_space() alone lies on a cPanel account whose quota is + // exhausted: it reports the FILESYSTEM. Prove with a real write that + // this account can still allocate, here — the last point at which a + // refusal costs the operator nothing, because after it the DB dump and + // the tree overwrite begin and this script has no file rollback. + $probeVerdict = probeWriteBytes($rootPath, 16 * 1024 * 1024); + if ($probeVerdict === 'unavailable') { + throw new RuntimeException('Impossibile usare storage/tmp: non esiste o non è scrivibile. Correggi i permessi prima di continuare.'); + } + if ($probeVerdict === 'nospace') { + throw new RuntimeException('Impossibile scrivere in storage/tmp: lo spazio disponibile o la quota dell\'account sono esauriti. Libera spazio (i backup automatici in storage/backups sono i primi candidati) e riprova.'); + } $writableDirs = ['storage', 'storage/tmp', 'storage/backups', 'app', 'public', 'installer']; foreach ($writableDirs as $dir) { @@ -601,6 +744,20 @@ function copyTree(string $src, string $dst, string $rootDst, array $skipRelative . formatBytes((float) $requiredBytes) ); } + // Quota probe for what the extraction actually lands (the uncompressed + // payload; the 100 MB above is margin, not something that gets written). + // The mysqldump ran between the two checks and may itself be what + // exhausted the account, so this cannot be inferred from pre-flight 1b. + $extractVerdict = probeWriteBytes($rootPath, $uncompressedBytes); + if ($extractVerdict !== '') { + $zip->close(); + throw new RuntimeException( + 'Impossibile riservare i ' . formatBytes((float) $uncompressedBytes) + . ' richiesti dall\'estrazione: ' . ($extractVerdict === 'unavailable' + ? 'storage/tmp non esiste o non è scrivibile.' + : 'lo spazio disponibile o la quota dell\'account sono esauriti.') + ); + } if (!$zip->extractTo($tempDir)) { $zip->close(); @@ -678,8 +835,11 @@ function copyTree(string $src, string $dst, string $rootDst, array $skipRelative $criticalFiles = ['.env', 'config.local.php', 'version.json']; foreach ($criticalFiles as $cf) { if (is_file($rootPath . '/' . $cf)) { - if (!copy($rootPath . '/' . $cf, $fileBackupDir . '/' . $cf)) { - throw new RuntimeException('Backup file critico fallito: ' . $cf); + if (!@copy($rootPath . '/' . $cf, $fileBackupDir . '/' . $cf)) { + throw new RuntimeException( + 'Backup file critico fallito: ' . $cf + . ' — ' . describeWriteFailure($rootPath, $fileBackupDir . '/' . $cf) + ); } } } diff --git a/tests/update-space-preflight.unit.php b/tests/update-space-preflight.unit.php index f86349576..555fd5c5b 100644 --- a/tests/update-space-preflight.unit.php +++ b/tests/update-space-preflight.unit.php @@ -8,14 +8,27 @@ * whichever file the loop had reached — a path inside vendor/ that says nothing * about the cause and made a healthy release look corrupt. Two changes fix * that: refuse before touching anything when the space is not there, and, when - * a copy does fail, say WHY. + * a write does fail, say WHY. * * Covered against the REAL class: * - the estimate is driven by the same directory list the rollback backup * copies, so the two can never disagree; - * - the write probe answers false when the requested size cannot be written; - * - a copy failure is described by cause (unwritable target, missing - * directory, unwritable directory) rather than by file name alone; + * - the write probe is BOUNDED BY CONSTRUCTION: an impossible request is + * refused from disk_free_space() in microseconds instead of being disproven + * by filling the volume, and it leaves no probe file behind; + * - the probe proves the WHOLE requirement, not a 16 MB constant — an account + * with room for a small file but not for the update must be refused; + * - the probe is TRI-STATE: "the probe could not be created" is not reported + * as "the disk is full"; + * - a write failure is described by cause (unwritable target, missing + * directory, exhausted space) with the cheap, specific checks running first, + * and the PHP error it falls back to is the caller's, not one the diagnosis + * produced itself; + * - the gate runs in both update entry points BEFORE the first byte is + * written, and is kept in installUpdate() where it re-measures; + * - copyDirectory() — not just the helper in isolation — reports a cause; + * - scripts/manual-upgrade.php, the fallback the updater's own message + * recommends, has the same probe and the same cause-naming; * - the preflight passes on a healthy installation, so it cannot become a * gate that blocks legitimate updates. * @@ -60,6 +73,7 @@ } $updater = new Updater($db, $root); +$src = (string) file_get_contents($root . '/app/Support/Updater.php'); echo "A. the estimate follows the directories the rollback backup copies\n"; $dirsRef = new ReflectionClassConstant(Updater::class, 'APP_BACKUP_DIRS'); @@ -77,23 +91,74 @@ // The guarantee that matters: a directory added to the backup must show up in // the estimate by construction. Reading the same constant in both places is // what provides it, so assert the backup really uses it. -$src = (string) file_get_contents($root . '/app/Support/Updater.php'); $check(substr_count($src, 'self::APP_BACKUP_DIRS') >= 3, 'backup, restore and estimate all read the same constant'); -echo "B. the write probe reflects what can actually be written\n"; -$check($call($updater, 'canWriteBytes', [1024]) === true, 'a small write succeeds on a healthy install'); -// An absurd request must be refused rather than filling the disk trying. -$huge = PHP_INT_MAX; +echo "B. the write probe is bounded by construction and proves the real requirement\n"; +$check($call($updater, 'probeWrite', [1024]) === '', 'a small write succeeds on a healthy install'); +// One primitive, three answers. The bool it replaced could not tell "the probe +// could not be created" from "the write ran out of room", and every downstream +// misreport traced back to that collapse. +$check(!str_contains($src, 'function canWriteBytes'), + 'the lossy bool probe is gone; probeWrite() is the single primitive'); + +// An impossible request must be REFUSED, not disproven by filling the volume. +// Before this was bounded, the probe wrote 1 MB chunks until ENOSPC — on a dev +// box that is the documented trigger for a MySQL abort, and in CI it starves +// the database service the next gate depends on. $freeBytes = @disk_free_space($root); -if (is_float($freeBytes) && $freeBytes > 0) { - $check($call($updater, 'canWriteBytes', [(int) $freeBytes + (1024 * 1024 * 1024)]) === false, - 'a request larger than the free space is refused'); +$freeBefore = is_float($freeBytes) && $freeBytes > 0 ? $freeBytes : null; +$impossible = $freeBefore !== null + ? (int) $freeBefore + (1024 * 1024 * 1024) + : 1024 * 1024 * 1024 * 1024; // 1 TiB: larger than any volume this runs on +$t0 = microtime(true); +$verdict = $call($updater, 'probeWrite', [$impossible]); +$elapsed = microtime(true) - $t0; +$check($elapsed < 5.0, sprintf('an impossible request is answered in %.3fs, without filling the disk', $elapsed)); +$check(count(glob($root . '/storage/tmp/.space_probe_*') ?: []) === 0, 'no probe file survives a refused probe'); +if ($freeBefore !== null) { + $check($verdict === 'nospace', 'a request larger than the free space is refused'); + $freeAfter = @disk_free_space($root); + $delta = is_float($freeAfter) ? $freeBefore - $freeAfter : 0.0; + $check($delta < 64 * 1024 * 1024, + sprintf('free space is essentially unchanged by the refusal (%s consumed)', number_format($delta))); } else { - echo " SKIP disk_free_space() unavailable on this filesystem\n"; + // disk_free_space() unreadable: the probe must degrade to a bounded write + // rather than refusing a legitimate update, so only the bound is assertable. + $check(in_array($verdict, ['', 'nospace', 'unavailable'], true), + 'with free space unreadable the probe still answers a bounded verdict'); } -echo "C. a copy failure is described by cause, not just by file name\n"; +// The cap used to live in the CALLER: the probe answered "can a small file be +// written", never "can the update fit". An account with 50 MB of headroom +// facing a 120 MB update passed both checks and died mid-copy anyway. +$check(!str_contains($src, 'min($needed'), 'the gate no longer caps the probe below the requirement'); +$check($call($updater, 'probeWrite', [$estimate]) === '', + 'the full estimate can be proven on this checkout (' . number_format($estimate) . ' bytes)'); +$check(count(glob($root . '/storage/tmp/.space_probe_*') ?: []) === 0, 'no probe file survives a full-size probe'); + +// Tri-state: "the probe could not be created" must not be reported as "no +// space". Built with a FILE where storage/tmp belongs, so mkdir cannot succeed +// even for uid 0 — a chmod-based precondition inverts under root. +$bareRef = new ReflectionClass(Updater::class); +$bare = $bareRef->newInstanceWithoutConstructor(); +$rootProp = $bareRef->getProperty('rootPath'); +$rootProp->setAccessible(true); +$blockedRoot = sys_get_temp_dir() . '/zz_preflight_blocked_' . bin2hex(random_bytes(4)); +mkdir($blockedRoot . '/storage', 0777, true); +file_put_contents($blockedRoot . '/storage/tmp', 'not a directory'); +$rootProp->setValue($bare, $blockedRoot); +$check($call($bare, 'probeWrite', [1024]) === 'unavailable', + 'an uncreatable working directory answers "unavailable", not "nospace"'); +$blockedTarget = $blockedRoot . '/target.txt'; +$reason = (string) $call($bare, 'describeWriteFailure', [$blockedTarget]); +$check(!str_contains($reason, 'spazio') && !str_contains($reason, 'quota'), + 'an unusable probe is never reported as exhausted space (' . $reason . ')'); +@unlink($blockedRoot . '/storage/tmp'); +@rmdir($blockedRoot . '/storage'); +@rmdir($blockedRoot); + +echo "C. a write failure is described by cause, not just by file name\n"; $tmp = sys_get_temp_dir() . '/zz_preflight_' . bin2hex(random_bytes(4)); mkdir($tmp, 0777, true); @@ -102,32 +167,181 @@ $check(str_contains($reason, 'directory') && str_contains($reason, 'esiste'), 'a missing destination directory is named as such (' . $reason . ')'); +// chmod() manufactures the precondition only where the OS enforces mode bits +// against the running user — is_writable() returns true for uid 0 on both a +// 0444 file and a 0555 directory. Assert what the function SHOULD say given the +// state that actually obtains, so a root run neither fails falsely nor passes +// while asserting nothing. $roFile = $tmp . '/readonly.txt'; file_put_contents($roFile, 'x'); chmod($roFile, 0444); +$enforced = !is_writable($roFile); $reason = (string) $call($updater, 'describeWriteFailure', [$roFile]); -$check(str_contains($reason, 'non è scrivibile'), 'an unwritable target file is named as such (' . $reason . ')'); +if ($enforced) { + $check(str_contains($reason, 'non è scrivibile'), 'an unwritable target file is named as such (' . $reason . ')'); +} else { + $check(trim($reason) !== '', + 'mode bits not enforced for this user (root?): a cause is still reported (' . $reason . ')'); +} chmod($roFile, 0644); $roDir = $tmp . '/rodir'; mkdir($roDir, 0555, true); +$enforced = !is_writable($roDir); $reason = (string) $call($updater, 'describeWriteFailure', [$roDir . '/new.txt']); -$check(str_contains($reason, 'directory') && str_contains($reason, 'scrivibile'), - 'an unwritable destination directory is named as such (' . $reason . ')'); +if ($enforced) { + $check(str_contains($reason, 'directory') && str_contains($reason, 'scrivibile'), + 'an unwritable destination directory is named as such (' . $reason . ')'); +} else { + $check(trim($reason) !== '', + 'mode bits not enforced for this user (root?): a cause is still reported (' . $reason . ')'); +} chmod($roDir, 0755); +// The fallback must report the CALLER's error. error_get_last() is +// process-global: a diagnosis that probes before reading it ends up reporting +// whatever its own I/O last produced. +@copy($tmp . '/there-is-no-such-source', $tmp . '/dest.txt'); +$reason = (string) $call($updater, 'describeWriteFailure', [$tmp . '/dest.txt']); +$check(str_contains($reason, 'copy('), + 'the fallback reports the failing copy, not an error the diagnosis produced (' . $reason . ')'); + // The message must never be empty: an empty reason is the original defect. $reason = (string) $call($updater, 'describeWriteFailure', [$tmp . '/plain.txt']); $check(trim($reason) !== '', 'a cause is always reported, even when nothing obvious is wrong'); +echo "D. copyDirectory itself reports the cause, not only the helper\n"; +$copySrc = $tmp . '/pkg'; +$copyDst = $tmp . '/live'; +mkdir($copySrc, 0777, true); +mkdir($copyDst, 0777, true); +file_put_contents($copySrc . '/nuovo.txt', 'contenuto'); +file_put_contents($copyDst . '/nuovo.txt', 'vecchio'); +chmod($copyDst . '/nuovo.txt', 0444); +$enforced = !is_writable($copyDst . '/nuovo.txt'); +$copyMessage = null; +try { + $call($updater, 'copyDirectory', [$copySrc, $copyDst]); +} catch (Throwable $e) { + $copyMessage = $e->getMessage(); +} +if ($enforced) { + $check($copyMessage !== null && str_contains($copyMessage, '—'), + 'a failed copy carries a cause segment (' . (string) $copyMessage . ')'); + $check($copyMessage !== null && str_contains($copyMessage, 'scrivibile'), + 'the cause names the unwritable target, not just the file name'); +} else { + $check($copyMessage === null, 'mode bits not enforced for this user (root?): the copy simply succeeds'); +} +chmod($copyDst . '/nuovo.txt', 0644); + +// Neither copy engine may keep the bare, causeless message: installUpdate() +// runs one straight after the other and the operator cannot tell them apart. +$check(!str_contains($src, "__('Errore nella copia del file: %s')"), + 'the bare copy-failure message is gone from Updater.php'); +$check(preg_match('/if \(!copy\(|if \(!mkdir\(/', $src) === 0, + 'no unsuppressed copy()/mkdir() is left to leak a warning into the response'); +$check(substr_count($src, 'describeWriteFailure') >= 6, + 'every write-failure site consults the diagnosis (' . substr_count($src, 'describeWriteFailure') . ' references)'); + foreach (glob($tmp . '/*') ?: [] as $f) { - is_dir($f) ? @rmdir($f) : @unlink($f); + if (is_dir($f)) { + foreach (glob($f . '/*') ?: [] as $inner) { + is_dir($inner) ? @rmdir($inner) : @unlink($inner); + } + @rmdir($f); + } else { + @unlink($f); + } } @rmdir($tmp); -echo "D. the preflight does not block a healthy installation\n"; +echo "E. the gate runs before the first byte is written, in BOTH entry points\n"; +$bodyOf = static function (string $source, string $signature): string { + $start = strpos($source, $signature); + if ($start === false) { + return ''; + } + $rest = substr($source, $start + strlen($signature)); + $end = preg_match('/\n (?:public|protected|private) function /', $rest, $m, PREG_OFFSET_CAPTURE) + ? (int) $m[0][1] + : strlen($rest); + return substr($rest, 0, $end); +}; +$check(substr_count($src, 'checkFreeSpaceForUpdate(') >= 4, + 'the gate has a declaration and at least three call sites (' . substr_count($src, 'checkFreeSpaceForUpdate(') . ')'); +$check(str_contains($src, 'private function checkFreeSpaceForUpdate(int $extraBytes = 0)'), + 'the gate can be asked for more than the rollback copy'); +foreach ([ + 'public function performUpdate(string $targetVersion): array', + 'public function performUpdateFromFile(string $uploadTempPath): array', +] as $signature) { + $body = $bodyOf($src, $signature); + $gate = strpos($body, 'checkFreeSpaceForUpdate('); + $backup = strpos($body, '$this->createBackup()'); + $label = trim(explode('(', $signature)[0]); + $check($body !== '' && $gate !== false && $backup !== false && $gate < $backup, + $label . ': the space gate precedes createBackup()'); +} +// The later gate must NOT be moved: it is the only measurement taken after the +// backup and the download have consumed their share. +$installBody = $bodyOf($src, 'public function installUpdate(string $sourcePath, string $targetVersion): array'); +$check(str_contains($installBody, 'checkFreeSpaceForUpdate()'), + 'installUpdate() keeps its own re-measuring gate'); + +echo "F. the CLI fallback got the same guarantees\n"; +$cli = (string) file_get_contents($root . '/scripts/manual-upgrade.php'); +$check(str_contains($cli, 'function probeWriteBytes('), 'the standalone script defines its own write probe'); +$check(str_contains($cli, 'function describeWriteFailure('), 'the standalone script defines its own diagnosis'); +// It must keep working on an installation whose application will not boot: +// the helpers are plain functions and no message goes through translation. +$check(!str_contains($cli, '__('), 'the script stays free of the translation function'); +// The script does load the autoloader late, for the post-migration backfills. +// The helpers and every check that uses them must sit BEFORE that point: they +// run on installations where the application itself will not boot. +$autoloadAt = strpos($cli, 'vendor/autoload.php'); +$check($autoloadAt !== false + && (int) strpos($cli, 'function probeWriteBytes(') < $autoloadAt + && (int) strpos($cli, 'function describeWriteFailure(') < $autoloadAt + && (int) strrpos($cli, 'probeWriteBytes($rootPath') < $autoloadAt, + 'probe and diagnosis are defined and used before any autoloader is required'); +$cliLines = preg_split('/\r?\n/', $cli) ?: []; +foreach (['Copia file fallita', 'Backup file critico fallito', 'Impossibile creare directory destinazione'] as $message) { + $window = ''; + foreach ($cliLines as $i => $text) { + if (str_contains($text, "'" . $message)) { + $window = implode("\n", array_slice($cliLines, $i, 5)); + break; + } + } + $check($window !== '' && str_contains($window, 'describeWriteFailure('), + '"' . $message . '" carries a cause'); +} +$freeChecks = 0; +$probedChecks = 0; +foreach ($cliLines as $i => $text) { + if (!str_contains($text, '@disk_free_space($rootPath)')) { + continue; + } + $freeChecks++; + if (str_contains(implode("\n", array_slice($cliLines, $i, 20)), 'probeWriteBytes(')) { + $probedChecks++; + } +} +$check($freeChecks > 0 && $freeChecks === $probedChecks, + "every disk_free_space() gate is backed by a real write probe ({$probedChecks}/{$freeChecks})"); + +echo "G. the preflight does not block a healthy installation\n"; $verdict = $call($updater, 'checkFreeSpaceForUpdate'); $check($verdict === null, 'this checkout passes the preflight (' . var_export($verdict, true) . ')'); +if ($freeBefore !== null) { + $refusal = $call($updater, 'checkFreeSpaceForUpdate', [(int) $freeBefore + (1024 * 1024 * 1024)]); + $check(is_string($refusal) && $refusal !== '', + 'an update that cannot fit is refused with a message'); +} else { + $check($call($updater, 'checkFreeSpaceForUpdate', [0]) === null, + 'with free space unreadable the gate still lets a healthy install through'); +} $check(count(glob($root . '/storage/tmp/.space_probe_*') ?: []) === 0, 'the probe file is removed after the check'); echo PHP_EOL . "Passed: {$passed} Failed: {$failed}" . PHP_EOL; From d362ef03bf16fcc183aea5ab034f1464e0a32352 Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Thu, 10 Sep 2026 11:34:06 +0200 Subject: [PATCH 03/10] fix(updater): describe a write failure before anything else can overwrite it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit describeWriteFailure() falls back to error_get_last(), which is process-global. Anything that performs I/O between the failure and the diagnosis replaces the error being reported: debugLog() ends in a file write to storage/logs/app.log, and ZipArchive::close() flushes the archive and can raise an error of its own. The previous commit got this right in two branches and wrong in two others, with the correct ones even carrying a comment explaining why the order matters. downloadUpdate()'s save failure logged error_get_last() first and only then asked for the cause; extractPackage()'s failure branch called $zip->close() and debugLog() ahead of it. On a read-only or full storage/logs — the case where the log write itself errors and the probe does not already answer "no space" — the operator was told why the log failed, not why the update did. Both now compute the cause as the first statement of their branch and log that local instead of a second error_get_last(). extractPackage() also reads $zip->status into a local before closing, which was already the safer order. The rule is now enforced rather than remembered: a source assertion walks every describeWriteFailure() call site in Updater.php back to its enclosing branch and fails if any logging or teardown precedes it. Comment lines are excluded from that scan on purpose — the branches that get this right document why they log second, and a raw text match would let the explanation trip the rule it documents. That assertion immediately earned itself. It found a fifth site neither the fix nor its review had noticed: performUpdateFromFile()'s own extraction branch, the sibling of the one in extractPackage(), closing the archive before naming the cause. Fixed here too. Verified: unit suite 43 passed / 0 failed, PHPStan level 5 clean over 540 files, locale keys and placeholders aligned. The real admin-UI upgrade was re-run end to end after these edits, since they sit on the extraction path: upload plus "Avvia" completes, and both space gates log a pass — 230,103,416 bytes required at the early gate (including the uploaded package) and 97,912,356 at the one inside installUpdate(), which is the split the earlier commit introduced. --- app/Support/Updater.php | 42 +++++++++++++++++---------- tests/update-space-preflight.unit.php | 42 +++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 16 deletions(-) diff --git a/app/Support/Updater.php b/app/Support/Updater.php index fac85244d..22f328e18 100644 --- a/app/Support/Updater.php +++ b/app/Support/Updater.php @@ -1445,16 +1445,18 @@ public function downloadUpdate(string $version): array $bytesWritten = file_put_contents($zipPath, $fileContent); if ($bytesWritten === false) { + // Describe first: debugLog() writes to disk and would replace + // the file_put_contents() error that error_get_last() has to + // report. Name the cause because on a full account this is the + // FIRST write to fail, and "Download fallito" alone sends the + // operator looking for a corrupt release instead of free space. + $cause = $this->describeWriteFailure($zipPath); $this->debugLog('ERROR', 'Impossibile salvare file', [ 'path' => $zipPath, - 'error' => error_get_last() + 'error' => $cause ]); - // Name the cause: on a full account this is the FIRST write to - // fail, and "Download fallito" alone sends the operator looking - // for a corrupt release instead of for free space. throw new Exception( - __('Impossibile salvare il file di aggiornamento') - . ' — ' . $this->describeWriteFailure($zipPath) + __('Impossibile salvare il file di aggiornamento') . ' — ' . $cause ); } @@ -1591,16 +1593,22 @@ public function downloadUpdate(string $version): array } if (!$extractionSuccess) { + // Describe FIRST, before anything else in this branch. Two + // things would otherwise destroy the evidence: $zip->close() + // flushes the archive and can raise its own error, and + // debugLog() writes to disk — either replaces the extraction + // error that error_get_last() has to report. It also has to + // run before the cleanup below deletes the extraction path and + // the ZIP, since once they are gone the probe can no longer + // see the state that produced the failure. + $cause = $this->describeWriteFailure($extractPath); + $zipStatus = $zip->status; $zip->close(); $this->debugLog('ERROR', 'Estrazione fallita definitivamente', [ 'destination' => $extractPath, - 'zip_status' => $zip->status, - 'last_error' => error_get_last() + 'zip_status' => $zipStatus, + 'last_error' => $cause ]); - // Describe BEFORE the cleanup below deletes the extraction path - // and the ZIP: once they are gone the probe can no longer see - // the state that produced the failure. - $cause = $this->describeWriteFailure($extractPath); // Clean up if (is_dir($extractPath)) { $this->deleteDirectory($extractPath); @@ -1978,12 +1986,14 @@ public function performUpdateFromFile(string $uploadTempPath): array } if (!$zip->extractTo($extractPath)) { + // Describe before close(): flushing the archive can raise an + // error of its own and replace the extraction failure that + // error_get_last() has to report. An extraction that runs out + // of room says nothing about the package; say which it was. + $cause = $this->describeWriteFailure($extractPath); $zip->close(); - // An extraction that runs out of room says nothing about the - // package; say which of the two it was. throw new Exception( - __('Estrazione del pacchetto fallita') - . ' — ' . $this->describeWriteFailure($extractPath) + __('Estrazione del pacchetto fallita') . ' — ' . $cause ); } $zip->close(); diff --git a/tests/update-space-preflight.unit.php b/tests/update-space-preflight.unit.php index 555fd5c5b..aa27493b8 100644 --- a/tests/update-space-preflight.unit.php +++ b/tests/update-space-preflight.unit.php @@ -210,6 +210,48 @@ $reason = (string) $call($updater, 'describeWriteFailure', [$tmp . '/plain.txt']); $check(trim($reason) !== '', 'a cause is always reported, even when nothing obvious is wrong'); +// Same rule, enforced across every call site rather than one at a time. The +// diagnosis has to be the first thing its failure branch does: error_get_last() +// is process-global, so anything performing I/O ahead of it — debugLog(), which +// ends in a file write, or ZipArchive::close(), which flushes — replaces the very +// error the diagnosis exists to report. When this shipped the rule was honoured at +// two call sites and broken at two others; without a gate the next site added +// picks whichever neighbour it happens to copy. +$updaterLines = preg_split('/\r?\n/', $src) ?: []; +$outOfOrder = []; +foreach ($updaterLines as $i => $line) { + if (!str_contains($line, '$this->describeWriteFailure(')) { + continue; + } + // Walk back to the line opening the enclosing branch. + $start = $i; + for ($j = $i - 1; $j >= 0 && ($i - $j) < 40; $j--) { + $t = trim($updaterLines[$j]); + if (str_ends_with($t, '{') + && (str_starts_with($t, 'if (') || str_starts_with($t, '} else') || str_starts_with($t, 'foreach ('))) { + $start = $j; + break; + } + } + // Comments are excluded on purpose: the branches that get this right carry a + // comment explaining WHY they call debugLog() second, and scanning raw text + // would let that explanation trip the very rule it documents. + $between = array_filter( + array_slice($updaterLines, $start + 1, max(0, $i - $start - 1)), + static function (string $l): bool { + $t = ltrim($l); + return $t !== '' && !str_starts_with($t, '//') && !str_starts_with($t, '*') && !str_starts_with($t, '/*'); + } + ); + $code = implode("\n", $between); + if (str_contains($code, 'debugLog(') || str_contains($code, '->close()')) { + $outOfOrder[] = $i + 1; + } +} +$check($outOfOrder === [], + 'the diagnosis runs before any logging or teardown in every failure branch that uses it' + . ($outOfOrder === [] ? '' : ' — violated at line(s) ' . implode(', ', $outOfOrder))); + echo "D. copyDirectory itself reports the cause, not only the helper\n"; $copySrc = $tmp . '/pkg'; $copyDst = $tmp . '/live'; From 50d0d72c7f488ae5e99a4d58d5238e0e433526cc Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Thu, 10 Sep 2026 12:16:08 +0200 Subject: [PATCH 04/10] fix(updater): gate before the first write, size the extraction, reconcile the thresholds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Six findings from the review of the previous two commits. Each verified against the code as it now stands; the ones already closed by those commits are noted at the end rather than touched again. The early gate ran too late by one step. performUpdate() applied the pre-update patch first, and applySinglePatch() writes patched files straight over the tree with no rollback of its own — so a quota that ran out mid-patch left the application half-patched, and the comment above the gate claiming it refused "before the first byte is written" was simply false. The gate now precedes Step 0. performUpdateFromFile() already had the right order. The package was sized from its compressed bytes. That is the only figure available before a download, but a ZIP is free to expand past any fixed ratio, so the x4 allowance is a guess that the archive itself can disprove. Once the archive is open the real number is cheap: extractionSpaceError() sums the uncompressed entries and proves that amount before extractTo() writes anything, on both extraction paths. scripts/manual-upgrade.php already did exactly this; the Updater now matches it. The CLI script declared a 200 MB floor and then proved 16 MB, so a quota sitting between the two passed the check and failed on the first real write. One named constant now feeds both. Its database dump was also unguarded — redirected straight to disk with no size limit, on a path with no file rollback — so the dump is estimated from information_schema and proven before mysqldump runs. Three free-space thresholds disagreed: a blocking 200 MB in the constructor, an advertised 100 MB in the requirements panel, and the ~95 MB the gate actually computes. The constructor's was the worst of the three, because it ran first and threw: with free space between the computed need and 200 MB it made the accurate check unreachable, and since UpdateController never wrapped its eight constructions, /admin/updates answered 500 — costing the operator the one page that would have named the problem. Free space is now advisory at construction (recorded, surfaced in the panel, never fatal), the panel reports max(floor, estimate) instead of a literal, and index() degrades to a page that states the failing precondition. The panel also gained a write-probe row, since disk_free_space() cannot see an exhausted account quota and a green panel in front of a refusing gate is worse than no panel. A test assertion could not fail. strpos() returns false when a symbol is absent and (int) false is 0, which compares below every offset — so the check guarding the CLI helpers passed precisely when those helpers had disappeared. Presence is now asserted before order. Already closed by the previous two commits, re-verified rather than re-fixed: copyDirectory() carrying the cause, the symlink-replacement branch having its own message, the test no longer driving an unbounded probe, the permission cases no longer inverting under root, and the diagnosis preceding every log. Verified: unit suite 43 passed / 0 failed, PHPStan level 5 clean over 540 files, locale keys and placeholders aligned across the five locales. The real admin-UI upgrade was re-run end to end after these edits — upload plus "Avvia" completes, both gates log a pass, and the new extraction check does not block a legitimate package. /admin/updates was opened in a browser: it renders, and now reports "Richiesto: 200 MB" against 101.34 GB free plus a "Quota di scrittura" row reading "Scrittura riuscita". --- app/Controllers/UpdateController.php | 33 +++++- app/Support/Updater.php | 156 ++++++++++++++++++++++---- locale/da_DK.json | 10 +- locale/de_DE.json | 10 +- locale/en_US.json | 10 +- locale/fr_FR.json | 10 +- locale/it_IT.json | 10 +- scripts/manual-upgrade.php | 46 +++++++- tests/update-space-preflight.unit.php | 12 +- 9 files changed, 262 insertions(+), 35 deletions(-) diff --git a/app/Controllers/UpdateController.php b/app/Controllers/UpdateController.php index 12881c79f..4cdea5f77 100644 --- a/app/Controllers/UpdateController.php +++ b/app/Controllers/UpdateController.php @@ -23,7 +23,38 @@ public function index(Request $request, Response $response, mysqli $db): Respons // Admin-only access check removed - relying on Middleware - $updater = new Updater($db); + // The constructor refuses to build when a genuinely fatal precondition + // fails (storage/tmp or storage/backups unwritable, no ZipArchive, no + // HTTP transport). Letting that escape turns this page — the one place + // that would name the problem — into a blank 500, which is the opposite + // of what an operator in trouble needs. Degrade to a page that says what + // to fix instead. + try { + $updater = new Updater($db); + } catch (\Throwable $e) { + ob_start(); + ?> +
+

+ +

+

+ +

+ 
getMessage(), ENT_QUOTES, 'UTF-8');
+                ?>
+
+ getBody()->write($html); + return $response->withStatus(503); + } // Check for updates $updateInfo = $updater->checkForUpdates(); diff --git a/app/Support/Updater.php b/app/Support/Updater.php index 22f328e18..6e3feedff 100644 --- a/app/Support/Updater.php +++ b/app/Support/Updater.php @@ -66,6 +66,18 @@ class Updater */ private const SPACE_PROBE_STRIDE = 1024; + /** + * Coarse free-space floor for the cheap advisory checks. + * + * ONE value, so the constructor's sanity check and the requirements panel + * cannot drift apart from each other or from the real requirement. It is + * deliberately NOT the authority on whether an update fits — that is + * checkFreeSpaceForUpdate(), which measures the tree and proves the + * quota with a real write. This is only a floor below which nothing is + * worth attempting. + */ + private const MIN_FREE_SPACE_BYTES = 200 * 1024 * 1024; + /** * Memoised write-probe verdicts for this update run, keyed by the number of * bytes asked for. describeWriteFailure() is called once per failing file — @@ -75,6 +87,9 @@ class Updater */ private array $probeVerdicts = []; + /** Free-space shortfall seen at construction: advisory, never fatal. */ + private ?string $spaceWarning = null; + /** @var array Files/directories to preserve during update */ private array $preservePaths = [ '.env', @@ -156,11 +171,19 @@ public function __construct(mysqli $db) $issues[] = "Né cURL né allow_url_fopen disponibili"; } - // Check disk space (need at least 200MB free) + // Free space is ADVISORY here, never fatal. Constructing an Updater to + // INSPECT state (the admin updates page builds one on every render) must + // not throw over a condition that only matters when installing — an + // uncaught throw here costs the operator the very page that would tell + // them what is wrong. checkFreeSpaceForUpdate() is the authority at + // install time; this only records the shortfall for the panel to show. $freeSpace = @disk_free_space($this->rootPath); - if ($freeSpace !== false && $freeSpace < 200 * 1024 * 1024) { - $issues[] = sprintf("Spazio disco insufficiente: %s disponibili, servono almeno 200MB", - $this->formatBytes((float)$freeSpace)); + if ($freeSpace !== false && $freeSpace < self::MIN_FREE_SPACE_BYTES) { + $this->spaceWarning = sprintf( + __('Spazio su disco basso: %1$s disponibili, sotto il minimo di %2$s.'), + $this->formatBytes((float) $freeSpace), + $this->formatBytes((float) self::MIN_FREE_SPACE_BYTES) + ); } // Load GitHub API token from settings (if configured) @@ -179,7 +202,8 @@ public function __construct(mysqli $db) 'openssl_available' => extension_loaded('openssl'), 'zip_available' => class_exists('ZipArchive'), 'free_space_mb' => $freeSpace !== false ? round($freeSpace / 1024 / 1024) : 'unknown', - 'pre_flight_issues' => $issues ?: 'none' + 'pre_flight_issues' => $issues ?: 'none', + 'space_warning' => $this->spaceWarning ?? 'none' ]); if (!empty($issues)) { @@ -1531,6 +1555,12 @@ public function downloadUpdate(string $version): array } } + $extractSpaceError = $this->extractionSpaceError($zip); + if ($extractSpaceError !== null) { + $zip->close(); + throw new Exception($extractSpaceError); + } + $extractionSuccess = $zip->extractTo($extractPath); // If extraction failed, try fallback to storage/tmp @@ -1985,6 +2015,12 @@ public function performUpdateFromFile(string $uploadTempPath): array } } + $extractSpaceError = $this->extractionSpaceError($zip); + if ($extractSpaceError !== null) { + $zip->close(); + throw new Exception($extractSpaceError); + } + if (!$zip->extractTo($extractPath)) { // Describe before close(): flushing the archive can raise an // error of its own and replace the extraction failure that @@ -2759,6 +2795,48 @@ private function runWriteProbe(int $bytes): string * first becomes the default answer for everything below it, and on a nearly * full volume it would report "no space" for a missing directory. */ + /** + * Refuse an extraction that provably will not fit, before it writes an entry. + * + * The early gate sizes the package from its COMPRESSED bytes, because that is + * the only figure available before the download. A ZIP is free to expand well + * past any fixed ratio, so once the archive is open the real number is + * knowable and cheap: sum it and prove it. Reuses the messages the gate + * already owns, so this adds no new translation keys. + * + * @return string|null the message to show, or null when it fits + */ + private function extractionSpaceError(ZipArchive $zip): ?string + { + $uncompressed = 0; + for ($i = 0; $i < $zip->numFiles; $i++) { + // statIndex() returns false for an unreadable entry; when it returns + // an array, 'size' is always present. + $stat = $zip->statIndex($i); + if ($stat !== false) { + $uncompressed += (int) $stat['size']; + } + } + if ($uncompressed <= 0) { + return null; // nothing measurable: let the extraction proceed + } + + $verdict = $this->probeWrite($uncompressed, true); + if ($verdict === 'unavailable') { + return sprintf( + __('Impossibile usare la directory di lavoro dell\'aggiornamento (%s): non esiste o non è scrivibile dall\'utente del web server. Correggi i permessi e riprova.'), + $this->rootPath . '/storage/tmp' + ); + } + if ($verdict === 'nospace') { + return sprintf( + __('Impossibile riservare i %1$s richiesti dall\'aggiornamento: lo spazio disponibile o la quota dell\'account sono esauriti. Libera spazio (i backup automatici in storage/backups sono i primi candidati) e riprova.'), + $this->formatBytes($uncompressed) + ); + } + return null; + } + private function describeWriteFailure(string $targetPath): string { // error_get_last() is process-global: capture it BEFORE this function @@ -4380,20 +4458,51 @@ public function checkRequirements(): array if (!$writable) $allMet = false; } - $freeSpace = disk_free_space($this->rootPath); + // Report the SAME figure installUpdate() will enforce. A panel that + // advertises a smaller number than the gate is worse than no panel: it + // shows all-green on an install the update is about to refuse. + $freeSpace = @disk_free_space($this->rootPath); if ($freeSpace === false) { $freeSpace = 0; } - $minSpace = 100 * 1024 * 1024; + $minSpace = max(self::MIN_FREE_SPACE_BYTES, $this->estimateUpdateSpace()); $spaceMet = $freeSpace >= $minSpace; $requirements[] = [ 'name' => __('Spazio libero'), - 'required' => '100MB', + 'required' => $this->formatBytes((float) $minSpace), 'current' => $freeSpace > 0 ? $this->formatBytes($freeSpace) : __('Non disponibile'), 'met' => $spaceMet ]; if (!$spaceMet) $allMet = false; + // disk_free_space() reports the FILESYSTEM. On a cPanel account it can + // show tens of gigabytes free while the account's own quota is exhausted + // — the exact condition that made a healthy release look corrupt. Only a + // real write answers it, so the panel gets its own bounded probe rather + // than inheriting the filesystem's optimism. + $quotaVerdict = $this->probeWrite(self::SPACE_PROBE_BYTES); + $quotaMet = $quotaVerdict === ''; + $requirements[] = [ + 'name' => __('Quota di scrittura'), + 'required' => $this->formatBytes((float) self::SPACE_PROBE_BYTES), + 'current' => match ($quotaVerdict) { + '' => __('Scrittura riuscita'), + 'nospace' => __('Spazio o quota esauriti'), + default => __('storage/tmp non utilizzabile'), + }, + 'met' => $quotaMet + ]; + if (!$quotaMet) $allMet = false; + + if ($this->spaceWarning !== null) { + $requirements[] = [ + 'name' => __('Avviso spazio'), + 'required' => $this->formatBytes((float) self::MIN_FREE_SPACE_BYTES), + 'current' => $this->spaceWarning, + 'met' => false + ]; + } + return [ 'met' => $allMet, 'requirements' => $requirements @@ -4540,6 +4649,22 @@ public function performUpdate(string $targetVersion): array $result = null; try { + // PRE-FLIGHT (issue #422): refuse BEFORE the first byte is written — + // and that includes Step 0. applySinglePatch() writes patched files + // straight over the tree with no rollback of its own, so a quota that + // runs out mid-patch leaves the application half-patched. Steps 1 and + // 2 then write the backup archive, the release ZIP and its extracted + // tree, together as much as the rollback copy the gate in + // installUpdate() protects. That later gate stays: it re-measures + // once these steps have consumed their share, which is the only + // measurement valid there. + $packageBytes = $this->releaseAssetBytes($targetVersion); + $spaceError = $this->checkFreeSpaceForUpdate($packageBytes > 0 ? $packageBytes * 4 : 0); + if ($spaceError !== null) { + $this->debugLog('ERROR', 'Preflight iniziale: spazio insufficiente', ['detail' => $spaceError]); + throw new Exception($spaceError); + } + // Step 0: Apply pre-update patch (if available) $this->debugLog('INFO', '>>> STEP 0: Pre-update patch check <<<'); $patchResult = $this->applyPreUpdatePatch($targetVersion); @@ -4553,21 +4678,6 @@ public function performUpdate(string $targetVersion): array ]); } - // PRE-FLIGHT (issue #422): refuse BEFORE the first byte is written. - // Steps 1 and 2 write the backup archive, the release ZIP and its - // extracted tree — together as much as the rollback copy the gate in - // installUpdate() protects. On an account whose quota is exhausted - // the update used to die inside one of them with "Backup fallito" or - // "Download fallito" and no mention of space at all. The gate in - // installUpdate() stays: it re-measures once these steps have - // consumed their share, which is the only measurement valid there. - $packageBytes = $this->releaseAssetBytes($targetVersion); - $spaceError = $this->checkFreeSpaceForUpdate($packageBytes > 0 ? $packageBytes * 4 : 0); - if ($spaceError !== null) { - $this->debugLog('ERROR', 'Preflight iniziale: spazio insufficiente', ['detail' => $spaceError]); - throw new Exception($spaceError); - } - // Step 1: Backup $this->debugLog('INFO', '>>> STEP 1: Creazione backup <<<'); $backupResult = $this->createBackup(); diff --git a/locale/da_DK.json b/locale/da_DK.json index 8f6b3631d..501af85eb 100644 --- a/locale/da_DK.json +++ b/locale/da_DK.json @@ -7517,5 +7517,13 @@ "Impossibile riservare i %1$s richiesti dall'aggiornamento: lo spazio disponibile o la quota dell'account sono esauriti. Libera spazio (i backup automatici in storage/backups sono i primi candidati) e riprova.": "Kunne ikke reservere de %1$s, som opdateringen kræver: den tilgængelige plads eller kontokvoten er opbrugt. Frigør plads (de automatiske sikkerhedskopier i storage/backups er de første kandidater), og prøv igen.", "Impossibile usare la directory di lavoro dell'aggiornamento (%s): non esiste o non è scrivibile dall'utente del web server. Correggi i permessi e riprova.": "Opdateringens arbejdsmappe (%s) kan ikke bruges: den findes ikke eller kan ikke skrives af webserverens bruger. Ret rettighederne, og prøv igen.", "Impossibile creare directory: %s — %s": "Kunne ikke oprette mappe: %s — %s", - "Impossibile sostituire il collegamento simbolico nella destinazione: %s — %s": "Kunne ikke erstatte det symbolske link i destinationen: %s — %s" + "Impossibile sostituire il collegamento simbolico nella destinazione: %s — %s": "Kunne ikke erstatte det symbolske link i destinationen: %s — %s", + "Spazio su disco basso: %1$s disponibili, sotto il minimo di %2$s.": "Lav diskplads: %1$s tilgængelig, under minimum på %2$s.", + "Quota di scrittura": "Skrivekvote", + "Scrittura riuscita": "Skrivning lykkedes", + "Spazio o quota esauriti": "Plads eller kvote opbrugt", + "storage/tmp non utilizzabile": "storage/tmp kan ikke bruges", + "Avviso spazio": "Pladsadvarsel", + "Aggiornamenti non disponibili": "Opdateringer utilgængelige", + "Il sistema di aggiornamento non può essere avviato perché una condizione preliminare non è soddisfatta. Correggi quanto indicato qui sotto e ricarica la pagina.": "Opdateringssystemet kan ikke startes, fordi en forudsætning ikke er opfyldt. Ret det, der er anført nedenfor, og genindlæs siden." } diff --git a/locale/de_DE.json b/locale/de_DE.json index 2caee86bd..665c7d949 100644 --- a/locale/de_DE.json +++ b/locale/de_DE.json @@ -7517,5 +7517,13 @@ "Impossibile riservare i %1$s richiesti dall'aggiornamento: lo spazio disponibile o la quota dell'account sono esauriti. Libera spazio (i backup automatici in storage/backups sono i primi candidati) e riprova.": "Die für das Update benötigten %1$s konnten nicht reserviert werden: Der verfügbare Speicherplatz oder das Kontingent des Kontos ist erschöpft. Geben Sie Speicherplatz frei (die automatischen Sicherungen in storage/backups sind die ersten Kandidaten) und versuchen Sie es erneut.", "Impossibile usare la directory di lavoro dell'aggiornamento (%s): non esiste o non è scrivibile dall'utente del web server. Correggi i permessi e riprova.": "Das Arbeitsverzeichnis des Updates (%s) kann nicht verwendet werden: Es existiert nicht oder ist für den Benutzer des Webservers nicht beschreibbar. Korrigieren Sie die Berechtigungen und versuchen Sie es erneut.", "Impossibile creare directory: %s — %s": "Verzeichnis konnte nicht erstellt werden: %s — %s", - "Impossibile sostituire il collegamento simbolico nella destinazione: %s — %s": "Der symbolische Link am Ziel konnte nicht ersetzt werden: %s — %s" + "Impossibile sostituire il collegamento simbolico nella destinazione: %s — %s": "Der symbolische Link am Ziel konnte nicht ersetzt werden: %s — %s", + "Spazio su disco basso: %1$s disponibili, sotto il minimo di %2$s.": "Wenig Speicherplatz: %1$s verfügbar, unter dem Minimum von %2$s.", + "Quota di scrittura": "Schreibkontingent", + "Scrittura riuscita": "Schreiben erfolgreich", + "Spazio o quota esauriti": "Speicherplatz oder Kontingent erschöpft", + "storage/tmp non utilizzabile": "storage/tmp nicht verwendbar", + "Avviso spazio": "Speicherplatz-Warnung", + "Aggiornamenti non disponibili": "Updates nicht verfügbar", + "Il sistema di aggiornamento non può essere avviato perché una condizione preliminare non è soddisfatta. Correggi quanto indicato qui sotto e ricarica la pagina.": "Das Update-System kann nicht gestartet werden, weil eine Voraussetzung nicht erfüllt ist. Beheben Sie den unten genannten Punkt und laden Sie die Seite neu." } diff --git a/locale/en_US.json b/locale/en_US.json index 4064b153f..39f5b1505 100644 --- a/locale/en_US.json +++ b/locale/en_US.json @@ -7517,5 +7517,13 @@ "Impossibile riservare i %1$s richiesti dall'aggiornamento: lo spazio disponibile o la quota dell'account sono esauriti. Libera spazio (i backup automatici in storage/backups sono i primi candidati) e riprova.": "Could not reserve the %1$s the update needs: the available space or the account quota is exhausted. Free some space (the automatic backups in storage/backups are the first candidates) and try again.", "Impossibile usare la directory di lavoro dell'aggiornamento (%s): non esiste o non è scrivibile dall'utente del web server. Correggi i permessi e riprova.": "Cannot use the update working directory (%s): it does not exist or is not writable by the web server user. Fix the permissions and try again.", "Impossibile creare directory: %s — %s": "Unable to create directory: %s — %s", - "Impossibile sostituire il collegamento simbolico nella destinazione: %s — %s": "Unable to replace the symbolic link at the destination: %s — %s" + "Impossibile sostituire il collegamento simbolico nella destinazione: %s — %s": "Unable to replace the symbolic link at the destination: %s — %s", + "Spazio su disco basso: %1$s disponibili, sotto il minimo di %2$s.": "Low disk space: %1$s available, below the %2$s minimum.", + "Quota di scrittura": "Write quota", + "Scrittura riuscita": "Write succeeded", + "Spazio o quota esauriti": "Space or quota exhausted", + "storage/tmp non utilizzabile": "storage/tmp unusable", + "Avviso spazio": "Space warning", + "Aggiornamenti non disponibili": "Updates unavailable", + "Il sistema di aggiornamento non può essere avviato perché una condizione preliminare non è soddisfatta. Correggi quanto indicato qui sotto e ricarica la pagina.": "The update system cannot start because a precondition is not met. Fix what is listed below and reload the page." } diff --git a/locale/fr_FR.json b/locale/fr_FR.json index c22d4ad60..c43df384f 100644 --- a/locale/fr_FR.json +++ b/locale/fr_FR.json @@ -7517,5 +7517,13 @@ "Impossibile riservare i %1$s richiesti dall'aggiornamento: lo spazio disponibile o la quota dell'account sono esauriti. Libera spazio (i backup automatici in storage/backups sono i primi candidati) e riprova.": "Impossible de réserver les %1$s requis par la mise à jour : l'espace disponible ou le quota du compte est épuisé. Libérez de l'espace (les sauvegardes automatiques dans storage/backups sont les premières candidates) et réessayez.", "Impossibile usare la directory di lavoro dell'aggiornamento (%s): non esiste o non è scrivibile dall'utente del web server. Correggi i permessi e riprova.": "Impossible d'utiliser le répertoire de travail de la mise à jour (%s) : il n'existe pas ou n'est pas accessible en écriture par l'utilisateur du serveur web. Corrigez les permissions et réessayez.", "Impossibile creare directory: %s — %s": "Impossible de créer le répertoire : %s — %s", - "Impossibile sostituire il collegamento simbolico nella destinazione: %s — %s": "Impossible de remplacer le lien symbolique de destination : %s — %s" + "Impossibile sostituire il collegamento simbolico nella destinazione: %s — %s": "Impossible de remplacer le lien symbolique de destination : %s — %s", + "Spazio su disco basso: %1$s disponibili, sotto il minimo di %2$s.": "Espace disque faible : %1$s disponibles, en dessous du minimum de %2$s.", + "Quota di scrittura": "Quota d'écriture", + "Scrittura riuscita": "Écriture réussie", + "Spazio o quota esauriti": "Espace ou quota épuisé", + "storage/tmp non utilizzabile": "storage/tmp inutilisable", + "Avviso spazio": "Avertissement d'espace", + "Aggiornamenti non disponibili": "Mises à jour indisponibles", + "Il sistema di aggiornamento non può essere avviato perché una condizione preliminare non è soddisfatta. Correggi quanto indicato qui sotto e ricarica la pagina.": "Le système de mise à jour ne peut pas démarrer car une condition préalable n'est pas remplie. Corrigez ce qui est indiqué ci-dessous et rechargez la page." } diff --git a/locale/it_IT.json b/locale/it_IT.json index 00b25a3b2..798e7ff80 100644 --- a/locale/it_IT.json +++ b/locale/it_IT.json @@ -7517,5 +7517,13 @@ "Impossibile riservare i %1$s richiesti dall'aggiornamento: lo spazio disponibile o la quota dell'account sono esauriti. Libera spazio (i backup automatici in storage/backups sono i primi candidati) e riprova.": "Impossibile riservare i %1$s richiesti dall'aggiornamento: lo spazio disponibile o la quota dell'account sono esauriti. Libera spazio (i backup automatici in storage/backups sono i primi candidati) e riprova.", "Impossibile usare la directory di lavoro dell'aggiornamento (%s): non esiste o non è scrivibile dall'utente del web server. Correggi i permessi e riprova.": "Impossibile usare la directory di lavoro dell'aggiornamento (%s): non esiste o non è scrivibile dall'utente del web server. Correggi i permessi e riprova.", "Impossibile creare directory: %s — %s": "Impossibile creare directory: %s — %s", - "Impossibile sostituire il collegamento simbolico nella destinazione: %s — %s": "Impossibile sostituire il collegamento simbolico nella destinazione: %s — %s" + "Impossibile sostituire il collegamento simbolico nella destinazione: %s — %s": "Impossibile sostituire il collegamento simbolico nella destinazione: %s — %s", + "Spazio su disco basso: %1$s disponibili, sotto il minimo di %2$s.": "Spazio su disco basso: %1$s disponibili, sotto il minimo di %2$s.", + "Quota di scrittura": "Quota di scrittura", + "Scrittura riuscita": "Scrittura riuscita", + "Spazio o quota esauriti": "Spazio o quota esauriti", + "storage/tmp non utilizzabile": "storage/tmp non utilizzabile", + "Avviso spazio": "Avviso spazio", + "Aggiornamenti non disponibili": "Aggiornamenti non disponibili", + "Il sistema di aggiornamento non può essere avviato perché una condizione preliminare non è soddisfatta. Correggi quanto indicato qui sotto e ricarica la pagina.": "Il sistema di aggiornamento non può essere avviato perché una condizione preliminare non è soddisfatta. Correggi quanto indicato qui sotto e ricarica la pagina." } diff --git a/scripts/manual-upgrade.php b/scripts/manual-upgrade.php index d534f82e2..0be85a1e9 100644 --- a/scripts/manual-upgrade.php +++ b/scripts/manual-upgrade.php @@ -22,6 +22,11 @@ // ============================================================ define('UPGRADE_PASSWORD', 'pinakes2026'); define('MAX_ZIP_SIZE', 512 * 1024 * 1024); // 512 MB (aligned with scripts/.user.ini) +// Coarse floor for an upgrade. One value, used by BOTH the disk_free_space() +// gate and the write probe that backs it: a probe smaller than the declared +// floor lets a quota between the two pass the check and fail on the first +// real write. +define('MIN_UPGRADE_FREE_BYTES', 200 * 1024 * 1024); // ============================================================ // BOOTSTRAP @@ -598,15 +603,18 @@ function copyTree(string $src, string $dst, string $rootDst, array $skipRelative } $freeSpace = @disk_free_space($rootPath); - if ($freeSpace !== false && $freeSpace < 200 * 1024 * 1024) { - throw new RuntimeException('Spazio disco insufficiente: ' . formatBytes((int)$freeSpace) . ' disponibili, servono almeno 200 MB.'); + if ($freeSpace !== false && $freeSpace < MIN_UPGRADE_FREE_BYTES) { + throw new RuntimeException('Spazio disco insufficiente: ' . formatBytes((int)$freeSpace) . ' disponibili, servono almeno ' . formatBytes(MIN_UPGRADE_FREE_BYTES) . '.'); } // disk_free_space() alone lies on a cPanel account whose quota is // exhausted: it reports the FILESYSTEM. Prove with a real write that // this account can still allocate, here — the last point at which a // refusal costs the operator nothing, because after it the DB dump and // the tree overwrite begin and this script has no file rollback. - $probeVerdict = probeWriteBytes($rootPath, 16 * 1024 * 1024); + // Prove the SAME amount the gate above demands: a probe smaller than the + // declared floor lets a quota sitting between the two pass and then fail + // on the first real write. + $probeVerdict = probeWriteBytes($rootPath, MIN_UPGRADE_FREE_BYTES); if ($probeVerdict === 'unavailable') { throw new RuntimeException('Impossibile usare storage/tmp: non esiste o non è scrivibile. Correggi i permessi prima di continuare.'); } @@ -647,6 +655,38 @@ function copyTree(string $src, string $dst, string $rootDst, array $skipRelative } $backupFile = $backupDir . '/pre_upgrade_' . str_replace('.', '_', $currentVersion) . '_' . date('Ymd_His') . '.sql'; + // The dump is redirected straight to disk with no size limit of its own, + // so a database larger than the coarse pre-flight floor can still exhaust + // the quota mid-write — and this script has no file rollback. Estimate it + // from information_schema and prove that much really fits first. The + // estimate is deliberately generous: an SQL dump is text, so it runs + // larger than the stored byte count it is derived from. + $dumpEstimate = 0; + $sizeRow = $db->query( + "SELECT COALESCE(SUM(data_length + index_length), 0) AS bytes + FROM information_schema.tables + WHERE table_schema = DATABASE()" + ); + if ($sizeRow instanceof mysqli_result) { + $row = $sizeRow->fetch_assoc(); + $sizeRow->free(); + $dumpEstimate = (int) (((float) ($row['bytes'] ?? 0)) * 1.5); + } + if ($dumpEstimate > 0) { + $dumpVerdict = probeWriteBytes($rootPath, $dumpEstimate); + if ($dumpVerdict === 'unavailable') { + throw new RuntimeException('Impossibile usare storage/tmp per verificare lo spazio del backup: non esiste o non è scrivibile. Correggi i permessi prima di continuare.'); + } + if ($dumpVerdict === 'nospace') { + throw new RuntimeException( + 'Spazio insufficiente per il backup del database: servono circa ' + . formatBytes((float) $dumpEstimate) + . '. Libera spazio (i backup automatici in storage/backups sono i primi candidati) e riprova.' + ); + } + $log[] = '[OK] Spazio per il dump verificato (~' . formatBytes((float) $dumpEstimate) . ')'; + } + $mysqldumpBin = null; foreach (['/usr/bin/mysqldump', '/usr/local/bin/mysqldump', '/opt/homebrew/bin/mysqldump'] as $candidate) { if (is_file($candidate) && is_executable($candidate)) { diff --git a/tests/update-space-preflight.unit.php b/tests/update-space-preflight.unit.php index aa27493b8..f66301dba 100644 --- a/tests/update-space-preflight.unit.php +++ b/tests/update-space-preflight.unit.php @@ -342,10 +342,16 @@ static function (string $l): bool { // The helpers and every check that uses them must sit BEFORE that point: they // run on installations where the application itself will not boot. $autoloadAt = strpos($cli, 'vendor/autoload.php'); +// Presence first, THEN order. strpos() returns false when the symbol is absent, +// and (int) false is 0, which compares below every offset — an assertion written +// that way passes precisely when the thing it guards has disappeared. +$probeDefAt = strpos($cli, 'function probeWriteBytes('); +$diagDefAt = strpos($cli, 'function describeWriteFailure('); +$probeUseAt = strrpos($cli, 'probeWriteBytes($rootPath'); $check($autoloadAt !== false - && (int) strpos($cli, 'function probeWriteBytes(') < $autoloadAt - && (int) strpos($cli, 'function describeWriteFailure(') < $autoloadAt - && (int) strrpos($cli, 'probeWriteBytes($rootPath') < $autoloadAt, + && $probeDefAt !== false && $probeDefAt < $autoloadAt + && $diagDefAt !== false && $diagDefAt < $autoloadAt + && $probeUseAt !== false && $probeUseAt < $autoloadAt, 'probe and diagnosis are defined and used before any autoloader is required'); $cliLines = preg_split('/\r?\n/', $cli) ?: []; foreach (['Copia file fallita', 'Backup file critico fallito', 'Impossibile creare directory destinazione'] as $message) { From e52cf325b7256546561f6c9c7d1c9bcdfe4d718e Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Thu, 10 Sep 2026 12:59:58 +0200 Subject: [PATCH 05/10] perf(updater): stop paying update-sized costs on every render of the updates panel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous commit made the requirements panel report what the gate actually enforces, which was right, and paid for it in the wrong currency. checkRequirements() runs on every render of /admin/updates, and it was walking APP_BACKUP_DIRS — around 5.100 files once vendor/ is counted — and writing then deleting a 16 MB probe each time. That is update-sized work on a page an operator reloads while trying to free space: the screen itself was consuming the resource they were there to reclaim. The estimate is now cached against the installed version. The version is the right key because it is what changes when those directories change: an update replaces the tree and bumps version.json together, so the first read after an update misses and recomputes once. A rebuild of public/assets without a version bump leaves the number stale, which is a development scenario, not a production one — and it cannot mislead the operator, because the gate never reads the cache. It always measures for real, every time. The panel's probe drops from 16 MB to 1 MB. The two probes answer different questions: the panel asks "can this account write at all", which one megabyte settles as well as sixteen, while the gate asks "can it write what the update needs" and keeps proving the full requirement. Conflating the two is what made a cheap check expensive. estimateUpdateSpace() also gained a per-instance memo, so the two gate call sites inside one update no longer walk the tree twice. Measured on this checkout: 5.121 files and 73 MB walked, ~95 MB estimated. Panel render went from 0.039 s to 0.001 s once the cache is warm, and per-render I/O from 16 MB to 1 MB. Cache invalidation verified by forcing a wrong version key: the estimate was recomputed rather than trusted, and the entry rewritten. Verified: unit suite 43 passed / 0 failed, PHPStan level 5 clean, locales aligned, and the real admin-UI upgrade re-run end to end — upload plus "Avvia" completes and both gates still log a pass with the full requirement measured. --- app/Support/Updater.php | 66 +++++++++++++++++++++++++++++++++++++---- 1 file changed, 60 insertions(+), 6 deletions(-) diff --git a/app/Support/Updater.php b/app/Support/Updater.php index 6e3feedff..e20cb674d 100644 --- a/app/Support/Updater.php +++ b/app/Support/Updater.php @@ -78,6 +78,14 @@ class Updater */ private const MIN_FREE_SPACE_BYTES = 200 * 1024 * 1024; + /** + * Probe size for the requirements panel, which renders far more often than + * an update runs. It answers "can this account write at all", not "can it + * write what the update needs" — that is the gate's question, and the gate + * keeps the full-size probe. + */ + private const PANEL_PROBE_BYTES = 1024 * 1024; + /** * Memoised write-probe verdicts for this update run, keyed by the number of * bytes asked for. describeWriteFailure() is called once per failing file — @@ -90,6 +98,9 @@ class Updater /** Free-space shortfall seen at construction: advisory, never fatal. */ private ?string $spaceWarning = null; + /** Per-instance memo for estimateUpdateSpace(): the walk is not free. */ + private ?int $estimateCache = null; + /** @var array Files/directories to preserve during update */ private array $preservePaths = [ '.env', @@ -2578,13 +2589,49 @@ public function installUpdate(string $sourcePath, string $targetVersion): array */ private function estimateUpdateSpace(): int { + if ($this->estimateCache !== null) { + return $this->estimateCache; + } $total = 0; foreach (self::APP_BACKUP_DIRS as $dir) { $total += $this->directorySize($this->rootPath . '/' . $dir); } // 30% margin: the copy writes new files before the old ones are gone, // and a partially-fitting update is the failure mode being prevented. - return (int) ($total * 1.3); + return $this->estimateCache = (int) ($total * 1.3); + } + + /** + * The same figure, cheap enough to put on a page that renders often. + * + * estimateUpdateSpace() walks APP_BACKUP_DIRS — around 5.000 files once + * vendor/ is included. That is a fair price to pay before an update, and the + * wrong price to pay on every render of the updates panel, which is exactly + * the page an operator reloads while trying to free space. + * + * Cached against the installed version, because the version is what changes + * when those directories change: an update replaces the tree AND bumps + * version.json, so the next read after an update misses and recomputes once. + * A rebuild of public/assets without a version bump would leave the number + * stale, which is a development scenario, not a production one — and the + * gate never reads this, it always measures for real. + */ + private function cachedUpdateSpaceEstimate(): int + { + $version = $this->getCurrentVersion(); + $settings = new SettingsRepository($this->db); + $cached = $settings->get('updater', 'space_estimate', null); + + if (is_string($cached) && $cached !== '') { + $parts = explode(':', $cached, 2); + if (count($parts) === 2 && $parts[0] === $version && ctype_digit($parts[1])) { + return (int) $parts[1]; + } + } + + $estimate = $this->estimateUpdateSpace(); + $settings->set('updater', 'space_estimate', $version . ':' . $estimate); + return $estimate; } /** Size of a directory in bytes; 0 when it does not exist or cannot be read. */ @@ -4465,7 +4512,7 @@ public function checkRequirements(): array if ($freeSpace === false) { $freeSpace = 0; } - $minSpace = max(self::MIN_FREE_SPACE_BYTES, $this->estimateUpdateSpace()); + $minSpace = max(self::MIN_FREE_SPACE_BYTES, $this->cachedUpdateSpaceEstimate()); $spaceMet = $freeSpace >= $minSpace; $requirements[] = [ 'name' => __('Spazio libero'), @@ -4478,13 +4525,20 @@ public function checkRequirements(): array // disk_free_space() reports the FILESYSTEM. On a cPanel account it can // show tens of gigabytes free while the account's own quota is exhausted // — the exact condition that made a healthy release look corrupt. Only a - // real write answers it, so the panel gets its own bounded probe rather - // than inheriting the filesystem's optimism. - $quotaVerdict = $this->probeWrite(self::SPACE_PROBE_BYTES); + // real write answers it, so the panel gets its own probe rather than + // inheriting the filesystem's optimism. + // + // A SMALL one, deliberately. The panel asks "can this account write at + // all?", which one megabyte answers exactly as well as sixteen; the gate + // keeps the full-size probe because it asks the harder question, "can it + // write what the update needs?". Sixteen megabytes written and deleted on + // every render of this page is churn an operator does not need on the + // very screen they reload while trying to free space. + $quotaVerdict = $this->probeWrite(self::PANEL_PROBE_BYTES); $quotaMet = $quotaVerdict === ''; $requirements[] = [ 'name' => __('Quota di scrittura'), - 'required' => $this->formatBytes((float) self::SPACE_PROBE_BYTES), + 'required' => $this->formatBytes((float) self::PANEL_PROBE_BYTES), 'current' => match ($quotaVerdict) { '' => __('Scrittura riuscita'), 'nospace' => __('Spazio o quota esauriti'), From 504cc4484d9b03b20fd107eb5e35f188817e79a6 Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Thu, 10 Sep 2026 15:58:06 +0200 Subject: [PATCH 06/10] feat(updater): disable the in-app update on the official Docker image MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The intended policy was always that the official image upgrades by moving the container to a new image, and the docblock on isRunningInContainer() has said "cannot (and must not)" since it was written. Nothing enforced it, and the image contradicted it: the very first Dockerfile chowns the whole tree to www-data, so the code is writable, and the image README told operators the Admin → Updates button works. It does work today — that is the bug. Half of what an update changes survives a container recreate. The schema migrations land in the database volume and stay applied; the new code lives in the container layer and is discarded. Recreate from the old image afterwards and you are running old code against a migrated schema, with nothing to tell you. Both entry points now refuse before taking the lock and before maintenance mode, because there is no reason to take a site down for an update that is refused outright. The message says why and gives the one command that does the job. The refusal keys on the official-image marker, not on ContainerRuntime::detected(). Container-ness is the wrong predicate: it is true for any container, including community images that keep the code in a writable volume where an in-app update is legitimate and survives. Blocking on it would refuse someone else's working setup to enforce a policy about ours. ContainerRuntime gained a narrow officialImage() accessor for exactly this, and the test asserts the refusal does NOT reference detected(), so the distinction cannot erode. The docblock on isRunningInContainer() is corrected while I am here. It claimed the official image is read-only, which was never true — the image was born writable three weeks before that text was written. What actually reaches that branch is a hardened deployment (read-only rootfs, :ro bind mount, mismatched uid) or a community image whose code volume is currently read-only. Verified: unit suite 51 passed / 0 failed, PHPStan level 5 clean, the refusal message translated in all five locales, and the real admin-UI upgrade re-run end to end on a normal (non-container) install — it still completes, which is the risk this change had to clear. The image README is corrected in the pinakes-docker repo, where it currently promises the opposite; that edit is left uncommitted there because the repo has unrelated work in flight. --- app/Support/ContainerRuntime.php | 13 ++++++ app/Support/Updater.php | 56 +++++++++++++++++++++--- locale/da_DK.json | 3 +- locale/de_DE.json | 3 +- locale/en_US.json | 3 +- locale/fr_FR.json | 3 +- locale/it_IT.json | 3 +- tests/update-space-preflight.unit.php | 61 +++++++++++++++++++++++++++ 8 files changed, 135 insertions(+), 10 deletions(-) diff --git a/app/Support/ContainerRuntime.php b/app/Support/ContainerRuntime.php index f2740e8cc..394cd9cd6 100644 --- a/app/Support/ContainerRuntime.php +++ b/app/Support/ContainerRuntime.php @@ -35,6 +35,19 @@ public static function detected(): bool && preg_match('/\b(docker|containerd|kubepods|libpod)\b/', $cgroup) === 1; } + /** + * True only on the official Pinakes image, not on containers in general. + * + * The distinction is load-bearing. detected() is true for any container, + * including community images that keep the code in a writable volume where + * an in-app update legitimately works. Policy that belongs to OUR image must + * key on the marker that image writes, so it cannot reach anyone else's. + */ + public static function officialImage(): bool + { + return is_file(self::OFFICIAL_IMAGE_MARKER); + } + /** Recognize only explicit truthy values; "false" must not mean Docker. */ private static function isTruthyFlag(string $value): bool { diff --git a/app/Support/Updater.php b/app/Support/Updater.php index e20cb674d..52adcd08b 100644 --- a/app/Support/Updater.php +++ b/app/Support/Updater.php @@ -1858,6 +1858,14 @@ public function saveUploadedPackage(\Psr\Http\Message\UploadedFileInterface $upl */ public function performUpdateFromFile(string $uploadTempPath): array { + // Same refusal as the automatic route: the manual upload is the fallback + // an operator reaches for next, and it rewrites the same container layer. + $imageBlock = $this->officialImageUpdateBlock(); + if ($imageBlock !== null) { + $this->debugLog('INFO', 'Aggiornamento manuale rifiutato: immagine Docker ufficiale'); + return ['success' => false, 'error' => $imageBlock, 'backup_path' => null]; + } + $lockFile = $this->rootPath . '/storage/cache/update.lock'; $lockHandle = null; @@ -2842,6 +2850,32 @@ private function runWriteProbe(int $bytes): string * first becomes the default answer for everything below it, and on a nearly * full volume it would report "no space" for a missing directory. */ + /** + * Refuse an in-app update on the official Docker image. + * + * On that image the upgrade path is to move the container to the new image, + * not to rewrite the code in place. The reason is not that the files resist + * — they are chowned to www-data and perfectly writable — but that only HALF + * of what an update changes survives a container recreate: the schema + * migrations land in the database volume and persist, while the new code + * lives in the container layer and is thrown away. Recreating from the old + * image then runs old code against a migrated schema, silently. + * + * Keyed on the official-image marker, NOT on ContainerRuntime::detected(). + * Container-ness is the wrong predicate: community images keep the code in a + * writable volume where an in-app update is legitimate and survives, and + * this project has no business refusing theirs. We answer for our image. + * + * @return string|null the message to show, or null when the update may run + */ + private function officialImageUpdateBlock(): ?string + { + if (!ContainerRuntime::officialImage()) { + return null; + } + return __('Questa è l\'immagine Docker ufficiale di Pinakes: l\'aggiornamento dall\'applicazione è disattivato di proposito. Il codice verrebbe riscritto solo nel layer del container e andrebbe perso alla prima ricreazione, mentre le migrazioni del database resterebbero applicate — lasciando codice vecchio su uno schema nuovo. Aggiorna spostando il container sulla nuova immagine: "docker compose pull && docker compose up -d".'); + } + /** * Refuse an extraction that provably will not fit, before it writes an entry. * @@ -3437,11 +3471,15 @@ private function removeOrphansInDirectory(string $currentDir, string $newDir, st * PHP and are reported for manual fixing (or for the CLI upgrade path). * * True when the app is running inside a container (Docker/Podman/Kubernetes). - * On the official image the app files are baked in and owned by the image, so - * the in-app updater cannot (and must not) overwrite them — the operator moves - * the container to the new image instead. Detection is best-effort across the - * common signals; a false negative only falls back to the generic permission - * message, never a wrong action. + * This only picks the WORDING of a permission failure that has already + * happened; it decides nothing. Note that the official image does NOT reach + * here: its files are chowned to www-data and perfectly writable, and the + * in-app update is refused earlier and for a different reason — see + * officialImageUpdateBlock(). What lands here is a hardened deployment (a + * read-only rootfs, a :ro bind mount, a mismatched uid) or a community image + * whose code volume is currently read-only. Detection is best-effort across + * the common signals; a false negative only falls back to the generic + * permission message, never a wrong action. */ private function isRunningInContainer(): bool { @@ -4610,6 +4648,14 @@ public function getChangelog(string $fromVersion): array */ public function performUpdate(string $targetVersion): array { + // Before the lock and before maintenance mode: there is no reason to take + // the site down for an update that is refused outright. + $imageBlock = $this->officialImageUpdateBlock(); + if ($imageBlock !== null) { + $this->debugLog('INFO', 'Aggiornamento in-app rifiutato: immagine Docker ufficiale'); + return ['success' => false, 'error' => $imageBlock, 'backup_path' => null]; + } + $lockFile = $this->rootPath . '/storage/cache/update.lock'; $lockHandle = null; diff --git a/locale/da_DK.json b/locale/da_DK.json index 501af85eb..ba6ed0cae 100644 --- a/locale/da_DK.json +++ b/locale/da_DK.json @@ -7525,5 +7525,6 @@ "storage/tmp non utilizzabile": "storage/tmp kan ikke bruges", "Avviso spazio": "Pladsadvarsel", "Aggiornamenti non disponibili": "Opdateringer utilgængelige", - "Il sistema di aggiornamento non può essere avviato perché una condizione preliminare non è soddisfatta. Correggi quanto indicato qui sotto e ricarica la pagina.": "Opdateringssystemet kan ikke startes, fordi en forudsætning ikke er opfyldt. Ret det, der er anført nedenfor, og genindlæs siden." + "Il sistema di aggiornamento non può essere avviato perché una condizione preliminare non è soddisfatta. Correggi quanto indicato qui sotto e ricarica la pagina.": "Opdateringssystemet kan ikke startes, fordi en forudsætning ikke er opfyldt. Ret det, der er anført nedenfor, og genindlæs siden.", + "Questa è l'immagine Docker ufficiale di Pinakes: l'aggiornamento dall'applicazione è disattivato di proposito. Il codice verrebbe riscritto solo nel layer del container e andrebbe perso alla prima ricreazione, mentre le migrazioni del database resterebbero applicate — lasciando codice vecchio su uno schema nuovo. Aggiorna spostando il container sulla nuova immagine: \"docker compose pull && docker compose up -d\".": "Dette er det officielle Pinakes-Docker-image: opdatering inde fra applikationen er deaktiveret med vilje. Den nye kode ville kun blive skrevet til containerlaget og gå tabt, næste gang containeren genskabes, mens databasemigreringerne ville forblive anvendt — hvilket efterlader gammel kode på et nyt skema. Opdater i stedet ved at flytte containeren til det nye image: \"docker compose pull && docker compose up -d\"." } diff --git a/locale/de_DE.json b/locale/de_DE.json index 665c7d949..ababd8b75 100644 --- a/locale/de_DE.json +++ b/locale/de_DE.json @@ -7525,5 +7525,6 @@ "storage/tmp non utilizzabile": "storage/tmp nicht verwendbar", "Avviso spazio": "Speicherplatz-Warnung", "Aggiornamenti non disponibili": "Updates nicht verfügbar", - "Il sistema di aggiornamento non può essere avviato perché una condizione preliminare non è soddisfatta. Correggi quanto indicato qui sotto e ricarica la pagina.": "Das Update-System kann nicht gestartet werden, weil eine Voraussetzung nicht erfüllt ist. Beheben Sie den unten genannten Punkt und laden Sie die Seite neu." + "Il sistema di aggiornamento non può essere avviato perché una condizione preliminare non è soddisfatta. Correggi quanto indicato qui sotto e ricarica la pagina.": "Das Update-System kann nicht gestartet werden, weil eine Voraussetzung nicht erfüllt ist. Beheben Sie den unten genannten Punkt und laden Sie die Seite neu.", + "Questa è l'immagine Docker ufficiale di Pinakes: l'aggiornamento dall'applicazione è disattivato di proposito. Il codice verrebbe riscritto solo nel layer del container e andrebbe perso alla prima ricreazione, mentre le migrazioni del database resterebbero applicate — lasciando codice vecchio su uno schema nuovo. Aggiorna spostando il container sulla nuova immagine: \"docker compose pull && docker compose up -d\".": "Dies ist das offizielle Pinakes-Docker-Image: Die Aktualisierung aus der Anwendung heraus ist bewusst deaktiviert. Der neue Code würde nur in die Container-Schicht geschrieben und beim nächsten Neuerstellen des Containers verloren gehen, während die Datenbank-Migrationen angewendet blieben — alter Code auf neuem Schema. Aktualisieren Sie stattdessen, indem Sie den Container auf das neue Image umstellen: \"docker compose pull && docker compose up -d\"." } diff --git a/locale/en_US.json b/locale/en_US.json index 39f5b1505..bc9d7a264 100644 --- a/locale/en_US.json +++ b/locale/en_US.json @@ -7525,5 +7525,6 @@ "storage/tmp non utilizzabile": "storage/tmp unusable", "Avviso spazio": "Space warning", "Aggiornamenti non disponibili": "Updates unavailable", - "Il sistema di aggiornamento non può essere avviato perché una condizione preliminare non è soddisfatta. Correggi quanto indicato qui sotto e ricarica la pagina.": "The update system cannot start because a precondition is not met. Fix what is listed below and reload the page." + "Il sistema di aggiornamento non può essere avviato perché una condizione preliminare non è soddisfatta. Correggi quanto indicato qui sotto e ricarica la pagina.": "The update system cannot start because a precondition is not met. Fix what is listed below and reload the page.", + "Questa è l'immagine Docker ufficiale di Pinakes: l'aggiornamento dall'applicazione è disattivato di proposito. Il codice verrebbe riscritto solo nel layer del container e andrebbe perso alla prima ricreazione, mentre le migrazioni del database resterebbero applicate — lasciando codice vecchio su uno schema nuovo. Aggiorna spostando il container sulla nuova immagine: \"docker compose pull && docker compose up -d\".": "This is the official Pinakes Docker image: updating from inside the application is disabled on purpose. The new code would be written only into the container layer and lost the next time the container is recreated, while the database migrations would stay applied — leaving old code on a new schema. Update by moving the container to the new image instead: \"docker compose pull && docker compose up -d\"." } diff --git a/locale/fr_FR.json b/locale/fr_FR.json index c43df384f..459443b35 100644 --- a/locale/fr_FR.json +++ b/locale/fr_FR.json @@ -7525,5 +7525,6 @@ "storage/tmp non utilizzabile": "storage/tmp inutilisable", "Avviso spazio": "Avertissement d'espace", "Aggiornamenti non disponibili": "Mises à jour indisponibles", - "Il sistema di aggiornamento non può essere avviato perché una condizione preliminare non è soddisfatta. Correggi quanto indicato qui sotto e ricarica la pagina.": "Le système de mise à jour ne peut pas démarrer car une condition préalable n'est pas remplie. Corrigez ce qui est indiqué ci-dessous et rechargez la page." + "Il sistema di aggiornamento non può essere avviato perché una condizione preliminare non è soddisfatta. Correggi quanto indicato qui sotto e ricarica la pagina.": "Le système de mise à jour ne peut pas démarrer car une condition préalable n'est pas remplie. Corrigez ce qui est indiqué ci-dessous et rechargez la page.", + "Questa è l'immagine Docker ufficiale di Pinakes: l'aggiornamento dall'applicazione è disattivato di proposito. Il codice verrebbe riscritto solo nel layer del container e andrebbe perso alla prima ricreazione, mentre le migrazioni del database resterebbero applicate — lasciando codice vecchio su uno schema nuovo. Aggiorna spostando il container sulla nuova immagine: \"docker compose pull && docker compose up -d\".": "Ceci est l'image Docker officielle de Pinakes : la mise à jour depuis l'application est désactivée à dessein. Le nouveau code ne serait écrit que dans la couche du conteneur et serait perdu à la prochaine recréation, tandis que les migrations de la base de données resteraient appliquées — laissant du code ancien sur un schéma nouveau. Mettez plutôt à jour en basculant le conteneur sur la nouvelle image : \"docker compose pull && docker compose up -d\"." } diff --git a/locale/it_IT.json b/locale/it_IT.json index 798e7ff80..c069f70b5 100644 --- a/locale/it_IT.json +++ b/locale/it_IT.json @@ -7525,5 +7525,6 @@ "storage/tmp non utilizzabile": "storage/tmp non utilizzabile", "Avviso spazio": "Avviso spazio", "Aggiornamenti non disponibili": "Aggiornamenti non disponibili", - "Il sistema di aggiornamento non può essere avviato perché una condizione preliminare non è soddisfatta. Correggi quanto indicato qui sotto e ricarica la pagina.": "Il sistema di aggiornamento non può essere avviato perché una condizione preliminare non è soddisfatta. Correggi quanto indicato qui sotto e ricarica la pagina." + "Il sistema di aggiornamento non può essere avviato perché una condizione preliminare non è soddisfatta. Correggi quanto indicato qui sotto e ricarica la pagina.": "Il sistema di aggiornamento non può essere avviato perché una condizione preliminare non è soddisfatta. Correggi quanto indicato qui sotto e ricarica la pagina.", + "Questa è l'immagine Docker ufficiale di Pinakes: l'aggiornamento dall'applicazione è disattivato di proposito. Il codice verrebbe riscritto solo nel layer del container e andrebbe perso alla prima ricreazione, mentre le migrazioni del database resterebbero applicate — lasciando codice vecchio su uno schema nuovo. Aggiorna spostando il container sulla nuova immagine: \"docker compose pull && docker compose up -d\".": "Questa è l'immagine Docker ufficiale di Pinakes: l'aggiornamento dall'applicazione è disattivato di proposito. Il codice verrebbe riscritto solo nel layer del container e andrebbe perso alla prima ricreazione, mentre le migrazioni del database resterebbero applicate — lasciando codice vecchio su uno schema nuovo. Aggiorna spostando il container sulla nuova immagine: \"docker compose pull && docker compose up -d\"." } diff --git a/tests/update-space-preflight.unit.php b/tests/update-space-preflight.unit.php index f66301dba..680b05bbb 100644 --- a/tests/update-space-preflight.unit.php +++ b/tests/update-space-preflight.unit.php @@ -392,5 +392,66 @@ static function (string $l): bool { } $check(count(glob($root . '/storage/tmp/.space_probe_*') ?: []) === 0, 'the probe file is removed after the check'); +echo "H. the official Docker image refuses the in-app update\n"; + +// The upgrade path on that image is to move the container to the new image. An +// in-app update there would write the code into the container layer, which is +// discarded on the next recreate, while the schema migrations land in the +// database volume and persist — old code on a new schema, silently. +$runtimeSrc = (string) file_get_contents($root . '/app/Support/ContainerRuntime.php'); +$check(str_contains($runtimeSrc, 'public static function officialImage(): bool'), + 'ContainerRuntime exposes the official-image marker'); +$check(preg_match('/function officialImage\(\): bool\s*\{\s*return is_file\(self::OFFICIAL_IMAGE_MARKER\);/', $runtimeSrc) === 1, + 'officialImage() keys on the marker alone'); + +// The distinction is the whole point: detected() is true for ANY container, +// including community images with a writable code volume where an in-app update +// is legitimate. Blocking on container-ness would refuse someone else's setup. +$blockBody = $bodyOf($src, 'private function officialImageUpdateBlock(): ?string'); +$check($blockBody !== '' && str_contains($blockBody, 'ContainerRuntime::officialImage()'), + 'the refusal keys on the official image'); +$check($blockBody !== '' && !str_contains($blockBody, 'ContainerRuntime::detected()'), + 'the refusal does NOT key on container-ness (community images stay free to update)'); + +// Both entry points, and before the lock: no reason to put the site into +// maintenance for an update that is refused outright. +foreach ([ + 'public function performUpdate(string $targetVersion): array', + 'public function performUpdateFromFile(string $uploadTempPath): array', +] as $signature) { + $body = $bodyOf($src, $signature); + $blockAt = strpos($body, 'officialImageUpdateBlock('); + $lockAt = strpos($body, 'flock('); + $maintAt = strpos($body, 'enableMaintenanceMode('); + $label = str_contains($signature, 'FromFile') ? 'performUpdateFromFile' : 'performUpdate'; + $check($blockAt !== false && $lockAt !== false && $maintAt !== false + && $blockAt < $lockAt && $blockAt < $maintAt, + "{$label}: the image refusal precedes the lock and maintenance mode"); +} + +// On a normal install the marker is absent, so nothing is blocked. +$check($call($updater, 'officialImageUpdateBlock') === null, + 'a non-container install is never refused by the image check'); + +// The message ships in every locale: an operator hitting this needs it in their +// own language, and it is the only instruction they get. +$blockKey = "Questa \u{e8} l'immagine Docker ufficiale di Pinakes"; +$missing = []; +foreach (['it_IT', 'en_US', 'de_DE', 'fr_FR', 'da_DK'] as $loc) { + $data = json_decode((string) file_get_contents($root . "/locale/{$loc}.json"), true); + $found = false; + foreach ((array) $data as $k => $v) { + if (is_string($k) && str_starts_with($k, $blockKey) && is_string($v) && trim($v) !== '') { + $found = true; + break; + } + } + if (!$found) { + $missing[] = $loc; + } +} +$check($missing === [], 'the refusal message is translated in all five locales' + . ($missing === [] ? '' : ' — missing in ' . implode(', ', $missing))); + echo PHP_EOL . "Passed: {$passed} Failed: {$failed}" . PHP_EOL; exit($failed === 0 ? 0 : 1); From efc78c1a5250bb4553c9ade084b404601d1bda38 Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Thu, 10 Sep 2026 23:19:39 +0200 Subject: [PATCH 07/10] fix(updater): verify cumulative capacity on actual write destinations --- app/Controllers/UpdateController.php | 45 +++- app/Support/BackupManager.php | 2 +- app/Support/Updater.php | 255 ++++++++++++++--------- locale/da_DK.json | 3 +- locale/de_DE.json | 3 +- locale/en_US.json | 3 +- locale/fr_FR.json | 3 +- locale/it_IT.json | 3 +- scripts/manual-upgrade.php | 163 +++++++++------ tests/manual-upgrade-space.unit.php | 66 ++++++ tests/update-api-prerequisites.unit.php | 52 +++++ tests/update-space-destinations.unit.php | 124 +++++++++++ tests/update-space-preflight.unit.php | 23 +- 13 files changed, 560 insertions(+), 185 deletions(-) create mode 100644 tests/manual-upgrade-space.unit.php create mode 100644 tests/update-api-prerequisites.unit.php create mode 100644 tests/update-space-destinations.unit.php diff --git a/app/Controllers/UpdateController.php b/app/Controllers/UpdateController.php index 4cdea5f77..a2130aee9 100644 --- a/app/Controllers/UpdateController.php +++ b/app/Controllers/UpdateController.php @@ -92,7 +92,14 @@ public function checkUpdates(Request $request, Response $response, mysqli $db): // Admin-only access check removed - $updater = new Updater($db); + try { + $updater = new Updater($db); + } catch (\Throwable $e) { + return $this->jsonResponse($response, [ + 'success' => false, + 'error' => $e->getMessage(), + ], 503); + } $updateInfo = $updater->checkForUpdates(); return $this->jsonResponse($response, $updateInfo); @@ -123,7 +130,14 @@ public function performUpdate(Request $request, Response $response, mysqli $db): return $this->jsonResponse($response, ['error' => __('Versione non specificata')], 400); } - $updater = new Updater($db); + try { + $updater = new Updater($db); + } catch (\Throwable $e) { + return $this->jsonResponse($response, [ + 'success' => false, + 'error' => $e->getMessage(), + ], 503); + } // Check requirements first $requirements = $updater->checkRequirements(); @@ -196,7 +210,14 @@ public function getHistory(Request $request, Response $response, mysqli $db): Re // Admin-only access check removed - $updater = new Updater($db); + try { + $updater = new Updater($db); + } catch (\Throwable $e) { + return $this->jsonResponse($response, [ + 'success' => false, + 'error' => $e->getMessage(), + ], 503); + } $history = $updater->getUpdateHistory(); return $this->jsonResponse($response, ['history' => $history]); @@ -213,7 +234,14 @@ public function checkAvailable(Request $request, Response $response, mysqli $db) return $this->jsonResponse($response, ['available' => false]); } - $updater = new Updater($db); + try { + $updater = new Updater($db); + } catch (\Throwable $e) { + return $this->jsonResponse($response, [ + 'success' => false, + 'error' => $e->getMessage(), + ], 503); + } $updateInfo = $updater->checkForUpdates(); return $this->jsonResponse($response, [ @@ -654,7 +682,14 @@ public function installManualUpdate(Request $request, Response $response, mysqli ], 400); } - $updater = new Updater($db); + try { + $updater = new Updater($db); + } catch (\Throwable $e) { + return $this->jsonResponse($response, [ + 'success' => false, + 'error' => $e->getMessage(), + ], 503); + } // Check requirements first $requirements = $updater->checkRequirements(); diff --git a/app/Support/BackupManager.php b/app/Support/BackupManager.php index 8c7e9c453..3d8391805 100644 --- a/app/Support/BackupManager.php +++ b/app/Support/BackupManager.php @@ -107,7 +107,7 @@ private function outOfSpace(string $dir): bool { $bytes = 1024 * 1024; $free = @disk_free_space($dir); - if (is_float($free) && $free > 0 && $free < $bytes) { + if (is_float($free) && $free < $bytes) { return true; } $probe = $dir . '/.backup_probe_' . bin2hex(random_bytes(4)); diff --git a/app/Support/Updater.php b/app/Support/Updater.php index 52adcd08b..3f67b6609 100644 --- a/app/Support/Updater.php +++ b/app/Support/Updater.php @@ -91,7 +91,7 @@ class Updater * bytes asked for. describeWriteFailure() is called once per failing file — * inside a bundled-plugin loop that does not re-throw — and must not write a * probe onto the very disk it is diagnosing twenty times over. - * @var array + * @var array */ private array $probeVerdicts = []; @@ -1566,7 +1566,7 @@ public function downloadUpdate(string $version): array } } - $extractSpaceError = $this->extractionSpaceError($zip); + $extractSpaceError = $this->extractionSpaceError($zip, $extractPath); if ($extractSpaceError !== null) { $zip->close(); throw new Exception($extractSpaceError); @@ -1630,6 +1630,11 @@ public function downloadUpdate(string $version): array throw new Exception(__('Impossibile riaprire il file ZIP')); } + $extractSpaceError = $this->extractionSpaceError($zip, $extractPath); + if ($extractSpaceError !== null) { + $zip->close(); + throw new Exception($extractSpaceError); + } $extractionSuccess = $zip->extractTo($extractPath); } @@ -1986,7 +1991,7 @@ public function performUpdateFromFile(string $uploadTempPath): array // before the backup and the extraction. $uploadedZip = $uploadTempPath . '/update.zip'; $uploadedBytes = is_file($uploadedZip) ? (int) filesize($uploadedZip) : 0; - $spaceError = $this->checkFreeSpaceForUpdate($uploadedBytes > 0 ? $uploadedBytes * 4 : 0); + $spaceError = $this->checkFreeSpaceForUpdate($uploadedBytes > 0 ? $uploadedBytes * 4 : 0, null, true); if ($spaceError !== null) { $this->debugLog('ERROR', 'Preflight iniziale: spazio insufficiente', ['detail' => $spaceError]); throw new Exception($spaceError); @@ -2034,7 +2039,7 @@ public function performUpdateFromFile(string $uploadTempPath): array } } - $extractSpaceError = $this->extractionSpaceError($zip); + $extractSpaceError = $this->extractionSpaceError($zip, $extractPath); if ($extractSpaceError !== null) { $zip->close(); throw new Exception($extractSpaceError); @@ -2468,7 +2473,7 @@ public function installUpdate(string $sourcePath, string $targetVersion): array // error naming a file in vendor/ that says nothing about the real // cause, which is exactly how a healthy release once looked broken. // Refuse here, with the numbers, instead of failing halfway. - $spaceError = $this->checkFreeSpaceForUpdate(); + $spaceError = $this->checkFreeSpaceForUpdate(0, $sourcePath); if ($spaceError !== null) { $this->debugLog('ERROR', 'Preflight: spazio insufficiente', ['detail' => $spaceError]); throw new Exception($spaceError); @@ -2681,91 +2686,161 @@ private function directorySize(string $path): int * top of the rollback copy (the release ZIP and its extracted tree, * the pre-update backup archive). 0 when there is nothing to add. */ - private function checkFreeSpaceForUpdate(int $extraBytes = 0): ?string + private function checkFreeSpaceForUpdate(int $extraBytes = 0, ?string $sourcePath = null, bool $beforeBackup = false): ?string { - $needed = $this->estimateUpdateSpace(); - if ($needed <= 0) { - return null; // nothing measurable to copy: let the update proceed - } - if ($extraBytes > 0) { - $needed += $extraBytes; - } - - $free = @disk_free_space($this->rootPath); - if (is_float($free) && $free > 0 && $free < $needed) { - return sprintf( - __('Spazio su disco insufficiente per aggiornare: servono circa %1$s, disponibili %2$s. Libera spazio (i backup automatici in storage/backups sono i primi candidati) e riprova.'), - $this->formatBytes($needed), - $this->formatBytes((int) $free) - ); - } + $requirements = $this->installationSpaceRequirements($sourcePath); + $tmp = $this->rootPath . '/storage/tmp'; + $requirements[$tmp] = ($requirements[$tmp] ?? 0) + $this->estimateUpdateSpace() + max(0, $extraBytes); + if ($beforeBackup) { + // The SQL dump and its ZIP coexist. Full backups also include uploads. + $dumpBytes = 0; + $result = $this->db->query("SELECT COALESCE(SUM(data_length + index_length), 0) AS bytes + FROM information_schema.tables WHERE table_schema = DATABASE()"); + if ($result instanceof \mysqli_result) { + $row = $result->fetch_assoc(); + $dumpBytes = (int) ceil((float) ($row['bytes'] ?? 0) * 1.5); + $result->free(); + } + $archiveBytes = max(1024 * 1024, $dumpBytes); + if ((new SettingsRepository($this->db))->get('backup', 'pre_update_include_files', '0') === '1') { + $archiveBytes += $this->directorySize($this->rootPath . '/public/uploads') + + $this->directorySize($this->rootPath . '/storage/uploads/plugins'); + } + $backup = $this->rootPath . '/storage/backups'; + $requirements[$backup] = ($requirements[$backup] ?? 0) + (int) ceil($archiveBytes * 1.1); + $systemTmp = sys_get_temp_dir(); + $requirements[$systemTmp] = ($requirements[$systemTmp] ?? 0) + max(1024 * 1024, $dumpBytes); + } + return $this->checkSpaceRequirements($requirements); + } - // Fresh probe: this gate is called again after the backup and the - // download have consumed disk, and a memoised verdict from before them - // would answer for a state that no longer exists. - $verdict = $this->probeWrite($needed, true); - if ($verdict === 'unavailable') { - // NOT a space problem: the working directory itself is missing or - // read-only. Saying "free up space" here would reproduce the exact - // misdiagnosis this preflight was added to remove. - return sprintf( - __('Impossibile usare la directory di lavoro dell\'aggiornamento (%s): non esiste o non è scrivibile dall\'utente del web server. Correggi i permessi e riprova.'), - $this->rootPath . '/storage/tmp' - ); + /** + * Budget writes by destination directory, including nested mounts. Before the + * package is known, walk the installed application; at install time use the + * actual incoming tree. Budgeting complete files is deliberately conservative. + * @return array + */ + private function installationSpaceRequirements(?string $sourcePath): array + { + $requirements = [$this->rootPath => 4096, $this->rootPath . '/storage/backups' => 4096]; + $trees = $sourcePath !== null ? ['' => $sourcePath] : []; + if ($sourcePath === null) { + foreach (self::APP_BACKUP_DIRS as $dir) { + $trees[$dir] = $this->rootPath . '/' . $dir; + } } - if ($verdict === 'nospace') { - return sprintf( - __('Impossibile riservare i %1$s richiesti dall\'aggiornamento: lo spazio disponibile o la quota dell\'account sono esauriti. Libera spazio (i backup automatici in storage/backups sono i primi candidati) e riprova.'), - $this->formatBytes($needed) + foreach ($trees as $prefix => $tree) { + if (!is_dir($tree)) { + continue; + } + $iterator = new \RecursiveIteratorIterator( + new \RecursiveDirectoryIterator($tree, \FilesystemIterator::SKIP_DOTS), + \RecursiveIteratorIterator::SELF_FIRST ); + foreach ($iterator as $item) { + if ($item->isLink()) { + continue; + } + $relative = ($prefix === '' ? '' : $prefix . '/') + . substr(str_replace('\\', '/', $item->getPathname()), strlen($tree) + 1); + $target = $this->rootPath . '/' . $relative; + foreach ($this->skipPaths as $skip) { + if (str_starts_with($relative, $skip)) { + continue 2; + } + } + foreach ($this->preservePaths as $preserve) { + // Bundled plugin files are installed by updateBundledPlugins(). + if ($preserve !== 'storage/plugins' && str_starts_with($relative, $preserve) && file_exists($target)) { + continue 2; + } + } + if ($this->isCustomLocalePath($relative) && file_exists($target)) { + continue; + } + $dir = $item->isDir() ? $target : dirname($target); + $bytes = $item->isDir() ? 4096 : max(4096, (int) ceil($item->getSize() / 4096) * 4096); + $requirements[$dir] = ($requirements[$dir] ?? 0) + $bytes; + } } + return $requirements; + } - $this->debugLog('INFO', 'Preflight spazio superato', [ - 'needed_bytes' => $needed, - 'extra_bytes' => $extraBytes, - 'disk_free_bytes' => is_float($free) ? (int) $free : null, - ]); + /** + * Aggregate simultaneous writes on each filesystem before probing. Missing + * directories use their nearest existing ancestor, where mkdir will allocate. + * @param array $requirements + */ + private function checkSpaceRequirements(array $requirements): ?string + { + $volumes = []; + foreach ($requirements as $path => $bytes) { + $dir = $path; + while (!is_dir($dir) && !file_exists($dir) && dirname($dir) !== $dir) { + $dir = dirname($dir); + } + $stat = @stat($dir); + if (!is_dir($dir) || !is_writable($dir) || $stat === false) { + return sprintf( + __('Impossibile usare la directory di lavoro dell\'aggiornamento (%s): non esiste o non è scrivibile dall\'utente del web server. Correggi i permessi e riprova.'), $path + ); + } + $key = (string) $stat['dev']; + if (!isset($volumes[$key])) { + $volumes[$key] = ['path' => $dir, 'bytes' => 0]; + } + $volumes[$key]['bytes'] += max(0, $bytes); + } + // A previous panel/diagnostic probe cannot establish current capacity. + $this->probeVerdicts = []; + foreach ($volumes as $volume) { + $dir = $volume['path']; + $needed = $volume['bytes']; + $free = @disk_free_space($dir); + if (is_float($free) && $free < $needed) { + return sprintf( + __('Spazio su disco insufficiente per aggiornare: servono circa %1$s, disponibili %2$s. Libera spazio (i backup automatici in storage/backups sono i primi candidati) e riprova.'), + $this->formatBytes($needed), $this->formatBytes($free) + ) . ' (' . $dir . ')'; + } + $verdict = $this->runWriteProbe($needed, $dir); + if ($verdict === 'unknown') { + return sprintf(__('Impossibile verificare lo spazio disponibile in %s. Controlla la configurazione del filesystem e riprova.'), $dir); + } + if ($verdict === 'unavailable') { + return sprintf( + __('Impossibile usare la directory di lavoro dell\'aggiornamento (%s): non esiste o non è scrivibile dall\'utente del web server. Correggi i permessi e riprova.'), $dir + ); + } + if ($verdict === 'nospace') { + return sprintf( + __('Impossibile riservare i %1$s richiesti dall\'aggiornamento: lo spazio disponibile o la quota dell\'account sono esauriti. Libera spazio (i backup automatici in storage/backups sono i primi candidati) e riprova.'), + $this->formatBytes($needed) + ) . ' (' . $dir . ')'; + } + } return null; } /** - * Can this account still write $bytes under storage/tmp, and if not, why. - * - * Returns '' when it can, 'nospace' when the write itself ran out of room - * (a full filesystem or an exhausted account quota), and 'unavailable' when - * the probe could never be created at all (storage/tmp missing or not - * writable). Those are three different failures and must not share a - * message — collapsing them into a bool is what let a permission problem be - * reported as an exhausted quota. - * - * Bounded BY CONSTRUCTION, not by the discipline of its callers: - * - a request the filesystem provably cannot hold is refused from - * disk_free_space() without writing a byte, so asking for more than the - * volume holds answers in microseconds instead of filling it; - * - above SPACE_PROBE_BYTES the range is allocated by touching one byte per - * block instead of streaming every byte — the same blocks are consumed - * (a hole consumes no quota, a touched block does) at a fraction of the - * I/O, so the full requirement can be proven without risking a timeout; - * - when free space cannot be read at all, the write falls back to the - * dense bounded form: an unreadable disk_free_space() must never turn - * into a refusal, it is common on locked-down shared hosting. - * - * @param bool $fresh Re-measure instead of answering from the memo. - * @return string '' | 'nospace' | 'unavailable' + * Prove a complete write, or report nospace/unavailable/unknown. Large probes + * require a readable filesystem bound; a small sample never proves a larger + * requirement. Cached diagnostics are scoped to both path and byte count. */ - private function probeWrite(int $bytes, bool $fresh = false): string + private function probeWrite(int $bytes, bool $fresh = false, ?string $directory = null): string { if ($bytes <= 0) { return ''; } + $key = ($directory ?? $this->rootPath . '/storage/tmp') . ':' . $bytes; if ($fresh) { $this->probeVerdicts = []; - } elseif (isset($this->probeVerdicts[$bytes])) { - return $this->probeVerdicts[$bytes]; + } elseif (isset($this->probeVerdicts[$key])) { + return $this->probeVerdicts[$key]; } - $verdict = $this->runWriteProbe($bytes); - $this->probeVerdicts[$bytes] = $verdict; + $verdict = $this->runWriteProbe($bytes, $directory); + $this->probeVerdicts[$key] = $verdict; return $verdict; } @@ -2773,11 +2848,11 @@ private function probeWrite(int $bytes, bool $fresh = false): string * The probe itself. Sparse allocation would not do: a hole consumes no * quota, and the quota is precisely what this is testing. * - * @return string '' | 'nospace' | 'unavailable' + * @return string '' | 'nospace' | 'unavailable' | 'unknown' */ - private function runWriteProbe(int $bytes): string + private function runWriteProbe(int $bytes, ?string $directory = null): string { - $dir = $this->rootPath . '/storage/tmp'; + $dir = $directory ?? $this->rootPath . '/storage/tmp'; if (!is_dir($dir) && !@mkdir($dir, 0775, true) && !is_dir($dir)) { return 'unavailable'; } @@ -2786,15 +2861,14 @@ private function runWriteProbe(int $bytes): string // there is nothing to prove by writing, and trying would fill the // volume before it could answer. $free = @disk_free_space($dir); - if (is_float($free) && $free > 0 && $free < $bytes) { + if (is_float($free) && $free < $bytes) { return 'nospace'; } - // Free space unreadable (open_basedir, disabled function): fall back to - // the bounded dense write rather than refusing a legitimate update. + // Without a filesystem bound, do not attempt an arbitrarily large write + // or report a small sample as proof of the whole requirement. $strided = $bytes > self::SPACE_PROBE_BYTES; if ($strided && !is_float($free)) { - $bytes = self::SPACE_PROBE_BYTES; - $strided = false; + return 'unknown'; } $probe = $dir . '/.space_probe_' . bin2hex(random_bytes(4)); @@ -2887,7 +2961,7 @@ private function officialImageUpdateBlock(): ?string * * @return string|null the message to show, or null when it fits */ - private function extractionSpaceError(ZipArchive $zip): ?string + private function extractionSpaceError(ZipArchive $zip, ?string $destination = null): ?string { $uncompressed = 0; for ($i = 0; $i < $zip->numFiles; $i++) { @@ -2902,20 +2976,9 @@ private function extractionSpaceError(ZipArchive $zip): ?string return null; // nothing measurable: let the extraction proceed } - $verdict = $this->probeWrite($uncompressed, true); - if ($verdict === 'unavailable') { - return sprintf( - __('Impossibile usare la directory di lavoro dell\'aggiornamento (%s): non esiste o non è scrivibile dall\'utente del web server. Correggi i permessi e riprova.'), - $this->rootPath . '/storage/tmp' - ); - } - if ($verdict === 'nospace') { - return sprintf( - __('Impossibile riservare i %1$s richiesti dall\'aggiornamento: lo spazio disponibile o la quota dell\'account sono esauriti. Libera spazio (i backup automatici in storage/backups sono i primi candidati) e riprova.'), - $this->formatBytes($uncompressed) - ); - } - return null; + return $this->checkSpaceRequirements([ + $destination ?? $this->rootPath . '/storage/tmp' => $uncompressed, + ]); } private function describeWriteFailure(string $targetPath): string @@ -2938,7 +3001,7 @@ private function describeWriteFailure(string $targetPath): string // Only a probe that actually ran out of room may claim a space problem; // 'unavailable' means the probe could not be created, which says // nothing about the failure being described. - if ($this->probeWrite(1024 * 1024) === 'nospace') { + if ($this->probeWrite(1024 * 1024, false, $dir) === 'nospace') { return __('spazio su disco o quota dell\'account esauriti'); } @@ -4759,7 +4822,7 @@ public function performUpdate(string $targetVersion): array // once these steps have consumed their share, which is the only // measurement valid there. $packageBytes = $this->releaseAssetBytes($targetVersion); - $spaceError = $this->checkFreeSpaceForUpdate($packageBytes > 0 ? $packageBytes * 4 : 0); + $spaceError = $this->checkFreeSpaceForUpdate($packageBytes > 0 ? $packageBytes * 4 : 0, null, true); if ($spaceError !== null) { $this->debugLog('ERROR', 'Preflight iniziale: spazio insufficiente', ['detail' => $spaceError]); throw new Exception($spaceError); diff --git a/locale/da_DK.json b/locale/da_DK.json index ba6ed0cae..c476ec440 100644 --- a/locale/da_DK.json +++ b/locale/da_DK.json @@ -7526,5 +7526,6 @@ "Avviso spazio": "Pladsadvarsel", "Aggiornamenti non disponibili": "Opdateringer utilgængelige", "Il sistema di aggiornamento non può essere avviato perché una condizione preliminare non è soddisfatta. Correggi quanto indicato qui sotto e ricarica la pagina.": "Opdateringssystemet kan ikke startes, fordi en forudsætning ikke er opfyldt. Ret det, der er anført nedenfor, og genindlæs siden.", - "Questa è l'immagine Docker ufficiale di Pinakes: l'aggiornamento dall'applicazione è disattivato di proposito. Il codice verrebbe riscritto solo nel layer del container e andrebbe perso alla prima ricreazione, mentre le migrazioni del database resterebbero applicate — lasciando codice vecchio su uno schema nuovo. Aggiorna spostando il container sulla nuova immagine: \"docker compose pull && docker compose up -d\".": "Dette er det officielle Pinakes-Docker-image: opdatering inde fra applikationen er deaktiveret med vilje. Den nye kode ville kun blive skrevet til containerlaget og gå tabt, næste gang containeren genskabes, mens databasemigreringerne ville forblive anvendt — hvilket efterlader gammel kode på et nyt skema. Opdater i stedet ved at flytte containeren til det nye image: \"docker compose pull && docker compose up -d\"." + "Questa è l'immagine Docker ufficiale di Pinakes: l'aggiornamento dall'applicazione è disattivato di proposito. Il codice verrebbe riscritto solo nel layer del container e andrebbe perso alla prima ricreazione, mentre le migrazioni del database resterebbero applicate — lasciando codice vecchio su uno schema nuovo. Aggiorna spostando il container sulla nuova immagine: \"docker compose pull && docker compose up -d\".": "Dette er det officielle Pinakes-Docker-image: opdatering inde fra applikationen er deaktiveret med vilje. Den nye kode ville kun blive skrevet til containerlaget og gå tabt, næste gang containeren genskabes, mens databasemigreringerne ville forblive anvendt — hvilket efterlader gammel kode på et nyt skema. Opdater i stedet ved at flytte containeren til det nye image: \"docker compose pull && docker compose up -d\".", + "Impossibile verificare lo spazio disponibile in %s. Controlla la configurazione del filesystem e riprova.": "Den ledige plads i %s kunne ikke kontrolleres. Kontrollér filsystemets konfiguration, og prøv igen." } diff --git a/locale/de_DE.json b/locale/de_DE.json index ababd8b75..26ff7f6fa 100644 --- a/locale/de_DE.json +++ b/locale/de_DE.json @@ -7526,5 +7526,6 @@ "Avviso spazio": "Speicherplatz-Warnung", "Aggiornamenti non disponibili": "Updates nicht verfügbar", "Il sistema di aggiornamento non può essere avviato perché una condizione preliminare non è soddisfatta. Correggi quanto indicato qui sotto e ricarica la pagina.": "Das Update-System kann nicht gestartet werden, weil eine Voraussetzung nicht erfüllt ist. Beheben Sie den unten genannten Punkt und laden Sie die Seite neu.", - "Questa è l'immagine Docker ufficiale di Pinakes: l'aggiornamento dall'applicazione è disattivato di proposito. Il codice verrebbe riscritto solo nel layer del container e andrebbe perso alla prima ricreazione, mentre le migrazioni del database resterebbero applicate — lasciando codice vecchio su uno schema nuovo. Aggiorna spostando il container sulla nuova immagine: \"docker compose pull && docker compose up -d\".": "Dies ist das offizielle Pinakes-Docker-Image: Die Aktualisierung aus der Anwendung heraus ist bewusst deaktiviert. Der neue Code würde nur in die Container-Schicht geschrieben und beim nächsten Neuerstellen des Containers verloren gehen, während die Datenbank-Migrationen angewendet blieben — alter Code auf neuem Schema. Aktualisieren Sie stattdessen, indem Sie den Container auf das neue Image umstellen: \"docker compose pull && docker compose up -d\"." + "Questa è l'immagine Docker ufficiale di Pinakes: l'aggiornamento dall'applicazione è disattivato di proposito. Il codice verrebbe riscritto solo nel layer del container e andrebbe perso alla prima ricreazione, mentre le migrazioni del database resterebbero applicate — lasciando codice vecchio su uno schema nuovo. Aggiorna spostando il container sulla nuova immagine: \"docker compose pull && docker compose up -d\".": "Dies ist das offizielle Pinakes-Docker-Image: Die Aktualisierung aus der Anwendung heraus ist bewusst deaktiviert. Der neue Code würde nur in die Container-Schicht geschrieben und beim nächsten Neuerstellen des Containers verloren gehen, während die Datenbank-Migrationen angewendet blieben — alter Code auf neuem Schema. Aktualisieren Sie stattdessen, indem Sie den Container auf das neue Image umstellen: \"docker compose pull && docker compose up -d\".", + "Impossibile verificare lo spazio disponibile in %s. Controlla la configurazione del filesystem e riprova.": "Der verfügbare Speicherplatz in %s kann nicht überprüft werden. Prüfen Sie die Dateisystemkonfiguration und versuchen Sie es erneut." } diff --git a/locale/en_US.json b/locale/en_US.json index bc9d7a264..767f443e2 100644 --- a/locale/en_US.json +++ b/locale/en_US.json @@ -7526,5 +7526,6 @@ "Avviso spazio": "Space warning", "Aggiornamenti non disponibili": "Updates unavailable", "Il sistema di aggiornamento non può essere avviato perché una condizione preliminare non è soddisfatta. Correggi quanto indicato qui sotto e ricarica la pagina.": "The update system cannot start because a precondition is not met. Fix what is listed below and reload the page.", - "Questa è l'immagine Docker ufficiale di Pinakes: l'aggiornamento dall'applicazione è disattivato di proposito. Il codice verrebbe riscritto solo nel layer del container e andrebbe perso alla prima ricreazione, mentre le migrazioni del database resterebbero applicate — lasciando codice vecchio su uno schema nuovo. Aggiorna spostando il container sulla nuova immagine: \"docker compose pull && docker compose up -d\".": "This is the official Pinakes Docker image: updating from inside the application is disabled on purpose. The new code would be written only into the container layer and lost the next time the container is recreated, while the database migrations would stay applied — leaving old code on a new schema. Update by moving the container to the new image instead: \"docker compose pull && docker compose up -d\"." + "Questa è l'immagine Docker ufficiale di Pinakes: l'aggiornamento dall'applicazione è disattivato di proposito. Il codice verrebbe riscritto solo nel layer del container e andrebbe perso alla prima ricreazione, mentre le migrazioni del database resterebbero applicate — lasciando codice vecchio su uno schema nuovo. Aggiorna spostando il container sulla nuova immagine: \"docker compose pull && docker compose up -d\".": "This is the official Pinakes Docker image: updating from inside the application is disabled on purpose. The new code would be written only into the container layer and lost the next time the container is recreated, while the database migrations would stay applied — leaving old code on a new schema. Update by moving the container to the new image instead: \"docker compose pull && docker compose up -d\".", + "Impossibile verificare lo spazio disponibile in %s. Controlla la configurazione del filesystem e riprova.": "Unable to verify available space in %s. Check the filesystem configuration and try again." } diff --git a/locale/fr_FR.json b/locale/fr_FR.json index 459443b35..cc50d7522 100644 --- a/locale/fr_FR.json +++ b/locale/fr_FR.json @@ -7526,5 +7526,6 @@ "Avviso spazio": "Avertissement d'espace", "Aggiornamenti non disponibili": "Mises à jour indisponibles", "Il sistema di aggiornamento non può essere avviato perché una condizione preliminare non è soddisfatta. Correggi quanto indicato qui sotto e ricarica la pagina.": "Le système de mise à jour ne peut pas démarrer car une condition préalable n'est pas remplie. Corrigez ce qui est indiqué ci-dessous et rechargez la page.", - "Questa è l'immagine Docker ufficiale di Pinakes: l'aggiornamento dall'applicazione è disattivato di proposito. Il codice verrebbe riscritto solo nel layer del container e andrebbe perso alla prima ricreazione, mentre le migrazioni del database resterebbero applicate — lasciando codice vecchio su uno schema nuovo. Aggiorna spostando il container sulla nuova immagine: \"docker compose pull && docker compose up -d\".": "Ceci est l'image Docker officielle de Pinakes : la mise à jour depuis l'application est désactivée à dessein. Le nouveau code ne serait écrit que dans la couche du conteneur et serait perdu à la prochaine recréation, tandis que les migrations de la base de données resteraient appliquées — laissant du code ancien sur un schéma nouveau. Mettez plutôt à jour en basculant le conteneur sur la nouvelle image : \"docker compose pull && docker compose up -d\"." + "Questa è l'immagine Docker ufficiale di Pinakes: l'aggiornamento dall'applicazione è disattivato di proposito. Il codice verrebbe riscritto solo nel layer del container e andrebbe perso alla prima ricreazione, mentre le migrazioni del database resterebbero applicate — lasciando codice vecchio su uno schema nuovo. Aggiorna spostando il container sulla nuova immagine: \"docker compose pull && docker compose up -d\".": "Ceci est l'image Docker officielle de Pinakes : la mise à jour depuis l'application est désactivée à dessein. Le nouveau code ne serait écrit que dans la couche du conteneur et serait perdu à la prochaine recréation, tandis que les migrations de la base de données resteraient appliquées — laissant du code ancien sur un schéma nouveau. Mettez plutôt à jour en basculant le conteneur sur la nouvelle image : \"docker compose pull && docker compose up -d\".", + "Impossibile verificare lo spazio disponibile in %s. Controlla la configurazione del filesystem e riprova.": "Impossible de vérifier l’espace disponible dans %s. Vérifiez la configuration du système de fichiers et réessayez." } diff --git a/locale/it_IT.json b/locale/it_IT.json index c069f70b5..092c839e1 100644 --- a/locale/it_IT.json +++ b/locale/it_IT.json @@ -7526,5 +7526,6 @@ "Avviso spazio": "Avviso spazio", "Aggiornamenti non disponibili": "Aggiornamenti non disponibili", "Il sistema di aggiornamento non può essere avviato perché una condizione preliminare non è soddisfatta. Correggi quanto indicato qui sotto e ricarica la pagina.": "Il sistema di aggiornamento non può essere avviato perché una condizione preliminare non è soddisfatta. Correggi quanto indicato qui sotto e ricarica la pagina.", - "Questa è l'immagine Docker ufficiale di Pinakes: l'aggiornamento dall'applicazione è disattivato di proposito. Il codice verrebbe riscritto solo nel layer del container e andrebbe perso alla prima ricreazione, mentre le migrazioni del database resterebbero applicate — lasciando codice vecchio su uno schema nuovo. Aggiorna spostando il container sulla nuova immagine: \"docker compose pull && docker compose up -d\".": "Questa è l'immagine Docker ufficiale di Pinakes: l'aggiornamento dall'applicazione è disattivato di proposito. Il codice verrebbe riscritto solo nel layer del container e andrebbe perso alla prima ricreazione, mentre le migrazioni del database resterebbero applicate — lasciando codice vecchio su uno schema nuovo. Aggiorna spostando il container sulla nuova immagine: \"docker compose pull && docker compose up -d\"." + "Questa è l'immagine Docker ufficiale di Pinakes: l'aggiornamento dall'applicazione è disattivato di proposito. Il codice verrebbe riscritto solo nel layer del container e andrebbe perso alla prima ricreazione, mentre le migrazioni del database resterebbero applicate — lasciando codice vecchio su uno schema nuovo. Aggiorna spostando il container sulla nuova immagine: \"docker compose pull && docker compose up -d\".": "Questa è l'immagine Docker ufficiale di Pinakes: l'aggiornamento dall'applicazione è disattivato di proposito. Il codice verrebbe riscritto solo nel layer del container e andrebbe perso alla prima ricreazione, mentre le migrazioni del database resterebbero applicate — lasciando codice vecchio su uno schema nuovo. Aggiorna spostando il container sulla nuova immagine: \"docker compose pull && docker compose up -d\".", + "Impossibile verificare lo spazio disponibile in %s. Controlla la configurazione del filesystem e riprova.": "Impossibile verificare lo spazio disponibile in %s. Controlla la configurazione del filesystem e riprova." } diff --git a/scripts/manual-upgrade.php b/scripts/manual-upgrade.php index 0be85a1e9..e25f79991 100644 --- a/scripts/manual-upgrade.php +++ b/scripts/manual-upgrade.php @@ -196,18 +196,19 @@ function formatBytes(float $bytes): string * Bounded by construction: a request the filesystem provably cannot hold is * refused without writing anything, and above 16 MB the range is allocated by * touching one byte per 1 KiB block instead of streaming every byte (the same - * blocks are charged, at a thousandth of the I/O). When free space cannot be - * read at all the write falls back to a bounded 16 MB — an unreadable - * disk_free_space() must never become a refusal. + * blocks are charged, at a thousandth of the I/O). If free space is unreadable, + * a large request returns unknown without allocating: neither an unbounded write + * nor a successful 16 MB sample can safely establish the requested capacity. * * Deliberately duplicated from app/Support/Updater.php: this script must keep * working with no autoloader and no application classes, because it is the tool * used when the application itself will not boot. * * @return string '' when the space is there, 'nospace' when the write ran out - * of room, 'unavailable' when the probe could not be created + * of room, 'unavailable' when the probe could not be created, + * 'unknown' when a large request cannot be bounded */ -function probeWriteBytes(string $rootPath, int $bytes): string +function probeWriteBytes(string $rootPath, int $bytes, ?string $directory = null): string { if ($bytes <= 0) { return ''; @@ -215,19 +216,18 @@ function probeWriteBytes(string $rootPath, int $bytes): string $denseLimit = 16 * 1024 * 1024; $stride = 1024; - $dir = rtrim(str_replace('\\', '/', $rootPath), '/') . '/storage/tmp'; + $dir = $directory ?? rtrim(str_replace('\\', '/', $rootPath), '/') . '/storage/tmp'; if (!is_dir($dir) && !@mkdir($dir, 0775, true) && !is_dir($dir)) { return 'unavailable'; } $free = @disk_free_space($dir); - if (is_float($free) && $free > 0 && $free < $bytes) { + if (is_float($free) && $free < $bytes) { return 'nospace'; } $strided = $bytes > $denseLimit; if ($strided && !is_float($free)) { - $bytes = $denseLimit; - $strided = false; + return 'unknown'; } $probe = $dir . '/.upgrade_probe_' . bin2hex(random_bytes(4)); @@ -274,6 +274,71 @@ function probeWriteBytes(string $rootPath, int $bytes): string return $ok ? '' : 'nospace'; } +/** + * Verify simultaneous writes on each destination filesystem. No application + * bootstrap is required: this script also repairs installations that cannot boot. + * @param array $requirements + */ +function verifyUpgradeSpace(string $rootPath, array $requirements): void +{ + $volumes = []; + foreach ($requirements as $path => $bytes) { + $dir = $path; + while (!is_dir($dir) && !file_exists($dir) && dirname($dir) !== $dir) { + $dir = dirname($dir); + } + $stat = @stat($dir); + if (!is_dir($dir) || !is_writable($dir) || $stat === false) { + throw new RuntimeException('Directory di aggiornamento non scrivibile: ' . $path); + } + $key = (string) $stat['dev']; + if (!isset($volumes[$key])) { + $volumes[$key] = ['path' => $dir, 'bytes' => 0]; + } + $volumes[$key]['bytes'] += max(4096, $bytes); + } + foreach ($volumes as $volume) { + $dir = $volume['path']; + $required = $volume['bytes']; + $verdict = probeWriteBytes($rootPath, $required, $dir); + if ($verdict !== '') { + $reason = match ($verdict) { + 'nospace' => "spazio su disco o quota dell'account insufficienti", + 'unknown' => 'spazio disponibile non verificabile: controlla la configurazione del filesystem', + default => 'directory non scrivibile', + }; + throw new RuntimeException('Preflight fallito in ' . $dir . ': ' . $reason + . ' (richiesti ' . formatBytes($required) . ').'); + } + } +} + +/** @return array Space needed for the actual copy destinations. */ +function upgradeCopyRequirements(string $source, string $dest, array $preserve): array +{ + $requirements = [$dest => 4096]; + $iterator = new RecursiveIteratorIterator( + new RecursiveDirectoryIterator($source, FilesystemIterator::SKIP_DOTS), + RecursiveIteratorIterator::SELF_FIRST + ); + foreach ($iterator as $item) { + if ($item->isLink()) { + continue; + } + $relative = substr($item->getPathname(), strlen($source) + 1); + foreach ($preserve as $skip) { + if ($relative === $skip || str_starts_with($relative, $skip . '/')) { + continue 2; + } + } + $target = $dest . '/' . $relative; + $dir = $item->isDir() ? $target : dirname($target); + $bytes = $item->isDir() ? 4096 : max(4096, (int) ceil($item->getSize() / 4096) * 4096); + $requirements[$dir] = ($requirements[$dir] ?? 0) + $bytes; + } + return $requirements; +} + /** * Why a write to this path failed, in words an operator can act on. * @@ -296,7 +361,7 @@ function describeWriteFailure(string $rootPath, string $targetPath): string if (!is_writable($dir)) { return 'la directory di destinazione non è scrivibile'; } - if (probeWriteBytes($rootPath, 1024 * 1024) === 'nospace') { + if (probeWriteBytes($rootPath, 1024 * 1024, $dir) === 'nospace') { return 'spazio su disco o quota dell\'account esauriti'; } @@ -602,25 +667,11 @@ function copyTree(string $src, string $dst, string $rootDst, array $skipRelative throw new RuntimeException('Estensione ZipArchive non disponibile. Contatta il provider hosting.'); } - $freeSpace = @disk_free_space($rootPath); - if ($freeSpace !== false && $freeSpace < MIN_UPGRADE_FREE_BYTES) { - throw new RuntimeException('Spazio disco insufficiente: ' . formatBytes((int)$freeSpace) . ' disponibili, servono almeno ' . formatBytes(MIN_UPGRADE_FREE_BYTES) . '.'); - } - // disk_free_space() alone lies on a cPanel account whose quota is - // exhausted: it reports the FILESYSTEM. Prove with a real write that - // this account can still allocate, here — the last point at which a - // refusal costs the operator nothing, because after it the DB dump and - // the tree overwrite begin and this script has no file rollback. - // Prove the SAME amount the gate above demands: a probe smaller than the - // declared floor lets a quota sitting between the two pass and then fail - // on the first real write. - $probeVerdict = probeWriteBytes($rootPath, MIN_UPGRADE_FREE_BYTES); - if ($probeVerdict === 'unavailable') { - throw new RuntimeException('Impossibile usare storage/tmp: non esiste o non è scrivibile. Correggi i permessi prima di continuare.'); - } - if ($probeVerdict === 'nospace') { - throw new RuntimeException('Impossibile scrivere in storage/tmp: lo spazio disponibile o la quota dell\'account sono esauriti. Libera spazio (i backup automatici in storage/backups sono i primi candidati) e riprova.'); - } + verifyUpgradeSpace($rootPath, [ + $rootPath => 4096, + $rootPath . '/storage/tmp' => MIN_UPGRADE_FREE_BYTES, + $rootPath . '/storage/backups' => 4096, + ]); $writableDirs = ['storage', 'storage/tmp', 'storage/backups', 'app', 'public', 'installer']; foreach ($writableDirs as $dir) { @@ -672,20 +723,19 @@ function copyTree(string $src, string $dst, string $rootDst, array $skipRelative $sizeRow->free(); $dumpEstimate = (int) (((float) ($row['bytes'] ?? 0)) * 1.5); } - if ($dumpEstimate > 0) { - $dumpVerdict = probeWriteBytes($rootPath, $dumpEstimate); - if ($dumpVerdict === 'unavailable') { - throw new RuntimeException('Impossibile usare storage/tmp per verificare lo spazio del backup: non esiste o non è scrivibile. Correggi i permessi prima di continuare.'); - } - if ($dumpVerdict === 'nospace') { - throw new RuntimeException( - 'Spazio insufficiente per il backup del database: servono circa ' - . formatBytes((float) $dumpEstimate) - . '. Libera spazio (i backup automatici in storage/backups sono i primi candidati) e riprova.' - ); + $criticalBytes = 0; + foreach (['.env', 'config.local.php', 'version.json'] as $criticalFile) { + $criticalPath = $rootPath . '/' . $criticalFile; + if (is_file($criticalPath)) { + $criticalBytes += (int) filesize($criticalPath); } - $log[] = '[OK] Spazio per il dump verificato (~' . formatBytes((float) $dumpEstimate) . ')'; } + verifyUpgradeSpace($rootPath, [ + $rootPath => 4096, + $rootPath . '/storage/tmp' => MIN_UPGRADE_FREE_BYTES, + $backupDir => max(0, $dumpEstimate) + $criticalBytes, + ]); + $log[] = '[OK] Spazio cumulativo per dump, backup critici e riserva verificato'; $mysqldumpBin = null; foreach (['/usr/bin/mysqldump', '/usr/local/bin/mysqldump', '/opt/homebrew/bin/mysqldump'] as $candidate) { @@ -775,28 +825,11 @@ function copyTree(string $src, string $dst, string $rootDst, array $skipRelative } } $requiredBytes = $uncompressedBytes + (100 * 1024 * 1024); // 100 MB safety margin - $freeSpaceNow = @disk_free_space($rootPath); - if ($freeSpaceNow !== false && $freeSpaceNow < $requiredBytes) { - $zip->close(); - throw new RuntimeException( - 'Spazio disco insufficiente per estrazione: disponibili ' - . formatBytes((int) $freeSpaceNow) . ', richiesti almeno ' - . formatBytes((float) $requiredBytes) - ); - } - // Quota probe for what the extraction actually lands (the uncompressed - // payload; the 100 MB above is margin, not something that gets written). - // The mysqldump ran between the two checks and may itself be what - // exhausted the account, so this cannot be inferred from pre-flight 1b. - $extractVerdict = probeWriteBytes($rootPath, $uncompressedBytes); - if ($extractVerdict !== '') { + try { + verifyUpgradeSpace($rootPath, [$tempDir => $requiredBytes]); + } catch (Throwable $e) { $zip->close(); - throw new RuntimeException( - 'Impossibile riservare i ' . formatBytes((float) $uncompressedBytes) - . ' richiesti dall\'estrazione: ' . ($extractVerdict === 'unavailable' - ? 'storage/tmp non esiste o non è scrivibile.' - : 'lo spazio disponibile o la quota dell\'account sono esauriti.') - ); + throw $e; } if (!$zip->extractTo($tempDir)) { @@ -867,6 +900,12 @@ function copyTree(string $src, string $dst, string $rootDst, array $skipRelative 'config.local.php', ]; + // Recheck after extraction, using the actual incoming files and mounts. + $copyRequirements = upgradeCopyRequirements($extractedRoot, $rootPath, $preservePaths); + $backupTarget = $rootPath . '/storage/backups'; + $copyRequirements[$backupTarget] = ($copyRequirements[$backupTarget] ?? 0) + $criticalBytes; + verifyUpgradeSpace($rootPath, $copyRequirements); + // 8b. Backup critical files before overwriting $fileBackupDir = $rootPath . '/storage/backups/pre_upgrade_files_' . date('Ymd_His'); if (!is_dir($fileBackupDir)) { diff --git a/tests/manual-upgrade-space.unit.php b/tests/manual-upgrade-space.unit.php new file mode 100644 index 000000000..22ed31c0f --- /dev/null +++ b/tests/manual-upgrade-space.unit.php @@ -0,0 +1,66 @@ + 200 * 1024 * 1024, + $root . '/storage/backups' => 250 * 1024 * 1024, + ]); + } catch (RuntimeException $e) { + $refused = str_contains($e->getMessage(), '450 MB'); + } + $check($refused, '300 MiB cannot hold a 250 MiB dump plus a 200 MiB reserve'); + $check((glob($root . '/storage/tmp/.upgrade_probe_*') ?: []) === [], 'no probe files remain'); +} finally { + unlink($fixture); + rmdir($root . '/storage/tmp'); + rmdir($root . '/storage/backups'); + rmdir($root . '/storage'); + rmdir($root); +} +echo "Passed: $passed Failed: $failed\n"; +exit($failed ? 1 : 0); diff --git a/tests/update-api-prerequisites.unit.php b/tests/update-api-prerequisites.unit.php new file mode 100644 index 000000000..332334e98 --- /dev/null +++ b/tests/update-api-prerequisites.unit.php @@ -0,0 +1,52 @@ + ['tipo_utente' => 'admin'], 'csrf_token' => 'test-token']; + $request = (new Slim\Psr7\Factory\ServerRequestFactory())->createServerRequest('POST', '/admin/updates') + ->withParsedBody(['version' => '1.0.0', 'csrf_token' => 'test-token']); + $root = dirname(__DIR__) . '/storage/tmp'; + if (!is_dir($root)) { mkdir($root, 0775, true); } + $upload = $root . '/api_prerequisites_' . bin2hex(random_bytes(6)); + mkdir($upload); + $passed = $failed = 0; + $check = static function (bool $ok, string $label) use (&$passed, &$failed): void { + echo ($ok ? 'OK ' : 'FAIL ') . $label . "\n"; + $ok ? $passed++ : $failed++; + }; + try { + foreach (['checkUpdates', 'performUpdate', 'getHistory', 'checkAvailable', 'installManualUpdate'] as $method) { + $_SESSION['manual_update_path'] = $upload; + $response = $controller->$method($request, new Slim\Psr7\Response(), $db); + $body = json_decode((string) $response->getBody(), true); + $check($response->getStatusCode() === 503, $method . ' returns service unavailable'); + $check(str_contains($response->getHeaderLine('Content-Type'), 'application/json') + && ($body['success'] ?? null) === false && ($body['error'] ?? '') === 'prerequisite unavailable', + $method . ' returns a controlled JSON error'); + } + $_SESSION['user']['tipo_utente'] = 'staff'; + foreach (['performUpdate', 'installManualUpdate'] as $method) { + $response = $controller->$method($request, new Slim\Psr7\Response(), $db); + $check($response->getStatusCode() === 403, $method . ' still checks authorization first'); + } + $_SESSION['user']['tipo_utente'] = 'admin'; + $request = $request->withParsedBody(['version' => '1.0.0', 'csrf_token' => 'invalid']); + $response = $controller->performUpdate($request, new Slim\Psr7\Response(), $db); + $check($response->getStatusCode() === 403, 'CSRF failure is still rejected before constructing Updater'); + } finally { + rmdir($upload); + } + echo "Passed: $passed Failed: $failed\n"; + exit($failed ? 1 : 0); +} diff --git a/tests/update-space-destinations.unit.php b/tests/update-space-destinations.unit.php new file mode 100644 index 000000000..c0c76664f --- /dev/null +++ b/tests/update-space-destinations.unit.php @@ -0,0 +1,124 @@ + ($GLOBALS['spaceQuota'][$dir] ?? PHP_INT_MAX)) { + return false; + } + $GLOBALS['spaceWritten'] += strlen($data); + return \fwrite($handle, $data); + } +} +namespace { + function __(string $s, ...$args): string { return $args ? sprintf($s, ...$args) : $s; } + require dirname(__DIR__) . '/app/Support/Updater.php'; + $root = sys_get_temp_dir() . '/space_destinations_' . bin2hex(random_bytes(6)); + foreach (['storage/tmp', 'storage/backups', 'public/assets', 'app', 'package/public/assets'] as $dir) { + mkdir($root . '/' . $dir, 0775, true); + } + file_put_contents($root . '/package/public/assets/new.js', 'new asset'); + $ref = new ReflectionClass(App\Support\Updater::class); + $updater = $ref->newInstanceWithoutConstructor(); + $ref->getProperty('rootPath')->setValue($updater, $root); + $ref->getProperty('backupPath')->setValue($updater, $root . '/storage/backups'); + $ref->getProperty('estimateCache')->setValue($updater, 8192); + $call = static fn(string $method, array $args = []) => $ref->getMethod($method)->invoke($updater, ...$args); + $reset = static function () use ($ref, $updater): void { + $GLOBALS['spaceFree'] = $GLOBALS['spaceDevices'] = $GLOBALS['spaceQuota'] = $GLOBALS['spaceOpened'] = []; + $GLOBALS['spaceDefaultFree'] = 1024.0 * 1024 * 1024; + $GLOBALS['spaceWritten'] = 0; + $ref->getProperty('probeVerdicts')->setValue($updater, []); + }; + $passed = $failed = 0; + $check = static function (bool $ok, string $label) use (&$passed, &$failed): void { + echo ($ok ? 'OK ' : 'FAIL ') . $label . "\n"; + $ok ? $passed++ : $failed++; + }; + $cleanup = static function (string $dir) use (&$cleanup): void { + foreach (glob($dir . '/*') ?: [] as $p) { + is_dir($p) ? $cleanup($p) : unlink($p); + } + rmdir($dir); + }; + try { + $reset(); + $spaceFree[$root] = 0.0; + $spaceDevices[$root . '/storage/tmp'] = 200; + $check($call('checkFreeSpaceForUpdate') !== null, 'a full code volume is refused even when tmp has capacity'); + $check($spaceWritten === 0, 'zero free bytes is refused without allocating'); + + $reset(); + $spaceFree[$root . '/storage/backups'] = 0.0; + $spaceDevices[$root . '/storage/backups'] = 200; + $check($call('checkFreeSpaceForUpdate') !== null, 'a full separate backup volume is refused'); + + $reset(); + $spaceFree[$root . '/public/assets'] = 0.0; + $spaceDevices[$root . '/public/assets'] = 200; + $check($call('checkFreeSpaceForUpdate', [0, $root . '/package']) !== null, + 'the actual package exposes a full nested destination mount'); + + $reset(); + $spaceQuota[$root] = 100 * 1024; + $error = $call('checkSpaceRequirements', [[$root => 60 * 1024, $root . '/storage/tmp' => 60 * 1024]]); + $check($error !== null && str_contains($error, 'quota'), 'simultaneous writes share one cumulative quota check'); + + $reset(); + $spaceDevices[$root . '/storage/tmp'] = 200; + $check($call('checkSpaceRequirements', [[$root => 8192, $root . '/storage/tmp' => 8192]]) === null, + 'healthy separate filesystems pass'); + $check(in_array($root, $spaceOpened, true) && in_array($root . '/storage/tmp', $spaceOpened, true), + 'both destination filesystems receive a real write probe'); + + $reset(); + $spaceDefaultFree = false; + $check($call('probeWrite', [32 * 1024 * 1024, true]) === 'unknown', 'an unreadable capacity cannot prove a large request'); + $check($spaceWritten === 0 && $spaceOpened === [], 'unknown large capacity does not start an unbounded allocation'); + $check($call('probeWrite', [4096, true]) === '', 'a small request is still proved completely when the measurement is unavailable'); + $check($spaceWritten === 4096, 'the small probe writes every requested byte'); + $check($call('checkSpaceRequirements', [[$root => 32 * 1024 * 1024]]) !== null, + 'unknown capacity is a preflight error, not success'); + + $reset(); + $check($call('checkSpaceRequirements', [[$root . '/new/subdirectory' => 8192]]) === null, + 'new destinations use an existing writable ancestor'); + $check($spaceOpened === [$root], 'the probe is placed where the new directory will allocate'); + $check((glob($root . '/.space_probe_*') ?: []) === [], 'probe files are cleaned up'); + + $zipPath = $root . '/payload.zip'; + $zip = new ZipArchive(); + $zip->open($zipPath, ZipArchive::CREATE); + $zip->addFromString('file.txt', str_repeat('a', 8192)); + $zip->close(); + $zip->open($zipPath); + $reset(); + $check($call('extractionSpaceError', [$zip, $root . '/storage/tmp']) === null, 'first extraction preflight passes'); + $spaceFree[$root . '/storage/tmp'] = 0.0; + $check($call('extractionSpaceError', [$zip, $root . '/storage/tmp']) !== null, + 'a repeated extraction check observes capacity consumed since the first check'); + $zip->close(); + } finally { + $cleanup($root); + } + echo "Passed: $passed Failed: $failed\n"; + exit($failed ? 1 : 0); +} diff --git a/tests/update-space-preflight.unit.php b/tests/update-space-preflight.unit.php index 680b05bbb..23f2bbf5a 100644 --- a/tests/update-space-preflight.unit.php +++ b/tests/update-space-preflight.unit.php @@ -125,7 +125,7 @@ } else { // disk_free_space() unreadable: the probe must degrade to a bounded write // rather than refusing a legitimate update, so only the bound is assertable. - $check(in_array($verdict, ['', 'nospace', 'unavailable'], true), + $check(in_array($verdict, ['nospace', 'unavailable', 'unknown'], true), 'with free space unreadable the probe still answers a bounded verdict'); } @@ -312,7 +312,7 @@ static function (string $l): bool { }; $check(substr_count($src, 'checkFreeSpaceForUpdate(') >= 4, 'the gate has a declaration and at least three call sites (' . substr_count($src, 'checkFreeSpaceForUpdate(') . ')'); -$check(str_contains($src, 'private function checkFreeSpaceForUpdate(int $extraBytes = 0)'), +$check(str_contains($src, 'private function checkFreeSpaceForUpdate(int $extraBytes = 0,'), 'the gate can be asked for more than the rollback copy'); foreach ([ 'public function performUpdate(string $targetVersion): array', @@ -328,7 +328,7 @@ static function (string $l): bool { // The later gate must NOT be moved: it is the only measurement taken after the // backup and the download have consumed their share. $installBody = $bodyOf($src, 'public function installUpdate(string $sourcePath, string $targetVersion): array'); -$check(str_contains($installBody, 'checkFreeSpaceForUpdate()'), +$check(str_contains($installBody, 'checkFreeSpaceForUpdate(0, $sourcePath)'), 'installUpdate() keeps its own re-measuring gate'); echo "F. the CLI fallback got the same guarantees\n"; @@ -365,19 +365,10 @@ static function (string $l): bool { $check($window !== '' && str_contains($window, 'describeWriteFailure('), '"' . $message . '" carries a cause'); } -$freeChecks = 0; -$probedChecks = 0; -foreach ($cliLines as $i => $text) { - if (!str_contains($text, '@disk_free_space($rootPath)')) { - continue; - } - $freeChecks++; - if (str_contains(implode("\n", array_slice($cliLines, $i, 20)), 'probeWriteBytes(')) { - $probedChecks++; - } -} -$check($freeChecks > 0 && $freeChecks === $probedChecks, - "every disk_free_space() gate is backed by a real write probe ({$probedChecks}/{$freeChecks})"); +$check(substr_count($cli, 'verifyUpgradeSpace($rootPath,') >= 4, + 'the CLI rechecks destination capacity before backup, extraction and installation'); +$check(str_contains($cli, '$backupDir => max(0, $dumpEstimate) + $criticalBytes'), + 'the dump and critical backups join the cumulative destination budget'); echo "G. the preflight does not block a healthy installation\n"; $verdict = $call($updater, 'checkFreeSpaceForUpdate'); From c848896d0b683b3ee707e1a2e205c16934a81e55 Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Sat, 3 Oct 2026 13:34:43 +0200 Subject: [PATCH 08/10] Keep updater internals out of the response, and log every failure to build it Six endpoints echoed the raw message of any exception thrown while building the Updater, and checkAvailable also answers staff. The constructor's own refusal, a missing host precondition such as storage/tmp not writable or no ZipArchive, is now an UpdaterPreflightException; its message is written for the operator and names paths relative to the installation only. UpdateController::updaterUnavailable() logs every such failure through SecureLogger, then shows the cause only to an administrator and only for a precondition. Any other exception, and any failure seen by staff, gets a generic message that points at the application log. The two error_log() calls left in the controller go through SecureLogger too. tests/update-api-prerequisites.unit.php covers all five JSON endpoints and the updates page: the precondition shown to an administrator, an unexpected exception answered generically without its server path but kept in the log, and staff getting no cause at all. Against the previous controller it fails. --- app/Controllers/UpdateController.php | 38 ++++++++--- app/Support/Updater.php | 2 +- app/Support/UpdaterPreflightException.php | 19 ++++++ locale/da_DK.json | 3 +- locale/de_DE.json | 3 +- locale/en_US.json | 3 +- locale/fr_FR.json | 3 +- locale/it_IT.json | 3 +- tests/update-api-prerequisites.unit.php | 83 ++++++++++++++++++++--- 9 files changed, 132 insertions(+), 25 deletions(-) create mode 100644 app/Support/UpdaterPreflightException.php diff --git a/app/Controllers/UpdateController.php b/app/Controllers/UpdateController.php index 88e1e6cae..f631ca5e9 100644 --- a/app/Controllers/UpdateController.php +++ b/app/Controllers/UpdateController.php @@ -8,6 +8,7 @@ use App\Support\BackupManager; use App\Support\Csrf; use App\Support\SecureLogger; +use App\Support\UpdaterPreflightException; use mysqli; use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; @@ -42,7 +43,7 @@ public function index(Request $request, Response $response, mysqli $db): Respons 

getMessage(), ENT_QUOTES, 'UTF-8');
+                    echo htmlspecialchars($this->updaterUnavailable($e, 'index'), ENT_QUOTES, 'UTF-8');
                 ?>
jsonResponse($response, [ 'success' => false, - 'error' => $e->getMessage(), + 'error' => $this->updaterUnavailable($e, 'checkUpdates'), ], 503); } $updateInfo = $updater->checkForUpdates(); @@ -135,7 +136,7 @@ public function performUpdate(Request $request, Response $response, mysqli $db): } catch (\Throwable $e) { return $this->jsonResponse($response, [ 'success' => false, - 'error' => $e->getMessage(), + 'error' => $this->updaterUnavailable($e, 'performUpdate'), ], 503); } @@ -215,7 +216,7 @@ public function getHistory(Request $request, Response $response, mysqli $db): Re } catch (\Throwable $e) { return $this->jsonResponse($response, [ 'success' => false, - 'error' => $e->getMessage(), + 'error' => $this->updaterUnavailable($e, 'getHistory'), ], 503); } $history = $updater->getUpdateHistory(); @@ -239,7 +240,7 @@ public function checkAvailable(Request $request, Response $response, mysqli $db) } catch (\Throwable $e) { return $this->jsonResponse($response, [ 'success' => false, - 'error' => $e->getMessage(), + 'error' => $this->updaterUnavailable($e, 'checkAvailable'), ], 503); } $updateInfo = $updater->checkForUpdates(); @@ -485,7 +486,7 @@ public function clearMaintenance(Request $request, Response $response): Response if (file_exists($maintenanceFile)) { // nosemgrep: php.lang.security.unlink-use.unlink-use -- constant internal path (storage/.maintenance), not user input if (@unlink($maintenanceFile)) { - error_log("[Updater] Maintenance mode cleared manually by admin user " . ($_SESSION['user']['id'] ?? 'unknown')); + SecureLogger::info('[Updater] Maintenance mode cleared manually by admin user ' . ($_SESSION['user']['id'] ?? 'unknown')); return $this->jsonResponse($response, [ 'success' => true, 'message' => __('Modalità manutenzione disattivata') @@ -634,7 +635,7 @@ public function uploadUpdate(Request $request, Response $response, mysqli $db): ], 500); } catch (\Throwable $e) { - error_log('[UpdateController] Upload failed: ' . $e->getMessage() . "\n" . $e->getTraceAsString()); + SecureLogger::error('[UpdateController] Upload failed (' . get_class($e) . '): ' . $e->getMessage()); return $this->jsonResponse($response, [ 'success' => false, 'error' => __('Errore durante il caricamento del pacchetto') @@ -687,7 +688,7 @@ public function installManualUpdate(Request $request, Response $response, mysqli } catch (\Throwable $e) { return $this->jsonResponse($response, [ 'success' => false, - 'error' => $e->getMessage(), + 'error' => $this->updaterUnavailable($e, 'installManualUpdate'), ], 503); } @@ -792,6 +793,27 @@ private function restoreFailureStatus(array $result): int /** * Helper: Send JSON response */ + /** + * What to say when the updater cannot be built, after logging why. + * + * The full exception always goes to the application log. Only an + * administrator, and only for a missing host precondition + * (UpdaterPreflightException, whose message is written for the operator), + * sees the cause; anyone else, or any unexpected exception, gets a generic + * message, since a raw exception can carry server paths and internals and + * some of these endpoints also answer staff. + */ + private function updaterUnavailable(\Throwable $e, string $action): string + { + SecureLogger::error('[UpdateController] ' . $action . ': updater unavailable (' . get_class($e) . '): ' . $e->getMessage()); + + if ($e instanceof UpdaterPreflightException && ($_SESSION['user']['tipo_utente'] ?? '') === 'admin') { + return $e->getMessage(); + } + + return __("Il sistema di aggiornamento non è disponibile. Il dettaglio è nel registro dell'applicazione."); + } + private function jsonResponse(Response $response, array $data, int $status = 200): Response { $response->getBody()->write(json_encode($data, JSON_UNESCAPED_UNICODE)); diff --git a/app/Support/Updater.php b/app/Support/Updater.php index 1ceebcbfd..6c691195d 100644 --- a/app/Support/Updater.php +++ b/app/Support/Updater.php @@ -219,7 +219,7 @@ public function __construct(mysqli $db) if (!empty($issues)) { $this->debugLog('ERROR', 'Pre-flight check fallito', ['issues' => $issues]); - throw new \RuntimeException( + throw new UpdaterPreflightException( __('Controllo pre-aggiornamento fallito') . ': ' . implode(', ', $issues) ); } diff --git a/app/Support/UpdaterPreflightException.php b/app/Support/UpdaterPreflightException.php new file mode 100644 index 000000000..6f954e358 --- /dev/null +++ b/app/Support/UpdaterPreflightException.php @@ -0,0 +1,19 @@ + is_file($logFile) ? (int) filesize($logFile) : 0; + $loggedSince = static function (int $offset, string $needle) use ($logFile): bool { + clearstatcache(); + $tail = is_file($logFile) ? (string) file_get_contents($logFile, false, null, $offset) : ''; + return str_contains($tail, $needle); + }; + + $preflight = 'Controllo pre-aggiornamento fallito: storage/tmp non scrivibile'; + $internal = 'fopen(/home/account/secret/storage/tmp/x): failed to open stream'; + $generic = __("Il sistema di aggiornamento non è disponibile. Il dettaglio è nel registro dell'applicazione."); + $jsonEndpoints = ['checkUpdates', 'performUpdate', 'getHistory', 'checkAvailable', 'installManualUpdate']; + $call = static function (string $method) use ($controller, $request, $db, $upload): array { + $_SESSION['manual_update_path'] = $upload; + $response = $controller->$method($request, new Slim\Psr7\Response(), $db); + return [$response, json_decode((string) $response->getBody(), true)]; + }; + try { - foreach (['checkUpdates', 'performUpdate', 'getHistory', 'checkAvailable', 'installManualUpdate'] as $method) { - $_SESSION['manual_update_path'] = $upload; - $response = $controller->$method($request, new Slim\Psr7\Response(), $db); - $body = json_decode((string) $response->getBody(), true); - $check($response->getStatusCode() === 503, $method . ' returns service unavailable'); + // An administrator, and a missing host precondition: the cause is the answer. + App\Support\Updater::$failure = new App\Support\UpdaterPreflightException($preflight); + foreach ($jsonEndpoints as $method) { + $offset = $logSize(); + [$response, $body] = $call($method); + $check($response->getStatusCode() === 503, "{$method} returns service unavailable"); $check(str_contains($response->getHeaderLine('Content-Type'), 'application/json') - && ($body['success'] ?? null) === false && ($body['error'] ?? '') === 'prerequisite unavailable', - $method . ' returns a controlled JSON error'); + && ($body['success'] ?? null) === false && ($body['error'] ?? '') === $preflight, + "{$method} tells an administrator which precondition is missing"); + $check($loggedSince($offset, "{$method}: updater unavailable"), "{$method} logs the failure"); } + + // An unexpected exception: logged in full, never echoed. + App\Support\Updater::$failure = new \RuntimeException($internal); + foreach ($jsonEndpoints as $method) { + $offset = $logSize(); + [$response, $body] = $call($method); + $error = (string) ($body['error'] ?? ''); + $check($response->getStatusCode() === 503 && $error === $generic && !str_contains((string) $response->getBody(), '/home/account'), + "{$method} answers an unexpected exception with the generic message, without the server path"); + $check($loggedSince($offset, '/home/account/secret'), "{$method} keeps the detail in the log"); + } + + // The settings page degrades the same way. + $_SESSION['manual_update_path'] = $upload; + ob_start(); + try { + $page = $controller->index($request, new Slim\Psr7\Response(), $db); + } finally { + ob_end_clean(); + } + $html = (string) $page->getBody(); + $check($page->getStatusCode() === 503 && str_contains($html, htmlspecialchars($generic, ENT_QUOTES, 'UTF-8')) && !str_contains($html, '/home/account'), + 'the updates page shows the generic message for an unexpected exception'); + + // Staff reach checkAvailable: they get no cause, not even a precondition. $_SESSION['user']['tipo_utente'] = 'staff'; + App\Support\Updater::$failure = new App\Support\UpdaterPreflightException($preflight); + [$response, $body] = $call('checkAvailable'); + $check($response->getStatusCode() === 503 && ($body['error'] ?? '') === $generic, 'checkAvailable gives staff the generic message'); + foreach (['performUpdate', 'installManualUpdate'] as $method) { - $response = $controller->$method($request, new Slim\Psr7\Response(), $db); - $check($response->getStatusCode() === 403, $method . ' still checks authorization first'); + [$response] = $call($method); + $check($response->getStatusCode() === 403, "{$method} still checks authorization first"); } $_SESSION['user']['tipo_utente'] = 'admin'; $request = $request->withParsedBody(['version' => '1.0.0', 'csrf_token' => 'invalid']); $response = $controller->performUpdate($request, new Slim\Psr7\Response(), $db); $check($response->getStatusCode() === 403, 'CSRF failure is still rejected before constructing Updater'); } finally { - rmdir($upload); + @rmdir($upload); } echo "Passed: $passed Failed: $failed\n"; exit($failed ? 1 : 0); From 1a5a15255ab19355ebb2a1d3f1ff162dadd5c00e Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Sat, 3 Oct 2026 17:56:48 +0200 Subject: [PATCH 09/10] Report every unwritable update destination and keep the CLI upgrade usable - checkSpaceRequirements() collects every unwritable directory (relative to the root) instead of stopping at the first, through one message shared with the installUpdate() preflight. - The update panel labels its estimate "at least": it is the rollback copy only, a floor of what the installer will require, not the same figure. - manual-upgrade.php gets --assume-space-checked (and a matching web checkbox): on hosts where free space cannot be measured, an administrator who checked it by hand can proceed; a volume measured as full still refuses. The waiver is written to the upgrade log. - manual-upgrade.php computes the installed version after the render label, so early exits such as a rejected CSRF token no longer print an undefined variable. - probeWrite() loses its unused $fresh parameter; orphaned docblocks in Updater, BackupManager and UpdateController are reattached to their functions; a dead translation key is removed. --- app/Controllers/UpdateController.php | 6 +- app/Support/BackupManager.php | 60 +++++------- app/Support/Updater.php | 119 ++++++++++++++--------- locale/da_DK.json | 4 +- locale/de_DE.json | 4 +- locale/en_US.json | 4 +- locale/fr_FR.json | 4 +- locale/it_IT.json | 4 +- scripts/manual-upgrade.php | 38 +++++++- tests/manual-upgrade-space.unit.php | 38 ++++++++ tests/update-space-destinations.unit.php | 33 ++++++- tests/update-space-preflight.unit.php | 14 +++ 12 files changed, 224 insertions(+), 104 deletions(-) diff --git a/app/Controllers/UpdateController.php b/app/Controllers/UpdateController.php index f631ca5e9..2972886a0 100644 --- a/app/Controllers/UpdateController.php +++ b/app/Controllers/UpdateController.php @@ -790,9 +790,6 @@ private function restoreFailureStatus(array $result): int }; } - /** - * Helper: Send JSON response - */ /** * What to say when the updater cannot be built, after logging why. * @@ -814,6 +811,9 @@ private function updaterUnavailable(\Throwable $e, string $action): string return __("Il sistema di aggiornamento non è disponibile. Il dettaglio è nel registro dell'applicazione."); } + /** + * Helper: Send JSON response + */ private function jsonResponse(Response $response, array $data, int $status = 200): Response { $response->getBody()->write(json_encode($data, JSON_UNESCAPED_UNICODE)); diff --git a/app/Support/BackupManager.php b/app/Support/BackupManager.php index 198945c0a..fa2af6178 100644 --- a/app/Support/BackupManager.php +++ b/app/Support/BackupManager.php @@ -176,12 +176,6 @@ private function outOfSpace(string $dir): bool // Create // --------------------------------------------------------------------- - /** - * Create a backup ZIP. - * - * @param string $scope 'full' (DB + files) or 'db' (database only) - * @return array{success: bool, name: string|null, path: string|null, size: int, error: string|null} - */ /** * Build a backup filename carrying its origin. * @@ -461,14 +455,6 @@ private function pruneOldBackups(string $justWritten): void } } - /** - * @return array - */ - /** - * Origin encoded in a filename, for archives whose manifest predates it. - * Unknown or absent suffix means the automatic shape, which is what every - * archive written before this existed actually was. - */ /** True only for a directory whose sole entry is a database.sql file. */ private static function isLegacyBackupDirectory(string $dir): bool { @@ -661,6 +647,20 @@ public function restoreFromUploadedZip(string $tmpPath, int $size): array return $this->restoreZip($dest); } + /** + * True from the instant the import may have executed its first DROP TABLE + * until it has finished. + * + * MySQL cannot roll back DDL, so between those two points the database is + * neither the one the site was serving nor the one the archive describes. + * Two things must follow from that and neither did: the outcome has to say + * so, and the site must stay closed. An interrupted restore reported as an + * ordinary failure invites the operator to retry into the wreckage, and the + * maintenance flag was being lifted on the way out — including by the + * fatal-error shutdown handler, which is exactly the case this describes. + */ + private bool $databaseReplacementStarted = false; + /** * Entry point for both restore paths (stored backup + uploaded ZIP). * @@ -677,20 +677,6 @@ public function restoreFromUploadedZip(string $tmpPath, int $size): array * * @return array{success: bool, safety_backup: string|null, error: string|null, partial?: bool, restored_phase?: string} */ - /** - * True from the instant the import may have executed its first DROP TABLE - * until it has finished. - * - * MySQL cannot roll back DDL, so between those two points the database is - * neither the one the site was serving nor the one the archive describes. - * Two things must follow from that and neither did: the outcome has to say - * so, and the site must stay closed. An interrupted restore reported as an - * ordinary failure invites the operator to retry into the wreckage, and the - * maintenance flag was being lifted on the way out — including by the - * fatal-error shutdown handler, which is exactly the case this describes. - */ - private bool $databaseReplacementStarted = false; - private function restoreZip(string $zipPath): array { $lockFile = $this->rootPath . '/storage/cache/update.lock'; @@ -1090,15 +1076,6 @@ private function removeDir(string $dir): void // Internals // --------------------------------------------------------------------- - /** - * Dump every table to $filepath (DROP/CREATE/INSERT). Returns the table count. - * - * Binary columns are NOT supported by this dump path: every value is emitted - * as a single-quoted, real_escape_string()-escaped text literal (no _binary - * or hex literal), so any BLOB/BINARY bytes would be corrupted on a - * backup/restore round-trip. The schema is text/numeric/datetime only — - * adding a binary column requires changing this serialization first. - */ /** * The columns of $table a restore may write, in declaration order. * @@ -1197,6 +1174,15 @@ private function insertableColumns(string $table): array return $columns; } + /** + * Dump every table to $filepath (DROP/CREATE/INSERT). Returns the table count. + * + * Binary columns are NOT supported by this dump path: every value is emitted + * as a single-quoted, real_escape_string()-escaped text literal (no _binary + * or hex literal), so any BLOB/BINARY bytes would be corrupted on a + * backup/restore round-trip. The schema is text/numeric/datetime only — + * adding a binary column requires changing this serialization first. + */ private function dumpDatabaseTo(string $filepath): int { $handle = fopen($filepath, 'w'); diff --git a/app/Support/Updater.php b/app/Support/Updater.php index 6c691195d..e4e1b7d23 100644 --- a/app/Support/Updater.php +++ b/app/Support/Updater.php @@ -506,10 +506,6 @@ private function fetchVerifiedReleaseAsset(string $version, string $assetName): return ['status' => 'verified', 'content' => $content]; } - /** - * Extract final HTTP status code from response headers (handles redirects). - * @param array $headers - */ /** * GET a URL through the HTTP stream wrapper, returning [body, responseHeaders]. * @@ -540,6 +536,10 @@ private function httpGetWithHeaders(string $url, $context): array return [$body, is_array($wrapperData) ? $wrapperData : []]; } + /** + * Extract final HTTP status code from response headers (handles redirects). + * @param array $headers + */ private function extractFinalHttpStatus(array $headers): int { for ($i = count($headers) - 1; $i >= 0; $i--) { @@ -2452,19 +2452,7 @@ public function installUpdate(string $sourcePath, string $targetVersion): array $unwritable = $this->verifyWritableTargets($sourcePath, $this->rootPath); if ($unwritable !== []) { $this->debugLog('ERROR', 'Preflight: percorsi non scrivibili', ['paths' => $unwritable]); - // This branch is reached only AFTER verifyWritableTargets already - // found unwritable app files; isRunningInContainer() just refines - // the MESSAGE. A community image that mounts the code from a - // writable volume passes the writability check and never lands - // here — so the wording must state the real condition (files not - // writable) rather than presume "you're on the Docker image". - if ($this->isRunningInContainer()) { - throw new Exception(__('I file dell\'applicazione non sono scrivibili dall\'utente del web server e Pinakes gira dentro un container. Sull\'immagine ufficiale (fabiodalez/pinakes) il codice è incluso nell\'immagine e in sola lettura per scelta: aggiorna scaricando la nuova immagine con «docker compose pull && docker compose up -d» (il database e i volumi storage/uploads restano al sicuro). Se usi un\'immagine community che monta il codice da un volume scrivibile questo pulsante funziona normalmente — se vedi questo messaggio quel volume è al momento in sola lettura, controlla il suo mount.')); - } - throw new Exception(sprintf( - __('Aggiornamento annullato prima di ogni modifica: il processo PHP non può scrivere in questi percorsi: %s. Correggi i permessi (proprietario/scrittura per l\'utente del web server) e riprova, oppure esegui l\'aggiornamento da riga di comando come proprietario dei file: php scripts/manual-upgrade.php '), - implode(', ', array_slice($unwritable, 0, 15)) . (count($unwritable) > 15 ? ', …' : '') - )); + throw new Exception($this->unwritablePathsMessage($unwritable)); } // PRE-FLIGHT (issue #422): free space. The next step copies the whole @@ -2592,9 +2580,6 @@ public function installUpdate(string $sourcePath, string $targetVersion): array } } - /** - * Backup application files for atomic rollback - */ /** * Space needed by an update, in bytes: the rollback copy of the directories * backupAppFiles() duplicates, plus a margin for the new files landing @@ -2766,6 +2751,30 @@ private function installationSpaceRequirements(?string $sourcePath): array return $requirements; } + /** + * Why an update cannot write where it must: the paths, and on a container + * the wording that tells the official read-only image from a community + * image whose code volume is mounted read-only. Shared by the space gate, + * which runs first, and the #205 writability preflight in installUpdate(), + * so whichever trips the operator reads the same guidance. + * + * @param list $paths paths relative to the installation where they lie inside it + */ + private function unwritablePathsMessage(array $paths): string + { + // isRunningInContainer() only refines the MESSAGE. A community image + // that mounts the code from a writable volume never lands here, so the + // wording states the real condition (files not writable) rather than + // presume "you're on the Docker image". + if ($this->isRunningInContainer()) { + return __('I file dell\'applicazione non sono scrivibili dall\'utente del web server e Pinakes gira dentro un container. Sull\'immagine ufficiale (fabiodalez/pinakes) il codice è incluso nell\'immagine e in sola lettura per scelta: aggiorna scaricando la nuova immagine con «docker compose pull && docker compose up -d» (il database e i volumi storage/uploads restano al sicuro). Se usi un\'immagine community che monta il codice da un volume scrivibile questo pulsante funziona normalmente — se vedi questo messaggio quel volume è al momento in sola lettura, controlla il suo mount.'); + } + return sprintf( + __('Aggiornamento annullato prima di ogni modifica: il processo PHP non può scrivere in questi percorsi: %s. Correggi i permessi (proprietario/scrittura per l\'utente del web server) e riprova, oppure esegui l\'aggiornamento da riga di comando come proprietario dei file: php scripts/manual-upgrade.php '), + implode(', ', array_slice($paths, 0, 15)) . (count($paths) > 15 ? ', …' : '') + ); + } + /** * Aggregate simultaneous writes on each filesystem before probing. Missing * directories use their nearest existing ancestor, where mkdir will allocate. @@ -2774,6 +2783,7 @@ private function installationSpaceRequirements(?string $sourcePath): array private function checkSpaceRequirements(array $requirements): ?string { $volumes = []; + $unwritable = []; foreach ($requirements as $path => $bytes) { $dir = $path; while (!is_dir($dir) && !file_exists($dir) && dirname($dir) !== $dir) { @@ -2781,9 +2791,13 @@ private function checkSpaceRequirements(array $requirements): ?string } $stat = @stat($dir); if (!is_dir($dir) || !is_writable($dir) || $stat === false) { - return sprintf( - __('Impossibile usare la directory di lavoro dell\'aggiornamento (%s): non esiste o non è scrivibile dall\'utente del web server. Correggi i permessi e riprova.'), $path - ); + // Keep going: a read-only code tree has many such directories, + // and the operator needs the whole list, worded as the #205 + // writability preflight words it, not the first path met. + $root = rtrim(str_replace('\\', '/', $this->rootPath), '/') . '/'; + $normalized = str_replace('\\', '/', $path); + $unwritable[str_starts_with($normalized, $root) ? substr($normalized, strlen($root)) : $normalized] = true; + continue; } $key = (string) $stat['dev']; if (!isset($volumes[$key])) { @@ -2791,6 +2805,11 @@ private function checkSpaceRequirements(array $requirements): ?string } $volumes[$key]['bytes'] += max(0, $bytes); } + if ($unwritable !== []) { + $paths = array_keys($unwritable); + sort($paths); + return $this->unwritablePathsMessage($paths); + } // A previous panel/diagnostic probe cannot establish current capacity. $this->probeVerdicts = []; foreach ($volumes as $volume) { @@ -2827,15 +2846,15 @@ private function checkSpaceRequirements(array $requirements): ?string * require a readable filesystem bound; a small sample never proves a larger * requirement. Cached diagnostics are scoped to both path and byte count. */ - private function probeWrite(int $bytes, bool $fresh = false, ?string $directory = null): string + private function probeWrite(int $bytes, ?string $directory = null): string { if ($bytes <= 0) { return ''; } + // checkSpaceRequirements() clears the cache before it probes, so only + // the panel and the diagnostics reuse a verdict here. $key = ($directory ?? $this->rootPath . '/storage/tmp') . ':' . $bytes; - if ($fresh) { - $this->probeVerdicts = []; - } elseif (isset($this->probeVerdicts[$key])) { + if (isset($this->probeVerdicts[$key])) { return $this->probeVerdicts[$key]; } @@ -2918,12 +2937,6 @@ private function runWriteProbe(int $bytes, ?string $directory = null): string return $ok ? '' : 'nospace'; } - /** - * Why a write to this path failed, in words an operator can act on. - * Cheapest and most specific first: a broad, side-effecting probe placed - * first becomes the default answer for everything below it, and on a nearly - * full volume it would report "no space" for a missing directory. - */ /** * Refuse an in-app update on the official Docker image. * @@ -2981,6 +2994,12 @@ private function extractionSpaceError(ZipArchive $zip, ?string $destination = nu ]); } + /** + * Why a write to this path failed, in words an operator can act on. + * Cheapest and most specific first: a broad, side-effecting probe placed + * first becomes the default answer for everything below it, and on a nearly + * full volume it would report "no space" for a missing directory. + */ private function describeWriteFailure(string $targetPath): string { // error_get_last() is process-global: capture it BEFORE this function @@ -3001,7 +3020,7 @@ private function describeWriteFailure(string $targetPath): string // Only a probe that actually ran out of room may claim a space problem; // 'unavailable' means the probe could not be created, which says // nothing about the failure being described. - if ($this->probeWrite(1024 * 1024, false, $dir) === 'nospace') { + if ($this->probeWrite(1024 * 1024, $dir) === 'nospace') { return __('spazio su disco o quota dell\'account esauriti'); } @@ -3009,6 +3028,9 @@ private function describeWriteFailure(string $targetPath): string return $message !== '' ? $message : __('causa sconosciuta'); } + /** + * Backup application files for atomic rollback + */ private function backupAppFiles(): string { $timestamp = date('Y-m-d_His'); @@ -4044,15 +4066,6 @@ public function runMigrations(string $fromVersion, string $toVersion): array } } - /** - * Re-apply installer/database/triggers.sql using the DELIMITER-aware - * splitter. Called after a successful migration run so that loan-integrity - * triggers are kept current on upgrades (they cannot live inside migration - * files because those run under the starting version's non-DELIMITER-aware - * runner). Idempotent (triggers.sql is DROP + CREATE) and non-fatal — a - * missing TRIGGER privilege only logs a warning; the same overlap rules are - * enforced at the application layer. - */ /** * Re-derive availability caches after migrations that rewrite prestiti rows * (e.g. the 0.7.20 loan-state cleanup flips attivo/stato directly, which a @@ -4072,6 +4085,15 @@ private function recalculateAfterMigrations(): void } } + /** + * Re-apply installer/database/triggers.sql using the DELIMITER-aware + * splitter. Called after a successful migration run so that loan-integrity + * triggers are kept current on upgrades (they cannot live inside migration + * files because those run under the starting version's non-DELIMITER-aware + * runner). Idempotent (triggers.sql is DROP + CREATE) and non-fatal — a + * missing TRIGGER privilege only logs a warning; the same overlap rules are + * enforced at the application layer. + */ private function reapplyTriggers(): void { $triggersFile = $this->rootPath . '/installer/database/triggers.sql'; @@ -4621,9 +4643,12 @@ public function checkRequirements(): array if (!$writable) $allMet = false; } - // Report the SAME figure installUpdate() will enforce. A panel that - // advertises a smaller number than the gate is worse than no panel: it - // shows all-green on an install the update is about to refuse. + // A FLOOR, and labelled as one. This is the rollback copy (x1.3), the + // one part of the requirement cheap enough to compute on every render. + // The gate in installUpdate() also adds the incoming tree, the package + // and the pre-update backup, all measured once the release is known, so + // it can refuse an update this row shows as green. Saying "at least" + // keeps the row honest instead of presenting the floor as the answer. $freeSpace = @disk_free_space($this->rootPath); if ($freeSpace === false) { $freeSpace = 0; @@ -4632,7 +4657,7 @@ public function checkRequirements(): array $spaceMet = $freeSpace >= $minSpace; $requirements[] = [ 'name' => __('Spazio libero'), - 'required' => $this->formatBytes((float) $minSpace), + 'required' => sprintf(__('almeno %s'), $this->formatBytes((float) $minSpace)), 'current' => $freeSpace > 0 ? $this->formatBytes($freeSpace) : __('Non disponibile'), 'met' => $spaceMet ]; diff --git a/locale/da_DK.json b/locale/da_DK.json index e1d3da1bc..2f9df73b4 100644 --- a/locale/da_DK.json +++ b/locale/da_DK.json @@ -7670,7 +7670,6 @@ "Annata %1$d conservata separatamente come volume «%2$s» per mantenere i dati descrittivi.": "Årgang %1$d bevaret separat som bind »%2$s« for at bevare de beskrivende oplysninger.", "Le annate con dati descrittivi discordanti restano separate con un volume distinto.": "Årgange med modstridende beskrivende oplysninger bevares separat med en særskilt bindbetegnelse.", "Spazio su disco insufficiente per aggiornare: servono circa %1$s, disponibili %2$s. Libera spazio (i backup automatici in storage/backups sono i primi candidati) e riprova.": "Ikke nok diskplads til opdateringen: der kræves ca. %1$s, %2$s er tilgængelige. Frigør plads (de automatiske sikkerhedskopier i storage/backups er de første kandidater), og prøv igen.", - "Impossibile scrivere %1$s di prova: lo spazio disponibile o la quota dell'account sono esauriti. L'aggiornamento richiede circa %2$s. Libera spazio (i backup automatici in storage/backups sono i primi candidati) e riprova.": "Kunne ikke skrive en testfil på %1$s: den tilgængelige plads eller kontoens kvote er opbrugt. Opdateringen kræver ca. %2$s. Frigør plads (de automatiske sikkerhedskopier i storage/backups er de første kandidater), og prøv igen.", "spazio su disco o quota dell'account esauriti": "diskplads eller kontokvote opbrugt", "il file di destinazione esiste e non è scrivibile": "destinationsfilen findes og kan ikke skrives", "la directory di destinazione non esiste": "destinationsmappen findes ikke", @@ -7926,5 +7925,6 @@ "Puoi collegare al massimo 20 autori a un articolo.": "Du kan højst knytte 20 forfattere til en artikel.", "Indica prima l’indirizzo o il percorso della risorsa: senza, non c’è nulla da mostrare nel catalogo pubblico.": "Angiv først ressourcens adresse eller sti: uden den er der intet at vise i det offentlige katalog.", "Questo identificativo GND è già assegnato a un altro autore.": "Denne GND-identifikator er allerede tildelt en anden forfatter.", - "Il sistema di aggiornamento non è disponibile. Il dettaglio è nel registro dell'applicazione.": "Opdateringssystemet er ikke tilgængeligt. Detaljerne står i applikationsloggen." + "Il sistema di aggiornamento non è disponibile. Il dettaglio è nel registro dell'applicazione.": "Opdateringssystemet er ikke tilgængeligt. Detaljerne står i applikationsloggen.", + "almeno %s": "mindst %s" } diff --git a/locale/de_DE.json b/locale/de_DE.json index 9bc0087c8..8ecfb3154 100644 --- a/locale/de_DE.json +++ b/locale/de_DE.json @@ -7670,7 +7670,6 @@ "Annata %1$d conservata separatamente come volume «%2$s» per mantenere i dati descrittivi.": "Jahrgang %1$d separat als Band „%2$s“ beibehalten, um die beschreibenden Daten zu erhalten.", "Le annate con dati descrittivi discordanti restano separate con un volume distinto.": "Jahrgänge mit widersprüchlichen Beschreibungsdaten bleiben unter einer eigenen Bandbezeichnung getrennt.", "Spazio su disco insufficiente per aggiornare: servono circa %1$s, disponibili %2$s. Libera spazio (i backup automatici in storage/backups sono i primi candidati) e riprova.": "Nicht genügend Speicherplatz für das Update: benötigt werden etwa %1$s, verfügbar sind %2$s. Geben Sie Speicherplatz frei (die automatischen Sicherungen in storage/backups sind die ersten Kandidaten) und versuchen Sie es erneut.", - "Impossibile scrivere %1$s di prova: lo spazio disponibile o la quota dell'account sono esauriti. L'aggiornamento richiede circa %2$s. Libera spazio (i backup automatici in storage/backups sono i primi candidati) e riprova.": "Eine Testdatei von %1$s konnte nicht geschrieben werden: der verfügbare Speicher oder das Kontingent des Kontos ist erschöpft. Das Update benötigt etwa %2$s. Geben Sie Speicherplatz frei (die automatischen Sicherungen in storage/backups sind die ersten Kandidaten) und versuchen Sie es erneut.", "spazio su disco o quota dell'account esauriti": "Speicherplatz oder Kontingent des Kontos erschöpft", "il file di destinazione esiste e non è scrivibile": "die Zieldatei existiert und ist nicht beschreibbar", "la directory di destinazione non esiste": "das Zielverzeichnis existiert nicht", @@ -7926,5 +7925,6 @@ "Puoi collegare al massimo 20 autori a un articolo.": "Einem Artikel können höchstens 20 Autoren zugeordnet werden.", "Indica prima l’indirizzo o il percorso della risorsa: senza, non c’è nulla da mostrare nel catalogo pubblico.": "Geben Sie zuerst die Adresse oder den Pfad der Ressource an: ohne sie gibt es im öffentlichen Katalog nichts anzuzeigen.", "Questo identificativo GND è già assegnato a un altro autore.": "Diese GND-Kennung ist bereits einem anderen Autor zugeordnet.", - "Il sistema di aggiornamento non è disponibile. Il dettaglio è nel registro dell'applicazione.": "Das Aktualisierungssystem ist nicht verfügbar. Die Einzelheiten stehen im Anwendungsprotokoll." + "Il sistema di aggiornamento non è disponibile. Il dettaglio è nel registro dell'applicazione.": "Das Aktualisierungssystem ist nicht verfügbar. Die Einzelheiten stehen im Anwendungsprotokoll.", + "almeno %s": "mindestens %s" } diff --git a/locale/en_US.json b/locale/en_US.json index 3e0fdb214..29bbc4b45 100644 --- a/locale/en_US.json +++ b/locale/en_US.json @@ -7670,7 +7670,6 @@ "Annata %1$d conservata separatamente come volume «%2$s» per mantenere i dati descrittivi.": "Year %1$d retained separately as volume “%2$s” to preserve its descriptive data.", "Le annate con dati descrittivi discordanti restano separate con un volume distinto.": "Years with conflicting descriptive data remain separate under a distinct volume.", "Spazio su disco insufficiente per aggiornare: servono circa %1$s, disponibili %2$s. Libera spazio (i backup automatici in storage/backups sono i primi candidati) e riprova.": "Not enough disk space to update: about %1$s is needed, %2$s available. Free some space (the automatic backups in storage/backups are the first candidates) and try again.", - "Impossibile scrivere %1$s di prova: lo spazio disponibile o la quota dell'account sono esauriti. L'aggiornamento richiede circa %2$s. Libera spazio (i backup automatici in storage/backups sono i primi candidati) e riprova.": "Could not write a %1$s test file: the available space or the account quota is exhausted. The update needs about %2$s. Free some space (the automatic backups in storage/backups are the first candidates) and try again.", "spazio su disco o quota dell'account esauriti": "disk space or account quota exhausted", "il file di destinazione esiste e non è scrivibile": "the destination file exists and is not writable", "la directory di destinazione non esiste": "the destination directory does not exist", @@ -7926,5 +7925,6 @@ "Puoi collegare al massimo 20 autori a un articolo.": "You can link at most 20 authors to an article.", "Indica prima l’indirizzo o il percorso della risorsa: senza, non c’è nulla da mostrare nel catalogo pubblico.": "Enter the resource address or path first: without it there is nothing to show in the public catalogue.", "Questo identificativo GND è già assegnato a un altro autore.": "This GND identifier is already assigned to another author.", - "Il sistema di aggiornamento non è disponibile. Il dettaglio è nel registro dell'applicazione.": "The update system is unavailable. The details are in the application log." + "Il sistema di aggiornamento non è disponibile. Il dettaglio è nel registro dell'applicazione.": "The update system is unavailable. The details are in the application log.", + "almeno %s": "at least %s" } diff --git a/locale/fr_FR.json b/locale/fr_FR.json index d04cd58f8..e3fa4533a 100644 --- a/locale/fr_FR.json +++ b/locale/fr_FR.json @@ -7670,7 +7670,6 @@ "Annata %1$d conservata separatamente come volume «%2$s» per mantenere i dati descrittivi.": "Année %1$d conservée séparément comme volume « %2$s » pour préserver les données descriptives.", "Le annate con dati descrittivi discordanti restano separate con un volume distinto.": "Les années dont les données descriptives sont contradictoires restent séparées sous un volume distinct.", "Spazio su disco insufficiente per aggiornare: servono circa %1$s, disponibili %2$s. Libera spazio (i backup automatici in storage/backups sono i primi candidati) e riprova.": "Espace disque insuffisant pour la mise à jour : environ %1$s sont nécessaires, %2$s disponibles. Libérez de l'espace (les sauvegardes automatiques dans storage/backups sont les premières candidates) et réessayez.", - "Impossibile scrivere %1$s di prova: lo spazio disponibile o la quota dell'account sono esauriti. L'aggiornamento richiede circa %2$s. Libera spazio (i backup automatici in storage/backups sono i primi candidati) e riprova.": "Impossible d'écrire un fichier de test de %1$s : l'espace disponible ou le quota du compte est épuisé. La mise à jour nécessite environ %2$s. Libérez de l'espace (les sauvegardes automatiques dans storage/backups sont les premières candidates) et réessayez.", "spazio su disco o quota dell'account esauriti": "espace disque ou quota du compte épuisé", "il file di destinazione esiste e non è scrivibile": "le fichier de destination existe et n'est pas accessible en écriture", "la directory di destinazione non esiste": "le répertoire de destination n'existe pas", @@ -7926,5 +7925,6 @@ "Puoi collegare al massimo 20 autori a un articolo.": "Vous pouvez lier au maximum 20 auteurs à un article.", "Indica prima l’indirizzo o il percorso della risorsa: senza, non c’è nulla da mostrare nel catalogo pubblico.": "Indiquez d’abord l’adresse ou le chemin de la ressource : sans elle, il n’y a rien à afficher dans le catalogue public.", "Questo identificativo GND è già assegnato a un altro autore.": "Cet identifiant GND est déjà attribué à un autre auteur.", - "Il sistema di aggiornamento non è disponibile. Il dettaglio è nel registro dell'applicazione.": "Le système de mise à jour est indisponible. Le détail figure dans le journal de l'application." + "Il sistema di aggiornamento non è disponibile. Il dettaglio è nel registro dell'applicazione.": "Le système de mise à jour est indisponible. Le détail figure dans le journal de l'application.", + "almeno %s": "au moins %s" } diff --git a/locale/it_IT.json b/locale/it_IT.json index 78089ce5c..29035a502 100644 --- a/locale/it_IT.json +++ b/locale/it_IT.json @@ -7670,7 +7670,6 @@ "Annata %1$d conservata separatamente come volume «%2$s» per mantenere i dati descrittivi.": "Annata %1$d conservata separatamente come volume «%2$s» per mantenere i dati descrittivi.", "Le annate con dati descrittivi discordanti restano separate con un volume distinto.": "Le annate con dati descrittivi discordanti restano separate con un volume distinto.", "Spazio su disco insufficiente per aggiornare: servono circa %1$s, disponibili %2$s. Libera spazio (i backup automatici in storage/backups sono i primi candidati) e riprova.": "Spazio su disco insufficiente per aggiornare: servono circa %1$s, disponibili %2$s. Libera spazio (i backup automatici in storage/backups sono i primi candidati) e riprova.", - "Impossibile scrivere %1$s di prova: lo spazio disponibile o la quota dell'account sono esauriti. L'aggiornamento richiede circa %2$s. Libera spazio (i backup automatici in storage/backups sono i primi candidati) e riprova.": "Impossibile scrivere %1$s di prova: lo spazio disponibile o la quota dell'account sono esauriti. L'aggiornamento richiede circa %2$s. Libera spazio (i backup automatici in storage/backups sono i primi candidati) e riprova.", "spazio su disco o quota dell'account esauriti": "spazio su disco o quota dell'account esauriti", "il file di destinazione esiste e non è scrivibile": "il file di destinazione esiste e non è scrivibile", "la directory di destinazione non esiste": "la directory di destinazione non esiste", @@ -7926,5 +7925,6 @@ "Puoi collegare al massimo 20 autori a un articolo.": "Puoi collegare al massimo 20 autori a un articolo.", "Indica prima l’indirizzo o il percorso della risorsa: senza, non c’è nulla da mostrare nel catalogo pubblico.": "Indica prima l’indirizzo o il percorso della risorsa: senza, non c’è nulla da mostrare nel catalogo pubblico.", "Questo identificativo GND è già assegnato a un altro autore.": "Questo identificativo GND è già assegnato a un altro autore.", - "Il sistema di aggiornamento non è disponibile. Il dettaglio è nel registro dell'applicazione.": "Il sistema di aggiornamento non è disponibile. Il dettaglio è nel registro dell'applicazione." + "Il sistema di aggiornamento non è disponibile. Il dettaglio è nel registro dell'applicazione.": "Il sistema di aggiornamento non è disponibile. Il dettaglio è nel registro dell'applicazione.", + "almeno %s": "almeno %s" } diff --git a/scripts/manual-upgrade.php b/scripts/manual-upgrade.php index e25f79991..b70e5cf1d 100644 --- a/scripts/manual-upgrade.php +++ b/scripts/manual-upgrade.php @@ -65,10 +65,19 @@ // idempotent migrations); auth/CSRF are meaningless with shell access // and are satisfied synthetically below. // ============================================================ +// Set by --assume-space-checked (CLI) or the matching checkbox (web): the +// operator has measured the free space by hand on a host where PHP cannot. +$upgradeAssumeSpace = false; if (PHP_SAPI === 'cli') { - $zipArg = $argv[1] ?? ''; + $cliArgs = array_slice($argv, 1); + $flagAt = array_search('--assume-space-checked', $cliArgs, true); + if ($flagAt !== false) { + $upgradeAssumeSpace = true; + array_splice($cliArgs, (int) $flagAt, 1); + } + $zipArg = $cliArgs[0] ?? ''; if ($zipArg === '--count-sql-statements') { - $sqlFile = $argv[2] ?? ''; + $sqlFile = $cliArgs[1] ?? ''; if ($sqlFile === '' || !is_file($sqlFile)) { fwrite(STDERR, "ERROR: SQL file not found: {$sqlFile}\n"); exit(1); @@ -82,8 +91,10 @@ exit(0); } if ($zipArg === '' || in_array($zipArg, ['-h', '--help'], true)) { - fwrite(STDOUT, "Usage: php scripts/manual-upgrade.php /path/to/pinakes-vX.Y.Z.zip\n" - . " php scripts/manual-upgrade.php --count-sql-statements /path/to/migration.sql\n"); + fwrite(STDOUT, "Usage: php scripts/manual-upgrade.php [--assume-space-checked] /path/to/pinakes-vX.Y.Z.zip\n" + . " php scripts/manual-upgrade.php --count-sql-statements /path/to/migration.sql\n" + . " --assume-space-checked continue when PHP cannot measure the free space on this host\n" + . " (you have checked it yourself); a full disk still stops the upgrade\n"); exit($zipArg === '' ? 1 : 0); } $zipReal = realpath($zipArg); @@ -301,6 +312,14 @@ function verifyUpgradeSpace(string $rootPath, array $requirements): void $dir = $volume['path']; $required = $volume['bytes']; $verdict = probeWriteBytes($rootPath, $required, $dir); + if ($verdict === 'unknown' && !empty($GLOBALS['upgradeAssumeSpace'])) { + // Only "cannot be measured" is waived. A write that runs out of + // room, or a directory that cannot be written, still stops here. + if (isset($GLOBALS['log']) && is_array($GLOBALS['log'])) { + $GLOBALS['log'][] = '[ATTENZIONE] Spazio in ' . $dir . ' non misurabile da PHP: si prosegue perché è stato dichiarato verificato a mano (richiesti ' . formatBytes($required) . ').'; + } + continue; + } if ($verdict !== '') { $reason = match ($verdict) { 'nospace' => "spazio su disco o quota dell'account insufficienti", @@ -646,6 +665,9 @@ function copyTree(string $src, string $dst, string $rootDst, array $skipRelative } // Regenerate token after use $_SESSION['upgrade_csrf'] = bin2hex(random_bytes(32)); + if (PHP_SAPI !== 'cli') { + $upgradeAssumeSpace = ($_POST['assume_space_checked'] ?? '') === '1'; + } $log[] = '=== Pinakes Manual Upgrade — ' . date('Y-m-d H:i:s') . ' ==='; try { @@ -1131,6 +1153,8 @@ function copyTree(string $src, string $dst, string $rootDst, array $skipRelative // UI // ============================================================ +render: +// After the label: the early exits jump here and must still know the installed version. $currentVersion = '?'; $versionFile = $rootPath . '/version.json'; if (is_file($versionFile)) { @@ -1138,7 +1162,6 @@ function copyTree(string $src, string $dst, string $rootDst, array $skipRelative $currentVersion = $vj['version'] ?? '?'; } -render: // CLI: plain-text report on stdout/stderr, exit code for scripting — no HTML. if (PHP_SAPI === 'cli') { foreach ($log as $logLine) { @@ -1226,6 +1249,11 @@ function copyTree(string $src, string $dst, string $rootDst, array $skipRelative + +
  • Backup automatico del database prima dell'upgrade
  • Preserva: copertine, uploads, .env, plugin, configurazioni
    • diff --git a/tests/manual-upgrade-space.unit.php b/tests/manual-upgrade-space.unit.php index 22ed31c0f..0b180b571 100644 --- a/tests/manual-upgrade-space.unit.php +++ b/tests/manual-upgrade-space.unit.php @@ -55,6 +55,39 @@ function stat($path) { } $check($refused, '300 MiB cannot hold a 250 MiB dump plus a 200 MiB reserve'); $check((glob($root . '/storage/tmp/.upgrade_probe_*') ?: []) === [], 'no probe files remain'); + + // A host where PHP cannot measure free space: without an override the + // recovery tool would be the second route closed, after the in-app one. + // The override waives "cannot be measured" only, and says so in the log. + $cliFree = false; + $GLOBALS['upgradeAssumeSpace'] = false; + $GLOBALS['log'] = []; + $refused = false; + try { + CliSpaceFixture\verifyUpgradeSpace($root, [$root . '/storage/tmp' => 200 * 1024 * 1024]); + } catch (RuntimeException $e) { + $refused = str_contains($e->getMessage(), 'non verificabile'); + } + $check($refused, 'unmeasurable space stops the upgrade unless the operator confirms it'); + $GLOBALS['upgradeAssumeSpace'] = true; + $passedThrough = true; + try { + CliSpaceFixture\verifyUpgradeSpace($root, [$root . '/storage/tmp' => 200 * 1024 * 1024]); + } catch (RuntimeException $e) { + $passedThrough = false; + } + $check($passedThrough, 'with --assume-space-checked an unmeasurable volume no longer blocks'); + $check(count(array_filter($GLOBALS['log'], static fn($line) => str_contains((string) $line, 'verificato a mano'))) === 1, + 'the waived check is written to the upgrade log'); + $cliFree = 1024.0 * 1024; + $refused = false; + try { + CliSpaceFixture\verifyUpgradeSpace($root, [$root . '/storage/tmp' => 200 * 1024 * 1024]); + } catch (RuntimeException $e) { + $refused = str_contains($e->getMessage(), 'insufficienti'); + } + $check($refused, 'the override never waives a volume that is actually full'); + $GLOBALS['upgradeAssumeSpace'] = false; } finally { unlink($fixture); rmdir($root . '/storage/tmp'); @@ -62,5 +95,10 @@ function stat($path) { rmdir($root . '/storage'); rmdir($root); } +// Early exits (a rejected CSRF token, ...) `goto render`: anything the page prints +// must be computed after that label, or it is undefined on exactly those requests. +$renderAt = strpos($source, "\nrender:\n"); +$versionAt = strpos($source, '$currentVersion = \'?\';'); +$check($renderAt !== false && $versionAt !== false && $versionAt > $renderAt, 'the installed version is computed after the render label, so early exits still show it'); echo "Passed: $passed Failed: $failed\n"; exit($failed ? 1 : 0); diff --git a/tests/update-space-destinations.unit.php b/tests/update-space-destinations.unit.php index c0c76664f..1928c17f7 100644 --- a/tests/update-space-destinations.unit.php +++ b/tests/update-space-destinations.unit.php @@ -31,6 +31,7 @@ function fwrite($handle, string $data): int|false { namespace { function __(string $s, ...$args): string { return $args ? sprintf($s, ...$args) : $s; } require dirname(__DIR__) . '/app/Support/Updater.php'; + require_once dirname(__DIR__) . '/app/Support/ContainerRuntime.php'; $root = sys_get_temp_dir() . '/space_destinations_' . bin2hex(random_bytes(6)); foreach (['storage/tmp', 'storage/backups', 'public/assets', 'app', 'package/public/assets'] as $dir) { mkdir($root . '/' . $dir, 0775, true); @@ -91,9 +92,9 @@ function __(string $s, ...$args): string { return $args ? sprintf($s, ...$args) $reset(); $spaceDefaultFree = false; - $check($call('probeWrite', [32 * 1024 * 1024, true]) === 'unknown', 'an unreadable capacity cannot prove a large request'); + $check($call('probeWrite', [32 * 1024 * 1024]) === 'unknown', 'an unreadable capacity cannot prove a large request'); $check($spaceWritten === 0 && $spaceOpened === [], 'unknown large capacity does not start an unbounded allocation'); - $check($call('probeWrite', [4096, true]) === '', 'a small request is still proved completely when the measurement is unavailable'); + $check($call('probeWrite', [4096]) === '', 'a small request is still proved completely when the measurement is unavailable'); $check($spaceWritten === 4096, 'the small probe writes every requested byte'); $check($call('checkSpaceRequirements', [[$root => 32 * 1024 * 1024]]) !== null, 'unknown capacity is a preflight error, not success'); @@ -116,6 +117,34 @@ function __(string $s, ...$args): string { return $args ? sprintf($s, ...$args) $check($call('extractionSpaceError', [$zip, $root . '/storage/tmp']) !== null, 'a repeated extraction check observes capacity consumed since the first check'); $zip->close(); + + // An unusable code tree: the space gate runs before the #205 + // writability preflight, so it must give the same answer, every path + // listed, rather than one "working directory" with an absolute path. + // A FILE where a directory belongs makes the destination unusable even + // for uid 0, where a chmod-based precondition would invert. + $reset(); + file_put_contents($root . '/blocked-app', 'not a directory'); + file_put_contents($root . '/blocked-assets', 'not a directory'); + try { + $message = (string) $call('checkSpaceRequirements', [[ + $root . '/blocked-app/Support' => 8192, + $root . '/blocked-assets/js' => 8192, + $root . '/storage/tmp' => 8192, + ]]); + } finally { + unlink($root . '/blocked-app'); + unlink($root . '/blocked-assets'); + } + if (App\Support\ContainerRuntime::detected()) { + $check(str_contains($message, 'container'), 'in a container the unusable tree gets the container guidance'); + } else { + $check(str_contains($message, 'blocked-app/Support, blocked-assets/js') && str_contains($message, 'manual-upgrade.php'), + 'every unusable destination is listed, relative to the installation, with the #205 wording'); + } + $check(!str_contains($message, 'directory di lavoro') && !str_contains($message, $root), + 'the gate no longer stops at the first path with an absolute working-directory message'); + $check($spaceOpened === [], 'nothing is probed when a destination cannot be written'); } finally { $cleanup($root); } diff --git a/tests/update-space-preflight.unit.php b/tests/update-space-preflight.unit.php index 23f2bbf5a..02ae45726 100644 --- a/tests/update-space-preflight.unit.php +++ b/tests/update-space-preflight.unit.php @@ -137,6 +137,20 @@ 'the full estimate can be proven on this checkout (' . number_format($estimate) . ' bytes)'); $check(count(glob($root . '/storage/tmp/.space_probe_*') ?: []) === 0, 'no probe file survives a full-size probe'); +// The panel shows only the rollback copy, the part cheap enough to compute on +// every render; the gate adds the incoming tree, the package and the backup. +// So the row is a floor and must say so, or it reads green for an update the +// gate refuses. +$spaceRow = null; +foreach ($updater->checkRequirements()['requirements'] as $row) { + if (($row['name'] ?? '') === __('Spazio libero')) { + $spaceRow = $row; + } +} +$floor = trim(str_replace('%s', '', __('almeno %s'))); +$check(is_array($spaceRow) && str_starts_with((string) $spaceRow['required'], $floor), + 'the panel presents the space figure as a minimum (' . (string) ($spaceRow['required'] ?? 'missing') . ')'); + // Tri-state: "the probe could not be created" must not be reported as "no // space". Built with a FILE where storage/tmp belongs, so mkdir cannot succeed // even for uid 0 — a chmod-based precondition inverts under root. From 3e66c0e2eda00605fe2b8759ec198e85a2ccd9e8 Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Sun, 4 Oct 2026 11:47:30 +0200 Subject: [PATCH 10/10] Keep a dense write probe when the operator waives an unmeasurable space check With --assume-space-checked the preflight skipped every write test, because each requirement is above the 16 MiB dense limit and the strided probe gives up when free space is unknown. A full disk was then found halfway through the dump. The waiver now writes 16 MiB densely first: only when that succeeds does the upgrade continue and log the manual confirmation. --- scripts/manual-upgrade.php | 14 +++++++++----- tests/manual-upgrade-space.unit.php | 17 +++++++++++++++++ 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/scripts/manual-upgrade.php b/scripts/manual-upgrade.php index b70e5cf1d..7202a82ba 100644 --- a/scripts/manual-upgrade.php +++ b/scripts/manual-upgrade.php @@ -313,12 +313,16 @@ function verifyUpgradeSpace(string $rootPath, array $requirements): void $required = $volume['bytes']; $verdict = probeWriteBytes($rootPath, $required, $dir); if ($verdict === 'unknown' && !empty($GLOBALS['upgradeAssumeSpace'])) { - // Only "cannot be measured" is waived. A write that runs out of - // room, or a directory that cannot be written, still stops here. - if (isset($GLOBALS['log']) && is_array($GLOBALS['log'])) { - $GLOBALS['log'][] = '[ATTENZIONE] Spazio in ' . $dir . ' non misurabile da PHP: si prosegue perché è stato dichiarato verificato a mano (richiesti ' . formatBytes($required) . ').'; + // Only "cannot be measured" is waived. A bounded dense write still + // proves the account can write right now, so a full disk or an + // exhausted quota stops here instead of halfway through the dump. + $verdict = probeWriteBytes($rootPath, 16 * 1024 * 1024, $dir); + if ($verdict === '') { + if (isset($GLOBALS['log']) && is_array($GLOBALS['log'])) { + $GLOBALS['log'][] = '[ATTENZIONE] Spazio in ' . $dir . ' non misurabile da PHP: si prosegue perché è stato dichiarato verificato a mano (richiesti ' . formatBytes($required) . ').'; + } + continue; } - continue; } if ($verdict !== '') { $reason = match ($verdict) { diff --git a/tests/manual-upgrade-space.unit.php b/tests/manual-upgrade-space.unit.php index 0b180b571..4fefa62ad 100644 --- a/tests/manual-upgrade-space.unit.php +++ b/tests/manual-upgrade-space.unit.php @@ -21,6 +21,8 @@ use RecursiveDirectoryIterator; use FilesystemIterator; function disk_free_space($path) { return $GLOBALS['cliFree']; } +// A disk that is full although PHP cannot say so: every write fails. +function fwrite($handle, $data) { return empty($GLOBALS['cliWritesFail']) ? \fwrite($handle, $data) : false; } function stat($path) { $stat = \stat($path); if ($stat !== false) { $stat['dev'] = 1; } @@ -87,6 +89,21 @@ function stat($path) { $refused = str_contains($e->getMessage(), 'insufficienti'); } $check($refused, 'the override never waives a volume that is actually full'); + // Unmeasurable AND full: the waiver still writes 16 MiB, which fails, so + // the upgrade stops before the dump instead of halfway through it. + $cliFree = false; + $GLOBALS['cliWritesFail'] = true; + $GLOBALS['log'] = []; + $refused = false; + try { + CliSpaceFixture\verifyUpgradeSpace($root, [$root . '/storage/tmp' => 200 * 1024 * 1024]); + } catch (RuntimeException $e) { + $refused = str_contains($e->getMessage(), 'insufficienti'); + } + $GLOBALS['cliWritesFail'] = false; + $check($refused, 'the override still refuses an unmeasurable volume that cannot be written'); + $check($GLOBALS['log'] === [], 'a refused waiver is not logged as verified by hand'); + $check((glob($root . '/storage/tmp/.upgrade_probe_*') ?: []) === [], 'the dense probe leaves no file behind'); $GLOBALS['upgradeAssumeSpace'] = false; } finally { unlink($fixture);