From 711e5d6b3fa53c64ecacd35d891634e375e2a066 Mon Sep 17 00:00:00 2001 From: jekkos Date: Mon, 21 Sep 2026 11:54:41 +0000 Subject: [PATCH] fix(security): make abortEncryptionConversion fail loudly on restore failure The rollback path restored the .env backup with a suppressed file_put_contents() and an unchecked file_get_contents(). If the restore failed after the key had already been rotated, .env was left holding the new CI4 key while the DB still held CI3-era ciphertext, so the data became undecryptable after the next restart. Now the backup read is checked for false and the restore goes through the existing atomicWriteFile() helper; either failure throws so the error is surfaced instead of silently corrupting the config. Adds a regression test that forces an unreadable backup and asserts the throw plus that .env is left untouched. --- app/Helpers/security_helper.php | 10 ++++++++-- tests/helpers/security_helperTest.php | 28 +++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/app/Helpers/security_helper.php b/app/Helpers/security_helper.php index 30d986745..862cde10b 100644 --- a/app/Helpers/security_helper.php +++ b/app/Helpers/security_helper.php @@ -572,9 +572,15 @@ function abortEncryptionConversion(): void return; } - @chmod($configPath, 0640); $configFile = file_get_contents($backupPath); - @file_put_contents($configPath, $configFile); + if ($configFile === false) { + throw new RuntimeException(lang('Error.unable_to_read_env_file', ['filePath' => $backupPath])); + } + + if (!atomicWriteFile($configPath, $configFile)) { + throw new RuntimeException(lang('Error.unable_to_persist_encryption_key', ['filePath' => $configPath])); + } + log_message('info', "Restored $configPath from backup"); } diff --git a/tests/helpers/security_helperTest.php b/tests/helpers/security_helperTest.php index 234521c45..75782e179 100644 --- a/tests/helpers/security_helperTest.php +++ b/tests/helpers/security_helperTest.php @@ -564,6 +564,34 @@ class security_helperTest extends CIUnitTestCase $this->assertSame("encryption.key='old'\n", file_get_contents($this->envPath)); } + public function testAbortEncryptionConversionThrowsWhenBackupUnreadable(): void + { + // Force a failed backup read: file_exists() is true (it is a directory) + // but file_get_contents() returns false. The restore must then throw + // instead of silently writing an empty .env and destroying the active key. + if (!is_dir(dirname($this->backupPath))) { + mkdir(dirname($this->backupPath), 0750, true); + } + @unlink($this->backupPath); + mkdir($this->backupPath); + + file_put_contents($this->envPath, "encryption.key='new'\n"); + $before = (string) file_get_contents($this->envPath); + + $threw = false; + + try { + abortEncryptionConversion(); + } catch (RuntimeException $e) { + $threw = true; + } finally { + @rmdir($this->backupPath); + } + + $this->assertTrue($threw, 'a failed backup read must throw instead of failing silently'); + $this->assertSame($before, (string) file_get_contents($this->envPath), '.env must be left untouched when the backup is unreadable'); + } + public function testRemoveBackupDeletesBackupFile(): void { if (!is_dir(dirname($this->backupPath))) {