Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
95 changes: 94 additions & 1 deletion crates/host-core/src/permissions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,10 @@ impl PermissionManager {
// low-risk grant. Medium preserves the normal approval path.
_ => Risk::Medium,
},
name if name.starts_with("mcp_") => Risk::Low,
// MCP servers are user-configured but their tools are opaque; a
// self-declared annotation comes from the server, so it is not
// trusted and never lowers the approval path.
name if name.starts_with("mcp_") => Risk::Medium,
_ => Risk::Medium,
}
}
Expand Down Expand Up @@ -695,6 +698,96 @@ mod tests {
assert_eq!(d, Some(PermissionDecision::AllowSession));
}

#[test]
fn mcp_tools_default_to_medium_and_ignore_declared_risk() {
for declared in [
None,
Some("low"),
Some("medium"),
Some("high"),
Some("bogus"),
] {
assert!(
matches!(
PermissionManager::tool_risk_with_declared("mcp_srv_tool", declared),
Risk::Medium
),
"mcp tool with declared {declared:?} must be medium"
);
}
}

#[test]
fn mcp_tools_prompt_under_ask_and_accept_edits() {
let pm = PermissionManager::default();
for mode in ["ask", "accept-edits"] {
for declared in [None, Some("low")] {
let d =
pm.evaluate_auto_with_permission_mode_and_risk(PermissionEvaluationParams {
session_id: "s",
tool_name: "mcp_srv_tool",
mode: "agent",
permission_mode: mode,
session_grants: &no_grants(),
declared_risk: declared,
requires_external_path_permission: false,
plan_safe_actions: None,
});
assert!(
d.is_none(),
"mcp tool must prompt under {mode} ({declared:?})"
);
}
}
}

#[test]
fn mcp_tools_auto_allow_under_auto() {
let pm = PermissionManager::default();
let d = pm.evaluate_auto_with_permission_mode(
"s",
"mcp_srv_tool",
"agent",
"auto",
&no_grants(),
);
assert_eq!(d, Some(PermissionDecision::AllowOnce));
}

#[test]
fn mcp_session_grant_skips_prompt() {
let pm = PermissionManager::default();
let mut grants = HashMap::new();
grants.insert("s".to_string(), vec!["mcp_srv_tool".to_string()]);
let d = pm.evaluate_auto_with_permission_mode("s", "mcp_srv_tool", "agent", "ask", &grants);
assert_eq!(d, Some(PermissionDecision::AllowSession));
let other =
pm.evaluate_auto_with_permission_mode("s", "mcp_srv_other", "agent", "ask", &grants);
assert!(other.is_none(), "grant is scoped to the exact tool name");
}

#[test]
fn mcp_tools_denied_in_contract_modes() {
let pm = PermissionManager::default();
let mut grants = HashMap::new();
grants.insert("s".to_string(), vec!["mcp_srv_tool".to_string()]);
for contract in ["plan", "goal"] {
for mode in ["ask", "accept-edits", "auto"] {
assert_eq!(
pm.evaluate_auto_with_permission_mode(
"s",
"mcp_srv_tool",
contract,
mode,
&grants
),
Some(PermissionDecision::Deny),
"mcp tool must be denied in {contract} + {mode}"
);
}
}
}

