Add ability for users to connect custom MCP tools as a connector to search spaces #688
Conversation
|
@manojag115 is attempting to deploy a commit to the Rohan Verma's projects Team on Vercel. A member of the Team first needs to authorize it. |
![recurseml[bot]](https://avatars.githubusercontent.com/in/1054195?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review by RecurseML
🔍 Review performed on 7ac7cd5..cd83a59
| Severity | Location | Issue | Delete |
|---|---|---|---|
 |
surfsense_web/components/assistant-ui/connector-popup/connect-forms/components/mcp-connect-form.tsx:265 | API key auth field mismatch |  |
|
surfsense_backend/app/agents/new_chat/tools/mcp_tool.py:169 | SSL verification disabled | |
✅ Files analyzed, no issues (24)
• MCP_FRONTEND_GUIDE.md
• MCP_IMPLEMENTATION.md
• surfsense_backend/alembic/versions/60_add_mcp_connector_type.py
• surfsense_backend/app/agents/new_chat/chat_deepagent.py
• surfsense_backend/app/agents/new_chat/tools/registry.py
• surfsense_backend/app/db.py
• surfsense_backend/app/routes/search_source_connectors_routes.py
• surfsense_backend/app/schemas/__init__.py
• surfsense_backend/app/schemas/search_source_connector.py
• surfsense_backend/app/tasks/chat/stream_new_chat.py
• surfsense_backend/app/utils/connector_naming.py
• surfsense_web/components/assistant-ui/connector-popup/components/connector-card.tsx
• surfsense_web/components/assistant-ui/connector-popup/connect-forms/index.tsx
• surfsense_web/components/assistant-ui/connector-popup/connector-configs/components/mcp-config.tsx
• surfsense_web/components/assistant-ui/connector-popup/connector-configs/index.tsx
• surfsense_web/components/assistant-ui/connector-popup/connector-configs/views/connector-connect-view.tsx
• surfsense_web/components/assistant-ui/connector-popup/constants/connector-constants.ts
• surfsense_web/components/assistant-ui/connector-popup/tabs/active-connectors-tab.tsx
• surfsense_web/components/assistant-ui/connector-popup/views/connector-accounts-list-view.tsx
• surfsense_web/contracts/enums/connector.ts
• surfsense_web/contracts/enums/connectorIcons.tsx
• surfsense_web/contracts/types/connector.types.ts
• surfsense_web/contracts/types/mcp.types.ts
• surfsense_web/lib/apis/connectors-api.service.ts
| <div className="space-y-2"> | ||
| <Label>API Key Value *</Label> | ||
| <Input | ||
| value={tool.auth_config.key_value || ""} |
There was a problem hiding this comment.
Frontend-backend field name mismatch in MCP connector API key authentication. The connect form uses 'key_value' for the API key value (line 265), but the backend expects 'api_key' (mcp_tool.py line 56). When a user creates an MCP connector with API key authentication through the connect form, the auth config will be saved with 'key_value' instead of 'api_key'. At runtime, when the agent tries to call the MCP tool, _build_auth_headers() will not find the 'api_key' field (it'll be an empty string), resulting in API requests being sent without the API key in headers. This will cause 401 Unauthorized errors from the remote API. The edit form (mcp-config.tsx line 266) correctly uses 'api_key', showing this is an inconsistency bug.
React with 👍 to tell me that this comment was useful, or 👎 if not (and I'll stop posting more comments like this in the future)
| logger.info(f"Making {method} request to {endpoint}") | ||
|
|
||
| # Make HTTP request (disable SSL verification for user-provided endpoints) | ||
| async with httpx.AsyncClient(timeout=30.0, verify=False) as client: |
There was a problem hiding this comment.
SSL certificate verification is disabled (verify=False) for all HTTP requests to user-provided MCP tool endpoints. This makes the application vulnerable to man-in-the-middle (MITM) attacks. An attacker on the network could intercept HTTPS requests to MCP endpoints, read sensitive data (API keys, request/response payloads), and modify responses. Since MCP tools are user-defined and can call any API (including APIs handling sensitive data), this could lead to credential theft, data exfiltration, or injection of malicious responses into the agent's context. While the PR description mentions this is intentional for 'user endpoints', this is a critical security vulnerability that should not be in production code. Best practice: SSL verification should be enabled by default, with an optional per-tool configuration flag if users need to disable it for specific self-signed certificate scenarios.
React with 👍 to tell me that this comment was useful, or 👎 if not (and I'll stop posting more comments like this in the future)
1 participant
Description
Backend:
Frontend:
Key Details:
Motivation and Context
FIX #625
Screenshots
API Changes
Change Type
Testing Performed
Checklist
High-level PR Summary
This PR introduces a Model Context Protocol (MCP) connector feature that allows users to define custom API endpoints as tools that the AI agent can invoke during conversations. The implementation includes a database migration to add the
MCP_CONNECTORtype, backend CRUD endpoints for managing MCP connectors with dynamic LangChain tool generation from JSON configurations, and a frontend UI for creating multi-tool connectors with support for multiple HTTP methods (GET, POST, PUT, PATCH, DELETE) and authentication types (Bearer, API Key, Basic Auth, None). Tools are loaded dynamically from the database on each chat session with permission-based access control, and the feature includes comprehensive documentation guides for both backend and frontend implementation.⏱️ Estimated Review Time: 30-90 minutes
💡 Review Order Suggestion
MCP_IMPLEMENTATION.mdMCP_FRONTEND_GUIDE.mdsurfsense_backend/alembic/versions/60_add_mcp_connector_type.pysurfsense_backend/app/db.pysurfsense_backend/app/schemas/search_source_connector.pysurfsense_backend/app/schemas/__init__.pysurfsense_web/contracts/enums/connector.tssurfsense_web/contracts/enums/connectorIcons.tsxsurfsense_web/contracts/types/connector.types.tssurfsense_web/contracts/types/mcp.types.tssurfsense_backend/app/agents/new_chat/tools/mcp_tool.pysurfsense_backend/app/agents/new_chat/tools/registry.pysurfsense_backend/app/routes/search_source_connectors_routes.pysurfsense_web/lib/apis/connectors-api.service.tssurfsense_web/components/assistant-ui/connector-popup/connect-forms/components/mcp-connect-form.tsxsurfsense_web/components/assistant-ui/connector-popup/connector-configs/components/mcp-config.tsxsurfsense_web/components/assistant-ui/connector-popup/connect-forms/index.tsxsurfsense_web/components/assistant-ui/connector-popup/connector-configs/index.tsxsurfsense_web/components/assistant-ui/connector-popup/connector-configs/views/connector-connect-view.tsxsurfsense_web/components/assistant-ui/connector-popup/constants/connector-constants.tssurfsense_backend/app/agents/new_chat/chat_deepagent.pysurfsense_backend/app/tasks/chat/stream_new_chat.pysurfsense_backend/app/utils/connector_naming.pysurfsense_web/components/assistant-ui/connector-popup/components/connector-card.tsxsurfsense_web/components/assistant-ui/connector-popup/tabs/active-connectors-tab.tsxsurfsense_web/components/assistant-ui/connector-popup/views/connector-accounts-list-view.tsx