Skip to content

ai-assistant: docs/mcp: Add MCP STRIDE threat model#883

Open
illume wants to merge 1 commit into
headlamp-k8s:mainfrom
illume:docs/ai-assistant-mcp-threat-model
Open

ai-assistant: docs/mcp: Add MCP STRIDE threat model#883
illume wants to merge 1 commit into
headlamp-k8s:mainfrom
illume:docs/ai-assistant-mcp-threat-model

Conversation

@illume

@illume illume commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Adds docs/mcp/mcp-stride-threat-model.md — a STRIDE threat model for the MCP integration, covering spoofing, tampering, information disclosure, denial of service, and elevation of privilege risks with mitigations.

Assisted by Copilot.

Part of the ai-assistant MCP documentation series.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new STRIDE threat model document for the AI Assistant’s MCP integration, intended to capture key risks (Spoofing/Tampering/Repudiation/Info Disclosure/DoS/EoP) and recommended mitigations for running MCP tools via the desktop integration.

Changes:

  • Introduces a detailed MCP architecture overview (components, data flow, trust boundaries).
  • Documents STRIDE threats with mitigations, risks, and suggested fixes.
  • Provides a prioritized action list and a summary table of threats/severity/status.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +9 to +19
| Component | Location | Role |
|-----------|----------|------|
| **MCPClientCore** | `ai-common/src/mcp/MCPClientCore.ts` | Platform-agnostic client: initialize servers, discover tools, execute with validation |
| **MCPToolStateStore** | `ai-common/src/mcp/MCPToolStateStore.ts` | Per-tool enabled state and usage statistics, persisted to `mcp-tools-config.json` |
| **mcpServerConfig** | `ai-common/src/mcp/mcpServerConfig.ts` | Server config builder, env var expansion, tool argument validation |
| **MCPClient** | `app/electron/mcp/MCPClient.ts` | Electron main process wrapper, IPC handlers, confirmation dialogs |
| **ElectronMCPClient** | `ai-ui/src/mcp/electron-client.ts` | Renderer process IPC bridge via `window.desktopApi.mcp` |
| **MCPSettings** | `ai-ui/src/components/settings/MCPSettings.tsx` | Settings UI for server management |
| **MCPServerEditor** | `ai-ui/src/components/settings/MCPServerEditor.tsx` | Form dialog for adding/editing servers |
| **MCPConfigEditorDialog** | `ai-ui/src/components/settings/MCPConfigEditorDialog.tsx` | JSON editor for direct config editing |
| **MultiServerMCPClient** | `@anthropic-ai/sdk` (external) | MCP SDK client that spawns and communicates with server processes |
Comment on lines +5 to +6
The MCP (Model Context Protocol) integration allows the AI Assistant to connect to external tool servers via the MCP standard. MCP servers are spawned as local processes by the Electron main process and communicate over stdio pipes. The AI Assistant discovers available tools, validates arguments, and executes tools on behalf of the user during chat interactions.

| | |
|---|---|
| **Threat** | A tool call could hang indefinitely, blocking the AI Assistant's response flow. |
| **Mitigation** | Tool execution has a hardcoded 2-minute timeout. |
Comment on lines +265 to +270
#### D4: Synchronous File Writes Block Event Loop

| | |
|---|---|
| **Threat** | `MCPToolStateStore` uses synchronous file writes (`writeFileSync`) to persist tool state. Under heavy tool usage, this blocks the Electron main process event loop. |
| **Mitigation** | Writes occur on state changes (enablement, usage recording), not on every tool call. |
Comment on lines +156 to +160
| | |
|---|---|
| **Threat** | `validateToolArgs()` performs basic type checking but does not validate enum constraints, string patterns, or deeply nested structures. A prompt injection attack could cause the AI to pass malicious arguments that pass validation but exploit the tool. |
| **Mitigation** | Schema validation checks required fields and basic types. Returns `valid: true` on unknown schema types. |
| **Risk** | **Medium** — depends on what the tool does with the arguments. |
@illume
illume force-pushed the docs/ai-assistant-mcp-threat-model branch 2 times, most recently from 03e8445 to e3d91e8 Compare July 8, 2026 11:52
@illume
illume requested review from ashu8912 and Copilot July 8, 2026 11:54

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 5 comments.

