Address remaining CodeRabbit review comments

- Fix decryptValue() to use explicit null/empty check instead of empty()
  (handles string "0" correctly)
- Guard checkEncryption() result in migration before proceeding
- Check read success before writing backup restoration
- Consistent DIRECTORY_SEPARATOR usage in paths

GitHub-Issue: #4554
This commit is contained in:
Ollama
2026-06-03 20:47:41 +02:00
parent 8735c0765b
commit 0a4eac597c
2 changed files with 22 additions and 8 deletions

View File

@@ -33,7 +33,9 @@ class Convert_to_ci4 extends Migration
if (!empty(config('Encryption')->key)) {
$this->convert_ci3_encrypted_data();
} else {
checkEncryption();
if (!checkEncryption()) {
throw new \RuntimeException('Unable to persist encryption key. Migration cannot proceed without encryption.');
}
}
removeBackup();
@@ -66,7 +68,11 @@ class Convert_to_ci4 extends Migration
$decrypted_data = $this->decrypt_ci3_data($ci3_encrypted_data);
checkEncryption();
if (!checkEncryption()) {
abortEncryptionConversion();
removeBackup();
throw new RedirectException('login');
}
try {
$ci4_encrypted_data = $this->encrypt_data($decrypted_data);

View File

@@ -214,16 +214,24 @@ function loadEncryptionKeyFromWritable(): ?string
function abortEncryptionConversion(): void
{
$configPath = ROOTPATH . '.env';
$backupPath = WRITEPATH . '/backup/.env.bak';
$backupPath = WRITEPATH . 'backup' . DIRECTORY_SEPARATOR . '.env.bak';
if (!file_exists($backupPath)) {
return;
}
@chmod($configPath, 0640);
$configFile = file_get_contents($backupPath);
@file_put_contents($configPath, $configFile);
log_message('info', "Restored $configPath from backup");
if ($configFile === false) {
log_message('error', 'Could not read backup file for restoration');
return;
}
if (file_put_contents($configPath, $configFile) !== false) {
@chmod($configPath, 0640);
log_message('info', "Restored $configPath from backup");
} else {
log_message('error', "Failed to restore $configPath from backup");
}
}
/**
@@ -233,7 +241,7 @@ function abortEncryptionConversion(): void
*/
function removeBackup(): void
{
$backupPath = WRITEPATH . '/backup/.env.bak';
$backupPath = WRITEPATH . 'backup' . DIRECTORY_SEPARATOR . '.env.bak';
if (file_exists($backupPath)) {
unlink($backupPath);
@@ -253,7 +261,7 @@ function removeBackup(): void
*/
function decryptValue(?string $encryptedValue, string $default = ''): string
{
if (empty($encryptedValue)) {
if ($encryptedValue === null || $encryptedValue === '') {
return $default;
}