#[test]
fn contract_mode_admits_plugin_tools_only_with_plan_safe_actions() {
let pm = PermissionManager::default();
Expand Down
7 changes: 4 additions & 3 deletions crates/host-core/src/tools/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -954,9 +954,10 @@ fn count_lines_fast(path: &Path) -> std::io::Result<usize> {
/// user's MCP servers both live in Electron main, so both are forwarded over
/// `plugins.execute` instead of being executed here.
///
/// `mcp_` is treated exactly like `plugin_` for risk and read-only-mode
/// purposes: the user typed the command or URL into the MCP editor themselves,
/// which is at least as deliberate as accepting a plugin's manifest.
/// `mcp_` is treated like `plugin_` for dispatch and read-only-mode purposes.
/// For risk it matches a plugin tool without a valid declaration (`medium`):
/// the user configured the server, but its tools and any risk they self-declare
/// are opaque, so they keep the normal approval path (`permissions.rs`).
pub fn is_desktop_dispatched(tool_name: &str) -> bool {
tool_name.starts_with("plugin_") || tool_name.starts_with("mcp_")
}
Expand Down
1 change: 1 addition & 0 deletions docs/adr/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ Each ADR includes:

| ID | Title | Status |
|---|---|---|
| mcp-tool-approval-risk | [User MCP tools keep the normal approval path](mcp-tool-approval-risk.md) | Accepted |
| models-dev-catalog-authority | [models.dev owns published model metadata](models-dev-catalog-authority.md) | Accepted for implementation |
| pi-ai-core-0991-authority | [Pi 0.99.1 account model authority](pi-ai-core-0991-authority.md) | Superseded for chat model metadata |
| plan-tool-declarations-and-execution-denials | [Keep known tool declarations while denying contract-mode execution](plan-tool-declarations-and-execution-denials.md) | Accepted for implementation |
Expand Down
40 changes: 40 additions & 0 deletions docs/adr/mcp-tool-approval-risk.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
# ADR: User MCP tools keep the normal approval path

- Status: Accepted
- Date: 2026-10-03
- Decision: D640

## Context

User-configured MCP servers expose tools under the `mcp_<serverId>_<tool>`
namespace (ADR 0056). `PermissionManager` classified every `mcp_` tool as
`low` risk, which auto-allows in every mode, so under `ask` a server's tools
could write files, reach the network or run commands without an approval card.
The user chose to launch the server, but its tool list and behavior come from
the server and can change between versions. Servers may also annotate their
own tools as read-only or low risk; that claim comes from the party being
gated.

## Decision

Classify `mcp_` tools as `medium` risk, the same as a plugin tool without a
valid declaration. Under `ask` and `accept-edits` each call shows the approval
card with the reason "MCP server tool requires approval"; allow-once covers
one call and allow-session covers that exact tool name for the session. `auto`
runs the tool without a card, and Plan/Goal keep denying it. Server-declared
risk or annotations are ignored and never lower the path.

Dispatch over `plugins.execute`, read-only-mode handling and the `mcp_`
namespace are unchanged. No host protocol, schema or persistence change.

## Consequences

Users see an approval card for MCP tool calls in `ask` and `accept-edits`
until they grant the tool for the session. A per-server or per-tool persistent
allowlist, if wanted later, is a separate decision.

## Alternatives

Trusting server annotations would let a server mark itself safe. Using `high`
risk would add friction without a different settlement path, since `medium`
already prompts in every non-`auto` mode.
9 changes: 9 additions & 0 deletions docs/spec/03-runtime/03-tools-and-permissions.md
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,15 @@ Initial denylist (extensible):
| medium | low-risk network/metadata | Confirm or allow by policy |
| high | Write/Edit/Bash | Confirm by default |

Tools from user-configured MCP servers (`mcp_<serverId>_<tool>`) are classified
`medium`, the same as a plugin tool without a valid declared risk. A risk level
self-declared by an MCP server is not trusted, unlike the risk in a plugin
manifest the user accepted. Under `ask` and `accept-edits` an MCP tool call
shows an approval card with reason "MCP server tool requires approval"; an
`allow-session` grant suppresses further prompts for that tool name in that
session (grants are in-memory only). `auto` auto-allows it, and the Plan/Goal
contract-mode hard deny still applies (D640, ADR `mcp-tool-approval-risk`).

### Decision Types

- `allow-once`
Expand Down
7 changes: 7 additions & 0 deletions docs/spec/05-security/01-security.md
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,13 @@ and byte size, and only then creates the `plan_approvals` record with
structured title/question fields. Renderer and sidecar state cannot write or
replace an artifact.

Tools from user-configured MCP servers (`mcp_<serverId>_<tool>`) are never
low-risk by default: host-core classifies them `medium` and ignores any risk
level the MCP server declares for itself. `ask` and `accept-edits` require
approval (an `allow-session` grant covers the same tool name for the rest of
that session, in memory only), `auto` auto-allows, and Plan/Goal still deny
them (D640, ADR `mcp-tool-approval-risk`).

## 4.1 Skill market egress

The renderer does not fetch skill catalogs or SKILL.md documents. Electron
Expand Down
22 changes: 22 additions & 0 deletions docs/spec/06-delivery/04-e2e-test-plan.md
Original file line number Diff line number Diff line change
Expand Up @@ -3315,6 +3315,28 @@ identify the platform validation still needed.
`07-plugins/04-plugin-security.md` §8.1
- **Status**: Client and session-isolation unit-covered; full desktop journey Draft

#### E2E-MCP-tool-requires-approval: User MCP tools prompt under ask and accept-edits

- **Preconditions**: A project-bound Agent session; one user-configured stdio
MCP server whose tool list annotates a tool as read-only/low risk.
- **Steps**: 1) With the session in `ask`, ask the agent to call the MCP tool.
2) Answer the card with allow-once, call it again, then answer with
allow-session and call it a third time. 3) Switch to `accept-edits` in a new
session and repeat the call. 4) Switch to `auto` and call it. 5) Switch to
Plan, then Goal, and call it.
- **Expected**: Under `ask` and `accept-edits` every call shows an approval card
with reason "MCP server tool requires approval" at `medium` risk, regardless of
the server's self-declared annotation. Allow-once covers only that call;
allow-session suppresses further cards for the same `mcp_<serverId>_<tool>`
name in that session only, and does not cover other tools of the server.
`auto` runs the tool without a card. Plan and Goal deny it even with a
session grant.
- **Specs linked**: `03-runtime/03-tools-and-permissions.md`,
`05-security/01-security.md`, D640, ADR `mcp-tool-approval-risk`
- **Acceptance**: E (tools & permissions) + Security
- **Status**: Unit-covered (host-core `permissions.rs` MCP risk and mode tests);
desktop journey Draft

