diff --git a/app/Services/Deployment/UpdateChannel.php b/app/Services/Deployment/UpdateChannel.php index 97d063e..80dc2fe 100644 --- a/app/Services/Deployment/UpdateChannel.php +++ b/app/Services/Deployment/UpdateChannel.php @@ -593,8 +593,10 @@ final class UpdateChannel /** * Den Server auf eine Version festnageln — oder die Decke abnehmen. * - * `null` nimmt sie ab. Rückgabe `false` heißt: die Form stimmt nicht, es - * wurde nichts geschrieben. + * `null` nimmt sie ab. Rückgabe `false` heißt: die Form stimmt nicht, oder + * das Schreiben ist fehlgeschlagen (volle Platte, Rechteproblem auf + * storage/app/deploy) — in beiden Fällen wurde nichts Halbes hinterlassen, + * das der Agent falsch lesen müsste. * * Die Decke wirkt NICHT dadurch, dass hier etwas ausgelöst wird. Der Agent * liest die Datei bei jedem Takt und rechnet `behind` dagegen; Knopf und @@ -619,7 +621,10 @@ final class UpdateChannel return false; } - $this->writeAtomic(self::CEILING, $tag); + if (! $this->writeAtomic(self::CEILING, $tag)) { + return false; + } + $this->requestCheck($by); return true; @@ -631,18 +636,28 @@ final class UpdateChannel * Nicht aus der Statusdatei des Agenten: die ist seine Rückmeldung und * hinkt bis zu einem Takt hinterher. Was die Konsole selbst geschrieben * hat, muss sie sofort anzeigen können. + * + * Zwischen `File::exists()` und `File::get()` liegt ein Zeitfenster — + * verschwindet die Datei genau dort (ein zweiter Aufruf nimmt die Decke + * ab, während dieser noch liest), wirft `File::get()` eine + * FileNotFoundException. Dasselbe Muster wie readJson() und lastLog() + * weiter unten: state() darf unter keinen Umständen werfen. */ private function ceiling(): ?string { - $path = storage_path('app/'.self::CEILING); + try { + $path = storage_path('app/'.self::CEILING); - if (! File::exists($path)) { + if (! File::exists($path)) { + return null; + } + + $tag = trim((string) File::get($path)); + + return $tag === '' ? null : $tag; + } catch (Throwable) { return null; } - - $tag = trim((string) File::get($path)); - - return $tag === '' ? null : $tag; } /** @@ -651,13 +666,34 @@ final class UpdateChannel * `write()` daneben benutzt File::put und taugt dafür nicht: der Agent * liest diese Datei bei jedem Takt, und eine halbe Zeile ist genau die * Art Decke, die er als kaputt ablehnen müsste. + * + * Prüft beide Rückgabewerte, statt ihnen zu glauben: `File::move()` ruft + * nur `rename()` auf, und ein fehlgeschlagenes `rename()` liefert `false` + * ohne von sich aus eine Ausnahme zu werfen. Schlägt einer der beiden + * Schritte fehl — volle Platte, Rechteproblem — räumt diese Methode die + * `.tmp` weg und meldet `false`, statt einen Erfolg zu behaupten, der + * nicht stattfand. */ - private function writeAtomic(string $relative, string $contents): void + private function writeAtomic(string $relative, string $contents): bool { $path = storage_path('app/'.$relative); - File::ensureDirectoryExists(dirname($path)); - File::put($path.'.tmp', $contents); - File::move($path.'.tmp', $path); + $tmp = $path.'.tmp'; + + try { + File::ensureDirectoryExists(dirname($path)); + + if (File::put($tmp, $contents) === false || ! File::move($tmp, $path)) { + File::delete($tmp); + + return false; + } + + return true; + } catch (Throwable) { + File::delete($tmp); + + return false; + } } /** diff --git a/tests/Feature/ReleaseCeilingConsoleTest.php b/tests/Feature/ReleaseCeilingConsoleTest.php index c381314..81f9c08 100644 --- a/tests/Feature/ReleaseCeilingConsoleTest.php +++ b/tests/Feature/ReleaseCeilingConsoleTest.php @@ -81,3 +81,44 @@ it('does not mark a ceiling that is still ahead', function () { expect(app(UpdateChannel::class)->state()['ceiling_passed'])->toBeFalse(); }); + +it('does not throw when the ceiling file exists but cannot be read as a file', function () { + // Kein echtes Wettrennen im Test moeglich, aber derselbe Codepfad wie ein + // Verschwinden zwischen File::exists() und File::get(): ein Verzeichnis + // existiert (File::exists() prueft file_exists(), das ist fuer + // Verzeichnisse wahr), ist aber keine Datei (File::get() prueft + // isFile() und wirft eine FileNotFoundException). Ohne Absicherung + // wuerfe state() genau hier — die Vorgabe, die es unter keinen + // Umstaenden darf. + // + // Bewusst kein expect(fn () => ...)->not->toThrow(Throwable::class): + // Throwable ist ein Interface, class_exists() verneint es, und die + // Pruefung faellt intern auf einen Substring-Vergleich zurueck, der + // nichts mehr ueber ein tatsaechliches Werfen aussagt. Ein direkter + // Aufruf laesst eine durchschlagende Exception den Test regulaer als + // fehlgeschlagen melden — das ist der verlaessliche Nachweis. + File::ensureDirectoryExists(storage_path('app/deploy/release-ceiling')); + + $state = app(UpdateChannel::class)->state(); + + expect($state['ceiling'])->toBeNull(); +}); + +it('does not claim success when writing the ceiling fails', function () { + // Kein File-Facade-Mock: eine echte Rechteverweigerung auf einem + // Verzeichnis, das dieser (nicht-root) Testbenutzer selbst besitzt. + // File::put() auf die .tmp darin schlaegt wirklich fehl, nicht nur dem + // Namen nach. + $deployDir = storage_path('app/deploy'); + chmod($deployDir, 0500); + + try { + $result = app(UpdateChannel::class)->setCeiling('chef@example.com', 'v1.8.0'); + } finally { + chmod($deployDir, 0755); + } + + expect($result)->toBeFalse() + ->and(File::exists($deployDir.'/release-ceiling.tmp'))->toBeFalse() + ->and(File::exists($deployDir.'/release-ceiling'))->toBeFalse(); +});