Skip to content

Commit d2a58ec

Browse files
paulbalandanpaulbalandan
committed
validate key:rotate inputs upfront and harden .env rewrite
1 parent d98572d commit d2a58ec

3 files changed

Lines changed: 156 additions & 21 deletions

File tree

‎system/Commands/Encryption/RotateKey.php‎

Lines changed: 32 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,14 @@ protected function execute(array $arguments, array $options): int
122122
return EXIT_ERROR;
123123
}
124124

125+
$length = $options['length'];
126+
127+
if (! is_numeric($length) || (int) $length < 1) {
128+
CLI::error('The --length option must be a positive integer.');
129+
130+
return EXIT_ERROR;
131+
}
132+
125133
$previousKeys = $this->mergePreviousKeys($currentKey, $this->parsePreviousKeys(), (int) $keep);
126134

127135
// Write previousKeys first. If the subsequent `key:generate` call fails,
@@ -257,30 +265,35 @@ private function writePreviousKeys(array $previousKeys): bool
257265
return false;
258266
}
259267

260-
$contents = (string) file_get_contents($envFile);
261-
$value = implode(',', $previousKeys);
262-
$replacement = "\nencryption.previousKeys = {$value}";
263-
264-
if (! str_contains($contents, 'encryption.previousKeys')) {
265-
// Insert right after the `encryption.key` line so the two stay grouped.
266-
$injected = (string) preg_replace(
267-
'/^([#\s]*encryption\.key[=\s]*[^\r\n]*)$/m',
268-
'$1' . $replacement,
269-
$contents,
270-
1,
271-
);
268+
$contents = (string) file_get_contents($envFile);
269+
$value = implode(',', $previousKeys);
270+
271+
// Match an actual setting line, not a substring buried in a comment. The optional
272+
// `export` prefix mirrors what DotEnv accepts.
273+
$previousKeysPattern = '/^(\h*(?:export\h+)?encryption\.previousKeys\h*=\h*)[^\r\n]*$/m';
274+
275+
if (preg_match($previousKeysPattern, $contents) === 1) {
276+
$contents = (string) preg_replace($previousKeysPattern, '$1' . $value, $contents, 1);
272277

273-
// The append fallback shouldn't trigger because `key:generate` writes
274-
// the `encryption.key` line just before this method runs.
275-
return file_put_contents($envFile, $injected !== $contents ? $injected : $contents . $replacement) !== false;
278+
return file_put_contents($envFile, $contents) !== false;
276279
}
277280

278-
$contents = (string) preg_replace(
279-
'/^[#\s]*encryption\.previousKeys[=\s]*[^\r\n]*$/m',
280-
trim($replacement),
281+
// Insert right after the `encryption.key` line so the two stay grouped.
282+
$injected = (string) preg_replace(
283+
'/^(\h*(?:export\h+)?encryption\.key\h*=\h*[^\r\n]*)$/m',
284+
"$1\nencryption.previousKeys = {$value}",
281285
$contents,
286+
1,
282287
);
283288

284-
return file_put_contents($envFile, $contents) !== false;
289+
if ($injected === $contents) {
290+
// @codeCoverageIgnoreStart
291+
// Fallback: append to the end. Shouldn't trigger because `key:generate`
292+
// writes the `encryption.key` line just before this method runs.
293+
$injected = $contents . "\nencryption.previousKeys = {$value}";
294+
// @codeCoverageIgnoreEnd
295+
}
296+
297+
return file_put_contents($envFile, $injected) !== false;
285298
}
286299
}

‎tests/system/Commands/Encryption/RotateKeyTest.php‎

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -397,6 +397,125 @@ public function testRotateRejectsNonNumericKeepValue(): void
397397
$this->assertSame(self::SEED_KEY, env('encryption.key'));
398398
}
399399

