fix(ssh): preserve sessions when recycling mux connections (#11377)

app/Helpers/SshMultiplexingHelper.php

@@ -87,15 +87,48 @@ class SshMultiplexingHelper
             return false;
         }
 
-        self::storeConnectionMetadata($server);
-
         return true;
     }
 
     public static function removeMuxFile(Server $server): void
     {
-        Process::run(self::muxControlCommand($server, 'exit'));
-        self::clearConnectionMetadata($server);
+        $checkProcess = Process::run(self::muxControlCommand($server, 'check'));
+        $pid = preg_match('/pid=(\d+)/', $checkProcess->output().$checkProcess->errorOutput(), $matches)
+            ? $matches[1]
+            : null;
+
+        if ($pid !== null) {
+            self::markMuxProcessAsRetiring($pid, self::muxSocket($server));
+        }
+
+        $stopProcess = Process::run(self::muxControlCommand($server, 'stop'));
+
+        if ($pid !== null && ! $stopProcess->successful()) {
+            self::unmarkMuxProcessAsRetiring($pid, self::muxSocket($server));
+        }
+    }
+
+    public static function markMuxProcessAsRetiring(string $pid, string $muxSocket, ?string $processStartTime = null): void
+    {
+        $processStartTime ??= self::processStartTime($pid);
+        Cache::forever(self::muxProcessRetirementKey($pid, $muxSocket, $processStartTime), true);
+    }
+
+    public static function isMuxProcessRetiring(string $pid, string $muxSocket, ?string $processStartTime = null): bool
+    {
+        $processStartTime ??= self::processStartTime($pid);
+        $key = self::muxProcessRetirementKey($pid, $muxSocket, $processStartTime);
+        if (! Cache::has($key)) {
+            return false;
+        }
+
+        return true;
+    }
+
+    public static function unmarkMuxProcessAsRetiring(string $pid, string $muxSocket, ?string $processStartTime = null): void
+    {
+        $processStartTime ??= self::processStartTime($pid);
+        Cache::forget(self::muxProcessRetirementKey($pid, $muxSocket, $processStartTime));
     }
 
     public static function generateScpCommand(Server $server, string $source, string $dest): string
@@ -242,25 +275,6 @@ class SshMultiplexingHelper
         return $process->exitCode() === 0 && str_contains($process->output(), 'health_check_ok');
     }
 
-    public static function isConnectionExpired(Server $server): bool
-    {
-        $connectionAge = self::getConnectionAge($server);
-        $maxAge = config('constants.ssh.mux_max_age');
-
-        return $connectionAge !== null && $connectionAge > $maxAge;
-    }
-
-    public static function getConnectionAge(Server $server): ?int
-    {
-        $connectionTime = Cache::get("ssh_mux_connection_time_{$server->uuid}");
-
-        if ($connectionTime === null) {
-            return null;
-        }
-
-        return time() - $connectionTime;
-    }
-
     public static function refreshMultiplexedConnection(Server $server): bool
     {
         self::removeMuxFile($server);
@@ -273,6 +287,28 @@ class SshMultiplexingHelper
         return 'ssh_mux_lock_'.(gethostname() ?: 'unknown').'_'.$server->uuid;
     }
 
+    private static function muxProcessRetirementKey(string $pid, string $muxSocket, ?string $processStartTime): string
+    {
+        return 'ssh_mux_retiring_'.hash('sha256', self::processScope().'|'.$pid.'|'.$processStartTime.'|'.$muxSocket);
+    }
+
+    private static function processScope(): string
+    {
+        return (gethostname() ?: 'unknown').'|'.(@readlink('/proc/self/ns/pid') ?: 'unknown');
+    }
+
+    private static function processStartTime(string $pid): ?string
+    {
+        $stat = @file_get_contents("/proc/{$pid}/stat");
+        if ($stat === false || ! preg_match('/^\d+ \(.*\) (.*)$/', trim($stat), $matches)) {
+            return null;
+        }
+
+        $fields = preg_split('/\s+/', $matches[1]);
+
+        return $fields[19] ?? null;
+    }
+
     private static function masterConnectionExists(Server $server): bool
     {
         return Process::run(self::muxControlCommand($server, 'check'))->exitCode() === 0;
@@ -284,14 +320,6 @@ class SshMultiplexingHelper
             return false;
         }
 
