-
Notifications
You must be signed in to change notification settings - Fork 16
Fix work in Cline #12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 1 commit
cf9ecdd
b65cc8d
0becdf3
c491aa5
2c480e4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Create MCP server with executeQuery and listGraphs tools - Add GET /api/sse endpoint for SSE connections - Add POST /api/messages endpoint for client messages - Update documentation with SSE configuration example Co-authored-by: gkorland <753206+gkorland@users.noreply.github.com>
- Loading branch information
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,6 +1,7 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import express from 'express'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { config } from './config'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { mcpRoutes } from './routes/mcp.routes'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { sseRoutes } from './routes/sse.routes'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { authenticateMCP } from './middleware/auth.middleware'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { falkorDBService } from './services/falkordb.service'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -14,6 +15,9 @@ app.use(express.urlencoded({ extended: true })); | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Apply authentication to MCP routes | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| app.use('/api/mcp', authenticateMCP, mcpRoutes); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // SSE routes for MCP protocol (no authentication for SSE transport) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| app.use('/api', sseRoutes); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
15
to
+19
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Apply authentication to MCP routes | |
| app.use('/api/mcp', authenticateMCP, mcpRoutes); | |
| // SSE routes for MCP protocol (no authentication for SSE transport) | |
| app.use('/api', sseRoutes); | |
| // Lightweight auth for SSE MCP endpoints | |
| const requireSseAuth = (req: any, res: any, next: any) => { | |
| const nodeEnv = config.server.nodeEnv; | |
| const allowUnauthenticated = process.env.ALLOW_UNAUTHENTICATED_SSE === 'true'; | |
| // Allow unauthenticated SSE only in development or when explicitly opted-in | |
| if (nodeEnv === 'development' || allowUnauthenticated) { | |
| return next(); | |
| } | |
| const sharedSecret = process.env.SSE_SHARED_SECRET; | |
| if (!sharedSecret) { | |
| console.warn('SSE_SHARED_SECRET is not set; rejecting SSE request in non-development environment.'); | |
| return res.status(503).json({ error: 'SSE is not configured' }); | |
| } | |
| const headerSecret = req.headers['x-sse-shared-secret'] as string | undefined; | |
| const querySecret = (req.query && (req.query.sse_shared_secret as string | undefined)) || undefined; | |
| const providedSecret = headerSecret || querySecret; | |
| if (providedSecret === sharedSecret) { | |
| return next(); | |
| } | |
| return res.status(401).json({ error: 'Unauthorized' }); | |
| }; | |
| // Apply authentication to MCP routes | |
| app.use('/api/mcp', authenticateMCP, mcpRoutes); | |
| // SSE routes for MCP protocol (protected by lightweight auth) | |
| app.use('/api', requireSseAuth, sseRoutes); |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,72 @@ | ||||||||||||||||||||
| import { Router, Request, Response } from 'express'; | ||||||||||||||||||||
| import { SSEServerTransport } from '@modelcontextprotocol/sdk/server/sse.js'; | ||||||||||||||||||||
| import { createMCPServer } from '../services/mcp.server'; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| const router = Router(); | ||||||||||||||||||||
|
|
||||||||||||||||||||
| // Store active SSE transports by session ID | ||||||||||||||||||||
| const transports: { [sessionId: string]: SSEServerTransport } = {}; | ||||||||||||||||||||
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| /** | ||||||||||||||||||||
| * SSE endpoint for MCP connections | ||||||||||||||||||||
| * This establishes the Server-Sent Events connection for the MCP protocol | ||||||||||||||||||||
| */ | ||||||||||||||||||||
| router.get('/sse', async (req: Request, res: Response) => { | ||||||||||||||||||||
| try { | ||||||||||||||||||||
| // Create a new SSE transport | ||||||||||||||||||||
| const transport = new SSEServerTransport('/api/messages', res); | ||||||||||||||||||||
|
||||||||||||||||||||
| const transport = new SSEServerTransport('/api/messages', res); | |
| const baseUrl = (req.baseUrl || '').replace(/\/$/, ''); | |
| const messagesEndpoint = `${baseUrl}/messages`; | |
| const transport = new SSEServerTransport(messagesEndpoint, res); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in commit 2c480e4. The message endpoint is now derived from req.baseUrl to handle different base paths and reverse proxy configurations.
Copilot
AI
Feb 7, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using a plain object as a session map is more error-prone than Map (key collisions with inherited properties, less explicit APIs). Consider switching to const transports = new Map<string, SSEServerTransport>() and using set/get/delete for clearer and safer session management.
Copilot
AI
Feb 7, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using a plain object as a session map is more error-prone than Map (key collisions with inherited properties, less explicit APIs). Consider switching to const transports = new Map<string, SSEServerTransport>() and using set/get/delete for clearer and safer session management.
Copilot
AI
Feb 7, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In Express, req.query.sessionId can be string | string[] | undefined. Casting to string can cause incorrect behavior (e.g., an array becomes truthy but not a valid session key). Parse/validate the query param type explicitly (reject arrays or pick the first value) before using it as a lookup key.
| const sessionId = req.query.sessionId as string; | |
| const rawSessionId = req.query.sessionId; | |
| let sessionId: string | undefined; | |
| if (Array.isArray(rawSessionId)) { | |
| sessionId = rawSessionId[0]; | |
| } else if (typeof rawSessionId === 'string') { | |
| sessionId = rawSessionId; | |
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in commit 2c480e4. Added explicit type checking to handle array, string, and undefined cases for the sessionId query parameter.
Copilot
AI
Feb 7, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using a plain object as a session map is more error-prone than Map (key collisions with inherited properties, less explicit APIs). Consider switching to const transports = new Map<string, SSEServerTransport>() and using set/get/delete for clearer and safer session management.
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,77 @@ | ||||||
| import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; | ||||||
| import { z } from 'zod'; | ||||||
| import { falkorDBService } from './falkordb.service'; | ||||||
|
|
||||||
| /** | ||||||
| * Create and configure the MCP server with FalkorDB tools | ||||||
| */ | ||||||
| export function createMCPServer(): McpServer { | ||||||
| const server = new McpServer({ | ||||||
| name: 'FalkorDB MCP Server', | ||||||
| version: '1.0.0', | ||||||
| }); | ||||||
|
|
||||||
| // Tool: Execute a Cypher query on a graph | ||||||
| server.tool( | ||||||
| 'executeQuery', | ||||||
| { | ||||||
| graphName: z.string().describe('The name of the graph to query'), | ||||||
| query: z.string().describe('The Cypher query to execute'), | ||||||
| params: z.record(z.any()).optional().describe('Optional parameters for the query'), | ||||||
|
||||||
| params: z.record(z.any()).optional().describe('Optional parameters for the query'), | |
| params: z.record(z.unknown()).optional().describe('Optional parameters for the query'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in commit 2c480e4. Changed from z.any() to z.record(z.string(), z.unknown()) for safer parameter validation.
Copilot
AI
Feb 7, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Returning raw error.message to the MCP client can leak sensitive internal details (DB error strings, connection info, stack-adjacent messages). Consider logging the full error server-side and returning a more generic client-facing message (optionally with a request/session correlation id).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in commit 2c480e4. Error messages now log full details server-side while returning generic messages to clients to avoid leaking sensitive database information.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
POST /api/messagesendpoint requires asessionIdquery parameter (as implemented insse.routes.ts). Document the requiredsessionIdand note that clients should use the message URL provided/constructed by the SSE handshake (rather than calling/api/messagesdirectly without parameters).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in commit 2c480e4. Updated README to document that
sessionIdis required and clients should use the URL provided by the SSE handshake.