refactor: improve UX on key:generate

paulbalandan · paulbalandan · commit ee5ca4d69c8e · 2026-05-12T00:32:07.000+08:00
diff --git a/system/Commands/Encryption/GenerateKey.php b/system/Commands/Encryption/GenerateKey.php
@@ -104,17 +104,29 @@ protected function execute(array $arguments, array $options): int
         $currentKey = env('encryption.key', '');
 
         if ($currentKey !== '' && $options['force'] === false) {
-            CLI::error('Setting new encryption key aborted.');
+            if ($this->isInteractive()) {
+                CLI::write('Setting new encryption key cancelled.', 'yellow');
 
-            if (! $this->isInteractive()) {
-                CLI::error('If you want, use the "--force" option to force overwrite the existing key.');
+                return EXIT_SUCCESS;
             }
 
+            CLI::error('Setting new encryption key aborted: pass --force to overwrite the existing key in non-interactive mode.');
+
+            return EXIT_ERROR;
+        }
+
+        $envFile = ((new Paths())->envDirectory ?? ROOTPATH) . '.env'; // @phpstan-ignore nullCoalesce.property
+        $baseEnv = ROOTPATH . 'env';
+
+        if (! is_file($envFile) && ! is_file($baseEnv)) {
+            CLI::write('Both default shipped `env` file and custom `.env` are missing.', 'yellow');
+            CLI::write(sprintf('Here\'s your new key instead: %s', CLI::color($encodedKey, 'yellow')));
+
             return EXIT_ERROR;
         }
 
-        if (! $this->writeNewEncryptionKeyToFile($currentKey, $encodedKey)) {
-            CLI::write('Error in setting new encryption key to .env file.');
+        if (! $this->writeNewEncryptionKeyToFile($currentKey, $encodedKey, $envFile, $baseEnv)) {
+            CLI::error(sprintf('Failed to write new encryption key to %s.', clean_path($envFile)));
 
             return EXIT_ERROR;
         }
@@ -125,7 +137,7 @@ protected function execute(array $arguments, array $options): int
         $dotenv = new DotEnv((new Paths())->envDirectory ?? ROOTPATH); // @phpstan-ignore nullCoalesce.property
         $dotenv->load();
 
-        CLI::write('Application\'s new encryption key was successfully set.', 'green');
+        CLI::write(sprintf('New encryption key written to %s.', clean_path($envFile)), 'green');
         CLI::newLine();
 
         return EXIT_SUCCESS;
@@ -146,24 +158,19 @@ private function generateRandomKey(string $prefix, int $length): string
     }
 
     /**
-     * Writes the new encryption key to .env file.
+     * Writes the new encryption key to .env file. The caller is responsible
+     * for ensuring at least one of `$envFile` or `$baseEnv` exists.
      */
-    private function writeNewEncryptionKeyToFile(string $oldKey, string $newKey): bool
+    private function writeNewEncryptionKeyToFile(string $oldKey, string $newKey, string $envFile, string $baseEnv): bool
     {
-        $baseEnv = ROOTPATH . 'env';
-        $envFile = ((new Paths())->envDirectory ?? ROOTPATH) . '.env'; // @phpstan-ignore nullCoalesce.property
-
         if (! is_file($envFile)) {
-            if (! is_file($baseEnv)) {
-                CLI::write('Both default shipped `env` file and custom `.env` are missing.', 'yellow');
-                CLI::write('Here\'s your new key instead: ' . CLI::color($newKey, 'yellow'));
-
-                return false;
-            }
-
             copy($baseEnv, $envFile);
         }
 
+        if (! is_writable($envFile)) {
+            return false;
+        }
+
         $oldFileContents = (string) file_get_contents($envFile);
 
         // Match an active setting line, preserving any leading whitespace and `export` prefix.
diff --git a/tests/system/Commands/Encryption/GenerateKeyTest.php b/tests/system/Commands/Encryption/GenerateKeyTest.php
@@ -17,11 +17,11 @@
 use CodeIgniter\Config\Services;
 use CodeIgniter\Superglobals;
 use CodeIgniter\Test\CIUnitTestCase;
-use CodeIgniter\Test\Filters\CITestStreamFilter;
 use CodeIgniter\Test\Mock\MockInputOutput;
 use CodeIgniter\Test\StreamFilterTrait;
 use PHPUnit\Framework\Attributes\Group;
 use PHPUnit\Framework\Attributes\PreserveGlobalState;
+use PHPUnit\Framework\Attributes\RequiresOperatingSystem;
 use PHPUnit\Framework\Attributes\RunInSeparateProcess;
 use PHPUnit\Framework\Attributes\WithoutErrorHandler;
 
@@ -70,14 +70,6 @@ protected function tearDown(): void
         CLI::reset();
     }
 
-    /**
-     * Gets buffer contents then releases it.
-     */
-    protected function getBuffer(): string
-    {
-        return $this->getStreamFilterBuffer();
-    }
-
     protected function resetEnvironment(): void
     {
         putenv('encryption.key');
@@ -88,31 +80,35 @@ protected function resetEnvironment(): void
     public function testGenerateKeyShowsEncodedKey(): void
     {
         command('key:generate --show');
-        $this->assertStringContainsString('hex2bin:', $this->getBuffer());
+        $this->assertStringContainsString('hex2bin:', $this->getStreamFilterBuffer());
 
+        $this->resetStreamFilterBuffer();
         command('key:generate --prefix base64 --show');
-        $this->assertStringContainsString('base64:', $this->getBuffer());
+        $this->assertStringContainsString('base64:', $this->getStreamFilterBuffer());
 
+        $this->resetStreamFilterBuffer();
         command('key:generate --prefix hex2bin --show');
-        $this->assertStringContainsString('hex2bin:', $this->getBuffer());
+        $this->assertStringContainsString('hex2bin:', $this->getStreamFilterBuffer());
     }
 
     #[PreserveGlobalState(false)]
     #[RunInSeparateProcess]
     public function testGenerateKeyCreatesNewKey(): void
     {
         command('key:generate');
-        $this->assertStringContainsString('successfully set.', $this->getBuffer());
+        $this->assertStringContainsString(sprintf('New encryption key written to ROOTPATH%s.env.', DIRECTORY_SEPARATOR), $this->getStreamFilterBuffer());
         $this->assertStringContainsString(env('encryption.key'), (string) file_get_contents($this->envPath));
         $this->assertStringContainsString('hex2bin:', (string) file_get_contents($this->envPath));
 
+        $this->resetStreamFilterBuffer();
         command('key:generate --prefix base64 --force');
-        $this->assertStringContainsString('successfully set.', $this->getBuffer());
+        $this->assertStringContainsString(sprintf('New encryption key written to ROOTPATH%s.env.', DIRECTORY_SEPARATOR), $this->getStreamFilterBuffer());
         $this->assertStringContainsString(env('encryption.key'), (string) file_get_contents($this->envPath));
         $this->assertStringContainsString('base64:', (string) file_get_contents($this->envPath));
 
+        $this->resetStreamFilterBuffer();
         command('key:generate --prefix hex2bin --force');
-        $this->assertStringContainsString('successfully set.', $this->getBuffer());
+        $this->assertStringContainsString(sprintf('New encryption key written to ROOTPATH%s.env.', DIRECTORY_SEPARATOR), $this->getStreamFilterBuffer());
         $this->assertStringContainsString(env('encryption.key'), (string) file_get_contents($this->envPath));
         $this->assertStringContainsString('hex2bin:', (string) file_get_contents($this->envPath));
     }
@@ -123,8 +119,9 @@ public function testDefaultShippedEnvIsMissing(): void
         command('key:generate');
         rename(ROOTPATH . 'lostenv', ROOTPATH . 'env');
 
-        $this->assertStringContainsString('Both default shipped', $this->getBuffer());
-        $this->assertStringContainsString('Error in setting', $this->getBuffer());
+        $this->assertStringContainsString('Both default shipped', $this->getStreamFilterBuffer());
+        $this->assertStringContainsString('Here\'s your new key instead:', $this->getStreamFilterBuffer());
+        $this->assertStringNotContainsString('Failed to write', $this->getStreamFilterBuffer());
     }
 
     /**
@@ -136,7 +133,7 @@ public function testKeyGenerateWhenKeyIsMissingInDotEnvFile(): void
 
         command('key:generate');
 
-        $this->assertStringContainsString('Application\'s new encryption key was successfully set.', $this->getBuffer());
+        $this->assertStringContainsString(sprintf('New encryption key written to ROOTPATH%s.env.', DIRECTORY_SEPARATOR), $this->getStreamFilterBuffer());
         $this->assertSame("\nencryption.key = " . env('encryption.key'), file_get_contents($this->envPath));
     }
 
@@ -152,9 +149,9 @@ public function testKeyGenerateWhenNewHexKeyIsSubsequentlyCommentedOut(): void
         ));
         $this->assertSame(1, $count, 'Failed commenting out the previously set application key.');
 
-        CITestStreamFilter::$buffer = '';
+        $this->resetStreamFilterBuffer();
         command('key:generate --force');
-        $this->assertStringContainsString('was successfully set.', $this->getBuffer());
+        $this->assertStringContainsString(sprintf('New encryption key written to ROOTPATH%s.env.', DIRECTORY_SEPARATOR), $this->getStreamFilterBuffer());
         $this->assertNotSame($key, env('encryption.key', $key), 'Failed replacing the commented out key.');
     }
 
@@ -170,9 +167,9 @@ public function testKeyGenerateWhenNewBase64KeyIsSubsequentlyCommentedOut(): voi
         ));
         $this->assertSame(1, $count, 'Failed commenting out the previously set application key.');
 
-        CITestStreamFilter::$buffer = '';
+        $this->resetStreamFilterBuffer();
         command('key:generate --force');
-        $this->assertStringContainsString('was successfully set.', $this->getBuffer());
+        $this->assertStringContainsString(sprintf('New encryption key written to ROOTPATH%s.env.', DIRECTORY_SEPARATOR), $this->getStreamFilterBuffer());
         $this->assertNotSame($key, env('encryption.key', $key), 'Failed replacing the commented out key.');
     }
 
@@ -190,10 +187,10 @@ public function testKeyGenerateReplacesUnloadedKeyInDotEnvFile(): void
         $this->assertSame('', env('encryption.key', ''));
 
         command('key:generate --force');
+        $this->assertStringContainsString(sprintf('New encryption key written to ROOTPATH%s.env.', DIRECTORY_SEPARATOR), $this->getStreamFilterBuffer());
 
-        $this->assertStringContainsString('was successfully set.', $this->getBuffer());
-
-        $contents = (string) file_get_contents($this->envPath);
+        $contents = @file_get_contents($this->envPath);
+        $this->assertIsString($contents, 'Failed to read .env file contents.');
         $this->assertStringNotContainsString($existingKey, $contents);
         $this->assertStringContainsString('encryption.key = ' . env('encryption.key'), $contents);
     }
@@ -234,7 +231,7 @@ public function testKeyGenerateNotFooledByCommentMentioningEncryptionKey(): void
         );
     }
 
-    public function testKeyGenerateAbortsWhenOverwritePromptIsDeclined(): void
+    public function testKeyGenerateCancelsWhenOverwritePromptIsDeclined(): void
     {
         command('key:generate');
         $key = env('encryption.key', '');
@@ -244,12 +241,13 @@ public function testKeyGenerateAbortsWhenOverwritePromptIsDeclined(): void
         $io->setInputs(['n']);
         CLI::setInputOutput($io);
 
+        $this->resetStreamFilterBuffer();
         command('key:generate');
 
         $this->assertSame($key, env('encryption.key', ''), 'Existing key should not change.');
         $this->assertStringContainsString($key, (string) file_get_contents($this->envPath));
         $this->assertStringContainsString('Overwrite existing key?', $io->getOutput());
-        $this->assertStringContainsString('Setting new encryption key aborted.', $io->getOutput());
+        $this->assertStringContainsString('Setting new encryption key cancelled.', $io->getOutput());
     }
 
     public function testKeyGenerateOverwritesWhenOverwritePromptIsConfirmed(): void
@@ -262,12 +260,13 @@ public function testKeyGenerateOverwritesWhenOverwritePromptIsConfirmed(): void
         $io->setInputs(['y']);
         CLI::setInputOutput($io);
 
+        $this->resetStreamFilterBuffer();
         command('key:generate --prefix base64');
 
         $this->assertNotSame($oldKey, env('encryption.key', $oldKey));
         $this->assertStringContainsString('base64:', (string) file_get_contents($this->envPath));
         $this->assertStringContainsString('Overwrite existing key?', $io->getOutput());
-        $this->assertStringContainsString('successfully set.', $io->getOutput());
+        $this->assertStringContainsString(sprintf('New encryption key written to ROOTPATH%s.env.', DIRECTORY_SEPARATOR), $io->getOutput());
     }
 
     #[PreserveGlobalState(false)]
@@ -279,19 +278,20 @@ public function testKeyGenerateAbortsNonInteractivelyWithExistingKey(): void
         $this->assertNotSame('', $key);
 
         $this->resetStreamFilterBuffer();
-
         command('key:generate --no-interaction');
 
         $this->assertSame($key, env('encryption.key', ''), 'Existing key should not change.');
-        $this->assertStringContainsString('Setting new encryption key aborted.', $this->getBuffer());
-        $this->assertStringContainsString('--force', $this->getBuffer());
+        $this->assertStringContainsString(
+            'Setting new encryption key aborted: pass --force to overwrite the existing key in non-interactive mode.',
+            $this->getStreamFilterBuffer(),
+        );
     }
 
     public function testKeyGenerateErrorsOnInvalidPrefixNonInteractively(): void
     {
         command('key:generate --prefix invalid --show --no-interaction');
 
-        $this->assertStringContainsString('Invalid prefix "invalid"', $this->getBuffer());
+        $this->assertStringContainsString('Invalid prefix "invalid"', $this->getStreamFilterBuffer());
     }
 
     public function testKeyGeneratePromptsForInvalidPrefix(): void
@@ -305,4 +305,20 @@ public function testKeyGeneratePromptsForInvalidPrefix(): void
         $this->assertStringContainsString('Please provide a valid prefix to use.', $io->getOutput());
         $this->assertStringContainsString('hex2bin:', $io->getOutput());
     }
+
+    #[RequiresOperatingSystem('Linux|Darwin')]
+    public function testKeyGenerateErrorsWhenEnvFileIsNotWritable(): void
+    {
+        command('key:generate');
+        chmod($this->envPath, 0o444);
+
+        try {
+            $this->resetStreamFilterBuffer();
+            command('key:generate --force');
+
+            $this->assertStringContainsString(sprintf('Failed to write new encryption key to ROOTPATH%s.env.', DIRECTORY_SEPARATOR), $this->getStreamFilterBuffer());
+        } finally {
+            chmod($this->envPath, 0o644);
+        }
+    }
 }
diff --git a/user_guide_src/source/changelogs/v4.8.0.rst b/user_guide_src/source/changelogs/v4.8.0.rst
@@ -31,6 +31,7 @@ Behavior Changes
 - **Commands:** Several built-in commands have been migrated from ``BaseCommand`` to the modern ``AbstractCommand`` style. Applications that extend a built-in command to override
   behaviour may need to re-implement against the modern API (``configure()`` + ``execute()`` and the ``#[Command]`` attribute) once the class it extends is migrated, or, preferably, compose instead of extending. Invocations on the command line are unaffected.
 - **Commands:** The success and error messages from ``debugbar:clear``, ``cache:clear``, and ``cache:info`` now include the affected path or cache driver/handler so the user can see which resource was acted on (or rejected). Scripts asserting on the prior literal text will need to be updated.
+- **Commands:** Declining the ``key:generate`` overwrite prompt interactively now returns ``EXIT_SUCCESS`` instead of ``EXIT_ERROR``. Output messages were also reworded; CI/automation that branches on the exit code or greps the previous wording will need updating.
 - **Database:** The Postgre driver's ``$db->error()['code']`` previously always returned ``''``. It now returns the 5-character SQLSTATE string for query and transaction failures (e.g., ``'42P01'``), or ``'08006'`` for connection-level failures. Code that relied on ``$db->error()['code'] === ''`` will need updating.
 - **Filters:** HTTP method matching for method-based filters is now case-sensitive. The keys in ``Config\Filters::$methods`` must exactly match the request method
   (e.g., ``GET``, ``POST``). Lowercase method names (e.g., ``post``) will no longer match.
