Merge remote-tracking branch 'OpensourcePOS/master' into feat-job-queue-phase-1

CHANGELOG.md

@@ -34,7 +34,7 @@ All notable changes to this project will be documented in this file.
 
 ## [Unreleased]
 
-## [3.4.2] - 2026-09-24
+## [3.4.2] - 2026-09-30
 - Fix writable folder permission check (#4270) (#4273) by @jekkos
 - Extended payment delete fix (#4274) by @jekkos
 - Upgrade github workflow (#3708) (#4280) by @jekkos
@@ -262,6 +262,14 @@ All notable changes to this project will be documented in this file.
 - fix(i18n): swap print_delay_autoreturn number/required messages in 5 locales (#4699) by @Rayan Abdul Cader
 - chore(release): unified git-cliff release workflow (changelog + tag + optional bump) (#4711) by @jekkos
 - fix(release): push changelog/bump to master via admin PAT (GITHUB_TOKEN blocked by branch protection) by @jekkos
+- docs: add 3.4.2 changelog by @github-actions[bot]
+- chore: bump version to 3.4.3 by @github-actions[bot]
+- fix(security): HTML-escape attribute dropdown option labels in items attributes view (#4715) by @jekkos
+- fix(release): keep package-lock.json version in sync on bump (#4718) by @jekkos
+- fix(security): strip all HTML tags from $.notify alert messages (#4716) by @jekkos
+- chore: strip advisory IDs from code comments and changelog (#4720) by @jekkos
+- fix(security): report unwritable .env.lock, make throttle limits configurable (#4714) by @jekkos
+- chore: reset 3.4.2 (undo premature 3.4.3 bump + stale changelog) for re-cut by @jekkos
 
 ## [3.4.1] - 2025-06-05
 - Feature: PSR-12 Compliant Indentation by @objecttothis in ([#4196](https://github.com/opensourcepos/opensourcepos/pull/4196))

app/Config/App.php

@@ -307,7 +307,7 @@ class App extends BaseConfig
     /**
      * Validates and returns a trusted hostname.
      *
-     * Security: Prevents Host Header Injection attacks (GHSA-jchf-7hr6-h4f3)
+     * Security: Prevents Host Header Injection attacks
      * by validating the HTTP_HOST against a whitelist of allowed hostnames.
      *
      * In production: Fails fast if allowedHostnames is not configured.

app/Config/Encryption.php

@@ -112,8 +112,47 @@ class Encryption extends BaseConfig
         parent::__construct();
 
         if ($this->key === '') {
-            $envKey = getenv('ENCRYPTION_KEY');
-            $this->key = $envKey === false ? '' : $envKey;
+            // Fallback sources (notably the ENCRYPTION_KEY Docker var, which the
+            // parent never reads) were not decode-parsed, so run them through
+            // the same parser to keep hex2bin:/base64: keys consistent.
+            $this->key = self::parseKey(self::resolveKey(
+                (string) ($_SERVER['encryption.key'] ?? ''),
+                (string) ($_ENV['encryption.key'] ?? ''),
+                (string) getenv('encryption.key'),
+                (string) getenv('ENCRYPTION_KEY'),
+            ));
         }
     }
+
+    /**
+     * Decode a key's `hex2bin:`/`base64:` prefix, mirroring
+     * BaseConfig::parseEncryptionKey(); kept static so it is unit-testable.
+     */
+    public static function parseKey(string $key): string
+    {
+        if (str_starts_with($key, 'hex2bin:')) {
+            return (string) hex2bin(substr($key, 8));
+        }
+
+        if (str_starts_with($key, 'base64:')) {
+            return (string) base64_decode(substr($key, 7), true);
+        }
+
+        return $key;
+    }
+
+    /**
+     * Return the first non-empty source (highest precedence first). Cascading
+     * past empty strings (vs `??`) avoids a blank value shadowing the real key.
+     */
+    public static function resolveKey(string ...$sources): string
+    {
+        foreach ($sources as $source) {
+            if ($source !== '') {
+                return $source;
+            }
+        }
+
+        return '';
+    }
 }

app/Database/Migrations/20220127000000_convertToCI4.php

@@ -7,6 +7,7 @@ use CodeIgniter\Database\Exceptions\DatabaseException;
 use CodeIgniter\Database\Forge;
 use CodeIgniter\Database\Migration;
 use CodeIgniter\HTTP\Exceptions\RedirectException;
+use RuntimeException;
 
 class ConvertToCI4 extends Migration
 {
@@ -32,18 +33,31 @@ class ConvertToCI4 extends Migration
 
         $existingKey = (string) config('Encryption')->key;
 
+        // A valid CI4 key requires no write — just confirm it is usable.
+        if ($existingKey !== '' && strlen($existingKey) >= 64) {
+            checkEncryption();
+
+            return;
+        }
+
+        // Every branch below writes to .env. If the runtime user cannot write
+        // it (e.g. Docker/Compose with a read-only .env mount), fail with an
+        // actionable message instead of a raw fopen() error deep in the writer.
+        if (!envFileIsWritable()) {
+            log_message('critical', 'Encryption key not provisioned and .env is not writable. Run `php spark env:provision` to generate one.');
+
+            throw new RuntimeException(lang('Error.encryption_key_not_provisioned'));
+        }
+
         if ($existingKey !== '' && strlen($existingKey) < 64) {
             // Old CI3-era key: decrypt, rotate, re-encrypt, persist — all under
             // a single .env lock (see convertCI3EncryptedData).
             $this->convertCI3EncryptedData($existingKey);
-        } elseif ($existingKey === '') {
+        } else {
             // No key at all: provision a fresh one (single atomic write), then
             // drop the incidental pre-write backup left behind by the rotation.
             rotateEncryptionKey(null);
             removeBackup();
-        } else {
-            // Key already present and a valid CI4 key: confirm it is usable.
-            checkEncryption();
         }
     }
 

app/Filters/Throttle.php

@@ -8,22 +8,37 @@ use CodeIgniter\HTTP\ResponseInterface;
 use Config\Services;
 
 /**
- * Rate limits login/migrate POST attempts, keyed by IP and by submitted
- * username, to mitigate brute-force and credential-stuffing attacks
- * (GHSA-hm9c-xchj-xgcp). Backed by CodeIgniter's cache-based Throttler,
- * so limits are per-server (not shared across nodes on file cache).
+ * Rate limits login/migrate POST attempts by IP and submitted username to
+ * mitigate brute-force/credential-stuffing. Backed by CodeIgniter's
+ * cache-based Throttler, so limits are per-server (not shared on file cache).
+ *
+ * Tunable via `throttle.capacity` (default 5; 0 disables) and
+ * `throttle.seconds` (window, default 60) in .env.
  */
 class Throttle implements FilterInterface
 {
-    private const CAPACITY = 5;
-    private const SECONDS  = 60;
-
     public function before(RequestInterface $request, $arguments = null)
     {
         if ($request->getMethod() !== 'POST') {
             return null;
         }
 
+        // Non-positive integer = explicit disable; missing/non-numeric falls
+        // back to the default so a typo (e.g. "five") cannot bypass lockout.
+        $capacity = filter_var(env('throttle.capacity'), FILTER_VALIDATE_INT);
+        if ($capacity === false) {
+            $capacity = 5;
+        }
+        if ($capacity <= 0) {
+            return null;
+        }
+
+        $seconds = filter_var(env('throttle.seconds'), FILTER_VALIDATE_INT);
+        if ($seconds === false) {
+            $seconds = 60;
+        }
+        $seconds = max(1, $seconds);
+
         helper('security');
 
         $throttler = Services::throttler();
@@ -34,8 +49,8 @@ class Throttle implements FilterInterface
         $username    = is_scalar($rawUsername) ? strtolower((string) $rawUsername) : '';
         $usernameKey = $username !== '' ? 'login-user-' . hash_hmac('sha256', $username, $secret) : null;
 
-        $ipOk       = $throttler->check($ipKey, self::CAPACITY, self::SECONDS);
-        $usernameOk = $usernameKey === null || $throttler->check($usernameKey, self::CAPACITY, self::SECONDS);
+        $ipOk       = $throttler->check($ipKey, $capacity, $seconds);
+        $usernameOk = $usernameKey === null || $throttler->check($usernameKey, $capacity, $seconds);
 
         if (!$ipOk || !$usernameOk) {
             log_message('warning', 'Login throttled for IP {ip} (username: {username})', [

app/Helpers/security_helper.php

@@ -236,17 +236,38 @@ function writeNewEncryptionKey(string $configFile, string $key, string $oldKey):
 /**
  * Returns true when the current process can write to (or create) .env.
  *
- * A missing .env file is considered writable when the directory is writable.
+ * The write path (temp file + lock, then rename) needs write permission on
+ * the DIRECTORY, not on .env itself — a bind-mounted .env can be writable
+ * while the dir (or a root-owned .env.lock) is not, so .env's own mode is not
+ * a reliable signal.
  *
  * @return bool
  */
 function envFileIsWritable(): bool
 {
     $configPath = config('SecurityEnv')->envPath;
+    $lockPath   = config('SecurityEnv')->lockPath;
+    $dir        = dirname($configPath);
 
-    return file_exists($configPath)
-        ? is_writable($configPath)
-        : is_writable(dirname($configPath));
+    if (!is_writable($dir)) {
+        return false;
+    }
+
+    // Windows-only: rename() can't replace a read-only destination, so an
+    // existing .env must itself be writable (POSIX rename() can, if the dir is).
+    if (PHP_OS_FAMILY === 'Windows'
+        && file_exists($configPath)
+        && !is_writable($configPath)
+    ) {
+        return false;
+    }
+
+    // An existing mutex file (e.g. left by a prior root env:provision) must be writable.
+    if (file_exists($lockPath) && !is_writable($lockPath)) {
+        return false;
+    }
+
+    return true;
 }
 
 /**

app/Models/Item.php

@@ -529,7 +529,7 @@ class Item extends Model
      */
     public function updateMultiple(array $itemData, string $itemIds): bool
     {
-        // Query Builder bypasses $allowedFields, so the whitelist is enforced here (GHSA-49mq-h2g4-grr9)
+        // Query Builder bypasses $allowedFields, so the whitelist is enforced here
         $itemData = array_intersect_key($itemData, array_flip(self::ALLOWED_BULK_EDIT_FIELDS));
 
         if (empty($itemData)) {

tests/Config/EncryptionTest.php

@@ -0,0 +1,54 @@
+<?php
+
+namespace Tests\Config;
+
+use CodeIgniter\Test\CIUnitTestCase;
+use Config\Encryption;
+
+/**
+ * Pure-function tests for the app Encryption config's key helpers
+ * (resolveKey() / parseKey()) — no global env mutation, so no cross-test leaks.
+ */
+class EncryptionTest extends CIUnitTestCase
+{
+    public function testHighestPrecedenceNonEmptySourceWins(): void
+    {
+        $this->assertSame(
+            'server-key',
+            Encryption::resolveKey('server-key', 'env-key', 'getenv-key', 'docker-key')
+        );
+    }
+
+    public function testEmptyStringDoesNotShadowLaterSource(): void
+    {
+        // A `??`-based lookup would stop at a higher-precedence empty string;
+        // the cascade must skip empties and reach the real key.
+        $this->assertSame(
+            'getenv-key',
+            Encryption::resolveKey('', '', 'getenv-key', 'docker-key')
+        );
+
+        $this->assertSame(
+            'docker-key',
+            Encryption::resolveKey('', '', '', 'docker-key')
+        );
+    }
+
+    public function testAllEmptySourcesReturnEmptyString(): void
+    {
+        $this->assertSame('', Encryption::resolveKey('', '', '', ''));
+    }
+
+    public function testPrefixedFallbackKeyIsDecoded(): void
+    {
+        // Fallback-selected keys (notably ENCRYPTION_KEY, which BaseConfig
+        // never reads) must be decoded like BaseConfig does, so a
+        // hex2bin:/base64:-prefixed value is not stored verbatim.
+        $this->assertSame("\xab\xcd", Encryption::parseKey('hex2bin:abcd'));
+        $this->assertSame("\x68\x65\x6c\x6c\x6f", Encryption::parseKey('base64:aGVsbG8='));
+
+        // No prefix / empty value passes through unchanged.
+        $this->assertSame('plain-key', Encryption::parseKey('plain-key'));
+        $this->assertSame('', Encryption::parseKey(''));
+    }
+}

tests/Controllers/HomeTest.php

@@ -303,7 +303,7 @@ class HomeTest extends CIUnitTestCase
 
     /**
      * Test non-admin cannot view admin password change form
-     * BOLA vulnerability fix: GHSA-q58g-gg7v-f9rf
+     * BOLA vulnerability fix.
      *
      * @return void
      */
@@ -319,7 +319,7 @@ class HomeTest extends CIUnitTestCase
 
     /**
      * Test non-admin cannot change admin password
-     * BOLA vulnerability fix: GHSA-q58g-gg7v-f9rf
+     * BOLA vulnerability fix.
      *
      * @return void
      */
@@ -448,7 +448,7 @@ class HomeTest extends CIUnitTestCase
 
     /**
      * Test non-admin cannot view another non-admin's password form
-     * IDOR vulnerability fix: GHSA-mcc2-8rp2-q6ch
+     * IDOR vulnerability fix.
      *
      * @return void
      */
@@ -470,7 +470,7 @@ class HomeTest extends CIUnitTestCase
 
     /**
      * Test non-admin cannot change another non-admin's password
-     * IDOR vulnerability fix: GHSA-mcc2-8rp2-q6ch
+     * IDOR vulnerability fix.
      *
      * @return void
      */
@@ -503,7 +503,7 @@ class HomeTest extends CIUnitTestCase
     }
 
     /**
-     * Regression test for GHSA-9gr6-4mm4-4wrq: Home::__construct() previously
+     * Regression test: Home::__construct() previously
      * read the raw (single-decoded) URI segment to decide whether to skip
      * Secure_Controller's module-grant check for 'logout'. A route whose
      * double-decoded method name resolves to 'logout' must still be treated

tests/Controllers/ItemKitsControllerTest.php

@@ -111,7 +111,7 @@ class ItemKitsControllerTest extends CIUnitTestCase
         $itemKitId = $this->createItemKit();
         $this->loginAsAdmin();
 
-        // <svg onload=alert(document.domain)> URL-encoded three times (GHSA-3vpv-jqr3-7256 PoC).
+        // <svg onload=alert(document.domain)> URL-encoded three times.
         // The framework's router decodes this twice before routing; the controller used to apply
         // a third urldecode(), turning the remaining %3C.../%3E into a live <svg onload=...> tag.
         // With that urldecode() removed, the value must stay percent-encoded text and never

tests/Controllers/ItemsControllerTest.php

@@ -103,7 +103,7 @@ class ItemsControllerTest extends CIUnitTestCase
     }
 
     /**
-     * Regression test for GHSA-92cx-fc8x-7wmm: `tax_names[]` containing `<`/`>`
+     * Regression test: `tax_names[]` containing `<`/`>`
      * (the stored-XSS vector) must be rejected by postSave.
      */
     public function testPostSaveRejectsMaliciousTaxName(): void
@@ -190,7 +190,7 @@ class ItemsControllerTest extends CIUnitTestCase
     }
 
     /**
-     * Regression test for GHSA-cm7j-957q-8pgg: an attribute definition whose
+     * Regression test: an attribute definition whose
      * `definition_name` contains HTML must be entity-escaped when rendered in the
      * items attributes dropdown, not emitted as a live (executable) tag.
      */

tests/Controllers/LoginTest.php

@@ -9,7 +9,7 @@ use CodeIgniter\Test\FeatureTestTrait;
 
 /**
  * Test suite for the Login controller, including the CI Throttler
- * mitigation for brute-force/credential-stuffing (GHSA-hm9c-xchj-xgcp).
+ * mitigation for brute-force/credential-stuffing.
  */
 class LoginTest extends CIUnitTestCase
 {

tests/Controllers/ReportsControllerTest.php

@@ -10,7 +10,7 @@ use App\Models\Employee;
 use Config\OSPOS;
 
 /**
- * Regression tests for GHSA-9gr6-4mm4-4wrq
+ * Regression tests for the reports permission bypass
  *
  * Reports::__construct() previously derived the report method name from
  * $request->getUri()->getSegment(2), which CodeIgniter decodes once, while

tests/Controllers/SalesControllerTest.php

@@ -14,7 +14,7 @@ use Tests\Support\EmployeeFixtureTrait;
 use Tests\Support\SaleFixtureTrait;
 
 /**
- * Regression tests for GHSA-3xf6-8fmq-44wg.
+ * Regression tests for the Sales per-endpoint access-control bypass.
  *
  * A cashier holding only the base "sales" grant (no "reports_sales") must
  * not be able to reach the per-sale endpoints that getManage() gates

tests/Filters/ThrottleTest.php

@@ -161,4 +161,102 @@ class ThrottleTest extends CIUnitTestCase
         $this->assertNotNull($result);
         $this->assertSame(429, $result->getStatusCode());
     }
+
+    public function testCustomCapacityIsHonored(): void
+    {
+        $ip     = '203.0.113.7';
+        $prev   = $this->captureEnv('throttle.capacity');
+
+        $this->putEnv('throttle.capacity', '2');
+
+        try {
+            $this->assertNull($this->filter->before($this->makeRequest('POST', $ip, 'c1')));
+            $this->assertNull($this->filter->before($this->makeRequest('POST', $ip, 'c2')));
+
+            $result = $this->filter->before($this->makeRequest('POST', $ip, 'c3'));
+
+            $this->assertNotNull($result);
+            $this->assertSame(429, $result->getStatusCode());
+        } finally {
+            $this->restoreEnv('throttle.capacity', $prev);
+        }
+    }
+
+    public function testZeroCapacityDisablesThrottling(): void
+    {
+        $ip   = '203.0.113.8';
+        $prev = $this->captureEnv('throttle.capacity');
+
+        $this->putEnv('throttle.capacity', '0');
+
+        try {
+            for ($i = 0; $i < 10; $i++) {
+                $result = $this->filter->before($this->makeRequest('POST', $ip, "z{$i}"));
+                $this->assertNull($result, "Attempt {$i} should not be throttled when disabled");
+            }
+        } finally {
+            $this->restoreEnv('throttle.capacity', $prev);
+        }
+    }
+
+    public function testInvalidCapacityFallsBackToDefault(): void
+    {
+        // A non-numeric value must fall back to the default (5), not disable.
+        $ip   = '203.0.113.9';
+        $prev = $this->captureEnv('throttle.capacity');
+
+        $this->putEnv('throttle.capacity', 'five');
+
+        try {
+            for ($i = 0; $i < 5; $i++) {
+                $this->assertNull($this->filter->before($this->makeRequest('POST', $ip, "v{$i}")));
+            }
+
+            // 6th attempt exceeds the default capacity of 5.
+            $result = $this->filter->before($this->makeRequest('POST', $ip, 'v6'));
+            $this->assertNotNull($result);
+            $this->assertSame(429, $result->getStatusCode());
+        } finally {
+            $this->restoreEnv('throttle.capacity', $prev);
+        }
+    }
+
+    private function captureEnv(string $key): array
+    {
+        return [
+            'putenv'  => getenv($key),
+            'hasENV'  => array_key_exists($key, $_ENV),
+            'ENV'     => $_ENV[$key] ?? null,
+            'hasSRV'  => array_key_exists($key, $_SERVER),
+            'SERVER'  => $_SERVER[$key] ?? null,
+        ];
+    }
+
+    private function putEnv(string $key, string $value): void
+    {
+        putenv("{$key}={$value}");
+        $_ENV[$key]    = $value;
+        $_SERVER[$key] = $value;
+    }
+
+    private function restoreEnv(string $key, array $prev): void
+    {
+        if ($prev['putenv'] === false) {
+            putenv($key);
+        } else {
+            putenv("{$key}={$prev['putenv']}");
+        }
+
+        if ($prev['hasENV']) {
+            $_ENV[$key] = $prev['ENV'];
+        } else {
+            unset($_ENV[$key]);
+        }
+
+        if ($prev['hasSRV']) {
+            $_SERVER[$key] = $prev['SERVER'];
+        } else {
+            unset($_SERVER[$key]);
+        }
+    }
 }

tests/Models/CustomerRewardPointsTest.php

@@ -9,7 +9,7 @@ use Config\Database;
 use Tests\Support\ConcurrentDbRaceTrait;
 
 /**
- * Regression tests for GHSA-995p-52qw-5hh2: adjustRewardPoints() must apply
+ * Regression tests: adjustRewardPoints() must apply
  * its balance check and its write in a single atomic UPDATE, so that two
  * concurrent reward-point spends against the same customer can never both
  * read the same stale balance and double-spend it.

tests/Models/GiftcardTest.php

@@ -9,7 +9,7 @@ use Config\Database;
 use Tests\Support\ConcurrentDbRaceTrait;
 
 /**
- * Regression tests for GHSA-995p-52qw-5hh2: decrementGiftcardValue() must
+ * Regression tests: decrementGiftcardValue() must
  * apply its balance check and its write in a single atomic UPDATE, so that
  * two concurrent decrements against the same gift card can never both read
  * the same stale balance and double-spend it.

tests/Models/ItemBulkUpdateTest.php

@@ -7,7 +7,7 @@ use CodeIgniter\Test\CIUnitTestCase;
 use CodeIgniter\Test\DatabaseTestTrait;
 
 /**
- * Regression coverage for GHSA-49mq-h2g4-grr9 (mass assignment in bulk edit).
+ * Regression coverage for mass assignment in bulk edit.
  *
  * Item::update_multiple() writes through the Query Builder, which bypasses the
  * model's $allowedFields, so these assertions go straight to the items table.

tests/Models/ItemQuantityTest.php

@@ -10,7 +10,7 @@ use Tests\Support\ConcurrentDbRaceTrait;
 use Tests\Support\ItemFixtureTrait;
 
 /**
- * Regression tests for GHSA-995p-52qw-5hh2: changeQuantity() must apply
+ * Regression tests: changeQuantity() must apply
  * its write in a single atomic upsert, so that two concurrent sales of the
  * same item/location can never both read the same stale quantity and
  * oversell stock. Unlike the gift card and reward point spends, there is

tests/Models/ReceivingTest.php

@@ -9,7 +9,7 @@ use Tests\Support\EmployeeFixtureTrait;
 use Tests\Support\ItemFixtureTrait;
 
 /**
- * Regression tests for GHSA-995p-52qw-5hh2: Receiving::delete_value() must
+ * Regression tests: Receiving::delete_value() must
  * correctly reverse the stock quantity change it applied via
  * Item_quantity::changeQuantity(), using the same atomic upsert as the
  * sale-checkout and sale-cancel paths.

tests/Models/SaleTest.php

@@ -10,7 +10,7 @@ use Tests\Support\EmployeeFixtureTrait;
 use Tests\Support\ItemFixtureTrait;
 
 /**
- * Regression tests for GHSA-995p-52qw-5hh2: Sale::save_value() must reject
+ * Regression tests: Sale::save_value() must reject
  * (and roll back) a payment that would overdraw a gift card or a customer's
  * reward points, instead of silently applying a stale/negative balance.
  */

tests/helpers/security_helperTest.php

@@ -301,14 +301,31 @@ class security_helperTest extends CIUnitTestCase
         $this->assertTrue(envFileIsWritable());
     }
 
-    public function testEnvFileIsWritableReturnsFalseWhenFileIsReadonly(): void
+    public function testEnvFileIsWritableReturnsFalseWhenLockFileIsReadOnly(): void
     {
+        $this->skipIfRoot();
         file_put_contents($this->envPath, "# tmp\n");
-        chmod($this->envPath, 0444);
+        file_put_contents($this->lockPath, "");
+        chmod($this->lockPath, 0444);
 
-        $this->assertFalse(envFileIsWritable());
+        try {
+            $this->assertFalse(envFileIsWritable());
+        } finally {
+            @unlink($this->lockPath);
+        }
+    }
 
-        chmod($this->envPath, 0644);
+    public function testEnvFileIsWritableReturnsFalseWhenDirectoryIsNotWritable(): void
+    {
+        $this->skipIfRoot();
+        file_put_contents($this->envPath, "# tmp\n");
+        chmod($this->sandbox, 0500);
+
+        try {
+            $this->assertFalse(envFileIsWritable());
+        } finally {
+            chmod($this->sandbox, 0700);
+        }
     }
 
     // -- checkEncryption() --
@@ -340,16 +357,18 @@ class security_helperTest extends CIUnitTestCase
 
     public function testCheckEncryptionThrowsWhenKeyEmptyAndEnvNotWritable(): void
     {
+        $this->skipIfRoot();
         config('Encryption')->key = '';
         file_put_contents($this->envPath, "encryption.key=''\n");
-        chmod($this->envPath, 0444);
+        file_put_contents($this->lockPath, "");
+        chmod($this->lockPath, 0444);
 
         try {
             $this->expectException(RuntimeException::class);
             $this->expectExceptionMessage('provisioned');
             checkEncryption();
         } finally {
-            chmod($this->envPath, 0644);
+            @unlink($this->lockPath);
         }
     }
 
@@ -392,11 +411,9 @@ class security_helperTest extends CIUnitTestCase
 
     public function testCheckEncryptionRollsBackWhenSaveAllFails(): void
     {
-        // Regression guard (thread #2): if the post-rotation saveAll() fails,
-        // the freshly rotated .env key must be restored from the backup so the
-        // original CI3 ciphertext stays decryptable. A failing fake Appconfig
-        // (injected via CI3SecretConverter) forces saveAll() to throw without
-        // a real database.
+        // If the post-rotation saveAll() fails, the rotated key must be rolled
+        // back so the original CI3 ciphertext stays decryptable. A failing fake
+        // Appconfig (injected via CI3SecretConverter) makes saveAll() throw.
         $oldKey     = bin2hex(random_bytes(16)); // < 64 chars -> CI3 era
         $plaintext  = ['smtp_pass' => 'keep-me-safe'];
         $ciphertext = array_map(fn ($v) => $this->ci3Encrypt($v, $oldKey), $plaintext);
@@ -453,17 +470,19 @@ class security_helperTest extends CIUnitTestCase
 
     public function testCheckThrottleEncryptionThrowsWhenKeyMissingAndEnvNotWritable(): void
     {
+        $this->skipIfRoot();
         putenv('throttle.key');
         unset($_ENV['throttle.key'], $_SERVER['throttle.key']);
         file_put_contents($this->envPath, "encryption.key='abc'\n");
-        chmod($this->envPath, 0444);
+        file_put_contents($this->lockPath, "");
+        chmod($this->lockPath, 0444);
 
         try {
             $this->expectException(RuntimeException::class);
             $this->expectExceptionMessage('provisioned');
             checkThrottleEncryption();
         } finally {
-            chmod($this->envPath, 0644);
+            @unlink($this->lockPath);
         }
     }
 
@@ -657,4 +676,22 @@ class security_helperTest extends CIUnitTestCase
 
         $this->assertFileDoesNotExist($this->backupPath);
     }
+
+    /**
+     * When PHPUnit runs as root, is_writable() reports 0444 files as writable
+     * (and root bypasses directory permission bits), so the "not writable"
+     * fixtures cannot be faked reliably. Skip those tests under root.
+     */
+    private function skipIfRoot(): void
+    {
+        $isRoot = false;
+        if (function_exists('posix_geteuid')) {
+            $isRoot = posix_geteuid() === 0;
+        } elseif (function_exists('get_current_user')) {
+            $isRoot = in_array(get_current_user(), ['root', '0'], true);
+        }
+        if ($isRoot) {
+            $this->markTestSkipped('is_writable() is bypassed when running as root; cannot fake a non-writable fixture');
+        }
+    }
 }