diff --git a/app/Actions/Database/StartRedis.php b/app/Actions/Database/StartRedis.php index c5d2d66f8e..7fc89b25ce 100644 --- a/app/Actions/Database/StartRedis.php +++ b/app/Actions/Database/StartRedis.php @@ -4,7 +4,6 @@ namespace App\Actions\Database; use App\Exceptions\DatabaseStartException; use App\Helpers\SslHelper; -use App\Models\EnvironmentVariable; use App\Models\SslCertificate; use App\Models\StandaloneRedis; use App\Traits\ExecutesDatabaseStartCommands; @@ -24,7 +23,9 @@ class StartRedis private ?SslCertificate $ssl_certificate = null; - private ?string $resolvedRedisPassword = null; + private string $resolvedRedisPassword = ''; + + private bool $redisPasswordFromSecretManager = false; private ?string $resolvedRedisUsername = null; @@ -245,21 +246,26 @@ class StartRedis private function generate_environment_variables() { $environment_variables = collect(); + // The stored REDIS_PASSWORD value, unresolved: the v4.3.23 server password of legacy databases. + $this->resolvedRedisPassword = (string) $this->database->redis_password; + $passwordVariable = $this->database->serverPasswordEnvironmentVariable(); foreach ($this->database->runtime_environment_variables as $env) { $usesSecretManager = $this->database->environmentVariableUsesSecretManager($env); + if ($passwordVariable?->is($env)) { + $rawValue = (string) $this->database->resolveSecretManagerEnvironmentVariableValue($env); + + if ($rawValue !== '') { + // Credentials below are placed directly in the compose file (command). + $this->resolvedRedisPassword = $this->database->formatComposeFileValue($env, $rawValue); + $this->redisPasswordFromSecretManager = $usesSecretManager; + } + } + if ($env->is_shared) { $environment_variables->push($env->key.'='.$this->database->resolveSecretManagerEnvironmentVariable($env)); - if ($env->key === 'REDIS_PASSWORD') { - $this->resolvedRedisPassword = $this->redisPasswordForComposeFile($env); - - if (! $usesSecretManager) { - $this->database->update(['redis_password' => $this->resolvedRedisPassword]); - } - } - if ($env->key === 'REDIS_USERNAME') { $this->resolvedRedisUsername = $this->database->resolveSecretManagerEnvironmentVariableValue($env); @@ -268,15 +274,11 @@ class StartRedis } } } else { - if ($env->key === 'REDIS_PASSWORD' && ! $usesSecretManager) { - $env->update(['value' => $this->database->redis_password]); - } elseif ($env->key === 'REDIS_USERNAME' && ! $usesSecretManager) { + if ($env->key === 'REDIS_USERNAME' && ! $usesSecretManager) { $env->update(['value' => $this->database->redis_username]); } - if ($env->key === 'REDIS_PASSWORD') { - $this->resolvedRedisPassword = $this->redisPasswordForComposeFile($env); - } elseif ($env->key === 'REDIS_USERNAME') { + if ($env->key === 'REDIS_USERNAME') { $this->resolvedRedisUsername = $this->database->resolveSecretManagerEnvironmentVariableValue($env); } @@ -289,20 +291,9 @@ class StartRedis return $environment_variables->all(); } - /** - * The password is placed directly in the compose command, so a remote secret value is escaped - * the same way as its environment entry. Other values stay unchanged. - */ - private function redisPasswordForComposeFile(EnvironmentVariable $env): ?string - { - $value = $this->database->resolveSecretManagerEnvironmentVariableValue($env); - - return $value === null ? null : $this->database->formatComposeFileValue($env, $value); - } - private function buildStartCommand(): string { - $redisPassword = $this->resolvedRedisPassword ?? $this->database->redis_password; + $redisPassword = $this->requirePassArgument(); $hasRedisConf = ! is_null($this->database->redis_conf) && ! empty($this->database->redis_conf); $redisConfPath = '/usr/local/etc/redis/redis.conf'; @@ -336,6 +327,21 @@ class StartRedis return $command; } + /** + * The password argument of the start command. Docker Compose splits the command like a shell, so the + * password is quoted. Databases created before this release keep their unquoted v4.3.23 argument when + * quoting would change it (quotes, backslashes, whitespace, ; & | < >): Compose splits or cuts the old + * command there, so quoting now would change the password or options and can lose data. + */ + private function requirePassArgument(): string + { + $keepsUnquotedPassword = $this->database->legacy_password_quoting + && ! $this->redisPasswordFromSecretManager + && strpbrk($this->resolvedRedisPassword, "\\'\";&|<> \t\r\n") !== false; + + return $keepsUnquotedPassword ? $this->resolvedRedisPassword : escapeshellarg($this->resolvedRedisPassword); + } + private function add_custom_redis() { if (is_null($this->database->redis_conf) || empty($this->database->redis_conf)) { diff --git a/app/Models/StandaloneRedis.php b/app/Models/StandaloneRedis.php index 3bfcc5434e..2e6f158cf1 100644 --- a/app/Models/StandaloneRedis.php +++ b/app/Models/StandaloneRedis.php @@ -66,6 +66,8 @@ class StandaloneRedis extends BaseModel 'redis_password', 'internal_db_url', 'external_db_url', + // Internal start-command state, not part of the API. + 'legacy_password_quoting', ]; protected $casts = [ @@ -74,6 +76,7 @@ class StandaloneRedis extends BaseModel 'health_check_timeout' => 'integer', 'health_check_retries' => 'integer', 'health_check_start_period' => 'integer', + 'legacy_password_quoting' => 'boolean', 'public_port_timeout' => 'integer', 'restart_count' => 'integer', 'last_restart_at' => 'datetime', @@ -313,7 +316,7 @@ class StandaloneRedis extends BaseModel get: function () { $redis_version = $this->getRedisVersion(); $username_part = version_compare($redis_version, '6.0', '>=') ? rawurlencode($this->redis_username).':' : ''; - $encodedPass = rawurlencode($this->redis_password); + $encodedPass = rawurlencode($this->connectionPassword()); $scheme = $this->enable_ssl ? 'rediss' : 'redis'; $port = $this->enable_ssl ? 6380 : 6379; $url = "{$scheme}://{$username_part}{$encodedPass}@{$this->uuid}:{$port}/0"; @@ -338,7 +341,7 @@ class StandaloneRedis extends BaseModel } $redis_version = $this->getRedisVersion(); $username_part = version_compare($redis_version, '6.0', '>=') ? rawurlencode($this->redis_username).':' : ''; - $encodedPass = rawurlencode($this->redis_password); + $encodedPass = rawurlencode($this->connectionPassword()); $scheme = $this->enable_ssl ? 'rediss' : 'redis'; $url = "{$scheme}://{$username_part}{$encodedPass}@{$serverIp}:{$this->public_port}/0"; @@ -381,6 +384,47 @@ class StandaloneRedis extends BaseModel return $this->morphMany(EnvironmentVariable::class, 'resourceable'); } + /** + * The REDIS_PASSWORD variable whose resolved value sets the server password. Databases created + * before this release (legacy_password_quoting) keep their v4.3.23 server password, the stored + * value as it is, unless the variable reads a remote secret. + */ + public function serverPasswordEnvironmentVariable(): ?EnvironmentVariable + { + $environmentVariable = $this->runtime_environment_variables()->where('key', 'REDIS_PASSWORD')->first(); + + if (! $environmentVariable) { + return null; + } + + if ($this->legacy_password_quoting && ! $this->environmentVariableUsesSecretManager($environmentVariable)) { + return null; + } + + return $environmentVariable; + } + + /** + * The server password for connection URLs. A remote secret is not fetched here, so the URL + * shows its reference instead of a stale or wrong password. + */ + public function connectionPassword(): string + { + $environmentVariable = $this->serverPasswordEnvironmentVariable(); + + if ($environmentVariable) { + $password = $this->environmentVariableUsesSecretManager($environmentVariable) + ? (string) $environmentVariable->value + : (string) $this->resolveSecretManagerEnvironmentVariableValue($environmentVariable); + + if ($password !== '') { + return $password; + } + } + + return (string) $this->redis_password; + } + public function persistentStorages() { return $this->morphMany(LocalPersistentVolume::class, 'resource'); diff --git a/database/migrations/2026_10_05_204113_add_legacy_password_quoting_to_standalone_redis.php b/database/migrations/2026_10_05_204113_add_legacy_password_quoting_to_standalone_redis.php new file mode 100644 index 0000000000..7ecb900946 --- /dev/null +++ b/database/migrations/2026_10_05_204113_add_legacy_password_quoting_to_standalone_redis.php @@ -0,0 +1,30 @@ +boolean('legacy_password_quoting')->default(false); + }); + + DB::table('standalone_redis')->update(['legacy_password_quoting' => true]); + } + + public function down(): void + { + Schema::table('standalone_redis', function (Blueprint $table) { + $table->dropColumn('legacy_password_quoting'); + }); + } +}; diff --git a/tests/Feature/Api/LegacyPasswordQuotingApiTest.php b/tests/Feature/Api/LegacyPasswordQuotingApiTest.php index 549f955497..78d3dd548b 100644 --- a/tests/Feature/Api/LegacyPasswordQuotingApiTest.php +++ b/tests/Feature/Api/LegacyPasswordQuotingApiTest.php @@ -38,4 +38,4 @@ it('does not return the internal legacy password quoting flag in the database AP expect($single->json())->not->toHaveKey('legacy_password_quoting') ->and(collect($list->json())->firstWhere('uuid', $database->uuid))->not->toHaveKey('legacy_password_quoting'); -})->with(['keydb' => 'create_standalone_keydb', 'dragonfly' => 'create_standalone_dragonfly']); +})->with(['keydb' => 'create_standalone_keydb', 'dragonfly' => 'create_standalone_dragonfly', 'redis' => 'create_standalone_redis']); diff --git a/tests/Feature/RedisLegacyPasswordQuotingTest.php b/tests/Feature/RedisLegacyPasswordQuotingTest.php new file mode 100644 index 0000000000..dec7077109 --- /dev/null +++ b/tests/Feature/RedisLegacyPasswordQuotingTest.php @@ -0,0 +1,187 @@ + 0]); + config(['app.maintenance.store' => 'array', 'cache.default' => 'array']); + Process::fake(); + Queue::fake(); + Server::flushIdentityMap(); + + $team = Team::factory()->create(); + $privateKey = PrivateKey::factory()->create(['team_id' => $team->id]); + $server = Server::factory()->create(['team_id' => $team->id, 'private_key_id' => $privateKey->id]); + $this->destination = StandaloneDocker::query()->where('server_id', $server->id)->firstOrFail(); + $project = Project::factory()->create(['team_id' => $team->id]); + $this->environment = Environment::factory()->create(['project_id' => $project->id]); + + SharedEnvironmentVariable::query()->create([ + 'key' => 'RPW', + 'value' => 's3cret', + 'type' => 'environment', + 'environment_id' => $this->environment->id, + 'project_id' => $project->id, + 'team_id' => $team->id, + ]); + + $this->executor = new class + { + public array $commands = []; + + public function execute(array $commands, $database, Activity $activity): Activity + { + $this->commands = $commands; + + return $activity; + } + }; + app()->instance(DatabaseStartCommandExecutor::class, $this->executor); +}); + +function redisLegacyQuotingDatabase(object $test, string $password, bool $legacy, array $otherData = []): StandaloneRedis +{ + $database = create_standalone_redis($test->environment->id, $test->destination, ['redis_password' => $password, 'image' => 'redis:7.2', ...$otherData]); + if ($legacy) { + DB::table($database->getTable())->where('id', $database->id)->update(['legacy_password_quoting' => true]); + } + + return $database->fresh(); +} + +function redisLegacyQuotingStartCommand(object $executor, string $uuid): string +{ + foreach ($executor->commands as $command) { + if (preg_match("#^echo '([^']+)' \| base64 -d \| tee \S+/docker-compose\.yml#", $command, $matches)) { + return Yaml::parse(base64_decode($matches[1]))['services'][$uuid]['command']; + } + } + + throw new RuntimeException('No docker-compose.yml was written.'); +} + +it('keeps the v4.3.23 server password and URL of a legacy database with a shared REDIS_PASSWORD reference', function () { + $database = redisLegacyQuotingDatabase($this, '{{environment.RPW}}', legacy: true, otherData: ['is_public' => true, 'public_port' => 16379]); + + StartRedis::run($database, new Activity); + + $database = $database->fresh(); + $encodedReference = rawurlencode('{{environment.RPW}}'); + expect(redisLegacyQuotingStartCommand($this->executor, $database->uuid)) + ->toBe("redis-server --requirepass '{{environment.RPW}}' --appendonly yes") + ->and($database->internal_db_url)->toContain(":{$encodedReference}@") + ->and($database->external_db_url)->toContain(":{$encodedReference}@") + ->and($database->runtime_environment_variables()->where('key', 'REDIS_PASSWORD')->first()->value)->toBe('{{environment.RPW}}') + ->and($database->legacy_password_quoting)->toBeTrue(); +}); + +it('keeps the unquoted v4.3.23 argument of a legacy password that quoting would change', function (string $password) { + $database = redisLegacyQuotingDatabase($this, $password, legacy: true); + + StartRedis::run($database, new Activity); + + expect(redisLegacyQuotingStartCommand($this->executor, $database->uuid)) + ->toBe("redis-server --requirepass {$password} --appendonly yes"); +})->with([ + 'backslash' => 'ab\c', + 'quote' => "a'b", + 'space' => 'a b', + 'semicolon' => 'ab;cd', +]); + +it('starts a new database with the resolved shared REDIS_PASSWORD and shows it in the URLs', function () { + $database = redisLegacyQuotingDatabase($this, '{{environment.RPW}}', legacy: false, otherData: ['is_public' => true, 'public_port' => 16379]); + + StartRedis::run($database, new Activity); + + $database = $database->fresh(); + expect(redisLegacyQuotingStartCommand($this->executor, $database->uuid)) + ->toBe("redis-server --requirepass 's3cret' --appendonly yes") + ->and($database->internal_db_url)->toContain(':s3cret@') + ->and($database->external_db_url)->toContain(':s3cret@') + ->and($database->legacy_password_quoting)->toBeFalse(); +}); + +it('quotes special characters of the password of a new database', function (string $password) { + SharedEnvironmentVariable::query()->where('key', 'RPW')->firstOrFail()->update(['value' => $password]); + $local = redisLegacyQuotingDatabase($this, $password, legacy: false); + $shared = redisLegacyQuotingDatabase($this, '{{environment.RPW}}', legacy: false); + + StartRedis::run($local, new Activity); + $localCommand = redisLegacyQuotingStartCommand($this->executor, $local->uuid); + StartRedis::run($shared, new Activity); + $sharedCommand = redisLegacyQuotingStartCommand($this->executor, $shared->uuid); + + $expected = 'redis-server --requirepass '.escapeshellarg($password).' --appendonly yes'; + expect($localCommand)->toBe($expected) + ->and($sharedCommand)->toBe($expected) + ->and($shared->fresh()->internal_db_url)->toContain(':'.rawurlencode($password).'@'); +})->with([ + 'quote and space' => "pa ss'w", + 'backslash' => 'ab\c', + 'double quote' => 'a"b', +]); + +it('quotes the password next to a custom redis.conf without requirepass', function () { + $database = redisLegacyQuotingDatabase($this, 'a b', legacy: false, otherData: ['redis_conf' => 'maxmemory 100mb']); + + StartRedis::run($database, new Activity); + + expect(redisLegacyQuotingStartCommand($this->executor, $database->uuid)) + ->toBe("redis-server /usr/local/etc/redis/redis.conf --requirepass 'a b'"); +}); + +it('quotes a remote secret password of a legacy database exactly and shows the reference in the URL', function () { + Http::fake(['https://api.doppler.com/*' => Http::response(['REDIS_PASSWORD' => 'ab\\c'])]); + $token = IntegrationToken::query()->create([ + 'team_id' => $this->environment->project->team_id, + 'provider' => 'doppler', + 'name' => 'Doppler', + 'token' => 'the-secret-token', + 'capabilities' => ['secrets'], + ]); + $database = redisLegacyQuotingDatabase($this, '{{vault.REDIS_PASSWORD}}', legacy: true); + $database->secretManagerLink()->create(['integration_token_id' => $token->id]); + + expect($database->fresh()->internal_db_url)->toContain(':'.rawurlencode('{{vault.REDIS_PASSWORD}}').'@'); + + StartRedis::run($database->fresh(), new Activity); + + expect(redisLegacyQuotingStartCommand($this->executor, $database->uuid)) + ->toBe("redis-server --requirepass 'ab\\c' --appendonly yes"); +}); + +it('marks only Redis databases that exist before the migration as legacy', function () { + $existing = create_standalone_redis($this->environment->id, $this->destination); + + $migration = require database_path('migrations/2026_10_05_204113_add_legacy_password_quoting_to_standalone_redis.php'); + $migration->down(); + expect(Schema::hasColumn('standalone_redis', 'legacy_password_quoting'))->toBeFalse(); + $migration->up(); + + $new = create_standalone_redis($this->environment->id, $this->destination); + + expect($existing->fresh()->legacy_password_quoting)->toBeTrue() + ->and($new->fresh()->legacy_password_quoting)->toBeFalse(); +}); diff --git a/tests/Feature/SecretManagers/RemoteSecretResourceLifecycleTest.php b/tests/Feature/SecretManagers/RemoteSecretResourceLifecycleTest.php index 1a8d128001..d3846d9ae3 100644 --- a/tests/Feature/SecretManagers/RemoteSecretResourceLifecycleTest.php +++ b/tests/Feature/SecretManagers/RemoteSecretResourceLifecycleTest.php @@ -199,7 +199,7 @@ test('redis remote passwords are escaped the same way in the environment and the $startCommand = (new ReflectionMethod($action, 'buildStartCommand'))->invoke($action); expect($environmentVariables)->toContain('REDIS_PASSWORD=p4$$word') - ->and($startCommand)->toContain('--requirepass p4$$word'); + ->and($startCommand)->toContain("--requirepass 'p4\$\$word'"); }); test('a service restart keeps the service running when remote secrets cannot be fetched', function () { diff --git a/tests/Feature/SecretManagers/SecretManagerLinkTest.php b/tests/Feature/SecretManagers/SecretManagerLinkTest.php index 3cad814ea6..acaa33f7a7 100644 --- a/tests/Feature/SecretManagers/SecretManagerLinkTest.php +++ b/tests/Feature/SecretManagers/SecretManagerLinkTest.php @@ -264,7 +264,7 @@ test('redis remote credentials stay deployment-local and use raw values in the s ->and($username->fresh()->value)->toBe('{{vault.REDIS_USERNAME}}') ->and($environmentVariables)->toContain('REDIS_PASSWORD=p4$$$$word') ->and($environmentVariables)->toContain('REDIS_USERNAME=remote-user') - ->and($startCommand)->toContain('--requirepass p4$$$$word'); + ->and($startCommand)->toContain("--requirepass 'p4\$\$\$\$word'"); }); test('all deployable environment-variable resources support secret managers', function (string $resourceClass) {