Improve file storage handling

This commit is contained in:
Andras Bacsai
2026-09-22 21:13:39 +02:00
parent 121c376c78
commit 99dd634ba3
8 changed files with 277 additions and 25 deletions
@@ -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,
+11 -8
View File
@@ -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,
+73
View File
@@ -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
{
+74 -1
View File
@@ -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.')
+87
View File
@@ -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);
+2 -1
View File
@@ -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}');
});