diff --git a/app/Jobs/ApplicationDeploymentJob.php b/app/Jobs/ApplicationDeploymentJob.php index 3944b6c0dc..38209ab97b 100644 --- a/app/Jobs/ApplicationDeploymentJob.php +++ b/app/Jobs/ApplicationDeploymentJob.php @@ -231,6 +231,13 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue private Collection|string $build_secrets; + /** + * Build-time variables that the helper container has in its environment for build secrets. + * + * @var array + */ + private array $helperBuildSecretEnvironmentKeys = []; + public function tags() { // Do not remove this one, it needs to properly identify which worker is running the job @@ -1086,7 +1093,7 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue $command = serverDockerConfigPrefix($this->server)."{$this->coolify_variables} docker compose --env-file {$workdir}/.env --project-name {$this->application->uuid} --project-directory {$workdir} -f {$workdir}{$this->docker_compose_location} pull --ignore-buildable {$services}"; } else { $workdir = $this->workdir; - $command = executeInDocker($this->deployment_uuid, "{$this->coolify_variables} docker compose --env-file {$workdir}/.env --project-name {$this->application->uuid} --project-directory {$workdir} -f {$workdir}{$this->docker_compose_location} pull --ignore-buildable {$services}"); + $command = executeInDocker($this->deployment_uuid, $this->withoutHelperBuildSecretEnvironment("{$this->coolify_variables} docker compose --env-file {$workdir}/.env --project-name {$this->application->uuid} --project-directory {$workdir} -f {$workdir}{$this->docker_compose_location} pull --ignore-buildable {$services}")); } $this->execute_remote_command([ @@ -1137,7 +1144,7 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue try { $this->execute_remote_command( - [executeInDocker($this->deployment_uuid, "cd {$this->workdir} && {$start_command}"), 'hidden' => false, 'type' => 'stdout', 'command_hidden' => true], + [executeInDocker($this->deployment_uuid, $this->withoutHelperBuildSecretEnvironment("cd {$this->workdir} && {$start_command}")), 'hidden' => false, 'type' => 'stdout', 'command_hidden' => true], ); } catch (\RuntimeException $e) { if (str_contains($e->getMessage(), "matching `'") || str_contains($e->getMessage(), 'unexpected EOF')) { @@ -1182,7 +1189,7 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue ); } else { $this->execute_remote_command( - [executeInDocker($this->deployment_uuid, "cd {$this->basedir} && {$start_command}"), 'hidden' => false, 'type' => 'stdout', 'command_hidden' => true], + [executeInDocker($this->deployment_uuid, $this->withoutHelperBuildSecretEnvironment("cd {$this->basedir} && {$start_command}")), 'hidden' => false, 'type' => 'stdout', 'command_hidden' => true], ); } } else { @@ -1201,7 +1208,7 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue $command .= " --env-file {$this->workdir}/.env"; $command .= " --project-name {$this->application->uuid} --project-directory {$this->workdir} -f {$this->workdir}{$this->docker_compose_location} up -d"; $this->execute_remote_command( - [executeInDocker($this->deployment_uuid, $command), 'hidden' => false, 'type' => 'stdout', 'command_hidden' => true], + [executeInDocker($this->deployment_uuid, $this->withoutHelperBuildSecretEnvironment($command)), 'hidden' => false, 'type' => 'stdout', 'command_hidden' => true], ); $this->write_deployment_configurations(); } @@ -5460,6 +5467,8 @@ COPY ./nginx.conf /etc/nginx/conf.d/default.conf"); private function generate_docker_env_flags_for_secrets() { + $this->helperBuildSecretEnvironmentKeys = []; + // Only generate env flags if build secrets are enabled and the deployment builds an image if (! $this->application->settings->use_build_secrets || ! $this->deploymentPassesBuildtimeVariables()) { return ''; @@ -5483,26 +5492,51 @@ COPY ./nginx.conf /etc/nginx/conf.d/default.conf"); return ''; } - // Get database env vars to check for multiline flag - $env_vars = $this->pull_request_id === 0 - ? $this->application->environment_variables()->where('is_buildtime', true)->get() - : $this->application->environment_variables_preview()->where('is_buildtime', true)->get(); - - // 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 [ - 'key' => $key, - 'value' => $value, - 'is_multiline' => $env ? $env->is_multiline : false, - ]; - }); + $vars_array = $this->withRawBuildSecretValues($variables)->map(fn ($value, $key): array => [ + 'key' => $this->validatedBuildtimeEnvironmentVariableKey((string) $key, 'the build secret environment'), + 'value' => $value, + ]); + $this->helperBuildSecretEnvironmentKeys = $vars_array->keys()->map(fn ($key): string => (string) $key)->values()->all(); return generateDockerEnvFlags($vars_array); } + /** + * The build-time values are escaped or quoted for .env files and shell build arguments, where the + * shell removes the escapes again. A build secret gets its value from the helper container as it + * is, so it gets the raw value of each variable instead. + * + * @param Collection $variables + * @return Collection + */ + private function withRawBuildSecretValues(Collection $variables): Collection + { + $environmentVariables = $this->pull_request_id === 0 + ? $this->application->environment_variables() + : $this->application->environment_variables_preview()->whereNotIn('id', $this->previewVariableIdsWithProductionValues()); + $rawValues = $environmentVariables + ->where('is_buildtime', true) + ->whereIn('key', $variables->keys()->all()) + ->get() + ->mapWithKeys(fn (EnvironmentVariable $env): array => [$env->key => $this->resolve_environment_variable_raw($env)]); + + return $variables->map(fn ($value, $key) => $rawValues->get($key, $value)); + } + + /** + * Docker Compose prefers the process environment over `--env-file`. A command in the helper container + * that pulls or starts services must not see the build-time values that the container holds for build + * secrets, so the services get the values of the runtime .env file. + */ + private function withoutHelperBuildSecretEnvironment(string $command): string + { + if ($this->helperBuildSecretEnvironmentKeys === []) { + return $command; + } + + return 'unset '.implode(' ', $this->helperBuildSecretEnvironmentKeys)."; {$command}"; + } + private function generate_build_secrets(Collection $variables) { if ($variables->isEmpty()) { @@ -5520,7 +5554,7 @@ COPY ./nginx.conf /etc/nginx/conf.d/default.conf"); ->implode(' '); // Docker ignores secret values for the build cache, so the hash goes in as a build arg - $this->build_secrets .= ' --build-arg '.escapeshellarg('COOLIFY_BUILD_SECRETS_HASH='.$this->generate_secrets_hash($variables)); + $this->build_secrets .= ' --build-arg '.escapeshellarg('COOLIFY_BUILD_SECRETS_HASH='.$this->generate_secrets_hash($this->withRawBuildSecretValues($variables))); } private function generate_secrets_hash($variables) @@ -6051,7 +6085,7 @@ COPY ./nginx.conf /etc/nginx/conf.d/default.conf"); 'environment' => $key, ]; } - $secretsHash = $this->generate_secrets_hash($variables); + $secretsHash = $this->generate_secrets_hash($this->withRawBuildSecretValues($variables)); $services = data_get($composeFile, 'services', []); foreach ($services as $serviceName => &$service) { diff --git a/bootstrap/helpers/docker.php b/bootstrap/helpers/docker.php index 4bdf9ca2d6..62be1532ad 100644 --- a/bootstrap/helpers/docker.php +++ b/bootstrap/helpers/docker.php @@ -2124,7 +2124,7 @@ function generateDockerBuildArgs($variables): Collection /** * Generate Docker environment flags from environment variables collection * - * @param Collection|array $variables Collection of variables with 'key', 'value', and optionally 'is_multiline' + * @param Collection|array $variables Collection of variables with 'key' and 'value' * @return string Space-separated environment flags */ function generateDockerEnvFlags($variables): string @@ -2140,11 +2140,6 @@ function generateDockerEnvFlags($variables): string throw new InvalidArgumentException('Invalid environment variable key.'); } - $isMultiline = is_array($var) ? data_get($var, 'is_multiline', false) : ($var->is_multiline ?? false); - if ($isMultiline) { - $value = trim($value, "'"); - } - return '-e '.escapeshellarg("{$key}={$value}"); }) ->implode(' '); diff --git a/tests/Feature/ApplicationDeploymentControlVarFilteringTest.php b/tests/Feature/ApplicationDeploymentControlVarFilteringTest.php index a4527e59d6..fc3fdcf363 100644 --- a/tests/Feature/ApplicationDeploymentControlVarFilteringTest.php +++ b/tests/Feature/ApplicationDeploymentControlVarFilteringTest.php @@ -2200,3 +2200,56 @@ YAML; 'production' => [0], 'preview' => [7], ]); + +it('hides the build secret values of the helper container from commands that start Compose services', function () { + [$application, $server] = makeDeploymentControlVarFixture(['build_pack' => 'dockercompose']); + $application->settings()->update(['use_build_secrets' => true]); + createApplicationEnvironmentVariable($application, [ + 'key' => 'DATABASE_URL', + 'value' => 'postgresql://user:pass@db:5432/app', + 'is_literal' => true, + ]); + + [$job, $reflection] = makeControlVarFilteringJob($application->fresh(), $server, ['build_pack' => 'dockercompose']); + $envFlags = invokeDeploymentJobMethod($job, $reflection, 'generate_docker_env_flags_for_secrets'); + expect($envFlags)->toBe("-e 'DATABASE_URL=postgresql://user:pass@db:5432/app'"); + + // The helper container has the variable in its environment, as `docker run -e` gives it. + $command = invokeDeploymentJobMethod($job, $reflection, 'withoutHelperBuildSecretEnvironment', 'echo "[${DATABASE_URL-unset}]"'); + $process = new Process(['bash', '-c', $command], env: ['DATABASE_URL' => "'postgresql://user:pass@db:5432/app'"]); + $process->mustRun(); + + expect(trim($process->getOutput()))->toBe('[unset]'); +}); + +it('leaves commands unchanged when the helper container has no build secret values', function () { + [$application, $server] = makeDeploymentControlVarFixture(['build_pack' => 'dockercompose']); + createApplicationEnvironmentVariable($application, ['key' => 'DATABASE_URL', 'value' => 'value']); + + [$job, $reflection] = makeControlVarFilteringJob($application->fresh(), $server, ['build_pack' => 'dockercompose']); + expect(invokeDeploymentJobMethod($job, $reflection, 'generate_docker_env_flags_for_secrets'))->toBe(''); + + expect(invokeDeploymentJobMethod($job, $reflection, 'withoutHelperBuildSecretEnvironment', 'docker compose up -d')) + ->toBe('docker compose up -d'); +}); + +it('gives build secrets the stored value without the quotes that Coolify adds', function () { + [$application, $server] = makeDeploymentControlVarFixture(['build_pack' => 'dockerfile']); + $application->settings()->update(['use_build_secrets' => true]); + createApplicationEnvironmentVariable($application, ['key' => 'LITERAL', 'value' => "'quoted' value", 'is_literal' => true]); + createApplicationEnvironmentVariable($application, ['key' => 'MULTILINE', 'value' => "line1\nline2", 'is_multiline' => true]); + createApplicationEnvironmentVariable($application, ['key' => 'NORMAL', 'value' => "'kept'"]); + + [$job, $reflection] = makeControlVarFilteringJob($application->fresh(), $server, ['build_pack' => 'dockerfile']); + $envFlags = invokeDeploymentJobMethod($job, $reflection, 'generate_docker_env_flags_for_secrets'); + + // Read the values as the helper container gets them from `docker run -e`. + $process = new Process(['bash', '-c', 'printf "%s\n" '.$envFlags.' | sed -n "s/^\\(LITERAL\\|MULTILINE\\|NORMAL\\)=/&/p"']); + $process->mustRun(); + $output = $process->getOutput(); + + expect($output) + ->toContain("LITERAL='quoted' value\n") + ->toContain("MULTILINE=line1\n") + ->toContain("NORMAL='kept'\n"); +}); diff --git a/tests/Feature/EnvironmentVariable/MultilineEnvironmentVariableTest.php b/tests/Feature/EnvironmentVariable/MultilineEnvironmentVariableTest.php index f71811b24a..4f9fed3fe7 100644 --- a/tests/Feature/EnvironmentVariable/MultilineEnvironmentVariableTest.php +++ b/tests/Feature/EnvironmentVariable/MultilineEnvironmentVariableTest.php @@ -16,7 +16,7 @@ test('generateDockerBuildArgs returns only keys without values', function () { test('generateDockerBuildArgs works with collection of objects', function () { $variables = collect([ (object) ['key' => 'VAR1', 'value' => 'value1', 'is_multiline' => false], - (object) ['key' => 'VAR2', 'value' => "'multiline\nvalue'", 'is_multiline' => true], + (object) ['key' => 'VAR2', 'value' => "multiline\nvalue"], ]); $buildArgs = generateDockerBuildArgs($variables); @@ -59,7 +59,7 @@ test('generateDockerBuildArgs handles variables without is_multiline', function test('generateDockerEnvFlags produces correct format', function () { $variables = [ ['key' => 'NORMAL_VAR', 'value' => 'value', 'is_multiline' => false], - ['key' => 'MULTILINE_VAR', 'value' => "'line1\nline2'", 'is_multiline' => true], + ['key' => 'MULTILINE_VAR', 'value' => "line1\nline2"], ]; $envFlags = generateDockerEnvFlags($variables); @@ -73,7 +73,7 @@ test('generateDockerEnvFlags produces correct format', function () { test('generateDockerEnvFlags works with collection input', function () { $variables = collect([ (object) ['key' => 'VAR1', 'value' => 'value1', 'is_multiline' => false], - (object) ['key' => 'VAR2', 'value' => "'multiline\nvalue'", 'is_multiline' => true], + (object) ['key' => 'VAR2', 'value' => "multiline\nvalue"], ]); $envFlags = generateDockerEnvFlags($variables);