boban
/
CluPilotCloud
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Cover the unreadable branch and make rollback pick live deterministically 

The only test that reached SecretCipher::decrypt() failing was an accident of
a fixture bug fixed in the previous commit: it encrypted with the wrong key,
so decryption threw and the migration's catch(Throwable) branch ran without
anyone meaning to test it. Fixing that fixture silently removed the only
coverage of a branch that decides a payment key's mode, so it gets its own
test with a deliberately unreadable row.

down() picked whichever of the :live/:test rows an unordered get() happened to
return first to restore to the bare key, and deleted the other — so which
credential survived a rollback depended on row order, not a decision. It now
processes :live rows first unconditionally, then :test rows only where the
bare key is still free, so the live credential always wins when both exist.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
feature/betriebsmodus
nexxo 2026-07-30 11:32:58 +02:00
parent 727f2e41db
commit ac44717d1b
2 changed files with 72 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
database/migrations/2026_08_01_090000_give_every_secret_a_test_slot.php
View File

@ -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();

51
tests/Feature/SecretSlotMigrationTest.php
View File

@ -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();
});