fix(ssh): rely on lazy multiplexed connections

Remove explicit SSH master pre-warming and lock handling so OpenSSH manages ControlMaster creation lazily from real ssh/scp commands. Add cleanup for duplicate mux processes and update coverage around mux command options and stale process cleanup.

tests/Feature/SshMultiplexingLockTest.php

@@ -6,15 +6,14 @@ use App\Models\PrivateKey;
 use App\Models\Server;
 use App\Models\User;
 use Illuminate\Foundation\Testing\RefreshDatabase;
-use Illuminate\Support\Facades\Cache;
 use Illuminate\Support\Facades\File;
 use Illuminate\Support\Facades\Process;
 use Illuminate\Support\Facades\Storage;
 
 /**
- * Tests for the per-server lock that prevents concurrent workers from each
- * spawning their own SSH master, which leaves orphaned non-master ssh
- * connections that ControlPersist never reaps (memory leak).
+ * SSH multiplexing now relies on OpenSSH's native lazy ControlMaster handling.
+ * Coolify should add mux options to real ssh/scp commands, but must not pre-warm
+ * background masters with separate `ssh -fN` processes.
  */
 uses(RefreshDatabase::class);
 
@@ -23,80 +22,91 @@ function makeMuxServer(): Server
     $user = User::factory()->create();
     $team = $user->teams()->first();
 
+    $privateKeyContent = "-----BEGIN OPENSSH PRIVATE KEY-----\n".
+        "b3BlbnNzaC1rZXktdjEAAAAABG5vbmUAAAAEbm9uZQAAAAAAAAABAAAAMwAAAAtzc2gtZW\n".
+        "QyNTUxOQAAACBbhpqHhqv6aI67Mj9abM3DVbmcfYhZAhC7ca4d9UCevAAAAJi/QySHv0Mk\n".
+        "hwAAAAtzc2gtZWQyNTUxOQAAACBbhpqHhqv6aI67Mj9abM3DVbmcfYhZAhC7ca4d9UCevA\n".
+        "AAAECBQw4jg1WRT2IGHMncCiZhURCts2s24HoDS0thHnnRKVuGmoeGq/pojrsyP1pszcNV\n".
+        "uZx9iFkCELtxrh31QJ68AAAAEXNhaWxANzZmZjY2ZDJlMmRkAQIDBA==\n".
+        '-----END OPENSSH PRIVATE KEY-----';
+
     $privateKey = PrivateKey::create([
         'name' => 'mux-test-key-'.uniqid(),
-        'private_key' => "-----BEGIN OPENSSH PRIVATE KEY-----\n".
-            "b3BlbnNzaC1rZXktdjEAAAAABG5vbmUAAAAEbm9uZQAAAAAAAAABAAAAMwAAAAtzc2gtZW\n".
-            "QyNTUxOQAAACBbhpqHhqv6aI67Mj9abM3DVbmcfYhZAhC7ca4d9UCevAAAAJi/QySHv0Mk\n".
-            "hwAAAAtzc2gtZWQyNTUxOQAAACBbhpqHhqv6aI67Mj9abM3DVbmcfYhZAhC7ca4d9UCevA\n".
-            "AAAECBQw4jg1WRT2IGHMncCiZhURCts2s24HoDS0thHnnRKVuGmoeGq/pojrsyP1pszcNV\n".
-            "uZx9iFkCELtxrh31QJ68AAAAEXNhaWxANzZmZjY2ZDJlMmRkAQIDBA==\n".
-            '-----END OPENSSH PRIVATE KEY-----',
+        'private_key' => $privateKeyContent,
         'team_id' => $team->id,
     ]);
 
+    Storage::fake('ssh-keys');
+    Storage::disk('ssh-keys')->put("ssh_key@{$privateKey->uuid}", $privateKeyContent);
+
     return Server::factory()->create([
         'team_id' => $team->id,
         'private_key_id' => $privateKey->id,
     ]);
 }
 
