diff --git a/app/Http/Controllers/Api/ServiceApplicationsController.php b/app/Http/Controllers/Api/ServiceApplicationsController.php index de07f2e33..a144b3ea6 100644 --- a/app/Http/Controllers/Api/ServiceApplicationsController.php +++ b/app/Http/Controllers/Api/ServiceApplicationsController.php @@ -455,9 +455,8 @@ public function logs_by_uuid(Request $request): JsonResponse } $containerName = $serviceApplication->name.'-'.$serviceApplication->service->uuid; - $safeContainerName = escapeshellarg($containerName); - $status = getContainerStatus($server, $safeContainerName); + $status = getContainerStatus($server, $containerName); if ($status !== 'running') { return response()->json([ 'message' => 'Service application container is not running.', diff --git a/bootstrap/helpers/docker.php b/bootstrap/helpers/docker.php index e6643a337..0440ae352 100644 --- a/bootstrap/helpers/docker.php +++ b/bootstrap/helpers/docker.php @@ -178,13 +178,18 @@ function executeInDocker(string $containerId, string $command) return "docker exec {$containerId} bash -c '{$escapedCommand}'"; } -function getContainerStatus(Server $server, string $container_id, bool $all_data = false, bool $throwError = false) +function buildContainerStatusCommand(Server $server, string $container_id): string { if ($server->isSwarm()) { - $container = instant_remote_process(["docker service ls --filter 'name={$container_id}' --format '{{json .}}' "], $server, $throwError); - } else { - $container = instant_remote_process(["docker inspect --format '{{json .}}' {$container_id}"], $server, $throwError); + return 'docker service ls --filter '.escapeshellarg("name={$container_id}")." --format '{{json .}}' "; } + + return "docker inspect --format '{{json .}}' ".escapeshellarg($container_id); +} + +function getContainerStatus(Server $server, string $container_id, bool $all_data = false, bool $throwError = false) +{ + $container = instant_remote_process([buildContainerStatusCommand($server, $container_id)], $server, $throwError); if (! $container) { return 'exited'; } diff --git a/tests/Feature/Security/CommandInjectionSecurityTest.php b/tests/Feature/Security/CommandInjectionSecurityTest.php index 0234a9539..42c08c29d 100644 --- a/tests/Feature/Security/CommandInjectionSecurityTest.php +++ b/tests/Feature/Security/CommandInjectionSecurityTest.php @@ -1016,12 +1016,20 @@ 'stop action' => 'Actions/Service/StopServiceApplication.php', ]); - test('service application logs endpoint escapes container name before docker helpers', function () { + test('docker status helper escapes container names', function () { + $source = file_get_contents(base_path('bootstrap/helpers/docker.php')); + + expect($source)->toContain('function buildContainerStatusCommand') + ->and($source)->toContain('escapeshellarg("name={$container_id}")') + ->and($source)->toContain('escapeshellarg($container_id)'); + }); + + test('service application logs endpoint passes raw container name to docker helpers', function () { $source = file_get_contents(app_path('Http/Controllers/Api/ServiceApplicationsController.php')); - expect($source)->toContain('$safeContainerName = escapeshellarg($containerName)') - ->and($source)->toContain('getContainerStatus($server, $safeContainerName)') - ->and($source)->toContain('getContainerLogs($server, $containerName, $lines)'); + expect($source)->toContain('getContainerStatus($server, $containerName)') + ->and($source)->toContain('getContainerLogs($server, $containerName, $lines)') + ->and($source)->not->toContain('$safeContainerName = escapeshellarg($containerName)'); }); });