fix(mcp): cover the remaining prototype-name mutation and doctor paths

The own-property gate only covered unscoped removal and getMcpConfigByName.
The scoped paths still used bare lookups:

- removeMcpConfig's project/user/local existence checks accepted inherited
  members, so 'mcp remove constructor -s user' reported a successful removal
  while leaving the configuration untouched;
- addMcpConfig's already-exists checks rejected valid 'mcp add constructor';
- doctor's servers[name] and activeServers[name] fabricated definitions for
  prototype names.

Gate all of them on Object.hasOwn. Also reject the name '__proto__' at add
time: it passes the character check but assigning it on a plain object hits
the prototype setter instead of creating an own property, so the server
would be reported as added and silently vanish.

Restore NODE_ENV in the test teardown alongside the config state.

Author: 0xfandom

src/services/mcp/config.protoName.test.ts

@@ -6,7 +6,11 @@ import {
   saveCurrentProjectConfig,
   saveGlobalConfig,
 } from '../../utils/config.js'
-import { getMcpConfigByName } from './config.js'
+import {
+  addMcpConfig,
+  getMcpConfigByName,
+  removeMcpConfig,
+} from './config.js'
 
 // The MCP `servers` maps are plain objects built from JSON config, so a bare
 // `servers[name]` lookup exposes inherited Object.prototype members. A user
@@ -24,7 +28,10 @@ const PROTO_NAMES = [
 let savedGlobalMcp: ReturnType<typeof getGlobalConfig>['mcpServers']
 let savedProjectMcp: ReturnType<typeof getCurrentProjectConfig>['mcpServers']
 
+let savedNodeEnv: string | undefined
+
 beforeEach(() => {
+  savedNodeEnv = process.env.NODE_ENV
   process.env.NODE_ENV = 'test'
   savedGlobalMcp = getGlobalConfig().mcpServers
   savedProjectMcp = getCurrentProjectConfig().mcpServers
@@ -44,6 +51,11 @@ afterEach(() => {
     ...config,
     mcpServers: savedProjectMcp,
   }))
+  if (savedNodeEnv === undefined) {
+    delete process.env.NODE_ENV
+  } else {
+    process.env.NODE_ENV = savedNodeEnv
+  }
 })
 
 test('resolves a real server by name', () => {
@@ -64,3 +76,30 @@ test('returns null for Object.prototype member names', () => {
     expect(getMcpConfigByName(name)).toBeNull()
   }
 })
+
+test('rejects adding a server named __proto__', async () => {
+  // "__proto__" passes the character check but assigning it on a plain object
+  // hits the prototype setter, so the server would be reported as added and
+  // silently vanish rather than becoming an own property.
+  await expect(
+    addMcpConfig('__proto__', { command: 'echo', args: [] }, 'user'),
+  ).rejects.toThrow('reserved')
+})
+
+test('reports proto-name removal as not found instead of succeeding', async () => {
+  // Before the fix the scoped-removal existence checks accepted inherited
+  // members, so `mcp remove constructor -s user` claimed success while leaving
+  // the real configuration untouched.
+  for (const name of PROTO_NAMES) {
+    await expect(removeMcpConfig(name, 'user')).rejects.toThrow(
+      'No user-scoped MCP server found',
+    )
+  }
+})
+
+test('still allows adding and removing a real server name', async () => {
+  await addMcpConfig('addedserver', { command: 'echo', args: [] }, 'user')
+  expect(getMcpConfigByName('addedserver')).not.toBeNull()
+  await removeMcpConfig('addedserver', 'user')
+  expect(getMcpConfigByName('addedserver')).toBeNull()
+})

src/services/mcp/config.ts

@@ -633,6 +633,13 @@ export async function addMcpConfig(
     )
   }
 
+  // "__proto__" passes the character check but cannot be stored: assigning it
+  // on a plain object invokes the prototype setter instead of creating an own
+  // property, so the server would be reported as added and silently vanish.
+  if (name === '__proto__') {
+    throw new Error(`Cannot add MCP server "${name}": this name is reserved.`)
+  }
+
   // Block reserved server name "claude-in-chrome"
   if (isClaudeInChromeMCPServer(name)) {
     throw new Error(`Cannot add MCP server "${name}": this name is reserved.`)
@@ -682,21 +689,21 @@ export async function addMcpConfig(
   switch (scope) {
     case 'project': {
       const { servers } = getProjectMcpConfigsFromCwd()
-      if (servers[name]) {
+      if (Object.hasOwn(servers, name)) {
         throw new Error(`MCP server ${name} already exists in .mcp.json`)
       }
       break
     }
     case 'user': {
       const globalConfig = getGlobalConfig()
-      if (globalConfig.mcpServers?.[name]) {
+      if (Object.hasOwn(globalConfig.mcpServers ?? {}, name)) {
         throw new Error(`MCP server ${name} already exists in user config`)
       }
       break
     }
     case 'local': {
       const projectConfig = getCurrentProjectConfig()
-      if (projectConfig.mcpServers?.[name]) {
+      if (Object.hasOwn(projectConfig.mcpServers ?? {}, name)) {
         throw new Error(`MCP server ${name} already exists in local config`)
       }
       break
@@ -774,7 +781,7 @@ export async function removeMcpConfig(
     case 'project': {
       const { servers: existingServers } = getProjectMcpConfigsFromCwd()
 
-      if (!existingServers[name]) {
+      if (!Object.hasOwn(existingServers, name)) {
         throw new Error(`No MCP server found with name: ${name} in .mcp.json`)
       }
 
@@ -799,7 +806,7 @@ export async function removeMcpConfig(
 
     case 'user': {
       const config = getGlobalConfig()
-      if (!config.mcpServers?.[name]) {
+      if (!Object.hasOwn(config.mcpServers ?? {}, name)) {
         throw new Error(`No user-scoped MCP server found with name: ${name}`)
       }
       saveGlobalConfig(current => {
@@ -815,7 +822,7 @@ export async function removeMcpConfig(
     case 'local': {
       // Check if server exists before updating
       const config = getCurrentProjectConfig()
-      if (!config.mcpServers?.[name]) {
+      if (!Object.hasOwn(config.mcpServers ?? {}, name)) {
         throw new Error(`No project-local MCP server found with name: ${name}`)
       }
       saveCurrentProjectConfig(current => {

src/services/mcp/doctor.ts

@@ -240,7 +240,10 @@ function buildScopeDefinitions(
   activeConfig: ScopedMcpServerConfig | undefined,
   deps: McpDoctorDependencies,
 ): McpDoctorDefinition[] {
-  const config = servers[name]
+  // Own-property lookup: these maps are plain objects from JSON config, so a
+  // bare servers[name] resolves inherited Object.prototype members and would
+  // fabricate a definition for a name like 'constructor'.
+  const config = Object.hasOwn(servers, name) ? servers[name] : undefined
   if (!config) {
     return []
   }
@@ -540,7 +543,9 @@ async function buildServerReport(
   }
   const { servers: activeServers } = await deps.getAllMcpConfigs()
   const serverDisabled = deps.isMcpServerDisabled(name)
-  const runtimeConfig = activeServers[name] ?? undefined
+  const runtimeConfig = Object.hasOwn(activeServers, name)
+    ? activeServers[name]
+    : undefined
   const activeConfig = serverDisabled ? undefined : runtimeConfig
 
   const definitions = [