From a36228297632177c85a45384ee6f9fc5988bc7de Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Tue, 10 Mar 2026 17:37:13 +0100 Subject: [PATCH 1/2] chore: prepare for PR --- bootstrap/helpers/parsers.php | 24 ++++++++++---- bootstrap/helpers/services.php | 5 +++ .../NestedEnvironmentVariableParsingTest.php | 33 +++++++++++++++++++ 3 files changed, 56 insertions(+), 6 deletions(-) diff --git a/bootstrap/helpers/parsers.php b/bootstrap/helpers/parsers.php index fa40857ac..6b66de8a2 100644 --- a/bootstrap/helpers/parsers.php +++ b/bootstrap/helpers/parsers.php @@ -442,9 +442,9 @@ function applicationParser(Application $resource, int $pull_request_id = 0, ?int $value = str($value); $regex = '/\$(\{?([a-zA-Z_\x80-\xff][a-zA-Z0-9_\x80-\xff]*)\}?)/'; preg_match_all($regex, $value, $valueMatches); - if (count($valueMatches[1]) > 0) { - foreach ($valueMatches[1] as $match) { - $match = replaceVariables($match); + if (count($valueMatches[2]) > 0) { + foreach ($valueMatches[2] as $match) { + $match = str($match); if ($match->startsWith('SERVICE_')) { if ($magicEnvironments->has($match->value())) { continue; @@ -1509,6 +1509,18 @@ function serviceParser(Service $resource): Collection return collect([]); } $services = data_get($yaml, 'services', collect([])); + + // Clean up corrupted environment variables from previous parser bugs + // (keys starting with $ or ending with } should not exist as env var names) + $resource->environment_variables() + ->where('resourceable_type', get_class($resource)) + ->where('resourceable_id', $resource->id) + ->where(function ($q) { + $q->where('key', 'LIKE', '$%') + ->orWhere('key', 'LIKE', '%}'); + }) + ->delete(); + $topLevel = collect([ 'volumes' => collect(data_get($yaml, 'volumes', [])), 'networks' => collect(data_get($yaml, 'networks', [])), @@ -1686,9 +1698,9 @@ function serviceParser(Service $resource): Collection $value = str($value); $regex = '/\$(\{?([a-zA-Z_\x80-\xff][a-zA-Z0-9_\x80-\xff]*)\}?)/'; preg_match_all($regex, $value, $valueMatches); - if (count($valueMatches[1]) > 0) { - foreach ($valueMatches[1] as $match) { - $match = replaceVariables($match); + if (count($valueMatches[2]) > 0) { + foreach ($valueMatches[2] as $match) { + $match = str($match); if ($match->startsWith('SERVICE_')) { if ($magicEnvironments->has($match->value())) { continue; diff --git a/bootstrap/helpers/services.php b/bootstrap/helpers/services.php index bd741b76e..20b184a01 100644 --- a/bootstrap/helpers/services.php +++ b/bootstrap/helpers/services.php @@ -128,6 +128,11 @@ function replaceVariables(string $variable): Stringable return $str->replaceFirst('{', '')->before('}'); } + // Handle bare $VAR format (no braces) + if ($str->startsWith('$')) { + return $str->replaceFirst('$', ''); + } + return $str; } diff --git a/tests/Unit/NestedEnvironmentVariableParsingTest.php b/tests/Unit/NestedEnvironmentVariableParsingTest.php index 65e8738cc..b98f49dd7 100644 --- a/tests/Unit/NestedEnvironmentVariableParsingTest.php +++ b/tests/Unit/NestedEnvironmentVariableParsingTest.php @@ -206,6 +206,39 @@ expect($split['default'])->toBe('${SERVICE_URL_CONFIG}/v2/config.json'); }); +test('replaceVariables strips leading dollar sign from bare $VAR format', function () { + // Bug #8851: When a compose value is $SERVICE_USER_POSTGRES (bare $VAR, no braces), + // replaceVariables must strip the $ so the parsed name is SERVICE_USER_POSTGRES. + // Without this, the fallback code path creates a DB entry with key=$SERVICE_USER_POSTGRES. + expect(replaceVariables('$SERVICE_USER_POSTGRES')->value())->toBe('SERVICE_USER_POSTGRES') + ->and(replaceVariables('$SERVICE_PASSWORD_POSTGRES')->value())->toBe('SERVICE_PASSWORD_POSTGRES') + ->and(replaceVariables('$SERVICE_FQDN_APPWRITE')->value())->toBe('SERVICE_FQDN_APPWRITE'); +}); + +test('bare dollar variable in bash-style fallback does not capture trailing brace', function () { + // Bug #8851: ${_APP_DOMAIN:-$SERVICE_FQDN_APPWRITE} causes the regex to + // capture "SERVICE_FQDN_APPWRITE}" (with trailing }) because \}? in the regex + // greedily matches the closing brace of the outer ${...} construct. + // The fix uses capture group 2 (clean variable name) instead of group 1. + $value = '${_APP_DOMAIN:-$SERVICE_FQDN_APPWRITE}'; + + $regex = '/\$(\{?([a-zA-Z_\x80-\xff][a-zA-Z0-9_\x80-\xff]*)\}?)/'; + preg_match_all($regex, $value, $valueMatches); + + // Group 2 should contain clean variable names without any braces + expect($valueMatches[2])->toContain('_APP_DOMAIN') + ->and($valueMatches[2])->toContain('SERVICE_FQDN_APPWRITE'); + + // Verify no match in group 2 has trailing } + foreach ($valueMatches[2] as $match) { + expect($match)->not->toEndWith('}', "Variable name '{$match}' should not end with }"); + } + + // Group 1 (previously used) would have the bug — SERVICE_FQDN_APPWRITE} + // This demonstrates why group 2 must be used instead + expect($valueMatches[1])->toContain('SERVICE_FQDN_APPWRITE}'); +}); + test('operator precedence with nesting', function () { // The first :- at depth 0 should be used, not the one inside nested braces $input = '${A:-${B:-default}}'; From 0679e91c85f283a527f1db46fb00c6a18f415659 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Tue, 10 Mar 2026 18:06:01 +0100 Subject: [PATCH 2/2] fix(parser): use firstOrCreate instead of updateOrCreate for environment variables Prevent unnecessary updates to existing environment variable records. The previous implementation would update matching records, but the intent is to retrieve or create the record without modifying existing ones. --- bootstrap/helpers/parsers.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bootstrap/helpers/parsers.php b/bootstrap/helpers/parsers.php index 6b66de8a2..99ce9185a 100644 --- a/bootstrap/helpers/parsers.php +++ b/bootstrap/helpers/parsers.php @@ -1940,7 +1940,7 @@ function serviceParser(Service $resource): Collection } else { $value = generateEnvValue($command, $resource); - $resource->environment_variables()->updateOrCreate([ + $resource->environment_variables()->firstOrCreate([ 'key' => $key->value(), 'resourceable_type' => get_class($resource), 'resourceable_id' => $resource->id,