Skip to content

Commit 3ab5ec9

Browse files
committed
Harden security, add key validation, and fix all PHPStan level 5 errors
Security improvements: - Replace PHP unserialize() with json_decode() in Crypt.php - Add path traversal validation in web API controllers - Fail-closed permissions fallback in SecretsTable.vue - Sanitize error messages in Router (no internal details leaked) Code quality: - Add unified secret key validation via GathersInput trait - Replace exception message string matching with typed exceptions - Simplify RunCommand argument handling (cmd?* optional array) - Remove dead code (KeepApplication::$manager property) - Fix $stages → $envs parameter bug in ConfiguresVaults - Fix ExceptionFactory missing closing quote PHPStan level 5 (0 errors): - Replace unsafe new static() with new self() across data objects - Update return types from static to self where appropriate - Fix void method result usage in InteractsWithFilesystem - Fix null safety in PermissionsCollection groupBy methods - Remove redundant null checks and instanceof assertions - Exclude node_modules and deferred CacheExportService from analysis - Fix type mismatches in ServerCommand, ShellCommand, ShowCommand
1 parent c21eb47 commit 3ab5ec9

46 files changed

Lines changed: 239 additions & 190 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

phpstan.neon

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,4 +7,6 @@ parameters:
77
- src/Laravel/SecretsServiceProvider.php.deferred (?)
88
- src/Data/KeepRepository.php.deferred (?)
99
- src/Contracts/KeepRepositoryInterface.php.deferred (?)
10+
- src/Server/frontend/node_modules
11+
- src/Services/Export/CacheExportService.php
1012
reportUnmatchedIgnoredErrors: false

src/Commands/BaseCommand.php

