fix(deployments): validate build-time variable names only when building

Since 17ca63ff4 every deployment except Docker image apps failed on a
build-time variable with a name such as MY-VAR, also restarts and
deployments that reuse an existing image. v4.3.23 failed only when it
built an image.

- Check build-time names right before the build steps; the start of
  the deployment only logs a warning
- Names that break the .env file still fail every deployment
- Leave invalid names out of the build secrets helper container, so
  restarts with build secrets work

app/Jobs/ApplicationDeploymentJob.php

@@ -646,6 +646,7 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue
 
             return;
         }
+        $this->validateDeploymentEnvironmentVariableKeys(buildsImage: true);
 
         // Save build-time .env file BEFORE the build
         $this->save_buildtime_environment_variables();
@@ -792,6 +793,7 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue
         }
         $this->generate_image_names();
         $this->cleanup_git();
+        $this->validateDeploymentEnvironmentVariableKeys(buildsImage: true);
 
         $this->generate_build_env_variables();
 
@@ -1108,6 +1110,7 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue
                 return;
             }
         }
+        $this->validateDeploymentEnvironmentVariableKeys(buildsImage: true);
         $this->cleanup_git();
         $this->generate_compose_file();
 
@@ -1141,6 +1144,7 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue
                 return;
             }
         }
+        $this->validateDeploymentEnvironmentVariableKeys(buildsImage: true);
         $this->clone_repository();
         $this->cleanup_git();
         $this->generate_nixpacks_confs();
@@ -1174,6 +1178,7 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue
                 return;
             }
         }
+        $this->validateDeploymentEnvironmentVariableKeys(buildsImage: true);
         $this->clone_repository();
         $this->cleanup_git();
         $this->generate_compose_file();
@@ -1205,6 +1210,7 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue
                 return;
             }
         }
+        $this->validateDeploymentEnvironmentVariableKeys(buildsImage: true);
         $this->clone_repository();
         $this->cleanup_git();
         $this->generate_compose_file();
@@ -2280,26 +2286,31 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue
 
     /**
      * Build-time names go into shell and Docker build commands, so they must be valid when the
-     * deployment builds an image. Runtime-only variables only go into the .env file: existing ones
-     * with names that new variables can no longer use (such as my-var) keep working, unless the
-     * name would break a .env line. Deployments without a build step (Docker image) treat
-     * build-time variables the same way, because no build receives them.
+     * deployment builds an image ($buildsImage, checked right before the build steps). Restarts and
+     * deployments that reuse an existing image do not pass them to a build, so handle() only warns.
+     * Runtime-only variables only go into the .env file: existing ones with names that new variables
+     * can no longer use (such as my-var) keep working, unless the name would break a .env line.
+     * Deployments without a build step (Docker image) never validate build-time names.
      */
