secrets commands. #2
Conversation
📝 WalkthroughWalkthroughAdds three new CLI commands for secret management (key generation, editing, and viewing), PHPUnit tests for each command, two runtime composer dependencies, and removal of a PSR-4 autoload-dev mapping. Changes
Sequence DiagramsequenceDiagram
actor User as CLI User
participant Gen as secrets:key:generate
participant Edit as secrets:edit
participant Show as secrets:show
participant SM as SecretManager
participant FS as File System
participant Editor as External Editor
rect `#78B4C8`!10
Note over Gen,FS: Key generation flow
User->>Gen: run command (--env/--force/--show)
Gen->>FS: check/create key path
FS-->>Gen: path result
Gen->>SM: generateKey(path, force)
SM->>FS: write key file
SM-->>Gen: success
Gen-->>User: output key/path
end
rect `#78C896`!08
Note over Edit,Editor: Edit encrypted secrets
User->>Edit: run command (--env --editor)
Edit->>FS: load key file & encrypted secrets
FS-->>Edit: files
Edit->>SM: decrypt contents
SM-->>Edit: plaintext
Edit->>Editor: open temp file
Editor->>Editor: user edits
Editor-->>Edit: closed
Edit->>SM: re-encrypt updated contents
SM->>FS: write encrypted secrets
SM-->>Edit: success
Edit-->>User: saved message
end
rect `#C8A0B0`!06
Note over Show,SM: Display secrets
User->>Show: run command (--env --key --force)
Show->>FS: load key file & encrypted secrets
FS-->>Show: files
Show->>SM: decrypt secrets
SM-->>Show: decrypted data
Show-->>User: display (filtered) secrets
end
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI Agents
In @composer.json:
- Around line 15-17: The project references a non-existent class
Neuron\Data\Settings\SecretManager (used across commands/tests), but the
neuron-php/data package only provides SettingManager and settings sources; fix
by either implementing a SecretManager class in the neuron-php/data package
(matching the API used by callers) or remove/replace all uses of SecretManager
with the provided SettingManager or an appropriate settings source; update
imports/usages in files that reference SecretManager to point to the implemented
class or to SettingManager (e.g., replace Neuron\Data\Settings\SecretManager
usage in command classes and tests), and ensure composer.json and autoloading
reflect the new/changed class.
In @src/Cli/Commands/Secrets/EditCommand.php:
- Around line 117-120: The EditCommand currently checks
$this->input->hasOption('verbose') in execute() but the configure() method
doesn't register that option; update EditCommand::configure() to add the verbose
option by calling $this->addOption('verbose', 'v', false, 'Verbose output')
(matching how VersionCommand defines it) so --verbose / -v is declared and
appears in help; keep the existing check in execute() that calls
$this->output->write($e->getTraceAsString()) when
$this->input->hasOption('verbose') to preserve behavior.
In @src/Cli/Commands/Secrets/Key/GenerateCommand.php:
- Around line 145-148: The GenerateCommand class is checking
$this->input->hasOption('verbose') in execute() but never defines that option in
configure(); update the GenerateCommand::configure() method to add a 'verbose'
option (same signature used in EditCommand or other commands in this PR) so
hasOption()/getOption('verbose') calls are valid, and apply the same fix to any
other command classes with the same pattern.
In @src/Cli/Commands/Secrets/ShowCommand.php:
- Around line 142-145: The command checks $this->input->hasOption('verbose') in
execute (around the error handling block) but never defines that option; add a
verbose option in the command configuration (e.g., in the configure() method of
the ShowCommand class) by calling addOption('verbose', null,
InputOption::VALUE_NONE, 'Enable verbose error output') so hasOption() and has()
work consistently with other commands and the stack trace is only printed when
the option is provided.
- Around line 68-72: The call to Output::confirm does not exist and will throw
at runtime; replace the check that uses $this->output->confirm(...) with
$this->input->confirm(...) in the Secrets\ShowCommand class so the confirmation
uses the Input object; update the conditional that currently reads if(
!$this->output->confirm(...) ) to call $this->input->confirm(...) and keep the
existing cancel branch (info log and return 0) intact.
In @tests/Cli/Commands/Secrets/EditCommandTest.php:
- Around line 165-192: The test method testExecuteWithError assumes
/root/cannot_write_here is non-writable which can fail in CI; update
testExecuteWithError to create a controlled non-writable path (e.g., use
sys_get_temp_dir() with a unique folder or file, set restrictive permissions, or
remove the file) or mock the file system calls used by your command so that
calling $this->command->execute() reliably fails; ensure you restore
permissions/cleanup after the test and keep references to the same Input
creation and Output capture logic so assertions on the execute() return value
and $outputContent remain valid.
🧹 Nitpick comments (5)
src/Cli/Commands/Secrets/EditCommand.php (1)
85-85: Remove unused variable or use it meaningfully.The
$keyvariable is assigned but never used. If you don't need the generated key value here, callgenerateKey()without assignment.🔎 Proposed fix
- $key = $this->secretManager->generateKey( $keyPath ); + $this->secretManager->generateKey( $keyPath );tests/Cli/Commands/Secrets/Key/GenerateCommandTest.php (1)
163-177: Consider extracting sharedremoveDirectoryhelper to a base test class.This helper method is duplicated across
GenerateCommandTest,ShowCommandTest, andEditCommandTest. Consider extracting it to a sharedSecretsTestCasebase class to reduce duplication.tests/Cli/Commands/Secrets/ShowCommandTest.php (2)
63-63: Remove unused$keyvariable.The return value from
generateKey()is assigned but never used.🔎 Proposed fix
- $key = $this->secretManager->generateKey( $keyPath ); + $this->secretManager->generateKey( $keyPath );
104-104: Remove unused$keyvariable.Same issue as line 63.
🔎 Proposed fix
- $key = $this->secretManager->generateKey( $keyPath ); + $this->secretManager->generateKey( $keyPath );tests/Cli/Commands/Secrets/EditCommandTest.php (1)
79-79: Remove unused$keyvariable.The return value from
generateKey()is assigned but never used.🔎 Proposed fix
- $key = $secretManager->generateKey( $keyPath ); + $secretManager->generateKey( $keyPath );
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
composer.jsonsrc/Cli/Commands/Secrets/EditCommand.phpsrc/Cli/Commands/Secrets/Key/GenerateCommand.phpsrc/Cli/Commands/Secrets/ShowCommand.phptests/Cli/Commands/Secrets/EditCommandTest.phptests/Cli/Commands/Secrets/Key/GenerateCommandTest.phptests/Cli/Commands/Secrets/ShowCommandTest.php
🧰 Additional context used
🧬 Code graph analysis (5)
src/Cli/Commands/Secrets/ShowCommand.php (3)
src/Cli/Commands/Command.php (2)
Command(15-431)addOption(115-137)src/Cli/Console/Input.php (1)
getOption(173-176)src/Cli/Console/Output.php (6)
warning(191-194)info(169-172)title(391-402)error(202-205)section(377-383)newLine(363-369)
tests/Cli/Commands/Secrets/ShowCommandTest.php (4)
src/Cli/Commands/Secrets/ShowCommand.php (5)
ShowCommand(22-167)configure(45-51)getName(29-32)getDescription(37-40)execute(56-151)src/Cli/Console/Input.php (2)
Input(11-499)parse(85-131)src/Cli/Console/Output.php (1)
Output(9-403)src/Cli/Commands/Command.php (2)
setInput(61-65)setOutput(73-77)
tests/Cli/Commands/Secrets/Key/GenerateCommandTest.php (4)
src/Cli/Console/Input.php (2)
Input(11-499)parse(85-131)src/Cli/Console/Output.php (1)
Output(9-403)src/Cli/Console/TestStream.php (1)
TestStream(12-128)src/Cli/Commands/Command.php (2)
setInput(61-65)setOutput(73-77)
src/Cli/Commands/Secrets/Key/GenerateCommand.php (5)
src/Cli/Commands/Command.php (2)
Command(15-431)addOption(115-137)src/Cli/Commands/Secrets/EditCommand.php (4)
getName(28-31)getDescription(36-39)configure(44-49)execute(54-126)src/Cli/Commands/Secrets/ShowCommand.php (4)
getName(29-32)getDescription(37-40)configure(45-51)execute(56-151)src/Cli/Console/Input.php (1)
getOption(173-176)src/Cli/Console/Output.php (6)
error(202-205)info(169-172)warning(191-194)success(180-183)newLine(363-369)section(377-383)
tests/Cli/Commands/Secrets/EditCommandTest.php (4)
src/Cli/Commands/Secrets/EditCommand.php (5)
EditCommand(21-127)configure(44-49)getName(28-31)getDescription(36-39)execute(54-126)src/Cli/Console/Input.php (3)
Input(11-499)parse(85-131)getOption(173-176)src/Cli/Console/Output.php (1)
Output(9-403)src/Cli/Commands/Command.php (2)
setInput(61-65)setOutput(73-77)
🪛 GitHub Actions: CI
src/Cli/Commands/Secrets/EditCommand.php
[error] 75-75: Class "Neuron\Data\Settings\SecretManager" not found
tests/Cli/Commands/Secrets/ShowCommandTest.php
[error] 27-27: Class "Neuron\Data\Settings\SecretManager" not found
[error] 27-27: Class "Neuron\Data\Settings\SecretManager" not found
[error] 27-27: Class "Neuron\Data\Settings\SecretManager" not found
tests/Cli/Commands/Secrets/Key/GenerateCommandTest.php
[error] 69-69: Class "Neuron\Data\Settings\SecretManager" not found
[error] 109-109: Class "Neuron\Data\Settings\SecretManager" not found
tests/Cli/Commands/Secrets/EditCommandTest.php
[error] 78-78: Class "Neuron\Data\Settings\SecretManager" not found
🪛 PHPMD (2.15.0)
src/Cli/Commands/Secrets/EditCommand.php
85-85: Avoid unused local variables such as '$key'. (undefined)
(UnusedLocalVariable)
tests/Cli/Commands/Secrets/ShowCommandTest.php
63-63: Avoid unused local variables such as '$key'. (undefined)
(UnusedLocalVariable)
104-104: Avoid unused local variables such as '$key'. (undefined)
(UnusedLocalVariable)
tests/Cli/Commands/Secrets/EditCommandTest.php
79-79: Avoid unused local variables such as '$key'. (undefined)
(UnusedLocalVariable)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (7)
src/Cli/Commands/Secrets/EditCommand.php (1)
21-31: Command structure follows established patterns.The command registration with
getName()andgetDescription()follows the framework conventions shown in other commands.tests/Cli/Commands/Secrets/Key/GenerateCommandTest.php (1)
55-87: Test coverage for key generation is comprehensive.The test validates file existence, key format (64 hex characters), and expected output messages. Good coverage of the happy path.
src/Cli/Commands/Secrets/Key/GenerateCommand.php (2)
117-124: Security consideration: Key display via--showflag.Displaying the key on terminal is useful for initial setup but carries risk if terminal history is logged. The warning on line 123 appropriately cautions users. Consider also mentioning terminal history in the warning.
100-104: No action needed. Theconfirm()method is properly defined in the baseCommandclass (line 382 of src/Cli/Commands/Command.php) with signatureprotected function confirm( string $message, bool $default = false ): bool. The code will execute without runtime errors.Likely an incorrect or invalid review comment.
tests/Cli/Commands/Secrets/ShowCommandTest.php (1)
57-93: Test setup and assertions are thorough.The test creates realistic encrypted data, exercises the command, and verifies both output content and success status.
src/Cli/Commands/Secrets/ShowCommand.php (1)
159-166: Environment key detection is consistent withGenerateCommand.The environment variable naming convention (
NEURON_{NAME}_KEY) matches the pattern used inGenerateCommandlines 134-136.tests/Cli/Commands/Secrets/EditCommandTest.php (1)
74-117: Test for existing key scenario is well-structured.The test properly sets up the environment, uses
echoas a no-op editor, and validates both file state and output messages.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI Agents
In @src/Cli/Commands/Secrets/EditCommand.php:
- Line 97: The assignment to $key in EditCommand::execute (the line calling
$this->secretManager->generateKey($keyPath)) creates an unused variable; either
remove the "$key = " prefix and call $this->secretManager->generateKey($keyPath)
for its side effects, or keep the assignment and use $key (for example by
logging via $this->logger or outputting to the console) so it is no longer
unused. Ensure you modify the code around the generateKey call in
EditCommand.php accordingly and run static analysis to confirm no
unused-variable warnings remain.
🧹 Nitpick comments (1)
src/Cli/Commands/Secrets/Key/GenerateCommand.php (1)
69-78: Consider extracting directory creation logic.The directory creation logic here is duplicated in
EditCommand(lines 86-95). While this is functional, consider extracting it to a shared helper method in the baseCommandclass for better maintainability.Example refactor
Add to base
Commandclass:protected function ensureDirectoryExists( string $path ): bool { if( !is_dir( $path ) ) { if( !mkdir( $path, 0755, true ) ) { $this->output->error( "Failed to create directory: {$path}" ); return false; } } return true; }Then in both commands:
- // Ensure directory exists - $dir = dirname( $keyPath ); - if( !is_dir( $dir ) ) - { - if( !mkdir( $dir, 0755, true ) ) - { - $this->output->error( "Failed to create directory: {$dir}" ); - return 1; - } - } + $dir = dirname( $keyPath ); + if( !$this->ensureDirectoryExists( $dir ) ) + { + return 1; + }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/Cli/Commands/Secrets/EditCommand.phpsrc/Cli/Commands/Secrets/Key/GenerateCommand.phpsrc/Cli/Commands/Secrets/ShowCommand.phptests/Cli/Commands/Secrets/EditCommandTest.phptests/Cli/Commands/Secrets/ShowCommandTest.php
🧰 Additional context used
🧬 Code graph analysis (4)
src/Cli/Commands/Secrets/Key/GenerateCommand.php (3)
src/Cli/Commands/Command.php (2)
Command(15-431)addOption(115-137)src/Cli/Console/Input.php (1)
getOption(173-176)src/Cli/Console/Output.php (6)
error(202-205)info(169-172)warning(191-194)success(180-183)newLine(363-369)section(377-383)
src/Cli/Commands/Secrets/EditCommand.php (3)
src/Cli/Commands/Command.php (2)
Command(15-431)addOption(115-137)src/Cli/Console/Input.php (1)
getOption(173-176)src/Cli/Console/Output.php (5)
info(169-172)warning(191-194)error(202-205)success(180-183)newLine(363-369)
tests/Cli/Commands/Secrets/EditCommandTest.php (4)
src/Cli/Commands/Secrets/EditCommand.php (5)
EditCommand(21-139)configure(44-50)getName(28-31)getDescription(36-39)execute(55-138)src/Cli/Console/Input.php (3)
Input(11-499)parse(85-131)getOption(173-176)src/Cli/Console/Output.php (1)
Output(9-403)src/Cli/Commands/Command.php (2)
setInput(61-65)setOutput(73-77)
tests/Cli/Commands/Secrets/ShowCommandTest.php (2)
src/Cli/IO/TestInputReader.php (2)
TestInputReader(28-198)addResponse(59-63)src/Cli/Commands/Command.php (3)
setInput(61-65)setOutput(73-77)setInputReader(341-345)
🪛 GitHub Actions: CI
src/Cli/Commands/Secrets/EditCommand.php
[error] 76-76: Class "Neuron\Data\Settings\SecretManager" not found
🪛 PHPMD (2.15.0)
src/Cli/Commands/Secrets/EditCommand.php
97-97: Avoid unused local variables such as '$key'. (undefined)
(UnusedLocalVariable)
tests/Cli/Commands/Secrets/EditCommandTest.php
79-79: Avoid unused local variables such as '$key'. (undefined)
(UnusedLocalVariable)
tests/Cli/Commands/Secrets/ShowCommandTest.php
64-64: Avoid unused local variables such as '$key'. (undefined)
(UnusedLocalVariable)
105-105: Avoid unused local variables such as '$key'. (undefined)
(UnusedLocalVariable)
149-149: Avoid unused local variables such as '$key'. (undefined)
(UnusedLocalVariable)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (11)
src/Cli/Commands/Secrets/Key/GenerateCommand.php (2)
44-51: LGTM! Verbose option now properly configured.The previously flagged issue about the
verboseoption being checked but not configured has been resolved. The option is now properly registered inconfigure()and used in error handling.
135-137: Environment variable naming convention is consistent.The environment variable naming logic correctly matches the pattern used in
ShowCommand::checkEnvironmentKey()(lines 162-164), ensuring keys can be sourced from environment variables when the file is absent.src/Cli/Commands/Secrets/EditCommand.php (1)
44-50: LGTM! Configuration issues resolved.Both previously flagged issues have been addressed:
- The
verboseoption is now properly configured (line 49)- Directory creation logic is present (lines 86-95)
tests/Cli/Commands/Secrets/EditCommandTest.php (2)
213-245: LGTM! Error test improved with controlled non-writable directory.The error handling test has been improved from the previous version. It now creates a controlled non-writable directory with proper cleanup, addressing the previous concern about relying on system paths like
/root/cannot_write_here.
16-34: Comprehensive test coverage.The test suite thoroughly covers:
- Command metadata validation
- Base secrets editing with existing keys
- Environment-specific secret editing
- Automatic directory creation when missing
- Error handling with proper cleanup
Also applies to: 74-117, 122-160, 165-208
tests/Cli/Commands/Secrets/ShowCommandTest.php (2)
142-234: Excellent production safeguard testing.The production confirmation test thoroughly covers all three scenarios:
- User confirms - secrets are displayed
- User declines - operation cancelled gracefully
- Force flag - confirmation bypassed
This ensures the security feature works as intended.
58-94: Comprehensive test coverage for ShowCommand.The test suite validates:
- Base secrets display with security reminder
- Key-specific filtering (showing only requested sections)
- Error handling for missing files with helpful guidance
All critical paths are covered.
Also applies to: 99-137, 239-264
src/Cli/Commands/Secrets/ShowCommand.php (4)
45-52: LGTM! Configuration issues resolved.The verbose option is now properly configured (line 51), addressing the previous concern. All command options are correctly registered.
64-74: LGTM! Confirmation method correctly called.The production confirmation correctly uses
$this->confirm()(line 69), which is a method on the baseCommandclass. The previously reported issue about calling$this->output->confirm()has been resolved.
160-167: Environment key detection aligns with GenerateCommand.The
checkEnvironmentKey()method uses the same naming convention asGenerateCommand(lines 135-137), ensuring consistency between key generation and lookup. This allows keys to be sourced from environment variables when files are not present.
108-125: Clean YAML filtering implementation.The specific key filtering logic cleanly extracts and validates the requested section, providing clear error messages when keys are not found.
Codecov Report❌ Patch coverage is
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
tests/Cli/Commands/Secrets/ShowCommandTest.php (4)
58-94: Consider removing unused variable assignment.Line 64 assigns
$keybut never uses it. The key is persisted to disk bygenerateKey(), so the return value isn't necessary here.🔎 Proposed fix
- $key = $this->secretManager->generateKey( $keyPath ); + $this->secretManager->generateKey( $keyPath );
99-137: Consider removing unused variable assignment.Line 105 has the same unused
$keyassignment as in the previous test method.🔎 Proposed fix
- $key = $this->secretManager->generateKey( $keyPath ); + $this->secretManager->generateKey( $keyPath );
142-234: Excellent test coverage for production confirmation workflow.This test comprehensively validates three scenarios: user confirms, user cancels, and force flag bypasses confirmation. The use of
TestInputReaderto simulate user responses is well done.One minor note: Line 149 has an unused
$keyvariable assignment (same pattern as other test methods).🔎 Optional fix for unused variable
- $key = $this->secretManager->generateKey( $keyPath ); + $this->secretManager->generateKey( $keyPath );
239-283: LGTM: Environment variable fallback is properly tested.The test correctly validates that secrets can be decrypted using a key from an environment variable when the key file doesn't exist. The cleanup at line 282 properly removes the environment variable.
One minor note: Line 246 has an unused
$keyvariable assignment (same pattern as other test methods).🔎 Optional fix for unused variable
- $key = $this->secretManager->generateKey( $keyPath ); + $this->secretManager->generateKey( $keyPath );src/Cli/Commands/Secrets/EditCommand.php (1)
78-100: LGTM: Directory creation and key generation logic.The code properly ensures the directory exists before generating the key (lines 86-95), addressing previous review concerns. The key generation provides clear user guidance about
.gitignore.One minor note: Line 97's
$keyvariable assignment is unused. While this doesn't affect functionality (the key is persisted to disk), you could remove the assignment for cleaner code.🔎 Optional cleanup
- $this->secretManager->generateKey( $keyPath ); + $this->secretManager->generateKey( $keyPath );(Note: The assignment is already without the
$key =prefix, so no change needed. This is correct as-is.)
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/Cli/Commands/Secrets/EditCommand.phpsrc/Cli/Commands/Secrets/Key/GenerateCommand.phpsrc/Cli/Commands/Secrets/ShowCommand.phptests/Cli/Commands/Secrets/Key/GenerateCommandTest.phptests/Cli/Commands/Secrets/ShowCommandTest.php
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/Cli/Commands/Secrets/Key/GenerateCommandTest.php
- src/Cli/Commands/Secrets/ShowCommand.php
🧰 Additional context used
🧬 Code graph analysis (2)
src/Cli/Commands/Secrets/EditCommand.php (3)
src/Cli/Commands/Command.php (2)
Command(15-431)addOption(115-137)src/Cli/Console/Input.php (1)
getOption(173-176)src/Cli/Console/Output.php (5)
info(169-172)warning(191-194)error(202-205)success(180-183)newLine(363-369)
tests/Cli/Commands/Secrets/ShowCommandTest.php (3)
src/Cli/Commands/Secrets/ShowCommand.php (5)
ShowCommand(22-168)configure(45-52)getName(29-32)getDescription(37-40)execute(57-152)src/Cli/IO/TestInputReader.php (2)
TestInputReader(28-198)addResponse(59-63)src/Cli/Commands/Command.php (3)
setInput(61-65)setOutput(73-77)setInputReader(341-345)
🪛 PHPMD (2.15.0)
tests/Cli/Commands/Secrets/ShowCommandTest.php
64-64: Avoid unused local variables such as '$key'. (undefined)
(UnusedLocalVariable)
105-105: Avoid unused local variables such as '$key'. (undefined)
(UnusedLocalVariable)
149-149: Avoid unused local variables such as '$key'. (undefined)
(UnusedLocalVariable)
246-246: Avoid unused local variables such as '$key'. (undefined)
(UnusedLocalVariable)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (12)
tests/Cli/Commands/Secrets/ShowCommandTest.php (4)
18-29: LGTM: Test setup is well-structured.The setup creates an isolated temporary directory with a unique ID and properly initializes the command and SecretManager for testing.
31-37: LGTM: Proper cleanup in tearDown.The tearDown method ensures test artifacts are removed, preventing pollution across test runs.
288-313: LGTM: Error handling is properly tested.The test validates that appropriate error messages and exit codes are returned when the secrets file doesn't exist.
318-332: LGTM: Safe recursive directory removal.The helper method properly handles recursive directory removal with appropriate safety checks.
src/Cli/Commands/Secrets/EditCommand.php (3)
44-50: LGTM: Command options properly configured.All options including
verboseare now properly registered in theconfigure()method, addressing previous review feedback.
102-123: LGTM: Secret editing and user guidance.The edit flow is well-structured with clear success/failure handling and helpful reminders for first-time setup.
125-135: LGTM: Proper error handling with verbose support.Exception handling correctly uses the configured
verboseoption to conditionally display stack traces.src/Cli/Commands/Secrets/Key/GenerateCommand.php (5)
44-51: LGTM: Command options properly configured.All options including
verboseare properly registered, addressing previous review feedback.
63-95: LGTM: Directory creation for both key types.The code properly ensures directories exist for both environment-specific keys (lines 69-78) and master keys (lines 85-94), addressing previous review concerns.
97-117: LGTM: Proper safeguards against accidental key overwrite.The command correctly prevents accidental overwrites and requires explicit confirmation when
--forceis used, with clear warnings about the consequences.
122-158: LGTM: Key display and export instructions respect --show flag.The code correctly uses the
$keyreturn value at line 124 and conditionally displays it based on the--showflag (lines 129-136). The export command instructions at lines 151-158 also properly check the--showflag before displaying the actual key value, addressing previous security concerns about unconditional key exposure.
160-170: LGTM: Proper error handling with verbose support.Exception handling correctly uses the configured
verboseoption to conditionally display stack traces.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
tests/Cli/Commands/Secrets/EditCommandTest.php (3)
79-79: Remove unused variable.The return value from
generateKey()is assigned but never used in this test.🔎 Suggested fix
- $key = $secretManager->generateKey( $keyPath ); + $secretManager->generateKey( $keyPath );
271-271: Use proper syntax for unsetting environment variable.
putenv('EDITOR')without=may not reliably unset the variable on all PHP versions and systems. Useputenv('EDITOR=')instead.🔎 Suggested fix
- putenv( 'EDITOR' ); + putenv( 'EDITOR=' );
277-309: Consider mocking for more reliable error testing.While the 0000 permissions approach is an improvement over the previous implementation, it may still fail to trigger an error when running as root (common in containerized CI environments). For fully reliable error testing, consider mocking the
SecretManageror filesystem operations to force an exception.🔎 Alternative approach using a mock
Instead of relying on filesystem permissions, you could mock the SecretManager to throw an exception:
// Create a mock that throws an exception $mockSecretManager = $this->createMock(SecretManager::class); $mockSecretManager->method('generateKey') ->willThrowException(new \Exception('Simulated error')); // Inject the mock into the command // (This would require modifying EditCommand to accept SecretManager via constructor)Note: This would require refactoring
EditCommandto accept aSecretManagerdependency via constructor injection rather than instantiating it internally.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/Cli/Commands/Secrets/EditCommand.phpsrc/Cli/Commands/Secrets/ShowCommand.phptests/Cli/Commands/Secrets/EditCommandTest.php
🚧 Files skipped from review as they are similar to previous changes (2)
- src/Cli/Commands/Secrets/ShowCommand.php
- src/Cli/Commands/Secrets/EditCommand.php
🧰 Additional context used
🧬 Code graph analysis (1)
tests/Cli/Commands/Secrets/EditCommandTest.php (2)
src/Cli/Commands/Secrets/EditCommand.php (5)
EditCommand(21-151)configure(44-50)getName(28-31)getDescription(36-39)execute(55-150)src/Cli/Commands/Command.php (2)
setInput(61-65)setOutput(73-77)
🪛 PHPMD (2.15.0)
tests/Cli/Commands/Secrets/EditCommandTest.php
79-79: Avoid unused local variables such as '$key'. (undefined)
(UnusedLocalVariable)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (2)
tests/Cli/Commands/Secrets/EditCommandTest.php (2)
16-26: Setup follows best practices.The use of
sys_get_temp_dir()withuniqid()ensures test isolation and prevents conflicts between concurrent test runs.
314-328: Helper method is well-implemented.The recursive directory removal correctly handles nested structures and filters out
.and..entries.
Note
Introduces CLI support for managing encrypted secrets.
secrets:edit,secrets:show, andsecrets:key:generatecommands with environment-specific paths, safety prompts (production confirmation), and support for editor selection and env-var keys; outputs formatted YAML when showing secretsneuron-php/dataandsymfony/yaml; cleans dev autoload mappingWritten by Cursor Bugbot for commit aa1560d. This will update automatically on new commits. Configure here.
Summary by CodeRabbit
New Features
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.