Merge branch 'v4.x' into allow-dep

tests/Unit/ApplicationWatchPathsTest.php

@@ -0,0 +1,368 @@
+<?php
+
+use App\Models\Application;
+
+/**
+ * This matches the CURRENT (broken) behavior without negation support
+ * which is what the old Application.php had
+ */
+function matchWatchPathsCurrentBehavior(array $changed_files, ?array $watch_paths): array
+{
+    if (is_null($watch_paths) || empty($watch_paths)) {
+        return [];
+    }
+
+    $matches = [];
+    foreach ($changed_files as $file) {
+        foreach ($watch_paths as $pattern) {
+            $pattern = trim($pattern);
+            if (empty($pattern)) {
+                continue;
+            }
+            // Old implementation just uses fnmatch directly
+            // This means !patterns are treated as literal strings
+            if (fnmatch($pattern, $file)) {
+                $matches[] = $file;
+                break;
+            }
+        }
+    }
+
+    return $matches;
+}
+
+/**
+ * Use the shared implementation from Application model
+ */
+function matchWatchPaths(array $changed_files, ?array $watch_paths): array
+{
+    $modifiedFiles = collect($changed_files);
+    $watchPaths = is_null($watch_paths) ? null : collect($watch_paths);
+
+    $result = Application::matchPaths($modifiedFiles, $watchPaths);
+
+    return $result->toArray();
+}
+
+it('returns false when watch paths is null', function () {
+    $changed_files = ['docker-compose.yml', 'README.md'];
+    $watch_paths = null;
+
+    $matches = matchWatchPaths($changed_files, $watch_paths);
+    expect($matches)->toBeEmpty();
+});
+
+it('triggers with exact match', function () {
+    $watch_paths = ['docker-compose.yml', 'Dockerfile'];
+
+    // Exact match should return matches
+    $matches = matchWatchPaths(['docker-compose.yml'], $watch_paths);
+    expect($matches)->toHaveCount(1);
+    expect($matches)->toEqual(['docker-compose.yml']);
+
+    $matches = matchWatchPaths(['Dockerfile'], $watch_paths);
+    expect($matches)->toHaveCount(1);
+    expect($matches)->toEqual(['Dockerfile']);
+
+    // Non-matching file should return empty
+    $matches = matchWatchPaths(['README.md'], $watch_paths);
+    expect($matches)->toBeEmpty();
+});
+
+it('triggers with wildcard patterns', function () {
+    $watch_paths = ['*.yml', 'src/**/*.php', 'config/*'];
+
+    // Wildcard matches
+    expect(matchWatchPaths(['docker-compose.yml'], $watch_paths))->not->toBeEmpty();
+    expect(matchWatchPaths(['production.yml'], $watch_paths))->not->toBeEmpty();
+    expect(matchWatchPaths(['src/Controllers/UserController.php'], $watch_paths))->not->toBeEmpty();
+    expect(matchWatchPaths(['src/Models/User.php'], $watch_paths))->not->toBeEmpty();
+    expect(matchWatchPaths(['config/app.php'], $watch_paths))->not->toBeEmpty();
+
+    // Non-matching files
+    expect(matchWatchPaths(['README.md'], $watch_paths))->toBeEmpty();
+    expect(matchWatchPaths(['src/index.js'], $watch_paths))->toBeEmpty();
+    expect(matchWatchPaths(['configurations/deep/file.php'], $watch_paths))->toBeEmpty();
+});
+
+it('triggers with multiple files', function () {
+    $watch_paths = ['docker-compose.yml', '*.env'];
+
+    // At least one file matches
+    $changed_files = ['README.md', 'docker-compose.yml', 'package.json'];
+    $matches = matchWatchPaths($changed_files, $watch_paths);
+    expect($matches)->not->toBeEmpty();
+    expect($matches)->toContain('docker-compose.yml');
+
+    // No files match
+    $changed_files = ['README.md', 'package.json', 'src/index.js'];
+    $matches = matchWatchPaths($changed_files, $watch_paths);
+    expect($matches)->toBeEmpty();
+});
+
+it('handles leading slash include and negation', function () {
+    // Include with leading slash - leading slash patterns may not match as expected with fnmatch
+    // The current implementation doesn't handle leading slashes specially
+    expect(matchWatchPaths(['docs/index.md'], ['/docs/**']))->toEqual([]);
+
+    // With only negation patterns, files that DON'T match the exclusion are included
+    // docs/index.md DOES match docs/**, so it should be excluded
+    expect(matchWatchPaths(['docs/index.md'], ['!/docs/**']))->toEqual(['docs/index.md']);
+
+    // src/app.ts does NOT match docs/**, so it should be included
+    expect(matchWatchPaths(['src/app.ts'], ['!/docs/**']))->toEqual(['src/app.ts']);
+});
+
+it('triggers with complex patterns', function () {
+    // fnmatch doesn't support {a,b} syntax, so we need to use separate patterns
+    $watch_paths = ['**/*.js', '**/*.jsx', '**/*.ts', '**/*.tsx'];
+
+    // JavaScript/TypeScript files should match
+    expect(matchWatchPaths(['src/index.js'], $watch_paths))->not->toBeEmpty();
+    expect(matchWatchPaths(['components/Button.jsx'], $watch_paths))->not->toBeEmpty();
+    expect(matchWatchPaths(['types/user.ts'], $watch_paths))->not->toBeEmpty();
+    expect(matchWatchPaths(['pages/Home.tsx'], $watch_paths))->not->toBeEmpty();
+
+    // Deeply nested files should match
+    expect(matchWatchPaths(['src/components/ui/Button.tsx'], $watch_paths))->not->toBeEmpty();
+
+    // Non-matching files
+    expect(matchWatchPaths(['README.md'], $watch_paths))->toBeEmpty();
+    expect(matchWatchPaths(['package.json'], $watch_paths))->toBeEmpty();
+});
+
+it('triggers with question mark pattern', function () {
+    $watch_paths = ['test?.txt', 'file-?.yml'];
+
+    // Single character wildcard matches
+    expect(matchWatchPaths(['test1.txt'], $watch_paths))->not->toBeEmpty();
+    expect(matchWatchPaths(['testA.txt'], $watch_paths))->not->toBeEmpty();
+    expect(matchWatchPaths(['file-1.yml'], $watch_paths))->not->toBeEmpty();
+    expect(matchWatchPaths(['file-B.yml'], $watch_paths))->not->toBeEmpty();
+
+    // Non-matching files
+    expect(matchWatchPaths(['test.txt'], $watch_paths))->toBeEmpty();
+    expect(matchWatchPaths(['test12.txt'], $watch_paths))->toBeEmpty();
+    expect(matchWatchPaths(['file.yml'], $watch_paths))->toBeEmpty();
+});
+
+it('triggers with character set pattern', function () {
+    $watch_paths = ['[abc]test.txt', 'file[0-9].yml'];
+
+    // Character set matches
+    expect(matchWatchPaths(['atest.txt'], $watch_paths))->not->toBeEmpty();
+    expect(matchWatchPaths(['btest.txt'], $watch_paths))->not->toBeEmpty();
+    expect(matchWatchPaths(['ctest.txt'], $watch_paths))->not->toBeEmpty();
+    expect(matchWatchPaths(['file1.yml'], $watch_paths))->not->toBeEmpty();
+    expect(matchWatchPaths(['file9.yml'], $watch_paths))->not->toBeEmpty();
+
+    // Non-matching files
+    expect(matchWatchPaths(['dtest.txt'], $watch_paths))->toBeEmpty();
+    expect(matchWatchPaths(['test.txt'], $watch_paths))->toBeEmpty();
+    expect(matchWatchPaths(['fileA.yml'], $watch_paths))->toBeEmpty();
+});
+
+it('triggers with empty watch paths', function () {
+    $watch_paths = [];
+
+    $matches = matchWatchPaths(['any-file.txt'], $watch_paths);
+    expect($matches)->toBeEmpty();
+});
+
+it('triggers with whitespace only patterns', function () {
+    $watch_paths = ['', '  ', '	'];
+
+    $matches = matchWatchPaths(['any-file.txt'], $watch_paths);
+    expect($matches)->toBeEmpty();
+});
+
+it('triggers for docker compose typical patterns', function () {
+    $watch_paths = ['docker-compose*.yml', '.env*', 'Dockerfile*', 'services/**'];
+
+    // Docker Compose related files
+    expect(matchWatchPaths(['docker-compose.yml'], $watch_paths))->not->toBeEmpty();
+    expect(matchWatchPaths(['docker-compose.prod.yml'], $watch_paths))->not->toBeEmpty();
+    expect(matchWatchPaths(['docker-compose-dev.yml'], $watch_paths))->not->toBeEmpty();
+
+    // Environment files
+    expect(matchWatchPaths(['.env'], $watch_paths))->not->toBeEmpty();
+    expect(matchWatchPaths(['.env.local'], $watch_paths))->not->toBeEmpty();
+    expect(matchWatchPaths(['.env.production'], $watch_paths))->not->toBeEmpty();
+
+    // Dockerfile variations
+    expect(matchWatchPaths(['Dockerfile'], $watch_paths))->not->toBeEmpty();
+    expect(matchWatchPaths(['Dockerfile.prod'], $watch_paths))->not->toBeEmpty();
+
+    // Service files
+    expect(matchWatchPaths(['services/api/app.js'], $watch_paths))->not->toBeEmpty();
+    expect(matchWatchPaths(['services/web/index.html'], $watch_paths))->not->toBeEmpty();
+
+    // Non-matching files (e.g., documentation, configs outside services)
+    expect(matchWatchPaths(['README.md'], $watch_paths))->toBeEmpty();
+    expect(matchWatchPaths(['package.json'], $watch_paths))->toBeEmpty();
+    expect(matchWatchPaths(['config/nginx.conf'], $watch_paths))->toBeEmpty();
+});
+
+it('handles negation pattern with non matching file', function () {
+    // Test case: file that does NOT match the exclusion pattern should trigger
+    $changed_files = ['docker-compose/index.ts'];
+    $watch_paths = ['!docker-compose-test/**'];
+
+    // Since the file docker-compose/index.ts does NOT match the exclusion pattern docker-compose-test/**
+    // it should trigger the deployment (file is included by default when only exclusion patterns exist)
+    // This means: "deploy everything EXCEPT files in docker-compose-test/**"
+    $matches = matchWatchPaths($changed_files, $watch_paths);
+    expect($matches)->not->toBeEmpty();
+    expect($matches)->toEqual(['docker-compose/index.ts']);
+
+    // Test the opposite: file that DOES match the exclusion pattern should NOT trigger
+    $changed_files = ['docker-compose-test/index.ts'];
+    $matches = matchWatchPaths($changed_files, $watch_paths);
+    expect($matches)->toBeEmpty();
+
+    // Test with deeper path
+    $changed_files = ['docker-compose-test/sub/dir/file.ts'];
+    $matches = matchWatchPaths($changed_files, $watch_paths);
+    expect($matches)->toBeEmpty();
+});
+
+it('handles mixed inclusion and exclusion patterns', function () {
+    // Include all JS files but exclude test directories
+    $watch_paths = ['**/*.js', '!**/*test*/**'];
+
+    // Should match: JS files not in test directories
+    expect(matchWatchPaths(['src/index.js'], $watch_paths))->not->toBeEmpty();
+    expect(matchWatchPaths(['components/Button.js'], $watch_paths))->not->toBeEmpty();
+
+    // Should NOT match: JS files in test directories
+    expect(matchWatchPaths(['test/unit/app.js'], $watch_paths))->toBeEmpty();
+    expect(matchWatchPaths(['src/test-utils/helper.js'], $watch_paths))->toBeEmpty();
+    expect(matchWatchPaths(['docker-compose-test/index.js'], $watch_paths))->toBeEmpty();
+
+    // Should NOT match: non-JS files
+    expect(matchWatchPaths(['README.md'], $watch_paths))->toBeEmpty();
+});
+
+it('handles multiple negation patterns', function () {
+    // Exclude multiple directories
+    $watch_paths = ['!tests/**', '!docs/**', '!*.md'];
+
+    // Should match: files not in excluded patterns
+    expect(matchWatchPaths(['src/index.js'], $watch_paths))->not->toBeEmpty();
+    expect(matchWatchPaths(['docker-compose.yml'], $watch_paths))->not->toBeEmpty();
+
+    // Should NOT match: files in excluded patterns
+    expect(matchWatchPaths(['tests/unit/test.js'], $watch_paths))->toBeEmpty();
+    expect(matchWatchPaths(['docs/api.html'], $watch_paths))->toBeEmpty();
+    expect(matchWatchPaths(['README.md'], $watch_paths))->toBeEmpty();
+    expect(matchWatchPaths(['CHANGELOG.md'], $watch_paths))->toBeEmpty();
+});
+
+it('demonstrates current broken behavior with negation patterns', function () {
+    // This test demonstrates the CURRENT broken behavior
+    // where negation patterns are treated as literal strings
+    $changed_files = ['docker-compose/index.ts'];
+    $watch_paths = ['!docker-compose-test/**'];
+
+    // With the current broken implementation, this returns empty
+    // because it tries to match files starting with literal "!"
+    $matches = matchWatchPathsCurrentBehavior($changed_files, $watch_paths);
+    expect($matches)->toBeEmpty(); // This is why your webhook doesn't trigger!
+
+    // Even if the file had ! in the path, fnmatch would treat ! as a literal character
+    // not as a negation operator, so it still wouldn't match the pattern correctly
+    $changed_files = ['test/file.ts'];
+    $matches = matchWatchPathsCurrentBehavior($changed_files, $watch_paths);
+    expect($matches)->toBeEmpty();
+});
+
+it('handles order based matching with conflicting patterns', function () {
+    // Test case 1: Exclude then include - last pattern (include) should win
+    $changed_files = ['docker-compose/index.ts'];
+    $watch_paths = ['!docker-compose/**', 'docker-compose/**'];
+
+    $matches = matchWatchPaths($changed_files, $watch_paths);
+    expect($matches)->not->toBeEmpty();
+    expect($matches)->toEqual(['docker-compose/index.ts']);
+
+    // Test case 2: Include then exclude - last pattern (exclude) should win
+    $watch_paths = ['docker-compose/**', '!docker-compose/**'];
+
+    $matches = matchWatchPaths($changed_files, $watch_paths);
+    expect($matches)->toBeEmpty();
+});
+
+it('handles order based matching with multiple overlapping patterns', function () {
+    $changed_files = ['src/test/unit.js', 'src/components/Button.js', 'test/integration.js'];
+
+    // Include all JS, then exclude test dirs, then re-include specific test file
+    $watch_paths = [
+        '**/*.js',              // Include all JS files
+        '!**/test/**',          // Exclude all test directories
+        'src/test/unit.js',      // Re-include this specific test file
+    ];
+
+    $matches = matchWatchPaths($changed_files, $watch_paths);
+
+    // src/test/unit.js should be included (last specific pattern wins)
+    // src/components/Button.js should be included (only matches first pattern)
+    // test/integration.js should be excluded (matches exclude pattern, no override)
+    expect($matches)->toHaveCount(2);
+    expect($matches)->toContain('src/test/unit.js');
+    expect($matches)->toContain('src/components/Button.js');
+    expect($matches)->not->toContain('test/integration.js');
+});
+
+it('handles order based matching with specific overrides', function () {
+    $changed_files = [
+        'docs/api.md',
+        'docs/guide.md',
+        'docs/internal/secret.md',
+        'src/index.js',
+    ];
+
+    // Exclude all docs, then include specific docs subdirectory
+    $watch_paths = [
+        '!docs/**',             // Exclude all docs
+        'docs/internal/**',     // But include internal docs
+        'src/**',                // Include src files
+    ];
+
+    $matches = matchWatchPaths($changed_files, $watch_paths);
+
+    // Only docs/internal/secret.md and src/index.js should be included
+    expect($matches)->toHaveCount(2);
+    expect($matches)->toContain('docs/internal/secret.md');
+    expect($matches)->toContain('src/index.js');
+    expect($matches)->not->toContain('docs/api.md');
+    expect($matches)->not->toContain('docs/guide.md');
+});
+
+it('preserves order precedence in pattern matching', function () {
+    $changed_files = ['app/config.json'];
+
+    // Multiple conflicting patterns - last match should win
+    $watch_paths = [
+        '**/*.json',        // Include (matches)
+        '!app/**',          // Exclude (matches)
+        'app/*.json',       // Include (matches) - THIS SHOULD WIN
+    ];
+
+    $matches = matchWatchPaths($changed_files, $watch_paths);
+
+    // File should be included because last matching pattern is inclusive
+    expect($matches)->not->toBeEmpty();
+    expect($matches)->toEqual(['app/config.json']);
+
+    // Now reverse the last two patterns
+    $watch_paths = [
+        '**/*.json',        // Include (matches)
+        'app/*.json',       // Include (matches)
+        '!app/**',          // Exclude (matches) - THIS SHOULD WIN
+    ];
+
+    $matches = matchWatchPaths($changed_files, $watch_paths);
+
+    // File should be excluded because last matching pattern is exclusive
+    expect($matches)->toBeEmpty();
+});

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;
+        }
+    }
+}