#### E2E-024L: Resident plugin service is supervised and visible

- **Preconditions**: `examples/plugins/hello` enabled with `background.service` granted.
Expand Down
14 changes: 14 additions & 0 deletions docs/spec/08-meta/decisions-log.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ This log freezes previously open questions into concrete decisions.
| D637 | Remove the Windows frameless resize rim | **Disable the Windows main window's thick frame while retaining Electron 43.6 native frameless edge and corner resizing. Apply a 4 DIP native rounded shape by default; authorized plugin themes may choose an integer radius from 0 to 24 DIP. Keep the D635 minimum-size contract and existing work-panel resize ownership. See ADR 0317 and E2E-167.** | The thick frame paints an unwanted left, bottom, and right rim that themes cannot remove. Native hit testing and shape keep resizing and transparent outer corners without renderer resize IPC. |
| D638 | Publish native Linux arm64 artifacts | **Amend D126 / D285 / D603 / ADR 0022: tag releases publish native Linux arm64 AppImage, deb, and rpm packages, built on GitHub's native `ubuntu-22.04-arm` runner and carrying an arm64 `pi-desktop-host-core`. The static Linux targets drop their pinned `arch` and take the workflow's `--x64` / `--arm64` flag; `linux.artifactName` becomes `PI-Desktop-<version>-linux-<arch>.AppImage`; each Linux lane verifies its architecture-named updater feed (`latest-linux.yml` on x64, `latest-linux-arm64.yml` on arm64); the ASAR export reads `linux-unpacked` or `linux-arm64-unpacked` and publishes `PI-Desktop-<version>-linux-<arch>.asar`. `pi-host-bundle` builds both Linux architectures and `PUBLISHED_TARGETS` gains `linux-arm64`. Updater ownership, signing, and delivery modes are unchanged. See ADR 0318, issue #1281, and E2E-192a.** | arm64 Linux devices could not install or run the published x64 artifact, and a cross-built or emulated lane would ship a mismatched Rust sidecar. |
| D639 | models.dev owns published chat-model metadata | **Supersede D136 / D266 and ADR `pi-ai-core-0991-authority` for chat metadata: the bundled and explicitly refreshed models.dev catalog supplies published chat-model limits, modalities, reasoning metadata, names, and prices. Prefer the selected official publisher; when it has no record, accept another publisher only for a safe, unambiguous match, otherwise keep generic metadata. The checked-in preset identities are the priority set; do not assert an unsupported fixed count of 39. Live endpoint/OAuth discovery still owns selectable IDs. Pi remains responsible for OAuth, wire identity, transport and typed non-chat operations, but never supplies sibling chat limits, reasoning or prices. Explicit user binding overrides remain authoritative. No credentials are sent to models.dev; no host schema/protocol or persistence change. See ADR `models-dev-catalog-authority` and E2E-162 / E2E-MODEL-catalog-window-correction-reaches-saved-bindings.** | A Pi sibling default assigned a 272,000-token window to GPT models whose selected models.dev records publish 1,050,000 tokens, changing the Settings display and runtime context budget. |
| D640 | User MCP tools keep the normal approval path | **`mcp_<serverId>_<tool>` calls are `medium` risk in host-core: under `ask` and `accept-edits` each call shows the approval card ("MCP server tool requires approval"), allow-once and allow-session keep their usual scope (one call / that exact tool name in that session), `auto` runs without a card, and Plan/Goal still deny. Annotations or risk values the MCP server declares about its own tools are ignored and never lower the path. Dispatch, read-only-mode handling and the `mcp_` namespace are unchanged; no host protocol or persistence change. See ADR `mcp-tool-approval-risk` and E2E-MCP-tool-requires-approval.** | MCP tools were auto-allowed as `low` risk, so a configured server could write files, call networks or run commands without any prompt under `ask`. Configuring a server is consent to launch it, not to every action its opaque tools take. |
| D450 | Signed macOS GitHub Releases | **Amend D078 / ADR 0022: GitHub tag releases Developer ID-sign, notarize (`notarytool` via electron-builder 26), staple, and Gatekeeper-verify macOS DMG/ZIP before upload, using identity `Developer ID Application: XingYu Liu (DUV63RKYTW)` / team `DUV63RKYTW` from Actions secrets (`CSC_LINK`, `CSC_KEY_PASSWORD`, `APPLE_ID`, `APPLE_APP_SPECIFIC_PASSWORD`, `APPLE_TEAM_ID`). Missing secrets fail the job. Local unsigned packaging without a certificate remains. `workflow_dispatch` may set `sign_macos: false` only for unsigned debug artifacts. Packaged macOS uses in-app `electron-updater` (ZIP + merged `latest-mac.yml`); Linux deb/rpm and Windows portable ZIP stay notify-and-link. No afterPack/afterSign adhoc codesign (ADR 0278).** | Production DMGs must open without a Gatekeeper warning, and signed macOS installs can download and restart into a new tag. See ADR 0289, E2E-196c, E2E-067A. |

