Skip to content

Comments

fix (ai/mcp): prevent mutation of customEnv#5583

Merged
lgrammel merged 5 commits intovercel:mainfrom
cgoinglove:fix-customenv-mutation
Apr 7, 2025
Merged

fix (ai/mcp): prevent mutation of customEnv#5583
lgrammel merged 5 commits intovercel:mainfrom
cgoinglove:fix-customenv-mutation

Conversation

@cgoinglove
Copy link
Contributor

@cgoinglove cgoinglove commented Apr 7, 2025

Description

This PR fixes an issue where the original customEnv object passed to functions in get-environment.ts was being mutated due to JavaScript's pass-by-reference behavior for objects.

Problem

When a configuration object with an env property was passed to createChildProcess, any modifications to the environment inside the function would also affect the original object, potentially causing unexpected behavior in subsequent code.

In our project, we encountered this issue specifically when using serverConfig with the MCP client:

const serverConfig = await findMcpServerConfig(id);
// {
//   command: 'node server.js',
//   env: { PORT: '3000' }
// };

const client = await createMCPClient({
      name,
      onUncaughtError,
      transport: new StdioMCPTransport(serverConfig),
});

return Response.json(...)

After the createMCPClient call, we discovered that the original serverConfig.env was being modified. This could cause problems when:

  1. The same config object is reused for multiple operations
  2. The config needs to be stored back to the database in its original form
  3. The config is used for comparison or validation later in the code

This change preserves the immutability of configuration objects throughout the application, making the code more predictable and preventing hard-to-trace bugs that might occur when objects are unexpectedly modified.

image

@lgrammel lgrammel changed the title Fix: Prevent mutation of customEnv in get-environment.ts fix (core): prevent mutation of customEnv Apr 7, 2025
@lgrammel
Copy link
Collaborator

lgrammel commented Apr 7, 2025

  1. please add a patch changeset for ai with the PR title as description
  2. please add tests for get-environment.ts that reproduce the issue

@lgrammel lgrammel changed the title fix (core): prevent mutation of customEnv fix (ai/mcp): prevent mutation of customEnv Apr 7, 2025
@lgrammel lgrammel merged commit 3e88f4d into vercel:main Apr 7, 2025
6 of 7 checks passed
@cgoinglove
Copy link
Contributor Author

cgoinglove commented Apr 7, 2025

@lgrammel Thanks for your review!

Let's gooo 🚀

samdenty added a commit that referenced this pull request Apr 7, 2025
* origin/v5:
  fix (ai/mcp): prevent mutation of customEnv (#5583)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants