rosslh/coolify
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix(env-parser): capture clean variable names without trailing braces in bash-style defaults (#8855)

Browse source
This commit is contained in:
Andras Bacsai 2026-03-10 18:06:51 +01:00 committed by GitHub
commit 201998638a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 57 additions and 7 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

26
bootstrap/helpers/parsers.php
View file

@ -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;
@ -1928,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,

5
bootstrap/helpers/services.php
View file

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

33
tests/Unit/NestedEnvironmentVariableParsingTest.php
View file

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