400+
public function testRotateRejectsNegativeLengthValue(): void
401+
{
402+
$this->seedEnv(self::SEED_KEY);
403+
$envContentsBefore = (string) file_get_contents($this->envPath);
404+
405+
command('key:rotate --force --length=-1');
406+
407+
$this->assertSame(
408+
<<<'EOT'
409+
410+
The --length option must be a positive integer.
411+
412+
EOT,
413+
$this->getUndecoratedBuffer(),
414+
);
415+
$this->assertSame(self::SEED_KEY, env('encryption.key'));
416+
$this->assertSame(
417+
$envContentsBefore,
418+
(string) file_get_contents($this->envPath),
419+
'Validation must reject the run before any .env mutation.',
420+
);
421+
}
422+
423+
public function testRotateRejectsZeroLengthValue(): void
424+
{
425+
$this->seedEnv(self::SEED_KEY);
426+
$envContentsBefore = (string) file_get_contents($this->envPath);
427+
428+
command('key:rotate --force --length=0');
429+
430+
$this->assertSame(
431+
<<<'EOT'
432+
433+
The --length option must be a positive integer.
434+
435+
EOT,
436+
$this->getUndecoratedBuffer(),
437+
);
438+
$this->assertSame(self::SEED_KEY, env('encryption.key'));
439+
$this->assertSame($envContentsBefore, (string) file_get_contents($this->envPath));
440+
}
441+
442+
public function testRotateRejectsNonNumericLengthValue(): void
443+
{
444+
$this->seedEnv(self::SEED_KEY);
445+
$envContentsBefore = (string) file_get_contents($this->envPath);
446+
447+
command('key:rotate --force --length=abc');
448+
449+
$this->assertSame(
450+
<<<'EOT'
451+
452+
The --length option must be a positive integer.
453+
454+
EOT,
455+
$this->getUndecoratedBuffer(),
456+
);
457+
$this->assertSame(self::SEED_KEY, env('encryption.key'));
458+
$this->assertSame($envContentsBefore, (string) file_get_contents($this->envPath));
459+
}
460+
461+
public function testRotateIgnoresCommentMentioningPreviousKeysWhenInserting(): void
462+
{
463+
$envContents = "# encryption.previousKeys is for decryption fallback\nencryption.key = " . self::SEED_KEY . "\n";
464+
file_put_contents($this->envPath, $envContents);
465+
$this->resetEnvironment();
466+
(new DotEnv(ROOTPATH))->load();
467+
468+
command('key:rotate --force');
469+
470+
$contents = (string) file_get_contents($this->envPath);
471+
$this->assertMatchesRegularExpression(
472+
'/^encryption\.previousKeys = ' . preg_quote(self::SEED_KEY, '/') . '$/m',
473+
$contents,
474+
'A real `encryption.previousKeys` setting must be written even when a comment mentions the name.',
475+
);
476+
$this->assertSame(self::SEED_KEY, env('encryption.previousKeys'));
477+
}
478+
479+
public function testRotateReplacesPreviousKeysLineWithExportPrefix(): void
480+
{
481+
$existing = 'hex2bin:' . str_repeat('a', 64);
482+
$envContents = 'encryption.key = ' . self::SEED_KEY . "\nexport encryption.previousKeys = {$existing}\n";
483+
file_put_contents($this->envPath, $envContents);
484+
$this->resetEnvironment();
485+
(new DotEnv(ROOTPATH))->load();
486+
487+
command('key:rotate --force');
488+
489+
$contents = (string) file_get_contents($this->envPath);
490+
$this->assertMatchesRegularExpression(
491+
'/^export encryption\.previousKeys = ' . preg_quote(self::SEED_KEY . ',' . $existing, '/') . '$/m',
492+
$contents,
493+
'The existing `export` prefix should be preserved and the value rewritten.',
494+
);
495+
$this->assertSame(
496+
self::SEED_KEY . ',' . $existing,
497+
env('encryption.previousKeys'),
498+
);
499+
}
500+
501+
public function testRotateInsertsAfterExportPrefixedEncryptionKey(): void
502+
{
503+
$envContents = 'export encryption.key = ' . self::SEED_KEY . "\n";
504+
file_put_contents($this->envPath, $envContents);
505+
$this->resetEnvironment();
506+
(new DotEnv(ROOTPATH))->load();
507+
508+
command('key:rotate --force');
509+
510+
$contents = (string) file_get_contents($this->envPath);
511+
$this->assertMatchesRegularExpression(
512+
'/^export encryption\.key = hex2bin:[a-f0-9]{64}\nencryption\.previousKeys = ' . preg_quote(self::SEED_KEY, '/') . '$/m',
513+
$contents,
514+
'`encryption.previousKeys` should be inserted on the line directly after an `export`-prefixed `encryption.key`.',
515+
);
516+
$this->assertSame(self::SEED_KEY, env('encryption.previousKeys'));
517+
}
518+
400519
public function testRotateErrorsWhenEnvFileIsNotWritable(): void
401520
{
402521
$this->seedEnv(self::SEED_KEY);

‎user_guide_src/source/libraries/encryption.rst‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -242,13 +242,16 @@ The command reads ``encryption.key`` from your environment, prepends it to
242242
``encryption.previousKeys`` (newest first, deduplicated), and writes a fresh ``encryption.key``.
243243
Useful options:
244244

245-
- ``--prefix`` (``hex2bin`` or ``base64``, default ``hex2bin``) and ``--length`` (default ``32``)
246-
control how the new key is generated, mirroring ``key:generate``.
245+
- ``--prefix`` (``hex2bin`` or ``base64``, default ``hex2bin``) and ``--length`` (positive
246+
integer, default ``32``) control how the new key is generated, mirroring ``key:generate``.
247247
- ``--keep=N`` caps the retained ``previousKeys`` list to the ``N`` most recent entries. ``N`` must
248248
be a non-negative integer; ``0`` (the default) keeps every previous key.
249249
- ``--force`` / ``-f`` skips the interactive confirmation. Required when running with
250250
``--no-interaction``.
251251

252+
All three options are validated up-front, so an invalid value cannot leave the **.env** file
253+
half-rotated.
254+
252255
Padding
253256
=======
254257

0 commit comments

Comments
 (0)