From 0620496c5fc527b9f2fe810c7e9014e8244d510a Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Sun, 19 Apr 2026 15:17:47 +0200 Subject: [PATCH 1/2] fix(server): exclude persistent resources from container prune Prevent docker container prune from removing containers labeled as database, application, or service types. Previously only proxy containers were excluded, risking accidental cleanup of active resources. --- app/Actions/Server/CleanupDocker.php | 2 +- tests/Unit/Actions/Server/CleanupDockerTest.php | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/app/Actions/Server/CleanupDocker.php b/app/Actions/Server/CleanupDocker.php index 0d9ca0153..98cce088b 100644 --- a/app/Actions/Server/CleanupDocker.php +++ b/app/Actions/Server/CleanupDocker.php @@ -48,7 +48,7 @@ public function handle(Server $server, bool $deleteUnusedVolumes = false, bool $ ); $commands = [ - 'docker container prune -f --filter "label=coolify.managed=true" --filter "label!=coolify.proxy=true"', + 'docker container prune -f --filter "label=coolify.managed=true" --filter "label!=coolify.proxy=true" --filter "label!=coolify.type=database" --filter "label!=coolify.type=application" --filter "label!=coolify.type=service"', $imagePruneCmd, 'docker builder prune -af', "docker images --filter before=$helperImageWithVersion --filter reference=$helperImage | grep $helperImage | awk '{print $3}' | xargs -r docker rmi -f", diff --git a/tests/Unit/Actions/Server/CleanupDockerTest.php b/tests/Unit/Actions/Server/CleanupDockerTest.php index fc8b8ab9b..1a6a0d3d6 100644 --- a/tests/Unit/Actions/Server/CleanupDockerTest.php +++ b/tests/Unit/Actions/Server/CleanupDockerTest.php @@ -437,6 +437,16 @@ expect($buildImagesToDelete->pluck('tag')->toArray())->toContain('commit1-build'); }); +it('container prune excludes persistent resource types', function () { + $sourceFile = file_get_contents(__DIR__.'/../../../../app/Actions/Server/CleanupDocker.php'); + + expect($sourceFile)->toContain('label!=coolify.type=database'); + expect($sourceFile)->toContain('label!=coolify.type=application'); + expect($sourceFile)->toContain('label!=coolify.type=service'); + expect($sourceFile)->toContain('label!=coolify.proxy=true'); + expect($sourceFile)->toContain('label=coolify.managed=true'); +}); + it('preserves build image for currently running tag', function () { $images = collect([ ['repository' => 'app-uuid', 'tag' => 'commit1', 'created_at' => '2024-01-01 10:00:00', 'image_ref' => 'app-uuid:commit1'], From 5019c8db928afd34c0c9d17c5d20019fa053c344 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Sun, 19 Apr 2026 15:26:47 +0200 Subject: [PATCH 2/2] fix(api): use explicit team ID for S3 storage lookup in backup endpoints Replace `ownedByCurrentTeam()` (session-based) with `ownedByCurrentTeamAPI($teamId)` (explicit team ID) when resolving S3 storage in create_backup and update_backup. Session-based team resolution is unreliable in API context where auth is token-based. Add `S3Storage::ownedByCurrentTeamAPI(int $teamId)` scope and update feature tests to use real model instances instead of Mockery mocks. --- .../Controllers/Api/DatabasesController.php | 8 +- app/Models/S3Storage.php | 7 + .../Feature/DatabaseBackupCreationApiTest.php | 208 ++++++++++++------ 3 files changed, 146 insertions(+), 77 deletions(-) diff --git a/app/Http/Controllers/Api/DatabasesController.php b/app/Http/Controllers/Api/DatabasesController.php index f3783696d..8241e5fba 100644 --- a/app/Http/Controllers/Api/DatabasesController.php +++ b/app/Http/Controllers/Api/DatabasesController.php @@ -747,7 +747,7 @@ public function create_backup(Request $request) } if ($request->filled('s3_storage_uuid')) { - $existsInTeam = S3Storage::ownedByCurrentTeam()->where('uuid', $request->s3_storage_uuid)->exists(); + $existsInTeam = S3Storage::ownedByCurrentTeamAPI($teamId)->where('uuid', $request->s3_storage_uuid)->exists(); if (! $existsInTeam) { return response()->json([ 'message' => 'Validation failed.', @@ -774,7 +774,7 @@ public function create_backup(Request $request) // Convert s3_storage_uuid to s3_storage_id if (isset($backupData['s3_storage_uuid'])) { - $s3Storage = S3Storage::ownedByCurrentTeam()->where('uuid', $backupData['s3_storage_uuid'])->first(); + $s3Storage = S3Storage::ownedByCurrentTeamAPI($teamId)->where('uuid', $backupData['s3_storage_uuid'])->first(); if ($s3Storage) { $backupData['s3_storage_id'] = $s3Storage->id; } elseif ($request->boolean('save_s3')) { @@ -982,7 +982,7 @@ public function update_backup(Request $request) ], 422); } if ($request->filled('s3_storage_uuid')) { - $existsInTeam = S3Storage::ownedByCurrentTeam()->where('uuid', $request->s3_storage_uuid)->exists(); + $existsInTeam = S3Storage::ownedByCurrentTeamAPI($teamId)->where('uuid', $request->s3_storage_uuid)->exists(); if (! $existsInTeam) { return response()->json([ 'message' => 'Validation failed.', @@ -1015,7 +1015,7 @@ public function update_backup(Request $request) // Convert s3_storage_uuid to s3_storage_id if (isset($backupData['s3_storage_uuid'])) { - $s3Storage = S3Storage::ownedByCurrentTeam()->where('uuid', $backupData['s3_storage_uuid'])->first(); + $s3Storage = S3Storage::ownedByCurrentTeamAPI($teamId)->where('uuid', $backupData['s3_storage_uuid'])->first(); if ($s3Storage) { $backupData['s3_storage_id'] = $s3Storage->id; } elseif ($request->boolean('save_s3')) { diff --git a/app/Models/S3Storage.php b/app/Models/S3Storage.php index d6feccc7e..e02e07a4e 100644 --- a/app/Models/S3Storage.php +++ b/app/Models/S3Storage.php @@ -66,6 +66,13 @@ public static function ownedByCurrentTeam(array $select = ['*']) return S3Storage::whereTeamId(currentTeam()->id)->select($selectArray->all())->orderBy('name'); } + public static function ownedByCurrentTeamAPI(int $teamId, array $select = ['*']) + { + $selectArray = collect($select)->concat(['id']); + + return S3Storage::whereTeamId($teamId)->select($selectArray->all())->orderBy('name'); + } + public function isUsable() { return $this->is_usable; diff --git a/tests/Feature/DatabaseBackupCreationApiTest.php b/tests/Feature/DatabaseBackupCreationApiTest.php index 893141de3..4588cf9de 100644 --- a/tests/Feature/DatabaseBackupCreationApiTest.php +++ b/tests/Feature/DatabaseBackupCreationApiTest.php @@ -1,5 +1,12 @@ 0]); + $this->team = Team::factory()->create(); $this->user = User::factory()->create(); $this->team->members()->attach($this->user->id, ['role' => 'owner']); - // Create an API token for the user - $this->token = $this->user->createToken('test-token', ['*'], $this->team->id); + session(['currentTeam' => $this->team]); + + $this->token = $this->user->createToken('test-token', ['*']); $this->bearerToken = $this->token->plainTextToken; - // Mock a database - we'll use Mockery to avoid needing actual database setup - $this->database = \Mockery::mock(StandalonePostgresql::class); - $this->database->shouldReceive('getAttribute')->with('id')->andReturn(1); - $this->database->shouldReceive('getAttribute')->with('uuid')->andReturn('test-db-uuid'); - $this->database->shouldReceive('getAttribute')->with('postgres_db')->andReturn('testdb'); - $this->database->shouldReceive('type')->andReturn('standalone-postgresql'); - $this->database->shouldReceive('getMorphClass')->andReturn('App\Models\StandalonePostgresql'); -}); + $this->server = Server::factory()->create(['team_id' => $this->team->id]); + $this->destination = StandaloneDocker::where('server_id', $this->server->id)->first(); + $this->project = Project::factory()->create(['team_id' => $this->team->id]); + $this->environment = Environment::factory()->create(['project_id' => $this->project->id]); -afterEach(function () { - \Mockery::close(); + $this->database = StandalonePostgresql::create([ + 'name' => 'test-postgres', + 'image' => 'postgres:15-alpine', + 'postgres_user' => 'postgres', + 'postgres_password' => 'password', + 'postgres_db' => 'testdb', + 'environment_id' => $this->environment->id, + 'destination_id' => $this->destination->id, + 'destination_type' => $this->destination->getMorphClass(), + ]); + + $this->s3Storage = S3Storage::create([ + 'name' => 'test-s3', + 'region' => 'us-east-1', + 'key' => 'test-key', + 'secret' => 'test-secret', + 'bucket' => 'test-bucket', + 'endpoint' => 'https://s3.example.com', + 'team_id' => $this->team->id, + 'is_usable' => true, + ]); }); describe('POST /api/v1/databases/{uuid}/backups', function () { - test('creates backup configuration with minimal required fields', function () { - // This is a unit-style test using mocks to avoid database dependency - // For full integration testing, this should be run inside Docker + test('creates backup with s3 storage via API token', function () { + $response = $this->withHeaders([ + 'Authorization' => 'Bearer '.$this->bearerToken, + 'Content-Type' => 'application/json', + ])->postJson("/api/v1/databases/{$this->database->uuid}/backups", [ + 'frequency' => '0 2 * * 0', + 'save_s3' => true, + 's3_storage_uuid' => $this->s3Storage->uuid, + 'enabled' => true, + ]); + + $response->assertStatus(201); + $response->assertJsonStructure(['uuid', 'message']); + + $backup = ScheduledDatabaseBackup::where('uuid', $response->json('uuid'))->first(); + expect($backup)->not->toBeNull(); + expect($backup->s3_storage_id)->toBe($this->s3Storage->id); + expect($backup->save_s3)->toBeTrue(); + expect($backup->team_id)->toBe($this->team->id); + }); + + test('creates backup without s3 storage', function () { + $response = $this->withHeaders([ + 'Authorization' => 'Bearer '.$this->bearerToken, + 'Content-Type' => 'application/json', + ])->postJson("/api/v1/databases/{$this->database->uuid}/backups", [ + 'frequency' => 'daily', + ]); + + $response->assertStatus(201); + $response->assertJsonStructure(['uuid', 'message']); + }); + + test('rejects s3_storage_uuid from another team', function () { + $otherTeam = Team::factory()->create(); + $otherS3 = S3Storage::create([ + 'name' => 'other-s3', + 'region' => 'us-east-1', + 'key' => 'other-key', + 'secret' => 'other-secret', + 'bucket' => 'other-bucket', + 'endpoint' => 'https://s3.example.com', + 'team_id' => $otherTeam->id, + 'is_usable' => true, + ]); $response = $this->withHeaders([ 'Authorization' => 'Bearer '.$this->bearerToken, 'Content-Type' => 'application/json', - ])->postJson('/api/v1/databases/test-db-uuid/backups', [ - 'frequency' => 'daily', + ])->postJson("/api/v1/databases/{$this->database->uuid}/backups", [ + 'frequency' => '0 2 * * 0', + 'save_s3' => true, + 's3_storage_uuid' => $otherS3->uuid, ]); - // Since we're mocking, this test verifies the endpoint exists and basic validation - // Full integration tests should be run in Docker environment - expect($response->status())->toBeIn([201, 404, 422]); + $response->assertStatus(422); + $response->assertJsonValidationErrors(['s3_storage_uuid']); }); test('validates frequency is required', function () { $response = $this->withHeaders([ 'Authorization' => 'Bearer '.$this->bearerToken, 'Content-Type' => 'application/json', - ])->postJson('/api/v1/databases/test-db-uuid/backups', [ + ])->postJson("/api/v1/databases/{$this->database->uuid}/backups", [ 'enabled' => true, ]); @@ -63,83 +130,78 @@ $response = $this->withHeaders([ 'Authorization' => 'Bearer '.$this->bearerToken, 'Content-Type' => 'application/json', - ])->postJson('/api/v1/databases/test-db-uuid/backups', [ + ])->postJson("/api/v1/databases/{$this->database->uuid}/backups", [ 'frequency' => 'daily', 'save_s3' => true, ]); - // Should fail validation because s3_storage_uuid is missing - expect($response->status())->toBeIn([404, 422]); - }); - - test('rejects invalid frequency format', function () { - $response = $this->withHeaders([ - 'Authorization' => 'Bearer '.$this->bearerToken, - 'Content-Type' => 'application/json', - ])->postJson('/api/v1/databases/test-db-uuid/backups', [ - 'frequency' => 'invalid-frequency', - ]); - - expect($response->status())->toBeIn([404, 422]); + $response->assertStatus(422); + $response->assertJsonValidationErrors(['s3_storage_uuid']); }); test('rejects request without authentication', function () { - $response = $this->postJson('/api/v1/databases/test-db-uuid/backups', [ + $response = $this->postJson("/api/v1/databases/{$this->database->uuid}/backups", [ 'frequency' => 'daily', ]); $response->assertStatus(401); }); +}); - test('validates retention fields are integers with minimum 0', function () { - $response = $this->withHeaders([ - 'Authorization' => 'Bearer '.$this->bearerToken, - 'Content-Type' => 'application/json', - ])->postJson('/api/v1/databases/test-db-uuid/backups', [ +describe('PATCH /api/v1/databases/{uuid}/backups/{scheduled_backup_uuid}', function () { + test('updates backup to use s3 storage via API token', function () { + $backup = ScheduledDatabaseBackup::create([ 'frequency' => 'daily', - 'database_backup_retention_amount_locally' => -1, + 'enabled' => true, + 'database_id' => $this->database->id, + 'database_type' => $this->database->getMorphClass(), + 'team_id' => $this->team->id, ]); - expect($response->status())->toBeIn([404, 422]); - }); - - test('accepts valid cron expressions', function () { $response = $this->withHeaders([ 'Authorization' => 'Bearer '.$this->bearerToken, 'Content-Type' => 'application/json', - ])->postJson('/api/v1/databases/test-db-uuid/backups', [ - 'frequency' => '0 2 * * *', // Daily at 2 AM + ])->patchJson("/api/v1/databases/{$this->database->uuid}/backups/{$backup->uuid}", [ + 'save_s3' => true, + 's3_storage_uuid' => $this->s3Storage->uuid, ]); - // Will fail with 404 because database doesn't exist, but validates the request format - expect($response->status())->toBeIn([201, 404, 422]); + $response->assertStatus(200); + $backup->refresh(); + expect($backup->s3_storage_id)->toBe($this->s3Storage->id); + expect($backup->save_s3)->toBeTrue(); }); - test('accepts predefined frequency values', function () { - $frequencies = ['every_minute', 'hourly', 'daily', 'weekly', 'monthly', 'yearly']; + test('rejects s3_storage_uuid from another team on update', function () { + $otherTeam = Team::factory()->create(); + $otherS3 = S3Storage::create([ + 'name' => 'other-s3', + 'region' => 'us-east-1', + 'key' => 'other-key', + 'secret' => 'other-secret', + 'bucket' => 'other-bucket', + 'endpoint' => 'https://s3.example.com', + 'team_id' => $otherTeam->id, + 'is_usable' => true, + ]); - foreach ($frequencies as $frequency) { - $response = $this->withHeaders([ - 'Authorization' => 'Bearer '.$this->bearerToken, - 'Content-Type' => 'application/json', - ])->postJson('/api/v1/databases/test-db-uuid/backups', [ - 'frequency' => $frequency, - ]); - - // Will fail with 404 because database doesn't exist, but validates the request format - expect($response->status())->toBeIn([201, 404, 422]); - } - }); - - test('rejects extra fields not in allowed list', function () { - $response = $this->withHeaders([ - 'Authorization' => 'Bearer '.$this->bearerToken, - 'Content-Type' => 'application/json', - ])->postJson('/api/v1/databases/test-db-uuid/backups', [ + $backup = ScheduledDatabaseBackup::create([ 'frequency' => 'daily', - 'invalid_field' => 'invalid_value', + 'enabled' => true, + 'database_id' => $this->database->id, + 'database_type' => $this->database->getMorphClass(), + 'team_id' => $this->team->id, ]); - expect($response->status())->toBeIn([404, 422]); + $response = $this->withHeaders([ + 'Authorization' => 'Bearer '.$this->bearerToken, + 'Content-Type' => 'application/json', + ])->patchJson("/api/v1/databases/{$this->database->uuid}/backups/{$backup->uuid}", [ + 'save_s3' => true, + 's3_storage_uuid' => $otherS3->uuid, + ]); + + $response->assertStatus(422); + $response->assertJsonValidationErrors(['s3_storage_uuid']); }); });