Merge branch 'v4.x' into feature/watch-paths-for-services

tests/Feature/IpAllowlistTest.php

@@ -8,7 +8,7 @@ test('IP allowlist with single IPs', function () {
     ];
 
     foreach ($testCases as $case) {
-        $result = check_ip_against_allowlist($case['ip'], $case['allowlist']);
+        $result = checkIPAgainstAllowlist($case['ip'], $case['allowlist']);
         expect($result)->toBe($case['expected']);
     }
 });
@@ -24,7 +24,7 @@ test('IP allowlist with CIDR notation', function () {
     ];
 
     foreach ($testCases as $case) {
-        $result = check_ip_against_allowlist($case['ip'], $case['allowlist']);
+        $result = checkIPAgainstAllowlist($case['ip'], $case['allowlist']);
         expect($result)->toBe($case['expected']);
     }
 });
@@ -40,16 +40,16 @@ test('IP allowlist with 0.0.0.0 allows all', function () {
 
     // Test 0.0.0.0 without subnet
     foreach ($testIps as $ip) {
-        $result = check_ip_against_allowlist($ip, ['0.0.0.0']);
+        $result = checkIPAgainstAllowlist($ip, ['0.0.0.0']);
         expect($result)->toBeTrue();
     }
 
     // Test 0.0.0.0 with any subnet notation - should still allow all
     foreach ($testIps as $ip) {
-        expect(check_ip_against_allowlist($ip, ['0.0.0.0/0']))->toBeTrue();
-        expect(check_ip_against_allowlist($ip, ['0.0.0.0/8']))->toBeTrue();
-        expect(check_ip_against_allowlist($ip, ['0.0.0.0/24']))->toBeTrue();
-        expect(check_ip_against_allowlist($ip, ['0.0.0.0/32']))->toBeTrue();
+        expect(checkIPAgainstAllowlist($ip, ['0.0.0.0/0']))->toBeTrue();
+        expect(checkIPAgainstAllowlist($ip, ['0.0.0.0/8']))->toBeTrue();
+        expect(checkIPAgainstAllowlist($ip, ['0.0.0.0/24']))->toBeTrue();
+        expect(checkIPAgainstAllowlist($ip, ['0.0.0.0/32']))->toBeTrue();
     }
 });
 