-it('establishes a master with ssh -fN and never the orphan-prone ssh -fNM', function () {
+it('does not prewarm a background ssh master', function () {
     config(['constants.ssh.mux_enabled' => true]);
     $server = makeMuxServer();
 
-    Process::fake([
-        '*-O check*' => Process::result(exitCode: 1), // no existing master
-        '*-fN *' => Process::result(exitCode: 0),      // establish succeeds
-    ]);
+    Process::fake();
 
     expect(SshMultiplexingHelper::ensureMultiplexedConnection($server))->toBeTrue();
 
-    Process::assertRan(fn ($process) => str_contains($process->command, 'ssh -fN ')
-        && ! str_contains($process->command, 'ssh -fNM'));
+    Process::assertNothingRan();
 });
 
-it('reuses an existing healthy master without spawning a new one', function () {
-    config([
-        'constants.ssh.mux_enabled' => true,
-        'constants.ssh.mux_health_check_enabled' => true,
-    ]);
+it('adds native openssh multiplexing options to ssh commands', function () {
+    config(['constants.ssh.mux_enabled' => true]);
+    $server = makeMuxServer();
+    Storage::disk('ssh-keys')->put("ssh_key@{$server->privateKey->uuid}", $server->privateKey->private_key);
+
+    Process::fake();
+
+    $command = SshMultiplexingHelper::generateSshCommand($server, 'echo ok');
+
+    expect($command)
+        ->toContain('-o ControlMaster=auto')
+        ->toContain("-o ControlPath=/var/www/html/storage/app/ssh/mux/mux_{$server->uuid}")
+        ->toContain('-o ControlPersist=3600')
+        ->not->toContain('ssh -fN')
+        ->not->toContain('-O check');
+
+    Process::assertNothingRan();
+});
+
+it('omits native multiplexing options when ssh multiplexing is disabled for a command', function () {
+    config(['constants.ssh.mux_enabled' => true]);
+    $server = makeMuxServer();
+    Storage::disk('ssh-keys')->put("ssh_key@{$server->privateKey->uuid}", $server->privateKey->private_key);
+
+    $command = SshMultiplexingHelper::generateSshCommand($server, 'echo ok', disableMultiplexing: true);
+
+    expect($command)
+        ->not->toContain('-o ControlMaster=auto')
+        ->not->toContain('-o ControlPath=')
+        ->not->toContain('-o ControlPersist=');
+});
+
+it('adds native openssh multiplexing options to scp commands', function () {
+    config(['constants.ssh.mux_enabled' => true]);
     $server = makeMuxServer();
 
-    Process::fake([
-        '*-O check*' => Process::result(exitCode: 0),
-        '*health_check_ok*' => Process::result(output: 'health_check_ok', exitCode: 0),
-    ]);
+    Process::fake();
 
-    expect(SshMultiplexingHelper::ensureMultiplexedConnection($server))->toBeTrue();
+    $command = SshMultiplexingHelper::generateScpCommand($server, '/tmp/source', '/tmp/dest');
 
-    Process::assertNotRan(fn ($process) => str_contains($process->command, 'ssh -fN'));
+    expect($command)
+        ->toContain('-o ControlMaster=auto')
+        ->toContain("-o ControlPath=/var/www/html/storage/app/ssh/mux/mux_{$server->uuid}")
+        ->toContain('-o ControlPersist=3600')
+        ->not->toContain('ssh -fN')
+        ->not->toContain('-O check');
+
+    Process::assertNothingRan();
 });
 
-it('does not spawn a master when the per-server lock is already held', function () {
-    config([
-        'constants.ssh.mux_enabled' => true,
-        'constants.ssh.mux_lock_timeout' => 0,
-    ]);
-    $server = makeMuxServer();
-
-    Process::fake([
-        '*-O check*' => Process::result(exitCode: 1), // forces the slow path
-    ]);
-
-    // Simulate another worker on the same host holding the lock for this server.
-    $lockKey = 'ssh_mux_lock_'.(gethostname() ?: 'unknown').'_'.$server->uuid;
-    $held = Cache::lock($lockKey, 30);
-    expect($held->get())->toBeTrue();
-
-    expect(SshMultiplexingHelper::ensureMultiplexedConnection($server))->toBeFalse();
-
-    Process::assertNotRan(fn ($process) => str_contains($process->command, 'ssh -fN '));
-
-    $held->release();
-});
-
-it('returns false and runs no ssh when multiplexing is disabled', function () {
+it('returns false and runs no process when multiplexing is globally disabled', function () {
     config(['constants.ssh.mux_enabled' => false]);
     $server = makeMuxServer();
 
@@ -115,9 +125,8 @@ it('kills only old orphaned ssh masters whose control socket no longer exists',
     $liveSocket = $muxDir.'/mux_live_'.uniqid();
     $orphanSocket = $muxDir.'/mux_orphan_'.uniqid();
     $youngSocket = $muxDir.'/mux_young_'.uniqid();
-    File::put($liveSocket, 'x'); // live master owns its socket file; the others do not
+    File::put($liveSocket, 'x');
 
-    // Columns: pid ppid etimes args
     Process::fake([
         'ps*' => Process::result(output: "111 1 5000 ssh -fN -o ControlMaster=auto -o ControlPath={$liveSocket} root@1.2.3.4\n".
             "222 1 5000 ssh -fN -o ControlMaster=auto -o ControlPath={$orphanSocket} root@1.2.3.4\n".
@@ -130,11 +139,8 @@ it('kills only old orphaned ssh masters whose control socket no longer exists',
     $method->setAccessible(true);
     $method->invoke($job);
 
-    // Old orphan: killed.
     Process::assertRan(fn ($process) => str_contains($process->command, 'kill') && str_contains($process->command, '222'));
-    // Live master (socket exists): spared.
     Process::assertNotRan(fn ($process) => str_contains($process->command, 'kill') && str_contains($process->command, '111'));
-    // Young process (may be mid-establish): spared despite missing socket.
     Process::assertNotRan(fn ($process) => str_contains($process->command, 'kill') && str_contains($process->command, '333'));
 
     File::delete($liveSocket);
@@ -142,8 +148,7 @@ it('kills only old orphaned ssh masters whose control socket no longer exists',
 
 it('kills only old orphaned cloudflared proxies whose parent ssh is gone', function () {
     config(['constants.ssh.mux_orphan_reap_enabled' => true]);
-    // pid 100 = live ssh master; 200 = its legit child; 300 = old orphan;
-    // 400 = young orphan (parent ssh may still be starting up).
+
     Process::fake([
         'ps*' => Process::result(output: "100 1 5000 ssh -fN -o ControlMaster=auto root@1.2.3.4\n".
             "200 100 5000 cloudflared access ssh --hostname host.example.com\n".
@@ -158,11 +163,8 @@ it('kills only old orphaned cloudflared proxies whose parent ssh is gone', funct
     $method->setAccessible(true);
     $method->invoke($job);
 
-    // Old orphan (parent not ssh): killed.
     Process::assertRan(fn ($process) => str_contains($process->command, 'kill') && str_contains($process->command, '300'));
-    // Legit proxy (parent ssh alive): spared.
     Process::assertNotRan(fn ($process) => str_contains($process->command, 'kill') && str_contains($process->command, '200'));
-    // Young orphan: spared.
     Process::assertNotRan(fn ($process) => str_contains($process->command, 'kill') && str_contains($process->command, '400'));
 });
 
@@ -171,7 +173,7 @@ it('dry-run mode logs orphans but kills nothing when reaping is disabled', funct
     $muxDir = storage_path('app/ssh/mux');
     File::ensureDirectoryExists($muxDir);
 
-    $orphanSocket = $muxDir.'/mux_orphan_'.uniqid(); // no file: a real old orphan
+    $orphanSocket = $muxDir.'/mux_orphan_'.uniqid();
 
     Process::fake([
         'ps*' => Process::result(output: "222 1 5000 ssh -fN -o ControlMaster=auto -o ControlPath={$orphanSocket} root@1.2.3.4\n"),
@@ -183,16 +185,38 @@ it('dry-run mode logs orphans but kills nothing when reaping is disabled', funct
     $method->setAccessible(true);
     $method->invoke($job);
 
-    // Orphan detected, but dry-run: nothing killed.
     Process::assertNotRan(fn ($process) => str_contains($process->command, 'kill'));
 });
 
+it('resets duplicate ssh mux process groups atomically when reaping is enabled', function () {
+    config(['constants.ssh.mux_orphan_reap_enabled' => true]);
+    $muxDir = storage_path('app/ssh/mux');
+    File::ensureDirectoryExists($muxDir);
+    $controlPath = $muxDir.'/mux_duplicate_'.uniqid();
+    File::put($controlPath, 'socket');
+
+    Process::fake([
+        'ps*' => Process::result(output: "111 1 5000 ssh -fN -o ControlMaster=auto -o ControlPath={$controlPath} root@1.2.3.4\n".
+            "222 1 5000 ssh -fN -o ControlMaster=auto -o ControlPath={$controlPath} root@1.2.3.4\n"),
+        'kill*' => Process::result(exitCode: 0),
+    ]);
+
+    $job = new CleanupStaleMultiplexedConnections;
+    $method = new ReflectionMethod($job, 'cleanupDuplicateSshProcesses');
+    $method->setAccessible(true);
+    $method->invoke($job);
+
+    Process::assertRan(fn ($process) => str_contains($process->command, 'kill') && str_contains($process->command, '111'));
+    Process::assertRan(fn ($process) => str_contains($process->command, 'kill') && str_contains($process->command, '222'));
+    expect(file_exists($controlPath))->toBeFalse();
+});
+
 it('removes mux files for non-existent servers when reaping is enabled', function () {
     config(['constants.ssh.mux_orphan_reap_enabled' => true]);
     Storage::fake('ssh-mux');
     $file = 'mux_ghost'.uniqid();
     Storage::disk('ssh-mux')->put($file, 'x');
-    Process::fake(); // the `ssh -O exit` close command
+    Process::fake();
 
     $job = new CleanupStaleMultiplexedConnections;
     $method = new ReflectionMethod($job, 'cleanupNonExistentServerConnections');