starred/coolify
1
0
Fork 0
mirror of https://github.com/coollabsio/coolify.git synced 2026-11-06 13:10:37 -04:00
Code Issues Packages Projects Releases Wiki Activity

fix(storage): resolve ${VAR:-default} bind sources and safe conversion (#12227)

Browse Source
This commit is contained in:
Andras Bacsai
2026-10-20 11:41:54 +02:00
committed by GitHub
parent 5925c8403a
commit 2ade696375
5 changed files with 164 additions and 66 deletions
Download Patch File Download Diff File Expand all files Collapse all files
app/Livewire/Project/Service/FileStorage.php
+6 -2
View File
@@ -167,10 +167,12 @@ class FileStorage extends Component
$this->fileStorage->content = null;
$this->fileStorage->is_based_on_git = false;
$changedFields = auditChangedFields($this->fileStorage);
$this->fileStorage->save();
$this->fileStorage->saveStorageOnServer();
$this->fileStorage->save();
$this->auditFileStorageUpdate($changedFields, 'converted_to_directory');
} catch (\Throwable $e) {
$this->fileStorage->refresh();
return handleError($e, $this);
} finally {
$this->dispatch('storageCountsChanged')->to(Storage::class);
@@ -216,10 +218,12 @@ class FileStorage extends Component
$this->fileStorage->is_based_on_git = true;
}
$changedFields = auditChangedFields($this->fileStorage);
$this->fileStorage->save();
$this->fileStorage->saveStorageOnServer();
$this->fileStorage->save();
$this->auditFileStorageUpdate($changedFields, 'converted_to_file');
} catch (\Throwable $e) {
$this->fileStorage->refresh();
return handleError($e, $this);
} finally {
$this->dispatch('storageCountsChanged')->to(Storage::class);
bootstrap/helpers/parsers.php
+56 -20
View File
@@ -237,6 +237,52 @@ function validateComposeContentVolumeSource(array $volume): void
validateComposeArrayVolumeSource($source);
}
/**
* Replaces a `${VAR:-default}` volume source with its default, as the short volume syntax does.
* An empty default becomes `${VAR}`. Other sources do not change.
*/
function resolveComposeVolumeSourceDefault(string $source): string
{
if (! preg_match('/^\$\{([^}]+)\}$/', $source, $matches) || ! str_contains($matches[1], ':-')) {
return $source;
}
[$varName, $defaultValue] = explode(':-', $matches[1], 2);
return $defaultValue !== '' ? $defaultValue : '${'.$varName.'}';
}
/**
* Resolves a long-syntax bind source as Docker Compose does with the `.env` file that Coolify writes:
* `${VAR:-default}` is the value of VAR when the resource sets it to a non-empty value, else the default.
*
* @throws Exception If the value of the variable is not a shell-safe path
*/
function resolveComposeLongBindSource(string $source, Application|Service $resource, bool $isPreview = false): string
{
if (! preg_match('/^\$\{([A-Za-z_][A-Za-z0-9_]*):-[^}]*\}$/', $source, $matches)) {
return resolveComposeVolumeSourceDefault($source);
}
$variables = match (true) {
$resource instanceof Service => $resource->environment_variables(),
$isPreview => $resource->runtime_environment_variables_preview()->where('is_runtime', true),
default => $resource->runtime_environment_variables()->where('is_runtime', true),
};
$value = $variables->where('key', $matches[1])->first()?->real_value;
if (blank($value)) {
return resolveComposeVolumeSourceDefault($source);
}
try {
validateShellSafePath($value, 'volume source');
} catch (Exception $e) {
throw new Exception("Invalid Docker volume definition (array syntax): the value of {$matches[1]} is not a safe path. ".$e->getMessage(), 0, $e);
}
return $value;
}
/**
* Keep the existing array-source forms, but inspect the default that was previously skipped.
*/
@@ -932,26 +978,8 @@ function parseDockerVolumeString(string $volumeString): array
}
}
// Handle environment variable expansion in source
// Example: ${VOLUME_DB_PATH:-db} should extract default value if present
if ($source && preg_match('/^\$\{([^}]+)\}$/', $source, $matches)) {
$varContent = $matches[1];
// Check if there's a default value with :-
if (strpos($varContent, ':-') !== false) {
$parts = explode(':-', $varContent, 2);
$varName = $parts[0];
$defaultValue = isset($parts[1]) ? $parts[1] : '';
// If there's a non-empty default value, use it for source
if ($defaultValue !== '') {
$source = $defaultValue;
} else {
// Empty default value, keep the variable reference for env resolution
$source = '${'.$varName.'}';
}
}
// Otherwise keep the variable as-is for later expansion (no default value)
if ($source) {
$source = resolveComposeVolumeSourceDefault((string) $source);
}
// Validate source path for command injection attempts
@@ -1500,6 +1528,10 @@ function applicationParser(Application $resource, int $pull_request_id = 0, ?int
// Validate source and target for command injection (array/long syntax)
if ($source !== null && ! empty($source->value())) {
validateComposeArrayVolumeSource($source->value());
// A named volume keeps its source as written, so its volume name does not change.
if ($type->value() === 'bind') {
$source = str(resolveComposeLongBindSource($source->value(), $resource, $isPullRequest));
}
}
validateComposeContentVolumeSource($volume);
if ($target !== null && ! empty($target->value())) {
@@ -2921,6 +2953,10 @@ function serviceParser(Service $resource): Collection
// Validate source and target for command injection (array/long syntax)
if ($source !== null && ! empty($source->value())) {
validateComposeArrayVolumeSource($source->value());
// A named volume keeps its source as written, so its volume name does not change.
if ($type->value() === 'bind') {
$source = str(resolveComposeLongBindSource($source->value(), $resource));
}
}
validateComposeContentVolumeSource($volume);
if ($target !== null && ! empty($target->value())) {
resources/views/livewire/project/service/file-storage.blade.php
+43 -43
View File
@@ -11,9 +11,9 @@
@elseif ($isReadOnly)
<x-callout type="info" title="Read-only mount">
@if ($fileStorage->is_directory)
This directory is mounted as read-only and cannot be modified from the UI.
This directory is mounted as read-only. You can convert it to a file, but you cannot delete it from the UI.
@else
This file is mounted as read-only and cannot be modified from the UI.
This file is mounted as read-only. You cannot edit or delete it from the UI.
@endif
</x-callout>
@endif
@@ -45,58 +45,64 @@
@endif
<form wire:submit='submit' class="flex flex-col gap-4">
<x-unsaved-bar action="submit" />
@if (!$isReadOnly)
@can('update', $resource)
<div class="flex flex-wrap items-center gap-2">
@if ($fileStorage->is_host_file)
@can('update', $resource)
<div class="flex flex-wrap items-center gap-2">
@if ($fileStorage->is_host_file)
@if (!$isReadOnly)
<x-modal-confirmation :ignoreWire="false" title="Confirm Host File Mount Removal?"
buttonTitle="Delete" isErrorButton submitAction="delete" :checkboxes="$deletionCheckboxes"
:actions="$deletionActions"
confirmationText="{{ $fs_path }}"
confirmationLabel="Please confirm the execution of the actions by entering the Filepath below"
shortConfirmationLabel="Filepath" />
@elseif ($fileStorage->is_directory)
<x-modal-confirmation :ignoreWire="false" title="Confirm Directory Conversion to File?"
buttonTitle="Convert to file" submitAction="convertToFile" :actions="[
'All files in this directory will be permanently deleted and an empty file will be created in its place.',
]"
confirmationText="{{ $fs_path }}"
confirmationLabel="Please confirm the execution of the actions by entering the Filepath below"
shortConfirmationLabel="Filepath" :confirmWithPassword="false" step2ButtonText="Convert to file" />
@if ($resource instanceof \App\Models\Application)
<x-modal-input buttonTitle="Configure Backup" title="Configure Directory Backup"
:wireIgnore="false">
<livewire:project.application.backup.create :application="$resource"
:selected-target-key="'directory:' . $fileStorage->id"
wire:key="configure-directory-backup-{{ $fileStorage->id }}" />
</x-modal-input>
@endif
@endif
@elseif ($fileStorage->is_directory)
<x-modal-confirmation :ignoreWire="false" title="Confirm Directory Conversion to File?"
buttonTitle="Convert to file" submitAction="convertToFile" :actions="[
'All files in this directory will be permanently deleted and an empty file will be created in its place.',
]"
confirmationText="{{ $fs_path }}"
confirmationLabel="Please confirm the execution of the actions by entering the Filepath below"
shortConfirmationLabel="Filepath" :confirmWithPassword="false" step2ButtonText="Convert to file" />
@if ($resource instanceof \App\Models\Application && !$isReadOnly)
<x-modal-input buttonTitle="Configure Backup" title="Configure Directory Backup"
:wireIgnore="false">
<livewire:project.application.backup.create :application="$resource"
:selected-target-key="'directory:' . $fileStorage->id"
wire:key="configure-directory-backup-{{ $fileStorage->id }}" />
</x-modal-input>
@endif
@if (!$isReadOnly)
<x-modal-confirmation :ignoreWire="false" title="Confirm Directory Deletion?" buttonTitle="Delete"
isErrorButton submitAction="delete" :checkboxes="$deletionCheckboxes" :actions="$deletionActions"
confirmationText="{{ $fs_path }}"
confirmationLabel="Please confirm the execution of the actions by entering the Filepath below"
shortConfirmationLabel="Filepath" />
@else
@if (!$fileStorage->is_binary && !$fileStorage->is_too_large)
<x-modal-confirmation :ignoreWire="false" title="Confirm File Conversion to Directory?"
buttonTitle="Convert to directory" submitAction="convertToDirectory" :actions="[
'The selected file will be permanently deleted and an empty directory will be created in its place.',
]"
confirmationText="{{ $fs_path }}"
confirmationLabel="Please confirm the execution of the actions by entering the Filepath below"
shortConfirmationLabel="Filepath" :confirmWithPassword="false"
step2ButtonText="Convert to directory" />
@endif
<x-forms.button type="button" wire:click="loadStorageOnServer">Load from
server</x-forms.button>
@endif
@else
@if (!$fileStorage->is_binary && !$fileStorage->is_too_large)
<x-modal-confirmation :ignoreWire="false" title="Confirm File Conversion to Directory?"
buttonTitle="Convert to directory" submitAction="convertToDirectory" :actions="[
'The selected file will be permanently deleted and an empty directory will be created in its place.',
]"
confirmationText="{{ $fs_path }}"
confirmationLabel="Please confirm the execution of the actions by entering the Filepath below"
shortConfirmationLabel="Filepath" :confirmWithPassword="false"
step2ButtonText="Convert to directory" />
@endif
<x-forms.button type="button" wire:click="loadStorageOnServer">Load from
server</x-forms.button>
@if (!$isReadOnly)
<x-modal-confirmation :ignoreWire="false" title="Confirm File Deletion?" buttonTitle="Delete"
isErrorButton submitAction="delete" :checkboxes="$deletionCheckboxes" :actions="$deletionActions"
confirmationText="{{ $fs_path }}"
confirmationLabel="Please confirm the execution of the actions by entering the Filepath below"
shortConfirmationLabel="Filepath" />
@endif
</div>
@endcan
@endif
</div>
@endcan
@if (!$isReadOnly)
@if (!$fileStorage->is_directory && !$fileStorage->is_host_file)
@can('update', $resource)
@if (data_get($resource, 'settings.is_preserve_repository_enabled'))
@@ -125,12 +131,6 @@
@else
{{-- Read-only view --}}
@if (!$fileStorage->is_directory && !$fileStorage->is_host_file)
@can('update', $resource)
<div class="flex gap-2">
<x-forms.button type="button" wire:click="loadStorageOnServer">Load from
server</x-forms.button>
</div>
@endcan
@if (data_get($resource, 'settings.is_preserve_repository_enabled'))
<div class="w-full sm:w-96">
<x-forms.checkbox disabled label="Is this based on the Git repository?"
tests/Feature/ComposeLongBindOptionsTest.php
+45 -1
View File
@@ -65,9 +65,10 @@ YAML;
/**
* Parses the Compose file as a Service or a Compose Application, twice like a second save.
*
* @param array<string, string> $variables Environment variables that the resource sets before the parse
* @return array{volume: mixed, volumes: array<int, mixed>, storages: Collection, directory: string}
*/
function parseLongBind(string $kind, string $compose): array
function parseLongBind(string $kind, string $compose, array $variables = []): array
{
if ($kind === 'service') {
$resource = Service::factory()->create([
@@ -77,6 +78,9 @@ function parseLongBind(string $kind, string $compose): array
'destination_id' => test()->destination->id,
'destination_type' => test()->destination->getMorphClass(),
])->fresh();
foreach ($variables as $key => $value) {
$resource->environment_variables()->create(['key' => $key, 'value' => $value]);
}
serviceParser($resource);
$parsed = serviceParser($resource->fresh())->toArray();
$owner = $resource->applications()->firstOrFail();
@@ -89,6 +93,9 @@ function parseLongBind(string $kind, string $compose): array
'destination_id' => test()->destination->id,
'destination_type' => test()->destination->getMorphClass(),
])->fresh();
foreach ($variables as $key => $value) {
$resource->environment_variables()->create(['key' => $key, 'value' => $value]);
}
applicationParser($resource);
$parsed = applicationParser($resource->fresh())->toArray();
$owner = $resource;
@@ -180,3 +187,40 @@ YAML);
'bind' => ['create_host_path' => true],
]);
})->with(['service', 'application']);
it('uses the default of a ${VAR:-default} long-form bind source as the host path', function (string $kind) {
$result = parseLongBind($kind, longBindCompose("'\${AO_CONFIG_FILE:-./agent-orchestrator.yaml}'", LONG_BIND_READ_ONLY_OPTIONS));
expect($result['volume']['source'])->toBe($result['directory'].'/agent-orchestrator.yaml')
->and($result['storages']->pluck('fs_path')->all())->toBe([$result['directory'].'/agent-orchestrator.yaml']);
})->with(['service', 'application']);
it('keeps the volume name of a long-form named volume with a ${VAR:-default} source', function (string $kind) {
$result = parseLongBind($kind, <<<'YAML'
services:
probe:
image: alpine:3.22
volumes:
- type: volume
source: '${DATA_VOLUME:-pgdata}'
target: /fixture
YAML);
expect($result['volume']['source'])->toEndWith('_data-volume-pgdata');
})->with(['service', 'application']);
it('uses the value that the resource sets for a ${VAR:-default} long-form bind source', function (string $kind, string $value, string $expected) {
$result = parseLongBind($kind, longBindCompose("'\${AO_CONFIG_FILE:-./agent-orchestrator.yaml}'", LONG_BIND_READ_ONLY_OPTIONS), ['AO_CONFIG_FILE' => $value]);
$expected = str_replace('{directory}', $result['directory'], $expected);
expect($result['volume']['source'])->toBe($expected)
->and($result['storages']->pluck('fs_path')->all())->toBe([$expected]);
})->with(['service', 'application'])->with([
'absolute value' => ['/opt/ao-custom.yaml', '/opt/ao-custom.yaml'],
'relative value' => ['./custom.yaml', '{directory}/custom.yaml'],
'empty value' => ['', '{directory}/agent-orchestrator.yaml'],
]);
it('rejects an unsafe value for a ${VAR:-default} long-form bind source', function (string $kind) {
parseLongBind($kind, longBindCompose("'\${AO_CONFIG_FILE:-./agent-orchestrator.yaml}'", LONG_BIND_READ_ONLY_OPTIONS), ['AO_CONFIG_FILE' => '/opt/x;touch /tmp/pwned']);
})->with(['service', 'application'])->throws(Exception::class, 'the value of AO_CONFIG_FILE is not a safe path');
tests/Feature/StorageHostPathTest.php
+14
View File
@@ -449,3 +449,17 @@ test('changing the content of a mount outside the resource directory needs deplo
->assertOk();
expect($storage->fresh()->content)->toBe('listen 9090;');
})->with(['application', 'database', 'service']);
test('a failed conversion keeps the storage type that is on the server', function (string $action, bool $isDirectory) {
// The legacy parser stored this raw source, so the server step rejects it.
$volume = storageHostPathFileVolume('${AO_CONFIG_FILE:-./agent-orchestrator.yaml}', null, $isDirectory);
Livewire::test(FileStorage::class, ['fileStorage' => $volume])
->call($action)
->assertDispatched('error');
expect($volume->fresh()->is_directory)->toBe($isDirectory);
})->with([
'to file' => ['convertToFile', true],
'to directory' => ['convertToDirectory', false],
]);