fix(api): preserve omitted env var values and flags on PATCH (#12240)

This commit is contained in:
Andras Bacsai authored and GitHub committed 2026-10-27 17:00:56 +01:00
1 parent 69bc72a3b5
commit e78659dfed
5 files changed
+220

No files matched your search

@@ -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']);
}
@@ -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']);
}
@@ -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 = [
@@ -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]);
@@ -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);