Skip to content

Commit 95774a0

Browse files
authored
Merge pull request #29 from dreamfactorysoftware/2026-04-security-scan
Security: fix session leaks, OAuth redirect validation, host header injection
2 parents e1b50d2 + 9569b7e commit 95774a0

3 files changed

Lines changed: 46 additions & 24 deletions

File tree

daemon/src/server.ts

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,15 @@ const app = express();
2525
const PORT = Number(process.env.MCP_DAEMON_PORT ?? 8006);
2626
const HOST = process.env.MCP_DAEMON_HOST ?? '127.0.0.1';
2727

28+
// MCP clients (Claude Desktop, etc.) are external — CORS must be permissive.
29+
// The daemon is already protected by requiring a DreamFactory session token.
2830
app.use(cors());
2931
app.use(express.json({ limit: '10mb' }));
3032
app.use(express.urlencoded({ extended: true }));
3133

34+
// Internal API key for PHP proxy -> daemon communication
35+
const INTERNAL_API_KEY = process.env.MCP_INTERNAL_KEY ?? '';
36+
3237
/**
3338
* Send 401 Unauthorized response
3439
*/
@@ -46,25 +51,28 @@ function sendUnauthorized(res: Response): void {
4651
const sessionManager = new SessionService();
4752
const sessions = new Map<string, SessionEntry>();
4853

49-
// Health check endpoints
54+
// Health check endpoints — do not expose session IDs
5055
app.get('/health', (_req, res) => {
5156
res.json({
5257
status: 'ok',
5358
timestamp: Math.floor(Date.now() / 1000),
54-
sessions: Array.from(sessions.keys())
59+
active_sessions: sessions.size,
5560
});
5661
});
5762

5863
app.get('/ping', (_req, res) => {
5964
res.json({
6065
status: 'ok',
6166
timestamp: Math.floor(Date.now() / 1000),
62-
sessions: Array.from(sessions.keys())
67+
active_sessions: sessions.size,
6368
});
6469
});
6570

66-
// Cache management endpoint
71+
// Cache management endpoint — requires internal API key from PHP proxy
6772
app.post('/mcp/cache/clear', (req, res) => {
73+
if (INTERNAL_API_KEY && req.headers['x-mcp-internal-key'] !== INTERNAL_API_KEY) {
74+
return res.status(403).json({ error: 'Forbidden: invalid internal key' });
75+
}
6876
const service = typeof req.body === 'object' ? req.body?.service : undefined;
6977
if (service) {
7078
for (const [sessionId, entry] of sessions.entries()) {

src/Http/Controllers/McpOAuthController.php

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -221,11 +221,21 @@ public function authorizeGet(Request $request, string $mcpService)
221221
]);
222222
}
223223

224-
// Validate redirect URI
224+
// Validate redirect URI is present and registered with the client
225225
if (empty($redirectUri)) {
226226
return $this->errorResponse('invalid_request', 'Missing redirect_uri');
227227
}
228228

229+
$registeredUris = $client->redirect_uris ?? [];
230+
if (!empty($registeredUris) && !in_array($redirectUri, $registeredUris, true)) {
231+
Log::warning('MCP OAuth: redirect_uri not registered', [
232+
'provided' => $redirectUri,
233+
'registered' => $registeredUris,
234+
'service' => $mcpService,
235+
]);
236+
return $this->errorResponse('invalid_request', 'redirect_uri is not registered for this client');
237+
}
238+
229239
// ============================================================
230240
// SSO CHECK: Try to find existing DreamFactory session
231241
// If user is already logged in, skip the login page entirely
@@ -951,17 +961,16 @@ private function getExistingDfSession(Request $request): ?array
951961
*/
952962
private function getBaseUrl(Request $request): string
953963
{
954-
$scheme = $request->header('X-Forwarded-Proto', $request->getScheme());
955-
$host = $request->header('X-Forwarded-Host', $request->getHost());
956-
957-
// For proxied requests, don't include port (proxy handles it)
958-
// Only include port for direct requests with non-standard ports
959-
if ($request->header('X-Forwarded-Proto')) {
960-
// Proxied request - don't add port
961-
return "{$scheme}://{$host}";
964+
// Prefer configured APP_URL to prevent host header injection attacks.
965+
// Only fall back to request headers for local/dev environments.
966+
$configuredUrl = rtrim(config('app.url', ''), '/');
967+
if (!empty($configuredUrl) && $configuredUrl !== 'http://localhost') {
968+
return $configuredUrl;
962969
}
963970

964-
// Direct request - check if we need to include port
971+
$scheme = $request->getScheme();
972+
$host = $request->getHost();
973+
965974
$port = $request->getPort();
966975
$url = "{$scheme}://{$host}";
967976
if (($scheme === 'http' && $port != 80) || ($scheme === 'https' && $port != 443)) {

src/Http/Middleware/McpStreamMiddleware.php

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -45,12 +45,17 @@ class McpStreamMiddleware
4545
/**
4646
* CORS headers for MCP endpoints
4747
*/
48-
private const CORS_HEADERS = [
49-
'Access-Control-Allow-Origin' => '*',
50-
'Access-Control-Allow-Methods' => 'GET, POST, DELETE, OPTIONS',
51-
'Access-Control-Allow-Headers' => 'Content-Type, Authorization, mcp-session-id',
52-
'Access-Control-Expose-Headers' => 'WWW-Authenticate',
53-
];
48+
// MCP clients (Claude Desktop, etc.) are external — CORS must be permissive.
49+
// Endpoints are protected by requiring a DreamFactory session/Bearer token.
50+
private static function corsHeaders(): array
51+
{
52+
return [
53+
'Access-Control-Allow-Origin' => '*',
54+
'Access-Control-Allow-Methods' => 'GET, POST, DELETE, OPTIONS',
55+
'Access-Control-Allow-Headers' => 'Content-Type, Authorization, mcp-session-id',
56+
'Access-Control-Expose-Headers' => 'WWW-Authenticate',
57+
];
58+
}
5459

5560
/**
5661
* Handle an incoming request.
@@ -76,7 +81,7 @@ public function handle(Request $request, Closure $next)
7681

7782
// Handle OPTIONS preflight for any MCP path
7883
if ($method === 'OPTIONS') {
79-
return response('', 200)->withHeaders(self::CORS_HEADERS);
84+
return response('', 200)->withHeaders(self::corsHeaders());
8085
}
8186

8287
$mcpService = $matches[1];
@@ -100,7 +105,7 @@ public function handle(Request $request, Closure $next)
100105

101106
// Add CORS headers to response
102107
if ($response) {
103-
foreach (self::CORS_HEADERS as $key => $value) {
108+
foreach (self::corsHeaders() as $key => $value) {
104109
$response->headers->set($key, $value);
105110
}
106111
return $response;
@@ -116,7 +121,7 @@ public function handle(Request $request, Closure $next)
116121
private function handleRfc8414WellKnown(Request $request, Closure $next, string $wellKnownType, string $mcpService, string $method)
117122
{
118123
if ($method === 'OPTIONS') {
119-
return response('', 200)->withHeaders(self::CORS_HEADERS);
124+
return response('', 200)->withHeaders(self::corsHeaders());
120125
}
121126

122127
$controllerMethod = self::RFC8414_WELL_KNOWN[$wellKnownType] ?? null;
@@ -129,7 +134,7 @@ private function handleRfc8414WellKnown(Request $request, Closure $next, string
129134
$response = $controller->$controllerMethod($request, $mcpService);
130135

131136
if ($response) {
132-
foreach (self::CORS_HEADERS as $key => $value) {
137+
foreach (self::corsHeaders() as $key => $value) {
133138
$response->headers->set($key, $value);
134139
}
135140
return $response;

0 commit comments

Comments
 (0)