## B. Secondary implementation defaults
Expand Down Expand Up @@ -7395,3 +7396,16 @@ must keep splitting are covered by `markdown-blocks.test.mjs`.
knows only Raspberry Pi CPU parts for Linux arm64. Speech-to-text and the
rest of the app have no architecture-specific dependency. See ADR 0318,
issue #1281, and E2E-192a.

## 2026-10-03 — User MCP tools keep the normal approval path (D640)

- D640 moves `mcp_` tools from `low` to `medium` risk in
`PermissionManager`. Under `ask` and `accept-edits` every call shows the
approval card with the reason "MCP server tool requires approval"; `auto`
runs it without a card, and Plan/Goal deny it even with a session grant.
- Allow-session covers only the exact `mcp_<serverId>_<tool>` name in that
session, not the server's other tools. Risk annotations the server declares
about its own tools are not trusted and never lower the path.
- Dispatch over `plugins.execute`, read-only-mode handling and the `mcp_`
namespace are unchanged. See ADR `mcp-tool-approval-risk` and
E2E-MCP-tool-requires-approval.
7 changes: 7 additions & 0 deletions docs/zh-CN/spec/03-runtime/03-tools-and-permissions.md
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,13 @@ tool/protocol 名称,请求中单独携带固定的 shell ID。
| 中等 | 低风险 network/metadata | 政策确认或允许 |
| 高 | Write/Edit/Bash | 默认确认 |

用户配置的 MCP 服务器提供的工具(`mcp_<serverId>_<tool>`)归类为 `medium`,
与未声明有效风险的插件工具相同。MCP 服务器自行声明的风险级别不被信任,
这与用户已接受的插件 manifest 中的风险不同。在 `ask` 和 `accept-edits` 下,
MCP 工具调用会显示审批卡片,原因为 "MCP server tool requires approval";
`allow-session` 授权会在该会话内对该工具名不再提示(授权仅保存在内存中)。
`auto` 自动允许,Plan/Goal 合约模式的硬拒绝仍然生效(D640,ADR `mcp-tool-approval-risk`)。

### 决策类型

- `allow-once`
Expand Down
5 changes: 5 additions & 0 deletions docs/zh-CN/spec/05-security/01-security.md
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,11 @@ CDP 插件工具在 Plan 中仍被拒绝)。 Bash 在 Plan 中仍然可用:
结构化 title/question 字段。 Renderer 和 sidecar 状态无法写入或
替换一个工件。

用户配置的 MCP 服务器提供的工具(`mcp_<serverId>_<tool>`)默认绝不视为低风险:
host-core 将其归类为 `medium`,并忽略 MCP 服务器为自身声明的任何风险级别。
`ask` 和 `accept-edits` 需要审批(`allow-session` 授权在该会话剩余时间内覆盖同一
工具名,仅保存在内存中),`auto` 自动允许,Plan/Goal 仍然拒绝(D640,ADR `mcp-tool-approval-risk`)。

## 4.1 技能市场出网

渲染层不拉取技能目录或 SKILL.md。Electron 主进程按公网策略发起 HTTPS 请求(ADR 0243 / D413,由 ADR 0272 / D436 修订):仅 `https`、共享的公网主机语法检查、`redirect: "manual"`,以及按请求实际会走的线路逐跳判定。每一跳之前,客户端都会向承载 `net.fetch` 的会话询问它自己的代理判定(`Session.resolveProxy`):`proxied` 线路上按线路判定,因此容忍解析器自身产物的那一类(`benchmark`,TUN fake-IP);`direct` 或读不出线路时默认保留完整的本地分类,回环、RFC1918、ULA、link-local、mapped IPv6 以及其他所有非公网类别一律拒绝。显式 `allowFakeIp` 选项仅可为透明路由器/TUN 部署额外放行 benchmark 占位地址。安装只通过 `skills.create` 写入 markdown。内联相邻 markdown 后仍受 128 KiB 宿主上限约束。
Expand Down
Loading
Loading