From 9978929779c472db218511146a28433615519642 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Fri, 2 Oct 2026 16:00:41 +0200 Subject: [PATCH] fix(databases): keep the server password of existing KeyDB and Dragonfly databases The start command now quotes the password. Databases created before this release keep the unquoted v4.3.23 argument when quoting would change the password the server receives, until their password changes. Remote secret passwords are always quoted exactly. Co-Authored-By: Claude Opus 5.5 --- app/Actions/Database/StartDragonfly.php | 26 ++- app/Actions/Database/StartKeydb.php | 28 +++- app/Models/StandaloneDragonfly.php | 4 + app/Models/StandaloneKeydb.php | 4 + ...assword_quoting_to_keydb_and_dragonfly.php | 36 ++++ ...eydbDragonflyLegacyPasswordQuotingTest.php | 157 ++++++++++++++++++ 6 files changed, 248 insertions(+), 7 deletions(-) create mode 100644 database/migrations/2026_10_02_120000_add_legacy_password_quoting_to_keydb_and_dragonfly.php create mode 100644 tests/Feature/KeydbDragonflyLegacyPasswordQuotingTest.php diff --git a/app/Actions/Database/StartDragonfly.php b/app/Actions/Database/StartDragonfly.php index c392558620..eff973c36d 100644 --- a/app/Actions/Database/StartDragonfly.php +++ b/app/Actions/Database/StartDragonfly.php @@ -25,6 +25,8 @@ class StartDragonfly private string $resolvedRedisPassword; + private bool $redisPasswordFromSecretManager = false; + public function handle(StandaloneDragonfly $database, ?Activity $activity = null) { $this->database = $database; @@ -196,8 +198,8 @@ class StartDragonfly private function buildStartCommand(): string { - $escapedRedisPassword = escapeshellarg($this->resolvedRedisPassword); - $command = "dragonfly --requirepass {$escapedRedisPassword}"; + $passwordArgument = $this->requirePassArgument(); + $command = "dragonfly --requirepass {$passwordArgument}"; if ($this->database->enable_ssl) { $sslArgs = [ @@ -251,9 +253,12 @@ class StartDragonfly foreach ($this->database->runtime_environment_variables as $env) { $rawValue = (string) $this->database->resolveSecretManagerEnvironmentVariableValue($env); $resolvedValue = (string) $this->database->formatEnvironmentVariableValue($env, $rawValue); + // Credentials below are placed directly in the compose file (healthcheck, command). + $composeFileValue = $this->database->formatComposeFileValue($env, $rawValue); $environment_variables->push($env->key.'='.$resolvedValue); if ($env->key === 'REDIS_PASSWORD') { - $this->resolvedRedisPassword = $rawValue; + $this->resolvedRedisPassword = $composeFileValue; + $this->redisPasswordFromSecretManager = $this->database->environmentVariableUsesSecretManager($env); } } @@ -263,4 +268,19 @@ class StartDragonfly return $environment_variables->all(); } + + /** + * 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 the password the server receives (backslashes and quotes), so their server + * password stays the same. + */ + private function requirePassArgument(): string + { + $keepsUnquotedPassword = $this->database->legacy_password_quoting + && ! $this->redisPasswordFromSecretManager + && strpbrk($this->resolvedRedisPassword, '\\\'"') !== false; + + return $keepsUnquotedPassword ? $this->resolvedRedisPassword : escapeshellarg($this->resolvedRedisPassword); + } } diff --git a/app/Actions/Database/StartKeydb.php b/app/Actions/Database/StartKeydb.php index b1b3e85ef8..4940f38f58 100644 --- a/app/Actions/Database/StartKeydb.php +++ b/app/Actions/Database/StartKeydb.php @@ -25,6 +25,8 @@ class StartKeydb private string $resolvedRedisPassword; + private bool $redisPasswordFromSecretManager = false; + public function handle(StandaloneKeydb $database, ?Activity $activity = null) { $this->database = $database; @@ -251,9 +253,12 @@ class StartKeydb foreach ($this->database->runtime_environment_variables as $env) { $rawValue = (string) $this->database->resolveSecretManagerEnvironmentVariableValue($env); $resolvedValue = (string) $this->database->formatEnvironmentVariableValue($env, $rawValue); + // Credentials below are placed directly in the compose file (healthcheck, command). + $composeFileValue = $this->database->formatComposeFileValue($env, $rawValue); $environment_variables->push($env->key.'='.$resolvedValue); if ($env->key === 'REDIS_PASSWORD') { - $this->resolvedRedisPassword = $rawValue; + $this->resolvedRedisPassword = $composeFileValue; + $this->redisPasswordFromSecretManager = $this->database->environmentVariableUsesSecretManager($env); } } @@ -281,7 +286,7 @@ class StartKeydb { $hasKeydbConf = ! is_null($this->database->keydb_conf) && ! empty($this->database->keydb_conf); $keydbConfPath = '/etc/keydb/keydb.conf'; - $escapedRedisPassword = escapeshellarg($this->resolvedRedisPassword); + $passwordArgument = $this->requirePassArgument(); if ($hasKeydbConf) { $confContent = $this->database->keydb_conf; @@ -290,10 +295,10 @@ class StartKeydb if ($hasRequirePass) { $command = "keydb-server $keydbConfPath"; } else { - $command = "keydb-server $keydbConfPath --requirepass {$escapedRedisPassword}"; + $command = "keydb-server $keydbConfPath --requirepass {$passwordArgument}"; } } else { - $command = "keydb-server --requirepass {$escapedRedisPassword} --appendonly yes"; + $command = "keydb-server --requirepass {$passwordArgument} --appendonly yes"; } if ($this->database->enable_ssl) { @@ -309,4 +314,19 @@ class StartKeydb 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 the password the server receives (backslashes and quotes), so their server + * password stays the same. + */ + private function requirePassArgument(): string + { + $keepsUnquotedPassword = $this->database->legacy_password_quoting + && ! $this->redisPasswordFromSecretManager + && strpbrk($this->resolvedRedisPassword, '\\\'"') !== false; + + return $keepsUnquotedPassword ? $this->resolvedRedisPassword : escapeshellarg($this->resolvedRedisPassword); + } } diff --git a/app/Models/StandaloneDragonfly.php b/app/Models/StandaloneDragonfly.php index da4804dd2d..05bcba5d18 100644 --- a/app/Models/StandaloneDragonfly.php +++ b/app/Models/StandaloneDragonfly.php @@ -73,6 +73,7 @@ class StandaloneDragonfly extends BaseModel 'health_check_retries' => 'integer', 'health_check_start_period' => 'integer', 'dragonfly_password' => 'encrypted', + 'legacy_password_quoting' => 'boolean', 'public_port_timeout' => 'integer', 'restart_count' => 'integer', 'last_restart_at' => 'datetime', @@ -100,6 +101,9 @@ class StandaloneDragonfly extends BaseModel if ($database->isDirty('status')) { $database->last_online_at = now(); } + if ($database->exists && $database->isDirty('dragonfly_password')) { + $database->legacy_password_quoting = false; + } }); } diff --git a/app/Models/StandaloneKeydb.php b/app/Models/StandaloneKeydb.php index f4dbaec210..cb29dd612a 100644 --- a/app/Models/StandaloneKeydb.php +++ b/app/Models/StandaloneKeydb.php @@ -74,6 +74,7 @@ class StandaloneKeydb extends BaseModel 'health_check_retries' => 'integer', 'health_check_start_period' => 'integer', 'keydb_password' => 'encrypted', + 'legacy_password_quoting' => 'boolean', 'public_port_timeout' => 'integer', 'restart_count' => 'integer', 'last_restart_at' => 'datetime', @@ -101,6 +102,9 @@ class StandaloneKeydb extends BaseModel if ($database->isDirty('status')) { $database->last_online_at = now(); } + if ($database->exists && $database->isDirty('keydb_password')) { + $database->legacy_password_quoting = false; + } }); } diff --git a/database/migrations/2026_10_02_120000_add_legacy_password_quoting_to_keydb_and_dragonfly.php b/database/migrations/2026_10_02_120000_add_legacy_password_quoting_to_keydb_and_dragonfly.php new file mode 100644 index 0000000000..0eb7d8aed6 --- /dev/null +++ b/database/migrations/2026_10_02_120000_add_legacy_password_quoting_to_keydb_and_dragonfly.php @@ -0,0 +1,36 @@ +boolean('legacy_password_quoting')->default(false); + }); + + DB::table($tableName)->update(['legacy_password_quoting' => true]); + } + } + + public function down(): void + { + foreach (self::TABLES as $tableName) { + Schema::table($tableName, function (Blueprint $table) { + $table->dropColumn('legacy_password_quoting'); + }); + } + } +}; diff --git a/tests/Feature/KeydbDragonflyLegacyPasswordQuotingTest.php b/tests/Feature/KeydbDragonflyLegacyPasswordQuotingTest.php new file mode 100644 index 0000000000..6f0e25f794 --- /dev/null +++ b/tests/Feature/KeydbDragonflyLegacyPasswordQuotingTest.php @@ -0,0 +1,157 @@ + 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]); + + $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); +}); + +/** + * @return array{command: string, healthcheck: list} + */ +function legacyPasswordQuotingStartedService(object $executor, string $uuid): array +{ + foreach ($executor->commands as $command) { + if (preg_match("#^echo '([^']+)' \| base64 -d \| tee \S+/docker-compose\.yml#", $command, $matches)) { + $service = Yaml::parse(base64_decode($matches[1]))['services'][$uuid]; + + return ['command' => $service['command'], 'healthcheck' => $service['healthcheck']['test']]; + } + } + + throw new RuntimeException('No docker-compose.yml was written.'); +} + +dataset('legacy-password-quoting-engines', [ + 'keydb' => ['create_standalone_keydb', StartKeydb::class, 'keydb_password', 'keydb-server --requirepass %s --appendonly yes'], + 'dragonfly' => ['create_standalone_dragonfly', StartDragonfly::class, 'dragonfly_password', 'dragonfly --requirepass %s'], +]); + +it('keeps the unquoted v4.3.23 start command for a database created before the upgrade', function (string $create, string $start, string $passwordColumn, string $commandFormat) { + $database = $create($this->environment->id, $this->destination, [$passwordColumn => 'ab\c']); + DB::table($database->getTable())->where('id', $database->id)->update(['legacy_password_quoting' => true]); + + $start::run($database->fresh(), new Activity); + + expect(legacyPasswordQuotingStartedService($this->executor, $database->uuid)['command']) + ->toBe(sprintf($commandFormat, 'ab\c')); +})->with('legacy-password-quoting-engines'); + +it('quotes the exact stored password for a database created after the upgrade', function (string $create, string $start, string $passwordColumn, string $commandFormat) { + $database = $create($this->environment->id, $this->destination, [$passwordColumn => 'ab\c']); + + $start::run($database->fresh(), new Activity); + + $service = legacyPasswordQuotingStartedService($this->executor, $database->uuid); + expect($database->fresh()->legacy_password_quoting)->toBeFalse() + ->and($service['command'])->toBe(sprintf($commandFormat, "'ab\\c'")) + ->and($service['healthcheck'][3])->toBe('ab\c'); +})->with('legacy-password-quoting-engines'); + +it('quotes legacy passwords that v4.3.23 passed unchanged or could not start with', function (string $create, string $start, string $passwordColumn, string $commandFormat) { + $database = $create($this->environment->id, $this->destination, [$passwordColumn => 'a(b)c']); + DB::table($database->getTable())->where('id', $database->id)->update(['legacy_password_quoting' => true]); + + $start::run($database->fresh(), new Activity); + + expect(legacyPasswordQuotingStartedService($this->executor, $database->uuid)['command']) + ->toBe(sprintf($commandFormat, "'a(b)c'")); +})->with('legacy-password-quoting-engines'); + +it('uses the exact quoting after the password of a legacy database changes', function (string $create, string $start, string $passwordColumn, string $commandFormat) { + $database = $create($this->environment->id, $this->destination, [$passwordColumn => 'ab\c']); + DB::table($database->getTable())->where('id', $database->id)->update(['legacy_password_quoting' => true]); + + $database = $database->fresh(); + $database->update(['name' => 'renamed']); + expect($database->fresh()->legacy_password_quoting)->toBeTrue(); + + $database->update([$passwordColumn => 'new\pass']); + $start::run($database->fresh(), new Activity); + + expect($database->fresh()->legacy_password_quoting)->toBeFalse() + ->and(legacyPasswordQuotingStartedService($this->executor, $database->uuid)['command']) + ->toBe(sprintf($commandFormat, "'new\\pass'")); +})->with('legacy-password-quoting-engines'); + +it('quotes a remote secret password of a legacy database exactly', function (string $create, string $start, string $passwordColumn, string $commandFormat) { + 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 = $create($this->environment->id, $this->destination, [$passwordColumn => 'stored']); + DB::table($database->getTable())->where('id', $database->id)->update(['legacy_password_quoting' => true]); + $database->secretManagerLink()->create(['integration_token_id' => $token->id]); + $database->runtime_environment_variables()->create(['key' => 'REDIS_PASSWORD', 'value' => '{{vault.REDIS_PASSWORD}}']); + + $start::run($database->fresh(), new Activity); + + expect(legacyPasswordQuotingStartedService($this->executor, $database->uuid)['command']) + ->toBe(sprintf($commandFormat, "'ab\\c'")); +})->with('legacy-password-quoting-engines'); + +it('marks only databases that exist before the migration as legacy', function () { + $existingKeydb = create_standalone_keydb($this->environment->id, $this->destination); + $existingDragonfly = create_standalone_dragonfly($this->environment->id, $this->destination); + + $migration = require database_path('migrations/2026_10_02_120000_add_legacy_password_quoting_to_keydb_and_dragonfly.php'); + $migration->down(); + expect(Schema::hasColumn('standalone_keydbs', 'legacy_password_quoting'))->toBeFalse(); + $migration->up(); + + $newKeydb = create_standalone_keydb($this->environment->id, $this->destination); + $newDragonfly = create_standalone_dragonfly($this->environment->id, $this->destination); + + expect($existingKeydb->fresh()->legacy_password_quoting)->toBeTrue() + ->and($existingDragonfly->fresh()->legacy_password_quoting)->toBeTrue() + ->and($newKeydb->fresh()->legacy_password_quoting)->toBeFalse() + ->and($newDragonfly->fresh()->legacy_password_quoting)->toBeFalse(); +});