fix(deployments): validate environment variable names used in Docker commands

Reject names that are not portable identifiers before a deployment starts
and when Docker flags are built. Quote the full KEY=value assignment for
docker run -e flags, and declare generated Dockerfile ARGs as keys only.

app/Jobs/ApplicationDeploymentJob.php

@@ -311,6 +311,13 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue
             return;
         }
 
+        try {
+            $this->validateDeploymentEnvironmentVariableKeys();
+        } catch (Exception $e) {
+            $this->fail($e);
+            throw $e;
+        }
+
         $this->application_deployment_queue->update([
             'status' => ApplicationDeploymentStatus::IN_PROGRESS->value,
             'horizon_job_worker' => gethostname(),
@@ -2049,6 +2056,17 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue
         }
     }
 
+    private function validateDeploymentEnvironmentVariableKeys(): void
+    {
+        $environmentVariables = $this->pull_request_id === 0
+            ? $this->application->environment_variables()->get(['key'])
+            : $this->application->environment_variables_preview()->get(['key']);
+
+        foreach ($environmentVariables as $environmentVariable) {
+            $this->validatedBuildtimeEnvironmentVariableKey((string) $environmentVariable->key, 'the deployment environment');
+        }
+    }
+
     private function logInvalidBuildtimeEnvironmentVariableKey(string $key, string $origin): void
     {
         $displayKey = ValidationPatterns::displayShellEnvironmentVariableKey($key);
@@ -2102,6 +2120,7 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue
                 $this->application_deployment_queue->addLogEntry('Creating build-time .env file in /artifacts (outside Docker context).', hidden: true);
                 $this->execute_remote_command([
                     executeInDocker($this->deployment_uuid, "echo '$envs_base64' | base64 -d | tee ".self::BUILD_TIME_ENV_PATH.' > /dev/null'),
+                    'skip_command_log' => true,
                 ]);
 
                 if (isDev()) {
@@ -2147,6 +2166,7 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue
             $contents_base64 = base64_encode($contents);
             $this->execute_remote_command([
                 executeInDocker($this->deployment_uuid, "echo '$contents_base64' | base64 -d | tee {$path} > /dev/null"),
+                'skip_command_log' => true,
             ]);
         }
 
@@ -3024,6 +3044,8 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue
 
         return 'env '.$variables
             ->map(function ($value, $key) {
+                $key = $this->validatedBuildtimeEnvironmentVariableKey((string) $key, 'the Railpack environment');
+
                 return escapeShellValue("{$key}={$value}");
             })
             ->implode(' ').' ';
@@ -3037,6 +3059,8 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue
 
         return ' '.$variables
             ->map(function ($value, $key) {
+                $key = $this->validatedBuildtimeEnvironmentVariableKey((string) $key, 'the Railpack environment');
+
                 return '--secret '.escapeShellValue("id={$key},env={$key}");
             })
             ->implode(' ');
@@ -3951,7 +3975,8 @@ COPY ./nginx.conf /etc/nginx/conf.d/default.conf");
             // Traditional build args approach - generate COOLIFY_ variables locally
             $coolify_envs = $this->generate_coolify_env_variables(forBuildTime: true);
             $coolify_envs->each(function ($value, $key) {
-                $this->build_args->push("--build-arg '{$key}'");
+                $key = $this->validatedBuildtimeEnvironmentVariableKey((string) $key, 'the Coolify build environment');
+                $this->build_args->push('--build-arg '.escapeshellarg($key));
             });
         }
 
@@ -4506,7 +4531,7 @@ COPY ./nginx.conf /etc/nginx/conf.d/default.conf");
             $this->build_args = generateDockerBuildArgs($vars_with_metadata);
 
             if ($secrets_hash) {
-                $this->build_args->push("--build-arg COOLIFY_BUILD_SECRETS_HASH={$secrets_hash}");
+                $this->build_args->push('--build-arg '.escapeshellarg("COOLIFY_BUILD_SECRETS_HASH={$secrets_hash}"));
             }
         }
     }
@@ -4543,6 +4568,7 @@ COPY ./nginx.conf /etc/nginx/conf.d/default.conf");
 
         // Map to simple array format for the helper function
         $vars_array = $variables->map(function ($value, $key) use ($env_vars) {
+            $key = $this->validatedBuildtimeEnvironmentVariableKey((string) $key, 'the build secret environment');
             $env = $env_vars->firstWhere('key', $key);
 
             return [
@@ -4553,7 +4579,7 @@ COPY ./nginx.conf /etc/nginx/conf.d/default.conf");
         });
 
         $env_flags = generateDockerEnvFlags($vars_array);
-        $env_flags .= " -e COOLIFY_BUILD_SECRETS_HASH={$secrets_hash}";
+        $env_flags .= ' -e '.escapeshellarg("COOLIFY_BUILD_SECRETS_HASH={$secrets_hash}");
 
         return $env_flags;
     }
