Skip to content

Commit 0649d08

Browse files
committed
Add input validation and path traversal protection to MCP API
- Add McpSearchRequest form request for search endpoint validation - Add sanitization methods for platform, version, and path segments - Protect against path traversal attacks in docs service - Add security tests for validation and traversal protection
1 parent 0f76ddf commit 0649d08

4 files changed

Lines changed: 180 additions & 12 deletions

File tree

app/Http/Controllers/McpController.php

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
namespace App\Http\Controllers;
44

5+
use App\Http\Requests\McpSearchRequest;
56
use App\Services\DocsSearchService;
67
use Illuminate\Http\JsonResponse;
78
use Illuminate\Http\Request;
@@ -113,18 +114,16 @@ public function health(): JsonResponse
113114

114115
// REST API endpoints for simpler integrations
115116

116-
public function searchApi(Request $request): JsonResponse
117+
public function searchApi(McpSearchRequest $request): JsonResponse
117118
{
118-
$query = $request->input('q', '');
119-
$platform = $request->input('platform');
120-
$version = $request->input('version');
121-
$limit = (int) $request->input('limit', 10);
122-
123-
if (empty($query)) {
124-
return response()->json(['error' => 'Missing query parameter: q'], 400);
125-
}
126-
127-
$results = $this->docsSearch->search($query, $platform, $version, $limit);
119+
$validated = $request->validated();
120+
121+
$results = $this->docsSearch->search(
122+
$validated['q'],
123+
$validated['platform'] ?? null,
124+
$validated['version'] ?? null,
125+
$validated['limit'] ?? 10
126+
);
128127

129128
return response()->json(['results' => $results]);
130129
}
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
<?php
2+
3+
namespace App\Http\Requests;
4+
5+
use Illuminate\Foundation\Http\FormRequest;
6+
7+
class McpSearchRequest extends FormRequest
8+
{
9+
public function authorize(): bool
10+
{
11+
return true;
12+
}
13+
14+
public function rules(): array
15+
{
16+
return [
17+
'q' => ['required', 'string', 'max:500'],
18+
'platform' => ['nullable', 'string', 'in:desktop,mobile'],
19+
'version' => ['nullable', 'string', 'regex:/^[0-9]+$/'],
20+
'limit' => ['nullable', 'integer', 'min:1', 'max:100'],
21+
];
22+
}
23+
24+
public function messages(): array
25+
{
26+
return [
27+
'platform.in' => 'Platform must be either desktop or mobile.',
28+
'version.regex' => 'Version must be a numeric value.',
29+
'limit.max' => 'Limit cannot exceed 100.',
30+
];
31+
}
32+
}

app/Services/DocsSearchService.php

Lines changed: 67 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,15 @@ public function __construct()
1818

