mirror of
https://github.com/opensourcepos/opensourcepos.git
synced 2026-10-02 15:45:09 -04:00
* fix(security): report unwritable .env.lock, make throttle limits configurable
envFileIsWritable() previously checked is_writable(.env) (the file), which passes in Docker even when the mutex file is root-owned by a prior env:provision run. It now checks the real write path: the directory (to create .env.tmp.* + .env.lock) and any existing .env.lock must be writable, so the app throws the clear 'run env:provision' error instead of crashing on 'Unable to open .env.lock'.
The Throttle filter now reads throttle.capacity / throttle.seconds from env (default 5 per 60s); capacity <= 0 disables throttling, so operators serving sequential HTTP clients (e.g. Zabbix) are not caught by the lockout.
* fix(security): address CodeRabbit review findings
- Throttle: validate throttle.capacity as an integer before treating a non-positive value as 'disabled', so a non-numeric value (e.g. 'five') falls back to the default instead of silently bypassing the lockout. Apply the same validation to throttle.seconds.
- envFileIsWritable(): also reject an existing non-writable .env on Windows (where rename() cannot replace a read-only destination); keep the check Windows-only since POSIX rename() replaces a read-only dest when the directory is writable.
- Tests: restore the prior throttle.capacity env state in ThrottleTest (capture/restore instead of delete); add an invalid-capacity fallback case; skip the not-writable fixtures when running as root (where is_writable() is bypassed).
* fix(migration): guard ConvertToCI4 key-write branches with envFileIsWritable()
The migration's 'no key' and 'CI3 key' branches called rotateEncryptionKey()/rotateEncryptionKeyTransaction() directly, bypassing the envFileIsWritable() guard that checkEncryption() uses. On a fresh Docker/Compose install where the web runtime cannot write /app/.env.lock, this produced a raw 'fopen(/app/.env.lock): Permission denied' error instead of the actionable 'run php spark env:provision' message.
A valid CI4 key now short-circuits to checkEncryption() (no write); every write branch is gated on envFileIsWritable() first.
* fix: read provisioned encryption.key so config sees it
env:provision persists the key as 'encryption.key' in .env (matching the
throttle path and .env.example), but Config\Encryption only read the
ENCRYPTION_KEY env var. On a fresh Docker instance the provisioned key was
invisible to config('Encryption')->key, so the app believed no key existed
and tried to write one -- hitting the .env.lock permission wall.
Read encryption.key (via $_SERVER/$_ENV/getenv) first, then fall back to
ENCRYPTION_KEY for Docker '-e' usage. Mirrors checkThrottleEncryption().
* fix: cascade encryption.key lookup past empty-string sources
* refactor: extract Encryption::resolveKey() and test it without global env mutation
* fix(security): decode fallback-selected encryption key like BaseConfig
A key picked in the constructor fallback (notably ENCRYPTION_KEY, which
BaseConfig never inspects) was assigned verbatim, bypassing the
hex2bin:/base64: decode the parent applies to `encryption.key`. Route the
selected key through a parseKey() helper mirroring BaseConfig's
parseEncryptionKey() so prefixed values decrypt consistently, and add a
pure regression test.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* chore: remove advisory ID from comments and tighten verbose comments
Per review: treat GHSA advisory IDs like secrets (drop from code) and
replace the multi-paragraph comments with concise one-liners that keep
the non-obvious "why". No logic changes.
---------
Co-authored-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
102 lines
3.6 KiB
PHP
102 lines
3.6 KiB
PHP
<?php
|
|
|
|
namespace App\Database\Migrations;
|
|
|
|
use App\Libraries\CI3SecretConverter;
|
|
use CodeIgniter\Database\Exceptions\DatabaseException;
|
|
use CodeIgniter\Database\Forge;
|
|
use CodeIgniter\Database\Migration;
|
|
use CodeIgniter\HTTP\Exceptions\RedirectException;
|
|
use RuntimeException;
|
|
|
|
class ConvertToCI4 extends Migration
|
|
{
|
|
/**
|
|
* Constructor.
|
|
*/
|
|
public function __construct(?Forge $forge = null)
|
|
{
|
|
parent::__construct($forge);
|
|
helper('security');
|
|
}
|
|
|
|
/**
|
|
* Perform a migration step.
|
|
*/
|
|
public function up(): void
|
|
{
|
|
helper('migration');
|
|
|
|
if (!executeScript(APPPATH . 'Database/Migrations/sqlscripts/3.4.0_CI4Conversion.sql')) {
|
|
throw new DatabaseException('Migration script 3.4.0_CI4Conversion.sql failed. Check logs for details.');
|
|
}
|
|
|
|
$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);
|
|
} 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();
|
|
}
|
|
}
|
|
|
|
/**
|
|
* Revert a migration step.
|
|
*/
|
|
public function down(): void {}
|
|
|
|
/**
|
|
* Decrypts legacy CI3-encrypted secrets with the old key, rotates to a
|
|
* fresh CI4 key, re-encrypts under the new key, verifies the round trip,
|
|
* and persists the result.
|
|
*
|
|
* The actual decryption/encryption is delegated to CI3SecretConverter so
|
|
* the docker startup command (env:provision) shares the same code path.
|
|
*
|
|
* @param string $oldKey the CI3-era key currently in .env
|
|
* @throws RedirectException
|
|
*/
|
|
private function convertCI3EncryptedData(string $oldKey): void
|
|
{
|
|
$converter = new CI3SecretConverter();
|
|
|
|
// DB read, safe outside the .env lock.
|
|
$plain = $converter->decryptAll($oldKey);
|
|
|
|
// Backup -> rotate -> re-encrypt -> verify -> persist under one .env
|
|
// lock. On any failure the transaction restores the pre-rotation .env
|
|
// (still holding the lock); on success it drops the backup.
|
|
rotateEncryptionKeyTransaction($oldKey, static function () use ($plain, $converter): void {
|
|
$encrypted = $converter->encryptAll($plain);
|
|
|
|
// Verify the round trip before committing so we never lose data.
|
|
if (array_diff_assoc($plain, $converter->verifyAll($encrypted)) !== []) {
|
|
throw new RedirectException('login'); // TODO: Need to figure out how to pass the error to the Login controller so that it gets displayed.
|
|
}
|
|
|
|
$converter->saveAll($encrypted);
|
|
});
|
|
}
|
|
}
|