tests/Unit/ValidGitRepositoryUrlTest.php

@@ -0,0 +1,357 @@
+<?php
+
+use App\Rules\ValidGitRepositoryUrl;
+use Illuminate\Support\Facades\Validator;
+use Tests\TestCase;
+
+uses(TestCase::class);
+
+it('validates standard GitHub URLs', function () {
+    $rule = new ValidGitRepositoryUrl;
+
+    $validUrls = [
+        'https://github.com/user/repo',
+        'https://github.com/user/repo.git',
+        'https://github.com/user/repo-with-dashes',
+        'https://github.com/user/repo_with_underscores',
+        'https://github.com/user/repo.with.dots',
+        'https://github.com/organization/repository',
+    ];
+
+    foreach ($validUrls as $url) {
+        $validator = Validator::make(['url' => $url], ['url' => $rule]);
+        expect($validator->passes())->toBeTrue("Failed for URL: {$url}");
+    }
+});
+
+it('validates GitLab URLs', function () {
+    $rule = new ValidGitRepositoryUrl;
+
+    $validUrls = [
+        'https://gitlab.com/user/repo',
+        'https://gitlab.com/user/repo.git',
+        'https://gitlab.com/organization/repository',
+    ];
+
+    foreach ($validUrls as $url) {
+        $validator = Validator::make(['url' => $url], ['url' => $rule]);
+        expect($validator->passes())->toBeTrue("Failed for URL: {$url}");
+    }
+});
+
+it('validates Bitbucket URLs', function () {
+    $rule = new ValidGitRepositoryUrl;
+
+    $validUrls = [
+        'https://bitbucket.org/user/repo',
+        'https://bitbucket.org/user/repo.git',
+        'https://bitbucket.org/organization/repository',
+    ];
+
+    foreach ($validUrls as $url) {
+        $validator = Validator::make(['url' => $url], ['url' => $rule]);
+        expect($validator->passes())->toBeTrue("Failed for URL: {$url}");
+    }
+});
+
+it('validates tangled.sh URLs with @ symbol', function () {
+    $rule = new ValidGitRepositoryUrl;
+
+    $validUrls = [
+        'https://tangled.org/@tangled.org/site',
+        'https://tangled.org/@user/repo',
+        'https://tangled.org/@organization/project',
+    ];
+
+    foreach ($validUrls as $url) {
+        $validator = Validator::make(['url' => $url], ['url' => $rule]);
+        expect($validator->passes())->toBeTrue("Failed for URL: {$url}");
+    }
+});
+
+it('validates SourceHut URLs with ~ symbol', function () {
+    $rule = new ValidGitRepositoryUrl;
+
+    $validUrls = [
+        'https://git.sr.ht/~user/repo',
+        'https://git.sr.ht/~user/project',
+        'https://git.sr.ht/~organization/repository',
+    ];
+
+    foreach ($validUrls as $url) {
+        $validator = Validator::make(['url' => $url], ['url' => $rule]);
+        expect($validator->passes())->toBeTrue("Failed for URL: {$url}");
+    }
+});
+
+it('validates other Git hosting services', function () {
+    $rule = new ValidGitRepositoryUrl;
+
+    $validUrls = [
+        'https://codeberg.org/user/repo',
+        'https://codeberg.org/user/repo.git',
+        'https://gitea.com/user/repo',
+        'https://gitea.com/user/repo.git',
+    ];
+
+    foreach ($validUrls as $url) {
+        $validator = Validator::make(['url' => $url], ['url' => $rule]);
+        expect($validator->passes())->toBeTrue("Failed for URL: {$url}");
+    }
+});
+
+it('validates SSH URLs when allowed', function () {
+    $rule = new ValidGitRepositoryUrl(allowSSH: true);
+
+    $validUrls = [
+        'git@github.com:user/repo.git',
+        'git@gitlab.com:user/repo.git',
+        'git@bitbucket.org:user/repo.git',
+    ];
+
+    foreach ($validUrls as $url) {
+        $validator = Validator::make(['url' => $url], ['url' => $rule]);
+        expect($validator->passes())->toBeTrue("Failed for SSH URL: {$url}");
+    }
+});
+
+it('rejects SSH URLs when not allowed', function () {
+    $rule = new ValidGitRepositoryUrl(allowSSH: false);
+
+    $invalidUrls = [
+        'git@github.com:user/repo.git',
+        'git@gitlab.com:user/repo.git',
+    ];
+
+    foreach ($invalidUrls as $url) {
+        $validator = Validator::make(['url' => $url], ['url' => $rule]);
+        expect($validator->fails())->toBeTrue("SSH URL should be rejected: {$url}");
+        expect($validator->errors()->first('url'))->toContain('SSH URLs are not allowed');
+    }
+});
+
+it('validates git:// protocol URLs', function () {
+    $rule = new ValidGitRepositoryUrl;
+
+    $validUrls = [
+        'git://github.com/user/repo.git',
+        'git://gitlab.com/user/repo.git',
+        'git://git.sr.ht:~user/repo.git',
+    ];
+
+    foreach ($validUrls as $url) {
+        $validator = Validator::make(['url' => $url], ['url' => $rule]);
+        expect($validator->passes())->toBeTrue("Failed for git:// URL: {$url}");
+    }
+});
+
+it('rejects URLs with query parameters', function () {
+    $rule = new ValidGitRepositoryUrl;
+
+    $invalidUrls = [
+        'https://github.com/user/repo?ref=main',
+        'https://github.com/user/repo?token=abc123',
+        'https://github.com/user/repo?utm_source=test',
+    ];
+
+    foreach ($invalidUrls as $url) {
+        $validator = Validator::make(['url' => $url], ['url' => $rule]);
+        expect($validator->fails())->toBeTrue("URL with query parameters should be rejected: {$url}");
+        expect($validator->errors()->first('url'))->toContain('invalid characters');
+    }
+});
+
+it('rejects URLs with fragments', function () {
+    $rule = new ValidGitRepositoryUrl;
+
+    $invalidUrls = [
+        'https://github.com/user/repo#main',
+        'https://github.com/user/repo#readme',
+    ];
+
+    foreach ($invalidUrls as $url) {
+        $validator = Validator::make(['url' => $url], ['url' => $rule]);
+        expect($validator->fails())->toBeTrue("URL with fragments should be rejected: {$url}");
+        expect($validator->errors()->first('url'))->toContain('invalid characters');
+    }
+});
+
+it('rejects internal/localhost URLs', function () {
+    $rule = new ValidGitRepositoryUrl;
+
+    $invalidUrls = [
+        'https://localhost/user/repo',
+        'https://127.0.0.1/user/repo',
+        'https://0.0.0.0/user/repo',
+        'https://::1/user/repo',
+        'https://example.local/user/repo',
+    ];
+
+    foreach ($invalidUrls as $url) {
+        $validator = Validator::make(['url' => $url], ['url' => $rule]);
+        expect($validator->fails())->toBeTrue("Internal URL should be rejected: {$url}");
+        $errorMessage = $validator->errors()->first('url');
+        expect(in_array($errorMessage, [
+            'The url cannot point to internal hosts.',
+            'The url cannot use IP addresses.',
+            'The url is not a valid URL.',
+        ]))->toBeTrue("Unexpected error message: {$errorMessage}");
+    }
+});
+
+it('rejects IP addresses when not allowed', function () {
+    $rule = new ValidGitRepositoryUrl(allowIP: false);
+
+    $invalidUrls = [
+        'https://192.168.1.1/user/repo',
+        'https://10.0.0.1/user/repo',
+        'https://172.16.0.1/user/repo',
+    ];
+
+    foreach ($invalidUrls as $url) {
+        $validator = Validator::make(['url' => $url], ['url' => $rule]);
+        expect($validator->fails())->toBeTrue("IP address URL should be rejected: {$url}");
+        expect($validator->errors()->first('url'))->toContain('IP addresses');
+    }
+});
+
+it('allows IP addresses when explicitly allowed', function () {
+    $rule = new ValidGitRepositoryUrl(allowIP: true);
+
+    $validUrls = [
+        'https://192.168.1.1/user/repo',
+        'https://10.0.0.1/user/repo',
+    ];
+
+    foreach ($validUrls as $url) {
+        $validator = Validator::make(['url' => $url], ['url' => $rule]);
+        expect($validator->passes())->toBeTrue("IP address URL should be allowed: {$url}");
+    }
+});
+
+it('rejects dangerous shell metacharacters', function () {
+    $rule = new ValidGitRepositoryUrl;
+
+    $dangerousChars = [';', '|', '&', '$', '`', '(', ')', '{', '}', '[', ']', '<', '>', '\n', '\r', '\0', '"', "'", '\\', '!', '?', '*', '^', '%', '=', '+', '#'];
+
+    foreach ($dangerousChars as $char) {
+        $url = "https://github.com/user/repo{$char}";
+        $validator = Validator::make(['url' => $url], ['url' => $rule]);
+        expect($validator->fails())->toBeTrue("URL with dangerous character '{$char}' should be rejected");
+        expect($validator->errors()->first('url'))->toContain('invalid characters');
+    }
+});
+
+it('rejects command substitution patterns', function () {
+    $rule = new ValidGitRepositoryUrl;
+
+    $dangerousPatterns = [
+        'https://github.com/user/$(whoami)',
+        'https://github.com/user/${USER}',
+        'https://github.com/user;;',
+        'https://github.com/user&&',
+        'https://github.com/user||',
+        'https://github.com/user>>',
+        'https://github.com/user<<',
+        'https://github.com/user\\n',
+        'https://github.com/user../',
+    ];
+
+    foreach ($dangerousPatterns as $url) {
+        $validator = Validator::make(['url' => $url], ['url' => $rule]);
+        expect($validator->fails())->toBeTrue("URL with dangerous pattern should be rejected: {$url}");
+        $errorMessage = $validator->errors()->first('url');
+        expect(in_array($errorMessage, [
+            'The url contains invalid characters.',
+            'The url contains invalid patterns.',
+        ]))->toBeTrue("Unexpected error message: {$errorMessage}");
+    }
+});
+
+it('rejects invalid URL formats', function () {
+    $rule = new ValidGitRepositoryUrl;
+
+    $invalidUrls = [
+        'not-a-url',
+        'ftp://github.com/user/repo',
+        'file:///path/to/repo',
+        'ssh://github.com/user/repo',
+        'https://',
+        'http://',
+        'git@',
+    ];
+
+    foreach ($invalidUrls as $url) {
+        $validator = Validator::make(['url' => $url], ['url' => $rule]);
+        expect($validator->fails())->toBeTrue("Invalid URL format should be rejected: {$url}");
+    }
+});
+
+it('accepts empty values', function () {
+    $rule = new ValidGitRepositoryUrl;
+
+    $validator = Validator::make(['url' => ''], ['url' => $rule]);
+    expect($validator->passes())->toBeTrue('Empty URL should be accepted');
+
+    $validator = Validator::make(['url' => null], ['url' => $rule]);
+    expect($validator->passes())->toBeTrue('Null URL should be accepted');
+});
+
+it('validates complex repository paths', function () {
+    $rule = new ValidGitRepositoryUrl;
+
+    $validUrls = [
+        'https://github.com/user/repo-with-many-dashes',
+        'https://github.com/user/repo_with_many_underscores',
+        'https://github.com/user/repo.with.many.dots',
+        'https://github.com/user/repo@version',
+        'https://github.com/user/repo~backup',
+        'https://github.com/user/repo@version~backup',
+    ];
+
+    foreach ($validUrls as $url) {
+        $validator = Validator::make(['url' => $url], ['url' => $rule]);
+        expect($validator->passes())->toBeTrue("Complex path should be valid: {$url}");
+    }
+});
+
+it('validates nested repository paths', function () {
+    $rule = new ValidGitRepositoryUrl;
+
+    $validUrls = [
+        'https://github.com/org/suborg/repo',
+        'https://gitlab.com/group/subgroup/project',
+        'https://tangled.org/@org/suborg/project',
+        'https://git.sr.ht/~user/project/subproject',
+    ];
+
+    foreach ($validUrls as $url) {
+        $validator = Validator::make(['url' => $url], ['url' => $rule]);
+        expect($validator->passes())->toBeTrue("Nested path should be valid: {$url}");
+    }
+});
+
+it('provides meaningful error messages', function () {
+    $rule = new ValidGitRepositoryUrl;
+
+    $testCases = [
+        [
+            'url' => 'https://github.com/user; rm -rf /',
+            'expectedError' => 'invalid characters',
+        ],
+        [
+            'url' => 'https://github.com/user/repo?token=secret',
+            'expectedError' => 'invalid characters',
+        ],
+        [
+            'url' => 'https://localhost/user/repo',
+            'expectedError' => 'internal hosts',
+        ],
+    ];
+
+    foreach ($testCases as $testCase) {
+        $validator = Validator::make(['url' => $testCase['url']], ['url' => $rule]);
+        expect($validator->fails())->toBeTrue("Should fail for: {$testCase['url']}");
+        expect($validator->errors()->first('url'))->toContain($testCase['expectedError']);
+    }
+});