From 5f78e3dfb3a6a833fd8871454174556409aa7c8f Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Thu, 15 Oct 2026 14:23:06 +0200 Subject: [PATCH] fix(settings): stop saving read-only credentials from client input Instance backup settings, started ClickHouse/KeyDB/Dragonfly databases, and destinations saved read-only fields back to the database, so a password manager autofill could overwrite stored credentials. Closes #12173 Co-Authored-By: Claude Opus 5.5 --- app/Livewire/Destination/Show.php | 6 +- .../Project/Database/Clickhouse/General.php | 7 +- .../Project/Database/Dragonfly/General.php | 5 +- .../Project/Database/Keydb/General.php | 5 +- app/Livewire/SettingsBackup.php | 24 +---- .../views/livewire/settings-backup.blade.php | 7 +- .../ReadOnlyFieldsAreNotPersistedTest.php | 95 +++++++++++++++++++ .../Feature/SettingsBackupEmptyStateTest.php | 33 ++++++- 8 files changed, 144 insertions(+), 38 deletions(-) create mode 100644 tests/Feature/ReadOnlyFieldsAreNotPersistedTest.php diff --git a/app/Livewire/Destination/Show.php b/app/Livewire/Destination/Show.php index e887abed9e..05b295b88b 100644 --- a/app/Livewire/Destination/Show.php +++ b/app/Livewire/Destination/Show.php @@ -20,10 +20,10 @@ class Show extends Component #[Validate(['string', 'required'])] public string $name; - #[Validate(['string', 'required', 'max:255', 'regex:/^[a-zA-Z0-9][a-zA-Z0-9._-]*$/'])] + #[Locked] public string $network; - #[Validate(['string', 'required'])] + #[Locked] public string $serverIp; public function mount(string $destination_uuid) @@ -49,8 +49,6 @@ class Show extends Component if ($toModel) { $this->validate(); $this->destination->name = $this->name; - $this->destination->network = $this->network; - $this->destination->server->ip = $this->serverIp; $changedFields = auditChangedFields($this->destination); $this->destination->save(); if ($changedFields !== []) { diff --git a/app/Livewire/Project/Database/Clickhouse/General.php b/app/Livewire/Project/Database/Clickhouse/General.php index ea8d8f197b..2cd0bfecee 100644 --- a/app/Livewire/Project/Database/Clickhouse/General.php +++ b/app/Livewire/Project/Database/Clickhouse/General.php @@ -122,8 +122,11 @@ class General extends Component $this->validate(); $this->database->name = $this->name; $this->database->description = $this->description; - $this->database->clickhouse_admin_user = $this->clickhouseAdminUser; - $this->database->clickhouse_admin_password = $this->clickhouseAdminPassword; + // After the first start the credentials are read-only, so autofilled values must not be saved. + if (! $this->database->started_at) { + $this->database->clickhouse_admin_user = $this->clickhouseAdminUser; + $this->database->clickhouse_admin_password = $this->clickhouseAdminPassword; + } $this->database->image = $this->image; $this->database->ports_mappings = $this->portsMappings; $this->database->is_public = $this->isPublic; diff --git a/app/Livewire/Project/Database/Dragonfly/General.php b/app/Livewire/Project/Database/Dragonfly/General.php index 1505aad781..b3c5bcc945 100644 --- a/app/Livewire/Project/Database/Dragonfly/General.php +++ b/app/Livewire/Project/Database/Dragonfly/General.php @@ -110,7 +110,10 @@ class General extends Component $this->validate(); $this->database->name = $this->name; $this->database->description = $this->description; - $this->database->dragonfly_password = $this->dragonflyPassword; + // After the first start the password is read-only, so autofilled values must not be saved. + if (! $this->database->started_at) { + $this->database->dragonfly_password = $this->dragonflyPassword; + } $this->database->image = $this->image; $this->database->ports_mappings = $this->portsMappings; $this->database->is_public = $this->isPublic; diff --git a/app/Livewire/Project/Database/Keydb/General.php b/app/Livewire/Project/Database/Keydb/General.php index 2ae64f3121..fd84bad209 100644 --- a/app/Livewire/Project/Database/Keydb/General.php +++ b/app/Livewire/Project/Database/Keydb/General.php @@ -114,7 +114,10 @@ class General extends Component $this->database->name = $this->name; $this->database->description = $this->description; $this->database->keydb_conf = $this->keydbConf; - $this->database->keydb_password = $this->keydbPassword; + // After the first start the password is read-only, so autofilled values must not be saved. + if (! $this->database->started_at) { + $this->database->keydb_password = $this->keydbPassword; + } $this->database->image = $this->image; $this->database->ports_mappings = $this->portsMappings; $this->database->is_public = $this->isPublic; diff --git a/app/Livewire/SettingsBackup.php b/app/Livewire/SettingsBackup.php index a7e8b89488..fb4cda6d97 100644 --- a/app/Livewire/SettingsBackup.php +++ b/app/Livewire/SettingsBackup.php @@ -31,21 +31,9 @@ class SettingsBackup extends Component #[Locked] public $executions = []; - #[Validate(['required'])] - public string $uuid; - - #[Validate(['required'])] - public string $name; - #[Validate(['nullable'])] public ?string $description = null; - #[Validate(['required'])] - public string $postgres_user; - - #[Validate(['required'])] - public string $postgres_password; - public function mount() { if (! isInstanceAdmin()) { @@ -57,11 +45,7 @@ class SettingsBackup extends Component $this->database = StandalonePostgresql::find(0); $s3s = S3Storage::whereTeamId(0)->get() ?? []; if ($this->database) { - $this->uuid = $this->database->uuid; - $this->name = $this->database->name; $this->description = $this->database->description; - $this->postgres_user = $this->database->postgres_user; - $this->postgres_password = $this->database->postgres_password; if ($this->database->status !== 'running') { $this->database->status = 'running'; @@ -124,11 +108,7 @@ class SettingsBackup extends Component } $this->s3s = S3Storage::whereTeamId(0)->get(); - $this->uuid = $this->database->uuid; - $this->name = $this->database->name; $this->description = $this->database->description; - $this->postgres_user = $this->database->postgres_user; - $this->postgres_password = $this->database->postgres_password; $this->executions = $this->backup->executions; } catch (\Exception $e) { @@ -142,11 +122,9 @@ class SettingsBackup extends Component $this->authorize('update', $this->settings); $this->validate(); + // Credentials come from the coolify-db container and are display-only, so only the description is saved. $this->database->update([ - 'name' => $this->name, 'description' => $this->description, - 'postgres_user' => $this->postgres_user, - 'postgres_password' => $this->postgres_password, ]); $this->dispatch('success', 'Backup updated.'); } catch (\Throwable $e) { diff --git a/resources/views/livewire/settings-backup.blade.php b/resources/views/livewire/settings-backup.blade.php index c89bf148cf..1d4bc85e13 100644 --- a/resources/views/livewire/settings-backup.blade.php +++ b/resources/views/livewire/settings-backup.blade.php @@ -12,13 +12,12 @@
- +
- +
- - +
diff --git a/tests/Feature/ReadOnlyFieldsAreNotPersistedTest.php b/tests/Feature/ReadOnlyFieldsAreNotPersistedTest.php new file mode 100644 index 0000000000..4075717d90 --- /dev/null +++ b/tests/Feature/ReadOnlyFieldsAreNotPersistedTest.php @@ -0,0 +1,95 @@ + 0]); + + $this->team = Team::factory()->create(); + $this->owner = User::factory()->create(); + $this->team->members()->attach($this->owner->id, ['role' => 'owner']); + + $this->server = Server::factory()->create(['team_id' => $this->team->id]); + $this->destination = StandaloneDocker::where('server_id', $this->server->id)->first(); + $project = Project::factory()->create(['team_id' => $this->team->id]); + $this->environment = Environment::factory()->create(['project_id' => $project->id]); + + $this->actingAs($this->owner); + session(['currentTeam' => $this->team]); +}); + +dataset('read-only-credential-components', [ + 'keydb' => ['create_standalone_keydb', KeydbGeneral::class, ['keydbPassword' => 'keydb_password']], + 'dragonfly' => ['create_standalone_dragonfly', DragonflyGeneral::class, ['dragonflyPassword' => 'dragonfly_password']], + 'clickhouse' => ['create_standalone_clickhouse', ClickhouseGeneral::class, [ + 'clickhouseAdminUser' => 'clickhouse_admin_user', + 'clickhouseAdminPassword' => 'clickhouse_admin_password', + ]], +]); + +it('does not save read-only credentials of a started database', function (string $createDatabase, string $generalClass, array $fields) { + $database = $createDatabase($this->environment->id, $this->destination, ['started_at' => now()])->fresh(); + $stored = collect($fields)->mapWithKeys(fn (string $column) => [$column => $database->{$column}]); + + $component = Livewire::test($generalClass, ['database' => $database])->set('description', 'Updated'); + foreach (array_keys($fields) as $property) { + $component->set($property, 'AutofilledLogin1234567890'); + } + $component->call('submit')->assertHasNoErrors(); + + $database->refresh(); + expect($database->description)->toBe('Updated'); + foreach ($stored as $column => $value) { + expect($database->{$column})->toBe($value); + } +})->with('read-only-credential-components'); + +it('saves credentials of a database that was never started', function (string $createDatabase, string $generalClass, array $fields) { + $database = $createDatabase($this->environment->id, $this->destination)->fresh(); + + $component = Livewire::test($generalClass, ['database' => $database]); + foreach (array_keys($fields) as $property) { + $component->set($property, 'ChangedBeforeStart1234567890'); + } + $component->call('submit')->assertHasNoErrors(); + + $database->refresh(); + foreach ($fields as $column) { + expect($database->{$column})->toBe('ChangedBeforeStart1234567890'); + } +})->with('read-only-credential-components'); + +it('rejects changes to the read-only destination network and server IP', function (string $property) { + $component = Livewire::test(DestinationShow::class, ['destination_uuid' => $this->destination->uuid]); + + expect(fn () => $component->set($property, 'tampered'))->toThrow(CannotUpdateLockedPropertyException::class); +})->with(['network', 'serverIp']); + +it('saves the destination name without changing the network or server IP', function () { + $network = $this->destination->network; + $ip = (string) $this->server->ip; + + Livewire::test(DestinationShow::class, ['destination_uuid' => $this->destination->uuid]) + ->set('name', 'renamed-destination') + ->call('submit') + ->assertHasNoErrors(); + + expect($this->destination->fresh()->name)->toBe('renamed-destination') + ->and($this->destination->fresh()->network)->toBe($network) + ->and((string) $this->server->fresh()->ip)->toBe($ip); +}); diff --git a/tests/Feature/SettingsBackupEmptyStateTest.php b/tests/Feature/SettingsBackupEmptyStateTest.php index 605350beb8..30392acc49 100644 --- a/tests/Feature/SettingsBackupEmptyStateTest.php +++ b/tests/Feature/SettingsBackupEmptyStateTest.php @@ -216,11 +216,38 @@ test('configure backup creates the instance database instead of using a team dat Livewire::test(SettingsBackup::class) ->call('addCoolifyDatabase') - ->assertHasNoErrors() - ->assertSet('postgres_password', 'instance-secret'); + ->assertHasNoErrors(); $instanceDatabase = StandalonePostgresql::query()->findOrFail(0); - expect($instanceDatabase->scheduledBackups()->sole()->team_id)->toBe(0) + expect($instanceDatabase->postgres_password)->toBe('instance-secret') + ->and($instanceDatabase->scheduledBackups()->sole()->team_id)->toBe(0) ->and($tenantDatabase->fresh()->scheduledBackups()->exists())->toBeFalse() ->and($tenantDatabase->fresh()->postgres_password)->toBe('tenant-secret'); }); + +test('saving instance backup settings only persists the description', function () { + $database = settingsBackupInstanceDatabaseWithoutSchedule(); + $database->forceFill(['postgres_password' => 'instance-db-secret'])->save(); + $database->scheduledBackups()->create([ + 'enabled' => true, + 'save_s3' => false, + 'frequency' => '0 0 * * *', + 'team_id' => 0, + ]); + + Livewire::test(SettingsBackup::class) + ->assertSee('Instance database') + ->assertSee('coolify') + ->assertDontSeeHtml('wire:model=postgres_user') + ->assertDontSeeHtml('wire:model=postgres_password') + ->assertDontSee('instance-db-secret', stripInitialData: false) + ->set('description', 'Updated description') + ->call('submit') + ->assertHasNoErrors(); + + $database->refresh(); + expect($database->description)->toBe('Updated description') + ->and($database->name)->toBe('coolify-db') + ->and($database->postgres_user)->toBe('coolify') + ->and($database->postgres_password)->toBe('instance-db-secret'); +});