@@ -66,44 +66,44 @@ test('IP allowlist with mixed entries', function () {
     ];
 
     foreach ($testCases as $case) {
-        $result = check_ip_against_allowlist($case['ip'], $allowlist);
+        $result = checkIPAgainstAllowlist($case['ip'], $allowlist);
         expect($result)->toBe($case['expected']);
     }
 });
 
 test('IP allowlist handles empty and invalid entries', function () {
     // Empty allowlist blocks all
-    expect(check_ip_against_allowlist('192.168.1.1', []))->toBeFalse();
-    expect(check_ip_against_allowlist('192.168.1.1', ['']))->toBeFalse();
+    expect(checkIPAgainstAllowlist('192.168.1.1', []))->toBeFalse();
+    expect(checkIPAgainstAllowlist('192.168.1.1', ['']))->toBeFalse();
 
     // Handles spaces
-    expect(check_ip_against_allowlist('192.168.1.100', [' 192.168.1.100 ']))->toBeTrue();
-    expect(check_ip_against_allowlist('10.0.0.5', [' 10.0.0.0/8 ']))->toBeTrue();
+    expect(checkIPAgainstAllowlist('192.168.1.100', [' 192.168.1.100 ']))->toBeTrue();
+    expect(checkIPAgainstAllowlist('10.0.0.5', [' 10.0.0.0/8 ']))->toBeTrue();
 
     // Invalid entries are skipped
-    expect(check_ip_against_allowlist('192.168.1.1', ['invalid.ip']))->toBeFalse();
-    expect(check_ip_against_allowlist('192.168.1.1', ['192.168.1.0/33']))->toBeFalse(); // Invalid mask
-    expect(check_ip_against_allowlist('192.168.1.1', ['192.168.1.0/-1']))->toBeFalse(); // Invalid mask
+    expect(checkIPAgainstAllowlist('192.168.1.1', ['invalid.ip']))->toBeFalse();
+    expect(checkIPAgainstAllowlist('192.168.1.1', ['192.168.1.0/33']))->toBeFalse(); // Invalid mask
+    expect(checkIPAgainstAllowlist('192.168.1.1', ['192.168.1.0/-1']))->toBeFalse(); // Invalid mask
 });
 
 test('IP allowlist with various subnet sizes', function () {
     // /32 - single host
-    expect(check_ip_against_allowlist('192.168.1.1', ['192.168.1.1/32']))->toBeTrue();
-    expect(check_ip_against_allowlist('192.168.1.2', ['192.168.1.1/32']))->toBeFalse();
+    expect(checkIPAgainstAllowlist('192.168.1.1', ['192.168.1.1/32']))->toBeTrue();
+    expect(checkIPAgainstAllowlist('192.168.1.2', ['192.168.1.1/32']))->toBeFalse();
 
     // /31 - point-to-point link
-    expect(check_ip_against_allowlist('192.168.1.0', ['192.168.1.0/31']))->toBeTrue();
-    expect(check_ip_against_allowlist('192.168.1.1', ['192.168.1.0/31']))->toBeTrue();
-    expect(check_ip_against_allowlist('192.168.1.2', ['192.168.1.0/31']))->toBeFalse();
+    expect(checkIPAgainstAllowlist('192.168.1.0', ['192.168.1.0/31']))->toBeTrue();
+    expect(checkIPAgainstAllowlist('192.168.1.1', ['192.168.1.0/31']))->toBeTrue();
+    expect(checkIPAgainstAllowlist('192.168.1.2', ['192.168.1.0/31']))->toBeFalse();
 
     // /16 - class B
-    expect(check_ip_against_allowlist('172.16.0.1', ['172.16.0.0/16']))->toBeTrue();
-    expect(check_ip_against_allowlist('172.16.255.255', ['172.16.0.0/16']))->toBeTrue();
-    expect(check_ip_against_allowlist('172.17.0.1', ['172.16.0.0/16']))->toBeFalse();
+    expect(checkIPAgainstAllowlist('172.16.0.1', ['172.16.0.0/16']))->toBeTrue();
+    expect(checkIPAgainstAllowlist('172.16.255.255', ['172.16.0.0/16']))->toBeTrue();
+    expect(checkIPAgainstAllowlist('172.17.0.1', ['172.16.0.0/16']))->toBeFalse();
 
     // /0 - all addresses
-    expect(check_ip_against_allowlist('1.1.1.1', ['0.0.0.0/0']))->toBeTrue();
-    expect(check_ip_against_allowlist('255.255.255.255', ['0.0.0.0/0']))->toBeTrue();
+    expect(checkIPAgainstAllowlist('1.1.1.1', ['0.0.0.0/0']))->toBeTrue();
+    expect(checkIPAgainstAllowlist('255.255.255.255', ['0.0.0.0/0']))->toBeTrue();
 });
 
 test('IP allowlist comma-separated string input', function () {
@@ -111,10 +111,10 @@ test('IP allowlist comma-separated string input', function () {
     $allowlistString = '192.168.1.100,10.0.0.0/8,172.16.0.0/16';
     $allowlist = explode(',', $allowlistString);
 
-    expect(check_ip_against_allowlist('192.168.1.100', $allowlist))->toBeTrue();
-    expect(check_ip_against_allowlist('10.5.5.5', $allowlist))->toBeTrue();
-    expect(check_ip_against_allowlist('172.16.10.10', $allowlist))->toBeTrue();
-    expect(check_ip_against_allowlist('8.8.8.8', $allowlist))->toBeFalse();
+    expect(checkIPAgainstAllowlist('192.168.1.100', $allowlist))->toBeTrue();
+    expect(checkIPAgainstAllowlist('10.5.5.5', $allowlist))->toBeTrue();
+    expect(checkIPAgainstAllowlist('172.16.10.10', $allowlist))->toBeTrue();
+    expect(checkIPAgainstAllowlist('8.8.8.8', $allowlist))->toBeFalse();
 });
 
 test('ValidIpOrCidr validation rule', function () {

tests/Unit/PrivateKeyStorageTest.php

@@ -0,0 +1,316 @@
+<?php
+
+use App\Models\PrivateKey;
+use Illuminate\Foundation\Testing\RefreshDatabase;
+use Illuminate\Support\Facades\Storage;
+use Tests\TestCase;
+
+class PrivateKeyStorageTest extends TestCase
+{
+    use RefreshDatabase;
+
+    protected function setUp(): void
+    {
+        parent::setUp();
+
+        // Set up a test team for the tests
+        $this->actingAs(\App\Models\User::factory()->create());
+    }
+
+    protected function getValidPrivateKey(): string
+    {
+        return '-----BEGIN OPENSSH PRIVATE KEY-----
+b3BlbnNzaC1rZXktdjEAAAAABG5vbmUAAAAEbm9uZQAAAAAAAAABAAAAMwAAAAtzc2gtZW
+QyNTUxOQAAACBbhpqHhqv6aI67Mj9abM3DVbmcfYhZAhC7ca4d9UCevAAAAJi/QySHv0Mk
+hwAAAAtzc2gtZWQyNTUxOQAAACBbhpqHhqv6aI67Mj9abM3DVbmcfYhZAhC7ca4d9UCevA
+AAAECBQw4jg1WRT2IGHMncCiZhURCts2s24HoDS0thHnnRKVuGmoeGq/pojrsyP1pszcNV
+uZx9iFkCELtxrh31QJ68AAAAEXNhaWxANzZmZjY2ZDJlMmRkAQIDBA==
+-----END OPENSSH PRIVATE KEY-----';
+    }
+
+    /** @test */
+    public function it_successfully_stores_private_key_in_filesystem()
+    {
+        Storage::fake('ssh-keys');
+
+        $privateKey = PrivateKey::createAndStore([
+            'name' => 'Test Key',
+            'description' => 'Test Description',
+            'private_key' => $this->getValidPrivateKey(),
+            'team_id' => currentTeam()->id,
+        ]);
+
+        $this->assertDatabaseHas('private_keys', [
+            'id' => $privateKey->id,
+            'name' => 'Test Key',
+        ]);
+
+        $filename = "ssh_key@{$privateKey->uuid}";
+        Storage::disk('ssh-keys')->assertExists($filename);
+
+        $storedContent = Storage::disk('ssh-keys')->get($filename);
+        $this->assertEquals($privateKey->private_key, $storedContent);
+    }
+
+    /** @test */
+    public function it_throws_exception_when_storage_fails()
+    {
+        Storage::fake('ssh-keys');
+
+        // Mock Storage::put to return false (simulating storage failure)
+        Storage::shouldReceive('disk')
+            ->with('ssh-keys')
+            ->andReturn(
+                \Mockery::mock()
+                    ->shouldReceive('exists')
+                    ->andReturn(true)
+                    ->shouldReceive('put')
+                    ->with(\Mockery::any(), 'test')
+                    ->andReturn(true)
+                    ->shouldReceive('delete')
+                    ->with(\Mockery::any())
+                    ->andReturn(true)
+                    ->shouldReceive('put')
+                    ->with(\Mockery::pattern('/ssh_key@/'), \Mockery::any())
+                    ->andReturn(false) // Simulate storage failure
+                    ->getMock()
+            );
+
+        $this->expectException(\Exception::class);
+        $this->expectExceptionMessage('Failed to write SSH key to filesystem');
+
+        PrivateKey::createAndStore([
+            'name' => 'Test Key',
+            'description' => 'Test Description',
+            'private_key' => $this->getValidPrivateKey(),
+            'team_id' => currentTeam()->id,
+        ]);
+
+        // Assert that no database record was created due to transaction rollback
+        $this->assertDatabaseMissing('private_keys', [
+            'name' => 'Test Key',
+        ]);
+    }
+
+    /** @test */
+    public function it_throws_exception_when_storage_directory_is_not_writable()
+    {
+        Storage::fake('ssh-keys');
+
+        // Mock Storage disk to simulate directory not writable
+        Storage::shouldReceive('disk')
+            ->with('ssh-keys')
+            ->andReturn(
+                \Mockery::mock()
+                    ->shouldReceive('exists')
+                    ->with('')
+                    ->andReturn(true)
+                    ->shouldReceive('put')
+                    ->with(\Mockery::pattern('/\.test_write_/'), 'test')
+                    ->andReturn(false) // Simulate directory not writable
+                    ->getMock()
+            );
+
+        $this->expectException(\Exception::class);
+        $this->expectExceptionMessage('SSH keys storage directory is not writable');
+
+        PrivateKey::createAndStore([
+            'name' => 'Test Key',
+            'description' => 'Test Description',
+            'private_key' => $this->getValidPrivateKey(),
+            'team_id' => currentTeam()->id,
+        ]);
+    }
+
+    /** @test */
+    public function it_creates_storage_directory_if_not_exists()
+    {
+        Storage::fake('ssh-keys');
+
+        // Mock Storage disk to simulate directory not existing, then being created
+        Storage::shouldReceive('disk')
+            ->with('ssh-keys')
+            ->andReturn(
+                \Mockery::mock()
+                    ->shouldReceive('exists')
+                    ->with('')
+                    ->andReturn(false) // Directory doesn't exist
+                    ->shouldReceive('makeDirectory')
+                    ->with('')
+                    ->andReturn(true) // Successfully create directory
+                    ->shouldReceive('put')
+                    ->with(\Mockery::pattern('/\.test_write_/'), 'test')
+                    ->andReturn(true) // Directory is writable after creation
+                    ->shouldReceive('delete')
+                    ->with(\Mockery::pattern('/\.test_write_/'))
+                    ->andReturn(true)
+                    ->shouldReceive('put')
+                    ->with(\Mockery::pattern('/ssh_key@/'), \Mockery::any())
+                    ->andReturn(true)
+                    ->shouldReceive('exists')
+                    ->with(\Mockery::pattern('/ssh_key@/'))
+                    ->andReturn(true)
+                    ->shouldReceive('get')
+                    ->with(\Mockery::pattern('/ssh_key@/'))
+                    ->andReturn($this->getValidPrivateKey())
+                    ->getMock()
+            );
+
+        $privateKey = PrivateKey::createAndStore([
+            'name' => 'Test Key',
+            'description' => 'Test Description',
+            'private_key' => $this->getValidPrivateKey(),
+            'team_id' => currentTeam()->id,
+        ]);
+
+        $this->assertDatabaseHas('private_keys', [
+            'id' => $privateKey->id,
+            'name' => 'Test Key',
+        ]);
+    }
+
+    /** @test */
+    public function it_throws_exception_when_file_content_verification_fails()
+    {
+        Storage::fake('ssh-keys');
+
+        // Mock Storage disk to simulate file being created but with wrong content
+        Storage::shouldReceive('disk')
+            ->with('ssh-keys')
+            ->andReturn(
+                \Mockery::mock()
+                    ->shouldReceive('exists')
+                    ->with('')
+                    ->andReturn(true)
+                    ->shouldReceive('put')
+                    ->with(\Mockery::pattern('/\.test_write_/'), 'test')
+                    ->andReturn(true)
+                    ->shouldReceive('delete')
+                    ->with(\Mockery::pattern('/\.test_write_/'))
+                    ->andReturn(true)
+                    ->shouldReceive('put')
+                    ->with(\Mockery::pattern('/ssh_key@/'), \Mockery::any())
+                    ->andReturn(true) // File created successfully
+                    ->shouldReceive('exists')
+                    ->with(\Mockery::pattern('/ssh_key@/'))
+                    ->andReturn(true) // File exists
+                    ->shouldReceive('get')
+                    ->with(\Mockery::pattern('/ssh_key@/'))
+                    ->andReturn('corrupted content') // But content is wrong
+                    ->shouldReceive('delete')
+                    ->with(\Mockery::pattern('/ssh_key@/'))
+                    ->andReturn(true) // Clean up bad file
+                    ->getMock()
+            );
+
+        $this->expectException(\Exception::class);
+        $this->expectExceptionMessage('SSH key file content verification failed');
+
+        PrivateKey::createAndStore([
+            'name' => 'Test Key',
+            'description' => 'Test Description',
+            'private_key' => $this->getValidPrivateKey(),
+            'team_id' => currentTeam()->id,
+        ]);
+
+        // Assert that no database record was created due to transaction rollback
+        $this->assertDatabaseMissing('private_keys', [
+            'name' => 'Test Key',
+        ]);
+    }
+
+    /** @test */
+    public function it_successfully_deletes_private_key_from_filesystem()
+    {
+        Storage::fake('ssh-keys');
+
+        $privateKey = PrivateKey::createAndStore([
+            'name' => 'Test Key',
+            'description' => 'Test Description',
+            'private_key' => $this->getValidPrivateKey(),
+            'team_id' => currentTeam()->id,
+        ]);
+
+        $filename = "ssh_key@{$privateKey->uuid}";
+        Storage::disk('ssh-keys')->assertExists($filename);
+
+        $privateKey->delete();
+
+        Storage::disk('ssh-keys')->assertMissing($filename);
+    }
+
+    /** @test */
+    public function it_handles_database_transaction_rollback_on_storage_failure()
+    {
+        Storage::fake('ssh-keys');
+
+        // Count initial private keys
+        $initialCount = PrivateKey::count();
+
+        // Mock storage failure after database save
+        Storage::shouldReceive('disk')
+            ->with('ssh-keys')
+            ->andReturn(
+                \Mockery::mock()
+                    ->shouldReceive('exists')
+                    ->with('')
+                    ->andReturn(true)
+                    ->shouldReceive('put')
+                    ->with(\Mockery::pattern('/\.test_write_/'), 'test')
+                    ->andReturn(true)
+                    ->shouldReceive('delete')
+                    ->with(\Mockery::pattern('/\.test_write_/'))
+                    ->andReturn(true)
+                    ->shouldReceive('put')
+                    ->with(\Mockery::pattern('/ssh_key@/'), \Mockery::any())
+                    ->andReturn(false) // Storage fails
+                    ->getMock()
+            );
+
+        try {
+            PrivateKey::createAndStore([
+                'name' => 'Test Key',
+                'description' => 'Test Description',
+                'private_key' => $this->getValidPrivateKey(),
+                'team_id' => currentTeam()->id,
+            ]);
+        } catch (\Exception $e) {
+            // Expected exception
+        }
+
+        // Assert that database was rolled back
+        $this->assertEquals($initialCount, PrivateKey::count());
+        $this->assertDatabaseMissing('private_keys', [
+            'name' => 'Test Key',
+        ]);
+    }
+
+    /** @test */
+    public function it_successfully_updates_private_key_with_transaction()
+    {
+        Storage::fake('ssh-keys');
+
+        $privateKey = PrivateKey::createAndStore([
+            'name' => 'Test Key',
+            'description' => 'Test Description',
+            'private_key' => $this->getValidPrivateKey(),
+            'team_id' => currentTeam()->id,
+        ]);
+
+        $newPrivateKey = str_replace('Test', 'Updated', $this->getValidPrivateKey());
+
+        $privateKey->updatePrivateKey([
+            'name' => 'Updated Key',
+            'private_key' => $newPrivateKey,
+        ]);
+
+        $this->assertDatabaseHas('private_keys', [
+            'id' => $privateKey->id,
+            'name' => 'Updated Key',
+        ]);
+
+        $filename = "ssh_key@{$privateKey->uuid}";
+        $storedContent = Storage::disk('ssh-keys')->get($filename);
+        $this->assertEquals($newPrivateKey, $storedContent);
+    }
+}

tests/Unit/SshRetryMechanismTest.php

@@ -0,0 +1,189 @@
+<?php
+
+namespace Tests\Unit;
+
+use App\Helpers\SshRetryHandler;
+use App\Traits\SshRetryable;
+use Tests\TestCase;
+
+class SshRetryMechanismTest extends TestCase
+{
+    public function test_ssh_retry_handler_exists()
+    {
+        $this->assertTrue(class_exists(\App\Helpers\SshRetryHandler::class));
+    }
+
+    public function test_ssh_retryable_trait_exists()
+    {
+        $this->assertTrue(trait_exists(\App\Traits\SshRetryable::class));
+    }
+
+    public function test_retry_on_ssh_connection_errors()
+    {
+        $handler = new class
+        {
+            use SshRetryable;
+
+            // Make methods public for testing
+            public function test_is_retryable_ssh_error($error)
+            {
+                return $this->isRetryableSshError($error);
+            }
+        };
+
+        // Test various SSH error patterns
+        $sshErrors = [
+            'kex_exchange_identification: read: Connection reset by peer',
+            'Connection refused',
+            'Connection timed out',
+            'ssh_exchange_identification: Connection closed by remote host',
+            'Broken pipe',
+            'No route to host',
+            'Network is unreachable',
+        ];
+
+        foreach ($sshErrors as $error) {
+            $this->assertTrue(
+                $handler->test_is_retryable_ssh_error($error),
+                "Failed to identify as retryable: $error"
+            );
+        }
+    }
+
+    public function test_non_ssh_errors_are_not_retryable()
+    {
+        $handler = new class
+        {
+            use SshRetryable;
+
+            // Make methods public for testing
+            public function test_is_retryable_ssh_error($error)
+            {
+                return $this->isRetryableSshError($error);
+            }
+        };
+
+        // Test non-SSH errors
+        $nonSshErrors = [
+            'Command not found',
+            'Permission denied',
+            'File not found',
+            'Syntax error',
+            'Invalid argument',
+        ];
+
+        foreach ($nonSshErrors as $error) {
+            $this->assertFalse(
+                $handler->test_is_retryable_ssh_error($error),
+                "Incorrectly identified as retryable: $error"
+            );
+        }
+    }
+
+    public function test_exponential_backoff_calculation()
+    {
+        $handler = new class
+        {
+            use SshRetryable;
+
+            // Make method public for testing
+            public function test_calculate_retry_delay($attempt)
+            {
+                return $this->calculateRetryDelay($attempt);
+            }
+        };
+
+        // Test with default config values
+        config(['constants.ssh.retry_base_delay' => 2]);
+        config(['constants.ssh.retry_max_delay' => 30]);
+        config(['constants.ssh.retry_multiplier' => 2]);
+
+        // Attempt 0: 2 seconds
+        $this->assertEquals(2, $handler->test_calculate_retry_delay(0));
+
+        // Attempt 1: 4 seconds
+        $this->assertEquals(4, $handler->test_calculate_retry_delay(1));
+
+        // Attempt 2: 8 seconds
+        $this->assertEquals(8, $handler->test_calculate_retry_delay(2));
+
+        // Attempt 3: 16 seconds
+        $this->assertEquals(16, $handler->test_calculate_retry_delay(3));
+
+        // Attempt 4: Should be capped at 30 seconds
+        $this->assertEquals(30, $handler->test_calculate_retry_delay(4));
+
+        // Attempt 5: Should still be capped at 30 seconds
+        $this->assertEquals(30, $handler->test_calculate_retry_delay(5));
+    }
+
+    public function test_retry_succeeds_after_failures()
+    {
+        $attemptCount = 0;
+
+        config(['constants.ssh.max_retries' => 3]);
+
+        // Simulate a function that fails twice then succeeds using the public static method
+        $result = SshRetryHandler::retry(
+            function () use (&$attemptCount) {
+                $attemptCount++;
+                if ($attemptCount < 3) {
+                    throw new \RuntimeException('kex_exchange_identification: Connection reset by peer');
+                }
+
+                return 'success';
+            },
+            ['test' => 'retry_test'],
+            true
+        );
+
+        $this->assertEquals('success', $result);
+        $this->assertEquals(3, $attemptCount);
+    }
+
+    public function test_retry_fails_after_max_attempts()
+    {
+        $attemptCount = 0;
+
+        config(['constants.ssh.max_retries' => 3]);
+
+        $this->expectException(\RuntimeException::class);
+        $this->expectExceptionMessage('Connection reset by peer');
+
+        // Simulate a function that always fails using the public static method
+        SshRetryHandler::retry(
+            function () use (&$attemptCount) {
+                $attemptCount++;
+                throw new \RuntimeException('Connection reset by peer');
+            },
+            ['test' => 'retry_test'],
+            true
+        );
+    }
+
+    public function test_non_retryable_errors_fail_immediately()
+    {
+        $attemptCount = 0;
+
+        config(['constants.ssh.max_retries' => 3]);
+
+        $this->expectException(\RuntimeException::class);
+        $this->expectExceptionMessage('Command not found');
+
+        try {
+            // Simulate a non-retryable error using the public static method
+            SshRetryHandler::retry(
+                function () use (&$attemptCount) {
+                    $attemptCount++;
+                    throw new \RuntimeException('Command not found');
+                },
+                ['test' => 'non_retryable_test'],
+                true
+            );
+        } catch (\RuntimeException $e) {
+            // Should only attempt once since it's not retryable
+            $this->assertEquals(1, $attemptCount);
+            throw $e;
+        }
+    }
+}