refactor: enhance SSH command handling and auto-detect sudo requirements for Docker commands

inc/InstallProtocolManager.php

@@ -1737,10 +1737,11 @@ class InstallProtocolManager
         $binaryCmd = '/usr/local/bin/aivpn-server';
         
         // Verify the binary exists, fallback to other locations if needed
+        // Use auto-detection for sudo requirement (null = auto-detect for docker commands)
         $checkCmd = sprintf('docker exec -i %s test -f %s && echo "found" || echo "not found"',
             escapeshellarg($containerName),
             escapeshellarg($binaryCmd));
-        $checkResult = (string) $server->executeCommand($checkCmd, true);
+        $checkResult = (string) $server->executeCommand($checkCmd, null);
         if (strpos($checkResult, 'found') === false) {
             // Try alternative locations
             $fallbacks = [
@@ -1753,7 +1754,7 @@ class InstallProtocolManager
                 $checkCmd = sprintf('docker exec -i %s test -f %s && echo "found" || echo "not found"',
                     escapeshellarg($containerName),
                     escapeshellarg($loc));
-                $checkResult = (string) $server->executeCommand($checkCmd, true);
+                $checkResult = (string) $server->executeCommand($checkCmd, null);
                 if (strpos($checkResult, 'found') !== false) {
                     $binaryCmd = $loc;
                     break;
@@ -1784,38 +1785,50 @@ class InstallProtocolManager
         Logger::appendInstall($server->getId(), 'Adding AIVPN client via builtin add_client: ' . $clientName . ' in ' . $containerName);
         
         try {
-                    $output = (string) $server->executeCommand($cmd, true);
+            // Use auto-detection for sudo requirement (null = auto-detect for docker commands)
-                } catch (Exception $e) {
+            $output = (string) $server->executeCommand($cmd, null);
-                    // Container may be restarting or unavailable - try host binary fallback
+        } catch (Exception $e) {
-                    Logger::appendInstall($server->getId(), 'AIVPN add_client docker exec failed (container may be restarting): ' . $e->getMessage());
+            Logger::appendInstall($server->getId(), 'AIVPN add_client docker exec failed: ' . $e->getMessage());
-                    $hostResult = self::runAivpnAddClientViaHostBinary($server, $clientName, $serverHost, $serverPort);
+            $hostResult = self::runAivpnAddClientViaHostBinary($server, $clientName, $serverHost, $serverPort);
-                    if ($hostResult !== null) {
+            if ($hostResult !== null) {
-                        return $hostResult;
+                return $hostResult;
-                    }
+            }
-                    return ['success' => true, 'connection_key' => '', 'connection_uri' => ''];
+            return ['success' => true, 'connection_key' => '', 'connection_uri' => ''];
-                }
+        }
         
-                // Check if docker exec returned an error (container not running, etc.)
+        $trimmedOutput = trim($output);
-                $trimmedOutput = trim($output);
+        if ($trimmedOutput === '' ||
-                if ($trimmedOutput === '' ||
+            stripos($trimmedOutput, 'Error response from daemon') !== false ||
-                    stripos($trimmedOutput, 'Error response from daemon') !== false ||
+            stripos($trimmedOutput, 'is restarting') !== false ||
-                    stripos($trimmedOutput, 'is restarting') !== false ||
+            stripos($trimmedOutput, 'No such container') !== false ||
-                    stripos($trimmedOutput, 'No such container') !== false ||
+            stripos($trimmedOutput, 'executable file not found') !== false) {
-                    stripos($trimmedOutput, 'executable file not found') !== false) {
+            Logger::appendInstall($server->getId(), 'AIVPN add_client container unavailable, trying host binary fallback');
-                    // Container unavailable - try host binary fallback
+            $hostResult = self::runAivpnAddClientViaHostBinary($server, $clientName, $serverHost, $serverPort);
-                    Logger::appendInstall($server->getId(), 'AIVPN add_client container unavailable, trying host binary fallback');
+            if ($hostResult !== null) {
-                    $hostResult = self::runAivpnAddClientViaHostBinary($server, $clientName, $serverHost, $serverPort);
+                return $hostResult;
-                    if ($hostResult !== null) {
+            }
-                        return $hostResult;
+            return ['success' => true, 'connection_key' => '', 'connection_uri' => ''];
-                    }
+        }
-                    return ['success' => true, 'connection_key' => '', 'connection_uri' => ''];
+
-                }
+        if (stripos($trimmedOutput, 'error') !== false || stripos($trimmedOutput, 'failed') !== false) {
+            Logger::appendInstall($server->getId(), 'AIVPN add_client returned error: ' . substr($trimmedOutput, 0, 200));
+            $hostResult = self::runAivpnAddClientViaHostBinary($server, $clientName, $serverHost, $serverPort);
+            if ($hostResult !== null) {
+                return $hostResult;
+            }
+            return ['success' => false, 'error' => $trimmedOutput];
+        }
         
         $parsed = self::parseAivpnAddClientOutput($output);
 
         if (empty($parsed['connection_uri']) && empty($parsed['connection_key'])) {
             $head = substr(str_replace(["\r", "\n"], ' ', $trimmedOutput), 0, 220);
-            throw new Exception('AIVPN add_client succeeded but no connection key found in output: ' . $head);
+            Logger::appendInstall($server->getId(), 'AIVPN add_client no connection key in output: ' . $head);
+            $hostResult = self::runAivpnAddClientViaHostBinary($server, $clientName, $serverHost, $serverPort);
+            if ($hostResult !== null) {
+                return $hostResult;
+            }
+            return ['success' => false, 'error' => 'No connection key found'];
         }
 
         $result = ['success' => true];

inc/ServerMonitoring.php

@@ -1118,8 +1118,8 @@ class ServerMonitoring
             
             $sshOptions = '-o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -o ConnectTimeout=5';
             $sshCmd = sprintf(
-                "sshpass -p '%s' ssh -p %d %s %s@%s %s 2>/dev/null",
+                "sshpass -p %s ssh -p %d %s %s@%s %s 2>/dev/null",
-                $password,
+                escapeshellarg($password),
                 $port,
                 $sshOptions,
                 $username,
@@ -1200,8 +1200,8 @@ class ServerMonitoring
             
             $sshOptions = '-o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -o ConnectTimeout=5';
             $sshCmd = sprintf(
-                "sshpass -p '%s' ssh -p %d %s %s@%s %s 2>/dev/null",
+                "sshpass -p %s ssh -p %d %s %s@%s %s 2>/dev/null",
-                $password,
+                escapeshellarg($password),
                 $port,
                 $sshOptions,
                 $username,

inc/VpnClient.php

@@ -810,8 +810,8 @@ class VpnClient
 
         $escaped = escapeshellarg($cmd);
         $sshCmd = sprintf(
-            "sshpass -p '%s' ssh -p %d -q -o LogLevel=ERROR -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no -o PreferredAuthentications=password -o PubkeyAuthentication=no %s@%s %s 2>&1",
+            "sshpass -p %s ssh -p %d -q -o LogLevel=ERROR -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no -o PreferredAuthentications=password -o PubkeyAuthentication=no %s@%s %s 2>&1",
-            $serverData['password'],
+            escapeshellarg($serverData['password']),
             $serverData['port'],
             $serverData['username'],
             $serverData['host'],
@@ -1308,8 +1308,8 @@ class VpnClient
         $run = static function (string $cmd) use ($serverData): string {
             $escapedCommand = escapeshellarg($cmd);
             $sshCommand = sprintf(
-                "sshpass -p '%s' ssh  -p %d -q -o LogLevel=ERROR -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no -o PreferredAuthentications=password -o PubkeyAuthentication=no %s@%s %s 2>&1",
+                "sshpass -p %s ssh  -p %d -q -o LogLevel=ERROR -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no -o PreferredAuthentications=password -o PubkeyAuthentication=no %s@%s %s 2>&1",
-                $serverData['password'],
+                escapeshellarg($serverData['password']),
                 $serverData['port'],
                 $serverData['username'],
                 $serverData['host'],

inc/VpnServer.php

@@ -9,6 +9,13 @@ class VpnServer
     private $serverId;
     private $data;
     
+    /**
+     * Cache for docker sudo requirements per server.
+     * null = not tested, true = needs sudo, false = no sudo needed
+     * @var array<int, bool|null>
+     */
+    private static $dockerSudoCache = [];
+
     public function __construct(?int $serverId = null)
     {
         $this->serverId = $serverId;
@@ -427,8 +434,8 @@ class VpnServer
         } else {
             $sshOptions .= " -o PreferredAuthentications=password -o PubkeyAuthentication=no";
             $testCommand = sprintf(
-                "sshpass -p '%s' ssh -p %d %s %s@%s 'echo test' 2>/dev/null",
+                "sshpass -p %s ssh -p %d %s %s@%s 'echo test' 2>/dev/null",
-                $this->data['password'],
+                escapeshellarg($this->data['password']),
                 $this->data['port'],
                 $sshOptions,
                 $this->data['username'],
@@ -446,12 +453,23 @@ class VpnServer
     }
 
     /**
-     * Execute command on remote server
+     * Execute command on remote server.
+     *
+     * @param string $command The command to execute
+     * @param bool|null $sudo True = use sudo, false = no sudo, null = auto-detect for docker commands
+     * @return string The command output
      */
-    public function executeCommand(string $command, bool $sudo = false): string
+    public function executeCommand(string $command, bool $sudo = null): string
     {
         $baseCommand = $command;
         $pathPrefix = 'export PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:$PATH; ';
+        $isDockerCommand = preg_match('/(^|\\n)docker(\\s|$)/', ltrim($baseCommand));
+        
+        // Auto-detect sudo requirement for docker commands when $sudo is null
+        if ($sudo === null && $isDockerCommand) {
+            $sudo = $this->detectDockerSudoRequirement();
+        }
+        
         $escapedCommand = '';
         $needsSudo = false;
 
@@ -477,7 +495,7 @@ class VpnServer
                 $escapedCommand
             );
         } else {
-            $needsSudo = $sudo && strtolower((string) ($this->data['username'] ?? '')) !== 'root';
+            $needsSudo = ($sudo ?? false) && strtolower((string) ($this->data['username'] ?? '')) !== 'root';
             if ($needsSudo) {
                 // Suppress sudo prompt text to keep command output machine-parseable.
                 $command = "echo '{$this->data['password']}' | sudo -S -p '' " . $command;
@@ -488,8 +506,8 @@ class VpnServer
 
             $sshOptions .= " -o PreferredAuthentications=password -o PubkeyAuthentication=no";
             $sshCommand = sprintf(
-                "sshpass -p '%s' ssh -p %d %s %s@%s %s 2>&1",
+                "sshpass -p %s ssh -p %d %s %s@%s %s 2>&1",
-                $this->data['password'],
+                escapeshellarg($this->data['password']),
                 $this->data['port'],
                 $sshOptions,
                 $this->data['username'],
@@ -504,13 +522,18 @@ class VpnServer
         if (
             empty($this->data['ssh_key'])
             && !empty($needsSudo)
-            && preg_match('/(^|\\n)docker(\\s|$)/', ltrim($baseCommand))
+            && $isDockerCommand
             && preg_match('/incorrect password attempts|sorry, try again|a password is required/i', $output)
         ) {
+            // Update cache: this server doesn't need sudo for docker
+            if ($this->serverId !== null) {
+                self::$dockerSudoCache[$this->serverId] = false;
+            }
+            
             $escapedBaseCommand = escapeshellarg($pathPrefix . $baseCommand);
             $sshCommandNoSudo = sprintf(
-                "sshpass -p '%s' ssh -p %d %s %s@%s %s 2>&1",
+                "sshpass -p %s ssh -p %d %s %s@%s %s 2>&1",
-                $this->data['password'],
+                escapeshellarg($this->data['password']),
                 $this->data['port'],
                 $sshOptions,
                 $this->data['username'],
@@ -527,6 +550,78 @@ class VpnServer
         return $output;
     }
 
+    /**
+     * Detect whether docker commands require sudo on this server.
+     * Uses a simple test command to check if docker works without sudo.
+     * Results are cached per server instance.
+     *
+     * @return bool True if sudo is needed, false if docker works without sudo
+     */
+    private function detectDockerSudoRequirement(): bool
+    {
+        // Return cached result if available
+        if ($this->serverId !== null && array_key_exists($this->serverId, self::$dockerSudoCache)) {
+            return self::$dockerSudoCache[$this->serverId];
+        }
+
+        // Test if docker works without sudo using a simple version check
+        $testCmd = 'docker --version 2>&1';
+        $pathPrefix = 'export PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:$PATH; ';
+        
+        $sshOptions = '-o LogLevel=ERROR -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no';
+        $keyFile = '';
+
+        if (!empty($this->data['ssh_key'])) {
+            $keyFile = tempnam(sys_get_temp_dir(), 'sshkey');
+            file_put_contents($keyFile, self::normalizeSshKey($this->data['ssh_key']));
+            chmod($keyFile, 0600);
+            $sshOptions .= " -i {$keyFile} -o IdentitiesOnly=yes -o PubkeyAuthentication=yes -o PreferredAuthentications=publickey";
+
+            $preparedCommand = $pathPrefix . $testCmd;
+            $escapedCommand = escapeshellarg($preparedCommand);
+
+            $sshCommand = sprintf(
+                "ssh -p %d %s %s@%s %s 2>&1",
+                $this->data['port'],
+                $sshOptions,
+                $this->data['username'],
+                $this->data['host'],
+                $escapedCommand
+            );
+        } else {
+            // For password auth, first try without sudo
+            $preparedCommand = $pathPrefix . $testCmd;
+            $escapedCommand = escapeshellarg($preparedCommand);
+
+            $sshOptions .= " -o PreferredAuthentications=password -o PubkeyAuthentication=no";
+            $sshCommand = sprintf(
+                "sshpass -p %s ssh -p %d %s %s@%s %s 2>&1",
+                escapeshellarg($this->data['password']),
+                $this->data['port'],
+                $sshOptions,
+                $this->data['username'],
+                $this->data['host'],
+                $escapedCommand
+            );
+        }
+
+        $output = shell_exec($sshCommand) ?? '';
+
+        if ($keyFile && file_exists($keyFile)) {
+            unlink($keyFile);
+        }
+
+        // Check if docker command succeeded (output contains "version")
+        $dockerWorks = stripos($output, 'version') !== false;
+        
+        // Cache the result
+        if ($this->serverId !== null) {
+            self::$dockerSudoCache[$this->serverId] = !$dockerWorks;
+        }
+
+        return !$dockerWorks;
+    }
+
     /**
      * Install Docker on remote server
      */