rosslh/coolify
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix(sources): prevent 500 when deleting GitLab/GitHub apps

Browse source 
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.
This commit is contained in:
Andras Bacsai 2026-08-01 18:34:38 +02:00
parent f06bacc8e0
commit 5b370713c3
9 changed files with 150 additions and 41 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
app/Livewire/Source/Github/Change.php
View file

@ -478,6 +478,9 @@ public function delete()
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) {

3
app/Livewire/Source/Gitlab/Change.php
View file

@ -334,6 +334,9 @@ public function delete()
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) {

10
app/Policies/GithubAppPolicy.php
View file

@ -44,6 +44,11 @@ public function update(User $user, GithubApp $githubApp): bool
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 @@ public function delete(User $user, GithubApp $githubApp): bool
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);
}

10
app/Policies/GitlabAppPolicy.php
View file

@ -44,6 +44,11 @@ public function update(User $user, GitlabApp $gitlabApp): bool
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 @@ public function delete(User $user, GitlabApp $gitlabApp): bool
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);
}

4
resources/views/livewire/source/github/change.blade.php
View file

@ -15,6 +15,7 @@
<x-forms.button canGate="view" :canResource="$github_app"
wire:click.prevent="testConnection">Test Connection</x-forms.button>
@endif
@if ($github_app)
@can('delete', $github_app)
@if ($applications->count() > 0)
<x-modal-confirmation title="Confirm GitHub App Deletion?" isErrorButton buttonTitle="Delete"
@ -31,6 +32,7 @@
step2ButtonText="Permanently Delete" />
@endif
@endcan
@endif
</div>
</div>
<div class="subtitle">Your Private GitHub App for private repositories.</div>
@ -232,6 +234,7 @@ class="bg-transparent border-transparent hover:bg-transparent hover:border-trans
<div class="flex flex-col sm:flex-row sm:items-center gap-2 pb-4">
<h1>GitHub App</h1>
<div class="flex gap-2">
@if ($github_app)
@can('delete', $github_app)
<x-modal-confirmation title="Confirm GitHub App Deletion?" isErrorButton buttonTitle="Delete"
submitAction="delete" :actions="['The selected GitHub App will be permanently deleted.']" confirmationText="{{ data_get($github_app, 'name') }}"
@ -239,6 +242,7 @@ class="bg-transparent border-transparent hover:bg-transparent hover:border-trans
shortConfirmationLabel="GitHub App Name" :confirmWithPassword="false"
step2ButtonText="Permanently Delete" />
@endcan
@endif
</div>
</div>
<div class="flex items-center justify-center min-h-[calc(100vh-12rem)]">

4
resources/views/livewire/source/gitlab/change.blade.php
View file

@ -9,6 +9,7 @@
<div class="flex gap-2">
<x-forms.button canGate="update" :canResource="$gitlab_app" type="submit">Save</x-forms.button>
<x-forms.button wire:click.prevent="testConnection">Test Connection</x-forms.button>
@if ($gitlab_app)
@can('delete', $gitlab_app)
<x-modal-confirmation title="Confirm GitLab App Deletion?" isErrorButton buttonTitle="Delete"
submitAction="delete" :actions="['The selected GitLab App will be permanently deleted.']"
@ -17,6 +18,7 @@
shortConfirmationLabel="GitLab App Name" :confirmWithPassword="false"
step2ButtonText="Permanently Delete" />
@endcan
@endif
</div>
</div>
<div class="subtitle">Your GitLab App for private repositories.</div>
@ -120,6 +122,7 @@ class="flex items-center justify-between w-full px-1 py-2 text-left select-none
<div class="flex flex-col sm:flex-row sm:items-center gap-2">
<h1>GitLab App</h1>
<div class="flex gap-2">
@if ($gitlab_app)
@can('delete', $gitlab_app)
<x-modal-confirmation title="Confirm GitLab App Deletion?" isErrorButton buttonTitle="Delete"
submitAction="delete" :actions="['The selected GitLab App will be permanently deleted.']"
@ -128,6 +131,7 @@ class="flex items-center justify-between w-full px-1 py-2 text-left select-none
shortConfirmationLabel="GitLab App Name" :confirmWithPassword="false"
step2ButtonText="Permanently Delete" />
@endcan
@endif
</div>
</div>
<div class="subtitle">Connect your GitLab instance to deploy private repositories.</div>

35
tests/Feature/GitlabAppAuthorizationTest.php
View file

@ -144,4 +144,39 @@
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();
});
});

22
tests/Unit/Policies/GithubAppPolicyTest.php
View file

@ -144,6 +144,26 @@
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 @@
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;

22
tests/Unit/Policies/GitlabAppPolicyTest.php
View file

@ -144,6 +144,26 @@
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 @@
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;