mirror of
https://github.com/opensourcepos/opensourcepos.git
synced 2026-09-23 03:05:00 -04:00
Address CodeRabbit Major findings from the 4th re-review of the env helper and its callers: 1. Hold .env.lock for the entire CI3 -> CI4 conversion transaction (backup -> rotate -> re-encrypt -> verify -> persist -> cleanup) so a concurrent worker cannot interleave a key write between the rotation and the ciphertext save. Split rotateEncryptionKey into a lock-free core (rotateEncryptionKeyUnlock) plus the existing lock wrapper and a new rotateEncryptionKeyTransaction that owns the lock across the full unit and performs both the in-lock rollback (abortEncryptionConversion) and the in-lock backup removal on success. 2. Treat the legacy value '0' as non-empty data so key rotation still persists the re-encrypted ciphertext when '0' is the only stored secret (array_filter would have dropped it and skipped saveAll). 3. Wrap the post-rotation re-encrypt/verify/saveAll sequence in a catch (Throwable) across all three call-sites so CI4 EncryptionException, ReflectionException from batch_save, a failed round-trip verify, and any other failure all roll the .env key back to the pre-rotation state. 4. In Docker Compose, use long-syntax bind with create_host_path: false and document in INSTALL.md that the host .env must be a regular file (a missing one is no longer auto-created as a directory, and the mount now rejects a missing source on Compose implementations that support the flag). Files touched: app/Helpers/security_helper.php, app/Commands/EnvProvision.php, app/Database/Migrations/20220127000000_convertToCI4.php, docker-compose.yml, INSTALL.md. All 4 existing helper tests still pass via CI.