From 3f17ff189a89d8ae3bcf23f16fd66b7f0d3e2cd7 Mon Sep 17 00:00:00 2001 From: jekkos Date: Tue, 15 Sep 2026 17:29:23 +0000 Subject: [PATCH] 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 --- app/Config/SecurityEnv.php | 37 +++++++++++ app/Helpers/security_helper.php | 22 +++---- tests/helpers/security_helperTest.php | 92 +++++++++++++++++++-------- 3 files changed, 112 insertions(+), 39 deletions(-) create mode 100644 app/Config/SecurityEnv.php diff --git a/app/Config/SecurityEnv.php b/app/Config/SecurityEnv.php new file mode 100644 index 000000000..89c974e4b --- /dev/null +++ b/app/Config/SecurityEnv.php @@ -0,0 +1,37 @@ +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; } diff --git a/tests/helpers/security_helperTest.php b/tests/helpers/security_helperTest.php index e9c32ad1e..234521c45 100644 --- a/tests/helpers/security_helperTest.php +++ b/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() --