mirror of
https://github.com/coollabsio/coolify.git
synced 2026-09-26 09:20:54 -04:00
Improve file storage handling (#11943)
This commit is contained in:
@@ -5294,11 +5294,16 @@ class ApplicationsController extends Controller
|
||||
], 422);
|
||||
}
|
||||
|
||||
$fsPath = str($request->fs_path)->trim()->start('/')->value();
|
||||
$mountPath = str($request->mount_path)->trim()->start('/')->value();
|
||||
|
||||
validateShellSafePath($fsPath, 'storage source path');
|
||||
validateShellSafePath($mountPath, 'storage destination path');
|
||||
try {
|
||||
$fsPath = confinePathToBase(application_configuration_dir().'/'.$application->uuid, $request->fs_path, 'storage source path');
|
||||
$mountPath = validateFileMountPath($request->mount_path, 'storage destination path');
|
||||
LocalFileVolume::assertRemotePathIsConfined($application->workdir(), $fsPath, $application->destination->server);
|
||||
} catch (\Throwable $e) {
|
||||
return response()->json([
|
||||
'message' => 'Validation failed.',
|
||||
'errors' => ['fs_path' => $e->getMessage()],
|
||||
], 422);
|
||||
}
|
||||
|
||||
$storage = LocalFileVolume::create([
|
||||
'fs_path' => $fsPath,
|
||||
|
||||
@@ -4264,11 +4264,16 @@ class DatabasesController extends Controller
|
||||
], 422);
|
||||
}
|
||||
|
||||
$fsPath = str($request->fs_path)->trim()->start('/')->value();
|
||||
$mountPath = str($request->mount_path)->trim()->start('/')->value();
|
||||
|
||||
validateShellSafePath($fsPath, 'storage source path');
|
||||
validateShellSafePath($mountPath, 'storage destination path');
|
||||
try {
|
||||
$fsPath = confinePathToBase(database_configuration_dir().'/'.$database->uuid, $request->fs_path, 'storage source path');
|
||||
$mountPath = validateFileMountPath($request->mount_path, 'storage destination path');
|
||||
LocalFileVolume::assertRemotePathIsConfined($database->workdir(), $fsPath, $database->destination->server);
|
||||
} catch (\Throwable $e) {
|
||||
return response()->json([
|
||||
'message' => 'Validation failed.',
|
||||
'errors' => ['fs_path' => $e->getMessage()],
|
||||
], 422);
|
||||
}
|
||||
|
||||
$storage = LocalFileVolume::create([
|
||||
'fs_path' => $fsPath,
|
||||
|
||||
@@ -2551,11 +2551,16 @@ class ServicesController extends Controller
|
||||
], 422);
|
||||
}
|
||||
|
||||
$fsPath = str($request->fs_path)->trim()->start('/')->value();
|
||||
$mountPath = str($request->mount_path)->trim()->start('/')->value();
|
||||
|
||||
validateShellSafePath($fsPath, 'storage source path');
|
||||
validateShellSafePath($mountPath, 'storage destination path');
|
||||
try {
|
||||
$fsPath = confinePathToBase(service_configuration_dir().'/'.$service->uuid, $request->fs_path, 'storage source path');
|
||||
$mountPath = validateFileMountPath($request->mount_path, 'storage destination path');
|
||||
LocalFileVolume::assertRemotePathIsConfined($service->workdir(), $fsPath, $service->server);
|
||||
} catch (\Throwable $e) {
|
||||
return response()->json([
|
||||
'message' => 'Validation failed.',
|
||||
'errors' => ['fs_path' => $e->getMessage()],
|
||||
], 422);
|
||||
}
|
||||
|
||||
$storage = LocalFileVolume::create([
|
||||
'fs_path' => $fsPath,
|
||||
|
||||
@@ -312,14 +312,17 @@ class Storage extends Component
|
||||
'file_storage_directory_destination' => 'required|string',
|
||||
]);
|
||||
|
||||
$this->file_storage_directory_source = trim($this->file_storage_directory_source);
|
||||
$this->file_storage_directory_source = str($this->file_storage_directory_source)->start('/')->value();
|
||||
$this->file_storage_directory_destination = trim($this->file_storage_directory_destination);
|
||||
$this->file_storage_directory_destination = str($this->file_storage_directory_destination)->start('/')->value();
|
||||
|
||||
// Validate paths to prevent command injection
|
||||
validateShellSafePath($this->file_storage_directory_source, 'storage source path');
|
||||
validateShellSafePath($this->file_storage_directory_destination, 'storage destination path');
|
||||
$this->file_storage_directory_source = confinePathToBase(
|
||||
$this->fileStorageHostPath(),
|
||||
$this->file_storage_directory_source,
|
||||
'storage source path'
|
||||
);
|
||||
$this->file_storage_directory_destination = validateFileMountPath(
|
||||
$this->file_storage_directory_destination,
|
||||
'storage destination path'
|
||||
);
|
||||
$server = $this->resource->service?->server ?? $this->resource->destination->server;
|
||||
LocalFileVolume::assertRemotePathIsConfined($this->fileStorageHostPath(), $this->file_storage_directory_source, $server);
|
||||
|
||||
LocalFileVolume::create([
|
||||
'fs_path' => $this->file_storage_directory_source,
|
||||
|
||||
@@ -125,6 +125,11 @@ class LocalFileVolume extends BaseModel
|
||||
$path = $workdir.$path;
|
||||
}
|
||||
|
||||
if (! $this->isAdminControlledComposeMount()) {
|
||||
$path = str(confinePathToBase($workdir, $path->value(), 'storage path'));
|
||||
$this->assertRemotePathIsConfined($workdir, $path->value(), $server);
|
||||
}
|
||||
|
||||
// Validate and escape path to prevent command injection
|
||||
validateShellSafePath($path, 'storage path');
|
||||
$escapedPath = escapeshellarg($path);
|
||||
@@ -204,6 +209,11 @@ class LocalFileVolume extends BaseModel
|
||||
$path = $workdir.$path;
|
||||
}
|
||||
|
||||
if (! $this->isAdminControlledComposeMount()) {
|
||||
$path = str(confinePathToBase($workdir, $path->value(), 'storage path'));
|
||||
$this->assertRemotePathIsConfined($workdir, $path->value(), $server);
|
||||
}
|
||||
|
||||
// Validate and escape path to prevent command injection
|
||||
validateShellSafePath($path, 'storage path');
|
||||
$escapedPath = escapeshellarg($path);
|
||||
@@ -264,6 +274,11 @@ class LocalFileVolume extends BaseModel
|
||||
$path = $workdir.$path;
|
||||
}
|
||||
|
||||
if (! $this->isAdminControlledComposeMount()) {
|
||||
$path = str(confinePathToBase($workdir, $path->value(), 'storage path'));
|
||||
$this->assertRemotePathIsConfined($workdir, $path->value(), $server);
|
||||
}
|
||||
|
||||
// Validate and escape resolved path (may differ from fs_path if relative)
|
||||
validateShellSafePath($path, 'storage path');
|
||||
$escapedPath = escapeshellarg($path);
|
||||
@@ -315,6 +330,64 @@ class LocalFileVolume extends BaseModel
|
||||
return instant_remote_process($commands, $server);
|
||||
}
|
||||
|
||||
/**
|
||||
* Reject symlink escapes immediately before a managed path is used remotely.
|
||||
*/
|
||||
public static function assertRemotePathIsConfined(string $baseDirectory, string $path, Server $server): void
|
||||
{
|
||||
$escapedBase = escapeshellarg($baseDirectory);
|
||||
$escapedPath = escapeshellarg($path);
|
||||
$result = instant_remote_process([
|
||||
"base=\$(realpath -m -- {$escapedBase}) && target=\$(realpath -m -- {$escapedPath}) && case \"\$target\" in \"\$base\"|\"\$base\"/*) echo OK ;; *) echo NOK ;; esac",
|
||||
], $server, false);
|
||||
|
||||
if (trim((string) $result) !== 'OK') {
|
||||
throw new \RuntimeException('Invalid storage path: resolved path must stay inside the resource configuration directory.');
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Raw Compose bind mounts keep administrator-selected host path semantics.
|
||||
*/
|
||||
protected function isAdminControlledComposeMount(): bool
|
||||
{
|
||||
$compose = data_get($this->resource, 'docker_compose_raw')
|
||||
?? data_get($this->resource, 'service.docker_compose_raw');
|
||||
|
||||
if (! is_string($compose) || $compose === '') {
|
||||
return false;
|
||||
}
|
||||
|
||||
try {
|
||||
$services = data_get(Yaml::parse($compose), 'services', []);
|
||||
foreach ($services as $service) {
|
||||
foreach (data_get($service, 'volumes', []) as $volume) {
|
||||
if (is_string($volume)) {
|
||||
$parsed = parseDockerVolumeString($volume);
|
||||
$source = data_get($parsed, 'source');
|
||||
$target = data_get($parsed, 'target');
|
||||
} else {
|
||||
$source = data_get($volume, 'source');
|
||||
$target = data_get($volume, 'target');
|
||||
}
|
||||
|
||||
if ((string) $target !== $this->mount_path || ! sourceIsLocal(str((string) $source))) {
|
||||
continue;
|
||||
}
|
||||
|
||||
$resolvedSource = replaceLocalSource(str((string) $source), str($this->resource->workdir()));
|
||||
if (normalizeUnixPath($resolvedSource->value()) === normalizeUnixPath($this->fs_path)) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
}
|
||||
} catch (\Throwable) {
|
||||
return false;
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
// Accessor for convenient access
|
||||
protected function plainMountPath(): Attribute
|
||||
{
|
||||
|
||||
@@ -17,7 +17,9 @@ use App\Models\Team;
|
||||
use App\Models\User;
|
||||
use Illuminate\Foundation\Testing\RefreshDatabase;
|
||||
use Illuminate\Support\Facades\Bus;
|
||||
use Illuminate\Support\Facades\Crypt;
|
||||
use Illuminate\Support\Facades\DB;
|
||||
use Illuminate\Support\Facades\Process;
|
||||
use Illuminate\Support\Str;
|
||||
use Livewire\Livewire;
|
||||
|
||||
@@ -35,7 +37,7 @@ beforeEach(function () {
|
||||
$keyId = DB::table('private_keys')->insertGetId([
|
||||
'uuid' => (string) Str::uuid(),
|
||||
'name' => 'Test Key',
|
||||
'private_key' => 'test-key',
|
||||
'private_key' => Crypt::encryptString('test-key'),
|
||||
'team_id' => $this->team->id,
|
||||
'created_at' => now(),
|
||||
'updated_at' => now(),
|
||||
@@ -45,6 +47,7 @@ beforeEach(function () {
|
||||
'team_id' => $this->team->id,
|
||||
'private_key_id' => $keyId,
|
||||
]);
|
||||
Process::fake(fn () => Process::result(output: 'OK'));
|
||||
|
||||
StandaloneDocker::withoutEvents(function () {
|
||||
$this->destination = StandaloneDocker::firstOrCreate(
|
||||
@@ -84,6 +87,76 @@ test('livewire file storage rejects parent segments and does not create a local
|
||||
expect(LocalFileVolume::query()->count())->toBe(0);
|
||||
});
|
||||
|
||||
test('livewire directory storage rejects a server absolute path and has no side effects', function () {
|
||||
Livewire::test(Storage::class, ['resource' => $this->application])
|
||||
->set('file_storage_directory_source', '/root/.ssh')
|
||||
->set('file_storage_directory_destination', '/data')
|
||||
->call('submitFileStorageDirectory')
|
||||
->assertDispatched('error');
|
||||
|
||||
expect(LocalFileVolume::query()->count())->toBe(0);
|
||||
Bus::assertNotDispatched(ServerStorageSaveJob::class);
|
||||
});
|
||||
|
||||
test('livewire directory storage accepts a path inside the application root', function () {
|
||||
$source = application_configuration_dir().'/'.$this->application->uuid.'/data/new';
|
||||
|
||||
$component = Livewire::test(Storage::class, ['resource' => $this->application])
|
||||
->set('file_storage_directory_source', $source)
|
||||
->set('file_storage_directory_destination', '/data')
|
||||
->call('submitFileStorageDirectory');
|
||||
$component->assertDispatched('success');
|
||||
|
||||
expect(LocalFileVolume::query()->sole()->fs_path)->toBe($source);
|
||||
});
|
||||
|
||||
test('livewire directory storage rejects a remote symlink escape without side effects', function () {
|
||||
Process::fake(fn () => Process::result(output: 'NOK'));
|
||||
|
||||
Livewire::test(Storage::class, ['resource' => $this->application])
|
||||
->set('file_storage_directory_source', $this->application->workdir().'/linked/outside')
|
||||
->set('file_storage_directory_destination', '/data')
|
||||
->call('submitFileStorageDirectory')
|
||||
->assertDispatched('error');
|
||||
|
||||
expect(LocalFileVolume::query()->count())->toBe(0);
|
||||
Bus::assertNotDispatched(ServerStorageSaveJob::class);
|
||||
});
|
||||
|
||||
test('normal team members cannot create directory storage', function () {
|
||||
$member = User::factory()->create();
|
||||
$member->teams()->attach($this->team, ['role' => 'member']);
|
||||
$this->actingAs($member);
|
||||
session(['currentTeam' => $this->team]);
|
||||
|
||||
Livewire::test(Storage::class, ['resource' => $this->application])
|
||||
->set('file_storage_directory_source', application_configuration_dir().'/'.$this->application->uuid.'/data')
|
||||
->set('file_storage_directory_destination', '/data')
|
||||
->call('submitFileStorageDirectory')
|
||||
->assertDispatched('error');
|
||||
|
||||
expect(LocalFileVolume::query()->count())->toBe(0);
|
||||
});
|
||||
|
||||
test('an administrator cannot create storage on a cross-team resource', function () {
|
||||
$otherTeam = Team::factory()->create();
|
||||
$otherProject = Project::factory()->create(['team_id' => $otherTeam->id]);
|
||||
$otherEnvironment = Environment::factory()->create(['project_id' => $otherProject->id]);
|
||||
$otherApplication = Application::factory()->create([
|
||||
'environment_id' => $otherEnvironment->id,
|
||||
'destination_id' => $this->destination->id,
|
||||
'destination_type' => $this->destination->getMorphClass(),
|
||||
]);
|
||||
|
||||
Livewire::test(Storage::class, ['resource' => $otherApplication])
|
||||
->set('file_storage_directory_source', application_configuration_dir().'/'.$otherApplication->uuid.'/data')
|
||||
->set('file_storage_directory_destination', '/data')
|
||||
->call('submitFileStorageDirectory')
|
||||
->assertDispatched('error');
|
||||
|
||||
expect(LocalFileVolume::query()->count())->toBe(0);
|
||||
});
|
||||
|
||||
test('file mount modal shows the calculated host file path above the destination input', function () {
|
||||
Livewire::test(Storage::class, ['resource' => $this->application])
|
||||
->assertSeeText('Create a managed file on the host and mount it inside the container.')
|
||||
|
||||
@@ -16,6 +16,9 @@ use App\Models\Team;
|
||||
use App\Models\User;
|
||||
use Illuminate\Foundation\Testing\RefreshDatabase;
|
||||
use Illuminate\Support\Facades\Bus;
|
||||
use Illuminate\Support\Facades\Crypt;
|
||||
use Illuminate\Support\Facades\DB;
|
||||
use Illuminate\Support\Facades\Process;
|
||||
use Illuminate\Support\Str;
|
||||
|
||||
uses(RefreshDatabase::class);
|
||||
@@ -39,6 +42,16 @@ beforeEach(function () {
|
||||
$this->bearerToken = $token->getKey().'|'.$plainTextToken;
|
||||
|
||||
$this->server = Server::factory()->create(['team_id' => $this->team->id]);
|
||||
$keyId = DB::table('private_keys')->insertGetId([
|
||||
'uuid' => (string) Str::uuid(),
|
||||
'name' => 'Storage Test Key',
|
||||
'private_key' => Crypt::encryptString('test-key'),
|
||||
'team_id' => $this->team->id,
|
||||
'created_at' => now(),
|
||||
'updated_at' => now(),
|
||||
]);
|
||||
$this->server->update(['private_key_id' => $keyId]);
|
||||
Process::fake(fn () => Process::result(output: 'OK'));
|
||||
$this->destination = StandaloneDocker::where('server_id', $this->server->id)->first();
|
||||
$this->project = Project::factory()->create(['team_id' => $this->team->id]);
|
||||
$this->environment = Environment::factory()->create(['project_id' => $this->project->id]);
|
||||
@@ -89,6 +102,7 @@ function createTestServiceApplication($context): array
|
||||
{
|
||||
$service = Service::factory()->create([
|
||||
'environment_id' => $context->environment->id,
|
||||
'server_id' => $context->server->id,
|
||||
'destination_id' => $context->destination->id,
|
||||
'destination_type' => $context->destination->getMorphClass(),
|
||||
]);
|
||||
@@ -170,6 +184,22 @@ describe('GET /api/v1/applications/{uuid}/storages', function () {
|
||||
});
|
||||
|
||||
describe('POST /api/v1/applications/{uuid}/storages', function () {
|
||||
test('rejects an application directory mount outside its managed root without side effects', function () {
|
||||
$app = createTestApplication($this);
|
||||
|
||||
$this->withHeaders([
|
||||
'Authorization' => 'Bearer '.$this->bearerToken,
|
||||
])->postJson("/api/v1/applications/{$app->uuid}/storages", [
|
||||
'type' => 'file',
|
||||
'is_directory' => true,
|
||||
'fs_path' => '/root/.ssh/authorized_keys',
|
||||
'mount_path' => '/data',
|
||||
])->assertUnprocessable();
|
||||
|
||||
expect($app->fileStorages()->exists())->toBeFalse();
|
||||
Bus::assertNotDispatched(ServerStorageSaveJob::class);
|
||||
});
|
||||
|
||||
test('creates a persistent storage', function () {
|
||||
$app = createTestApplication($this);
|
||||
|
||||
@@ -655,6 +685,63 @@ test('rejects host paths when creating persistent storage through the API', func
|
||||
expect($resource->persistentStorages()->count())->toBe($storageCountBefore);
|
||||
})->with(['application', 'database', 'service']);
|
||||
|
||||
test('rejects directory mounts outside each resource configuration root', function (string $resourceType) {
|
||||
if ($resourceType === 'database') {
|
||||
$resource = createTestDatabase($this);
|
||||
$url = "/api/v1/databases/{$resource->uuid}/storages";
|
||||
$payload = [];
|
||||
} else {
|
||||
[$service, $resource] = createTestServiceApplication($this);
|
||||
$url = "/api/v1/services/{$service->uuid}/storages";
|
||||
$payload = ['resource_uuid' => $resource->uuid];
|
||||
}
|
||||
|
||||
$this->withHeaders([
|
||||
'Authorization' => 'Bearer '.$this->bearerToken,
|
||||
])->postJson($url, [...$payload, ...[
|
||||
'type' => 'file',
|
||||
'is_directory' => true,
|
||||
'fs_path' => '/etc/shadow',
|
||||
'mount_path' => '/data',
|
||||
]])->assertUnprocessable();
|
||||
|
||||
expect($resource->fileStorages()->exists())->toBeFalse();
|
||||
Bus::assertNotDispatched(ServerStorageSaveJob::class);
|
||||
})->with(['database', 'service']);
|
||||
|
||||
test('rejects a directory mount through a remote symlink before creating storage', function (string $resourceType) {
|
||||
if ($resourceType === 'application') {
|
||||
$resource = createTestApplication($this);
|
||||
$url = "/api/v1/applications/{$resource->uuid}/storages";
|
||||
$payload = [];
|
||||
$base = application_configuration_dir().'/'.$resource->uuid;
|
||||
} elseif ($resourceType === 'database') {
|
||||
$resource = createTestDatabase($this);
|
||||
$url = "/api/v1/databases/{$resource->uuid}/storages";
|
||||
$payload = [];
|
||||
$base = database_configuration_dir().'/'.$resource->uuid;
|
||||
} else {
|
||||
[$service, $resource] = createTestServiceApplication($this);
|
||||
$url = "/api/v1/services/{$service->uuid}/storages";
|
||||
$payload = ['resource_uuid' => $resource->uuid];
|
||||
$base = service_configuration_dir().'/'.$service->uuid;
|
||||
}
|
||||
|
||||
Process::fake(fn () => Process::result(output: 'NOK'));
|
||||
|
||||
$response = $this->withHeaders(['Authorization' => 'Bearer '.$this->bearerToken])
|
||||
->postJson($url, [...$payload, ...[
|
||||
'type' => 'file',
|
||||
'is_directory' => true,
|
||||
'fs_path' => $base.'/linked/outside',
|
||||
'mount_path' => '/data',
|
||||
]]);
|
||||
$response->assertUnprocessable();
|
||||
|
||||
expect($resource->fileStorages()->exists())->toBeFalse();
|
||||
Bus::assertNotDispatched(ServerStorageSaveJob::class);
|
||||
})->with(['application', 'database', 'service']);
|
||||
|
||||
test('rejects host paths when updating persistent storage through the API', function (string $resourceType) {
|
||||
if ($resourceType === 'application') {
|
||||
$resource = createTestApplication($this);
|
||||
|
||||
@@ -204,7 +204,8 @@ test('confined path resolver rejects paths that escape the resource configuratio
|
||||
test('local file volume write sink keeps saved managed file paths for compatibility', function () {
|
||||
$source = file_get_contents(__DIR__.'/../../app/Models/LocalFileVolume.php');
|
||||
|
||||
expect($source)->not->toContain('confinePathToBase($workdir, $path->value(), \'storage path\')')
|
||||
expect($source)->toContain('confinePathToBase($workdir, $path->value(), \'storage path\')')
|
||||
->and($source)->toContain('assertRemotePathIsConfined')
|
||||
->and($source)->toContain('tee {$escapedPath}');
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user