fix: prevent sporadic SSH permission denied by validating key content

The root cause of sporadic "Permission denied (publickey)" errors was
that validateSshKey() only checked if the key file existed on disk,
never verifying its content matched the database. When keys were rotated
or updated, the stale file persisted and SSH used the wrong key.

Changes:
- validateSshKey() now refreshes key from DB and compares file content
- Server saved event detects private_key_id changes to invalidate mux
- PrivateKey storeInFileSystem() uses file locking to prevent races
- PrivateKey saved event auto-resyncs file on key content changes
- Enforces 0600 permissions on key files

Fixes coollabsio/coolify#7724

app/Helpers/SshMultiplexingHelper.php

@@ -8,6 +8,7 @@
 use Illuminate\Support\Facades\Hash;
 use Illuminate\Support\Facades\Log;
 use Illuminate\Support\Facades\Process;
+use Illuminate\Support\Facades\Storage;
 
 class SshMultiplexingHelper
 {
@@ -208,13 +209,37 @@ private static function isMultiplexingEnabled(): bool
 
     private static function validateSshKey(PrivateKey $privateKey): void
     {
-        $keyLocation = $privateKey->getKeyLocation();
-        $checkKeyCommand = "ls $keyLocation 2>/dev/null";
-        $keyCheckProcess = Process::run($checkKeyCommand);
+        $privateKey->refresh();
 
-        if ($keyCheckProcess->exitCode() !== 0) {
+        $keyLocation = $privateKey->getKeyLocation();
+        $filename = "ssh_key@{$privateKey->uuid}";
+        $disk = Storage::disk('ssh-keys');
+
+        $needsRewrite = false;
+
+        if (! $disk->exists($filename)) {
+            $needsRewrite = true;
+        } else {
+            $diskContent = $disk->get($filename);
+            if ($diskContent !== $privateKey->private_key) {
+                Log::warning('SSH key file content does not match database, resyncing', [
+                    'key_uuid' => $privateKey->uuid,
+                ]);
+                $needsRewrite = true;
+            }
+        }
+
+        if ($needsRewrite) {
             $privateKey->storeInFileSystem();
         }
+
+        // Ensure correct permissions (SSH requires 0600)
+        if (file_exists($keyLocation)) {
+            $currentPerms = fileperms($keyLocation) & 0777;
+            if ($currentPerms !== 0600) {
+                chmod($keyLocation, 0600);
+            }
+        }
     }
 
     private static function getCommonSshOptions(Server $server, string $sshKeyLocation, int $connectionTimeout, int $serverInterval, bool $isScp = false): string

app/Models/PrivateKey.php

@@ -65,6 +65,20 @@ protected static function booted()
             }
         });
 
+        static::saved(function ($key) {
+            if ($key->wasChanged('private_key')) {
+                try {
+                    $key->storeInFileSystem();
+                    refresh_server_connection($key);
+                } catch (\Exception $e) {
+                    \Illuminate\Support\Facades\Log::error('Failed to resync SSH key after update', [
+                        'key_uuid' => $key->uuid,
+                        'error' => $e->getMessage(),
+                    ]);
+                }
+            }
+        });
+
         static::deleted(function ($key) {
             self::deleteFromStorage($key);
         });
