From 5cf1abc0fc0cbdc39276d03607c342633a8a6cc0 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Mon, 2 Nov 2026 14:49:11 +0100 Subject: [PATCH] fix(env): keep pre-upgrade values for legacy compose variables Closes #12408 Co-Authored-By: Claude Opus 5.5 --- app/Models/EnvironmentVariable.php | 6 ++- app/Traits/HasSecretManager.php | 44 ++++++++++++++-- bootstrap/helpers/shared.php | 48 ++++++++++++++++++ ...ationDeploymentControlVarFilteringTest.php | 50 +++++++++++++++++++ .../EnvironmentVariableLegacyEscapingTest.php | 34 +++++++++++++ 5 files changed, 177 insertions(+), 5 deletions(-) diff --git a/app/Models/EnvironmentVariable.php b/app/Models/EnvironmentVariable.php index b40ca8ebb3..430727a502 100644 --- a/app/Models/EnvironmentVariable.php +++ b/app/Models/EnvironmentVariable.php @@ -291,7 +291,11 @@ class EnvironmentVariable extends BaseModel $value = $this->get_real_environment_variables_internal($this->value, $resource, null, $revealLockedSharedVariables, $sharedVariables); - return $value === null || ! $escapeLegacyValue || ! $this->uses_legacy_escaping ? $value : $this->legacyEscapedValue($value); + if ($value === null || ! $escapeLegacyValue || ! $this->uses_legacy_escaping) { + return $value; + } + + return method_exists($resource, 'useExactEscaping') && $resource->useExactEscaping($this) ? $value : $this->legacyEscapedValue($value); } /** diff --git a/app/Traits/HasSecretManager.php b/app/Traits/HasSecretManager.php index dd9195c9af..621a29ca01 100644 --- a/app/Traits/HasSecretManager.php +++ b/app/Traits/HasSecretManager.php @@ -146,11 +146,46 @@ trait HasSecretManager * 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. + * it to exact escaping. Compose references that got the stored value before the upgrade + * (such as `FOO=${FOO:-}`) use exact escaping, so they keep getting that value. So do old + * .env lines that Docker Compose cannot read, because they stop the deployment. */ public function useExactEscaping(EnvironmentVariable $environmentVariable): bool { - return ! $environmentVariable->uses_legacy_escaping; + if (! $environmentVariable->uses_legacy_escaping) { + return true; + } + + if (! $this instanceof Service && ! $this instanceof Application) { + return false; + } + + if ($this->legacyEnvFileLineIsUnreadable($environmentVariable)) { + return true; + } + + $parsesCompose = $this instanceof Service + || ($this->build_pack === 'dockercompose' && ! $this->settings?->is_raw_compose_deployment_enabled); + + return $parsesCompose + && dockerComposeVariablesWithStoredValueBeforeExactEscaping($this->docker_compose_raw)->contains($environmentVariable->key); + } + + /** + * The old escaping puts literal and multiline values in single quotes without escaping. Docker + * Compose ends the value at the next quote, so a value with a quote breaks the whole .env file. + */ + private function legacyEnvFileLineIsUnreadable(EnvironmentVariable $environmentVariable): bool + { + if (! $environmentVariable->is_literal && ! $environmentVariable->is_multiline) { + return false; + } + + $value = $this->resolvedEnvironmentVariableValue($environmentVariable); + + return is_string($value) + && str_contains($value, "'") + && ! (json_validate($value) && (str_starts_with($value, '{') || str_starts_with($value, '['))); } /** @@ -210,8 +245,9 @@ trait HasSecretManager } 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; + // Service and application .env files strip the old single quotes (values with a quote use exact + // escaping); database environment lists keep them. + return ! ($this instanceof Service || $this instanceof Application); } return escapeEnvVariables($value) !== $value diff --git a/bootstrap/helpers/shared.php b/bootstrap/helpers/shared.php index 10255bcebd..e852ec50f0 100644 --- a/bootstrap/helpers/shared.php +++ b/bootstrap/helpers/shared.php @@ -5605,6 +5605,54 @@ function dockerComposeEnvironmentVariableShouldBeUnset(?string $dockerCompose, s return extractDockerComposeEnvironmentVariableReferences($dockerCompose)->contains($key); } +/** + * Variables whose stored value the Compose parser before v4.5.0 wrote straight into the `environment:` + * list, so their containers got the value without the old .env escaping: references with a default or + * under another name (`FOO=${FOO:-}`, `BAR=${FOO}`, `BAR=$FOO`) and references in such a default. + * A plain `FOO=${FOO}` reference kept reading the escaped .env value. + * + * @return Collection + */ +function dockerComposeVariablesWithStoredValueBeforeExactEscaping(?string $dockerCompose): Collection +{ + static $cache = []; + if (blank($dockerCompose)) { + return collect(); + } + + $cacheKey = hash('xxh128', $dockerCompose); + if (isset($cache[$cacheKey])) { + return $cache[$cacheKey]; + } + + $keys = collect(); + foreach (extractHardcodedEnvironmentVariables($dockerCompose) as $assignment) { + $value = $assignment['value'] ?? null; + if (! is_string($value) || ! str_starts_with($value, '$') || replaceVariables($value)->value() === (string) $assignment['key']) { + continue; + } + + $reference = extractBalancedBraceContent($value, 0); + if ($reference === null) { + if (preg_match('/\A\$([A-Za-z_][A-Za-z0-9_]*)\z/', $value, $match) === 1) { + $keys->push($match[1]); + } + + continue; + } + + $split = splitOnOperatorOutsideNested($reference['content']); + $keys->push($split === null ? $reference['content'] : $split['variable']); + if ($split !== null) { + $keys = $keys->concat(extractDockerComposeEnvironmentVariableReferences($split['default'])); + } + } + + $cache = [$cacheKey => $keys->reject(fn (string $key): bool => str_starts_with($key, 'SERVICE_'))->unique()->values()]; + + return $cache[$cacheKey]; +} + /** @param Collection $environmentVariables */ function preserveLegacyDockerComposeNullAssignments(Collection $environment, Collection $environmentVariables): Collection { diff --git a/tests/Feature/ApplicationDeploymentControlVarFilteringTest.php b/tests/Feature/ApplicationDeploymentControlVarFilteringTest.php index fc3fdcf363..ebdecee8da 100644 --- a/tests/Feature/ApplicationDeploymentControlVarFilteringTest.php +++ b/tests/Feature/ApplicationDeploymentControlVarFilteringTest.php @@ -1842,6 +1842,56 @@ it('keeps the old runtime env format for variables saved before exact escaping', ->toContain('EXACT_PLAIN='.escapeComposeEnvFileValue('a\'b"c\\d', allowInterpolation: true)); }); +it('gives legacy compose variables the value that the compose file got before exact escaping', function (bool $rawCompose, string $expectedDefault) { + $compose = <<<'YAML' +services: + app: + image: alpine + environment: + - TRUSTED_HOST_PATTERNS=${TRUSTED_HOST_PATTERNS:-} + - PLAIN_REFERENCE=${PLAIN_REFERENCE} +YAML; + [$application, $server] = makeDeploymentControlVarFixture([ + 'build_pack' => 'dockercompose', + 'docker_compose_raw' => $compose, + 'docker_compose' => $compose, + ]); + $application->settings()->update(['is_raw_compose_deployment_enabled' => $rawCompose]); + + foreach (['TRUSTED_HOST_PATTERNS', 'PLAIN_REFERENCE'] as $key) { + $variable = createApplicationEnvironmentVariable($application, ['key' => $key, 'value' => '^example\\.com$', 'is_runtime' => true, 'is_buildtime' => false]); + $variable->uses_legacy_escaping = true; + $variable->save(); + } + + [$job, $reflection] = makeControlVarFilteringJob($application->fresh(), $server); + + /** @var Collection $runtimeEnvs */ + $runtimeEnvs = invokeDeploymentJobMethod($job, $reflection, 'generate_runtime_environment_variables'); + + expect($runtimeEnvs) + ->toContain('TRUSTED_HOST_PATTERNS='.$expectedDefault) + ->toContain('PLAIN_REFERENCE=^example\\\\.com$'); +})->with([ + 'parsed compose' => [false, '"^example\\\\.com$"'], + 'raw compose deployment' => [true, '^example\\\\.com$'], +]); + +it('writes a readable runtime env line for legacy literal values with a single quote', function () { + [$application, $server] = makeDeploymentControlVarFixture(); + + $variable = createApplicationEnvironmentVariable($application, ['key' => 'LEGACY_QUOTE', 'value' => "it's", 'is_literal' => true, 'is_runtime' => true, 'is_buildtime' => false]); + $variable->uses_legacy_escaping = true; + $variable->save(); + + [$job, $reflection] = makeControlVarFilteringJob($application->fresh(), $server); + + /** @var Collection $runtimeEnvs */ + $runtimeEnvs = invokeDeploymentJobMethod($job, $reflection, 'generate_runtime_environment_variables'); + + expect($runtimeEnvs)->toContain('LEGACY_QUOTE="it\'s"'); +}); + it('keeps the old runtime env format for preview variables saved before exact escaping', function () { [$application, $server] = makeDeploymentControlVarFixture(); diff --git a/tests/Feature/EnvironmentVariableLegacyEscapingTest.php b/tests/Feature/EnvironmentVariableLegacyEscapingTest.php index 96382a231c..6823dfc7b5 100644 --- a/tests/Feature/EnvironmentVariableLegacyEscapingTest.php +++ b/tests/Feature/EnvironmentVariableLegacyEscapingTest.php @@ -95,6 +95,40 @@ test('legacy service variables keep the old env file format', function () { ->and($this->service->composeEnvironmentFileLine($empty))->toBe('EMPTY='); }); +test('legacy service variables keep the value that the compose file got before exact escaping', function () { + $this->service->update(['docker_compose_raw' => <<<'YAML' +services: + app: + image: alpine + environment: + - WITH_DEFAULT=${WITH_DEFAULT:-x} + - OTHER_NAME=${RENAMED} + - NESTED=${NESTED:-${NESTED_DEFAULT}} + - PLAIN=${PLAIN} +YAML]); + $variables = collect(['WITH_DEFAULT', 'RENAMED', 'NESTED_DEFAULT', 'PLAIN']) + ->mapWithKeys(fn (string $key) => [$key => legacyVariable($this->service, ['key' => $key, 'value' => 'a\\.b'])]); + + expect($this->service->composeEnvironmentFileLine($variables['WITH_DEFAULT']))->toBe('WITH_DEFAULT="a\\\\.b"') + ->and($this->service->composeEnvironmentFileLine($variables['RENAMED']))->toBe('RENAMED="a\\\\.b"') + ->and($this->service->composeEnvironmentFileLine($variables['NESTED_DEFAULT']))->toBe('NESTED_DEFAULT="a\\\\.b"') + ->and($this->service->composeEnvironmentFileLine($variables['PLAIN']))->toBe('PLAIN=a\\\\.b') + ->and($this->service->legacyEscapingChangesValue($variables['WITH_DEFAULT']))->toBeFalse() + ->and($this->service->legacyEscapingChangesValue($variables['PLAIN']))->toBeTrue() + ->and($variables['WITH_DEFAULT']->fresh()->real_value)->toBe('a\\.b'); +}); + +test('legacy literal and multiline variables with a single quote get a line that docker compose can read', function () { + $literal = legacyVariable($this->service, ['key' => 'LITERAL_QUOTE', 'value' => "it's", 'is_literal' => true]); + $multiline = legacyVariable($this->service, ['key' => 'MULTILINE_QUOTE', 'value' => "it's\n\"two\"", 'is_multiline' => true]); + $withoutQuote = legacyVariable($this->service, ['key' => 'LITERAL_PLAIN', 'value' => 'x$y', 'is_literal' => true]); + + expect($this->service->composeEnvironmentFileLine($literal))->toBe('LITERAL_QUOTE="it\'s"') + ->and($this->service->composeEnvironmentFileLine($multiline))->toBe("MULTILINE_QUOTE=\"it's\n\\\"two\\\"\"") + ->and($this->service->composeEnvironmentFileLine($withoutQuote))->toBe("LITERAL_PLAIN='x\$y'") + ->and($this->service->legacyEscapingChangesValue($literal))->toBeFalse(); +}); + test('legacy database variables keep the old compose environment format', function () { $database = StandalonePostgresql::forceCreate([ 'uuid' => 'postgres-legacy-test',