From 0a4eac597c5d627929cd6e5fb4d82b38eecd1258 Mon Sep 17 00:00:00 2001
From: Ollama <ollama@steganos.dev>
Date: Wed, 3 Jun 2026 20:47:41 +0200
Subject: [PATCH] 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
---
 .../20220127000000_convert_to_ci4.php         | 10 ++++++++--
 app/Helpers/security_helper.php               | 20 +++++++++++++------
 2 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/app/Database/Migrations/20220127000000_convert_to_ci4.php b/app/Database/Migrations/20220127000000_convert_to_ci4.php
index ad4b7366b..6604fc751 100644
--- a/app/Database/Migrations/20220127000000_convert_to_ci4.php
+++ b/app/Database/Migrations/20220127000000_convert_to_ci4.php
@@ -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);
diff --git a/app/Helpers/security_helper.php b/app/Helpers/security_helper.php
index e2c8e7a7a..12f8f0c2b 100644
--- a/app/Helpers/security_helper.php
+++ b/app/Helpers/security_helper.php
@@ -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;
     }