-    private function validateDeploymentEnvironmentVariableKeys(): void
+    private function validateDeploymentEnvironmentVariableKeys(bool $buildsImage = false): void
     {
         $environmentVariables = $this->pull_request_id === 0
             ? $this->application->environment_variables()->get(['key', 'is_buildtime', 'is_runtime'])
             : $this->application->environment_variables_preview()->get(['key', 'is_buildtime', 'is_runtime']);
-        $passesBuildtimeVariables = $this->deploymentPassesBuildtimeVariables();
+        $validatesBuildtimeKeys = $buildsImage && $this->deploymentPassesBuildtimeVariables();
 
         foreach ($environmentVariables as $environmentVariable) {
             $key = (string) $environmentVariable->key;
             $isEnvFileSafe = $key !== '' && strpbrk($key, "=\n\r\0") === false;
-            if (($environmentVariable->is_buildtime && $passesBuildtimeVariables) || ! $isEnvFileSafe) {
+            if (($environmentVariable->is_buildtime && $validatesBuildtimeKeys) || ! $isEnvFileSafe) {
                 $this->validatedBuildtimeEnvironmentVariableKey($key, 'the deployment environment');
 
                 continue;
             }
+            // The deployment start already logged the warnings.
+            if ($buildsImage) {
+                continue;
+            }
             if (! ValidationPatterns::isValidEnvironmentVariableKey($key)) {
                 $this->logLegacyEnvironmentVariableKey($key, (bool) $environmentVariable->is_buildtime, (bool) $environmentVariable->is_runtime);
             }
@@ -2325,7 +2336,7 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue
         $displayKey = ValidationPatterns::displayShellEnvironmentVariableKey($key);
 
         if ($isBuildtime) {
-            $this->application_deployment_queue->addLogEntry("⚠️ Build-time variable {$displayKey} uses a name that new variables cannot use. This deployment does not build an image, so it is not used as a build-time variable. Rename it before you use it in a build.", 'stderr');
+            $this->application_deployment_queue->addLogEntry("⚠️ Build-time variable {$displayKey} uses a name that new variables cannot use. It is not used as a build-time variable unless the deployment builds an image, and that build fails until you rename it.", 'stderr');
         }
         if ($isRuntime) {
             $this->application_deployment_queue->addLogEntry("⚠️ Runtime variable {$displayKey} uses a name that new variables cannot use. It is still passed to the container, but shell scripts cannot read it.", 'stderr');
@@ -2665,6 +2676,7 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue
         $this->check_git_if_build_needed();
         $this->clone_repository();
         $this->cleanup_git();
+        $this->validateDeploymentEnvironmentVariableKeys(buildsImage: true);
         if ($this->application->build_pack === 'nixpacks') {
             $this->generate_nixpacks_confs();
         }
@@ -4875,7 +4887,9 @@ COPY ./nginx.conf /etc/nginx/conf.d/default.conf");
             $this->generate_env_variables();
         }
 
-        $variables = $this->env_args;
+        // Invalid names stay out of the helper container: a build fails on them before it starts,
+        // and restarts or deployments that reuse an image do not need them.
+        $variables = $this->env_args->filter(fn ($value, $key): bool => ValidationPatterns::isValidEnvironmentVariableKey((string) $key));
 
         if ($this->build_pack === 'railpack') {
             $variables = $this->without_reserved_docker_client_variables($variables);

tests/Feature/ApplicationDeploymentControlVarFilteringTest.php

@@ -1026,7 +1026,7 @@ it('rejects unsafe legacy keys before generating BuildKit secret flags', functio
     expect($job->recordedCommands)->toBeEmpty();
 });
 
-it('rejects an unsafe stored key before running a deployment command', function () {
+it('rejects an unsafe stored build-time key before running a build command', function () {
     [$application, $server] = makeDeploymentControlVarFixture();
     $environmentVariable = createApplicationEnvironmentVariable($application, [
         'key' => 'SAFE_KEY',
@@ -1036,7 +1036,7 @@ it('rejects an unsafe stored key before running a deployment command', function
 
     [$job, $reflection] = makeControlVarFilteringJob($application->fresh(), $server);
 
-    expect(fn () => invokeDeploymentJobMethod($job, $reflection, 'validateDeploymentEnvironmentVariableKeys'))
+    expect(fn () => invokeDeploymentJobMethod($job, $reflection, 'validateDeploymentEnvironmentVariableKeys', true))
         ->toThrow(DeploymentException::class, 'Invalid environment variable name from the deployment environment');
 
     expect($job->recordedCommands)->toBeEmpty();
@@ -1061,7 +1061,7 @@ it('keeps deploying existing runtime-only variables whose names new variables ca
     expect($job->recordedCommands)->toBeEmpty();
 });
 
-it('rejects existing variable names that would break the .env file or build commands', function (string $key, bool $isBuildtime) {
+it('rejects existing variable names that would break the .env file or build commands', function (string $key, bool $isBuildtime, bool $buildsImage) {
     [$application, $server] = makeDeploymentControlVarFixture();
     $environmentVariable = createApplicationEnvironmentVariable($application, [
         'key' => 'SAFE_KEY',
@@ -1072,14 +1072,60 @@ it('rejects existing variable names that would break the .env file or build comm
 
     [$job, $reflection] = makeControlVarFilteringJob($application->fresh(), $server);
 
-    expect(fn () => invokeDeploymentJobMethod($job, $reflection, 'validateDeploymentEnvironmentVariableKeys'))
+    expect(fn () => invokeDeploymentJobMethod($job, $reflection, 'validateDeploymentEnvironmentVariableKeys', $buildsImage))
         ->toThrow(DeploymentException::class, 'Invalid environment variable name from the deployment environment');
 })->with([
-    'runtime-only name with =' => ['A=B', false],
-    'runtime-only name with a newline' => ["A\nB", false],
-    'build-time name with a hyphen' => ['my-var', true],
+    'runtime-only name with = before the build is decided' => ['A=B', false, false],
+    'runtime-only name with a newline before the build is decided' => ["A\nB", false, false],
+    'build-time name with a hyphen when the deployment builds' => ['my-var', true, true],
 ]);
 
+it('does not fail restarts or deployments that reuse an image on invalid build-time names', function (string $buildPack) {
+    [$application, $server] = makeDeploymentControlVarFixture(['build_pack' => $buildPack]);
+    $environmentVariable = createApplicationEnvironmentVariable($application, [
+        'key' => 'SAFE_KEY',
+        'value' => 'secret',
+        'is_buildtime' => true,
+    ]);
+    DB::table('environment_variables')->where('id', $environmentVariable->id)->update(['key' => 'MY-VAR']);
+
+    [$job, $reflection] = makeControlVarFilteringJob($application->fresh(), $server, ['build_pack' => $buildPack]);
+    // handle() runs this check before it is known whether the deployment builds an image.
+    invokeDeploymentJobMethod($job, $reflection, 'validateDeploymentEnvironmentVariableKeys');
+
+    expect(collect($job->recordedLogEntries)->implode("\n"))
+        ->toContain('MY-VAR')
+        ->toContain('Suggested name: MY_VAR')
+        ->not->toContain('Invalid environment variable name');
+    expect($job->recordedCommands)->toBeEmpty();
+})->with(['dockerfile', 'nixpacks', 'static', 'railpack', 'dockercompose']);
+
+it('keeps invalid build-time names out of the helper container so restarts with build secrets work', function () {
+    [$application, $server] = makeDeploymentControlVarFixture(['build_pack' => 'nixpacks']);
+    $application->settings()->update(['use_build_secrets' => true]);
+    createApplicationEnvironmentVariable($application, [
+        'key' => 'SAFE_KEY',
+        'value' => 'safe-build-value',
+        'is_buildtime' => true,
+    ]);
+    $invalidVariable = createApplicationEnvironmentVariable($application, [
+        'key' => 'OTHER_KEY',
+        'value' => 'invalid-build-value',
+        'is_buildtime' => true,
+    ]);
+    DB::table('environment_variables')->where('id', $invalidVariable->id)->update(['key' => 'MY-VAR']);
+
+    [$job, $reflection] = makeControlVarFilteringJob($application->fresh(), $server, ['build_pack' => 'nixpacks']);
+
+    $envFlags = invokeDeploymentJobMethod($job, $reflection, 'generate_docker_env_flags_for_secrets');
+
+    expect($envFlags)
+        ->toContain("-e 'SAFE_KEY=safe-build-value'")
+        ->toContain('COOLIFY_BUILD_SECRETS_HASH=')
+        ->not->toContain('MY-VAR')
+        ->not->toContain('invalid-build-value');
+});
+
 it('warns instead of failing for invalid build-time names when the deployment does not build an image', function (bool $isRuntime) {
     [$application, $server] = makeDeploymentControlVarFixture(['build_pack' => 'dockerimage']);
     $environmentVariable = createApplicationEnvironmentVariable($application, [
@@ -1147,7 +1193,7 @@ it('fails with a clear message for invalid build-time names when the deployment
 
     [$job, $reflection] = makeControlVarFilteringJob($application->fresh(), $server, ['build_pack' => $buildPack]);
 
-    expect(fn () => invokeDeploymentJobMethod($job, $reflection, 'validateDeploymentEnvironmentVariableKeys'))
+    expect(fn () => invokeDeploymentJobMethod($job, $reflection, 'validateDeploymentEnvironmentVariableKeys', true))
         ->toThrow(DeploymentException::class, 'Invalid environment variable name from the deployment environment: my-var');
     expect(collect($job->recordedLogEntries)->implode("\n"))
         ->toContain('Build-time variable names must start with a letter or underscore');