From bf5af2f2dc5aaa2b730b65cac0d70628baaa301a Mon Sep 17 00:00:00 2001 From: Ollama Date: Fri, 22 May 2026 14:57:37 +0200 Subject: [PATCH] fix: address CodeRabbit review comments for encryption key persistence - Always mirror encryption key to both .env and WRITEPATH (Docker safety) - Guard array key access with isset() before reading in Encryption.php - Fix encrypt_value() to not treat string '0' as empty - Improve error logging for failed encryption attempts --- app/Config/Encryption.php | 1 + app/Controllers/Config.php | 2 +- app/Helpers/security_helper.php | 16 ++++++---------- 3 files changed, 8 insertions(+), 11 deletions(-) diff --git a/app/Config/Encryption.php b/app/Config/Encryption.php index a70f115ab..1a16f9f00 100644 --- a/app/Config/Encryption.php +++ b/app/Config/Encryption.php @@ -147,6 +147,7 @@ class Encryption extends BaseConfig $data = json_decode($content, true); if ( !is_array($data) + || !isset($data['key']) || !is_string($data['key']) || strlen($data['key']) < 64 ) { diff --git a/app/Controllers/Config.php b/app/Controllers/Config.php index 261f70a00..2344a9698 100644 --- a/app/Controllers/Config.php +++ b/app/Controllers/Config.php @@ -493,7 +493,7 @@ class Config extends Secure_Controller if (!empty($passwordInput)) { $password = encrypt_value($passwordInput); if (empty($password)) { - log_message('error', 'SMTP password encryption failed'); + log_message('error', 'SMTP password encryption failed - credentials not saved'); return $this->response->setJSON([ 'success' => false, diff --git a/app/Helpers/security_helper.php b/app/Helpers/security_helper.php index 2de5ce95b..148a04d3d 100644 --- a/app/Helpers/security_helper.php +++ b/app/Helpers/security_helper.php @@ -28,15 +28,11 @@ function check_encryption(): bool config('Encryption')->key = $key; // Try to persist the key - attempt multiple locations - $persisted = false; - - // Attempt 1: ROOTPATH/.env (standard location) - $persisted = write_encryption_key_to_env($key, $old_key); - - // Attempt 2: WRITEPATH/config/encryption.key (Docker/container fallback) - if (!$persisted) { - $persisted = write_encryption_key_to_writable($key, $old_key); - } + // Write both locations when possible. The writable copy is the durable one + // in containerized deployments where .env may be ephemeral. + $envPersisted = write_encryption_key_to_env($key, $old_key); + $writablePersisted = write_encryption_key_to_writable($key, $old_key); + $persisted = $envPersisted || $writablePersisted; if ($persisted) { log_message('info', 'Encryption key initialized successfully'); @@ -304,7 +300,7 @@ function decrypt_value(?string $encrypted_value, string $default = ''): string */ function encrypt_value(?string $value, bool $require = true): string { - if (empty($value)) { + if ($value === null || $value === '') { return ''; }