From 4bf94fac2d2c006ed2d6c2760d6001543cda06de Mon Sep 17 00:00:00 2001 From: pannous Date: Sun, 15 Mar 2026 03:06:21 +0100 Subject: [PATCH 1/3] 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 | 33 ++++++- app/Models/PrivateKey.php | 67 ++++++++++--- app/Models/Server.php | 2 +- tests/Unit/SshKeyValidationTest.php | 133 ++++++++++++++++++++++++++ 4 files changed, 215 insertions(+), 20 deletions(-) create mode 100644 tests/Unit/SshKeyValidationTest.php diff --git a/app/Helpers/SshMultiplexingHelper.php b/app/Helpers/SshMultiplexingHelper.php index 54d5714a6..b9a3e98f3 100644 --- a/app/Helpers/SshMultiplexingHelper.php +++ b/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 diff --git a/app/Models/PrivateKey.php b/app/Models/PrivateKey.php index 7163ae7b5..b453e999d 100644 --- a/app/Models/PrivateKey.php +++ b/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) diff --git a/app/Models/Server.php b/app/Models/Server.php index 508b9833b..527c744a5 100644 --- a/app/Models/Server.php +++ b/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); } }); diff --git a/tests/Unit/SshKeyValidationTest.php b/tests/Unit/SshKeyValidationTest.php new file mode 100644 index 000000000..9a6c6c04b --- /dev/null +++ b/tests/Unit/SshKeyValidationTest.php @@ -0,0 +1,133 @@ +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' + ); + } +} From 2f96a759dfc92e02eb7a17c89929e33f13e14de7 Mon Sep 17 00:00:00 2001 From: pannous Date: Mon, 16 Mar 2026 10:40:22 +0100 Subject: [PATCH 2/3] test: add behavioral ssh key stale-file regression --- tests/Unit/SshKeyValidationTest.php | 56 +++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/tests/Unit/SshKeyValidationTest.php b/tests/Unit/SshKeyValidationTest.php index 9a6c6c04b..fbcf7725d 100644 --- a/tests/Unit/SshKeyValidationTest.php +++ b/tests/Unit/SshKeyValidationTest.php @@ -4,7 +4,9 @@ use App\Helpers\SshMultiplexingHelper; use App\Models\PrivateKey; +use Illuminate\Support\Facades\File; use Illuminate\Support\Facades\Storage; +use Illuminate\Support\Str; use Tests\TestCase; /** @@ -130,4 +132,58 @@ public function test_private_key_saved_event_resyncs_on_key_change() 'PrivateKey saved event should resync key file' ); } + + public function test_validate_ssh_key_rewrites_stale_file_and_fixes_permissions() + { + $diskRoot = sys_get_temp_dir().'/coolify-ssh-test-'.Str::uuid(); + File::ensureDirectoryExists($diskRoot); + config(['filesystems.disks.ssh-keys.root' => $diskRoot]); + app('filesystem')->forgetDisk('ssh-keys'); + + $privateKey = new class extends PrivateKey + { + public int $storeCallCount = 0; + + public function refresh() + { + return $this; + } + + public function getKeyLocation() + { + return Storage::disk('ssh-keys')->path("ssh_key@{$this->uuid}"); + } + + public function storeInFileSystem() + { + $this->storeCallCount++; + $filename = "ssh_key@{$this->uuid}"; + $disk = Storage::disk('ssh-keys'); + $disk->put($filename, $this->private_key); + $keyLocation = $disk->path($filename); + chmod($keyLocation, 0600); + + return $keyLocation; + } + }; + + $privateKey->uuid = (string) Str::uuid(); + $privateKey->private_key = 'NEW_PRIVATE_KEY_CONTENT'; + + $filename = "ssh_key@{$privateKey->uuid}"; + $disk = Storage::disk('ssh-keys'); + $disk->put($filename, 'OLD_PRIVATE_KEY_CONTENT'); + $staleKeyPath = $disk->path($filename); + chmod($staleKeyPath, 0644); + + $reflection = new \ReflectionMethod(SshMultiplexingHelper::class, 'validateSshKey'); + $reflection->setAccessible(true); + $reflection->invoke(null, $privateKey); + + $this->assertSame('NEW_PRIVATE_KEY_CONTENT', $disk->get($filename)); + $this->assertSame(1, $privateKey->storeCallCount); + $this->assertSame(0600, fileperms($staleKeyPath) & 0777); + + File::deleteDirectory($diskRoot); + } } From 6325e41aec29e5e02e16f0b8df0dbc1ae5b38531 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Mon, 16 Mar 2026 21:27:10 +0100 Subject: [PATCH 3/3] fix(ssh): handle chmod failures gracefully and simplify key management - Log warnings instead of silently failing when chmod 0600 fails - Remove redundant refresh() call before SSH key validation - Remove storeInFileSystem() call from updatePrivateKey() transaction - Remove @unlink() of lock file after filesystem store - Refactor unit tests to use real temp disk and anonymous class stub instead of reflection-only checks --- app/Helpers/SshMultiplexingHelper.php | 9 +- app/Models/PrivateKey.php | 17 +- tests/Unit/SshKeyValidationTest.php | 267 ++++++++++++++------------ 3 files changed, 155 insertions(+), 138 deletions(-) diff --git a/app/Helpers/SshMultiplexingHelper.php b/app/Helpers/SshMultiplexingHelper.php index b9a3e98f3..aa9d06996 100644 --- a/app/Helpers/SshMultiplexingHelper.php +++ b/app/Helpers/SshMultiplexingHelper.php @@ -209,8 +209,6 @@ private static function isMultiplexingEnabled(): bool private static function validateSshKey(PrivateKey $privateKey): void { - $privateKey->refresh(); - $keyLocation = $privateKey->getKeyLocation(); $filename = "ssh_key@{$privateKey->uuid}"; $disk = Storage::disk('ssh-keys'); @@ -236,8 +234,11 @@ private static function validateSshKey(PrivateKey $privateKey): void // Ensure correct permissions (SSH requires 0600) if (file_exists($keyLocation)) { $currentPerms = fileperms($keyLocation) & 0777; - if ($currentPerms !== 0600) { - chmod($keyLocation, 0600); + if ($currentPerms !== 0600 && ! chmod($keyLocation, 0600)) { + Log::warning('Failed to set SSH key file permissions to 0600', [ + 'key_uuid' => $privateKey->uuid, + 'path' => $keyLocation, + ]); } } } diff --git a/app/Models/PrivateKey.php b/app/Models/PrivateKey.php index b453e999d..1521678f3 100644 --- a/app/Models/PrivateKey.php +++ b/app/Models/PrivateKey.php @@ -5,6 +5,7 @@ use App\Traits\HasSafeStringAttribute; use DanHarrin\LivewireRateLimiting\WithRateLimiting; use Illuminate\Support\Facades\DB; +use Illuminate\Support\Facades\Log; use Illuminate\Support\Facades\Storage; use Illuminate\Validation\ValidationException; use OpenApi\Attributes as OA; @@ -71,7 +72,7 @@ protected static function booted() $key->storeInFileSystem(); refresh_server_connection($key); } catch (\Exception $e) { - \Illuminate\Support\Facades\Log::error('Failed to resync SSH key after update', [ + Log::error('Failed to resync SSH key after update', [ 'key_uuid' => $key->uuid, 'error' => $e->getMessage(), ]); @@ -235,15 +236,17 @@ public function storeInFileSystem() } // Ensure correct permissions for SSH (0600 required) - if (file_exists($keyLocation)) { - chmod($keyLocation, 0600); + if (file_exists($keyLocation) && ! chmod($keyLocation, 0600)) { + Log::warning('Failed to set SSH key file permissions to 0600', [ + 'key_uuid' => $this->uuid, + 'path' => $keyLocation, + ]); } return $keyLocation; } finally { flock($lockHandle, LOCK_UN); fclose($lockHandle); - @unlink($lockFile); } } @@ -291,12 +294,6 @@ public function updatePrivateKey(array $data) return DB::transaction(function () use ($data) { $this->update($data); - try { - $this->storeInFileSystem(); - } catch (\Exception $e) { - throw new \Exception('Failed to update SSH key: '.$e->getMessage()); - } - return $this; }); } diff --git a/tests/Unit/SshKeyValidationTest.php b/tests/Unit/SshKeyValidationTest.php index fbcf7725d..adc6847d1 100644 --- a/tests/Unit/SshKeyValidationTest.php +++ b/tests/Unit/SshKeyValidationTest.php @@ -20,126 +20,26 @@ */ class SshKeyValidationTest extends TestCase { - public function test_validate_ssh_key_method_exists() + private string $diskRoot; + + protected function setUp(): void { - $reflection = new \ReflectionMethod(SshMultiplexingHelper::class, 'validateSshKey'); - $this->assertTrue($reflection->isStatic(), 'validateSshKey should be a static method'); - } + parent::setUp(); - 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' - ); - } - - public function test_validate_ssh_key_rewrites_stale_file_and_fixes_permissions() - { - $diskRoot = sys_get_temp_dir().'/coolify-ssh-test-'.Str::uuid(); - File::ensureDirectoryExists($diskRoot); - config(['filesystems.disks.ssh-keys.root' => $diskRoot]); + $this->diskRoot = sys_get_temp_dir().'/coolify-ssh-test-'.Str::uuid(); + File::ensureDirectoryExists($this->diskRoot); + config(['filesystems.disks.ssh-keys.root' => $this->diskRoot]); app('filesystem')->forgetDisk('ssh-keys'); + } + protected function tearDown(): void + { + File::deleteDirectory($this->diskRoot); + parent::tearDown(); + } + + private function makePrivateKey(string $keyContent = 'TEST_KEY_CONTENT'): PrivateKey + { $privateKey = new class extends PrivateKey { public int $storeCallCount = 0; @@ -168,22 +68,141 @@ public function storeInFileSystem() }; $privateKey->uuid = (string) Str::uuid(); - $privateKey->private_key = 'NEW_PRIVATE_KEY_CONTENT'; + $privateKey->private_key = $keyContent; + + return $privateKey; + } + + private function callValidateSshKey(PrivateKey $privateKey): void + { + $reflection = new \ReflectionMethod(SshMultiplexingHelper::class, 'validateSshKey'); + $reflection->setAccessible(true); + $reflection->invoke(null, $privateKey); + } + + public function test_validate_ssh_key_rewrites_stale_file_and_fixes_permissions() + { + $privateKey = $this->makePrivateKey('NEW_PRIVATE_KEY_CONTENT'); $filename = "ssh_key@{$privateKey->uuid}"; $disk = Storage::disk('ssh-keys'); $disk->put($filename, 'OLD_PRIVATE_KEY_CONTENT'); - $staleKeyPath = $disk->path($filename); - chmod($staleKeyPath, 0644); + $keyPath = $disk->path($filename); + chmod($keyPath, 0644); - $reflection = new \ReflectionMethod(SshMultiplexingHelper::class, 'validateSshKey'); - $reflection->setAccessible(true); - $reflection->invoke(null, $privateKey); + $this->callValidateSshKey($privateKey); $this->assertSame('NEW_PRIVATE_KEY_CONTENT', $disk->get($filename)); $this->assertSame(1, $privateKey->storeCallCount); - $this->assertSame(0600, fileperms($staleKeyPath) & 0777); + $this->assertSame(0600, fileperms($keyPath) & 0777); + } - File::deleteDirectory($diskRoot); + public function test_validate_ssh_key_creates_missing_file() + { + $privateKey = $this->makePrivateKey('MY_KEY_CONTENT'); + + $filename = "ssh_key@{$privateKey->uuid}"; + $disk = Storage::disk('ssh-keys'); + $this->assertFalse($disk->exists($filename)); + + $this->callValidateSshKey($privateKey); + + $this->assertTrue($disk->exists($filename)); + $this->assertSame('MY_KEY_CONTENT', $disk->get($filename)); + $this->assertSame(1, $privateKey->storeCallCount); + } + + public function test_validate_ssh_key_skips_rewrite_when_content_matches() + { + $privateKey = $this->makePrivateKey('SAME_KEY_CONTENT'); + + $filename = "ssh_key@{$privateKey->uuid}"; + $disk = Storage::disk('ssh-keys'); + $disk->put($filename, 'SAME_KEY_CONTENT'); + $keyPath = $disk->path($filename); + chmod($keyPath, 0600); + + $this->callValidateSshKey($privateKey); + + $this->assertSame(0, $privateKey->storeCallCount, 'Should not rewrite when content matches'); + $this->assertSame('SAME_KEY_CONTENT', $disk->get($filename)); + } + + public function test_validate_ssh_key_fixes_permissions_without_rewrite() + { + $privateKey = $this->makePrivateKey('KEY_CONTENT'); + + $filename = "ssh_key@{$privateKey->uuid}"; + $disk = Storage::disk('ssh-keys'); + $disk->put($filename, 'KEY_CONTENT'); + $keyPath = $disk->path($filename); + chmod($keyPath, 0644); + + $this->callValidateSshKey($privateKey); + + $this->assertSame(0, $privateKey->storeCallCount, 'Should not rewrite when content matches'); + $this->assertSame(0600, fileperms($keyPath) & 0777, 'Should fix permissions even without rewrite'); + } + + public function test_store_in_file_system_enforces_correct_permissions() + { + $privateKey = $this->makePrivateKey('KEY_FOR_PERM_TEST'); + $privateKey->storeInFileSystem(); + + $filename = "ssh_key@{$privateKey->uuid}"; + $keyPath = Storage::disk('ssh-keys')->path($filename); + + $this->assertSame(0600, fileperms($keyPath) & 0777); + } + + public function test_store_in_file_system_lock_file_persists() + { + // Use the real storeInFileSystem to verify lock file behavior + $disk = Storage::disk('ssh-keys'); + $uuid = (string) Str::uuid(); + $filename = "ssh_key@{$uuid}"; + $keyLocation = $disk->path($filename); + $lockFile = $keyLocation.'.lock'; + + $privateKey = new class extends PrivateKey + { + public function refresh() + { + return $this; + } + + public function getKeyLocation() + { + return Storage::disk('ssh-keys')->path("ssh_key@{$this->uuid}"); + } + + protected function ensureStorageDirectoryExists() + { + // No-op in test — directory already exists + } + }; + + $privateKey->uuid = $uuid; + $privateKey->private_key = 'LOCK_TEST_KEY'; + + $privateKey->storeInFileSystem(); + + // Lock file should persist (not be deleted) to prevent flock race conditions + $this->assertFileExists($lockFile, 'Lock file should persist after storeInFileSystem'); + } + + public function test_server_model_detects_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('private_key_id')", + $source, + 'Server saved event should detect private_key_id changes' + ); } }