diff --git a/app/Http/Controllers/GitHubAppWebhookController.php b/app/Http/Controllers/GitHubAppWebhookController.php index 9f9c3a036..d9e3de281 100644 --- a/app/Http/Controllers/GitHubAppWebhookController.php +++ b/app/Http/Controllers/GitHubAppWebhookController.php @@ -198,6 +198,10 @@ protected function handleRepositoryEvent(string $event, array $payload): JsonRes ->get() ->filter(fn (Plugin $plugin): bool => $plugin->isActive()); + if ($event === 'release') { + $plugins = $plugins->filter(fn (Plugin $plugin): bool => $plugin->claimReleaseEvent($payload)); + } + $syncService = app(PluginSyncService::class); foreach ($plugins as $plugin) { diff --git a/app/Http/Controllers/PluginWebhookController.php b/app/Http/Controllers/PluginWebhookController.php index a1b57fc23..938539988 100644 --- a/app/Http/Controllers/PluginWebhookController.php +++ b/app/Http/Controllers/PluginWebhookController.php @@ -29,6 +29,10 @@ public function __invoke(Request $request, string $secret, PluginSyncService $sy } if ($event === 'release') { + if (! $plugin->claimReleaseEvent($this->payload($request))) { + return response()->json(['success' => true, 'message' => 'Release event ignored']); + } + // Sync plugin metadata to update latest_version $syncService->sync($plugin); @@ -69,4 +73,17 @@ public function __invoke(Request $request, string $secret, PluginSyncService $sy 'releases_sync' => 'queued', ]); } + + /** + * The event payload. A webhook left on GitHub's default form content type sends it as JSON in + * a "payload" field. + * + * @return array + */ + protected function payload(Request $request): array + { + $payload = $request->input('payload'); + + return is_string($payload) ? (json_decode($payload, true) ?? []) : $request->all(); + } } diff --git a/app/Jobs/Concerns/ResolvesGitHubToken.php b/app/Jobs/Concerns/ResolvesGitHubToken.php index 1974904fb..29888cfa2 100644 --- a/app/Jobs/Concerns/ResolvesGitHubToken.php +++ b/app/Jobs/Concerns/ResolvesGitHubToken.php @@ -4,6 +4,8 @@ use App\Models\Plugin; use App\Services\GitHubAppService; +use Illuminate\Http\Client\ConnectionException; +use Illuminate\Support\Facades\Http; use Illuminate\Support\Facades\Log; trait ResolvesGitHubToken @@ -29,7 +31,7 @@ protected function resolveGitHubTokenFor(Plugin $plugin): ?string if ($installation) { $token = $appService->getInstallationToken($installation); - if ($token) { + if ($token && $this->gitHubAcceptsToken($plugin, $token)) { Log::debug('[GitHub] Using installation token', [ 'plugin_id' => $plugin->id, 'user_id' => $user->id, @@ -42,14 +44,16 @@ protected function resolveGitHubTokenFor(Plugin $plugin): ?string } // Priority 2: User OAuth token - if ($user && $user->hasGitHubToken()) { + $userToken = $user?->getGitHubToken(); + + if ($userToken && $this->gitHubAcceptsToken($plugin, $userToken)) { Log::debug('[GitHub] Using plugin owner OAuth token', [ 'plugin_id' => $plugin->id, 'user_id' => $user->id, 'github_username' => $user->github_username, ]); - return $user->getGitHubToken(); + return $userToken; } // Priority 3: Platform token @@ -62,4 +66,40 @@ protected function resolveGitHubTokenFor(Plugin $plugin): ?string return $platformToken; } + + /** + * Whether GitHub still accepts a stored token for the plugin's repository. Access can be revoked + * on GitHub's side without us hearing about it, and a revoked token fails everything it's handed + * to, Satis builds included. Only a 401 (revoked) or 404 (can't see the repository) counts + * against a token, since a rate limit or an outage says nothing about the token itself. The + * platform token is the last resort, so it isn't tried. + */ + protected function gitHubAcceptsToken(Plugin $plugin, string $token): bool + { + $repo = $plugin->getRepositoryOwnerAndName(); + + if (! $repo) { + return true; + } + + try { + $response = Http::withToken($token) + ->accept('application/vnd.github+json') + ->timeout(10) + ->get("https://api.github.com/repos/{$repo['owner']}/{$repo['repo']}"); + } catch (ConnectionException) { + return true; + } + + if ($response->unauthorized() || $response->notFound()) { + Log::warning('[GitHub] Token rejected, trying the next one', [ + 'plugin_id' => $plugin->id, + 'status' => $response->status(), + ]); + + return false; + } + + return true; + } } diff --git a/app/Jobs/SyncPluginReleases.php b/app/Jobs/SyncPluginReleases.php index e6003c008..1b3533819 100644 --- a/app/Jobs/SyncPluginReleases.php +++ b/app/Jobs/SyncPluginReleases.php @@ -56,7 +56,7 @@ public function handle(SatisService $satisService): void 'plugin_id' => $this->plugin->id, 'owner' => $repo['owner'], 'repo' => $repo['repo'], - 'token_source' => $token ? ($this->plugin->user?->hasGitHubToken() ? 'user_oauth' : 'platform') : 'none', + 'has_token' => $token !== null, ]); $releases = $this->fetchReleases($repo['owner'], $repo['repo'], $token); @@ -104,7 +104,7 @@ public function handle(SatisService $satisService): void 'plugin_name' => $this->plugin->name, ]); - $result = $satisService->build([$this->plugin], $this->getGitHubToken()); + $result = $satisService->build([$this->plugin], $token); if ($result['success'] ?? false) { $this->plugin->update(['satis_synced_at' => now()]); diff --git a/app/Models/Plugin.php b/app/Models/Plugin.php index 09fa1cf78..16d9671d5 100644 --- a/app/Models/Plugin.php +++ b/app/Models/Plugin.php @@ -36,6 +36,7 @@ use Illuminate\Database\Eloquent\Relations\HasMany; use Illuminate\Database\Eloquent\Relations\HasOne; use Illuminate\Support\Collection; +use Illuminate\Support\Facades\Cache; use Illuminate\Support\Facades\Notification; class Plugin extends Model @@ -818,6 +819,25 @@ public function isReachableViaGitHubApp(): bool return app(GitHubAppService::class)->findInstallationForRepo($this->user, $repo['owner'], $repo['repo']) !== null; } + /** + * Claim a GitHub release event, so each published release is only synced once. GitHub sends + * created, published and released for a single publish, and a repository the GitHub App covers + * can still have its old per-repository webhook, which delivers every event a second time. Only + * the first published event for each release gets through. + * + * @param array $payload + */ + public function claimReleaseEvent(array $payload): bool + { + $releaseId = $payload['release']['id'] ?? null; + + if (($payload['action'] ?? null) !== 'published' || ! $releaseId) { + return false; + } + + return Cache::add("plugin_{$this->id}_published_release_{$releaseId}", true, now()->addHour()); + } + public function getRepositoryOwnerAndName(): ?array { if (! $this->repository_url) { diff --git a/app/Services/PluginSyncService.php b/app/Services/PluginSyncService.php index c3cd7d461..a6bc4dc7b 100644 --- a/app/Services/PluginSyncService.php +++ b/app/Services/PluginSyncService.php @@ -2,6 +2,7 @@ namespace App\Services; +use App\Jobs\Concerns\ResolvesGitHubToken; use App\Jobs\GeneratePluginOgImage; use App\Models\Plugin; use App\Support\CommonMark\CommonMark; @@ -10,6 +11,8 @@ class PluginSyncService { + use ResolvesGitHubToken; + public function sync(Plugin $plugin): bool { Log::info('[PluginSync] Starting sync', ['plugin_id' => $plugin->id, 'name' => $plugin->name]); @@ -28,7 +31,7 @@ public function sync(Plugin $plugin): bool 'repo' => $repo['repo'], ]); - $token = $this->getGitHubToken($plugin); + $token = $this->resolveGitHubTokenFor($plugin); Log::info('[PluginSync] Token resolved', [ 'plugin_id' => $plugin->id, @@ -192,34 +195,6 @@ protected function fetchFileFromGitHub(string $owner, string $repo, string $path return null; } - protected function getGitHubToken(Plugin $plugin): ?string - { - $user = $plugin->user; - $repo = $plugin->getRepositoryOwnerAndName(); - - // Priority 1: Installation token (GitHub App) - if ($user && $repo && $user->isUsingGitHubApp()) { - $appService = app(GitHubAppService::class); - $installation = $appService->findInstallationForRepo($user, $repo['owner'], $repo['repo']); - - if ($installation) { - $token = $appService->getInstallationToken($installation); - - if ($token) { - return $token; - } - } - } - - // Priority 2: User OAuth token - if ($user && $user->hasGitHubToken()) { - return $user->getGitHubToken(); - } - - // Priority 3: Platform token - return config('services.github.token'); - } - protected function extractIosVersion(array $nativephpData): ?string { return $nativephpData['ios']['min_version'] ?? null; diff --git a/tests/Feature/GitHubAppRepositoryEventsTest.php b/tests/Feature/GitHubAppRepositoryEventsTest.php index 2f1208d36..8aa8538b8 100644 --- a/tests/Feature/GitHubAppRepositoryEventsTest.php +++ b/tests/Feature/GitHubAppRepositoryEventsTest.php @@ -48,6 +48,15 @@ private function sendAppWebhook(string $event, array $payload): TestResponse ], $body); } + private function releasePayload(string $action, int $releaseId): array + { + return [ + 'action' => $action, + 'release' => ['id' => $releaseId, 'tag_name' => "v1.0.{$releaseId}"], + 'repository' => ['full_name' => 'acme/camera-plugin'], + ]; + } + private function coveredPlugin(array $attributes = []): Plugin { $user = User::factory()->withGitHubApp()->create(); @@ -90,12 +99,64 @@ public function test_release_event_from_the_app_syncs_releases(): void $mock->shouldReceive('sync')->once()->andReturn(true); }); - $this->sendAppWebhook('release', ['repository' => ['full_name' => 'acme/camera-plugin']]) + $this->sendAppWebhook('release', $this->releasePayload('published', 101)) ->assertOk(); Bus::assertDispatched(SyncPluginReleases::class, fn (SyncPluginReleases $job) => $job->plugin->is($plugin)); } + public function test_only_the_published_release_event_is_synced(): void + { + Bus::fake([SyncPluginReleases::class]); + $this->coveredPlugin(); + + $this->mock(PluginSyncService::class, function (MockInterface $mock): void { + $mock->shouldReceive('sync')->once()->andReturn(true); + }); + + // GitHub sends all three of these when a release is published + foreach (['created', 'published', 'released'] as $action) { + $this->sendAppWebhook('release', $this->releasePayload($action, 101))->assertOk(); + } + + Bus::assertDispatchedTimes(SyncPluginReleases::class, 1); + } + + public function test_a_release_the_repository_webhook_already_delivered_is_not_synced_again(): void + { + Bus::fake([SyncPluginReleases::class]); + $plugin = $this->coveredPlugin(['last_synced_at' => now()]); + + $this->mock(PluginSyncService::class, function (MockInterface $mock): void { + $mock->shouldReceive('sync')->once()->andReturn(true); + }); + + $this->postJson(route('webhooks.plugins', $plugin->webhook_secret), $this->releasePayload('published', 101), [ + 'X-GitHub-Event' => 'release', + ])->assertOk(); + + $this->sendAppWebhook('release', $this->releasePayload('published', 101)) + ->assertOk() + ->assertJson(['plugins' => 0]); + + Bus::assertDispatchedTimes(SyncPluginReleases::class, 1); + } + + public function test_each_published_release_is_synced(): void + { + Bus::fake([SyncPluginReleases::class]); + $this->coveredPlugin(); + + $this->mock(PluginSyncService::class, function (MockInterface $mock): void { + $mock->shouldReceive('sync')->twice()->andReturn(true); + }); + + $this->sendAppWebhook('release', $this->releasePayload('published', 101))->assertOk(); + $this->sendAppWebhook('release', $this->releasePayload('published', 102))->assertOk(); + + Bus::assertDispatchedTimes(SyncPluginReleases::class, 2); + } + public function test_events_for_inactive_plugins_are_ignored(): void { $this->coveredPlugin(['is_active' => false]); diff --git a/tests/Feature/PluginSyncServiceTest.php b/tests/Feature/PluginSyncServiceTest.php index ff886ca7a..3332b6feb 100644 --- a/tests/Feature/PluginSyncServiceTest.php +++ b/tests/Feature/PluginSyncServiceTest.php @@ -3,8 +3,10 @@ namespace Tests\Feature; use App\Models\Plugin; +use App\Models\User; use App\Services\PluginSyncService; use Illuminate\Foundation\Testing\RefreshDatabase; +use Illuminate\Http\Client\Request; use Illuminate\Support\Facades\Http; use Illuminate\Support\Facades\Queue; use Tests\TestCase; @@ -172,6 +174,32 @@ public function test_sync_does_not_overwrite_name_when_taken_by_another_plugin() $this->assertEquals('acme/original-name', $plugin->fresh()->name); } + public function test_sync_skips_an_owner_token_github_has_revoked(): void + { + config(['services.github.token' => 'ghp_platform']); + + Http::fake([ + 'api.github.com/repos/acme/test-plugin' => Http::response(['message' => 'Bad credentials'], 401), + 'api.github.com/repos/acme/test-plugin/contents/composer.json' => Http::response([ + 'content' => base64_encode(json_encode(['name' => 'acme/test-plugin'])), + ]), + 'api.github.com/*' => Http::response([], 404), + 'raw.githubusercontent.com/*' => Http::response('', 404), + ]); + + $plugin = Plugin::factory() + ->for(User::factory()->withGitHubApp()->create(['github_token' => encrypt('ghu_revoked')])) + ->create([ + 'name' => 'acme/test-plugin', + 'repository_url' => 'https://github.com/acme/test-plugin', + ]); + + $this->assertTrue((new PluginSyncService)->sync($plugin)); + + Http::assertSent(fn (Request $request) => str_ends_with($request->url(), '/contents/composer.json') + && $request->hasHeader('Authorization', 'Bearer ghp_platform')); + } + public function test_sync_sets_mobile_min_version_to_null_when_not_in_composer_data(): void { $composerJson = json_encode([ diff --git a/tests/Feature/PluginWebhookTest.php b/tests/Feature/PluginWebhookTest.php index e3e123459..da8b84bd4 100644 --- a/tests/Feature/PluginWebhookTest.php +++ b/tests/Feature/PluginWebhookTest.php @@ -2,9 +2,11 @@ namespace Tests\Feature; +use App\Jobs\SyncPluginReleases; use App\Models\Plugin; use App\Services\PluginSyncService; use Illuminate\Foundation\Testing\RefreshDatabase; +use Illuminate\Support\Facades\Bus; use PHPUnit\Framework\Attributes\Test; use Tests\TestCase; @@ -79,6 +81,91 @@ public function non_ping_event_succeeds_for_unapproved_but_active_plugin(): void ->assertJson(['success' => true]); } + #[Test] + public function published_release_queues_a_release_sync(): void + { + Bus::fake([SyncPluginReleases::class]); + + $plugin = Plugin::factory()->create(['last_synced_at' => now()]); + + $this->mock(PluginSyncService::class, function ($mock) { + $mock->shouldReceive('sync')->once()->andReturn(true); + }); + + $this->postJson( + route('webhooks.plugins', $plugin->webhook_secret), + ['action' => 'published', 'release' => ['id' => 101]], + ['X-GitHub-Event' => 'release'] + )->assertOk()->assertJson(['message' => 'Release sync queued']); + + Bus::assertDispatched(SyncPluginReleases::class, fn (SyncPluginReleases $job) => $job->plugin->is($plugin)); + } + + #[Test] + public function release_events_other_than_published_are_ignored(): void + { + Bus::fake([SyncPluginReleases::class]); + + $plugin = Plugin::factory()->create(['last_synced_at' => now()]); + + $this->mock(PluginSyncService::class, function ($mock) { + $mock->shouldNotReceive('sync'); + }); + + foreach (['created', 'released', 'prereleased', 'edited', 'deleted'] as $action) { + $this->postJson( + route('webhooks.plugins', $plugin->webhook_secret), + ['action' => $action, 'release' => ['id' => 101]], + ['X-GitHub-Event' => 'release'] + )->assertOk()->assertJson(['message' => 'Release event ignored']); + } + + Bus::assertNotDispatched(SyncPluginReleases::class); + } + + #[Test] + public function the_same_release_delivered_twice_is_only_synced_once(): void + { + Bus::fake([SyncPluginReleases::class]); + + $plugin = Plugin::factory()->create(['last_synced_at' => now()]); + + $this->mock(PluginSyncService::class, function ($mock) { + $mock->shouldReceive('sync')->once()->andReturn(true); + }); + + $deliver = fn () => $this->postJson( + route('webhooks.plugins', $plugin->webhook_secret), + ['action' => 'published', 'release' => ['id' => 101]], + ['X-GitHub-Event' => 'release'] + ); + + $deliver()->assertOk()->assertJson(['message' => 'Release sync queued']); + $deliver()->assertOk()->assertJson(['message' => 'Release event ignored']); + + Bus::assertDispatchedTimes(SyncPluginReleases::class, 1); + } + + #[Test] + public function release_payload_sent_as_form_data_is_read(): void + { + Bus::fake([SyncPluginReleases::class]); + + $plugin = Plugin::factory()->create(['last_synced_at' => now()]); + + $this->mock(PluginSyncService::class, function ($mock) { + $mock->shouldReceive('sync')->once()->andReturn(true); + }); + + $this->post( + route('webhooks.plugins', $plugin->webhook_secret), + ['payload' => json_encode(['action' => 'published', 'release' => ['id' => 101]])], + ['X-GitHub-Event' => 'release'] + )->assertOk()->assertJson(['message' => 'Release sync queued']); + + Bus::assertDispatchedTimes(SyncPluginReleases::class, 1); + } + #[Test] public function invalid_secret_returns_404(): void { diff --git a/tests/Feature/SatisSync/SatisBuildTokenTest.php b/tests/Feature/SatisSync/SatisBuildTokenTest.php new file mode 100644 index 000000000..b66cdcc3e --- /dev/null +++ b/tests/Feature/SatisSync/SatisBuildTokenTest.php @@ -0,0 +1,123 @@ + 'https://satis.test', + 'services.satis.api_key' => 'test-key', + 'services.github.token' => 'ghp_platform', + ]); + } + + public function test_the_build_gets_the_owners_token_when_github_accepts_it(): void + { + $plugin = $this->paidPluginFor(User::factory()->withGitHubApp()->create(['github_token' => encrypt('ghu_owner')])); + + Http::fake([ + 'api.github.com/repos/acme/camera-plugin' => Http::response(['full_name' => 'acme/camera-plugin']), + 'satis.test/*' => Http::response(['job_id' => 'test-123'], 202), + ]); + + (new SatisService)->buildForPlugin($plugin); + + $this->assertSatisBuiltWithToken('ghu_owner'); + } + + public function test_the_build_skips_an_owner_token_github_has_revoked(): void + { + $plugin = $this->paidPluginFor(User::factory()->withGitHubApp()->create(['github_token' => encrypt('ghu_revoked')])); + + Http::fake([ + 'api.github.com/repos/acme/camera-plugin' => Http::response(['message' => 'Bad credentials'], 401), + 'satis.test/*' => Http::response(['job_id' => 'test-123'], 202), + ]); + + (new SatisService)->buildForPlugin($plugin); + + $this->assertSatisBuiltWithToken('ghp_platform'); + } + + public function test_the_build_skips_an_installation_token_that_cannot_see_the_repository(): void + { + $user = User::factory()->withGitHubApp()->create(['github_token' => encrypt('ghu_owner')]); + $installation = GitHubInstallation::factory()->for($user)->create(['account_login' => 'acme']); + Cache::put("github_installation_token_{$installation->installation_id}", 'ghs_installation', now()->addMinutes(55)); + + $plugin = $this->paidPluginFor($user); + + Http::fake([ + 'api.github.com/repos/acme/camera-plugin' => fn (Request $request) => $request->hasHeader('Authorization', 'Bearer ghs_installation') + ? Http::response(['message' => 'Not Found'], 404) + : Http::response(['full_name' => 'acme/camera-plugin']), + 'satis.test/*' => Http::response(['job_id' => 'test-123'], 202), + ]); + + (new SatisService)->buildForPlugin($plugin); + + $this->assertSatisBuiltWithToken('ghu_owner'); + } + + public function test_the_build_keeps_the_owners_token_when_github_is_having_trouble(): void + { + $plugin = $this->paidPluginFor(User::factory()->withGitHubApp()->create(['github_token' => encrypt('ghu_owner')])); + + Http::fake([ + 'api.github.com/repos/acme/camera-plugin' => Http::response(['message' => 'Server Error'], 502), + 'satis.test/*' => Http::response(['job_id' => 'test-123'], 202), + ]); + + (new SatisService)->buildForPlugin($plugin); + + $this->assertSatisBuiltWithToken('ghu_owner'); + } + + public function test_a_release_sync_builds_with_the_token_it_fetched_releases_with(): void + { + $plugin = $this->paidPluginFor(User::factory()->withGitHubApp()->create(['github_token' => encrypt('ghu_revoked')])); + + Http::fake([ + 'api.github.com/repos/acme/camera-plugin' => Http::response(['message' => 'Bad credentials'], 401), + 'api.github.com/repos/acme/camera-plugin/releases*' => Http::response([]), + 'satis.test/*' => Http::response(['job_id' => 'test-123'], 202), + ]); + + (new SyncPluginReleases($plugin))->handle(new SatisService); + + Http::assertSent(fn (Request $request) => str_contains($request->url(), '/releases') + && $request->hasHeader('Authorization', 'Bearer ghp_platform')); + + $this->assertSatisBuiltWithToken('ghp_platform'); + } + + private function paidPluginFor(User $user): Plugin + { + return Plugin::factory()->paid()->approved()->for($user)->create([ + 'repository_url' => 'https://github.com/acme/camera-plugin', + ]); + } + + private function assertSatisBuiltWithToken(string $token): void + { + Http::assertSent(fn (Request $request) => $request->url() === 'https://satis.test/api/build' + && $request['github_token'] === $token); + } +}