From 43d302f10c0b126f0a4498cddc138f2f6ff8cbfa Mon Sep 17 00:00:00 2001 From: chengzhi-c Date: Fri, 16 Oct 2026 00:01:33 +0800 Subject: [PATCH 1/5] feat(host-core): deny unsafe calls in Plan and Goal instead of listing safe ones MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Plan and Goal decided what they allowed by naming tools. That list answered the wrong question: it asked where a tool came from rather than what the call would do, so a read-only MCP search was refused for being an MCP tool while the mode prompt — guidance, not a boundary — told the model not to mutate anything. The gate now classifies by effect. Read-only builtins reach the ordinary permission path; a builtin outside that set is denied outright, so a tool added later is denied until someone classifies it. Bash and an unclassifiable MCP tool take a permission card in every permission mode, because neither carries a signal the host can read: what a command does is decided by the shell that runs it, and an MCP tool's name means nothing while its server's own risk annotation is not evidence. A plugin tool is admitted when it declares a non-empty planSafeActions list, which the plugin runtime also enforces, and refused otherwise — an approval the callee would reject is not a gate. A trusted-extension tool is withheld in a contract mode. It executes in the sidecar, so no host call is made for it and the permission path cannot see it. The runtime's mode filter moves to the same shape so the catalog and the host agree, and a refused card now reports an ordinary denial rather than a mode denial. --- crates/host-core/src/permissions.rs | 247 ++++++++++++------ crates/host-core/src/rpc/mod.rs | 144 ++++++---- ...n-operating-state-and-approval-boundary.md | 10 +- ...checkpoint-artifact-and-execution-epoch.md | 7 + docs/adr/0211-plan-safe-plugin-actions.md | 19 ++ ...0330-contract-modes-deny-what-is-unsafe.md | 167 ++++++++++++ docs/adr/README.md | 1 + .../03-runtime/03-tools-and-permissions.md | 94 +++++-- docs/spec/03-runtime/08-error-codes.md | 11 +- docs/spec/06-delivery/04-e2e-test-plan.md | 39 +-- docs/spec/07-plugins/03-plugin-api.md | 14 +- docs/spec/07-plugins/04-plugin-security.md | 5 +- .../12-plugin-ipc-and-host-services.md | 9 +- .../13-plugin-permissions-matrix.md | 12 +- .../03-runtime/03-tools-and-permissions.md | 79 +++--- docs/zh-CN/spec/03-runtime/08-error-codes.md | 4 +- .../spec/06-delivery/04-e2e-test-plan.md | 36 +-- docs/zh-CN/spec/07-plugins/03-plugin-api.md | 12 +- .../spec/07-plugins/04-plugin-security.md | 12 +- .../12-plugin-ipc-and-host-services.md | 8 +- .../13-plugin-permissions-matrix.md | 10 +- .../src/fixed-tool-runtime.test.ts | 14 +- .../agent-runtime/src/mode-tool-access.ts | 41 +++ packages/agent-runtime/src/runtime.test.ts | 36 +++ packages/agent-runtime/src/runtime.ts | 90 ++++--- 25 files changed, 804 insertions(+), 317 deletions(-) create mode 100644 docs/adr/0330-contract-modes-deny-what-is-unsafe.md diff --git a/crates/host-core/src/permissions.rs b/crates/host-core/src/permissions.rs index 10cb225a1e..91705d5ce2 100644 --- a/crates/host-core/src/permissions.rs +++ b/crates/host-core/src/permissions.rs @@ -110,6 +110,18 @@ pub struct PendingPermission { pub created_at: String, } +/// How a contract mode admits a tool that `PermissionManager::plan_mode_allows` +/// accepted. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ContractAdmission { + /// Fall through to the ordinary permission path. + Allowed, + /// Nothing says this call is safe, so a human decides before it runs. + Approval, + /// Denied outright by the contract mode. + Denied, +} + #[derive(Default)] pub struct PermissionManager { pending: HashMap, @@ -137,24 +149,84 @@ impl PermissionManager { } } - /// The shared contract-mode allowlist. Plan and Goal expose the same - /// read/inspect core plus Bash; only their submit tool differs, and that one - /// is a sidecar-side tool that never reaches this gate. `new_context` is - /// sidecar-side too, and listed so the two sides of the bridge agree. + /// The builtin tools a contract mode may run without a card. + /// + /// This is an allowlist, not a denylist, and that is the point: a builtin + /// added later is denied until someone decides what it does, so the mode + /// cannot widen by omission. Every entry is read-only — it inspects the + /// workspace, asks the user a question, or hands the negotiation back to + /// the host. `new_context` and `ToolSearch` never reach this gate (they are + /// sidecar-side) but are listed so the two sides of the bridge agree. + /// + /// `Write`, `Edit`, `TodoWrite`, `GenerateImages` and the delegation tools + /// stay out: they change files, session state, or spawn work. A contract + /// mode denies them outright, and no permission mode or session grant + /// re-enables them. + const CONTRACT_MODE_READ_ONLY: [&str; 7] = [ + "Read", + "Glob", + "Grep", + "BrowserPreview", + "asktool", + "new_context", + "ToolSearch", + ]; + + /// Whether a contract mode lets a call reach the ordinary permission path. + /// + /// A `true` answer is not a grant: the caller still applies risk, + /// permission mode, grants, and the external-path rule. A `false` answer is + /// a hard deny that no permission mode, session grant, or scratch path can + /// re-enable. + /// + /// This is the hard-deny question only. `Bash` and an unclassifiable MCP + /// tool answer `false` here and `Approval` from [`Self::contract_admission`], + /// so a caller that needs to distinguish a mode denial from a card the user + /// then refused must ask `contract_admission`, not this. pub fn plan_mode_allows(tool_name: &str) -> bool { - matches!( - tool_name, - "Read" | "Glob" | "Grep" | "Bash" | "BrowserPreview" | "new_context" - ) + Self::CONTRACT_MODE_READ_ONLY.contains(&tool_name) + } + + /// The contract modes' own gate. + /// + /// A read-only builtin falls through to the ordinary permission path, and + /// the one submit tool that belongs to this mode is added by the runtime. + /// `Bash` is gated rather than parsed: what a command does is decided by + /// the shell that runs it, and any static reading of the string would be + /// wrong in the permissive direction, so it takes a card every time. + /// + /// An MCP tool carries no classification signal at all — the name means + /// nothing and a server's self-declared annotation is untrusted — so it + /// also takes a card rather than running unread. A plugin tool does carry + /// one: a non-empty `planSafeActions` list means the plugin runtime already + /// restricts it to actions the user accepted, so it falls through. + /// Without that list it is refused, because the plugin runtime refuses it + /// too and an approval card the callee then ignores would be theater. + pub fn contract_admission( + tool_name: &str, + plan_safe_actions: Option<&[String]>, + ) -> ContractAdmission { + if Self::plan_mode_allows(tool_name) { + return ContractAdmission::Allowed; + } + if tool_name == "Bash" || tool_name.starts_with("mcp_") { + return ContractAdmission::Approval; + } + if tool_name.starts_with("plugin_") + && plan_safe_actions.is_some_and(|actions| !actions.is_empty()) + { + return ContractAdmission::Allowed; + } + ContractAdmission::Denied } /// Auto-decision with an effective permission mode (D115). /// /// `permission_mode` is the already-resolved effective mode — the /// caller collapses `inherit` against the global default before calling. - /// The contract modes' hard deny for unavailable tools stays above every - /// permission mode: `auto` cannot re-enable Write/Edit/plugins in Plan or - /// Goal. + /// A contract mode's hard deny stays above every permission mode: `auto` + /// cannot re-enable `Write`/`Edit` in Plan or Goal, and cannot skip the card + /// an unclassified call needs. #[cfg(test)] pub fn evaluate_auto_with_permission_mode( &self, @@ -202,30 +274,16 @@ impl PermissionManager { requires_external_path_permission, plan_safe_actions, } = params; - // The contract modes' tool allowlist is authoritative. This check - // intentionally precedes low-risk classification, auto, grants, and - // scratch paths, and covers Goal as well as Plan (D198). - // - // Plugin tools get a narrow carve-out: a plugin may declare a - // non-empty `planSafeActions` list (ADR 0211). When the runtime - // forwards that list, host-core admits the plugin tool in - // contract modes and the plugin-runtime enforces the per-action - // restriction at execute time. Without the list the plugin tool - // stays Plan-denied, exactly as ADR 0052 / ADR 0053 require. - if crate::sessions::is_contract_mode(mode) && !Self::plan_mode_allows(tool_name) { - if tool_name.starts_with("plugin_") { - if let Some(actions) = plan_safe_actions { - if !actions.is_empty() { - // Fall through; plugin-runtime will gate the - // actual action. - } else { - return Some(PermissionDecision::Deny); - } - } else { - return Some(PermissionDecision::Deny); - } - } else { - return Some(PermissionDecision::Deny); + // A contract mode decides first, and that decision precedes low-risk + // classification, auto, grants, and scratch paths, so none of them can + // re-enable what it denied (D198, and Goal shares Plan's gate). + if crate::sessions::is_contract_mode(mode) { + match Self::contract_admission(tool_name, plan_safe_actions) { + ContractAdmission::Denied => return Some(PermissionDecision::Deny), + // No permission mode short-circuits an unclassified call: an + // opaque tool is exactly the case where `auto` must not decide. + ContractAdmission::Approval => return None, + ContractAdmission::Allowed => {} } } @@ -525,61 +583,58 @@ mod tests { } } + /// A contract mode denies what changes state, whatever the permission mode + /// would otherwise allow. `auto` and `accept-edits` cannot re-enable it, + /// and neither can a session grant. #[test] - fn plan_bash_follows_permission_mode() { - let pm = PermissionManager::default(); - assert_eq!( - pm.evaluate_auto_with_permission_mode("s", "Bash", "plan", "ask", &no_grants()), - None - ); - assert_eq!( - pm.evaluate_auto_with_permission_mode("s", "Bash", "plan", "auto", &no_grants()), - Some(PermissionDecision::AllowOnce) - ); - } - - #[test] - fn plan_denial_wins_over_grants_and_scratch_exceptions() { + fn contract_modes_deny_state_changing_builtins_in_every_permission_mode() { let pm = PermissionManager::default(); let mut grants = HashMap::new(); grants.insert( "s".to_string(), - vec!["Write".to_string(), "plugin_x_run".to_string()], + vec![ + "Write".to_string(), + "Edit".to_string(), + "TodoWrite".to_string(), + "GenerateImages".to_string(), + ], ); - for tool in ["Write", "Edit", "plugin_x_run", "unknown"] { - assert_eq!( - pm.evaluate_auto_with_permission_mode("s", tool, "plan", "auto", &grants), - Some(PermissionDecision::Deny), - "{tool} must be denied in plan" - ); + for contract in ["plan", "goal"] { + for mode in ["ask", "accept-edits", "auto"] { + for tool in [ + "Write", + "Edit", + "TodoWrite", + "GenerateImages", + "an_unclassified_builtin", + ] { + assert_eq!( + pm.evaluate_auto_with_permission_mode("s", tool, contract, mode, &grants), + Some(PermissionDecision::Deny), + "{contract} + {mode} must deny {tool}" + ); + } + } } } + /// Bash is gated, not parsed. Reading the command string would be wrong in + /// the permissive direction, so a contract mode takes a card every time + /// instead of letting `auto` decide what the shell may do. #[test] - fn goal_mode_shares_plans_hard_deny_and_bash_semantics() { + fn contract_modes_always_take_a_card_for_bash() { let pm = PermissionManager::default(); let mut grants = HashMap::new(); - grants.insert( - "s".to_string(), - vec!["Write".to_string(), "plugin_x_run".to_string()], - ); - for tool in ["Write", "Edit", "plugin_x_run", "unknown"] { + grants.insert("s".to_string(), vec!["Bash".to_string()]); + for contract in ["plan", "goal"] { for mode in ["ask", "accept-edits", "auto"] { assert_eq!( - pm.evaluate_auto_with_permission_mode("s", tool, "goal", mode, &grants), - Some(PermissionDecision::Deny), - "{tool} must be denied in goal + {mode}" + pm.evaluate_auto_with_permission_mode("s", "Bash", contract, mode, &grants), + None, + "{contract} + {mode} must prompt even with a session grant" ); } } - assert_eq!( - pm.evaluate_auto_with_permission_mode("s", "Bash", "goal", "ask", &no_grants()), - None - ); - assert_eq!( - pm.evaluate_auto_with_permission_mode("s", "Bash", "goal", "auto", &no_grants()), - Some(PermissionDecision::AllowOnce) - ); } #[test] @@ -766,8 +821,12 @@ mod tests { assert!(other.is_none(), "grant is scoped to the exact tool name"); } + /// An MCP tool has no classification signal, so a contract mode routes it + /// to a card. The grant below is deliberately present and deliberately not + /// enough: `auto` must not stand in for a decision about a tool whose name + /// is all the host knows. #[test] - fn mcp_tools_denied_in_contract_modes() { + fn contract_modes_route_unclassified_mcp_tools_to_a_card() { let pm = PermissionManager::default(); let mut grants = HashMap::new(); grants.insert("s".to_string(), vec!["mcp_srv_tool".to_string()]); @@ -781,13 +840,17 @@ mod tests { mode, &grants ), - Some(PermissionDecision::Deny), - "mcp tool must be denied in {contract} + {mode}" + None, + "mcp tool must prompt in {contract} + {mode}" ); } } } + /// The escape hatch is a declaration, not a default: a plugin tool is + /// admitted only when the plugin runtime will also hold it to the same + /// action list. Without one the tool is refused, because an approval the + /// callee then rejects would be theater. #[test] fn contract_mode_admits_plugin_tools_only_with_plan_safe_actions() { let pm = PermissionManager::default(); @@ -833,6 +896,38 @@ mod tests { assert_eq!(admitted, Some(PermissionDecision::AllowOnce)); } + /// A read-only builtin keeps whatever permission behavior it already had. + /// The contract mode neither grants it nor denies it, so the decision is + /// identical to the one Agent mode reaches — including for the tools the + /// risk classifier only rates medium, which still prompt under `ask`. + #[test] + fn contract_modes_leave_read_only_builtins_on_the_normal_path() { + let pm = PermissionManager::default(); + for mode in ["ask", "accept-edits", "auto"] { + for tool in ["Read", "Glob", "Grep", "BrowserPreview", "asktool"] { + let in_agent = + pm.evaluate_auto_with_permission_mode("s", tool, "agent", mode, &no_grants()); + assert!( + in_agent != Some(PermissionDecision::Deny), + "fixture assumption: {tool} is not denied in Agent mode" + ); + for contract in ["plan", "goal"] { + assert_eq!( + pm.evaluate_auto_with_permission_mode( + "s", + tool, + contract, + mode, + &no_grants() + ), + in_agent, + "{contract} + {mode} must not change {tool}'s normal decision" + ); + } + } + } + } + #[test] fn args_preview_truncates_long_strings() { let mut pm = PermissionManager::default(); diff --git a/crates/host-core/src/rpc/mod.rs b/crates/host-core/src/rpc/mod.rs index ea707e45fa..38ebf48fb3 100644 --- a/crates/host-core/src/rpc/mod.rs +++ b/crates/host-core/src/rpc/mod.rs @@ -22,7 +22,9 @@ use crate::artifacts; use crate::audit; use crate::background_tasks; use crate::notifications; -use crate::permissions::{PermissionDecision, PermissionEvaluationParams, PermissionManager}; +use crate::permissions::{ + ContractAdmission, PermissionDecision, PermissionEvaluationParams, PermissionManager, +}; use crate::plans; use crate::plugin_sessions; use crate::plugin_usage; @@ -4214,8 +4216,14 @@ async fn handle_request( let error_code = if cancelled { "TOOL_ABORTED" } else if sessions::is_contract_mode(&durable_mode) - && !PermissionManager::plan_mode_allows(&p.tool_name) + && PermissionManager::contract_admission( + &p.tool_name, + p.plan_safe_actions.as_deref(), + ) == ContractAdmission::Denied { + // A mode denial, not a card the user refused: `Bash` and + // an unclassifiable MCP tool reach a card, so denying + // one of those is an ordinary `TOOL_DENIED`. match p.tool_name.as_str() { "Write" => "WRITE_DISABLED_IN_PLAN", "Edit" => "EDIT_DISABLED_IN_PLAN", @@ -9789,7 +9797,7 @@ mod tests { } #[tokio::test] - async fn plan_authorization_uses_durable_mode_and_keeps_bash_permission_semantics() { + async fn plan_authorization_uses_durable_mode_and_gates_bash_behind_a_card() { let data_dir = tempfile::tempdir().unwrap(); let project = data_dir.path().join("project"); fs::create_dir_all(&project).unwrap(); @@ -9855,72 +9863,81 @@ mod tests { assert_eq!(result["ok"], false); } - let bash = handle_request( + // Bash is gated, not parsed: a contract mode takes a card even under + // `auto`, because what a command does is decided by the shell that runs + // it and reading the string would be wrong in the permissive direction. + let (notify_tx, mut notify_rx) = mpsc::unbounded_channel(); + let pending_state = state.clone(); + let pending_session = session_id.clone(); + let pending_task = tokio::spawn(async move { + handle_request( + pending_state, + "tools.execute", + json!({ + "sessionId": pending_session, + "toolCallId": "bash-auto", + "toolName": "Bash", + "args": { "command": plan_bash_command }, + "expectedCommandShellId": command_shell_id, + "expectedCommandShellDialect": crate::tools::shell::dialect_for_id(command_shell_id), + "mode": "agent" + }), + notify_tx, + ) + .await + }); + let notification = notify_rx.recv().await.unwrap(); + let notification: Value = serde_json::from_str(¬ification).unwrap(); + assert_eq!(notification["method"], "permissions.request"); + let request_id = notification["params"]["requestId"].as_str().unwrap(); + handle_request( state.clone(), - "tools.execute", - json!({ - "sessionId": session_id, - "toolCallId": "bash-auto", - "toolName": "Bash", - "args": { "command": plan_bash_command }, - "expectedCommandShellId": command_shell_id, - "expectedCommandShellDialect": crate::tools::shell::dialect_for_id(command_shell_id), - "mode": "agent" - }), + "permissions.resolve", + json!({ "requestId": request_id, "decision": "allow-once" }), tx.clone(), ) .await .unwrap(); + let bash = pending_task.await.unwrap().unwrap(); assert_eq!(bash["ok"], true); assert_eq!(bash["content"]["stdout"], "plan-bash"); - sessions::configure_session_with_thinking( - &state.lock().await.db, - &session_id, - "plan", - None, - None, - None, - Some("ask"), - ) - .unwrap(); - #[cfg(windows)] - let ask_bash_command = "[Console]::Out.Write('ask-bash')"; - #[cfg(not(windows))] - let ask_bash_command = "printf ask-bash"; - let (notify_tx, mut notify_rx) = mpsc::unbounded_channel(); - let pending_state = state.clone(); - let pending_task = tokio::spawn(async move { + // A card the user refuses is an ordinary denial, not a mode denial: the + // mode offered the call, the user declined it. + let (deny_tx, mut deny_rx) = mpsc::unbounded_channel(); + let denied_state = state.clone(); + let denied_session = session_id.clone(); + let denied_task = tokio::spawn(async move { handle_request( - pending_state, + denied_state, "tools.execute", json!({ - "sessionId": session_id, - "toolCallId": "bash-ask", + "sessionId": denied_session, + "toolCallId": "bash-denied", "toolName": "Bash", - "args": { "command": ask_bash_command }, + "args": { "command": plan_bash_command }, "expectedCommandShellId": command_shell_id, "expectedCommandShellDialect": crate::tools::shell::dialect_for_id(command_shell_id), "mode": "agent" }), - notify_tx, + deny_tx, ) .await }); - let notification = notify_rx.recv().await.unwrap(); + let notification = deny_rx.recv().await.unwrap(); let notification: Value = serde_json::from_str(¬ification).unwrap(); - assert_eq!(notification["method"], "permissions.request"); let request_id = notification["params"]["requestId"].as_str().unwrap(); handle_request( state, "permissions.resolve", - json!({ "requestId": request_id, "decision": "allow-once" }), + json!({ "requestId": request_id, "decision": "deny" }), tx, ) .await .unwrap(); - let pending_result = pending_task.await.unwrap().unwrap(); - assert_eq!(pending_result["ok"], true); + let denied = denied_task.await.unwrap().unwrap(); + assert_eq!(denied["errorCode"], "TOOL_DENIED"); + assert_eq!(denied["ok"], false); } #[tokio::test] @@ -9987,28 +10004,45 @@ mod tests { assert_eq!(result["ok"], false); } - // Bash still follows the permission mode rather than the allowlist. + // Bash takes a card here too, so the strongest permission mode cannot + // stand in for a decision about what the command will do. #[cfg(windows)] let goal_bash_command = "[Console]::Out.Write('goal-bash')"; #[cfg(not(windows))] let goal_bash_command = "printf goal-bash"; let command_shell_id = crate::tools::shell::default_shell_id(); - let bash = handle_request( - state.clone(), - "tools.execute", - json!({ - "sessionId": session_id, - "toolCallId": "goal-bash-auto", - "toolName": "Bash", - "args": { "command": goal_bash_command }, - "expectedCommandShellId": command_shell_id, - "expectedCommandShellDialect": crate::tools::shell::dialect_for_id(command_shell_id), - "mode": "agent" - }), - tx.clone(), + let (notify_tx, mut notify_rx) = mpsc::unbounded_channel(); + let pending_state = state.clone(); + let pending_task = tokio::spawn(async move { + handle_request( + pending_state, + "tools.execute", + json!({ + "sessionId": session_id, + "toolCallId": "goal-bash-auto", + "toolName": "Bash", + "args": { "command": goal_bash_command }, + "expectedCommandShellId": command_shell_id, + "expectedCommandShellDialect": crate::tools::shell::dialect_for_id(command_shell_id), + "mode": "agent" + }), + notify_tx, + ) + .await + }); + let notification = notify_rx.recv().await.unwrap(); + let notification: Value = serde_json::from_str(¬ification).unwrap(); + assert_eq!(notification["method"], "permissions.request"); + let request_id = notification["params"]["requestId"].as_str().unwrap(); + handle_request( + state, + "permissions.resolve", + json!({ "requestId": request_id, "decision": "allow-once" }), + tx, ) .await .unwrap(); + let bash = pending_task.await.unwrap().unwrap(); assert_eq!(bash["ok"], true); assert_eq!(bash["content"]["stdout"], "goal-bash"); } diff --git a/docs/adr/0052-plan-operating-state-and-approval-boundary.md b/docs/adr/0052-plan-operating-state-and-approval-boundary.md index 32fe298211..11aefe54e3 100644 --- a/docs/adr/0052-plan-operating-state-and-approval-boundary.md +++ b/docs/adr/0052-plan-operating-state-and-approval-boundary.md @@ -12,6 +12,11 @@ ADR 0053 replaces this checkpoint with immutable unique host-written with no replay. The structured proposal/request-changes details below remain historical context for the superseded design. +ADR 0330 amends §3 and the "Command-text classification to permit Bash in Plan" +alternative below: a contract mode denies by what a call would do rather than by +naming allowed tools, and `Bash` takes a permission card in every permission mode +instead of being governed by the selected one. + ## Context PI-Desktop's former product selector treated Chat and Agent as two tool @@ -185,8 +190,9 @@ approval identity. ### Command-text classification to permit Bash in Plan Rejected because a general shell command cannot be proven read-only reliably. -The existing permission mode is the explicit control; Plan's Write/Edit/plugin -denials remain exact tool policy. +*(Amended by ADR 0330: the rejection stands, and it is now why Bash takes a +permission card in every permission mode rather than being auto-allowed under +`auto`. Plan's denials are an effect classification, not an exact tool policy.)* ## Related docs diff --git a/docs/adr/0053-plan-checkpoint-artifact-and-execution-epoch.md b/docs/adr/0053-plan-checkpoint-artifact-and-execution-epoch.md index 470b7c7ef5..ff7013a6a5 100644 --- a/docs/adr/0053-plan-checkpoint-artifact-and-execution-epoch.md +++ b/docs/adr/0053-plan-checkpoint-artifact-and-execution-epoch.md @@ -28,6 +28,13 @@ tool, which ADR 0064 restores as `new_context`); Write, Edit, plugin tools, and unknown tools remain host-denied. `SubmitPlan` is the only assistant tool call in its batch and is valid only for the active Plan turn. +*(Amended by ADR 0330: "unknown tools" now means any builtin outside the +read-only set, and the set is an allowlist so a builtin added later is denied +until it is classified. `Bash` takes a permission card in every permission mode +rather than following the selected one. An MCP tool is no longer denied by +provenance — it takes a card. A plugin tool without `planSafeActions` stays +denied.)* + The input is exactly: ```ts diff --git a/docs/adr/0211-plan-safe-plugin-actions.md b/docs/adr/0211-plan-safe-plugin-actions.md index cdb3b39419..2476b498cf 100644 --- a/docs/adr/0211-plan-safe-plugin-actions.md +++ b/docs/adr/0211-plan-safe-plugin-actions.md @@ -60,6 +60,11 @@ the model knows which actions it can actually call. The full plugin tool is still available to the renderer (skill catalog, plugin page); the filter is on the agent-visible tool list, not on registration. +*(Amended by ADR 0330 §5: a contract mode withholds only what it can never +permit. A plugin tool without `planSafeActions` is still withheld, because the +host denies it, but that is no longer the only reason a name can leave the +catalog — an MCP tool stays declared so its approval card is reachable.)* + ### 3. Runtime forwards the declaration to host-core When the runtime calls `tools.execute` for a `plugin_*` tool it includes @@ -77,6 +82,14 @@ gate. The list does not change any other rule: low-risk auto-allow, the external-path prompt, session grants, and the `auto` mode permission posture all apply the same way they do for any Agent-mode plugin call. +*(Amended by ADR 0330: the gate is now `contract_admission`, and it answers with +three outcomes instead of two. `Write`, `Edit`, and every builtin outside the +read-only set are denied; `Bash` and an MCP tool take a permission card, because +neither carries a usable classification signal; a `planSafeActions` plugin tool +reaches the ordinary permission path exactly as described above. "Unknown tools" +are denied because the read-only set is an allowlist, not because a name list +happens to omit them.)* + ### 5. Plugin-runtime enforces the action restriction `apps/desktop/electron/main/plugin-runtime.ts` validates each plugin @@ -121,12 +134,18 @@ process-wide and macOS spends `Cmd+W` on its own close-window command.)* - Plan and Goal modes can inspect external resources through any plugin whose author opts in. The bundled Browser plugin is the first beneficiary; MCP tools can do the same with the same declaration. + *(Amended by ADR 0330: an MCP tool needs no declaration — it takes an + approval card instead, so a server whose tools are read-only is usable + without the user maintaining a list.)* - Plugin authors remain responsible for declaring exactly which actions are read-only. A wrong declaration fails at plugin registration, not at the user's prompt. - The contract modes' hard deny for `Write`, `Edit`, and unknown tools is unchanged. A plugin that does not declare `planSafeActions` keeps the ADR 0052 / ADR 0053 behavior. + *(Amended by ADR 0330: still true, and the mechanism is now an explicit + read-only allowlist. `planSafeActions` is no longer how a plugin becomes + visible in Plan and Goal; it is how a plugin tool is admitted without a card.)* - The defense-in-depth check in the plugin-runtime means a bug in the runtime or host-core cannot escalate a Plan call into a mutation: the plugin rejects it itself. diff --git a/docs/adr/0330-contract-modes-deny-what-is-unsafe.md b/docs/adr/0330-contract-modes-deny-what-is-unsafe.md new file mode 100644 index 0000000000..91d08743a5 --- /dev/null +++ b/docs/adr/0330-contract-modes-deny-what-is-unsafe.md @@ -0,0 +1,167 @@ +# ADR 0330: Contract modes deny what is unsafe + +- Status: Accepted +- Date: 2026-10-15 +- Deciders: PI-Desktop core +- Amends: ADR 0052 §3, ADR 0053 §1, ADR 0211 §2 and §4 +- Related: ADR 0064, ADR 0087, ADR 0089, `mcp-tool-approval-risk` + +## Context + +Plan and Goal decide which tools they allow by naming them. `plan_mode_allows` +holds a short list of read-only builtins, and every other name is denied: any +MCP tool, any plugin tool without `planSafeActions`, and any builtin added after +that list was written. + +That shape has two costs. + +The first is that a name is not a classification. `mcp_ctx_docs_search_docs` +says nothing about whether it mutates, and the server's own risk annotation is +not evidence — `tool_risk_with_declared` already pins `mcp_` to `Medium` for +exactly that reason. So the list cannot distinguish a read-only search from a +write, and it answers by refusing both. A user who asks Plan mode to look +something up in a configured MCP server gets nothing back, while the mode prompt +tells the model to inspect the workspace and the contract is meant to be about +mutation, not about provenance. + +The second is that the list is the boundary. The mode prompt already tells the +model not to change anything in Plan or Goal, and that guidance is not a +boundary. The real gate has to be in the permission path, and a gate built out of +a name list answers the wrong question: it asks where a tool came from instead of +what the call would do. + +## Decision + +### 1. A contract mode denies by effect, not by name + +A read-only builtin reaches the ordinary permission path. A builtin whose purpose +is to change state is denied outright. Nothing in between is admitted by default. + +The read-only set is an allowlist: `Read`, `Glob`, `Grep`, `BrowserPreview`, +`asktool`, `new_context`, `ToolSearch`, plus the one submit tool that belongs to +the mode. `Write`, `Edit`, `TodoWrite`, `GenerateImages`, and the delegation +tools are outside it and stay denied in every permission mode. + +It is deliberately an allowlist rather than a denylist. A builtin added later is +denied until someone decides what it does, so the mode cannot widen by omission. +`auto` and `accept-edits` cannot re-enable a denied tool, and neither can a +session grant or a scratch path. + +### 2. Bash is gated, not parsed + +A shell command's effect is decided by the shell that runs it. Any static reading +of the command string would be wrong, and it would be wrong in the permissive +direction — that is the failure mode worth avoiding. + +So `Bash` takes a permission card in a contract mode in **every** permission +mode, including `auto`. The host does not try to decide whether a particular +command mutates. A `Bash` session grant still suppresses later prompts for the +rest of that session, which is how a user who has decided what they are doing +avoids answering the same question repeatedly. + +This is a behavior change: `auto` no longer auto-allows `Bash` in Plan or Goal. +The mode selector keeps its meaning for every other tool. + +### 3. An unclassifiable tool goes to a card + +An MCP tool has no classification signal at all. It is routed to a permission +card rather than admitted, and the tool stays in the model's catalog so the card +is reachable. `auto` does not stand in for the decision, because "we could not +tell" is exactly the case where an automatic policy has nothing to go on. + +The card is the fail-closed answer that still leaves the capability usable: +nothing runs until a human has seen the tool name. + +### 4. `planSafeActions` stays the plugin escape hatch + +A plugin tool does carry a classification signal, because the plugin runtime +enforces it. A non-empty `planSafeActions` list means the runtime will reject any +`action` outside the list, so the tool reaches the ordinary permission path. + +A plugin tool without that list is refused, not routed to a card. The +plugin-runtime already rejects the same call with `PERMISSION_DENIED`, so an +approval the callee then ignores would not be a gate. + +### 5. The runtime withholds only what the mode can never permit + +`isToolAllowedInMode` keeps its allowlist shape for builtins, because a builtin +withheld here is one the model cannot discover at all. An MCP tool is not +withheld: it is declared so its card is reachable. A plugin tool without +`planSafeActions` is withheld, because the host denies it anyway. + +A trusted-extension tool is withheld in a contract mode. It executes in the +sidecar process, so no `tools.execute` call reaches the host and the permission +path cannot gate it — withholding it is the only boundary that exists. This is +not a statement about the extension's trust level: an extension is user-granted +trusted code, and it keeps working in Agent mode. It is that a contract mode +must not offer a capability whose calls the host never sees. + +### 6. What this does not fix + +A plugin may contribute MCP tools, and that registration path carries no +`planSafeActions` declaration of its own. The host therefore decides from the +list the caller sends, which it cannot cross-check against the plugin registry. +An `agentTools` contribution is different: the plugin runtime refuses it in a +contract mode when the list is empty, so the declaration and the enforcement +agree. Closing the gap for plugin-contributed MCP tools means giving that path +the same runtime check, which is separate work and is not claimed here. + +## Consequences + +- Plan and Goal can use a read-only MCP tool after one approval, remembered per + tool for the session. This is the capability the per-server `planSafeTools` + list was reaching for, without a per-server declaration to maintain. +- `Bash` in Plan and Goal always shows a card. On a platform with no read-only + filesystem primitive this is the honest gate, but it is friction, and the UI + must make it visible rather than let `auto` imply otherwise. +- A mutating `Bash` command can still run once the user approves it. The gate + bounds who decides, not what the command does; a future read-only view would + bound the latter. +- A builtin that is not on the read-only list is denied until it is classified. + Adding a builtin now requires a decision about Plan and Goal, which is the + intended cost. +- `planSafeActions` is no longer how a plugin becomes visible in Plan and Goal; + it is how a plugin tool is admitted without a card. +- A trusted-extension tool is unavailable in Plan and Goal. An extension that + needs to contribute inspection capability has to do it through a plugin tool + with `planSafeActions`, which the host can gate. + +## Alternatives rejected + +### Keep the name list and extend it per server + +Rejected because it moves the maintenance burden onto the user and still cannot +classify a tool whose name carries no meaning. It also answers the wrong +question: the mode's contract is about mutation, not provenance. + +### Deny by a list of known-mutating tools + +Rejected because it fails open. A builtin or plugin tool added later would be +admitted in Plan and Goal by default, and `auto` would then allow it without a +card. The allowlist fails closed on the same input. + +### Parse `Bash` commands to find mutations + +Rejected because it cannot be made reliable. Shell syntax has too many ways to +reach the same write, and a parser that misses one produces a gate that reports +success while the mutation happens. + +### Allow opaque MCP and plugin tools outright + +Rejected because it would need a classification signal that does not exist. +Treating "unknown" as "safe" is the default this ADR exists to avoid. + +### Route an opaque plugin tool to a card as well + +Rejected because the plugin-runtime refuses the same call. A card whose approval +the callee ignores would mislead the user about what it did. + +## References + +- `crates/host-core/src/permissions.rs` +- `crates/host-core/src/rpc/mod.rs` +- `packages/agent-runtime/src/mode-tool-access.ts` +- `packages/agent-runtime/src/runtime.ts` +- `apps/desktop/electron/main/plugin-runtime.ts` +- `docs/spec/03-runtime/03-tools-and-permissions.md` +- `docs/spec/03-runtime/06-host-rpc-protocol.md` diff --git a/docs/adr/README.md b/docs/adr/README.md index 0efb820a23..426878e9e2 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -375,6 +375,7 @@ Each ADR includes: | 0327 | [Automatic remote Host version realignment](0327-remote-host-version-realignment.md) | Accepted for experimental implementation (amends ADR 0326 item 6; Linux SSH qualification pending) | | 0328 | [Open arbitrary local files in the embedded Browser](0328-browser-arbitrary-local-files.md) | Accepted (D657; amends ADR 0170; issue #1627) | | 0329 | [Isolate parallel sessions of one project in git worktrees](0329-session-worktree-isolation.md) | Accepted (Windows E2E pending) | +| 0330 | [Contract modes deny what is unsafe](0330-contract-modes-deny-what-is-unsafe.md) | Accepted (amends ADR 0052 §3, ADR 0053 §1, ADR 0211 §2 and §4) | | plugin-fetch-redirect-policy | [Host-enforced plugin fetch redirect policy](plugin-fetch-redirect-policy.md) | Accepted (implementation candidate for #1475) | | local-agent-remote-workspace-tools | [Local Agent and model requests for remote workspaces](local-agent-remote-workspace-tools.md) | Accepted for implementation | diff --git a/docs/spec/03-runtime/03-tools-and-permissions.md b/docs/spec/03-runtime/03-tools-and-permissions.md index c698857922..af2769eae1 100644 --- a/docs/spec/03-runtime/03-tools-and-permissions.md +++ b/docs/spec/03-runtime/03-tools-and-permissions.md @@ -9,10 +9,12 @@ |---|---| | Default mode | Agent | | Agent tools | Read / Glob / Grep / Write / Edit / Bash + registered plugin tools + optional Jev classifier | -| Plan tools | Read / Glob / Grep / BrowserPreview / Bash / SubmitPlan + plugin tools that declare plan-safe actions | -| Goal tools | Read / Glob / Grep / BrowserPreview / Bash / SubmitGoal + plugin tools that declare plan-safe actions | -| Plan and Goal hard deny | Write / Edit / plugin tools without `planSafeActions` / unknown tools / the other kind's submit tool | -| Plugin `planSafeActions` | Non-empty array of `action` strings; runtime hides plugin tools without one in Plan/Goal, host admits listed tools, plugin-runtime rejects any action outside the list (ADR 0211) | +| Plan tools | Read / Glob / Grep / BrowserPreview / asktool / SubmitPlan, plus MCP tools and `planSafeActions` plugin tools through a permission card | +| Goal tools | Read / Glob / Grep / BrowserPreview / asktool / SubmitGoal, plus MCP tools and `planSafeActions` plugin tools through a permission card | +| Plan and Goal hard deny | Write / Edit / TodoWrite / GenerateImages / the delegation tools / the other kind's submit tool. A builtin that is not on the read-only list is denied until it is classified | +| Plan and Goal Bash | Always takes a permission card; the permission mode does not skip it | +| Plan and Goal unclassified tools | An MCP tool has no classification signal, so it takes a permission card instead of running unread; a `plugin_` tool needs a non-empty `planSafeActions` list | +| Plugin `planSafeActions` | Non-empty array of `action` strings; host admits listed tools, plugin-runtime rejects any action outside the list (ADR 0211) | | Local permission approval | No automatic deadline; explicit decision or cancellation required | | allow-session scope | toolName | | Bash style | non-interactive; selected host catalog shell with streamed output | @@ -240,8 +242,8 @@ as binary content. touch the project. The lexical check only skips the prompt; execution still goes through the full resolver, so it is not an escape vector. Plan and Goal do not expose Write/Edit, so the scratch auto-allow rule cannot make those tools - available in either. A contract-mode Bash call may still create or mutate - scratch data when its permission mode allows it. + available in either. A contract-mode Bash call still takes its permission card, + and the command it runs may create or mutate scratch data. - **Artifacts.** Successful scratch writes are not recorded in the `artifacts` table; artifact-driven file tabs represent workspace deliverables only, while the Files surface may still browse the active @@ -513,8 +515,14 @@ 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`). +session (grants are in-memory only). `auto` auto-allows it in Agent mode. + +In Plan and Goal an MCP tool is unclassifiable — the name says nothing about +whether it mutates, and the server's own annotation is not evidence — so it takes +a card in every permission mode, `auto` included. That is the one place a +contract mode overrides the selected permission mode for a non-Bash tool. The +tool stays in the model's catalog so the card is reachable (D640, ADR +`mcp-tool-approval-risk`). ### Decision Types @@ -539,6 +547,9 @@ How high-risk tool calls get approved is governed by a **permission mode**: An explicit outside-workspace path is an exception to the low-risk row: it is auto-allowed only in `auto`; `ask` and `accept-edits` both confirm it. +A contract mode adds its own exceptions above this table: `Bash` and an +unclassifiable MCP tool confirm in every mode, including `auto`. + Resolution order per tool call (host-core `tools.execute`): 1. Session's persisted `permission_mode`, unless it is `inherit` @@ -550,18 +561,21 @@ Rules: - The session value is stored in `sessions.permission_mode` (`inherit | ask | accept-edits | auto`, default `inherit`, schema v5) and set via `session.configure` `permissionMode`. -- Plan's hard deny wins over every permission mode for Write/Edit and plugin - tools that lack `planSafeActions`. `auto` cannot re-enable a hidden or denied tool. +- A contract mode's hard deny wins over every permission mode for `Write`, + `Edit`, `TodoWrite`, `GenerateImages`, and the delegation tools. `auto` cannot + re-enable a hidden or denied tool. - Low-risk tools (`Read`/`Glob`/`Grep`) inside the session roots auto-allow in every mode, as before. - `BrowserPreview` is an explicit read-only UI inspection capability and is available in both operating modes. -- Plan retains the permission-mode selector. Bash is confirmed under `ask` and - `accept-edits`, and is auto-allowed under `auto`; therefore Plan is planning - intent, not a strict read-only security profile. +- Plan retains the permission-mode selector. `Bash` takes a confirmation in every + permission mode, so the selector does not change whether Plan may run a command + — only how the rest of the tools behave. Plan is planning intent, not a strict + read-only security profile, and the UI states that tradeoff. - `allow-session` grants continue to work under `ask` and stay scoped to the session; under `accept-edits`/`auto` they are simply never needed. -- Scratch-directory writes (D114) stay prompt-free in every mode. +- Scratch-directory writes (D114) stay prompt-free in every mode. Plan and Goal + do not expose `Write`/`Edit`, so that rule cannot make them available there. - UI: Settings → segmented global default; composer shows a per-session chip in Agent, Plan, and Goal whose menu offers the three effective modes without a separate global-default/inherit entry. The chip and selected menu item @@ -622,16 +636,36 @@ for the current key-log policy. | Mode | Read/Glob/Grep | BrowserPreview | Write/Edit | Bash | Plugins | |---|---|---|---|---|---| | Agent | allow | allow | permission policy | permission policy | registered risk policy | -| Plan | allow | allow | deny | `ask`/`accept-edits`: confirm; `auto`: allow | plan-safe actions only | -| Goal | allow | allow | deny | `ask`/`accept-edits`: confirm; `auto`: allow | plan-safe actions only | +| Plan | allow | allow | deny | confirm (every permission mode) | `planSafeActions` only | +| Goal | allow | allow | deny | confirm (every permission mode) | `planSafeActions` only | ### Notes -- Plan and Goal hard-deny Write/Edit and plugin tools without `planSafeActions` before permission UI; a direct host - call cannot bypass the matrix. Plugin tools that declare a non-empty list are admitted; the runner still rejects any action outside that list (ADR 0211). +- A contract mode denies by what a call would do, not by where its name sits in a + list. The read-only builtins fall through to the ordinary permission path; a + builtin that is not on that list — `Write`, `Edit`, `TodoWrite`, + `GenerateImages`, the delegation tools — is hard-denied before the permission + UI, and a direct host call cannot bypass the matrix. An unclassified builtin is + denied rather than allowed, so a tool added later cannot widen the mode by + omission. +- Plan and Goal Bash always takes a permission card, in every permission mode. + What a command does is decided by the shell that runs it, so the host does not + read the command string to decide; `auto` therefore cannot stand in for the + user's decision here. A `Bash` session grant still suppresses later prompts for + the rest of that session. +- An MCP tool carries no classification signal — its name says nothing and a + server's self-declared risk annotation is untrusted — so a contract mode routes + it to a permission card rather than letting `auto` admit it unread. The tool + stays visible to the model so its card is reachable. +- A `plugin_` tool is admitted when it declares a non-empty `planSafeActions` + list, because the plugin-runtime then rejects any action outside that list + (ADR 0211). Without one it is refused: an approval the callee would reject + anyway is not a gate. +- A trusted-extension tool is withheld in Plan and Goal. It runs in the sidecar + process, so no `tools.execute` call reaches the host and the permission path + cannot gate it; withholding it is the only boundary available. Extensions keep + working unchanged in Agent mode. - Agent mode uses permission cards or the selected automatic policy for Write/Edit/Bash and registered plugin tools. -- Plan and Goal Bash may mutate workspace or scratch state when the user selected Auto; - the UI must make that tradeoff visible. - allow-session is remembered per toolName for the active session only - Session grants follow `sessionId` across project-tab switches and are never inherited by another session or Temporary conversation @@ -720,21 +754,23 @@ session's root unchanged. ## 11. Plugin Tools Plugins can contribute tools via `agentTools`. Agent mode sees every registered -plugin tool. Plan and Goal see only tools whose `planSafeActions` list is -non-empty (ADR 0211 / D384): +plugin tool. Plan and Goal admit a plugin tool only when its `planSafeActions` +list is non-empty (ADR 0211 / D384): 1. manifest declaration 2. user grants `agent.tool.register` 3. PluginManager registers them into the ToolHost 4. execution goes through the unified permission/audit/timeout wrapper -A plugin tool without `planSafeActions` is hidden from the model in Plan and -Goal. A direct attempt returns `PLUGIN_DISABLED_IN_PLAN` — the `_IN_PLAN` codes -are shared by both contract modes rather than duplicated per kind — and is -audited as a contract-mode policy denial. When the list is present, host-core -admits the tool and the plugin-runtime rejects any `action` outside the list -with `PERMISSION_DENIED`. Missing or invalid plugin risk defaults to `medium` -for Agent and never grants contract-mode access by itself. +A plugin tool without `planSafeActions` is refused in Plan and Goal. A direct +attempt returns `PLUGIN_DISABLED_IN_PLAN` — the `_IN_PLAN` codes are shared by +both contract modes rather than duplicated per kind — and is audited as a +contract-mode policy denial. The refusal is deliberate rather than routed to a +card: the plugin-runtime refuses the same call with `PERMISSION_DENIED`, so an +approval the callee then rejects would not be a gate. When the list is present, +host-core admits the tool and the plugin-runtime rejects any `action` outside the +list with `PERMISSION_DENIED`. Missing or invalid plugin risk defaults to +`medium` for Agent and never grants contract-mode access by itself. Naming: - Internal full name: `plugin..` diff --git a/docs/spec/03-runtime/08-error-codes.md b/docs/spec/03-runtime/08-error-codes.md index a73dde68a5..d05e92d9fe 100644 --- a/docs/spec/03-runtime/08-error-codes.md +++ b/docs/spec/03-runtime/08-error-codes.md @@ -139,8 +139,8 @@ does not turn temporary thread pressure into a host process exit. | `PERMISSION_REQUIRED` | no | waiting for user decision | | `WRITE_DISABLED_IN_PLAN` | no | contract-mode hard-deny for Write | | `EDIT_DISABLED_IN_PLAN` | no | contract-mode hard-deny for Edit | -| `PLUGIN_DISABLED_IN_PLAN` | no | contract-mode hard-deny for every plugin tool | -| `TOOL_DISABLED_IN_PLAN` | no | contract-mode hard-deny for an unknown/unlisted tool | +| `PLUGIN_DISABLED_IN_PLAN` | no | contract-mode refusal of a plugin tool that declares no `planSafeActions` | +| `TOOL_DISABLED_IN_PLAN` | no | contract-mode hard-deny for a builtin outside the read-only set | | `PLAN_NOT_ACTIVE` | no | a submit tool ran while no contract was being negotiated | | `PLAN_KIND_MISMATCH` | no | `SubmitPlan` in Goal mode, or `SubmitGoal` in Plan mode | | `PLAN_APPROVAL_REQUIRED` | no | SubmitPlan/SubmitGoal is waiting for a separate approval | @@ -530,9 +530,10 @@ Examples: 1. Every IPC failure returns `AppError.code` 2. No raw untyped string-only failures on main paths -3. Plan/Goal hard-denies use explicit tool-specific codes; Bash is never denied - by either contract mode solely because of the operating mode and instead - follows permission policy +3. Plan/Goal hard-denies use explicit tool-specific codes. Bash is not denied by + either contract mode because of the operating mode; it takes a permission card + in every permission mode, so the user still decides, and an approved command + runs 4. Host numeric codes map to stable string codes 5. Invalid shell settings, no-effective-shell/stale-pin, artifact-write, expiry, scheduled-rejection, and restart-interruption paths map to stable diff --git a/docs/spec/06-delivery/04-e2e-test-plan.md b/docs/spec/06-delivery/04-e2e-test-plan.md index 89654cfddf..cba9a619b1 100644 --- a/docs/spec/06-delivery/04-e2e-test-plan.md +++ b/docs/spec/06-delivery/04-e2e-test-plan.md @@ -2535,8 +2535,9 @@ identify the platform validation still needed. Click the same Composer chip to return to Agent. 4) Begin a turn and try to toggle mode mid-run or while a pending Plan approval is visible. - **Expected**: The Composer chip updates the active session `mode` (Plan and - Goal hard-deny Write/Edit and plugin tools while Bash follows the selected - permission mode; Agent allows its normal tools per permission settings). The + Goal hard-deny Write/Edit and refuse a plugin tool without `planSafeActions`, + while Bash takes a permission card in every permission mode; Agent allows its + normal tools per permission settings). The chip is disabled while a turn or active pending approval exists and re-enables after the session returns idle/planning. No top-bar mode control is rendered. - **Specs linked**: `04-ux/08-component-spec.md` (§2, §11), @@ -3001,8 +3002,8 @@ identify the platform validation still needed. #### E2E-019c: Permission modes govern high-risk approval (D115/D132) - **Preconditions**: Agent mode; project open; global default `ask`. -- **Steps**: 1) With a newly inherited session and global default Ask every time, open the composer menu — expect Ask every time to be selected with no global-default/inherit label — then ask the Agent to write a workspace file and expect a permission card. 2) Switch the session chip to Accept edits; repeat — expect no card for Write/Edit but still a card for Bash. 3) Switch to Auto — expect no card for Bash either. 4) Create another inherited session after setting the global default to Accept edits in Settings — expect the composer chip and menu selection to display Accept edits directly and Write/Edit to be auto-allowed. 5) Switch the session to Plan, then Goal, with Auto set — expect Write/Edit/plugin denied but Bash allowed without confirmation. -- **Expected**: Effective mode = session override → global default → ask; Plan and Goal Write/Edit/plugin hard denies outrank every mode while their Bash follows the selected mode; the composer chip and menu always display the effective mode without default/inherit provenance. +- **Steps**: 1) With a newly inherited session and global default Ask every time, open the composer menu — expect Ask every time to be selected with no global-default/inherit label — then ask the Agent to write a workspace file and expect a permission card. 2) Switch the session chip to Accept edits; repeat — expect no card for Write/Edit but still a card for Bash. 3) Switch to Auto — expect no card for Bash either. 4) Create another inherited session after setting the global default to Accept edits in Settings — expect the composer chip and menu selection to display Accept edits directly and Write/Edit to be auto-allowed. 5) Switch the session to Plan, then Goal, with Auto set — expect Write/Edit denied, a plugin tool without `planSafeActions` refused, and Bash still showing a permission card despite Auto. +- **Expected**: Effective mode = session override → global default → ask; Plan and Goal Write/Edit hard denies outrank every mode, while Bash takes a card in every permission mode including Auto and an unclassifiable MCP tool does the same; the composer chip and menu always display the effective mode without default/inherit provenance. - **Specs linked**: `03-runtime/03-tools-and-permissions.md §6`, `03-runtime/04-data-storage.md`, `08-meta/decisions-log.md` (D115/D132) - **Acceptance**: E (permission modes resolve and enforce host-side) - **Milestone**: M5 @@ -7426,21 +7427,28 @@ must keep splitting are covered by `markdown-blocks.test.mjs`. #### E2E-105: Plan policy remains host-authoritative - **Preconditions**: A project-bound session is idle in Plan with BrowserPreview, - a plugin tool, and a forged `requestedMode = "agent"` fixture. + a plugin tool, an MCP tool, and a forged `requestedMode = "agent"` fixture. - **Steps**: 1) Inspect visible Plan tools. 2) Use Read/Glob/Grep and - BrowserPreview. 3) Attempt Write, Edit, plugin, and - unknown tools through the host with every permission mode. 4) Run Bash under - Ask, Accept edits, and Auto. -- **Expected**: Plan denies Write/Edit/plugin/unknown tools regardless of the - forged mode, grants, or Auto; Bash follows the selected permission mode. The - runtime remains one pi Agent and all denials are audited. + BrowserPreview. 3) Attempt Write, Edit, a builtin outside the read-only set, + a plugin tool without `planSafeActions`, and an MCP tool through the host with + every permission mode. 4) Run Bash under Ask, Accept edits, and Auto. +- **Expected**: Plan denies Write/Edit and any builtin outside the read-only set + regardless of the forged mode, grants, or Auto; a plugin tool without + `planSafeActions` is refused with `PLUGIN_DISABLED_IN_PLAN`; an MCP tool and + Bash each show a permission card in **every** permission mode, Auto included, + and the approved call then runs. The runtime remains one pi Agent and all + denials are audited. - **Specs linked**: `03-runtime/02-agent-runtime.md`, `03-runtime/03-tools-and-permissions.md`, `03-runtime/05-host-core-rust.md`, - `03-runtime/06-host-rpc-protocol.md`, `05-security/01-security.md`, ADR 0053 + `03-runtime/06-host-rpc-protocol.md`, `05-security/01-security.md`, ADR 0053, + ADR 0330 - **Acceptance**: E (tools and permissions), Security - **Milestone**: M6 - **Status**: Automated (passed 2026-08-04): `test:e2e:plan` plus host-core - permission/policy and agent-runtime tool-composition tests + permission/policy and agent-runtime tool-composition tests. The Bash and MCP + card behavior is covered by `permissions::tests` and the two + `rpc::tests::{plan,goal}_authorization*` cases; `test:e2e:plan` has not been + re-run against the ADR 0330 posture. #### E2E-PLAN-WORKSPACE: Missing workspace does not strand a contract turn @@ -9215,8 +9223,9 @@ must keep splitting are covered by `markdown-blocks.test.mjs`. `EnterGoalMode`, then `SubmitGoal(title, markdown, question)`. 2) Inspect the exact Markdown bytes in the new `.pi/goal/*.md` artifact and the matching `plan_approvals` row. 3) Confirm the shared approval card exposes only - Approve/Reject and that Goal denies Write/Edit/plugin tools while Bash follows - the selected permission mode. 4) Approve with Ask and observe the same Agent + Approve/Reject and that Goal denies Write/Edit and refuses a plugin tool without + `planSafeActions`, while Bash takes a permission card in every permission mode. + 4) Approve with Ask and observe the same Agent resume in Agent mode. 5) Inspect the final response for criterion-by-criterion verification or an explicit boundary, then reload the session. - **Expected**: Goal uses the Plan approval pipeline without a second planner; diff --git a/docs/spec/07-plugins/03-plugin-api.md b/docs/spec/07-plugins/03-plugin-api.md index 024d37f0bc..043e9e5c5f 100644 --- a/docs/spec/07-plugins/03-plugin-api.md +++ b/docs/spec/07-plugins/03-plugin-api.md @@ -348,12 +348,14 @@ pi.agent.registerTool(tool: { pi.agent.unregisterTool(name: string): Promise ``` -Registered plugin agent tools are Agent-only contributions. During Plan the -host filters them out of the model tool list and rejects direct execution with -`PLUGIN_DISABLED_IN_PLAN`, including when the manifest risk is `low`, the user -has an `allow-session` grant, or the session permission mode is `auto`. A -successful plan approval makes the same registered tools eligible again under -the selected Agent permission policy. +Registered plugin agent tools are Agent-only contributions unless the tool +declares a non-empty `planSafeActions` list (ADR 0211). During Plan the host +filters out every contribution without that list and rejects direct execution +with `PLUGIN_DISABLED_IN_PLAN`, including when the manifest risk is `low`, the +user has an `allow-session` grant, or the session permission mode is `auto`. A +tool that does declare the list is admitted in Plan and the plugin runtime still +rejects any `action` outside it. A successful plan approval makes every +registered tool eligible again under the selected Agent permission policy. ```ts type ToolExecContext = { diff --git a/docs/spec/07-plugins/04-plugin-security.md b/docs/spec/07-plugins/04-plugin-security.md index 8147483514..74eed9c076 100644 --- a/docs/spec/07-plugins/04-plugin-security.md +++ b/docs/spec/07-plugins/04-plugin-security.md @@ -316,11 +316,14 @@ are admitted. Plan is an additional host policy boundary for agent tools: -- no plugin tool is visible or executable in Plan; +- a plugin tool without a non-empty `planSafeActions` list is neither visible nor + executable in Plan; - the deny precedes manifest risk, declared/granted permissions, session grants, and the `auto` permission mode; - a direct forged `tools.execute` call returns `PLUGIN_DISABLED_IN_PLAN` and is audited; it is not forwarded to the plugin runtime; +- a tool that does declare the list is admitted, and the plugin runtime rejects + any `action` outside it before the plugin sees the call (ADR 0211); - plugin commands and panels may remain usable as explicit user UI actions, but they cannot become model-callable Plan tools or silently mutate Plan state. diff --git a/docs/spec/07-plugins/12-plugin-ipc-and-host-services.md b/docs/spec/07-plugins/12-plugin-ipc-and-host-services.md index 47a59d8d8d..6894e6a121 100644 --- a/docs/spec/07-plugins/12-plugin-ipc-and-host-services.md +++ b/docs/spec/07-plugins/12-plugin-ipc-and-host-services.md @@ -254,10 +254,11 @@ permission gate and result envelope stay in host-core: `{ executionId, sessionId, toolCallId, toolName, args, turnId }`. `turnId` is the runtime turn identity, forwarded unchanged so the plugin tool context can be matched against the `session:turnEnded` event. -3. Plan calls fail at the host policy step with `PLUGIN_DISABLED_IN_PLAN`; they - never reach Electron or the plugin runtime. Agent calls continue with - Electron main executing the registered plugin tool JS and answering via RPC - `plugins.resolveExecution` `{ executionId, ok, content, errorCode? }`. +3. Plan calls for a tool without a `planSafeActions` list fail at the host policy + step with `PLUGIN_DISABLED_IN_PLAN`; they never reach Electron or the plugin + runtime. Agent calls continue with Electron main executing the registered + plugin tool JS and answering via RPC `plugins.resolveExecution` + `{ executionId, ok, content, errorCode? }`. 4. host-core resolves the pending execution and returns a standard `ToolsExecuteResult` to the sidecar. Dispatch waits up to 150s (`DESKTOP_TOOL_DISPATCH_TIMEOUT_MS`, above both the 110s plugin tool budget diff --git a/docs/spec/07-plugins/13-plugin-permissions-matrix.md b/docs/spec/07-plugins/13-plugin-permissions-matrix.md index 7b59d066aa..3b52832350 100644 --- a/docs/spec/07-plugins/13-plugin-permissions-matrix.md +++ b/docs/spec/07-plugins/13-plugin-permissions-matrix.md @@ -116,11 +116,13 @@ plugin, so it carries three bounds the other modes do not: ## 3A. Plan operating-state rule -Every `agentTools` contribution is denied in Plan, regardless of this matrix's -risk or default policy. `agent.tool.register` authorizes registration for -Agent, not visibility in Plan. The host returns `PLUGIN_DISABLED_IN_PLAN` for a -direct Plan call and records the denial. Plugin tools become eligible only -after the same Agent is approved into Agent mode. +An `agentTools` contribution without a non-empty `planSafeActions` list is denied +in Plan, regardless of this matrix's risk or default policy. `agent.tool.register` +authorizes registration for Agent, not visibility in Plan. The host returns +`PLUGIN_DISABLED_IN_PLAN` for a direct Plan call and records the denial. A +contribution that does declare the list is admitted in Plan, and the plugin +runtime rejects any `action` outside it (ADR 0211). Every contribution becomes +eligible again after the same Agent is approved into Agent mode. ## 4. Permission display copy diff --git a/docs/zh-CN/spec/03-runtime/03-tools-and-permissions.md b/docs/zh-CN/spec/03-runtime/03-tools-and-permissions.md index 821ffbdb4f..3718c32055 100644 --- a/docs/zh-CN/spec/03-runtime/03-tools-and-permissions.md +++ b/docs/zh-CN/spec/03-runtime/03-tools-and-permissions.md @@ -12,10 +12,12 @@ |---|---| | 默认模式 | Agent | | Agent 工具 | 读取 / Glob / Grep / 写入 / 编辑 / Bash + 已注册的插件工具 | -| Plan 工具 | 读取 / Glob / Grep / BrowserPreview / Bash / SubmitPlan + 声明 plan-safe 动作的插件工具 | -| Goal 工具 | 读取 / Glob / Grep / BrowserPreview / Bash / SubmitGoal + 声明 plan-safe 动作的插件工具 | -| Plan 和 Goal 硬拒绝 | 写入 / 编辑 / 没有 `planSafeActions` 的插件工具 / 未知工具 / 另一类的提交工具 | -| 插件 `planSafeActions` | 非空的 `action` 字符串数组；运行时在 Plan/Goal 中隐藏没有该列表的插件工具，host 放行已列出的工具，plugin-runtime 拒绝列表外的任何动作（ADR 0211） | +| Plan 工具 | 读取 / Glob / Grep / BrowserPreview / asktool / SubmitPlan，以及经权限卡放行的 MCP 工具和 `planSafeActions` 插件工具 | +| Goal 工具 | 读取 / Glob / Grep / BrowserPreview / asktool / SubmitGoal，以及经权限卡放行的 MCP 工具和 `planSafeActions` 插件工具 | +| Plan 和 Goal 硬拒绝 | 写入 / 编辑 / TodoWrite / GenerateImages / 委派工具 / 另一类的提交工具。不在只读清单内的内置工具在分类前一律拒绝 | +| Plan 和 Goal 的 Bash | 始终弹出权限卡；权限模式不会跳过它 | +| Plan 和 Goal 的未分类工具 | MCP 工具没有任何分类信号，因此弹出权限卡而不是未经审查地运行；`plugin_` 工具需要非空的 `planSafeActions` 列表 | +| 插件 `planSafeActions` | 非空的 `action` 字符串数组；host 放行已列出的工具，plugin-runtime 拒绝列表外的任何动作（ADR 0211） | | 本地权限确认 | 无自动截止时间；必须明确决定或取消 | | 允许会话范围 | 工具名称 | | 重击风格 | 非交互式；具有流输出的选定主机目录外壳 | @@ -422,7 +424,12 @@ tool/protocol 名称，请求中单独携带固定的 shell ID。 这与用户已接受的插件 manifest 中的风险不同。在 `ask` 和 `accept-edits` 下， MCP 工具调用会显示审批卡片，原因为 "MCP server tool requires approval"； `allow-session` 授权会在该会话内对该工具名不再提示（授权仅保存在内存中）。 -`auto` 自动允许，Plan/Goal 合约模式的硬拒绝仍然生效（D640，ADR `mcp-tool-approval-risk`）。 +Agent 模式下 `auto` 自动允许。 + +在 Plan 和 Goal 中，MCP 工具无法分类——名称说明不了它是否改变状态，服务器自己的标注也不构成 +证据——因此它在每种权限模式下都弹出权限卡，包括 `auto`。这是契约模式在 Bash 之外唯一覆盖所选 +权限模式的地方。该工具保留在模型的工具目录中，权限卡才够得着（D640，ADR +`mcp-tool-approval-risk`）。 ### 决策类型 @@ -458,15 +465,14 @@ MCP 工具调用会显示审批卡片，原因为 "MCP server tool requires appr - 会话值存储在 `sessions.permission_mode` 中 （`inherit | ask | accept-edits | auto`，默认 `inherit`，架构 v5）和 通过 `session.configure` `permissionMode` 设置。 -- Plan 的硬拒绝胜过 Write/Edit 以及缺少 `planSafeActions` 的插件 - 工具的所有权限模式。 `auto` 无法重新启用隐藏或拒绝的工具。 -- 会话根目录内的低风险工具（`allow-once`/`allow-session`/`deny`）自动允许 +- 契约模式的硬拒绝胜过所有权限模式，适用于 `Write`、`Edit`、`TodoWrite`、 + `GenerateImages` 和委派工具。 `auto` 无法重新启用隐藏或拒绝的工具。 +- 会话根目录内的低风险工具（`Read`/`Glob`/`Grep`）自动允许 每种模式都和以前一样。 - `BrowserPreview` 是显式只读 UI 检查功能，并且是 在两种操作模式下均可用。 -- Plan 保留权限模式选择器。 Bash 在 `ask` 下得到确认并且 - `accept-edits`，并且在 `auto` 下自动允许；因此 Plan 正在规划 - 意图，而不是严格的只读安全配置文件。 +- Plan 保留权限模式选择器。 `Bash` 在每种权限模式下都需要确认，因此该选择器不改变 + Plan 是否可以运行命令，只影响其余工具的行为。Plan 是规划意图，而不是严格的只读安全配置。 - `allow-session` 赠款继续在 `ask` 下运作，范围仅限于 会议；在 `BrowserPreview`/`ask` 下，根本不需要它们。 - 暂存目录写入 (D114) 在每种模式下都保持无提示。 @@ -524,20 +530,29 @@ MVP 可以通过写入 SQLite 或日志文件来启动。 ## 10. 操作模式矩阵 -| 模式 | Read/Glob/Grep | BrowserPreview | Write/Edit | 重击 | 插件 | +| 模式 | Read/Glob/Grep | BrowserPreview | Write/Edit | Bash | 插件 | |---|---|---|---|---|---| | Agent | 允许 | 允许 | 许可政策 | 许可政策 | 注册风险政策 | -| Plan | 允许 | 允许 | 否认 | Plan/`ask`：确认； `auto`：允许 | 仅 plan-safe 动作 | -| Goal | 允许 | 允许 | 否认 | Plan/`ask`：确认； `auto`：允许 | 仅 plan-safe 动作 | +| Plan | 允许 | 允许 | 拒绝 | 确认（所有权限模式） | 仅 `planSafeActions` | +| Goal | 允许 | 允许 | 拒绝 | 确认（所有权限模式） | 仅 `planSafeActions` | ### 注释 -- 权限 UI 之前，Plan 和 Goal 硬拒绝 Write/Edit 以及没有 `planSafeActions` 的插件工具；直接主机 - 调用不能绕过矩阵。声明了非空列表的插件工具会被放行；运行器仍会拒绝列表外的任何动作（ADR 0211）。 -- Agent 模式使用权限卡或选定的自动策略 +- 契约模式按调用的效果拒绝，而不是按名称是否在清单中。只读内置工具走普通权限路径； + 不在该清单内的内置工具（`Write`、`Edit`、`TodoWrite`、`GenerateImages`、委派工具）在权限 UI 之前 + 硬拒绝，直接主机调用不能绕过矩阵。未分类的内置工具按拒绝处理而非放行，因此之后新增的工具 + 不会因遗漏而放宽该模式。 +- Plan 和 Goal 的 Bash 在每种权限模式下都弹出权限卡。命令做什么由运行它的 shell 决定， + host 不读取命令字符串来判定；因此 `auto` 不能代替用户的决定。`Bash` 的会话授权仍会抑制 + 该会话后续的重复确认。 +- MCP 工具没有任何分类信号——名称说明不了什么，服务器自报的风险标注也不可信——因此契约模式 + 把它交给权限卡，而不是让 `auto` 未经审查地放行。该工具对模型保持可见，权限卡才够得着。 +- 当 `plugin_` 工具声明了非空的 `planSafeActions` 列表时被放行，因为 plugin-runtime 随后会拒绝 + 列表外的任何动作（ADR 0211）。没有该列表则直接拒绝：一个调用方本就会拒绝的批准并不构成门禁。 +- 受信扩展工具在 Plan 和 Goal 中被扣留。它在 sidecar 进程内运行，因此没有任何 `tools.execute` + 调用到达 host，权限路径无法对它设门；扣留它是唯一可用的边界。扩展在 Agent 模式下不受影响。 +- Agent 模式使用权限卡或选定的自动策略处理 Write/Edit/Bash 和注册的插件工具。 -- 当用户选择“自动”时，Plan 和 Goal Bash 可能会改变工作区或暂存状态； - 用户界面必须使这种权衡可见。 -- 仅针对活动会话记住每个 toolName 的允许会话 +- allow-session 仅针对活动会话按 toolName 记忆 - 会话授权遵循 `sessionId` 跨项目选项卡开关，并且永远不会 由另一个会话或临时会话继承 @@ -608,20 +623,20 @@ sidecar 不附加覆盖，委托使用会话的有效权限模式；因此父会 ## 11. 插件工具 插件可通过 `agentTools` 贡献工具。Agent 模式能看到每一个已注册的 -插件工具。Plan 和 Goal 只看到 `planSafeActions` 列表 -非空的工具（ADR 0211 / D384）： +插件工具。Plan 和 Goal 仅在 `planSafeActions` 列表非空时放行插件工具 +（ADR 0211 / D384）： -1. 舱单声明 +1. 清单声明 2. 用户授予 `agent.tool.register` -3.PluginManager将它们注册到ToolHost中 -4.执行经过统一的permission/audit/timeout包装器 - -没有 `planSafeActions` 的插件工具在 Plan 和 Goal 中对模型隐藏。 -直接尝试返回 `PLUGIN_DISABLED_IN_PLAN` — `_IN_PLAN` 代码由两个合约共享 -模式而不是每种重复 - 并且作为合同模式政策进行审核 -否认。当列表存在时，host-core 放行该工具，plugin-runtime 拒绝列表外的任何 `action`， -返回 `PERMISSION_DENIED`。对于 Agent，缺失或无效的插件风险默认为 `medium`，并且从不 -仅凭风险授予合同模式访问权限。 +3. PluginManager 将它们注册到 ToolHost 中 +4. 执行经过统一的 permission/audit/timeout 包装器 + +没有 `planSafeActions` 的插件工具在 Plan 和 Goal 中被拒绝。直接尝试返回 +`PLUGIN_DISABLED_IN_PLAN` — `_IN_PLAN` 代码由两个契约模式共享而不是各自重复 — 并且作为 +契约模式策略拒绝被审计。这里是直接拒绝而非转交权限卡，因为 plugin-runtime 会对同一调用 +返回 `PERMISSION_DENIED`：一个调用方随后会拒绝的批准并不构成门禁。当列表存在时， +host-core 放行该工具，plugin-runtime 拒绝列表外的任何 `action`，返回 `PERMISSION_DENIED`。 +对于 Agent，缺失或无效的插件风险默认为 `medium`，并且从不仅凭风险授予契约模式访问权限。 命名： - 内部全名：`plugin..` diff --git a/docs/zh-CN/spec/03-runtime/08-error-codes.md b/docs/zh-CN/spec/03-runtime/08-error-codes.md index 49365f0183..9bb5b524c5 100644 --- a/docs/zh-CN/spec/03-runtime/08-error-codes.md +++ b/docs/zh-CN/spec/03-runtime/08-error-codes.md @@ -141,8 +141,8 @@ stdio 与 Tokio 的动态阻塞池隔离，因此后一种情况 | `PERMISSION_REQUIRED` | 不 | 等待用户决定 | | `WRITE_DISABLED_IN_PLAN` | 不 | Write 的契约模式硬拒绝 | | `EDIT_DISABLED_IN_PLAN` | 不 | 编辑的契约模式硬拒绝 | -| `PLUGIN_DISABLED_IN_PLAN` | 不 | 每个插件工具的契约模式硬拒绝 | -| `TOOL_DISABLED_IN_PLAN` | 不 | unknown/unlisted 工具的契约模式硬拒绝 | +| `PLUGIN_DISABLED_IN_PLAN` | 不 | 契约模式拒绝未声明 `planSafeActions` 的插件工具 | +| `TOOL_DISABLED_IN_PLAN` | 不 | 只读清单之外的内置工具的契约模式硬拒绝 | | `PLAN_NOT_ACTIVE` | 不 | 在没有协商合同的情况下运行了提交工具 | | `PLAN_KIND_MISMATCH` | 不 | Goal 模式下的 `SubmitPlan`，或 Plan 模式下的 `SubmitGoal` | | `PLAN_APPROVAL_REQUIRED` | 不 | SubmitPlan/SubmitGoal 正在等待单独的批准 | diff --git a/docs/zh-CN/spec/06-delivery/04-e2e-test-plan.md b/docs/zh-CN/spec/06-delivery/04-e2e-test-plan.md index 09968e7ad3..caf86a95cd 100644 --- a/docs/zh-CN/spec/06-delivery/04-e2e-test-plan.md +++ b/docs/zh-CN/spec/06-delivery/04-e2e-test-plan.md @@ -878,10 +878,9 @@ task-candidate E2E 从请求工作树运行，但使用主工作区已经准备 单击同一 Composer 芯片返回 Agent。 4) 开始回合并尝试 在运行中或待处理的 Plan 批准可见时切换模式。 - **预期**：Composer 芯片更新活动会话 `mode`（Plan 和 - Goal 硬否认 Write/Edit 和插件工具，而 Bash 遵循所选的 - 权限模式； Agent 根据权限设置允许其正常工具）。的 - 当存在轮流或活动待批准时芯片被禁用并重新启用 - 会话返回 idle/planning 后。不呈现顶栏模式控件。 + Goal 硬拒绝 Write/Edit，并拒绝未声明 `planSafeActions` 的插件工具，而 Bash 在每一种 + 权限模式下都弹出权限卡；Agent 根据权限设置允许其正常工具）。当存在轮流或活动待批准时 + 芯片被禁用并重新启用会话返回 idle/planning 后。不呈现顶栏模式控件。 - **链接规格**：`04-ux/08-component-spec.md`（§2，§11）， `03-runtime/03-tools-and-permissions.md`（§10）， `03-runtime/04-data-storage.md` (§8) @@ -1152,8 +1151,8 @@ task-candidate E2E 从请求工作树运行，但使用主工作区已经准备 #### E2E-019c：权限模式管理高风险批准 (D115/D132) - **先决条件**：Agent 模式；项目开放；全局默认`ask`。 -- **步骤**： 1) 使用新继承的会话和全局默认“每次询问”，打开编写器菜单 - 期望“每次询问”都被选中，且没有 global-default/inherit 标签 - 然后要求 Agent 写入工作区文件并期望获得权限卡。 2) 将会话芯片切换为接受编辑；重复一下——预计 Write/Edit 不会出现任何卡，但 Bash 仍然会出现卡。 3) 切换到“自动”——预计 Bash 也不会卡。 4) 在设置中将全局默认值设置为接受编辑后创建另一个继承会话 - 期望输入框芯片和菜单选择直接显示接受编辑并且自动允许 Write/Edit。 5) 将会话切换到 Plan，然后切换到 Goal，并设置自动 — 预计 Write/Edit/plugin 会被拒绝，但 Bash 会允许而无需确认。 -- **预期**：有效模式=会话覆盖→全局默认→询问； Plan 和 Goal Write/Edit/plugin 硬否认超越每种模式，而他们的 Bash 遵循所选模式；输入框芯片和菜单始终显示有效模式，无需 default/inherit 出处。 +- **步骤**： 1) 使用新继承的会话和全局默认“每次询问”，打开编写器菜单 - 期望“每次询问”都被选中，且没有 global-default/inherit 标签 - 然后要求 Agent 写入工作区文件并期望获得权限卡。 2) 将会话芯片切换为接受编辑；重复一下——预计 Write/Edit 不会出现任何卡，但 Bash 仍然会出现卡。 3) 切换到“自动”——预计 Bash 也不会卡。 4) 在设置中将全局默认值设置为接受编辑后创建另一个继承会话 - 期望输入框芯片和菜单选择直接显示接受编辑并且自动允许 Write/Edit。 5) 将会话切换到 Plan，然后切换到 Goal，并设置自动 — 预计 Write/Edit 被拒绝、未声明 `planSafeActions` 的插件工具被拒绝，而 Bash 即便在 Auto 下仍弹出权限卡。 +- **预期**：有效模式=会话覆盖→全局默认→询问； Plan 和 Goal 的 Write/Edit 硬拒绝超越每一种模式，而 Bash 在每一种权限模式下（含 Auto）都弹出权限卡，无法分类的 MCP 工具同样如此；输入框芯片和菜单始终显示有效模式，无需 default/inherit 出处。 - **链接规格**：`03-runtime/03-tools-and-permissions.md §6`、`03-runtime/04-data-storage.md`、`08-meta/decisions-log.md` (D115/D132) - **接受**：E（权限模式解析并强制主机端） - **里程碑**：M5 @@ -4226,21 +4225,24 @@ eleven-tool-round desktop paths are verified by #### E2E-105：Plan 策略仍然具有主机权威 - **先决条件**：项目绑定会话在 Plan 和 BrowserPreview 中处于空闲状态， - 一个插件工具和一个伪造的 `requestedMode = "agent"` 夹具。 + 一个插件工具、一个 MCP 工具和一个伪造的 `requestedMode = "agent"` 夹具。 - **步骤**：1) 检查可见的 Plan 工具。 2) 使用 Read/Glob/Grep 和 - BrowserPreview。 3) 尝试编写、编辑、插入和 - 各种权限模式下通过主机的未知工具。 4) 在下面运行 Bash - 询问、接受编辑和自动。 -- **预期**：Plan 拒绝 Write/Edit/plugin/unknown 工具，无论 - 伪造模式、补助金或自动； Bash 遵循所选的权限模式。的 - 运行时仍为 1 pi Agent 并且所有拒绝都会经过审核。 + BrowserPreview。 3) 在各种权限模式下通过主机尝试 Write、Edit、只读清单之外的内置工具、 + 未声明 `planSafeActions` 的插件工具，以及一个 MCP 工具。 4) 在 Ask、Accept edits 和 + Auto 下运行 Bash。 +- **预期**：无论伪造模式、授权还是 Auto，Plan 都拒绝 Write/Edit 以及只读清单之外的任何内置 + 工具；未声明 `planSafeActions` 的插件工具以 `PLUGIN_DISABLED_IN_PLAN` 被拒绝；MCP 工具和 + Bash 在**每一种**权限模式下（含 Auto）各自弹出权限卡，批准后调用才运行。运行时仍为 + 1 pi Agent，且所有拒绝都会经过审核。 - **链接规格**：`03-runtime/02-agent-runtime.md`， `03-runtime/03-tools-and-permissions.md`、`03-runtime/05-host-core-rust.md`、 - `03-runtime/06-host-rpc-protocol.md`、`05-security/01-security.md`、ADR 0053 + `03-runtime/06-host-rpc-protocol.md`、`05-security/01-security.md`、ADR 0053、ADR 0330 - **接受**：E（工具和权限）、安全 - **里程碑**：M6 - **状态**：自动（2026-08-04 通过）：`test:e2e:plan` 加 host-core - permission/policy 和代理运行时工具组合测试 + permission/policy 和代理运行时工具组合测试。Bash 与 MCP 的权限卡行为由 + `permissions::tests` 以及两个 `rpc::tests::{plan,goal}_authorization*` 用例覆盖； + `test:e2e:plan` 尚未针对 ADR 0330 的姿态重跑。 #### E2E-106：SubmitPlan 拒绝进入可编辑计划并重新提交新工件 @@ -5438,8 +5440,8 @@ eleven-tool-round desktop paths are verified by `EnterGoalMode`，然后是 `SubmitGoal(title, markdown, question)`。 2) 检查 新 `.pi/goal/*.md` 工件中的确切 Markdown 字节以及匹配的 `plan_approvals` 行。 3) 确认共享审批卡仅公开 - Approve/Reject 和 Goal 否认 Write/Edit/plugin 工具，而 Bash 紧随其后 - 所选的权限模式。 4) 通过询问进行批准并遵守相同的 Agent + Approve/Reject，Goal 拒绝 Write/Edit 并拒绝未声明 `planSafeActions` 的插件工具， + 而 Bash 在每一种权限模式下都弹出权限卡。 4) 通过 Ask 批准并让同一 Agent 在 Agent 模式下恢复。 5）逐个标准检查最终响应 验证或显式边界，然后重新加载会话。 - **预期**：Goal 使用 Plan 审批管道，无需第二个规划者； diff --git a/docs/zh-CN/spec/07-plugins/03-plugin-api.md b/docs/zh-CN/spec/07-plugins/03-plugin-api.md index 5cbba36182..720648e016 100644 --- a/docs/zh-CN/spec/07-plugins/03-plugin-api.md +++ b/docs/zh-CN/spec/07-plugins/03-plugin-api.md @@ -285,12 +285,12 @@ pi.agent.registerTool(tool: { pi.agent.unregisterTool(name: string): Promise ``` -注册的插件代理工具是仅代理的贡献。在 Plan 期间 -主机将它们从模型工具列表中过滤出来并拒绝直接执行 -`PLUGIN_DISABLED_IN_PLAN`，包括当明显风险为 `low` 时，用户 -具有 `allow-session` 授予，或者会话权限模式为 `auto`。一个 -成功的计划批准使相同的注册工具再次符合资格 -所选的 Agent 权限策略。 +注册的插件代理工具默认是仅代理的贡献，除非该工具声明了非空的 +`planSafeActions` 列表（ADR 0211）。在 Plan 期间，主机过滤掉所有没有该列表的贡献， +并以 `PLUGIN_DISABLED_IN_PLAN` 拒绝直接执行，包括当 manifest 风险为 `low`、用户 +具有 `allow-session` 授权，或者会话权限模式为 `auto` 时。声明了该列表的工具在 Plan 中 +被放行，插件运行时仍会拒绝列表外的任何 `action`。一次成功的计划批准使所有已注册工具 +再次符合所选 Agent 权限策略。 ```ts type ToolExecContext = { diff --git a/docs/zh-CN/spec/07-plugins/04-plugin-security.md b/docs/zh-CN/spec/07-plugins/04-plugin-security.md index 947ed43e46..8ad246a610 100644 --- a/docs/zh-CN/spec/07-plugins/04-plugin-security.md +++ b/docs/zh-CN/spec/07-plugins/04-plugin-security.md @@ -211,11 +211,13 @@ CSS 无法脚本化，但它可能会产生误导：主题仍然是第三方代 Plan 是代理工具的附加主机策略边界： -- Plan 中没有可见或可执行的插件工具； -- 拒绝先于明显风险、declared/granted 权限、会话 - grants，以及 `auto` 权限模式； -- 直接伪造的 `tools.execute` 调用返回 `PLUGIN_DISABLED_IN_PLAN` 并且是 - 已审核；它不会转发到插件运行时； +- 没有非空 `planSafeActions` 列表的插件工具在 Plan 中既不可见也不可执行； +- 拒绝先于 manifest 风险、declared/granted 权限、会话授权， + 以及 `auto` 权限模式； +- 直接伪造的 `tools.execute` 调用返回 `PLUGIN_DISABLED_IN_PLAN` 并且被 + 审计；它不会转发到插件运行时； +- 声明了该列表的工具会被放行，插件运行时在插件看到调用之前拒绝列表外的任何 `action` + （ADR 0211）； - 插件命令和面板仍可用作显式用户 UI 操作， 但它们不能成为模型可调用的 Plan 工具或默默地改变 Plan 状态。 diff --git a/docs/zh-CN/spec/07-plugins/12-plugin-ipc-and-host-services.md b/docs/zh-CN/spec/07-plugins/12-plugin-ipc-and-host-services.md index 4141cc02d5..67910a07f5 100644 --- a/docs/zh-CN/spec/07-plugins/12-plugin-ipc-and-host-services.md +++ b/docs/zh-CN/spec/07-plugins/12-plugin-ipc-and-host-services.md @@ -212,10 +212,10 @@ toast 加上 `pluginChanged` 到渲染器。 `{ executionId, sessionId, toolCallId, toolName, args, turnId }`。`turnId` 是 运行时回合身份，原样转发，以便插件工具上下文能与 `session:turnEnded` 事件对应。 -3. Plan 调用在主机策略步骤失败并显示 `PLUGIN_DISABLED_IN_PLAN`；他们 - 永远不会达到 Electron 或插件运行时。 Agent 呼叫继续 - Electron主要执行注册的插件工具JS并通过RPC应答 - `plugins.resolveExecution` `{ executionId, ok, content, errorCode? }`。 +3. 对没有 `planSafeActions` 列表的 Plan 调用在主机策略步骤失败并显示 + `PLUGIN_DISABLED_IN_PLAN`；它们永远不会达到 Electron 或插件运行时。 + Agent 调用继续由 Electron main 执行注册的插件工具 JS，并通过 RPC + `plugins.resolveExecution` `{ executionId, ok, content, errorCode? }` 应答。 4. host-core 解析待执行并返回一个标准 `ToolsExecuteResult` 到 sidecar。调度最多等待 150 秒 （`DESKTOP_TOOL_DISPATCH_TIMEOUT_MS`，高于 110 秒的插件工具预算，也高于最宽的 diff --git a/docs/zh-CN/spec/07-plugins/13-plugin-permissions-matrix.md b/docs/zh-CN/spec/07-plugins/13-plugin-permissions-matrix.md index e59af06d5d..b8e50941a6 100644 --- a/docs/zh-CN/spec/07-plugins/13-plugin-permissions-matrix.md +++ b/docs/zh-CN/spec/07-plugins/13-plugin-permissions-matrix.md @@ -113,11 +113,11 @@ manifest 里的字段负责回答「能做到多远」。两个字段都由主 ## 3A。 Plan 操作状态规则 -每个 `agentTools` 贡献都会在 Plan 中被拒绝，无论此矩阵的值如何 -风险或违约政策。 `agent.tool.register` 授权注册 -Agent，在 Plan 中不可见。主机返回 `PLUGIN_DISABLED_IN_PLAN` -直接 Plan 调用并记录拒绝。仅插件工具符合资格 -在同一个 Agent 被批准进入 Agent 模式后。 +没有非空 `planSafeActions` 列表的 `agentTools` 贡献会在 Plan 中被拒绝，无论此矩阵的风险 +或默认策略如何。 `agent.tool.register` 授权的是 Agent 侧的注册，而不是 Plan 中的可见性。 +主机对直接的 Plan 调用返回 `PLUGIN_DISABLED_IN_PLAN` 并记录该拒绝。声明了该列表的贡献 +在 Plan 中被放行，插件运行时拒绝列表外的任何 `action`（ADR 0211）。所有贡献在同一个 +Agent 被批准进入 Agent 模式后重新可用。 ## 4. 权限显示副本 diff --git a/packages/agent-runtime/src/fixed-tool-runtime.test.ts b/packages/agent-runtime/src/fixed-tool-runtime.test.ts index c160f26971..7e53c0aece 100644 --- a/packages/agent-runtime/src/fixed-tool-runtime.test.ts +++ b/packages/agent-runtime/src/fixed-tool-runtime.test.ts @@ -287,12 +287,16 @@ describe("explicit MCP selection", () => { } finally { await f.runtime.dispose(); await wire.close(); } }); - it("cannot expose selected MCP tools in Plan mode", async () => { - const wire = await wireFixture([]); - const f = runtimeFixture([], selectedTools, flashProvider(), false, "plan"); + it("offers selected MCP tools in Plan mode and still routes them through Host approval", async () => { + const wire = await wireFixture([{ name: selectedTools[0].name }]); + const f = runtimeFixture([], selectedTools, flashProvider(), true, "plan"); try { - await expect(f.runtime.prompt({ text: "Read", mcpServerIds: ["chosen"] })).rejects.toMatchObject({ errorCode: "TOOL_DENIED" }); - expect(wire.requests).toHaveLength(0); + await f.runtime.prompt({ text: "Read", mcpServerIds: ["chosen"] }); + // The tool is reachable, so its approval card can be shown; the Host is + // still what decides, and it refused here. + expect(f.executed).toEqual([selectedTools[0].name]); + expect(f.rows.find(row => row.toolName === selectedTools[0].name)).toMatchObject({ isError: true }); + expect(JSON.stringify(wire.requests.at(-1)?.messages)).toContain("Permission denied"); } finally { await f.runtime.dispose(); await wire.close(); } }); diff --git a/packages/agent-runtime/src/mode-tool-access.ts b/packages/agent-runtime/src/mode-tool-access.ts index 1c8b9da2f8..6b00edd67b 100644 --- a/packages/agent-runtime/src/mode-tool-access.ts +++ b/packages/agent-runtime/src/mode-tool-access.ts @@ -4,10 +4,51 @@ import type { Mode } from "@pi-desktop/shared"; /** Known editing/delegation names stay declared across contract-mode changes. */ const RETAINED_MODE_TOOLS = new Set(["Write", "Edit", "Task", "TaskWait", "TaskList", "TaskStop"]); +/** + * A contract mode withholds tools whose only purpose is to change state. They + * stay declared so a strict gateway never sees an undeclared call, and the + * execution guard below refuses them, but the model is never offered them as a + * real option. + * + * Every other tool is declared, including one this module cannot classify: an + * MCP or plugin tool reaches the model and the host permission path decides + * whether the call runs. Withholding is reserved for a name the mode can never + * permit, because a withheld tool is one the model cannot discover at all. + * + * The host enforces the same posture from the other side in + * `PermissionManager::contract_admission`. A name denied here must also be + * denied there, or the model gets a tool it can never use. + */ +const CONTRACT_MODE_WITHHELD = new Set([ + "Write", + "Edit", + "TodoWrite", + "Task", + "TaskWait", + "TaskList", + "TaskStop", + // Agent-only or state-changing local tools. They are already absent from a + // contract mode's builtin list, so listing them changes no observable + // behavior today; it keeps this set the single answer to "what does a + // contract mode refuse" instead of leaving the agreement to coincidence. + "GenerateImages", + "PluginCheck", + "PluginScaffold", + "PluginPack", + "Skill", + "ScheduledTaskCreate", + "ScheduledTaskUpdate", + "ScheduledTaskDelete", +]); + export function retainModeToolDeclaration(name: string): boolean { return RETAINED_MODE_TOOLS.has(name); } +export function contractModeWithholdsTool(name: string): boolean { + return CONTRACT_MODE_WITHHELD.has(name); +} + export function modeToolDenial(name: string, mode: Mode): string { return `${name} is not allowed in ${mode} mode. No action was performed. Continue read-only inspection and planning with the permitted tools; implementation requires Agent mode or approval of the submitted plan or goal.`; } diff --git a/packages/agent-runtime/src/runtime.test.ts b/packages/agent-runtime/src/runtime.test.ts index 7e252daa9d..3758fa2288 100644 --- a/packages/agent-runtime/src/runtime.test.ts +++ b/packages/agent-runtime/src/runtime.test.ts @@ -2,6 +2,7 @@ import { mkdir, mkdtemp, rm, symlink, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { describe, expect, it, vi, beforeEach } from "vitest"; +import { Type } from "typebox"; import { type Agent, type AgentMessage, type AgentTool } from "@earendil-works/pi-agent-core"; import { createAssistantMessageEventStream, @@ -223,6 +224,41 @@ function runtimeTool(runtime: DesktopAgentRuntime, name: string): AgentTool { return tool; } +describe("contract-mode tool catalog", () => { + /** + * A trusted-extension tool runs in this process, so no host permission call + * happens for it. If a contract mode declared it, the model could mutate the + * workspace with no card and no denial, which is the one hole the mode's own + * gate cannot close. Withholding it is the boundary. + */ + it("withholds a trusted-extension tool in contract modes but keeps it in Agent mode", () => { + const extensionTool = { + name: "fx_write", + label: "fx_write", + description: "extension tool", + parameters: Type.Object({}), + execute: async () => ({ content: [{ type: "text" as const, text: "ok" }], details: {} }), + }; + const withExtensionRunner = (runtime: DesktopAgentRuntime) => { + (runtime as unknown as { extensionRunner: unknown }).extensionRunner = { + getAgentTools: () => [extensionTool], + }; + return runtime as unknown as { isToolAllowedInMode: (name: string) => boolean }; + }; + + const plan = withExtensionRunner(createRuntime({ mode: "plan" })); + expect(plan.isToolAllowedInMode("fx_write")).toBe(false); + // A read-only builtin is still admitted, so the check is not blanket. + expect(plan.isToolAllowedInMode("Read")).toBe(true); + + const goal = withExtensionRunner(createRuntime({ mode: "goal" })); + expect(goal.isToolAllowedInMode("fx_write")).toBe(false); + + const agent = withExtensionRunner(createRuntime({ mode: "agent" })); + expect(agent.isToolAllowedInMode("fx_write")).toBe(true); + }); +}); + /** Minimal pi-ai assistant message; overrides carry the shape under test. */ function assistantMessage(overrides: { content: unknown[]; diff --git a/packages/agent-runtime/src/runtime.ts b/packages/agent-runtime/src/runtime.ts index aeeeedb62a..3b27b2297a 100644 --- a/packages/agent-runtime/src/runtime.ts +++ b/packages/agent-runtime/src/runtime.ts @@ -8,7 +8,12 @@ import { TOOL_ACTIVATION_SECTION, toolDeclarationPolicy, toolActivationSection, import { orderSystemRows, SystemTranscriptJournal } from "./system-transcript-journal.js"; import { planWorkspaceRequiredResult } from "./plan-workspace-error.js"; import { accountModelStream } from "./request-usage.js"; -import { modeToolDenial, retainModeToolDeclaration, withModeExecutionGuard } from "./mode-tool-access.js"; +import { + contractModeWithholdsTool, + modeToolDenial, + retainModeToolDeclaration, + withModeExecutionGuard, +} from "./mode-tool-access.js"; import { restoreHostedSearchReplay } from "./hosted-search-replay.js"; import { requestExtensionUi } from "./extensions/ui-request.js"; import { readLocalRequestErrorDetails } from "./local-request-errors.js"; @@ -3871,18 +3876,12 @@ Do not invent objections or turn speculative risks into blockers. Stop when the ? [createJevClassifierTool(this.jevApiKey)] : []; - // Plugins contribute Agent tools by default. Plan/Goal modes only - // expose plugins that declare plan-safe actions (ADR 0211); the - // host still enforces the per-action restriction at execute time. - const visiblePluginTools = - this.mode === "agent" - ? this.pluginTools - : this.pluginTools.filter( - (def) => - Array.isArray(def.planSafeActions) && - def.planSafeActions.length > 0, - ); - const pluginTools: AgentTool[] = visiblePluginTools.map((def) => { + // Plugins contribute Agent tools by default. Every contribution reaches + // the catalog and the mode filter decides what survives: a plugin tool + // needs `planSafeActions` (ADR 0211), while an MCP tool has no such + // declaration and must stay visible for its approval card to be reachable + // (ADR 0330). The host still enforces both at execute time. + const pluginTools: AgentTool[] = this.pluginTools.map((def) => { // Plan/Goal modes annotate the description so the model knows which // actions it may actually call. const baseDescription = @@ -4026,6 +4025,11 @@ Do not invent objections or turn speculative risks into blockers. Stop when the return this.declarationPolicy?.tools ?? this.activeTools(); } + private isExtensionTool(name: string): boolean { + return this.extensionRunner?.getAgentTools().some((tool) => tool.name === name) ?? false; + } + + /** Whether the plugin runtime will let this plugin tool run in a contract mode. */ private isPlanSafePluginTool(name: string): boolean { return this.pluginTools.some( (def) => @@ -4035,26 +4039,29 @@ Do not invent objections or turn speculative risks into blockers. Stop when the ); } + /** + * What the model may see in a contract mode. A withheld name leaves the + * catalog and is refused at execute time; every other name is declared, and + * the host permission path decides whether the call runs. Withholding is + * reserved for calls the mode can never permit, so `ToolSearch` stays and an + * unclassifiable MCP tool can still be discovered and then approved on its + * own card. + */ private isToolAllowedInMode(name: string): boolean { - const kind = proposalKindForMode(this.mode); - if (!kind) return true; - // Contract modes are read-only: inspection tools, plan-safe plugin - // actions (ADR 0211), and the one submit tool that belongs to this kind. - if (this.isPlanSafePluginTool(name)) return true; - return new Set([ - "Read", - "Glob", - "Grep", - "BrowserPreview", - "Bash", - ASK_TOOL_NAME, - CONTEXT_COMPACTION_TOOL_NAME, - SUBMIT_TOOL_NAMES[kind], - ]).has(name); + if (!proposalKindForMode(this.mode)) return true; + if (contractModeWithholdsTool(name)) return false; + // A trusted-extension tool runs in this process, so the host permission + // path never sees the call and cannot gate it. A contract mode therefore + // cannot offer one: withholding it is the only boundary available. + if (this.isExtensionTool(name)) return false; + // A plugin tool is admitted only when the plugin runtime will also hold it to + // a declared action list. One without a list is refused by the host, so + // declaring it would only offer the model a call that cannot succeed. + return !name.startsWith("plugin_") || this.isPlanSafePluginTool(name); } private isCoreTool(name: string): boolean { - return ( + if ( name === CONTEXT_COMPACTION_TOOL_NAME || retainModeToolDeclaration(name) || MODE_TRANSITION_TOOL_NAMES.has(name) || @@ -4064,20 +4071,17 @@ Do not invent objections or turn speculative risks into blockers. Stop when the name === SUBAGENT_TOOL_NAME || name === SUBAGENT_WAIT_TOOL_NAME || name === SUBAGENT_LIST_TOOL_NAME || - name === SUBAGENT_STOP_TOOL_NAME || - (this.mode === "agent" - ? AGENT_CORE_TOOL_NAMES.has(name) - : proposalKindForMode(this.mode) - ? new Set([ - "Read", - "Glob", - "Grep", - "Bash", - "BrowserPreview", - ASK_TOOL_NAME, - ]).has(name) || this.isPlanSafePluginTool(name) - : CHAT_CORE_TOOL_NAMES.has(name)) - ); + name === SUBAGENT_STOP_TOOL_NAME + ) { + return true; + } + if (this.mode === "agent") return AGENT_CORE_TOOL_NAMES.has(name); + if (!proposalKindForMode(this.mode)) return CHAT_CORE_TOOL_NAMES.has(name); + // A contract mode declares everything it has not withheld. An MCP tool is + // unclassifiable, so it must be visible for its own card to be reachable; + // deferring it behind ToolSearch would leave the approval path unreachable + // for the tools that need it most. + return !contractModeWithholdsTool(name); } private activeTools(): AgentTool[] { From 9c9a2e41133fbfcfba2bc7a2a121223b9793c6cb Mon Sep 17 00:00:00 2001 From: chengzhi-c Date: Fri, 16 Oct 2026 15:31:23 +0800 Subject: [PATCH 2/5] feat(host-core): remember an approved contract-mode tool for the session Plan and Goal gate Bash and user MCP tools behind an approval card, but the card came back on every call: the gate answered who decides and left the repeated question in place. An exact-tool `allow-session` grant now suppresses later cards for that tool in that session, which is what the card is for, and it does not carry to another tool or another session. The same change closes the gap the ADR left open for plugin-contributed MCP tools. That registration path carries no enforced plan-safe action list, so the host could only take the caller's word for it. The execution callback now refuses Plan and Goal itself, before the server is called, instead of trusting admission metadata the caller supplies. MCP tools stay discoverable through ToolSearch rather than being declared core, so a server's schemas do not enter every contract-mode request. The catalog and the execution guard keep one predicate for what a contract mode permits, and the extension and retained-reference denials are covered by tests that fail without the guard. closes #1112 --- apps/desktop/electron/main/plugin-runtime.ts | 16 +- .../test/plugin-tool-invocations.test.mjs | 37 +++- crates/host-core/src/permissions.rs | 146 ++++---------- docs/adr/0211-plan-safe-plugin-actions.md | 24 +-- ...0330-contract-modes-deny-what-is-unsafe.md | 182 ++++-------------- docs/adr/README.md | 2 +- docs/spec/00-baseline.md | 9 +- .../03-runtime/03-tools-and-permissions.md | 22 +-- docs/spec/06-delivery/04-e2e-test-plan.md | 17 +- .../03-runtime/03-tools-and-permissions.md | 18 +- .../spec/06-delivery/04-e2e-test-plan.md | 12 +- .../src/fixed-tool-runtime.test.ts | 21 +- .../agent-runtime/src/mode-tool-access.ts | 42 +--- packages/agent-runtime/src/runtime.test.ts | 77 +++++--- packages/agent-runtime/src/runtime.ts | 32 +-- scripts/e2e-plan.mjs | 24 ++- 16 files changed, 258 insertions(+), 423 deletions(-) diff --git a/apps/desktop/electron/main/plugin-runtime.ts b/apps/desktop/electron/main/plugin-runtime.ts index 26e5344dab..36d44a8ef4 100644 --- a/apps/desktop/electron/main/plugin-runtime.ts +++ b/apps/desktop/electron/main/plugin-runtime.ts @@ -3961,11 +3961,17 @@ export class PluginRuntime { // Remote code the desktop cannot inspect; never silently auto-approved. risk: "medium", schema: tool.inputSchema, - execute: async (toolArgs, ctx) => this.mcpCalls.run( - ctx?.sessionId, - (signal) => client.callTool(tool.name, toolArgs, signal), - ctx?.signal, - ), + execute: async (toolArgs, ctx) => { + // MCP contributions cannot declare an enforced plan-safe action list. + if (ctx?.mode === "plan" || ctx?.mode === "goal") { + throw apiError("PERMISSION_DENIED", `plugin tool ${name} is not available in ${ctx.mode} mode`); + } + return this.mcpCalls.run( + ctx?.sessionId, + (signal) => client.callTool(tool.name, toolArgs, signal), + ctx?.signal, + ); + }, }); } } diff --git a/apps/desktop/test/plugin-tool-invocations.test.mjs b/apps/desktop/test/plugin-tool-invocations.test.mjs index 079440dfb3..36307c7439 100644 --- a/apps/desktop/test/plugin-tool-invocations.test.mjs +++ b/apps/desktop/test/plugin-tool-invocations.test.mjs @@ -1,4 +1,5 @@ import assert from "node:assert/strict"; +import { createServer } from "node:http"; import { fork } from "node:child_process"; import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { register } from "node:module"; @@ -74,12 +75,13 @@ const PLUGIN = ` }; `; -async function fixture(t, onInvoke) { +async function fixture(t, onInvoke, mcpServers = []) { const dir = mkdtempSync(join(tmpdir(), "pi-tool-invocations-")); const id = "demo.invocations"; - const permissions = ["agent.tool.register", "browser.cdp", "desktop.control", "session.read"]; + const permissions = ["agent.tool.register", "browser.cdp", "desktop.control", "session.read", ...(mcpServers.length ? ["mcp.server.remote"] : [])]; writeFileSync(join(dir, "manifest.json"), JSON.stringify({ schemaVersion: 1, id, name: id, version: "0.0.1", main: "main.js", permissions, + net: { domains: ["127.0.0.1"] }, contributes: { mcpServers }, })); writeFileSync(join(dir, "main.js"), PLUGIN); const calls = []; @@ -200,3 +202,34 @@ test("the tool deadline aborts child work and revokes its invocation", { timeout t.mock.timers.reset(); assert.equal(await runtime.invokePanelBridge(id, "probe.aborted", { sessionId: "a" }), true); }); + +test("plugin-contributed MCP tools reject contract modes before calling the server", { timeout: 10000 }, async (t) => { + const calls = []; + const server = createServer(async (req, res) => { + if (req.method !== "POST") { res.writeHead(405).end(); return; } + let body = ""; + for await (const chunk of req) body += chunk; + const message = JSON.parse(body); + if (message.id === undefined) { res.writeHead(202).end(); return; } + const result = message.method === "initialize" + ? { protocolVersion: message.params.protocolVersion, capabilities: { tools: {} }, serverInfo: { name: "fixture", version: "1" } } + : message.method === "tools/list" + ? { tools: [{ name: "mutate", inputSchema: { type: "object" } }] } + : (calls.push(message.params), { content: [{ type: "text", text: "called" }] }); + res.writeHead(200, { "content-type": "application/json" }); + res.end(JSON.stringify({ jsonrpc: "2.0", id: message.id, result })); + }); + await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve)); + t.after(() => new Promise((resolve) => { server.closeAllConnections(); server.close(resolve); })); + const { runtime } = await fixture(t, async () => ({}), [{ + id: "fixture", transport: "http", url: `http://127.0.0.1:${server.address().port}/mcp`, + }]); + const tool = runtime.getTools().find((entry) => entry.name.endsWith("mutate")); + assert.ok(tool); + for (const mode of ["plan", "goal"]) { + await assert.rejects(tool.execute({}, { sessionId: "contract", mode }), { code: "PERMISSION_DENIED" }); + } + assert.equal(calls.length, 0); + await tool.execute({}, { sessionId: "agent", mode: "agent" }); + assert.equal(calls.length, 1); +}); diff --git a/crates/host-core/src/permissions.rs b/crates/host-core/src/permissions.rs index 91705d5ce2..12cbaa1c46 100644 --- a/crates/host-core/src/permissions.rs +++ b/crates/host-core/src/permissions.rs @@ -110,8 +110,7 @@ pub struct PendingPermission { pub created_at: String, } -/// How a contract mode admits a tool that `PermissionManager::plan_mode_allows` -/// accepted. +/// Contract admission is independent of risk and automatic permission modes. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum ContractAdmission { /// Fall through to the ordinary permission path. @@ -149,19 +148,7 @@ impl PermissionManager { } } - /// The builtin tools a contract mode may run without a card. - /// - /// This is an allowlist, not a denylist, and that is the point: a builtin - /// added later is denied until someone decides what it does, so the mode - /// cannot widen by omission. Every entry is read-only — it inspects the - /// workspace, asks the user a question, or hands the negotiation back to - /// the host. `new_context` and `ToolSearch` never reach this gate (they are - /// sidecar-side) but are listed so the two sides of the bridge agree. - /// - /// `Write`, `Edit`, `TodoWrite`, `GenerateImages` and the delegation tools - /// stay out: they change files, session state, or spawn work. A contract - /// mode denies them outright, and no permission mode or session grant - /// re-enables them. + // Unclassified builtins stay denied; remote tools are gated separately. const CONTRACT_MODE_READ_ONLY: [&str; 7] = [ "Read", "Glob", @@ -171,42 +158,11 @@ impl PermissionManager { "new_context", "ToolSearch", ]; - - /// Whether a contract mode lets a call reach the ordinary permission path. - /// - /// A `true` answer is not a grant: the caller still applies risk, - /// permission mode, grants, and the external-path rule. A `false` answer is - /// a hard deny that no permission mode, session grant, or scratch path can - /// re-enable. - /// - /// This is the hard-deny question only. `Bash` and an unclassifiable MCP - /// tool answer `false` here and `Approval` from [`Self::contract_admission`], - /// so a caller that needs to distinguish a mode denial from a card the user - /// then refused must ask `contract_admission`, not this. - pub fn plan_mode_allows(tool_name: &str) -> bool { - Self::CONTRACT_MODE_READ_ONLY.contains(&tool_name) - } - - /// The contract modes' own gate. - /// - /// A read-only builtin falls through to the ordinary permission path, and - /// the one submit tool that belongs to this mode is added by the runtime. - /// `Bash` is gated rather than parsed: what a command does is decided by - /// the shell that runs it, and any static reading of the string would be - /// wrong in the permissive direction, so it takes a card every time. - /// - /// An MCP tool carries no classification signal at all — the name means - /// nothing and a server's self-declared annotation is untrusted — so it - /// also takes a card rather than running unread. A plugin tool does carry - /// one: a non-empty `planSafeActions` list means the plugin runtime already - /// restricts it to actions the user accepted, so it falls through. - /// Without that list it is refused, because the plugin runtime refuses it - /// too and an approval card the callee then ignores would be theater. pub fn contract_admission( tool_name: &str, plan_safe_actions: Option<&[String]>, ) -> ContractAdmission { - if Self::plan_mode_allows(tool_name) { + if Self::CONTRACT_MODE_READ_ONLY.contains(&tool_name) { return ContractAdmission::Allowed; } if tool_name == "Bash" || tool_name.starts_with("mcp_") { @@ -274,6 +230,9 @@ impl PermissionManager { requires_external_path_permission, plan_safe_actions, } = params; + let has_session_grant = session_grants + .get(session_id) + .is_some_and(|grants| grants.iter().any(|name| name == tool_name)); // A contract mode decides first, and that decision precedes low-risk // classification, auto, grants, and scratch paths, so none of them can // re-enable what it denied (D198, and Goal shares Plan's gate). @@ -282,7 +241,9 @@ impl PermissionManager { ContractAdmission::Denied => return Some(PermissionDecision::Deny), // No permission mode short-circuits an unclassified call: an // opaque tool is exactly the case where `auto` must not decide. - ContractAdmission::Approval => return None, + ContractAdmission::Approval => { + return has_session_grant.then_some(PermissionDecision::AllowSession); + } ContractAdmission::Allowed => {} } } @@ -291,11 +252,7 @@ impl PermissionManager { if permission_mode == "auto" { return Some(PermissionDecision::AllowOnce); } - if session_grants - .get(session_id) - .map(|g| g.iter().any(|t| t == tool_name)) - .unwrap_or(false) - { + if has_session_grant { return Some(PermissionDecision::AllowSession); } return None; @@ -313,11 +270,7 @@ impl PermissionManager { if mode_allows { return Some(PermissionDecision::AllowOnce); } - if session_grants - .get(session_id) - .map(|g| g.iter().any(|t| t == tool_name)) - .unwrap_or(false) - { + if has_session_grant { return Some(PermissionDecision::AllowSession); } None @@ -574,15 +527,6 @@ mod tests { )); } - #[test] - fn plan_mode_denies_unavailable_tools_regardless_of_permission_mode() { - let pm = PermissionManager::default(); - for mode in ["ask", "accept-edits", "auto"] { - let d = pm.evaluate_auto_with_permission_mode("s", "Write", "plan", mode, &no_grants()); - assert_eq!(d, Some(PermissionDecision::Deny), "plan + {mode}"); - } - } - /// A contract mode denies what changes state, whatever the permission mode /// would otherwise allow. `auto` and `accept-edits` cannot re-enable it, /// and neither can a session grant. @@ -618,21 +562,40 @@ mod tests { } } - /// Bash is gated, not parsed. Reading the command string would be wrong in - /// the permissive direction, so a contract mode takes a card every time - /// instead of letting `auto` decide what the shell may do. #[test] - fn contract_modes_always_take_a_card_for_bash() { + fn contract_modes_require_approval_unless_the_tool_has_a_session_grant() { let pm = PermissionManager::default(); - let mut grants = HashMap::new(); - grants.insert("s".to_string(), vec!["Bash".to_string()]); + let grants = HashMap::from([( + "s".to_string(), + vec!["Bash".to_string(), "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", "Bash", contract, mode, &grants), - None, - "{contract} + {mode} must prompt even with a session grant" - ); + for tool in ["Bash", "mcp_srv_tool"] { + assert_eq!( + pm.evaluate_auto_with_permission_mode( + "s", + tool, + contract, + mode, + &no_grants() + ), + None, + "{contract} + {mode} must prompt for {tool} without a grant" + ); + assert_eq!( + pm.evaluate_auto_with_permission_mode("s", tool, contract, mode, &grants), + Some(PermissionDecision::AllowSession), + "{contract} + {mode} must honor the grant for {tool}" + ); + assert_eq!( + pm.evaluate_auto_with_permission_mode( + "other", tool, contract, mode, &grants + ), + None, + "{contract} + {mode} must not share {tool}'s grant across sessions" + ); + } } } } @@ -687,7 +650,6 @@ mod tests { ); } } - assert!(!PermissionManager::plan_mode_allows("TodoWrite")); } #[test] @@ -821,32 +783,6 @@ mod tests { assert!(other.is_none(), "grant is scoped to the exact tool name"); } - /// An MCP tool has no classification signal, so a contract mode routes it - /// to a card. The grant below is deliberately present and deliberately not - /// enough: `auto` must not stand in for a decision about a tool whose name - /// is all the host knows. - #[test] - fn contract_modes_route_unclassified_mcp_tools_to_a_card() { - 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 - ), - None, - "mcp tool must prompt in {contract} + {mode}" - ); - } - } - } - /// The escape hatch is a declaration, not a default: a plugin tool is /// admitted only when the plugin runtime will also hold it to the same /// action list. Without one the tool is refused, because an approval the diff --git a/docs/adr/0211-plan-safe-plugin-actions.md b/docs/adr/0211-plan-safe-plugin-actions.md index 2476b498cf..8ff2433cdd 100644 --- a/docs/adr/0211-plan-safe-plugin-actions.md +++ b/docs/adr/0211-plan-safe-plugin-actions.md @@ -60,11 +60,6 @@ the model knows which actions it can actually call. The full plugin tool is still available to the renderer (skill catalog, plugin page); the filter is on the agent-visible tool list, not on registration. -*(Amended by ADR 0330 §5: a contract mode withholds only what it can never -permit. A plugin tool without `planSafeActions` is still withheld, because the -host denies it, but that is no longer the only reason a name can leave the -catalog — an MCP tool stays declared so its approval card is reachable.)* - ### 3. Runtime forwards the declaration to host-core When the runtime calls `tools.execute` for a `plugin_*` tool it includes @@ -75,21 +70,13 @@ in the `plugins.execute` notification alongside the session mode. ### 4. Host-core admits plan-safe plugin tools in contract modes `crates/host-core/src/permissions.rs` retains the existing contract-mode -hard deny (`plan_mode_allows`) for `Write`, `Edit`, unknown tools, and +hard deny (`contract_admission`, ADR 0330) for `Write`, `Edit`, unknown builtins, and plugin tools that arrive without `planSafeActions`. A plugin tool that arrives with a non-empty `planSafeActions` list now passes the contract gate. The list does not change any other rule: low-risk auto-allow, the external-path prompt, session grants, and the `auto` mode permission posture all apply the same way they do for any Agent-mode plugin call. -*(Amended by ADR 0330: the gate is now `contract_admission`, and it answers with -three outcomes instead of two. `Write`, `Edit`, and every builtin outside the -read-only set are denied; `Bash` and an MCP tool take a permission card, because -neither carries a usable classification signal; a `planSafeActions` plugin tool -reaches the ordinary permission path exactly as described above. "Unknown tools" -are denied because the read-only set is an allowlist, not because a name list -happens to omit them.)* - ### 5. Plugin-runtime enforces the action restriction `apps/desktop/electron/main/plugin-runtime.ts` validates each plugin @@ -133,19 +120,14 @@ process-wide and macOS spends `Cmd+W` on its own close-window command.)* - Plan and Goal modes can inspect external resources through any plugin whose author opts in. The bundled Browser plugin is the first - beneficiary; MCP tools can do the same with the same declaration. - *(Amended by ADR 0330: an MCP tool needs no declaration — it takes an - approval card instead, so a server whose tools are read-only is usable - without the user maintaining a list.)* + beneficiary. User MCP tools follow ADR 0330's approval policy; plugin-contributed + MCP tools have no plan-safe declaration and remain unavailable in Plan/Goal. - Plugin authors remain responsible for declaring exactly which actions are read-only. A wrong declaration fails at plugin registration, not at the user's prompt. - The contract modes' hard deny for `Write`, `Edit`, and unknown tools is unchanged. A plugin that does not declare `planSafeActions` keeps the ADR 0052 / ADR 0053 behavior. - *(Amended by ADR 0330: still true, and the mechanism is now an explicit - read-only allowlist. `planSafeActions` is no longer how a plugin becomes - visible in Plan and Goal; it is how a plugin tool is admitted without a card.)* - The defense-in-depth check in the plugin-runtime means a bug in the runtime or host-core cannot escalate a Plan call into a mutation: the plugin rejects it itself. diff --git a/docs/adr/0330-contract-modes-deny-what-is-unsafe.md b/docs/adr/0330-contract-modes-deny-what-is-unsafe.md index 91d08743a5..2588564572 100644 --- a/docs/adr/0330-contract-modes-deny-what-is-unsafe.md +++ b/docs/adr/0330-contract-modes-deny-what-is-unsafe.md @@ -3,165 +3,61 @@ - Status: Accepted - Date: 2026-10-15 - Deciders: PI-Desktop core -- Amends: ADR 0052 §3, ADR 0053 §1, ADR 0211 §2 and §4 -- Related: ADR 0064, ADR 0087, ADR 0089, `mcp-tool-approval-risk` +- Amends: ADR 0052 §3, ADR 0053 §1, ADR 0211 §4 +- Related: ADR 0087, ADR 0089, `mcp-tool-approval-risk` ## Context -Plan and Goal decide which tools they allow by naming them. `plan_mode_allows` -holds a short list of read-only builtins, and every other name is denied: any -MCP tool, any plugin tool without `planSafeActions`, and any builtin added after -that list was written. - -That shape has two costs. - -The first is that a name is not a classification. `mcp_ctx_docs_search_docs` -says nothing about whether it mutates, and the server's own risk annotation is -not evidence — `tool_risk_with_declared` already pins `mcp_` to `Medium` for -exactly that reason. So the list cannot distinguish a read-only search from a -write, and it answers by refusing both. A user who asks Plan mode to look -something up in a configured MCP server gets nothing back, while the mode prompt -tells the model to inspect the workspace and the contract is meant to be about -mutation, not about provenance. - -The second is that the list is the boundary. The mode prompt already tells the -model not to change anything in Plan or Goal, and that guidance is not a -boundary. The real gate has to be in the permission path, and a gate built out of -a name list answers the wrong question: it asks where a tool came from instead of -what the call would do. +Plan and Goal must keep direct mutation tools unavailable without making +read-only MCP capabilities inaccessible. Shell commands and opaque MCP tools +cannot be classified reliably by their names or self-reported risk. Mode prompts +provide guidance; permission checks decide whether a call may execute. ## Decision -### 1. A contract mode denies by effect, not by name - -A read-only builtin reaches the ordinary permission path. A builtin whose purpose -is to change state is denied outright. Nothing in between is admitted by default. - -The read-only set is an allowlist: `Read`, `Glob`, `Grep`, `BrowserPreview`, -`asktool`, `new_context`, `ToolSearch`, plus the one submit tool that belongs to -the mode. `Write`, `Edit`, `TodoWrite`, `GenerateImages`, and the delegation -tools are outside it and stay denied in every permission mode. - -It is deliberately an allowlist rather than a denylist. A builtin added later is -denied until someone decides what it does, so the mode cannot widen by omission. -`auto` and `accept-edits` cannot re-enable a denied tool, and neither can a -session grant or a scratch path. - -### 2. Bash is gated, not parsed - -A shell command's effect is decided by the shell that runs it. Any static reading -of the command string would be wrong, and it would be wrong in the permissive -direction — that is the failure mode worth avoiding. - -So `Bash` takes a permission card in a contract mode in **every** permission -mode, including `auto`. The host does not try to decide whether a particular -command mutates. A `Bash` session grant still suppresses later prompts for the -rest of that session, which is how a user who has decided what they are doing -avoids answering the same question repeatedly. - -This is a behavior change: `auto` no longer auto-allows `Bash` in Plan or Goal. -The mode selector keeps its meaning for every other tool. - -### 3. An unclassifiable tool goes to a card - -An MCP tool has no classification signal at all. It is routed to a permission -card rather than admitted, and the tool stays in the model's catalog so the card -is reachable. `auto` does not stand in for the decision, because "we could not -tell" is exactly the case where an automatic policy has nothing to go on. - -The card is the fail-closed answer that still leaves the capability usable: -nothing runs until a human has seen the tool name. - -### 4. `planSafeActions` stays the plugin escape hatch - -A plugin tool does carry a classification signal, because the plugin runtime -enforces it. A non-empty `planSafeActions` list means the runtime will reject any -`action` outside the list, so the tool reaches the ordinary permission path. - -A plugin tool without that list is refused, not routed to a card. The -plugin-runtime already rejects the same call with `PERMISSION_DENIED`, so an -approval the callee then ignores would not be a gate. - -### 5. The runtime withholds only what the mode can never permit - -`isToolAllowedInMode` keeps its allowlist shape for builtins, because a builtin -withheld here is one the model cannot discover at all. An MCP tool is not -withheld: it is declared so its card is reachable. A plugin tool without -`planSafeActions` is withheld, because the host denies it anyway. - -A trusted-extension tool is withheld in a contract mode. It executes in the -sidecar process, so no `tools.execute` call reaches the host and the permission -path cannot gate it — withholding it is the only boundary that exists. This is -not a statement about the extension's trust level: an extension is user-granted -trusted code, and it keeps working in Agent mode. It is that a contract mode -must not offer a capability whose calls the host never sees. - -### 6. What this does not fix - -A plugin may contribute MCP tools, and that registration path carries no -`planSafeActions` declaration of its own. The host therefore decides from the -list the caller sends, which it cannot cross-check against the plugin registry. -An `agentTools` contribution is different: the plugin runtime refuses it in a -contract mode when the list is empty, so the declaration and the enforcement -agree. Closing the gap for plugin-contributed MCP tools means giving that path -the same runtime check, which is separate work and is not claimed here. +- Read-only builtins use the ordinary permission path: `Read`, `Glob`, `Grep`, + `BrowserPreview`, `asktool`, `new_context`, and `ToolSearch`. The runtime also + permits the submit tool belonging to the current mode. +- Other builtins are denied before risk, permission mode, session grants, or + scratch-path exceptions. New builtins remain denied until classified. +- `Bash` and user MCP tools require explicit approval in every permission mode, + including `auto`. An exact-tool `allow-session` grant suppresses subsequent + prompts only in that session. +- Plugin tools require a non-empty `planSafeActions` list. Admission does not + grant permission: ordinary risk, external-path, and session-grant rules still + apply. The plugin runtime rejects unlisted actions before dispatch. +- Plugin-contributed MCP tools have no supported `planSafeActions` declaration. + Their execution callback rejects Plan and Goal independently of caller-supplied + admission metadata. +- Trusted-extension tools and Agent-only local tools remain unavailable in + contract modes, including through references retained before a mode change. + These tools can execute outside the host permission path. +- MCP tools remain discoverable through existing ToolSearch and explicit + selections. Admission does not make every MCP schema a core declaration. +- Refusing an approval card returns `TOOL_DENIED`; a contract-mode hard denial + retains its existing `_IN_PLAN` error code in both Plan and Goal. ## Consequences -- Plan and Goal can use a read-only MCP tool after one approval, remembered per - tool for the session. This is the capability the per-server `planSafeTools` - list was reaching for, without a per-server declaration to maintain. -- `Bash` in Plan and Goal always shows a card. On a platform with no read-only - filesystem primitive this is the honest gate, but it is friction, and the UI - must make it visible rather than let `auto` imply otherwise. -- A mutating `Bash` command can still run once the user approves it. The gate - bounds who decides, not what the command does; a future read-only view would - bound the latter. -- A builtin that is not on the read-only list is denied until it is classified. - Adding a builtin now requires a decision about Plan and Goal, which is the - intended cost. -- `planSafeActions` is no longer how a plugin becomes visible in Plan and Goal; - it is how a plugin tool is admitted without a card. -- A trusted-extension tool is unavailable in Plan and Goal. An extension that - needs to contribute inspection capability has to do it through a plugin tool - with `planSafeActions`, which the host can gate. - -## Alternatives rejected - -### Keep the name list and extend it per server - -Rejected because it moves the maintenance burden onto the user and still cannot -classify a tool whose name carries no meaning. It also answers the wrong -question: the mode's contract is about mutation, not provenance. - -### Deny by a list of known-mutating tools - -Rejected because it fails open. A builtin or plugin tool added later would be -admitted in Plan and Goal by default, and `auto` would then allow it without a -card. The allowlist fails closed on the same input. - -### Parse `Bash` commands to find mutations - -Rejected because it cannot be made reliable. Shell syntax has too many ways to -reach the same write, and a parser that misses one produces a gate that reports -success while the mutation happens. - -### Allow opaque MCP and plugin tools outright +Plan and Goal can use MCP capabilities after user approval without a separate +server allowlist. `auto` no longer automatically permits their Bash or MCP calls. +Agent-mode permission behavior is unchanged. -Rejected because it would need a classification signal that does not exist. -Treating "unknown" as "safe" is the default this ADR exists to avoid. +Approval is not filesystem isolation: an approved shell command or opaque MCP +call may mutate state. This policy controls authorization, not the effects of +arbitrary code. Plugin authors remain responsible for the semantics of actions +they declare plan-safe. -### Route an opaque plugin tool to a card as well +## Alternatives -Rejected because the plugin-runtime refuses the same call. A card whose approval -the callee ignores would mislead the user about what it did. +- Parsing shell commands cannot reliably establish whether they mutate files. +- Trusting MCP risk annotations lets remote code classify its own access. +- A builtin denylist admits newly added tools by omission; explicit classification + keeps unknown local execution paths closed. ## References - `crates/host-core/src/permissions.rs` -- `crates/host-core/src/rpc/mod.rs` - `packages/agent-runtime/src/mode-tool-access.ts` -- `packages/agent-runtime/src/runtime.ts` - `apps/desktop/electron/main/plugin-runtime.ts` - `docs/spec/03-runtime/03-tools-and-permissions.md` -- `docs/spec/03-runtime/06-host-rpc-protocol.md` diff --git a/docs/adr/README.md b/docs/adr/README.md index 426878e9e2..0e02e2bcc1 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -375,7 +375,7 @@ Each ADR includes: | 0327 | [Automatic remote Host version realignment](0327-remote-host-version-realignment.md) | Accepted for experimental implementation (amends ADR 0326 item 6; Linux SSH qualification pending) | | 0328 | [Open arbitrary local files in the embedded Browser](0328-browser-arbitrary-local-files.md) | Accepted (D657; amends ADR 0170; issue #1627) | | 0329 | [Isolate parallel sessions of one project in git worktrees](0329-session-worktree-isolation.md) | Accepted (Windows E2E pending) | -| 0330 | [Contract modes deny what is unsafe](0330-contract-modes-deny-what-is-unsafe.md) | Accepted (amends ADR 0052 §3, ADR 0053 §1, ADR 0211 §2 and §4) | +| 0330 | [Contract modes deny what is unsafe](0330-contract-modes-deny-what-is-unsafe.md) | Accepted (amends ADR 0052 §3, ADR 0053 §1, ADR 0211 §4) | | plugin-fetch-redirect-policy | [Host-enforced plugin fetch redirect policy](plugin-fetch-redirect-policy.md) | Accepted (implementation candidate for #1475) | | local-agent-remote-workspace-tools | [Local Agent and model requests for remote workspaces](local-agent-remote-workspace-tools.md) | Accepted for implementation | diff --git a/docs/spec/00-baseline.md b/docs/spec/00-baseline.md index a6c502e63f..475fca7b55 100644 --- a/docs/spec/00-baseline.md +++ b/docs/spec/00-baseline.md @@ -194,10 +194,11 @@ checkpoints, and one overflow retry. The model can request a new window through `new_context`; every compaction adds a transcript row and one warning. No user-facing settings** -44. Plan tools and policy: **Read / Glob / Grep / BrowserPreview / Bash plus - `EnterPlanMode` and `SubmitPlan`; Write/Edit/plugin and - unknown tools are denied. Bash follows `ask`, `accept-edits`, or `auto`, so - Plan is planning intent, not a strict read-only security profile.** +44. Plan tools and policy (ADR 0330 amendment): **Read / Glob / Grep / + BrowserPreview / asktool / Bash / SubmitPlan, discoverable user MCP tools, + and plugins with `planSafeActions`. Unclassified builtins remain denied. + Bash and user MCP tools require approval or an exact-tool session grant, + including under `auto`; this is authorization, not filesystem isolation.** 45. Plan checkpoint: **`SubmitPlan(title, markdown, question)` causes host-core to preserve the exact Markdown bytes in a new unique `/.pi/plan/*.md` artifact, while title/question remain diff --git a/docs/spec/03-runtime/03-tools-and-permissions.md b/docs/spec/03-runtime/03-tools-and-permissions.md index af2769eae1..2114917958 100644 --- a/docs/spec/03-runtime/03-tools-and-permissions.md +++ b/docs/spec/03-runtime/03-tools-and-permissions.md @@ -9,12 +9,12 @@ |---|---| | Default mode | Agent | | Agent tools | Read / Glob / Grep / Write / Edit / Bash + registered plugin tools + optional Jev classifier | -| Plan tools | Read / Glob / Grep / BrowserPreview / asktool / SubmitPlan, plus MCP tools and `planSafeActions` plugin tools through a permission card | -| Goal tools | Read / Glob / Grep / BrowserPreview / asktool / SubmitGoal, plus MCP tools and `planSafeActions` plugin tools through a permission card | +| Plan tools | Read / Glob / Grep / BrowserPreview / asktool / Bash / SubmitPlan, plus discoverable MCP tools and plugins with `planSafeActions`; ordinary permissions still apply | +| Goal tools | Read / Glob / Grep / BrowserPreview / asktool / Bash / SubmitGoal, plus discoverable MCP tools and plugins with `planSafeActions`; ordinary permissions still apply | | Plan and Goal hard deny | Write / Edit / TodoWrite / GenerateImages / the delegation tools / the other kind's submit tool. A builtin that is not on the read-only list is denied until it is classified | -| Plan and Goal Bash | Always takes a permission card; the permission mode does not skip it | -| Plan and Goal unclassified tools | An MCP tool has no classification signal, so it takes a permission card instead of running unread; a `plugin_` tool needs a non-empty `planSafeActions` list | -| Plugin `planSafeActions` | Non-empty array of `action` strings; host admits listed tools, plugin-runtime rejects any action outside the list (ADR 0211) | +| Plan and Goal Bash | Requires approval, including in `auto`, unless the session has an exact-tool grant | +| Plan and Goal unclassified tools | MCP tools require approval or an exact-tool session grant; `plugin_` tools require non-empty `planSafeActions` | +| Plugin `planSafeActions` | Non-empty array of `action` strings; runtime hides tools without it, host applies ordinary permissions, plugin-runtime rejects unlisted actions (ADR 0211) | | Local permission approval | No automatic deadline; explicit decision or cancellation required | | allow-session scope | toolName | | Bash style | non-interactive; selected host catalog shell with streamed output | @@ -571,9 +571,9 @@ Rules: - Plan retains the permission-mode selector. `Bash` takes a confirmation in every permission mode, so the selector does not change whether Plan may run a command — only how the rest of the tools behave. Plan is planning intent, not a strict - read-only security profile, and the UI states that tradeoff. -- `allow-session` grants continue to work under `ask` and stay scoped to the - session; under `accept-edits`/`auto` they are simply never needed. + read-only security profile. +- `allow-session` grants stay scoped to the exact tool and session. They also + suppress Plan/Goal Bash and MCP prompts under `accept-edits` and `auto`. - Scratch-directory writes (D114) stay prompt-free in every mode. Plan and Goal do not expose `Write`/`Edit`, so that rule cannot make them available there. - UI: Settings → segmented global default; composer shows a per-session chip in @@ -655,11 +655,11 @@ for the current key-log policy. - An MCP tool carries no classification signal — its name says nothing and a server's self-declared risk annotation is untrusted — so a contract mode routes it to a permission card rather than letting `auto` admit it unread. The tool - stays visible to the model so its card is reachable. + remains discoverable through ToolSearch or explicit selections. - A `plugin_` tool is admitted when it declares a non-empty `planSafeActions` list, because the plugin-runtime then rejects any action outside that list - (ADR 0211). Without one it is refused: an approval the callee would reject - anyway is not a gate. + (ADR 0211). Ordinary permissions still apply. Plugin-contributed MCP tools + have no such declaration and are also rejected at their execution callback. - A trusted-extension tool is withheld in Plan and Goal. It runs in the sidecar process, so no `tools.execute` call reaches the host and the permission path cannot gate it; withholding it is the only boundary available. Extensions keep diff --git a/docs/spec/06-delivery/04-e2e-test-plan.md b/docs/spec/06-delivery/04-e2e-test-plan.md index cba9a619b1..b98211c2b4 100644 --- a/docs/spec/06-delivery/04-e2e-test-plan.md +++ b/docs/spec/06-delivery/04-e2e-test-plan.md @@ -3824,8 +3824,8 @@ window; opening a normal panel afterward must still work. the server's self-declared annotation. Allow-once covers only that call; allow-session suppresses further cards for the same `mcp__` 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. + Agent's `auto` runs the tool without a card. Plan and Goal require approval + in every permission mode unless that exact tool has 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 @@ -7436,19 +7436,18 @@ must keep splitting are covered by `markdown-blocks.test.mjs`. regardless of the forged mode, grants, or Auto; a plugin tool without `planSafeActions` is refused with `PLUGIN_DISABLED_IN_PLAN`; an MCP tool and Bash each show a permission card in **every** permission mode, Auto included, - and the approved call then runs. The runtime remains one pi Agent and all - denials are audited. + and the approved call then runs. An exact-tool session grant suppresses repeat + prompts without authorizing other tools or sessions. The runtime remains one + pi Agent and all denials are audited. - **Specs linked**: `03-runtime/02-agent-runtime.md`, `03-runtime/03-tools-and-permissions.md`, `03-runtime/05-host-core-rust.md`, `03-runtime/06-host-rpc-protocol.md`, `05-security/01-security.md`, ADR 0053, ADR 0330 - **Acceptance**: E (tools and permissions), Security - **Milestone**: M6 -- **Status**: Automated (passed 2026-08-04): `test:e2e:plan` plus host-core - permission/policy and agent-runtime tool-composition tests. The Bash and MCP - card behavior is covered by `permissions::tests` and the two - `rpc::tests::{plan,goal}_authorization*` cases; `test:e2e:plan` has not been - re-run against the ADR 0330 posture. +- **Status**: Automated: `test:e2e:plan` covers approval-before-execution and + session-grant reuse; host-core authorization tests and agent-runtime MCP + selection tests cover the permission and catalog boundaries. #### E2E-PLAN-WORKSPACE: Missing workspace does not strand a contract turn diff --git a/docs/zh-CN/spec/03-runtime/03-tools-and-permissions.md b/docs/zh-CN/spec/03-runtime/03-tools-and-permissions.md index 3718c32055..6fc54547bf 100644 --- a/docs/zh-CN/spec/03-runtime/03-tools-and-permissions.md +++ b/docs/zh-CN/spec/03-runtime/03-tools-and-permissions.md @@ -12,12 +12,12 @@ |---|---| | 默认模式 | Agent | | Agent 工具 | 读取 / Glob / Grep / 写入 / 编辑 / Bash + 已注册的插件工具 | -| Plan 工具 | 读取 / Glob / Grep / BrowserPreview / asktool / SubmitPlan，以及经权限卡放行的 MCP 工具和 `planSafeActions` 插件工具 | -| Goal 工具 | 读取 / Glob / Grep / BrowserPreview / asktool / SubmitGoal，以及经权限卡放行的 MCP 工具和 `planSafeActions` 插件工具 | +| Plan 工具 | Read / Glob / Grep / BrowserPreview / asktool / Bash / SubmitPlan，以及可发现的 MCP 工具和声明 `planSafeActions` 的插件；普通权限规则仍适用 | +| Goal 工具 | Read / Glob / Grep / BrowserPreview / asktool / Bash / SubmitGoal，以及可发现的 MCP 工具和声明 `planSafeActions` 的插件；普通权限规则仍适用 | | Plan 和 Goal 硬拒绝 | 写入 / 编辑 / TodoWrite / GenerateImages / 委派工具 / 另一类的提交工具。不在只读清单内的内置工具在分类前一律拒绝 | -| Plan 和 Goal 的 Bash | 始终弹出权限卡；权限模式不会跳过它 | -| Plan 和 Goal 的未分类工具 | MCP 工具没有任何分类信号，因此弹出权限卡而不是未经审查地运行；`plugin_` 工具需要非空的 `planSafeActions` 列表 | -| 插件 `planSafeActions` | 非空的 `action` 字符串数组；host 放行已列出的工具，plugin-runtime 拒绝列表外的任何动作（ADR 0211） | +| Plan 和 Goal 的 Bash | 包括 `auto` 在内均需确认，已有精确工具名的会话授权除外 | +| Plan 和 Goal 的未分类工具 | MCP 工具需要确认或精确工具名的会话授权；`plugin_` 工具需要非空 `planSafeActions` | +| 插件 `planSafeActions` | 非空的 `action` 字符串数组；运行时隐藏无列表工具，host 应用普通权限，plugin-runtime 拒绝未列出的动作（ADR 0211） | | 本地权限确认 | 无自动截止时间；必须明确决定或取消 | | 允许会话范围 | 工具名称 | | 重击风格 | 非交互式；具有流输出的选定主机目录外壳 | @@ -473,8 +473,8 @@ Agent 模式下 `auto` 自动允许。 在两种操作模式下均可用。 - Plan 保留权限模式选择器。 `Bash` 在每种权限模式下都需要确认，因此该选择器不改变 Plan 是否可以运行命令，只影响其余工具的行为。Plan 是规划意图，而不是严格的只读安全配置。 -- `allow-session` 赠款继续在 `ask` 下运作，范围仅限于 - 会议；在 `BrowserPreview`/`ask` 下，根本不需要它们。 +- `allow-session` 仅适用于同一会话的精确工具名，也会在 `accept-edits` 和 `auto` 下 + 免去 Plan/Goal 的 Bash、MCP 重复确认。 - 暂存目录写入 (D114) 在每种模式下都保持无提示。 - UI：设置→分段全局默认；输入框在中显示每个会话的芯片 Agent、Plan 和 Goal 的菜单提供了三种有效模式，无需 @@ -545,9 +545,9 @@ MVP 可以通过写入 SQLite 或日志文件来启动。 host 不读取命令字符串来判定；因此 `auto` 不能代替用户的决定。`Bash` 的会话授权仍会抑制 该会话后续的重复确认。 - MCP 工具没有任何分类信号——名称说明不了什么，服务器自报的风险标注也不可信——因此契约模式 - 把它交给权限卡，而不是让 `auto` 未经审查地放行。该工具对模型保持可见，权限卡才够得着。 + 把它交给权限卡，而不是让 `auto` 未经审查地放行。工具仍可通过 ToolSearch 或显式选择发现。 - 当 `plugin_` 工具声明了非空的 `planSafeActions` 列表时被放行，因为 plugin-runtime 随后会拒绝 - 列表外的任何动作（ADR 0211）。没有该列表则直接拒绝：一个调用方本就会拒绝的批准并不构成门禁。 + 列表外的任何动作（ADR 0211），普通权限规则仍适用。插件贡献的 MCP 工具没有此类声明，执行回调也会拒绝 Plan/Goal。 - 受信扩展工具在 Plan 和 Goal 中被扣留。它在 sidecar 进程内运行，因此没有任何 `tools.execute` 调用到达 host，权限路径无法对它设门；扣留它是唯一可用的边界。扩展在 Agent 模式下不受影响。 - Agent 模式使用权限卡或选定的自动策略处理 diff --git a/docs/zh-CN/spec/06-delivery/04-e2e-test-plan.md b/docs/zh-CN/spec/06-delivery/04-e2e-test-plan.md index caf86a95cd..232ad6b95f 100644 --- a/docs/zh-CN/spec/06-delivery/04-e2e-test-plan.md +++ b/docs/zh-CN/spec/06-delivery/04-e2e-test-plan.md @@ -1619,7 +1619,7 @@ task-candidate E2E 从请求工作树运行，但使用主工作区已经准备 - **先决条件**：一个绑定项目的 Agent 会话；一个用户配置的 stdio MCP 服务器，其工具列表把某个工具标注为只读/低风险。 - **步骤**：1) 会话处于 `ask` 时，让代理调用该 MCP 工具。2) 以“允许一次”回应卡片后再次调用，再以“本会话允许”回应并第三次调用。3) 在新会话中切到 `accept-edits` 并重复调用。4) 切到 `auto` 调用。5) 依次切到 Plan、Goal 调用。 -- **预期**：在 `ask` 与 `accept-edits` 下，每次调用都显示审批卡片，原因为 "MCP server tool requires approval"，风险为 `medium`，与服务器自行声明的标注无关。“允许一次”只覆盖该次调用；“本会话允许”只在该会话内对同一 `mcp__` 名称不再提示，不覆盖该服务器的其他工具。`auto` 不显示卡片直接执行。Plan 与 Goal 即使存在会话授权也拒绝。 +- **预期**：在 `ask` 与 `accept-edits` 下，每次调用都显示审批卡片，原因为 "MCP server tool requires approval"，风险为 `medium`，与服务器自行声明的标注无关。“允许一次”只覆盖该次调用；“本会话允许”只在该会话内对同一 `mcp__` 名称不再提示，不覆盖该服务器的其他工具。Agent 的 `auto` 不显示卡片直接执行。Plan 与 Goal 在所有权限模式下都需确认，已有同一工具的会话授权除外。 - **关联规格**：`03-runtime/03-tools-and-permissions.md`、`05-security/01-security.md`、D640、ADR `mcp-tool-approval-risk` - **状态**：已有单元测试（host-core `permissions.rs` MCP 风险与模式测试）；桌面流程待验证 @@ -4232,17 +4232,15 @@ eleven-tool-round desktop paths are verified by Auto 下运行 Bash。 - **预期**：无论伪造模式、授权还是 Auto，Plan 都拒绝 Write/Edit 以及只读清单之外的任何内置 工具；未声明 `planSafeActions` 的插件工具以 `PLUGIN_DISABLED_IN_PLAN` 被拒绝；MCP 工具和 - Bash 在**每一种**权限模式下（含 Auto）各自弹出权限卡，批准后调用才运行。运行时仍为 - 1 pi Agent，且所有拒绝都会经过审核。 + Bash 在**每一种**权限模式下（含 Auto）各自弹出权限卡，批准后调用才运行。精确工具名的会话授权 + 免去重复确认，但不授权其他工具或会话。运行时仍为同一 pi Agent，且所有拒绝都会经过审计。 - **链接规格**：`03-runtime/02-agent-runtime.md`， `03-runtime/03-tools-and-permissions.md`、`03-runtime/05-host-core-rust.md`、 `03-runtime/06-host-rpc-protocol.md`、`05-security/01-security.md`、ADR 0053、ADR 0330 - **接受**：E（工具和权限）、安全 - **里程碑**：M6 -- **状态**：自动（2026-08-04 通过）：`test:e2e:plan` 加 host-core - permission/policy 和代理运行时工具组合测试。Bash 与 MCP 的权限卡行为由 - `permissions::tests` 以及两个 `rpc::tests::{plan,goal}_authorization*` 用例覆盖； - `test:e2e:plan` 尚未针对 ADR 0330 的姿态重跑。 +- **状态**：自动化：`test:e2e:plan` 覆盖批准前不执行和会话授权复用；host-core + 授权测试及 agent-runtime MCP 选择测试覆盖权限与目录边界。 #### E2E-106：SubmitPlan 拒绝进入可编辑计划并重新提交新工件 diff --git a/packages/agent-runtime/src/fixed-tool-runtime.test.ts b/packages/agent-runtime/src/fixed-tool-runtime.test.ts index 7e53c0aece..79b1f469c5 100644 --- a/packages/agent-runtime/src/fixed-tool-runtime.test.ts +++ b/packages/agent-runtime/src/fixed-tool-runtime.test.ts @@ -44,7 +44,7 @@ async function wireFixture(calls: Call[], beforeReply?: () => Promise) { await new Promise((resolve, reject) => server.close((error) => error ? reject(error) : resolve())); } }; } -function runtimeFixture(history: UiMessage[] = [], tools = pluginTools, provider = flashProvider(), denied = false, mode: "agent" | "plan" = "agent") { +function runtimeFixture(history: UiMessage[] = [], tools = pluginTools, provider = flashProvider(), denied = false, mode: "agent" | "plan" | "goal" = "agent") { const rows = structuredClone(history); const executed: string[] = []; const errors: unknown[] = []; @@ -287,6 +287,25 @@ describe("explicit MCP selection", () => { } finally { await f.runtime.dispose(); await wire.close(); } }); + it.each(["plan", "goal"] as const)("keeps large MCP catalogs discoverable without flooding %s requests", async (mode) => { + const tools = Array.from({ length: 129 }, (_, index) => ({ + name: `mcp_large_tool_${index}`, description: "MCP probe", + parameters: { type: "object", properties: {}, required: [] }, + })); + const wire = await wireFixture([ + { name: "ToolSearch", args: { query: tools[0].name } }, + { name: tools[0].name }, + ]); + const f = runtimeFixture([], tools, flashProvider(), true, mode); + try { + await f.prompt(); + expect(wire.requests[0].tools.length).toBeLessThanOrEqual(128); + expect(wire.requests[0].tools.map(tool => tool.function.name)).not.toContain(tools[1].name); + expect(f.executed).toEqual([tools[0].name]); + expect(f.rows.find(row => row.toolName === tools[0].name)).toMatchObject({ isError: true }); + } finally { await f.runtime.dispose(); await wire.close(); } + }); + it("offers selected MCP tools in Plan mode and still routes them through Host approval", async () => { const wire = await wireFixture([{ name: selectedTools[0].name }]); const f = runtimeFixture([], selectedTools, flashProvider(), true, "plan"); diff --git a/packages/agent-runtime/src/mode-tool-access.ts b/packages/agent-runtime/src/mode-tool-access.ts index 6b00edd67b..5281bebd36 100644 --- a/packages/agent-runtime/src/mode-tool-access.ts +++ b/packages/agent-runtime/src/mode-tool-access.ts @@ -4,49 +4,17 @@ import type { Mode } from "@pi-desktop/shared"; /** Known editing/delegation names stay declared across contract-mode changes. */ const RETAINED_MODE_TOOLS = new Set(["Write", "Edit", "Task", "TaskWait", "TaskList", "TaskStop"]); -/** - * A contract mode withholds tools whose only purpose is to change state. They - * stay declared so a strict gateway never sees an undeclared call, and the - * execution guard below refuses them, but the model is never offered them as a - * real option. - * - * Every other tool is declared, including one this module cannot classify: an - * MCP or plugin tool reaches the model and the host permission path decides - * whether the call runs. Withholding is reserved for a name the mode can never - * permit, because a withheld tool is one the model cannot discover at all. - * - * The host enforces the same posture from the other side in - * `PermissionManager::contract_admission`. A name denied here must also be - * denied there, or the model gets a tool it can never use. - */ -const CONTRACT_MODE_WITHHELD = new Set([ - "Write", - "Edit", - "TodoWrite", - "Task", - "TaskWait", - "TaskList", - "TaskStop", - // Agent-only or state-changing local tools. They are already absent from a - // contract mode's builtin list, so listing them changes no observable - // behavior today; it keeps this set the single answer to "what does a - // contract mode refuse" instead of leaving the agreement to coincidence. - "GenerateImages", - "PluginCheck", - "PluginScaffold", - "PluginPack", - "Skill", - "ScheduledTaskCreate", - "ScheduledTaskUpdate", - "ScheduledTaskDelete", +// Explicit classification also denies stale references to local Agent-only tools. +const CONTRACT_MODE_BUILTINS = new Set([ + "Read", "Glob", "Grep", "BrowserPreview", "Bash", "asktool", "new_context", "ToolSearch", ]); export function retainModeToolDeclaration(name: string): boolean { return RETAINED_MODE_TOOLS.has(name); } -export function contractModeWithholdsTool(name: string): boolean { - return CONTRACT_MODE_WITHHELD.has(name); +export function contractModeAllowsBuiltin(name: string): boolean { + return CONTRACT_MODE_BUILTINS.has(name); } export function modeToolDenial(name: string, mode: Mode): string { diff --git a/packages/agent-runtime/src/runtime.test.ts b/packages/agent-runtime/src/runtime.test.ts index 3758fa2288..affe059ca2 100644 --- a/packages/agent-runtime/src/runtime.test.ts +++ b/packages/agent-runtime/src/runtime.test.ts @@ -1,8 +1,7 @@ -import { mkdir, mkdtemp, rm, symlink, writeFile } from "node:fs/promises"; +import { mkdir, mkdtemp, readFile, rm, symlink, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { describe, expect, it, vi, beforeEach } from "vitest"; -import { Type } from "typebox"; import { type Agent, type AgentMessage, type AgentTool } from "@earendil-works/pi-agent-core"; import { createAssistantMessageEventStream, @@ -176,6 +175,7 @@ function createRuntime( customSystemPrompt: import("./custom-system-prompt.js").CustomSystemPrompt; projectInstructions: import("./project-instructions.js").ProjectInstructions; projectMemory: string; + trustedExtensions: ConstructorParameters[0]["trustedExtensions"]; pluginTools: PluginToolDef[]; subagents: SubagentDefinition[]; subagentProviders: Record; @@ -206,6 +206,7 @@ function createRuntime( workspaceRoot: overrides.workspaceRoot, scratchDir: overrides.scratchDir, customSystemPrompt: overrides.customSystemPrompt, + trustedExtensions: overrides.trustedExtensions, pluginTools: overrides.pluginTools, subagents: overrides.subagents, subagentProviders: overrides.subagentProviders, @@ -225,37 +226,49 @@ function runtimeTool(runtime: DesktopAgentRuntime, name: string): AgentTool { } describe("contract-mode tool catalog", () => { - /** - * A trusted-extension tool runs in this process, so no host permission call - * happens for it. If a contract mode declared it, the model could mutate the - * workspace with no card and no denial, which is the one hole the mode's own - * gate cannot close. Withholding it is the boundary. - */ - it("withholds a trusted-extension tool in contract modes but keeps it in Agent mode", () => { - const extensionTool = { - name: "fx_write", - label: "fx_write", - description: "extension tool", - parameters: Type.Object({}), - execute: async () => ({ content: [{ type: "text" as const, text: "ok" }], details: {} }), - }; - const withExtensionRunner = (runtime: DesktopAgentRuntime) => { - (runtime as unknown as { extensionRunner: unknown }).extensionRunner = { - getAgentTools: () => [extensionTool], - }; - return runtime as unknown as { isToolAllowedInMode: (name: string) => boolean }; - }; - - const plan = withExtensionRunner(createRuntime({ mode: "plan" })); - expect(plan.isToolAllowedInMode("fx_write")).toBe(false); - // A read-only builtin is still admitted, so the check is not blanket. - expect(plan.isToolAllowedInMode("Read")).toBe(true); - - const goal = withExtensionRunner(createRuntime({ mode: "goal" })); - expect(goal.isToolAllowedInMode("fx_write")).toBe(false); + it("withholds loaded extension tools and rejects retained references in contract modes", async () => { + const root = await mkdtemp(join(tmpdir(), "pi-contract-extension-")); + const entry = join(root, "extension.ts"); + const marker = join(root, "written.txt"); + await writeFile(entry, `import { writeFileSync } from "node:fs"; + export default function (pi) { pi.registerTool({ + name: "mcp_fx_write", label: "Write probe", description: "Extension probe", + parameters: { type: "object", properties: {} }, + execute: async () => { writeFileSync(${JSON.stringify(marker)}, "written"); + return { content: [{ type: "text", text: "ok" }], details: {} }; } + }); }`); + const runtime = createRuntime({ projectPath: root, host: { call: vi.fn().mockResolvedValue({}) }, + trustedExtensions: [{ id: entry, entry, label: "Probe", root, source: "plugin" }], + }); + const catalog = () => (runtime as unknown as { toolCatalog: Map }).toolCatalog; + try { + await runtime.loadTrustedExtensions(); + const retained = catalog().get("mcp_fx_write"); + expect(retained).toBeDefined(); + for (const mode of ["plan", "goal"] as const) { + runtime.setMode(mode); + expect(catalog().has("mcp_fx_write")).toBe(false); + expect(catalog().has("Read")).toBe(true); + await expect(retained!.execute("blocked", {})).rejects.toThrow(`not allowed in ${mode} mode`); + } + await expect(readFile(marker)).rejects.toMatchObject({ code: "ENOENT" }); + runtime.setMode("agent"); + await retained!.execute("allowed", {}); + expect(await readFile(marker, "utf8")).toBe("written"); + } finally { + await runtime.dispose(); + await rm(root, { recursive: true, force: true }); + } + }); - const agent = withExtensionRunner(createRuntime({ mode: "agent" })); - expect(agent.isToolAllowedInMode("fx_write")).toBe(true); + it.each(["plan", "goal"] as const)("rejects a retained Jev tool before input handling in %s mode", async (mode) => { + const runtime = createRuntime({ jevApiKey: "fixture" }); + try { + const retained = (runtime as unknown as { toolCatalog: Map }).toolCatalog.get("JevClassify")!; + runtime.setMode(mode); + // Invalid input prevents a regressed guard from contacting the paid service. + await expect(retained.execute("blocked", {})).rejects.toThrow(`not allowed in ${mode} mode`); + } finally { await runtime.dispose(); } }); }); diff --git a/packages/agent-runtime/src/runtime.ts b/packages/agent-runtime/src/runtime.ts index 3b27b2297a..853f3ad313 100644 --- a/packages/agent-runtime/src/runtime.ts +++ b/packages/agent-runtime/src/runtime.ts @@ -9,7 +9,7 @@ import { orderSystemRows, SystemTranscriptJournal } from "./system-transcript-jo import { planWorkspaceRequiredResult } from "./plan-workspace-error.js"; import { accountModelStream } from "./request-usage.js"; import { - contractModeWithholdsTool, + contractModeAllowsBuiltin, modeToolDenial, retainModeToolDeclaration, withModeExecutionGuard, @@ -4039,25 +4039,15 @@ Do not invent objections or turn speculative risks into blockers. Stop when the ); } - /** - * What the model may see in a contract mode. A withheld name leaves the - * catalog and is refused at execute time; every other name is declared, and - * the host permission path decides whether the call runs. Withholding is - * reserved for calls the mode can never permit, so `ToolSearch` stays and an - * unclassifiable MCP tool can still be discovered and then approved on its - * own card. - */ private isToolAllowedInMode(name: string): boolean { - if (!proposalKindForMode(this.mode)) return true; - if (contractModeWithholdsTool(name)) return false; - // A trusted-extension tool runs in this process, so the host permission - // path never sees the call and cannot gate it. A contract mode therefore - // cannot offer one: withholding it is the only boundary available. + const kind = proposalKindForMode(this.mode); + if (!kind) return true; + // Extension tools execute locally, outside the host permission path. if (this.isExtensionTool(name)) return false; - // A plugin tool is admitted only when the plugin runtime will also hold it to - // a declared action list. One without a list is refused by the host, so - // declaring it would only offer the model a call that cannot succeed. - return !name.startsWith("plugin_") || this.isPlanSafePluginTool(name); + return contractModeAllowsBuiltin(name) + || name === SUBMIT_TOOL_NAMES[kind] + || name.startsWith("mcp_") + || (name.startsWith("plugin_") && this.isPlanSafePluginTool(name)); } private isCoreTool(name: string): boolean { @@ -4077,11 +4067,7 @@ Do not invent objections or turn speculative risks into blockers. Stop when the } if (this.mode === "agent") return AGENT_CORE_TOOL_NAMES.has(name); if (!proposalKindForMode(this.mode)) return CHAT_CORE_TOOL_NAMES.has(name); - // A contract mode declares everything it has not withheld. An MCP tool is - // unclassifiable, so it must be visible for its own card to be reachable; - // deferring it behind ToolSearch would leave the approval path unreachable - // for the tools that need it most. - return !contractModeWithholdsTool(name); + return contractModeAllowsBuiltin(name) || this.isPlanSafePluginTool(name); } private activeTools(): AgentTool[] { diff --git a/scripts/e2e-plan.mjs b/scripts/e2e-plan.mjs index 7cfc8ce573..8fb14512d1 100644 --- a/scripts/e2e-plan.mjs +++ b/scripts/e2e-plan.mjs @@ -258,23 +258,21 @@ async function scenario105(binary, tempRoot) { assertToolFailure(result, code); } const marker = join(ctx.workspace, "plan-bash-marker.txt"); - const bash = await ctx.host.call( - "tools.execute", - bashParams( - session.id, - "e2e105-bash", - shell, - planBashCommand(shell.dialect, marker), - ), - ); + const params = bashParams(session.id, "e2e105-bash", shell, planBashCommand(shell.dialect, marker)); + const pending = ctx.host.call("tools.execute", params); + await waitFor(() => ctx.host.matchingNotifications("permissions.request").length === 1, 5_000, "Plan Bash approval"); + assert(!existsSync(marker), "Plan Bash ran before approval"); + const request = ctx.host.matchingNotifications("permissions.request")[0]; + await ctx.host.call("permissions.resolve", { requestId: request.params.requestId, decision: "allow-session" }); + const bash = await pending; assertToolSuccess(bash, "e2e105-bash"); assert(bash.commandShellId === shell.id, `Bash shell identity mismatch: ${shortJson(bash)}`); assert(String(bash.content?.stdout || "").includes("plan-auto"), shortJson(bash)); assert(existsSync(marker), "Plan Bash did not execute its marker command"); - assert( - ctx.host.matchingNotifications("permissions.request").length === 0, - "Plan policy scenario emitted a permission prompt", - ); + ctx.host.clearNotifications(); + const granted = await ctx.host.call("tools.execute", { ...params, toolCallId: "e2e105-bash-granted" }); + assertToolSuccess(granted, "e2e105-bash-granted"); + assert(ctx.host.matchingNotifications("permissions.request").length === 0, "Session grant did not suppress the second prompt"); const current = await ctx.host.call("session.get", { id: session.id }); assert(current.session?.mode === "plan", `forged mode changed session: ${shortJson(current)}`); return `mode=plan read=ok denied=4 shell=${shell.id}/${shell.dialect}`; From 020e648f74a95715d2e6289955017184feb4cf10 Mon Sep 17 00:00:00 2001 From: chengzhi-c Date: Fri, 16 Oct 2026 17:32:57 +0800 Subject: [PATCH 3/5] feat(host-core): decide contract-mode admission by effect, not by allowlist Plan and Goal classified builtins from a read-only allowlist, so a name nobody classified was refused and a read-only local tool was refused with it. Ask the question from the effect side instead: the contract modes deny the builtins whose purpose is to change files, session state, or spawned work, and every other name reaches the ordinary permission path, which still applies risk, the permission mode, session grants, and the external-path rule. Bash and user MCP tools keep taking a card in every mode. The list exists in Rust and TypeScript, so a test now fails when the two copies disagree. The spec, ADR 0330, the decision log, the RPC authorization gate, and the plan E2E scenario record the new posture. --- crates/host-core/src/permissions.rs | 127 +++++++++++++----- crates/host-core/src/rpc/mod.rs | 42 +++++- ...checkpoint-artifact-and-execution-epoch.md | 8 +- ...ated-external-paths-and-portable-search.md | 2 +- .../0064-codex-parity-context-compaction.md | 4 +- docs/adr/0211-plan-safe-plugin-actions.md | 7 +- ...0330-contract-modes-deny-what-is-unsafe.md | 55 ++++++-- ...tool-declarations-and-execution-denials.md | 3 +- docs/spec/00-baseline.md | 8 +- docs/spec/03-runtime/02-agent-runtime.md | 4 +- .../03-runtime/03-tools-and-permissions.md | 24 ++-- docs/spec/03-runtime/05-host-core-rust.md | 8 +- docs/spec/03-runtime/06-host-rpc-protocol.md | 27 ++-- docs/spec/03-runtime/08-error-codes.md | 2 +- .../03-runtime/10-session-state-machine.md | 7 +- docs/spec/04-ux/08-component-spec.md | 4 +- docs/spec/04-ux/09-interaction-patterns.md | 4 +- docs/spec/05-security/01-security.md | 15 ++- docs/spec/06-delivery/04-e2e-test-plan.md | 11 +- docs/spec/07-plugins/16-trusted-extensions.md | 2 +- docs/spec/08-meta/decisions-log.md | 8 +- docs/spec/README.md | 2 +- docs/zh-CN/spec/00-baseline.md | 9 +- .../zh-CN/spec/03-runtime/02-agent-runtime.md | 2 +- .../03-runtime/03-tools-and-permissions.md | 19 ++- .../spec/03-runtime/05-host-core-rust.md | 7 +- .../spec/03-runtime/06-host-rpc-protocol.md | 19 ++- docs/zh-CN/spec/03-runtime/08-error-codes.md | 2 +- .../03-runtime/10-session-state-machine.md | 12 +- docs/zh-CN/spec/04-ux/08-component-spec.md | 3 +- .../spec/04-ux/09-interaction-patterns.md | 4 +- docs/zh-CN/spec/05-security/01-security.md | 8 +- .../spec/06-delivery/04-e2e-test-plan.md | 11 +- .../spec/07-plugins/16-trusted-extensions.md | 2 +- docs/zh-CN/spec/08-meta/decisions-log.md | 10 +- docs/zh-CN/spec/README.md | 2 +- .../src/mode-tool-access.test.ts | 51 ++++++- .../agent-runtime/src/mode-tool-access.ts | 30 ++++- packages/agent-runtime/src/runtime.ts | 19 ++- scripts/e2e-plan.mjs | 15 ++- 40 files changed, 404 insertions(+), 195 deletions(-) diff --git a/crates/host-core/src/permissions.rs b/crates/host-core/src/permissions.rs index 12cbaa1c46..3a8f333ed5 100644 --- a/crates/host-core/src/permissions.rs +++ b/crates/host-core/src/permissions.rs @@ -148,32 +148,60 @@ impl PermissionManager { } } - // Unclassified builtins stay denied; remote tools are gated separately. - const CONTRACT_MODE_READ_ONLY: [&str; 7] = [ - "Read", - "Glob", - "Grep", - "BrowserPreview", - "asktool", - "new_context", - "ToolSearch", + /// Builtins a contract mode refuses: the ones that write files or session + /// state (`Write`, `Edit`, `TodoWrite`, `GenerateImages`, `PluginScaffold`, + /// `PluginPack`, the `ScheduledTask*` tools), the delegation family that + /// spawns and controls work (`Task*`), and `JevClassify`, whose Agent-only + /// paid call must not run from a reference retained across a mode switch. + /// Every other name reaches the ordinary permission path, so the gate + /// answers by effect instead of by provenance (ADR 0330). `PluginCheck` and + /// `Skill` are absent because they only read, and the runtime never creates + /// either outside Agent mode. `packages/agent-runtime/src/mode-tool-access.ts` + /// mirrors this list, and a test asserts the two stay identical. + const CONTRACT_MODE_STATE_CHANGING: [&str; 14] = [ + "Write", + "Edit", + "TodoWrite", + "GenerateImages", + "PluginScaffold", + "PluginPack", + "ScheduledTaskCreate", + "ScheduledTaskUpdate", + "ScheduledTaskDelete", + "Task", + "TaskWait", + "TaskList", + "TaskStop", + "JevClassify", ]; + + /// The contract modes' own gate, asked before risk, permission mode, + /// grants, and scratch paths. + /// + /// `Denied` is a hard deny that no permission mode or session grant can + /// re-enable. `Approval` routes the call to a permission card. `Allowed` + /// falls through to the ordinary permission path, which still applies + /// risk, the effective permission mode, grants, and the external-path rule. pub fn contract_admission( tool_name: &str, plan_safe_actions: Option<&[String]>, ) -> ContractAdmission { - if Self::CONTRACT_MODE_READ_ONLY.contains(&tool_name) { - return ContractAdmission::Allowed; + if Self::CONTRACT_MODE_STATE_CHANGING.contains(&tool_name) { + return ContractAdmission::Denied; } if tool_name == "Bash" || tool_name.starts_with("mcp_") { return ContractAdmission::Approval; } - if tool_name.starts_with("plugin_") - && plan_safe_actions.is_some_and(|actions| !actions.is_empty()) - { - return ContractAdmission::Allowed; + if tool_name.starts_with("plugin_") { + // The declaration is the escape hatch (ADR 0211): without one the + // plugin runtime refuses the call too, so a card would be theater. + return if plan_safe_actions.is_some_and(|actions| !actions.is_empty()) { + ContractAdmission::Allowed + } else { + ContractAdmission::Denied + }; } - ContractAdmission::Denied + ContractAdmission::Allowed } /// Auto-decision with an effective permission mode (D115). @@ -182,7 +210,7 @@ impl PermissionManager { /// caller collapses `inherit` against the global default before calling. /// A contract mode's hard deny stays above every permission mode: `auto` /// cannot re-enable `Write`/`Edit` in Plan or Goal, and cannot skip the card - /// an unclassified call needs. + /// `Bash` or a user MCP tool needs. #[cfg(test)] pub fn evaluate_auto_with_permission_mode( &self, @@ -239,8 +267,8 @@ impl PermissionManager { if crate::sessions::is_contract_mode(mode) { match Self::contract_admission(tool_name, plan_safe_actions) { ContractAdmission::Denied => return Some(PermissionDecision::Deny), - // No permission mode short-circuits an unclassified call: an - // opaque tool is exactly the case where `auto` must not decide. + // No permission mode short-circuits an opaque tool: `Bash` and + // an MCP tool are exactly the case where `auto` must not decide. ContractAdmission::Approval => { return has_session_grant.then_some(PermissionDecision::AllowSession); } @@ -529,39 +557,64 @@ mod tests { /// A contract mode denies what changes state, whatever the permission mode /// would otherwise allow. `auto` and `accept-edits` cannot re-enable it, - /// and neither can a session grant. + /// and neither can a session grant. The test drives the list itself, so it + /// pins the wiring for every name on it; a state-changing builtin missing + /// from the list is invisible here, which is the omission ADR 0330 accepts. #[test] fn contract_modes_deny_state_changing_builtins_in_every_permission_mode() { let pm = PermissionManager::default(); - let mut grants = HashMap::new(); - grants.insert( + let grants = HashMap::from([( "s".to_string(), - vec![ - "Write".to_string(), - "Edit".to_string(), - "TodoWrite".to_string(), - "GenerateImages".to_string(), - ], - ); + PermissionManager::CONTRACT_MODE_STATE_CHANGING + .iter() + .map(|name| (*name).to_string()) + .collect::>(), + )]); for contract in ["plan", "goal"] { for mode in ["ask", "accept-edits", "auto"] { - for tool in [ - "Write", - "Edit", - "TodoWrite", - "GenerateImages", - "an_unclassified_builtin", - ] { + for tool in PermissionManager::CONTRACT_MODE_STATE_CHANGING { assert_eq!( pm.evaluate_auto_with_permission_mode("s", tool, contract, mode, &grants), Some(PermissionDecision::Deny), - "{contract} + {mode} must deny {tool}" + "{contract} + {mode} denies {tool} even with a session grant" ); } } } } + /// The posture is deny-by-effect (ADR 0330): a builtin nobody classified is + /// not refused here, it follows the ordinary permission path. `auto` admits + /// it, every other permission mode asks. + #[test] + fn contract_modes_route_an_unclassified_builtin_to_the_normal_path() { + let pm = PermissionManager::default(); + for contract in ["plan", "goal"] { + assert_eq!( + pm.evaluate_auto_with_permission_mode( + "s", + "an_unclassified_builtin", + contract, + "auto", + &no_grants() + ), + Some(PermissionDecision::AllowOnce), + "{contract} + auto follows the ordinary path" + ); + assert_eq!( + pm.evaluate_auto_with_permission_mode( + "s", + "an_unclassified_builtin", + contract, + "ask", + &no_grants() + ), + None, + "{contract} + ask takes a card" + ); + } + } + #[test] fn contract_modes_require_approval_unless_the_tool_has_a_session_grant() { let pm = PermissionManager::default(); diff --git a/crates/host-core/src/rpc/mod.rs b/crates/host-core/src/rpc/mod.rs index 38ebf48fb3..1837553167 100644 --- a/crates/host-core/src/rpc/mod.rs +++ b/crates/host-core/src/rpc/mod.rs @@ -9843,7 +9843,7 @@ mod tests { "EDIT_DISABLED_IN_PLAN", ), ("plugin_demo_run", json!({}), "PLUGIN_DISABLED_IN_PLAN"), - ("UnknownTool", json!({}), "TOOL_DISABLED_IN_PLAN"), + ("TodoWrite", json!({}), "TOOL_DISABLED_IN_PLAN"), ] { let result = handle_request( state.clone(), @@ -9863,6 +9863,25 @@ mod tests { assert_eq!(result["ok"], false); } + // Deny by effect (ADR 0330): a builtin nobody classified is not a + // contract-mode denial. It reaches the ordinary path, where the tool + // registry rejects the unregistered name. + let unclassified = handle_request( + state.clone(), + "tools.execute", + json!({ + "sessionId": session_id, + "toolCallId": "unclassified-call", + "toolName": "UnknownTool", + "args": {}, + "mode": "agent" + }), + tx.clone(), + ) + .await + .unwrap(); + assert_eq!(unclassified["errorCode"], "TOOL_NOT_FOUND"); + // Bash is gated, not parsed: a contract mode takes a card even under // `auto`, because what a command does is decided by the shell that runs // it and reading the string would be wrong in the permissive direction. @@ -9983,7 +10002,7 @@ mod tests { "EDIT_DISABLED_IN_PLAN", ), ("plugin_demo_run", json!({}), "PLUGIN_DISABLED_IN_PLAN"), - ("UnknownTool", json!({}), "TOOL_DISABLED_IN_PLAN"), + ("TodoWrite", json!({}), "TOOL_DISABLED_IN_PLAN"), ] { let result = handle_request( state.clone(), @@ -10004,6 +10023,25 @@ mod tests { assert_eq!(result["ok"], false); } + // Deny by effect (ADR 0330): a builtin nobody classified is not a + // contract-mode denial. It reaches the ordinary path, where the tool + // registry rejects the unregistered name. + let unclassified = handle_request( + state.clone(), + "tools.execute", + json!({ + "sessionId": session_id, + "toolCallId": "goal-unclassified-call", + "toolName": "UnknownTool", + "args": {}, + "mode": "agent" + }), + tx.clone(), + ) + .await + .unwrap(); + assert_eq!(unclassified["errorCode"], "TOOL_NOT_FOUND"); + // Bash takes a card here too, so the strongest permission mode cannot // stand in for a decision about what the command will do. #[cfg(windows)] diff --git a/docs/adr/0053-plan-checkpoint-artifact-and-execution-epoch.md b/docs/adr/0053-plan-checkpoint-artifact-and-execution-epoch.md index ff7013a6a5..9e82191ee7 100644 --- a/docs/adr/0053-plan-checkpoint-artifact-and-execution-epoch.md +++ b/docs/adr/0053-plan-checkpoint-artifact-and-execution-epoch.md @@ -28,10 +28,10 @@ tool, which ADR 0064 restores as `new_context`); Write, Edit, plugin tools, and unknown tools remain host-denied. `SubmitPlan` is the only assistant tool call in its batch and is valid only for the active Plan turn. -*(Amended by ADR 0330: "unknown tools" now means any builtin outside the -read-only set, and the set is an allowlist so a builtin added later is denied -until it is classified. `Bash` takes a permission card in every permission mode -rather than following the selected one. An MCP tool is no longer denied by +*(Amended by ADR 0330: "unknown tools" now means a builtin whose purpose is to +change state; every other builtin, including one nobody classified, follows the +ordinary permission path. `Bash` takes a permission card in every permission +mode rather than following the selected one. An MCP tool is no longer denied by provenance — it takes a card. A plugin tool without `planSafeActions` stays denied.)* diff --git a/docs/adr/0057-permission-gated-external-paths-and-portable-search.md b/docs/adr/0057-permission-gated-external-paths-and-portable-search.md index 50dd1e616c..2bfb76d2f5 100644 --- a/docs/adr/0057-permission-gated-external-paths-and-portable-search.md +++ b/docs/adr/0057-permission-gated-external-paths-and-portable-search.md @@ -42,7 +42,7 @@ new workspace root, and implicit Bash cwd or recursive walks do not gain this exception. Successful external results report `root: "external"` where a root field exists and keep absolute canonical paths visible to the model. -Plan's existing hard deny for Write/Edit/plugin/unknown tools remains above this +Plan's existing hard deny for Write/Edit/plugin/state-changing builtins remains above this path rule. ### 2. Native search is the portable default diff --git a/docs/adr/0064-codex-parity-context-compaction.md b/docs/adr/0064-codex-parity-context-compaction.md index b09d76ab72..a9ced1e7ab 100644 --- a/docs/adr/0064-codex-parity-context-compaction.md +++ b/docs/adr/0064-codex-parity-context-compaction.md @@ -98,9 +98,11 @@ retained-tail recovery, ADR 0061's model-window-derived budgets, and ADR 0061's `floor(hardLimit * 0.9)`, matching the `should_roll_over` turn boundary. `hardLimit` remains the final request guard (D623). The name is synchronized across `activeTools()`, - `isCoreTool()`, the contract-mode allowlists, and the host-core + `isCoreTool()`, the contract-mode tool policy, and the host-core no-confirmation allowlist (`crates/host-core/src/permissions.rs`), where it replaces `"CompactContext"`. + *(Amended by ADR 0330: a contract mode keeps a deny list instead of an + allowlist, and `new_context` is admitted by falling through it.)* 5. **Two-tier reminders, once each per window.** With `remaining = hardLimit - tokens`: tier one at `remaining <= clamp(hardLimit * 0.15, 8k, 32k)` states the remaining budget diff --git a/docs/adr/0211-plan-safe-plugin-actions.md b/docs/adr/0211-plan-safe-plugin-actions.md index 8ff2433cdd..02d2a3731f 100644 --- a/docs/adr/0211-plan-safe-plugin-actions.md +++ b/docs/adr/0211-plan-safe-plugin-actions.md @@ -70,7 +70,8 @@ in the `plugins.execute` notification alongside the session mode. ### 4. Host-core admits plan-safe plugin tools in contract modes `crates/host-core/src/permissions.rs` retains the existing contract-mode -hard deny (`contract_admission`, ADR 0330) for `Write`, `Edit`, unknown builtins, and +hard deny (`contract_admission`, ADR 0330) for `Write`, `Edit`, the state-changing +builtins, and plugin tools that arrive without `planSafeActions`. A plugin tool that arrives with a non-empty `planSafeActions` list now passes the contract gate. The list does not change any other rule: low-risk auto-allow, the @@ -125,8 +126,8 @@ process-wide and macOS spends `Cmd+W` on its own close-window command.)* - Plugin authors remain responsible for declaring exactly which actions are read-only. A wrong declaration fails at plugin registration, not at the user's prompt. -- The contract modes' hard deny for `Write`, `Edit`, and unknown tools - is unchanged. A plugin that does not declare `planSafeActions` keeps +- The contract modes' hard deny for `Write`, `Edit`, and the state-changing + builtins is unchanged. A plugin that does not declare `planSafeActions` keeps the ADR 0052 / ADR 0053 behavior. - The defense-in-depth check in the plugin-runtime means a bug in the runtime or host-core cannot escalate a Plan call into a mutation: the diff --git a/docs/adr/0330-contract-modes-deny-what-is-unsafe.md b/docs/adr/0330-contract-modes-deny-what-is-unsafe.md index 2588564572..e276dbdf9e 100644 --- a/docs/adr/0330-contract-modes-deny-what-is-unsafe.md +++ b/docs/adr/0330-contract-modes-deny-what-is-unsafe.md @@ -4,22 +4,36 @@ - Date: 2026-10-15 - Deciders: PI-Desktop core - Amends: ADR 0052 §3, ADR 0053 §1, ADR 0211 §4 -- Related: ADR 0087, ADR 0089, `mcp-tool-approval-risk` +- Related: ADR 0064, ADR 0089, `mcp-tool-approval-risk` ## Context Plan and Goal must keep direct mutation tools unavailable without making -read-only MCP capabilities inaccessible. Shell commands and opaque MCP tools -cannot be classified reliably by their names or self-reported risk. Mode prompts +read-only capabilities inaccessible. Shell commands and opaque MCP tools cannot +be classified reliably by their names or self-reported risk. Mode prompts provide guidance; permission checks decide whether a call may execute. +An earlier revision of this policy classified builtins by an explicit read-only +allowlist: a builtin absent from the list was denied until it was classified. +That asks the question from the wrong side. It refuses read-only local tools +that change nothing, it makes every newly added builtin an immediate contract +mode regression, and it obscures the property that actually matters, which is +the effect of a call rather than the provenance of its name. + ## Decision -- Read-only builtins use the ordinary permission path: `Read`, `Glob`, `Grep`, - `BrowserPreview`, `asktool`, `new_context`, and `ToolSearch`. The runtime also - permits the submit tool belonging to the current mode. -- Other builtins are denied before risk, permission mode, session grants, or - scratch-path exceptions. New builtins remain denied until classified. +- A contract mode denies the builtins that act on state instead of inspecting + it: the ones that write files or session state (`Write`, `Edit`, `TodoWrite`, + `GenerateImages`, `PluginScaffold`, `PluginPack`, the `ScheduledTask*` tools), + the delegation family that spawns and controls work (`Task*`), and + `JevClassify`, whose paid provider call must not run from a reference retained + across a mode switch. +- Every other builtin is admitted by the gate and falls through to the ordinary + permission path, which still applies risk, the effective permission mode, + session grants, and the external-path rule. `Read`, `Glob`, `Grep`, + `BrowserPreview`, and a builtin nobody classified take that path. `asktool`, + `new_context`, `ToolSearch`, and the submit tool belonging to the current mode + are runtime-local and never reach the host gate. - `Bash` and user MCP tools require explicit approval in every permission mode, including `auto`. An exact-tool `allow-session` grant suppresses subsequent prompts only in that session. @@ -29,13 +43,16 @@ provide guidance; permission checks decide whether a call may execute. - Plugin-contributed MCP tools have no supported `planSafeActions` declaration. Their execution callback rejects Plan and Goal independently of caller-supplied admission metadata. -- Trusted-extension tools and Agent-only local tools remain unavailable in - contract modes, including through references retained before a mode change. - These tools can execute outside the host permission path. +- Trusted-extension tools remain unavailable in contract modes, including + through references retained before a mode change. They execute outside the + host permission path, so the host gate never sees them. - MCP tools remain discoverable through existing ToolSearch and explicit selections. Admission does not make every MCP schema a core declaration. - Refusing an approval card returns `TOOL_DENIED`; a contract-mode hard denial retains its existing `_IN_PLAN` error code in both Plan and Goal. +- The deny list is the contract. `crates/host-core/src/permissions.rs` and + `packages/agent-runtime/src/mode-tool-access.ts` mirror it in two languages, + and `mode-tool-access.test.ts` fails when the two copies drift apart. ## Consequences @@ -43,6 +60,16 @@ Plan and Goal can use MCP capabilities after user approval without a separate server allowlist. `auto` no longer automatically permits their Bash or MCP calls. Agent-mode permission behavior is unchanged. +A builtin added later is admitted by omission. That is the accepted cost of +answering by effect: the alternative refused read-only tools and turned every +new builtin into a contract-mode incident. The exposure is bounded but not +closed: a contract mode never builds the eight denied tools it does not declare, +it refuses the six declarations it retains (`Write`, `Edit`, and the `Task*` +family) before any host action, and the drift test keeps the two languages in +step. None of those catches a state-changing builtin nobody added to the list, +so it stays reachable in Plan until someone classifies it. Deciding what a new +builtin does to state is part of adding it. + Approval is not filesystem isolation: an approved shell command or opaque MCP call may mutate state. This policy controls authorization, not the effects of arbitrary code. Plugin authors remain responsible for the semantics of actions @@ -50,10 +77,12 @@ they declare plan-safe. ## Alternatives +- The read-only allowlist this ADR replaces: it refused read-only local tools, + required a classification decision for every new builtin, and still could not + classify `Bash` or an opaque MCP tool. - Parsing shell commands cannot reliably establish whether they mutate files. - Trusting MCP risk annotations lets remote code classify its own access. -- A builtin denylist admits newly added tools by omission; explicit classification - keeps unknown local execution paths closed. +- Gating on a tool's declared risk: the same remote code supplies it. ## References diff --git a/docs/adr/plan-tool-declarations-and-execution-denials.md b/docs/adr/plan-tool-declarations-and-execution-denials.md index da351fa5ba..e5b9f8071c 100644 --- a/docs/adr/plan-tool-declarations-and-execution-denials.md +++ b/docs/adr/plan-tool-declarations-and-execution-denials.md @@ -21,7 +21,8 @@ schemas too. These tools remain core declarations and are explicitly labelled unavailable in the contract mode. Unknown tools, missing subagent definitions, on-demand tools and non-plan-safe plugin tools do not gain new declarations. -Keep the current mode allowlist authoritative for execution. Block prohibited +Keep the current mode classification authoritative for execution (a contract +mode's deny list per ADR 0330). Block prohibited calls before extension call hooks or tool handlers run. Additionally wrap the registered handler with a fresh mode check, so an object retained before a mode switch cannot execute using stale authority. pi's existing error path diff --git a/docs/spec/00-baseline.md b/docs/spec/00-baseline.md index 475fca7b55..eaa66fe56e 100644 --- a/docs/spec/00-baseline.md +++ b/docs/spec/00-baseline.md @@ -196,9 +196,11 @@ No user-facing settings** 44. Plan tools and policy (ADR 0330 amendment): **Read / Glob / Grep / BrowserPreview / asktool / Bash / SubmitPlan, discoverable user MCP tools, - and plugins with `planSafeActions`. Unclassified builtins remain denied. - Bash and user MCP tools require approval or an exact-tool session grant, - including under `auto`; this is authorization, not filesystem isolation.** + and plugins with `planSafeActions`. Builtins that change files, session + state, or spawned work are hard-denied; every other builtin follows the + ordinary permission path. Bash and user MCP tools require approval or an + exact-tool session grant, including under `auto`; this is authorization, + not filesystem isolation.** 45. Plan checkpoint: **`SubmitPlan(title, markdown, question)` causes host-core to preserve the exact Markdown bytes in a new unique `/.pi/plan/*.md` artifact, while title/question remain diff --git a/docs/spec/03-runtime/02-agent-runtime.md b/docs/spec/03-runtime/02-agent-runtime.md index 3ac7499815..42d7d78988 100644 --- a/docs/spec/03-runtime/02-agent-runtime.md +++ b/docs/spec/03-runtime/02-agent-runtime.md @@ -671,7 +671,7 @@ with the kind to know which durable mode a session is in. `Agent / inactive` enters `Plan / planning` either when the user selects Plan while idle or when the Agent calls `EnterPlanMode`. In Plan, the Agent can -inspect, use context controls, run Bash through the selected permission mode, +inspect, use context controls, run Bash through its permission card, and call `SubmitPlan(title, markdown, question)`. Host-core preserves the submitted Markdown bytes in a new immutable `.pi/plan/.md` artifact, records its relative path/hash/size and @@ -680,7 +680,7 @@ structured title/question in `plan_approvals`, and moves the live state to Plan/Goal requests retain Write/Edit declarations and, when subagents are configured, the Task/TaskWait/TaskList/TaskStop declarations. They are marked -unavailable in the active mode. The execution allowlist remains unchanged: +unavailable in the active mode. The contract-mode gate is enforced the same way: prohibited calls are blocked before extension call hooks and handlers, and a handler retained across a mode change rechecks that mode before executing. The model receives an ordinary error tool result with the original call id; diff --git a/docs/spec/03-runtime/03-tools-and-permissions.md b/docs/spec/03-runtime/03-tools-and-permissions.md index 2114917958..fb571257eb 100644 --- a/docs/spec/03-runtime/03-tools-and-permissions.md +++ b/docs/spec/03-runtime/03-tools-and-permissions.md @@ -11,7 +11,7 @@ | Agent tools | Read / Glob / Grep / Write / Edit / Bash + registered plugin tools + optional Jev classifier | | Plan tools | Read / Glob / Grep / BrowserPreview / asktool / Bash / SubmitPlan, plus discoverable MCP tools and plugins with `planSafeActions`; ordinary permissions still apply | | Goal tools | Read / Glob / Grep / BrowserPreview / asktool / Bash / SubmitGoal, plus discoverable MCP tools and plugins with `planSafeActions`; ordinary permissions still apply | -| Plan and Goal hard deny | Write / Edit / TodoWrite / GenerateImages / the delegation tools / the other kind's submit tool. A builtin that is not on the read-only list is denied until it is classified | +| Plan and Goal hard deny | Write / Edit / TodoWrite / GenerateImages / PluginScaffold / PluginPack / the ScheduledTask tools / the delegation tools / the other kind's submit tool. Every other builtin follows the ordinary permission path | | Plan and Goal Bash | Requires approval, including in `auto`, unless the session has an exact-tool grant | | Plan and Goal unclassified tools | MCP tools require approval or an exact-tool session grant; `plugin_` tools require non-empty `planSafeActions` | | Plugin `planSafeActions` | Non-empty array of `action` strings; runtime hides tools without it, host applies ordinary permissions, plugin-runtime rejects unlisted actions (ADR 0211) | @@ -519,9 +519,8 @@ session (grants are in-memory only). `auto` auto-allows it in Agent mode. In Plan and Goal an MCP tool is unclassifiable — the name says nothing about whether it mutates, and the server's own annotation is not evidence — so it takes -a card in every permission mode, `auto` included. That is the one place a -contract mode overrides the selected permission mode for a non-Bash tool. The -tool stays in the model's catalog so the card is reachable (D640, ADR +a card in every permission mode, `auto` included, exactly as Bash does. The tool +stays in the model's catalog so the card is reachable (D640, ADR `mcp-tool-approval-risk`). ### Decision Types @@ -561,8 +560,8 @@ Rules: - The session value is stored in `sessions.permission_mode` (`inherit | ask | accept-edits | auto`, default `inherit`, schema v5) and set via `session.configure` `permissionMode`. -- A contract mode's hard deny wins over every permission mode for `Write`, - `Edit`, `TodoWrite`, `GenerateImages`, and the delegation tools. `auto` cannot +- A contract mode's hard deny wins over every permission mode for the + state-changing builtins listed in the hard-deny row above. `auto` cannot re-enable a hidden or denied tool. - Low-risk tools (`Read`/`Glob`/`Grep`) inside the session roots auto-allow in every mode, as before. @@ -641,12 +640,13 @@ for the current key-log policy. ### Notes - A contract mode denies by what a call would do, not by where its name sits in a - list. The read-only builtins fall through to the ordinary permission path; a - builtin that is not on that list — `Write`, `Edit`, `TodoWrite`, - `GenerateImages`, the delegation tools — is hard-denied before the permission - UI, and a direct host call cannot bypass the matrix. An unclassified builtin is - denied rather than allowed, so a tool added later cannot widen the mode by - omission. + list. Builtins whose purpose is to change state — `Write`, `Edit`, + `TodoWrite`, `GenerateImages`, `PluginScaffold`, `PluginPack`, the + `ScheduledTask*` tools, the `Task*` delegation tools — are hard-denied before + the permission UI, and a direct host call cannot bypass the matrix. Every other + builtin, including one nobody classified, falls through to the ordinary + permission path, which still applies risk, the effective permission mode, + grants, and the external-path rule. - Plan and Goal Bash always takes a permission card, in every permission mode. What a command does is decided by the shell that runs it, so the host does not read the command string to decide; `auto` therefore cannot stand in for the diff --git a/docs/spec/03-runtime/05-host-core-rust.md b/docs/spec/03-runtime/05-host-core-rust.md index c86dbad26f..3cbc08ffb7 100644 --- a/docs/spec/03-runtime/05-host-core-rust.md +++ b/docs/spec/03-runtime/05-host-core-rust.md @@ -112,8 +112,8 @@ notification.list explicit outside path is resolved only after host permission evaluation 2. Host resolves the durable session mode; request-supplied mode is never authoritative -3. Plan and Goal deny Write/Edit/plugin/unknown tools before permission evaluation -4. Plan and Goal Bash follow the durable permission mode and may mutate under Auto +3. Plan and Goal hard-deny state-changing builtins before permission evaluation +4. Plan and Goal Bash always takes a permission card, Auto included 5. Plan and Goal artifact bytes, path, hash, size, and approval/execution identity are host-authenticated 6. Plan/Goal approval is host-authenticated, durable, and atomic before Agent entry @@ -144,8 +144,8 @@ notification.list through the `session.endTurn` transaction; results already visible in the focused current chat and aborted turns create none 6. a durable Plan or Goal session cannot authorize Write/Edit/plugin tools through - a conflicting request mode, and Plan/Goal Bash follows the resolved permission - mode + a conflicting request mode, and Plan/Goal Bash takes a permission card in every + permission mode 7. SubmitPlan writes exact Markdown bytes to a new `.pi/plan/*.md` artifact and stores durable path/hash/size plus structured title/question in `plan_approvals`; approval is approve/reject-only, session/turn/version diff --git a/docs/spec/03-runtime/06-host-rpc-protocol.md b/docs/spec/03-runtime/06-host-rpc-protocol.md index e03cfbd781..501eab3d0a 100644 --- a/docs/spec/03-runtime/06-host-rpc-protocol.md +++ b/docs/spec/03-runtime/06-host-rpc-protocol.md @@ -945,17 +945,18 @@ not inherit this exception. Before generic permission evaluation, host-core applies the mode policy: -- Plan and Goal allow `Read`, `Glob`, `Grep`, `BrowserPreview`, `Bash`, and the - kind's submit tool (`SubmitPlan` / `SubmitGoal`) as applicable to the live - planning state. -- Plan and Goal deny `Write`, `Edit`, every plugin tool, and unknown tools under - all permission modes and grants. The host reads the session's **durable** mode - for this check, so a sidecar claiming `agent` in `tools.execute` cannot widen - it, and the `*_IN_PLAN` error codes are shared by both kinds. -- Plan and Goal `Bash` follows the resolved permission mode: `ask` and - `accept-edits` - emit `permissions.request`; `auto` executes without confirmation and may - mutate. The host re-resolves the effective shell ID/dialect and requires the +- Plan and Goal admit a builtin unless the next bullet denies it. `Read`, + `Glob`, `Grep`, and `BrowserPreview` therefore reach the ordinary permission + path, and the kind's submit tool (`SubmitPlan` / `SubmitGoal`) is allowed as + applicable to the live planning state. +- Plan and Goal deny `Write`, `Edit`, the other state-changing builtins, and a + plugin tool without `planSafeActions` under all permission modes and grants. + The host reads the session's **durable** mode for this check, so a sidecar + claiming `agent` in `tools.execute` cannot widen it, and the `*_IN_PLAN` error + codes are shared by both kinds. +- Plan and Goal `Bash` takes a permission card under every resolved permission + mode: `ask`, `accept-edits`, and `auto` all emit `permissions.request`, and an + approved command may mutate. The host re-resolves the effective shell ID/dialect and requires the exact `expectedCommandShellId` and `expectedCommandShellDialect` before permission evaluation and again before spawn; it streams stdout/stderr separately. A configured shell may fall back to the first available platform @@ -1328,8 +1329,8 @@ Tool outcomes (`TOOL_DENIED`, `TOOL_TIMEOUT`, `PATH_OUTSIDE_WORKSPACE`, 9. Forking through a message excludes every later source row and rejects an unknown message without creating a child 10. A forged `requestedMode` cannot authorize a tool against the durable mode; - Plan and Goal deny Write/Edit/plugin/unknown tools and apply permission - prompts to Bash according to `ask`/`accept-edits`/`auto` + Plan and Goal deny state-changing builtins and plugin tools without + `planSafeActions`, and ask for Bash in every permission mode 11. SubmitPlan and SubmitGoal write exact Markdown bytes to a unique `.pi/plan/*.md` or `.pi/goal/*.md` file with hash/size and structured title/question fields; only matching diff --git a/docs/spec/03-runtime/08-error-codes.md b/docs/spec/03-runtime/08-error-codes.md index d05e92d9fe..f0a77f0a2a 100644 --- a/docs/spec/03-runtime/08-error-codes.md +++ b/docs/spec/03-runtime/08-error-codes.md @@ -140,7 +140,7 @@ does not turn temporary thread pressure into a host process exit. | `WRITE_DISABLED_IN_PLAN` | no | contract-mode hard-deny for Write | | `EDIT_DISABLED_IN_PLAN` | no | contract-mode hard-deny for Edit | | `PLUGIN_DISABLED_IN_PLAN` | no | contract-mode refusal of a plugin tool that declares no `planSafeActions` | -| `TOOL_DISABLED_IN_PLAN` | no | contract-mode hard-deny for a builtin outside the read-only set | +| `TOOL_DISABLED_IN_PLAN` | no | contract-mode hard-deny for a builtin that changes state | | `PLAN_NOT_ACTIVE` | no | a submit tool ran while no contract was being negotiated | | `PLAN_KIND_MISMATCH` | no | `SubmitPlan` in Goal mode, or `SubmitGoal` in Plan mode | | `PLAN_APPROVAL_REQUIRED` | no | SubmitPlan/SubmitGoal is waiting for a separate approval | diff --git a/docs/spec/03-runtime/10-session-state-machine.md b/docs/spec/03-runtime/10-session-state-machine.md index 2bd5f3e9c9..6a679701f5 100644 --- a/docs/spec/03-runtime/10-session-state-machine.md +++ b/docs/spec/03-runtime/10-session-state-machine.md @@ -234,10 +234,9 @@ transcript-file line first, index transaction second. `EnterPlanMode`/`EnterGoalMode` converge on the same planning state, and approval resumes that Agent in Agent mode -10. Contract-mode policy permits Bash only through the selected permission mode - and - denies Write/Edit/plugins regardless of `auto` or session grants, in Goal - exactly as in Plan +10. Contract-mode policy asks for Bash in every permission mode and denies the + state-changing builtins and plugins regardless of `auto` or session grants, + in Goal exactly as in Plan 11. SubmitPlan/SubmitGoal writes an exact unique `.pi//*.md` artifact with hash/size, keeps title/question structured, and only approve/reject can resolve its diff --git a/docs/spec/04-ux/08-component-spec.md b/docs/spec/04-ux/08-component-spec.md index 3c6921c27f..8697f07c7f 100644 --- a/docs/spec/04-ux/08-component-spec.md +++ b/docs/spec/04-ux/08-component-spec.md @@ -2808,8 +2808,8 @@ present the interrupted terminal snapshot after restart. - The card is a session-scoped `region` with a localized plan title. - Approval, reject, and abort controls have explicit labels and keyboard focus. -- The selected permission mode exposes radio semantics and its Plan/Goal Bash - consequence is available in the accessible description. +- The selected permission mode exposes radio semantics and an accessible + description stating that Plan and Goal ask before Bash whatever the mode. - Resolution does not navigate to another session or take focus from a different session. diff --git a/docs/spec/04-ux/09-interaction-patterns.md b/docs/spec/04-ux/09-interaction-patterns.md index b9926b3278..25f97a1502 100644 --- a/docs/spec/04-ux/09-interaction-patterns.md +++ b/docs/spec/04-ux/09-interaction-patterns.md @@ -1003,8 +1003,8 @@ Agent calls a permission-gated tool (including Plan/Goal Bash under Ask or Accep calls `EnterPlanMode` / `EnterGoalMode`; the host persists/validates the matching contract mode and the renderer projects `planning`. 2. The Agent investigates with the selected contract tool set. Read/Glob/Grep and - BrowserPreview are allowed; Bash follows the visible permission mode. A - contract-mode Bash command may mutate under Auto, so the mode chip remains visible. + BrowserPreview are allowed; Bash takes a permission card whatever the visible + permission mode. An approved contract-mode Bash command may mutate, so the mode chip remains visible. While that turn is live `planning`, the Composer mode chip pulses and a compact Planning row occupies the same reserved tail slot as Working until completion or pending user interaction. A known runtime phase takes precedence; tool diff --git a/docs/spec/05-security/01-security.md b/docs/spec/05-security/01-security.md index 9b404585c8..1a1d1d67a6 100644 --- a/docs/spec/05-security/01-security.md +++ b/docs/spec/05-security/01-security.md @@ -96,11 +96,12 @@ sidecar's workspace file resolver remain restricted to the session workspace. Plan is not itself the workspace security boundary. Host-core resolves the durable session mode for every `tools.execute` call and applies the Plan matrix before permission modes, grants, plugin risk, or renderer/sidecar state. Plan -denies Write/Edit/plugin/unknown tools, while BrowserPreview is the explicit -read-only UI inspection exception (it reveals bundled `pi.browser` chrome; raw -CDP plugin tools stay denied in Plan). Bash remains available in Plan: Ask and -Accept edits prompt, and Auto runs without confirmation and may mutate the -workspace or scratch directory. The UI must state this tradeoff. `SubmitPlan` +denies state-changing builtins and plugin tools without `planSafeActions`, while +BrowserPreview is the explicit read-only UI inspection exception (it reveals +bundled `pi.browser` chrome; raw CDP plugin tools stay denied in Plan). Bash +remains available in Plan: every permission mode, Auto included, prompts, and an +approved command may mutate the workspace or scratch directory. The UI must +state this tradeoff. `SubmitPlan` preserves exact Markdown bytes in a new unique `/.pi/plan/*.md` file through host-core, validates the in-root artifact path, computes SHA-256 and byte size, and only then creates the `plan_approvals` record with @@ -338,8 +339,8 @@ host-core. They do not change the loopback-only rule above. 1. Renderer cannot `require('fs')` (sandbox + no nodeIntegration) — verified 2. Plan Write/Edit/plugin calls cannot run under any permission mode; Bash is - confirmed under Ask/Accept edits and may run without confirmation only under - explicit Auto + confirmed under Ask/Accept edits and under Auto alike, so approval is always + an explicit decision 3. Writing outside the workspace fails — verified (host tests) 4. API keys never appear in plaintext in exports/logs by default — verified 5. Non-whitelisted IPC channels are rejected — verified (M1) diff --git a/docs/spec/06-delivery/04-e2e-test-plan.md b/docs/spec/06-delivery/04-e2e-test-plan.md index b98211c2b4..4b026ad522 100644 --- a/docs/spec/06-delivery/04-e2e-test-plan.md +++ b/docs/spec/06-delivery/04-e2e-test-plan.md @@ -7429,12 +7429,13 @@ must keep splitting are covered by `markdown-blocks.test.mjs`. - **Preconditions**: A project-bound session is idle in Plan with BrowserPreview, a plugin tool, an MCP tool, and a forged `requestedMode = "agent"` fixture. - **Steps**: 1) Inspect visible Plan tools. 2) Use Read/Glob/Grep and - BrowserPreview. 3) Attempt Write, Edit, a builtin outside the read-only set, - a plugin tool without `planSafeActions`, and an MCP tool through the host with - every permission mode. 4) Run Bash under Ask, Accept edits, and Auto. -- **Expected**: Plan denies Write/Edit and any builtin outside the read-only set + BrowserPreview. 3) Attempt Write, Edit, a state-changing builtin, a plugin + tool without `planSafeActions`, and an MCP tool through the host with every + permission mode. 4) Run Bash under Ask, Accept edits, and Auto. +- **Expected**: Plan denies Write/Edit and any state-changing builtin regardless of the forged mode, grants, or Auto; a plugin tool without - `planSafeActions` is refused with `PLUGIN_DISABLED_IN_PLAN`; an MCP tool and + `planSafeActions` is refused with `PLUGIN_DISABLED_IN_PLAN`; a builtin nobody + classified reaches the ordinary path and fails as `TOOL_NOT_FOUND`; an MCP tool and Bash each show a permission card in **every** permission mode, Auto included, and the approved call then runs. An exact-tool session grant suppresses repeat prompts without authorizing other tools or sessions. The runtime remains one diff --git a/docs/spec/07-plugins/16-trusted-extensions.md b/docs/spec/07-plugins/16-trusted-extensions.md index 1250af2c69..deabf93d28 100644 --- a/docs/spec/07-plugins/16-trusted-extensions.md +++ b/docs/spec/07-plugins/16-trusted-extensions.md @@ -412,7 +412,7 @@ the UI broker's own prompt timeout does not extend that budget. tool is rejected with a diagnostic; the earlier registration wins. 2. Extension tools are non-core: they follow the same mode gating and ToolSearch deferral as plugin tools. They are available in Agent mode and - follow the existing per-mode allowlist elsewhere. + withheld in the contract modes, which never reach the host permission path. 3. Execution happens in the sidecar with the `ExtensionAPI` `execute` signature. No host permission prompt is raised; the trust decision was made at enablement. `onUpdate` streams map to tool execution update events. diff --git a/docs/spec/08-meta/decisions-log.md b/docs/spec/08-meta/decisions-log.md index 0f4a3bdd2d..3991d2bd77 100644 --- a/docs/spec/08-meta/decisions-log.md +++ b/docs/spec/08-meta/decisions-log.md @@ -342,8 +342,8 @@ Gold source: local Codex electron captures; latest row wins where rows conflict. | ID | Topic | Decision | Rationale | |---|---|---|---| -| D114 | Per-session scratch directory for agent temp files | **Each session gets `/scratch//` as a second containment root for `Read`/`Write`/`Edit` (absolute paths only; relative paths stay workspace-bound). The path is advertised in the system prompt and as `PI_SCRATCH_DIR` in Bash. Scratch writes auto-allow without a permission card and are excluded from the artifacts table. Scratch is created lazily, deleted with the session, and swept at startup (orphans, >7 days stale). Glob/Grep/BrowserPreview remain workspace-only. Plan does not expose Write/Edit; Plan Bash can still mutate scratch under its resolved permission mode.** | Temp/intermediate files (one-off scripts, downloaded data, drafts) were dirtying the user's project and git status; a host-owned root with identical lexical + symlink defenses keeps the sandbox model intact while giving the model a legitimate place for scratch work | -| D115 | Permission modes: global default + per-session override | *(composer presentation superseded by D132; checkpoint Plan clause superseded by D189)* **A permission mode governs high-risk tool approval: `ask` (confirm everything, default), `accept-edits` (auto-allow Write/Edit, confirm Bash/plugins), `auto` (auto-allow all). Global default lives in settings `defaultPermissionMode`; each session stores `permission_mode` (schema v5, default `inherit`) which overrides it when not inherit. Resolution: session override → global default → ask, enforced solely in host-core `tools.execute`. Plan retains the selector; its Write/Edit/plugin hard deny outranks every mode, while Bash follows the selected mode.** | Confirming every Write/Edit made long agent runs high-friction, but a single global toggle is too coarse — trusted scratch sessions and risky repo sessions need different postures; keeping enforcement host-side preserves the security boundary | +| D114 | Per-session scratch directory for agent temp files | **Each session gets `/scratch//` as a second containment root for `Read`/`Write`/`Edit` (absolute paths only; relative paths stay workspace-bound). The path is advertised in the system prompt and as `PI_SCRATCH_DIR` in Bash. Scratch writes auto-allow without a permission card and are excluded from the artifacts table. Scratch is created lazily, deleted with the session, and swept at startup (orphans, >7 days stale). Glob/Grep/BrowserPreview remain workspace-only. Plan does not expose Write/Edit; Plan Bash can still mutate scratch once approved (ADR 0330: every permission mode takes a card).** | Temp/intermediate files (one-off scripts, downloaded data, drafts) were dirtying the user's project and git status; a host-owned root with identical lexical + symlink defenses keeps the sandbox model intact while giving the model a legitimate place for scratch work | +| D115 | Permission modes: global default + per-session override | *(composer presentation superseded by D132; checkpoint Plan clause superseded by D189)* **A permission mode governs high-risk tool approval: `ask` (confirm everything, default), `accept-edits` (auto-allow Write/Edit, confirm Bash/plugins), `auto` (auto-allow all). Global default lives in settings `defaultPermissionMode`; each session stores `permission_mode` (schema v5, default `inherit`) which overrides it when not inherit. Resolution: session override → global default → ask, enforced solely in host-core `tools.execute`. Plan retains the selector; its Write/Edit/plugin hard deny outranks every mode, while Bash takes a permission card in every mode (ADR 0330).** | Confirming every Write/Edit made long agent runs high-friction, but a single global toggle is too coarse — trusted scratch sessions and risky repo sessions need different postures; keeping enforcement host-side preserves the security boundary | | D132 | Composer permission menu shows effective modes only | **The agent-mode composer chip and menu expose only `ask`, `accept-edits`, and `auto`, with the effective mode selected directly and no global-default/inherit entry or provenance label. Choosing an item stores that explicit session override. Existing `inherit` persistence and resolution from D115 remain unchanged until the user chooses a mode. This supersedes only D115's composer-presentation clause.** | The inherited entry repeated a selectable mode and exposed storage provenance instead of the permission posture the user is choosing; presenting the three effective modes makes the control direct without changing host enforcement | | D116 | Provider failures as assistant messages | **Every provider/model turn failure is attached to a durable `role=assistant`, `status=error` transcript message through optional `UiMessage.error`. The message shows a localized summary and stable code, keeps redacted provider detail behind an accessible disclosure, and offers context-appropriate Retry or Settings actions. Message-bound failures never use toast/global banner presentation and never re-enter later model context.** | Errors belong to the failed turn; preserving them in the transcript makes the response diagnosable after session switches/restarts without contaminating the next model request or exposing credentials | | D127 | Context-preserving reseed + transport retry | **Reseeding a recreated pi runtime from the persisted transcript restores tool call/result pairs (from tool rows' `toolCallId`/`toolArgs`/`toolResult`) in addition to user/assistant text and thinking. Interrupted tool rows restore as errored results; orphaned tool rows get a synthesized call-only assistant carrier so pairs stay adjacent and well-formed. Failed assistant turns stay transcript-only. Separately, request setup uses one bounded pi-ai retry for transient transport/provider failures; post-response stream recovery is defined by D186.** | Text-only reseed collapsed a session's context after any runtime recreation (regenerate/edit, config change, restart): the model lost every tool result it had gathered and — seeing its own history answer without visible tool use — stopped calling tools, degrading agent sessions into bare chat. The incident trigger was a single un-retried provider timeout that forced the user into regenerate. D127 corrects the initially duplicated D120 identifier; D120 remains the earlier application-update decision frozen by baseline 0.4.6. | @@ -475,7 +475,7 @@ section mirrors only marketplace/catalog items still blocking nothing. | D217 | Early plugin launcher warm-up | **As soon as Electron is ready, before backend/plugin boot completes, Electron starts creating and loading the retained plugin launcher while it remains hidden. Window creation uses one shared in-flight promise, so a shortcut received during warm-up joins the same renderer load; failures destroy the incomplete window, clear the promise, and remain retryable on the next invocation. Each show still refreshes the live plugin catalog. On macOS, showing the panel relies on its normal activation and skips a redundant application activation/window-stack move.** | Overlapping launcher renderer startup with backend boot and avoiding duplicate macOS activation work removes first-invocation delay and visible stutter without weakening plugin freshness, IPC, or sandbox boundaries (ADR 0080). | | D219 | Plugin launcher most-recently-used history | **The launcher renderer records each successfully opened plugin id with a timestamp in renderer-local `localStorage` (key `pi.desktop.pluginLaunchHistory`, capped at 24) and presents an empty query in most-recently-used order. A typed query ranks search relevance first and uses recency only as a tiebreaker, so unused plugins sort after the recent ones by name. Corrupted or unavailable storage degrades to the existing name order without breaking launching. This refines D211's presentation without changing IPC, host RPC, permissions, protocol v9, or storage schema v11.** | Keyboard-first launcher users reopen the same plugins repeatedly; remembering the last-used order keeps the most frequent target one Enter away while relevance stays authoritative once the user types. | | D221 | Renderer output size controls | *(bundled CJK clause superseded by D598)* **The renderer build sets `minify: "esbuild"` explicitly because `electron-vite` hard-defaults its renderer preset to `minify: false`. A `pi-drop-legacy-font-fallbacks` plugin strips comma-prefixed `woff`/`truetype` `src` entries with `enforce: "pre"`, before Vite registers them as assets, since the bundled Chromium always supports `woff2`; a face whose only source is a legacy format keeps it. `BrandLogo` imports 192x192 marks derived from the canonical masters at `src/assets/brand/logo-{light,dark}.png` rather than the 1024px installer icons in `build/`, refining D079/D094 without changing the visual identity. `packaging-footprint.test.mjs` asserts all three so a framework default or a reverted import cannot silently regress them. The two bundled CJK faces stay unsubset per ADR 0083 section 2.** | `out/renderer` fell from 31 MiB to 24 MiB with no feature loss; the unminified default and the installer-icon import were accidental costs, while CJK subsetting would drop glyphs from user content and needs an ADR revision | -| D198 | Goal mode is the second contract mode | **Goal is a third durable operating mode (`agent` / `plan` / `goal`) that reuses the Plan approval pipeline end to end. `EnterGoalMode` switches an active Agent turn into the Goal contract state; `SubmitGoal(title, markdown, question)` writes an immutable host-owned `.pi/goal/.md` artifact and inserts one pending `plan_approvals` row. The single approval table gains a `kind` column (`plan` or `goal`, defaulting to `plan`) in schema v11; the kind — not the projected planning state — selects the prompt, artifact directory, submit tool, and i18n namespace, so one approval bar and one execution queue serve both. Plan and Goal are together the contract modes: they share one tool allowlist (Read/Glob/Grep/BrowserPreview/Bash plus their own submit tool), one host hard deny for Write/Edit/plugin/unknown tools evaluated against the durable mode, the shared `*_IN_PLAN` error codes, and the `PLAN_REQUIRES_INTERACTIVE_SESSION` rejection of unattended runs. An approved Goal executes autonomously in Agent: it chooses its own approach and keeps working until every acceptance criterion is verified or a boundary blocks it, then reports criterion by criterion. The wire `kind` is optional and absent means `plan`, so this is additive inside protocol v9.** | Plan answers "carry out these steps"; users also need "reach this outcome, decide the steps yourself" (Claude Code's plan mode and Codex's goal-shaped runs). Making the goal statement, acceptance criteria, and boundaries an approved contract keeps autonomy auditable, and discriminating one pipeline by kind avoids a parallel table, approval surface, and permission boundary that would inevitably drift. | +| D198 | Goal mode is the second contract mode | **Goal is a third durable operating mode (`agent` / `plan` / `goal`) that reuses the Plan approval pipeline end to end. `EnterGoalMode` switches an active Agent turn into the Goal contract state; `SubmitGoal(title, markdown, question)` writes an immutable host-owned `.pi/goal/.md` artifact and inserts one pending `plan_approvals` row. The single approval table gains a `kind` column (`plan` or `goal`, defaulting to `plan`) in schema v11; the kind — not the projected planning state — selects the prompt, artifact directory, submit tool, and i18n namespace, so one approval bar and one execution queue serve both. Plan and Goal are together the contract modes: they share one tool policy — the same hard deny for state-changing builtins and plugin tools, Bash by card, and their own submit tool — evaluated against the durable mode, the shared `*_IN_PLAN` error codes, and the `PLAN_REQUIRES_INTERACTIVE_SESSION` rejection of unattended runs. An approved Goal executes autonomously in Agent: it chooses its own approach and keeps working until every acceptance criterion is verified or a boundary blocks it, then reports criterion by criterion. The wire `kind` is optional and absent means `plan`, so this is additive inside protocol v9.** | Plan answers "carry out these steps"; users also need "reach this outcome, decide the steps yourself" (Claude Code's plan mode and Codex's goal-shaped runs). Making the goal statement, acceptance criteria, and boundaries an approved contract keeps autonomy auditable, and discriminating one pipeline by kind avoids a parallel table, approval surface, and permission boundary that would inevitably drift. | | D144 | Sidebar primary chrome at 14px | **Expanded sidebar primary chrome (New task, Plugins, session titles, footer identity name, profile menu actions) uses `--text-base` (14px). Project/group titles and empty-state copy use `--text-md` (13px). Section labels use `--text-sm` (12px). Primary sidebar content must not use the micro `--text-xs` band.** | 13px sidebar body felt undersized next to the 14px chat surface; bumping only primary chrome keeps density while restoring visual balance without a global type-scale change. | | D145 | Disable browser text correction on editable fields | **Every text `input` and `textarea` in the desktop renderer disables browser text correction: `spellCheck={false}`, `autoCorrect="off"`, and `autoCapitalize="off"`. Shared `Input`/`Textarea` primitives default these values; raw fields (composer, message edit, command palette, global search, settings/plugins/projects search, model search, browser URL bar, provider model combo) set them explicitly. Checkboxes and non-text controls are unchanged.** | Coding prompts, paths, model ids, and URLs must not be red-underlined or auto-mutated by Chromium/OS text correction; the shell is an application, not a document editor. | @@ -1533,7 +1533,7 @@ D193, and D194. discriminator; a submit tool run against the other kind fails with `PLAN_KIND_MISMATCH`. - Plan and Goal are the contract modes. The host hard deny for - Write/Edit/plugin/unknown tools, the Bash-follows-permission-mode rule, the + state-changing builtins and plugin tools, the Bash-always-asks rule, the `*_IN_PLAN` error codes, the single-pending-approval invariant, and the `PLAN_REQUIRES_INTERACTIVE_SESSION` rejection of scheduled runs all apply to both, evaluated against the session's durable mode. diff --git a/docs/spec/README.md b/docs/spec/README.md index fc8b356f88..1007c64ace 100644 --- a/docs/spec/README.md +++ b/docs/spec/README.md @@ -87,7 +87,7 @@ docs/spec/ 6. SQLite owned by Rust only 7. Default mode = Agent; operating selector = Agent | Plan | Goal. Plan and Goal are contract states of the same Agent and are not strict read-only security - profiles because Bash follows the selected permission mode + profiles because Bash takes a permission card in every permission mode 8. SubmitPlan writes exact Markdown bytes to a new host-owned `.pi/plan/*.md` artifact; title/question stay structured in `plan_approvals`, approval opens the artifact, is approve/reject only, and diff --git a/docs/zh-CN/spec/00-baseline.md b/docs/zh-CN/spec/00-baseline.md index e015cc1684..f6404562d6 100644 --- a/docs/zh-CN/spec/00-baseline.md +++ b/docs/zh-CN/spec/00-baseline.md @@ -156,10 +156,11 @@ 一次溢出重试。该模型可以通过请求一个新窗口 `new_context`；每次压缩都会添加一行记录和一个警告。 没有面向用户的设置** -44. Plan 工具和策略：**Read / Glob / Grep / BrowserPreview / Bash plus - `EnterPlanMode` 和 `SubmitPlan`； Write/Edit/plugin 和 - 未知工具被拒绝。 Bash 遵循 `ask`、`accept-edits` 或 `auto`，因此 - Plan 是计划意图，而不是严格的只读安全配置文件。** +44. Plan 工具和策略（ADR 0330 修订）：**Read / Glob / Grep / BrowserPreview / + asktool / Bash / SubmitPlan、可发现的用户 MCP 工具，以及声明 + `planSafeActions` 的插件。会改变文件、会话状态或派生工作的内置工具被硬拒绝； + 其余内置工具走普通权限路径。Bash 和用户 MCP 工具需要确认或精确工具名的会话授权， + 包括 `auto` 在内；这是授权，不是文件系统隔离。** 45. Plan 检查点：**`SubmitPlan(title, markdown, question)` 导致 host-core 将确切的 Markdown 字节保留在新的唯一文件中 `/.pi/plan/*.md` 工件，而 title/question 仍然存在 diff --git a/docs/zh-CN/spec/03-runtime/02-agent-runtime.md b/docs/zh-CN/spec/03-runtime/02-agent-runtime.md index bc62d1abc8..511352fda0 100644 --- a/docs/zh-CN/spec/03-runtime/02-agent-runtime.md +++ b/docs/zh-CN/spec/03-runtime/02-agent-runtime.md @@ -509,7 +509,7 @@ Plan 和 Goal 是两种 **合约模式** (D198)。他们共用一个耐用的 当用户选择 Plan 时，`Agent / inactive` 进入 `Plan / planning` 空闲时或 Agent 调用 `EnterPlanMode` 时。在 Plan 中，Agent 可以 -检查、使用上下文控制、通过选定的权限模式运行 Bash， +检查、使用上下文控制、经权限卡运行 Bash， 并调用 `SubmitPlan(title, markdown, question)`。主机核心保留 在新的不可变中提交 Markdown 字节 `.pi/plan/.md` 工件，记录其相对 path/hash/size 和 diff --git a/docs/zh-CN/spec/03-runtime/03-tools-and-permissions.md b/docs/zh-CN/spec/03-runtime/03-tools-and-permissions.md index 6fc54547bf..0fb6b60295 100644 --- a/docs/zh-CN/spec/03-runtime/03-tools-and-permissions.md +++ b/docs/zh-CN/spec/03-runtime/03-tools-and-permissions.md @@ -14,7 +14,7 @@ | Agent 工具 | 读取 / Glob / Grep / 写入 / 编辑 / Bash + 已注册的插件工具 | | Plan 工具 | Read / Glob / Grep / BrowserPreview / asktool / Bash / SubmitPlan，以及可发现的 MCP 工具和声明 `planSafeActions` 的插件；普通权限规则仍适用 | | Goal 工具 | Read / Glob / Grep / BrowserPreview / asktool / Bash / SubmitGoal，以及可发现的 MCP 工具和声明 `planSafeActions` 的插件；普通权限规则仍适用 | -| Plan 和 Goal 硬拒绝 | 写入 / 编辑 / TodoWrite / GenerateImages / 委派工具 / 另一类的提交工具。不在只读清单内的内置工具在分类前一律拒绝 | +| Plan 和 Goal 硬拒绝 | 写入 / 编辑 / TodoWrite / GenerateImages / PluginScaffold / PluginPack / ScheduledTask 系列 / 委派工具 / 另一类的提交工具。其余内置工具走普通权限路径 | | Plan 和 Goal 的 Bash | 包括 `auto` 在内均需确认，已有精确工具名的会话授权除外 | | Plan 和 Goal 的未分类工具 | MCP 工具需要确认或精确工具名的会话授权；`plugin_` 工具需要非空 `planSafeActions` | | 插件 `planSafeActions` | 非空的 `action` 字符串数组；运行时隐藏无列表工具，host 应用普通权限，plugin-runtime 拒绝未列出的动作（ADR 0211） | @@ -427,9 +427,8 @@ MCP 工具调用会显示审批卡片，原因为 "MCP server tool requires appr Agent 模式下 `auto` 自动允许。 在 Plan 和 Goal 中，MCP 工具无法分类——名称说明不了它是否改变状态，服务器自己的标注也不构成 -证据——因此它在每种权限模式下都弹出权限卡，包括 `auto`。这是契约模式在 Bash 之外唯一覆盖所选 -权限模式的地方。该工具保留在模型的工具目录中，权限卡才够得着（D640，ADR -`mcp-tool-approval-risk`）。 +证据——因此它在每种权限模式下都弹出权限卡，包括 `auto`，与 Bash 完全一样。该工具保留在模型的 +工具目录中，权限卡才够得着（D640，ADR `mcp-tool-approval-risk`）。 ### 决策类型 @@ -465,8 +464,8 @@ Agent 模式下 `auto` 自动允许。 - 会话值存储在 `sessions.permission_mode` 中 （`inherit | ask | accept-edits | auto`，默认 `inherit`，架构 v5）和 通过 `session.configure` `permissionMode` 设置。 -- 契约模式的硬拒绝胜过所有权限模式，适用于 `Write`、`Edit`、`TodoWrite`、 - `GenerateImages` 和委派工具。 `auto` 无法重新启用隐藏或拒绝的工具。 +- 契约模式的硬拒绝胜过所有权限模式，适用于上表硬拒绝行中列出的会改变状态的内置工具。 + `auto` 无法重新启用隐藏或拒绝的工具。 - 会话根目录内的低风险工具（`Read`/`Glob`/`Grep`）自动允许 每种模式都和以前一样。 - `BrowserPreview` 是显式只读 UI 检查功能，并且是 @@ -537,10 +536,10 @@ MVP 可以通过写入 SQLite 或日志文件来启动。 | Goal | 允许 | 允许 | 拒绝 | 确认（所有权限模式） | 仅 `planSafeActions` | ### 注释 -- 契约模式按调用的效果拒绝，而不是按名称是否在清单中。只读内置工具走普通权限路径； - 不在该清单内的内置工具（`Write`、`Edit`、`TodoWrite`、`GenerateImages`、委派工具）在权限 UI 之前 - 硬拒绝，直接主机调用不能绕过矩阵。未分类的内置工具按拒绝处理而非放行，因此之后新增的工具 - 不会因遗漏而放宽该模式。 +- 契约模式按调用的效果拒绝，而不是按名称是否在清单中。会改变状态的内置工具（`Write`、`Edit`、 + `TodoWrite`、`GenerateImages`、`PluginScaffold`、`PluginPack`、`ScheduledTask*`、`Task*` 委派工具） + 在权限 UI 之前硬拒绝，直接主机调用不能绕过矩阵。其余内置工具（包括无人分类的新工具）走普通权限 + 路径，仍受风险、有效权限模式、会话授权与外部路径规则约束。 - Plan 和 Goal 的 Bash 在每种权限模式下都弹出权限卡。命令做什么由运行它的 shell 决定， host 不读取命令字符串来判定；因此 `auto` 不能代替用户的决定。`Bash` 的会话授权仍会抑制 该会话后续的重复确认。 diff --git a/docs/zh-CN/spec/03-runtime/05-host-core-rust.md b/docs/zh-CN/spec/03-runtime/05-host-core-rust.md index 803cb61956..51b17371d4 100644 --- a/docs/zh-CN/spec/03-runtime/05-host-core-rust.md +++ b/docs/zh-CN/spec/03-runtime/05-host-core-rust.md @@ -113,8 +113,8 @@ notification.list 仅在主机权限评估后才能解析显式外部路径 2、Host解析持久会话模式；请求提供模式永远不会 权威的 -3. Plan 和 Goal 在权限评估之前拒绝 Write/Edit/plugin/unknown 工具 -4. Plan 和 Goal Bash 遵循持久权限模式，在 Auto 下可能会发生变异 +3. Plan 和 Goal 在权限评估之前硬拒绝会改变状态的内置工具 +4. Plan 和 Goal Bash 在每种权限模式下（含 Auto）都弹出权限卡 5. Plan 和 Goal 工件字节、路径、散列、大小和 approval/execution 标识为 主机验证 6. Plan/Goal 批准在 Agent 条目之前经过主机验证、持久且原子 @@ -141,8 +141,7 @@ notification.list 通过 `session.endTurn` 交易；结果已经可见 集中当前的聊天和中止的回合不会创建任何内容 6. 持久 Plan 或 Goal 会话无法通过以下方式授权 Write/Edit/plugin 工具： - 冲突的请求模式，并且 Plan/Goal Bash 遵循已解析的权限 - 模式 + 冲突的请求模式，并且 Plan/Goal Bash 在每种权限模式下都弹出权限卡 7. SubmitPlan 将精确的 Markdown 字节写入新的 `.pi/plan/*.md` 工件并 将持久的 path/hash/size 和结构化的 title/question 存储在 `plan_approvals`；批准为 approve/reject-only、session/turn/version diff --git a/docs/zh-CN/spec/03-runtime/06-host-rpc-protocol.md b/docs/zh-CN/spec/03-runtime/06-host-rpc-protocol.md index b471398468..e51ce897db 100644 --- a/docs/zh-CN/spec/03-runtime/06-host-rpc-protocol.md +++ b/docs/zh-CN/spec/03-runtime/06-host-rpc-protocol.md @@ -667,16 +667,15 @@ type ToolsExecuteParams = { 在通用权限评估之前，host-core 应用模式策略： -- Plan 和 Goal 允许 `Read`、`Glob`、`Grep`、`BrowserPreview`、`Bash` 和 - 适用于实时的种类提交工具（`SubmitPlan` / `SubmitGoal`） - 规划状态。 -- Plan 和 Goal 拒绝 `Write`、`Edit`、每个插件工具以及以下未知工具 - 所有权限模式和授予。主机读取会话的**持久**模式 +- Plan 和 Goal 默认放行内置工具，除非下一条拒绝它。因此 `Read`、`Glob`、`Grep`、 + `BrowserPreview` 走普通权限路径，而该类别的提交工具（`SubmitPlan` / `SubmitGoal`） + 在实时规划状态下被允许。 +- Plan 和 Goal 在所有权限模式和授权下拒绝 `Write`、`Edit`、其余会改变状态的内置工具， + 以及未声明 `planSafeActions` 的插件工具。主机读取会话的**持久**模式 对于此检查，因此在 `tools.execute` 中声明 `agent` 的 sidecar 无法扩大 它和 `*_IN_PLAN` 错误代码是两种类型共享的。 -- Plan 和 Goal `Bash` 遵循已解析的权限模式：`ask` 和 - `accept-edits` - 发出 `permissions.request`； `auto` 无需确认即可执行，并且可能 +- Plan 和 Goal `Bash` 在每种已解析的权限模式下都弹出权限卡：`ask`、 + `accept-edits` 与 `auto` 都发出 `permissions.request`，批准后的命令可能 变异。主机重新解析有效 shell ID/dialect 并要求 精确之前的 `expectedCommandShellId` 和 `expectedCommandShellDialect` 权限评估并在生成前再次评估；它流式传输 stdout/stderr @@ -1045,8 +1044,8 @@ JSON-RPC 错误携带一个数字 `code` 以及 `data.errorCode`，后者是来 9. 分叉消息会排除后面的所有源行并拒绝 未创建子项的未知消息 10. 伪造的 `requestedMode` 无法授权工具进入持久模式； - Plan 和 Goal 拒绝 Write/Edit/plugin/unknown 工具并申请权限 - 根据 `requestedMode`/Write/Edit/plugin/unknown/Plan 提示 Bash + Plan 和 Goal 拒绝会改变状态的内置工具与未声明 `planSafeActions` 的插件工具， + 并在每种权限模式下为 Bash 弹出权限卡 11. SubmitPlan 和 SubmitGoal 将精确的 Markdown 字节写入唯一的 `.pi/plan/*.md` 或 `.pi/goal/*.md` 文件 hash/size 和结构化 title/question 字段；仅匹配 diff --git a/docs/zh-CN/spec/03-runtime/08-error-codes.md b/docs/zh-CN/spec/03-runtime/08-error-codes.md index 9bb5b524c5..89ae61bf37 100644 --- a/docs/zh-CN/spec/03-runtime/08-error-codes.md +++ b/docs/zh-CN/spec/03-runtime/08-error-codes.md @@ -142,7 +142,7 @@ stdio 与 Tokio 的动态阻塞池隔离，因此后一种情况 | `WRITE_DISABLED_IN_PLAN` | 不 | Write 的契约模式硬拒绝 | | `EDIT_DISABLED_IN_PLAN` | 不 | 编辑的契约模式硬拒绝 | | `PLUGIN_DISABLED_IN_PLAN` | 不 | 契约模式拒绝未声明 `planSafeActions` 的插件工具 | -| `TOOL_DISABLED_IN_PLAN` | 不 | 只读清单之外的内置工具的契约模式硬拒绝 | +| `TOOL_DISABLED_IN_PLAN` | 不 | 会改变状态的内置工具的契约模式硬拒绝 | | `PLAN_NOT_ACTIVE` | 不 | 在没有协商合同的情况下运行了提交工具 | | `PLAN_KIND_MISMATCH` | 不 | Goal 模式下的 `SubmitPlan`，或 Plan 模式下的 `SubmitGoal` | | `PLAN_APPROVAL_REQUIRED` | 不 | SubmitPlan/SubmitGoal 正在等待单独的批准 | diff --git a/docs/zh-CN/spec/03-runtime/10-session-state-machine.md b/docs/zh-CN/spec/03-runtime/10-session-state-machine.md index c2cd99233d..b6999faabf 100644 --- a/docs/zh-CN/spec/03-runtime/10-session-state-machine.md +++ b/docs/zh-CN/spec/03-runtime/10-session-state-machine.md @@ -29,9 +29,8 @@ Agent / running -- complete | fail | abort --> Agent / inactive ``` -两种合约模式都保留了权限模式选择器。他们的 `Bash` 政策是 -`ask` 或 -`accept-edits` = 确认，`auto` = 不确认，所以是合约模式 +两种合约模式都保留了权限模式选择器，但它在 Bash 上不再有区别： +`ask`、`accept-edits` 与 `auto` 都弹出权限卡，所以合约模式 表达协商意图，但不是严格的只读安全配置文件。 Write/Edit 和 在每个 Plan 或 Goal 权限模式下，插件工具仍被主机策略拒绝。 @@ -203,10 +202,9 @@ accept_prompt `queued`/reject/expiry/interruption 收敛于相同的规划状态，并且 批准恢复 Agent 模式下的 Agent -9. 合约模式策略仅通过选择的权限模式允许Bash - 和 - 在 Goal 中拒绝 Write/Edit/plugins，无论 `auto` 或会话授权如何 - 与 Plan 完全相同 +9. 合约模式策略在每种权限模式下都为 Bash 弹出权限卡， + 不论 `auto` 或会话授权如何都拒绝 Write/Edit/plugins 与改变状态的内置工具， + 在 Goal 中与 Plan 完全相同 10. SubmitPlan/SubmitGoal 使用以下命令写入精确唯一的 `.pi//*.md` 工件 hash/size， 保持 title/question 结构化，并且只有 approve/reject 可以解析其 diff --git a/docs/zh-CN/spec/04-ux/08-component-spec.md b/docs/zh-CN/spec/04-ux/08-component-spec.md index 5c18f2aaac..58ef8f1d39 100644 --- a/docs/zh-CN/spec/04-ux/08-component-spec.md +++ b/docs/zh-CN/spec/04-ux/08-component-spec.md @@ -1952,8 +1952,7 @@ approved/completed，或中断的终端卡；终端卡可能会保留 - 该卡是一个会话范围的 `region`，具有本地化的计划标题。 - 批准、拒绝和中止控制具有明确的标签和 键盘焦点。 -- 所选权限模式公开无线电语义及其 Plan/Goal Bash - 结果可在可访问的描述中获得。 +- 所选权限模式公开无线电语义，并在无障碍描述中说明 Plan 和 Goal 无论模式如何都会先询问 Bash。 - 解决方案不会导航到另一个会话或从某个会话转移焦点 不同的会话。 diff --git a/docs/zh-CN/spec/04-ux/09-interaction-patterns.md b/docs/zh-CN/spec/04-ux/09-interaction-patterns.md index f6bf833705..646365800b 100644 --- a/docs/zh-CN/spec/04-ux/09-interaction-patterns.md +++ b/docs/zh-CN/spec/04-ux/09-interaction-patterns.md @@ -767,8 +767,8 @@ Agent calls a permission-gated tool (including Plan/Goal Bash under Ask or Accep 调用 `EnterPlanMode` / `EnterGoalMode`；主机 persists/validates 匹配合约模式和渲染器项目 `planning`。 2. Agent 使用选定的合约工具集进行调查。 Read/Glob/Grep 和 - 允许使用 BrowserPreview； Bash 遵循可见权限模式。一个 - Contract-mode Bash 命令可能会在 Auto 下发生变化，因此模式芯片仍然可见。 + 允许使用 BrowserPreview； Bash 无论可见权限模式如何都弹出权限卡。批准后的 + 契约模式 Bash 命令可能改变状态，因此模式芯片仍然可见。 该回合处于实时 `planning` 时，Composer 模式芯片脉冲，紧凑的规划行占用与 Working 相同的预留底部位置，直到回合结束或等待用户操作。具体运行时阶段优先，工具和回答不会隐藏运行提示。 3. Agent 在其工具批次中单独调用 `SubmitPlan` 或 `SubmitGoal`。 Host-core 将准确的 Markdown 字节保留在新的不可变中 diff --git a/docs/zh-CN/spec/05-security/01-security.md b/docs/zh-CN/spec/05-security/01-security.md index 2f311d244f..ad04b7c2e9 100644 --- a/docs/zh-CN/spec/05-security/01-security.md +++ b/docs/zh-CN/spec/05-security/01-security.md @@ -91,10 +91,10 @@ Plan 本身并不是工作区安全边界。主机核解决了 每个 `tools.execute` 调用的持久会话模式并应用 Plan 矩阵 在权限模式、授予、插件风险或 renderer/sidecar 状态之前。 Plan -拒绝 Write/Edit/plugin/unknown 工具，而 BrowserPreview 是显式的 -只读 UI 检查异常（它打开随应用打包的 `pi.browser` chrome；原始 -CDP 插件工具在 Plan 中仍被拒绝）。 Bash 在 Plan 中仍然可用：询问并 -接受编辑提示，自动运行而无需确认，并且可能会改变 +拒绝会改变状态的内置工具与未声明 `planSafeActions` 的插件工具，而 +BrowserPreview 是显式的只读 UI 检查异常（它打开随应用打包的 `pi.browser` +chrome；原始 CDP 插件工具在 Plan 中仍被拒绝）。 Bash 在 Plan 中仍然可用： +每种权限模式（含 Auto）都弹出权限卡，批准后的命令可能会改变 工作区或临时目录。用户界面必须说明这种权衡。 `SubmitPlan` 在新的唯一 `/.pi/plan/*.md` 中保留精确的 Markdown 字节 通过 host-core 文件，验证根内工件路径，计算 SHA-256 diff --git a/docs/zh-CN/spec/06-delivery/04-e2e-test-plan.md b/docs/zh-CN/spec/06-delivery/04-e2e-test-plan.md index 232ad6b95f..e095938b5b 100644 --- a/docs/zh-CN/spec/06-delivery/04-e2e-test-plan.md +++ b/docs/zh-CN/spec/06-delivery/04-e2e-test-plan.md @@ -4227,11 +4227,12 @@ eleven-tool-round desktop paths are verified by - **先决条件**：项目绑定会话在 Plan 和 BrowserPreview 中处于空闲状态， 一个插件工具、一个 MCP 工具和一个伪造的 `requestedMode = "agent"` 夹具。 - **步骤**：1) 检查可见的 Plan 工具。 2) 使用 Read/Glob/Grep 和 - BrowserPreview。 3) 在各种权限模式下通过主机尝试 Write、Edit、只读清单之外的内置工具、 + BrowserPreview。 3) 在各种权限模式下通过主机尝试 Write、Edit、会改变状态的内置工具、 未声明 `planSafeActions` 的插件工具，以及一个 MCP 工具。 4) 在 Ask、Accept edits 和 Auto 下运行 Bash。 -- **预期**：无论伪造模式、授权还是 Auto，Plan 都拒绝 Write/Edit 以及只读清单之外的任何内置 - 工具；未声明 `planSafeActions` 的插件工具以 `PLUGIN_DISABLED_IN_PLAN` 被拒绝；MCP 工具和 +- **预期**：无论伪造模式、授权还是 Auto，Plan 都拒绝 Write/Edit 以及任何会改变状态的内置 + 工具；未声明 `planSafeActions` 的插件工具以 `PLUGIN_DISABLED_IN_PLAN` 被拒绝；无人分类的内置 + 工具走普通路径并以 `TOOL_NOT_FOUND` 失败；MCP 工具和 Bash 在**每一种**权限模式下（含 Auto）各自弹出权限卡，批准后调用才运行。精确工具名的会话授权 免去重复确认，但不授权其他工具或会话。运行时仍为同一 pi Agent，且所有拒绝都会经过审计。 - **链接规格**：`03-runtime/02-agent-runtime.md`， @@ -5938,8 +5939,8 @@ eleven-tool-round desktop paths are verified by ### US-UI-04 没有工作区上下文的 Composer - 打开 git 工作区时，composer 不显示项目、本地或分支 提示表面上方的标签。 -- 操作模式选择器在 Agent、Plan 和 Goal 之间切换；双方签订合同 - 模式保留了权限模式芯片并解释了它们的 Bash 权衡。 +- 操作模式选择器在 Agent、Plan 和 Goal 之间切换；两种契约 + 模式保留了权限模式芯片并解释了 Bash 在每种模式下都需要确认。 ### US-UI-05 区域设置 chrome - 在 zh-CN 系统语言环境中，侧边栏标签以中文呈现（项目/临时会话）， diff --git a/docs/zh-CN/spec/07-plugins/16-trusted-extensions.md b/docs/zh-CN/spec/07-plugins/16-trusted-extensions.md index ec0cd5aade..2a9647989e 100644 --- a/docs/zh-CN/spec/07-plugins/16-trusted-extensions.md +++ b/docs/zh-CN/spec/07-plugins/16-trusted-extensions.md @@ -240,7 +240,7 @@ UI broker 自身的提示超时不会延长该预算。 1. 注册的工具以其声明名称加入会话工具目录。与核心工具、插件工具或用户 MCP 工具同名的注册被拒绝并记诊断；先注册者胜出。 2. 扩展工具是非核心工具：与插件工具遵循相同的模式门控和 ToolSearch 延迟。它们 - 在 Agent 模式可用，其他模式遵循现有的按模式白名单。 + 在 Agent 模式可用，在契约模式中被扣留——它从不进入宿主权限路径。 3. 执行在 sidecar 内按 `ExtensionAPI` 的 `execute` 签名进行。不弹出宿主权限提示；信任决定已 在启用时做出。`onUpdate` 流映射到工具执行更新事件。 4. 每次执行写一条审计记录，含扩展 id、工具名和耗时。不记录参数。 diff --git a/docs/zh-CN/spec/08-meta/decisions-log.md b/docs/zh-CN/spec/08-meta/decisions-log.md index 6f5dd088c0..5800f83504 100644 --- a/docs/zh-CN/spec/08-meta/decisions-log.md +++ b/docs/zh-CN/spec/08-meta/decisions-log.md @@ -343,8 +343,8 @@ | 身份证号 | 主题 | 决定 | 基本原理 | |---|---|---|---| -| D114 | 代理临时文件的每个会话暂存目录 | **每个会话将 `/scratch//` 作为 strict/no-HTML/no-link/Electron/`/scratch//` 的第二个包含根（仅限绝对路径；相对路径保持工作区限制）。该路径在系统提示符中公布，并在 Bash 中公布为 `PI_SCRATCH_DIR`。 Scratch 在没有权限卡的情况下写入自动允许，并从工件表中排除。 Scratch 是惰性创建的，随会话一起删除，并在启动时清除（孤儿，>7 天陈旧）。 Glob/Grep/BrowserPreview 仍仅限工作区。 Plan 不公开 Write/Edit； Plan Bash 在其解析权限模式下仍然可以改变暂存。** | Temp/intermediate 文件（一次性脚本、下载的数据、草稿）污染了用户的项目和 git 状态；主机拥有的根具有相同的词法+符号链接防御，可以保持沙箱模型完整，同时为模型提供合法的临时工作空间 | -| D115 | 权限模式：全局默认+每会话覆盖 | *（Composer 演示文稿被 D132 取代；检查点 Plan 子句被 D189 取代）* **权限模式管理高风险工具审批：`ask`（确认所有内容，默认）、`accept-edits`（自动允许 Write/Edit、确认 Bash/plugins）、`auto` （自动允许全部）。全局默认值位于设置 `defaultPermissionMode` 中；每个会话都存储 `permission_mode`（架构 v5，默认 `inherit`），在不继承时会覆盖它。解决方案：会话覆盖→全局默认→询问，仅在 host-core `tools.execute` 中强制执行。 Plan 保留选择器；它的 Write/Edit/plugin 硬拒绝排名高于每种模式，而 Bash 则遵循所选模式。** | 确认每个 Write/Edit 都会使长代理运行高摩擦，但单个全局切换太粗糙 - 受信任的临时会话和有风险的回购会话需要不同的姿势；保持主机端强制执行可保护安全边界 | +| D114 | 代理临时文件的每个会话暂存目录 | **每个会话将 `/scratch//` 作为 strict/no-HTML/no-link/Electron/`/scratch//` 的第二个包含根（仅限绝对路径；相对路径保持工作区限制）。该路径在系统提示符中公布，并在 Bash 中公布为 `PI_SCRATCH_DIR`。 Scratch 在没有权限卡的情况下写入自动允许，并从工件表中排除。 Scratch 是惰性创建的，随会话一起删除，并在启动时清除（孤儿，>7 天陈旧）。 Glob/Grep/BrowserPreview 仍仅限工作区。 Plan 不公开 Write/Edit； Plan Bash 获批后仍可改变暂存（ADR 0330：每种权限模式都发权限卡）。** | Temp/intermediate 文件（一次性脚本、下载的数据、草稿）污染了用户的项目和 git 状态；主机拥有的根具有相同的词法+符号链接防御，可以保持沙箱模型完整，同时为模型提供合法的临时工作空间 | +| D115 | 权限模式：全局默认+每会话覆盖 | *（Composer 演示文稿被 D132 取代；检查点 Plan 子句被 D189 取代）* **权限模式管理高风险工具审批：`ask`（确认所有内容，默认）、`accept-edits`（自动允许 Write/Edit、确认 Bash/plugins）、`auto` （自动允许全部）。全局默认值位于设置 `defaultPermissionMode` 中；每个会话都存储 `permission_mode`（架构 v5，默认 `inherit`），在不继承时会覆盖它。解决方案：会话覆盖→全局默认→询问，仅在 host-core `tools.execute` 中强制执行。 Plan 保留选择器；它的 Write/Edit/plugin 硬拒绝排名高于每种模式，而 Bash 在每种模式下都弹出权限卡（ADR 0330）。** | 确认每个 Write/Edit 都会使长代理运行高摩擦，但单个全局切换太粗糙 - 受信任的临时会话和有风险的回购会话需要不同的姿势；保持主机端强制执行可保护安全边界 | | D132 | Composer权限菜单仅显示有效模式 | **代理模式 Composer 芯片和菜单仅公开 `ask`、`accept-edits` 和 `auto`，直接选择有效模式，没有 global-default/inherit 条目或出处标签。选择一个项目会存储显式会话覆盖。在用户选择模式之前，D115 中现有的 `inherit` 持久性和分辨率保持不变。这仅取代 D115 的输入框演示条款。** | 继承的条目重复了可选择的模式并暴露了存储来源，而不是用户选择的权限状态；三种有效模式的呈现，在不改变主机执行的情况下，让控制变得直接 | | D116 | 提供商失败作为辅助消息 | **每个 provider/model 转向失败都会通过可选的 `UiMessage.error` 附加到持久的 `role=assistant`、`status=error` 转录消息。该消息显示本地化的摘要和稳定的代码，将经过编辑的提供程序详细信息保留在可访问的披露后面，并提供适合上下文的重试或设置操作。消息绑定失败从不使用 toast/global 横幅呈现，也从不重新进入以后的模型上下文。** | 错误属于失败的回合；将它们保留在转录本中使得响应在会话 switches/restarts 之后可诊断，而不会污染下一个模型请求或暴露凭据 | | D127 | 上下文保留重新播种+传输重试 | **除了 user/assistant 文本和思考之外，从持久记录中重新播种重新创建的 pi 运行时还会恢复工具 call/result 对（来自工具行的 `role=assistant`/`status=error`/`UiMessage.error`）。中断的工具行恢复为错误结果；孤立的工具行获得一个合成的仅呼叫辅助载体，因此对保持相邻且格式良好。失败的助理将仅保留成绩单。另外，请求设置使用一次有界 pi-ai 重试来处理暂时的 transport/provider 失败；响应后流恢复由 D186 定义。** | 在任何运行时重新创建（regenerate/edit、配置更改、重新启动）后，纯文本重新播种会崩溃会话的上下文：模型丢失了它收集的所有工具结果，并且在没有可见工具使用的情况下看到自己的历史答案，停止调用工具，将代理会话降级为裸聊天。事件触发器是一个未重试的提供程序超时，迫使用户重新生成。 D127 更正最初重复的 D120 标识符； D120 仍然是由基线 0.4.6 冻结的早期应用程序更新决策。 | @@ -473,7 +473,7 @@ | D217 | 启动完成后预热插件启动器 | **主应用及后端完成启动后，Electron 创建并加载保留的插件启动器，同时保持窗口隐藏。窗口创建使用一个共享的进行中 promise，因此预热期间收到的快捷键会复用同一次渲染器加载；若预热失败，则销毁未完成的窗口、清除 promise，并允许下次调用重试。每次显示窗口时仍会刷新实时插件目录。** | 将 BrowserWindow 分配和渲染器启动移出首次可见调用，使启动器的显示延迟更稳定，同时不拖慢主应用启动，也不削弱插件目录的新鲜度、IPC 或沙盒边界（ADR 0080）。 | | D219 | 插件启动器的最近使用历史 | **启动器渲染器把每个成功打开过的插件 id 连同时间戳记录在渲染器本地的 `localStorage` 里（键 `pi.desktop.pluginLaunchHistory`，上限 24 条），并在查询为空时按最近使用顺序呈现。键入查询后，先按搜索相关性排序，仅在同等相关时用最近使用作为决胜条件，因此未用过的插件按名称排在最近使用的之后。存储损坏或不可用时降级为原有的名称顺序，不影响启动。这只是细化 D211 的呈现，不改变 IPC、主机 RPC、权限、协议 v9 或存储架构 v11。** | 以键盘为先的启动器用户会反复打开同一批插件；记住上次使用顺序让最常用的目标始终只差一个 Enter，而用户一旦键入，相关性仍然是权威。 | | D221 | 渲染器产物体积控制 | *(内置 CJK 字体条款由 D598 取代)* **渲染器构建显式设置 `minify: "esbuild"`，因为 `electron-vite` 将其渲染器预设硬编码为 `minify: false`。`pi-drop-legacy-font-fallbacks` 插件以 `enforce: "pre"` 在 Vite 将 `url()` 登记为资源之前剥离逗号前缀的 `woff`/`truetype` `src` 条目，因为内置 Chromium 始终支持 `woff2`；唯一来源为旧格式的字体保持不变。`BrandLogo` 导入由规范母版派生的 192x192 标记 `src/assets/brand/logo-{light,dark}.png`，而非 `build/` 中的 1024px 安装器图标，在不改变视觉标识的前提下细化 D079/D094。`packaging-footprint.test.mjs` 对这三项均有断言，使框架默认值或被回退的导入无法静默回归。两个内置 CJK 字体依据 ADR 0083 第 2 节保持不裁剪。** | `out/renderer` 从 31 MiB 降至 24 MiB 且无功能损失；未压缩的默认值与安装器图标导入属于意外成本，而 CJK 子集化会丢失用户内容中的字形，需要修订 ADR | -| D198 | Goal模式为第二种合约模式 | **Goal 是第三种持久操作模式 (`agent` / `plan` / `goal`)，它端到端地重用 Plan 审批管道。 `EnterGoalMode` 将活跃的 Agent 切换为 Goal 合约状态； `SubmitGoal(title, markdown, question)` 写入主机拥有的不可变 `.pi/goal/.md` 工件并插入一个待处理的 `plan_approvals` 行。单个审批表在 schema v11 中获得 `kind` 列（`plan` 或 `goal`，默认为 `plan`）；这种类型（而不是预计的计划状态）选择提示、工件目录、提交工具和 i18n 命名空间，因此一个批准栏和一个执行队列同时服务于这两者。 Plan 和 Goal 是合约模式：它们共享一个工具白名单（Read/Glob/Grep/BrowserPreview/Bash 加上他们自己的提交工具）、针对持久模式评估的 Write/Edit/plugin/unknown 工具的一台主机硬拒绝、共享 `*_IN_PLAN` 错误代码以及对无人值守运行的 `PLAN_REQUIRES_INTERACTIVE_SESSION` 拒绝。经批准的 Goal 在 Agent 中自主执行：它选择自己的方法并继续工作，直到验证每个验收标准或边界阻止它，然后逐个报告标准。线 `kind` 是可选的，不存在意味着 `plan`，因此这是协议 v9 中的附加内容。** | Plan 回答“执行这些步骤”；用户还需要“达到这个结果，自己决定步骤”（Claude Code的计划模式和Codex的目标型运行）。在批准的合同中制定目标声明、验收标准和边界可以保持自治的可审核性，并且按类型区分一条管道可以避免出现不可避免的漂移的平行表、批准表面和许可边界。 | +| D198 | Goal模式为第二种合约模式 | **Goal 是第三种持久操作模式 (`agent` / `plan` / `goal`)，它端到端地重用 Plan 审批管道。 `EnterGoalMode` 将活跃的 Agent 切换为 Goal 合约状态； `SubmitGoal(title, markdown, question)` 写入主机拥有的不可变 `.pi/goal/.md` 工件并插入一个待处理的 `plan_approvals` 行。单个审批表在 schema v11 中获得 `kind` 列（`plan` 或 `goal`，默认为 `plan`）；这种类型（而不是预计的计划状态）选择提示、工件目录、提交工具和 i18n 命名空间，因此一个批准栏和一个执行队列同时服务于这两者。 Plan 和 Goal 是合约模式：它们共享同一套工具策略（对会改变状态的内置工具与插件工具硬拒绝，Bash 与 MCP 走权限卡，加上各自的提交工具）、针对持久模式评估、共享 `*_IN_PLAN` 错误代码以及对无人值守运行的 `PLAN_REQUIRES_INTERACTIVE_SESSION` 拒绝。经批准的 Goal 在 Agent 中自主执行：它选择自己的方法并继续工作，直到验证每个验收标准或边界阻止它，然后逐个报告标准。线 `kind` 是可选的，不存在意味着 `plan`，因此这是协议 v9 中的附加内容。** | Plan 回答“执行这些步骤”；用户还需要“达到这个结果，自己决定步骤”（Claude Code的计划模式和Codex的目标型运行）。在批准的合同中制定目标声明、验收标准和边界可以保持自治的可审核性，并且按类型区分一条管道可以避免出现不可避免的漂移的平行表、批准表面和许可边界。 | | D144 | 侧边栏主镀铬为 14 像素 | **扩展侧边栏主要镶边（新任务、插件、会话标题、页脚标识名称、配置文件菜单操作）使用 `--text-base` (14px)。 Project/group 标题和空状态副本使用 `--text-md` (13px)。部分标签使用 `--text-sm` (12px)。主要侧边栏内容不得使用微型 `--text-xs` 带。** | 13px 侧边栏主体在 14px 聊天界面旁边感觉尺寸过小；仅碰撞原色铬可以保持密度，同时恢复视觉平衡，而无需全局类型比例变化。 | | D145 | 禁用可编辑字段上的浏览器文本更正 | **桌面渲染器中的每个文本 `input` 和 `textarea` 都会禁用浏览器文本校正：`spellCheck={false}`、`autoCorrect="off"` 和 `autoCapitalize="off"`。共享 `Input`/`Textarea` 原语默认这些值；原始字段（编写器、消息编辑、命令选项板、全局搜索、settings/plugins/projects 搜索、模型搜索、浏览器 URL 栏、提供程序模型组合）显式设置它们。复选框和非文本控件保持不变。** | 编码提示、路径、模型 ID 和 URL 不得带有红色下划线或通过 Chromium/OS 文本更正自动更改； shell 是一个应用程序，而不是文档编辑器。 | @@ -1402,8 +1402,8 @@ D193 和 D194。 `plan_approvals` 行。 `plan_approvals.kind`（架构 v11）是唯一 鉴别器；针对另一种类型运行的提交工具失败 `PLAN_KIND_MISMATCH`。 -- Plan 和 Goal 是合约模式。主机硬否认 - Write/Edit/plugin/unknown 工具、Bash 遵循权限模式规则、 +- Plan 和 Goal 是合约模式。主机硬拒绝会改变状态的内置工具与 + plugin 工具、Bash 在每种权限模式下都弹出权限卡、 `*_IN_PLAN` 错误代码、单一待批准不变量以及 `PLAN_REQUIRES_INTERACTIVE_SESSION` 拒绝预定运行均适用于 两者均根据会话的持久模式进行评估。 diff --git a/docs/zh-CN/spec/README.md b/docs/zh-CN/spec/README.md index ba3bad7313..35359fdf81 100644 --- a/docs/zh-CN/spec/README.md +++ b/docs/zh-CN/spec/README.md @@ -87,7 +87,7 @@ docs/spec/ 6. SQLite 仅由 Rust 拥有 7. 默认模式=Agent；操作选择器 = Agent | Plan | Goal。 Plan 和 Goal 是相同 Agent 的合约状态，并且不是严格的只读安全性 - 配置文件，因为 Bash 遵循所选的权限模式 + 配置文件，因为 Bash 在每种权限模式下都弹出权限卡 8. SubmitPlan 将精确的 Markdown 字节写入新的主机拥有的 `.pi/plan/*.md`神器； title/question 保持结构化 `plan_approvals`，批准打开工件，仅限 approve/reject，并且 diff --git a/packages/agent-runtime/src/mode-tool-access.test.ts b/packages/agent-runtime/src/mode-tool-access.test.ts index 042010e7e2..c2d0fe87eb 100644 --- a/packages/agent-runtime/src/mode-tool-access.test.ts +++ b/packages/agent-runtime/src/mode-tool-access.test.ts @@ -1,10 +1,17 @@ +import { readFile } from "node:fs/promises"; import { createServer, type ServerResponse } from "node:http"; -import { afterEach, expect, it } from "vitest"; +import { fileURLToPath } from "node:url"; +import { afterEach, describe, expect, it } from "vitest"; import { Type } from "typebox"; import type { AgentEventEnvelope, Mode, UiMessage } from "@pi-desktop/shared"; import { DesktopAgentRuntime } from "./runtime.js"; import type { RuntimeProviderConfig } from "./provider-binding.js"; -import { modeToolDenial, withModeExecutionGuard } from "./mode-tool-access.js"; +import { + CONTRACT_MODE_STATE_CHANGING, + contractModeWithholdsTool, + modeToolDenial, + withModeExecutionGuard, +} from "./mode-tool-access.js"; const cleanups: Array<() => Promise> = []; afterEach(async () => { for (const cleanup of cleanups.splice(0).reverse()) await cleanup(); }); @@ -123,3 +130,43 @@ it("an old tool reference rechecks the current mode and resumes normal execution await tool.execute("allowed", {}); expect(calls).toBe(1); }); + +/** + * The runtime and host-core answer "what does a contract mode refuse" from two + * languages, so the lists drift silently: the host would refuse a name the + * model still sees, or admit one the runtime hides. The names are the contract. + */ +describe("contract-mode state-changing list", () => { + it("matches the host list in permissions.rs", async () => { + const host = await readFile( + fileURLToPath(new URL("../../../crates/host-core/src/permissions.rs", import.meta.url)), + "utf8", + ); + const block = host.match(/CONTRACT_MODE_STATE_CHANGING[^=]*=\s*\[([\s\S]*?)\];/); + expect(block, "host constant not found in permissions.rs").not.toBeNull(); + const names = [...block![1].matchAll(/"([^"]+)"/g)].map((match) => match[1]); + expect([...CONTRACT_MODE_STATE_CHANGING].sort()).toEqual([...names].sort()); + }); + + it("refuses a state-changing builtin and admits everything else", () => { + for (const name of CONTRACT_MODE_STATE_CHANGING) { + expect(contractModeWithholdsTool(name), name).toBe(true); + } + for (const name of [ + "Read", + "Glob", + "Grep", + "BrowserPreview", + "Bash", + "asktool", + "new_context", + "ToolSearch", + "SubmitPlan", + "mcp_srv_tool", + "plugin_x_run", + "an_unclassified_builtin", + ]) { + expect(contractModeWithholdsTool(name), name).toBe(false); + } + }); +}); diff --git a/packages/agent-runtime/src/mode-tool-access.ts b/packages/agent-runtime/src/mode-tool-access.ts index 5281bebd36..3fa92f9839 100644 --- a/packages/agent-runtime/src/mode-tool-access.ts +++ b/packages/agent-runtime/src/mode-tool-access.ts @@ -4,17 +4,37 @@ import type { Mode } from "@pi-desktop/shared"; /** Known editing/delegation names stay declared across contract-mode changes. */ const RETAINED_MODE_TOOLS = new Set(["Write", "Edit", "Task", "TaskWait", "TaskList", "TaskStop"]); -// Explicit classification also denies stale references to local Agent-only tools. -const CONTRACT_MODE_BUILTINS = new Set([ - "Read", "Glob", "Grep", "BrowserPreview", "Bash", "asktool", "new_context", "ToolSearch", +/** + * Builtins a contract mode refuses: the ones that write files or session state, + * the delegation family that spawns and controls work, and `JevClassify`, whose + * Agent-only paid call must not run from a reference retained across a mode + * switch. Every other name reaches the host permission path, which decides + * (ADR 0330). `crates/host-core/src/permissions.rs` mirrors this list, and + * `mode-tool-access.test.ts` asserts the two stay identical. + */ +export const CONTRACT_MODE_STATE_CHANGING: ReadonlySet = new Set([ + "Write", + "Edit", + "TodoWrite", + "GenerateImages", + "PluginScaffold", + "PluginPack", + "ScheduledTaskCreate", + "ScheduledTaskUpdate", + "ScheduledTaskDelete", + "Task", + "TaskWait", + "TaskList", + "TaskStop", + "JevClassify", ]); export function retainModeToolDeclaration(name: string): boolean { return RETAINED_MODE_TOOLS.has(name); } -export function contractModeAllowsBuiltin(name: string): boolean { - return CONTRACT_MODE_BUILTINS.has(name); +export function contractModeWithholdsTool(name: string): boolean { + return CONTRACT_MODE_STATE_CHANGING.has(name); } export function modeToolDenial(name: string, mode: Mode): string { diff --git a/packages/agent-runtime/src/runtime.ts b/packages/agent-runtime/src/runtime.ts index 853f3ad313..e8d4c245b3 100644 --- a/packages/agent-runtime/src/runtime.ts +++ b/packages/agent-runtime/src/runtime.ts @@ -9,7 +9,7 @@ import { orderSystemRows, SystemTranscriptJournal } from "./system-transcript-jo import { planWorkspaceRequiredResult } from "./plan-workspace-error.js"; import { accountModelStream } from "./request-usage.js"; import { - contractModeAllowsBuiltin, + contractModeWithholdsTool, modeToolDenial, retainModeToolDeclaration, withModeExecutionGuard, @@ -4039,15 +4039,19 @@ Do not invent objections or turn speculative risks into blockers. Stop when the ); } + /** + * What a contract mode offers the model. It refuses what changes state and a + * tool that runs outside the host permission path; everything else reaches + * the host, which gates `Bash`, an opaque MCP tool, and a plugin tool without + * a declaration (ADR 0330). + */ private isToolAllowedInMode(name: string): boolean { const kind = proposalKindForMode(this.mode); if (!kind) return true; // Extension tools execute locally, outside the host permission path. if (this.isExtensionTool(name)) return false; - return contractModeAllowsBuiltin(name) - || name === SUBMIT_TOOL_NAMES[kind] - || name.startsWith("mcp_") - || (name.startsWith("plugin_") && this.isPlanSafePluginTool(name)); + if (contractModeWithholdsTool(name)) return false; + return !name.startsWith("plugin_") || this.isPlanSafePluginTool(name); } private isCoreTool(name: string): boolean { @@ -4067,7 +4071,10 @@ Do not invent objections or turn speculative risks into blockers. Stop when the } if (this.mode === "agent") return AGENT_CORE_TOOL_NAMES.has(name); if (!proposalKindForMode(this.mode)) return CHAT_CORE_TOOL_NAMES.has(name); - return contractModeAllowsBuiltin(name) || this.isPlanSafePluginTool(name); + // An MCP tool stays deferred behind ToolSearch: a server's schemas do not + // belong in every contract-mode request, and the tool is still discoverable + // when the model goes looking for it. + return this.isToolAllowedInMode(name) && !name.startsWith("mcp_"); } private activeTools(): AgentTool[] { diff --git a/scripts/e2e-plan.mjs b/scripts/e2e-plan.mjs index 8fb14512d1..a9652f6da9 100644 --- a/scripts/e2e-plan.mjs +++ b/scripts/e2e-plan.mjs @@ -244,7 +244,7 @@ async function scenario105(binary, tempRoot) { "EDIT_DISABLED_IN_PLAN", ], ["plugin_fake_tool", {}, "PLUGIN_DISABLED_IN_PLAN"], - ["UnknownTool", {}, "TOOL_DISABLED_IN_PLAN"], + ["TodoWrite", {}, "TOOL_DISABLED_IN_PLAN"], ]; ctx.host.clearNotifications(); for (const [toolName, args, code] of forbidden) { @@ -257,6 +257,17 @@ async function scenario105(binary, tempRoot) { }); assertToolFailure(result, code); } + // Deny by effect (ADR 0330): a builtin nobody classified is not a + // contract-mode denial. It reaches the ordinary path, where the host + // rejects the unregistered name. + const unclassified = await ctx.host.call("tools.execute", { + sessionId: session.id, + toolCallId: "e2e105-unclassified", + toolName: "UnknownTool", + args: {}, + mode: "agent", + }); + assertToolFailure(unclassified, "TOOL_NOT_FOUND"); const marker = join(ctx.workspace, "plan-bash-marker.txt"); const params = bashParams(session.id, "e2e105-bash", shell, planBashCommand(shell.dialect, marker)); const pending = ctx.host.call("tools.execute", params); @@ -275,7 +286,7 @@ async function scenario105(binary, tempRoot) { assert(ctx.host.matchingNotifications("permissions.request").length === 0, "Session grant did not suppress the second prompt"); const current = await ctx.host.call("session.get", { id: session.id }); assert(current.session?.mode === "plan", `forged mode changed session: ${shortJson(current)}`); - return `mode=plan read=ok denied=4 shell=${shell.id}/${shell.dialect}`; + return `mode=plan read=ok denied=4 unclassified=TOOL_NOT_FOUND shell=${shell.id}/${shell.dialect}`; }, binary, tempRoot); } From fdfccc3eaba915a3d6b81715973197e536684061 Mon Sep 17 00:00:00 2001 From: chengzhi-c Date: Fri, 16 Oct 2026 20:11:03 +0800 Subject: [PATCH 4/5] fix(test): keep the compaction name guard on the contract-mode deny list `context-compaction.test.mjs` asserted that `new_context` appears in `crates/host-core/src/permissions.rs`. A contract mode keeps a deny list instead of a read-only allowlist, so the name is no longer listed there. The guard now asserts the name stays off that list, which is the same wiring requirement ADR 0064 states. --- apps/desktop/test/context-compaction.test.mjs | 9 ++++++++- docs/adr/0064-codex-parity-context-compaction.md | 10 +++++----- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/apps/desktop/test/context-compaction.test.mjs b/apps/desktop/test/context-compaction.test.mjs index 1dedb2a287..eef0aebb03 100644 --- a/apps/desktop/test/context-compaction.test.mjs +++ b/apps/desktop/test/context-compaction.test.mjs @@ -129,7 +129,14 @@ test("the hard boundary is enforced by the host, with a model-side escape hatch" runtime, /messages: \[\s*\.\.\.context\.messages,\s*\{\s*role: "system",\s*content: "",\s*sections: \{ \[CONTEXT_BUDGET_SECTION\]: reminder \}/, ); - assert.match(hostPermissions, /"new_context"/); + // ADR 0330: the contract modes keep a deny list instead of a read-only + // allowlist, so `new_context` is admitted by falling through it. Denying it + // there would take compaction away from Plan and Goal. + const contractDenyList = hostPermissions.match( + /CONTRACT_MODE_STATE_CHANGING[^=]*=\s*\[([\s\S]*?)\];/, + ); + assert.ok(contractDenyList, "host-core contract-mode deny list not found"); + assert.doesNotMatch(contractDenyList[1], /"new_context"/); }); test("a delegate gets the session's turn-boundary budget protection (ADR 0299)", () => { diff --git a/docs/adr/0064-codex-parity-context-compaction.md b/docs/adr/0064-codex-parity-context-compaction.md index a9ced1e7ab..7e5be5d585 100644 --- a/docs/adr/0064-codex-parity-context-compaction.md +++ b/docs/adr/0064-codex-parity-context-compaction.md @@ -97,12 +97,12 @@ retained-tail recovery, ADR 0061's model-window-derived budgets, and ADR 0061's the model requested `new_context` or the estimate reaches `floor(hardLimit * 0.9)`, matching the `should_roll_over` turn boundary. `hardLimit` remains the final request guard (D623). The name is synchronized - across `activeTools()`, - `isCoreTool()`, the contract-mode tool policy, and the host-core - no-confirmation allowlist (`crates/host-core/src/permissions.rs`), where it - replaces `"CompactContext"`. + across `activeTools()` and `isCoreTool()`. *(Amended by ADR 0330: a contract mode keeps a deny list instead of an - allowlist, and `new_context` is admitted by falling through it.)* + allowlist, so `new_context` is no longer named in + `crates/host-core/src/permissions.rs`. It is admitted by falling through the + deny list, and `apps/desktop/test/context-compaction.test.mjs` asserts it + stays off that list.)* 5. **Two-tier reminders, once each per window.** With `remaining = hardLimit - tokens`: tier one at `remaining <= clamp(hardLimit * 0.15, 8k, 32k)` states the remaining budget From 5c1934384571ad3b7b8fa3fdfdd42c70e6529689 Mon Sep 17 00:00:00 2001 From: chengzhi-c Date: Sun, 18 Oct 2026 09:06:55 +0800 Subject: [PATCH 5/5] docs(plan): state the effect-scoped contract-mode posture Review asked for the consequence of replacing the read-only allowlist with a deny list to be visible where the posture is defined: the gate admits every name it does not deny, so a builtin nobody classified reaches the ordinary permission path. ADR 0331 and the security spec now say so, name the builtins admitted by omission, and put network access in the deny-list criterion so a future egress builtin is classified instead of inherited. The host comment on the plugin carve-out points at the plugin-runtime execute path that enforces the declared actions. --- crates/host-core/src/permissions.rs | 3 +++ .../adr/0331-contract-modes-deny-what-is-unsafe.md | 14 +++++++++++++- docs/spec/05-security/01-security.md | 10 +++++++--- docs/zh-CN/spec/05-security/01-security.md | 6 ++++-- 4 files changed, 27 insertions(+), 6 deletions(-) diff --git a/crates/host-core/src/permissions.rs b/crates/host-core/src/permissions.rs index 6f32efe603..81e9a61a05 100644 --- a/crates/host-core/src/permissions.rs +++ b/crates/host-core/src/permissions.rs @@ -195,6 +195,9 @@ impl PermissionManager { if tool_name.starts_with("plugin_") { // The declaration is the escape hatch (ADR 0211): without one the // plugin runtime refuses the call too, so a card would be theater. + // The per-action restriction is enforced there, in + // `apps/desktop/electron/main/plugin-runtime.ts`, where a plugin + // tool's `execute` rejects an undeclared action in plan/goal. return if plan_safe_actions.is_some_and(|actions| !actions.is_empty()) { ContractAdmission::Allowed } else { diff --git a/docs/adr/0331-contract-modes-deny-what-is-unsafe.md b/docs/adr/0331-contract-modes-deny-what-is-unsafe.md index 74c60451e3..e2c022b97f 100644 --- a/docs/adr/0331-contract-modes-deny-what-is-unsafe.md +++ b/docs/adr/0331-contract-modes-deny-what-is-unsafe.md @@ -22,12 +22,17 @@ the effect of a call rather than the provenance of its name. ## Decision +Contract modes are effect-scoped rather than capability-scoped: the gate asks +what a call does, not where its name comes from. + - A contract mode denies the builtins that act on state instead of inspecting it: the ones that write files or session state (`Write`, `Edit`, `TodoWrite`, `GenerateImages`, `PluginScaffold`, `PluginPack`, the `ScheduledTask*` tools), the delegation family that spawns and controls work (`Task*`), and `JevClassify`, whose paid provider call must not run from a reference retained - across a mode switch. + across a mode switch. A builtin that reaches the network on its own belongs on + this list too; `Bash` and a user MCP tool are absent because a permission card + already covers what they can do. - Every other builtin is admitted by the gate and falls through to the ordinary permission path, which still applies risk, the effective permission mode, session grants, and the external-path rule. `Read`, `Glob`, `Grep`, @@ -70,6 +75,13 @@ step. None of those catches a state-changing builtin nobody added to the list, so it stays reachable in Plan until someone classifies it. Deciding what a new builtin does to state is part of adding it. +The gate admits names the allowlist never listed: `ToolSearch`, `PluginCheck`, +`Skill`, and any builtin added later. What a contract mode declares to the model +barely moves: `ToolSearch` is the one addition, because an MCP tool stays +deferred behind it, and `PluginCheck` and `Skill` pass the gate without ever +being built in a contract mode. The exposure sits in the gate, so the deny list +is the contract, not the runtime's declaration set. + Approval is not filesystem isolation: an approved shell command or opaque MCP call may mutate state. This policy controls authorization, not the effects of arbitrary code. Plugin authors remain responsible for the semantics of actions diff --git a/docs/spec/05-security/01-security.md b/docs/spec/05-security/01-security.md index 1a1d1d67a6..c34e37974f 100644 --- a/docs/spec/05-security/01-security.md +++ b/docs/spec/05-security/01-security.md @@ -95,9 +95,13 @@ sidecar's workspace file resolver remain restricted to the session workspace. Plan is not itself the workspace security boundary. Host-core resolves the durable session mode for every `tools.execute` call and applies the Plan matrix -before permission modes, grants, plugin risk, or renderer/sidecar state. Plan -denies state-changing builtins and plugin tools without `planSafeActions`, while -BrowserPreview is the explicit read-only UI inspection exception (it reveals +before permission modes, grants, plugin risk, or renderer/sidecar state. The +contract modes are effect-scoped rather than capability-scoped: they deny the +builtins that act on state and plugin tools without `planSafeActions`, and admit +every other name by omission, so a builtin nobody classified reaches the ordinary +permission path. A builtin that writes, delegates, spends a paid call, or reaches +the network on its own belongs on the deny list when it is added. BrowserPreview +is the explicit read-only UI inspection exception (it reveals bundled `pi.browser` chrome; raw CDP plugin tools stay denied in Plan). Bash remains available in Plan: every permission mode, Auto included, prompts, and an approved command may mutate the workspace or scratch directory. The UI must diff --git a/docs/zh-CN/spec/05-security/01-security.md b/docs/zh-CN/spec/05-security/01-security.md index ad04b7c2e9..13a5d50c84 100644 --- a/docs/zh-CN/spec/05-security/01-security.md +++ b/docs/zh-CN/spec/05-security/01-security.md @@ -90,8 +90,10 @@ Plan 本身并不是工作区安全边界。主机核解决了 每个 `tools.execute` 调用的持久会话模式并应用 Plan 矩阵 -在权限模式、授予、插件风险或 renderer/sidecar 状态之前。 Plan -拒绝会改变状态的内置工具与未声明 `planSafeActions` 的插件工具，而 +在权限模式、授予、插件风险或 renderer/sidecar 状态之前。 契约模式按效果而非 +能力划分：它拒绝会改变状态的内置工具与未声明 `planSafeActions` 的插件工具， +其余名字一律按省略放行，因此没人分类过的内置工具会走到普通权限路径。一个自己 +写文件、派发工作、花费付费调用或访问网络的内置工具，在加入时就该进入拒绝清单。 BrowserPreview 是显式的只读 UI 检查异常（它打开随应用打包的 `pi.browser` chrome；原始 CDP 插件工具在 Plan 中仍被拒绝）。 Bash 在 Plan 中仍然可用： 每种权限模式（含 Auto）都弹出权限卡，批准后的命令可能会改变