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(); +});