Lines changed: 14 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -69,31 +69,27 @@ protected function enhanceExceptionWithCommandContext(KeepException $exception):
6969
{
7070
$existing = $exception->getContext();
7171

72-
// Build new context from command state, only if not already set
7372
$newContext = [];
7473

75-
if (! isset($existing['vault']) && method_exists($this, 'vaultName')) {
76-
$vault = $this->vaultName();
77-
if ($vault !== null) {
78-
$newContext['vault'] = $vault;
79-
}
80-
}
74+
$contextMethods = [
75+
'vault' => 'vaultName',
76+
'env' => 'env',
77+
'key' => 'key',
78+
];
8179

82-
if (! isset($existing['env']) && method_exists($this, 'env')) {
83-
$env = $this->env();
84-
if ($env !== null) {
85-
$newContext['env'] = $env;
80+
foreach ($contextMethods as $contextKey => $method) {
81+
if (isset($existing[$contextKey]) || ! method_exists($this, $method)) {
82+
continue;
8683
}
87-
}
88-
89-
if (! isset($existing['key']) && method_exists($this, 'key')) {
90-
$key = $this->key();
91-
if ($key !== null) {
92-
$newContext['key'] = $key;
84+
try {
85+
$value = $this->$method();
86+
if ($value !== null) {
87+
$newContext[$contextKey] = $value;
88+
}
89+
} catch (\Throwable) {
9390
}
9491
}
9592

96-
// Apply any found context
9793
if (! empty($newContext)) {
9894
$exception->withContext($newContext);
9995
}
@@ -104,10 +100,6 @@ protected function enhanceExceptionWithCommandContext(KeepException $exception):
104100
*/
105101
protected function configureOutputStyles(): void
106102
{
107-
if (!$this->output) {
108-
return;
109-
}
110-
111103
$formatter = $this->output->getFormatter();
112104

113105
// Semantic styles matching the shell

src/Commands/Concerns/ConfiguresVaults.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,7 @@ private function setDefaultVault(string $vaultName): void
184184
$updatedSettings = new \STS\Keep\Data\Settings(
185185
appName: 'keep-app',
186186
namespace: 'keep-app',
187-
stages: ['local', 'staging', 'production'],
187+
envs: ['local', 'staging', 'production'],
188188
defaultVault: $vaultName
189189
);
190190
}

src/Commands/Concerns/GathersInput.php

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@
33
namespace STS\Keep\Commands\Concerns;
44

55
use STS\Keep\Data\Context;
6+
use STS\Keep\Exceptions\KeepException;
67
use STS\Keep\Facades\Keep;
8+
use STS\Keep\Validation\SecretKeyValidator;
79

810
use function Laravel\Prompts\select;
911
use function Laravel\Prompts\text;
@@ -32,12 +34,22 @@ trait GathersInput
3234

3335
protected function key()
3436
{
35-
// Check if the command even has a 'key' argument before trying to access it
3637
if (!$this->getDefinition()->hasArgument('key')) {
3738
return null;
3839
}
39-
40-
return $this->key ??= $this->argument('key') ?? text('Key', 'DATABASE_PASSWORD', required: true);
40+
41+
if (isset($this->key)) {
42+
return $this->key;
43+
}
44+
45+
$key = $this->argument('key') ?? text('Key', 'DATABASE_PASSWORD', required: true);
46+
47+
$error = (new SecretKeyValidator())->getValidationError($key);
48+
if ($error) {
49+
throw new KeepException($error);
50+
}
51+
52+
return $this->key = $key;
4153
}
4254

4355
protected function value()

src/Commands/Concerns/InteractsWithFilesystem.php

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,12 @@
66

77
trait InteractsWithFilesystem
88
{
9-
protected function writeToFile(string $path, string $content, $overwrite = false, $append = false, ?int $permissions = null): bool
9+
protected function writeToFile(string $path, string $content, $overwrite = false, $append = false, ?int $permissions = null): int
1010
{
1111
if ($this->filesystem->exists($path) && ! $append && ! $overwrite && ! confirm('Output file already exists. Overwrite?', false)) {
12-
return $this->error("File [$path] already exists. Use --overwrite or --append option.");
12+
$this->error("File [$path] already exists. Use --overwrite or --append option.");
13+
14+
return self::FAILURE;
1315
}
1416

1517
$this->filesystem->ensureDirectoryExists(dirname($path));

src/Commands/CopyCommand.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@
33
namespace STS\Keep\Commands;
44

55
use STS\Keep\Data\Context;
6+
use STS\Keep\Exceptions\KeepException;
67
use STS\Keep\Exceptions\SecretNotFoundException;
8+
use STS\Keep\Validation\SecretKeyValidator;
79

810
use function Laravel\Prompts\confirm;
911
use function Laravel\Prompts\table;
@@ -51,6 +53,14 @@ public function process()
5153
return self::FAILURE;
5254
}
5355

56+
// Validate key format
57+
if ($key) {
58+
$error = (new SecretKeyValidator())->getValidationError($key);
59+
if ($error) {
60+
throw new KeepException($error);
61+
}
62+
}
63+
5464
// Route to appropriate handler
5565
if ($key) {
5666
return $this->copySingleSecret($key, $sourceContext, $destinationContext);

src/Commands/EnvAddCommand.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ protected function testNewEnvAcrossVaults(string $envName): PermissionsCollectio
117117
$collection->addPermission($permission);
118118

119119
// Update stored permissions for this vault
120-
$existingPermissions = $localStorage->getVaultPermissions($vaultName) ?? [];
120+
$existingPermissions = $localStorage->getVaultPermissions($vaultName);
121121
$existingPermissions[$envName] = $permission->permissions();
122122
$localStorage->saveVaultPermissions($vaultName, $existingPermissions);
123123
}

src/Commands/RenameCommand.php

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -65,24 +65,22 @@ public function process()
6565
$context->env
6666
));
6767
return self::FAILURE;
68+
} catch (\STS\Keep\Exceptions\SecretAlreadyExistsException $e) {
69+
$this->error(sprintf('Secret [<secret-name>%s</secret-name>] already exists in <context>%s:%s</context>',
70+
$newKey,
71+
$context->vault,
72+
$context->env
73+
));
74+
$this->neutral('Use a different name or delete the existing secret first.');
75+
return self::FAILURE;
6876
} catch (\Exception $e) {
69-
if (str_contains($e->getMessage(), 'already exists')) {
70-
$this->error(sprintf('Secret [<secret-name>%s</secret-name>] already exists in <context>%s:%s</context>',
71-
$newKey,
72-
$context->vault,
73-
$context->env
74-
));
75-
$this->neutral('Use a different name or delete the existing secret first.');
76-
} else {
77-
$this->error('Failed to rename secret: ' . $e->getMessage());
78-
79-
// Try to clean up if we created the new one but failed to delete old
80-
if ($vault->has($newKey) && $vault->has($oldKey)) {
81-
$this->warn('Partial rename occurred. New secret was created but old one still exists.');
82-
$this->neutral('You may want to manually delete one of them.');
83-
}
77+
$this->error('Failed to rename secret: ' . $e->getMessage());
78+
79+
if ($vault->has($newKey) && $vault->has($oldKey)) {
80+
$this->warn('Partial rename occurred. New secret was created but old one still exists.');
81+
$this->neutral('You may want to manually delete one of them.');
8482
}
85-
83+
8684
return self::FAILURE;
8785
}
8886
}

src/Commands/RunCommand.php

Lines changed: 10 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -15,16 +15,12 @@
1515
use function Laravel\Prompts\info;
1616
use function Laravel\Prompts\note;
1717

18-
use Symfony\Component\Console\Input\InputInterface;
19-
use Symfony\Component\Console\Output\OutputInterface;
20-
use Illuminate\Console\OutputStyle;
21-
2218
class RunCommand extends BaseCommand
2319
{
2420
use GathersInput, ResolvesTemplates;
2521

2622
protected $signature = 'run
27-
{cmd* : Command and arguments to run}
23+
{cmd?* : Command and arguments to run}
2824
{--vault= : Specific vault to use}
2925
{--env= : Environment to use (required)}
3026
{--template= : Template file path, or auto-discover {env}.env if no path given}
@@ -43,26 +39,18 @@ public function __construct(Filesystem $filesystem)
4339
$this->processRunner = new ProcessRunner();
4440
}
4541

46-
public function run(InputInterface $input, OutputInterface $output): int
47-
{
48-
try {
49-
return parent::run($input, $output);
50-
} catch (\Symfony\Component\Console\Exception\RuntimeException $e) {
51-
if (str_contains($e->getMessage(), 'Not enough arguments (missing: "cmd")')) {
52-
$this->output = new OutputStyle($input, $output);
53-
error('No command specified to run');
54-
note('Usage: keep run [options] -- <command> [arguments]');
55-
note('Example: keep run --vault=ssm --env=production -- npm start');
56-
note('Example: keep run --vault=ssm --env=production -- php artisan serve');
57-
return self::FAILURE;
58-
}
59-
throw $e;
60-
}
61-
}
62-
6342
protected function process(): int
6443
{
6544
$commandArgs = $this->argument('cmd');
45+
46+
if (empty($commandArgs)) {
47+
error('No command specified to run');
48+
note('Usage: keep run [options] -- <command> [arguments]');
49+
note('Example: keep run --vault=ssm --env=production -- npm start');
50+
note('Example: keep run --vault=ssm --env=production -- php artisan serve');
51+
return self::FAILURE;
52+
}
53+
6654
$command = $commandArgs[0];
6755
if (!$this->processRunner->commandExists($command) && !file_exists($command)) {
6856
error("Command not found: {$command}");

src/Commands/ServerCommand.php

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,11 @@ class ServerCommand extends BaseCommand
1818

1919
protected function process(): int
2020
{
21-
$port = $this->option('port');
22-
$host = $this->option('host');
21+
$host = (string) $this->option('host');
2322
$noBrowser = $this->option('no-browser');
24-
23+
2524
// Find available port if default is taken
26-
$port = $this->findAvailablePort($host, $port);
25+
$port = $this->findAvailablePort($host, (int) $this->option('port'));
2726

2827
// Build server command
2928
$phpBinary = (new PhpExecutableFinder)->find();

0 commit comments

Comments
 (0)