Validate build-time environment variable names before writing the build .env file (#11575)

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

tests/Feature/ApplicationDeploymentControlVarFilteringTest.php

@@ -1,5 +1,6 @@
 <?php
 
+use App\Exceptions\DeploymentException;
 use App\Jobs\ApplicationDeploymentJob;
 use App\Models\Application;
 use App\Models\ApplicationDeploymentQueue;
@@ -11,6 +12,7 @@ use App\Models\Server;
 use App\Models\Team;
 use Illuminate\Foundation\Testing\RefreshDatabase;
 use Illuminate\Support\Collection;
+use Symfony\Component\Process\Process;
 
 uses(RefreshDatabase::class);
 
@@ -18,6 +20,10 @@ class TestableControlVarFilteringDeploymentJob extends ApplicationDeploymentJob
 {
     public array $recordedCommands = [];
 
+    public array $recordedLogEntries = [];
+
+    public array $writtenArtifacts = [];
+
     public ?string $writtenDockerfile = null;
 
     public function __construct() {}
@@ -36,6 +42,10 @@ class TestableControlVarFilteringDeploymentJob extends ApplicationDeploymentJob
             if (preg_match('/echo .*?([A-Za-z0-9+\\/=]{16,}).*?\\| base64 -d \\| tee \\/artifacts\\/test-app\\/Dockerfile > \\/dev\\/null/', $commandString, $matches) === 1) {
                 $this->writtenDockerfile = base64_decode($matches[1]) ?: null;
             }
+
+            if (preg_match('~echo .*?([A-Za-z0-9+/=]{8,}).*?\\| base64 -d \\| tee (/artifacts/[^ ]+) > /dev/null~', $commandString, $matches) === 1) {
+                $this->writtenArtifacts[$matches[2]] = base64_decode($matches[1]);
+            }
         }
     }
 }
