From 5b370713c3ac3a61d9a3fce1f29e03c049b85041 Mon Sep 17 00:00:00 2001
From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com>
Date: Sat, 1 Aug 2026 18:34:38 +0200
Subject: [PATCH] fix(sources): prevent 500 when deleting GitLab/GitHub apps
After delete, Livewire still re-renders the source change view (modal
$refresh / morph). Policy @can checks then call isAdminOfTeam() with a
null team_id and throw a TypeError (HTTP 500) before the redirect.
Guard null team_id in GitlabAppPolicy and GithubAppPolicy, clear the
Livewire model after delete, and skip @can when the model is gone.
---
app/Livewire/Source/Github/Change.php | 3 ++
app/Livewire/Source/Gitlab/Change.php | 3 ++
app/Policies/GithubAppPolicy.php | 10 ++++
app/Policies/GitlabAppPolicy.php | 10 ++++
.../livewire/source/github/change.blade.php | 50 ++++++++++---------
.../livewire/source/gitlab/change.blade.php | 36 +++++++------
tests/Feature/GitlabAppAuthorizationTest.php | 35 +++++++++++++
tests/Unit/Policies/GithubAppPolicyTest.php | 22 +++++++-
tests/Unit/Policies/GitlabAppPolicyTest.php | 22 +++++++-
9 files changed, 150 insertions(+), 41 deletions(-)
diff --git a/app/Livewire/Source/Github/Change.php b/app/Livewire/Source/Github/Change.php
index 1876b8c49..07755ec05 100644
--- a/app/Livewire/Source/Github/Change.php
+++ b/app/Livewire/Source/Github/Change.php
@@ -478,6 +478,9 @@ class Change extends Component
return;
}
$this->github_app->delete();
+ // Clear so post-delete Livewire re-render / modal $refresh does not re-run
+ // @can and canGate checks against a deleted model (null team_id TypeError).
+ $this->github_app = null;
return redirect()->route('source.all');
} catch (\Throwable $e) {
diff --git a/app/Livewire/Source/Gitlab/Change.php b/app/Livewire/Source/Gitlab/Change.php
index f22f1845d..dd0284582 100644
--- a/app/Livewire/Source/Gitlab/Change.php
+++ b/app/Livewire/Source/Gitlab/Change.php
@@ -334,6 +334,9 @@ class Change extends Component
return;
}
$this->gitlab_app->delete();
+ // Clear so post-delete Livewire re-render / modal $refresh does not re-run
+ // @can and canGate checks against a deleted model (null team_id TypeError).
+ $this->gitlab_app = null;
return redirect()->route('source.all');
} catch (\Throwable $e) {
diff --git a/app/Policies/GithubAppPolicy.php b/app/Policies/GithubAppPolicy.php
index 79dd79838..d152ab1d1 100644
--- a/app/Policies/GithubAppPolicy.php
+++ b/app/Policies/GithubAppPolicy.php
@@ -44,6 +44,11 @@ class GithubAppPolicy
return $user->canAccessSystemResources();
}
+ // Guard null team_id (e.g. post-delete Livewire re-render of @can checks).
+ if ($githubApp->team_id === null) {
+ return false;
+ }
+
return $user->isAdminOfTeam($githubApp->team_id);
}
@@ -56,6 +61,11 @@ class GithubAppPolicy
return $user->canAccessSystemResources();
}
+ // Guard null team_id (e.g. post-delete Livewire re-render of @can checks).
+ if ($githubApp->team_id === null) {
+ return false;
+ }
+
return $user->isAdminOfTeam($githubApp->team_id);
}
diff --git a/app/Policies/GitlabAppPolicy.php b/app/Policies/GitlabAppPolicy.php
index 56861e89b..8a9d773d6 100644
--- a/app/Policies/GitlabAppPolicy.php
+++ b/app/Policies/GitlabAppPolicy.php
@@ -44,6 +44,11 @@ class GitlabAppPolicy
return $user->canAccessSystemResources();
}
+ // Guard null team_id (e.g. post-delete Livewire re-render of @can checks).
+ if ($gitlabApp->team_id === null) {
+ return false;
+ }
+
return $user->isAdminOfTeam($gitlabApp->team_id);
}
@@ -56,6 +61,11 @@ class GitlabAppPolicy
return $user->canAccessSystemResources();
}
+ // Guard null team_id (e.g. post-delete Livewire re-render of @can checks).
+ if ($gitlabApp->team_id === null) {
+ return false;
+ }
+
return $user->isAdminOfTeam($gitlabApp->team_id);
}
diff --git a/resources/views/livewire/source/github/change.blade.php b/resources/views/livewire/source/github/change.blade.php
index 3b0ac9388..795c1b1fc 100644
--- a/resources/views/livewire/source/github/change.blade.php
+++ b/resources/views/livewire/source/github/change.blade.php
@@ -15,22 +15,24 @@
Test Connection
@endif
- @can('delete', $github_app)
- @if ($applications->count() > 0)
-
- @else
-
- @endif
- @endcan
+ @if ($github_app)
+ @can('delete', $github_app)
+ @if ($applications->count() > 0)
+
+ @else
+
+ @endif
+ @endcan
+ @endif
Your Private GitHub App for private repositories.
@@ -232,13 +234,15 @@
GitHub App
- @can('delete', $github_app)
-
- @endcan
+ @if ($github_app)
+ @can('delete', $github_app)
+
+ @endcan
+ @endif
diff --git a/resources/views/livewire/source/gitlab/change.blade.php b/resources/views/livewire/source/gitlab/change.blade.php
index b743fa0d7..011c342b3 100644
--- a/resources/views/livewire/source/gitlab/change.blade.php
+++ b/resources/views/livewire/source/gitlab/change.blade.php
@@ -9,14 +9,16 @@
Save
Test Connection
- @can('delete', $gitlab_app)
-
- @endcan
+ @if ($gitlab_app)
+ @can('delete', $gitlab_app)
+
+ @endcan
+ @endif
Your GitLab App for private repositories.
@@ -120,14 +122,16 @@
GitLab App
- @can('delete', $gitlab_app)
-
- @endcan
+ @if ($gitlab_app)
+ @can('delete', $gitlab_app)
+
+ @endcan
+ @endif
Connect your GitLab instance to deploy private repositories.
diff --git a/tests/Feature/GitlabAppAuthorizationTest.php b/tests/Feature/GitlabAppAuthorizationTest.php
index 4ff591120..860ef41c0 100644
--- a/tests/Feature/GitlabAppAuthorizationTest.php
+++ b/tests/Feature/GitlabAppAuthorizationTest.php
@@ -144,4 +144,39 @@ describe('GitLab App authorization', function () {
expect(Application::count())->toBe($applicationsBefore);
});
+
+ test('team owner can delete a gitlab app without type error', function () {
+ $this->actingAs($this->owner);
+ session(['currentTeam' => $this->team]);
+
+ $gitlabAppId = $this->gitlabApp->id;
+
+ Livewire::withQueryParams(['gitlab_app_uuid' => $this->gitlabApp->uuid])
+ ->test(Change::class)
+ ->call('delete')
+ ->assertRedirect(route('source.all'));
+
+ expect(GitlabApp::find($gitlabAppId))->toBeNull();
+ });
+
+ test('delete policy is safe when team_id becomes null after model is removed', function () {
+ $this->actingAs($this->owner);
+ session(['currentTeam' => $this->team]);
+
+ // Reproduce the post-delete Livewire re-render path: @can('delete') runs while
+ // the in-memory model may have a null team_id (TypeError in isAdminOfTeam).
+ $orphaned = new GitlabApp([
+ 'name' => 'Orphaned',
+ 'api_url' => 'https://gitlab.example.com/api/v4',
+ 'html_url' => 'https://gitlab.example.com',
+ 'is_system_wide' => false,
+ 'team_id' => null,
+ ]);
+
+ expect(fn () => $this->owner->can('delete', $orphaned))->not->toThrow(TypeError::class);
+ expect($this->owner->can('delete', $orphaned))->toBeFalse();
+ expect(fn () => $this->owner->can('update', $orphaned))->not->toThrow(TypeError::class);
+ expect($this->owner->can('update', $orphaned))->toBeFalse();
+ });
+
});
diff --git a/tests/Unit/Policies/GithubAppPolicyTest.php b/tests/Unit/Policies/GithubAppPolicyTest.php
index 5aedbf7b8..3e6eb173c 100644
--- a/tests/Unit/Policies/GithubAppPolicyTest.php
+++ b/tests/Unit/Policies/GithubAppPolicyTest.php
@@ -144,6 +144,26 @@ it('denies team member to delete non-system-wide github app', function () {
expect($policy->delete($user, $model))->toBeFalse();
});
+it('denies update when team_id is null without type error', function () {
+ $user = Mockery::mock(User::class)->makePartial();
+ $user->shouldNotReceive('isAdminOfTeam');
+
+ $model = mockGithubApp(teamId: null, isSystemWide: false);
+
+ $policy = new GithubAppPolicy;
+ expect($policy->update($user, $model))->toBeFalse();
+});
+
+it('denies delete when team_id is null without type error', function () {
+ $user = Mockery::mock(User::class)->makePartial();
+ $user->shouldNotReceive('isAdminOfTeam');
+
+ $model = mockGithubApp(teamId: null, isSystemWide: false);
+
+ $policy = new GithubAppPolicy;
+ expect($policy->delete($user, $model))->toBeFalse();
+});
+
it('denies restore of github app', function () {
$user = Mockery::mock(User::class)->makePartial();
@@ -162,7 +182,7 @@ it('denies force delete of github app', function () {
expect($policy->forceDelete($user, $model))->toBeFalse();
});
-function mockGithubApp(int $teamId, bool $isSystemWide): GithubApp
+function mockGithubApp(?int $teamId, bool $isSystemWide): GithubApp
{
$githubApp = Mockery::mock(GithubApp::class)->makePartial();
$githubApp->team_id = $teamId;
diff --git a/tests/Unit/Policies/GitlabAppPolicyTest.php b/tests/Unit/Policies/GitlabAppPolicyTest.php
index a2cd2a091..f0a67718a 100644
--- a/tests/Unit/Policies/GitlabAppPolicyTest.php
+++ b/tests/Unit/Policies/GitlabAppPolicyTest.php
@@ -144,6 +144,26 @@ it('denies team member to delete non-system-wide gitlab app', function () {
expect($policy->delete($user, $model))->toBeFalse();
});
+it('denies update when team_id is null without type error', function () {
+ $user = Mockery::mock(User::class)->makePartial();
+ $user->shouldNotReceive('isAdminOfTeam');
+
+ $model = mockGitlabApp(teamId: null, isSystemWide: false);
+
+ $policy = new GitlabAppPolicy;
+ expect($policy->update($user, $model))->toBeFalse();
+});
+
+it('denies delete when team_id is null without type error', function () {
+ $user = Mockery::mock(User::class)->makePartial();
+ $user->shouldNotReceive('isAdminOfTeam');
+
+ $model = mockGitlabApp(teamId: null, isSystemWide: false);
+
+ $policy = new GitlabAppPolicy;
+ expect($policy->delete($user, $model))->toBeFalse();
+});
+
it('denies restore of gitlab app', function () {
$user = Mockery::mock(User::class)->makePartial();
@@ -162,7 +182,7 @@ it('denies force delete of gitlab app', function () {
expect($policy->forceDelete($user, $model))->toBeFalse();
});
-function mockGitlabApp(int $teamId, bool $isSystemWide): GitlabApp
+function mockGitlabApp(?int $teamId, bool $isSystemWide): GitlabApp
{
$gitlabApp = Mockery::mock(GitlabApp::class)->makePartial();
$gitlabApp->team_id = $teamId;