From e78659dfedd9f978d1eb611fb906ab3c43c90bdf Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Tue, 27 Oct 2026 17:00:56 +0100 Subject: [PATCH] fix(api): preserve omitted env var values and flags on PATCH (#12240) --- .../Controllers/Api/DatabasesController.php | 1 + .../Controllers/Api/ServicesController.php | 2 + .../Api/EnvironmentVariableUpdateApiTest.php | 162 ++++++++++++++++++ tests/Feature/Api/ServerTransferApiTest.php | 32 ++++ .../DatabaseEnvironmentVariableApiTest.php | 23 +++ 5 files changed, 220 insertions(+) diff --git a/app/Http/Controllers/Api/DatabasesController.php b/app/Http/Controllers/Api/DatabasesController.php index 106216f7c9..78961ca93c 100644 --- a/app/Http/Controllers/Api/DatabasesController.php +++ b/app/Http/Controllers/Api/DatabasesController.php @@ -4148,6 +4148,7 @@ class DatabasesController extends Controller $updatedEnvs = collect(); foreach ($bulk_data as $item) { + $item = Arr::only($item, ['key', 'value', 'is_literal', 'is_multiline', 'is_shown_once', 'comment']); if (array_key_exists('key', $item)) { $item['key'] = ValidationPatterns::normalizeEnvironmentVariableKey((string) $item['key']); } diff --git a/app/Http/Controllers/Api/ServicesController.php b/app/Http/Controllers/Api/ServicesController.php index 8cb4235d6a..3265edc48e 100644 --- a/app/Http/Controllers/Api/ServicesController.php +++ b/app/Http/Controllers/Api/ServicesController.php @@ -22,6 +22,7 @@ use App\Support\ValidationPatterns; use Illuminate\Database\Eloquent\Model; use Illuminate\Http\JsonResponse; use Illuminate\Http\Request; +use Illuminate\Support\Arr; use Illuminate\Support\Collection; use Illuminate\Support\Facades\Validator; use OpenApi\Attributes as OA; @@ -1702,6 +1703,7 @@ class ServicesController extends Controller $updatedEnvs = collect(); foreach ($bulk_data as $item) { + $item = Arr::only($item, ['key', 'value', 'is_literal', 'is_multiline', 'is_shown_once', 'comment']); if (array_key_exists('key', $item)) { $item['key'] = ValidationPatterns::normalizeEnvironmentVariableKey((string) $item['key']); } diff --git a/tests/Feature/Api/EnvironmentVariableUpdateApiTest.php b/tests/Feature/Api/EnvironmentVariableUpdateApiTest.php index 97d100cc2a..fbb8fd64f6 100644 --- a/tests/Feature/Api/EnvironmentVariableUpdateApiTest.php +++ b/tests/Feature/Api/EnvironmentVariableUpdateApiTest.php @@ -252,6 +252,111 @@ describe('PATCH /api/v1/applications/{uuid}/envs', function () { $response->assertStatus(422); $response->assertJsonFragment(['uuid' => ['This field is not allowed.']]); }); + + test('keeps value and other flags when only is_buildtime is sent', function (bool $isPreview) { + $application = Application::factory()->create([ + 'environment_id' => $this->environment->id, + 'destination_id' => $this->destination->id, + 'destination_type' => $this->destination->getMorphClass(), + ]); + + $env = EnvironmentVariable::create([ + 'key' => 'NEXTAUTH_SECRET', + 'value' => 'super-secret', + 'resourceable_type' => Application::class, + 'resourceable_id' => $application->id, + 'is_preview' => $isPreview, + 'is_literal' => true, + 'is_multiline' => true, + 'is_shown_once' => true, + 'is_buildtime' => true, + 'is_runtime' => true, + ]); + + $payload = ['key' => 'NEXTAUTH_SECRET', 'is_buildtime' => false]; + if ($isPreview) { + $payload['is_preview'] = true; + } + + $this->withHeaders([ + 'Authorization' => 'Bearer '.$this->bearerToken, + 'Content-Type' => 'application/json', + ])->patchJson("/api/v1/applications/{$application->uuid}/envs", $payload) + ->assertStatus(201); + + $env->refresh(); + expect($env->value)->toBe('super-secret') + ->and($env->is_buildtime)->toBeFalse() + ->and($env->is_runtime)->toBeTrue() + ->and($env->is_literal)->toBeTruthy() + ->and($env->is_multiline)->toBeTrue() + ->and($env->is_shown_once)->toBeTruthy(); + })->with(['production' => false, 'preview' => true]); + + test('bulk update keeps value and other flags when only is_buildtime is sent', function () { + $application = Application::factory()->create([ + 'environment_id' => $this->environment->id, + 'destination_id' => $this->destination->id, + 'destination_type' => $this->destination->getMorphClass(), + ]); + + $env = EnvironmentVariable::create([ + 'key' => 'NEXTAUTH_SECRET', + 'value' => 'super-secret', + 'resourceable_type' => Application::class, + 'resourceable_id' => $application->id, + 'is_preview' => false, + 'is_literal' => true, + 'is_multiline' => true, + 'is_shown_once' => true, + 'is_buildtime' => true, + ]); + + $this->withHeaders([ + 'Authorization' => 'Bearer '.$this->bearerToken, + 'Content-Type' => 'application/json', + ])->patchJson("/api/v1/applications/{$application->uuid}/envs/bulk", [ + 'data' => [['key' => 'NEXTAUTH_SECRET', 'is_buildtime' => false]], + ])->assertStatus(201); + + $env->refresh(); + expect($env->value)->toBe('super-secret') + ->and($env->is_buildtime)->toBeFalse() + ->and($env->is_literal)->toBeTruthy() + ->and($env->is_multiline)->toBeTrue() + ->and($env->is_shown_once)->toBeTruthy(); + }); +}); + +describe('partial PATCH /api/v1/services/{uuid}/envs', function () { + test('keeps value when only a flag is sent', function () { + $service = Service::factory()->create([ + 'server_id' => $this->server->id, + 'destination_id' => $this->destination->id, + 'destination_type' => $this->destination->getMorphClass(), + 'environment_id' => $this->environment->id, + ]); + + $env = EnvironmentVariable::create([ + 'key' => 'SECRET', + 'value' => 'super-secret', + 'resourceable_type' => Service::class, + 'resourceable_id' => $service->id, + 'is_preview' => false, + ]); + + $this->withHeaders([ + 'Authorization' => 'Bearer '.$this->bearerToken, + 'Content-Type' => 'application/json', + ])->patchJson("/api/v1/services/{$service->uuid}/envs", [ + 'key' => 'SECRET', + 'is_literal' => true, + ])->assertStatus(201); + + $env->refresh(); + expect($env->value)->toBe('super-secret') + ->and($env->is_literal)->toBeTruthy(); + }); }); describe('application env PATCH keeps omitted flags', function () { @@ -417,6 +522,63 @@ describe('environment variable key validation for app and service APIs', functio }); }); +describe('bulk env update for services and databases', function () { + test('cannot move an environment variable to a resource of another team', function (string $resourceType) { + $resource = $resourceType === 'services' + ? Service::factory()->create([ + 'server_id' => $this->server->id, + 'destination_id' => $this->destination->id, + 'destination_type' => $this->destination->getMorphClass(), + 'environment_id' => $this->environment->id, + ]) + : StandalonePostgresql::create([ + 'name' => 'test-postgres', + 'image' => 'postgres:15-alpine', + 'postgres_user' => 'postgres', + 'postgres_password' => 'password', + 'postgres_db' => 'postgres', + 'environment_id' => $this->environment->id, + 'destination_id' => $this->destination->id, + 'destination_type' => $this->destination->getMorphClass(), + ]); + + $otherTeam = Team::factory()->create(); + $otherServer = Server::factory()->create(['team_id' => $otherTeam->id]); + $otherDestination = StandaloneDocker::where('server_id', $otherServer->id)->first(); + $victim = Application::factory()->create([ + 'environment_id' => Environment::factory()->create([ + 'project_id' => Project::factory()->create(['team_id' => $otherTeam->id])->id, + ])->id, + 'destination_id' => $otherDestination->id, + 'destination_type' => $otherDestination->getMorphClass(), + ]); + + $existing = EnvironmentVariable::create([ + 'key' => 'EXISTING', + 'value' => 'old-value', + 'resourceable_type' => $resource->getMorphClass(), + 'resourceable_id' => $resource->id, + ]); + + $this->withHeaders([ + 'Authorization' => 'Bearer '.$this->bearerToken, + 'Content-Type' => 'application/json', + ])->patchJson("/api/v1/{$resourceType}/{$resource->uuid}/envs/bulk", [ + 'data' => [ + ['key' => 'EXISTING', 'value' => 'new-value', 'resourceable_type' => Application::class, 'resourceable_id' => $victim->id], + ['key' => 'CREATED', 'value' => 'x', 'resourceable_type' => Application::class, 'resourceable_id' => $victim->id], + ], + ])->assertStatus(201); + + $existing->refresh(); + expect($existing->resourceable_id)->toBe($resource->id) + ->and($existing->resourceable_type)->toBe($resource->getMorphClass()) + ->and($existing->value)->toBe('new-value') + ->and($victim->environment_variables()->whereIn('key', ['EXISTING', 'CREATED'])->exists())->toBeFalse() + ->and($resource->environment_variables()->where('key', 'CREATED')->exists())->toBeTrue(); + })->with(['services', 'databases']); +}); + describe('env PATCH keeps the value when value is omitted', function () { beforeEach(function () { $this->headers = [ diff --git a/tests/Feature/Api/ServerTransferApiTest.php b/tests/Feature/Api/ServerTransferApiTest.php index a6705d7eca..ee10ca3192 100644 --- a/tests/Feature/Api/ServerTransferApiTest.php +++ b/tests/Feature/Api/ServerTransferApiTest.php @@ -258,6 +258,38 @@ describe('POST /api/v1/servers/import', function () { Queue::assertNotPushed(ValidateAndInstallServerJob::class); }); + test('rejects a database entry that names a model outside the database types', function () { + $otherTeam = Team::factory()->create(); + $otherKey = PrivateKey::factory()->create(['team_id' => $otherTeam->id]); + + $export = $this->withHeaders(transferHeaders($this->sensitiveToken)) + ->getJson("/api/v1/servers/{$this->server->uuid}/export") + ->json(); + + $this->application->forceDelete(); + $this->server->forceDelete(); + $this->privateKey->delete(); + + data_set($export, 'projects.0.environments.0.databases', [[ + 'type' => 'NotADatabase', + 'model' => Application::class, + 'attributes' => [ + 'name' => 'injected-app', + 'git_repository' => 'git@github.com:victim/private.git', + 'git_branch' => 'main', + 'build_pack' => 'nixpacks', + 'ports_exposes' => '80', + 'private_key_id' => $otherKey->id, + ], + ]]); + + $this->withHeaders(transferHeaders($this->sensitiveToken)) + ->postJson('/api/v1/servers/import', ['bundle' => $export, 'dry_run' => false]) + ->assertStatus(422); + + expect(Application::where('name', 'injected-app')->exists())->toBeFalse(); + }); + test('rejects import when the destination team is at its cloud server limit', function () { config()->set('constants.coolify.self_hosted', false); $this->team->update(['custom_server_limit' => 1]); diff --git a/tests/Feature/DatabaseEnvironmentVariableApiTest.php b/tests/Feature/DatabaseEnvironmentVariableApiTest.php index 1f1d46483e..ac078427c0 100644 --- a/tests/Feature/DatabaseEnvironmentVariableApiTest.php +++ b/tests/Feature/DatabaseEnvironmentVariableApiTest.php @@ -194,6 +194,29 @@ describe('PATCH /api/v1/databases/{uuid}/envs', function () { expect($env->value)->toBe('new_value'); }); + test('keeps value when only a flag is sent', function () { + $database = createDatabase($this); + + $env = EnvironmentVariable::create([ + 'key' => 'SECRET', + 'value' => 'super-secret', + 'resourceable_type' => StandalonePostgresql::class, + 'resourceable_id' => $database->id, + ]); + + $this->withHeaders([ + 'Authorization' => 'Bearer '.$this->bearerToken, + 'Content-Type' => 'application/json', + ])->patchJson("/api/v1/databases/{$database->uuid}/envs", [ + 'key' => 'SECRET', + 'is_literal' => true, + ])->assertStatus(201); + + $env->refresh(); + expect($env->value)->toBe('super-secret') + ->and($env->is_literal)->toBeTruthy(); + }); + test('returns 404 when environment variable does not exist', function () { $database = createDatabase($this);