Comment on lines +15 to +18
| **ElectronMCPClient** | `ai-ui/src/mcp/electron-client.ts` | Renderer process IPC bridge via `window.desktopApi.mcp` |
| **MCPSettings** | `ai-ui/src/components/settings/MCPSettings.tsx` | Settings UI for server management |
| **MCPServerEditor** | `ai-ui/src/components/settings/MCPServerEditor.tsx` | Form dialog for adding/editing servers |
| **MCPConfigEditorDialog** | `ai-ui/src/components/settings/MCPConfigEditorDialog.tsx` | JSON editor for direct config editing |
Comment on lines +11 to +14
| **MCPClientCore** | `ai-common/src/mcp/MCPClientCore.ts` | Platform-agnostic client: initialize servers, discover tools, execute with validation |
| **MCPToolStateStore** | `ai-common/src/mcp/MCPToolStateStore.ts` | Per-tool enabled state and usage statistics, persisted to `mcp-tools-config.json` |
| **mcpServerConfig** | `ai-common/src/mcp/mcpServerConfig.ts` | Server config builder, env var expansion, tool argument validation |
| **MCPClient** | `app/electron/mcp/MCPClient.ts` | Electron main process wrapper, IPC handlers, confirmation dialogs |
Comment on lines +98 to +99
| | |
|---|---|
| **Threat** | The `command` and `args` fields in MCP server configuration are passed directly to process spawning without sanitization. A malicious or careless configuration could include shell metacharacters, path traversal sequences, or argument injection payloads. |
| **Mitigation** | `MCPServerEditor` validates that name and command are non-empty. `MCPConfigEditorDialog` checks types. **Neither validates command content, shell metacharacters, or path traversal.** |
| **Risk** | **High** — the command is executed with full process privileges. |
| **Fix** | Validate command against an allowlist or known paths. Reject shell metacharacters (`|`, `;`, `&`, `` ` ``, `$()`) in command and args. Use `execFile` instead of shell-based execution. |
| | |
|---|---|
| **Threat** | The `MCPSettings` UI has an `autoApprove` toggle for servers. If implemented, this would bypass the confirmation flow for tool executions, removing the last line of defense against prompt-injection-driven tool abuse. |
| **Mitigation** | The `autoApprove` field appears in UI code but may not be fully implemented in `MCPClientCore`. |
STRIDE threat model for the MCP integration.

Covers the full threat surface of the MCP client-server architecture:

- Spoofing: impersonation of MCP servers; mitigated by process isolation and
  stdio-only communication (no TCP listeners).
- Tampering: malicious tool argument injection and result manipulation; mitigated
  by argument schema validation in `mcpServerConfig` and user confirmation dialogs.
- Information disclosure: sensitive data in tool outputs or server stderr;
  mitigated by scoping tool execution to approved servers only.
- Denial of service: unresponsive or crash-looping MCP servers; mitigated by
  per-call timeouts and process lifecycle management in `MCPClientCore`.
- Elevation of privilege: tool calls that exceed intended permissions; mitigated
  by the `InlineToolApprovalManager` consent flow.

Signed-off-by: René Dudfield <renedudfield@microsoft.com>
Co-authored-by: Ashu Ghildiyal <aghildiyal@microsoft.com>
Signed-off-by: Ashu Ghildiyal <aghildiyal@microsoft.com>
@illume
illume force-pushed the docs/ai-assistant-mcp-threat-model branch from e3d91e8 to df877eb Compare July 8, 2026 11:59
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