test(security): isolate helper FS tests via Config\SecurityEnv

Introduce Config\SecurityEnv holding envPath/backupPath/lockPath so the
security helper reads its target paths from shared configuration instead of
hardcoded ROOTPATH/WRITEPATH literals. security_helperTest.php now redirects
all three to a unique per-run sandbox under sys_get_temp_dir() and tears it
down in tearDown(), so the suite no longer reads/writes the repository's real
.env and is safe to run in parallel.

No helper signature changes; production callers unaffected.

Addresses CodeRabbit item 7 (issue #4700).

Co-Authored-By: opencode <bot@opencode.ai>

app/Config/SecurityEnv.php

@@ -0,0 +1,37 @@
+<?php
+
+namespace Config;
+
+use CodeIgniter\Config\BaseConfig;
+
+/**
+ * Resolves the on-disk locations the security_helper functions read/write for
+ * the runtime-generated .env secrets (encryption.key + throttle.key).
+ *
+ * Centralising these paths in a config object (mirroring Config\Encryption)
+ * lets the helper and the test suite agree on the same source of truth, and
+ * lets tests redirect all three to a per-run sandbox by mutating the shared
+ * instance — without changing any helper signature.
+ *
+ * The defaults are the production locations; tests override them in setUp().
+ */
+class SecurityEnv extends BaseConfig
+{
+    /**
+     * Path to the .env file that holds the two runtime-generated secrets.
+     */
+    public string $envPath = ROOTPATH . '.env';
+
+    /**
+     * Backup location used around encryption-key rotation so a failing
+     * conversion can be rolled back from the previous .env contents.
+     */
+    public string $backupPath = WRITEPATH . '/backup/.env.bak';
+
+    /**
+     * Dedicated mutex file for coordinating concurrent .env writes. Kept
+     * separate from .env itself because on Windows a file cannot be
+     * renamed/deleted while a handle to it is open.
+     */
+    public string $lockPath = ROOTPATH . '.env.lock';
+}

app/Helpers/security_helper.php

@@ -15,7 +15,7 @@ use Random\RandomException;
  */
 function lockEnvFile()
 {
-    $lockPath = ROOTPATH . '.env.lock';
+    $lockPath = config('SecurityEnv')->lockPath;
 
     $handle = @fopen($lockPath, 'c+');
     if ($handle === false) {
@@ -77,7 +77,7 @@ function initializeEnvFile(string $configPath): bool
  */
 function writeEnvKey(string $envKey, string $value): bool
 {
-    $configPath = ROOTPATH . '.env';
+    $configPath = config('SecurityEnv')->envPath;
 
     if (!initializeEnvFile($configPath)) {
         return false;
@@ -242,7 +242,7 @@ function writeNewEncryptionKey(string $configFile, string $key, string $oldKey):
  */
 function envFileIsWritable(): bool
 {
-    $configPath = ROOTPATH . '.env';
+    $configPath = config('SecurityEnv')->envPath;
 
     return file_exists($configPath)
         ? is_writable($configPath)
@@ -296,7 +296,7 @@ function checkEncryption(?CI3SecretConverter $converter = null): bool
         if (array_diff_assoc($plain, $converter->verifyAll($encrypted)) !== []) {
             abortEncryptionConversion();
 
-            throw new RuntimeException(lang('Error.unable_to_persist_encryption_key', ['filePath' => ROOTPATH . '.env']));
+            throw new RuntimeException(lang('Error.unable_to_persist_encryption_key', ['filePath' => config('SecurityEnv')->envPath]));
         }
 
         if (!empty(array_filter($plain))) {
@@ -368,8 +368,8 @@ function rotateEncryptionKey(?string $oldKey = null): string
     $encryption = new Encryption();
     $key = bin2hex($encryption->createKey());
 
-    $configPath = ROOTPATH . '.env';
-    $backupPath = WRITEPATH . '/backup/.env.bak';
+    $configPath = config('SecurityEnv')->envPath;
+    $backupPath = config('SecurityEnv')->backupPath;
 
     if (!initializeEnvFile($configPath)) {
         throw new RuntimeException(lang('Error.unable_to_create_env_file', ['filePath' => $configPath]));
@@ -421,7 +421,7 @@ function rotateEncryptionKey(?string $oldKey = null): string
  */
 function provisionThrottleKey(): string
 {
-    $configPath = ROOTPATH . '.env';
+    $configPath = config('SecurityEnv')->envPath;
 
     if (!initializeEnvFile($configPath)) {
         throw new RuntimeException(lang('Error.unable_to_create_env_file', ['filePath' => $configPath]));
@@ -461,7 +461,7 @@ function provisionThrottleKey(): string
     $_ENV['throttle.key'] = $key;
     $_SERVER['throttle.key'] = $key;
 
-    log_message('info', 'Provisioned throttle key in ' . ROOTPATH . '.env');
+    log_message('info', 'Provisioned throttle key in ' . config('SecurityEnv')->envPath);
 
     return $key;
 }
@@ -471,8 +471,8 @@ function provisionThrottleKey(): string
  */
 function abortEncryptionConversion(): void
 {
-    $configPath = ROOTPATH . '.env';
-    $backupPath = WRITEPATH . '/backup/.env.bak';
+    $configPath = config('SecurityEnv')->envPath;
+    $backupPath = config('SecurityEnv')->backupPath;
 
     if (!file_exists($backupPath)) {
         return;
@@ -489,7 +489,7 @@ function abortEncryptionConversion(): void
  */
 function removeBackup(): void
 {
-    $backupPath = WRITEPATH . '/backup/.env.bak';
+    $backupPath = config('SecurityEnv')->backupPath;
     if (!file_exists($backupPath)) {
         return;
     }

tests/helpers/security_helperTest.php

@@ -7,19 +7,20 @@ use Config\Encryption as EncryptionConfig;
 use Config\Services;
 
 /**
- * ROOTPATH/WRITEPATH are hard-defined constants that can't be redirected in
- * tests, so the filesystem-touching tests below operate on the project's
- * real .env and writable/backup/.env.bak. setUp()/tearDown() capture and
- * restore both files (and config('Encryption')->key and throttle.key) around
- * every test — do not remove those safeguards.
+ * The filesystem-touching helpers read their target paths from the shared
+ * Config\SecurityEnv instance. setUp()/tearDown() redirect all three
+ * (envPath / backupPath / lockPath) to a unique per-run sandbox under
+ * sys_get_temp_dir() and tear that sandbox down afterwards, so the tests never
+ * read or write the repository's real .env / writable backup. The
+ * config('Encryption')->key and throttle.key mutations are captured and
+ * restored here too — do not remove those safeguards.
  */
 class security_helperTest extends CIUnitTestCase
 {
+    private string $sandbox;
     private string $envPath;
     private string $backupPath;
     private string $lockPath;
-    private ?string $envContentsBefore;
-    private ?string $backupContentsBefore;
     private string $encryptionKeyBefore;
     private ?string $throttleKeyBefore;
     private bool $hadThrottleServer = false;
@@ -30,12 +31,29 @@ class security_helperTest extends CIUnitTestCase
         parent::setUp();
         require_once __DIR__ . '/../../app/Helpers/security_helper.php';
 
-        $this->envPath    = ROOTPATH . '.env';
-        $this->backupPath = WRITEPATH . '/backup/.env.bak';
-        $this->lockPath   = ROOTPATH . '.env.lock';
+        // Redirect all filesystem-touching helpers to a unique per-run sandbox.
+        $this->sandbox    = sys_get_temp_dir() . '/ospos_sech_' . getmypid() . '_' . bin2hex(random_bytes(4));
+        $this->envPath    = $this->sandbox . '/.env';
+        $this->backupPath = $this->sandbox . '/backup/.env.bak';
+        $this->lockPath   = $this->sandbox . '/.env.lock';
+
+        if (!is_dir($this->sandbox)) {
+            mkdir($this->sandbox, 0700, true);
+        }
+        if (!is_dir(dirname($this->backupPath))) {
+            mkdir(dirname($this->backupPath), 0700, true);
+        }
+        // Seed .env so initializeEnvFile() treats it as already present (no-op).
+        file_put_contents($this->envPath, "# OSPOS Configuration\n\n");
+
+        $se                  = config('SecurityEnv');
+        $se->envPath         = $this->envPath;
+        $se->backupPath      = $this->backupPath;
+        $se->lockPath        = $this->lockPath;
+
+        // Read-back so the helper and the assertions observe the same instance.
+        $this->assertSame($this->envPath, config('SecurityEnv')->envPath);
 
-        $this->envContentsBefore    = file_exists($this->envPath) ? file_get_contents($this->envPath) : null;
-        $this->backupContentsBefore = file_exists($this->backupPath) ? file_get_contents($this->backupPath) : null;
         $this->encryptionKeyBefore  = (string) config('Encryption')->key;
         $this->throttleKeyBefore   = (string) env('throttle.key', '');
         $this->hadThrottleServer   = array_key_exists('throttle.key', $_SERVER);
@@ -44,22 +62,13 @@ class security_helperTest extends CIUnitTestCase
 
     protected function tearDown(): void
     {
-        if ($this->envContentsBefore === null) {
-            @unlink($this->envPath);
-        } else {
-            file_put_contents($this->envPath, $this->envContentsBefore);
-        }
+        // Restore the shared config defaults so no other test sees the sandbox.
+        $se                  = config('SecurityEnv');
+        $se->envPath         = ROOTPATH . '.env';
+        $se->backupPath      = WRITEPATH . '/backup/.env.bak';
+        $se->lockPath        = ROOTPATH . '.env.lock';
 
-        if ($this->backupContentsBefore === null) {
-            @unlink($this->backupPath);
-        } else {
-            file_put_contents($this->backupPath, $this->backupContentsBefore);
-        }
-
-        @unlink($this->lockPath);
-        foreach (glob(ROOTPATH . '.env.tmp.*') as $stray) {
-            @unlink($stray);
-        }
+        $this->removeTree($this->sandbox);
 
         config('Encryption')->key = $this->encryptionKeyBefore;
 
@@ -68,6 +77,33 @@ class security_helperTest extends CIUnitTestCase
         parent::tearDown();
     }
 
+    /**
+     * Recursively removes a sandbox directory and everything beneath it.
+     * No-op when $dir does not exist.
+     */
+    private function removeTree(string $dir): void
+    {
+        if (!is_dir($dir)) {
+            return;
+        }
+
+        $items = scandir($dir) ?: [];
+        foreach ($items as $item) {
+            if ($item === '.' || $item === '..') {
+                continue;
+            }
+            $path   = $dir . '/' . $item;
+            $isTree = is_dir($path) && !is_link($path);
+            if ($isTree) {
+                $this->removeTree($path);
+            } else {
+                @unlink($path);
+            }
+        }
+
+        @rmdir($dir);
+    }
+
     private function restoreThrottleKey(): void
     {
         putenv('throttle.key');
@@ -252,7 +288,7 @@ class security_helperTest extends CIUnitTestCase
 
         $this->assertTrue($result);
         $this->assertSame('hello world', file_get_contents($this->envPath));
-        $this->assertSame([], glob(ROOTPATH . '.env.tmp.*'));
+        $this->assertSame([], glob($this->envPath . '.tmp.*'));
     }
 
     // -- envFileIsWritable() --