diff --git a/AGENTS.md b/AGENTS.md index 6aaa0a8604..dbd3457a5f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -153,6 +153,7 @@ Because the "server" and the test share one PHP process, they share the phpunit - Apply authorization consistently across Livewire actions, API and web controllers, actions, downloads, exports, search, event listeners, and any other path that exposes or changes protected data. - Default to denying access when a policy or ownership relationship is missing or ambiguous. Members must not gain access to administrative, credential, security, billing, or instance-wide data merely because they belong to the team. - Add authorization regression tests for protected changes. Cover permitted access, member restrictions where applicable, and cross-team access; verify unauthorized reads and writes return `403` or otherwise reveal no protected data. +- Do not add a `TRUSTED_PROXIES` setting or change `TrustProxies` to use one. This caused problems in supported Coolify deployments. Fix IP-based rate limits and allow-lists at their call sites instead of changing proxy trust as a shortcut. ### Event Broadcasting - Laravel Reverb WebSocket server for real-time updates (port 6001) and a Node terminal WebSocket server (port 6002), both run inside the `coolify` container as s6 services diff --git a/app/Actions/Service/StartService.php b/app/Actions/Service/StartService.php index 3dc5c98b3f..fc5dc88bb8 100644 --- a/app/Actions/Service/StartService.php +++ b/app/Actions/Service/StartService.php @@ -48,7 +48,8 @@ class StartService $safeNetwork = escapeshellarg($service->destination->network); $serviceNames = data_get(Yaml::parse($compose), 'services', []); foreach ($serviceNames as $serviceName => $serviceConfig) { - $commands[] = "docker network connect --alias {$serviceName}-{$service->uuid} {$safeNetwork} {$serviceName}-{$service->uuid} >/dev/null 2>&1 || true"; + $containerName = escapeshellarg("{$serviceName}-{$service->uuid}"); + $commands[] = "docker network connect --alias {$containerName} {$safeNetwork} {$containerName} >/dev/null 2>&1 || true"; } } $commands = array_merge($commands, $this->logDrainNetworkConnectCommands($service)); diff --git a/app/Http/Controllers/Api/ApplicationsController.php b/app/Http/Controllers/Api/ApplicationsController.php index 02baa6c492..a23c8b2ed7 100644 --- a/app/Http/Controllers/Api/ApplicationsController.php +++ b/app/Http/Controllers/Api/ApplicationsController.php @@ -3653,7 +3653,7 @@ class ApplicationsController extends Controller 'is_preview' => ['type' => 'boolean', 'description' => 'The flag to indicate if the environment variable is used in preview deployments.'], 'is_literal' => ['type' => 'boolean', 'description' => 'The flag to indicate if the environment variable is a literal, nothing espaced.'], 'is_multiline' => ['type' => 'boolean', 'description' => 'The flag to indicate if the environment variable is multiline.'], - 'is_shown_once' => ['type' => 'boolean', 'description' => 'The flag to indicate if the environment variable\'s value is shown on the UI.'], + 'is_shown_once' => ['type' => 'boolean', 'description' => 'If true, the saved value is hidden in the UI and API responses. MCP never returns environment variable values.'], ], ), ), @@ -3871,7 +3871,7 @@ class ApplicationsController extends Controller 'is_preview' => ['type' => 'boolean', 'description' => 'The flag to indicate if the environment variable is used in preview deployments.'], 'is_literal' => ['type' => 'boolean', 'description' => 'The flag to indicate if the environment variable is a literal, nothing espaced.'], 'is_multiline' => ['type' => 'boolean', 'description' => 'The flag to indicate if the environment variable is multiline.'], - 'is_shown_once' => ['type' => 'boolean', 'description' => 'The flag to indicate if the environment variable\'s value is shown on the UI.'], + 'is_shown_once' => ['type' => 'boolean', 'description' => 'If true, the saved value is hidden in the UI and API responses. MCP never returns environment variable values.'], ], ), ], @@ -4092,7 +4092,7 @@ class ApplicationsController extends Controller 'is_preview' => ['type' => 'boolean', 'description' => 'The flag to indicate if the environment variable is used in preview deployments.'], 'is_literal' => ['type' => 'boolean', 'description' => 'The flag to indicate if the environment variable is a literal, nothing espaced.'], 'is_multiline' => ['type' => 'boolean', 'description' => 'The flag to indicate if the environment variable is multiline.'], - 'is_shown_once' => ['type' => 'boolean', 'description' => 'The flag to indicate if the environment variable\'s value is shown on the UI.'], + 'is_shown_once' => ['type' => 'boolean', 'description' => 'If true, the saved value is hidden in the UI and API responses. MCP never returns environment variable values.'], ], ), ), diff --git a/app/Http/Controllers/Api/DatabasesController.php b/app/Http/Controllers/Api/DatabasesController.php index b836876d81..8c3fd410aa 100644 --- a/app/Http/Controllers/Api/DatabasesController.php +++ b/app/Http/Controllers/Api/DatabasesController.php @@ -3475,6 +3475,10 @@ class DatabasesController extends Controller ]); } + if ($env->is_shown_once ?? false) { + $env->makeHidden(['value', 'real_value']); + } + return serializeApiResponse($env); } @@ -3580,7 +3584,7 @@ class DatabasesController extends Controller 'value' => ['type' => 'string', 'description' => 'The value of the environment variable.'], 'is_literal' => ['type' => 'boolean', 'description' => 'The flag to indicate if the environment variable is a literal, nothing espaced.'], 'is_multiline' => ['type' => 'boolean', 'description' => 'The flag to indicate if the environment variable is multiline.'], - 'is_shown_once' => ['type' => 'boolean', 'description' => 'The flag to indicate if the environment variable\'s value is shown on the UI.'], + 'is_shown_once' => ['type' => 'boolean', 'description' => 'If true, the saved value is hidden in the UI and API responses. MCP never returns environment variable values.'], ], ), ), @@ -3721,7 +3725,7 @@ class DatabasesController extends Controller 'value' => ['type' => 'string', 'description' => 'The value of the environment variable.'], 'is_literal' => ['type' => 'boolean', 'description' => 'The flag to indicate if the environment variable is a literal, nothing espaced.'], 'is_multiline' => ['type' => 'boolean', 'description' => 'The flag to indicate if the environment variable is multiline.'], - 'is_shown_once' => ['type' => 'boolean', 'description' => 'The flag to indicate if the environment variable\'s value is shown on the UI.'], + 'is_shown_once' => ['type' => 'boolean', 'description' => 'If true, the saved value is hidden in the UI and API responses. MCP never returns environment variable values.'], ], ), ], @@ -3852,7 +3856,7 @@ class DatabasesController extends Controller 'value' => ['type' => 'string', 'description' => 'The value of the environment variable.'], 'is_literal' => ['type' => 'boolean', 'description' => 'The flag to indicate if the environment variable is a literal, nothing espaced.'], 'is_multiline' => ['type' => 'boolean', 'description' => 'The flag to indicate if the environment variable is multiline.'], - 'is_shown_once' => ['type' => 'boolean', 'description' => 'The flag to indicate if the environment variable\'s value is shown on the UI.'], + 'is_shown_once' => ['type' => 'boolean', 'description' => 'If true, the saved value is hidden in the UI and API responses. MCP never returns environment variable values.'], ], ), ), diff --git a/app/Http/Controllers/Api/ServiceApplicationsController.php b/app/Http/Controllers/Api/ServiceApplicationsController.php index dda70c27eb..adb0cb678e 100644 --- a/app/Http/Controllers/Api/ServiceApplicationsController.php +++ b/app/Http/Controllers/Api/ServiceApplicationsController.php @@ -25,6 +25,7 @@ class ServiceApplicationsController extends Controller 'resourceable', 'resourceable_id', 'resourceable_type', + 'service', ]); $serialized = serializeApiResponse($serviceApplication); diff --git a/app/Http/Controllers/Api/ServicesController.php b/app/Http/Controllers/Api/ServicesController.php index a0859f0929..393f997b9b 100644 --- a/app/Http/Controllers/Api/ServicesController.php +++ b/app/Http/Controllers/Api/ServicesController.php @@ -1426,7 +1426,7 @@ class ServicesController extends Controller 'is_preview' => ['type' => 'boolean', 'description' => 'The flag to indicate if the environment variable is used in preview deployments.'], 'is_literal' => ['type' => 'boolean', 'description' => 'The flag to indicate if the environment variable is a literal, nothing espaced.'], 'is_multiline' => ['type' => 'boolean', 'description' => 'The flag to indicate if the environment variable is multiline.'], - 'is_shown_once' => ['type' => 'boolean', 'description' => 'The flag to indicate if the environment variable\'s value is shown on the UI.'], + 'is_shown_once' => ['type' => 'boolean', 'description' => 'If true, the saved value is hidden in the UI and API responses. MCP never returns environment variable values.'], ], ), ), @@ -1568,7 +1568,7 @@ class ServicesController extends Controller 'is_preview' => ['type' => 'boolean', 'description' => 'The flag to indicate if the environment variable is used in preview deployments.'], 'is_literal' => ['type' => 'boolean', 'description' => 'The flag to indicate if the environment variable is a literal, nothing espaced.'], 'is_multiline' => ['type' => 'boolean', 'description' => 'The flag to indicate if the environment variable is multiline.'], - 'is_shown_once' => ['type' => 'boolean', 'description' => 'The flag to indicate if the environment variable\'s value is shown on the UI.'], + 'is_shown_once' => ['type' => 'boolean', 'description' => 'If true, the saved value is hidden in the UI and API responses. MCP never returns environment variable values.'], ], ), ], @@ -1700,7 +1700,7 @@ class ServicesController extends Controller 'is_preview' => ['type' => 'boolean', 'description' => 'The flag to indicate if the environment variable is used in preview deployments.'], 'is_literal' => ['type' => 'boolean', 'description' => 'The flag to indicate if the environment variable is a literal, nothing espaced.'], 'is_multiline' => ['type' => 'boolean', 'description' => 'The flag to indicate if the environment variable is multiline.'], - 'is_shown_once' => ['type' => 'boolean', 'description' => 'The flag to indicate if the environment variable\'s value is shown on the UI.'], + 'is_shown_once' => ['type' => 'boolean', 'description' => 'If true, the saved value is hidden in the UI and API responses. MCP never returns environment variable values.'], ], ), ), diff --git a/app/Http/Controllers/Api/SharedEnvironmentVariablesController.php b/app/Http/Controllers/Api/SharedEnvironmentVariablesController.php index 3149c80b18..32145f4ec3 100644 --- a/app/Http/Controllers/Api/SharedEnvironmentVariablesController.php +++ b/app/Http/Controllers/Api/SharedEnvironmentVariablesController.php @@ -356,7 +356,7 @@ class SharedEnvironmentVariablesController extends Controller new OA\Property(property: 'value', type: 'string', nullable: true), new OA\Property(property: 'is_literal', type: 'boolean'), new OA\Property(property: 'is_multiline', type: 'boolean'), - new OA\Property(property: 'is_shown_once', type: 'boolean'), + new OA\Property(property: 'is_shown_once', type: 'boolean', description: 'If true, the saved value is hidden in the UI and API responses. MCP never returns environment variable values.'), new OA\Property(property: 'comment', type: 'string', nullable: true), ], ), diff --git a/app/Http/Controllers/Webhook/Bitbucket.php b/app/Http/Controllers/Webhook/Bitbucket.php index 03b253acc0..d4ec92cb21 100644 --- a/app/Http/Controllers/Webhook/Bitbucket.php +++ b/app/Http/Controllers/Webhook/Bitbucket.php @@ -74,10 +74,7 @@ class Bitbucket extends Controller } $applications = $this->manualWebhookApplications(Application::query()->where('git_branch', $branch), $full_name); if ($applications->isEmpty()) { - return response([ - 'status' => 'failed', - 'message' => "Nothing to do. No applications found with deploy key set, branch is '$branch' and Git Repository name has $full_name.", - ]); + return response([$this->unauthenticatedManualWebhookFailurePayload()]); } foreach ($applications as $application) { $webhook_secret = data_get($application, 'manual_webhook_secret_bitbucket'); @@ -278,7 +275,7 @@ class Bitbucket extends Controller } } - return response($return_payloads); + return $this->manualWebhookResponse($return_payloads); } catch (Exception $e) { return handleError($e); } diff --git a/app/Http/Controllers/Webhook/Concerns/MatchesManualWebhookApplications.php b/app/Http/Controllers/Webhook/Concerns/MatchesManualWebhookApplications.php index 65c92f1349..7aa2d6b48d 100644 --- a/app/Http/Controllers/Webhook/Concerns/MatchesManualWebhookApplications.php +++ b/app/Http/Controllers/Webhook/Concerns/MatchesManualWebhookApplications.php @@ -4,6 +4,7 @@ namespace App\Http\Controllers\Webhook\Concerns; use App\Models\Application; use Illuminate\Database\Eloquent\Builder; +use Illuminate\Http\Response; use Illuminate\Support\Collection; trait MatchesManualWebhookApplications @@ -61,6 +62,17 @@ trait MatchesManualWebhookApplications ]; } + protected function manualWebhookResponse(Collection $payloads): Response + { + $failure = $this->unauthenticatedManualWebhookFailurePayload(); + $authorizedPayloads = $payloads->reject(fn (array $payload): bool => $payload === $failure)->values(); + if ($authorizedPayloads->isEmpty() && $payloads->isNotEmpty()) { + return response([$failure]); + } + + return response($authorizedPayloads); + } + protected function canonicalManualWebhookRepository(?string $gitRepository): ?string { if (! is_string($gitRepository)) { diff --git a/app/Http/Controllers/Webhook/Gitea.php b/app/Http/Controllers/Webhook/Gitea.php index 9c40107217..97051d5beb 100644 --- a/app/Http/Controllers/Webhook/Gitea.php +++ b/app/Http/Controllers/Webhook/Gitea.php @@ -69,13 +69,13 @@ class Gitea extends Controller if ($x_gitea_event === 'push') { $applications = $this->manualWebhookApplications($applications->where('git_branch', $branch), $full_name); if ($applications->isEmpty()) { - return response("Nothing to do. No applications found with deploy key set, branch is '$branch' and Git Repository name has $full_name."); + return response([$this->unauthenticatedManualWebhookFailurePayload()]); } } if ($x_gitea_event === 'pull_request') { $applications = $this->manualWebhookApplications($applications->where('git_branch', $base_branch), $full_name); if ($applications->isEmpty()) { - return response("Nothing to do. No applications found with branch '$base_branch'."); + return response([$this->unauthenticatedManualWebhookFailurePayload()]); } } foreach ($applications as $application) { @@ -281,7 +281,7 @@ class Gitea extends Controller } } - return response($return_payloads); + return $this->manualWebhookResponse($return_payloads); } catch (Exception $e) { return handleError($e); } diff --git a/app/Http/Controllers/Webhook/Github.php b/app/Http/Controllers/Webhook/Github.php index 70c5eca19b..3272b27983 100644 --- a/app/Http/Controllers/Webhook/Github.php +++ b/app/Http/Controllers/Webhook/Github.php @@ -79,7 +79,7 @@ class Github extends Controller if ($x_github_event === 'push') { $applications = $this->manualWebhookApplications($applications->where('git_branch', $branch), $full_name); if ($applications->isEmpty()) { - return response("Nothing to do. No applications found with deploy key set, branch is '$branch' and Git Repository name has $full_name."); + return response([$this->unauthenticatedManualWebhookFailurePayload()]); } } if ($x_github_event === 'pull_request') { @@ -88,7 +88,7 @@ class Github extends Controller } $applications = $this->manualWebhookApplications($applications, $full_name); if ($applications->isEmpty()) { - return response("Nothing to do. No applications found for repo $full_name and branch '$base_branch'."); + return response([$this->unauthenticatedManualWebhookFailurePayload()]); } } $applicationsByServer = $applications->groupBy(function ($app) { @@ -239,7 +239,7 @@ class Github extends Controller } } - return response($return_payloads); + return $this->manualWebhookResponse($return_payloads); } catch (Exception $e) { return handleError($e); } diff --git a/app/Http/Controllers/Webhook/Gitlab.php b/app/Http/Controllers/Webhook/Gitlab.php index e86e51ab14..8ac1cab18a 100644 --- a/app/Http/Controllers/Webhook/Gitlab.php +++ b/app/Http/Controllers/Webhook/Gitlab.php @@ -416,10 +416,7 @@ class Gitlab extends Controller if ($x_gitlab_event === 'push') { $applications = $this->manualWebhookApplications($applications->where('git_branch', $branch), $full_name); if ($applications->isEmpty()) { - $return_payloads->push([ - 'status' => 'failed', - 'message' => "Nothing to do. No applications found with deploy key set, branch is '$branch' and Git Repository name has $full_name.", - ]); + $return_payloads->push($this->unauthenticatedManualWebhookFailurePayload()); return response($return_payloads); } @@ -427,10 +424,7 @@ class Gitlab extends Controller if ($x_gitlab_event === 'merge_request') { $applications = $this->manualWebhookApplications($applications->where('git_branch', $base_branch), $full_name); if ($applications->isEmpty()) { - $return_payloads->push([ - 'status' => 'failed', - 'message' => "Nothing to do. No applications found with branch '$base_branch'.", - ]); + $return_payloads->push($this->unauthenticatedManualWebhookFailurePayload()); return response($return_payloads); } @@ -641,7 +635,7 @@ class Gitlab extends Controller } } - return response($return_payloads); + return $this->manualWebhookResponse($return_payloads); } catch (Exception $e) { return handleError($e); } diff --git a/app/Livewire/Admin/Index.php b/app/Livewire/Admin/Index.php index f54f40ffd0..535ed2bf0c 100644 --- a/app/Livewire/Admin/Index.php +++ b/app/Livewire/Admin/Index.php @@ -30,8 +30,8 @@ class Index extends Component public function back() { $this->authorizeAdminAccess(); - if (session('impersonating')) { - session()->forget('impersonating'); + if (session('impersonator_id') === 0) { + session()->forget(['impersonator_id', 'impersonating']); $user = User::find(0); $team_to_switch_to = $user->resolveStoredTeam() ?? $user->teams->first(); Auth::login($user); @@ -54,7 +54,7 @@ class Index extends Component public function getSubscribers() { - if (Auth::id() !== 0 && ! session('impersonating')) { + if (Auth::id() !== 0 && session('impersonator_id') !== 0) { return redirect()->route('dashboard'); } $this->inactiveSubscribers = Team::whereRelation('subscription', 'stripe_invoice_paid', false)->count(); @@ -64,7 +64,7 @@ class Index extends Component public function switchUser(int $user_id) { $this->authorizeRootOnly(); - session(['impersonating' => true]); + session(['impersonator_id' => Auth::id(), 'impersonating' => true]); $user = User::find($user_id); if (! $user) { abort(404); @@ -78,7 +78,7 @@ class Index extends Component private function authorizeAdminAccess(): void { - if (! Auth::check() || (Auth::id() !== 0 && ! session('impersonating'))) { + if (! Auth::check() || (Auth::id() !== 0 && session('impersonator_id') !== 0)) { abort(403); } } diff --git a/app/Livewire/Boarding/Index.php b/app/Livewire/Boarding/Index.php index 4ea36b9b58..310d4219a1 100644 --- a/app/Livewire/Boarding/Index.php +++ b/app/Livewire/Boarding/Index.php @@ -308,11 +308,7 @@ class Index extends Component $this->privateKey = formatPrivateKey($this->privateKey); $foundServer = Server::whereIp($this->remoteServerHost)->first(); if ($foundServer) { - if ($foundServer->team_id === currentTeam()->id) { - return $this->dispatch('error', 'A server with this IP/Domain already exists in your team.'); - } - - return $this->dispatch('error', 'A server with this IP/Domain is already in use by another team.'); + return $this->dispatch('error', 'A server with this IP/Domain already exists.'); } $privateKeyId = $this->createdPrivateKey?->id ?? $this->selectedExistingPrivateKey; $this->createdPrivateKey = PrivateKey::ownedByCurrentTeam()->findOrFail($privateKeyId); diff --git a/app/Livewire/Server/New/ByIp.php b/app/Livewire/Server/New/ByIp.php index 1d1c0d1b8a..db0cc74d4b 100644 --- a/app/Livewire/Server/New/ByIp.php +++ b/app/Livewire/Server/New/ByIp.php @@ -143,11 +143,7 @@ class ByIp extends Component $this->authorize('create', Server::class); $foundServer = Server::whereIp($this->ip)->first(); if ($foundServer) { - if ($foundServer->team_id === currentTeam()->id) { - return $this->dispatch('error', 'A server with this IP/Domain already exists in your team.'); - } - - return $this->dispatch('error', 'A server with this IP/Domain is already in use by another team.'); + return $this->dispatch('error', 'A server with this IP/Domain already exists.'); } if (is_null($this->private_key_id)) { diff --git a/app/Livewire/Server/Show.php b/app/Livewire/Server/Show.php index a0365f8056..cd435b56b6 100644 --- a/app/Livewire/Server/Show.php +++ b/app/Livewire/Server/Show.php @@ -235,11 +235,7 @@ class Show extends Component ->first(); if ($foundServer) { $this->ip = $this->server->ip; - if ($foundServer->team_id === currentTeam()->id) { - throw new \Exception('A server with this IP/Domain already exists in your team.'); - } - - throw new \Exception('A server with this IP/Domain is already in use by another team.'); + throw new \Exception('A server with this IP/Domain already exists.'); } $this->server->name = $this->name; diff --git a/app/Livewire/Tags/Deployments.php b/app/Livewire/Tags/Deployments.php index e4afa5b606..d5c26e32c9 100644 --- a/app/Livewire/Tags/Deployments.php +++ b/app/Livewire/Tags/Deployments.php @@ -2,6 +2,7 @@ namespace App\Livewire\Tags; +use App\Models\Application; use App\Models\ApplicationDeploymentQueue; use Livewire\Component; @@ -19,7 +20,8 @@ class Deployments extends Component public function getDeployments() { try { - $this->deploymentsPerTagPerServer = ApplicationDeploymentQueue::whereIn('status', ['in_progress', 'queued'])->whereIn('application_id', $this->resourceIds)->get([ + $applicationIds = Application::ownedByCurrentTeam()->whereIn('id', $this->resourceIds)->pluck('id'); + $this->deploymentsPerTagPerServer = ApplicationDeploymentQueue::whereIn('status', ['in_progress', 'queued'])->whereIn('application_id', $applicationIds)->get([ 'id', 'application_id', 'application_name', diff --git a/app/Livewire/Team/AdminView.php b/app/Livewire/Team/AdminView.php index e609029754..a31a37d784 100644 --- a/app/Livewire/Team/AdminView.php +++ b/app/Livewire/Team/AdminView.php @@ -91,6 +91,8 @@ class AdminView extends Component public function render() { + abort_unless(isInstanceAdmin(), 403); + $search = trim($this->search); $teamId = currentTeam()->id; $users = User::query() diff --git a/app/Models/EnvironmentVariable.php b/app/Models/EnvironmentVariable.php index 176ffee98f..beef8c8f20 100644 --- a/app/Models/EnvironmentVariable.php +++ b/app/Models/EnvironmentVariable.php @@ -23,7 +23,7 @@ use OpenApi\Attributes as OA; 'is_runtime' => ['type' => 'boolean'], 'is_buildtime' => ['type' => 'boolean'], 'is_shared' => ['type' => 'boolean'], - 'is_shown_once' => ['type' => 'boolean'], + 'is_shown_once' => ['type' => 'boolean', 'description' => 'If true, the saved value is hidden in the UI and API responses. MCP never returns environment variable values.'], 'key' => ['type' => 'string'], 'value' => ['type' => 'string'], 'real_value' => ['type' => 'string'], diff --git a/app/Providers/RouteServiceProvider.php b/app/Providers/RouteServiceProvider.php index 79139c6b93..633216ea48 100644 --- a/app/Providers/RouteServiceProvider.php +++ b/app/Providers/RouteServiceProvider.php @@ -60,7 +60,14 @@ class RouteServiceProvider extends ServiceProvider }); RateLimiter::for('login', function (Request $request) { - return Limit::perMinute(5)->by((string) $request->email.'|'.auth_rate_limit_ip($request)); + $email = $request->input('email'); + $emailIdentity = normalize_email_identity(is_string($email) ? $email : null); + $limits = [Limit::perMinute(5)->by((is_string($email) ? $email : '').'|'.auth_rate_limit_ip($request))]; + if ($emailIdentity !== null) { + $limits[] = Limit::perMinute(5)->by('login:email-identity:'.sha1($emailIdentity)); + } + + return $limits; }); RateLimiter::for('two-factor', function (Request $request) { diff --git a/app/Services/Auth/OauthLoginService.php b/app/Services/Auth/OauthLoginService.php index 32524c7fa6..ebf6cc0a35 100644 --- a/app/Services/Auth/OauthLoginService.php +++ b/app/Services/Auth/OauthLoginService.php @@ -74,6 +74,10 @@ class OauthLoginService $providerUserId = (string) $providerUserId; $rawClaims = is_array($oauthUser->user ?? null) ? $oauthUser->user : []; + if ($provider === 'google' && filled($oauthSetting->tenant) && data_get($rawClaims, 'hd') !== $oauthSetting->tenant) { + throw new HttpException(403, 'Google account is not in the configured Workspace'); + } + $identityKey = [ 'provider' => $provider, 'issuer' => $provider, diff --git a/app/Services/ServerTransfer/ServerTransferImporter.php b/app/Services/ServerTransfer/ServerTransferImporter.php index 995555954b..e00943cf08 100644 --- a/app/Services/ServerTransfer/ServerTransferImporter.php +++ b/app/Services/ServerTransfer/ServerTransferImporter.php @@ -1079,6 +1079,22 @@ class ServerTransferImporter { foreach ($storages as $storage) { LocalFileVolume::withoutEvents(function () use ($storage, $resource) { + $isHostFile = (bool) data_get($storage, 'is_host_file', false); + $fsPath = data_get($storage, 'fs_path'); + if (! is_string($fsPath)) { + throw new RuntimeException('Invalid imported file storage path.'); + } + if ($isHostFile) { + $fsPath = validateHostFileMountPath($fsPath, 'imported host file source path'); + } + $chown = data_get($storage, 'chown'); + $chmod = data_get($storage, 'chmod'); + if (filled($chown) && (! is_string($chown) || ! preg_match('/\A(?:[A-Za-z_][A-Za-z0-9_.-]*|[0-9]+)(?::(?:[A-Za-z_][A-Za-z0-9_.-]*|[0-9]+))?\z/', $chown))) { + throw new RuntimeException('Invalid imported file owner.'); + } + if (filled($chmod) && (! is_string($chmod) || ! preg_match('/\A[0-7]{3,4}\z/', $chmod))) { + throw new RuntimeException('Invalid imported file mode.'); + } $uuid = filled(data_get($storage, 'uuid')) ? (string) data_get($storage, 'uuid') : new_public_id(); if (LocalFileVolume::where('uuid', $uuid)->exists()) { $uuid = new_public_id(); @@ -1087,13 +1103,13 @@ class ServerTransferImporter // uuid is not fillable and withoutEvents skips BaseModel's creating hook. $file = new LocalFileVolume; $file->forceFill([ - 'fs_path' => data_get($storage, 'fs_path'), + 'fs_path' => $fsPath, 'mount_path' => data_get($storage, 'mount_path'), 'content' => data_get($storage, 'content'), 'is_directory' => (bool) data_get($storage, 'is_directory', false), - 'is_host_file' => (bool) data_get($storage, 'is_host_file', false), - 'chown' => data_get($storage, 'chown'), - 'chmod' => data_get($storage, 'chmod'), + 'is_host_file' => $isHostFile, + 'chown' => $chown, + 'chmod' => $chmod, 'is_based_on_git' => (bool) data_get($storage, 'is_based_on_git', false), 'is_preview_suffix_enabled' => (bool) data_get($storage, 'is_preview_suffix_enabled', false), 'resource_type' => $resource->getMorphClass(), diff --git a/app/Services/ServerTransfer/ServerTransferMigrator.php b/app/Services/ServerTransfer/ServerTransferMigrator.php index f74dc8cc35..d65bd7da00 100644 --- a/app/Services/ServerTransfer/ServerTransferMigrator.php +++ b/app/Services/ServerTransfer/ServerTransferMigrator.php @@ -3,8 +3,10 @@ namespace App\Services\ServerTransfer; use App\Models\Server; +use App\Rules\SafeWebhookUrl; use Illuminate\Http\Client\ConnectionException; use Illuminate\Support\Facades\Http; +use Illuminate\Support\Facades\Validator; use RuntimeException; use Throwable; @@ -95,21 +97,35 @@ class ServerTransferMigrator { $targetUrl = rtrim(trim($targetUrl), '/'); if ($targetUrl === '' || ! filter_var($targetUrl, FILTER_VALIDATE_URL)) { - throw new RuntimeException('A valid target instance URL is required (e.g. http://localhost:8001).'); + throw new RuntimeException('A valid target instance URL is required (e.g. https://coolify.example.com).'); } - // From inside Docker, localhost is this container — use the host gateway for peer instances. - if (file_exists('/.dockerenv') || is_file('/run/.containerenv')) { - $targetUrl = (string) preg_replace( - '#^(https?://)(localhost|127\.0\.0\.1)(?=[:/]|$)#i', - '$1host.docker.internal', - $targetUrl - ); + if ($this->isLocalDevelopmentTarget($targetUrl)) { + if (file_exists('/.dockerenv') || is_file('/run/.containerenv')) { + return (string) preg_replace( + '#^(https?://)(localhost|127\.0\.0\.1)(?=[:/]|$)#i', + '$1host.docker.internal', + $targetUrl, + ); + } + + return $targetUrl; } + Validator::make(['target_url' => $targetUrl], [ + 'target_url' => ['required', new SafeWebhookUrl], + ])->validate(); + return $targetUrl; } + private function isLocalDevelopmentTarget(string $url): bool + { + return isDev() + && in_array(strtolower((string) parse_url($url, PHP_URL_SCHEME)), ['http', 'https'], true) + && in_array(strtolower((string) parse_url($url, PHP_URL_HOST)), ['localhost', '127.0.0.1', 'host.docker.internal'], true); + } + private function normalizeToken(string $targetToken): string { $token = trim($targetToken); @@ -140,6 +156,9 @@ class ServerTransferMigrator try { $response = Http::timeout(120) + ->withOptions($this->isLocalDevelopmentTarget($importUrl) + ? ['allow_redirects' => false] + : SafeWebhookUrl::httpClientOptions($importUrl)) ->acceptJson() ->withToken($token) ->asJson() diff --git a/openapi.json b/openapi.json index 964b50fe61..7cf0d40f39 100644 --- a/openapi.json +++ b/openapi.json @@ -3931,7 +3931,7 @@ }, "is_shown_once": { "type": "boolean", - "description": "The flag to indicate if the environment variable's value is shown on the UI." + "description": "If true, the saved value is hidden in the UI and API responses. MCP never returns environment variable values." } }, "type": "object" @@ -4023,7 +4023,7 @@ }, "is_shown_once": { "type": "boolean", - "description": "The flag to indicate if the environment variable's value is shown on the UI." + "description": "If true, the saved value is hidden in the UI and API responses. MCP never returns environment variable values." } }, "type": "object" @@ -4114,7 +4114,7 @@ }, "is_shown_once": { "type": "boolean", - "description": "The flag to indicate if the environment variable's value is shown on the UI." + "description": "If true, the saved value is hidden in the UI and API responses. MCP never returns environment variable values." } }, "type": "object" @@ -8770,7 +8770,7 @@ }, "is_shown_once": { "type": "boolean", - "description": "The flag to indicate if the environment variable's value is shown on the UI." + "description": "If true, the saved value is hidden in the UI and API responses. MCP never returns environment variable values." } }, "type": "object" @@ -8861,7 +8861,7 @@ }, "is_shown_once": { "type": "boolean", - "description": "The flag to indicate if the environment variable's value is shown on the UI." + "description": "If true, the saved value is hidden in the UI and API responses. MCP never returns environment variable values." } }, "type": "object" @@ -8951,7 +8951,7 @@ }, "is_shown_once": { "type": "boolean", - "description": "The flag to indicate if the environment variable's value is shown on the UI." + "description": "If true, the saved value is hidden in the UI and API responses. MCP never returns environment variable values." } }, "type": "object" @@ -19133,7 +19133,7 @@ }, "is_shown_once": { "type": "boolean", - "description": "The flag to indicate if the environment variable's value is shown on the UI." + "description": "If true, the saved value is hidden in the UI and API responses. MCP never returns environment variable values." } }, "type": "object" @@ -19228,7 +19228,7 @@ }, "is_shown_once": { "type": "boolean", - "description": "The flag to indicate if the environment variable's value is shown on the UI." + "description": "If true, the saved value is hidden in the UI and API responses. MCP never returns environment variable values." } }, "type": "object" @@ -19322,7 +19322,7 @@ }, "is_shown_once": { "type": "boolean", - "description": "The flag to indicate if the environment variable's value is shown on the UI." + "description": "If true, the saved value is hidden in the UI and API responses. MCP never returns environment variable values." } }, "type": "object" @@ -20379,7 +20379,8 @@ "type": "boolean" }, "is_shown_once": { - "type": "boolean" + "type": "boolean", + "description": "If true, the saved value is hidden in the UI and API responses. MCP never returns environment variable values." }, "comment": { "type": [ @@ -23092,7 +23093,8 @@ "type": "boolean" }, "is_shown_once": { - "type": "boolean" + "type": "boolean", + "description": "If true, the saved value is hidden in the UI and API responses. MCP never returns environment variable values." }, "key": { "type": "string" diff --git a/openapi.yaml b/openapi.yaml index b0843f9194..682b99e2e3 100644 --- a/openapi.yaml +++ b/openapi.yaml @@ -2578,7 +2578,7 @@ paths: description: 'The flag to indicate if the environment variable is multiline.' is_shown_once: type: boolean - description: "The flag to indicate if the environment variable's value is shown on the UI." + description: "If true, the saved value is hidden in the UI and API responses. MCP never returns environment variable values." type: object responses: '201': @@ -2639,7 +2639,7 @@ paths: description: 'The flag to indicate if the environment variable is multiline.' is_shown_once: type: boolean - description: "The flag to indicate if the environment variable's value is shown on the UI." + description: "If true, the saved value is hidden in the UI and API responses. MCP never returns environment variable values." type: object responses: '201': @@ -2683,7 +2683,7 @@ paths: properties: data: type: array - items: { properties: { key: { type: string, description: 'The key of the environment variable.' }, value: { type: string, description: 'The value of the environment variable.' }, is_preview: { type: boolean, description: 'The flag to indicate if the environment variable is used in preview deployments.' }, is_literal: { type: boolean, description: 'The flag to indicate if the environment variable is a literal, nothing espaced.' }, is_multiline: { type: boolean, description: 'The flag to indicate if the environment variable is multiline.' }, is_shown_once: { type: boolean, description: "The flag to indicate if the environment variable's value is shown on the UI." } }, type: object } + items: { properties: { key: { type: string, description: 'The key of the environment variable.' }, value: { type: string, description: 'The value of the environment variable.' }, is_preview: { type: boolean, description: 'The flag to indicate if the environment variable is used in preview deployments.' }, is_literal: { type: boolean, description: 'The flag to indicate if the environment variable is a literal, nothing espaced.' }, is_multiline: { type: boolean, description: 'The flag to indicate if the environment variable is multiline.' }, is_shown_once: { type: boolean, description: "If true, the saved value is hidden in the UI and API responses. MCP never returns environment variable values." } }, type: object } type: object responses: '201': @@ -5756,7 +5756,7 @@ paths: description: 'The flag to indicate if the environment variable is multiline.' is_shown_once: type: boolean - description: "The flag to indicate if the environment variable's value is shown on the UI." + description: "If true, the saved value is hidden in the UI and API responses. MCP never returns environment variable values." type: object responses: '201': @@ -5816,7 +5816,7 @@ paths: description: 'The flag to indicate if the environment variable is multiline.' is_shown_once: type: boolean - description: "The flag to indicate if the environment variable's value is shown on the UI." + description: "If true, the saved value is hidden in the UI and API responses. MCP never returns environment variable values." type: object responses: '201': @@ -5862,7 +5862,7 @@ paths: properties: data: type: array - items: { properties: { key: { type: string, description: 'The key of the environment variable.' }, value: { type: string, description: 'The value of the environment variable.' }, is_literal: { type: boolean, description: 'The flag to indicate if the environment variable is a literal, nothing espaced.' }, is_multiline: { type: boolean, description: 'The flag to indicate if the environment variable is multiline.' }, is_shown_once: { type: boolean, description: "The flag to indicate if the environment variable's value is shown on the UI." } }, type: object } + items: { properties: { key: { type: string, description: 'The key of the environment variable.' }, value: { type: string, description: 'The value of the environment variable.' }, is_literal: { type: boolean, description: 'The flag to indicate if the environment variable is a literal, nothing espaced.' }, is_multiline: { type: boolean, description: 'The flag to indicate if the environment variable is multiline.' }, is_shown_once: { type: boolean, description: "If true, the saved value is hidden in the UI and API responses. MCP never returns environment variable values." } }, type: object } type: object responses: '201': @@ -12098,7 +12098,7 @@ paths: description: 'The flag to indicate if the environment variable is multiline.' is_shown_once: type: boolean - description: "The flag to indicate if the environment variable's value is shown on the UI." + description: "If true, the saved value is hidden in the UI and API responses. MCP never returns environment variable values." type: object responses: '201': @@ -12161,7 +12161,7 @@ paths: description: 'The flag to indicate if the environment variable is multiline.' is_shown_once: type: boolean - description: "The flag to indicate if the environment variable's value is shown on the UI." + description: "If true, the saved value is hidden in the UI and API responses. MCP never returns environment variable values." type: object responses: '201': @@ -12207,7 +12207,7 @@ paths: properties: data: type: array - items: { properties: { key: { type: string, description: 'The key of the environment variable.' }, value: { type: string, description: 'The value of the environment variable.' }, is_preview: { type: boolean, description: 'The flag to indicate if the environment variable is used in preview deployments.' }, is_literal: { type: boolean, description: 'The flag to indicate if the environment variable is a literal, nothing espaced.' }, is_multiline: { type: boolean, description: 'The flag to indicate if the environment variable is multiline.' }, is_shown_once: { type: boolean, description: "The flag to indicate if the environment variable's value is shown on the UI." } }, type: object } + items: { properties: { key: { type: string, description: 'The key of the environment variable.' }, value: { type: string, description: 'The value of the environment variable.' }, is_preview: { type: boolean, description: 'The flag to indicate if the environment variable is used in preview deployments.' }, is_literal: { type: boolean, description: 'The flag to indicate if the environment variable is a literal, nothing espaced.' }, is_multiline: { type: boolean, description: 'The flag to indicate if the environment variable is multiline.' }, is_shown_once: { type: boolean, description: "If true, the saved value is hidden in the UI and API responses. MCP never returns environment variable values." } }, type: object } type: object responses: '201': @@ -12886,6 +12886,7 @@ paths: type: boolean is_shown_once: type: boolean + description: 'If true, the saved value is hidden in the UI and API responses. MCP never returns environment variable values.' comment: type: [string, 'null'] type: object @@ -14770,6 +14771,7 @@ components: type: boolean is_shown_once: type: boolean + description: 'If true, the saved value is hidden in the UI and API responses. MCP never returns environment variable values.' key: type: string value: diff --git a/routes/channels.php b/routes/channels.php index f75c30d0f1..8ab860660a 100644 --- a/routes/channels.php +++ b/routes/channels.php @@ -12,7 +12,6 @@ */ use App\Models\User; -use Illuminate\Support\Facades\Auth; use Illuminate\Support\Facades\Broadcast; Broadcast::channel('team.{teamId}', function (User $user, int $teamId) { @@ -23,10 +22,6 @@ Broadcast::channel('team.{teamId}', function (User $user, int $teamId) { return false; }); -Broadcast::channel('user.{userId}', function (User $user) { - if ($user->id === Auth::id()) { - return true; - } - - return false; +Broadcast::channel('user.{userId}', function (User $user, int $userId) { + return (int) $user->id === $userId; }); diff --git a/routes/webhooks.php b/routes/webhooks.php index 27796b8076..423b3cd713 100644 --- a/routes/webhooks.php +++ b/routes/webhooks.php @@ -7,20 +7,20 @@ use App\Http\Controllers\Webhook\Gitlab; use App\Http\Controllers\Webhook\Stripe; use Illuminate\Support\Facades\Route; -Route::middleware(['web', 'auth', 'throttle:30,1'])->group(function () { +Route::middleware(['web', 'auth', 'throttle:60,1'])->group(function () { Route::get('/source/github/redirect', [Github::class, 'redirect']); Route::get('/source/github/install', [Github::class, 'install']); Route::get('/source/gitlab/redirect', [Gitlab::class, 'redirect']); }); Route::post('/source/github/events', [Github::class, 'normal']); -Route::post('/source/github/events/manual', [Github::class, 'manual']); +Route::post('/source/github/events/manual', [Github::class, 'manual'])->middleware('throttle:60,1'); Route::post('/source/gitlab/events', [Gitlab::class, 'normal']); -Route::post('/source/gitlab/events/manual', [Gitlab::class, 'manual']); +Route::post('/source/gitlab/events/manual', [Gitlab::class, 'manual'])->middleware('throttle:60,1'); -Route::post('/source/bitbucket/events/manual', [Bitbucket::class, 'manual']); +Route::post('/source/bitbucket/events/manual', [Bitbucket::class, 'manual'])->middleware('throttle:60,1'); -Route::post('/source/gitea/events/manual', [Gitea::class, 'manual']); +Route::post('/source/gitea/events/manual', [Gitea::class, 'manual'])->middleware('throttle:60,1'); Route::post('/payments/stripe/events', [Stripe::class, 'events']); diff --git a/tests/Feature/AccessControlRegressionTest.php b/tests/Feature/AccessControlRegressionTest.php new file mode 100644 index 0000000000..b48bf537bb --- /dev/null +++ b/tests/Feature/AccessControlRegressionTest.php @@ -0,0 +1,84 @@ + 0]); +}); + +test('user broadcast channel accepts only its own user ID', function () { + $callback = Broadcast::getChannels()['user.{userId}']; + $user = User::factory()->create(); + + expect($callback($user, (int) $user->id))->toBeTrue() + ->and($callback($user, (int) $user->id + 1))->toBeFalse(); +}); + +test('team admin view checks instance admin access when it renders', function () { + $rootTeam = Team::factory()->create(['id' => 0]); + $rootUser = User::factory()->create(); + $rootTeam->members()->attach($rootUser->id, ['role' => 'owner']); + $this->actingAs($rootUser); + session(['currentTeam' => $rootTeam]); + + Livewire::test(AdminView::class)->assertOk(); + + $otherTeam = Team::factory()->create(); + $otherUser = User::factory()->create(); + $otherTeam->members()->attach($otherUser->id, ['role' => 'admin']); + $this->actingAs($otherUser); + session(['currentTeam' => $otherTeam]); + + expect(fn () => (new AdminView)->render())->toThrow(HttpException::class); +}); + +test('tag deployments include only applications in the current team', function () { + $team = Team::factory()->create(); + $user = User::factory()->create(); + $team->members()->attach($user->id, ['role' => 'owner']); + $this->actingAs($user); + session(['currentTeam' => $team]); + + $createDeployment = function (Team $owner, string $name): Application { + $server = Server::factory()->create(['team_id' => $owner->id]); + $project = Project::factory()->create(['team_id' => $owner->id]); + $environment = Environment::factory()->create(['project_id' => $project->id]); + $application = Application::factory()->create(['environment_id' => $environment->id]); + ApplicationDeploymentQueue::create([ + 'application_id' => $application->id, + 'application_name' => $name, + 'deployment_uuid' => fake()->uuid(), + 'server_id' => $server->id, + 'server_name' => $server->name, + 'status' => 'queued', + ]); + + return $application; + }; + + $owned = $createDeployment($team, 'Owned deployment'); + $foreign = $createDeployment(Team::factory()->create(), 'Foreign deployment'); + + $component = Livewire::test(TagDeployments::class) + ->set('resourceIds', [$owned->id, $foreign->id]) + ->call('getDeployments'); + $names = collect($component->get('deploymentsPerTagPerServer'))->flatten(1)->pluck('application_name'); + + expect($names->all())->toContain('Owned deployment') + ->not->toContain('Foreign deployment'); +}); diff --git a/tests/Feature/AdminAccessAuthorizationTest.php b/tests/Feature/AdminAccessAuthorizationTest.php index 478f381d5a..6816abd551 100644 --- a/tests/Feature/AdminAccessAuthorizationTest.php +++ b/tests/Feature/AdminAccessAuthorizationTest.php @@ -10,6 +10,10 @@ use Livewire\Livewire; uses(RefreshDatabase::class); +beforeEach(function () { + InstanceSettings::forceCreate(['id' => 0]); +}); + test('unauthenticated user cannot access admin route', function () { $response = $this->get('/admin'); @@ -33,7 +37,7 @@ test('root user can access admin page in cloud mode', function () { $rootTeam = Team::find(0) ?? Team::factory()->create(['id' => 0]); $rootUser = User::factory()->create(['id' => 0]); - $rootTeam->members()->attach($rootUser->id, ['role' => 'admin']); + $rootTeam->members()->syncWithoutDetaching([$rootUser->id => ['role' => 'admin']]); $this->actingAs($rootUser); session(['currentTeam' => ['id' => $rootTeam->id]]); @@ -48,7 +52,7 @@ test('root user gets 403 on admin page in self-hosted non-dev mode', function () $rootTeam = Team::find(0) ?? Team::factory()->create(['id' => 0]); $rootUser = User::factory()->create(['id' => 0]); - $rootTeam->members()->attach($rootUser->id, ['role' => 'admin']); + $rootTeam->members()->syncWithoutDetaching([$rootUser->id => ['role' => 'admin']]); $this->actingAs($rootUser); session(['currentTeam' => ['id' => $rootTeam->id]]); @@ -72,7 +76,6 @@ test('submitSearch requires admin authorization', function () { test('switchUser requires root user id 0', function () { config()->set('constants.coolify.self_hosted', false); - InstanceSettings::unguarded(fn () => InstanceSettings::query()->create(['id' => 0])); $rootUser = User::factory()->create(['id' => 0]); $rootTeam = Team::find(0); @@ -92,7 +95,6 @@ test('switchUser requires root user id 0', function () { test('back() redirects impersonator to admin index and clears session', function () { config()->set('constants.coolify.self_hosted', false); - InstanceSettings::unguarded(fn () => InstanceSettings::query()->create(['id' => 0])); $rootUser = User::factory()->create(['id' => 0]); $rootTeam = Team::find(0); @@ -100,6 +102,7 @@ test('back() redirects impersonator to admin index and clears session', function session([ 'currentTeam' => ['id' => $rootTeam->id], 'impersonating' => true, + 'impersonator_id' => 0, ]); Livewire::test(AdminIndex::class) @@ -107,12 +110,12 @@ test('back() redirects impersonator to admin index and clears session', function ->assertRedirect(route('admin.index')); expect(session('impersonating'))->toBeNull(); + expect(session('impersonator_id'))->toBeNull(); }); test('switchUser ignores Referer header and uses dashboard route', function () { config()->set('constants.coolify.self_hosted', false); - InstanceSettings::unguarded(fn () => InstanceSettings::query()->create(['id' => 0])); $rootUser = User::factory()->create(['id' => 0]); $rootTeam = Team::find(0); @@ -136,7 +139,7 @@ test('switchUser rejects non-root user', function () { $user = User::factory()->create(); $team->members()->attach($user->id, ['role' => 'admin']); - // Must set impersonating session to bypass mount() check + // A forged impersonating flag must not grant admin access. $this->actingAs($user); session([ 'currentTeam' => ['id' => $team->id], @@ -144,7 +147,6 @@ test('switchUser rejects non-root user', function () { ]); Livewire::test(AdminIndex::class) - ->call('switchUser', 999) ->assertForbidden(); }); diff --git a/tests/Feature/AdvisorySecurityRegressionTest.php b/tests/Feature/AdvisorySecurityRegressionTest.php new file mode 100644 index 0000000000..d35602ea4b --- /dev/null +++ b/tests/Feature/AdvisorySecurityRegressionTest.php @@ -0,0 +1,111 @@ + 'First.Name+tag@gmail.com'], [], [], ['REMOTE_ADDR' => '192.0.2.10']); + $second = Request::create('/login', 'POST', ['email' => 'firstname@gmail.com'], [], [], ['REMOTE_ADDR' => '198.51.100.10']); + + $firstLimits = $limiter($first); + $secondLimits = $limiter($second); + + expect($firstLimits)->toHaveCount(2) + ->and($firstLimits[0]->key)->not->toBe($secondLimits[0]->key) + ->and($firstLimits[1]->key)->toBe($secondLimits[1]->key); +}); + +it('restricts user broadcast channels to their own ID', function () { + $callback = Broadcast::getChannels()['user.{userId}']; + $user = User::factory()->create(); + + expect($callback($user, (int) $user->id))->toBeTrue() + ->and($callback($user, (int) $user->id + 1))->toBeFalse(); +}); + +it('does not require email verification for protected web routes', function () { + InstanceSettings::forceCreate(['id' => 0]); + $user = User::factory()->create(['email_verified_at' => null]); + Team::query()->update(['show_boarding' => false]); + Cache::flush(); + + $this->actingAs($user)->get('/analytics')->assertOk(); +}); + +it('does not apply the REST API allowlist to MCP and MCP switch routes', function () { + $mcp = Route::getRoutes()->match(Request::create('/mcp', 'POST')); + $enable = Route::getRoutes()->match(Request::create('/api/v1/mcp/enable', 'POST')); + $disable = Route::getRoutes()->match(Request::create('/api/v1/mcp/disable', 'POST')); + + expect($mcp)->not->toBeNull() + ->and($mcp->gatherMiddleware())->not->toContain(ApiAllowed::class) + ->and($enable->gatherMiddleware())->not->toContain(ApiAllowed::class) + ->and($disable->gatherMiddleware())->not->toContain(ApiAllowed::class); +}); + +it('throttles every manual webhook route', function (string $provider) { + $route = Route::getRoutes()->match(Request::create("/webhooks/source/{$provider}/events/manual", 'POST')); + + expect($route->gatherMiddleware())->toContain('throttle:60,1'); +})->with(['github', 'gitlab', 'bitbucket', 'gitea']); + +it('does not reveal how many applications share a manual webhook repository', function () { + $helper = new class + { + use MatchesManualWebhookApplications; + + public function reply(array $payloads): string + { + return $this->manualWebhookResponse(collect($payloads))->getContent(); + } + }; + $failure = ['status' => 'failed', 'message' => 'Invalid signature.']; + + expect($helper->reply([$failure, $failure]))->toBe($helper->reply([$failure])); + expect($helper->reply([$failure, ['status' => 'success', 'message' => 'queued']])) + ->not->toContain('Invalid signature.'); +}); + +it('never exposes a shown-once database variable even with sensitive read access', function () { + $variable = new EnvironmentVariable; + $variable->forceFill([ + 'key' => 'SECRET', + 'value' => 'secret-value', + 'is_shown_once' => true, + ]); + request()->attributes->set('can_read_sensitive', true); + + $method = new ReflectionMethod(DatabasesController::class, 'removeSensitiveEnvData'); + $result = $method->invoke(new DatabasesController, $variable); + + expect($result)->not->toHaveKey('value') + ->not->toHaveKey('real_value'); +}); + +it('does not include nested service and server details in a service application response', function () { + $application = new ServiceApplication; + $application->forceFill(['name' => 'app']); + $application->setRelation('service', (new Service)->forceFill(['name' => 'service'])); + $method = new ReflectionMethod(ServiceApplicationsController::class, 'removeSensitiveData'); + + $result = $method->invoke(new ServiceApplicationsController, $application); + + expect($result)->not->toHaveKey('service'); +}); diff --git a/tests/Feature/Api/ApiDataMaskingTest.php b/tests/Feature/Api/ApiDataMaskingTest.php new file mode 100644 index 0000000000..fbc8b3ddff --- /dev/null +++ b/tests/Feature/Api/ApiDataMaskingTest.php @@ -0,0 +1,50 @@ +forceFill([ + 'key' => 'SECRET', + 'value' => 'secret-value', + 'is_shown_once' => true, + ]); + request()->attributes->set('can_read_sensitive', true); + + $method = new ReflectionMethod(DatabasesController::class, 'removeSensitiveEnvData'); + $result = $method->invoke(new DatabasesController, $variable); + + expect($result)->not->toHaveKey('value') + ->not->toHaveKey('real_value'); +}); + +test('ordinary database variables remain visible with sensitive read access', function () { + $variable = new EnvironmentVariable; + $variable->forceFill([ + 'key' => 'SECRET', + 'value' => 'secret-value', + 'is_shown_once' => false, + ]); + request()->attributes->set('can_read_sensitive', true); + + $method = new ReflectionMethod(DatabasesController::class, 'removeSensitiveEnvData'); + $result = $method->invoke(new DatabasesController, $variable); + + expect($result)->toHaveKey('value'); +}); + +test('service application responses exclude their nested service', function () { + $application = new ServiceApplication; + $application->forceFill(['name' => 'app']); + $application->setRelation('service', (new Service)->forceFill(['name' => 'service'])); + $method = new ReflectionMethod(ServiceApplicationsController::class, 'removeSensitiveData'); + + $result = $method->invoke(new ServiceApplicationsController, $application); + + expect($result)->toHaveKey('name') + ->not->toHaveKey('service'); +}); diff --git a/tests/Feature/Auth/LoginRateLimitIPTest.php b/tests/Feature/Auth/LoginRateLimitIPTest.php index 9c2214c58a..77514a7760 100644 --- a/tests/Feature/Auth/LoginRateLimitIPTest.php +++ b/tests/Feature/Auth/LoginRateLimitIPTest.php @@ -69,7 +69,7 @@ test('successful login is still possible within rate limit', function () { expect($response->status())->not->toBe(429); }); -test('cloud login rate limits use the Cloudflare client ip', function () { +test('cloud login rate limits cannot be bypassed with a different Cloudflare client ip', function () { config()->set('constants.coolify.self_hosted', false); foreach (range(1, 5) as $attempt) { @@ -90,5 +90,5 @@ test('cloud login rate limits use the Cloudflare client ip', function () { 'email' => 'test@example.com', 'password' => 'wrong-password', ]) - ->assertRedirect(); + ->assertStatus(429); }); diff --git a/tests/Feature/EmailVerificationMiddlewareTest.php b/tests/Feature/EmailVerificationMiddlewareTest.php new file mode 100644 index 0000000000..7f5ce58d01 --- /dev/null +++ b/tests/Feature/EmailVerificationMiddlewareTest.php @@ -0,0 +1,24 @@ + 0]); + $user = User::factory()->create(['email_verified_at' => null]); + Team::query()->update(['show_boarding' => false]); + Cache::flush(); + + $this->actingAs($user)->get('/analytics')->assertOk(); +}); + +it('does not register an email verification notice route', function () { + expect(Route::has('verification.notice'))->toBeFalse() + ->and(Route::has('verify.verify'))->toBeTrue(); +}); diff --git a/tests/Feature/LoginAndMcpAccessTest.php b/tests/Feature/LoginAndMcpAccessTest.php new file mode 100644 index 0000000000..5d0325f291 --- /dev/null +++ b/tests/Feature/LoginAndMcpAccessTest.php @@ -0,0 +1,31 @@ + 'First.Name+tag@gmail.com'], [], [], ['REMOTE_ADDR' => '192.0.2.10']); + $second = Request::create('/login', 'POST', ['email' => 'firstname@gmail.com'], [], [], ['REMOTE_ADDR' => '198.51.100.10']); + + $firstLimits = $limiter($first); + $secondLimits = $limiter($second); + + expect($firstLimits)->toHaveCount(2) + ->and($firstLimits[0]->key)->not->toBe($secondLimits[0]->key) + ->and($firstLimits[1]->key)->toBe($secondLimits[1]->key); +}); + +it('keeps MCP independent from the REST API access check', function () { + $mcp = Route::getRoutes()->match(Request::create('/mcp', 'POST')); + $enable = Route::getRoutes()->match(Request::create('/api/v1/mcp/enable', 'POST')); + $disable = Route::getRoutes()->match(Request::create('/api/v1/mcp/disable', 'POST')); + + expect($mcp->gatherMiddleware())->not->toContain(ApiAllowed::class) + ->and($enable->gatherMiddleware())->not->toContain(ApiAllowed::class) + ->and($disable->gatherMiddleware())->not->toContain(ApiAllowed::class) + ->and($enable->gatherMiddleware())->toContain('auth:sanctum', 'api.token.team', 'api.ability:write') + ->and($disable->gatherMiddleware())->toContain('auth:sanctum', 'api.token.team', 'api.ability:write'); +}); diff --git a/tests/Feature/Mcp/McpEndpointTest.php b/tests/Feature/Mcp/McpEndpointTest.php index 450dd5023f..43f300a0fe 100644 --- a/tests/Feature/Mcp/McpEndpointTest.php +++ b/tests/Feature/Mcp/McpEndpointTest.php @@ -117,6 +117,17 @@ test('MCP endpoint rejects unauthenticated requests', function () { $response->assertStatus(401); }); +test('MCP endpoint works when the REST API is disabled and its IP allow-list excludes the client', function () { + InstanceSettings::query()->where('id', 0)->update(['is_api_enabled' => false, 'allowed_ips' => '192.0.2.10']); + Once::flush(); + $token = $this->user->createToken('mcp-read', ['read'])->plainTextToken; + + mcpListTools($token)->assertOk(); + test()->withHeader('Authorization', 'Bearer '.$token) + ->getJson('/api/v1/version') + ->assertForbidden(); +}); + test('MCP endpoint lists tools for an authenticated token', function () { $token = $this->user->createToken('mcp-read', ['read'])->plainTextToken; diff --git a/tests/Feature/Mcp/McpReadToolsTest.php b/tests/Feature/Mcp/McpReadToolsTest.php index 6e146e7190..21df936c94 100644 --- a/tests/Feature/Mcp/McpReadToolsTest.php +++ b/tests/Feature/Mcp/McpReadToolsTest.php @@ -556,6 +556,7 @@ test('list_env_keys never returns values and is team scoped', function () { 'resourceable_type' => Application::class, 'resourceable_id' => $this->application->id, 'is_preview' => false, + 'is_shown_once' => true, ]); $response = mcpReadCall('list_env_keys', [ @@ -567,6 +568,7 @@ test('list_env_keys never returns values and is team scoped', function () { $raw = json_encode($body); expect(collect($body['data']['keys'])->pluck('key'))->toContain('DATABASE_URL'); + expect(collect($body['data']['keys'])->firstWhere('key', 'DATABASE_URL')['is_shown_once'])->toBeTrue(); expect($raw)->not->toContain('postgres://secret'); expect($raw)->not->toContain('"value"'); expect($raw)->not->toContain('real_value'); @@ -1187,6 +1189,7 @@ test('list_shared_env_keys returns names without values and is team scoped', fun 'type' => 'project', 'team_id' => $this->team->id, 'project_id' => $this->project->id, + 'is_shown_once' => true, ]); $response = mcpReadCall('list_shared_env_keys', [ @@ -1197,6 +1200,7 @@ test('list_shared_env_keys returns names without values and is team scoped', fun $body = mcpReadJson($response); $raw = json_encode($body); expect(collect($body['data']['keys'])->pluck('key'))->toContain('SHARED_API_URL'); + expect(collect($body['data']['keys'])->firstWhere('key', 'SHARED_API_URL')['is_shown_once'])->toBeTrue(); expect($raw)->not->toContain('secret.example.com'); expect($raw)->not->toContain('"value"'); diff --git a/tests/Feature/Mcp/McpToggleApiTest.php b/tests/Feature/Mcp/McpToggleApiTest.php index 68d5d335a6..0de306cd01 100644 --- a/tests/Feature/Mcp/McpToggleApiTest.php +++ b/tests/Feature/Mcp/McpToggleApiTest.php @@ -20,6 +20,8 @@ beforeEach(function () { $this->team = Team::factory()->create(); $this->user = User::factory()->create(); $this->team->members()->attach($this->user->id, ['role' => 'owner']); + $rootTeam = Team::factory()->create(['id' => 0]); + $rootTeam->members()->attach($this->user->id, ['role' => 'owner']); session(['currentTeam' => $this->team]); }); @@ -56,7 +58,7 @@ test('POST /api/v1/mcp/enable enables MCP server with root token', function () { }); test('POST /api/v1/mcp/disable disables MCP server with root token', function () { - InstanceSettings::query()->where('id', 0)->update(['is_mcp_server_enabled' => true]); + InstanceSettings::query()->where('id', 0)->update(['is_mcp_server_enabled' => true, 'is_api_enabled' => false]); $token = makeRootMcpToken($this->user); $response = test()->withHeaders([ @@ -91,6 +93,17 @@ test('non-root token cannot disable MCP server', function () { expect(InstanceSettings::find(0)->is_mcp_server_enabled)->toBeTrue(); }); +test('root token can enable MCP server when the REST API is disabled', function () { + InstanceSettings::query()->where('id', 0)->update(['is_api_enabled' => false, 'allowed_ips' => '192.0.2.10']); + $token = makeRootMcpToken($this->user); + + test()->withHeaders(['Authorization' => 'Bearer '.$token]) + ->postJson('/api/v1/mcp/enable') + ->assertOk(); + + expect(InstanceSettings::find(0)->is_mcp_server_enabled)->toBeTrue(); +}); + test('unauthenticated request to /api/v1/mcp/enable returns 401', function () { $response = test()->postJson('/api/v1/mcp/enable'); $response->assertStatus(401); diff --git a/tests/Feature/OauthControllerTest.php b/tests/Feature/OauthControllerTest.php index dee651cc2f..b1a64369de 100644 --- a/tests/Feature/OauthControllerTest.php +++ b/tests/Feature/OauthControllerTest.php @@ -49,7 +49,7 @@ it('logs in an existing user when the oauth provider returns a mixed-case email' 'email' => 'UserName@example.edu', 'name' => 'Example User', 'id' => 'google-user-id', - 'user' => ['verified_email' => true], + 'user' => ['verified_email' => true, 'hd' => 'example.com'], ]); Socialite::shouldReceive('driver')->once()->with('google')->andReturn($provider); @@ -86,7 +86,7 @@ it('never moves an existing oauth identity when the provider email changes', fun 'email' => 'new@example.com', 'name' => 'Example User', 'id' => 'google-user-id', - 'user' => ['verified_email' => true], + 'user' => ['verified_email' => true, 'hd' => 'example.com'], ]); Socialite::shouldReceive('driver')->once()->with('google')->andReturn($provider); @@ -154,7 +154,7 @@ it('continues oauth login when another request creates the identity first', func 'email' => 'race@example.com', 'name' => 'Race User', 'id' => 'google-race-id', - 'user' => ['verified_email' => true], + 'user' => ['verified_email' => true, 'hd' => 'example.com'], ], OauthSetting::where('provider', 'google')->firstOrFail()); } finally { Event::forget($eventName); @@ -212,10 +212,11 @@ it('registers a new user from a verified provider identity', function () { 'email' => 'verified@example.com', 'name' => 'Verified User', 'id' => 'verified-google-id', - 'user' => ['verified_email' => true], + 'user' => ['verified_email' => true, 'hd' => 'example.com'], ], OauthSetting::where('provider', 'google')->firstOrFail()); expect($user->email)->toBe('verified@example.com'); + expect($user->email_verified_at)->toBeNull(); $this->assertAuthenticatedAs($user); $this->assertDatabaseHas('oauth_identities', [ 'user_id' => $user->id, @@ -238,7 +239,7 @@ it('does not link another provider identity to an account by shared email', func 'email' => 'shared@example.com', 'name' => 'Other Provider User', 'id' => 'google-user-id', - 'user' => ['verified_email' => true], + 'user' => ['verified_email' => true, 'hd' => 'example.com'], ], OauthSetting::where('provider', 'google')->firstOrFail()))->toThrow(HttpException::class); $this->assertGuest(); @@ -267,7 +268,7 @@ it('sends an OAuth user with confirmed two factor authentication to the Fortify 'email' => $user->email, 'name' => $user->name, 'id' => 'two-factor-google-id', - 'user' => ['verified_email' => true], + 'user' => ['verified_email' => true, 'hd' => 'example.com'], ]); Socialite::shouldReceive('driver')->once()->with('google')->andReturn($provider); @@ -295,7 +296,7 @@ it('completes OAuth login without a challenge when two factor authentication is 'email' => $user->email, 'name' => $user->name, 'id' => 'plain-google-id', - 'user' => ['verified_email' => true], + 'user' => ['verified_email' => true, 'hd' => 'example.com'], ]); Socialite::shouldReceive('driver')->once()->with('google')->andReturn($provider); @@ -381,3 +382,18 @@ it('rejects oauth logins when the provider does not return a valid user id', fun 'false id' => [false], 'float id' => [1.0], ]); + +it('rejects a Google account outside the configured Workspace even when its email is verified', function () { + $user = User::factory()->create(['email' => 'user@outside.example']); + $setting = OauthSetting::where('provider', 'google')->firstOrFail(); + + expect(fn () => app(OauthLoginService::class)->login('google', (object) [ + 'email' => $user->email, + 'name' => 'Outside User', + 'id' => 'google-outside-id', + 'user' => ['verified_email' => true, 'hd' => 'outside.example'], + ], $setting))->toThrow(HttpException::class); + + expect(OauthIdentity::count())->toBe(0); + $this->assertGuest(); +}); diff --git a/tests/Feature/OauthRegistrationPolicyTest.php b/tests/Feature/OauthRegistrationPolicyTest.php index 86186cca8c..09cede4586 100644 --- a/tests/Feature/OauthRegistrationPolicyTest.php +++ b/tests/Feature/OauthRegistrationPolicyTest.php @@ -48,5 +48,6 @@ it('allows password registration when no oauth provider is enabled', function () 'password_confirmation' => 'password', ]); - expect($user->email)->toBe('password@example.com'); + expect($user->email)->toBe('password@example.com') + ->and($user->email_verified_at)->toBeNull(); }); diff --git a/tests/Feature/OidcOauthControllerTest.php b/tests/Feature/OidcOauthControllerTest.php index 3812d78b18..970ce321a1 100644 --- a/tests/Feature/OidcOauthControllerTest.php +++ b/tests/Feature/OidcOauthControllerTest.php @@ -221,6 +221,7 @@ it('creates the root user when oidc provisions the first account', function () { $response->assertRedirect('/'); $this->assertDatabaseHas('users', ['id' => 0, 'email' => 'root@example.com']); + expect(User::whereEmail('root@example.com')->firstOrFail()->email_verified_at)->toBeNull(); $this->assertDatabaseHas('team_user', ['team_id' => 0, 'user_id' => 0, 'role' => 'owner']); expect(InstanceSettings::find(0)->is_registration_enabled)->toBeFalse(); }); @@ -291,3 +292,15 @@ it('logs callback failures with diagnostic context', function () { && $context['exception'] instanceof RuntimeException; }); }); + +it('does not mark a newly provisioned oidc account verified without a verified email claim', function () { + User::factory()->create(['email' => 'existing@example.com']); + OauthSetting::where('provider', 'oidc')->update(['allow_registration' => true, 'require_email_verified' => false]); + + fakeOidcProvider(['email' => 'unverified@example.com', 'email_verified' => false]); + + $this->get(route('auth.callback', 'oidc'))->assertRedirect('/'); + + $user = User::whereEmail('unverified@example.com')->firstOrFail(); + expect($user->email_verified_at)->toBeNull(); +}); diff --git a/tests/Feature/ServerUsernameValidationTest.php b/tests/Feature/ServerUsernameValidationTest.php index 4213ee21d3..d3b4343740 100644 --- a/tests/Feature/ServerUsernameValidationTest.php +++ b/tests/Feature/ServerUsernameValidationTest.php @@ -180,3 +180,17 @@ it('rejects unsafe SSH usernames during onboarding server validation', function ], ]); }); + +it('does not disclose another team through the duplicate server IP message', function () { + $this->actingAs($this->user); + $foreignTeam = Team::factory()->create(); + Server::factory()->create(['team_id' => $this->team->id, 'ip' => '192.0.2.40']); + Server::factory()->create(['team_id' => $foreignTeam->id, 'ip' => '192.0.2.41']); + + foreach (['192.0.2.40', '192.0.2.41'] as $ip) { + Livewire::test(ByIp::class, ['private_keys' => collect([$this->privateKey])]) + ->set('ip', $ip) + ->call('submit') + ->assertDispatched('error', 'A server with this IP/Domain already exists.'); + } +}); diff --git a/tests/Feature/Webhook/WebhookHmacTest.php b/tests/Feature/Webhook/WebhookHmacTest.php index dfd7990185..29c8814ded 100644 --- a/tests/Feature/Webhook/WebhookHmacTest.php +++ b/tests/Feature/Webhook/WebhookHmacTest.php @@ -7,10 +7,18 @@ use App\Models\Project; use App\Models\Server; use App\Models\Team; use Illuminate\Foundation\Testing\RefreshDatabase; +use Illuminate\Http\Request; use Illuminate\Support\Facades\DB; +use Illuminate\Support\Facades\Route; uses(RefreshDatabase::class); +test('manual webhook routes are rate limited', function (string $provider) { + $route = Route::getRoutes()->match(Request::create("/webhooks/source/{$provider}/events/manual", 'POST')); + + expect($route->gatherMiddleware())->toContain('throttle:60,1'); +})->with(['github', 'gitlab', 'bitbucket', 'gitea']); + function createApplicationWithWebhook(string $repo = 'test-org/test-repo', string $branch = 'main', array $overrides = []): Application { $team = Team::factory()->create(); @@ -387,7 +395,7 @@ describe('Manual Webhook Repository Matching', function () { $response->assertOk(); $content = $response->getContent(); - expect($content)->toContain('No applications found') + expect($content)->toContain('Invalid signature.') ->not->toContain('secret-github-app') ->not->toContain($app->uuid); }); @@ -481,7 +489,7 @@ describe('Manual Webhook Repository Matching', function () { $response->assertOk(); $content = $response->getContent(); - expect($content)->toContain('No applications found') + expect($content)->toContain('Invalid signature.') ->not->toContain("secret-{$provider}-app") ->not->toContain($app->uuid); })->with([ diff --git a/tests/Unit/ServerTransfer/ServerTransferExporterImporterTest.php b/tests/Unit/ServerTransfer/ServerTransferExporterImporterTest.php index 5f940108c3..e77ea16ad7 100644 --- a/tests/Unit/ServerTransfer/ServerTransferExporterImporterTest.php +++ b/tests/Unit/ServerTransfer/ServerTransferExporterImporterTest.php @@ -861,3 +861,33 @@ test('system-wide gitlab apps are not exported and re-link on import by uuid', f ->and($importedGlTeam->is_system_wide)->toBeFalse() ->and(GitlabApp::where('is_system_wide', true)->where('uuid', 'system-gitlab-public')->count())->toBe(1); }); + +test('import rejects unsafe host file paths before saving storage', function () { + $importFileStorages = new ReflectionMethod(ServerTransferImporter::class, 'importFileStorages'); + $storage = [ + 'fs_path' => '/etc/../passwd', + 'mount_path' => '/app/passwd', + 'is_host_file' => true, + ]; + + expect(fn () => $importFileStorages->invoke($this->importer, [$storage], $this->application)) + ->toThrow(Exception::class); + expect(LocalFileVolume::count())->toBe(0); +}); + +test('import rejects shell-like file ownership and mode metadata', function (string $field, string $value) { + $importFileStorages = new ReflectionMethod(ServerTransferImporter::class, 'importFileStorages'); + $storage = [ + 'fs_path' => './config.json', + 'mount_path' => '/app/config.json', + $field => $value, + ]; + + expect(fn () => $importFileStorages->invoke($this->importer, [$storage], $this->application)) + ->toThrow(RuntimeException::class); + expect(LocalFileVolume::count())->toBe(0); +})->with([ + 'owner command' => ['chown', '0:0; id'], + 'mode command' => ['chmod', '600; id'], + 'option mode' => ['chmod', '--reference=/etc/passwd'], +]); diff --git a/tests/Unit/ServerTransfer/ServerTransferMigratorTest.php b/tests/Unit/ServerTransfer/ServerTransferMigratorTest.php index 292a2aec73..15244fea02 100644 --- a/tests/Unit/ServerTransfer/ServerTransferMigratorTest.php +++ b/tests/Unit/ServerTransfer/ServerTransferMigratorTest.php @@ -12,6 +12,7 @@ use App\Services\ServerTransfer\ServerTransferClaimer; use App\Services\ServerTransfer\ServerTransferMigrator; use Illuminate\Foundation\Testing\RefreshDatabase; use Illuminate\Support\Facades\Http; +use Illuminate\Validation\ValidationException; use Tests\TestCase; uses(TestCase::class, RefreshDatabase::class); @@ -42,7 +43,7 @@ beforeEach(function () { test('migrate exports imports via http and completes locally', function () { Http::fake([ - 'http://target.test/api/v1/servers/import' => Http::response([ + 'http://8.8.8.8/api/v1/servers/import' => Http::response([ 'dry_run' => false, 'server_uuid' => $this->server->uuid, 'claimed' => true, @@ -54,13 +55,13 @@ test('migrate exports imports via http and completes locally', function () { $result = app(ServerTransferMigrator::class)->migrate( server: $this->server, - targetUrl: 'http://target.test', + targetUrl: 'http://8.8.8.8', targetToken: 'target-token-xyz', writeRemote: false, ); expect($result['server_uuid'])->toBe($this->server->uuid) - ->and($result['target_url'])->toBe('http://target.test') + ->and($result['target_url'])->toBe('http://8.8.8.8') ->and($result['import']['claimed'])->toBeTrue() ->and($result['message'])->toContain('migrated'); @@ -69,62 +70,82 @@ test('migrate exports imports via http and completes locally', function () { ->and((bool) $this->server->settings->force_disabled)->toBeTrue(); Http::assertSent(function ($request) { - return $request->url() === 'http://target.test/api/v1/servers/import' + return $request->url() === 'http://8.8.8.8/api/v1/servers/import' && $request->hasHeader('Authorization', 'Bearer target-token-xyz') && data_get($request->data(), 'claim') === true && data_get($request->data(), 'bundle.server.uuid') === $this->server->uuid; }); }); -test('migrate rewrites localhost target when running in docker style env', function () { - // Simulate container: create a temp marker if missing is hard; instead assert host rewrite helper via migrate call - // with Http fake matching host.docker.internal when /.dockerenv exists — skip if not in docker. - if (! file_exists('/.dockerenv') && ! is_file('/run/.containerenv')) { - expect(true)->toBeTrue(); +test('migrate rejects a private target before sending the bundle', function () { + config()->set('app.env', 'production'); + Http::fake(); - return; - } + expect(fn () => app(ServerTransferMigrator::class)->migrate( + $this->server, + 'http://127.0.0.1:8001', + 'token', + ))->toThrow(ValidationException::class); - Http::fake([ - 'http://host.docker.internal:8001/api/v1/servers/import' => Http::response([ - 'dry_run' => false, - 'server_uuid' => $this->server->uuid, - 'claimed' => true, - 'warnings' => [], - ], 201), - ]); + Http::assertNothingSent(); +}); + +test('migrate reaches a local peer instance in development', function () { + config()->set('app.env', 'local'); + Http::fake(fn () => Http::response([ + 'dry_run' => false, + 'server_uuid' => $this->server->uuid, + 'claimed' => true, + 'warnings' => [], + ], 201)); app(ServerTransferMigrator::class)->migrate( $this->server, 'http://localhost:8001', - 'token', + 'target-token', ); - Http::assertSent(fn ($request) => str_contains($request->url(), 'host.docker.internal:8001')); + $targetHost = file_exists('/.dockerenv') || is_file('/run/.containerenv') + ? 'host.docker.internal' + : 'localhost'; + Http::assertSent(fn ($request) => $request->url() === "http://{$targetHost}:8001/api/v1/servers/import"); +}); + +test('migrate still rejects other private targets in development', function () { + config()->set('app.env', 'local'); + Http::fake(); + + expect(fn () => app(ServerTransferMigrator::class)->migrate( + $this->server, + 'http://10.0.0.5:8001', + 'token', + ))->toThrow(ValidationException::class); + + Http::assertNothingSent(); }); test('migrate fails clearly when target is unreachable', function () { Http::fake([ - 'http://down.test/*' => Http::failedConnection(), + 'http://8.8.4.4/*' => Http::failedConnection(), ]); expect(fn () => app(ServerTransferMigrator::class)->migrate( $this->server, - 'http://down.test', + 'http://8.8.4.4', 'token', ))->toThrow(RuntimeException::class, 'Could not reach target'); }); test('migrate fails when target returns error', function () { Http::fake([ - 'http://target.test/api/v1/servers/import' => Http::response([ + 'http://8.8.8.8/api/v1/servers/import' => Http::response([ 'message' => 'A server with IP/domain already exists', ], 422), ]); expect(fn () => app(ServerTransferMigrator::class)->migrate( $this->server, - 'http://target.test', + 'http://8.8.8.8', 'token', ))->toThrow(RuntimeException::class, 'Target import failed'); @@ -136,7 +157,7 @@ test('migrate fails when target returns error', function () { test('migrate surfaces recovery guidance when complete fails after successful remote import', function () { Http::fake([ - 'http://target.test/api/v1/servers/import' => Http::response([ + 'http://8.8.8.8/api/v1/servers/import' => Http::response([ 'dry_run' => false, 'server_uuid' => $this->server->uuid, 'claimed' => true, @@ -152,7 +173,7 @@ test('migrate surfaces recovery guidance when complete fails after successful re expect(fn () => app(ServerTransferMigrator::class)->migrate( $this->server, - 'http://target.test', + 'http://8.8.8.8', 'token', ))->toThrow(RuntimeException::class, 'Retry complete'); });