@@ -92,7 +102,11 @@ function makeControlVarFilteringJob(Application $application, Server $server, ar
     $reflection = new ReflectionClass(ApplicationDeploymentJob::class);
 
     $queue = Mockery::mock(ApplicationDeploymentQueue::class);
-    $queue->shouldReceive('addLogEntry')->andReturnNull();
+    $queue->shouldReceive('addLogEntry')->andReturnUsing(function (string $message, string $type = 'stdout', bool $hidden = false) use ($job) {
+        $job->recordedLogEntries[] = $message;
+
+        return null;
+    });
 
     $properties = [
         'application' => $application->fresh(),
@@ -130,12 +144,12 @@ function makeControlVarFilteringJob(Application $application, Server $server, ar
     return [$job, $reflection];
 }
 
-function invokeDeploymentJobMethod(object $job, ReflectionClass $reflection, string $method): mixed
+function invokeDeploymentJobMethod(object $job, ReflectionClass $reflection, string $method, mixed ...$arguments): mixed
 {
     $reflectionMethod = $reflection->getMethod($method);
     $reflectionMethod->setAccessible(true);
 
-    return $reflectionMethod->invoke($job);
+    return $reflectionMethod->invoke($job, ...$arguments);
 }
 
 function readDeploymentJobProperty(object $job, ReflectionClass $reflection, string $property): mixed
@@ -205,6 +219,345 @@ it('filters buildpack control vars from preview build-time env files', function
     expect($buildtimeEnvs->contains(fn (string $env) => str($env)->startsWith('RAILPACK_NODE_VERSION=')))->toBeFalse();
 });
 
+it('rejects unsafe Nixpacks plan variable keys before writing the build-time env file', function (string $key) {
+    [$application, $server] = makeDeploymentControlVarFixture([
+        'build_pack' => 'nixpacks',
+    ]);
+
+    [$job, $reflection] = makeControlVarFilteringJob($application, $server, [
+        'nixpacks_plan_json' => collect([
+            'variables' => [
+                $key => 'value',
+            ],
+        ]),
+    ]);
+
+    expect(fn () => invokeDeploymentJobMethod($job, $reflection, 'generate_buildtime_environment_variables'))
+        ->toThrow(DeploymentException::class);
+})->with([
+    'command substitution' => 'X$(id)',
+    'backticks' => 'X`id`',
+    'newline' => "X\nid",
+    'shell assignment' => 'X=value',
+    'semicolon' => 'X;id',
+    'pipe' => 'X|id',
+    'ampersand' => 'X&id',
+    'leading dollar' => '$(id)',
+    'command substitution with arguments' => 'X$(docker run --rm -v /:/mnt alpine true)',
+]);
+
+it('keeps persisted dotted user build-time variable keys', function () {
+    [$application, $server] = makeDeploymentControlVarFixture();
+
+    createApplicationEnvironmentVariable($application, [
+        'key' => 'X.VALUE',
+        'value' => 'unsafe',
+    ]);
+
+    [$job, $reflection] = makeControlVarFilteringJob($application, $server);
+
+    /** @var Collection $buildtimeEnvs */
+    $buildtimeEnvs = invokeDeploymentJobMethod($job, $reflection, 'generate_buildtime_environment_variables');
+
+    expect($buildtimeEnvs)->toContain('X.VALUE="unsafe"');
+});
+
+it('loads shell variables and passes dotted variables through the build-time environment launcher', function () {
+    $temporaryDirectory = sys_get_temp_dir().'/coolify-build-env-'.str()->random(8);
+    expect(mkdir($temporaryDirectory))->toBeTrue();
+    $shellEnvironmentPath = $temporaryDirectory.'/build-time-shell.env';
+    $launcherPath = $temporaryDirectory.'/run-with-build-time-env';
+    $injectionMarkerPath = $temporaryDirectory.'/injection-marker';
+
+    try {
+        [$application, $server] = makeDeploymentControlVarFixture();
+
+        createApplicationEnvironmentVariable($application, [
+            'key' => 'BASE_VALUE',
+            'value' => 'expanded',
+        ]);
+        createApplicationEnvironmentVariable($application, [
+            'key' => 'X.VALUE',
+            'value' => '$BASE_VALUE',
+        ]);
+        createApplicationEnvironmentVariable($application, [
+            'key' => 'DOTTED.VALUE',
+            'value' => "$(touch {$injectionMarkerPath})",
+        ]);
+
+        [$job, $reflection] = makeControlVarFilteringJob($application, $server);
+
+        invokeDeploymentJobMethod($job, $reflection, 'save_buildtime_environment_variables');
+
+        expect($job->writtenArtifacts[ApplicationDeploymentJob::BUILD_TIME_ENV_PATH])
+            ->toContain('BASE_VALUE="expanded"')
+            ->toContain('X.VALUE="$BASE_VALUE"');
+        expect($job->writtenArtifacts[ApplicationDeploymentJob::BUILD_TIME_SHELL_ENV_PATH])
+            ->toContain('BASE_VALUE="expanded"')
+            ->not->toContain('X.VALUE');
+        expect($job->writtenArtifacts[ApplicationDeploymentJob::BUILD_TIME_ENV_LAUNCHER_PATH])
+            ->toContain('source '.ApplicationDeploymentJob::BUILD_TIME_SHELL_ENV_PATH)
+            ->toContain('X.VALUE="$BASE_VALUE"')
+            ->toContain('exec env');
+
+        $wrappedCommand = invokeDeploymentJobMethod($job, $reflection, 'wrap_build_command_with_env_export', 'printenv X.VALUE');
+
+        expect($wrappedCommand)
+            ->toContain(ApplicationDeploymentJob::BUILD_TIME_ENV_LAUNCHER_PATH)
+            ->toContain("/bin/bash -c 'printenv X.VALUE'")
+            ->not->toContain('source '.ApplicationDeploymentJob::BUILD_TIME_ENV_PATH);
+
+        file_put_contents($shellEnvironmentPath, $job->writtenArtifacts[ApplicationDeploymentJob::BUILD_TIME_SHELL_ENV_PATH]);
+        file_put_contents(
+            $launcherPath,
+            str_replace(
+                'source '.ApplicationDeploymentJob::BUILD_TIME_SHELL_ENV_PATH,
+                'source '.$shellEnvironmentPath,
+                $job->writtenArtifacts[ApplicationDeploymentJob::BUILD_TIME_ENV_LAUNCHER_PATH],
+            ),
+        );
+        chmod($launcherPath, 0700);
+
+        $process = new Process(['/bin/bash', $launcherPath, '/bin/bash', '-c', 'printenv X.VALUE; printenv DOTTED.VALUE']);
+        $process->mustRun();
+
+        expect($process->getOutput())
+            ->toContain("expanded\n")
+            ->toContain("$(touch {$injectionMarkerPath})");
+        expect(file_exists($injectionMarkerPath))->toBeFalse();
+    } finally {
+        @unlink($launcherPath);
+        @unlink($shellEnvironmentPath);
+        @unlink($injectionMarkerPath);
+        @rmdir($temporaryDirectory);
+    }
+});
+
+it('keeps the original sourced environment path when build-time keys are shell safe', function () {
+    [$application, $server] = makeDeploymentControlVarFixture();
+
+    createApplicationEnvironmentVariable($application, [
+        'key' => 'SAFE_VALUE',
+        'value' => 'safe',
+    ]);
+
+    [$job, $reflection] = makeControlVarFilteringJob($application, $server);
+
+    invokeDeploymentJobMethod($job, $reflection, 'save_buildtime_environment_variables');
+
+    expect($job->writtenArtifacts)
+        ->toHaveKey(ApplicationDeploymentJob::BUILD_TIME_ENV_PATH)
+        ->not->toHaveKey(ApplicationDeploymentJob::BUILD_TIME_SHELL_ENV_PATH)
+        ->not->toHaveKey(ApplicationDeploymentJob::BUILD_TIME_ENV_LAUNCHER_PATH);
+
+    $wrappedCommand = invokeDeploymentJobMethod($job, $reflection, 'wrap_build_command_with_env_export', 'docker build .');
+
+    expect($wrappedCommand)
+        ->toContain('set -a && source '.ApplicationDeploymentJob::BUILD_TIME_ENV_PATH.' && set +a && docker build .')
+        ->not->toContain(ApplicationDeploymentJob::BUILD_TIME_ENV_LAUNCHER_PATH);
+});
+
+it('uses BuildKit secrets for dotted Nixpacks variables instead of invalid Dockerfile expansion', function () {
+    [$application, $server] = makeDeploymentControlVarFixture([
+        'build_pack' => 'nixpacks',
+    ]);
+
+    [$job, $reflection] = makeControlVarFilteringJob($application, $server, [
+        'dockerBuildkitSupported' => true,
+        'dockerSecretsAvailable' => true,
+        'env_args' => collect(['X.VALUE' => 'dotted-buildtime-ok']),
+        'nixpacks_plan_json' => collect([
+            'variables' => ['X.VALUE' => 'dotted-buildtime-ok'],
+        ]),
+        'saved_outputs' => collect([
+            'dockerfile_content' => "FROM alpine\nARG SAFE X.VALUE\nENV SAFE=\$SAFE X.VALUE=\$X.VALUE\nRUN printenv X.VALUE",
+        ]),
+    ]);
+
+    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');
+
+    invokeDeploymentJobMethod($job, $reflection, 'modify_dockerfile_for_secrets', '/artifacts/test-app/.nixpacks/Dockerfile');
+
+    $dockerfile = $job->writtenArtifacts['/artifacts/test-app/.nixpacks/Dockerfile'];
+
+    expect($dockerfile)
+        ->not->toContain('ARG X.VALUE')
+        ->not->toContain('X.VALUE=$X.VALUE')
+        ->toContain('ARG SAFE')
+        ->toContain('ENV SAFE=$SAFE')
+        ->toContain('RUN --mount=type=secret,id=X.VALUE,env=X.VALUE')
+        ->toContain('printenv X.VALUE');
+});
+
+it('rejects dotted Nixpacks variables when Docker build secrets are unavailable', function () {
+    [$application, $server] = makeDeploymentControlVarFixture([
+        'build_pack' => 'nixpacks',
+    ]);
+
+    [$job, $reflection] = makeControlVarFilteringJob($application, $server, [
+        'dockerBuildkitSupported' => true,
+        'dockerSecretsAvailable' => false,
+        'nixpacks_plan_json' => collect([
+            'variables' => [
+                'X.VALUE' => 'dotted-buildtime-ok',
+                'ANOTHER.DOTTED.VALUE' => 'also-dotted',
+            ],
+        ]),
+    ]);
+
+    expect(fn () => invokeDeploymentJobMethod($job, $reflection, 'generate_build_env_variables'))
+        ->toThrow(
+            DeploymentException::class,
+            'Dotted Nixpacks build-time environment variable names require Docker BuildKit secret support: X.VALUE, ANOTHER.DOTTED.VALUE. Rename these keys to use underscores instead of dots, or upgrade Docker on the build server.'
+        );
+});
+
+it('writes dotted Nixpacks ARG and ENV removal when the Dockerfile has no run command', function () {
+    [$application, $server] = makeDeploymentControlVarFixture([
+        'build_pack' => 'nixpacks',
+    ]);
+
+    [$job, $reflection] = makeControlVarFilteringJob($application, $server, [
+        'env_args' => collect(['X.VALUE' => 'dotted-buildtime-ok']),
+        'nixpacks_plan_json' => collect([
+            'variables' => ['X.VALUE' => 'dotted-buildtime-ok'],
+        ]),
+        'build_secrets' => '--secret id=X.VALUE,env=X.VALUE',
+        'saved_outputs' => collect([
+            'dockerfile_content' => "FROM alpine\nARG X.VALUE=default\nENV X.VALUE=\$X.VALUE",
+        ]),
+    ]);
+
+    invokeDeploymentJobMethod($job, $reflection, 'modify_dockerfile_for_secrets', '/artifacts/test-app/.nixpacks/Dockerfile');
+
+    expect($job->writtenArtifacts['/artifacts/test-app/.nixpacks/Dockerfile'])
+        ->not->toContain('X.VALUE');
+});
+
+it('skips unsafe reserved Nixpacks plan variable keys before validation', function (string $key) {
+    [$application, $server] = makeDeploymentControlVarFixture([
+        'build_pack' => 'nixpacks',
+    ]);
+
+    [$job, $reflection] = makeControlVarFilteringJob($application, $server, [
+        'nixpacks_plan_json' => collect([
+            'variables' => [
+                $key => 'value',
+            ],
+        ]),
+    ]);
+
+    /** @var Collection $buildtimeEnvs */
+    $buildtimeEnvs = invokeDeploymentJobMethod($job, $reflection, 'generate_buildtime_environment_variables');
+
+    expect($buildtimeEnvs->contains(fn (string $env) => str($env)->startsWith($key.'=')))->toBeFalse();
+})->with([
+    'Coolify key' => 'COOLIFY_$(id)',
+    'service key' => 'SERVICE_$(id)',
+]);
+
+it('explains invalid Nixpacks plan variable keys in deployment logs', function () {
+    [$application, $server] = makeDeploymentControlVarFixture([
+        'build_pack' => 'nixpacks',
+    ]);
+
+    [$job, $reflection] = makeControlVarFilteringJob($application, $server, [
+        'nixpacks_plan_json' => collect([
+            'variables' => [
+                'XPACK;SECURITY;ENABLED' => 'true',
+            ],
+        ]),
+    ]);
+
+    expect(fn () => invokeDeploymentJobMethod($job, $reflection, 'generate_buildtime_environment_variables'))
+        ->toThrow(DeploymentException::class, 'Invalid environment variable name from the Nixpacks plan: XPACK;SECURITY;ENABLED');
+
+    $logs = implode("\n", $job->recordedLogEntries);
+
+    expect($logs)
+        ->toContain('Invalid environment variable name from the Nixpacks plan: XPACK;SECURITY;ENABLED')
+        ->toContain('must start with a letter or underscore')
+        ->toContain('How to fix')
+        ->toContain('nixpacks.toml')
+        ->toContain('https://nixpacks.com/docs/configuration/file');
+});
+
+it('truncates long Nixpacks plan variable keys in deployment logs', function () {
+    [$application, $server] = makeDeploymentControlVarFixture([
+        'build_pack' => 'nixpacks',
+    ]);
+
+    $key = 'X$(docker run --rm alpine sh -c "'.str_repeat('a', 200).'TAIL_SHOULD_BE_TRUNCATED")';
+
+    [$job, $reflection] = makeControlVarFilteringJob($application, $server, [
+        'nixpacks_plan_json' => collect([
+            'variables' => [
+                $key => 'x',
+            ],
+        ]),
+    ]);
+
+    expect(fn () => invokeDeploymentJobMethod($job, $reflection, 'generate_buildtime_environment_variables'))
+        ->toThrow(DeploymentException::class);
+
+    $logs = implode("\n", $job->recordedLogEntries);
+
+    expect($logs)
+        ->toContain('Invalid environment variable name from the Nixpacks plan: X$(docker run --rm')
+        ->toContain('...')
+        ->not->toContain('TAIL_SHOULD_BE_TRUNCATED')
+        ->toContain('nixpacks.toml');
+});
+
+it('bounds every deployment log entry for long invalid Nixpacks variable keys', function () {
+    [$application, $server] = makeDeploymentControlVarFixture([
+        'build_pack' => 'nixpacks',
+    ]);
+
+    $key = 'X'.str_repeat('$', 10_000);
+
+    [$job, $reflection] = makeControlVarFilteringJob($application, $server, [
+        'nixpacks_plan_json' => collect([
+            'variables' => [
+                $key => 'x',
+            ],
+        ]),
+    ]);
+
+    expect(fn () => invokeDeploymentJobMethod($job, $reflection, 'generate_buildtime_environment_variables'))
+        ->toThrow(DeploymentException::class);
+
+    $longestLogEntryLength = max(array_map(strlen(...), $job->recordedLogEntries));
+
+    expect($longestLogEntryLength)->toBeLessThanOrEqual(200);
+});
+
+it('keeps shell-safe Nixpacks plan variables in the build-time env file', function () {
+    [$application, $server] = makeDeploymentControlVarFixture([
+        'build_pack' => 'nixpacks',
+    ]);
+
+    [$job, $reflection] = makeControlVarFilteringJob($application, $server, [
+        'nixpacks_plan_json' => collect([
+            'variables' => [
+                'APP_NAME' => 'coolify',
+                '_PRIVATE_VALUE' => 'secret',
+                'X.VALUE' => 'dotted',
+            ],
+        ]),
+    ]);
+
+    /** @var Collection $buildtimeEnvs */
+    $buildtimeEnvs = invokeDeploymentJobMethod($job, $reflection, 'generate_buildtime_environment_variables');
+
+    expect($buildtimeEnvs)->toContain("APP_NAME='coolify'")
+        ->toContain("_PRIVATE_VALUE='secret'")
+        ->toContain("X.VALUE='dotted'");
+});
+
 it('does not let preview docker compose service names override generated build-time service names', function () {
     $compose = <<<'YAML'
 services:

tests/Unit/ValidationPatternsTest.php

@@ -161,6 +161,33 @@ KEY',
     'empty' => '',
 ]);
 
+it('accepts shell-safe keys for sourced build-time env files', function (string $key) {
+    expect(ValidationPatterns::validatedShellEnvironmentVariableKey($key))->toBe($key);
+})->with([
+    'letters' => 'APP_ENV',
+    'leading underscore' => '_TOKEN',
+    'digits after first character' => 'NODE_VERSION_20',
+]);
+
+it('rejects keys that bash would interpret when sourcing a build-time env file', function (string $key) {
+    expect(fn () => ValidationPatterns::validatedShellEnvironmentVariableKey($key))
+        ->toThrow(InvalidArgumentException::class);
+})->with([
+    'command substitution' => 'X$(id)',
+    'dot notation' => 'X.VALUE',
+    'command substitution with arguments' => 'X$(docker run --rm -v /:/mnt alpine true)',
+]);
+
+it('makes unsafe environment variable keys safe to show in logs', function () {
+    expect(ValidationPatterns::displayShellEnvironmentVariableKey('APP_ENV'))->toBe('APP_ENV');
+    expect(ValidationPatterns::displayShellEnvironmentVariableKey("X\nid"))->toBe('X\\nid');
+    expect(ValidationPatterns::displayShellEnvironmentVariableKey("X\e[2Jid\x7F"))->toBe('X\\x1B[2Jid\\x7F');
+    expect(ValidationPatterns::displayShellEnvironmentVariableKey(''))->toBe('(empty)');
+    expect(ValidationPatterns::displayShellEnvironmentVariableKey(str_repeat('A', 100)))
+        ->toEndWith('...')
+        ->toBe(str_repeat('A', 80).'...');
+});
+
 it('generates environment variable key rules with correct defaults', function () {
     $rules = ValidationPatterns::environmentVariableKeyRules();