diff --git a/.ai/lessons.md b/.ai/lessons.md index 190bbee34e..091f8b933c 100644 --- a/.ai/lessons.md +++ b/.ai/lessons.md @@ -20,6 +20,7 @@ - Do not use a passing unit test, a successful build, or a healthy process as proof for a reported UI failure. - Verify the exact live flow, persisted state, relevant logs, and queue state when they affect the result. - When the request covers more than one interface or resource type, inventory and verify each supported path. +- When a change alters how stored data reaches running containers, simulate the upgrade on the dev VM: start resources on the old code, then migrate and restart them on the new code, and compare the results. ## Preserve product scope - Do not replace required SPA navigation with a full-page redirect to hide a lifecycle or ordering defect. diff --git a/app/Jobs/ApplicationDeploymentJob.php b/app/Jobs/ApplicationDeploymentJob.php index e04580e76d..e0a89e9e75 100644 --- a/app/Jobs/ApplicationDeploymentJob.php +++ b/app/Jobs/ApplicationDeploymentJob.php @@ -216,8 +216,6 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue private bool $dockerSecretsSupported = false; - private bool $dockerSecretsAvailable = false; - private bool $useBuildtimeEnvironmentLauncher = false; private bool $skip_build = false; @@ -506,7 +504,6 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue $this->dockerBuildkitSupported = false; $this->dockerBuildxAvailable = false; $this->dockerSecretsSupported = false; - $this->dockerSecretsAvailable = false; $serverToCheck = $this->use_build_server ? $this->build_server : $this->server; $serverName = $this->use_build_server ? "build server ({$serverToCheck->name})" : "deployment server ({$serverToCheck->name})"; @@ -558,19 +555,16 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue } } - if ($this->dockerBuildkitSupported) { + if ($this->application->settings->use_build_secrets && $this->dockerBuildkitSupported) { $secretsTest = instant_remote_process( ["docker build --help 2>&1 | grep -q 'secret' && echo 'supported' || echo 'not-supported'"], $serverToCheck ); if (trim($secretsTest) === 'supported') { - $this->dockerSecretsAvailable = true; - if ($this->application->settings->use_build_secrets) { - $this->dockerSecretsSupported = true; - $this->application_deployment_queue->addLogEntry('Build secrets are enabled and will be used for enhanced security.'); - } - } elseif ($this->application->settings->use_build_secrets) { + $this->dockerSecretsSupported = true; + $this->application_deployment_queue->addLogEntry('Build secrets are enabled and will be used for enhanced security.'); + } else { $this->application_deployment_queue->addLogEntry("Docker on {$serverName} does not support build secrets. Using traditional build arguments."); } } @@ -578,7 +572,6 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue $this->dockerBuildkitSupported = false; $this->dockerBuildxAvailable = false; $this->dockerSecretsSupported = false; - $this->dockerSecretsAvailable = false; $this->application_deployment_queue->addLogEntry("Could not detect BuildKit capabilities on {$serverName}: {$e->getMessage()}"); } } @@ -1763,7 +1756,7 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue continue; } - $envs->push($env->key.'='.$this->resolve_environment_variable($env)); + $envs->push($this->runtimeEnvironmentAssignment($env)); } // Check for PORT environment variable mismatch with ports_exposes @@ -1834,7 +1827,7 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue continue; } - $envs->push($env->key.'='.$this->resolve_environment_variable($env)); + $envs->push($this->runtimeEnvironmentAssignment($env)); } // Fall back to production env vars for keys not overridden by preview vars, @@ -1852,7 +1845,7 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue continue; } - $envs->push($env->key.'='.$this->resolve_environment_variable($env)); + $envs->push($this->runtimeEnvironmentAssignment($env)); } } @@ -1872,6 +1865,32 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue return $envs; } + /** + * A KEY="value" line for the runtime .env file, which Docker Compose reads. Variables saved + * before exact escaping keep the old line, so their containers get the same value as before. + */ + private function runtimeEnvironmentAssignment(EnvironmentVariable $environmentVariable): string + { + if (! $this->application->useExactEscaping($environmentVariable)) { + return $environmentVariable->key.'='.$this->resolve_environment_variable($environmentVariable); + } + + $resolvedValue = (string) $environmentVariable->get_real_environment_variables_with_server( + $environmentVariable->value, + $this->application, + $this->mainServer, + ); + + if (RemoteSecretReferences::containsReference($environmentVariable->value) || RemoteSecretReferences::containsReference($resolvedValue)) { + return $environmentVariable->key.'='.escapeComposeEnvFileValue($this->resolve_environment_variable_raw($environmentVariable)); + } + + return $environmentVariable->key.'='.escapeComposeEnvFileValue( + $resolvedValue, + allowInterpolation: $this->application->environmentVariableAllowsInterpolation($environmentVariable, $resolvedValue), + ); + } + private function isGeneratedDockerComposeEnvironmentVariable(EnvironmentVariable $environmentVariable): bool { $key = str($environmentVariable->key); @@ -2121,6 +2140,7 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue ->where('is_buildtime', true) // ONLY build-time variables ->orderBy($this->application->settings->is_env_sorting_enabled ? 'key' : 'id') ->get(); + $sorted_environment_variables = $this->withoutLegacyEnvironmentVariableKeys($sorted_environment_variables); // For Docker Compose, filter out generated SERVICE_* variables as we generate these if ($this->build_pack === 'dockercompose') { @@ -2183,6 +2203,7 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue ->where('is_buildtime', true) // ONLY build-time variables ->orderBy($this->application->settings->is_env_sorting_enabled ? 'key' : 'id') ->get(); + $sorted_environment_variables = $this->withoutLegacyEnvironmentVariableKeys($sorted_environment_variables); // For Docker Compose, filter out generated SERVICE_* variables as we generate these with PR-specific values if ($this->build_pack === 'dockercompose') { @@ -2343,23 +2364,22 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue $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']); - $validatesBuildtimeKeys = $buildsImage && $this->deploymentPassesBuildtimeVariables(); + $passesBuildtimeVariables = $this->deploymentPassesBuildtimeVariables(); foreach ($environmentVariables as $environmentVariable) { $key = (string) $environmentVariable->key; - $isEnvFileSafe = $key !== '' && strpbrk($key, "=\n\r\0") === false; - if (($environmentVariable->is_buildtime && $validatesBuildtimeKeys) || ! $isEnvFileSafe) { - $this->validatedBuildtimeEnvironmentVariableKey($key, 'the deployment environment'); + if (ValidationPatterns::isValidEnvironmentVariableKey($key)) { + continue; + } - 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); + $isEnvFileSafe = $key !== '' && strpbrk($key, "=\n\r\0") === false; + $isLegacyBuildtimeKey = preg_match('/\A[A-Za-z0-9_.-]+\z/', $key) === 1; + $isPassedToBuild = $environmentVariable->is_buildtime && $passesBuildtimeVariables && $buildsImage; + if (! $isEnvFileSafe || ($isPassedToBuild && ! $isLegacyBuildtimeKey)) { + $this->validatedBuildtimeEnvironmentVariableKey($key, 'the deployment environment'); } + + $this->logLegacyEnvironmentVariableKey($key, (bool) $environmentVariable->is_buildtime, (bool) $environmentVariable->is_runtime, $passesBuildtimeVariables); } } @@ -2372,7 +2392,7 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue return $this->application->build_pack !== 'dockerimage'; } - private function logLegacyEnvironmentVariableKey(string $key, bool $isBuildtime, bool $isRuntime): void + private function logLegacyEnvironmentVariableKey(string $key, bool $isBuildtime, bool $isRuntime, bool $passesBuildtimeVariables): void { $suggestedKey = (string) preg_replace('/[^A-Za-z0-9_]/', '_', $key); if (preg_match('/\A[0-9]/', $suggestedKey) === 1) { @@ -2381,18 +2401,33 @@ 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. 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'); - } - if (! $isBuildtime && ! $isRuntime) { - $this->application_deployment_queue->addLogEntry("⚠️ Variable {$displayKey} uses a name that new variables cannot use. This deployment does not use it.", 'stderr'); + if ($isBuildtime && $passesBuildtimeVariables) { + $this->application_deployment_queue->addLogEntry("⚠️ Build-time variable {$displayKey} uses a name that new variables cannot use. It is skipped during the build".($isRuntime ? ', but is still passed to the container.' : '.'), 'stderr'); + } else { + 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'); + } + 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'); + } + if (! $isBuildtime && ! $isRuntime) { + $this->application_deployment_queue->addLogEntry("⚠️ Variable {$displayKey} uses a name that new variables cannot use. This deployment does not use it.", 'stderr'); + } } $this->application_deployment_queue->addLogEntry(' Suggested name: '.ValidationPatterns::displayShellEnvironmentVariableKey($suggestedKey), type: 'info'); } + /** + * Skip build-time variables with names that new variables can no longer use (such as my-var). + * validateDeploymentEnvironmentVariableKeys() logs a warning for them before the deployment starts. + */ + private function withoutLegacyEnvironmentVariableKeys(Collection $environmentVariables): Collection + { + return $environmentVariables->filter( + fn (EnvironmentVariable $environmentVariable): bool => ValidationPatterns::isValidEnvironmentVariableKey((string) $environmentVariable->key) + ); + } + private function logInvalidBuildtimeEnvironmentVariableKey(string $key, string $origin): void { $displayKey = ValidationPatterns::displayShellEnvironmentVariableKey($key); @@ -2428,6 +2463,33 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue $this->application_deployment_queue->addLogEntry(" Suggested name: {$displaySuggestedKey}", type: 'info'); } + /** + * Remove variables with dotted names from a Nixpacks or Railpack build. + * + * Nixpacks and Railpack cut these names at the first dot, and their Debian-based + * images run build commands through dash, which drops dotted names from the + * environment. The variables are still available at runtime. + */ + private function withoutDottedBuildVariables(Collection $variables, string $builder): Collection + { + $dottedKeys = $variables->keys() + ->map(fn ($key): string => (string) $key) + ->filter(fn (string $key): bool => str_contains($key, '.')) + ->values(); + + if ($dottedKeys->isEmpty()) { + return $variables; + } + + foreach ($dottedKeys as $key) { + $this->application_deployment_queue->addLogEntry('⚠️ Build-time variable '.ValidationPatterns::displayShellEnvironmentVariableKey($key)." has a dot in its name, which {$builder} cannot pass to the build. It is skipped during the build, but is still passed to the container.", 'stderr'); + $this->logSuggestedShellEnvironmentVariableKey($key); + } + $this->application_deployment_queue->addLogEntry(' If the build needs the value, rename the variable. Otherwise, disable "Available at Buildtime" for it.', type: 'info'); + + return $variables->reject(fn ($value, $key): bool => str_contains((string) $key, '.')); + } + private function save_buildtime_environment_variables() { $environment_variables = $this->generate_buildtime_environment_variables(); @@ -3218,7 +3280,10 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue // Do any modifications here // We need to generate envs here because nixpacks need to know to generate a proper Dockerfile $this->generate_env_variables(); - $merged_envs = collect(data_get($parsed, 'variables', []))->merge($this->env_args); + $merged_envs = $this->withoutDottedBuildVariables( + collect(data_get($parsed, 'variables', []))->merge($this->env_args), + 'Nixpacks', + ); $aptPkgs = data_get($parsed, 'phases.setup.aptPkgs', []); if (count($aptPkgs) === 0) { $aptPkgs = ['curl', 'wget']; @@ -3388,8 +3453,7 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue ? $this->application->railpack_environment_variables()->get() : $this->application->railpack_environment_variables_preview()->get(); - $variables = $genericBuildVariables - ->merge($railpackVariables) + $variables = $this->withoutLegacyEnvironmentVariableKeys($genericBuildVariables->merge($railpackVariables)) ->mapWithKeys(function (EnvironmentVariable $environmentVariable) { $value = $this->normalize_resolved_build_variable_value($environmentVariable); if (is_null($value) || $value === '') { @@ -3398,6 +3462,7 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue return [$environmentVariable->key => $value]; }); + $variables = $this->withoutDottedBuildVariables($variables, 'Railpack'); if ($this->application->install_command) { $variables->put('RAILPACK_INSTALL_CMD', $this->application->install_command); @@ -3899,6 +3964,7 @@ COPY ./nginx.conf /etc/nginx/conf.d/default.conf"); ->withoutBuildpackControlVariables() ->where('is_buildtime', true) ->get(); + $envs = $this->withoutLegacyEnvironmentVariableKeys($envs); if ($this->build_pack === 'dockercompose') { $envs = $envs->reject(fn (EnvironmentVariable $env) => $this->isGeneratedDockerComposeEnvironmentVariable($env)); @@ -3921,6 +3987,7 @@ COPY ./nginx.conf /etc/nginx/conf.d/default.conf"); ->withoutBuildpackControlVariables() ->where('is_buildtime', true) ->get(); + $envs = $this->withoutLegacyEnvironmentVariableKeys($envs); if ($this->build_pack === 'dockercompose') { $envs = $envs->reject(fn (EnvironmentVariable $env) => $this->isGeneratedDockerComposeEnvironmentVariable($env)); @@ -4930,21 +4997,6 @@ COPY ./nginx.conf /etc/nginx/conf.d/default.conf"); $this->analyzeBuildTimeVariables($variables); } - $requiresDottedEnvironmentSecrets = $this->application->build_pack === 'nixpacks' - && $variables->keys()->contains(fn ($key): bool => str_contains((string) $key, '.')); - - if ($requiresDottedEnvironmentSecrets) { - if (! $this->dockerSecretsAvailable) { - $dottedKeys = $variables->keys() - ->filter(fn ($key): bool => str_contains((string) $key, '.')) - ->implode(', '); - - throw new DeploymentException("Dotted Nixpacks build-time environment variable names require Docker BuildKit secret support: {$dottedKeys}. Rename these keys to use underscores instead of dots, or upgrade Docker on the build server."); - } - - $this->dockerSecretsSupported = true; - } - if ($this->dockerSecretsSupported) { $this->generate_build_secrets($variables); $this->build_args = ''; @@ -5129,6 +5181,7 @@ COPY ./nginx.conf /etc/nginx/conf.d/default.conf"); ->withoutBuildpackControlVariables() ->where('is_buildtime', true) ->get(); + $envs = $this->withoutLegacyEnvironmentVariableKeys($envs); foreach ($envs as $env) { $key = $this->validatedBuildtimeEnvironmentVariableKey((string) $env->key, 'the generated Dockerfile'); $argsToInsert->push("ARG {$key}"); @@ -5148,6 +5201,7 @@ COPY ./nginx.conf /etc/nginx/conf.d/default.conf"); ->withoutBuildpackControlVariables() ->where('is_buildtime', true) ->get(); + $envs = $this->withoutLegacyEnvironmentVariableKeys($envs); foreach ($envs as $env) { $key = $this->validatedBuildtimeEnvironmentVariableKey((string) $env->key, 'the generated Dockerfile'); $argsToInsert->push("ARG {$key}"); @@ -5238,37 +5292,6 @@ COPY ./nginx.conf /etc/nginx/conf.d/default.conf"); return; } - $dottedKeys = $variables->keys() - ->map(fn ($key): string => (string) $key) - ->filter(fn (string $key): bool => str_contains($key, '.')); - - if ($dottedKeys->isNotEmpty()) { - $originalDockerfile = $dockerfile; - $dockerfile = $dockerfile->map(function (string $line) use ($dottedKeys): ?string { - $trimmedLine = trim($line); - - if (! str_starts_with($trimmedLine, 'ARG ') && ! str_starts_with($trimmedLine, 'ENV ')) { - return $line; - } - - [$instruction, $arguments] = explode(' ', $trimmedLine, 2); - $filteredArguments = collect(preg_split('/\s+/', $arguments)) - ->reject(function (string $argument) use ($dottedKeys): bool { - $key = str($argument)->before('=')->toString(); - - return $dottedKeys->contains($key); - }); - - if ($filteredArguments->isEmpty()) { - return null; - } - - return $instruction.' '.$filteredArguments->implode(' '); - })->filter()->values(); - - $modified = $dockerfile->all() !== $originalDockerfile->values()->all(); - } - // Generate mount strings for all secrets $mountStrings = $variables->map(function ($value, $key) { $key = $this->validatedBuildtimeEnvironmentVariableKey((string) $key, 'the generated Dockerfile'); @@ -5279,7 +5302,7 @@ COPY ./nginx.conf /etc/nginx/conf.d/default.conf"); // Add mount for the secrets hash to ensure cache invalidation $mountStrings .= ' --mount=type=secret,id=COOLIFY_BUILD_SECRETS_HASH,env=COOLIFY_BUILD_SECRETS_HASH'; - $modified ??= false; + $modified = false; $dockerfile = $dockerfile->map(function ($line) use ($mountStrings, &$modified) { $trimmed = ltrim($line); diff --git a/app/Livewire/Project/Shared/EnvironmentVariable/Show.php b/app/Livewire/Project/Shared/EnvironmentVariable/Show.php index 738c4b7ba6..2e8be77ddf 100644 --- a/app/Livewire/Project/Shared/EnvironmentVariable/Show.php +++ b/app/Livewire/Project/Shared/EnvironmentVariable/Show.php @@ -95,6 +95,11 @@ class Show extends Component public array $problematicVariables = []; + /** + * The variable was saved before exact escaping, and the container gets a different value. + */ + public bool $legacyEscapingChangesValue = false; + protected $listeners = [ 'refresh', 'compose_loaded' => '$refresh', @@ -171,6 +176,10 @@ class Show extends Component } $this->hydrateValueFields(); + $this->legacyEscapingChangesValue = $this->env instanceof ModelsEnvironmentVariable + && is_object($this->env->resourceable) + && method_exists($this->env->resourceable, 'legacyEscapingChangesValue') + && $this->env->resourceable->legacyEscapingChangesValue($this->env); $this->valuesLoaded = true; } diff --git a/app/Models/EnvironmentVariable.php b/app/Models/EnvironmentVariable.php index a89d372b22..050fc15342 100644 --- a/app/Models/EnvironmentVariable.php +++ b/app/Models/EnvironmentVariable.php @@ -78,6 +78,7 @@ class EnvironmentVariable extends BaseModel 'is_preview' => 'boolean', 'is_runtime' => 'boolean', 'is_buildtime' => 'boolean', + 'uses_legacy_escaping' => 'boolean', 'version' => 'string', 'resourceable_type' => 'string', 'resourceable_id' => 'integer', @@ -93,6 +94,8 @@ class EnvironmentVariable extends BaseModel protected $hidden = [ 'value', 'real_value', + // Internal: decides the escaping of variables saved before exact escaping. + 'uses_legacy_escaping', ]; protected static function booted() @@ -192,6 +195,10 @@ class EnvironmentVariable extends BaseModel return $this->resourceable; } + /** + * The value that the container gets: the old escaped form for variables saved before exact + * escaping, otherwise the resolved value without escaping. + */ public function realValue(): Attribute { return Attribute::make( @@ -208,7 +215,7 @@ class EnvironmentVariable extends BaseModel return $this->resolveRealValue(revealLockedSharedVariables: false); } - private function resolveRealValue(bool $revealLockedSharedVariables = true): ?string + private function resolveRealValue(bool $revealLockedSharedVariables = true, bool $escapeLegacyValue = true): ?string { if (! $this->relationLoaded('resourceable')) { $this->load('resourceable'); @@ -229,20 +236,24 @@ class EnvironmentVariable extends BaseModel $resource->load('destination.server'); } - $real_value = $this->get_real_environment_variables_internal($this->value, $resource, null, $revealLockedSharedVariables); + $value = $this->get_real_environment_variables_internal($this->value, $resource, null, $revealLockedSharedVariables); + return $value === null || ! $escapeLegacyValue || ! $this->uses_legacy_escaping ? $value : $this->legacyEscapedValue($value); + } + + /** + * The escaping used before exact escaping. + */ + private function legacyEscapedValue(string $value): string + { // Skip escaping for valid JSON objects/arrays to prevent quote corruption (see #6160) - if (json_validate($real_value) && (str_starts_with($real_value, '{') || str_starts_with($real_value, '['))) { - return $real_value; + if (json_validate($value) && (str_starts_with($value, '{') || str_starts_with($value, '['))) { + return $value; } - if ($this->is_literal || $this->is_multiline) { - $real_value = '\''.$real_value.'\''; - } else { - $real_value = escapeEnvVariables($real_value); - } - - return $real_value; + return $this->is_literal || $this->is_multiline + ? '\''.$value.'\'' + : escapeEnvVariables($value); } protected function isReallyRequired(): Attribute @@ -335,12 +346,14 @@ class EnvironmentVariable extends BaseModel /** @return array */ public function logRedactionValues(): array { - $value = $this->real_value; - if (! is_string($value) || $value === '') { + $resolvedValue = $this->resolveRealValue(escapeLegacyValue: false); + if (! is_string($resolvedValue) || $resolvedValue === '') { return []; } - $values = [$value]; + // Build-time paths still write the old escaped form, so redact both forms. + $value = $this->legacyEscapedValue($resolvedValue); + $values = [$value, $resolvedValue]; if ($this->is_multiline || $this->is_literal) { $unquoted = str_starts_with($value, "'") && str_ends_with($value, "'") ? substr($value, 1, -1) diff --git a/app/Models/Service.php b/app/Models/Service.php index 0968211790..7827c085da 100644 --- a/app/Models/Service.php +++ b/app/Models/Service.php @@ -1618,16 +1618,24 @@ class Service extends BaseModel return 3; }); foreach ($sorted as $env) { - $resolvedValue = $this->resolveSecretManagerDotenvValue($env); if (! $env->preservesLegacyComposeNull() && dockerComposeEnvironmentVariableShouldBeUnset($this->docker_compose_raw ?? $this->docker_compose, $env->key, $env->value)) { continue; } - $envs->push("{$env->key}={$resolvedValue}"); + $envs->push($this->composeEnvironmentFileLine($env)); } instant_remote_write_file($this->server, $environmentFile, $envs->implode("\n")); instant_remote_process(["mv {$environmentFile} $workdir/.env"], $this->server); } + /** + * Build a KEY="value" line for the service .env file, which Docker Compose reads. + * Only plain values allow $VAR interpolation, so $SERVICE_* references keep working. + */ + public function composeEnvironmentFileLine(EnvironmentVariable $environmentVariable): string + { + return $environmentVariable->key.'='.$this->resolveSecretManagerDotenvValue($environmentVariable); + } + public function parse(bool $isNew = false): Collection { if ((int) $this->compose_parsing_version >= 3) { diff --git a/app/Services/ServerTransfer/ServerTransferExporter.php b/app/Services/ServerTransfer/ServerTransferExporter.php index 0d65f0223d..d31fee36c8 100644 --- a/app/Services/ServerTransfer/ServerTransferExporter.php +++ b/app/Services/ServerTransfer/ServerTransferExporter.php @@ -863,6 +863,7 @@ class ServerTransferExporter 'is_shown_once' => (bool) $variable->is_shown_once, 'is_shared' => (bool) $variable->is_shared, 'is_required' => (bool) ($variable->is_required ?? false), + 'uses_legacy_escaping' => (bool) $variable->uses_legacy_escaping, 'comment' => $variable->comment, 'order' => $variable->order, ]; diff --git a/app/Services/ServerTransfer/ServerTransferImporter.php b/app/Services/ServerTransfer/ServerTransferImporter.php index c8af0ab002..3d91ee783d 100644 --- a/app/Services/ServerTransfer/ServerTransferImporter.php +++ b/app/Services/ServerTransfer/ServerTransferImporter.php @@ -1031,6 +1031,8 @@ class ServerTransferImporter 'is_shown_once' => (bool) data_get($variable, 'is_shown_once', false), 'is_shared' => (bool) data_get($variable, 'is_shared', false), 'is_required' => (bool) data_get($variable, 'is_required', false), + // Exports from older versions have no flag; their values use the old escaping. + 'uses_legacy_escaping' => (bool) data_get($variable, 'uses_legacy_escaping', true), 'comment' => data_get($variable, 'comment'), 'order' => data_get($variable, 'order'), 'resourceable_type' => $resource->getMorphClass(), diff --git a/app/Traits/HasSecretManager.php b/app/Traits/HasSecretManager.php index 45a1d8eb3d..dd9195c9af 100644 --- a/app/Traits/HasSecretManager.php +++ b/app/Traits/HasSecretManager.php @@ -3,8 +3,13 @@ namespace App\Traits; use App\Exceptions\RemoteSecretException; +use App\Models\Application; use App\Models\EnvironmentVariable; use App\Models\SecretManagerLink; +use App\Models\Service; +use App\Models\StandaloneDragonfly; +use App\Models\StandaloneKeydb; +use App\Models\StandaloneRedis; use App\Support\RemoteSecretReferences; use App\Support\RemoteSecretValueFormatter; use Illuminate\Database\Eloquent\Model; @@ -69,7 +74,9 @@ trait HasSecretManager return RemoteSecretValueFormatter::dotenv($value); } - return $this->formatLocalEnvironmentVariableValue($environmentVariable, $value); + return $value === null ? null : ($this->useExactEscaping($environmentVariable) + ? escapeComposeEnvFileValue($value, $this->environmentVariableAllowsInterpolation($environmentVariable, $value)) + : $this->legacyFormatEnvironmentVariableValue($environmentVariable, $value)); } /** @@ -92,9 +99,16 @@ trait HasSecretManager */ public function formatComposeFileValue(EnvironmentVariable $environmentVariable, string $value): string { - return $this->environmentVariableUsesSecretManager($environmentVariable) - ? RemoteSecretValueFormatter::composeFile($value) - : $value; + if ($this->environmentVariableUsesSecretManager($environmentVariable)) { + return RemoteSecretValueFormatter::composeFile($value); + } + + if ($environmentVariable->key === 'REDIS_PASSWORD' && $this->useExactEscaping($environmentVariable) + && ($this instanceof StandaloneRedis || $this instanceof StandaloneKeydb || $this instanceof StandaloneDragonfly)) { + return escapeDollarSign($value); + } + + return $this->composeCommandValue($environmentVariable, $value); } /** @@ -119,6 +133,46 @@ trait HasSecretManager return null; } + if (! $this->useExactEscaping($environmentVariable)) { + return $this->legacyFormatEnvironmentVariableValue($environmentVariable, $value); + } + + return $this->environmentVariableAllowsInterpolation($environmentVariable, $value) + ? $value + : escapeDollarSign($value); + } + + /** + * Decides the escaping of a variable for all paths (service .env, database environment, + * Redis password). A variable saved before exact escaping keeps the old escaping, so its + * container gets the same value as before the upgrade. Saving the variable again switches + * it to exact escaping. + */ + public function useExactEscaping(EnvironmentVariable $environmentVariable): bool + { + return ! $environmentVariable->uses_legacy_escaping; + } + + /** + * A variable value for a Docker Compose command or healthcheck. Compose interpolates `$` there + * too, so the value gets the same `$` escaping as in the environment list, and the command uses + * the same value as the container. + */ + public function composeCommandValue(EnvironmentVariable $environmentVariable, string $value): string + { + if (! $this->useExactEscaping($environmentVariable) || $this->environmentVariableAllowsInterpolation($environmentVariable, $value)) { + return $value; + } + + return escapeDollarSign($value); + } + + /** + * The escaping used before exact escaping. Variables saved before the upgrade keep it, + * so their containers get the same values as before. + */ + public function legacyFormatEnvironmentVariableValue(EnvironmentVariable $environmentVariable, string $value): string + { if (json_validate($value) && (str_starts_with($value, '{') || str_starts_with($value, '['))) { return $value; } @@ -128,6 +182,55 @@ trait HasSecretManager : escapeEnvVariables($value); } + /** + * Whether the container gets a different value than the saved value, because this variable + * uses the old escaping. Secret manager values are not fetched here, so they never show it. + */ + public function legacyEscapingChangesValue(EnvironmentVariable $environmentVariable): bool + { + if ($this->useExactEscaping($environmentVariable)) { + return false; + } + + $value = $this->resolvedEnvironmentVariableValue($environmentVariable); + + if (blank($value) || RemoteSecretReferences::containsReference($value)) { + return false; + } + + $isRedisPassword = $environmentVariable->key === 'REDIS_PASSWORD' + && ($this instanceof StandaloneRedis || $this instanceof StandaloneKeydb || $this instanceof StandaloneDragonfly); + + if ($isRedisPassword && str_contains($value, '$')) { + return true; + } + + if (json_validate($value) && (str_starts_with($value, '{') || str_starts_with($value, '['))) { + return str_contains($value, '$') || (! $this instanceof Application && str_contains($value, "\n")); + } + + if ($environmentVariable->is_literal || $environmentVariable->is_multiline) { + // Service and application .env files strip the old single quotes; database environment lists keep them. + return ($this instanceof Service || $this instanceof Application) ? str_contains($value, "'") : true; + } + + return escapeEnvVariables($value) !== $value + || (($this instanceof Service || $this instanceof Application) && str_contains($value, ' #')); + } + + /** + * Only plain values allow Compose $VAR interpolation. + */ + public function environmentVariableAllowsInterpolation(EnvironmentVariable $environmentVariable, string $value): bool + { + $isJson = json_validate($value) && in_array(ltrim($value)[0] ?? '', ['{', '['], true); + + return ! $isJson + && ! $environmentVariable->is_literal + && ! $environmentVariable->is_multiline + && ! $this->environmentVariableUsesSecretManager($environmentVariable); + } + public function resolveSecretManagerEnvironmentVariableValue(EnvironmentVariable $environmentVariable): ?string { $value = $this->resolvedEnvironmentVariableValue($environmentVariable); diff --git a/bootstrap/helpers/docker.php b/bootstrap/helpers/docker.php index e7a39342f4..3e047b2079 100644 --- a/bootstrap/helpers/docker.php +++ b/bootstrap/helpers/docker.php @@ -1912,6 +1912,17 @@ function escapeBashEnvValue(?string $value): string return "'{$escaped}'"; } +function escapeComposeEnvFileValue(?string $value, bool $allowInterpolation = false): string +{ + $escaped = str_replace(['\\', '"'], ['\\\\', '\\"'], $value ?? ''); + + if (! $allowInterpolation) { + $escaped = str_replace('$', '\\$', $escaped); + } + + return '"'.$escaped.'"'; +} + /** * Escape a value for bash double-quoted strings (allows $VAR expansion) * diff --git a/database/migrations/2026_09_27_154836_add_uses_legacy_escaping_to_environment_variables_table.php b/database/migrations/2026_09_27_154836_add_uses_legacy_escaping_to_environment_variables_table.php new file mode 100644 index 0000000000..dae1855dd5 --- /dev/null +++ b/database/migrations/2026_09_27_154836_add_uses_legacy_escaping_to_environment_variables_table.php @@ -0,0 +1,38 @@ +boolean('uses_legacy_escaping')->default(true); + }); + + Schema::table('environment_variables', function (Blueprint $table) { + $table->boolean('uses_legacy_escaping')->default(false)->change(); + }); + } + + public function down(): void + { + if (! Schema::hasColumn('environment_variables', 'uses_legacy_escaping')) { + return; + } + + Schema::table('environment_variables', function (Blueprint $table) { + $table->dropColumn('uses_legacy_escaping'); + }); + } +}; diff --git a/resources/views/livewire/project/shared/environment-variable/show.blade.php b/resources/views/livewire/project/shared/environment-variable/show.blade.php index 3e05e533a6..b3c5977341 100644 --- a/resources/views/livewire/project/shared/environment-variable/show.blade.php +++ b/resources/views/livewire/project/shared/environment-variable/show.blade.php @@ -184,6 +184,15 @@ @endif + @if ($legacyEscapingChangesValue) + + This variable was saved before Coolify passed exact values, so the container gets a + changed value (for example, with extra backslashes or quotes). To pass the exact value, + delete this variable and add it again, then restart. If a database already uses this + value as a password, update the password in the database too. + + @endif + @if ($is_shared) @endif diff --git a/tests/Feature/ApplicationDeploymentControlVarFilteringTest.php b/tests/Feature/ApplicationDeploymentControlVarFilteringTest.php index 6099c44e7e..7ec9e37e37 100644 --- a/tests/Feature/ApplicationDeploymentControlVarFilteringTest.php +++ b/tests/Feature/ApplicationDeploymentControlVarFilteringTest.php @@ -550,84 +550,88 @@ it('keeps the original sourced environment path when build-time keys are shell s ->not->toContain(ApplicationDeploymentJob::BUILD_TIME_ENV_LAUNCHER_PATH); }); -it('uses BuildKit secrets for dotted Nixpacks variables instead of invalid Dockerfile expansion', function () { +it('skips dotted keys from the Nixpacks plan without failing the deployment', function () { + [$application, $server] = makeDeploymentControlVarFixture([ + 'build_pack' => 'nixpacks', + ]); + + createApplicationEnvironmentVariable($application, [ + 'key' => 'spring.profiles.active', + 'value' => 'prod', + ]); + + [$job, $reflection] = makeControlVarFilteringJob($application->fresh(), $server, [ + 'saved_outputs' => collect([ + 'nixpacks_plan' => json_encode([ + 'variables' => [ + 'X.VALUE' => 'dotted-from-toml', + 'SAFE_FROM_TOML' => 'ok-from-toml', + 'NIXPACKS_NODE_VERSION' => '22', + ], + ]), + 'nixpacks_type' => 'node', + ]), + ]); + + invokeDeploymentJobMethod($job, $reflection, 'generate_nixpacks_confs'); + + $variables = collect(readDeploymentJobProperty($job, $reflection, 'nixpacks_plan_json')->get('variables')); + + expect($variables->has('SAFE_FROM_TOML'))->toBeTrue(); + expect($variables->has('X.VALUE'))->toBeFalse(); + expect($variables->has('spring.profiles.active'))->toBeFalse(); + + $logs = implode("\n", $job->recordedLogEntries); + + expect($logs) + ->toContain('Build-time variable X.VALUE has a dot in its name, which Nixpacks cannot pass to the build') + ->toContain('Build-time variable spring.profiles.active has a dot in its name') + ->toContain('Suggested name: X_VALUE') + ->toContain('Available at Buildtime') + ->not->toContain('dotted-from-toml') + ->not->toContain('prod'); +}); + +it('still allows underscore Nixpacks plan keys through generate_build_env_variables', 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", + 'variables' => [ + 'SAFE_FROM_TOML' => 'ok-from-toml', + 'NIXPACKS_NODE_VERSION' => '22', + ], ]), ]); 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'); + expect(readDeploymentJobProperty($job, $reflection, 'dockerSecretsSupported'))->toBeFalse(); }); -it('rejects dotted Nixpacks variables when Docker build secrets are unavailable', function () { +it('mounts underscore nixpacks secrets on RUN lines when build secrets are enabled', function () { [$application, $server] = makeDeploymentControlVarFixture([ 'build_pack' => 'nixpacks', ]); [$job, $reflection] = makeControlVarFilteringJob($application, $server, [ - 'dockerBuildkitSupported' => true, - 'dockerSecretsAvailable' => false, + 'env_args' => collect(['SAFE_FROM_TOML' => 'ok-from-toml']), 'nixpacks_plan_json' => collect([ - 'variables' => [ - 'X.VALUE' => 'dotted-buildtime-ok', - 'ANOTHER.DOTTED.VALUE' => 'also-dotted', - ], + 'variables' => ['SAFE_FROM_TOML' => 'ok-from-toml'], ]), - ]); - - 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', + 'build_secrets' => '--secret id=SAFE_FROM_TOML,env=SAFE_FROM_TOML', 'saved_outputs' => collect([ - 'dockerfile_content' => "FROM alpine\nARG X.VALUE=default\nENV X.VALUE=\$X.VALUE", + 'dockerfile_content' => "FROM alpine\nARG SAFE_FROM_TOML\nRUN printenv SAFE_FROM_TOML", ]), ]); invokeDeploymentJobMethod($job, $reflection, 'modify_dockerfile_for_secrets', '/artifacts/test-app/.nixpacks/Dockerfile'); expect($job->writtenArtifacts['/artifacts/test-app/.nixpacks/Dockerfile']) - ->not->toContain('X.VALUE'); + ->toContain('RUN --mount=type=secret,id=SAFE_FROM_TOML,env=SAFE_FROM_TOML') + ->toContain('printenv SAFE_FROM_TOML'); }); it('skips unsafe reserved Nixpacks plan variable keys before validation', function (string $key) { @@ -976,6 +980,43 @@ it('filters buildpack control vars from dockerfile arg injection', function () { expect($job->writtenDockerfile)->not->toContain('ARG RAILPACK_NODE_VERSION='); }); +it('passes dotted keys as Docker build secrets for Dockerfile builds', function () { + [$application, $server] = makeDeploymentControlVarFixture([ + 'build_pack' => 'dockerfile', + ]); + + [$job, $reflection] = makeControlVarFilteringJob($application, $server, [ + 'dockerSecretsSupported' => true, + ]); + + invokeDeploymentJobMethod($job, $reflection, 'generate_build_secrets', collect([ + 'DOTTED.USER' => 'df-dotted', + 'SAFE' => 'ok', + ])); + + expect(readDeploymentJobProperty($job, $reflection, 'build_secrets')) + ->toContain('id=DOTTED.USER,env=DOTTED.USER') + ->toContain('id=SAFE,env=SAFE'); +}); + +it('keeps dotted dockerfile build args when secrets are not used', function () { + [$application, $server] = makeDeploymentControlVarFixture(); + + createApplicationEnvironmentVariable($application, [ + 'key' => 'DOTTED.USER', + 'value' => 'df-dotted', + ]); + + [$job, $reflection] = makeControlVarFilteringJob($application, $server); + + invokeDeploymentJobMethod($job, $reflection, 'generate_env_variables'); + + /** @var Collection $envArgs */ + $envArgs = readDeploymentJobProperty($job, $reflection, 'env_args'); + + expect($envArgs->get('DOTTED.USER'))->toBe('df-dotted'); +}); + it('does not write environment values into generated Dockerfile ARG declarations', function () { [$application, $server] = makeDeploymentControlVarFixture(); createApplicationEnvironmentVariable($application, [ @@ -1075,9 +1116,9 @@ it('rejects existing variable names that would break the .env file or build comm expect(fn () => invokeDeploymentJobMethod($job, $reflection, 'validateDeploymentEnvironmentVariableKeys', $buildsImage)) ->toThrow(DeploymentException::class, 'Invalid environment variable name from the deployment environment'); })->with([ - '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], + 'runtime-only name with =' => ['A=B', false, false], + 'runtime-only name with a newline' => ["A\nB", false, false], + 'build-time name with shell characters' => ['my;var', true, true], ]); it('does not fail restarts or deployments that reuse an image on invalid build-time names', function (string $buildPack) { @@ -1182,22 +1223,68 @@ it('still rejects Docker image variable names that would break the .env file', f ->toThrow(DeploymentException::class, 'Invalid environment variable name from the deployment environment'); }); -it('fails with a clear message for invalid build-time names when the deployment builds an image', function (string $buildPack) { - [$application, $server] = makeDeploymentControlVarFixture(['build_pack' => $buildPack]); +it('skips existing build-time variables whose names new variables cannot use', function () { + [$application, $server] = makeDeploymentControlVarFixture([ + 'build_pack' => 'dockerfile', + ]); $environmentVariable = createApplicationEnvironmentVariable($application, [ 'key' => 'SAFE_KEY', - 'value' => 'secret', + 'value' => 'legacy-value', + 'is_buildtime' => true, + 'is_runtime' => true, + ]); + createApplicationEnvironmentVariable($application, [ + 'key' => 'VALID_KEY', + 'value' => 'valid-value', + 'is_buildtime' => true, + 'is_runtime' => true, + ]); + // Names like my-var were accepted before the current rules; the model no longer allows them. + DB::table('environment_variables')->where('id', $environmentVariable->id)->update(['key' => 'my-var']); + + [$job, $reflection] = makeControlVarFilteringJob($application->fresh(), $server); + invokeDeploymentJobMethod($job, $reflection, 'validateDeploymentEnvironmentVariableKeys'); + + expect(collect($job->recordedLogEntries)->implode("\n")) + ->toContain('Build-time variable my-var uses a name that new variables cannot use. It is skipped during the build, but is still passed to the container.') + ->toContain('Suggested name: my_var') + ->not->toContain('legacy-value'); + + /** @var Collection $buildtimeEnvs */ + $buildtimeEnvs = invokeDeploymentJobMethod($job, $reflection, 'generate_buildtime_environment_variables'); + expect($buildtimeEnvs->contains(fn (string $env) => str_starts_with($env, 'my-var=')))->toBeFalse(); + expect($buildtimeEnvs->contains(fn (string $env) => str_starts_with($env, 'VALID_KEY=')))->toBeTrue(); + + invokeDeploymentJobMethod($job, $reflection, 'generate_env_variables'); + $envArgs = readDeploymentJobProperty($job, $reflection, 'env_args'); + expect($envArgs->has('my-var'))->toBeFalse(); + expect($envArgs->get('VALID_KEY'))->not->toBeNull(); + + /** @var Collection $runtimeEnvs */ + $runtimeEnvs = invokeDeploymentJobMethod($job, $reflection, 'generate_runtime_environment_variables'); + expect($runtimeEnvs->contains(fn (string $env) => str_starts_with($env, 'my-var=')))->toBeTrue(); +}); + +it('skips existing build-time variables whose names new variables cannot use from Railpack builds', function () { + [$application, $server] = makeDeploymentControlVarFixture([ + 'build_pack' => 'railpack', + ]); + $environmentVariable = createApplicationEnvironmentVariable($application, [ + 'key' => 'SAFE_KEY', + 'value' => 'legacy-value', 'is_buildtime' => true, ]); DB::table('environment_variables')->where('id', $environmentVariable->id)->update(['key' => 'my-var']); + [$job, $reflection] = makeControlVarFilteringJob($application->fresh(), $server, [ + 'build_pack' => 'railpack', + 'branch' => 'main', + ]); - [$job, $reflection] = makeControlVarFilteringJob($application->fresh(), $server, ['build_pack' => $buildPack]); + /** @var Collection $variables */ + $variables = invokeDeploymentJobMethod($job, $reflection, 'railpack_build_variables'); - 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'); -})->with(['dockerfile', 'nixpacks', 'static', 'railpack', 'dockercompose']); + expect($variables->has('my-var'))->toBeFalse(); +}); it('does not pass build-time variables to the helper container of Docker image deployments', function () { [$application, $server] = makeDeploymentControlVarFixture(['build_pack' => 'dockerimage']); @@ -1378,6 +1465,39 @@ it('builds railpack variables from generic buildtime vars railpack vars and cool expect($variables->has('RUNTIME_ONLY'))->toBeFalse(); }); +it('skips dotted keys from Railpack build variables without failing the deployment', function () { + [$application, $server] = makeDeploymentControlVarFixture([ + 'build_pack' => 'railpack', + ]); + + createApplicationEnvironmentVariable($application, [ + 'key' => 'APP_ENV', + 'value' => 'production', + 'is_runtime' => false, + 'is_buildtime' => true, + ]); + createApplicationEnvironmentVariable($application, [ + 'key' => 'spring.profiles.active', + 'value' => 'prod', + 'is_runtime' => true, + 'is_buildtime' => true, + ]); + + [$job, $reflection] = makeControlVarFilteringJob($application->fresh(), $server, [ + 'build_pack' => 'railpack', + 'branch' => 'main', + ]); + + /** @var Collection $variables */ + $variables = invokeDeploymentJobMethod($job, $reflection, 'railpack_build_variables'); + + expect($variables->get('APP_ENV'))->toBe('production'); + expect($variables->has('spring.profiles.active'))->toBeFalse(); + expect(implode("\n", $job->recordedLogEntries)) + ->toContain('Build-time variable spring.profiles.active has a dot in its name, which Railpack cannot pass to the build') + ->toContain('Suggested name: spring_profiles_active'); +}); + it('builds preview railpack variables without leaking stale nixpacks vars', function () { [$application, $server] = makeDeploymentControlVarFixture([ 'build_pack' => 'railpack', @@ -1430,6 +1550,237 @@ it('builds preview railpack variables without leaking stale nixpacks vars', func expect($variables->has('PREVIEW_RUNTIME_ONLY'))->toBeFalse(); }); +it('writes compose-safe runtime env files for user values', function () { + [$application, $server] = makeDeploymentControlVarFixture(); + + createApplicationEnvironmentVariable($application, [ + 'key' => 'QUOTED_VALUE', + 'value' => 'hello world; quotes" \'$`', + 'is_literal' => true, + 'is_runtime' => true, + 'is_buildtime' => false, + ]); + createApplicationEnvironmentVariable($application, [ + 'key' => 'EXPAND_ME', + 'value' => '$BOTH_PHASES', + 'is_literal' => false, + 'is_runtime' => true, + 'is_buildtime' => true, + ]); + createApplicationEnvironmentVariable($application, [ + 'key' => 'BOTH_PHASES', + 'value' => 'both-phases-value', + 'is_literal' => true, + 'is_runtime' => true, + 'is_buildtime' => true, + ]); + createApplicationEnvironmentVariable($application, [ + 'key' => 'INJECT_CMD', + 'value' => '$(touch /tmp/coolify-value-injection)', + 'is_literal' => true, + 'is_runtime' => true, + 'is_buildtime' => true, + ]); + createApplicationEnvironmentVariable($application, [ + 'key' => 'EDGE_QUOTES', + 'value' => "'keep these quotes'", + 'is_literal' => true, + 'is_runtime' => true, + 'is_buildtime' => false, + ]); + createApplicationEnvironmentVariable($application, [ + 'key' => 'MULTILINE_VALUE', + 'value' => "\$BOTH_PHASES\nsecond line", + 'is_literal' => false, + 'is_multiline' => true, + 'is_runtime' => true, + 'is_buildtime' => false, + ]); + + [$job, $reflection] = makeControlVarFilteringJob($application, $server); + + /** @var Collection $runtimeEnvs */ + $runtimeEnvs = invokeDeploymentJobMethod($job, $reflection, 'generate_runtime_environment_variables'); + + expect($runtimeEnvs) + ->toContain('QUOTED_VALUE='.escapeComposeEnvFileValue('hello world; quotes" \'$`')) + ->toContain('INJECT_CMD='.escapeComposeEnvFileValue('$(touch /tmp/coolify-value-injection)')) + ->toContain('EXPAND_ME='.escapeComposeEnvFileValue('$BOTH_PHASES', allowInterpolation: true)) + ->toContain('EDGE_QUOTES='.escapeComposeEnvFileValue("'keep these quotes'")) + ->toContain('MULTILINE_VALUE='.escapeComposeEnvFileValue("\$BOTH_PHASES\nsecond line")); +}); + +it('writes compose-safe preview runtime env files for user values', function () { + [$application, $server] = makeDeploymentControlVarFixture(); + + createApplicationEnvironmentVariable($application, [ + 'key' => 'PREVIEW_QUOTED_VALUE', + 'value' => 'preview quotes" \'$`', + 'is_preview' => true, + 'is_literal' => true, + 'is_runtime' => true, + 'is_buildtime' => false, + ]); + + [$job, $reflection] = makeControlVarFilteringJob($application->fresh(), $server, [ + 'pull_request_id' => 42, + ]); + + /** @var Collection $runtimeEnvs */ + $runtimeEnvs = invokeDeploymentJobMethod($job, $reflection, 'generate_runtime_environment_variables'); + + expect($runtimeEnvs) + ->toContain('PREVIEW_QUOTED_VALUE='.escapeComposeEnvFileValue('preview quotes" \'$`')); +}); + +it('writes compose-safe production fallbacks to preview runtime env files', function () { + [$application, $server] = makeDeploymentControlVarFixture(); + + createApplicationEnvironmentVariable($application, [ + 'key' => 'PREVIEW_TRIGGER', + 'value' => 'preview-value', + 'is_preview' => true, + 'is_literal' => false, + 'is_runtime' => true, + 'is_buildtime' => false, + ]); + createApplicationEnvironmentVariable($application, [ + 'key' => 'FALLBACK_QUOTED_VALUE', + 'value' => 'fallback quotes" \'$`', + 'is_literal' => true, + 'is_runtime' => true, + 'is_buildtime' => false, + ]); + $application->environment_variables_preview() + ->where('key', 'FALLBACK_QUOTED_VALUE') + ->delete(); + + [$job, $reflection] = makeControlVarFilteringJob($application->fresh(), $server, [ + 'pull_request_id' => 42, + ]); + + /** @var Collection $runtimeEnvs */ + $runtimeEnvs = invokeDeploymentJobMethod($job, $reflection, 'generate_runtime_environment_variables'); + + expect($runtimeEnvs) + ->toContain('FALLBACK_QUOTED_VALUE='.escapeComposeEnvFileValue('fallback quotes" \'$`')); +}); + +it('writes compose-safe runtime env values for remote secret references', function () { + [$application, $server] = makeDeploymentControlVarFixture(); + + createApplicationEnvironmentVariable($application, [ + 'key' => 'SECRET_TOKEN', + 'value' => '{{vault.API_TOKEN}}', + 'is_literal' => false, + 'is_runtime' => true, + 'is_buildtime' => false, + ]); + + $secret = "secret\$value'quoted\""; + [$job, $reflection] = makeControlVarFilteringJob($application, $server, [ + 'remote_secrets_cache' => ['API_TOKEN' => $secret], + ]); + + /** @var Collection $runtimeEnvs */ + $runtimeEnvs = invokeDeploymentJobMethod($job, $reflection, 'generate_runtime_environment_variables'); + + expect($runtimeEnvs)->toContain('SECRET_TOKEN='.escapeComposeEnvFileValue($secret)); +}); + +it('keeps the old runtime env format for variables saved before exact escaping', function () { + [$application, $server] = makeDeploymentControlVarFixture(); + + foreach ([ + ['key' => 'LEGACY_PLAIN', 'value' => 'a\'b"c\\d', 'is_literal' => false], + ['key' => 'LEGACY_LITERAL', 'value' => 'x$y', 'is_literal' => true], + ] as $attributes) { + $variable = createApplicationEnvironmentVariable($application, [...$attributes, 'is_runtime' => true, 'is_buildtime' => false]); + $variable->uses_legacy_escaping = true; + $variable->save(); + } + createApplicationEnvironmentVariable($application, [ + 'key' => 'EXACT_PLAIN', + 'value' => 'a\'b"c\\d', + 'is_literal' => false, + 'is_runtime' => true, + 'is_buildtime' => false, + ]); + + [$job, $reflection] = makeControlVarFilteringJob($application->fresh(), $server); + + /** @var Collection $runtimeEnvs */ + $runtimeEnvs = invokeDeploymentJobMethod($job, $reflection, 'generate_runtime_environment_variables'); + + expect($runtimeEnvs) + ->toContain('LEGACY_PLAIN=a\\\'b\\"c\\\\d') + ->toContain("LEGACY_LITERAL='x\$y'") + ->toContain('EXACT_PLAIN='.escapeComposeEnvFileValue('a\'b"c\\d', allowInterpolation: true)); +}); + +it('keeps the old runtime env format for preview variables saved before exact escaping', function () { + [$application, $server] = makeDeploymentControlVarFixture(); + + $variable = createApplicationEnvironmentVariable($application, [ + 'key' => 'LEGACY_PREVIEW', + 'value' => "array:'self'", + 'is_preview' => true, + 'is_runtime' => true, + 'is_buildtime' => false, + ]); + $variable->uses_legacy_escaping = true; + $variable->save(); + + [$job, $reflection] = makeControlVarFilteringJob($application->fresh(), $server, ['pull_request_id' => 42]); + + /** @var Collection $runtimeEnvs */ + $runtimeEnvs = invokeDeploymentJobMethod($job, $reflection, 'generate_runtime_environment_variables'); + + expect($runtimeEnvs)->toContain("LEGACY_PREVIEW=array:\\'self\\'"); +}); + +it('resolves exact runtime env values without loading each variable resource', function () { + [$application, $server] = makeDeploymentControlVarFixture(); + + foreach (['FIRST_EXACT', 'SECOND_EXACT'] as $key) { + createApplicationEnvironmentVariable($application, [ + 'key' => $key, + 'value' => "array:'self'", + 'is_runtime' => true, + 'is_buildtime' => false, + ]); + } + + [$job, $reflection] = makeControlVarFilteringJob($application->fresh(), $server); + + DB::enableQueryLog(); + /** @var Collection $runtimeEnvs */ + $runtimeEnvs = invokeDeploymentJobMethod($job, $reflection, 'generate_runtime_environment_variables'); + $applicationLookups = collect(DB::getQueryLog()) + ->filter(fn (array $query) => str($query['query'])->contains('from "applications"')); + DB::disableQueryLog(); + + expect($runtimeEnvs)->toContain('FIRST_EXACT='.escapeComposeEnvFileValue("array:'self'", allowInterpolation: true)) + ->and($applicationLookups)->toBeEmpty(); +}); + +it('keeps dollars literal in runtime JSON values', function () { + [$application, $server] = makeDeploymentControlVarFixture(); + + createApplicationEnvironmentVariable($application, [ + 'key' => 'JSON_CONFIG', + 'value' => '{"price":"$5"}', + 'is_runtime' => true, + 'is_buildtime' => false, + ]); + + [$job, $reflection] = makeControlVarFilteringJob($application->fresh(), $server); + + /** @var Collection $runtimeEnvs */ + $runtimeEnvs = invokeDeploymentJobMethod($job, $reflection, 'generate_runtime_environment_variables'); + + expect($runtimeEnvs)->toContain('JSON_CONFIG='.escapeComposeEnvFileValue('{"price":"$5"}')); +}); it('omits only unset Compose overrides from production and preview runtime files', function (int $pullRequestId) { Server::flushIdentityMap(); [$application, $server] = makeDeploymentControlVarFixture([ @@ -1444,8 +1795,8 @@ it('omits only unset Compose overrides from production and preview runtime files [$job, $reflection] = makeControlVarFilteringJob($application, $server, ['pull_request_id' => $pullRequestId]); $variables = invokeDeploymentJobMethod($job, $reflection, 'generate_runtime_environment_variables'); - expect($variables->contains('INHERITED='))->toBeFalse() - ->and($variables->contains('EMPTY='))->toBeTrue(); + expect($variables->contains(fn (string $line) => str_starts_with($line, 'INHERITED=')))->toBeFalse() + ->and($variables->contains('EMPTY='.escapeComposeEnvFileValue('')))->toBeTrue(); })->with([0, 42]); it('includes migrated null Compose overrides in production and preview runtime files', function (int $pullRequestId) { @@ -1461,5 +1812,5 @@ it('includes migrated null Compose overrides in production and preview runtime f } [$job, $reflection] = makeControlVarFilteringJob($application, $server, ['pull_request_id' => $pullRequestId]); - expect(invokeDeploymentJobMethod($job, $reflection, 'generate_runtime_environment_variables')->contains('LEGACY='))->toBeTrue(); + expect(invokeDeploymentJobMethod($job, $reflection, 'generate_runtime_environment_variables')->contains('LEGACY='.escapeComposeEnvFileValue('')))->toBeTrue(); })->with([0, 42]); diff --git a/tests/Feature/ApplicationNetworkNameVariableTest.php b/tests/Feature/ApplicationNetworkNameVariableTest.php index 4e1cebe8a2..b3aa59f45c 100644 --- a/tests/Feature/ApplicationNetworkNameVariableTest.php +++ b/tests/Feature/ApplicationNetworkNameVariableTest.php @@ -209,8 +209,8 @@ YAML); $env = composeNetworkDeploymentEnv($application); - expect($env)->toContain('SHARED_NETWORK=traefik_public') - ->and($env)->toContain('OTHER_NETWORK=edge_net'); + expect($env)->toContain('SHARED_NETWORK='.escapeComposeEnvFileValue('traefik_public', allowInterpolation: true)) + ->and($env)->toContain('OTHER_NETWORK='.escapeComposeEnvFileValue('edge_net', allowInterpolation: true)); $buildEnv = composeNetworkDeploymentEnv($application, method: 'generate_buildtime_environment_variables')->implode("\n"); @@ -231,5 +231,5 @@ YAML); $application->parse(); - expect(composeNetworkDeploymentEnv($application, 7))->toContain('SHARED_NETWORK=traefik_public'); + expect(composeNetworkDeploymentEnv($application, 7))->toContain('SHARED_NETWORK='.escapeComposeEnvFileValue('traefik_public', allowInterpolation: true)); }); diff --git a/tests/Feature/DatabaseEnvironmentVariableComposeTest.php b/tests/Feature/DatabaseEnvironmentVariableComposeTest.php new file mode 100644 index 0000000000..6bed132a35 --- /dev/null +++ b/tests/Feature/DatabaseEnvironmentVariableComposeTest.php @@ -0,0 +1,174 @@ +firstOrCreate(['id' => 0]); + + $team = Team::factory()->create(); + $server = Server::factory()->create(['team_id' => $team->id]); + $this->destination = $server->standaloneDockers()->firstOrFail(); + $project = Project::factory()->create(['team_id' => $team->id]); + $this->environment = Environment::factory()->create(['project_id' => $project->id]); +}); + +/** + * @param array $variables + * @return array + */ +function postgresComposeEnvironment(array $variables): array +{ + $database = StandalonePostgresql::forceCreate([ + 'uuid' => 'postgres-env-test', + 'name' => 'Postgres env test', + 'image' => 'postgres:16-alpine', + 'postgres_password' => 'password', + 'environment_id' => test()->environment->id, + 'destination_id' => test()->destination->id, + 'destination_type' => test()->destination->getMorphClass(), + ]); + + foreach ($variables as $variable) { + $database->runtime_environment_variables()->create([ + 'key' => $variable[0], + 'value' => $variable[1], + 'is_literal' => $variable[2] ?? false, + 'is_multiline' => $variable[3] ?? false, + ]); + } + + $action = new StartPostgresql; + $action->database = $database->fresh(); + + return (new ReflectionMethod($action, 'generate_environment_variables'))->invoke($action); +} + +test('literal database values are passed without added quotes and keep dollars literal', function () { + $environment = postgresComposeEnvironment([ + ['CSP', "array:'self',blob:,https://*", true], + ['SECRET', 'p4$$w"ord$X `id` C:\\path', true], + ]); + + expect($environment) + ->toContain("CSP=array:'self',blob:,https://*") + ->toContain('SECRET=p4$$$$w"ord$$X `id` C:\\path'); +}); + +test('plain database values are passed without backslash escaping', function () { + $environment = postgresComposeEnvironment([ + ['CSP', "array:'self',blob:,https://*"], + ['QUOTED', 'say "hi" C:\\path'], + ]); + + expect($environment) + ->toContain("CSP=array:'self',blob:,https://*") + ->toContain('QUOTED=say "hi" C:\\path'); +}); + +test('plain database values keep compose interpolation', function () { + $environment = postgresComposeEnvironment([ + ['URL', 'postgres://$USER@db'], + ]); + + expect($environment)->toContain('URL=postgres://$USER@db'); +}); + +test('multiline and json database values keep dollars literal', function () { + $environment = postgresComposeEnvironment([ + ['CERT', "line 'one'\nline \$two", false, true], + ['CONFIG', '{"price":"$5"}'], + ]); + + expect($environment) + ->toContain("CERT=line 'one'\nline \$\$two") + ->toContain('CONFIG={"price":"$$5"}'); +}); + +test('empty database values stay empty', function () { + expect(postgresComposeEnvironment([['EMPTY', null]]))->toContain('EMPTY='); +}); + +test('mariadb literal values are passed without added quotes', function () { + $database = StandaloneMariadb::forceCreate([ + 'uuid' => 'mariadb-env-test', + 'name' => 'MariaDB env test', + 'image' => 'mariadb:11', + 'mariadb_root_password' => 'password', + 'mariadb_password' => 'password', + 'environment_id' => $this->environment->id, + 'destination_id' => $this->destination->id, + 'destination_type' => $this->destination->getMorphClass(), + ]); + $database->runtime_environment_variables()->create([ + 'key' => 'CSP', + 'value' => "array:'self' \$HOME", + 'is_literal' => true, + ]); + + $action = new StartMariadb; + $action->database = $database->fresh(); + $environment = (new ReflectionMethod($action, 'generate_environment_variables'))->invoke($action); + + expect($environment)->toContain("CSP=array:'self' \$\$HOME"); +}); + +/** + * Compose interpolates `$` in healthchecks too, so the healthcheck must get the same value as the + * container environment. + */ +function databaseHealthcheckCredential(object $action, object $database, array $variable, string $property): string +{ + $environmentVariable = $database->runtime_environment_variables()->create([ + 'key' => $variable[0], + 'value' => $variable[1], + 'is_literal' => $variable[2] ?? false, + ]); + if ($variable[3] ?? false) { + $environmentVariable->uses_legacy_escaping = true; + $environmentVariable->save(); + } + $action->database = $database->fresh(); + (new ReflectionMethod($action, 'generate_environment_variables'))->invoke($action); + + return (new ReflectionProperty($action, $property))->getValue($action); +} + +test('database healthcheck credentials match the container environment', function (string $databaseType, array $variable, string $property, string $expected) { + [$modelClass, $actionClass, $attributes] = match ($databaseType) { + 'postgresql' => [StandalonePostgresql::class, StartPostgresql::class, ['image' => 'postgres:16-alpine', 'postgres_password' => 'password']], + 'mysql' => [StandaloneMysql::class, StartMysql::class, ['image' => 'mysql:8', 'mysql_root_password' => 'password', 'mysql_password' => 'password']], + 'clickhouse' => [StandaloneClickhouse::class, StartClickhouse::class, ['image' => 'clickhouse/clickhouse-server', 'clickhouse_admin_password' => 'password']], + }; + $database = $modelClass::forceCreate([ + 'uuid' => "{$databaseType}-healthcheck-test", + 'name' => "{$databaseType} healthcheck test", + ...$attributes, + 'environment_id' => test()->environment->id, + 'destination_id' => test()->destination->id, + 'destination_type' => test()->destination->getMorphClass(), + ]); + + expect(databaseHealthcheckCredential(new $actionClass, $database, $variable, $property))->toBe($expected); +})->with([ + 'postgres literal user' => ['postgresql', ['POSTGRES_USER', 'us$er', true], 'resolvedPostgresUser', 'us$$er'], + 'postgres plain user' => ['postgresql', ['POSTGRES_USER', 'us$er'], 'resolvedPostgresUser', 'us$er'], + 'mysql literal root password' => ['mysql', ['MYSQL_ROOT_PASSWORD', 'p4$$w', true], 'resolvedMysqlRootPassword', 'p4$$$$w'], + 'mysql legacy literal root password' => ['mysql', ['MYSQL_ROOT_PASSWORD', 'p4$$w', true, true], 'resolvedMysqlRootPassword', 'p4$$w'], + 'clickhouse literal password' => ['clickhouse', ['CLICKHOUSE_PASSWORD', 'p4$w', true], 'resolvedClickhousePassword', 'p4$$w'], + 'clickhouse plain password' => ['clickhouse', ['CLICKHOUSE_PASSWORD', 'p4$w'], 'resolvedClickhousePassword', 'p4$w'], +]); diff --git a/tests/Feature/DeploymentSensitiveCommandFailureTest.php b/tests/Feature/DeploymentSensitiveCommandFailureTest.php index 448178fd51..c6d104c9cb 100644 --- a/tests/Feature/DeploymentSensitiveCommandFailureTest.php +++ b/tests/Feature/DeploymentSensitiveCommandFailureTest.php @@ -505,7 +505,7 @@ it('keeps a literal vault reference as text when the application has no secret m $buildtime = invokeSensitiveFailureJobMethod($job, $reflection, 'generate_buildtime_environment_variables'); invokeSensitiveFailureJobMethod($job, $reflection, 'generate_env_variables'); - expect($runtime->first(fn (string $line): bool => str_starts_with($line, 'LITERAL_REF=')))->toBe("LITERAL_REF='{{vault.FOO}}'") + expect($runtime->first(fn (string $line): bool => str_starts_with($line, 'LITERAL_REF=')))->toBe('LITERAL_REF='.escapeComposeEnvFileValue('{{vault.FOO}}')) ->and($buildtime->first(fn (string $line): bool => str_starts_with($line, 'LITERAL_REF=')))->toBe("LITERAL_REF='{{vault.FOO}}'") ->and(readSensitiveFailureJobProperty($job, $reflection, 'env_args')->get('LITERAL_REF'))->toBe("'{{vault.FOO}}'") ->and(invokeSensitiveFailureJobMethod($job, $reflection, 'has_remote_buildtime_secret_references'))->toBeFalse(); @@ -531,5 +531,5 @@ it('resolves a literal vault reference when the application has a secret manager $runtime = invokeSensitiveFailureJobMethod($job, $reflection, 'generate_runtime_environment_variables'); - expect($runtime->first(fn (string $line): bool => str_starts_with($line, 'LITERAL_REF=')))->toBe("LITERAL_REF='resolved-remote-value'"); + expect($runtime->first(fn (string $line): bool => str_starts_with($line, 'LITERAL_REF=')))->toBe('LITERAL_REF='.escapeComposeEnvFileValue('resolved-remote-value')); }); diff --git a/tests/Feature/EnvironmentVariableCopyValueTest.php b/tests/Feature/EnvironmentVariableCopyValueTest.php index 36b8f96779..56ade1e2f2 100644 --- a/tests/Feature/EnvironmentVariableCopyValueTest.php +++ b/tests/Feature/EnvironmentVariableCopyValueTest.php @@ -96,6 +96,9 @@ test('copies embedded, literal and unknown references as stored', function () { test('copies literal values without .env-style quoting', function () { $env = createEnvironmentVariable(['value' => 'pa$$word', 'is_literal' => true]); + // Variables saved before exact escaping still show the quoted .env form. + $env->uses_legacy_escaping = true; + $env->save(); expect($env->real_value)->toBe("'pa\$\$word'"); assertCopiedValue($env, 'pa$$word'); diff --git a/tests/Feature/EnvironmentVariableLegacyEscapingTest.php b/tests/Feature/EnvironmentVariableLegacyEscapingTest.php new file mode 100644 index 0000000000..96382a231c --- /dev/null +++ b/tests/Feature/EnvironmentVariableLegacyEscapingTest.php @@ -0,0 +1,362 @@ +whereKey(0)->exists()) { + InstanceSettings::forceCreate(['id' => 0]); + } + + $this->team = Team::factory()->create(); + $this->user = User::factory()->create(); + $this->team->members()->attach($this->user->id, ['role' => 'owner']); + $server = Server::factory()->create(['team_id' => $this->team->id]); + $this->destination = $server->standaloneDockers()->firstOrFail(); + $project = Project::factory()->create(['team_id' => $this->team->id]); + $this->environment = Environment::factory()->create(['project_id' => $project->id]); + $this->resourceAttributes = [ + 'environment_id' => $this->environment->id, + 'destination_id' => $this->destination->id, + 'destination_type' => $this->destination->getMorphClass(), + ]; + + $this->service = Service::factory()->create([...$this->resourceAttributes, 'server_id' => $server->id]); +}); + +function legacyVariable(Model $resource, array $attributes): EnvironmentVariable +{ + $variable = $resource->environment_variables()->create($attributes); + $variable->uses_legacy_escaping = true; + $variable->save(); + + return $variable->fresh(); +} + +test('the migration marks existing variables as legacy and new variables as exact', function () { + $migration = require database_path('migrations/2026_09_27_154836_add_uses_legacy_escaping_to_environment_variables_table.php'); + $migration->down(); + + $existing = $this->service->environment_variables()->create(['key' => 'EXISTING', 'value' => "a'b"]); + + $migration->up(); + + expect($existing->fresh()->uses_legacy_escaping)->toBeTrue() + ->and($this->service->environment_variables()->create(['key' => 'NEW', 'value' => "a'b"])->fresh()->uses_legacy_escaping)->toBeFalse(); +}); + +test('changing the value or format of a legacy variable keeps legacy escaping', function (array $change) { + $variable = legacyVariable($this->service, ['key' => 'CSP', 'value' => "a'b"]); + + $variable->update($change); + + expect($variable->fresh()->uses_legacy_escaping)->toBeTrue(); +})->with([ + 'value' => [['value' => "a'c"]], + 'literal' => [['is_literal' => true]], + 'multiline' => [['is_multiline' => true]], +]); + +test('saving a legacy variable without a value or format change keeps legacy escaping', function () { + $variable = legacyVariable($this->service, ['key' => 'CSP', 'value' => "a'b"]); + + $variable->update(['value' => "a'b", 'comment' => 'note']); + + expect($variable->fresh()->uses_legacy_escaping)->toBeTrue(); +}); + +test('legacy service variables keep the old env file format', function () { + $plain = legacyVariable($this->service, ['key' => 'PLAIN', 'value' => 'a\'b"c\\d']); + $literal = legacyVariable($this->service, ['key' => 'LITERAL', 'value' => 'x$y', 'is_literal' => true]); + $empty = legacyVariable($this->service, ['key' => 'EMPTY', 'value' => null]); + + expect($this->service->composeEnvironmentFileLine($plain))->toBe('PLAIN=a\\\'b\\"c\\\\d') + ->and($this->service->composeEnvironmentFileLine($literal))->toBe("LITERAL='x\$y'") + ->and($this->service->composeEnvironmentFileLine($empty))->toBe('EMPTY='); +}); + +test('legacy database variables keep the old compose environment format', function () { + $database = StandalonePostgresql::forceCreate([ + 'uuid' => 'postgres-legacy-test', + 'name' => 'Postgres legacy test', + 'image' => 'postgres:16-alpine', + 'postgres_password' => 'password', + ...$this->resourceAttributes, + ]); + legacyVariable($database, ['key' => 'PLAIN', 'value' => "a'b"]); + legacyVariable($database, ['key' => 'LITERAL', 'value' => 'p4ssword', 'is_literal' => true]); + + $action = new StartPostgresql; + $action->database = $database->fresh(); + $environment = (new ReflectionMethod($action, 'generate_environment_variables'))->invoke($action); + + expect($environment)->toContain("PLAIN=a\\'b")->toContain("LITERAL='p4ssword'"); +}); + +test('legacy redis password variables keep the old start command', function () { + $redis = StandaloneRedis::forceCreate([ + 'uuid' => 'redis-legacy-test', + 'name' => 'Redis legacy test', + 'image' => 'redis:7-alpine', + ...$this->resourceAttributes, + ]); + legacyVariable($redis, ['key' => 'REDIS_PASSWORD', 'value' => 'p4$$word']); + + $action = new StartRedis; + $action->database = $redis->fresh(); + (new ReflectionMethod($action, 'generate_environment_variables'))->invoke($action); + + expect((new ReflectionMethod($action, 'buildStartCommand'))->invoke($action)) + ->toBe("redis-server --requirepass 'p4\$\$word' --appendonly yes"); +}); + +test('keydb column passwords keep the old start command', function () { + $keydb = StandaloneKeydb::forceCreate([ + 'uuid' => 'keydb-legacy-test', + 'name' => 'KeyDB legacy test', + 'image' => 'eqalpha/keydb:latest', + 'keydb_password' => 'p4$$word', + ...$this->resourceAttributes, + ]); + + $action = new StartKeydb; + $action->database = $keydb; + (new ReflectionMethod($action, 'generate_environment_variables'))->invoke($action); + + expect((new ReflectionMethod($action, 'buildStartCommand'))->invoke($action)) + ->toBe("keydb-server --requirepass 'p4\$\$word' --appendonly yes"); +}); + +test('saving in the editor without a value change keeps legacy escaping', function () { + $this->actingAs($this->user); + session(['currentTeam' => $this->team]); + $variable = legacyVariable($this->service, ['key' => 'CSP', 'value' => "a'b"]); + + Livewire::test(Show::class, ['env' => $variable, 'type' => 'service']) + ->call('loadValues') + ->set('comment', 'only the comment changes') + ->call('submit') + ->assertHasNoErrors(); + + expect($variable->fresh()) + ->comment->toBe('only the comment changes') + ->uses_legacy_escaping->toBeTrue(); +}); + +test('changing the value in the editor keeps legacy escaping', function () { + $this->actingAs($this->user); + session(['currentTeam' => $this->team]); + $variable = legacyVariable($this->service, ['key' => 'CSP', 'value' => "a'b"]); + + Livewire::test(Show::class, ['env' => $variable, 'type' => 'service']) + ->call('loadValues') + ->set('value', "a'c") + ->call('submit') + ->assertHasNoErrors(); + + expect($variable->fresh()->uses_legacy_escaping)->toBeTrue(); +}); + +test('instant saves keep legacy escaping', function () { + $this->actingAs($this->user); + session(['currentTeam' => $this->team]); + $variable = legacyVariable($this->service, ['key' => 'CSP', 'value' => "a'b"]); + + Livewire::test(Show::class, ['env' => $variable, 'type' => 'service']) + ->call('loadValues') + ->call('instantSave'); + + expect($variable->fresh()->uses_legacy_escaping)->toBeTrue(); +}); + +test('a variable that is deleted and added again gets exact escaping', function () { + legacyVariable($this->service, ['key' => 'CSP', 'value' => "a'b"])->delete(); + + $variable = $this->service->environment_variables()->create(['key' => 'CSP', 'value' => "a'b"]); + + expect($variable->fresh()->uses_legacy_escaping)->toBeFalse() + ->and($this->service->composeEnvironmentFileLine($variable->fresh()))->toBe('CSP="a\'b"'); +}); + +test('the editor shows the legacy escaping hint only when the container value differs from the saved value', function (bool $legacy, string $value, bool $expected) { + $this->actingAs($this->user); + session(['currentTeam' => $this->team]); + $variable = $legacy + ? legacyVariable($this->service, ['key' => 'CSP', 'value' => $value]) + : $this->service->environment_variables()->create(['key' => 'CSP', 'value' => $value]); + + $component = Livewire::test(Show::class, ['env' => $variable->fresh(), 'type' => 'service']) + ->call('loadValues') + ->assertSet('legacyEscapingChangesValue', $expected); + + $expected + ? $component->assertSee('delete this variable and add it again') + : $component->assertDontSee('delete this variable and add it again'); +})->with([ + 'legacy with a quote' => [true, "a'b", true], + 'legacy plain value' => [true, 'plain-value', false], + 'exact with a quote' => [false, "a'b", false], +]); + +/** + * Automation often sends the same values on every run, so only a real change may switch the escaping. + * + * @return array{0: array, 1: list} + */ +function apiLegacyEscapingFixture(object $test): array +{ + session(['currentTeam' => $test->team]); + $headers = ['Authorization' => 'Bearer '.$test->user->createToken('test-token', ['*'])->plainTextToken]; + $database = StandalonePostgresql::forceCreate([ + 'uuid' => 'postgres-api-legacy-test', + 'name' => 'Postgres api legacy test', + 'image' => 'postgres:16-alpine', + 'postgres_password' => 'password', + ...$test->resourceAttributes, + ]); + $application = Application::factory()->create($test->resourceAttributes); + + $test->routes = [ + 'service' => ["/api/v1/services/{$test->service->uuid}/envs", legacyVariable($test->service, ['key' => 'CSP', 'value' => "a'b"])], + 'service bulk' => ["/api/v1/services/{$test->service->uuid}/envs/bulk", legacyVariable($test->service, ['key' => 'BULK', 'value' => "a'b"])], + 'database' => ["/api/v1/databases/{$database->uuid}/envs", legacyVariable($database, ['key' => 'CSP', 'value' => "a'b"])], + 'database bulk' => ["/api/v1/databases/{$database->uuid}/envs/bulk", legacyVariable($database, ['key' => 'BULK', 'value' => "a'b"])], + 'application' => ["/api/v1/applications/{$application->uuid}/envs", legacyVariable($application, ['key' => 'CSP', 'value' => "a'b"])], + 'application bulk' => ["/api/v1/applications/{$application->uuid}/envs/bulk", legacyVariable($application, ['key' => 'BULK', 'value' => "a'b"])], + ]; + + return $headers; +} + +function patchLegacyEscapingVariable(object $test, array $headers, string $route, string $value): EnvironmentVariable +{ + [$url, $variable] = $test->routes[$route]; + $payload = ['key' => $variable->key, 'value' => $value]; + $test->withHeaders($headers) + ->patchJson($url, str_ends_with($url, '/bulk') ? ['data' => [$payload]] : $payload) + ->assertStatus(201); + + return $variable->fresh(); +} + +test('api updates with the same value keep legacy escaping', function (string $route) { + $headers = apiLegacyEscapingFixture($this); + + expect(patchLegacyEscapingVariable($this, $headers, $route, "a'b")->uses_legacy_escaping)->toBeTrue(); +})->with(['service', 'service bulk', 'database', 'database bulk', 'application', 'application bulk']); + +test('api updates with a new value keep legacy escaping', function (string $route) { + $headers = apiLegacyEscapingFixture($this); + + expect(patchLegacyEscapingVariable($this, $headers, $route, "a'c")->uses_legacy_escaping)->toBeTrue(); +})->with(['service', 'service bulk', 'database', 'database bulk', 'application', 'application bulk']); + +test('the resolved value shows what the container gets for exact and legacy variables', function (array $attributes, string $exact, string $legacy) { + $exactVariable = $this->service->environment_variables()->create(['key' => 'EXACT', ...$attributes]); + $legacyVariable = legacyVariable($this->service, ['key' => 'LEGACY', ...$attributes]); + + expect($exactVariable->fresh()->real_value)->toBe($exact) + ->and($legacyVariable->real_value)->toBe($legacy); +})->with([ + 'plain with quotes' => [['value' => 'a\'b"c\\d'], 'a\'b"c\\d', 'a\\\'b\\"c\\\\d'], + 'literal' => [['value' => 'x$y', 'is_literal' => true], 'x$y', "'x\$y'"], + 'json' => [['value' => '{"a":"b"}'], '{"a":"b"}', '{"a":"b"}'], +]); + +test('the resolved value of an exact shared reference has no escaping', function () { + SharedEnvironmentVariable::query()->create([ + 'key' => 'CSP', + 'value' => "array:'self'", + 'type' => 'environment', + 'environment_id' => $this->environment->id, + 'team_id' => $this->team->id, + ]); + $variable = $this->service->environment_variables()->create(['key' => 'CSP', 'value' => '{{environment.CSP}}']); + + expect($variable->fresh()->real_value)->toBe("array:'self'"); +}); + +test('log redaction covers the exact and the old escaped forms of a value', function () { + $variable = $this->service->environment_variables()->create(['key' => 'SECRET', 'value' => "p4ss'word"]); + + expect($variable->fresh()->logRedactionValues())->toContain("p4ss'word")->toContain("p4ss\\'word"); +}); + +test('the api shows the resolved value by escaping and never shows the legacy flag', function () { + session(['currentTeam' => $this->team]); + $token = $this->user->createToken('test-token', ['*'])->plainTextToken; + $this->service->environment_variables()->create(['key' => 'EXACT', 'value' => "a'b"]); + legacyVariable($this->service, ['key' => 'LEGACY', 'value' => "a'b"]); + + $response = $this->withHeaders(['Authorization' => "Bearer {$token}"]) + ->getJson("/api/v1/services/{$this->service->uuid}/envs") + ->assertOk(); + + $variables = collect($response->json())->keyBy('key'); + expect($variables['EXACT']['real_value'])->toBe("a'b") + ->and($variables['LEGACY']['real_value'])->toBe("a\\'b") + ->and($variables['EXACT'])->not->toHaveKey('uses_legacy_escaping') + ->and($variables['LEGACY'])->not->toHaveKey('uses_legacy_escaping'); +}); + +test('the editor shows the resolved value by escaping', function () { + $this->actingAs($this->user); + session(['currentTeam' => $this->team]); + SharedEnvironmentVariable::query()->create([ + 'key' => 'CSP', + 'value' => "array:'self'", + 'type' => 'environment', + 'environment_id' => $this->environment->id, + 'team_id' => $this->team->id, + ]); + $exact = $this->service->environment_variables()->create(['key' => 'EXACT', 'value' => '{{environment.CSP}}']); + $legacy = legacyVariable($this->service, ['key' => 'LEGACY', 'value' => '{{environment.CSP}}']); + + Livewire::test(Show::class, ['env' => $exact->fresh(), 'type' => 'service']) + ->call('loadValues') + ->assertSet('real_value', "array:'self'"); + Livewire::test(Show::class, ['env' => $legacy, 'type' => 'service']) + ->call('loadValues') + ->assertSet('real_value', "array:\\'self\\'"); +}); + +test('the legacy escaping hint follows how each resource type used the old escaping', function (string $type, array $attributes, bool $expected) { + $resource = match ($type) { + 'application' => Application::factory()->create($this->resourceAttributes), + 'database' => StandalonePostgresql::forceCreate([ + 'uuid' => 'postgres-hint-test', + 'name' => 'Postgres hint test', + 'image' => 'postgres:16-alpine', + 'postgres_password' => 'password', + ...$this->resourceAttributes, + ]), + }; + $variable = legacyVariable($resource, ['key' => 'HINT', ...$attributes]); + + expect($resource->legacyEscapingChangesValue($variable))->toBe($expected); +})->with([ + 'application plain with a quote' => ['application', ['value' => "a'b"], true], + 'application literal without a quote' => ['application', ['value' => 'x$y', 'is_literal' => true], false], + 'database literal adds quotes' => ['database', ['value' => 'x', 'is_literal' => true], true], + 'database plain value' => ['database', ['value' => 'plain'], false], +]); diff --git a/tests/Feature/RedisFamilyPasswordComposeEscapingTest.php b/tests/Feature/RedisFamilyPasswordComposeEscapingTest.php new file mode 100644 index 0000000000..103b16c03b --- /dev/null +++ b/tests/Feature/RedisFamilyPasswordComposeEscapingTest.php @@ -0,0 +1,101 @@ +firstOrCreate(['id' => 0]); + + $team = Team::factory()->create(); + $server = Server::factory()->create(['team_id' => $team->id]); + $destination = $server->standaloneDockers()->firstOrFail(); + $project = Project::factory()->create(['team_id' => $team->id]); + $environment = Environment::factory()->create(['project_id' => $project->id]); + + $this->databaseAttributes = [ + 'environment_id' => $environment->id, + 'destination_id' => $destination->id, + 'destination_type' => $destination->getMorphClass(), + ]; +}); + +/** + * Compose interpolates `$` in `command:` before it splits the string into words, + * so the password must be shell-quoted and every `$` must be doubled. + */ +function redisFamilyStartCommand(object $action, object $database): string +{ + $action->database = $database; + (new ReflectionMethod($action, 'generate_environment_variables'))->invoke($action); + + return (new ReflectionMethod($action, 'buildStartCommand'))->invoke($action); +} + +test('redis start command quotes the password and keeps dollars literal', function () { + $redis = StandaloneRedis::forceCreate([ + 'uuid' => 'redis-password-test', + 'name' => 'Redis password test', + 'image' => 'redis:7-alpine', + ...$this->databaseAttributes, + ]); + $redis->runtime_environment_variables()->create([ + 'key' => 'REDIS_PASSWORD', + 'value' => "p4\$\$w'o rd\$X", + ]); + + expect(redisFamilyStartCommand(new StartRedis, $redis)) + ->toBe("redis-server --requirepass 'p4\$\$\$\$w'\\''o rd\$\$X' --appendonly yes"); +}); + +test('keydb start command keeps dollars literal for exact password variables', function () { + $keydb = StandaloneKeydb::forceCreate([ + 'uuid' => 'keydb-password-test', + 'name' => 'KeyDB password test', + 'image' => 'eqalpha/keydb:latest', + 'keydb_password' => 'column-password', + ...$this->databaseAttributes, + ]); + $keydb->runtime_environment_variables()->create([ + 'key' => 'REDIS_PASSWORD', + 'value' => 'p4$$word', + ]); + + expect(redisFamilyStartCommand(new StartKeydb, $keydb)) + ->toBe("keydb-server --requirepass 'p4\$\$\$\$word' --appendonly yes"); +}); + +test('dragonfly start command keeps dollars literal for exact password variables', function () { + $dragonfly = StandaloneDragonfly::forceCreate([ + 'uuid' => 'dragonfly-password-test', + 'name' => 'Dragonfly password test', + 'image' => 'docker.dragonflydb.io/dragonflydb/dragonfly', + 'dragonfly_password' => 'column-password', + ...$this->databaseAttributes, + ]); + $dragonfly->runtime_environment_variables()->create([ + 'key' => 'REDIS_PASSWORD', + 'value' => 'p4$$word', + ]); + + expect(redisFamilyStartCommand(new StartDragonfly, $dragonfly)) + ->toBe("dragonfly --requirepass 'p4\$\$\$\$word'"); +}); + +test('keydb and dragonfly healthchecks use the compose-escaped password', function () { + expect(file_get_contents(__DIR__.'/../../app/Actions/Database/StartKeydb.php')) + ->toContain("'CMD', 'keydb-cli', '--pass', \$this->resolvedRedisPassword, 'ping'"); + expect(file_get_contents(__DIR__.'/../../app/Actions/Database/StartDragonfly.php')) + ->toContain("'CMD', 'redis-cli', '-a', \$this->resolvedRedisPassword, 'ping'"); +}); diff --git a/tests/Feature/SecretManagers/RemoteSecretResourceLifecycleTest.php b/tests/Feature/SecretManagers/RemoteSecretResourceLifecycleTest.php index d3846d9ae3..92038a1f55 100644 --- a/tests/Feature/SecretManagers/RemoteSecretResourceLifecycleTest.php +++ b/tests/Feature/SecretManagers/RemoteSecretResourceLifecycleTest.php @@ -149,11 +149,11 @@ test('literal service variables with remote secrets are written as valid dotenv expect(remoteSecretLifecycleWrittenEnvFile($service))->toContain('API_KEY="it\'s"'); }); -test('service variables without remote secrets keep their .env format', function () { +test('service variables without remote secrets use exact .env escaping', function () { $service = remoteSecretLifecycleService(); $service->environment_variables()->create(['key' => 'PLAIN', 'value' => 'say "hi"']); - expect(remoteSecretLifecycleWrittenEnvFile($service))->toContain('PLAIN=say \"hi\"'); + expect(remoteSecretLifecycleWrittenEnvFile($service))->toContain('PLAIN="say \"hi\""'); Http::assertNothingSent(); }); @@ -220,7 +220,7 @@ test('a literal service variable keeps reference text when no secret manager sou $service->ensureRemoteSecretsResolvable($service->environment_variables()->get()); - expect(remoteSecretLifecycleWrittenEnvFile($service))->toContain("TEMPLATE='Hello {{vault.NAME}}'"); + expect(remoteSecretLifecycleWrittenEnvFile($service))->toContain('TEMPLATE="Hello {{vault.NAME}}"'); Http::assertNothingSent(); }); diff --git a/tests/Feature/ServiceEnvironmentFileTest.php b/tests/Feature/ServiceEnvironmentFileTest.php new file mode 100644 index 0000000000..60946bda97 --- /dev/null +++ b/tests/Feature/ServiceEnvironmentFileTest.php @@ -0,0 +1,132 @@ +firstOrCreate(['id' => 0]); + + $this->team = Team::factory()->create(); + $server = Server::factory()->create(['team_id' => $this->team->id]); + $destination = $server->standaloneDockers()->firstOrFail(); + $project = Project::factory()->create(['team_id' => $this->team->id]); + $environment = Environment::factory()->create(['project_id' => $project->id]); + + $this->service = Service::factory()->create([ + 'environment_id' => $environment->id, + 'destination_id' => $destination->id, + 'destination_type' => $destination->getMorphClass(), + ]); +}); + +function serviceEnvironmentFileLine(Service $service, array $attributes): string +{ + $environmentVariable = $service->environment_variables()->create($attributes); + + return $service->composeEnvironmentFileLine($environmentVariable); +} + +test('literal service values keep quotes, dollars, and backticks exact', function () { + $line = serviceEnvironmentFileLine($this->service, [ + 'key' => 'CSP', + 'value' => 'array:\'self\',blob:,https://* "quoted" $HOME `id` C:\\path', + 'is_literal' => true, + ]); + + expect($line)->toBe('CSP="array:\'self\',blob:,https://* \\"quoted\\" \\$HOME `id` C:\\\\path"'); +}); + +test('non-literal service values keep single quotes without backslashes', function () { + $line = serviceEnvironmentFileLine($this->service, [ + 'key' => 'CSP', + 'value' => "array:'self',blob:,https://*", + ]); + + expect($line)->toBe('CSP="array:\'self\',blob:,https://*"'); +}); + +test('non-literal service values keep SERVICE variable interpolation', function () { + $line = serviceEnvironmentFileLine($this->service, [ + 'key' => 'DATABASE_URL', + 'value' => 'postgres://$SERVICE_USER_POSTGRES:${SERVICE_PASSWORD_POSTGRES}@postgres:5432/app', + ]); + + expect($line)->toBe('DATABASE_URL="postgres://$SERVICE_USER_POSTGRES:${SERVICE_PASSWORD_POSTGRES}@postgres:5432/app"'); +}); + +test('multiline json service values are quoted and not interpolated', function () { + $json = "{\n \"price\": \"\$5\",\n \"name\": \"it's\"\n}"; + + $line = serviceEnvironmentFileLine($this->service, [ + 'key' => 'CONFIG', + 'value' => $json, + ]); + + expect($line)->toBe("CONFIG=\"{\n \\\"price\\\": \\\"\\\$5\\\",\n \\\"name\\\": \\\"it's\\\"\n}\""); +}); + +test('json service values with leading whitespace are not interpolated', function () { + $line = serviceEnvironmentFileLine($this->service, [ + 'key' => 'CONFIG', + 'value' => "\n {\"price\":\"\$HOME\"}", + ]); + + expect($line)->toBe('CONFIG="{\\"price\\":\\"\\$HOME\\"}"'); +}); + +test('multiline service values are quoted and not interpolated', function () { + $line = serviceEnvironmentFileLine($this->service, [ + 'key' => 'CERT', + 'value' => "line 'one'\nline \$two", + 'is_multiline' => true, + ]); + + expect($line)->toBe("CERT=\"line 'one'\nline \\\$two\""); +}); + +test('remote secret service values are written literally', function () { + Http::fake([ + 'https://api.doppler.com/v3/configs/config/secrets/download*' => Http::response([ + 'API_KEY' => 'p4$$w\'ord"', + ]), + ]); + $token = IntegrationToken::query()->create([ + 'team_id' => $this->team->id, + 'provider' => 'doppler', + 'name' => 'Service secrets', + 'token' => 'the-secret-token', + 'capabilities' => ['secrets'], + ]); + $this->service->secretManagerLink()->create(['integration_token_id' => $token->id]); + + $line = serviceEnvironmentFileLine($this->service, [ + 'key' => 'API_KEY', + 'value' => '{{vault.API_KEY}}', + ]); + + expect($line)->toBe('API_KEY="p4$$$$w\'ord\\""'); +}); + +test('empty service values stay empty', function () { + $line = serviceEnvironmentFileLine($this->service, [ + 'key' => 'EMPTY', + 'value' => null, + ]); + + expect($line)->toBe('EMPTY='); +}); + +test('saveComposeConfigs writes compose env file lines', function () { + $source = file_get_contents(__DIR__.'/../../app/Models/Service.php'); + + expect($source)->toContain('$envs->push($this->composeEnvironmentFileLine($env));'); +}); diff --git a/tests/Unit/ComposeEnvFileEscapingTest.php b/tests/Unit/ComposeEnvFileEscapingTest.php new file mode 100644 index 0000000000..019c809440 --- /dev/null +++ b/tests/Unit/ComposeEnvFileEscapingTest.php @@ -0,0 +1,34 @@ +toBe('"runtime-only-value"'); +}); + +test('escapeComposeEnvFileValue keeps mixed quotes and dollars literal when interpolation is off', function () { + $value = 'hello world; quotes" \'$`'; + + $escaped = escapeComposeEnvFileValue($value, allowInterpolation: false); + + expect($escaped)->toBe('"hello world; quotes\\" \'\\$`"'); +}); + +test('escapeComposeEnvFileValue blocks command substitution when interpolation is off', function () { + $escaped = escapeComposeEnvFileValue('$(touch /tmp/should-not-exist)', allowInterpolation: false); + + expect($escaped)->toBe('"\\$(touch /tmp/should-not-exist)"'); +}); + +test('escapeComposeEnvFileValue leaves $VAR intact when interpolation is on', function () { + expect(escapeComposeEnvFileValue('$BOTH_PHASES', allowInterpolation: true))->toBe('"$BOTH_PHASES"'); +}); + +test('escapeComposeEnvFileValue preserves multiline literals', function () { + $escaped = escapeComposeEnvFileValue("line one\nline two", allowInterpolation: false); + + expect($escaped)->toBe("\"line one\nline two\""); +}); + +test('escapeComposeEnvFileValue escapes backslashes and double quotes', function () { + expect(escapeComposeEnvFileValue('C:\\path\\"quoted"')) + ->toBe('"C:\\\\path\\\\\\"quoted\\""'); +}); diff --git a/tests/Unit/ComposerAuthEnvEscapingTest.php b/tests/Unit/ComposerAuthEnvEscapingTest.php index ff6eae0aa1..6769367de9 100644 --- a/tests/Unit/ComposerAuthEnvEscapingTest.php +++ b/tests/Unit/ComposerAuthEnvEscapingTest.php @@ -74,10 +74,11 @@ it('realValue accessor still escapes non-JSON values with quotes', function () { $env->shouldReceive('getAttribute')->with('value')->andReturn('hello "world"'); $env->shouldReceive('getAttribute')->with('is_literal')->andReturn(false); $env->shouldReceive('getAttribute')->with('is_multiline')->andReturn(false); + $env->shouldReceive('getAttribute')->with('uses_legacy_escaping')->andReturn(true); $realValue = $env->real_value; - // Non-JSON should still be escaped by escapeEnvVariables + // Variables saved before exact escaping are still escaped by escapeEnvVariables expect($realValue)->toContain('\\"'); expect($realValue)->toBe('hello \\"world\\"'); }); diff --git a/tests/Unit/DatabaseStartActionResolvedCredentialsTest.php b/tests/Unit/DatabaseStartActionResolvedCredentialsTest.php new file mode 100644 index 0000000000..22b7ec2adb --- /dev/null +++ b/tests/Unit/DatabaseStartActionResolvedCredentialsTest.php @@ -0,0 +1,84 @@ +toContain(...$expected) + ->not->toContain(...$unexpected); +})->with([ + 'clickhouse' => [ + 'StartClickhouse', + ['$this->resolvedClickhouseUser', '$this->resolvedClickhousePassword'], + ['$this->database->clickhouse_admin_user, \'--password\'', '$this->database->clickhouse_admin_password, \'--query\''], + ], + 'dragonfly' => [ + 'StartDragonfly', + ['$this->resolvedRedisPassword'], + ['$this->database->dragonfly_password, \'ping\'', 'requirepass {$this->database->dragonfly_password}'], + ], + 'keydb' => [ + 'StartKeydb', + ['$this->resolvedRedisPassword'], + ['$this->database->keydb_password, \'ping\'', 'requirepass {$this->database->keydb_password}'], + ], + 'mongodb' => [ + 'StartMongodb', + ['$this->resolvedMongoDatabase', '$this->resolvedMongoUsername', '$this->resolvedMongoPassword'], + ['json_encode($this->database->mongo_initdb_database', 'json_encode($this->database->mongo_initdb_root_username', 'json_encode($this->database->mongo_initdb_root_password'], + ], + 'mysql' => [ + 'StartMysql', + ['$this->resolvedMysqlRootPassword'], + ['-p{$this->database->mysql_root_password}'], + ], + 'postgresql' => [ + 'StartPostgresql', + ['$this->resolvedPostgresUser', '$this->resolvedPostgresDatabase'], + ['$this->database->postgres_user, \'-d\'', '$this->database->postgres_db, \'-c\''], + ], +]); + +it('runs database start commands without persisting them through remote process', function (string $action) { + $source = file_get_contents(__DIR__."/../../app/Actions/Database/{$action}.php"); + + expect($source) + ->toContain('ExecutesDatabaseStartCommands') + ->toContain('executeDatabaseStartCommands(') + ->not->toContain('return remote_process('); +})->with([ + 'StartClickhouse', + 'StartDragonfly', + 'StartKeydb', + 'StartMariadb', + 'StartMongodb', + 'StartMysql', + 'StartPostgresql', + 'StartRedis', + 'StartSqlite', +]); + +it('queues database starts with identifiers instead of generated commands', function () { + $source = file_get_contents(__DIR__.'/../../app/Actions/Database/StartDatabase.php'); + + expect($source) + ->toContain('DatabaseStartJob::dispatch(') + ->not->toContain('StartPostgresql::run(') + ->not->toContain('StartRedis::run('); +}); + +it('keeps raw secret values separate from compose environment formatting', function (string $action, array $rawAssignments) { + $source = file_get_contents(__DIR__."/../../app/Actions/Database/{$action}.php"); + + expect($source) + ->toContain('$rawValue = (string) $this->database->resolveSecretManagerEnvironmentVariableValue($env);') + ->toContain('$resolvedValue = (string) $this->database->formatEnvironmentVariableValue($env, $rawValue);') + ->toContain('$environment_variables->push($env->key.\'=\'.$resolvedValue);') + ->toContain(...$rawAssignments); +})->with([ + 'clickhouse' => ['StartClickhouse', ['$this->resolvedClickhouseUser = $composeFileValue;', '$this->resolvedClickhousePassword = $composeFileValue;']], + 'dragonfly' => ['StartDragonfly', ['$this->resolvedRedisPassword = $this->database->formatComposeFileValue($env, $rawValue);', '$this->requirePassArgument()']], + 'keydb' => ['StartKeydb', ['$this->resolvedRedisPassword = $this->database->formatComposeFileValue($env, $rawValue);', '$this->requirePassArgument()']], + 'mongodb' => ['StartMongodb', ['$this->resolvedMongoUsername = $rawValue;', '$this->resolvedMongoPassword = $rawValue;', '$this->resolvedMongoDatabase = $rawValue;', 'json_encode($this->resolvedMongoPassword']], + 'mysql' => ['StartMysql', ['$this->resolvedMysqlRootPassword = $composeFileValue;']], + 'postgresql' => ['StartPostgresql', ['$this->resolvedPostgresUser = $composeFileValue;', '$this->resolvedPostgresDatabase = $composeFileValue;']], +]); diff --git a/tests/Unit/ServerTransfer/ServerTransferExporterImporterTest.php b/tests/Unit/ServerTransfer/ServerTransferExporterImporterTest.php index b2e009899d..1d57fe9aff 100644 --- a/tests/Unit/ServerTransfer/ServerTransferExporterImporterTest.php +++ b/tests/Unit/ServerTransfer/ServerTransferExporterImporterTest.php @@ -913,6 +913,26 @@ test('import rejects shell-like file ownership and mode metadata', function (str 'option mode' => ['chmod', '--reference=/etc/passwd'], ]); +test('transfer keeps the legacy escaping flag and treats old exports as legacy', function () { + $secret = $this->application->environment_variables()->where('key', 'APP_SECRET')->firstOrFail(); + $secret->uses_legacy_escaping = true; + $secret->save(); + + $bundle = $this->exporter->export($this->server); + $exported = collect($bundle['projects'][0]['environments'][0]['applications'][0]['environment_variables']) + ->firstWhere('key', 'APP_SECRET'); + + $importEnvVars = new ReflectionMethod($this->importer, 'importEnvVars'); + $importEnvVars->invoke($this->importer, [ + ['key' => 'EXACT_VAR', 'value' => 'a', 'uses_legacy_escaping' => false], + ['key' => 'OLD_EXPORT_VAR', 'value' => 'b'], + ], $this->application, false); + + expect($exported['uses_legacy_escaping'])->toBeTrue() + ->and($this->application->environment_variables()->where('key', 'EXACT_VAR')->first()->uses_legacy_escaping)->toBeFalse() + ->and($this->application->environment_variables()->where('key', 'OLD_EXPORT_VAR')->first()->uses_legacy_escaping)->toBeTrue(); +}); + test('transfer keeps whether a volume ignores the Compose driver options', function (?bool $exported, bool $expected) { LocalPersistentVolume::query()->update(['ignores_compose_driver_options' => (bool) $exported]); $bundle = $this->exporter->export($this->server);