-        if (self::getConnectionAge($server) === null) {
-            self::storeConnectionMetadata($server);
-        }
-
-        if (self::isConnectionExpired($server)) {
-            return false;
-        }
-
         if (config('constants.ssh.mux_health_check_enabled') && ! self::isConnectionHealthy($server)) {
             return false;
         }
@@ -382,14 +410,4 @@ class SshMultiplexingHelper
 
         return $options.'-p '.escapeshellarg((string) $server->port).' ';
     }
-
-    private static function storeConnectionMetadata(Server $server): void
-    {
-        Cache::put("ssh_mux_connection_time_{$server->uuid}", time(), config('constants.ssh.mux_persist_time') + 300);
-    }
-
-    private static function clearConnectionMetadata(Server $server): void
-    {
-        Cache::forget("ssh_mux_connection_time_{$server->uuid}");
-    }
 }

app/Jobs/CleanupStaleMultiplexedConnections.php

@@ -2,8 +2,8 @@
 
 namespace App\Jobs;
 
+use App\Helpers\SshMultiplexingHelper;
 use App\Models\Server;
-use Carbon\Carbon;
 use Illuminate\Bus\Queueable;
 use Illuminate\Contracts\Queue\ShouldQueue;
 use Illuminate\Foundation\Bus\Dispatchable;
@@ -51,7 +51,9 @@ class CleanupStaleMultiplexedConnections implements ShouldQueue
                 continue;
             }
 
-            if ($process['etimes'] >= $minAge && ! file_exists($pathMatch[1])) {
+            if ($process['etimes'] >= $minAge
+                && ! file_exists($pathMatch[1])
+                && ! SshMultiplexingHelper::isMuxProcessRetiring($process['pid'], $pathMatch[1])) {
                 $this->reapOrphan('ssh', $process);
             }
         }
@@ -169,14 +171,6 @@ class CleanupStaleMultiplexedConnections implements ShouldQueue
 
             if ($checkProcess->exitCode() !== 0) {
                 $this->removeMultiplexFile($muxFile, 'connection_check_failed');
-            } else {
-                $muxContent = Storage::disk('ssh-mux')->get($muxFile);
-                $establishedAt = Carbon::parse(substr($muxContent, 37));
-                $expirationTime = $establishedAt->addSeconds(config('constants.ssh.mux_persist_time'));
-
-                if (Carbon::now()->isAfter($expirationTime)) {
-                    $this->removeMultiplexFile($muxFile, 'expired');
-                }
             }
         }
     }
@@ -216,8 +210,20 @@ class CleanupStaleMultiplexedConnections implements ShouldQueue
         }
 
         $muxSocket = "/var/www/html/storage/app/ssh/mux/{$muxFile}";