1919
public function search(string $query, ?string $platform = null, ?string $version = null, int $limit = 10): array
2020
{
21+
if ($platform !== null && ! $this->sanitizePlatform($platform)) {
22+
return [];
23+
}
24+
if ($version !== null && ! $this->sanitizeVersion($version)) {
25+
return [];
26+
}
27+
28+
$limit = min(max(1, $limit), 100);
29+
2130
$pages = $this->getAllPages($platform, $version);
2231
$queryTerms = $this->tokenize($query);
2332

@@ -38,6 +47,15 @@ public function search(string $query, ?string $platform = null, ?string $version
3847

3948
public function getPage(string $platform, string $version, string $section, string $slug): ?array
4049
{
50+
$platform = $this->sanitizePlatform($platform);
51+
$version = $this->sanitizeVersion($version);
52+
$section = $this->sanitizePathSegment($section);
53+
$slug = $this->sanitizePathSegment($slug);
54+
55+
if (! $platform || ! $version || ! $section || ! $slug) {
56+
return null;
57+
}
58+
4159
$filePath = "{$this->docsPath}/{$platform}/{$version}/{$section}/{$slug}.md";
4260

4361
if (! file_exists($filePath)) {
@@ -60,6 +78,10 @@ public function getPageByPath(string $path): ?array
6078

6179
public function listApis(string $platform, string $version): array
6280
{
81+
if (! $this->sanitizePlatform($platform) || ! $this->sanitizeVersion($version)) {
82+
return [];
83+
}
84+
6385
return collect($this->getAllPages($platform, $version))
6486
->filter(fn ($page) => $page['section'] === 'apis')
6587
->sortBy('order')
@@ -69,6 +91,10 @@ public function listApis(string $platform, string $version): array
6991

7092
public function getNavigation(string $platform, string $version): array
7193
{
94+
if (! $this->sanitizePlatform($platform) || ! $this->sanitizeVersion($version)) {
95+
return [];
96+
}
97+
7298
$pages = $this->getAllPages($platform, $version);
7399

74100
$sections = [];
@@ -125,7 +151,14 @@ public function getLatestVersions(): array
125151

126152
protected function getAllPages(?string $platform = null, ?string $version = null): array
127153
{
128-
$cacheKey = "mcp_docs_pages_{$platform}_{$version}";
154+
if ($platform !== null && ! $this->sanitizePlatform($platform)) {
155+
return [];
156+
}
157+
if ($version !== null && ! $this->sanitizeVersion($version)) {
158+
return [];
159+
}
160+
161+
$cacheKey = 'mcp_docs_pages_'.($platform ?? 'all').'_'.($version ?? 'all');
129162

130163
if (config('app.env') !== 'local') {
131164
$cached = Cache::get($cacheKey);
@@ -283,4 +316,37 @@ protected function extractSnippet(string $content, array $queryTerms, int $lengt
283316

284317
return Str::limit(preg_replace('/\s+/', ' ', $content), $length);
285318
}
319+
320+
protected function sanitizePlatform(?string $platform): ?string
321+
{
322+
if ($platform === null) {
323+
return null;
324+
}
325+
326+
$allowed = ['desktop', 'mobile'];
327+
328+
return in_array($platform, $allowed, true) ? $platform : null;
329+
}
330+
331+
protected function sanitizeVersion(?string $version): ?string
332+
{
333+
if ($version === null) {
334+
return null;
335+
}
336+
337+
return preg_match('/^[0-9]+$/', $version) ? $version : null;
338+
}
339+
340+
protected function sanitizePathSegment(?string $segment): ?string
341+
{
342+
if ($segment === null || $segment === '') {
343+
return null;
344+
}
345+
346+
if (str_contains($segment, '..') || str_contains($segment, '/') || str_contains($segment, '\\')) {
347+
return null;
348+
}
349+
350+
return preg_match('/^[a-zA-Z0-9_-]+$/', $segment) ? $segment : null;
351+
}
286352
}

tests/Feature/McpSecurityTest.php

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
<?php
2+
3+
namespace Tests\Feature;
4+
5+
use Tests\TestCase;
6+
7+
class McpSecurityTest extends TestCase
8+
{
9+
public function test_search_rejects_path_traversal_in_platform(): void
10+
{
11+
$response = $this->getJson('/api/mcp/search?q=test&platform=..');
12+
13+
$response->assertStatus(422);
14+
$response->assertJsonValidationErrors(['platform']);
15+
}
16+
17+
public function test_search_rejects_path_traversal_in_version(): void
18+
{
19+
$response = $this->getJson('/api/mcp/search?q=test&version=..');
20+
21+
$response->assertStatus(422);
22+
$response->assertJsonValidationErrors(['version']);
23+
}
24+
25+
public function test_search_rejects_excessive_limit(): void
26+
{
27+
$response = $this->getJson('/api/mcp/search?q=test&limit=1200');
28+
29+
$response->assertStatus(422);
30+
$response->assertJsonValidationErrors(['limit']);
31+
}
32+
33+
public function test_search_accepts_valid_parameters(): void
34+
{
35+
$response = $this->getJson('/api/mcp/search?q=camera&platform=mobile&version=2&limit=10');
36+
37+
$response->assertStatus(200);
38+
$response->assertJsonStructure(['results']);
39+
}
40+
41+
public function test_search_allows_null_platform_and_version(): void
42+
{
43+
$response = $this->getJson('/api/mcp/search?q=camera');
44+
45+
$response->assertStatus(200);
46+
$response->assertJsonStructure(['results']);
47+
}
48+
49+
public function test_page_api_rejects_path_traversal(): void
50+
{
51+
$response = $this->getJson('/api/mcp/page/../../../etc/passwd');
52+
53+
$response->assertStatus(404);
54+
}
55+
56+
public function test_apis_endpoint_rejects_invalid_platform(): void
57+
{
58+
$response = $this->getJson('/api/mcp/apis/../1');
59+
60+
$response->assertStatus(200);
61+
$response->assertJson(['apis' => []]);
62+
}
63+
64+
public function test_navigation_endpoint_rejects_invalid_version(): void
65+
{
66+
$response = $this->getJson('/api/mcp/navigation/mobile/..');
67+
68+
$response->assertStatus(200);
69+
$response->assertJson(['navigation' => []]);
70+
}
71+
}

0 commit comments

Comments
 (0)