@@ -4568,7 +4594,9 @@ COPY ./nginx.conf /etc/nginx/conf.d/default.conf");
 
         $this->build_secrets = $variables
             ->map(function ($value, $key) {
-                return "--secret id={$key},env={$key}";
+                $key = $this->validatedBuildtimeEnvironmentVariableKey((string) $key, 'the build secret environment');
+
+                return '--secret '.escapeshellarg("id={$key},env={$key}");
             })
             ->implode(' ');
 
@@ -4659,11 +4687,8 @@ COPY ./nginx.conf /etc/nginx/conf.d/default.conf");
                 ->where('is_buildtime', true)
                 ->get();
             foreach ($envs as $env) {
-                if (data_get($env, 'is_multiline') === true) {
-                    $argsToInsert->push("ARG {$env->key}");
-                } else {
-                    $argsToInsert->push("ARG {$env->key}=".escapeBashEnvValue($this->resolve_environment_variable_raw($env)));
-                }
+                $key = $this->validatedBuildtimeEnvironmentVariableKey((string) $env->key, 'the generated Dockerfile');
+                $argsToInsert->push("ARG {$key}");
             }
             // Add Coolify variables as ARGs
             if ($this->coolify_variables) {
@@ -4681,11 +4706,8 @@ COPY ./nginx.conf /etc/nginx/conf.d/default.conf");
                 ->where('is_buildtime', true)
                 ->get();
             foreach ($envs as $env) {
-                if (data_get($env, 'is_multiline') === true) {
-                    $argsToInsert->push("ARG {$env->key}");
-                } else {
-                    $argsToInsert->push("ARG {$env->key}=".escapeBashEnvValue($this->resolve_environment_variable_raw($env)));
-                }
+                $key = $this->validatedBuildtimeEnvironmentVariableKey((string) $env->key, 'the generated Dockerfile');
+                $argsToInsert->push("ARG {$key}");
             }
             // Add Coolify variables as ARGs
             if ($this->coolify_variables) {
@@ -4805,7 +4827,11 @@ COPY ./nginx.conf /etc/nginx/conf.d/default.conf");
         }
 
         // Generate mount strings for all secrets
-        $mountStrings = $variables->map(fn ($value, $key) => "--mount=type=secret,id={$key},env={$key}")->implode(' ');
+        $mountStrings = $variables->map(function ($value, $key) {
+            $key = $this->validatedBuildtimeEnvironmentVariableKey((string) $key, 'the generated Dockerfile');
+
+            return "--mount=type=secret,id={$key},env={$key}";
+        })->implode(' ');
 
         // Add mount for the secrets hash to ensure cache invalidation
         $mountStrings .= ' --mount=type=secret,id=COOLIFY_BUILD_SECRETS_HASH,env=COOLIFY_BUILD_SECRETS_HASH';
@@ -4917,6 +4943,7 @@ COPY ./nginx.conf /etc/nginx/conf.d/default.conf");
 
             $argsToAdd = collect([]);
             foreach ($variables as $key => $value) {
+                $key = $this->validatedBuildtimeEnvironmentVariableKey((string) $key, 'the generated Dockerfile');
                 $argsToAdd->push("ARG {$key}");
             }
 
@@ -5068,6 +5095,7 @@ COPY ./nginx.conf /etc/nginx/conf.d/default.conf");
 
         $secrets = [];
         foreach ($variables as $key => $value) {
+            $key = $this->validatedBuildtimeEnvironmentVariableKey((string) $key, 'the Compose build secret environment');
             $secrets[$key] = [
                 'environment' => $key,
             ];
@@ -5084,7 +5112,7 @@ COPY ./nginx.conf /etc/nginx/conf.d/default.conf");
                 if (! isset($service['build']['secrets'])) {
                     $service['build']['secrets'] = [];
                 }
-                foreach ($variables as $key => $value) {
+                foreach (array_keys($secrets) as $key) {
                     if (! in_array($key, $service['build']['secrets'])) {
                         $service['build']['secrets'][] = $key;
                     }

bootstrap/helpers/docker.php

@@ -1713,6 +1713,10 @@ function generateDockerBuildArgs($variables): Collection
     return $variables->map(function ($var) {
         $key = is_array($var) ? data_get($var, 'key') : $var->key;
 
+        if (! ValidationPatterns::isValidEnvironmentVariableKey((string) $key)) {
+            throw new InvalidArgumentException('Invalid environment variable key.');
+        }
+
         // Only return the key - Docker will get the value from the environment
         return '--build-arg '.escapeshellarg((string) $key);
     });
@@ -1732,19 +1736,17 @@ function generateDockerEnvFlags($variables): string
         ->map(function ($var) {
             $key = is_array($var) ? data_get($var, 'key') : $var->key;
             $value = is_array($var) ? data_get($var, 'value') : $var->value;
-            $isMultiline = is_array($var) ? data_get($var, 'is_multiline', false) : ($var->is_multiline ?? false);
 
-            if ($isMultiline) {
-                // For multiline variables, strip surrounding quotes and escape for bash
-                $raw_value = trim($value, "'");
-                $escaped_value = str_replace(['\\', '"', '$', '`'], ['\\\\', '\\"', '\\$', '\\`'], $raw_value);
-
-                return "-e {$key}=\"{$escaped_value}\"";
+            if (! ValidationPatterns::isValidEnvironmentVariableKey((string) $key)) {
+                throw new InvalidArgumentException('Invalid environment variable key.');
             }
 
-            $escaped_value = escapeshellarg($value);
+            $isMultiline = is_array($var) ? data_get($var, 'is_multiline', false) : ($var->is_multiline ?? false);
+            if ($isMultiline) {
+                $value = trim($value, "'");
+            }
 
-            return "-e {$key}={$escaped_value}";
+            return '-e '.escapeshellarg("{$key}={$value}");
         })
         ->implode(' ');
 }

tests/Feature/ApplicationDeploymentControlVarFilteringTest.php

@@ -12,6 +12,7 @@ use App\Models\Server;
 use App\Models\Team;
 use Illuminate\Foundation\Testing\RefreshDatabase;
 use Illuminate\Support\Collection;
+use Illuminate\Support\Facades\DB;
 use Symfony\Component\Process\Process;
 
 uses(RefreshDatabase::class);
@@ -439,7 +440,7 @@ it('uses BuildKit secrets for dotted Nixpacks variables instead of invalid Docke
     invokeDeploymentJobMethod($job, $reflection, 'generate_build_env_variables');
 
     expect(readDeploymentJobProperty($job, $reflection, 'dockerSecretsSupported'))->toBeTrue();
-    expect(readDeploymentJobProperty($job, $reflection, 'build_secrets'))->toContain('--secret id=X.VALUE,env=X.VALUE');
+    expect(readDeploymentJobProperty($job, $reflection, 'build_secrets'))->toContain("--secret 'id=X.VALUE,env=X.VALUE'");
 
     invokeDeploymentJobMethod($job, $reflection, 'modify_dockerfile_for_secrets', '/artifacts/test-app/.nixpacks/Dockerfile');
 
@@ -810,6 +811,7 @@ it('filters buildpack control vars from dockerfile arg injection', function () {
     ]);
 
     [$job, $reflection] = makeControlVarFilteringJob($application, $server, [
+        'remote_secrets_cache' => [],
         'saved_outputs' => [
             'dockerfile' => "FROM php:8.4-cli\nRUN php -v",
         ],
@@ -817,12 +819,78 @@ it('filters buildpack control vars from dockerfile arg injection', function () {
 
     invokeDeploymentJobMethod($job, $reflection, 'add_build_env_variables_to_dockerfile');
 
-    expect($job->writtenDockerfile)->toContain('ARG APP_ENV=production');
+    expect($job->writtenDockerfile)->toContain('ARG APP_ENV');
     expect($job->writtenDockerfile)->toContain('ARG COOLIFY_BUILD_SECRETS_HASH=');
     expect($job->writtenDockerfile)->not->toContain('ARG NIXPACKS_NODE_VERSION=');
     expect($job->writtenDockerfile)->not->toContain('ARG RAILPACK_NODE_VERSION=');
 });
 
+it('does not write environment values into generated Dockerfile ARG declarations', function () {
+    [$application, $server] = makeDeploymentControlVarFixture();
+    createApplicationEnvironmentVariable($application, [
+        'key' => 'SAFE_KEY',
+        'value' => "value\nRUN touch /tmp/injected",
+        'is_runtime' => false,
+        'is_buildtime' => true,
+    ]);
+
+    [$job, $reflection] = makeControlVarFilteringJob($application, $server, [
+        'remote_secrets_cache' => [],
+        'saved_outputs' => ['dockerfile' => 'FROM alpine'],
+    ]);
+
+    invokeDeploymentJobMethod($job, $reflection, 'add_build_env_variables_to_dockerfile');
+
+    expect($job->writtenDockerfile)
+        ->toContain('ARG SAFE_KEY')
+        ->not->toContain('RUN touch /tmp/injected')
+        ->not->toContain('value');
+});
+
+it('rejects unsafe keys before generating Railpack secret flags', function () {
+    [$application, $server] = makeDeploymentControlVarFixture();
+    [$job, $reflection] = makeControlVarFilteringJob($application, $server);
+
+    expect(fn () => invokeDeploymentJobMethod(
+        $job,
+        $reflection,
+        'railpack_build_secret_flags',
+        collect(['BAD$(id)' => 'x']),
+    ))->toThrow(DeploymentException::class, 'Invalid environment variable name from the Railpack environment');
+
+    expect($job->recordedCommands)->toBeEmpty();
+});
+
+it('rejects unsafe legacy keys before generating BuildKit secret flags', function () {
+    [$application, $server] = makeDeploymentControlVarFixture();
+    [$job, $reflection] = makeControlVarFilteringJob($application, $server);
+
+    expect(fn () => invokeDeploymentJobMethod(
+        $job,
+        $reflection,
+        'generate_build_secrets',
+        collect(['BAD$(id)' => 'secret']),
+    ))->toThrow(DeploymentException::class, 'Invalid environment variable name from the build secret environment');
+
+    expect($job->recordedCommands)->toBeEmpty();
+});
+
+it('rejects an unsafe stored key before running a deployment command', function () {
+    [$application, $server] = makeDeploymentControlVarFixture();
+    $environmentVariable = createApplicationEnvironmentVariable($application, [
+        'key' => 'SAFE_KEY',
+        'value' => 'secret',
+    ]);
+    DB::table('environment_variables')->where('id', $environmentVariable->id)->update(['key' => 'BAD$(id)']);
+
+    [$job, $reflection] = makeControlVarFilteringJob($application->fresh(), $server);
+
+    expect(fn () => invokeDeploymentJobMethod($job, $reflection, 'validateDeploymentEnvironmentVariableKeys'))
+        ->toThrow(DeploymentException::class, 'Invalid environment variable name from the deployment environment');
+
+    expect($job->recordedCommands)->toBeEmpty();
+});
+
 it('injects raw escaped remote secrets into Dockerfile args and hashes the same values', function (int $pullRequestId, bool $isPreview) {
     [$application, $server] = makeDeploymentControlVarFixture();
 
@@ -854,7 +922,8 @@ it('injects raw escaped remote secrets into Dockerfile args and hashes the same
     );
 
     expect($job->writtenDockerfile)
-        ->toContain("ARG SECRET_TOKEN={$escapedSecret}")
+        ->toContain('ARG SECRET_TOKEN')
+        ->not->toContain($secret)
         ->toContain("ARG COOLIFY_BUILD_SECRETS_HASH={$expectedHash}")
         ->not->toContain('$$');
 })->with([

tests/Feature/EnvironmentVariable/MultilineEnvironmentVariableTest.php

@@ -64,8 +64,8 @@ test('generateDockerEnvFlags produces correct format', function () {
 
     $envFlags = generateDockerEnvFlags($variables);
 
-    expect($envFlags)->toContain('-e NORMAL_VAR=');
-    expect($envFlags)->toContain('-e MULTILINE_VAR="');
+    expect($envFlags)->toContain("-e 'NORMAL_VAR=value'");
+    expect($envFlags)->toContain("-e 'MULTILINE_VAR=line1");
     expect($envFlags)->toContain('line1');
     expect($envFlags)->toContain('line2');
 });
@@ -78,20 +78,41 @@ test('generateDockerEnvFlags works with collection input', function () {
 
     $envFlags = generateDockerEnvFlags($variables);
     expect($envFlags)->toBeString();
-    expect($envFlags)->toContain('-e VAR1=');
-    expect($envFlags)->toContain('-e VAR2="');
+    expect($envFlags)->toContain("-e 'VAR1=value1'");
+    expect($envFlags)->toContain("-e 'VAR2=multiline");
 });
 
-test('generateDockerBuildArgs escapes legacy keys', function () {
-    $variables = [
-        ['key' => 'BAD$(id)', 'value' => '1'],
-        ['key' => "BAD'KEY", 'value' => '1'],
-    ];
+test('docker argument helpers reject unsafe legacy keys', function (string $key) {
+    $variables = [['key' => $key, 'value' => '1']];
 
-    $buildArgs = generateDockerBuildArgs($variables);
+    expect(fn () => generateDockerBuildArgs($variables))->toThrow(InvalidArgumentException::class);
+    expect(fn () => generateDockerEnvFlags($variables))->toThrow(InvalidArgumentException::class);
+})->with([
+    'semicolon' => 'BAD;id',
+    'command substitution' => 'BAD$(id)',
+    'backticks' => 'BAD`id`',
+    'pipe' => 'BAD|id',
+    'logical operator' => 'BAD&&id',
+    'single quote' => "BAD'KEY",
+    'double quote' => 'BAD"KEY',
+    'backslash' => 'BAD\\KEY',
+    'space' => 'BAD KEY',
+    'newline' => "BAD\nKEY",
+    'control character' => "BAD\x1BKEY",
+    'equals sign' => 'BAD=KEY',
+    'option prefix' => '--BAD',
+    'parameter expansion' => 'BAD${PATH}',
+    'braces' => 'BAD{KEY}',
+    'unicode lookalike' => 'BＡD',
+    'empty' => '',
+    'leading digit' => '1BAD',
+]);
 
-    expect($buildArgs->values()->toArray())->toBe([
-        "--build-arg 'BAD$(id)'",
-        "--build-arg 'BAD'\''KEY'",
+test('generateDockerEnvFlags quotes the complete assignment', function () {
+    $flags = generateDockerEnvFlags([
+        ['key' => 'NORMAL_VAR', 'value' => 'value with spaces', 'is_multiline' => false],
+        ['key' => 'MULTILINE_VAR', 'value' => "line1\nline2", 'is_multiline' => true],
     ]);
+
+    expect($flags)->toBe("-e 'NORMAL_VAR=value with spaces' -e 'MULTILINE_VAR=line1\nline2'");
 });

tests/Feature/EnvironmentVariableKeyValidationTest.php

@@ -21,12 +21,24 @@ it('allows Docker-compatible environment variable keys in the add form', functio
             return data_get($data, 'key') === $key || data_get($data, '0.key') === $key;
         });
 })->with([
-    'starts with digit' => '1BAD',
-    'hyphen' => 'BAD-KEY',
+    'letters and underscore' => 'APP_ENV',
     'dot' => 'node.name',
     'uppercase dots' => 'XPACK.SECURITY.ENABLED',
 ]);
 
+it('rejects environment variable keys that are not portable identifiers in the add form', function (string $key) {
+    Livewire::test(Add::class)
+        ->set('key', $key)
+        ->set('value', 'value')
+        ->call('submit')
+        ->assertHasErrors(['key' => 'regex']);
+})->with([
+    'starts with digit' => '1BAD',
+    'hyphen' => 'BAD-KEY',
+    'command substitution' => 'BAD$(id)',
+    'semicolon' => 'BAD;KEY',
+]);
+
 it('trims surrounding whitespace in environment variable keys in the add form', function () {
     Livewire::test(Add::class)
         ->set('key', ' node.name ')

tests/Unit/EnvironmentVariableBuildpackControlTest.php

@@ -50,25 +50,29 @@ it('allows Docker-compatible environment variable keys on the model', function (
 
     expect($env->key)->toBe($key);
 })->with([
-    'starts with digit' => '1BAD',
-    'hyphen' => 'BAD-KEY',
+    'letters and underscore' => 'APP_ENV',
     'dot' => 'node.name',
     'uppercase dots' => 'XPACK.SECURITY.ENABLED',
-    'semicolon' => 'BAD;KEY',
 ]);
 
-it('rejects environment variable keys Docker cannot represent on the model', function () {
+it('rejects environment variable keys Docker cannot represent on the model', function (string $key) {
     $env = new EnvironmentVariable;
 
-    expect(function () use ($env) {
-        $env->key = 'BAD=KEY';
-    })->toThrow(InvalidArgumentException::class, 'Docker-compatible');
-});
+    expect(function () use ($env, $key) {
+        $env->key = $key;
+    })->toThrow(InvalidArgumentException::class, 'must start with a letter or underscore');
+})->with([
+    'equals' => 'BAD=KEY',
+    'starts with digit' => '1BAD',
+    'hyphen' => 'BAD-KEY',
+    'semicolon' => 'BAD;KEY',
+    'command substitution' => 'BAD$(id)',
+]);
 
 it('rejects shared environment variable keys Docker cannot represent on the model', function () {
     $env = new SharedEnvironmentVariable;
 
     expect(function () use ($env) {
         $env->key = 'BAD=KEY';
-    })->toThrow(InvalidArgumentException::class, 'Docker-compatible');
+    })->toThrow(InvalidArgumentException::class, 'must start with a letter or underscore');
 });