feat: update @modelcontextprotocol/sdk to version 1.13.2 and refactor… #229
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe implementation of the Changes
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (3)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
lib/ai/mcp.ts(1 hunks)package.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
`**/*.{ts,tsx}`: Use TypeScript for all code.
**/*.{ts,tsx}: Use TypeScript for all code.
📄 Source: CodeRabbit Inference Engine (.github/copilot-instructions.md)
List of files the instruction was applied to:
lib/ai/mcp.ts
`lib/**/*.{ts,tsx}`: Each file should limit its size within 600 lines of code. If exceeded, split into multiple files and group using a folder.
lib/**/*.{ts,tsx}: Each file should limit its size within 600 lines of code. If exceeded, split into multiple files and group using a folder.
📄 Source: CodeRabbit Inference Engine (.github/copilot-instructions.md)
List of files the instruction was applied to:
lib/ai/mcp.ts
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: e2e (22.x, 3, 3)
- GitHub Check: e2e (22.x, 2, 3)
- GitHub Check: e2e (22.x, 1, 3)
🔇 Additional comments (3)
package.json (1)
47-47: Verify the SDK version is current and secure.The dependency addition looks correct. Please ensure this is the latest stable version and check for any known security vulnerabilities.
What is the latest version of @modelcontextprotocol/sdk npm package and are there any known security vulnerabilities in version 1.13.2?lib/ai/mcp.ts (2)
3-3: LGTM: Clean import statement.The import follows TypeScript best practices and correctly imports from the new SDK.
12-15: Verify transport options parameter usage.Ensure that the
StreamableHTTPClientTransportconstructor correctly accepts the options parameter for headers configuration as suggested in the security fix above.#!/bin/bash # Description: Check the StreamableHTTPClientTransport constructor signature and options # Search for the StreamableHTTPClientTransport class definition to understand its constructor ast-grep --pattern $'class StreamableHTTPClientTransport { $$$ constructor($_, $_) { $$$ } $$$ }' # Also search for type definitions or interfaces related to transport options rg -A 10 "StreamableHTTPClientTransport" --type ts
| const url = new URL(process.env.MCP_ROUTER_SERVER_URL!); | ||
| // add x-api-key to the url | ||
| // biome-ignore lint/style/noNonNullAssertion: <explanation> | ||
| url.searchParams.set('x-api-key', process.env.MCP_ROUTER_SERVER_API_KEY!); |
There was a problem hiding this comment.
Security concern: API key exposed in URL query parameters.
Adding the API key as a URL query parameter poses security risks as query parameters can be logged in server logs, browser history, and proxy logs. Consider using headers or request body instead.
- const url = new URL(process.env.MCP_ROUTER_SERVER_URL!);
- // add x-api-key to the url
- // biome-ignore lint/style/noNonNullAssertion: <explanation>
- url.searchParams.set('x-api-key', process.env.MCP_ROUTER_SERVER_API_KEY!);
+ const url = new URL(process.env.MCP_ROUTER_SERVER_URL!);Then pass the API key via headers in the transport options:
- const transport = new StreamableHTTPClientTransport(url, {});
+ const transport = new StreamableHTTPClientTransport(url, {
+ headers: {
+ 'x-api-key': process.env.MCP_ROUTER_SERVER_API_KEY!
+ }
+ });🤖 Prompt for AI Agents
In lib/ai/mcp.ts around lines 7 to 10, the API key is currently added as a URL
query parameter, which exposes it to logs and browser history. To fix this,
remove the API key from the URL query parameters and instead include it in the
request headers when making the HTTP call. Update the code to set the
'x-api-key' header in the transport or fetch options rather than appending it to
the URL.
| export const createMCPClient = () => { | ||
| // biome-ignore lint/style/noNonNullAssertion: <explanation> | ||
| const url = new URL(process.env.MCP_ROUTER_SERVER_URL!); | ||
| // add x-api-key to the url | ||
| // biome-ignore lint/style/noNonNullAssertion: <explanation> | ||
| url.searchParams.set('x-api-key', process.env.MCP_ROUTER_SERVER_API_KEY!); | ||
|
|
||
| const transport = new StreamableHTTPClientTransport(url, {}); | ||
| return createMCPClientSDK({ | ||
| transport, | ||
| }); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling and environment variable validation.
The function uses non-null assertions without validating required environment variables, which could lead to runtime errors.
export const createMCPClient = () => {
+ const serverUrl = process.env.MCP_ROUTER_SERVER_URL;
+ const apiKey = process.env.MCP_ROUTER_SERVER_API_KEY;
+
+ if (!serverUrl) {
+ throw new Error('MCP_ROUTER_SERVER_URL environment variable is required');
+ }
+
+ if (!apiKey) {
+ throw new Error('MCP_ROUTER_SERVER_API_KEY environment variable is required');
+ }
+
+ let url: URL;
+ try {
+ url = new URL(serverUrl);
+ } catch (error) {
+ throw new Error(`Invalid MCP_ROUTER_SERVER_URL: ${error.message}`);
+ }
+
- // biome-ignore lint/style/noNonNullAssertion: <explanation>
- const url = new URL(process.env.MCP_ROUTER_SERVER_URL!);
- // add x-api-key to the url
- // biome-ignore lint/style/noNonNullAssertion: <explanation>
- url.searchParams.set('x-api-key', process.env.MCP_ROUTER_SERVER_API_KEY!);
- const transport = new StreamableHTTPClientTransport(url, {});
+ const transport = new StreamableHTTPClientTransport(url, {
+ headers: {
+ 'x-api-key': apiKey
+ }
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const createMCPClient = () => { | |
| // biome-ignore lint/style/noNonNullAssertion: <explanation> | |
| const url = new URL(process.env.MCP_ROUTER_SERVER_URL!); | |
| // add x-api-key to the url | |
| // biome-ignore lint/style/noNonNullAssertion: <explanation> | |
| url.searchParams.set('x-api-key', process.env.MCP_ROUTER_SERVER_API_KEY!); | |
| const transport = new StreamableHTTPClientTransport(url, {}); | |
| return createMCPClientSDK({ | |
| transport, | |
| }); | |
| }; | |
| export const createMCPClient = () => { | |
| const serverUrl = process.env.MCP_ROUTER_SERVER_URL; | |
| const apiKey = process.env.MCP_ROUTER_SERVER_API_KEY; | |
| if (!serverUrl) { | |
| throw new Error('MCP_ROUTER_SERVER_URL environment variable is required'); | |
| } | |
| if (!apiKey) { | |
| throw new Error('MCP_ROUTER_SERVER_API_KEY environment variable is required'); | |
| } | |
| let url: URL; | |
| try { | |
| url = new URL(serverUrl); | |
| } catch (error) { | |
| throw new Error(`Invalid MCP_ROUTER_SERVER_URL: ${error.message}`); | |
| } | |
| const transport = new StreamableHTTPClientTransport(url, { | |
| headers: { | |
| 'x-api-key': apiKey | |
| } | |
| }); | |
| return createMCPClientSDK({ | |
| transport, | |
| }); | |
| }; |
🤖 Prompt for AI Agents
In lib/ai/mcp.ts around lines 5 to 16, the function createMCPClient uses
non-null assertions on environment variables without validation, risking runtime
errors. Add checks to verify that MCP_ROUTER_SERVER_URL and
MCP_ROUTER_SERVER_API_KEY are defined before using them. If either is missing,
throw a clear error to prevent proceeding with invalid configuration. This
ensures safer and more predictable behavior.
Codecov ReportAttention: Patch coverage is
... and 1 file with indirect coverage changes 🚀 New features to boost your workflow:
|
… transport configuration in mcp.ts
… transport configuration in mcp.ts
Summary by CodeRabbit