diff --git a/database/migrations/2026_08_01_090000_give_every_secret_a_test_slot.php b/database/migrations/2026_08_01_090000_give_every_secret_a_test_slot.php index 3a92db9..dec975a 100644 --- a/database/migrations/2026_08_01_090000_give_every_secret_a_test_slot.php +++ b/database/migrations/2026_08_01_090000_give_every_secret_a_test_slot.php @@ -64,15 +64,33 @@ return new class extends Migration public function down(): void { - foreach (DB::table('app_secrets')->get() as $row) { - if (! Str::endsWith($row->key, [':live', ':test'])) { + $rows = DB::table('app_secrets')->get(); + + // Zwei Durchläufe über dieselbe Menge, nicht einer — und die + // Reihenfolge zwischen den beiden ist die Entscheidung, nicht eine + // Nebenwirkung. `:live` geht zuerst und beansprucht den nackten + // Schlüssel bedingungslos; `:test` sieht ihn danach schon vergeben und + // weicht. Ein einziger Durchlauf, der beide Endungen im selben Schritt + // behandelt, hätte stattdessen der Zeilenreihenfolge der Datenbank + // überlassen, welcher der beiden Schlüssel überlebt — bei einem + // Zahlungsschlüssel die falsche Art von Zufall. + foreach ($rows as $row) { + if (Str::endsWith($row->key, ':live')) { + DB::table('app_secrets')->where('id', $row->id)->update(['key' => Str::beforeLast($row->key, ':')]); + } + } + + foreach ($rows as $row) { + if (! Str::endsWith($row->key, ':test')) { continue; } $bare = Str::beforeLast($row->key, ':'); // Eine Zeile je Eintrag kann zurück; die zweite hat im alten Schema - // keinen Platz und würde am Eindeutigkeitsindex scheitern. + // keinen Platz und würde am Eindeutigkeitsindex scheitern. Existiert + // der nackte Schlüssel schon, hat ihn der Live-Durchlauf oben gerade + // erst belegt — der Testwert wird verworfen, nie der Live-Wert. if (DB::table('app_secrets')->where('key', $bare)->exists()) { DB::table('app_secrets')->where('id', $row->id)->delete(); diff --git a/tests/Feature/SecretSlotMigrationTest.php b/tests/Feature/SecretSlotMigrationTest.php index 837c3f0..4af4a8e 100644 --- a/tests/Feature/SecretSlotMigrationTest.php +++ b/tests/Feature/SecretSlotMigrationTest.php @@ -18,6 +18,11 @@ function runSlotMigration(): void (require base_path('database/migrations/2026_08_01_090000_give_every_secret_a_test_slot.php'))->up(); } +function runSlotMigrationDown(): void +{ + (require base_path('database/migrations/2026_08_01_090000_give_every_secret_a_test_slot.php'))->down(); +} + it('files a stripe test key in the test slot and switches the mode', function () { DB::table('app_secrets')->insert([ 'key' => 'stripe.secret', @@ -67,3 +72,49 @@ it('leaves an installation with no stripe key on live', function () { expect(OperatingMode::current())->toBe(OperatingMode::Live); }); + +it('files an unreadable stripe value in the live slot instead of aborting', function () { + // Fix round (Codex review): the earlier version of this file happened to + // exercise this branch by accident — its fixture encrypted with the wrong + // key, so every "test key" assertion secretly ran through this catch + // instead. Fixing that fixture silently deleted the only coverage of a + // branch that decides a payment key's mode. This row is deliberately + // unreadable on purpose, not by mistake. + DB::table('app_secrets')->insert([ + 'key' => 'stripe.secret', + 'value' => 'not-a-valid-payload-at-all', + 'created_at' => now(), 'updated_at' => now(), + ]); + + runSlotMigration(); + + expect(DB::table('app_secrets')->where('key', 'stripe.secret:live')->exists())->toBeTrue(); + expect(DB::table('app_secrets')->where('key', 'stripe.secret:test')->exists())->toBeFalse(); + expect(OperatingMode::current())->toBe(OperatingMode::Live); +}); + +it('keeps the live slot on rollback when both slots are occupied, not whichever the query happens to return first', function () { + // Fix round (Codex review): down() picked whichever of the two rows a + // plain, unordered get() returned first to rename back to the bare key, + // and deleted the other — so which credential survived a rollback + // depended on row order, not on a decision. Inserting the TEST row first + // (the lower id) reproduces the old bug: an unordered scan tends to + // return rows in id order, so the test slot would have claimed the bare + // key and the live one would have been the one discarded. + DB::table('app_secrets')->insert([ + 'key' => 'stripe.secret:test', + 'value' => 'test-slot-value', + 'created_at' => now(), 'updated_at' => now(), + ]); + DB::table('app_secrets')->insert([ + 'key' => 'stripe.secret:live', + 'value' => 'live-slot-value', + 'created_at' => now(), 'updated_at' => now(), + ]); + + runSlotMigrationDown(); + + expect(DB::table('app_secrets')->where('key', 'stripe.secret')->value('value'))->toBe('live-slot-value'); + expect(DB::table('app_secrets')->where('key', 'stripe.secret:test')->exists())->toBeFalse(); + expect(DB::table('app_secrets')->where('key', 'stripe.secret:live')->exists())->toBeFalse(); +});