From 48ba4ece3c1b43cb4b9627438c0ff4e4251e3511 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Sat, 28 Mar 2026 12:28:54 +0100 Subject: [PATCH] fix: harden GetLogs Livewire component with locked properties and input validation Add #[Locked] attributes to security-sensitive properties (resource, servicesubtype, server, container) to prevent client-side modification via Livewire wire protocol. Add container name validation using ValidationPatterns::isValidContainerName() and server ownership authorization via Server::ownedByCurrentTeam() in both getLogs() and downloadAllLogs() methods. Co-Authored-By: Claude Opus 4.6 --- app/Livewire/Project/Shared/GetLogs.php | 32 ++++- tests/Feature/GetLogsCommandInjectionTest.php | 110 ++++++++++++++++++ 2 files changed, 137 insertions(+), 5 deletions(-) create mode 100644 tests/Feature/GetLogsCommandInjectionTest.php diff --git a/app/Livewire/Project/Shared/GetLogs.php b/app/Livewire/Project/Shared/GetLogs.php index 22605e1bb..d0121bdc5 100644 --- a/app/Livewire/Project/Shared/GetLogs.php +++ b/app/Livewire/Project/Shared/GetLogs.php @@ -16,7 +16,9 @@ use App\Models\StandaloneMysql; use App\Models\StandalonePostgresql; use App\Models\StandaloneRedis; +use App\Support\ValidationPatterns; use Illuminate\Support\Facades\Process; +use Livewire\Attributes\Locked; use Livewire\Component; class GetLogs extends Component @@ -29,12 +31,16 @@ class GetLogs extends Component public string $errors = ''; + #[Locked] public Application|Service|StandalonePostgresql|StandaloneRedis|StandaloneMongodb|StandaloneMysql|StandaloneMariadb|StandaloneKeydb|StandaloneDragonfly|StandaloneClickhouse|null $resource = null; + #[Locked] public ServiceApplication|ServiceDatabase|null $servicesubtype = null; + #[Locked] public Server $server; + #[Locked] public ?string $container = null; public ?string $displayName = null; @@ -54,7 +60,7 @@ class GetLogs extends Component public function mount() { if (! is_null($this->resource)) { - if ($this->resource->getMorphClass() === \App\Models\Application::class) { + if ($this->resource->getMorphClass() === Application::class) { $this->showTimeStamps = $this->resource->settings->is_include_timestamps; } else { if ($this->servicesubtype) { @@ -63,7 +69,7 @@ public function mount() $this->showTimeStamps = $this->resource->is_include_timestamps; } } - if ($this->resource?->getMorphClass() === \App\Models\Application::class) { + if ($this->resource?->getMorphClass() === Application::class) { if (str($this->container)->contains('-pr-')) { $this->pull_request = 'Pull Request: '.str($this->container)->afterLast('-pr-')->beforeLast('_')->value(); } @@ -74,11 +80,11 @@ public function mount() public function instantSave() { if (! is_null($this->resource)) { - if ($this->resource->getMorphClass() === \App\Models\Application::class) { + if ($this->resource->getMorphClass() === Application::class) { $this->resource->settings->is_include_timestamps = $this->showTimeStamps; $this->resource->settings->save(); } - if ($this->resource->getMorphClass() === \App\Models\Service::class) { + if ($this->resource->getMorphClass() === Service::class) { $serviceName = str($this->container)->beforeLast('-')->value(); $subType = $this->resource->applications()->where('name', $serviceName)->first(); if ($subType) { @@ -118,10 +124,20 @@ public function toggleStreamLogs() public function getLogs($refresh = false) { + if (! Server::ownedByCurrentTeam()->where('id', $this->server->id)->exists()) { + $this->outputs = 'Unauthorized.'; + + return; + } if (! $this->server->isFunctional()) { return; } - if (! $refresh && ! $this->expandByDefault && ($this->resource?->getMorphClass() === \App\Models\Service::class || str($this->container)->contains('-pr-'))) { + if ($this->container && ! ValidationPatterns::isValidContainerName($this->container)) { + $this->outputs = 'Invalid container name.'; + + return; + } + if (! $refresh && ! $this->expandByDefault && ($this->resource?->getMorphClass() === Service::class || str($this->container)->contains('-pr-'))) { return; } if ($this->numberOfLines <= 0 || is_null($this->numberOfLines)) { @@ -194,9 +210,15 @@ public function copyLogs(): string public function downloadAllLogs(): string { + if (! Server::ownedByCurrentTeam()->where('id', $this->server->id)->exists()) { + return ''; + } if (! $this->server->isFunctional() || ! $this->container) { return ''; } + if (! ValidationPatterns::isValidContainerName($this->container)) { + return ''; + } if ($this->showTimeStamps) { if ($this->server->isSwarm()) { diff --git a/tests/Feature/GetLogsCommandInjectionTest.php b/tests/Feature/GetLogsCommandInjectionTest.php new file mode 100644 index 000000000..34824b48b --- /dev/null +++ b/tests/Feature/GetLogsCommandInjectionTest.php @@ -0,0 +1,110 @@ +getAttributes(Locked::class); + + expect($attributes)->not->toBeEmpty(); + }); + + test('server property has Locked attribute', function () { + $property = new ReflectionProperty(GetLogs::class, 'server'); + $attributes = $property->getAttributes(Locked::class); + + expect($attributes)->not->toBeEmpty(); + }); + + test('resource property has Locked attribute', function () { + $property = new ReflectionProperty(GetLogs::class, 'resource'); + $attributes = $property->getAttributes(Locked::class); + + expect($attributes)->not->toBeEmpty(); + }); + + test('servicesubtype property has Locked attribute', function () { + $property = new ReflectionProperty(GetLogs::class, 'servicesubtype'); + $attributes = $property->getAttributes(Locked::class); + + expect($attributes)->not->toBeEmpty(); + }); +}); + +describe('GetLogs container name validation in getLogs', function () { + test('getLogs method validates container name with ValidationPatterns', function () { + $method = new ReflectionMethod(GetLogs::class, 'getLogs'); + $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('downloadAllLogs method validates container name with ValidationPatterns', function () { + $method = new ReflectionMethod(GetLogs::class, 'downloadAllLogs'); + $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'); + }); +}); + +describe('GetLogs authorization checks', function () { + test('getLogs method checks server ownership via ownedByCurrentTeam', function () { + $method = new ReflectionMethod(GetLogs::class, 'getLogs'); + $startLine = $method->getStartLine(); + $endLine = $method->getEndLine(); + $lines = array_slice(file($method->getFileName()), $startLine - 1, $endLine - $startLine + 1); + $methodBody = implode('', $lines); + + expect($methodBody)->toContain('Server::ownedByCurrentTeam()'); + }); + + test('downloadAllLogs method checks server ownership via ownedByCurrentTeam', function () { + $method = new ReflectionMethod(GetLogs::class, 'downloadAllLogs'); + $startLine = $method->getStartLine(); + $endLine = $method->getEndLine(); + $lines = array_slice(file($method->getFileName()), $startLine - 1, $endLine - $startLine + 1); + $methodBody = implode('', $lines); + + expect($methodBody)->toContain('Server::ownedByCurrentTeam()'); + }); +}); + +describe('GetLogs container name injection payloads are blocked by validation', function () { + test('newline injection payload is rejected', function () { + // The exact PoC payload from the advisory + $payload = "postgresql 2>/dev/null\necho '===RCE-START==='\nid\nwhoami\nhostname\ncat /etc/hostname\necho '===RCE-END==='\n#"; + expect(ValidationPatterns::isValidContainerName($payload))->toBeFalse(); + }); + + test('semicolon injection payload is rejected', function () { + expect(ValidationPatterns::isValidContainerName('postgresql;id'))->toBeFalse(); + }); + + test('backtick injection payload is rejected', function () { + expect(ValidationPatterns::isValidContainerName('postgresql`id`'))->toBeFalse(); + }); + + test('command substitution injection payload is rejected', function () { + expect(ValidationPatterns::isValidContainerName('postgresql$(whoami)'))->toBeFalse(); + }); + + test('pipe injection payload is rejected', function () { + expect(ValidationPatterns::isValidContainerName('postgresql|cat /etc/passwd'))->toBeFalse(); + }); + + test('valid container names are accepted', function () { + expect(ValidationPatterns::isValidContainerName('postgresql'))->toBeTrue(); + expect(ValidationPatterns::isValidContainerName('my-app-container'))->toBeTrue(); + expect(ValidationPatterns::isValidContainerName('service_db.v2'))->toBeTrue(); + expect(ValidationPatterns::isValidContainerName('coolify-proxy'))->toBeTrue(); + }); +});