From f5ead3ed4379f9a541a324eaacea779d5bb18757 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Thu, 15 Oct 2026 23:41:13 +0200 Subject: [PATCH] fix(mcp): enforce allowed IPs on the MCP endpoint Closes #11881 --- app/Http/Middleware/ApiAllowed.php | 20 ++---------- app/Http/Middleware/ApiIpAllowed.php | 31 +++++++++++++++++++ .../livewire/settings/advanced.blade.php | 4 +-- routes/ai.php | 3 +- routes/api.php | 3 +- .../AdvisorySecurityRegressionTest.php | 8 +++-- tests/Feature/LoginAndMcpAccessTest.php | 8 +++-- tests/Feature/Mcp/McpEndpointTest.php | 25 ++++++++++++--- tests/Feature/Mcp/McpToggleApiTest.php | 13 +++++++- 9 files changed, 82 insertions(+), 33 deletions(-) create mode 100644 app/Http/Middleware/ApiIpAllowed.php diff --git a/app/Http/Middleware/ApiAllowed.php b/app/Http/Middleware/ApiAllowed.php index 21441a117b..34e20d2617 100644 --- a/app/Http/Middleware/ApiAllowed.php +++ b/app/Http/Middleware/ApiAllowed.php @@ -13,26 +13,10 @@ class ApiAllowed if (isCloud()) { return $next($request); } - $settings = instanceSettings(); - if ($settings->is_api_enabled === false) { + if (instanceSettings()->is_api_enabled === false) { return response()->json(['success' => true, 'message' => 'API is disabled.'], 403); } - if ($settings->allowed_ips) { - // Check for special case: 0.0.0.0 means allow all - if (trim($settings->allowed_ips) === '0.0.0.0') { - return $next($request); - } - - $allowedIps = explode(',', $settings->allowed_ips); - $allowedIps = array_map('trim', $allowedIps); - $allowedIps = array_filter($allowedIps); // Remove empty entries - - if (! empty($allowedIps) && ! checkIPAgainstAllowlist($request->ip(), $allowedIps)) { - return response()->json(['success' => true, 'message' => 'You are not allowed to access the API.'], 403); - } - } - - return $next($request); + return app(ApiIpAllowed::class)->handle($request, $next); } } diff --git a/app/Http/Middleware/ApiIpAllowed.php b/app/Http/Middleware/ApiIpAllowed.php new file mode 100644 index 0000000000..ede32567d0 --- /dev/null +++ b/app/Http/Middleware/ApiIpAllowed.php @@ -0,0 +1,31 @@ +allowed_ips); + // Empty or 0.0.0.0 means allow all + if ($allowedIps === '' || $allowedIps === '0.0.0.0') { + return $next($request); + } + + $allowedIps = array_filter(array_map('trim', explode(',', $allowedIps))); + + if (! empty($allowedIps) && ! checkIPAgainstAllowlist($request->ip(), $allowedIps)) { + return response()->json(['success' => true, 'message' => 'You are not allowed to access the API.'], 403); + } + + return $next($request); + } +} diff --git a/resources/views/livewire/settings/advanced.blade.php b/resources/views/livewire/settings/advanced.blade.php index 0e6242d572..aa094316c0 100644 --- a/resources/views/livewire/settings/advanced.blade.php +++ b/resources/views/livewire/settings/advanced.blade.php @@ -89,8 +89,8 @@ ['value' => false, 'label' => 'Disabled'], ]" />
-
diff --git a/routes/ai.php b/routes/ai.php index d79ff0ebae..d3f9bf83d2 100644 --- a/routes/ai.php +++ b/routes/ai.php @@ -1,7 +1,8 @@ middleware(['mcp.enabled', 'auth:sanctum', 'api.token.team', 'mcp.team.enabled']); + ->middleware(['mcp.enabled', 'auth:sanctum', 'api.token.team', ApiIpAllowed::class, 'mcp.team.enabled']); diff --git a/routes/api.php b/routes/api.php index a44b95a1d2..b0eba12a6b 100644 --- a/routes/api.php +++ b/routes/api.php @@ -40,6 +40,7 @@ use App\Http\Controllers\Api\TeamController; use App\Http\Controllers\Api\VolumeBackupsController; use App\Http\Controllers\Api\VultrController; use App\Http\Middleware\ApiAllowed; +use App\Http\Middleware\ApiIpAllowed; use Illuminate\Support\Facades\Route; Route::get('/health', [OtherController::class, 'healthcheck']); @@ -53,7 +54,7 @@ Route::post('/feedback', [OtherController::class, 'feedback']) ->middleware('throttle:feedback'); Route::group([ - 'middleware' => ['auth:sanctum', 'api.token.team', 'api.ability:write'], + 'middleware' => ['auth:sanctum', 'api.token.team', ApiIpAllowed::class, 'api.ability:write'], 'prefix' => 'v1', ], function () { Route::get('/enable', [OtherController::class, 'post_required']); diff --git a/tests/Feature/AdvisorySecurityRegressionTest.php b/tests/Feature/AdvisorySecurityRegressionTest.php index 452308d22a..4c14ce379e 100644 --- a/tests/Feature/AdvisorySecurityRegressionTest.php +++ b/tests/Feature/AdvisorySecurityRegressionTest.php @@ -4,6 +4,7 @@ use App\Http\Controllers\Api\DatabasesController; use App\Http\Controllers\Api\ServiceApplicationsController; use App\Http\Controllers\Webhook\Concerns\MatchesManualWebhookApplications; use App\Http\Middleware\ApiAllowed; +use App\Http\Middleware\ApiIpAllowed; use App\Models\EnvironmentVariable; use App\Models\InstanceSettings; use App\Models\Service; @@ -49,7 +50,7 @@ it('does not require email verification for protected web routes', function () { $this->actingAs($user)->get('/analytics')->assertOk(); }); -it('does not apply the REST API allowlist to MCP and MCP switch routes', function () { +it('applies only the IP allow-list, not the REST API switch, 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')); @@ -57,7 +58,10 @@ it('does not apply the REST API allowlist to MCP and MCP switch routes', functio 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); + ->and($disable->gatherMiddleware())->not->toContain(ApiAllowed::class) + ->and($mcp->gatherMiddleware())->toContain(ApiIpAllowed::class) + ->and($enable->gatherMiddleware())->toContain(ApiIpAllowed::class) + ->and($disable->gatherMiddleware())->toContain(ApiIpAllowed::class); }); it('throttles only failed authentication on manual webhook routes', function (string $provider) { diff --git a/tests/Feature/LoginAndMcpAccessTest.php b/tests/Feature/LoginAndMcpAccessTest.php index 5d0325f291..1c63176fcc 100644 --- a/tests/Feature/LoginAndMcpAccessTest.php +++ b/tests/Feature/LoginAndMcpAccessTest.php @@ -1,6 +1,7 @@ and($firstLimits[1]->key)->toBe($secondLimits[1]->key); }); -it('keeps MCP independent from the REST API access check', function () { +it('keeps MCP independent from the REST API switch but applies the IP allow-list', 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($mcp->gatherMiddleware())->toContain(ApiIpAllowed::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'); + ->and($enable->gatherMiddleware())->toContain('auth:sanctum', 'api.token.team', ApiIpAllowed::class, 'api.ability:write') + ->and($disable->gatherMiddleware())->toContain('auth:sanctum', 'api.token.team', ApiIpAllowed::class, 'api.ability:write'); }); diff --git a/tests/Feature/Mcp/McpEndpointTest.php b/tests/Feature/Mcp/McpEndpointTest.php index f09caca03d..f514caea93 100644 --- a/tests/Feature/Mcp/McpEndpointTest.php +++ b/tests/Feature/Mcp/McpEndpointTest.php @@ -117,15 +117,30 @@ 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']); +test('MCP endpoint works when the REST API is disabled', function () { + InstanceSettings::query()->where('id', 0)->update(['is_api_enabled' => false]); + Once::flush(); + $token = $this->user->createToken('mcp-read', ['read'])->plainTextToken; + + mcpListTools($token)->assertOk(); +}); + +test('MCP endpoint rejects a token from an IP outside the allow-list', function () { + InstanceSettings::query()->where('id', 0)->update(['allowed_ips' => '192.0.2.10']); + Once::flush(); + $token = $this->user->createToken('mcp-read', ['read'])->plainTextToken; + + mcpListTools($token) + ->assertForbidden() + ->assertJson(['message' => 'You are not allowed to access the API.']); +}); + +test('MCP endpoint accepts a token from an IP inside the allow-list', function () { + InstanceSettings::query()->where('id', 0)->update(['is_api_enabled' => false, 'allowed_ips' => '192.0.2.10, 127.0.0.0/8']); 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 () { diff --git a/tests/Feature/Mcp/McpToggleApiTest.php b/tests/Feature/Mcp/McpToggleApiTest.php index 0de306cd01..9759668262 100644 --- a/tests/Feature/Mcp/McpToggleApiTest.php +++ b/tests/Feature/Mcp/McpToggleApiTest.php @@ -94,7 +94,7 @@ test('non-root token cannot disable MCP server', function () { }); 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']); + InstanceSettings::query()->where('id', 0)->update(['is_api_enabled' => false]); $token = makeRootMcpToken($this->user); test()->withHeaders(['Authorization' => 'Bearer '.$token]) @@ -104,6 +104,17 @@ test('root token can enable MCP server when the REST API is disabled', function expect(InstanceSettings::find(0)->is_mcp_server_enabled)->toBeTrue(); }); +test('root token cannot enable MCP server from an IP outside the allow-list', 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') + ->assertForbidden(); + + expect(InstanceSettings::find(0)->is_mcp_server_enabled)->toBeFalse(); +}); + test('unauthenticated request to /api/v1/mcp/enable returns 401', function () { $response = test()->postJson('/api/v1/mcp/enable'); $response->assertStatus(401);