From 3379f85c0a8728c28cee0078ace288a7589c43f9 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Thu, 1 Oct 2026 20:31:00 +0200 Subject: [PATCH] fix(security): authorize import file checks and Vultr status refresh ImportForm::checkFile() and checkS3File() ran without authorization, so a member with view access could check whether paths exist on the server or probe S3. They now require update access, like the restore actions. Server\Show::checkVultrInstanceStatus() now requires view access, like the Hetzner and DigitalOcean checks. --- app/Livewire/Project/Database/ImportForm.php | 4 + app/Livewire/Server/Show.php | 1 + ...seImportFormFileCheckAuthorizationTest.php | 104 ++++++++++++++++++ tests/Feature/VultrServerLifecycleTest.php | 21 ++++ .../Database/ImportCheckFileButtonTest.php | 10 -- 5 files changed, 130 insertions(+), 10 deletions(-) create mode 100644 tests/Feature/DatabaseImportFormFileCheckAuthorizationTest.php diff --git a/app/Livewire/Project/Database/ImportForm.php b/app/Livewire/Project/Database/ImportForm.php index e16c7e331b..638101b3c1 100644 --- a/app/Livewire/Project/Database/ImportForm.php +++ b/app/Livewire/Project/Database/ImportForm.php @@ -403,6 +403,8 @@ class ImportForm extends Component public function checkFile() { + $this->authorize('update', $this->resource); + if (filled($this->customLocation)) { // Validate the custom location to prevent command injection if (! $this->validateServerPath($this->customLocation)) { @@ -524,6 +526,8 @@ class ImportForm extends Component public function checkS3File() { + $this->authorize('update', $this->resource); + if (! $this->s3StorageId) { $this->dispatch('error', 'Please select an S3 storage.'); diff --git a/app/Livewire/Server/Show.php b/app/Livewire/Server/Show.php index 707af97515..e596a9c2f9 100644 --- a/app/Livewire/Server/Show.php +++ b/app/Livewire/Server/Show.php @@ -590,6 +590,7 @@ class Show extends Component public function checkVultrInstanceStatus(bool $manual = false) { try { + $this->authorize('view', $this->server); if (! $this->server->vultr_instance_id || ! $this->server->cloudProviderToken) { $this->dispatch('error', 'This server is not associated with a Vultr instance or token.'); diff --git a/tests/Feature/DatabaseImportFormFileCheckAuthorizationTest.php b/tests/Feature/DatabaseImportFormFileCheckAuthorizationTest.php new file mode 100644 index 0000000000..a5de664ee1 --- /dev/null +++ b/tests/Feature/DatabaseImportFormFileCheckAuthorizationTest.php @@ -0,0 +1,104 @@ + 0]); + + $this->team = Team::factory()->create(); + $this->server = Server::factory()->create(['team_id' => $this->team->id]); + $destination = StandaloneDocker::firstOrCreate( + ['server_id' => $this->server->id, 'network' => 'coolify'], + ['uuid' => (string) Str::uuid(), 'name' => 'docker'] + ); + $project = Project::factory()->create(['team_id' => $this->team->id]); + $environment = Environment::factory()->create(['project_id' => $project->id]); + $this->database = StandalonePostgresql::create([ + 'uuid' => (string) Str::uuid(), + 'name' => 'db', + 'postgres_user' => 'postgres', + 'postgres_password' => 'password', + 'postgres_db' => 'db', + 'image' => 'postgres:17', + 'status' => 'running', + 'environment_id' => $environment->id, + 'destination_id' => $destination->id, + 'destination_type' => $destination->getMorphClass(), + ]); +}); + +class ImportFormFileCheckAuthorizationTestComponent extends ImportForm +{ + public function mount($database = null, $server = null): void + { + $this->resourceId = $database->id; + $this->resourceType = $database::class; + $this->serverId = $server->id; + $this->container = $database->uuid; + $this->resourceUuid = $database->uuid; + $this->resourceStatus = $database->status ?? ''; + $this->resourceDbType = $database->type(); + } + + public function render() + { + return view('livewire.project.database.import-form'); + } +} + +function importFormFileCheckAs(Team $team, string $role): Testable +{ + $user = User::factory()->create(); + $team->members()->attach($user, ['role' => $role]); + test()->actingAs($user); + session(['currentTeam' => $team]); + + return Livewire::test(ImportFormFileCheckAuthorizationTestComponent::class, [ + 'database' => test()->database, + 'server' => test()->server, + ]); +} + +it('forbids members from checking a server file', function () { + importFormFileCheckAs($this->team, 'member') + ->set('customLocation', 'not-an-absolute-path') + ->call('checkFile') + ->assertForbidden(); +}); + +it('forbids members from checking an S3 file', function () { + importFormFileCheckAs($this->team, 'member') + ->call('checkS3File') + ->assertForbidden(); +}); + +it('lets admins and owners check files', function (string $role) { + importFormFileCheckAs($this->team, $role) + ->set('customLocation', 'not-an-absolute-path') + ->call('checkFile') + ->assertDispatched('error', 'Invalid file path. Path must be absolute and contain only safe characters (alphanumerics, dots, dashes, underscores, slashes).') + ->call('checkS3File') + ->assertDispatched('error', 'Please select an S3 storage.'); +})->with(['admin', 'owner']); + +it('does nothing when an admin checks an empty server path', function () { + importFormFileCheckAs($this->team, 'admin') + ->set('customLocation', '') + ->call('checkFile') + ->assertNotDispatched('error') + ->assertSet('filename', null); +}); diff --git a/tests/Feature/VultrServerLifecycleTest.php b/tests/Feature/VultrServerLifecycleTest.php index fbda1e48bd..7efae02878 100644 --- a/tests/Feature/VultrServerLifecycleTest.php +++ b/tests/Feature/VultrServerLifecycleTest.php @@ -340,3 +340,24 @@ it('links a server to Vultr by matching IP', function () { 'vultr_instance_status' => 'active', ]); }); + +it('does not refresh Vultr status for a user outside the server team', function () { + Http::fake([ + 'https://api.vultr.com/v2/instances/instance-1' => Http::response([ + 'instance' => ['id' => 'instance-1', 'status' => 'active'], + ], 200), + ]); + + $component = Livewire::test(Show::class, ['server_uuid' => $this->server->uuid]); + + $outsider = User::factory()->create(); + Team::factory()->create()->members()->attach($outsider->id, ['role' => 'owner']); + $this->actingAs($outsider); + + $component->call('checkVultrInstanceStatus', true) + ->assertDispatched('error') + ->assertNotDispatched('success'); + + expect($this->server->fresh()->vultr_instance_status)->toBe('pending'); + Http::assertNothingSent(); +}); diff --git a/tests/Unit/Project/Database/ImportCheckFileButtonTest.php b/tests/Unit/Project/Database/ImportCheckFileButtonTest.php index 087a40a379..4a380381cd 100644 --- a/tests/Unit/Project/Database/ImportCheckFileButtonTest.php +++ b/tests/Unit/Project/Database/ImportCheckFileButtonTest.php @@ -2,16 +2,6 @@ use App\Livewire\Project\Database\ImportForm; -test('checkFile does nothing when customLocation is empty', function () { - $component = new ImportForm; - $component->customLocation = ''; - - // No server commands should be executed when customLocation is empty - $component->checkFile(); - - expect($component->filename)->toBeNull(); -}); - test('checkFile validates file exists on server when customLocation is filled', function () { $component = new ImportForm; $component->customLocation = '/tmp/backup.sql';