Escape commands passed to run and execute

Each element in commands should be treated as a single argument so we should escape it as an argument to prevent anything from breakint out to execute anything else.
utopia-php · May 10, 2024 · a6b4bc9 · a6b4bc9
1 parent b5eaf86
commit a6b4bc9
Show file tree

Hide file tree

Showing 2 changed files with 52 additions and 10 deletions.
diff --git a/src/Orchestration/Adapter/DockerCLI.php b/src/Orchestration/Adapter/DockerCLI.php
@@ -113,6 +113,12 @@ public function getStats(string $container = null, array $filters = []): array
 
         $result = Console::execute('docker stats --no-trunc --format "id={{.ID}}&name={{.Name}}&cpu={{.CPUPerc}}&memory={{.MemPerc}}&diskIO={{.BlockIO}}&memoryIO={{.MemUsage}}&networkIO={{.NetIO}}" --no-stream'.$containersString, '', $output);
 
+        $dump = function($value) {
+            $p = var_export($value, true);
+            $b = debug_backtrace();
+            print($b[0]['file'] . ':' . $b[0]['line'] . ' - ' . $p . "\n");
+        };
+        $dump($output);
         if ($result !== 0) {
             throw new Orchestration("Docker Error: {$output}");
         }
@@ -312,9 +318,7 @@ public function run(string $image,
         $output = '';
 
         foreach ($command as $key => $value) {
-            if (str_contains($value, ' ')) {
-                $command[$key] = "'".$value."'";
-            }
+            $command[$key] = \escapeshellarg($command[$key]);
         }
 
         $labelString = '';
@@ -348,7 +352,7 @@ public function run(string $image,
 
         $time = time();
 
-        $result = Console::execute('docker run'.
+        $cmd = 'docker run'.
         ' -d'.
         ($remove ? ' --rm' : '').
         (empty($network) ? '' : " --network=\"{$network}\"").
@@ -366,7 +370,16 @@ public function run(string $image,
         (empty($hostname) ? '' : " --hostname {$hostname}").
         (empty($vars) ? '' : ' '.\implode(' ', $vars)).
         " {$image}".
-        (empty($command) ? '' : ' '.implode(' ', $command)), '', $output, 30);
+        (empty($command) ? '' : ' '.implode(' ', $command));
+
+        $dump = function($value) {
+            $p = var_export($value, true);
+            $b = debug_backtrace();
+            print($b[0]['file'] . ':' . $b[0]['line'] . ' - ' . $p . "\n");
+        };
+        $dump($cmd);
+
+        $result = Console::execute($cmd, '', $output, 30);
 
         if ($result !== 0) {
             throw new Orchestration("Docker Error: {$output}");
@@ -389,9 +402,7 @@ public function execute(
         int $timeout = -1
     ): bool {
         foreach ($command as $key => $value) {
-            if (str_contains($value, ' ')) {
-                $command[$key] = "'".$value."'";
-            }
+            $command[$key] = \escapeshellarg($command[$key]);
         }
 
         $parsedVariables = [];

diff --git a/tests/Orchestration/Base.php b/tests/Orchestration/Base.php
@@ -2,6 +2,7 @@
 
 namespace Utopia\Tests;
 
+use Utopia\CLI\Console;
 use PHPUnit\Framework\TestCase;
 use Utopia\Orchestration\Orchestration;
 
@@ -474,6 +475,16 @@ public function testUsageStats(): void
             mountFolder: __DIR__.'/Resources',
             labels: ['utopia-container-type' => 'stats']
         );
+        $dump = function($value) {
+            $p = var_export($value, true);
+            $b = debug_backtrace();
+            print($b[0]['file'] . ':' . $b[0]['line'] . ' - ' . $p . "\n");
+        };
+        $dump($containerId1);
+
+        $output = '';
+        Console::execute("docker logs $containerId1", '', $output);
+        $dump($output);
 
         $this->assertNotEmpty($containerId1);
 
@@ -488,18 +499,38 @@ public function testUsageStats(): void
             workdir: '/usr/local/src/',
             mountFolder: __DIR__.'/Resources',
         );
+        $dump($containerId2);
+
+        $output = '';
+        Console::execute("docker logs $containerId2", '', $output);
+        $dump($output);
 
         $this->assertNotEmpty($containerId2);
         sleep(2);
 
+        $output = '';
+        Console::execute('docker ps -a', '', $output);
+        $dump($output);
+
+        $output = '';
+        Console::execute('docker stats --no-stream', '', $output);
+        $dump($output);
+
         // This allows CPU-heavy load check
         $output = '';
-        static::getOrchestration()->execute($containerId1, ['screen', '-d', '-m', "'stress --cpu 1 --timeout 5'"], $output); // Run in screen so it's background task
-        static::getOrchestration()->execute($containerId2, ['screen', '-d', '-m', "'stress --cpu 1 --timeout 5'"], $output);
+        static::getOrchestration()->execute($containerId1, ['stress --cpu 1 --timeout 5'], $output); // Run in screen so it's background task
+        $dump($output);
+        $output = '';
+        static::getOrchestration()->execute($containerId2, ['stress --cpu 1 --timeout 5'], $output);
+        $dump($output);
 
         // Set CPU stress-test start
         \sleep(1);
 
+        $output = '';
+        Console::execute('docker stats --no-stream', '', $output);
+        $dump($output);
+
         // Fetch stats, should include high CPU usage
         $stats = static::getOrchestration()->getStats();