feat(mcp): add instance-level MCP server toggle via API and UI#9862
Merged
andrasbacsai merged 8 commits intonextfrom May 5, 2026
Merged
feat(mcp): add instance-level MCP server toggle via API and UI#9862andrasbacsai merged 8 commits intonextfrom
andrasbacsai merged 8 commits intonextfrom
Conversation
Add Model Context Protocol server exposing Coolify infrastructure data to AI assistants. Includes tools for listing/fetching servers, projects, applications, databases, and services, scoped to authenticated team tokens. - Add CoolifyServer with 10 read-only tools (list/get for all resource types) - Add BuildsResponse and ResolvesTeam traits for shared tool logic - Add EnsureMcpEnabled middleware guarding /mcp routes - Add enable/disable MCP API endpoints (root-only) - Add is_mcp_server_enabled toggle in instance settings and advanced UI - Add migration for is_mcp_server_enabled column - Add feature tests for MCP endpoints and toggle API - Scrub sensitive keys (passwords, tokens, raw IDs) from all responses
…ests Replace 8× repeated per-type if-blocks in `queryDatabaseByUuidWithinTeam` and `queryResourcesByUuid` with a single loop over the new `STANDALONE_DATABASE_MODELS` constant. Add unit tests to guard the registry against drift (keys mirror `DATABASE_TYPES`, every entry is a valid Eloquent model with `team()`), and feature tests covering team-ownership, wrong-team, and unknown-UUID cases for `queryDatabaseByUuidWithinTeam`.
…ases() Add missing DB types to StandaloneDocker::databases() concat chain and cover all 8 standalone database types with feature tests.
Member
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
WalkthroughThis pull request introduces Model Context Protocol (MCP) server support to the Coolify platform, enabling AI-powered infrastructure management. New endpoints at ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 PHPStan (2.1.51)PHPStan was skipped because the sandbox runner could not parse its output. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/Mcp/Tools/GetInfrastructureOverview.php`:
- Around line 48-73: Replace the per-project relation count queries with an
eager count on the initial query: change the Project::where(...)->get() call to
include withCount(['applications','services','databases']) so each Project has
applications_count, services_count and databases_count; then inside the foreach
use those properties (e.g. $project->applications_count) for $apps, $services
and $databases, update the aggregations ($appCount, $serviceCount,
$databaseCount) to add those values, and use them when building
$projectSummaries['counts'] so no per-project relationship count queries are
executed in the loop.
In `@app/Mcp/Tools/ListApplications.php`:
- Around line 33-39: Validate the incoming tag before using it in the query:
ensure $tagName from $request->get('tag') is a non-empty string and
reject/return a tool error for any non-string (array/object/null) values, then
only call when($tagName, ...) to apply the whereHas filter; update the code
around $tagName and the Application::ownedByCurrentTeamAPI(...)->when(...) block
in ListApplications.php to perform the type check and early error response
rather than passing arbitrary payloads into whereHas.
In `@app/Mcp/Tools/ListDatabases.php`:
- Around line 35-53: The code is triggering N+1 queries by iterating $projects
and calling $project->databases(), so change the query to eager-load the
relation (use Project::where('team_id', $teamId)->with('databases')->get()),
then replace the manual loop with a single flatten/flatMap over the loaded
relation (e.g. $databases = $projects->flatMap(fn($p) => $p->databases) or
$projects->pluck('databases')->flatten(1)) and continue using
$databases->sortBy(...)->slice(...)->map(...) as before; ensure you reference
the loaded relation property (->databases) not the relation method
(->databases()) and update ListDatabases.php accordingly.
In `@app/Mcp/Tools/ListServices.php`:
- Around line 7-57: The current ListServices::handle builds the full $services
collection by iterating Project::where(...)->get() and merging each
$project->services(), causing N+1 queries and in-memory sorting/paging; instead
query services directly with a single Eloquent query joined/filtered by
Project.team_id (e.g. Service::whereHas('project', fn($q)=>$q->where('team_id',
$teamId)) or join to projects), apply orderBy('name') and use skip(take) (or
->offset()->limit() / paginate) to fetch only the page, get the total via a
count() query on the same filter, then map the paged results to the existing
summary structure and pass to respond/paginationMeta (keep using paginationArgs,
paginationMeta, respond).
In `@openapi.json`:
- Around line 8653-8756: Change the two MCP endpoints from GET to POST
semantics: in openapi.json replace the "get" objects under paths "/mcp/enable"
(operationId "enable-mcp") and "/mcp/disable" (operationId "disable-mcp") with
"post" (keeping the same responses and schemas), and in routes/api.php change
Route::get('/mcp/enable', ...) and Route::get('/mcp/disable', ...) to
Route::post('/mcp/enable', ...) and Route::post('/mcp/disable', ...); ensure any
middleware/auth usage remains unchanged and adjust any client calls/tests to use
POST.
In `@openapi.yaml`:
- Around line 5487-5508: The inline flow-style YAML mappings in the /mcp/enable
response schemas (and the corresponding /mcp/disable block) use brace-style
objects that fail YAML linting; update the response schema mappings under the
'200' and '403' responses for the /mcp/enable and /mcp/disable endpoints to use
block-style YAML mappings (expand properties and example into standard block
form instead of { type: string, example: '...' }) so each property (message,
type, example) is written as proper YAML key: value pairs; locate the response
blocks by operationId "enable-mcp" (and the disable-mcp equivalent) and rewrite
the schema mappings accordingly.
In `@routes/api.php`:
- Around line 36-39: These routes currently use GET but perform state changes;
update the route definitions to use a non-safe method (POST or PATCH) for the
endpoints that toggle state: change Route::get('/enable',
[OtherController::class, 'enable_api']), Route::get('/disable',
[OtherController::class, 'disable_api']), Route::get('/mcp/enable',
[OtherController::class, 'enable_mcp']), and Route::get('/mcp/disable',
[OtherController::class, 'disable_mcp']) to Route::post or Route::patch as
appropriate, and ensure any callers/forms/clients and CSRF middleware are
updated to send the correct HTTP verb when invoking OtherController::enable_api,
::disable_api, ::enable_mcp, and ::disable_mcp.
In `@tests/Feature/Mcp/McpEndpointTest.php`:
- Around line 11-24: The test suite can leak memoized InstanceSettings because
Once::flush() is only called in the disabled-state test; move the Once::flush()
call into the shared beforeEach setup so every test starts with a clean Once
cache. Update the beforeEach closure (the setup that currently resets
InstanceSettings, creates Team/User and attaches member) to call Once::flush()
at the start (or end) so tests that rely on InstanceSettings or Once memoization
(including the disabled-state test) cannot affect later examples.
In `@tests/Feature/StandaloneDockerDatabasesTest.php`:
- Around line 28-36: The helper attachDb lacks parameter and return type hints;
update its signature to declare the destination and environment parameter types
(e.g., Destination $destination, Environment $environment) and an explicit
return type of Illuminate\Database\Eloquent\Model so fixture type errors fail
fast; also add the necessary use imports (Destination, Environment, and
Illuminate\Database\Eloquent\Model) at the top of the file and keep the body
unchanged except for the typed signature.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 23cc380a-29f7-4201-b69d-410291e4b11b
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (33)
app/Http/Controllers/Api/OtherController.phpapp/Http/Kernel.phpapp/Http/Middleware/EnsureMcpEnabled.phpapp/Livewire/Settings/Advanced.phpapp/Mcp/Concerns/BuildsResponse.phpapp/Mcp/Concerns/ResolvesTeam.phpapp/Mcp/Servers/CoolifyServer.phpapp/Mcp/Tools/GetApplication.phpapp/Mcp/Tools/GetDatabase.phpapp/Mcp/Tools/GetInfrastructureOverview.phpapp/Mcp/Tools/GetServer.phpapp/Mcp/Tools/GetService.phpapp/Mcp/Tools/ListApplications.phpapp/Mcp/Tools/ListDatabases.phpapp/Mcp/Tools/ListProjects.phpapp/Mcp/Tools/ListServers.phpapp/Mcp/Tools/ListServices.phpapp/Models/InstanceSettings.phpapp/Models/StandaloneDocker.phpbootstrap/helpers/constants.phpbootstrap/helpers/shared.phpcomposer.jsondatabase/migrations/2026_04_22_183029_add_is_mcp_server_enabled_to_instance_settings_table.phpopenapi.jsonopenapi.yamlresources/views/livewire/settings/advanced.blade.phproutes/ai.phproutes/api.phptests/Feature/Mcp/McpEndpointTest.phptests/Feature/Mcp/McpToggleApiTest.phptests/Feature/QueryDatabaseByUuidWithinTeamTest.phptests/Feature/StandaloneDockerDatabasesTest.phptests/Unit/StandaloneDatabaseRegistryTest.php
Comment on lines
+48
to
+73
| $projects = Project::where('team_id', $teamId)->get(); | ||
|
|
||
| $appCount = 0; | ||
| $serviceCount = 0; | ||
| $databaseCount = 0; | ||
| $projectSummaries = []; | ||
|
|
||
| foreach ($projects as $project) { | ||
| $apps = $project->applications()->count(); | ||
| $services = $project->services()->count(); | ||
| $databases = $project->databases()->count(); | ||
|
|
||
| $appCount += $apps; | ||
| $serviceCount += $services; | ||
| $databaseCount += $databases; | ||
|
|
||
| $projectSummaries[] = [ | ||
| 'uuid' => $project->uuid, | ||
| 'name' => $project->name, | ||
| 'counts' => [ | ||
| 'applications' => $apps, | ||
| 'services' => $services, | ||
| 'databases' => $databases, | ||
| ], | ||
| ]; | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Pull the per-project counts into the initial query.
Right now this does three relation-count queries per project. withCount() keeps the overview cheap and avoids turning a self-hosted server into a query treadmill.
♻️ Suggested refactor
- $projects = Project::where('team_id', $teamId)->get();
+ $projects = Project::where('team_id', $teamId)
+ ->withCount(['applications', 'services', 'databases'])
+ ->get();
@@
foreach ($projects as $project) {
- $apps = $project->applications()->count();
- $services = $project->services()->count();
- $databases = $project->databases()->count();
+ $apps = $project->applications_count;
+ $services = $project->services_count;
+ $databases = $project->databases_count;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| $projects = Project::where('team_id', $teamId)->get(); | |
| $appCount = 0; | |
| $serviceCount = 0; | |
| $databaseCount = 0; | |
| $projectSummaries = []; | |
| foreach ($projects as $project) { | |
| $apps = $project->applications()->count(); | |
| $services = $project->services()->count(); | |
| $databases = $project->databases()->count(); | |
| $appCount += $apps; | |
| $serviceCount += $services; | |
| $databaseCount += $databases; | |
| $projectSummaries[] = [ | |
| 'uuid' => $project->uuid, | |
| 'name' => $project->name, | |
| 'counts' => [ | |
| 'applications' => $apps, | |
| 'services' => $services, | |
| 'databases' => $databases, | |
| ], | |
| ]; | |
| } | |
| $projects = Project::where('team_id', $teamId) | |
| ->withCount(['applications', 'services', 'databases']) | |
| ->get(); | |
| $appCount = 0; | |
| $serviceCount = 0; | |
| $databaseCount = 0; | |
| $projectSummaries = []; | |
| foreach ($projects as $project) { | |
| $apps = $project->applications_count; | |
| $services = $project->services_count; | |
| $databases = $project->databases_count; | |
| $appCount += $apps; | |
| $serviceCount += $services; | |
| $databaseCount += $databases; | |
| $projectSummaries[] = [ | |
| 'uuid' => $project->uuid, | |
| 'name' => $project->name, | |
| 'counts' => [ | |
| 'applications' => $apps, | |
| 'services' => $services, | |
| 'databases' => $databases, | |
| ], | |
| ]; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/Mcp/Tools/GetInfrastructureOverview.php` around lines 48 - 73, Replace
the per-project relation count queries with an eager count on the initial query:
change the Project::where(...)->get() call to include
withCount(['applications','services','databases']) so each Project has
applications_count, services_count and databases_count; then inside the foreach
use those properties (e.g. $project->applications_count) for $apps, $services
and $databases, update the aggregations ($appCount, $serviceCount,
$databaseCount) to add those values, and use them when building
$projectSummaries['counts'] so no per-project relationship count queries are
executed in the loop.
Comment on lines
+33
to
+39
| $tagName = $request->get('tag'); | ||
| $args = $this->paginationArgs($request); | ||
|
|
||
| $query = Application::ownedByCurrentTeamAPI($teamId) | ||
| ->when($tagName, function ($query, $tagName) { | ||
| $query->whereHas('tags', fn ($q) => $q->where('name', $tagName)); | ||
| }); |
Contributor
There was a problem hiding this comment.
Validate tag before using it in the query.
Right now any non-string payload can flow from the MCP request into whereHas(...), which will turn malformed calls into a query exception instead of a clean tool error. Reject non-string values up front and only apply the filter when tag is a real string.
🛠️ Proposed fix
- $tagName = $request->get('tag');
+ $tagName = $request->get('tag');
+ if ($tagName !== null && (! is_string($tagName) || trim($tagName) === '')) {
+ return Response::error('tag argument must be a non-empty string.');
+ }
$args = $this->paginationArgs($request);
$query = Application::ownedByCurrentTeamAPI($teamId)
- ->when($tagName, function ($query, $tagName) {
+ ->when($tagName !== null, function ($query) use ($tagName) {
$query->whereHas('tags', fn ($q) => $q->where('name', $tagName));
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| $tagName = $request->get('tag'); | |
| $args = $this->paginationArgs($request); | |
| $query = Application::ownedByCurrentTeamAPI($teamId) | |
| ->when($tagName, function ($query, $tagName) { | |
| $query->whereHas('tags', fn ($q) => $q->where('name', $tagName)); | |
| }); | |
| $tagName = $request->get('tag'); | |
| if ($tagName !== null && (! is_string($tagName) || trim($tagName) === '')) { | |
| return Response::error('tag argument must be a non-empty string.'); | |
| } | |
| $args = $this->paginationArgs($request); | |
| $query = Application::ownedByCurrentTeamAPI($teamId) | |
| ->when($tagName !== null, function ($query) use ($tagName) { | |
| $query->whereHas('tags', fn ($q) => $q->where('name', $tagName)); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/Mcp/Tools/ListApplications.php` around lines 33 - 39, Validate the
incoming tag before using it in the query: ensure $tagName from
$request->get('tag') is a non-empty string and reject/return a tool error for
any non-string (array/object/null) values, then only call when($tagName, ...) to
apply the whereHas filter; update the code around $tagName and the
Application::ownedByCurrentTeamAPI(...)->when(...) block in ListApplications.php
to perform the type check and early error response rather than passing arbitrary
payloads into whereHas.
Comment on lines
+35
to
+53
| $projects = Project::where('team_id', $teamId)->get(); | ||
| $databases = collect(); | ||
| foreach ($projects as $project) { | ||
| $databases = $databases->merge($project->databases()); | ||
| } | ||
|
|
||
| $total = $databases->count(); | ||
|
|
||
| $summaries = $databases | ||
| ->sortBy('name') | ||
| ->slice($args['offset'], $args['per_page']) | ||
| ->map(fn ($db) => [ | ||
| 'uuid' => $db->uuid, | ||
| 'name' => $db->name, | ||
| 'status' => $db->status ?? null, | ||
| 'type' => method_exists($db, 'type') ? $db->type() : class_basename($db), | ||
| ]) | ||
| ->values() | ||
| ->all(); |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Avoid the per-project database fan-out.
$project->databases() inside the loop turns this into one query per project. Eager-load the relation and flatten it before sorting/slicing so the MCP tool stays server-friendly instead of query-farm friendly.
♻️ Suggested refactor
- $projects = Project::where('team_id', $teamId)->get();
- $databases = collect();
- foreach ($projects as $project) {
- $databases = $databases->merge($project->databases());
- }
+ $projects = Project::where('team_id', $teamId)->with('databases')->get();
+ $databases = $projects->flatMap->databases;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/Mcp/Tools/ListDatabases.php` around lines 35 - 53, The code is triggering
N+1 queries by iterating $projects and calling $project->databases(), so change
the query to eager-load the relation (use Project::where('team_id',
$teamId)->with('databases')->get()), then replace the manual loop with a single
flatten/flatMap over the loaded relation (e.g. $databases =
$projects->flatMap(fn($p) => $p->databases) or
$projects->pluck('databases')->flatten(1)) and continue using
$databases->sortBy(...)->slice(...)->map(...) as before; ensure you reference
the loaded relation property (->databases) not the relation method
(->databases()) and update ListDatabases.php accordingly.
Comment on lines
+7
to
+57
| use App\Models\Project; | ||
| use Illuminate\Contracts\JsonSchema\JsonSchema; | ||
| use Laravel\Mcp\Request; | ||
| use Laravel\Mcp\Response; | ||
| use Laravel\Mcp\Server\Attributes\Description; | ||
| use Laravel\Mcp\Server\Attributes\Name; | ||
| use Laravel\Mcp\Server\Tool; | ||
|
|
||
| #[Name('list_services')] | ||
| #[Description('List services (multi-container stacks) owned by the authenticated team. Returns summary (uuid, name, status). Use get_service for full details.')] | ||
| class ListServices extends Tool | ||
| { | ||
| use BuildsResponse; | ||
| use ResolvesTeam; | ||
|
|
||
| public function handle(Request $request): Response | ||
| { | ||
| if ($error = $this->ensureAbility($request, 'read')) { | ||
| return $error; | ||
| } | ||
|
|
||
| $teamId = $this->resolveTeamId($request); | ||
| if (is_null($teamId)) { | ||
| return Response::error('Invalid token.'); | ||
| } | ||
|
|
||
| $args = $this->paginationArgs($request); | ||
|
|
||
| $projects = Project::where('team_id', $teamId)->get(); | ||
| $services = collect(); | ||
| foreach ($projects as $project) { | ||
| $services = $services->merge($project->services()->get()); | ||
| } | ||
|
|
||
| $total = $services->count(); | ||
|
|
||
| $summaries = $services | ||
| ->sortBy('name') | ||
| ->slice($args['offset'], $args['per_page']) | ||
| ->map(fn ($svc) => [ | ||
| 'uuid' => $svc->uuid, | ||
| 'name' => $svc->name, | ||
| 'status' => $svc->status ?? null, | ||
| ]) | ||
| ->values() | ||
| ->all(); | ||
|
|
||
| return $this->respond( | ||
| $summaries, | ||
| [], | ||
| $this->paginationMeta('list_services', $args, $total), |
Contributor
There was a problem hiding this comment.
Stop building the full service list in PHP before paging.
This does one query per project and then sorts/slices everything in memory, so it will fall over as teams grow. Pull services through a single query, order them in SQL, and apply skip/take there.
♻️ Suggested refactor
-use App\Models\Project;
+use App\Models\Service;
@@
- $projects = Project::where('team_id', $teamId)->get();
- $services = collect();
- foreach ($projects as $project) {
- $services = $services->merge($project->services()->get());
- }
-
- $total = $services->count();
-
- $summaries = $services
- ->sortBy('name')
- ->slice($args['offset'], $args['per_page'])
+ $services = Service::whereRelation('environment.project.team', 'id', $teamId);
+ $total = $services->count();
+
+ $summaries = $services
+ ->orderBy('name')
+ ->skip($args['offset'])
+ ->take($args['per_page'])
+ ->get()
->map(fn ($svc) => [
'uuid' => $svc->uuid,
'name' => $svc->name,
'status' => $svc->status ?? null,
])
- ->values()
->all();🧰 Tools
🪛 PHPMD (2.15.0)
[error] 30-30: Avoid using static access to class '\Laravel\Mcp\Response' in method 'handle'. (undefined)
(StaticAccess)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/Mcp/Tools/ListServices.php` around lines 7 - 57, The current
ListServices::handle builds the full $services collection by iterating
Project::where(...)->get() and merging each $project->services(), causing N+1
queries and in-memory sorting/paging; instead query services directly with a
single Eloquent query joined/filtered by Project.team_id (e.g.
Service::whereHas('project', fn($q)=>$q->where('team_id', $teamId)) or join to
projects), apply orderBy('name') and use skip(take) (or ->offset()->limit() /
paginate) to fetch only the page, get the total via a count() query on the same
filter, then map the paged results to the existing summary structure and pass to
respond/paginationMeta (keep using paginationArgs, paginationMeta, respond).
Comment on lines
+8653
to
+8756
| "\/mcp\/enable": { | ||
| "get": { | ||
| "summary": "Enable MCP Server", | ||
| "description": "Enable the MCP server endpoint at \/mcp (only with root permissions).", | ||
| "operationId": "enable-mcp", | ||
| "responses": { | ||
| "200": { | ||
| "description": "MCP server enabled.", | ||
| "content": { | ||
| "application\/json": { | ||
| "schema": { | ||
| "properties": { | ||
| "message": { | ||
| "type": "string", | ||
| "example": "MCP server enabled." | ||
| } | ||
| }, | ||
| "type": "object" | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "403": { | ||
| "description": "You are not allowed to enable the MCP server.", | ||
| "content": { | ||
| "application\/json": { | ||
| "schema": { | ||
| "properties": { | ||
| "message": { | ||
| "type": "string", | ||
| "example": "You are not allowed to enable the MCP server." | ||
| } | ||
| }, | ||
| "type": "object" | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "401": { | ||
| "$ref": "#\/components\/responses\/401" | ||
| }, | ||
| "400": { | ||
| "$ref": "#\/components\/responses\/400" | ||
| } | ||
| }, | ||
| "security": [ | ||
| { | ||
| "bearerAuth": [] | ||
| } | ||
| ] | ||
| } | ||
| }, | ||
| "\/mcp\/disable": { | ||
| "get": { | ||
| "summary": "Disable MCP Server", | ||
| "description": "Disable the MCP server endpoint at \/mcp (only with root permissions).", | ||
| "operationId": "disable-mcp", | ||
| "responses": { | ||
| "200": { | ||
| "description": "MCP server disabled.", | ||
| "content": { | ||
| "application\/json": { | ||
| "schema": { | ||
| "properties": { | ||
| "message": { | ||
| "type": "string", | ||
| "example": "MCP server disabled." | ||
| } | ||
| }, | ||
| "type": "object" | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "403": { | ||
| "description": "You are not allowed to disable the MCP server.", | ||
| "content": { | ||
| "application\/json": { | ||
| "schema": { | ||
| "properties": { | ||
| "message": { | ||
| "type": "string", | ||
| "example": "You are not allowed to disable the MCP server." | ||
| } | ||
| }, | ||
| "type": "object" | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "401": { | ||
| "$ref": "#\/components\/responses\/401" | ||
| }, | ||
| "400": { | ||
| "$ref": "#\/components\/responses\/400" | ||
| } | ||
| }, | ||
| "security": [ | ||
| { | ||
| "bearerAuth": [] | ||
| } | ||
| ] | ||
| } | ||
| }, |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -nP 'Route::(get|post|patch|delete)\s*\([^;]*\/mcp\/(enable|disable)' routes app tests
rg -nP 'Cache-Control|no-store|no-cache' app routesRepository: coollabsio/coolify
Length of output: 6033
Switch /mcp/enable and /mcp/disable from GET to POST.
These endpoints mutate instance state, so GET violates REST semantics. The routes in routes/api.php (lines 38–39) are currently defined as Route::get(), which leaves them vulnerable to prefetch bots, browser caching, and link scanners—basically giving your MCP server to the T-800 on a silver platter. No HTTP cache headers protect them either.
Update both the OpenAPI spec (openapi.json) and the route definitions in routes/api.php to use POST or PATCH. This isn't just pedantry; state-changing operations need POST semantics so browsers and intermediaries don't auto-trigger them on a whim.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openapi.json` around lines 8653 - 8756, Change the two MCP endpoints from GET
to POST semantics: in openapi.json replace the "get" objects under paths
"/mcp/enable" (operationId "enable-mcp") and "/mcp/disable" (operationId
"disable-mcp") with "post" (keeping the same responses and schemas), and in
routes/api.php change Route::get('/mcp/enable', ...) and
Route::get('/mcp/disable', ...) to Route::post('/mcp/enable', ...) and
Route::post('/mcp/disable', ...); ensure any middleware/auth usage remains
unchanged and adjust any client calls/tests to use POST.
Comment on lines
+5487
to
+5508
| /mcp/enable: | ||
| get: | ||
| summary: 'Enable MCP Server' | ||
| description: 'Enable the MCP server endpoint at /mcp (only with root permissions).' | ||
| operationId: enable-mcp | ||
| responses: | ||
| '200': | ||
| description: 'MCP server enabled.' | ||
| content: | ||
| application/json: | ||
| schema: | ||
| properties: | ||
| message: { type: string, example: 'MCP server enabled.' } | ||
| type: object | ||
| '403': | ||
| description: 'You are not allowed to enable the MCP server.' | ||
| content: | ||
| application/json: | ||
| schema: | ||
| properties: | ||
| message: { type: string, example: 'You are not allowed to enable the MCP server.' } | ||
| type: object |
Contributor
There was a problem hiding this comment.
Tighten the inline YAML mappings here.
YAMLlint is flagging the flow-style objects in both MCP response blocks; this will fail lint even though the schema is otherwise fine. Reformat these to block style or compact the braces.
🛠️ Suggested cleanup
- schema:
- properties: { message: { type: string, example: 'MCP server enabled.' } }
- type: object
+ schema:
+ type: object
+ properties:
+ message:
+ type: string
+ example: 'MCP server enabled.'Apply the same rewrite to the /mcp/disable response block as well.
Also applies to: 5516-5537
🧰 Tools
🪛 YAMLlint (1.38.0)
[error] 5499-5499: too many spaces inside braces
(braces)
[error] 5499-5499: too many spaces inside braces
(braces)
[error] 5507-5507: too many spaces inside braces
(braces)
[error] 5507-5507: too many spaces inside braces
(braces)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openapi.yaml` around lines 5487 - 5508, The inline flow-style YAML mappings
in the /mcp/enable response schemas (and the corresponding /mcp/disable block)
use brace-style objects that fail YAML linting; update the response schema
mappings under the '200' and '403' responses for the /mcp/enable and
/mcp/disable endpoints to use block-style YAML mappings (expand properties and
example into standard block form instead of { type: string, example: '...' }) so
each property (message, type, example) is written as proper YAML key: value
pairs; locate the response blocks by operationId "enable-mcp" (and the
disable-mcp equivalent) and rewrite the schema mappings accordingly.
Comment on lines
+36
to
+39
| Route::get('/enable', [OtherController::class, 'enable_api']); | ||
| Route::get('/disable', [OtherController::class, 'disable_api']); | ||
| Route::get('/mcp/enable', [OtherController::class, 'enable_mcp']); | ||
| Route::get('/mcp/disable', [OtherController::class, 'disable_mcp']); |
Contributor
There was a problem hiding this comment.
Don't expose state changes as GET.
These routes flip instance config, so GET makes them cache/prefetch bait and breaks HTTP semantics. POST or PATCH keeps the toggle from firing just because someone hovered a link.
🔧 Suggested fix
- Route::get('/mcp/enable', [OtherController::class, 'enable_mcp']);
- Route::get('/mcp/disable', [OtherController::class, 'disable_mcp']);
+ Route::post('/mcp/enable', [OtherController::class, 'enable_mcp']);
+ Route::post('/mcp/disable', [OtherController::class, 'disable_mcp']);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@routes/api.php` around lines 36 - 39, These routes currently use GET but
perform state changes; update the route definitions to use a non-safe method
(POST or PATCH) for the endpoints that toggle state: change
Route::get('/enable', [OtherController::class, 'enable_api']),
Route::get('/disable', [OtherController::class, 'disable_api']),
Route::get('/mcp/enable', [OtherController::class, 'enable_mcp']), and
Route::get('/mcp/disable', [OtherController::class, 'disable_mcp']) to
Route::post or Route::patch as appropriate, and ensure any callers/forms/clients
and CSRF middleware are updated to send the correct HTTP verb when invoking
OtherController::enable_api, ::disable_api, ::enable_mcp, and ::disable_mcp.
Comment on lines
+11
to
+24
| uses(RefreshDatabase::class); | ||
|
|
||
| beforeEach(function () { | ||
| InstanceSettings::query()->where('id', 0)->delete(); | ||
| InstanceSettings::query()->delete(); | ||
| $settings = new InstanceSettings(['is_mcp_server_enabled' => true]); | ||
| $settings->id = 0; | ||
| $settings->save(); | ||
|
|
||
| $this->team = Team::factory()->create(); | ||
| $this->user = User::factory()->create(); | ||
| $this->team->members()->attach($this->user->id, ['role' => 'owner']); | ||
| session(['currentTeam' => $this->team]); | ||
| }); |
Contributor
There was a problem hiding this comment.
Move the Once reset into shared setup.
Once::flush() only runs in the disabled-state test, so any memoized instance setting can leak into later cases and make the suite order-dependent. Put the flush in beforeEach so every test starts from the same state — less haunted test behavior, more actual server behavior.
🧪 Suggested test setup tweak
beforeEach(function () {
+ Once::flush();
InstanceSettings::query()->where('id', 0)->delete();
InstanceSettings::query()->delete();
$settings = new InstanceSettings(['is_mcp_server_enabled' => true]);
@@
test('MCP endpoint returns 404 when the instance setting is disabled', function () {
InstanceSettings::query()->where('id', 0)->update(['is_mcp_server_enabled' => false]);
- Once::flush();
$response = mcpPost(['jsonrpc' => '2.0', 'id' => 1, 'method' => 'tools/list']);Also applies to: 67-73
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/Feature/Mcp/McpEndpointTest.php` around lines 11 - 24, The test suite
can leak memoized InstanceSettings because Once::flush() is only called in the
disabled-state test; move the Once::flush() call into the shared beforeEach
setup so every test starts with a clean Once cache. Update the beforeEach
closure (the setup that currently resets InstanceSettings, creates Team/User and
attaches member) to call Once::flush() at the start (or end) so tests that rely
on InstanceSettings or Once memoization (including the disabled-state test)
cannot affect later examples.
Comment on lines
+28
to
+36
| function attachDb(string $modelClass, array $extra, $destination, $environment) | ||
| { | ||
| return $modelClass::create(array_merge([ | ||
| 'name' => 'test-'.strtolower(class_basename($modelClass)), | ||
| 'environment_id' => $environment->id, | ||
| 'destination_id' => $destination->id, | ||
| 'destination_type' => $destination->getMorphClass(), | ||
| ], $extra)); | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Type the test helper so fixture bugs fail fast.
attachDb() is shared across multiple tests, but $destination and $environment are untyped and the function has no return type. That weakens the safety net this file is supposed to add.
Proposed fix
+use Illuminate\Database\Eloquent\Model;
...
-function attachDb(string $modelClass, array $extra, $destination, $environment)
+function attachDb(string $modelClass, array $extra, StandaloneDocker $destination, Environment $environment): ModelAs per coding guidelines, use explicit return type declarations and type hints for all method parameters.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/Feature/StandaloneDockerDatabasesTest.php` around lines 28 - 36, The
helper attachDb lacks parameter and return type hints; update its signature to
declare the destination and environment parameter types (e.g., Destination
$destination, Environment $environment) and an explicit return type of
Illuminate\Database\Eloquent\Model so fixture type errors fail fast; also add
the necessary use imports (Destination, Environment, and
Illuminate\Database\Eloquent\Model) at the top of the file and keep the body
unchanged except for the typed signature.
…rvice/app listing - `/mcp/enable` and `/mcp/disable` now use POST (state-mutating ops) - `ListServices` queries DB directly instead of loading all projects into memory - `ListApplications` validates tag arg rejects empty string (not just falsy)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
is_mcp_server_enabledboolean toInstanceSettings(migration + model fillable/cast)EnsureMcpEnabledmiddleware — aborts 404 when MCP is disabled; registered asmcp.enabledalias inKernelGET /api/v1/mcp/enableandGET /api/v1/mcp/disableendpoints (root-token only, audited); OpenAPI specs updated/mcpendpoint URL when enabledlaravel/mcp ^0.6.7dependency; registerCoolifyServerat/mcpwithmcp.enabled+auth:sanctummiddleware viaroutes/ai.phpCoolifyServerexposes 10 read-only tools:get_infrastructure_overview,list_servers,get_server,list_projects,list_applications,get_application,list_databases,get_database,list_services,get_serviceBuildsResponsetrait handles sensitive-key scrubbing, pagination, and HATEOAS-style_actionshintsResolvesTeamtrait validates token abilities and resolves team scopeMcpEndpointTest(tool listing, project scoping, pagination cap, sensitive-data scrubbing, ability checks) andMcpToggleApiTest(root vs non-root, read-only token, unauthenticated)Kernelto use imported class names instead of FQCN strings