-        $closeCommand = "ssh -O exit -o ControlPath={$muxSocket} localhost 2>/dev/null";
-        Process::run($closeCommand);
+        $checkProcess = Process::run("ssh -O check -o ControlPath={$muxSocket} localhost");
+        $pid = preg_match('/pid=(\d+)/', $checkProcess->output().$checkProcess->errorOutput(), $matches)
+            ? $matches[1]
+            : null;
+
+        if ($pid !== null) {
+            SshMultiplexingHelper::markMuxProcessAsRetiring($pid, $muxSocket);
+        }
+
+        $closeCommand = "ssh -O stop -o ControlPath={$muxSocket} localhost 2>/dev/null";
+        $stopProcess = Process::run($closeCommand);
+        if ($pid !== null && ! $stopProcess->successful()) {
+            SshMultiplexingHelper::unmarkMuxProcessAsRetiring($pid, $muxSocket);
+        }
         Storage::disk('ssh-mux')->delete($muxFile);
 
         Log::info('Removed stale mux file', [

config/constants.php

@@ -71,7 +71,6 @@ return [
         'mux_persist_time' => env('SSH_MUX_PERSIST_TIME', 3600),
         'mux_health_check_enabled' => env('SSH_MUX_HEALTH_CHECK_ENABLED', true),
         'mux_health_check_timeout' => env('SSH_MUX_HEALTH_CHECK_TIMEOUT', 5),
-        'mux_max_age' => env('SSH_MUX_MAX_AGE', 1800), // 30 minutes
         'mux_lock_ttl' => env('SSH_MUX_LOCK_TTL', 30), // lock auto-release, seconds
         'mux_lock_timeout' => env('SSH_MUX_LOCK_TIMEOUT', 10), // max wait for lock, seconds
         'mux_orphan_min_age' => env('SSH_MUX_ORPHAN_MIN_AGE', 600), // min process age before reaping orphans, seconds

tests/Feature/SshMultiplexingLockTest.php

@@ -87,25 +87,22 @@ it('reuses an existing healthy master without spawning a new one', function () {
     Process::assertNotRan(fn ($process) => str_contains($process->command, 'ssh -fN'));
 });
 
-it('refreshes an expired master before reuse', function () {
+it('reuses a healthy master regardless of its absolute age', function () {
     config([
         'constants.ssh.mux_enabled' => true,
         'constants.ssh.mux_health_check_enabled' => false,
-        'constants.ssh.mux_max_age' => 10,
     ]);
     $server = makeMuxServer();
-    Cache::put("ssh_mux_connection_time_{$server->uuid}", time() - 30, 3600);
+    Cache::put("ssh_mux_connection_time_{$server->uuid}", time() - 7200, 10800);
 
     Process::fake([
         '*-O check*' => Process::result(exitCode: 0),
-        '*-O exit*' => Process::result(exitCode: 0),
-        '*-fN *' => Process::result(exitCode: 0),
     ]);
 
     expect(SshMultiplexingHelper::ensureMultiplexedConnection($server))->toBeTrue();
 
-    Process::assertRan(fn ($process) => str_contains($process->command, 'ssh -O exit'));
-    Process::assertRan(fn ($process) => str_contains($process->command, 'ssh -fN '));
+    Process::assertNotRan(fn ($process) => str_contains($process->command, 'ssh -O stop'));
+    Process::assertNotRan(fn ($process) => str_contains($process->command, 'ssh -fN '));
 });
 
 it('does not spawn a master when the per-server lock is already held', function () {
@@ -241,6 +238,101 @@ it('kills only old orphaned ssh masters whose control socket no longer exists',
     File::delete($liveSocket);
 });
 
+it('does not reap an ssh master that is intentionally retiring', function () {
+    config(['constants.ssh.mux_orphan_reap_enabled' => true]);
+    $muxDir = storage_path('app/ssh/mux');
+    $retiringSocket = $muxDir.'/mux_retiring_'.uniqid();
+    SshMultiplexingHelper::markMuxProcessAsRetiring('222', $retiringSocket);
+
+    Process::fake([
+        'ps*' => Process::result(output: "222 1 5000 ssh -fN -o ControlMaster=auto -o ControlPath={$retiringSocket} root@1.2.3.4\n"),
+        'kill*' => Process::result(exitCode: 0),
+    ]);
+
+    $job = new CleanupStaleMultiplexedConnections;
+    $method = new ReflectionMethod($job, 'cleanupOrphanedSshProcesses');
+    $method->setAccessible(true);
+    $method->invoke($job);
+
+    Process::assertNotRan(fn ($process) => str_contains($process->command, 'kill'));
+});
+
+it('does not treat a reused pid as retiring', function () {
+    $socket = storage_path('app/ssh/mux/mux_original');
+
+    SshMultiplexingHelper::markMuxProcessAsRetiring('222', $socket, '1000');
+
+    expect(SshMultiplexingHelper::isMuxProcessRetiring('222', $socket, '1000'))->toBeTrue()
+        ->and(SshMultiplexingHelper::isMuxProcessRetiring('222', $socket, '2000'))->toBeFalse();
+});
+
+it('scopes retirement markers to the current host and pid namespace', function () {
+    $method = new ReflectionMethod(SshMultiplexingHelper::class, 'processScope');
+    $method->setAccessible(true);
+
+    expect($method->invoke(null))
+        ->toBeString()
+        ->toStartWith((gethostname() ?: 'unknown').'|');
+});
+
+it('keeps a retirement marker for long-running ssh sessions', function () {
+    $socket = storage_path('app/ssh/mux/mux_retired');
+
+    SshMultiplexingHelper::markMuxProcessAsRetiring('222', $socket);
+    $this->travel((int) config('constants.ssh.mux_persist_time') * 2 + 1)->seconds();
+
+    expect(SshMultiplexingHelper::isMuxProcessRetiring('222', $socket))->toBeTrue();
+});
+
+it('reads the process start time used to distinguish pid reuse', function () {
+    $method = new ReflectionMethod(SshMultiplexingHelper::class, 'processStartTime');
+    $method->setAccessible(true);
+
+    expect($method->invoke(null, (string) getmypid()))->toMatch('/^\d+$/');
+});
+
+it('marks a successfully stopped mux process as retiring', function () {
+    $server = makeMuxServer();
+    Process::fake([
+        '*-O check*' => Process::result(output: 'Master running (pid=222)', exitCode: 0),
+        '*-O stop*' => Process::result(exitCode: 0),
+    ]);
+
+    SshMultiplexingHelper::removeMuxFile($server);
+
+    expect(SshMultiplexingHelper::isMuxProcessRetiring('222', "/var/www/html/storage/app/ssh/mux/mux_{$server->uuid}"))->toBeTrue();
+});
+
+it('marks a mux process as retiring before stopping it', function () {
+    $server = makeMuxServer();
+    Process::fake([
+        '*-O check*' => Process::result(output: 'Master running (pid=555)', exitCode: 0),
+        '*-O stop*' => function () use ($server) {
+            expect(SshMultiplexingHelper::isMuxProcessRetiring('555', "/var/www/html/storage/app/ssh/mux/mux_{$server->uuid}"))->toBeTrue();
+
+            return Process::result(exitCode: 0);
+        },
+    ]);
+
+    SshMultiplexingHelper::removeMuxFile($server);
+});
+
+it('does not mark a mux process as retiring when stop fails', function () {
+    $server = makeMuxServer();
+    Process::fake([
+        '*-O check*' => Process::result(output: 'Master running (pid=444)', exitCode: 0),
+        '*-O stop*' => function () use ($server) {
+            expect(SshMultiplexingHelper::isMuxProcessRetiring('444', "/var/www/html/storage/app/ssh/mux/mux_{$server->uuid}"))->toBeTrue();
+
+            return Process::result(exitCode: 1);
+        },
+    ]);
+
+    SshMultiplexingHelper::removeMuxFile($server);
+
+    expect(SshMultiplexingHelper::isMuxProcessRetiring('444', "/var/www/html/storage/app/ssh/mux/mux_{$server->uuid}"))->toBeFalse();
+});
+
 it('kills only old orphaned cloudflared proxies whose parent ssh is gone', function () {
     config(['constants.ssh.mux_orphan_reap_enabled' => true]);
 
@@ -294,7 +386,10 @@ it('removes mux files for non-existent servers when reaping is enabled', functio
     Storage::fake('ssh-mux');
     $file = 'mux_ghost'.uniqid();
     Storage::disk('ssh-mux')->put($file, 'x');
-    Process::fake();
+    Process::fake([
+        '*-O check*' => Process::result(errorOutput: 'Master running (pid=333)', exitCode: 0),
+        '*-O stop*' => Process::result(exitCode: 0),
+    ]);
 
     $job = new CleanupStaleMultiplexedConnections;
     $method = new ReflectionMethod($job, 'cleanupNonExistentServerConnections');
@@ -302,6 +397,72 @@ it('removes mux files for non-existent servers when reaping is enabled', functio
     $method->invoke($job);
 
     expect(Storage::disk('ssh-mux')->exists($file))->toBeFalse();
+    expect(SshMultiplexingHelper::isMuxProcessRetiring('333', "/var/www/html/storage/app/ssh/mux/{$file}"))->toBeTrue();
+    Process::assertRan(fn ($process) => str_contains($process->command, 'ssh -O stop'));
+    Process::assertNotRan(fn ($process) => str_contains($process->command, 'ssh -O exit'));
+});
+
+it('marks a stale mux process as retiring before stopping it', 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([
+        '*-O check*' => Process::result(output: 'Master running (pid=666)', exitCode: 0),
+        '*-O stop*' => function () use ($file) {
+            expect(SshMultiplexingHelper::isMuxProcessRetiring('666', "/var/www/html/storage/app/ssh/mux/{$file}"))->toBeTrue();
+
+            return Process::result(exitCode: 0);
+        },
+    ]);
+
+    $job = new CleanupStaleMultiplexedConnections;
+    $method = new ReflectionMethod($job, 'cleanupNonExistentServerConnections');
+    $method->setAccessible(true);
+    $method->invoke($job);
+});
+
+it('removes a stale mux retirement marker when stopping fails', function () {
+    config(['constants.ssh.mux_orphan_reap_enabled' => true]);
+    Storage::fake('ssh-mux');
+    $file = 'mux_ghost'.uniqid();
+    $muxSocket = "/var/www/html/storage/app/ssh/mux/{$file}";
+    Storage::disk('ssh-mux')->put($file, 'x');
+    Process::fake([
+        '*-O check*' => Process::result(output: 'Master running (pid=777)', exitCode: 0),
+        '*-O stop*' => function () use ($muxSocket) {
+            expect(SshMultiplexingHelper::isMuxProcessRetiring('777', $muxSocket))->toBeTrue();
+
+            return Process::result(exitCode: 1);
+        },
+    ]);
+
+    $job = new CleanupStaleMultiplexedConnections;
+    $method = new ReflectionMethod($job, 'cleanupNonExistentServerConnections');
+    $method->setAccessible(true);
+    $method->invoke($job);
+
+    expect(SshMultiplexingHelper::isMuxProcessRetiring('777', $muxSocket))->toBeFalse();
+});
+
+it('does not remove a healthy mux connection based on its absolute age', function () {
+    config(['constants.ssh.mux_orphan_reap_enabled' => true]);
+    Storage::fake('ssh-mux');
+    $server = makeMuxServer();
+    $file = "mux_{$server->uuid}";
+    Storage::disk('ssh-mux')->put($file, str_repeat('x', 37).now()->subHours(2)->toIso8601String());
+
+    Process::fake([
+        '*-O check*' => Process::result(exitCode: 0),
+    ]);
+
+    $job = new CleanupStaleMultiplexedConnections;
+    $method = new ReflectionMethod($job, 'cleanupStaleConnections');
+    $method->setAccessible(true);
+    $method->invoke($job);
+
+    expect(Storage::disk('ssh-mux')->exists($file))->toBeTrue();
+    Process::assertNotRan(fn ($process) => str_contains($process->command, 'ssh -O stop'));
 });
 
 it('keeps mux files for non-existent servers in dry-run mode', function () {