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}}';