Validate environment variable keys (#10773)

app/Http/Controllers/Api/ApplicationsController.php

@@ -2876,8 +2876,12 @@ public function update_env_by_uuid(Request $request)
 
         $this->authorize('manageEnvironment', $application);
 
+        if ($request->has('key')) {
+            $request->merge(['key' => ValidationPatterns::normalizeEnvironmentVariableKey((string) $request->key)]);
+        }
+
         $validator = customApiValidator($request->all(), [
-            'key' => 'string|required',
+            'key' => ValidationPatterns::environmentVariableKeyRules(),
             'value' => 'string|nullable',
             'is_preview' => 'boolean',
             'is_literal' => 'boolean',
@@ -3100,12 +3104,18 @@ public function create_bulk_envs(Request $request)
             ], 400);
         }
         $bulk_data = collect($bulk_data)->map(function ($item) {
-            return collect($item)->only(['key', 'value', 'is_preview', 'is_literal', 'is_multiline', 'is_shown_once', 'is_runtime', 'is_buildtime', 'comment']);
+            $item = collect($item)->only(['key', 'value', 'is_preview', 'is_literal', 'is_multiline', 'is_shown_once', 'is_runtime', 'is_buildtime', 'comment']);
+
+            if ($item->has('key')) {
+                $item->put('key', ValidationPatterns::normalizeEnvironmentVariableKey((string) $item->get('key')));
+            }
+
+            return $item;
         });
         $returnedEnvs = collect();
         foreach ($bulk_data as $item) {
             $validator = customApiValidator($item, [
-                'key' => 'string|required',
+                'key' => ValidationPatterns::environmentVariableKeyRules(),
                 'value' => 'string|nullable',
                 'is_preview' => 'boolean',
                 'is_literal' => 'boolean',
@@ -3302,8 +3312,12 @@ public function create_env(Request $request)
 
         $this->authorize('manageEnvironment', $application);
 
+        if ($request->has('key')) {
+            $request->merge(['key' => ValidationPatterns::normalizeEnvironmentVariableKey((string) $request->key)]);
+        }
+
         $validator = customApiValidator($request->all(), [
-            'key' => 'string|required',
+            'key' => ValidationPatterns::environmentVariableKeyRules(),
             'value' => 'string|nullable',
             'is_preview' => 'boolean',
             'is_literal' => 'boolean',

app/Http/Controllers/Api/DatabasesController.php

@@ -3133,8 +3133,12 @@ public function update_env_by_uuid(Request $request)
 
         $this->authorize('manageEnvironment', $database);
 
+        if ($request->has('key')) {
+            $request->merge(['key' => ValidationPatterns::normalizeEnvironmentVariableKey((string) $request->key)]);
+        }
+
         $validator = customApiValidator($request->all(), [
-            'key' => 'string|required',
+            'key' => ValidationPatterns::environmentVariableKeyRules(),
             'value' => 'string|nullable',
             'is_literal' => 'boolean',
             'is_multiline' => 'boolean',
@@ -3281,8 +3285,12 @@ public function create_bulk_envs(Request $request)
 
         $updatedEnvs = collect();
         foreach ($bulk_data as $item) {
+            if (array_key_exists('key', $item)) {
+                $item['key'] = ValidationPatterns::normalizeEnvironmentVariableKey((string) $item['key']);
+            }
+
             $validator = customApiValidator($item, [
-                'key' => 'string|required',
+                'key' => ValidationPatterns::environmentVariableKeyRules(),
                 'value' => 'string|nullable',
                 'is_literal' => 'boolean',
                 'is_multiline' => 'boolean',
@@ -3399,8 +3407,12 @@ public function create_env(Request $request)
 
         $this->authorize('manageEnvironment', $database);
 
+        if ($request->has('key')) {
+            $request->merge(['key' => ValidationPatterns::normalizeEnvironmentVariableKey((string) $request->key)]);
+        }
+
         $validator = customApiValidator($request->all(), [
-            'key' => 'string|required',
+            'key' => ValidationPatterns::environmentVariableKeyRules(),
             'value' => 'string|nullable',
             'is_literal' => 'boolean',
             'is_multiline' => 'boolean',

app/Http/Controllers/Api/ServicesController.php

@@ -1251,8 +1251,12 @@ public function update_env_by_uuid(Request $request)
 
         $this->authorize('manageEnvironment', $service);
 
+        if ($request->has('key')) {
+            $request->merge(['key' => ValidationPatterns::normalizeEnvironmentVariableKey((string) $request->key)]);
+        }
+
         $validator = customApiValidator($request->all(), [
-            'key' => 'string|required',
+            'key' => ValidationPatterns::environmentVariableKeyRules(),
             'value' => 'string|nullable',
             'is_literal' => 'boolean',
             'is_multiline' => 'boolean',
@@ -1400,8 +1404,12 @@ public function create_bulk_envs(Request $request)
 
         $updatedEnvs = collect();
         foreach ($bulk_data as $item) {
+            if (array_key_exists('key', $item)) {
+                $item['key'] = ValidationPatterns::normalizeEnvironmentVariableKey((string) $item['key']);
+            }
+
             $validator = customApiValidator($item, [
-                'key' => 'string|required',
+                'key' => ValidationPatterns::environmentVariableKeyRules(),
                 'value' => 'string|nullable',
                 'is_literal' => 'boolean',
                 'is_multiline' => 'boolean',
@@ -1519,8 +1527,12 @@ public function create_env(Request $request)
 
         $this->authorize('manageEnvironment', $service);
 
+        if ($request->has('key')) {
+            $request->merge(['key' => ValidationPatterns::normalizeEnvironmentVariableKey((string) $request->key)]);
+        }
+
         $validator = customApiValidator($request->all(), [
-            'key' => 'string|required',
+            'key' => ValidationPatterns::environmentVariableKeyRules(),
             'value' => 'string|nullable',
             'is_literal' => 'boolean',
             'is_multiline' => 'boolean',

app/Support/ValidationPatterns.php

@@ -95,9 +95,9 @@ class ValidationPatterns
 
     /**
      * Pattern for Docker-compatible environment variable keys.
-     * Docker environment entries are KEY=value strings, so keys must be non-empty and cannot contain '=' or NUL.
+     * Environment variable keys are later interpolated into shell commands as Docker build args, so only shell-safe identifier characters are allowed.
      */
-    public const ENVIRONMENT_VARIABLE_KEY_PATTERN = '/\A[^=\x00]+\z/u';
+    public const ENVIRONMENT_VARIABLE_KEY_PATTERN = '/\A[A-Za-z_][A-Za-z0-9_.]*\z/u';
 
     /**
      * Pattern for SQL-safe unquoted database identifiers (usernames, database names).
@@ -164,7 +164,7 @@ public static function environmentVariableKeyRules(bool $required = true, int $m
     public static function environmentVariableKeyMessages(string $field = 'key', string $label = 'key'): array
     {
         return [
-            "{$field}.regex" => "The {$label} must be a non-empty Docker-compatible environment variable key and cannot contain '=' or NUL characters.",
+            "{$field}.regex" => "The {$label} must start with a letter or underscore and may only contain letters, numbers, underscores, and dots.",
             "{$field}.max" => "The {$label} may not be greater than :max characters.",
         ];
     }

bootstrap/helpers/docker.php

@@ -1363,7 +1363,7 @@ function generateDockerBuildArgs($variables): Collection
         $key = is_array($var) ? data_get($var, 'key') : $var->key;
 
         // Only return the key - Docker will get the value from the environment
-        return "--build-arg {$key}";
+        return '--build-arg '.escapeshellarg((string) $key);
     });
 }
 

tests/Feature/Api/EnvironmentVariableUpdateApiTest.php

@@ -15,7 +15,7 @@
 uses(RefreshDatabase::class);
 
 beforeEach(function () {
-    InstanceSettings::updateOrCreate(['id' => 0]);
+    InstanceSettings::unguarded(fn () => InstanceSettings::firstOrCreate(['id' => 0], ['is_api_enabled' => true]));
 
     $this->team = Team::factory()->create();
     $this->user = User::factory()->create();
@@ -252,3 +252,94 @@
         $response->assertJsonFragment(['uuid' => ['This field is not allowed.']]);
     });
 });
+
+describe('environment variable key validation for app and service APIs', function () {
+    test('rejects invalid service environment variable keys on create update and bulk', 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,
+        ]);
+
+        EnvironmentVariable::create([
+            'key' => 'SAFE_KEY',
+            'value' => 'old-value',
+            'resourceable_type' => Service::class,
+            'resourceable_id' => $service->id,
+            'is_preview' => false,
+        ]);
+
+        $headers = [
+            'Authorization' => 'Bearer '.$this->bearerToken,
+            'Content-Type' => 'application/json',
+        ];
+
+        $this->withHeaders($headers)
+            ->postJson("/api/v1/services/{$service->uuid}/envs", [
+                'key' => 'BAD$(id)',
+                'value' => '1',
+            ])
+            ->assertStatus(422);
+
+        $this->withHeaders($headers)
+            ->patchJson("/api/v1/services/{$service->uuid}/envs", [
+                'key' => 'BAD$(id)',
+                'value' => '1',
+            ])
+            ->assertStatus(422);
+
+        $this->withHeaders($headers)
+            ->patchJson("/api/v1/services/{$service->uuid}/envs/bulk", [
+                'data' => [[
+                    'key' => 'BAD$(id)',
+                    'value' => '1',
+                ]],
+            ])
+            ->assertStatus(422);
+    });
+
+    test('rejects invalid application environment variable keys on create update and bulk', function () {
+        $application = Application::factory()->create([
+            'environment_id' => $this->environment->id,
+            'destination_id' => $this->destination->id,
+            'destination_type' => $this->destination->getMorphClass(),
+        ]);
+
+        EnvironmentVariable::create([
+            'key' => 'SAFE_KEY',
+            'value' => 'old-value',
+            'resourceable_type' => Application::class,
+            'resourceable_id' => $application->id,
+            'is_preview' => false,
+        ]);
+
+        $headers = [
+            'Authorization' => 'Bearer '.$this->bearerToken,
+            'Content-Type' => 'application/json',
+        ];
+
+        $this->withHeaders($headers)
+            ->postJson("/api/v1/applications/{$application->uuid}/envs", [
+                'key' => 'BAD$(id)',
+                'value' => '1',
+            ])
+            ->assertStatus(422);
+
+        $this->withHeaders($headers)
+            ->patchJson("/api/v1/applications/{$application->uuid}/envs", [
+                'key' => 'BAD$(id)',
+                'value' => '1',
+            ])
+            ->assertStatus(422);
+
+        $this->withHeaders($headers)
+            ->patchJson("/api/v1/applications/{$application->uuid}/envs/bulk", [
+                'data' => [[
+                    'key' => 'BAD$(id)',
+                    'value' => '1',
+                ]],
+            ])
+            ->assertStatus(422);
+    });
+});

tests/Feature/DatabaseEnvironmentVariableApiTest.php

@@ -14,7 +14,7 @@
 uses(RefreshDatabase::class);
 
 beforeEach(function () {
-    InstanceSettings::updateOrCreate(['id' => 0]);
+    InstanceSettings::unguarded(fn () => InstanceSettings::firstOrCreate(['id' => 0], ['is_api_enabled' => true]));
 
     $this->team = Team::factory()->create();
     $this->user = User::factory()->create();
@@ -344,3 +344,44 @@ function createDatabase($context): StandalonePostgresql
         $response->assertStatus(404);
     });
 });
+
+describe('environment variable key validation for database APIs', function () {
+    test('rejects invalid database environment variable keys on create update and bulk', function () {
+        $database = createDatabase($this);
+
+        EnvironmentVariable::create([
+            'key' => 'SAFE_KEY',
+            'value' => 'old-value',
+            'resourceable_type' => StandalonePostgresql::class,
+            'resourceable_id' => $database->id,
+        ]);
+
+        $headers = [
+            'Authorization' => 'Bearer '.$this->bearerToken,
+            'Content-Type' => 'application/json',
+        ];
+
+        $this->withHeaders($headers)
+            ->postJson("/api/v1/databases/{$database->uuid}/envs", [
+                'key' => 'BAD$(id)',
+                'value' => '1',
+            ])
+            ->assertStatus(422);
+
+        $this->withHeaders($headers)
+            ->patchJson("/api/v1/databases/{$database->uuid}/envs", [
+                'key' => 'BAD$(id)',
+                'value' => '1',
+            ])
+            ->assertStatus(422);
+
+        $this->withHeaders($headers)
+            ->patchJson("/api/v1/databases/{$database->uuid}/envs/bulk", [
+                'data' => [[
+                    'key' => 'BAD$(id)',
+                    'value' => '1',
+                ]],
+            ])
+            ->assertStatus(422);
+    });
+});

tests/Feature/EnvironmentVariable/MultilineEnvironmentVariableTest.php

@@ -9,8 +9,8 @@
     $buildArgs = generateDockerBuildArgs($variables);
 
     // Docker gets values from the environment, so only keys should be in build args
-    expect($buildArgs->first())->toBe('--build-arg SSH_PRIVATE_KEY');
+    expect($buildArgs->first())->toBe("--build-arg 'SSH_PRIVATE_KEY'");
-    expect($buildArgs->last())->toBe('--build-arg REGULAR_VAR');
+    expect($buildArgs->last())->toBe("--build-arg 'REGULAR_VAR'");
 });
 
 test('generateDockerBuildArgs works with collection of objects', function () {
@@ -22,8 +22,8 @@
     $buildArgs = generateDockerBuildArgs($variables);
     expect($buildArgs)->toHaveCount(2);
     expect($buildArgs->values()->toArray())->toBe([
-        '--build-arg VAR1',
+        "--build-arg 'VAR1'",
-        '--build-arg VAR2',
+        "--build-arg 'VAR2'",
     ]);
 });
 
@@ -38,7 +38,7 @@
     // The collection must be imploded to a string for command interpolation
     // This was the bug: Collection was interpolated as JSON instead of a space-separated string
     $argsString = $buildArgs->implode(' ');
-    expect($argsString)->toBe('--build-arg COOLIFY_URL --build-arg COOLIFY_BRANCH');
+    expect($argsString)->toBe("--build-arg 'COOLIFY_URL' --build-arg 'COOLIFY_BRANCH'");
 
     // Verify it does NOT produce JSON when cast to string
     expect($argsString)->not->toContain('{');
@@ -53,7 +53,7 @@
     $buildArgs = generateDockerBuildArgs($variables);
     $arg = $buildArgs->first();
 
-    expect($arg)->toBe('--build-arg NO_FLAG_VAR');
+    expect($arg)->toBe("--build-arg 'NO_FLAG_VAR'");
 });
 
 test('generateDockerEnvFlags produces correct format', function () {
@@ -81,3 +81,17 @@
     expect($envFlags)->toContain('-e VAR1=');
     expect($envFlags)->toContain('-e VAR2="');
 });
+
+test('generateDockerBuildArgs escapes legacy keys', function () {
+    $variables = [
+        ['key' => 'BAD$(id)', 'value' => '1'],
+        ['key' => "BAD'KEY", 'value' => '1'],
+    ];
+
+    $buildArgs = generateDockerBuildArgs($variables);
+
+    expect($buildArgs->values()->toArray())->toBe([
+        "--build-arg 'BAD$(id)'",
+        "--build-arg 'BAD'\''KEY'",
+    ]);
+});

tests/Unit/ValidationPatternsTest.php

@@ -132,24 +132,31 @@
         ->not->toContain('required');
 });
 
-it('accepts Docker-compatible environment variable keys', function (string $key) {
+it('accepts shell-safe environment variable keys', function (string $key) {
     expect(ValidationPatterns::isValidEnvironmentVariableKey($key))->toBeTrue();
 })->with([
     'letters' => 'APP_ENV',
     'leading underscore' => '_TOKEN',
     'railpack control variable' => 'RAILPACK_NODE_VERSION',
     'digits after first character' => 'NODE_VERSION_20',
-    'starts with digit' => '1BAD',
+    'lowercase' => 'node_version',
-    'hyphen' => 'BAD-KEY',
+    'dot notation' => 'node.name',
-    'dot' => 'node.name',
     'uppercase dots' => 'XPACK.SECURITY.ENABLED',
-    'semicolon' => 'BAD;KEY',
-    'space' => 'BAD KEY',
 ]);
 
-it('rejects environment variable keys Docker cannot represent', function (string $key) {
+it('rejects invalid environment variable keys', function (string $key) {
     expect(ValidationPatterns::isValidEnvironmentVariableKey($key))->toBeFalse();
 })->with([
+    'starts with digit' => '1BAD',
+    'hyphen' => 'BAD-KEY',
+    'semicolon' => 'BAD;KEY',
+    'space' => 'BAD KEY',
+    'command substitution' => 'BAD$(id)',
+    'backticks' => 'BAD`id`',
+    'pipe' => 'BAD|id',
+    'ampersand' => 'BAD&id',
+    'newline' => 'BAD
+KEY',
     'equals' => 'BAD=KEY',
     'empty' => '',
 ]);
@@ -164,7 +171,7 @@
 });
 
 it('normalizes environment variable keys by trimming surrounding whitespace', function () {
-    expect(ValidationPatterns::normalizeEnvironmentVariableKey(' node.name '))->toBe('node.name');
+    expect(ValidationPatterns::normalizeEnvironmentVariableKey(' APP_ENV '))->toBe('APP_ENV');
 });
 
 it('normalizes environment variable keys before model validation', function () {