From 81c59440bfac4c1ff3f249116336ecc61e44d482 Mon Sep 17 00:00:00 2001 From: 777genius Date: Fri, 17 Apr 2026 14:21:17 +0300 Subject: [PATCH] fix(extensions): harden mcp diagnostics output --- .../runtime/ExtensionsRuntimeAdapter.ts | 2 +- .../runtime/mcpDiagnosticsParser.ts | 51 +++++++++++++++++-- .../extensions/mcp/McpServersPanel.tsx | 23 +++++++-- src/renderer/store/slices/extensionsSlice.ts | 10 +++- src/shared/types/extensions/mcp.ts | 2 + src/shared/utils/extensionNormalizers.ts | 8 +++ .../McpHealthDiagnosticsService.test.ts | 9 +++- test/renderer/store/extensionsSlice.test.ts | 35 +++++++++++++ 8 files changed, 128 insertions(+), 12 deletions(-) diff --git a/src/main/services/extensions/runtime/ExtensionsRuntimeAdapter.ts b/src/main/services/extensions/runtime/ExtensionsRuntimeAdapter.ts index 66c21893..0b53f392 100644 --- a/src/main/services/extensions/runtime/ExtensionsRuntimeAdapter.ts +++ b/src/main/services/extensions/runtime/ExtensionsRuntimeAdapter.ts @@ -12,7 +12,7 @@ import type { CliFlavor } from '@shared/types'; import type { InstalledMcpEntry, McpServerDiagnostic } from '@shared/types/extensions'; const MCP_LIST_TIMEOUT_MS = 15_000; -const MCP_DIAGNOSE_TIMEOUT_MS = 30_000; +const MCP_DIAGNOSE_TIMEOUT_MS = 60_000; export interface ExtensionsRuntimeAdapter { readonly flavor: CliFlavor; diff --git a/src/main/services/extensions/runtime/mcpDiagnosticsParser.ts b/src/main/services/extensions/runtime/mcpDiagnosticsParser.ts index 5ab55bed..911b054f 100644 --- a/src/main/services/extensions/runtime/mcpDiagnosticsParser.ts +++ b/src/main/services/extensions/runtime/mcpDiagnosticsParser.ts @@ -3,6 +3,8 @@ import type { McpServerDiagnostic, McpServerHealthStatus } from '@shared/types/e interface McpDiagnoseJsonEntry { name?: string; target?: string; + scope?: 'local' | 'user' | 'project' | 'global' | 'dynamic' | 'managed'; + transport?: string; status?: 'connected' | 'needs-authentication' | 'failed' | 'timeout'; statusLabel?: string; } @@ -12,6 +14,10 @@ interface McpDiagnoseJsonPayload { diagnostics?: McpDiagnoseJsonEntry[]; } +const EMBEDDED_HTTP_URL_PATTERN = /https?:\/\/[^\s"'`]+/gi; +const SENSITIVE_FLAG_VALUE_PATTERN = + /(--(?:api[-_]?key|access[-_]?token|auth[-_]?token|token|secret|password|client[-_]?secret))(?:=([^\s]+)|\s+([^\s]+))/gi; + function extractJsonObject(raw: string): T { const trimmed = raw.trim(); try { @@ -45,6 +51,42 @@ function parseStatusChunk(statusChunk: string): { } } +function redactHttpUrl(urlString: string): string { + try { + const parsed = new URL(urlString); + if (parsed.protocol !== 'http:' && parsed.protocol !== 'https:') { + return urlString; + } + + if (!parsed.username && !parsed.password && !parsed.search && !parsed.hash) { + return urlString; + } + + if (parsed.username) parsed.username = '***'; + if (parsed.password) parsed.password = '***'; + + for (const key of new Set(parsed.searchParams.keys())) { + parsed.searchParams.set(key, 'REDACTED'); + } + + if (parsed.hash) { + parsed.hash = 'REDACTED'; + } + + return parsed.toString(); + } catch { + return urlString; + } +} + +function redactDiagnosticTarget(target: string): string { + return target + .replace(EMBEDDED_HTTP_URL_PATTERN, (match) => redactHttpUrl(match)) + .replace(SENSITIVE_FLAG_VALUE_PATTERN, (_match, flag: string, inlineValue?: string) => + inlineValue ? `${flag}=REDACTED` : `${flag} REDACTED` + ); +} + function parseDiagnosticLine(line: string, checkedAt: number): McpServerDiagnostic | null { const statusSeparatorIdx = line.lastIndexOf(' - '); if (statusSeparatorIdx === -1) { @@ -60,7 +102,7 @@ function parseDiagnosticLine(line: string, checkedAt: number): McpServerDiagnost } const name = descriptor.slice(0, nameSeparatorIdx).trim(); - const target = descriptor.slice(nameSeparatorIdx + 2).trim(); + const target = redactDiagnosticTarget(descriptor.slice(nameSeparatorIdx + 2).trim()); if (!name || !target) { return null; } @@ -102,6 +144,7 @@ export function parseMcpDiagnosticsJsonOutput(output: string): McpServerDiagnost return []; } + const redactedTarget = redactDiagnosticTarget(entry.target); const normalizedStatus: McpServerHealthStatus = entry.status === 'connected' ? 'connected' @@ -111,11 +154,13 @@ export function parseMcpDiagnosticsJsonOutput(output: string): McpServerDiagnost ? 'failed' : 'unknown'; - const rawLine = `${entry.name}: ${entry.target} - ${entry.statusLabel}`; + const rawLine = `${entry.name}: ${redactedTarget} - ${entry.statusLabel}`; return [ { name: entry.name, - target: entry.target, + target: redactedTarget, + scope: entry.scope, + transport: entry.transport, status: normalizedStatus, statusLabel: entry.statusLabel, rawLine, diff --git a/src/renderer/components/extensions/mcp/McpServersPanel.tsx b/src/renderer/components/extensions/mcp/McpServersPanel.tsx index e321a1bd..c799433a 100644 --- a/src/renderer/components/extensions/mcp/McpServersPanel.tsx +++ b/src/renderer/components/extensions/mcp/McpServersPanel.tsx @@ -17,6 +17,7 @@ import { useStore } from '@renderer/store'; import { formatRelativeTime } from '@renderer/utils/formatters'; import { CLI_NOT_FOUND_MARKER } from '@shared/constants/cli'; import { + getMcpDiagnosticKey, getPreferredMcpInstallationEntry, sanitizeMcpServerName, } from '@shared/utils/extensionNormalizers'; @@ -164,7 +165,12 @@ export const McpServersPanel = ({ const getDiagnostic = (server: McpCatalogItem): McpServerDiagnostic | null => { const installedEntry = getInstalledEntry(server); - return installedEntry ? (mcpDiagnostics[installedEntry.name] ?? null) : null; + return installedEntry + ? (mcpDiagnostics[getMcpDiagnosticKey(installedEntry.name, installedEntry.scope)] ?? + mcpDiagnostics[getMcpDiagnosticKey(installedEntry.name)] ?? + mcpDiagnostics[installedEntry.name] ?? + null) + : null; }; const allDiagnostics = useMemo( @@ -250,11 +256,18 @@ export const McpServersPanel = ({
{allDiagnostics.map((diagnostic) => (
-

{diagnostic.name}

+
+

{diagnostic.name}

+ {diagnostic.scope && ( + + {diagnostic.scope} + + )} +

) : ( -

Waiting for `claude mcp list` results...

+

+ Waiting for {diagnosticsCommand} results... +

)}
)} diff --git a/src/renderer/store/slices/extensionsSlice.ts b/src/renderer/store/slices/extensionsSlice.ts index c73b6b70..15fb4556 100644 --- a/src/renderer/store/slices/extensionsSlice.ts +++ b/src/renderer/store/slices/extensionsSlice.ts @@ -6,7 +6,11 @@ import { api } from '@renderer/api'; import { CLI_NOT_FOUND_MESSAGE } from '@shared/constants/cli'; import { isProjectScopedMcpScope } from '@shared/utils/mcpScopes'; -import { getMcpOperationKey, getPluginOperationKey } from '@shared/utils/extensionNormalizers'; +import { + getMcpDiagnosticKey, + getMcpOperationKey, + getPluginOperationKey, +} from '@shared/utils/extensionNormalizers'; import { findPaneByTabId, updatePane } from '../utils/paneHelpers'; @@ -555,7 +559,9 @@ export const createExtensionsSlice: StateCreator [entry.name, entry] as const) + diagnostics.map( + (entry) => [getMcpDiagnosticKey(entry.name, entry.scope), entry] as const + ) ), mcpDiagnosticsLoading: false, mcpDiagnosticsLastCheckedAt: Date.now(), diff --git a/src/shared/types/extensions/mcp.ts b/src/shared/types/extensions/mcp.ts index ec92a6b9..dcd720db 100644 --- a/src/shared/types/extensions/mcp.ts +++ b/src/shared/types/extensions/mcp.ts @@ -92,6 +92,8 @@ export type McpServerHealthStatus = 'connected' | 'needs-authentication' | 'fail export interface McpServerDiagnostic { name: string; target: string; + scope?: 'local' | 'user' | 'project' | 'global' | 'dynamic' | 'managed'; + transport?: string; status: McpServerHealthStatus; statusLabel: string; rawLine: string; diff --git a/src/shared/utils/extensionNormalizers.ts b/src/shared/utils/extensionNormalizers.ts index b0cdb96b..5601f944 100644 --- a/src/shared/utils/extensionNormalizers.ts +++ b/src/shared/utils/extensionNormalizers.ts @@ -120,6 +120,14 @@ export function getMcpOperationKey(registryId: string, scope: InstallScope): str return `mcp:${registryId}:${scope}`; } +/** + * Namespaced lookup key for MCP diagnostics. Scope is included when available + * so the same server name can coexist across global/project/local installs. + */ +export function getMcpDiagnosticKey(name: string, scope?: string | null): string { + return scope ? `mcp-diagnostic:${scope}:${name}` : `mcp-diagnostic:${name}`; +} + /** * Check whether a plugin has an installation for the selected scope. */ diff --git a/test/main/services/extensions/McpHealthDiagnosticsService.test.ts b/test/main/services/extensions/McpHealthDiagnosticsService.test.ts index afb7b4eb..c3db915f 100644 --- a/test/main/services/extensions/McpHealthDiagnosticsService.test.ts +++ b/test/main/services/extensions/McpHealthDiagnosticsService.test.ts @@ -25,7 +25,7 @@ alpic: https://mcp.alpic.ai (HTTP) - ! Needs authentication`); }); expect(diagnostics[3]).toMatchObject({ name: 'tavily-remote-mcp', - target: 'npx -y mcp-remote https://mcp.tavily.com/mcp/?tavilyApiKey=test', + target: 'npx -y mcp-remote https://mcp.tavily.com/mcp/?tavilyApiKey=REDACTED', status: 'failed', statusLabel: 'Failed to connect', }); @@ -58,7 +58,9 @@ another log line`); }, { name: 'tavily', - target: 'https://mcp.tavily.com/mcp', + target: 'https://mcp.tavily.com/mcp?token=secret', + scope: 'global', + transport: 'http', status: 'timeout', statusLabel: 'Timed out', }, @@ -74,6 +76,9 @@ another log line`); }), expect.objectContaining({ name: 'tavily', + target: 'https://mcp.tavily.com/mcp?token=REDACTED', + scope: 'global', + transport: 'http', status: 'failed', statusLabel: 'Timed out', }), diff --git a/test/renderer/store/extensionsSlice.test.ts b/test/renderer/store/extensionsSlice.test.ts index ceca353d..c9a5337c 100644 --- a/test/renderer/store/extensionsSlice.test.ts +++ b/test/renderer/store/extensionsSlice.test.ts @@ -55,6 +55,7 @@ vi.mock('../../../src/renderer/api', () => ({ import { api } from '../../../src/renderer/api'; import { + getMcpDiagnosticKey, getMcpOperationKey, getPluginOperationKey, } from '../../../src/shared/utils/extensionNormalizers'; @@ -833,6 +834,40 @@ describe('extensionsSlice', () => { expect(api.cliInstaller!.getStatus).toHaveBeenCalled(); expect(store.getState().apiKeys).toEqual([]); }); + + it('keys MCP diagnostics by scope when the same server exists in multiple scopes', async () => { + (api.mcpRegistry!.diagnose as ReturnType).mockResolvedValue([ + { + name: 'context7', + scope: 'global', + target: 'npx -y @upstash/context7-mcp', + status: 'connected', + statusLabel: 'Connected', + rawLine: 'context7: npx -y @upstash/context7-mcp - Connected', + checkedAt: 1, + }, + { + name: 'context7', + scope: 'project', + target: 'uvx context7-project', + status: 'failed', + statusLabel: 'Failed to connect', + rawLine: 'context7: uvx context7-project - Failed to connect', + checkedAt: 1, + }, + ]); + + await store.getState().runMcpDiagnostics('/tmp/project-a'); + + expect(store.getState().mcpDiagnostics).toMatchObject({ + [getMcpDiagnosticKey('context7', 'global')]: expect.objectContaining({ + target: 'npx -y @upstash/context7-mcp', + }), + [getMcpDiagnosticKey('context7', 'project')]: expect.objectContaining({ + target: 'uvx context7-project', + }), + }); + }); }); describe('skills state hardening', () => {