From 3b47dbf241222f8e711b39cc8381edb9ffb0ceed Mon Sep 17 00:00:00 2001 From: uldisrudzitis Date: Wed, 22 Jul 2026 11:18:56 +0300 Subject: [PATCH 1/3] Fix .env file multiple/concurrent writes --- .../app/Http/Middleware/SetupMiddleware.php | 61 +++++++- .../tests/Feature/SetupMiddlewareTest.php | 143 ++++++++++++++++++ 2 files changed, 199 insertions(+), 5 deletions(-) create mode 100644 _api_app/tests/Feature/SetupMiddlewareTest.php diff --git a/_api_app/app/Http/Middleware/SetupMiddleware.php b/_api_app/app/Http/Middleware/SetupMiddleware.php index 2f4547dd..bca3bc4b 100644 --- a/_api_app/app/Http/Middleware/SetupMiddleware.php +++ b/_api_app/app/Http/Middleware/SetupMiddleware.php @@ -25,17 +25,25 @@ public function handle(Request $request, Closure $next): Response // figure out more effective way for comparison protected function updateEnvFile() { + $env_path = base_path() . '/.env'; $env_example = $this->parseEnvFile(base_path() . '/.env.example'); - $env = $this->parseEnvFile(base_path() . '/.env'); + $env = $this->parseEnvFile($env_path); + $placeholders = ['[YOUR_APP_KEY]', '[YOUR_APP_ID]', '']; + + // .env.example is the authoritative key list: only its keys end up in .env, any + // others are dropped. Keep the existing value when set (and not a placeholder), + // regenerate APP_KEY/APP_ID only when absent/empty/placeholder, else fall back to + // the example value. Once rewritten, .env matches the example key set so + // $env === $env_new on the next run — no perpetual per-request rewrite. $env_new = []; foreach ($env_example as $key => $value) { - if (isset($env[$key])) { + if (array_key_exists($key, $env) && ! in_array($env[$key], $placeholders, true)) { $env_new[$key] = $env[$key]; - } elseif ($key == 'APP_KEY' || $key == 'APP_ID') { + } elseif (($key == 'APP_KEY' || $key == 'APP_ID') && in_array($env[$key] ?? '', $placeholders, true)) { $env_new[$key] = Helpers::uuid_v4(); } else { - $env_new[$key] = $value; + $env_new[$key] = $env[$key] ?? $value; } } @@ -45,7 +53,50 @@ protected function updateEnvFile() foreach ($env_new as $key => $value) { $content .= $key . '=' . $value . "\n"; } - file_put_contents(base_path() . '/.env', $content); + $this->writeEnvFile($env_path, $content); + } + } + + /** + * Write the .env file without exposing a truncated/partial file to concurrent + * readers (including Laravel's own env loader, which takes no lock). + * + * When the directory is writable we do an atomic temp-file + rename() swap — the + * strongest guarantee, protecting every reader without cooperation. The temp file + * must live in the same directory: rename() is only atomic on the same filesystem, + * so a system-temp path would degrade to a non-atomic copy+delete. When only the + * file itself is writable (hardened deployment: read-only app root), we fall back to + * a locked in-place write. Storage.php uses advisory flock for its XML files, but + * those are only ever accessed through its own locked methods; .env has readers + * outside our control, hence the preference for rename() here. + */ + protected function writeEnvFile(string $path, string $content): void + { + $mode = file_exists($path) ? (fileperms($path) & 0777) : 0644; + + if (is_writable(dirname($path))) { + $tmp = $path . '.' . getmypid() . '.' . uniqid() . '.tmp'; + if (file_put_contents($tmp, $content) !== false) { + @chmod($tmp, $mode); + if (@rename($tmp, $path)) { + return; + } + @unlink($tmp); + } + } + + // Read-only app root, .env writable: locked in-place write. 'c' creates if + // missing and does not truncate before we hold the lock. Returns false (no-op) + // when .env is missing and the dir is read-only — creation is an install step. + $fp = @fopen($path, 'c'); + if ($fp !== false) { + if (flock($fp, LOCK_EX)) { + ftruncate($fp, 0); + fwrite($fp, $content); + fflush($fp); + flock($fp, LOCK_UN); + } + fclose($fp); } } diff --git a/_api_app/tests/Feature/SetupMiddlewareTest.php b/_api_app/tests/Feature/SetupMiddlewareTest.php new file mode 100644 index 00000000..15f6f53b --- /dev/null +++ b/_api_app/tests/Feature/SetupMiddlewareTest.php @@ -0,0 +1,143 @@ +setAccessible(true); + + return $ref->invokeArgs(new SetupMiddleware, $args); +} + +/** + * Parse a KEY=VALUE .env-style string into an array (test-side, order preserving). + */ +function parseEnv(string $content): array +{ + $res = []; + foreach (preg_split('/\R/', trim($content)) as $line) { + if ($line === '') { + continue; + } + [$key, $value] = array_pad(explode('=', $line, 2), 2, ''); + $res[$key] = $value; + } + + return $res; +} + +beforeEach(function () { + $this->dir = sys_get_temp_dir() . '/berta_setup_' . uniqid(); + mkdir($this->dir, 0777, true); +}); + +afterEach(function () { + // Ensure the dir is writable so cleanup can remove its contents. + @chmod($this->dir, 0777); + foreach (glob($this->dir . '/*') ?: [] as $f) { + @chmod($f, 0666); + @unlink($f); + } + foreach (glob($this->dir . '/.*') ?: [] as $f) { + if (! is_dir($f)) { + @unlink($f); + } + } + @rmdir($this->dir); +}); + +it('preserves existing APP_KEY and APP_ID instead of regenerating them', function () { + file_put_contents($this->dir . '/.env.example', "APP_ENV=production\nAPP_KEY=[YOUR_APP_KEY]\nAPP_ID=[YOUR_APP_ID]\n"); + file_put_contents($this->dir . '/.env', "APP_ENV=production\nAPP_KEY=real-app-key\nAPP_ID=real-app-id\n"); + + $this->app->setBasePath($this->dir); + invokeSetup('updateEnvFile'); + + $env = parseEnv(file_get_contents($this->dir . '/.env')); + expect($env['APP_KEY'])->toBe('real-app-key'); + expect($env['APP_ID'])->toBe('real-app-id'); +}); + +it('creates .env from .env.example and generates keys on first run', function () { + file_put_contents($this->dir . '/.env.example', "APP_ENV=production\nAPP_KEY=[YOUR_APP_KEY]\nAPP_ID=[YOUR_APP_ID]\n"); + + $this->app->setBasePath($this->dir); + invokeSetup('updateEnvFile'); + + expect(file_exists($this->dir . '/.env'))->toBeTrue(); + $env = parseEnv(file_get_contents($this->dir . '/.env')); + expect($env['APP_ENV'])->toBe('production'); + expect($env['APP_KEY'])->not->toBeIn(['[YOUR_APP_KEY]', '']); + expect($env['APP_ID'])->not->toBeIn(['[YOUR_APP_ID]', '']); +}); + +it('regenerates keys that still hold the example placeholder', function () { + file_put_contents($this->dir . '/.env.example', "APP_KEY=[YOUR_APP_KEY]\nAPP_ID=[YOUR_APP_ID]\n"); + file_put_contents($this->dir . '/.env', "APP_KEY=[YOUR_APP_KEY]\nAPP_ID=\n"); + + $this->app->setBasePath($this->dir); + invokeSetup('updateEnvFile'); + + $env = parseEnv(file_get_contents($this->dir . '/.env')); + expect($env['APP_KEY'])->not->toBeIn(['[YOUR_APP_KEY]', '']); + expect($env['APP_ID'])->not->toBe(''); +}); + +it('drops keys not in .env.example and does not rewrite on a second run', function () { + file_put_contents($this->dir . '/.env.example', "APP_ENV=production\nAPP_KEY=[YOUR_APP_KEY]\nAPP_ID=[YOUR_APP_ID]\n"); + file_put_contents($this->dir . '/.env', "APP_KEY=real-app-key\nAPP_ID=real-app-id\nDB_HOST=localhost\n"); + + $this->app->setBasePath($this->dir); + invokeSetup('updateEnvFile'); + + $afterFirst = file_get_contents($this->dir . '/.env'); + $env = parseEnv($afterFirst); + expect($env)->not->toHaveKey('DB_HOST'); // key absent from .env.example is dropped + expect($env['APP_ENV'])->toBe('production'); // example key added + expect($env['APP_KEY'])->toBe('real-app-key'); // existing value kept + + invokeSetup('updateEnvFile'); + // Idempotent: once .env matches the example key set, no further rewrite happens. + expect(file_get_contents($this->dir . '/.env'))->toBe($afterFirst); +}); + +it('writes atomically and preserves file permissions when the directory is writable', function () { + $path = $this->dir . '/.env'; + file_put_contents($path, "APP_KEY=old\n"); + chmod($path, 0640); + + invokeSetup('writeEnvFile', [$path, "APP_KEY=new\n"]); + + expect(file_get_contents($path))->toBe("APP_KEY=new\n"); + expect(fileperms($path) & 0777)->toBe(0640); +}); + +it('falls back to a locked in-place write when only the file is writable', function () { + $path = $this->dir . '/.env'; + file_put_contents($path, "APP_KEY=old\n"); + chmod($path, 0644); + chmod($this->dir, 0555); // read-only directory + if (is_writable($this->dir)) { + $this->markTestSkipped('Directory permissions are not enforced (running as root?).'); + } + + invokeSetup('writeEnvFile', [$path, "APP_KEY=new\n"]); + + expect(file_get_contents($path))->toBe("APP_KEY=new\n"); +}); + +it('no-ops without error when .env is missing and the directory is read-only', function () { + $path = $this->dir . '/.env'; + chmod($this->dir, 0555); // read-only directory, no .env present + if (is_writable($this->dir)) { + $this->markTestSkipped('Directory permissions are not enforced (running as root?).'); + } + + invokeSetup('writeEnvFile', [$path, "APP_KEY=new\n"]); + + expect(file_exists($path))->toBeFalse(); +}); From 34e859862362aa08f68239a61f524f4976fb45ee Mon Sep 17 00:00:00 2001 From: uldisrudzitis Date: Wed, 22 Jul 2026 13:34:00 +0300 Subject: [PATCH 2/3] Improved .env file locking --- _api_app/.gitignore | 1 + .../app/Http/Middleware/SetupMiddleware.php | 110 ++++++++++++++---- _api_app/storage/framework/.gitignore | 1 + .../tests/Feature/SetupMiddlewareTest.php | 95 ++++++++++++++- 4 files changed, 183 insertions(+), 24 deletions(-) diff --git a/_api_app/.gitignore b/_api_app/.gitignore index fbb71010..e86ca676 100644 --- a/_api_app/.gitignore +++ b/_api_app/.gitignore @@ -8,6 +8,7 @@ .env .env.backup .env.production +.env_* .phpactor.json .phpunit.result.cache Homestead.json diff --git a/_api_app/app/Http/Middleware/SetupMiddleware.php b/_api_app/app/Http/Middleware/SetupMiddleware.php index bca3bc4b..41349269 100644 --- a/_api_app/app/Http/Middleware/SetupMiddleware.php +++ b/_api_app/app/Http/Middleware/SetupMiddleware.php @@ -27,36 +27,100 @@ protected function updateEnvFile() { $env_path = base_path() . '/.env'; $env_example = $this->parseEnvFile(base_path() . '/.env.example'); - $env = $this->parseEnvFile($env_path); - - $placeholders = ['[YOUR_APP_KEY]', '[YOUR_APP_ID]', '']; - - // .env.example is the authoritative key list: only its keys end up in .env, any - // others are dropped. Keep the existing value when set (and not a placeholder), - // regenerate APP_KEY/APP_ID only when absent/empty/placeholder, else fall back to - // the example value. Once rewritten, .env matches the example key set so - // $env === $env_new on the next run — no perpetual per-request rewrite. - $env_new = []; - foreach ($env_example as $key => $value) { - if (array_key_exists($key, $env) && ! in_array($env[$key], $placeholders, true)) { - $env_new[$key] = $env[$key]; - } elseif (($key == 'APP_KEY' || $key == 'APP_ID') && in_array($env[$key] ?? '', $placeholders, true)) { - $env_new[$key] = Helpers::uuid_v4(); - } else { - $env_new[$key] = $env[$key] ?? $value; - } + + // Nothing to reconcile against. Without this guard an unreadable/missing + // .env.example would produce an empty $env_new and wipe .env. + if (empty($env_example)) { + return; + } + + // The whole read-decide-write cycle must be serialized, not just the write: + // two workers both seeing a placeholder APP_KEY would each mint a different + // uuid_v4() and the last write would silently discard the other's value. + $lock = $this->acquireReconcileLock(); + if ($lock === false) { + return; } - if ($env !== $env_new) { - $content = ''; + try { + $env = $this->parseEnvFile($env_path); + + $placeholders = ['[YOUR_APP_KEY]', '[YOUR_APP_ID]', '']; + + // .env.example is the authoritative key list: only its keys end up in .env, any + // others are dropped. Keep the existing value when set (and not a placeholder), + // regenerate APP_KEY/APP_ID only when absent/empty/placeholder, else fall back to + // the example value. Once rewritten, .env matches the example key set so + // $env === $env_new on the next run — no perpetual per-request rewrite. + $env_new = []; + foreach ($env_example as $key => $value) { + if (array_key_exists($key, $env) && ! in_array($env[$key], $placeholders, true)) { + $env_new[$key] = $env[$key]; + } elseif (($key == 'APP_KEY' || $key == 'APP_ID') && in_array($env[$key] ?? '', $placeholders, true)) { + $env_new[$key] = Helpers::uuid_v4(); + } else { + $env_new[$key] = $env[$key] ?? $value; + } + } + + if ($env !== $env_new) { + $content = ''; - foreach ($env_new as $key => $value) { - $content .= $key . '=' . $value . "\n"; + foreach ($env_new as $key => $value) { + $content .= $key . '=' . $value . "\n"; + } + $this->writeEnvFile($env_path, $content); + } + } finally { + if (is_resource($lock)) { + flock($lock, LOCK_UN); + fclose($lock); } - $this->writeEnvFile($env_path, $content); } } + /** + * Acquire the exclusive, non-blocking lock guarding the whole reconcile transaction. + * + * Locks a dedicated file rather than .env itself: the atomic write replaces .env via + * rename(), so its inode changes mid-transaction and a second worker would lock a + * different inode — flock is per-inode, so that would not mutually exclude. The lock + * is non-blocking because this runs on every request; serializing all API traffic is + * not acceptable, and skipping is safe since the reconcile is idempotent. + * + * @return resource|null|false Resource when locked, null when no lock is available + * (proceed unlocked, degraded), false when another worker + * holds it (skip this cycle). + */ + protected function acquireReconcileLock() + { + $path = storage_path('framework/.env.lock'); + $is_new = ! file_exists($path); + + $fp = @fopen($path, 'c'); + + // No lock infrastructure (e.g. storage/ not writable) — degrade to an unlocked + // reconcile rather than never syncing .env at all. + if ($fp === false) { + return null; + } + + // Opening with 'c' needs write permission, so a lock file created by a different + // user (deploy/CLI/root) would lock the web-server user out and silently disable + // the lock for every later request. Same shared-write convention as Storage. + if ($is_new) { + @chmod($path, 0666); + } + + if (! flock($fp, LOCK_EX | LOCK_NB)) { + fclose($fp); + + return false; + } + + return $fp; + } + /** * Write the .env file without exposing a truncated/partial file to concurrent * readers (including Laravel's own env loader, which takes no lock). diff --git a/_api_app/storage/framework/.gitignore b/_api_app/storage/framework/.gitignore index 05c4471f..336eabbc 100644 --- a/_api_app/storage/framework/.gitignore +++ b/_api_app/storage/framework/.gitignore @@ -1,3 +1,4 @@ +.env.lock compiled.php config.php down diff --git a/_api_app/tests/Feature/SetupMiddlewareTest.php b/_api_app/tests/Feature/SetupMiddlewareTest.php index 15f6f53b..04de4088 100644 --- a/_api_app/tests/Feature/SetupMiddlewareTest.php +++ b/_api_app/tests/Feature/SetupMiddlewareTest.php @@ -32,12 +32,23 @@ function parseEnv(string $content): array beforeEach(function () { $this->dir = sys_get_temp_dir() . '/berta_setup_' . uniqid(); - mkdir($this->dir, 0777, true); + // storage/framework must exist for storage_path() to resolve the reconcile lock file + // once the base path is pointed at the temp dir. + mkdir($this->dir . '/storage/framework', 0777, true); }); afterEach(function () { // Ensure the dir is writable so cleanup can remove its contents. @chmod($this->dir, 0777); + // Explicit rather than a GLOB_BRACE dotfile glob, which is not portable off glibc. + @unlink($this->dir . '/storage/framework/.env.lock'); + foreach (glob($this->dir . '/storage/framework/*') ?: [] as $f) { + if (! is_dir($f)) { + @unlink($f); + } + } + @rmdir($this->dir . '/storage/framework'); + @rmdir($this->dir . '/storage'); foreach (glob($this->dir . '/*') ?: [] as $f) { @chmod($f, 0666); @unlink($f); @@ -105,6 +116,88 @@ function parseEnv(string $content): array expect(file_get_contents($this->dir . '/.env'))->toBe($afterFirst); }); +it('skips the reconcile while another worker holds the lock', function () { + file_put_contents($this->dir . '/.env.example', "APP_KEY=[YOUR_APP_KEY]\nAPP_ID=[YOUR_APP_ID]\n"); + file_put_contents($this->dir . '/.env', "APP_KEY=[YOUR_APP_KEY]\nAPP_ID=[YOUR_APP_ID]\n"); + $before = file_get_contents($this->dir . '/.env'); + + $this->app->setBasePath($this->dir); + + // Simulate a concurrent worker mid-transaction. + $holder = fopen($this->dir . '/storage/framework/.env.lock', 'c'); + expect(flock($holder, LOCK_EX | LOCK_NB))->toBeTrue(); + + invokeSetup('updateEnvFile'); + expect(file_get_contents($this->dir . '/.env'))->toBe($before); // no write happened + + // Once the other worker is done, the reconcile proceeds. + flock($holder, LOCK_UN); + fclose($holder); + + invokeSetup('updateEnvFile'); + $env = parseEnv(file_get_contents($this->dir . '/.env')); + expect($env['APP_KEY'])->not->toBe('[YOUR_APP_KEY]'); +}); + +it('creates the lock file group- and other-writable', function () { + file_put_contents($this->dir . '/.env.example', "APP_KEY=[YOUR_APP_KEY]\n"); + + $this->app->setBasePath($this->dir); + invokeSetup('updateEnvFile'); + + $lock = $this->dir . '/storage/framework/.env.lock'; + expect(file_exists($lock))->toBeTrue(); + // Otherwise a lock file created by another user would silently lock the web-server + // user out of the lock, degrading every later reconcile to unlocked. + expect(fileperms($lock) & 0666)->toBe(0666); +}); + +it('mints APP_KEY only once across repeated reconciles', function () { + file_put_contents($this->dir . '/.env.example', "APP_KEY=[YOUR_APP_KEY]\nAPP_ID=[YOUR_APP_ID]\n"); + file_put_contents($this->dir . '/.env', "APP_KEY=[YOUR_APP_KEY]\nAPP_ID=[YOUR_APP_ID]\n"); + + $this->app->setBasePath($this->dir); + + invokeSetup('updateEnvFile'); + $first = parseEnv(file_get_contents($this->dir . '/.env')); + + invokeSetup('updateEnvFile'); + $second = parseEnv(file_get_contents($this->dir . '/.env')); + + expect($second['APP_KEY'])->toBe($first['APP_KEY']); + expect($second['APP_ID'])->toBe($first['APP_ID']); +}); + +it('does not wipe .env when .env.example is missing', function () { + $original = "APP_KEY=real-app-key\nAPP_ID=real-app-id\n"; + file_put_contents($this->dir . '/.env', $original); // no .env.example present + + $this->app->setBasePath($this->dir); + invokeSetup('updateEnvFile'); + + expect(file_get_contents($this->dir . '/.env'))->toBe($original); +}); + +it('still reconciles when the lock file cannot be created', function () { + // No storage/ directory: storage_path() is unresolvable, so the lock degrades. + // The dotfile must go too, otherwise rmdir() fails and the lock would still work, + // making this test pass without exercising the degraded path. + @unlink($this->dir . '/storage/framework/.env.lock'); + array_map('unlink', glob($this->dir . '/storage/framework/*') ?: []); + rmdir($this->dir . '/storage/framework'); + rmdir($this->dir . '/storage'); + expect(is_dir($this->dir . '/storage'))->toBeFalse(); + + file_put_contents($this->dir . '/.env.example', "APP_ENV=production\nAPP_KEY=[YOUR_APP_KEY]\n"); + + $this->app->setBasePath($this->dir); + invokeSetup('updateEnvFile'); + + $env = parseEnv(file_get_contents($this->dir . '/.env')); + expect($env['APP_ENV'])->toBe('production'); + expect($env['APP_KEY'])->not->toBeIn(['[YOUR_APP_KEY]', '']); +}); + it('writes atomically and preserves file permissions when the directory is writable', function () { $path = $this->dir . '/.env'; file_put_contents($path, "APP_KEY=old\n"); From 290ad5ad3d5e682e7ba71d04766a240dd86420b3 Mon Sep 17 00:00:00 2001 From: uldisrudzitis Date: Wed, 22 Jul 2026 13:46:01 +0300 Subject: [PATCH 3/3] Update gitignore --- _api_app/.gitignore | 1 - 1 file changed, 1 deletion(-) diff --git a/_api_app/.gitignore b/_api_app/.gitignore index e86ca676..fbb71010 100644 --- a/_api_app/.gitignore +++ b/_api_app/.gitignore @@ -8,7 +8,6 @@ .env .env.backup .env.production -.env_* .phpactor.json .phpunit.result.cache Homestead.json