@@ -185,29 +199,52 @@ public function storeInFileSystem()
     {
         $filename = "ssh_key@{$this->uuid}";
         $disk = Storage::disk('ssh-keys');
+        $keyLocation = $this->getKeyLocation();
+        $lockFile = $keyLocation.'.lock';
 
         // Ensure the storage directory exists and is writable
         $this->ensureStorageDirectoryExists();
 
-        // Attempt to store the private key
-        $success = $disk->put($filename, $this->private_key);
-
-        if (! $success) {
-            throw new \Exception("Failed to write SSH key to filesystem. Check disk space and permissions for: {$this->getKeyLocation()}");
+        // Use file locking to prevent concurrent writes from corrupting the key
+        $lockHandle = fopen($lockFile, 'c');
+        if ($lockHandle === false) {
+            throw new \Exception("Failed to open lock file for SSH key: {$lockFile}");
         }
 
-        // Verify the file was actually created and has content
-        if (! $disk->exists($filename)) {
-            throw new \Exception("SSH key file was not created: {$this->getKeyLocation()}");
-        }
+        try {
+            if (! flock($lockHandle, LOCK_EX)) {
+                throw new \Exception("Failed to acquire lock for SSH key: {$keyLocation}");
+            }
 
-        $storedContent = $disk->get($filename);
-        if (empty($storedContent) || $storedContent !== $this->private_key) {
-            $disk->delete($filename); // Clean up the bad file
-            throw new \Exception("SSH key file content verification failed: {$this->getKeyLocation()}");
-        }
+            // Attempt to store the private key
+            $success = $disk->put($filename, $this->private_key);
 
-        return $this->getKeyLocation();
+            if (! $success) {
+                throw new \Exception("Failed to write SSH key to filesystem. Check disk space and permissions for: {$keyLocation}");
+            }
+
+            // Verify the file was actually created and has content
+            if (! $disk->exists($filename)) {
+                throw new \Exception("SSH key file was not created: {$keyLocation}");
+            }
+
+            $storedContent = $disk->get($filename);
+            if (empty($storedContent) || $storedContent !== $this->private_key) {
+                $disk->delete($filename); // Clean up the bad file
+                throw new \Exception("SSH key file content verification failed: {$keyLocation}");
+            }
+
+            // Ensure correct permissions for SSH (0600 required)
+            if (file_exists($keyLocation)) {
+                chmod($keyLocation, 0600);
+            }
+
+            return $keyLocation;
+        } finally {
+            flock($lockHandle, LOCK_UN);
+            fclose($lockHandle);
+            @unlink($lockFile);
+        }
     }
 
     public static function deleteFromStorage(self $privateKey)

app/Models/Server.php

@@ -135,7 +135,7 @@ protected static function booted()
             $server->forceFill($payload);
         });
         static::saved(function ($server) {
-            if ($server->privateKey?->isDirty()) {
+            if ($server->wasChanged('private_key_id') || $server->privateKey?->isDirty()) {
                 refresh_server_connection($server->privateKey);
             }
         });

tests/Unit/SshKeyValidationTest.php

@@ -0,0 +1,133 @@
+<?php
+
+namespace Tests\Unit;
+
+use App\Helpers\SshMultiplexingHelper;
+use App\Models\PrivateKey;
+use Illuminate\Support\Facades\Storage;
+use Tests\TestCase;
+
+/**
+ * Tests for SSH key validation to prevent sporadic "Permission denied" errors.
+ *
+ * The root cause: validateSshKey() only checked file existence, not content.
+ * When a key was rotated in the DB but the old file persisted on disk,
+ * SSH would use the stale key and fail with "Permission denied (publickey)".
+ *
+ * @see https://github.com/coollabsio/coolify/issues/7724
+ */
+class SshKeyValidationTest extends TestCase
+{
+    public function test_validate_ssh_key_method_exists()
+    {
+        $reflection = new \ReflectionMethod(SshMultiplexingHelper::class, 'validateSshKey');
+        $this->assertTrue($reflection->isStatic(), 'validateSshKey should be a static method');
+    }
+
+    public function test_validate_ssh_key_accepts_private_key_parameter()
+    {
+        $reflection = new \ReflectionMethod(SshMultiplexingHelper::class, 'validateSshKey');
+        $parameters = $reflection->getParameters();
+
+        $this->assertCount(1, $parameters);
+        $this->assertEquals('privateKey', $parameters[0]->getName());
+
+        $type = $parameters[0]->getType();
+        $this->assertNotNull($type);
+        $this->assertEquals(PrivateKey::class, $type->getName());
+    }
+
+    public function test_store_in_file_system_sets_correct_permissions()
+    {
+        // Verify that storeInFileSystem enforces chmod 0600 via code inspection
+        $reflection = new \ReflectionMethod(PrivateKey::class, 'storeInFileSystem');
+        $this->assertTrue(
+            $reflection->isPublic(),
+            'storeInFileSystem should be public'
+        );
+
+        // Verify the method source contains chmod for 0600
+        $filename = $reflection->getFileName();
+        $startLine = $reflection->getStartLine();
+        $endLine = $reflection->getEndLine();
+        $source = implode('', array_slice(file($filename), $startLine - 1, $endLine - $startLine + 1));
+
+        $this->assertStringContainsString('chmod', $source, 'storeInFileSystem should set file permissions');
+        $this->assertStringContainsString('0600', $source, 'storeInFileSystem should enforce 0600 permissions');
+    }
+
+    public function test_store_in_file_system_uses_file_locking()
+    {
+        // Verify the method uses flock to prevent race conditions
+        $reflection = new \ReflectionMethod(PrivateKey::class, 'storeInFileSystem');
+        $filename = $reflection->getFileName();
+        $startLine = $reflection->getStartLine();
+        $endLine = $reflection->getEndLine();
+        $source = implode('', array_slice(file($filename), $startLine - 1, $endLine - $startLine + 1));
+
+        $this->assertStringContainsString('flock', $source, 'storeInFileSystem should use file locking');
+        $this->assertStringContainsString('LOCK_EX', $source, 'storeInFileSystem should use exclusive locks');
+    }
+
+    public function test_validate_ssh_key_checks_content_not_just_existence()
+    {
+        // Verify validateSshKey compares file content with DB value
+        $reflection = new \ReflectionMethod(SshMultiplexingHelper::class, 'validateSshKey');
+        $filename = $reflection->getFileName();
+        $startLine = $reflection->getStartLine();
+        $endLine = $reflection->getEndLine();
+        $source = implode('', array_slice(file($filename), $startLine - 1, $endLine - $startLine + 1));
+
+        // Should read file content and compare, not just check existence with `ls`
+        $this->assertStringNotContainsString('ls $keyLocation', $source, 'Should not use ls to check key existence');
+        $this->assertStringContainsString('private_key', $source, 'Should compare against DB key content');
+        $this->assertStringContainsString('refresh', $source, 'Should refresh key from database');
+    }
+
+    public function test_server_model_detects_private_key_id_changes()
+    {
+        // Verify the Server model's saved event checks for private_key_id changes
+        $reflection = new \ReflectionMethod(\App\Models\Server::class, 'booted');
+        $filename = $reflection->getFileName();
+        $startLine = $reflection->getStartLine();
+        $endLine = $reflection->getEndLine();
+        $source = implode('', array_slice(file($filename), $startLine - 1, $endLine - $startLine + 1));
+
+        $this->assertStringContainsString(
+            'wasChanged',
+            $source,
+            'Server saved event should detect private_key_id changes via wasChanged()'
+        );
+        $this->assertStringContainsString(
+            'private_key_id',
+            $source,
+            'Server saved event should specifically check private_key_id'
+        );
+    }
+
+    public function test_private_key_saved_event_resyncs_on_key_change()
+    {
+        // Verify PrivateKey model resyncs file and mux on key content change
+        $reflection = new \ReflectionMethod(PrivateKey::class, 'booted');
+        $filename = $reflection->getFileName();
+        $startLine = $reflection->getStartLine();
+        $endLine = $reflection->getEndLine();
+        $source = implode('', array_slice(file($filename), $startLine - 1, $endLine - $startLine + 1));
+
+        $this->assertStringContainsString(
+            "wasChanged('private_key')",
+            $source,
+            'PrivateKey saved event should detect key content changes'
+        );
+        $this->assertStringContainsString(
+            'refresh_server_connection',
+            $source,
+            'PrivateKey saved event should invalidate mux connections'
+        );
+        $this->assertStringContainsString(
+            'storeInFileSystem',
+            $source,
+            'PrivateKey saved event should resync key file'
+        );
+    }
+}